Repository navigation
refactor(agents): port confirmation and delegation resolution to Go #762
Description
Problem
Confirmation, pre-approval and delegation resolution live in packages/agents/source/lib/lib.sh, with a read-only scope lookup in .devtools/scripts/preapproval.sh (48 lines). The order in which authorities answer decides whether a commit, PR or merge may write, and it is shell today.
Part of #588.
Acceptance criteria
- AC1: Given no authority answering, when an action needs confirmation, then it holds with status 75, prints the hold message on standard error, and nothing is written.
- AC2: Given a pre-approval lookup that fails or finds no grant, when confirmation resolves, then the delegation check is still consulted exactly as the shell path does: a delegation exit 0 grants (standard output names the delegation), exit 1 refuses with status 1 and the refusal on standard error, and any other exit or a missing checker holds with status 75. A failed lookup alone never grants.
- AC3: Given an explicit confirmation, a session pre-approval and a delegation check that answer differently, when confirmation resolves, then the first source in that order decides and standard output names it, with status, standard output and standard error equal to the shell path's for each of: explicit only, pre-approval only, delegation only, explicit over pre-approval, pre-approval over delegation.
- AC4: Given unattended mode and an explicit confirmation, when confirmation resolves, then it is rejected with the shell path's message and status.
- AC5: Given a pre-approval recorded for another session or another action, when confirmation resolves, then it is not honoured.
- AC6: Given the delegation check, when it is called, then the repository, the action and every forwarded argument (including a pinned head and an owner-path signal) reach the checker unchanged.
- AC7: Given a consumer on an older pinned binary that lacks the replacement, when it runs after this change, then it sees today's behaviour. Given a replacement that is present and fails, then the caller refuses with the replacement's nonzero status and diagnostic on standard error. Given a replacement that returns partial output with status 0, then the caller refuses with a nonzero status and a diagnostic, and the partial output is excluded from every downstream input. In both cases the shell path never runs and nothing runs twice.
- AC8: Generated skill output (the copies of
lib.shunder the generated skill directories) is regenerated from the canonical source in the same pull request, andmise run check-generated-skillspasses.
Scope and non-goals
Port the authority precedence: an explicit confirmation, then the session pre-approval record, then the machine delegation check, with exit statuses and messages unchanged. The biometric grant and revoke tasks stay out of scope.
Non-goal: changing the grant or revoke tasks or any behaviour a consumer on an older pinned binary relies on. Non-goal: removing the shell fallback, which is deferred to a follow-up issue filed under D9 when this lands.
Approach
Meet the acceptance criteria with failing cases first. The mechanism is the implementer's choice within those outcomes.
Dependencies
Blocked by #753 and by #755 (context discovery). Part of #588. The fallback removal is deferred to a follow-up issue filed when this lands. Touches a gate path.
Size
S. One pull request.
Verify plan
- AC1: a case with every source absent; assert status 75, the stderr message and no write.
- AC2: a failing lookup then each of delegation exit 0, 1, 2 and a missing checker; assert the grant, refusal and hold results, status, stdout and stderr separately.
- AC3: the five named precedence cases; assert status, stdout and stderr separately against the shell path.
- AC4: unattended mode with an explicit confirmation; assert the rejection message and status.
- AC5: records for another session and for another action; assert neither is honoured.
- AC6: a recording checker; assert the exact argument vector for a merge call with a pinned head and an owner-path signal.
- AC7: stub binaries for absent, present-and-failing (nonzero), and present-with-partial-output-and-status-0; assert the legacy result, then for each supported case the refusal status, the stream and text of the diagnostic, exclusion of the partial output, zero fallback and zero repeat execution.
- AC8:
mise run generate-skillsleaves no diff andmise run check-generated-skillspasses. - Definition of done: D1 each AC is verified at the merged head, with the commands and results cited in the pull request's Verification. D2 an independent review approves the exact merged head, and every finding has a recorded disposition. D3 required checks pass with no gate loosened. D4 the leak scans the repository specifies are clean, and every post made for this work (comments, descriptions) passes the per-post leak review before it is posted. D5 every commit is signed. D6 the documentation matches, or the description says
No doc impactwith the reason. D7 every commit carries anAssisted-bytrailer and the pull request carries theprovenance:agent-assistedlabel. D8 the change stays within the pull request size limits. D9 follow-up work, including deferred review findings, is filed as issues, each with a Problem section and numbered acceptance criteria and linking this issue; the pull request's Related section lists them, and any code that defers work links its issue. D10 the merge closes this issue through a closing reference, the project board shows the state Done, and a finalStatus:comment names the merge commit.
Risks
A wrong precedence could grant a write no authority approved. AC1 to AC3 pin it. Undo: revert the change.
Operator authority
A standing rule names the operator: the canonical packages/agents/source/lib/lib.sh and the generated scripts/lib.sh copies are golden, constitutional or owner paths (.agents/agent-skills.conf:4,5), so the pull request needs the owner's exact-head review at merge.
Activity
tbhb-agent commentedon Oct 26, 2026
Refinement: NOT READY — authority parity and verification need clearer requirements
Model: gpt-6.1-sol
Checked
tbhb-dev/devtools. Checkedorigin/mainatf130498bd7383fe02f8b9d21965f555866abd98c.Gaps
High — Observable authority parity is incomplete, and AC2 is ambiguous. “Exit statuses and messages unchanged” and AC3’s reference to existing tests do not fully state the required outcomes. Current behavior includes unattended rejection of explicit confirmation, session/action isolation, delegation grant/refuse/hold statuses, separate output streams, and forwarding delegation arguments (
packages/agents/source/lib/lib.sh:742,packages/agents/source/lib/lib.sh:755,packages/agents/source/lib/lib.sh:774;tests/skills/agent-merge-pr/delegation.bats:29). State those observable outcomes, including permitted side effects. Clarify whether AC2 rejects only the failed pre-approval result or terminates the entire action: today, failed pre-approval proceeds to delegation (packages/agents/source/lib/lib.sh:784). Preserve or explicitly resolve that distinction.High — The Verify plan does not prove every AC or the compatibility requirements. Reusing AC1’s no-authority fixtures cannot establish regenerated output for AC4. Replace that entry with generation consistency evidence, such as
mise run check-generated-skills, and identify the relevant authority cases for AC3 (AGENTS.md:203,tests/skills/source/lib/lib.bats:376,tests/skills/source/lib/masked-status.bats:70). Add checks proving older-binary parity and refusal without fallback after supported-command failure or partial output, asserting stdout, stderr, status and side effects separately. These are existing requirements in Scope and Approach.High — “Operator authority: None” omits required owner review. The change necessarily touches protected canonical and generated library paths. They match golden, constitutional and owner patterns (
.agents/agent-skills.conf:4,.agents/agent-skills.conf:5,.agents/agent-skills.conf:9). Name the applicable protected paths and exact-head owner-review requirement in Operator authority; retain that the issue grants no write authority. Existing policy requires exact-head approval for golden changes (AGENTS.md:68).Medium — The body prescribes design outside Operator authority. Scope and Approach require a particular subcommand, probing and retained shell-routing mechanism. Move those implementation decisions into the PR, or record an actual operator decision in Operator authority. Keep the requirements here as observable compatibility outcomes: older supported consumers continue working, and failed or incomplete supported execution refuses without using another authority path. The Go migration itself is established by Replace complex shell with tested Go, starting with gates and merging #588.
Medium — The claimed fallback-removal owner is unnamed. The non-goal says a separate XS issue owns removal, but provides no issue reference. Name that follow-up and classify it as deferred work rather than a blocker, or remove the unsupported ownership claim.
Split
No additional split is needed. Keep protection and discovery in their named predecessor issues, authority resolution here, and fallback removal in its identified follow-up.
Definition of done
The definition of done for #762 (Feature). Tick each item and give its evidence.
Boxes describe planning coverage, not completed work. Partial coverage remains unchecked.
D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link.
Evidence: refactor(agents): port confirmation and delegation resolution to Go #762 maps verification bullets to AC IDs, but does not require merged-head verification and PR evidence.
D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.
Evidence: Not covered by the live criteria or named issues.
D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.
Evidence: Replace complex shell with tested Go, starting with gates and merging #588 AC8 requires applicable checks and owner protection; refactor(agents): port confirmation and delegation resolution to Go #762 does not fully carry forward the head-specific completion requirement and says Operator authority is “None.”
D4 The leak scans come back clean: gitleaks, devtools'
check-leaks.shwith the repository's private-strings list, and the leak review of each post.Evidence: The full scan requirement is not covered.
D5 The author signed each commit, the squash included.
Evidence: Not covered.
D6 The docs match the change (README,
docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.Evidence: Neither documentation coverage nor a justified no-impact statement is required.
D7 The commits and the squash have Assisted-by trailers, and the pull request has the
provenance:agent-assistedlabel.Evidence: Not covered.
D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
Evidence: “S. One pull request” estimates size but does not cover the limit or exact-head exception.
D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue.
Evidence: Fallback removal is deferred, but its issue and the follow-up requirements are absent.
D10 The merge closes the issue through a closing reference and the board shows Done. The final
Status:comment names the merge commit.Evidence: Not covered.
Round: 1 of 3
tbhb-agent commentedon Oct 26, 2026
Refinement: NOT READY — authority failure semantics and fallback ownership remain inconsistent
Model: gpt-6.1-sol
Checked
tbhb-dev/devtools. Checkedorigin/mainatf130498bd7383fe02f8b9d21965f555866abd98c..agents/agent-skills.conf:4,.agents/agent-skills.conf:5and.agents/agent-skills.conf:9.Gaps
High — AC2 contradicts current confirmation behavior. A failed pre-approval lookup does not necessarily refuse or hold the action: confirmation proceeds to delegation, which can grant it (
packages/agents/source/lib/lib.sh:784,packages/agents/source/lib/lib.sh:790). A delegation checker’s abnormal exit is normalized to 75 rather than propagated unchanged (packages/agents/source/lib/lib.sh:765). Clarify whether AC2 rejects only the failed source’s result while allowing subsequent authority, preserving current behavior, or intentionally changes the action’s outcome. State the resulting status and streams for a failed pre-approval followed by granting delegation, and verify that case.High — Authority parity and its verification remain incomplete. AC3 covers conflicting authorities but does not clearly cover unattended rejection of explicit confirmation, session/action isolation, or delegation argument forwarding. These are observable authorization behavior, established at
packages/agents/source/lib/lib.sh:777,packages/agents/source/lib/lib.sh:747andpackages/agents/source/lib/lib.sh:764; existing cases demonstrate isolation and pinned-head/owner arguments (tests/skills/source/lib/lib.bats:382,tests/skills/agent-merge-pr/delegation.bats:17,tests/skills/agent-merge-pr/delegation.bats:29). Require parity for those outcomes and add corresponding verification. Replace the unspecified “2x2” with identifiable precedence cases, asserting status, stdout and stderr separately as AC3 requires.Medium — refactor(agents): remove the shell fallback for discovery and worktree readiness #772 does not own this fallback removal. refactor(agents): port confirmation and delegation resolution to Go #762 says refactor(agents): remove the shell fallback for discovery and worktree readiness #772 removes its fallback, but live refactor(agents): remove the shell fallback for discovery and worktree readiness #772 limits removal to refactor(agents): port skill capability and worktree context discovery to Go #755’s discovery/readiness functions and explicitly excludes fallbacks owned by other ports. Name a follow-up whose scope actually includes confirmation/delegation fallback removal, or remove the incorrect ownership claim. Keep it classified as deferred work rather than a blocker.
Medium — The abbreviated completion requirements omit several checklist obligations. Extend the Verify plan’s completion requirements to cover the post leak review, a reason accompanying “No doc impact,” follow-up issue content and PR Related/code links, and the board’s Done state. These are the specific uncovered portions of D4, D6, D9 and D10 below; no implementation design is required.
Split
No additional split is needed. Keep protection and discovery in the named predecessors, authority resolution here, and fallback retirement in a correctly scoped follow-up.
Definition of done
The definition of done for #762 (Feature). Tick each item and give its evidence.
Boxes indicate planning coverage, not completed work.
D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link.
Evidence: The Verify plan explicitly requires each AC’s merged-head verification and commands/results in the PR’s Verification.
D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.
Evidence: The Verify plan requires independent approval of that head and a recorded disposition for every finding.
D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.
Evidence: The Verify plan requires passing checks with no gate loosened; Operator authority requires exact-head owner review.
D4 The leak scans come back clean: gitleaks, devtools'
check-leaks.shwith the repository's private-strings list, and the leak review of each post.Evidence: The Verify plan requires repository-specified leak scans to be clean, but does not explicitly cover each post’s leak review.
D5 The author signed each commit, the squash included.
Evidence: The Verify plan requires every commit to be signed, encompassing the squash commit.
D6 The docs match the change (README,
docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.Evidence: Documentation matching or “No doc impact” is required, but the alternative lacks its reason and specified Verification location.
D7 The commits and the squash have Assisted-by trailers, and the pull request has the
provenance:agent-assistedlabel.Evidence: The Verify plan explicitly requires provenance trailers and the provenance label.
D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
Evidence: The Verify plan requires staying within PR size limits; it does not rely on an exception.
D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue.
Evidence: Filing follow-up issues is required, but their minimum content, PR Related listing and code links are absent. The named fallback follow-up also excludes this work.
D10 The merge closes the issue through a closing reference and the board shows Done. The final
Status:comment names the merge commit.Evidence: Issue closure and the final merge-commit comment are required; the closing reference and board Done state are not explicit.
Round: 2 of 3
tbhb-agent commentedon Oct 26, 2026
Refinement: READY — authority-resolution requirements and verification are sufficient
Model: gpt-6.1-sol
Checked
tbhb-dev/devtools. Checkedorigin/mainatf130498bd7383fe02f8b9d21965f555866abd98c.packages/agents/source/lib/lib.sh:742,packages/agents/source/lib/lib.sh:755andpackages/agents/source/lib/lib.sh:774.tests/skills/source/lib/lib.bats:376,tests/skills/source/lib/masked-status.bats:70,tests/skills/agent-merge-pr/delegation.bats:17).tools/generate-skills.sh:111..agents/agent-skills.conf:4,.agents/agent-skills.conf:5,.agents/agent-skills.conf:9).Gaps
None.
Split
No additional split is needed. Keep protection and discovery in their named predecessors, authority resolution here, and fallback removal in the required follow-up.
Definition of done
The definition of done for #762 (Feature). Tick each item and give its evidence.
Ticks indicate planning coverage, not completed work.
D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link.
Evidence: The Verify plan explicitly requires each AC’s merged-head verification and commands/results in the PR’s Verification.
D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.
Evidence: The Verify plan requires independent approval of that head and a recorded disposition for every finding; D9 covers deferred findings.
D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.
Evidence: The Verify plan requires passing checks with no gate loosened. Operator authority requires exact-head owner review.
D4 The leak scans come back clean: gitleaks, devtools'
check-leaks.shwith the repository's private-strings list, and the leak review of each post.Evidence: D4 requires clean repository-specified leak scans and a leak review before every post.
D5 The author signed each commit, the squash included.
Evidence: D5 requires every commit to be signed, encompassing the squash.
D6 The docs match the change (README,
docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.Evidence: D6 requires matching documentation or a reasoned
No doc impactstatement in the description, alongside D1’s Verification evidence requirement.D7 The commits and the squash have Assisted-by trailers, and the pull request has the
provenance:agent-assistedlabel.Evidence: D7 requires every commit’s
Assisted-bytrailer and the specified PR label.D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
Evidence: D8 requires staying within PR size limits; no exception is relied upon.
D9 The author files follow-ups and deferred findings as issues with at least a Problem and numbered AC and lists them under the Related section of the pull request. Any work left in the code links its issue.
Evidence: D9 explicitly covers issue content, deferred findings, PR Related entries and code links, including the deferred fallback removal.
D10 The merge closes the issue through a closing reference and the board shows Done. The final
Status:comment names the merge commit.Evidence: D10 explicitly requires the closing reference, board state and final merge-commit comment.
Round: 3 of 3