a294ec2a2b8bacc44eb1d326e2abb571ae5af820
2 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
sim
|
7dd9e59f6b |
test(#3090): stop exempting violations under categories that do not fit
An allow-test-rule annotation citing a category that does not apply is worse than no annotation, because it reads as reviewed. Eight were confirmed by reading the assertions each one covered, and auditing the rest found five more plus one refutation — a converter test whose wording described the wrong mechanism while the covered assertion genuinely was deployed-text. The instructive one used the CANONICAL string for the same mistake: STATE.md command output labelled as a deployed artifact. A canonical string is not evidence the category fits, which is why normalising strings alone would have laundered the problem rather than fixed it. Every mapping the audit had inferred rather than code-verified was spot-checked before rewriting, and the ones that turned out not to fit were re-annotated rather than relabelled. Fourteen STATE.md assertions had a typed extractor available all along and now use it; their annotations came out because nothing needs exempting. Eight assertions genuinely need a production change first — CLI stdout and stderr with no structured mode — and are tagged pending-migration-to-typed-ir citing #3090, which is what that category is for. It had zero real uses before this, while one file carried a real citation to migration issue #2974 under a non-canonical tag. Six annotations covered assertions that do no text matching at all. An exemption for a violation that does not exist is noise that makes the real ones harder to audit; those are removed. atomic-write-coverage gains the annotation it always warranted — its own docstring describes a structural-regression-guard while the file carried none. Fifty-nine non-canonical strings across roughly thirty files are normalised, and the allow-test-rule allowlist is regenerated to match. 472 annotations became 463: every one now uses a canonical category, and the two remaining non-canonical strings are ESLint RuleTester fixtures, not annotations. Refs #3057 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> |
||
|
Tom Boucher
|
60bc6ddd6e |
fix(#2568): commit the debug session doc on the manager-driven terminal path (#2731)
* test(#2568): failing-first contract for the debug manager's doc commit agents/gsd-debug-session-manager.md contains zero occurrences of 'commit', so on the manager-driven path — the normal /gsd-debug flow — nothing consults commit_docs and session docs are left untracked. The step exists only in gsd-debugger.md, which does not reach the end of a multi-cycle session. Two layers. The spec placement is asserted against the shipped agent text because that text is what the orchestrator executes; the commit_docs gate the fix relies on is exercised behaviorally through the CLI in temp git projects, because 'the CLI no-ops when disabled' is the assumption that makes calling it unconditionally correct — asserting it in prose would be assuming the load-bearing part. The most important case is the negative: CONTINUE_REQUIRED is non-terminal and must NOT commit. A fix that satisfied the positive cases by committing unconditionally would strand a half-finished session looking done, which is worse than the bug. RED expected on the five spec assertions; the three CLI gate tests pin existing behavior and pass both sides. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DPq9ovaovP2UvSVLjD4Lso * fix(#2568): commit the debug session doc on the manager-driven terminal path agents/gsd-debug-session-manager.md contained zero occurrences of "commit", so on the normal /gsd-debug flow nothing ever consulted commit_docs and session docs were left untracked — reproduced by the reporter across three consecutive real sessions. An obligation left behind during a responsibility move. gsd-debugger.md commits the doc at :1214, but that runs only when the debugger carries a fix to completion inside a single spawn. The multi-cycle manager took over checkpointing, fix application, archival to resolved/, and the terminal summary — every step that finishes a session — without the commit step. The debugger's copy still exists and still works on its own path, so "is this handled anywhere" answered yes. The manager now commits before both terminal shapes, and explicitly NOT on CONTINUE_REQUIRED. That exclusion is the load-bearing part: the manager's own contract already forbids fabricating a terminal summary on that path, and committing there would be the same lie in git form — a half-finished session stranded looking done. CHECKPOINT REACHED likewise does not commit. Two facts kept the fix small. The manager already carries the gsd_run preamble (:96, used at :97 for resolve-model), so no new plumbing. And cmdCommit (src/commands.cts:809-814) already gates on commit_docs and returns skipped_commit_docs_false when disabled — so the agent calls it unconditionally and correctness follows from the CLI rather than from a second copy of the config check that could drift. Calling git commit directly was rejected for exactly that reason: it would bypass a user's explicit commit_docs:false. In-session fix code is staged by specific file, never git add -A, which would sweep unrelated working-tree changes into a debug commit. Body-only edit — frontmatter untouched, so the research-profiles/AGENTS.md ripple does not apply. gsd-debugger.md's own commit step is preserved; the single-spawn path still ends there, and a double commit is harmless since the second finds nothing to stage. Fixes #2568 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DPq9ovaovP2UvSVLjD4Lso * fix(#2568): bind the commit paths, define gsd_run in-block, and make the fix commit idempotent Three defects in my own first cut, all found by the isolated adversarial pass. 1. BLOCKER — {debug_dir} was a dangling substitution. I copied it verbatim from the issue's suggested patch without checking scope. It is a real variable in gsd-core/workflows/debug.md, where the ORCHESTRATOR receives it from init JSON and uses it to build debug_file_path — but this agent never receives it: <session_parameters> declares only slug, debug_file_path, symptoms_prefilled, tdd_mode, goal and specialist_dispatch_enabled. An unbound token makes --files resolve to a nonexistent path; cmdCommit skips missing explicit files, staging stays empty, and the doc silently never commits — reproducing #2568 through a different broken path. Same class as #2684's dangling placeholders. Now spelled literally as .planning/debug/resolved/{slug}.md, matching gsd-debugger.md:1214 and this file's own prose. 2. MAJOR — gsd_run was undefined in the commit block's shell. Shell state does not persist across tool invocations, and the Step 2 preamble is ~230 lines and an entire spawn-and-loop earlier. gsd-debugger.md redeclares the full preamble immediately before each of its own call sites; the commit block now does the same, byte-identical to this file's existing definition. 3. MAJOR — the in-session fix commit was not idempotent. gsd-debugger.md's archive_session already commits the fix on the confirmed-checkpoint path, which is the standard find_and_fix flow, so `git add X && git commit` would hit an empty diff, exit non-zero, and abort the step before the summary was returned. Guarded with `git diff --cached --quiet || git commit`. The doc-commit half was already safe — query commit treats an empty diff as nothing_to_commit and exits 0 — and that asymmetry is now stated rather than assumed. Three tests added for exactly these, since the reviewer correctly noted the suite would have caught none of them: every token substituted into a commit command must be a declared session parameter; the preamble must sit in the same block as the call with no step boundary between; and the fix commit must carry the staged-content guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DPq9ovaovP2UvSVLjD4Lso * fix(#2568): keep the single canonical gsd_run preamble The previous commit added a second preamble to the commit block, on the isolated review's theory that shell state does not persist across tool invocations. That theory is reasonable in general and wrong for this corpus: the repo enforces "each agent .md using gsd_run contains exactly ONE canonical preamble, before the first gsd_run call". Adding a second broke that invariant and five suites with it — the B-agents preamble check, runtime-launcher-parity, and three slash- namespace guards. The single Step 2 preamble already precedes the commit call, so coverage was never actually missing. Reverted to one, and the test now asserts the real property — exactly one preamble, positioned before the call that needs it — rather than the locality I had wrongly encoded. Recorded because the direction of the mistake matters: the review was right that the question needed asking and wrong about the answer, and I shipped the wrong answer without checking the invariant that already governs it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DPq9ovaovP2UvSVLjD4Lso * fix(#2568): use the canonical /gsd:debug slash form in the new prose My explanatory sentence wrote `/gsd-debug`, the retired syntax. Three slash-namespace guards caught it: the #3443 invariant, the #1975 folded bug-2543 check, and the retired-syntax scan over Claude-facing source. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DPq9ovaovP2UvSVLjD4Lso * chore(#2568): backfill changeset PR number Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DPq9ovaovP2UvSVLjD4Lso --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |