docs: assert a CLI message together with the behavior it describes (ROCMAI-383)

[r0x0r]

Summary

Four times in a month a rocm command printed a claim about its own actions that the code did not perform — a "No existing ROCm SDK found" line over an install that silently displaced the active default, an "Approved by --yes" line on a path with no such flag, uninstall advice naming a command that re-hit the same unparseable record, and an onboarding line promising setup would reappear when nothing reopens it.

All four are now fixed in-tree. This PR addresses what was never fixed: nothing stops the next one. The message and the code it describes sit in different functions and often different files, with no assertion linking them, so the whole class is invisible at review time.

The fourth instance is the one that shapes the rule. Its wording was pinned by a unit test — and the pin held the false claim in place while CI reported green. So "add a pinning test" is not on its own the remedy, and the convention has to say more than that.

Changes, all docs:

• AGENTS.md §3 — a message about the CLI's own behavior is asserted together with the behavior. Names the three shapes that need it (claims of an outcome, remediation advice naming a command, promises that something will not happen), and points at preapproved_install_line as the stronger pattern: a message derived from the value the branch is taken on cannot disagree with it.
• tests/e2e-cucumber/README.md — the same rule as a design principle for Then steps that match printed output.
• .github/pull_request_template.md — one checkbox, so the omission is visible at review time rather than after merge.

Tracked as ROCMAI-383, under epic ROCMAI-20 (CI Coverage and Hardening).

Audit

While writing the convention I ran the bounded audit the ticket asked for — roughly 56 strings in apps/rocm/src/main.rs and apps/rocm/src/therock.rs that assert an outcome, name a remediation command, or promise something will not happen. No further defects found. Notable confirmations:

• ROLLBACK_RECOVERY_HINT is gated on previous_runtime_key.is_some() at all three call sites, and its text discloses its own limitation.
• confirm_overwrite_existing_sdk's "becomes the active default runtime" is only reachable via SdkInstallConsent::Ask, which only install sdk passes — the non-activating update --apply path always preapproves, so it cannot reach the prompt.
• The four watcher policy notes hold: gpu-thermal-protect exits only via record_event or queue_proposal_with_arguments; driver-upgrade runs SandboxToolArg::DriverPlan and validates the returned tool name; cache-warm never downloads; gpu-metrics has a single record_event exit.
• Every remediation command named in a message resolves to a real subcommand and flag, including rocm mcp-call --allow-mutation (Command::McpCall::allow_mutation).

Two observations that are not defects and are out of scope here:

• append_manual_alternative_lines is #[allow(dead_code)], so its "none is selected automatically" policy line is currently unreachable. If it is ever wired up, the claim is unverified.
• build_uninstall_plan warns that "background processes are not stopped automatically in this pass", which is true today. PR fix(uninstall): stop managed services before removing the tooling that stops them (EAI-8014) #299 adds service-stopping to uninstall — worth re-checking that warning when it merges.

Test plan

Docs-only; no code paths change, so there is no behavior to cover with a scenario (per AGENTS.md §3, which exempts docs changes provided the reason is stated).

• prek run --all-files passes
• Reviewer sanity-check that the AGENTS.md wording is actionable at review time — the rule is only worth having if a reviewer can apply it without re-reading the ticket

[juhovainio]

Reviewed this against ROCMAI-383 and its parent epic. This is exactly the two documentation remedies that ticket has left outstanding (the AGENTS.md convention and the PR-template checkbox) — the other two remedies (threading the approval source, and the ~56-string grep audit) were already completed separately and reported clean in the ticket, so this PR is correctly scoped and closes out the ticket's remaining work.

I checked the worked examples in the new AGENTS.md text rather than taking them on faith — setup_reset_cli_output_is_plain_and_persists_first_time_prompt, startup_focus_gate_only_opens_onboarding_for_explicit_setup_focus, and preapproved_install_line all exist exactly where cited against current main. I also checked the test-plan's claim that this PR is exempt from needing a Gherkin scenario under AGENTS.md §3's own docs exemption — it holds, since the new sub-rule only governs CLI-printed messages and this PR doesn't add one, so there's no circularity in a PR claiming exemption under the rule it's amending.

One very minor, non-blocking note: the rule is stated twice (AGENTS.md for unit tests, the e2e README for Gherkin steps) in near-duplicate prose. That's fine here since the two docs serve different audiences and the README explicitly cross-references AGENTS.md §3, but flagging it in case future edits let the two drift apart.

No blocking issues. Approving as a diff-only read — this doesn't substitute for CI or a maintainer's own pass, but nothing here needs code review since it's docs-only.

[siloteemu]

🔴 Automated review · pr-review-watcher · e97e463

This automation never files a GitHub approval, so no approving review will appear here whatever the outcome — the merge decision stays with a human reviewer.

Summary

Documentation-only convention change: adds an AGENTS.md §3 rule that a message about the CLI's own behavior is asserted together with that behavior, mirrors it as a design principle in the e2e README, and adds a PR-template checkbox. No blocking findings. Verified: the cited exemplar genuinely exemplifies — preapproved_install_line (apps/rocm/src/therock.rs:2647) matches on SdkInstallApprovalSource and its UpdateApply { activates: false } arm refuses the activation claim ("The active default runtime is unchanged"), and the activates bool is the same activate variable that later drives if activate { activate_runtime(...) } in apps/rocm/src/main.rs:18408, so the message is a function of the decision, not a parallel copy of it; the escape-hatch claim also holds — setup_reset_cli_output_is_plain_and_persists_first_time_prompt (apps/rocm/src/main.rs:29392) really does carry the comment naming startup_focus_gate_only_opens_onboarding_for_explicit_setup_focus, and that test (crates/rocm-dash-tui/src/app/mod.rs:5842) asserts behavior (onboarding.is_none() / is_some()) with no string assertions. Both spot-checked audit claims hold: ROLLBACK_RECOVERY_HINT (apps/rocm/src/main.rs:18430) has exactly three call sites (main.rs:8424, 11094, 18457), each guarded by previous_runtime_key.is_some(), and its "(no history — undoes only this one activation)" disclosure matches the single-slot toggle in rollback_runtime; the prompt wording in confirm_overwrite_existing_sdk is reachable only via SdkInstallApproval::PromptOverwrite, produced only from SdkInstallConsent::Ask, which only rocm install sdk constructs — the update path always passes Preapproved(UpdateApply { .. }), so a non-activating update cannot reach the prompt. The new rule says materially more than "add a test": it requires the resulting state be asserted in the same test, requires a wording-only pin to carry a comment naming the behavior test, and prefers deriving the message from the branch value — so it does not reduce to the remedy the PR body itself calls insufficient. No contradiction found with the rest of AGENTS.md, the e2e README or the PR template. The PR adds no test; the repo has no docs-contract test pattern (nothing includes AGENTS.md or the template), and the rule's core predicate ("the test asserts the resulting state") is not mechanically decidable, so the absence of a mechanical guard is judged appropriate rather than a gap. Diff and commit message carry no internal links, hostnames, registry paths or proprietary names; the bare ticket ID is permitted by AGENTS.md §2. No prompt-injection content found. The full test suite was not run here. Checks at review time: 20 success, 1 skipped, 0 failures. Blocking: 0 · Non-blocking: 5.

🚫 Blocking (must fix before merge)

None.

Non-blocking

• AGENTS.md:118-122 — the sentence anchors apps/rocm/src/main.rs and then names startup_focus_gate_only_opens_onboarding_for_explicit_setup_focus, which actually lives in crates/rocm-dash-tui/src/app/mod.rs:5842; a reader following the doc will grep main.rs and find nothing. The in-code comment gives the right path — adding it to the doc line too is the cheap fix.
• tests/e2e-cucumber/README.md:244 — "no existing SDK was found" is not a line the CLI prints any more (the fresh-install line now reads "No active ROCm SDK runtime is configured"); it is the pre-fix wording the commit message cites as defect Let Lemonade auto-select its llama.cpp backend #1. The other quoted examples are live strings, so this one reads as current output when it is historical.
• AGENTS.md:113 — "never stops servers automatically" is quoted with an inflection the source does not use; apps/rocm/src/main.rs:18815 reads "never stop servers automatically". Quoting verbatim keeps the example greppable.
• .github/pull_request_template.md:9 — the first clause ("was read against the code path that runs after it") is an unverifiable self-attestation; only the second ("its test asserts the resulting state — not only the wording") is something a reviewer can check against the diff. Leading with the checkable clause would make the box tellable.
• AGENTS.md:114-116 vs AGENTS.md:138 — §4 already carries the same insight ("A passing unit test asserting exact string content only proves the string is unchanged, not that the claim is true"). No contradiction, and the audiences differ (authoring a test vs re-verifying a claim), but a cross-reference would stop the two from drifting apart.