Skip to content

Run the skill-directory integrity gate in the merge entry points #982

Description

@tbhb-agent

Problem

A clean checkout whose committed merge scripts differ from the pinned base's apm.lock.yaml runs those scripts as they are, and no Go code on main can say so. The merge group in internal/agentrepo/merge_approval.go holds only saved-approval evaluation; the merge transaction is still the shell in squash-merge.sh and merge-agent.sh, and main has no integrity subcommand in merge-policy.py or anywhere else.

The #981 library compares a skill directory with the base lock. This issue adds the part that library leaves out: a private copy of the base's skill directories that helpers can run from, removed on every exit path, and agentrepo merge integrity, the one command the gate pass and the final check each call. It supersedes the wiring half of the oversized open PR #600 (the other halves are #980 and #981) as AC3 of #499. Part of #324, within #588.

Ownership across #980, #981 and #982, identical in all three bodies:

Concern Owner Others
Pinned-base settings reader and agentrepo config repository --base #980 #864 and #865 call it for the gates and squash lint, #872 for cleanup's disposable_ignored; each gains #980 as a native blocker and a divergent-checkout AC
Pinned-binary selection and refusal (no pinned agentrepo, or below the floor, exits 2 with a reason) #1010, a native blocker of #874, #875 and #876; its logic is tested Go with thin forwarding shims, and its lib.sh change needs owner exact-head review after #753, a Compatibility: deprecates declaration and a migration note None of #980, #981 and #982 selects a binary
apm.lock.yaml parser, internal/mergepolicy/lock.go, including dependency, deployment and active_owner fields #981 #859 is blocked by #981, consumes the parser and keeps only the ownership layer and its command
Expected map, directory walk and comparison library #981 Tested directly; no production caller until #982
Base snapshot, agentrepo merge integrity and snapshot cleanup on every exit path #982 #864 (first gate pass) and #870 (final check) call the command, run helpers from the snapshot and remove it
Consumer contracts and existing package callers internal/mergepolicy.Approved and its caller in internal/agentrepo/merge_approval.go stay as they are; the new APIs gain callers only in #859, #982, #864 and #870 #864 and #870 each gain #982 as a blocker and an AC for integrity refusal, snapshot use and cleanup; live head and base re-reads stay with them
Post-merge cleanup read of disposable_ignored (squash-merge.sh:372) #872 Calls the #980 reader
Retirement of PR #600 Closed as superseded, unmerged, once the pull requests for #980, #981 and #982 have all merged; it is not rebased and no replacement takes code from it The note on #600 cites those three; #499 stays open for the binary-refusal issue, its AC5 documentation follow-up and the caller issues
Documentation of merge authority and settings (#499 AC5) A follow-up issue filed by #982 under its D9 duty Not a blocker for any of the three

Acceptance criteria

  1. AC1: pull request and base oid resolution Dropped: the pull request reader is The merge scripts read pull request snapshots, reviews and the local diff through one Go reader #862 and the transaction that passes the oid on is The unattended merge gates run in Go #864 and The merge gates and confirmation chain run in Go before the write #870, so this command takes the base oid as input.
  2. AC2: Given a base oid, when a snapshot is made, then a directory only its owner can read holds the base blobs of the merge-pr and agent-merge-pr skill directories byte for byte, with the executable bit kept on 100755 entries.
  3. AC3: Given a merge-pr or agent-merge-pr directory with a file that differs from the base lock or an extra file, when agentrepo merge integrity --base <oid> runs, then it exits 1 with a message that begins skill directory integrity: and names the path, which no other refusal uses.
  4. AC4: Given a committed file swapped between two runs of the command with the same oid, when the second run happens, then it refuses with the same message as a fresh first run would.
  5. AC5: Given a difference in only the merge-pr directory, or only in the agent-merge-pr directory, when the command runs, then it refuses in both cases.
  6. AC6: Given a pass, a refusal or a failure part-way through the snapshot, when the command or snapshot user returns, then the snapshot directory is gone and the exit status, stdout and stderr are what they would be without cleanup.
  7. AC7: Given directories that match the base lock, when the command runs, then it exits 0 and prints nothing.
  8. AC8: entry scripts as one exec line and deletion of replaced shell Dropped: the wrapper cutovers are The lint, populate-description, squash-message and review scripts become wrappers over agentrepo merge #874, The squash-merge and merge scripts become wrappers over agentrepo merge #875 and The preflight scripts become wrappers over agentrepo merge #876, fallback retirement is The wrappers stop falling back to the frozen shell after the N-2 window #882 and deletion is The frozen entry-point bodies of five merge-pr scripts are removed #883 and The frozen squash-merge body and pull-request.sh are removed #884.
  9. AC9: Non-functional: with Python and uv both absent from PATH, the command and its tests pass, and the tests pass on macOS, Linux and Windows in CI (owner-only mode and executable-bit assertions apply to Unix only).
  10. AC10: The diff adds no new shell logic, changes no shell or Python file, and touches no path matched by golden_paths, constitutional_paths or owner_paths.
  11. AC11: Given a divergent checkout whose committed helper or verifier would write a sentinel file, when the command runs, then it refuses and the sentinel never appears.
  12. AC12: Given a base oid that is not a commit in the local repository, or a base object that is missing or unreadable, when the command runs or a snapshot is made, then it refuses naming the oid or path, fetches nothing, and leaves no snapshot directory.
  13. AC13: Given agentrepo merge integrity --help, when it runs, then the text documents --base and the exit statuses 0 and 1.

Scope and non-goals

In scope: the snapshot lifecycle and the integrity subcommand in Go, with tests, calling the #981 package.

Non-goal: the lock parser, expected map, walk and comparison, owned by #981; #859 consumes that parser, and this issue adds none.

Non-goal: calling the command. The first gate pass (#864) and the final check before the write (#870) call it; each is edited to gain #982 as a native blocker and an AC for integrity refusal, running helpers from the trusted snapshot, and snapshot removal on every exit path. Each also owns the live re-read of the pull request, head and base and the hold when the base moved; the command checks only against the oid it is given.

Non-goal: wrapper cutover, shell deletion and fallback retirement (#874, #875, #876, #882, #883, #884), the pinned configuration read (#980) and pinned binary selection or refusal (the binary-refusal issue in the table).

Non-goal: the rest of #499. Its AC1 (machine reader) is delivered by #942 and adopted by #864. Its AC2 is #980 for the reader and #864, #865 and #872 for use. Its AC5 (AGENTS.md and migration note) has no owner, so this issue's D9 duty is to file that follow-up. #600 is closed as superseded, unmerged, once the pull requests for #980, #981 and #982 have merged, as the table states.

Approach

Fail the Go tests first, using scratch repositories with local and remote package records. Add the snapshot and the subcommand beside merge_approval.go in internal/agentrepo and internal/mergepolicy, built on the #981 package. The command reads blobs from the given oid with git cat-file, writes the snapshot under a temporary directory with owner-only permissions, and removes it with a deferred cleanup that never replaces the original error. It reads no branch and runs nothing from the checkout. Command tests run the real kind and assert stdout, stderr and exit status; Windows runs the same tests with the Unix-only mode assertions skipped.

Dependencies

Blocked by #981 (native link in place). #981 owns the one lock parser in internal/mergepolicy/lock.go; this issue consumes it.

Blocks, as edits to other issues made before this issue is marked ready: #864 and #870 each gain #982 as a native blocked-by link and the ACs above. #859 is edited by #981's handoff to consume the shared parser, not by this issue.

Not blockers: #333 and #753 gate only the callers. Nothing calls this command until #864 and #870, and each already lists #333 and #753 or must add them before it switches a caller. Registering a subcommand edits the file #753 would protect, so #753 protects it only after that merges.

Size

M. One pull request. Estimated added code lines under the physical-line rule of tools/script_lines.py, comments and blanks excluded: internal/mergepolicy/snapshot.go about 120, internal/agentrepo/merge_integrity.go about 60, internal/mergepolicy/snapshot_test.go about 220, internal/agentrepo/merge_integrity_test.go about 140, about 540 in total with no deletions. No shell, Python or generated output changes, so no generated-copy exclusion is needed. If the count passes 800, split the snapshot from the subcommand and say so before opening the pull request.

Verify plan

  • AC2, AC12: Go tests on a scratch repository: file contents and modes of the snapshot, a missing blob, an absent oid and a snapshot that fails half way, asserting no directory remains and nothing was fetched.
  • AC3, AC5, AC7, AC11: command tests with a changed, an extra and an unmodified directory in each skill, and a committed helper that writes a sentinel, asserting stdout, stderr and exit status.
  • AC4: a test that swaps a committed file between two runs.
  • AC6: tests for pass, refusal and failure comparing exit status, stdout and stderr, and asserting the directory is gone.
  • AC9: the CI matrix run on macOS, Linux and Windows, and a test with PATH holding neither Python nor uv.
  • AC10: the diff shows no shell, Python or protected-path change, and mise run lint-shell passes.
  • AC13: a command test that captures --help and finds --base and both statuses.
  • Definition of done: D1 each AC verified on the merged head with commands and results cited in the PR's Verification. D2 independent review of the exact merged head. D3 required checks, mise run check, mise run cover-go-check and mise run cover-py-check. D4 check-leaks.sh with the private-strings list and gitleaks on the diff and each post. D5 signed commits, the squash included. D6 help text per AC13, otherwise "No doc impact". D7 Assisted-by trailers on the commits and the squash, and the provenance:agent-assisted label. D8 the line count against 800. D9 follow-ups filed as issues with a Problem and numbered ACs and listed under Related in the PR: the documentation follow-up for merge gates take authority and gate settings from the environment and the working tree #499's AC5; any deferred code links its issue. D10 Closes #982, the board showing Done, and a final Status: comment naming the merge commit. devtools-2 does not apply since no skill source changes. devtools-3: the PR says "No consumer impact" because nothing calls the new subcommand.

Risks

A parser or compare mismatch with the frozen policy could refuse a good directory or pass a bad one; the #981 fixtures are the control. A cleanup miss could leave snapshots in the temporary directory; AC6 and AC12 cover it. When the callers land, a stale deployment blocks merging and a re-pin must be merged from a checkout of the base commit. Undo: revert; nothing calls the command yet.

Operator authority

None.

Activity

  1. tbhb-dev-reviews commented on Oct 31, 2026

    @tbhb-dev-reviews

    Refinement: NOT READY — the integrity gate lacks a bounded Go integration point and conflicts with the existing cutover plan

    Checked

    Read #982, parent #324, freeze #588, siblings #980 and #981, #859, source requirement #499, relevant cutover children #862/#874/#875/#876/#882 and protection prerequisite #753. Compared origin/main at 591fd70 with the live main ref; they match. Read open PR #600 and merged PR #942 and checked other open PRs. No gate or test suite was run; the supplied mechanical PASS was accepted.

    AC1–AC10 each appear in Verify. Both skill directories, pre-gate and final checks, changed/extra files, success/hold/failure cleanup, portability and no-new-shell intent are covered in the plan. Risks and revert are stated. The native parent is #324 and the native blocked-by link to #981 exists.

    Gaps

    1. Missing integration prerequisites and conflicting ownership. On main, agentrepo merge only exposes saved approval evaluation (internal/agentrepo/merge_approval.go:16, internal/agentrepo/merge_approval.go:53); it has no merge transaction to wrap. The report and actual write still live in shell (packages/agents/source/skills/merge-pr/shared/scripts/preflight.sh:40, packages/agents/source/skills/merge-pr/shared/scripts/squash-merge.sh:294). AC8 duplicates The lint, populate-description, squash-message and review scripts become wrappers over agentrepo merge #874/The squash-merge and merge scripts become wrappers over agentrepo merge #875/The preflight scripts become wrappers over agentrepo merge #876's wrapper cutovers, while AC1 overlaps The merge scripts read pull request snapshots, reviews and the local diff through one Go reader #862's reader. Name the exact Go integration owner and add native prerequisites for the transaction, binary distribution (Ship the Go binaries as release assets installed through mise's github backend #333), and enforcement protection (build(agents): protect replacement Go enforcement paths #753), as applicable. A rebase note is not a readiness dependency. Keep this issue to integrity/snapshot wiring after that integration exists, or revise the epic and affected children explicitly.

    2. The 800-line claim is unsubstantiated. Counting origin/main with the physical code-line rule in tools/script_lines.py:19, the five merge-pr entry bodies total 707 lines and their five agent-merge-pr counterparts 26: 733 source lines removed before adding Go wiring, tests or shims. This excludes populate-description and shared helper removal. Generated copies do not make source deletions free. Provide a path-by-path added/deleted code budget for the revised scope, with exclusions named; split before implementation if it exceeds 800. The current M label alone does not establish one-PR fit.

    3. Trust and freshness need observable contracts. AC2 specifies git cat-file and mode bits rather than proving that the verifier/helpers actually execute from the trusted base snapshot. Move mechanism into Approach; require private access, executable-mode behavior, refusal on missing/unreadable base objects or incomplete snapshot creation, and a divergent-checkout verifier/helper sentinel that must never execute. AC1's “every later read” must mean pinned code/policy reads, not stale approvals/checks: preserve fresh final PR/head/base validation and define the hold/refusal when the live base moves. The existing write is pinned to the reviewed head (packages/agents/source/skills/merge-pr/shared/scripts/squash-merge.sh:294). Verify must exercise base movement and trust isolation, not just a PR lookup stub.

    4. Compatibility, shell freeze and authority disagree with the proposed deletion. Move the merge-pr, merge policy and agent-merge-pr scripts into agentrepo merge #324 and The lint, populate-description, squash-message and review scripts become wrappers over agentrepo merge #874/The squash-merge and merge scripts become wrappers over agentrepo merge #875/The preflight scripts become wrappers over agentrepo merge #876/The wrappers stop falling back to the frozen shell after the N-2 window #882 reserve fallback retirement and owner-tier deletion for later bounded work. AC8 immediately deletes replaced shell, and AC9 can be read as making the whole merge Python-free even though the remaining gate explicitly requires uv (packages/agents/source/skills/merge-pr/shared/scripts/merge-agent.sh:27, packages/agents/source/skills/merge-pr/shared/scripts/merge-agent.sh:71). Limit AC9 to this integrity path or add the real prerequisites. Preserve the agreed N-2 rollout, or identify the approved security transition and consumer steps (docs/compatibility.md:3, docs/compatibility.md:7). Name exactly what is deleted: main has no merge-policy.py integrity subcommand/caller to remove. If protected helpers are changed/deleted, “Operator authority: None” is false; .agents/agent-skills.conf:4 and .agents/agent-skills.conf:9 protect them. Otherwise explicitly exclude those paths and defer their deletion. Retain AC10: no new shell logic, only permitted forwarding shims.

    5. The fix(merge): pin gate policy and require machine authority #600 replacement still has gaps and an open duplicate. PR fix(merge): pin gate policy and require machine authority #600 remains OPEN and already implements the snapshot/integrity behavior in shell. It is not merged delivery, but its retirement/supersession must be explicit. PR feat(config): read merge authority from machine configuration #942 delivered a strict machine reader, not workflow adoption; merge still calls the generic config machine reader (packages/agents/source/lib/lib.sh:233). Thus merge gates take authority and gate settings from the environment and the working tree #499 AC1 is not already wired. Likewise Read gate settings from the pinned base in Go #980 defines a pinned repository lookup but does not explicitly require merge gates to consume it. Assign the remaining merge gates take authority and gate settings from the environment and the working tree #499 AC1/AC2 adoption and AC5 documentation work to named issues; do not claim Read gate settings from the pinned base in Go #980/Verify merge skill directories against base lock hashes in Go #981/Run the skill-directory integrity gate in the merge entry points #982 completely replace fix(merge): pin gate policy and require machine authority #600 while those outcomes are unowned.

    6. The The merge policy reads the APM lockfile and base-owned deployments in Go #859/Verify merge skill directories against base lock hashes in Go #981 parser overlap is only conditionally acknowledged. Both own lock parsing and fixtures; Verify merge skill directories against base lock hashes in Go #981 says to reuse whichever lands first, while The merge policy reads the APM lockfile and base-owned deployments in Go #859 still owns its parser and neither fixes the shared contract/order. Establish one parser owner and tested interface, update both scopes, and add the corresponding native blocker. Run the skill-directory integrity gate in the merge entry points #982 should consume that result, without adding a third parser.

    7. Finish the verification and delivery plan. AC6 tests status and removal but omits its promised report preservation; assert report/streams as well. AC9's absent-uv test does not prove absence of Python; test both absent for the scoped path. Add explicit evidence for D2–D7/D9–D10 and repository checks/coverage and consumer documentation below. Existing AC tracing is useful, but it is not the full DoD.

    Split

    Keep #980 as pinned configuration/binary selection, #981 as integrity library atop the single agreed lock parser, and #982 as bounded trusted-snapshot lifecycle plus the two integrity calls. Leave general reader/transaction ports and wrapper/fallback retirement with the existing #324 children, adding native dependencies instead of reimplementing them. Reconcile #859's parser ownership, name the remaining #499 adoption/docs owner, and retire #600 explicitly. Recount #982 after those boundaries are written.

    Definition of done

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

    • 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.
    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.
    • 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.
    • D5 The author signed each commit, the squash included.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
    • 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.
    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.
    • devtools-1 mise run check is green, and so are the coverage floors mise run cover-go-check and mise run cover-py-check, which check doesn't run.
    • devtools-2 A change to skill sources under packages/agents/source/ commits what mise run generate-skills, then mise run skill-tokens, then apm install rewrite, in that order, and check-generated-skills (part of lint) passes.
    • devtools-3 A consumer-visible change (the vendored payload, a skill, a reusable workflow, a task or a pin) has its consumer steps in docs/agent-skills-migration.md or the adoption docs, or the PR's Verification says "No consumer impact" and why.

    Ticks above mark planned coverage (AC-by-AC verification and regeneration), not completed implementation or merged-head evidence. D8 remains open because the source-line budget is not credible yet; other unticked duties are not specified by this issue's ACs/Verify plan.

    Round: 1 of 3

  2. tbhb-dev-reviews commented on Oct 31, 2026

    @tbhb-dev-reviews

    Refinement: NOT READY — bounded Go scope, but replacement ownership and consumer contracts still disagree

    Checked

    Accepted the supplied mechanical PASS without rerunning it. Read #982 and round 1, native parent #324 and its child list, freeze #588, siblings #980/#981/#859/#864/#870, #753, source requirement #499, open PRs #600/#780 and merged PR #942; searched open and related merged/closed PRs. origin/main and GitHub main both resolve to 591fd70. No tests were run.

    Every active AC (AC2–AC7 and AC9–AC12) appears in Verify; AC1/AC8 are explicitly dropped. Privacy, executable modes, cleanup preserving streams/status, repeat checks, absent objects, no fetch, sentinel isolation, portability and Python/uv independence have tests planned. The roughly 540-line Go-only budget fits M and one PR. Risks and revert are stated. The native #981 blocker is sufficient for this standalone command/library; transaction and distribution need not block unused code. AC10 honors the shell freeze. The current protected-path patterns permit this scope without owner authority (.agents/agent-skills.conf:4, .agents/agent-skills.conf:9).

    Main has only saved approval evaluation (internal/agentrepo/merge_approval.go:16, internal/agentrepo/merge_approval.go:53); the Python dispatcher has no integrity operation (packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:778). No merged PR delivers this command. #942 supplies the machine reader only. #600 remains an overlapping open shell/Python implementation; replacement rather than reuse is appropriate under #588.

    Gaps

    1. The downstream handoff is asserted only here. Run the skill-directory integrity gate in the merge entry points #982 assigns the first integrity call to The unattended merge gates run in Go #864 and the final call to The merge gates and confirmation chain run in Go before the write #870, but neither current issue includes Run the skill-directory integrity gate in the merge entry points #982 in its ACs, Verify plan or dependencies. Neither owns helpers executing from the trusted snapshot. Reconcile those consumer contracts and native blockers now, including integrity refusal, trusted-snapshot use and cleanup tests, while retaining live head/base validation with the transaction owners. Likewise Verify merge skill directories against base lock hashes in Go #981 claims sole parser ownership but The merge policy reads the APM lockfile and base-owned deployments in Go #859 still scopes the parser port and has only Ship the Go binaries as release assets installed through mise's github backend #333 as a native blocker. Update The merge policy reads the APM lockfile and base-owned deployments in Go #859 to consume Verify merge skill directories against base lock hashes in Go #981 and add that native edge. These are issue-contract fixes, not requests to expand Run the skill-directory integrity gate in the merge entry points #982's implementation. Otherwise the caller issues can complete their written contracts without adopting this work. The actual write still lives in shell (packages/agents/source/skills/merge-pr/shared/scripts/squash-merge.sh:294).

    2. The fix(merge): pin gate policy and require machine authority #600 replacement boundary still names the wrong owner. Run the skill-directory integrity gate in the merge entry points #982 assigns pinned binary selection to Read gate settings from the pinned base in Go #980; Read gate settings from the pinned base in Go #980 now explicitly excludes it and proposes an unfiled separate issue. Main still falls back from the repository pin to PATH (packages/agents/source/lib/lib.sh:146, packages/agents/source/lib/lib.sh:149). File/link that missing owner and reconcile fix(merge): pin gate policy and require machine authority #600's retirement condition: Run the skill-directory integrity gate in the merge entry points #982 says all replacement work must merge, Verify merge skill directories against base lock hashes in Go #981 says three children, and Read gate settings from the pinned base in Go #980 says it closes after the first replacement PR merges once the replacements are ready. Keep Run the skill-directory integrity gate in the merge entry points #982 independent of rollout, but remove the false Read gate settings from the pinned base in Go #980 assignment and make the remaining merge gates take authority and gate settings from the environment and the working tree #499 ownership explicit.

    3. Finish the delivery plan: D9 must require follow-up issues with Problem/numbered ACs and PR Related links; D10 must include board Done and the merge commit in the final Status comment. Carry the merged-head AC evidence format into PR Verification.

    Split

    No further split is needed for the revised snapshot lifecycle plus standalone command. Keep parser/comparison in #981, calls and trusted-snapshot consumption with explicitly updated consumer issues, and binary rollout/docs/wrapper retirement with named owners. Recount if implementation approaches 800 code lines.

    Definition of done

    Ticks mean planned coverage, not completed implementation or merged-head evidence.

    • D1 Verify each AC on the merged head and cite each ID with commands/results or links in PR Verification. The AC matrix supplies the plan; retain this delivery format.
    • D2 Independent approval of the exact merged head, with findings resolved or filed. Exact-head review and follow-up filing are planned.
    • D3 Required checks pass on that head; owner approval for any loosened gate. Checks are planned; this bounded scope adds enforcement and excludes protected edits.
    • D4 Clean gitleaks, check-leaks.sh with the private-strings list, and leak review of each post. Verify names these.
    • D5 Signed commits, including squash. Signed commits are planned; retain squash coverage.
    • D6 Matching docs/help or justified No doc impact in PR Verification. Exit-status help is planned.
    • D7 Assisted-by on commits and squash, plus provenance:agent-assisted on the PR. Trailers and label are planned; retain squash coverage.
    • D8 PR within size limits, or exact-head owner exception. Path budget and split threshold cover this.
    • D9 Follow-ups/deferred findings filed with Problem and numbered ACs, listed under PR Related, and linked from any deferred code. Verify names follow-ups but omits these conditions.
    • D10 Closing reference, board Done, and final Status comment naming the merge commit. Verify omits board and commit evidence.
    • devtools-1 mise run check, cover-go-check and cover-py-check. All are planned.
    • devtools-2 Required generation, tokens, deployment and generated-output checks. Inapplicable to this Go-only scope.
    • devtools-3 Adoption documentation or justified No consumer impact in PR Verification. The unused-command rationale is supplied.

    Round: 2 of 3

  3. tbhb-dev-reviews commented on Oct 31, 2026

    @tbhb-dev-reviews

    Refinement: NOT READY — the bounded Go work is specified, but required handoffs remain unmade

    Checked

    Accepted the supplied mechanical PASS without rerunning it. Read #982 and its two earlier Refinement comments, native parent #324, freeze #588, siblings #980/#981/#859/#864/#870, wrapper issues #874/#875/#876, #932, #753, source requirement #499, open PR #600 and merged PR #942. Searched open PRs and related merged/closed PRs. Local origin/main and live main both resolve to 591fd70. Read the named code on that ref. No tests or suites were run, and no agents were contacted.

    Every active AC (AC2–AC7 and AC9–AC13) is traced in Verify; AC1 and AC8 are explicitly dropped. The ACs describe observable outcomes: private snapshots, bytes/modes, refusal diagnostics, repeated checking, stream/status preservation, cleanup, absent objects without fetching, helper isolation, help and portability without Python/uv. Implementation choices stay in Approach. The roughly 540-code-line budget fits M and one PR, with an explicit 800-line split threshold. Risks and revert are stated. AC10 respects the shell freeze and excludes currently protected paths, supporting Operator authority: None for this scope (.agents/agent-skills.conf:4, .agents/agent-skills.conf:9).

    Main exposes only saved approval evaluation under merge (internal/agentrepo/merge_approval.go:16, internal/agentrepo/merge_approval.go:53); the Python dispatcher has no integrity operation (packages/agents/source/skills/merge-pr/shared/scripts/merge-policy.py:778). No merged PR delivers this command. #600 remains an overlapping OPEN shell/Python implementation, with a replacement plan now agreed across #980/#981/#982. #942 delivered only the machine reader, with workflow adoption expressly excluded.

    Gaps

    1. The required consumer edits and native dependencies still do not exist. Live The unattended merge gates run in Go #864 and The merge gates and confirmation chain run in Go before the write #870 have no Run the skill-directory integrity gate in the merge entry points #982 blocker and no AC/Verify coverage for integrity refusal, trusted-snapshot helper use or snapshot cleanup. Live The merge policy reads the APM lockfile and base-owned deployments in Go #859 still owns parser behavior and lists only Ship the Go binaries as release assets installed through mise's github backend #333 as its native blocker, rather than consuming Verify merge skill directories against base lock hashes in Go #981. The shared ownership table is consistent now, but promises in producer issues do not change consumer contracts. Make those scope/AC/Verify edits and native edges before Ready, as Run the skill-directory integrity gate in the merge entry points #982 itself requires. Retain live head/base validation with the transaction owners. The actual write remains in shell at packages/agents/source/skills/merge-pr/shared/scripts/squash-merge.sh:294. D9's option to file caller follow-ups later cannot substitute for the explicit before-Ready requirement.

    2. The binary-refusal owner remains unfiled/unlinked in the stated contract. All three bodies explicitly require that issue to be filed and linked as a native blocker of The lint, populate-description, squash-message and review scripts become wrappers over agentrepo merge #874/The squash-merge and merge scripts become wrappers over agentrepo merge #875/The preflight scripts become wrappers over agentrepo merge #876 before any is Ready. Those wrapper issues still have no such new edge; the table supplies no issue number. The binary resolver, permanent launcher and configuration bridges are resolved in Go #932 already owns resolver/launcher work, but specifies status 2 and a different transition boundary, whereas this table specifies status 1. Assign the requirement to a concrete issue, reconcile it with The binary resolver, permanent launcher and configuration bridges are resolved in Go #932, and update wrapper contracts/native edges; avoid competing resolvers and new shell logic. Main still falls back from the pin to PATH at packages/agents/source/lib/lib.sh:146 and packages/agents/source/lib/lib.sh:149. This is a backlog-contract prerequisite, not a demand to make an unused command wait for rollout.

    3. Complete the promised fix(merge): pin gate policy and require machine authority #600 handoff record. Its live body still says Closes merge gates take authority and gate settings from the environment and the working tree #499 and it has no comments; the table promises a note there citing Read gate settings from the pinned base in Go #980/Verify merge skill directories against base lock hashes in Go #981/Run the skill-directory integrity gate in the merge entry points #982 and closure as superseded only after all three merge. Record that plan on fix(merge): pin gate policy and require machine authority #600 so it cannot be mistaken for the active delivery vehicle. Keep merge gates take authority and gate settings from the environment and the working tree #499 open for callers, binary refusal and AC5 documentation. The present Run the skill-directory integrity gate in the merge entry points #982 text correctly assigns machine-reader adoption to The unattended merge gates run in Go #864; preserve that assignment when updating The unattended merge gates run in Go #864, since the current shared bridge still calls config machine (packages/agents/source/lib/lib.sh:233).

    The previous D9/D10 delivery-plan omissions are fixed. They are not remaining gaps.

    Split

    No further code split is needed for the snapshot lifecycle and standalone command. Keep parser/comparison in #981, consumers in #859/#864/#870, and rollout in the wrapper/binary issues. This is the third NOT READY round: the backlog owner should rethink the unresolved handoff package (split, spike, or close as not planned), rather than send an operator decision or begin implementation. The remaining work is concrete issue-contract reconciliation.

    Definition of done

    The definition of done for #982 (Feature). Ticks mark coverage in the ACs and Verify plan, not completed implementation or merged-head evidence.

    • 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.
    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head.
    • 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.
    • D5 The author signed each commit, the squash included.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit.
    • 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.
    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit.
    • devtools-1 mise run check is green, and so are the coverage floors mise run cover-go-check and mise run cover-py-check, which check doesn't run.
    • devtools-2 A change to skill sources under packages/agents/source/ commits what mise run generate-skills, then mise run skill-tokens, then apm install rewrite, in that order, and check-generated-skills (part of lint) passes. Inapplicable to this Go-only scope.
    • devtools-3 A consumer-visible change (the vendored payload, a skill, a reusable workflow, a task or a pin) has its consumer steps in docs/agent-skills-migration.md or the adoption docs, or the PR's Verification says "No consumer impact" and why. The issue supplies the unused-command rationale.

    D1–D10 are covered by the explicit delivery paragraph; AC13 covers help, and the path budget covers D8. Protected-path edits and gate loosening are excluded by the scoped change. These ticks do not waive the before-Ready contract gaps above.

    Round: 3 of 3

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:activeA refinement round is open; not ready for implementation

    Fields

    Priority

    None yet

    Work type

    None yet

    Projects

    • Status
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions