Repository navigation
refactor(agents): port unattended review bounds and reporting to Go #770
Description
Problem
packages/agents/source/lib/agent.sh (173 lines) holds the unattended review round bound, lint command selection, the configuration digest and the report transport for the agent workflows. A rejected digest or a mail failure handled wrongly can reset the bound or hide a write status.
Part of #588.
Acceptance criteria
- AC1: A rejected digest is retained across preflight, and the finding counter survives it.
- AC2: A mail failure prints the unsent report and keeps the status of the write that already happened.
- AC3: The bound and report cases in the agent workflow suites pass against the Go path.
- AC4: Generated skill output is regenerated in the same PR.
Scope and non-goals
Port the bounded unattended state to Go: the persistent finding counter, lint selection, the configuration digest and report delivery. Keep the counter format so state written by an older binary still reads. 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 bounded-state contract. Blocked by the context discovery port (#755) and the stamp port (#769), which supply the context and the review-state format. 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 retained-digest and mail-failure cases first, then port the Bats 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 — behavioral coverage and verification need concrete contracts
Checked
tbhb-dev/devtools. Read refactor(agents): port unattended review bounds and reporting to Go #770’s live body, labels (none), and every comment (none). There are no earlier NOT READY comments.origin/mainand Replace complex shell with tested Go, starting with gates and merging #588. Moving these algorithms intoagentrepo, retaining a frozen compatibility fallback, and regenerating outputs matches the Go-port direction. No unnecessary guard, configuration knob, or layer is proposed.packages/agents/source/lib/agent.shis not one of the specified owner-tier paths. It is nevertheless explicitly golden and owner-protected by.agents/agent-skills.conf:4and.agents/agent-skills.conf:9.Gaps
Blocker — acceptance criteria do not cover the scoped behavior. AC3 describes tests passing rather than an observable outcome. Lint selection and configuration-digest behavior have no corresponding criteria, despite being in scope (
packages/agents/source/lib/agent.sh:137,packages/agents/source/lib/agent.sh:149). State the behavior to preserve, including configuration failures. Make the compatibility and refusal promises testable: older state remains readable, an absent command uses the frozen path, and a supported command’s failure or partial output refuses without fallback. These are relevant compatibility and enforcement criteria, not additional features.Blocker — the Verify plan cites IDs without specifying proof. “Write its failing case first” and “same fixtures as AC1” do not establish the assertions, workflow coverage, or generation check. Specify:
tests/skills/redesign.bash:14.tests/skills/agent-pr/publish.bats:36andtests/skills/agent-merge-pr/lifecycle.bats:28.mise run check-generated-skills; digest and mail fixtures cannot prove generated output is current.Add ID-linked checks for the missing criteria from gap 1.
Blocker — dependencies remain prose-only. Record refactor(agents): port skill capability and worktree context discovery to Go #755 and refactor(agents): port review snapshots and stamps to Go #769 as native blocked-by links. Their open states agree with the body; the missing links are the discrepancy.
Blocker — gate-review authority is unstated. “Operator authority: None” can describe the absence of a requested privileged edit, but it omits the existing owner-review requirement for the named gate library. State the applicable exact-head owner review requirement without implying that this issue grants authority. No enumerated owner-tier file edit is currently proposed.
Split
No split needed. The existing bounds, lint selection, digest, and reporting form a plausible S-sized migration in one PR after the prerequisites. Refinement should clarify that boundary.
Definition of done
These boxes assess planning coverage, not completed work. An unchecked item is not automatically another readiness blocker.
check-leaks.shwith the private-strings lists, and the leak review of every post. — No coverage in the ACs or Verify plan.docs/, CHANGELOG or help text are updated, or the PR's Verification says "No doc impact" and why. — No documentation assessment is planned.provenance:agent-assistedon the PR. — No coverage in the ACs or Verify plan.Status:comment names the merge commit. — No coverage in the ACs or Verify plan.Round: 1 of 3