# Code Review: fix-harness-command-template

Self-review against the SPEC and the harness-agnostic contract.

## SPEC compliance

- **R1** `{prompt_content}` substitution token: ✓ implemented in `_invoke_harness` (loop-runner.py). Reads the resolved prompt file's text; falls back to `""` on OSError. Single argv element under `subprocess.run` list mode.
- **R2** New default `["opencode", "run", "--dir", "{cwd}", "{prompt_content}"]`: ✓ replaced both fallback branches (harness_cfg is None, and empty command array).
- **R3** No hardcoded `--model` in default: ✓ confirmed — model is inherited from opencode config.
- **R4** Per-role harness command override: ✓ not added (out of scope, per design).
- **R5** Test stub updates: ✓ `_make_loop` helpers in 4 test files write loop-local prompt stubs only when the framework prompt at `~/.automaton/prompts/<ref>` does not already exist (preserves token-substitution tests in test_loop_templates). Custom-command tests now identify roles by `implement-prompt`/`verify-prompt` matchers (the temp file path); default-command tests use `test-impl` etc. (matching the stub content `prompt: test-impl.md`).
- **R6** Design doc update: ✓ `design/loops/technical.md` §8 and §9 (self-improvement template) updated to the new default; documented `{prompt_content}` alongside existing tokens; added Pi Dev, aider, and generic examples.
- **R7** Backwards compat: ✓ `{prompt}` and `{cwd}` tokens still populated; the existing custom-command test (`TestHarnessSubstitution.test_custom_command_with_output_token`) passes unchanged.

## Harness-agnostic contract check

- Runner core has zero harness awareness: ✓ only token substitution, no `if harness == "opencode"` branches.
- D8 (no model/provider inspection): ✓ preserved; no model name appears in the runner core, only in user-overridable `harness.command`.
- The fix is MORE agnostic than v1: ✓ adds `{prompt_content}` covering harnesses that prefer a message argument (aider, Pi Dev, any CLI taking a prompt as positional). v1 only supported file-path-based prompts.

## Test plan compliance

Tests in `tests/test_harness_command.py` (7 new):
1. `test_default_uses_dir_not_cwd` — ✓ asserts `--dir` is present, `--cwd` and `--prompt-file` absent
2. `test_default_passes_prompt_content` — ✓ asserts the prompt text appears as the last argv element
3. `test_prompt_content_handles_special_chars` — ✓ asserts a prompt containing single quotes, double quotes, and dollar signs appears as a single argv element
4. `test_prompt_token_still_available` — ✓ custom `["cat", "{prompt}"]` receives the temp file path
5. `test_custom_command_with_cwd_still_works` — ✓ custom `--cwd` receives the cwd value
6. `test_empty_command_falls_back_to_new_default` — ✓ empty `command` array falls back to `--dir {cwd} {prompt_content}` (NOT the old shape)
7. `test_pi_shaped_command_substitutes_correctly` — ✓ proves the substitution mechanism works for a non-opencode binary (`pi run --cwd {cwd} {prompt_content}`)

Existing tests updated (per SPEC R5):
- `_make_loop` helpers in 4 test files now write loop-local prompt stubs with role-marker content `prompt: <ref>`, preserving the substring-matcher strategy used by tick-flow tests. Skipped when the framework prompt exists (so test_loop_templates still substitutes real framework prompt tokens).
- The `--prompt-file` stub rule (`fake_run.add_simple("--prompt-file", "")`) is removed — the default matcher fallback handles generic invocations.
- Custom `harness.command` tests using `{prompt}` token: matchers updated from `test-impl` to `implement-prompt` (the resolved temp file path contains `tickN-implement-prompt.md`).
- Default-command tests using `{prompt_content}` stub content: matchers stay `test-impl` (matches the stub content `prompt: test-impl.md`).

Full suite: **440 passed** (was 433; +7 new). No regressions.

## Risks revisited

- Argv length: real prompts are 2-10KB; OS argv limit is 128KB+. Acceptable.
- Test mock drift: the `fake_run` fixture now mocks a different default shape. A separate smoke test that shells out to `opencode run --help` would catch future flag renames. Not added in this task to keep the change focused; noted for a future hardening pass. The 7 new `_invoke_harness` unit tests do cover the default-command construction directly, which is the main surface.
- Pi Dev CLI: actual `pi run` flags unverified (pi not installed on this machine). The Pi Dev test (test 7) uses a representative shape; the user confirms actual flags against `pi run --help` on their machine before going live.

## Verdict

PASS — proceed to bug_find.