fix: resolve-agent --alias flag (Claude-Code model alias adapter)#414
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a recurring Claude Code sub-agent dispatch failure by adding a deterministic model-ID → short-alias adapter at the fab resolve-agent boundary, so orchestrators can pass an Agent-tool-valid model= value without hand-mapping.
Changes:
- Add
--aliastofab resolve-agentand implementinternal/agent.ModelAlias(prefix-based mapping toopus/sonnet/haiku/fable). - Add/extend Go tests covering the alias mapping and the CLI output with/without
--alias. - Repoint pipeline skill/docs prose to use
fab resolve-agent <stage> --aliasfor Agent-tool dispatches and document the new flag.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/go/fab/internal/agent/agent.go | Adds ModelAlias helper and Claude-family prefix mapping table. |
| src/go/fab/internal/agent/agent_test.go | Adds unit tests for ModelAlias. |
| src/go/fab/cmd/fab/resolve_agent.go | Adds --alias flag and applies aliasing before formatting output. |
| src/go/fab/cmd/fab/resolve_agent_test.go | Adds command-level tests for --alias vs default output. |
| src/kit/skills/_cli-fab.md | Documents fab resolve-agent <stage> [--alias] and semantics. |
| src/kit/skills/_preamble.md | Updates canonical dispatch contract to describe --alias usage for Agent tool. |
| src/kit/skills/_pipeline.md | Repoints bracket dispatch instructions to call resolve-agent --alias. |
| src/kit/skills/fab-ff.md | Repoints wrapper prose to call resolve-agent --alias. |
| src/kit/skills/fab-fff.md | Repoints wrapper + ship/review-pr steps to call resolve-agent --alias. |
| src/kit/skills/fab-continue.md | Repoints sequencer/dispatch contract text and table rows to --alias. |
| docs/specs/stage-models.md | Updates adapter-boundary prose to make --alias the deterministic mechanism. |
| docs/specs/skills/SPEC-_preamble.md | Updates SPEC mirror to reference --alias adapter. |
| docs/memory/_shared/context-loading.md | Updates memory docs to reflect --alias adapter boundary. |
| docs/memory/_shared/index.md | Updates index blurb for context-loading memory entry. |
| docs/memory/pipeline/execution-skills.md | Updates pipeline memory narrative and changelog to reflect --alias. |
| docs/memory/pipeline/index.md | Updates pipeline index metadata date for planning-skills entry. |
| docs/memory/memory-docs/index.md | Updates memory-docs index metadata date for templates entry. |
| fab/changes/260613-yky7-resolve-agent-alias-flag/intake.md | Adds intake artifact documenting the problem and solution. |
| fab/changes/260613-yky7-resolve-agent-alias-flag/plan.md | Adds plan artifact capturing requirements/tasks/acceptance. |
| fab/changes/260613-yky7-resolve-agent-alias-flag/.status.yaml | Adds status artifact for the change pipeline. |
| fab/changes/260613-yky7-resolve-agent-alias-flag/.history.jsonl | Adds history artifact for pipeline execution trace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| The bracket defines everything else: pre-flight (intake prerequisite + intake gate), context loading, resumability, Steps 1–3 (apply → review → hydrate), the auto-rework loop with its per-cycle choreography, and the exhaustion stop. | ||
|
|
||
| > **Per-stage model**: each stage dispatch in the bracket resolves `fab resolve-agent <stage>` first, surfaces the resolved `model=/effort=` (so a skipped or mis-resolved tier is visible, not silent), then dispatches through the two seams — model via the Agent tool's `model` param (empty ⇒ omit/inherit) and effort via an imperative instruction in the dispatch prompt (``Operate at `<effort>` reasoning effort for this task.``; empty effort ⇒ omit, since the Agent tool has no effort param) — see `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. | ||
| > **Per-stage model**: each stage dispatch in the bracket resolves the stage's profile first, surfaces the resolved `model=/effort=` (so a skipped or mis-resolved tier is visible, not silent), then dispatches through the two seams — model via the Agent tool's `model` param, resolved with `fab resolve-agent <stage> --alias` so the alias is Agent-tool-valid (empty ⇒ omit/inherit), and effort via an imperative instruction in the dispatch prompt (``Operate at `<effort>` reasoning effort for this task.``; empty effort ⇒ omit, since the Agent tool has no effort param) — see `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. |
There was a problem hiding this comment.
Fixed — updated SPEC-fab-ff.md to document the dispatch resolve as fab resolve-agent <stage> --alias (attributed to 260613-yky7), matching the skill's per-stage dispatch contract. (0a9dea0)
| The bracket defines pre-flight (intake prerequisite + intake gate), context loading, resumability, Steps 1–3 (apply → review → hydrate), the auto-rework loop with its per-cycle choreography, and the exhaustion stop. The two steps below are fff-only. | ||
|
|
||
| > **Per-stage model**: every stage dispatch (the bracket's Steps 1–3 and the fff-only Steps 4–5 below) resolves `fab resolve-agent <stage>` first, surfaces the resolved `model=/effort=` (so a skipped or mis-resolved tier is visible, not silent), then dispatches through the two seams — model via the Agent tool's `model` param (empty ⇒ omit/inherit) and effort via an imperative instruction in the dispatch prompt (``Operate at `<effort>` reasoning effort for this task.``; empty effort ⇒ omit, since the Agent tool has no effort param) — see `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. | ||
| > **Per-stage model**: every stage dispatch (the bracket's Steps 1–3 and the fff-only Steps 4–5 below) resolves the stage's profile first, surfaces the resolved `model=/effort=` (so a skipped or mis-resolved tier is visible, not silent), then dispatches through the two seams — model via the Agent tool's `model` param, resolved with `fab resolve-agent <stage> --alias` so the alias is Agent-tool-valid (empty ⇒ omit/inherit), and effort via an imperative instruction in the dispatch prompt (``Operate at `<effort>` reasoning effort for this task.``; empty effort ⇒ omit, since the Agent tool has no effort param) — see `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution. |
There was a problem hiding this comment.
Fixed — updated SPEC-fab-fff.md so the Steps 4–5 resolve calls read fab resolve-agent ship --alias / fab resolve-agent review-pr --alias, matching fab-fff.md's dispatch contract. (0a9dea0)
| Advance through the 6-stage Fab pipeline one step at a time. Each invocation handles the current stage's work and transitions to the next. When called with a stage argument, resets to that stage and re-runs from there. | ||
|
|
||
| > **Per-stage model (one-stage sequencer)**: post-intake `/fab-continue` is a **one-stage sequencer** — it dispatches its stage as a sub-agent (see Normal Flow Step 1), and per-stage model selection (`fab resolve-agent <stage>` → `agent.tiers`, see `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution) is resolved **once, immediately before that dispatch**, surfaced (so a skipped/mis-resolved tier is visible), and applied through both seams — model via the Agent tool's `model` param, effort via an imperative instruction in the sub-agent's prompt (empty effort ⇒ omit). There is no foreground execution path for apply/review/hydrate to leave the tier merely advisory: every post-intake stage runs dispatched, so `fab resolve-agent <stage>` applies uniformly. (Intake is pre-boundary — it runs in the main session and is not tiered by `/fab-continue`.) | ||
| > **Per-stage model (one-stage sequencer)**: post-intake `/fab-continue` is a **one-stage sequencer** — it dispatches its stage as a sub-agent (see Normal Flow Step 1), and per-stage model selection (`fab resolve-agent <stage>` → `agent.tiers`, see `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution) is resolved **once, immediately before that dispatch**, surfaced (so a skipped/mis-resolved tier is visible), and applied through both seams — model via the Agent tool's `model` param (resolved with `fab resolve-agent <stage> --alias` so the alias is Agent-tool-valid), effort via an imperative instruction in the sub-agent's prompt (empty effort ⇒ omit). There is no foreground execution path for apply/review/hydrate to leave the tier merely advisory: every post-intake stage runs dispatched, so `fab resolve-agent <stage>` applies uniformly. (Intake is pre-boundary — it runs in the main session and is not tiered by `/fab-continue`.) |
There was a problem hiding this comment.
Fixed — reconciled SPEC-fab-continue.md: the one-stage-sequencer resolve, the Per-stage model paragraph, the review-once resolve, and the dispatch annotation now all read fab resolve-agent <stage> --alias (attributed to 260613-yky7). (0a9dea0)
| > **Dispatch**: All sub-skill invocations use the Agent tool (`general-purpose` subagent) per `_preamble.md` § Subagent Dispatch. Each subagent reads the target skill file, follows the specified behavior, and returns a structured result to the pipeline. Every `/fab-continue`-behavior subagent prompt MUST include: **"do NOT run `fab status` commands; return results only"** — the orchestrator runs those stages' transitions (finish/fail/reset) itself. This is the **universal block contract**, not an override this orchestrator imposes: post-intake `/fab-continue` blocks (Apply / Review / Hydrate Behavior) never own their transitions for **any** caller — plain `/fab-continue` is itself a one-stage sequencer that dispatches the block identically and runs the transition after it returns (see `fab-continue.md` Normal Flow Step 1). The orchestrator here is therefore a **pure sequencer**: dispatch block → read returned status/findings → decide proceed / loop / stop; it never reaches into block internals. | ||
| > | ||
| > **Per-stage model resolution** (see `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution for the canonical contract): immediately **before** dispatching each stage's sub-agent, run `fab resolve-agent <stage>` and **surface** the resolved `model=/effort=` lines (carry them into the dispatch prompt and/or echo them in this orchestrator's step output, so a skipped or mis-resolved tier is visible rather than silent), then dispatch through the two seams: the **model** half via the Agent tool's `model` parameter (empty model ⇒ omit/inherit), and the **effort** half via an explicit imperative instruction in the dispatch prompt — ``Operate at `<effort>` reasoning effort for this task.`` (empty effort ⇒ omit the instruction; the Agent tool has no effort param). The Claude Code adapter is the Agent tool's `model` parameter; the resolution itself is provider-neutral. The `review` stage (Step 2) resolves **once** and applies the same model AND the same effort-prompt instruction to both reviewer sub-agents AND the merge. | ||
| > **Per-stage model resolution** (see `_preamble.md` § Subagent Dispatch → Per-Stage Model Resolution for the canonical contract): immediately **before** dispatching each stage's sub-agent, run `fab resolve-agent <stage> --alias` and **surface** the resolved `model=/effort=` lines (carry them into the dispatch prompt and/or echo them in this orchestrator's step output, so a skipped or mis-resolved tier is visible rather than silent), then dispatch through the two seams: the **model** half via the Agent tool's `model` parameter — the `--alias` flag emits the Agent-tool-valid short alias directly on the `model=` line (empty model ⇒ omit/inherit) — and the **effort** half via an explicit imperative instruction in the dispatch prompt — ``Operate at `<effort>` reasoning effort for this task.`` (empty effort ⇒ omit the instruction; the Agent tool has no effort param). The Claude Code adapter is the Agent tool's `model` parameter; the resolution itself is provider-neutral. The `review` stage (Step 2) resolves **once** and applies the same model AND the same effort-prompt instruction to both reviewer sub-agents AND the merge. |
There was a problem hiding this comment.
Fixed — updated SPEC-_pipeline.md (Summary line 5 and the flow block's Steps 1–3 / resolve note) to read fab resolve-agent <stage> --alias, matching the authoritative bracket. (0a9dea0)
| for in, want := range cases { | ||
| t.Run(in, func(t *testing.T) { | ||
| if got := ModelAlias(in); got != want { |
There was a problem hiding this comment.
Fixed — the empty-string case now uses the subtest name "empty" (falling back when the input is empty), so output reads TestModelAlias/empty instead of TestModelAlias/. (0a9dea0)
| **Harness-adapter boundary (Claude Code).** The resolution (stage→tier→`{model, effort}`) is **provider-neutral**. Injecting the resolved model into the actual dispatch is **harness-specific**: for Claude Code that adapter is the **Agent tool's `model` parameter**, and the effort half is injected via the subagent-prompt instruction described above. One concrete harness detail: the Agent tool's `model` param takes a **short alias** (`opus` / `sonnet` / `haiku` / `fable`), **not** the full versioned id (`claude-opus-4-8`) that `fab resolve-agent` emits — the orchestrator maps the resolved id to the alias at the dispatch seam. This is named explicitly as the Claude-Code adapter, not as universal truth — and the coupling is not new (fab's entire subagent-dispatch design is already Claude-Code-shaped). | ||
| **Harness-adapter boundary (Claude Code).** The resolution (stage→tier→`{model, effort}`) is **provider-neutral**. Injecting the resolved model into the actual dispatch is **harness-specific**: for Claude Code that adapter is the **Agent tool's `model` parameter**, and the effort half is injected via the subagent-prompt instruction described above. One concrete harness detail: the Agent tool's `model` param takes a **short alias** (`opus` / `sonnet` / `haiku` / `fable`), **not** the full versioned id (`claude-opus-4-8`) the plain command emits. So for Agent-tool dispatch, **resolve the model half with `fab resolve-agent <stage> --alias`** — the `--alias` flag emits the Agent-tool-valid short alias directly on the `model=` line (empty ⇒ omit/inherit; a non-Claude override passes through verbatim), so no agent ever hand-maps the id. This is named explicitly as the Claude-Code adapter, not as universal truth — and the coupling is not new (fab's entire subagent-dispatch design is already Claude-Code-shaped). *(The operator launcher path is the deliberate exception: it appends `--model <full-id>` to a `claude` **CLI** invocation, which accepts full IDs, so it resolves WITHOUT `--alias`.)* | ||
|
|
||
| **Review resolves once.** The `review` stage spawns two reviewer sub-agents (inward + outward) plus a merge. Resolve `fab resolve-agent review` **once** and apply the same profile to all three — the same model and the same effort-prompt instruction govern both reviewers and the merge; the mechanical merge runs at the reviewer's tier (an accepted stage-granularity tradeoff). |
There was a problem hiding this comment.
Fixed — the 'Review resolves once' paragraph in _preamble.md now reads fab resolve-agent review --alias, consistent with the canonical Agent-tool adapter contract (no agent hand-maps the id). (0a9dea0)
Meta
Pipeline: intake ✓ → apply ✓ → review ✓ → hydrate ✓ → ship → review-pr
Impact:
impl: +73/−21 (net +52)
tests: +67/−0 (net +67)
total: +140/−21 (net +119) ← excludes
fab/,docs/Summary
The Agent tool's
modelparameter is a hard enum (["sonnet","opus","haiku","fable"]) and rejects full model IDs likeclaude-opus-4-8. PR #413 documented this mismatch and told dispatching agents to hand-map the ID to the alias at every sub-agent dispatch — but a live run showed exactly that hand-map being fumbled, producing "Invalid tool parameters." This change adds a--aliasflag tofab resolve-agentthat emits the Claude-Code short alias on themodel=line, so orchestrators passfab resolve-agent <stage> --aliasfor Agent-tool dispatches and never hand-map again. The default (no flag) remains byte-identical to today, and the operator/CLI path (which accepts full IDs) is untouched.Changes
internal/agent.ModelAlias— new exported function; prefix-based mapping (claude-opus-→opus, etc.); empty and unmapped IDs pass through verbatim (provider-neutral, not a validator)cmd/fab/resolve_agent.go--aliasflag — boolean flag; when set, appliesModelAliastoprofile.Modelbefore formatting themodel=line;effort=line unaffectedModelAliasunit tests (each alias, dated variant, empty, unmapped) +resolve-agent --aliascommand-level tests (with/without flag, empty-model case)_preamble.md,fab-ff.md,fab-fff.md,fab-continue.md,_pipeline.mdeach repoint their Agent-tool dispatch resolve calls from hand-map prose tofab resolve-agent <stage> --alias_cli-fab.md— documents the new--aliasflag per the constitution CLI constraintdocs/specs/stage-models.md+SPEC-_preamble.md— adapter prose updated to describe--aliasas the deterministic mechanism; drift-guarded tier tables untouchedpipeline/execution-skills.md, context-loading, and index files updated to reflect the--aliasadapter boundary