|
| 1 | +--- |
| 2 | +title: "CLI Integration Changes Require Strict Tests" |
| 3 | +status: accepted |
| 4 | +tags: |
| 5 | + - "hooks" |
| 6 | + - "plugin" |
| 7 | + - "rule" |
| 8 | + - "testing" |
| 9 | + - "validation" |
| 10 | +--- |
| 11 | + |
| 12 | +## Rule |
| 13 | + |
| 14 | +Any change to plugin code that invokes the bundled `archcore` CLI — directly or through `bin/archcore` — MUST be accompanied by tests that assert the exact subcommand and arguments invoked. A passing test that did not verify *which* CLI subcommand ran does not satisfy this rule. |
| 15 | + |
| 16 | +In particular: |
| 17 | + |
| 18 | +1. Every shell-out from a `bin/*` script to the launcher (`"$LAUNCHER"` / `"$SCRIPT_DIR/archcore"`) MUST be covered by a unit test that asserts the invoked subcommand via the `MOCK_ARCHCORE_LOG` mechanism (see `mock_archcore_logging` in `test/helpers/common.bash`). |
| 19 | +2. Every `args` array in `.mcp.json` and `.codex.mcp.json`, and every subcommand in any new `hooks/*.json`-referenced script, MUST be covered by the allowlist guard in `test/structure/cli-contract.bats`. |
| 20 | +3. Every prescriptive `` `archcore <subcmd>` `` reference in `README.md` MUST be guarded by `test/structure/readme-cli-references.bats`. |
| 21 | +4. Skill or agent prose that instructs the agent to run `archcore <subcmd>` as a shell command MUST be reviewed against the canonical CLI surface and either pinned by an additional structure test or rewritten to delegate through the launcher (preferred). |
| 22 | + |
| 23 | +A change is "covered" only when the test would fail if the code regressed to a phantom subcommand. |
| 24 | + |
| 25 | +## Rationale |
| 26 | + |
| 27 | +A real bug shipped because no test caught it: `bin/validate-archcore` invoked `archcore validate`, which is not a CLI subcommand (the canonical surface is `config | doctor | help | hooks | init | mcp | status | update`). The launcher returned exit 1 on every PostToolUse mutation, but the hook wraps the call in `|| true` and uses `timeout 2`, so production silently logged nothing while the test suite reported green — `mock_archcore` returned canned output regardless of the subcommand. |
| 28 | + |
| 29 | +This is a structural class of failure, not a one-off: |
| 30 | + |
| 31 | +- Hook scripts run with short timeouts and `|| true` error suppression. A wrong subcommand fails silently in production. |
| 32 | +- A test that asserts only `assert_success` is satisfied by the silent failure. |
| 33 | +- README and design docs that reference `archcore validate` look correct because the *string* is plausible and the CLI never prevented anyone from typing it. |
| 34 | + |
| 35 | +Locking the contract at the test layer closes this gap so it cannot return through inattention or a CLI version bump. |
| 36 | + |
| 37 | +The why-now: the bug was caught manually in Codex CLI, where hook output is more visible. We do not want to depend on accidental visibility for a contract this important. |
| 38 | + |
| 39 | +## Examples |
| 40 | + |
| 41 | +### Good |
| 42 | + |
| 43 | +```bash |
| 44 | +# Unit test (test/unit/validate-archcore.bats): |
| 45 | +@test "validate-archcore calls archcore doctor (not validate)" { |
| 46 | + export MOCK_ARCHCORE_LOG="$BATS_TEST_TMPDIR/archcore.log" |
| 47 | + mock_archcore_logging "All checks passed ✓" |
| 48 | + run_with_fixture validate-archcore claude-code/mcp-create.json |
| 49 | + assert_success |
| 50 | + grep -qx 'doctor' "$MOCK_ARCHCORE_LOG" \ |
| 51 | + || fail "expected 'doctor', got: $(cat "$MOCK_ARCHCORE_LOG")" |
| 52 | + ! grep -qx 'validate' "$MOCK_ARCHCORE_LOG" \ |
| 53 | + || fail "phantom subcommand 'validate' was invoked" |
| 54 | +} |
| 55 | +``` |
| 56 | + |
| 57 | +```bash |
| 58 | +# Structure test (test/structure/cli-contract.bats): |
| 59 | +ARCHCORE_SUBCOMMANDS="config doctor help hooks init mcp status update" |
| 60 | + |
| 61 | +@test "bin/validate-archcore invokes only allowlisted subcommands" { |
| 62 | + local sub |
| 63 | + for sub in $(grep -oE '"\$LAUNCHER"[[:space:]]+[a-z][a-z0-9-]*' \ |
| 64 | + "$PLUGIN_ROOT/bin/validate-archcore" \ |
| 65 | + | sed -E 's/^"\$LAUNCHER"[[:space:]]+//'); do |
| 66 | + case " $ARCHCORE_SUBCOMMANDS " in |
| 67 | + *" $sub "*) ;; |
| 68 | + *) fail "phantom subcommand '$sub'" ;; |
| 69 | + esac |
| 70 | + done |
| 71 | +} |
| 72 | +``` |
| 73 | + |
| 74 | +### Bad |
| 75 | + |
| 76 | +```bash |
| 77 | +# Mock that swallows any input — phantom subcommand passes silently. |
| 78 | +mock_archcore "All checks passed ✓" |
| 79 | +run_with_fixture validate-archcore claude-code/mcp-create.json |
| 80 | +assert_success # <-- meaningless; even `archcore unicorn` would pass |
| 81 | +``` |
| 82 | + |
| 83 | +```bash |
| 84 | +# Asserting only on the script's stdout. The launcher returns 1, the |
| 85 | +# hook swallows the error, the script prints nothing. Test passes. |
| 86 | +run_with_fixture validate-archcore claude-code/mcp-create.json |
| 87 | +assert_success |
| 88 | +assert_output "" |
| 89 | +``` |
| 90 | + |
| 91 | +```markdown |
| 92 | +<!-- README.md prose without a guarding test --> |
| 93 | +- **Validation** — runs `archcore some-future-name` after every document mutation |
| 94 | +``` |
| 95 | + |
| 96 | +## Enforcement |
| 97 | + |
| 98 | +The rule is enforced by these tests, which already ship in the plugin: |
| 99 | + |
| 100 | +- **`test/structure/cli-contract.bats`** — the allowlist guard. Scans `bin/*` scripts, `.mcp.json`, `.codex.mcp.json`, and `hooks/*.json`-referenced scripts; fails if any subcommand passed to the launcher is not in the canonical surface. Also ships a sentinel that fails on any executable reference to the historical phantoms `archcore validate` and `archcore sync`. |
| 101 | +- **`test/structure/readme-cli-references.bats`** — every code-quoted `` `archcore <subcmd>` `` in `README.md` must be allowlisted. |
| 102 | +- **`test/unit/validate-archcore.bats`** and **`test/unit/session-start.bats`** — invocation-log assertions using `MOCK_ARCHCORE_LOG`. Pattern is documented in `plugin-testing.guide.md` step 7. |
| 103 | +- **Live cross-check** — `cli-contract.bats` parses `archcore --help` when the launcher resolves the binary and fails when the hardcoded allowlist drifts from the live surface. |
| 104 | + |
| 105 | +When `bin/CLI_VERSION` bumps, the live cross-check signals which subcommands changed; the hardcoded allowlist must be updated in lockstep, and any new subcommand the plugin starts invoking gets its own invocation-log assertion before merge. |
| 106 | + |
| 107 | +A change that does not satisfy this rule is rejected in code review. The rule applies to: |
| 108 | + |
| 109 | +- Any script under `bin/` that calls `"$LAUNCHER"` or `archcore` |
| 110 | +- `.mcp.json` and `.codex.mcp.json` `args` arrays |
| 111 | +- Any new hook config (`hooks/*.json`) referencing a CLI-invoking script |
| 112 | +- README and other user-facing prescriptive docs naming `` `archcore <subcmd>` `` invocations |
| 113 | +- Skill or agent prompt text that instructs the agent to run an `archcore` shell command |
0 commit comments