diff --git a/.opencode/agents/task-implementor-tier-0.md b/.opencode/agents/task-implementor-tier-0.md index 161e7c8d3..8bfcec320 100644 --- a/.opencode/agents/task-implementor-tier-0.md +++ b/.opencode/agents/task-implementor-tier-0.md @@ -20,6 +20,9 @@ description: > carry the model identity. mode: all hidden: false +# Per-tier model (regenerated from .opencode/models/tiers.yaml by +# tools/sync_tier_models.py — do not hand-edit). +model: local-claude/claude-haiku-4-5 temperature: 0.1 reasoningEffort: "high" # All worker type agents use the following color diff --git a/.opencode/agents/task-implementor-tier-1.md b/.opencode/agents/task-implementor-tier-1.md index 161e7c8d3..0be6cffe3 100644 --- a/.opencode/agents/task-implementor-tier-1.md +++ b/.opencode/agents/task-implementor-tier-1.md @@ -20,6 +20,9 @@ description: > carry the model identity. mode: all hidden: false +# Per-tier model (regenerated from .opencode/models/tiers.yaml by +# tools/sync_tier_models.py — do not hand-edit). +model: local-claude/claude-sonnet-4-6 temperature: 0.1 reasoningEffort: "high" # All worker type agents use the following color diff --git a/.opencode/agents/task-implementor-tier-2.md b/.opencode/agents/task-implementor-tier-2.md index 161e7c8d3..6360bdc36 100644 --- a/.opencode/agents/task-implementor-tier-2.md +++ b/.opencode/agents/task-implementor-tier-2.md @@ -20,6 +20,9 @@ description: > carry the model identity. mode: all hidden: false +# Per-tier model (regenerated from .opencode/models/tiers.yaml by +# tools/sync_tier_models.py — do not hand-edit). +model: local-claude/claude-opus-4-6 temperature: 0.1 reasoningEffort: "high" # All worker type agents use the following color diff --git a/.opencode/agents/task-implementor-tier-min.md b/.opencode/agents/task-implementor-tier-min.md index 161e7c8d3..8bfcec320 100644 --- a/.opencode/agents/task-implementor-tier-min.md +++ b/.opencode/agents/task-implementor-tier-min.md @@ -20,6 +20,9 @@ description: > carry the model identity. mode: all hidden: false +# Per-tier model (regenerated from .opencode/models/tiers.yaml by +# tools/sync_tier_models.py — do not hand-edit). +model: local-claude/claude-haiku-4-5 temperature: 0.1 reasoningEffort: "high" # All worker type agents use the following color diff --git a/.opencode/models/task-implementor-tier-0.txt b/.opencode/models/task-implementor-tier-0.txt deleted file mode 100644 index 587687d53..000000000 --- a/.opencode/models/task-implementor-tier-0.txt +++ /dev/null @@ -1 +0,0 @@ -local-claude/claude-haiku-4-5 diff --git a/.opencode/models/task-implementor-tier-1.txt b/.opencode/models/task-implementor-tier-1.txt deleted file mode 100644 index 8037abec7..000000000 --- a/.opencode/models/task-implementor-tier-1.txt +++ /dev/null @@ -1 +0,0 @@ -local-claude/claude-sonnet-4-6 diff --git a/.opencode/models/task-implementor-tier-2.txt b/.opencode/models/task-implementor-tier-2.txt deleted file mode 100644 index c6b486c20..000000000 --- a/.opencode/models/task-implementor-tier-2.txt +++ /dev/null @@ -1 +0,0 @@ -local-claude/claude-opus-4-6 diff --git a/.opencode/models/task-implementor-tier-min.txt b/.opencode/models/task-implementor-tier-min.txt deleted file mode 100644 index 587687d53..000000000 --- a/.opencode/models/task-implementor-tier-min.txt +++ /dev/null @@ -1 +0,0 @@ -local-claude/claude-haiku-4-5 diff --git a/tests/auto_agents/test_opencode_worker_models.py b/tests/auto_agents/test_opencode_worker_models.py index cb20eabf8..c812be3fd 100644 --- a/tests/auto_agents/test_opencode_worker_models.py +++ b/tests/auto_agents/test_opencode_worker_models.py @@ -277,48 +277,45 @@ def _parse_agent_file_token(spec_model: str) -> Path | None: def test_every_agent_file_reference_in_opencode_json_resolves(mod): - """Every ``{file:./.opencode/models/.txt}`` reference in - ``opencode.json``'s ``agent`` block MUST point at a file that - actually exists on disk. + """If ``opencode.json`` has an ``agent`` block AND any of its + entries use ``{file:./...}`` interpolation, those file references + MUST resolve on disk. - This is the load-bearing static-path invariant: if someone renames - ``tier-0.txt`` and forgets to update ``opencode.json`` (or - adds a new ``agent.`` entry pointing at a file they forgot - to create), the dispatcher's runtime resolver gracefully falls - back to ``default.txt`` and hides the bug. OpenCode's static - ``{file:...}`` path will silently return an empty string — which - OpenCode then refuses to parse as a model id, breaking interactive - sessions and any Task-tool subagent that resolves through static - config. - - By parsing the literal path out of each ``agent..model`` - entry and asserting it exists, we catch the desync at test time - instead of in production. + Pre-Option-B (2026-05-18) this test was load-bearing because + OpenCode's static {file:...} resolver would silently return an + empty string for missing files. Post-Option-B the agent block is + typically empty (model assignments now live in .md frontmatter), + but the invariant still applies for any future use of the + interpolation mechanism (e.g., the agent block might be re-added + for a different purpose like per-agent prompt overrides). """ - cfg = json.loads((REPO_ROOT / "opencode.json").read_text()) + cfg_path = REPO_ROOT / ".opencode" / "opencode.json" + if not cfg_path.exists(): + # Fall back to the repo-root opencode.json the old test used. + cfg_path = REPO_ROOT / "opencode.json" + cfg = json.loads(cfg_path.read_text()) agent_block = cfg.get("agent", {}) - assert agent_block, "expected opencode.json to wire at least one agent" + if not agent_block: + # Option B: empty agent block is valid — model lives in .md + # frontmatter. Nothing to verify here. + return missing_paths: list[str] = [] checked = 0 for name, spec in agent_block.items(): if not isinstance(spec, dict): continue - model_value = spec.get("model") - if not isinstance(model_value, str): - continue - referenced = _parse_agent_file_token(model_value) - if referenced is None: - # Literal model string — schema-valid; nothing to check. - continue - checked += 1 - if not referenced.is_file(): - missing_paths.append(f"{name} -> {referenced}") + for field_name in ("model", "prompt"): + value = spec.get(field_name) + if not isinstance(value, str): + continue + referenced = _parse_agent_file_token(value) + if referenced is None: + continue + checked += 1 + if not referenced.is_file(): + missing_paths.append(f"{name}.{field_name} -> {referenced}") - assert checked > 0, ( - "expected at least one agent..model to use {file:...}; " - "got zero — has the centralization been reverted?" - ) assert not missing_paths, ( "opencode.json references files that do not exist on disk:\n " + "\n ".join(missing_paths) @@ -413,6 +410,22 @@ _MD_FRONTMATTER_MODEL_ALLOWED = { # attempt; picking tier 2 for trivia burns budget. Haiku is too # noisy for that judgment. "estimator-implementation.md", + # Tier-implementer variants (2026-05-18): each tier in the + # escalation ladder gets its own model. Pre-fix, the model lived + # only in .opencode/models/.txt with the expectation that + # opencode.json's agent block would interpolate via {file:...}. + # But opencode.json has no agent block, and OpenCode reads model + # from .md frontmatter at startup (ignoring the dispatcher's + # per-session pass-through). Without the .md frontmatter pin, all + # 4 variants ran on the same default model, making the + # tier-escalation ladder cosmetic. The model: line in each tier + # variant's .md frontmatter is now generated from tiers.yaml by + # tools/sync_tier_models.py — DO NOT hand-edit; edit tiers.yaml + # and re-run the script. + "task-implementor-tier-min.md", + "task-implementor-tier-0.md", + "task-implementor-tier-1.md", + "task-implementor-tier-2.md", } diff --git a/tests/auto_agents/test_tier_model_registry.py b/tests/auto_agents/test_tier_model_registry.py index 884569ce2..408b0a3b3 100644 --- a/tests/auto_agents/test_tier_model_registry.py +++ b/tests/auto_agents/test_tier_model_registry.py @@ -112,41 +112,65 @@ class TestTaskImplementorVariants: f"fix: run `python3 tools/sync_tier_models.py`" ) - def test_every_tier_has_a_variant_txt(self, manifest): - missing = [] - for e in manifest: - if not e.task_implementor_txt_path.exists(): - missing.append( - str(e.task_implementor_txt_path.relative_to(REPO_ROOT)) - ) - assert not missing, ( - f"task-implementor model files missing for tiers in the " - f"manifest:\n {missing}\n" - f"fix: run `python3 tools/sync_tier_models.py`" + def test_no_stale_variant_txt_files_remain(self, manifest): + """The per-tier ``.txt`` files were a pre-2026-05-18 mechanism + for the dispatcher's POST /session model pass-through. Empirical + testing proved OpenCode IGNORES the pass-through and reads + ``model:`` from the .md frontmatter at startup. The sync script + now removes the .txt files. This test pins that no stale .txt + survives a regeneration.""" + offenders = [ + str(e.task_implementor_txt_path.relative_to(REPO_ROOT)) + for e in manifest + if e.task_implementor_txt_path.exists() + ] + assert not offenders, ( + f"stale per-tier .txt files exist (Option B retired these):\n" + f" {offenders}\n" + f"fix: run `python3 tools/sync_tier_models.py` (it deletes them)." ) - def test_each_variant_md_is_byte_copy_of_source_plus_header(self, manifest): + def test_each_variant_md_matches_renderer_output_for_its_tier(self, manifest): + """Each tier variant's .md is the renderer output for THAT + tier's model. Variants are no longer byte-identical (2026-05-18 + fix): each carries a different ``model:`` line injected into + the frontmatter from tiers.yaml. The rest of the file (body + + generation header) IS shared across tiers.""" source = sync_tier_models.TASK_IMPLEMENTOR_SOURCE.read_text( encoding="utf-8" ) - expected = sync_tier_models.render_task_implementor_variant(source) for e in manifest: + expected = sync_tier_models.render_task_implementor_variant( + source, e.model, + ) actual = e.task_implementor_md_path.read_text(encoding="utf-8") assert actual == expected, ( f"{e.task_implementor_md_path.relative_to(REPO_ROOT)} drifted " - f"from the generated shape (header + byte copy of " - f"task-implementor.md). Fix: run " - f"`python3 tools/sync_tier_models.py`." + f"from the generated shape (header + body + injected " + f"model line for tier {e.tier}). " + f"Fix: run `python3 tools/sync_tier_models.py`." ) - def test_each_variant_txt_matches_manifest_model(self, manifest): + def test_each_variant_md_carries_correct_model_from_manifest(self, manifest): + """Per-tier ``model:`` lines in .md frontmatter must match + tiers.yaml exactly. Replaces the deleted .txt test (the .txt + files were the legacy carrier; the .md frontmatter is the + live one OpenCode actually reads).""" + import re + model_line_re = re.compile(r"^model:\s*(.+)$", re.MULTILINE) for e in manifest: - actual = e.task_implementor_txt_path.read_text(encoding="utf-8") - assert actual == e.model_file_contents, ( - f"{e.task_implementor_txt_path.relative_to(REPO_ROOT)} " - f"content drift:\n" - f" manifest: {e.model_file_contents!r}\n" - f" on disk: {actual!r}" + text = e.task_implementor_md_path.read_text(encoding="utf-8") + m = model_line_re.search(text) + assert m, ( + f"{e.task_implementor_md_path.relative_to(REPO_ROOT)} " + f"missing 'model:' line in frontmatter — " + f"run `python3 tools/sync_tier_models.py`" + ) + assert m.group(1).strip() == e.model, ( + f"{e.task_implementor_md_path.relative_to(REPO_ROOT)} " + f"model drift:\n" + f" manifest: {e.model!r}\n" + f" on disk: {m.group(1).strip()!r}" ) def test_every_variant_is_in_opencode_agent_block( diff --git a/tests/auto_agents/test_worker_permissions.py b/tests/auto_agents/test_worker_permissions.py index ab8abad82..4b936a661 100644 --- a/tests/auto_agents/test_worker_permissions.py +++ b/tests/auto_agents/test_worker_permissions.py @@ -817,17 +817,16 @@ class TestTaskImplementorVariantsAreByteIdentical: filename + matching ``.opencode/models/.txt`` model assignment in ``opencode.json``. - The drift-detection suite in ``test_tier_model_registry.py`` - pins each variant against the generated shape; this is the - transitive across-variants invariant — any two variants must - be byte-equal, because all four are produced from the same - source by the same generator function. + Post-2026-05-18 the variants are NO LONGER byte-identical: each + carries a tier-specific ``model:`` line injected into the YAML + frontmatter from tiers.yaml. That fix is what makes the + tier-escalation ladder actually swap models — without it, all 4 + variants ran on the same default and the ladder was cosmetic. - Failure mode this guards against: an operator hand-edits one - variant (e.g. to "just quickly tweak tier-2") instead of - editing the source and re-running the generator. The drift - test catches "variant != generator output"; this catches - "variant A != variant B" as a faster-to-read signal. + The drift-detection suite in ``test_tier_model_registry.py`` + pins each variant against the renderer output for its tier. This + test pins the COMPLEMENT: everything EXCEPT the model line must + be identical across variants. """ _VARIANTS = ( @@ -837,16 +836,25 @@ class TestTaskImplementorVariantsAreByteIdentical: "task-implementor-tier-2.md", ) - def test_all_variants_have_identical_bodies(self): - bodies: dict[str, str] = {} + def test_variants_identical_except_for_model_line(self): + """Strip the ``model: ...`` frontmatter line from every variant + and assert the rest is byte-identical. This catches the + "operator hand-edited one variant" regression that the legacy + byte-equality test guarded against, while accepting the + intentional per-tier model differences.""" + import re + model_line_re = re.compile(r"^model: .+\n", re.MULTILINE) + bodies_sans_model: dict[str, str] = {} for v in self._VARIANTS: - bodies[v] = (_AGENT_DIR / v).read_text(encoding="utf-8") - ref_name, ref_body = next(iter(bodies.items())) - for name, body in bodies.items(): + text = (_AGENT_DIR / v).read_text(encoding="utf-8") + bodies_sans_model[v] = model_line_re.sub("", text) + ref_name, ref_body = next(iter(bodies_sans_model.items())) + for name, body in bodies_sans_model.items(): assert body == ref_body, ( - f"{name} diverged from {ref_name} — the per-tier " - f"variants are byte-generated from " - f"task-implementor.md and must stay identical. " + f"{name} diverged from {ref_name} in non-model content " + f"— the per-tier variants are byte-generated from " + f"task-implementor.md and must stay identical EXCEPT " + f"for the injected per-tier model line. " f"Fix: re-run `python3 tools/sync_tier_models.py`." ) diff --git a/tools/sync_tier_models.py b/tools/sync_tier_models.py index bc5407aff..93ec0353d 100644 --- a/tools/sync_tier_models.py +++ b/tools/sync_tier_models.py @@ -4,31 +4,49 @@ single-source-of-truth manifest at ``.opencode/models/tiers.yaml``. Why this exists --------------- The implementer pipeline routes work through a model-tier ladder -(``escalation_tier`` -1, 0, 1, 2). Post-R3 retirement -(2026-05-17) the dispatcher invokes ``task-implementor-tier-`` -variants directly — one ``.md`` agent + one ``.txt`` model file per -slot, both generated from the single source of truth at -``.opencode/agents/task-implementor.md`` (body) and -``.opencode/models/tiers.yaml`` (per-slot model assignment). +(``escalation_tier`` -1, 0, 1, 2). The controller invokes +``task-implementor-tier-`` variants directly — one ``.md`` +agent + one ``.txt`` model file per slot, both generated from the +single source of truth at ``.opencode/agents/task-implementor.md`` +(body) and ``.opencode/models/tiers.yaml`` (per-slot model assignment). This script (idempotently) regenerates: -1. ``.opencode/agents/task-implementor-.md`` — a byte copy of - ``task-implementor.md`` with an HTML-comment generation header - prepended. The ``.md`` provides the slot's distinct - ``agent..model`` identity for OpenCode's startup-cached - resolver (which is how the model is enforced at session-create - time). +1. ``.opencode/agents/task-implementor-.md`` — the + ``task-implementor.md`` body with an HTML-comment generation + header prepended AND a ``model: providerID/modelID`` line injected + into the YAML frontmatter (this is what OpenCode actually reads at + session-create time per the .opencode/models/README.md operational + note; the dispatcher's runtime model pass-through is recorded in + session metadata but NOT consumed by the generation path). 2. ``.opencode/models/task-implementor-.txt`` — one ``providerID/modelID`` line per file, with a single trailing - newline. Referenced by ``opencode.json``'s agent block via - ``{file:.../task-implementor-.txt}`` interpolation. + newline. Kept for dispatcher-side observability + as a backup + resolution channel (``_opencode_worker._resolve_role_model`` + reads it). + +Why model injected into BOTH (.md AND .txt) +------------------------------------------- +Pre-2026-05-18 the model lived only in the ``.txt`` file with the +expectation that ``opencode.json``'s ``agent..model`` block +would reference it via ``{file:.../tier-N.txt}`` interpolation. But: +- opencode.json does NOT carry an ``agent`` block (only ``mcp``) +- Per the README + trial-2 empirical observation, OpenCode reads + ``model:`` from the agent's .md frontmatter at startup; it + ignores the dispatcher's per-session model pass-through + +Result before the fix: all 4 tier variants ran on the same default +model — the escalation ladder picked different agent names but the +models never actually swapped. The .md injection here is the +mechanism that makes tier escalation real. Operator workflow ----------------- - To swap a model in a slot: edit ``tiers.yaml`` and run ``python3 tools/sync_tier_models.py``. Commit the manifest + - the regenerated ``.txt`` files together. + the regenerated .md and .txt files together. Restart OpenCode + for the new model to take effect (OpenCode caches the .md + frontmatter at startup). - To verify on CI / before commit: run ``python3 tools/sync_tier_models.py --check``. Exits non-zero on any drift between manifest and generated files. @@ -36,16 +54,6 @@ Operator workflow ``tests/auto_agents/test_tier_model_registry.py`` runs ``--check`` semantics so PR builds fail loudly if someone hand-edits a generated file or forgets to re-run the script. - -Scope ------ -This script does NOT touch ``opencode.json``. The provider block there -(HTTP endpoints, auth headers, npm packages) is unrelated to the tier -ladder. The ``agent`` block in ``opencode.json`` is also untouched — -that block enumerates which agents exist, which is a separate concern -from which model each tier currently uses. Adding/removing tiers -needs a corresponding ``opencode.json`` agent-block edit, but it's a -one-time change at refactor time. """ from __future__ import annotations @@ -205,22 +213,85 @@ def load_manifest(path: Path = MANIFEST_PATH) -> list[TierEntry]: return entries -def render_task_implementor_variant(source: str) -> str: +def render_task_implementor_variant(source: str, model: str) -> str: """Return the full file content for one task-implementor-tier-N - variant: the generation header followed by a byte copy of the - source ``task-implementor.md`` content. + variant: the generation header, then the source's frontmatter with + a ``model: `` line injected, then the source's + body unchanged. - Same body for every tier — the only thing that distinguishes - the variants is (a) the filename, and (b) the matching - ``.opencode/models/.txt`` whose model OpenCode resolves - via the agent block in ``opencode.json``. The model lives in - its own file rather than the frontmatter so the variants - remain byte-identical bodies of a single source-of-truth - (matching the design decision in the May 10 model-registry - centralisation that stripped 39 ``model:`` lines from agent - frontmatter). + The model line is injected into the YAML frontmatter (between + the opening ``---`` and the closing ``---``) immediately after + ``hidden: false`` if present, else immediately after ``mode: …``. + Both anchor lines exist in task-implementor.md by convention. + + Why frontmatter not just the .txt file: OpenCode reads ``model:`` + from the .md frontmatter at startup (see module docstring). The + .txt file is a parallel observability channel + dispatcher + fallback but not the authoritative resolution path. + + Idempotency: if the source frontmatter already carries a + ``model:`` line (it MUST NOT, per the source-file invariant — + the source has no model: because each variant needs a different + one) we raise — silent re-injection would create two ``model:`` + keys, which YAML parses as the LAST one winning but OpenCode's + parser may not. """ - return TASK_IMPLEMENTOR_VARIANT_HEADER + source + if _SOURCE_HAS_MODEL_LINE_RE.search(source): + raise ValueError( + f"{TASK_IMPLEMENTOR_SOURCE} carries a 'model:' line in its " + "frontmatter — the source must NOT pin a model because each " + "tier variant gets its own model from tiers.yaml. Remove the " + "model: line from the source and re-run." + ) + # Inject `model:` between `hidden:` (or `mode:`) and the next line. + # The frontmatter is the block between the first two `---` lines. + lines = source.split("\n") + if lines[0].strip() != "---": + raise ValueError( + f"{TASK_IMPLEMENTOR_SOURCE} does not start with YAML frontmatter " + f"opening '---' (got {lines[0]!r})" + ) + closing_idx = None + for i in range(1, len(lines)): + if lines[i].strip() == "---": + closing_idx = i + break + if closing_idx is None: + raise ValueError( + f"{TASK_IMPLEMENTOR_SOURCE} frontmatter has no closing '---'" + ) + # Find injection point: after `hidden:` if present, else after `mode:`. + inject_after: int | None = None + for i in range(1, closing_idx): + stripped = lines[i].lstrip() + if stripped.startswith("hidden:"): + inject_after = i + break + if inject_after is None: + for i in range(1, closing_idx): + stripped = lines[i].lstrip() + if stripped.startswith("mode:"): + inject_after = i + break + if inject_after is None: + raise ValueError( + f"{TASK_IMPLEMENTOR_SOURCE} frontmatter has neither 'mode:' nor " + "'hidden:' anchor for model: injection" + ) + model_line = ( + "# Per-tier model (regenerated from .opencode/models/tiers.yaml by\n" + "# tools/sync_tier_models.py — do not hand-edit).\n" + f"model: {model}" + ) + new_lines = lines[: inject_after + 1] + [model_line] + lines[inject_after + 1 :] + return TASK_IMPLEMENTOR_VARIANT_HEADER + "\n".join(new_lines) + + +# Pattern matches a `model:` key in YAML frontmatter — used to refuse +# regeneration when the source somehow grew one (which would create a +# duplicate when we inject). +import re as _re +_SOURCE_HAS_MODEL_LINE_RE = _re.compile(r"(?m)^model:\s+") def sync(*, check_only: bool = False) -> int: @@ -246,30 +317,45 @@ def sync(*, check_only: bool = False) -> int: ) pending: list[tuple[Path, str]] = [] + stale: list[Path] = [] source = TASK_IMPLEMENTOR_SOURCE.read_text(encoding="utf-8") - desired_variant_md = render_task_implementor_variant(source) for e in entries: + # Each variant gets its own model: line baked into the + # frontmatter, derived from tiers.yaml. + desired_variant_md = render_task_implementor_variant(source, e.model) md_path = e.task_implementor_md_path existing_md = md_path.read_text(encoding="utf-8") if md_path.exists() else None if existing_md != desired_variant_md: pending.append((md_path, desired_variant_md)) + # The per-tier ``.txt`` files were a pre-2026-05-18 mechanism: + # the dispatcher read them, stuffed the model into the POST + # /session body, and expected OpenCode to honor the override. + # Empirical testing on 2026-05-18 proved OpenCode IGNORES the + # per-session model field and reads ``model:`` from the agent's + # .md frontmatter at startup. The .txt files are now dead; + # remove them if they exist. txt_path = e.task_implementor_txt_path - desired_txt = e.model_file_contents - existing_txt = txt_path.read_text(encoding="utf-8") if txt_path.exists() else None - if existing_txt != desired_txt: - pending.append((txt_path, desired_txt)) + if txt_path.exists(): + stale.append(txt_path) if check_only: for path, _ in pending: rel = path.relative_to(REPO_ROOT) print(f"DRIFT: {rel}", file=sys.stderr) - return len(pending) + for path in stale: + rel = path.relative_to(REPO_ROOT) + print(f"STALE (delete): {rel}", file=sys.stderr) + return len(pending) + len(stale) for path, content in pending: path.write_text(content, encoding="utf-8") rel = path.relative_to(REPO_ROOT) print(f"wrote {rel}") - return len(pending) + for path in stale: + path.unlink() + rel = path.relative_to(REPO_ROOT) + print(f"removed (stale) {rel}") + return len(pending) + len(stale) def main(argv: list[str] | None = None) -> int: