Skip to content

chore(checks): retire the watch-checks shell fallback #996

Open

Description

@tbhb-agent

Problem

After #977 the watch-pr entry point probes for agentrepo checks watch and keeps the frozen shell watcher as the fallback for older binaries. #978 adds a byte-copy oracle and a parity test that run that frozen shell. Retirement waits on #333, #544 and the N-2 window, and no issue tracks the deletion, so the shell logic stays in the tree after the window ends.

Acceptance criteria

  1. AC1: Given the retirement release recorded in AC3, watch-checks.sh only probes for and executes agentrepo checks watch. Supported: arguments, stdout, stderr and exit status pass through unchanged, including operational failures (1, 2, 76). Absent binary and binary without the subcommand: nothing runs, one actionable message goes to stderr naming the missing capability and how to install a supporting binary (the release-asset pin from feat(distribution): migrate consumer tool pins to release assets #544), and the script exits with exit status 127. The probe makes no reads and no side effects. Only the existing probe-and-exec boundary remains: no new decision or diagnostic logic in shell.
  2. AC2: Remove the frozen fallback body, test(checks): prove watch command parity against the pinned shell #978's oracle fixture and its parity test, and any remaining comparison fixtures, from the canonical source, packages/agents/claude/.apm/skills, packages/agents/codex/.apm/skills, and the deployed .claude/skills and .agents/skills. Standalone Go behavior tests of the command stay.
  3. AC3: Before implementation, record in docs/compatibility.md and docs/agent-skills-migration.md the release that first shipped the feat(checks): route the watch wrapper to the Go command #977 wrapper (the warning start), the retirement release (at least two releases later, per the policy's warning period), and the evidence for both: git tag --contains <#977 merge commit>, the tag list, and the release that first carries the assets from Ship the Go binaries as release assets installed through mise's github backend #333 and the pin migration from feat(distribution): migrate consumer tool pins to release assets #544.
  4. AC4: Non-functional: mise run check, mise run cover-go-check and mise run cover-py-check pass, no gate is loosened, and the regenerated skills, token records and lock are committed.

Scope and non-goals

In scope: removing the fallback after the window, and the documentation of the retirement release. Non-goals: changing the Go watcher, shortening the window, and any distribution change (#333, #544).

Approach

Resolve the retirement release from the AC3 procedure first, and stop if the current release is earlier. List the fallback, oracle and parity-test paths from the merged #977 and #978 changes, then delete them together. Keep only the existing probe and its refusal path. Run mise run generate-skills, then mise run skill-tokens, then apm install, and commit what they rewrite.

Dependencies

Blocked (native links) by #977, #978, #333 and #544. #978 owns the oracle and parity test that this change deletes; without the link it could land later and restore the frozen implementation. Also blocked by the end of the window per AC3. Native parent: #793. Part of #588.

Size

S.

Verify plan

  • AC1: with scripted stand-in binaries, run the entry point with a supporting binary and assert byte-equal arguments, stdout, stderr and statuses, including a supported run returning 1, 2 and 76 that is never retried or replaced. Run with no binary and with a binary lacking the subcommand, and assert exit status 127, that the message is on stderr and names the installation step, that stdout is empty, and that the probe made no reads. Inspect the script to assert it contains only probe-and-exec logic.
  • AC2: take the artifact list from the merged feat(checks): route the watch wrapper to the Go command #977 and test(checks): prove watch command parity against the pinned shell #978 changes. Assert that none of those paths exists, and that git ls-files plus git grep over packages/agents, .claude/skills, .agents/skills, tests and docs find no executable reference to them, by name or by the baseline blob a0347a715fab580ece45edfcb3d13509620459a8. Assert the Go watcher tests still run and pass.
  • AC3: read both documents and assert they name the warning-start release, the retirement release and the evidence. Assert the current release is not earlier than the retirement release.
  • AC4: run mise run check, mise run cover-go-check and mise run cover-py-check, then mise run generate-skills, mise run skill-tokens, apm install and mise run check-generated-skills, and assert exit 0 with no uncommitted change. Compare packaged and deployed entry points, token metadata and lock changes independently.
  • D1–D10: verify every AC on the merged head and cite its evidence in the PR; obtain independent exact-head approval and passing required checks; complete gitleaks, the repository leak scan and the review of every content post; sign commits and squash; update docs; include Assisted-by trailers and the provenance:agent-assisted label; check the actual PR size; file follow-ups with a Problem and numbered ACs and list them in PR Related; close the issue with a closing reference, move it to Done and record the merge commit in the final Status comment.

Risks

Removing the fallback early breaks consumers on an older binary, so the change waits for the AC3 evidence. Reverting the commit restores the fallback.

Operator authority

None.

Activity

  1. added theissue type on Oct 31, 2026
  2. tbhb-dev-reviews commented on Oct 31, 2026

    @tbhb-dev-reviews

    Refinement: NOT READY — define the retirement boundary and complete verification

    Checked

    Gaps

    1. Make the retirement boundary auditable. AC1 names an elapsed N-2 window without identifying its start or warning releases. docs/compatibility.md:3–5 requires backward compatibility and at least two releases of warning before removing a file. Record the first supporting/warning release and the eligible retirement release, or a precise procedure and prerequisite evidence for determining them before implementation. Completion of Ship the Go binaries as release assets installed through mise's github backend #333/feat(distribution): migrate consumer tool pins to release assets #544 alone does not prove the window elapsed.
    2. Complete dependencies and removal scope. Add native blocked-by test(checks): prove watch command parity against the pinned shell #978, which owns the oracle/comparison test being removed; otherwise that change can land later and restore the frozen implementation. Set the intended native parent (feat(checks): move pull request watching into Go #793 for the watcher migration, or explicitly choose another). Identify the fallback, oracle and comparison-test artifacts in AC2/Verify. Searching unspecified “frozen body markers” cannot prove renamed artifacts and generated/deployed copies are absent. Verify artifact absence and no executable references across canonical, packaged and deployed output, while preserving standalone Go behavior tests.
    3. Complete AC1 verification. Include an absent binary, an unsupported binary and supported executions that fail operationally. Define the refusal status, stream and actionable installation guidance; assert unchanged arguments, stdout, stderr and statuses on the supported path. Explicitly preserve Replace complex shell with tested Go, starting with gates and merging #588's freeze: only the thin existing probe/exec boundary remains; any new decision or diagnostic behavior belongs in tested Go, with no new shell logic.
    4. Complete repository checks and completion evidence. Add cover-go-check and cover-py-check, which check does not run, and generate-skills → skill-tokens → apm install, followed by generated/package/deployment inspection. Add D1–D10 completion obligations to Verify: PR AC evidence on the merged head, exact-head review, scans, signatures, provenance, actual size, follow-ups and issue/board closure. These are future implementation checks, not checks to run during refinement.

    Split

    No split needed. Keep one S retirement PR after its prerequisites and compatibility window, without distribution or watcher behavior changes.

    Definition of done

    Ticks mark coverage in the current ACs and Verify plan, not completed implementation. D1 is partial: IDs are traced, but cases and merged-head PR evidence remain missing. D8 has a plausible S estimate but no actual size check.

    • 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.

    Round: 1 of 3

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

    @tbhb-dev-reviews

    Refinement: NOT READY — native prerequisite and parent links remain missing

    Checked

    Gaps

    1. Apply the native links already claimed by Dependencies. The live blocked-by endpoint returns only feat(checks): route the watch wrapper to the Go command #977, Ship the Go binaries as release assets installed through mise's github backend #333 and feat(distribution): migrate consumer tool pins to release assets #544; test(checks): prove watch command parity against the pinned shell #978 is absent. The native parent endpoint returns 404, and feat(checks): move pull request watching into Go #793's native child list excludes chore(checks): retire the watch-checks shell fallback #996. Add native blocked-by test(checks): prove watch command parity against the pinned shell #978 and native parent feat(checks): move pull request watching into Go #793. Body text does not establish these links: without test(checks): prove watch command parity against the pinned shell #978, its oracle/parity implementation can land after retirement and restore removed shell. The substantive AC/Verify changes otherwise address round 1.

    Split

    No split needed. Keep one S retirement PR after the prerequisites and documented compatibility window.

    Definition of done

    Ticks mark coverage in ACs and Verify, not completed implementation. D2 and D9 need the small completion details noted below.

    • D1 Verify every AC on the merged head and cite each AC ID and evidence in the PR Verification section.
    • D2 Independent approval of the exact merged head is covered. Also explicitly require resolving every review finding or filing a follow-up.
    • D3 Required checks pass on that head. AC4 prohibits gate loosening.
    • D4 Gitleaks, repository private-string leak scan and review of every content post are covered.
    • D5 Sign every commit and the squash.
    • D6 Update compatibility and consumer migration documentation.
    • D7 Include Assisted-by trailers and the provenance:agent-assisted PR label.
    • D8 Check actual PR size against the limits.
    • D9 Follow-ups with Problem and numbered ACs, listed in PR Related, are covered. Also explicitly require any deferred work left in code to link its issue.
    • D10 Closing reference, board Done and final Status: comment with merge commit are covered.
    • devtools-1 AC4 requires check, cover-go-check and cover-py-check.
    • devtools-2 Approach and Verify require generate-skills, skill-tokens and apm install in order, committed outputs, check-generated-skills and independent deployment inspection.
    • devtools-3 AC3 requires docs/agent-skills-migration.md consumer guidance.

    Complete the D2/D9 wording alongside the native metadata correction; no further scope expansion is needed.

    Round: 2 of 3

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

    @tbhb-dev-reviews

    Refinement: READY — native prerequisites and retirement safeguards are in place

    Checked

    Gaps

    No blocking Definition of Ready gaps remain. Two completion details from round 2 remain implicit in the D1–D10 Verify bullet: resolve every review finding or file it as a follow-up (D2), and link any deferred work left in code to its issue (D9). The checklist below preserves those obligations. These are non-blocking DoD wording omissions under the current warn-only DoD policy.

    Split

    No split needed. Keep one S retirement PR after its prerequisites and compatibility window.

    Definition of done

    Ticks mark coverage in the current ACs and Verify plan, not completed implementation. D2 and D9 remain unticked because their complete wording is not explicit in the plan.

    • 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.

    Round: 3 of 3

  5. added
    refinement:readyPassed the Definition of Ready; may be picked up
    and removed
    refinement:activeA refinement round is open; not ready for implementation
    on Oct 31, 2026
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
      Backlog

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions