From 7ac598c0294e9c179ba30cfd36fbac724d200235 Mon Sep 17 00:00:00 2001 From: Martin Zeman Date: Wed, 24 Jun 2026 03:48:08 +0200 Subject: [PATCH] feat: model-level qualifier resolution for rules and agents (#60) Files in a model qualifier directory (rules///Rule.md) were silently dropped: the pipeline resolved variants with [provider] only, the source walk skipped model subdirectories, and build_valid_qualifiers hyphen-split model IDs into junk qualifiers (4, 6 from claude-opus-4-6). Phase 1 wires the full user > provider/model > provider > base chain for rules and agents: provider config gains a model field (overridable with --model), the walk descends into known model-ID subdirectories, and an unknown subdirectory in a provider qualifier dir is a validation error. Skill overlays, provenance model recording, and the release matrix are deferred to Phase 2. --- CHANGELOG.md | 1 + defaults.yaml | 4 + docs/todos/2026-07-06.md | 3 + src/assemble/pipeline_tests.rs | 2 + src/cli/assemble/mod.rs | 34 ++++++- src/cli/assemble/pipeline.rs | 9 +- src/cli/assemble/sources.rs | 111 +++++++++++++++++--- src/cli/assemble/tests.rs | 3 + src/cli/install/mod.rs | 5 +- src/cli/install/tests.rs | 15 ++- src/cli/mod.rs | 19 +++- src/cli/release/mod.rs | 1 + src/provider/mod.rs | 4 + src/provider/tests.rs | 2 + tests/model_qualifier.rs | 178 +++++++++++++++++++++++++++++++++ 15 files changed, 368 insertions(+), 23 deletions(-) create mode 100644 docs/todos/2026-07-06.md create mode 100644 tests/model_qualifier.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 50f2fe7..0ebe71e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Added +- Model-level qualifier resolution for rules and agents (PROV-0005 Phase 1). Assembly now resolves the full `user/` > `provider//` > `provider/` > base precedence: a file at `rules///Rule.md` overrides the base for that provider and model, and the long-documented `user/` overlay is finally wired for rules and agents too. Each provider gains a default `model` in `defaults.yaml` (an exact ID from `config/models.yaml`); `forge assemble --model ` and `forge install --model ` override it for providers that list that model. Qualifier directory names are validated as exact model IDs: model IDs are no longer split into segments, so a directory named `4` or `6` (from `claude-opus-4-6`) is no longer a valid qualifier, and a model-only file under `rules///` is collected instead of silently dropped. An unrecognized model-qualifier subdirectory is skipped with a warning rather than dropped silently. Skill overlays, recording the model in `.manifest`/provenance sidecars, and the per-model release matrix are deferred to Phase 2. (#60) - `forge drift --target ` verifies a module's assembled `build/` against where it was deployed, scoped to the module's own files. It mirrors `forge install --target`: each provider's `build/` is compared to `/`, so files built but not yet deployed surface as local-only, deployment edits surface as frontmatter/body drift, and this module's deployed files (per the target `.manifest` plus provenance attribution) that are no longer built surface as drift. Unlike `--upstream` against a multi-module tree such as `~/.claude`, other modules' files are never reported. `--target` and `--upstream` are mutually exclusive. (#61) - `forge validate` sanity-checks Claude Code plugin scaffolding when `.claude-plugin/plugin.json` is present: each manifest (`plugin.json`, `.claude-plugin/marketplace.json`, `hooks/hooks.json`) must be valid JSON, and every hook script referenced via `${CLAUDE_PLUGIN_ROOT}` must exist and be executable (the most common cause of hooks silently not firing). It deliberately makes no assertions about the plugin or marketplace field schema, so it does not break when Claude Code's plugin format changes. Non-plugin modules are unaffected: the check runs only when `plugin.json` exists. (#59) - `.forge` consumer manifests accept `git:` sources pinned to a 40-hex commit SHA. `forge install` clones the remote via `gix` into a content-addressed cache at `~/.cache/forge/git////`, materializes the pinned tree, and feeds it through the standard assemble + deploy pipeline. HTTPS-only; `ssh://`, `git://`, `git@host:` shorthand, and userinfo URLs are rejected at parse time. Branch names, tags, and abbreviated SHAs are rejected in favor of explicit 40-char commit hashes. Cache hits are instant; the bare clone is reused across SHA pins within the same repository. (#53) diff --git a/defaults.yaml b/defaults.yaml index c1eeef1..5f6e218 100644 --- a/defaults.yaml +++ b/defaults.yaml @@ -19,6 +19,7 @@ dashboard: providers: claude: target: ".claude" + model: claude-opus-4-6 aliases: - claudecode keep_fields: @@ -51,6 +52,7 @@ providers: gemini: target: ".gemini" + model: gemini-2.5-pro aliases: - geminicli assembly: @@ -68,6 +70,7 @@ providers: codex: target: ".codex" + model: gpt-5.5 aliases: - codexcli assembly: @@ -90,6 +93,7 @@ providers: opencode: target: ".opencode" + model: claude-opus-4-6 assembly: - kebab-case-agents - strip-links diff --git a/docs/todos/2026-07-06.md b/docs/todos/2026-07-06.md new file mode 100644 index 0000000..ef98805 --- /dev/null +++ b/docs/todos/2026-07-06.md @@ -0,0 +1,3 @@ +# Backlog 2026-07-06 + +- [ ] #forge-cli/install `forge install --target ~` deploys stale artifacts when a pre-existing `build/` directory is present — it neither rebuilds nor verifies freshness against source. Repro: edit a skill source in forge-dev, run `make install` (delegates to `forge install --target ~`) with an old `build/` in place; the outdated build output lands in the target. Session workaround was moving `build/` aside before installing. Suggested fix: rebuild into a temp dir, or compare source vs `build/` mtimes/hashes and warn or fail on staleness. diff --git a/src/assemble/pipeline_tests.rs b/src/assemble/pipeline_tests.rs index a3e6389..5a3372a 100644 --- a/src/assemble/pipeline_tests.rs +++ b/src/assemble/pipeline_tests.rs @@ -21,6 +21,7 @@ fn make_provider(assembly: Option>) -> ProviderConfig { models: None, effort: None, aliases: None, + model: None, } } @@ -161,6 +162,7 @@ fn unknown_rule_collected_as_error() { models: None, effort: None, aliases: None, + model: None, }, ); diff --git a/src/cli/assemble/mod.rs b/src/cli/assemble/mod.rs index ad86fea..321ed96 100644 --- a/src/cli/assemble/mod.rs +++ b/src/cli/assemble/mod.rs @@ -34,7 +34,9 @@ use crate::cli::config; /// gemini/agents/security-architect.md (with remapped tools) /// gemini/agents/security-architect.yaml /// ``` -pub fn execute(path: &str) -> Result { +/// Assemble, selecting model variants with `model_override` (the `--model` +/// flag) in place of each provider's configured default model. +pub fn execute_with_model(path: &str, model_override: Option<&str>) -> Result { let module_root = Path::new(path); if !module_root.is_dir() { return Err(Error::new( @@ -95,12 +97,15 @@ pub fn execute(path: &str) -> Result { let model_tiers = provider_config.models.clone().unwrap_or_default(); let effort_tiers = provider_config.effort.clone().unwrap_or_default(); + let active_model = + resolve_active_model(model_override, provider_config, &models, provider_name); for source in &source_files { if let Some(deployed) = assemble_source_for_provider( source, module_root, provider_name, + active_model.as_deref(), provider_config, &provider_build_dir, &tool_mappings, @@ -118,11 +123,32 @@ pub fn execute(path: &str) -> Result { Ok(result) } +/// Choose the model ID whose `provider//` variants win this assembly: +/// `--model` when it is a valid model for the provider, otherwise the +/// provider's configured default. An override that names another provider's +/// model is ignored so a single `--model` flag is safe across all providers. +fn resolve_active_model( + model_override: Option<&str>, + provider_config: &commands::provider::ProviderConfig, + models: &std::collections::HashMap>, + provider_name: &str, +) -> Option { + if let Some(override_id) = model_override + && models + .get(provider_name) + .is_some_and(|ids| ids.iter().any(|id| id == override_id)) + { + return Some(override_id.to_string()); + } + provider_config.model.clone() +} + #[allow(clippy::too_many_arguments)] fn assemble_source_for_provider( source: &sources::SourceFile, module_root: &Path, provider_name: &str, + active_model: Option<&str>, provider_config: &commands::provider::ProviderConfig, provider_build_dir: &Path, tool_mappings: &std::collections::HashMap, @@ -159,6 +185,7 @@ fn assemble_source_for_provider( source, module_root, provider_name, + active_model, &kind_keep_fields, model_tiers, effort_tiers, @@ -172,9 +199,12 @@ fn assemble_source_for_provider( .relative_path .strip_prefix(&format!("{}/", source.kind)) .unwrap_or(&source.relative_path); + // Qualifier-only files live one or two levels deep (rules//file + // or rules///file); deploy them flat under the kind, so + // strip every qualifier segment down to the basename. let relative_within_kind = if source.qualifier.is_some() { stripped_kind - .split_once('/') + .rsplit_once('/') .map_or(stripped_kind, |(_, filename)| filename) } else { stripped_kind diff --git a/src/cli/assemble/pipeline.rs b/src/cli/assemble/pipeline.rs index 87dc33b..f602c4c 100644 --- a/src/cli/assemble/pipeline.rs +++ b/src/cli/assemble/pipeline.rs @@ -22,10 +22,12 @@ use crate::cli::config::read_file; /// → references stripped /// → tool names remapped (Read → read_file for gemini) /// ``` +#[allow(clippy::too_many_arguments)] pub fn assemble_source( source: &SourceFile, module_root: &Path, provider_name: &str, + model: Option<&str>, keep_fields: &[String], model_tiers: &HashMap>, effort_tiers: &HashMap, @@ -38,7 +40,12 @@ pub fn assemble_source( let source_dir = Path::new(&source.full_path).parent().unwrap_or(module_root); let filename = extract_filename(&source.full_path); - let variant = assemble::variants::resolve(source_dir, &filename, &[provider_name.to_string()]); + // Resolution precedence (PROV-0005): user/ > provider/model/ > provider/ > base. + let mut qualifiers = vec!["user".to_string(), provider_name.to_string()]; + if let Some(model_id) = model { + qualifiers.push(model_id.to_string()); + } + let variant = assemble::variants::resolve(source_dir, &filename, &qualifiers); let variant_content = match &variant { Some(vp) => Some(read_file(vp)?), diff --git a/src/cli/assemble/sources.rs b/src/cli/assemble/sources.rs index 3cd8af6..8c8e60f 100644 --- a/src/cli/assemble/sources.rs +++ b/src/cli/assemble/sources.rs @@ -22,6 +22,7 @@ fn parse_targets(content: &str) -> Option> { /// qualifier: None, /// } /// ``` +#[derive(Debug)] pub struct SourceFile { /// Relative path from the module root (e.g. "rules/MyRule.md"). pub relative_path: String, @@ -172,6 +173,7 @@ fn walk_content_dir( module_root, sources, &base_filenames, + valid_qualifiers, )?; } @@ -181,8 +183,14 @@ fn walk_content_dir( /// Walk a qualifier subdirectory, collecting only qualifier-only files. /// /// Files that share a name with a base file are variant overrides, handled -/// by `variants::resolve` during assembly of the base file. Only files -/// with no base counterpart are collected as qualifier-only sources. +/// by `variants::resolve` during assembly of the base file. Only files with +/// no base counterpart are collected as qualifier-only sources. +/// +/// A subdirectory whose name is a valid qualifier (a model ID from +/// `config/models.yaml`) is descended into so model-only files under +/// `rules///` are collected, tagged with the model ID as +/// their qualifier. Any other subdirectory is a validation error rather than +/// being silently dropped. fn walk_qualifier_dir( dir: &Path, qualifier_name: &str, @@ -190,6 +198,7 @@ fn walk_qualifier_dir( module_root: &Path, sources: &mut Vec, base_filenames: &HashSet, + valid_qualifiers: &HashSet, ) -> Result<(), Error> { let entries = fs::read_dir(dir) .map_err(|e| Error::new(ErrorKind::Io, format!("cannot read {}: {e}", dir.display())))?; @@ -198,7 +207,39 @@ fn walk_qualifier_dir( let entry = entry.map_err(|e| Error::new(ErrorKind::Io, format!("directory entry error: {e}")))?; let path = entry.path(); - if path.is_dir() || path.extension().unwrap_or_default() != "md" { + + if path.is_dir() { + let subdir = path + .file_name() + .unwrap_or_default() + .to_string_lossy() + .to_string(); + if valid_qualifiers.contains(&subdir) { + walk_qualifier_dir( + &path, + &subdir, + kind, + module_root, + sources, + base_filenames, + valid_qualifiers, + )?; + } else { + // Not an exact model ID from config/models.yaml. Surfacing this + // as a warning (rather than silently dropping the files, as + // before) lets modules still carrying short or retired model + // directory names see what is being skipped. Phase 2 hardens + // this to a validation error once those modules migrate to + // exact model IDs. + eprintln!( + "warning: skipping unknown model qualifier directory '{subdir}' in {} (expected an exact model ID from config/models.yaml)", + dir.display() + ); + } + continue; + } + + if path.extension().unwrap_or_default() != "md" { continue; } let filename = path @@ -323,8 +364,9 @@ fn collect_skill_files( /// Build the set of valid qualifier names from provider names and model IDs. /// -/// A qualifier is valid if it matches either a provider name (e.g., "claude") -/// or a segment of any model ID (e.g., "sonnet" from "claude-sonnet-4-6"). +/// A qualifier is valid if it is a provider name (e.g. `claude`) or an exact +/// model ID from `config/models.yaml` (e.g. `claude-opus-4-6`). Model IDs are +/// never split into segments: a directory named `4` or `6` is not a qualifier. pub fn build_valid_qualifiers( provider_names: &[String], models: &std::collections::HashMap>, @@ -333,14 +375,8 @@ pub fn build_valid_qualifiers( for name in provider_names { qualifiers.insert(name.clone()); } - for model_ids in models.values() { - for model_id in model_ids { - for segment in model_id.split('-') { - if !segment.is_empty() { - qualifiers.insert(segment.to_string()); - } - } - } + for model_id in models.values().flatten() { + qualifiers.insert(model_id.clone()); } qualifiers } @@ -396,11 +432,54 @@ mod tests { } #[test] - fn build_qualifiers_includes_model_tier_segments() { + fn build_qualifiers_uses_exact_model_ids_not_segments() { let providers = vec!["claude".to_string()]; let qualifiers = build_valid_qualifiers(&providers, &make_models()); - assert!(qualifiers.contains("sonnet")); - assert!(qualifiers.contains("opus")); + assert!(qualifiers.contains("claude-opus-4-6")); + assert!(qualifiers.contains("claude-sonnet-4-6")); + // Segments of model IDs are not qualifiers — this is the junk-qualifier + // bug the exact-ID rule fixes. + assert!(!qualifiers.contains("sonnet")); + assert!(!qualifiers.contains("opus")); + assert!(!qualifiers.contains("4")); + assert!(!qualifiers.contains("6")); + } + + #[test] + fn unknown_subdirectory_in_qualifier_dir_is_skipped() { + let dir = tempfile::tempdir().unwrap(); + let rules = scaffold_kind(dir.path(), "rules"); + std::fs::write(rules.join("BaseRule.md"), BASE_RULE).unwrap(); + let provider = rules.join("claude"); + std::fs::create_dir(&provider).unwrap(); + let stray = provider.join("not-a-model"); + std::fs::create_dir(&stray).unwrap(); + std::fs::write(stray.join("Stray.md"), QUALIFIER_ONLY).unwrap(); + + let valid = HashSet::from(["claude".to_string(), "claude-opus-4-6".to_string()]); + let sources = collect(dir.path(), &valid).expect("unknown subdir warns, not errors"); + assert!( + sources.iter().all(|s| !s.relative_path.contains("Stray")), + "files under an unknown model qualifier dir are skipped" + ); + } + + #[test] + fn model_only_file_collected_with_model_qualifier() { + let dir = tempfile::tempdir().unwrap(); + let rules = scaffold_kind(dir.path(), "rules"); + std::fs::write(rules.join("BaseRule.md"), BASE_RULE).unwrap(); + let model_dir = rules.join("claude").join("claude-opus-4-6"); + std::fs::create_dir_all(&model_dir).unwrap(); + std::fs::write(model_dir.join("OpusOnly.md"), QUALIFIER_ONLY).unwrap(); + + let valid = HashSet::from(["claude".to_string(), "claude-opus-4-6".to_string()]); + let sources = collect(dir.path(), &valid).unwrap(); + let model_only = sources + .iter() + .find(|s| s.relative_path.contains("OpusOnly")) + .expect("model-only file must be collected, not dropped"); + assert_eq!(model_only.qualifier, Some("claude-opus-4-6".to_string())); } #[test] diff --git a/src/cli/assemble/tests.rs b/src/cli/assemble/tests.rs index 8e50026..fdb84b6 100644 --- a/src/cli/assemble/tests.rs +++ b/src/cli/assemble/tests.rs @@ -97,6 +97,7 @@ fn assemble_source_maps_agent_model_and_effort_tiers() { &source, std::path::Path::new("/tmp"), "codex", + None, &[ "name".to_string(), "description".to_string(), @@ -149,6 +150,7 @@ fn assemble_source_maps_all_codex_tiers() { &source, std::path::Path::new("/tmp"), "codex", + None, &[ "name".to_string(), "description".to_string(), @@ -205,6 +207,7 @@ fn assemble_source_keeps_explicit_effort_over_tier_effort() { &source, std::path::Path::new("/tmp"), "codex", + None, &[ "name".to_string(), "description".to_string(), diff --git a/src/cli/install/mod.rs b/src/cli/install/mod.rs index 8f214e3..12ad7f0 100644 --- a/src/cli/install/mod.rs +++ b/src/cli/install/mod.rs @@ -12,7 +12,7 @@ use super::deploy; /// ``` /// /// Returns only the deployment result — assembly is an internal step. -#[allow(clippy::fn_params_excessive_bools)] +#[allow(clippy::fn_params_excessive_bools, clippy::too_many_arguments)] pub fn execute( path: &str, target: Option<&str>, @@ -21,8 +21,9 @@ pub fn execute( prune: bool, interactive: bool, dry_run: bool, + model: Option<&str>, ) -> Result { - assemble::execute(path)?; + assemble::execute_with_model(path, model)?; deploy::execute( path, target, diff --git a/src/cli/install/tests.rs b/src/cli/install/tests.rs index 3904bc7..cc47e9c 100644 --- a/src/cli/install/tests.rs +++ b/src/cli/install/tests.rs @@ -11,7 +11,16 @@ fn write_module_yaml(directory: &std::path::Path) { #[test] fn execute_errors_on_missing_module() { - let result = execute("/nonexistent/module", None, &[], false, false, false, false); + let result = execute( + "/nonexistent/module", + None, + &[], + false, + false, + false, + false, + None, + ); assert!(result.is_err()); } @@ -26,6 +35,7 @@ fn execute_errors_on_directory_without_module_yaml() { false, false, false, + None, ); let error = result.expect_err("expected install to refuse non-module directory"); let message = error.to_string(); @@ -58,6 +68,7 @@ fn execute_succeeds_on_empty_module() { false, false, false, + None, ); assert!(result.is_ok()); } @@ -76,6 +87,7 @@ fn execute_unknown_provider_lists_available_choices() { false, false, false, + None, ); let error = result.expect_err("unknown provider must error"); let message = error.to_string(); @@ -110,6 +122,7 @@ fn execute_provider_filter_skips_unrequested_providers() { false, false, false, + None, ) .expect("install should succeed for known provider"); diff --git a/src/cli/mod.rs b/src/cli/mod.rs index df6fad6..c4c9dae 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -89,6 +89,12 @@ enum Command { /// Show what would be pruned without moving files. #[arg(long)] dry_run: bool, + + /// Override each provider's default model when selecting + /// `provider//` qualifier variants (exact model ID from + /// config/models.yaml; ignored for providers that lack it). + #[arg(long, value_name = "MODEL_ID")] + model: Option, }, /// Assemble module content into build/ @@ -96,6 +102,12 @@ enum Command { /// Module root to assemble (must contain module.yaml). Defaults to `.`. #[arg(long, value_name = "DIR", default_value = ".")] source: String, + + /// Override each provider's default model when selecting + /// `provider//` qualifier variants (exact model ID from + /// config/models.yaml; ignored for providers that lack it). + #[arg(long, value_name = "MODEL_ID")] + model: Option, }, /// Deploy assembled files from build/ to provider directories @@ -278,6 +290,7 @@ pub fn run() -> i32 { interactive, no_prune, dry_run, + model, } => ( install::execute( &source, @@ -287,10 +300,14 @@ pub fn run() -> i32 { !no_prune, interactive, dry_run, + model.as_deref(), ), "deployed", ), - Command::Assemble { source } => (assemble::execute(&source), "assembled"), + Command::Assemble { source, model } => ( + assemble::execute_with_model(&source, model.as_deref()), + "assembled", + ), Command::Deploy { source, target, diff --git a/src/cli/release/mod.rs b/src/cli/release/mod.rs index 4c0999f..aa719fd 100644 --- a/src/cli/release/mod.rs +++ b/src/cli/release/mod.rs @@ -44,6 +44,7 @@ pub fn execute(path: &str, embed: bool) -> Result { false, false, false, + None, )?; result.installed.clear(); result.skipped.clear(); diff --git a/src/provider/mod.rs b/src/provider/mod.rs index 7c34454..53eae6e 100644 --- a/src/provider/mod.rs +++ b/src/provider/mod.rs @@ -61,6 +61,10 @@ pub struct ProviderConfig { pub models: Option>>, pub effort: Option>, pub aliases: Option>, + /// Default target model ID for this provider (an exact ID from + /// `config/models.yaml`). Selects which `provider//` variant + /// directory wins during assembly; `--model` overrides it. + pub model: Option, } impl ProviderConfig { diff --git a/src/provider/tests.rs b/src/provider/tests.rs index 31822fa..d0619ef 100644 --- a/src/provider/tests.rs +++ b/src/provider/tests.rs @@ -166,6 +166,7 @@ fn provider_with_aliases(target: &str, aliases: Vec<&str>) -> ProviderConfig { keep_fields: None, models: None, effort: None, + model: None, aliases: Some(aliases.into_iter().map(String::from).collect()), } } @@ -209,6 +210,7 @@ fn matches_target_no_aliases() { keep_fields: None, models: None, effort: None, + model: None, aliases: None, }; assert!(config.matches_target("opencode", "opencode")); diff --git a/tests/model_qualifier.rs b/tests/model_qualifier.rs new file mode 100644 index 0000000..8542119 --- /dev/null +++ b/tests/model_qualifier.rs @@ -0,0 +1,178 @@ +//! Integration tests for model-level qualifier resolution (#60, Phase 1). +//! +//! Resolution precedence is `user/` > `provider//` > `provider/` > +//! base. A file placed in a model qualifier directory must override the base +//! for the matching provider+model and must not silently disappear. + +use assert_cmd::Command; +use std::fs; +use std::path::Path; + +fn forge() -> Command { + Command::cargo_bin("forge").unwrap() +} + +fn scaffold(root: &Path) { + fs::write( + root.join("module.yaml"), + "name: model-fixture\nversion: 0.1.0\ndescription: model qualifier fixture\nevents: []\n", + ) + .unwrap(); + fs::write(root.join("defaults.yaml"), "").unwrap(); +} + +fn write_rule(root: &Path, relative: &str, body: &str) { + let path = root.join("rules").join(relative); + fs::create_dir_all(path.parent().unwrap()).unwrap(); + fs::write( + path, + format!("---\nname: Base\ndescription: r\n---\n\n{body}\n"), + ) + .unwrap(); +} + +fn assemble(module: &Path) { + forge() + .args(["assemble", "--source", module.to_str().unwrap()]) + .assert() + .success(); +} + +#[test] +fn model_variant_overrides_base_for_matching_model() { + let module = tempfile::tempdir().unwrap(); + scaffold(module.path()); + write_rule(module.path(), "Base.md", "BASE BODY"); + // claude's default model is claude-opus-4-6 (from embedded defaults). + write_rule( + module.path(), + "claude/claude-opus-4-6/Base.md", + "OPUS VARIANT BODY", + ); + + assemble(module.path()); + + let claude = fs::read_to_string(module.path().join("build/claude/rules/Base.md")).unwrap(); + assert!( + claude.contains("OPUS VARIANT BODY"), + "claude (model claude-opus-4-6) must get the model variant: {claude}" + ); + + // gemini's default model is gemini-2.5-pro, so the claude-opus variant + // must not leak into the gemini build. + let gemini = fs::read_to_string(module.path().join("build/gemini/rules/Base.md")).unwrap(); + assert!( + gemini.contains("BASE BODY"), + "gemini must fall back to the base body: {gemini}" + ); +} + +#[test] +fn user_overlay_wins_over_model_variant() { + let module = tempfile::tempdir().unwrap(); + scaffold(module.path()); + write_rule(module.path(), "Base.md", "BASE BODY"); + write_rule( + module.path(), + "claude/claude-opus-4-6/Base.md", + "OPUS VARIANT BODY", + ); + write_rule(module.path(), "user/Base.md", "USER OVERLAY BODY"); + + assemble(module.path()); + + let claude = fs::read_to_string(module.path().join("build/claude/rules/Base.md")).unwrap(); + assert!( + claude.contains("USER OVERLAY BODY"), + "user/ overlay must win over the model variant: {claude}" + ); +} + +#[test] +fn model_only_file_is_deployed_not_dropped() { + let module = tempfile::tempdir().unwrap(); + scaffold(module.path()); + write_rule(module.path(), "Base.md", "BASE BODY"); + write_rule( + module.path(), + "claude/claude-opus-4-6/OpusOnly.md", + "OPUS ONLY BODY", + ); + + assemble(module.path()); + + assert!( + module + .path() + .join("build/claude/rules/OpusOnly.md") + .is_file(), + "a model-only file must deploy for its provider, not silently vanish" + ); + assert!( + !module + .path() + .join("build/gemini/rules/OpusOnly.md") + .exists(), + "a claude model-only file must not deploy for gemini" + ); +} + +#[test] +fn model_override_flag_selects_variant() { + let module = tempfile::tempdir().unwrap(); + scaffold(module.path()); + write_rule(module.path(), "Base.md", "BASE BODY"); + write_rule( + module.path(), + "claude/claude-sonnet-4-6/Base.md", + "SONNET VARIANT BODY", + ); + + // Default model is claude-opus-4-6, so without --model the sonnet variant + // is not selected; with --model claude-sonnet-4-6 it wins. + forge() + .args([ + "assemble", + "--source", + module.path().to_str().unwrap(), + "--model", + "claude-sonnet-4-6", + ]) + .assert() + .success(); + + let claude = fs::read_to_string(module.path().join("build/claude/rules/Base.md")).unwrap(); + assert!( + claude.contains("SONNET VARIANT BODY"), + "--model claude-sonnet-4-6 must select the sonnet variant: {claude}" + ); +} + +#[test] +fn unknown_subdirectory_in_qualifier_dir_warns_and_skips() { + let module = tempfile::tempdir().unwrap(); + scaffold(module.path()); + write_rule(module.path(), "Base.md", "BASE BODY"); + // A non-model subdirectory inside the claude qualifier directory. Phase 1 + // warns and skips it (rather than silently dropping or hard-failing). + fs::create_dir_all(module.path().join("rules/claude/not-a-model")).unwrap(); + fs::write( + module.path().join("rules/claude/not-a-model/Stray.md"), + "stray\n", + ) + .unwrap(); + + let output = forge() + .args(["assemble", "--source", module.path().to_str().unwrap()]) + .assert() + .success(); + let stderr = String::from_utf8_lossy(&output.get_output().stderr).into_owned(); + assert!( + stderr.contains("not-a-model"), + "the skipped directory must be named in a warning: {stderr}" + ); + assert!( + !module.path().join("build/claude/rules/Stray.md").exists(), + "the stray file must not be deployed" + ); +}