automaton/tasks/fix-harness-command-template/CODE_REVIEW.md

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.