Repository navigation
feat(agentrepo): add prose check-suppressions state - #1033
Conversation
tbhb-agent
added
change:feat
4e52b5b to
c38ff5f
Compare
![tbhb-dev-reviews[bot]](https://avatars.githubusercontent.com/u/288164213?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed head: c38ff5f
Review: CHANGES REQUESTED
Loosens: no
Owner-tier: no
Reviewed the exact head against the live body, labels and both Refinement comments of #886. All 25 individual checks in the single rollup read were COMPLETED/SUCCESS. GitHub reports MERGEABLE, with mergeStateStatus BLOCKED. GitHub verifies the head's SSH signature (verified=true, reason=valid). The PR and sole commit close only #886.
Acceptance criteria
| AC | Result | Evidence at reviewed head |
|---|---|---|
| AC1 | Not fully met | Normal baseline status, state path and retained bytes are asserted at internal/agentrepo/prose_test.go:324. However, internal/agentrepo/prose.go:127 resolves a relative target against the invocation directory, whereas the shell first changes to the checkout root. See R1. |
| AC2 | Met | Atomic directory creation and duplicate guidance: internal/agentrepo/prose.go:172; exact status 2 and diagnostic plus shell comparison: internal/agentrepo/prose_test.go:412. |
| AC3 | Met | Target hash names the state: internal/agentrepo/prose.go:172; distinct targets and evidence comparison: internal/agentrepo/prose_test.go:426. |
| AC4 | Not fully met | Another-target refusal is implemented at internal/agentrepo/prose.go:117 and tested against shell verify/diff at internal/agentrepo/prose_test.go:442. R1 additionally rejects the same root-relative target from a subdirectory. |
| AC5 | Met | State-root and target-file validation: internal/agentrepo/prose.go:138; shell-created fixtures, wrong root, malformed path and missing state: internal/agentrepo/prose_test.go:349 and internal/agentrepo/prose_test.go:456. |
| AC6 | Met for tested root invocation | Unified diff and status handling: internal/agentrepo/prose.go:343; changed, unchanged and missing-original cases, current and legacy roots: internal/agentrepo/prose_test.go:234 and internal/agentrepo/prose_test.go:369. |
| AC7 | Met | Named-state removal: internal/agentrepo/prose.go:80; isolation and accepted legacy state: internal/agentrepo/prose_test.go:203. |
| AC8 | Met | Workflow-state exclusion: internal/agentrepo/prose.go:324; notes-only edits, silent shell verification on Unix, and native digest equality: internal/agentrepo/prose_test.go:248. |
| AC9 | Met | Native assertions run outside the Unix-only oracle subtests: internal/agentrepo/prose_test.go:177 and internal/agentrepo/prose_test.go:184. Current-head Linux, macOS and Windows test jobs passed both coverage and race executions; their logs show internal/agentrepo passing. Windows uses recorded shell layout at internal/agentrepo/prose_test.go:107 and skips live shell comparisons explicitly. |
| AC10 | Not fully met | Paired Go/shell evidence and shell verify/diff/cleanup comparisons: internal/agentrepo/prose_test.go:266. These always invoke Go from the fixture root (internal/agentrepo/prose_test.go:97), hiding R1's divergent target identity for subdirectory invocations. |
All implementation changes are Go: internal/agentrepo/command.go:96, internal/agentrepo/prose.go:24 and internal/agentrepo/prose_test.go:184. The shell oracle and existing callers are unchanged. No new shell logic, verification implementation, sanction suffix or caller switch was added. Help text describes the three implemented operations at internal/agentrepo/prose.go:27; no other documented interface changed.
Size and rebase
Fresh scc 4.1.0 measurement on an existing checkout whose HEAD is exactly c38ff5f, with no tracked checkout differences: internal/agentrepo/prose.go contributes 340 code lines; internal/agentrepo/prose_test.go contributes 444 code lines. Their subtotal is the previously reported 784. The added registration in internal/agentrepo/command.go:96 contributes one more code line: 785 added + 0 deleted code lines, below the 800 limit. Tests count as code. There are 0 added + 0 deleted prose-file lines, below the 2,000 limit. No prose files, generated output or fixtures are changed. Raw diff size is +845/-0; 60 lines are comments or blank lines.
The patches for 4e52b5b and this head have the same stable patch ID (add09818f7affb1ef1ca8749bcfc41703df9999f), and range-diff reports equality. Their parents differ; the prose implementation and tests are byte-identical. The only direct difference among the three affected files is another command registration inherited from the new base. No feature-content change occurred in the rebase.
No changed path matches the requested owner-tier set: ownership files, agent-skills configuration, review policy, harness settings, workflows/actions, shared libraries, agent/guard scripts or merge-pr scripts.
Findings
R1 — P2: Resolve relative targets from the same checkout root as the shell (internal/agentrepo/prose.go:127). checkout() records p.root but never changes the process working directory. canonicalTarget() then uses filepath.Abs(filepath.Dir(target)), which resolves against the invocation directory. For example, from docs/, --baseline README.md records docs/README.md and can report success without retaining the root README.md; the unchanged shell changes to the repository root first and baselines README.md. Similarly, --diff STATE README.md from docs/ falsely reports a target mismatch for a state created at the root. This breaks target identity and shell interoperability depending on where the command is invoked. Resolve relative target parents against p.root before evaluating symlinks, preserving absolute targets, and add a Go/shell comparison that invokes the command from a nested directory. Existing root-only setup at internal/agentrepo/prose_test.go:97 hides this regression.
Open verification gap: D1 requires the author's PR Verification section to cite each AC ID with its command/result or a link. The current description lists a single aggregate TestProse command without an AC1-AC10 mapping. Add the required evidence when fixing R1.
Round-1 dispositions: R1 open; no earlier findings to close. No findings are deferred to an unfiled follow-up.
Over-engineering
No additional P2 found. No new configuration knob, guard script or independent layer is introduced. The state writer, watched-file digest and dictionary snapshots support the AC10 shell-state contract. The simpler correction for R1 is root-based resolution in the existing function, without a new context layer.
Gates, Coverage and CodeQL
Loosens: no. No gate, check, exemption, allowlist or existing enforcement path changes. Only tightens: no in the literal sense; this is additive state support and leaves existing gates unchanged. It satisfies the no-loosening constraint.
Re-read current-head contexts: coverage concluded SUCCESS and merged the native-platform data, but its upload annotation/log reports HTTP 403 because repository Code Quality is not enabled. The existing nonfatal upload setting is at .github/workflows/ci.yml:292 and is unchanged by this PR. A successful coverage job therefore does not prove successful publication or a passing Go coverage-floor check. CodeQL concluded SUCCESS with the explicit result "No new alerts in code changed by this pull request"; both Go and Actions analysis jobs also succeeded. No mechanical gate was rerun during this review.
Base Definition of Done
- D1: Not met. R1 leaves AC coverage incomplete, and the author's PR Verification lacks the required per-AC command/result mapping. Test entry points exist at internal/agentrepo/prose_test.go:184, internal/agentrepo/prose_test.go:234, internal/agentrepo/prose_test.go:248 and internal/agentrepo/prose_test.go:266.
- D2: Not met. This independent exact-head review requests changes; R1 remains open.
- D3: All 25 individual current-head check contexts completed successfully. No gate is loosened; see the three-file diff and the Coverage qualification above.
- D4: Current-head gitleaks and lint checks passed; the lint workflow includes the private-identifier range scan at .github/workflows/ci.yml:79. This review body passed check-leaks-with-lists before posting and receives gh-post's own scan.
- D5: Head satisfied: GitHub reports verified=true, reason=valid. Squash signing remains a merge-time obligation.
- D6: Added help matches the state-only command at internal/agentrepo/prose.go:27. Existing shell/caller documentation remains applicable; no caller switches occur.
- D7: Head satisfied: the sole commit contains Assisted-by and Signed-off-by, and the PR has provenance:agent-assisted. Squash attribution remains a merge-time obligation.
- D8: Fresh size is 785/800 changed code lines, including tests, and 0/2,000 prose-file lines; no exception is needed.
- D9: No deferred findings or TODOs introduced; R1 is requested for this PR, not deferred. Related closes only #886.
- D10: Pending merge. The closing reference is correct, but no merge commit, Done board state or final Status comment exists yet.
Repository-specific DoD: devtools-1 remains unproven for the Go floor: the existing native coverage and combined-report tasks do not invoke cover-go-check (.devtools/tasks/go.toml:200, .devtools/tasks/go.toml:228), and the PR cites no separate pass. Provide the author's exact-head floor result; this reviewer did not rerun it. The Python coverage job passed. devtools-2 is inapplicable because skill sources are unchanged. For devtools-3, the new command is unused and additive; no consumer migration is needed, but the PR should explicitly record that rationale in Verification as requested by the issue's DoD.
Round: 1 of 3
A prose workflow keeps evidence between steps, and callers can't move to the Go binary mid-run without losing it. Writing the shell script's state layout lets the shell script or the Go binary continue a workflow the other started. The change is additive. The shell script and its callers stay unchanged, which keeps it within the N-2 compatibility policy. Fixes #886 Assisted-by: claude-code:claude-sonnet-5-5 Signed-off-by: Tony Burns <tony@tonyburns.net>
c38ff5f to
fe76797
Compare
There was a problem hiding this comment.
Reviewed head: fe76797
Review: CHANGES REQUESTED
Loosens: no
Owner-tier: no
Reviewed the exact head against the live body, labels and both Refinement comments of #886, and the round-1 CHANGES_REQUESTED review at c38ff5f. The single requested check-rollup read showed all 25 individual checks COMPLETED/SUCCESS. GitHub reports MERGEABLE and mergeStateStatus BLOCKED. GitHub verifies this head's signature: verified=true, reason=valid. The sole commit and PR close only #886.
Acceptance criteria
| AC | Result | Evidence at reviewed head |
|---|---|---|
| AC1 | Met | Baseline retains the document and reports its state at internal/agentrepo/prose.go:156 and internal/agentrepo/prose.go:213; exact status, state path and original bytes at internal/agentrepo/prose_test.go:329. Root-relative identity is corrected at internal/agentrepo/prose.go:128. |
| AC2 | Met | Atomic state-directory creation and duplicate cleanup guidance at internal/agentrepo/prose.go:171; exact status 2, stderr and shell comparison at internal/agentrepo/prose_test.go:419. |
| AC3 | Met | Target hash distinguishes states at internal/agentrepo/prose.go:171; distinct target state paths and evidence comparison at internal/agentrepo/prose_test.go:433. |
| AC4 | Met | Supplied-target comparison and diagnostic at internal/agentrepo/prose.go:117; status 2 and comparison with shell diff/verify at internal/agentrepo/prose_test.go:449. The nested-directory regression at internal/agentrepo/prose_test.go:490 exercises the corrected same-target identity. |
| AC5 | Met | Current/legacy root and target-file validation at internal/agentrepo/prose.go:137; shell-created state, malformed path, wrong root and missing-state diagnostics at internal/agentrepo/prose_test.go:350 and internal/agentrepo/prose_test.go:463. |
| AC6 | Met | Original unified diff and status handling at internal/agentrepo/prose.go:342; exact normalized output, status and stderr for changed/unchanged/missing-original targets, current and legacy roots, at internal/agentrepo/prose_test.go:376. |
| AC7 | Met | Named-state removal at internal/agentrepo/prose.go:80; isolation and accepted legacy-state cleanup at internal/agentrepo/prose_test.go:208. |
| AC8 | Met | Workflow-state exclusion at internal/agentrepo/prose.go:323; notes-only edits, silent shell verification on Unix and native digest equality at internal/agentrepo/prose_test.go:253. |
| AC9 | Met | Native assertions remain outside Unix-only oracle skips: internal/agentrepo/prose_test.go:182 and internal/agentrepo/prose_test.go:189. Exact-head Linux, macOS and Windows jobs passed coverage and race executions; their logs show internal/agentrepo passing. Windows uses recorded shell fixtures at internal/agentrepo/prose_test.go:112; live shell comparisons are explicitly skipped there. |
| AC10 | Met | Paired Go/shell state bytes and shell verify/diff/cleanup outcomes at internal/agentrepo/prose_test.go:271. The added nested invocation at internal/agentrepo/prose_test.go:490 compares successful target matching with the unchanged shell on Unix. |
Behavior lives in tested Go: internal/agentrepo/command.go:96, internal/agentrepo/prose.go:24 and internal/agentrepo/prose_test.go:189. No shell logic, caller switch, verification implementation or sanction suffix was added. Help at internal/agentrepo/prose.go:27 matches the three implemented operations. Existing callers and their documentation remain applicable; the PR's Why section explains that no caller needs a newer binary, but the required explicit Verification evidence remains incomplete below.
Size
Fresh scc 4.1.0 measurement on the existing clean checkout whose HEAD is exactly fe76797: internal/agentrepo/prose.go has 338 added code lines; internal/agentrepo/prose_test.go has 460 added code lines; the diff adds one code registration at internal/agentrepo/command.go:96. 799 added + 0 deleted code lines against the 800 limit. Tests count as code. 0 added + 0 deleted prose-file lines against the 2,000 limit. No prose files, fixtures or generated output changed. Raw diff is +863/-0; the other 64 lines are comments or blank lines. The locally available scc 4.1.0 binary was used; the specified Linux binary path is unavailable on this Mac. The whole command.go file is not counted as newly added code.
No changed path is owner-tier: no CODEOWNERS, agent-skills.conf, .agents/review-instructions.md, config/claude/automode.json, .claude/settings*.json, .codex/config.toml, workflow/action, lib.sh, agent.sh, guard script or merge-pr script changes.
Amendment and round-1 dispositions
The complete git diff c38ff5f..fe76797 changes only internal/agentrepo/prose.go and internal/agentrepo/prose_test.go: +26/-8 raw lines. canonicalTarget now resolves through the existing root-based path helper before evaluating the parent; tests gain runIn and NestedDirectory. No other change rides along, and command.go is unchanged between those heads. Fresh changed-code size increases from 785 to 799, a net 14 code lines.
- R1 CLOSED: internal/agentrepo/prose.go:128 uses p.path, whose root-relative/absolute behavior is at internal/agentrepo/prose.go:265; baseline and supplied-target comparison both use canonicalTarget at internal/agentrepo/prose.go:160 and internal/agentrepo/prose.go:118. The nested-directory Go/shell status regression is at internal/agentrepo/prose_test.go:490. It directly covers the former false mismatch; the shared correction also fixes baseline resolution.
- Per-AC Verification evidence OPEN: the description still contains aggregate TestProse verification rather than the required AC1-AC10 command/result or link mapping. See R2.
- Go coverage-floor evidence OPEN: no author result for cover-go-check is present in the description or PR comments. Passing coverage contexts do not establish that floor. See R3.
- No-consumer-impact Verification rationale OPEN: the rationale exists in Why, but the required explicit declaration and explanation are still absent from Verification; include it with R2.
Findings
R2 — P2: Complete the author's required Verification evidence (PR description, Verification; test anchors internal/agentrepo/prose_test.go:189, internal/agentrepo/prose_test.go:239, internal/agentrepo/prose_test.go:253 and internal/agentrepo/prose_test.go:271). The first Refinement's D1 explicitly requires each AC ID with its command and result, or a link, in the PR's Verification section. The amended description still lists only the aggregate TestProse command and a nested-directory explanation. Add AC1-AC10 mappings, exact-head results and platform/skip evidence. Also record the requested devtools-3 declaration, "No consumer impact", with the existing unused/additive-command rationale in Verification, or provide adoption documentation if consumer steps are required. This carries forward round 1's evidence gap; the reviewer's AC table does not replace author evidence.
R3 — P2: Supply exact-head Go coverage-floor evidence (.devtools/tasks/go.toml:228; .testcoverage.yml:12). The issue's devtools-1 requires cover-go-check independently of check. Native CI invokes cover-go-binary at .github/workflows/ci.yml:216; the combined job invokes cover-go-merge at .github/workflows/ci.yml:284. Neither task enforces thresholds (.devtools/tasks/go.toml:200 and .devtools/tasks/go.toml:214). The green coverage context does not demonstrate the configured 75% file / 80% package / 85% total floors. Record the author's exact-head cover-go-check command/result or an existing verifiable run; address an actual failure if it reports one. This is missing evidence, not a claim that the floor failed. No workflow change or exemption is requested. This reviewer did not rerun any mechanical gate.
Over-engineering
No additional P2 found. No new guard, configuration knob or unrelated layer is introduced. The state writer, watched-file digest and dictionary snapshots serve AC10's shell interoperability contract; the amendment reuses the existing path helper and adds a focused regression case.
Gates, Coverage and CodeQL
Loosens: no. The complete three-file diff adds Go state support and tests without modifying, weakening, skipping or exempting any existing gate or check. The .fix-prose exclusion at internal/agentrepo/prose.go:323 preserves the existing shell rule at packages/agents/source/skills/fix-prose/shared/scripts/check-suppressions.sh:228 and implements AC8. Only tightens: no, literally; this is additive state support with existing enforcement unchanged. It satisfies the no-loosening requirement.
Re-read exact-head contexts: coverage is COMPLETED/SUCCESS and its log merges all four native-platform reports, but its annotation and upload log report HTTP 403 because repository Code Quality is not enabled. The pre-existing nonfatal upload configuration at .github/workflows/ci.yml:292 is unchanged. Coverage publication did not succeed, and this context does not prove the Go floor. The Python test log separately reports the required 100% coverage reached. CodeQL is COMPLETED/SUCCESS with "No new alerts in code changed by this pull request"; analyze (go) and analyze (actions) are also COMPLETED/SUCCESS.
Base Definition of Done
- D1: UNMET. Implementation/test evidence supports AC1-AC10 above, but the author has not supplied the required per-AC Verification mapping (R2). Merged-head confirmation remains a merge-time obligation.
- D2: UNMET. R1 is closed; R2 and R3 remain open. This independent exact-head review requests changes.
- D3: All 25 individual head checks completed successfully; the three-file Go-only diff loosens no gate. See the Coverage qualification above.
- D4: Head gitleaks and lint checks passed; lint includes the private-identifier range scan at .github/workflows/ci.yml:79. This review body passed check-leaks-with-lists before posting and receives gh-post's own per-post scan.
- D5: Head satisfied: GitHub verified=true, reason=valid for the sole commit. The squash signature is pending merge and cannot yet be certified.
- D6: Help matches the state-only interface at internal/agentrepo/prose.go:27. Existing shell/caller behavior and documentation stay applicable; no caller switches occur.
- D7: Head satisfied: the sole commit has Assisted-by and Signed-off-by, and the PR carries provenance:agent-assisted. Squash attribution is pending merge.
- D8: Fresh scc changed-code size is 799/800 including tests; prose-file size is 0/2,000. No size exception is needed.
- D9: No deferred findings or TODOs introduced. R2/R3 are requested for this PR, not deferred to an unfiled follow-up; Related closes only #886.
- D10: Pending merge. Fixes #886 is correct; a merge commit, Done board state and final Status comment cannot yet be certified.
Repository-specific DoD: devtools-1 remains UNMET for Go coverage-floor evidence (R3); Python's 100% floor passed in the current-head test log. devtools-2 is inapplicable because skill sources and generated copies are unchanged. devtools-3's explicit Verification rationale remains open under R2; the change is unused and additive, with no caller/config migration identified.
Round: 2 of 3
|
Acceptance criteria for #886 mapped to the tests and commands that cover them. Each command runs from the repository root.
The relative-target regression from the first review round is |
|
Coverage evidence at the exact head Exit status 0. |
There was a problem hiding this comment.
Reviewed head: fe76797
Review: APPROVED
Loosens: no
Owner-tier: no
R2 and R3 decisions
R2 CLOSED. I accept the author's AC1–AC10 evidence comment as the D1 evidence location for this PR under the author's agreed handling with the maintainers. Every named test exists in the exact-head diff: internal/agentrepo/prose_test.go:189, internal/agentrepo/prose_test.go:200, internal/agentrepo/prose_test.go:239, internal/agentrepo/prose_test.go:253 and internal/agentrepo/prose_test.go:271. Each explicit Go command selects the stated existing entry point; “same command” inherits the state command. AC9 names actual CI matrix jobs, whose commands are mise run cover-go-binary and mise run test-go-race at .github/workflows/ci.yml:216 and .github/workflows/ci.yml:226. The comment provides the command mapping, and the successful exact-head CI executions provide the results: Linux, macOS and Windows. Together these satisfy D1's command/result-or-link evidence requirement for this PR. Moving the map into the description would add no missing verification evidence and would require repeating publication gates. I also accept the description's “The command is additive. Callers keep using the shell script, and none needs a newer binary.” and Risk's “Compatibility: none” as the substantive no-consumer-impact declaration and rationale. No description edit is required.
R3 CLOSED. The author's exact-head coverage result, posted at 07:41:51Z and present in the timeline after 07:26Z, records exit 0 and PASS independently for file 75%, package 80% and total 85%; total measured coverage is 92.8% (3780/4072), with internal/agentrepo at 92.3%. These meet .testcoverage.yml:12. I verified both task definitions at origin/main and the reviewed head: devtools:cover-go generates coverage.out at .devtools/tasks/go.toml:167 and runs go tool go-test-coverage --config .testcoverage.yml at .devtools/tasks/go.toml:171; devtools:cover-go-check runs that identical threshold command against an existing profile at .devtools/tasks/go.toml:230. The freshly generated-profile result is accepted as equivalent threshold evidence. No floor is below 75/80/85. The green Coverage context alone would not establish this.
The head is identical to round 2; git diff fe76797652eddb1a54ad7990139dc03f56d3e82e fe76797652eddb1a54ad7990139dc03f56d3e82e is empty. R1 remains closed: root-relative canonicalization at internal/agentrepo/prose.go:128 and the regression at internal/agentrepo/prose_test.go:490 are unchanged. No mechanical gates were rerun.
AC table
| AC | Result | Exact-head evidence |
|---|---|---|
| AC1 | Met | Baseline retains the original and reports its state: internal/agentrepo/prose.go:156, internal/agentrepo/prose.go:213; exact exit 0, stdout and original bytes: internal/agentrepo/prose_test.go:329. |
| AC2 | Met | Atomic directory creation rejects duplicate state with cleanup guidance: internal/agentrepo/prose.go:171; exact exit 2, diagnostic and shell comparison: internal/agentrepo/prose_test.go:419. |
| AC3 | Met | Target-hashed state identity: internal/agentrepo/prose.go:171; distinct state paths and shell evidence equality: internal/agentrepo/prose_test.go:433. |
| AC4 | Met | Supplied-target comparison and mismatch diagnostic: internal/agentrepo/prose.go:117; exact exit 2 and shell diff/verify comparison: internal/agentrepo/prose_test.go:449. Root-relative regression: internal/agentrepo/prose_test.go:490. |
| AC5 | Met | Current/legacy state validation: internal/agentrepo/prose.go:137; unchanged-shell-created state, malformed paths, wrong root and missing state: internal/agentrepo/prose_test.go:350 and internal/agentrepo/prose_test.go:463. |
| AC6 | Met | Original unified diff and status handling: internal/agentrepo/prose.go:342; changed, unchanged and missing-original cases across current/legacy roots assert both streams and status: internal/agentrepo/prose_test.go:376. |
| AC7 | Met | Only the supplied state is removed: internal/agentrepo/prose.go:80; cleanup isolation, including legacy state: internal/agentrepo/prose_test.go:208. |
| AC8 | Met | Workflow-state exclusion: internal/agentrepo/prose.go:323; note-only edits preserve silent shell verification on Unix and native digest equality: internal/agentrepo/prose_test.go:253. |
| AC9 | Met | Native assertions precede or surround the Unix-only oracle skips: internal/agentrepo/prose_test.go:182 and internal/agentrepo/prose_test.go:189. The linked Linux/macOS/Windows coverage and race logs all report internal/agentrepo passing. Windows uses recorded shell fixtures at internal/agentrepo/prose_test.go:112 and skips live shell comparisons; Linux/macOS execute them. |
| AC10 | Met | Paired Go/shell baseline bytes and unchanged-shell verify, diff and cleanup compare status, both streams and filesystem effects: internal/agentrepo/prose_test.go:271. |
Size
Fresh scc 4.1.0 measurement on the existing clean checkout at the exact head: internal/agentrepo/prose.go has 338 code lines and internal/agentrepo/prose_test.go has 460; internal/agentrepo/command.go:96 adds one code registration. 799 added + 0 deleted code lines, below 800; 0 added + 0 deleted prose-file lines, within 2,000. Tests count as code. Raw diff is +863/-0; the remaining 64 lines are blank or comments. No fixtures or generated files changed. The installed macOS scc 4.1.0 binary was used because the specified Linux executable path is unavailable here.
Findings
No open findings. R1 remains closed; R2 and R3 are closed above. No deferred implementation items or undisclosed follow-up requirements.
Loosens: no. The complete diff changes only internal/agentrepo/command.go, internal/agentrepo/prose.go and internal/agentrepo/prose_test.go. It adds unused Go state operations and tests without changing existing enforcement, callers, thresholds or exemptions. The exclusion at internal/agentrepo/prose.go:323 preserves the unchanged shell rule at packages/agents/source/skills/fix-prose/shared/scripts/check-suppressions.sh:228. Test-only oracle skips at internal/agentrepo/prose_test.go:182 retain native Windows assertions and do not disable an existing check.
Owner-tier: no. None of those three paths changes CODEOWNERS, agent-skills.conf, review-instructions.md, harness settings, workflows/actions, shared libraries, guards or merge authority. The owner/constitutional path patterns at .agents/agent-skills.conf:5 and .agents/agent-skills.conf:9 do not match them.
The single required check-rollup read showed all 25 checks COMPLETED/SUCCESS. GitHub reports MERGEABLE, with mergeStateStatus BLOCKED at that read. GitHub commit verification reports verified=true, reason=valid for the sole head commit.
CodeQL: SUCCESS, with “No new alerts in code changed by this pull request”; both analyze jobs also succeeded. Coverage: the context succeeded but publication failed with HTTP 403 because repository Code Quality is not enabled. The unchanged .github/workflows/ci.yml:292 makes that upload nonfatal; CI's binary/merge coverage tasks do not enforce these floors. R3 is satisfied by the separate author's threshold result, not by the failed upload. Python CI reports 100% coverage, meeting its configured floor.
Over-engineering
No P2 found. State writing at internal/agentrepo/prose.go:183, dictionary snapshots at internal/agentrepo/prose.go:236 and watched-file digests at internal/agentrepo/prose.go:292 implement the existing shell evidence contract required by AC10. Omitting them would break interoperability. Root-relative resolution reuses the path helper at internal/agentrepo/prose.go:265. No simpler form preserves the required contract with fewer responsibilities; no extra configuration or speculative abstraction is introduced.
DoD
- D1: Accepted author AC evidence map plus linked exact-head CI results, as decided above. Confirmation against the eventual merged commit remains a merge-time responsibility.
- D2: This independent review approves the exact head; all three findings have explicit closed dispositions.
- D3: All 25 checks completed successfully; no gate is loosened.
- D4: Head gitleaks and lint checks passed; lint includes the private-identifier range scan at .github/workflows/ci.yml:79. This body passes the required private-list scan before publication and the posting wrapper's per-post review.
- D5: Head commit is signed and verified. The future squash signature remains to be verified at merge.
- D6: Help at internal/agentrepo/prose.go:27 matches the three implemented operations. Existing shell callers and their documentation remain applicable; no caller switches occur.
- D7: Head has Assisted-by and Signed-off-by trailers; the PR has provenance:agent-assisted. The future squash attribution remains a merge-time obligation.
- D8: Fresh measured size is 799/800 code and 0/2,000 prose; no exception needed.
- D9: No deferred findings, new TODOs or unfiled follow-ups. Related correctly says Fixes #886.
- D10: Pending merge: verify the closing reference, Done board state and final Status comment naming the merge commit. These future artifacts cannot yet be certified.
Repository-specific DoD: devtools-1 is satisfied by successful CI plus the author's equivalent Go threshold run and Python's 100% coverage result. devtools-2 is inapplicable: no skill source or generated output changed. devtools-3 is satisfied by the accepted additive/no-new-binary compatibility rationale. Pending merge artifacts above do not require a code or evidence amendment before approval.
Round: 3 of 3
Summary
agentrepo prose check-suppressionsadds baseline, diff and cleanup operations for prose workflow evidence. The state layout matches what the shell script writes. The unchanged shell accepts state the Go command creates, and the Go command accepts state the shell created.Why
The suppression check keeps its state handling in shell, and the Go migration needs the same evidence available from Go before any caller changes. The command is additive. Callers keep using the shell script, and none needs a newer binary.
Verification
go test ./internal/agentrepo -run '^TestProse' -count=1passes. The tests run the unchanged shell script and the Go command on the same fixtures and compare status, both output streams, and filesystem effects.TestProseState/NestedDirectoryruns a relative target from a subdirectory and checks it resolves at the checkout root like the shell does.go vet ./internal/agentrepoandGOOS=windows go vet ./internal/agentrepopass.mise run lint-go,mise run lint-proseand cspell on the changed files pass.Risk
A mistake in state identity could clean another workflow's evidence or admit overlapping writers. No caller uses the command yet, so reverting this change removes it and leaves the shell path as before.
Compatibility: none
Related
Fixes #886