Repository navigation
refactor(agents): port review snapshots and stamps to Go #769
Description
Problem
The review snapshot directories and stamp read, write, verify and clear functions in packages/agents/source/lib/lib.sh bind a review to a draft, tree, head and configuration. They are shell, and a masked failure while computing a digest can let a stale stamp pass.
Part of #588.
Acceptance criteria
- AC1: Matching output with a failed digest status does not produce a stamp.
- AC2: Moving the draft, the staged tree,
HEADor the trusted policy ref invalidates the stamp. - AC3: The stamp and freshness cases in
tests/skills/source/lib/lib.batsandmasked-status.batspass against the Go path. - AC4: Generated skill output is regenerated in the same PR.
Scope and non-goals
Port snapshot creation and the stamp lifecycle to Go, keeping the stamp format so stamps written by an older binary still verify. Probe for the subcommand first. An older binary uses the frozen shell path, and a supported command that fails refuses without falling back.
Non-goal: changing any behaviour a consumer on an older pinned binary relies on, or removing the shell fallback, which a separate XS issue owns.
Approach
Port the logic to tested Go behind an agentrepo subcommand. The shell caller probes for it without writes. An older binary that lacks it keeps the frozen shell path. A supported command that fails or returns partial output refuses and never falls back. Regenerate skill output from the canonical sources in the same pull request.
Dependencies
one persistent-state contract. Blocked by the context discovery port (#755) and the trusted policy port (#763), which supply the bindings. Touches a gate path.
Size
S. One pull request, as the dependencies above describe.
Verify plan
- AC1: write its failing case first, then show it pass. Write the failed-digest case first and watch it fail, then port the stamp cases.
- AC2: write its failing case first, then show it pass, using the same fixtures as AC1.
- AC3: write its failing case first, then show it pass, using the same fixtures as AC1.
- AC4: write its failing case first, then show it pass, using the same fixtures as AC1.
Risks
A fallback that ran after a supported command failed would mask an enforcement error, and a looser port would weaken a gate. The failing-first cases pin both. Undo: revert the change, which restores the shell path.
Operator authority
None.
Activity
tbhb-agent commentedon Oct 31, 2026
Refinement: NOT READY — stamp freshness and compatibility need explicit verification
Checked
tbhb-dev/devtools. Reviewed againstorigin/mainat0754f8a84190.Gaps
Blocker — AC2 leaves policy freshness ambiguous. “Moving … the trusted policy ref” could mean changing its authority name or advancing its commit. Current behavior preserves stamps across unrelated trusted commits and retains the starting audit commit (
tests/skills/source/lib/lib.bats:510); authority changes invalidate them (tests/skills/source/lib/lib.bats:629). State these distinct outcomes explicitly, including invalidation for changed policy content/status/authority and repository configuration. The latter already binds unattended stamps (packages/agents/source/lib/lib.sh:722).Blocker — Verify references every AC but does not prove each one. Reusing AC1 fixtures does not establish staged-tree/HEAD invalidation, all relevant stamp lifecycle cases, or generated-output consistency. Give AC2 its mutation matrix, AC3 the targeted suites and evidence that they exercise Go, and AC4 generation/token/deployment steps plus
mise run check-generated-skills. AC3 currently describes executing tests rather than an observable product outcome; express the preserved lifecycle behavior as acceptance criteria and put suite execution in Verify.Blocker — compatibility and refusal promises lack acceptance coverage. The scope promises old-format stamp verification, unchanged older-binary behavior, and refusal without fallback after supported-command failure or partial output. Add observable criteria and corresponding fixtures for those promises. Cover failed validation as well as failed creation: the existing matching-digest failure case rejects an already-written stamp (
tests/skills/source/lib/masked-status.bats:63), while AC1 only prohibits producing one. These are relevant compatibility and gate-integrity requirements, not optional expansion.Blocker — operator authority is understated. None of the issue’s explicitly named modification paths belongs to the enumerated owner-tier categories such as CODEOWNERS, configuration, or workflow files. Nevertheless, the canonical library and generated library copies match this repository’s golden, constitutional, and owner-path patterns (
.agents/agent-skills.conf:4,.agents/agent-skills.conf:5,.agents/agent-skills.conf:9). Replace “None” with the applicable exact-head owner-review requirement, while granting no write authority.Readiness bookkeeping — record dependencies and delivery coverage. Add native blocked-by links for refactor(agents): port skill capability and worktree context discovery to Go #755 and refactor(agents): port trusted review policy and prompt assembly to Go #763. Identify the claimed fallback-removal follow-up, and add the missing Definition of done verification commitments below.
Split
No split required. Keep snapshot creation and stamp lifecycle together; fallback removal remains separate. Clarify the boundary with #763 so policy freshness has one consistent contract.
Definition of done
Planning coverage only; these boxes do not report completed implementation.
No doc impactincludes a reason — absent.Assisted-by; PR hasprovenance:agent-assisted— absent.Status:comment names the merge commit — absent.Round: 1 of 3