Skip to content

build(agents): protect replacement Go enforcement paths #753

Description

@tbhb-agent

Problem

The ports in #588 replace shell enforcement with Go commands. The shell scripts that enforce merge policy sit under protected path patterns in .agents/agent-skills.conf, so a change to them needs owner review. The Go packages that will replace them have no comparable scrutiny yet, so a later change could loosen enforcement without a reviewer being told to look for it at the exact head.

Part of #588.

Acceptance criteria

  1. AC1: A pull request that changes any file in a covered replacement package (the enforcement packages named in Scope and the file that registers agentrepo subcommands) is classified as changing a golden file. The existing golden-file waiver then applies unchanged: the required reviewer's approval on the exact head that states Loosens: no lets a golden-only change merge with no owner approval, a missing or duplicated Loosens answer refuses, and Loosens: yes reaches the operator. None of these paths joins owner_paths or constitutional_paths.
  2. AC2: Protection is preserved and bounded. Every path matched by the existing golden_paths, constitutional_paths and owner_paths patterns before this change is still matched afterwards, and none gains or loses a match. The deliberately unprotected cases stay unprotected, and pure helper packages with no enforcement role are excluded, as the existing negative cases at tests/skills/agent-merge-pr/policy.bats:354,380 already require. The stale-head, deletion and rename checks apply the heightened-review requirement to the new paths.
  3. AC3: Rollout order is provable. build(agents): protect replacement Go enforcement paths #753 merges before any pull request that switches a caller to a Go enforcement replacement. Each such port issue lists build(agents): protect replacement Go enforcement paths #753 under its own Dependencies, and its pull request cites the merged build(agents): protect replacement Go enforcement paths #753 commit as evidence. A port that is not yet filed acquires the prerequisite by listing build(agents): protect replacement Go enforcement paths #753 under Dependencies, which refinement checks.

Scope and non-goals

Covered: the packages and cmd entry points that hold replacement enforcement (merge policy, gates, signing, review stamps, confirmation, delegation), and the file that registers agentrepo subcommands, because registering a replacement is itself a change to what runs. The coverage boundary is exactly those paths. Pure helpers with no enforcement role (formatting, path utilities) stay outside it.

Non-goal: a Go port. This issue ports nothing and adds no subcommand, probe or fallback. Non-goal: loosening or reordering any existing pattern.

Approach

Meet AC1 to AC3. The pattern edits and the order of the test work are the implementer's choice.

Dependencies

Blocks the enforcement ports that follow it: #755, #756 and the later ports in #588 that move enforcement into Go. #754 does not move enforcement and is not blocked by this issue. No blockers of its own. Refinement also reconciles the owner-protection wording in #588 and #755 with this issue's golden-only classification.

Size

XS. One pull request.

Verify plan

  • AC1: for each covered path class, a policy case that changes a file there and expects the golden-file finding. One case per class shows a missing Loosens answer refusing, Loosens: no from the required reviewer passing with no owner approval, and Loosens: yes reaching the operator, with no owner-review label in any of them.
  • AC2: a case that runs a fixed list of paths matched by each existing pattern before and after and compares the results; the negative cases for excluded helpers and unprotected paths; and a stale-head, deletion and rename case for a new path, each keeping the heightened-review requirement.
  • AC1 (registration and construction): include one case per covered path class and one for the registration file; mise run test-hooks tests/skills/agent-merge-pr shows each case failing against the old patterns and passing against the new ones.
  • AC3: at refinement, refactor(agents): port skill capability and worktree context discovery to Go #755 and refactor(agents): port the gh-post requirement adapter to Go #756 list build(agents): protect replacement Go enforcement paths #753 as a blocker in their own bodies. At merge of each port, its description cites the merged build(agents): protect replacement Go enforcement paths #753 commit.
  • 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 impact with the reason. D7 every commit carries an Assisted-by trailer and the pull request carries the provenance:agent-assisted label. 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 final Status: comment names the merge commit.

Risks

A pattern that is too broad flags unrelated changes for heightened review. A pattern that is too narrow leaves an enforcement path unprotected. The per-path cases pin both. Undo: revert the commit, which returns to the current patterns.

Operator authority

A standing rule names the operator: the single edit to .agents/agent-skills.conf needs the owner's exact-head review. Adding the new paths means adding them to golden_paths in .agents/agent-skills.conf, and that file is itself golden, constitutional and an owner path today. This one pull request therefore needs that review at merge. No mechanism avoids it, because the pattern lists are read from that file. The edit only tightens, grants no write authority, and is the only owner request in this work: later changes to the covered Go paths take the agent-waivable golden route.

Activity

  1. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: NOT READY — scope, protection semantics, and verification need clarification
    Model: gpt-6.1-sol

    Checked

    Read live #753, its comments (none), and named issue #588. Used an independent clone and verified origin identifies tbhb-dev/devtools. Compared repository claims against origin/main at f130498bd7383fe02f8b9d21965f555866abd98c.

    • AC1: Observable in principle, but “each protected replacement package” leaves the coverage boundary undefined. Define the enforcement responsibilities covered, including dispatcher registration, without prescribing package layout.
    • AC2: States a useful compatibility outcome. Its verification needs to retain the existing positive and negative protection cases.
    • AC3: Specifies test maintenance rather than a single observable product outcome. Put the named suite and case requirements in Verify; state the protection behavior as the criterion.
    • Parity: The configuration-only scope does not replace a shell algorithm, so a full shell-to-Go exit/output contract is unnecessary if the unrelated port text is removed. Existing protection behavior and exceptions still need an explicit preservation contract.
    • Repository evidence: Current protection patterns name shell enforcement and configuration paths, without Go replacement coverage (.agents/agent-skills.conf:4, .agents/agent-skills.conf:5, .agents/agent-skills.conf:9). Dispatcher registration exists at internal/agentrepo/command.go:85.

    Gaps

    1. High — The body describes two different changes. Scope and Dependencies say configuration extension only; Approach prescribes a Go subcommand, caller probing, fallback behavior, and generated output. Risks and Undo describe that port rather than protection configuration. Remove those port requirements and align Risks and Undo with protection changes. Under the requirements-only rule, command structure and generation design belong in the pull request.

    2. High — AC1 does not resolve the existing owner-approval exception. Current policy can waive golden-only findings after a qualifying exact-head App review (packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:576). Constitutional findings prevent that golden-only waiver (packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:523). Specify whether replacement enforcement always requires owner approval, as AC1 says, or retains the existing exception. Preserve the intended policy tier of each migrated responsibility; merely matching a golden pattern does not prove unconditional owner approval.

    3. High — The coverage boundary and rollout condition are not verifiable. Identify which enforcement responsibilities this issue protects and make protection before caller activation an explicit acceptance outcome. “No blockers” is credible for advance protection, but the body does not establish how coverage remains complete as replacement responsibilities arrive. This needs a behavioral boundary, not a prescribed directory structure.

    4. High — Operator authority is incorrect. “None” conflicts with Dependencies, Replace complex shell with tested Go, starting with gates and merging #588’s owner-review requirement, and the existing protection of the configuration itself (.agents/agent-skills.conf:4, .agents/agent-skills.conf:5). State that the protection update requires the existing exact-head owner approval process. This grants no implementation or publication authority.

    5. Medium — Verify does not prove every AC. “Failing case first” alone does not establish expected results, and AC2 should already pass before adding protection. Map AC1 to missing approval, stale-head approval, qualifying current-head approval, and the chosen exception semantics. Map AC2 to all retained protected and deliberately unprotected cases, including deletion and rename behavior. Existing examples are at tests/skills/agent-merge-pr/policy.bats:109, tests/skills/agent-merge-pr/policy.bats:124, tests/skills/agent-merge-pr/policy.bats:220, and tests/skills/agent-merge-pr/policy.bats:346. Include the dispatcher and rollout outcome explicitly.

    6. Medium — Completion requirements lack planning coverage. Neither build(agents): protect replacement Go enforcement paths #753 nor named Replace complex shell with tested Go, starting with gates and merging #588 explicitly covers D1–D10 below. Add a completion contract covering these obligations, including AC-indexed merged-head evidence and exact-head review. The current size estimate and generic gate references cover only fragments.

    Split

    XS, one PR is reasonable after narrowing this to protection configuration and focused regression coverage. Keep Go implementation, caller switching, and fallback removal in their separate issues. No further split is needed for the configuration-only piece.

    Definition of done

    The definition of done for #753 (Feature). Tick each item and give its evidence.

    Boxes indicate planning coverage, not completed work. None is fully covered by the reviewed issue text.

    • 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: Verify names AC IDs but omits merged-head verification and the required 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: No complete requirement in build(agents): protect replacement Go enforcement paths #753 or Replace complex shell with tested Go, starting with gates and merging #588.
    • 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 checks; the complete exact-head condition is absent.
    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post.
      Evidence: The complete scan and post-review requirement is absent.
    • D5 The author signed each commit, the squash included.
      Evidence: No complete requirement in build(agents): protect replacement Go enforcement paths #753 or Replace complex shell with tested Go, starting with gates and merging #588.
    • 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 verification nor the alternative is stated.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.
      Evidence: No complete requirement in build(agents): protect replacement Go enforcement paths #753 or Replace complex shell with tested Go, starting with gates and merging #588.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
      Evidence: “XS” estimates size but does not cover the limits or 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: No complete follow-up requirement is stated.
    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.
      Evidence: No closure, board, or final-status requirement is stated.

    Round: 1 of 3

  2. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: NOT READY — rollout evidence, regression coverage, and completion contract remain incomplete
    Model: gpt-6.1-sol

    Checked

    Read live #753, its Round 1 comment once, and named issues #588, #754, #755 and #756. Verified the independent clone’s origin identifies tbhb-dev/devtools. Repository references below describe origin/main at f130498bd7383fe02f8b9d21965f555866abd98c.

    Round 1 findings:

    1. Partly resolved — conflicting scope and implementation prescriptions. “Non-goal: a Go port” removes the unrelated subcommand, fallback and generation work. Risks and undo now concern protection changes. However, Approach still dictates pattern editing and test order, and AC3 prescribes a particular test file and case structure outside Operator authority.
    2. Resolved — protection tier and exception. “The existing waiver for golden-only changes applies” explicitly preserves the exception, and AC1 excludes constitutional classification. This matches golden-only waiver evaluation at packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:576; constitutional findings are distinct at packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:523.
    3. Partly resolved — coverage boundary and rollout. Scope now identifies enforcement responsibilities, dispatcher registration and excluded helpers. AC4 explicitly requires protection “before any caller switches”. Its verification still does not establish that ordering or complete dependency coverage.
    4. Resolved — Operator authority. “The owner's exact-head review at merge” correctly recognizes protection of the configuration itself. It is both golden and constitutional at .agents/agent-skills.conf:4 and .agents/agent-skills.conf:5. No write authority is granted.
    5. Partly resolved — verification. AC1 now names the golden finding, owner requirement and waiver outcome. AC2 adds a before/after comparison, but only for previously matching paths. Retained negative cases and stale-head approval behavior remain absent from the plan.
    6. Unresolved — completion requirements. The live Verify plan still ends with AC-specific checks and dependency inspection. Neither build(agents): protect replacement Go enforcement paths #753 nor the named issues supplies the complete D1–D10 completion contract.

    The configuration currently lacks replacement Go protection (.agents/agent-skills.conf:4, .agents/agent-skills.conf:9). Dispatcher registration is an existing enforcement surface at internal/agentrepo/command.go:85.

    Gaps

    1. Medium — Requirements still prescribe implementation. Move the pattern-editing procedure, test order, and AC3’s named test-file/case structure into the eventual pull request. Keep observable requirements in the issue: enforcement changes receive the chosen protection, dispatcher changes are included, and unrelated helpers remain outside it. Verification may name the existing suite without making its internal organization an acceptance criterion.

    2. High — AC4’s proposed check does not prove rollout ordering. A dependency reference in a future port PR description does not establish that build(agents): protect replacement Go enforcement paths #753 merged before caller activation. The proposed refinement-time check also cannot inspect PR descriptions that do not yet exist.

      Require evidence that build(agents): protect replacement Go enforcement paths #753 is merged before a dependent caller-switch change lands, and check blocker references in the dependent issues themselves. refactor(agents): port skill capability and worktree context discovery to Go #755 and refactor(agents): port the gh-post requirement adapter to Go #756 currently name build(agents): protect replacement Go enforcement paths #753 as a blocker. docs(inventory): refresh the shell migration inventory and owners #754 describes inventory work, says it has no blockers, and is incorrectly included among enforcement ports. Correct that dependency classification and state how later enforcement ports acquire the prerequisite.

    3. Medium — Verification still omits material protection boundaries. AC2’s fixed list of previously matched paths covers positive examples only. Scope excludes helpers, and Risks claims the cases constrain excessive matching, but no negative check is planned. Retain existing deliberately unprotected cases and add representative excluded-helper checks; existing positive and negative assertions are at tests/skills/agent-merge-pr/policy.bats:346 and tests/skills/agent-merge-pr/policy.bats:397.

      Explicitly retain the existing missing/current-owner, stale-head and deletion/rename checks rather than requiring duplicate tests: tests/skills/agent-merge-pr/policy.bats:109, tests/skills/agent-merge-pr/policy.bats:124, and tests/skills/agent-merge-pr/policy.bats:220. Include evidence that new protection supplies the intended owner-path signal as well as the golden finding; those outputs are computed separately at packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:508 and packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:528.

    4. Medium — D1–D10 still lack complete planning coverage. Add a completion contract covering the checklist below. AC-indexed checks, an XS estimate and owner approval cover portions of it, but do not establish merged-head evidence, full independent review, signing, leak checks, provenance, documentation disposition, follow-up handling or closure reporting.

    Split

    XS, one PR remains reasonable for protection configuration and focused regression coverage. No further split is needed. Go ports and caller activation remain in their separate issues.

    Definition of done

    The definition of done for #753 (Feature). Tick each item and give its evidence.

    Boxes indicate planning coverage, not completed work. No item is fully covered by the live issue or its named issues.

    • 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: Verify maps AC IDs to checks but omits merged-head verification and PR evidence requirements.

    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.

      Evidence: Operator authority requires exact-head owner review, but does not cover independent review and finding disposition.

    • 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 checks; the complete merged-head and gate-approval condition is absent.

    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post.

      Evidence: No complete scan and post-review requirement appears.

    • D5 The author signed each commit, the squash included.

      Evidence: No commit-signing completion requirement appears.

    • 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 verification nor its stated alternative appears.

    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.

      Evidence: Neither provenance requirement appears.

    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.

      Evidence: XS estimates scope but does not require compliance with size limits or the 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: No complete deferred-finding and follow-up contract appears.

    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.

      Evidence: No closing-reference, board-state or final-status requirement appears.

    Round: 2 of 3

  3. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: NOT READY — completion coverage and AC3 still need refinement
    Model: gpt-6.1-sol

    Checked

    Read live #753, its Round 1 comment once, and named issues #588, #754, #755 and #756. Verified the independent clone’s origin identifies tbhb-dev/devtools. Repository evidence describes origin/main at f130498bd7383fe02f8b9d21965f555866abd98c.

    Round 1 findings:

    1. Partly resolved — conflicting scope and implementation prescriptions. “Non-goal: a Go port” removes the unrelated implementation work. Approach leaves implementation choices open, and Risks concerns protection changes. AC3 still makes test construction an acceptance requirement; see Gap 1.
    2. Resolved — protection tier and exception. “The existing waiver for golden-only changes applies” settles the exception, and AC1 excludes constitutional classification. These are distinct policy outcomes at packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:523 and packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:576.
    3. Resolved — coverage boundary and rollout. Scope identifies enforcement responsibilities, registration and excluded helpers. “build(agents): protect replacement Go enforcement paths #753 merges before any pull request that switches a caller” establishes ordering. AC4 requires blocker references and merged-commit evidence; refactor(agents): port skill capability and worktree context discovery to Go #755 and refactor(agents): port the gh-post requirement adapter to Go #756 name build(agents): protect replacement Go enforcement paths #753 as a blocker, while docs(inventory): refresh the shell migration inventory and owners #754 is correctly excluded.
    4. Resolved — Operator authority. “The owner's exact-head review at merge” correctly recognizes protection of the configuration itself at .agents/agent-skills.conf:4 and .agents/agent-skills.conf:5. It grants no write authority.
    5. Resolved — behavioral verification. Verify now covers golden classification, owner requirements, the waiver, retained pattern matches, negative cases, stale approval, deletion, rename, registration and rollout. Existing approval and path-boundary evidence is at tests/skills/agent-merge-pr/policy.bats:109, tests/skills/agent-merge-pr/policy.bats:124, tests/skills/agent-merge-pr/policy.bats:220 and tests/skills/agent-merge-pr/policy.bats:346. The targeted command accepts the specified directory at mise.toml:590.
    6. Partly resolved — completion requirements. “The independent review approves the exact merged head” and the new completion paragraph add substantial coverage, but several explicit D1–D10 obligations remain unstated; see Gap 2.

    The current patterns lack replacement Go coverage at .agents/agent-skills.conf:4 and .agents/agent-skills.conf:9. Registration exists at internal/agentrepo/command.go:85. No additional scope, dependency or authority defect was found.

    Gaps

    1. Medium — AC3 prescribes test construction rather than a product outcome. Move the per-protection cases, separate registration case and red/green comparison into Verify. AC1 already requires protection of both replacement packages and registration; retain that observable behavior without prescribing how tests are organized.

    2. Medium — The completion paragraph does not fully cover the requested definition of done. Complete the contract with:

      • AC-indexed evidence verified on the merged head, using commands and results or links.
      • Disposition of every independent-review finding.
      • The named leak scans and review of each post.
      • Explicit Assisted-by trailers and the provenance label.
      • A reason when declaring no documentation impact.
      • Follow-up issue content, Related references and links from deferred code.
      • Closing reference, board Done state and final Status: comment naming the merge commit.

      These are remaining parts of the Round 1 completion finding, not additional implementation requirements.

    Split

    XS, one PR remains reasonable for protection configuration and focused regression coverage. No further split is needed. Go implementation and caller activation remain in their separate issues.

    Definition of done

    The definition of done for #753 (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: AC-indexed PR evidence is required, but merged-head verification and the evidence format are unspecified.

    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.

      Evidence: Exact merged-head independent approval is required; disposition of every finding is not.

    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.

      Evidence: The completion contract requires passing checks and prohibits gate loosening, alongside exact merged-head review.

    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post.

      Evidence: Clean leak scans are required generically; the named scans and post reviews are absent.

    • D5 The author signed each commit, the squash included.

      Evidence: The completion contract 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 a no-impact statement is required, but the explanation is absent.

    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.

      Evidence: Recording provenance does not explicitly require either artifact.

    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.

      Evidence: The completion contract requires staying within PR size limits.

    • 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; their contents, deferred-finding coverage and links are unspecified.

    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.

      Evidence: Merge closure is required; the closing reference, board state and final status evidence are unspecified.

    Round: 3 of 3

  4. added theissue type on Oct 26, 2026
  5. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: NOT READY — AC3 and follow-up completion coverage remain incomplete
    Model: gpt-6.1-sol

    Checked

    Read live #753, its Round 1 comment once, and named issues #588, #754, #755 and #756. Verified the independent clone’s origin identifies tbhb-dev/devtools. Repository evidence below refers to origin/main at f130498bd7383fe02f8b9d21965f555866abd98c.

    Round 1 findings:

    1. Partly resolved — conflicting scope and implementation prescriptions. “Non-goal: a Go port” removes the unrelated port work. Approach leaves implementation choices open, and Risks concerns protection changes. AC3 still prescribes test construction; see Gap 1.
    2. Resolved — protection tier and exception. “The existing waiver for golden-only changes applies” preserves the exception, and AC1 excludes constitutional classification. These outcomes are distinct at packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:523 and packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:576.
    3. Resolved — coverage boundary and rollout. “build(agents): protect replacement Go enforcement paths #753 merges before any pull request that switches a caller” establishes ordering. Scope identifies enforcement responsibilities, registration and excluded helpers. AC4 requires dependent-issue blockers and merged-commit evidence. refactor(agents): port skill capability and worktree context discovery to Go #755 and refactor(agents): port the gh-post requirement adapter to Go #756 list build(agents): protect replacement Go enforcement paths #753 under Dependencies; docs(inventory): refresh the shell migration inventory and owners #754 correctly remains unblocked.
    4. Resolved — Operator authority. “The owner's exact-head review at merge” correctly recognizes the configuration’s existing protection at .agents/agent-skills.conf:4 and .agents/agent-skills.conf:5. The issue grants no write authority.
    5. Resolved — behavioral verification. “Missing-owner, current-owner, stale-head, deletion and rename” checks now accompany positive and negative path coverage, waiver behavior, registration and rollout evidence. Relevant existing cases appear at tests/skills/agent-merge-pr/policy.bats:109, tests/skills/agent-merge-pr/policy.bats:124, tests/skills/agent-merge-pr/policy.bats:220 and tests/skills/agent-merge-pr/policy.bats:346. The targeted suite accepts a directory argument at mise.toml:590.
    6. Partly resolved — completion requirements. “D1 each AC is verified at the merged head” begins a substantial completion contract. Follow-up requirements and the final board state remain incomplete; see Gap 2.

    Current patterns lack replacement Go coverage at .agents/agent-skills.conf:4 and .agents/agent-skills.conf:9. Dispatcher registration exists at internal/agentrepo/command.go:85. No additional dependency, size or authority defect was found.

    Gaps

    1. Medium — AC3 prescribes test construction rather than an observable protection outcome. Move its policy-case coverage and old-pattern/new-pattern comparison into Verify, which already contains those checks. AC1 supplies the observable protection requirement, including registration. Keep acceptance criteria about behavior without prescribing the test structure.

    2. Medium — The completion contract omits parts of D9 and D10. D9 requires follow-up issues stating scope and linking build(agents): protect replacement Go enforcement paths #753, but does not require a Problem and numbered AC, listing under the PR’s Related section, or issue links from deferred code. Add those obligations and explicitly include deferred findings. D10 requires the board’s final status without identifying Done; specify that state.

    Split

    XS, one PR is reasonable for protection configuration and focused regression coverage. No further split is needed. Go implementation and caller activation remain separate work.

    Definition of done

    The definition of done for #753 (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 completion contract explicitly requires merged-head verification and commands/results cited by AC 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 contract requires independent approval of the exact merged 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 completion contract requires passing checks and prohibits gate loosening.

    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post.

      Evidence: The contract requires the repository-specified scans and per-post review. Those scans include both commands at packages/agents/source/skills/pr/shared/scripts/create-pr.sh:189; the scanner consumes private-string variables at .devtools/scripts/check-leaks.sh:114.

    • 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 impact statement in the PR description.

    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.

      Evidence: D7 explicitly requires the trailer on every commit 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: D8 requires staying within PR size limits.

    • 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: Follow-up issues are required, but their minimum contents, Related listing and deferred-code links 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: The closing reference and final comment are required; the board state is described only as final status.

    Round: 1 of 3

  6. tbhb-agent commented on Oct 26, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: READY — protection scope, rollout, and completion requirements are covered
    Model: gpt-6.1-sol

    Checked

    Read live #753, its Round 1 findings, and named issues #588, #754, #755 and #756. Verified the independent clone’s origin identifies tbhb-dev/devtools. Repository evidence refers to origin/main at f130498bd7383fe02f8b9d21965f555866abd98c.

    Round 1 findings:

    1. Resolved — conflicting scope and implementation prescriptions. “Non-goal: a Go port” confines the change to protection. Approach leaves implementation choices open; test construction now appears in Verify.
    2. Resolved — protection tier and exception. “The existing waiver for golden-only changes applies” preserves the exception, and AC1 excludes constitutional classification. These are distinct outcomes at packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:523 and packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:576.
    3. Resolved — coverage boundary and rollout. Scope names enforcement responsibilities, registration and excluded helpers. “build(agents): protect replacement Go enforcement paths #753 merges before any pull request that switches a caller” establishes ordering. AC3 requires dependency references and merged-commit evidence. Both refactor(agents): port skill capability and worktree context discovery to Go #755 and refactor(agents): port the gh-post requirement adapter to Go #756 list build(agents): protect replacement Go enforcement paths #753 under Dependencies; docs(inventory): refresh the shell migration inventory and owners #754 remains outside enforcement rollout.
    4. Resolved — Operator authority. “The owner's exact-head review at merge” correctly recognizes protection of the configuration at .agents/agent-skills.conf:4 and .agents/agent-skills.conf:5. It grants no write authority.
    5. Resolved — verification coverage. “Missing-owner, current-owner, stale-head, deletion and rename” checks accompany classification, waiver, retained matches, negative cases and registration coverage. Existing behavioral evidence appears at tests/skills/agent-merge-pr/policy.bats:109, tests/skills/agent-merge-pr/policy.bats:124, tests/skills/agent-merge-pr/policy.bats:220 and tests/skills/agent-merge-pr/policy.bats:346. Verify maps AC3 to dependent-issue and port-merge evidence.
    6. Resolved — completion requirements. “Definition of done” now explicitly covers D1–D10, including follow-up issue contents, Related references, deferred-code links and board state Done.

    Current patterns lack replacement Go coverage at .agents/agent-skills.conf:4 and .agents/agent-skills.conf:9. Dispatcher registration exists at internal/agentrepo/command.go:85. The proposed verification covers every AC, and no additional dependency, authority or requirements-only defect remains.

    Gaps

    None.

    Split

    XS, one PR is reasonable for protection configuration and focused regression coverage. No further split is needed. Go ports and caller activation remain separate work.

    Definition of done

    The definition of done for #753 (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 completion contract requires merged-head verification and commands/results cited by AC 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: D2 requires independent approval of the exact merged head and disposition of 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 completion contract requires passing checks and prohibits gate loosening.

    • D4 The leak scans come back clean: gitleaks, devtools' check-leaks.sh with the repository's private-strings list, and the leak review of each post.

      Evidence: D4 requires repository-specified scans and per-post review. Publication invokes leak lint and gitleaks at packages/agents/source/skills/pr/shared/scripts/create-pr.sh:190; private-string inputs are documented at .devtools/scripts/check-leaks.sh:114.

    • 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 impact statement in the PR description.

    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.

      Evidence: D7 explicitly requires the trailer on every commit 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: D8 requires staying within PR size limits.

    • 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 requires each obligation, including deferred review findings and links from deferred code.

    • 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 Done and final comment naming the merge commit.

    Round: 2 of 3

  7. 43 remaining items

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    refinement:readyPassed the Definition of Ready; may be picked up

    Type

    Fields

    Priority

    None yet

    Work type

    None yet

    Projects

    • Status
      In progress

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions