Skip to content

The replacements and policy Bats cases are retired where Go covers them #878

Open

Description

@tbhb-agent

Problem

tests/skills/agent-merge-pr/replacements.bats (235 lines, 19 cases) and policy.bats (409 lines, 31 cases) test gate replacement pairing, owner approval, required-check selection, review readers, APM lockfile reading and protection through the shell scripts. After the wrappers exec Go, those behaviors have Go tests from several children, so the Bats cases either duplicate them or need a real shell.

Part of #324, within #588. Child 16 in #324's Children list. Retires cases of tests/skills/agent-merge-pr/replacements.bats and tests/skills/agent-merge-pr/policy.bats.

Acceptance criteria

  1. AC1: Given each of the 50 cases in the files named above, when the ledger is read, then the case is deleted with the name of the Go test that covers it, or kept with a one-line reason it needs a real shell. The ledger also lists any case a prerequisite issue added to these files before this change, with the same disposition.
  2. AC2: Given the Go tests named in the ledger, when mise run test-go runs, then each passes and asserts the same outcome as the Bats case it replaces (the coverage comes from children 01 to 03, the review-reader tests, and the protection tests of build(agents): protect replacement Go enforcement paths #753).
  3. AC3: Given the cases kept, when mise run test-hooks runs on the files, then they pass against the wrappers.
  4. AC4: Given a deleted case, when a regression is introduced in the behavior it covered, then a named Go test fails; the pull request shows one such mutation per ten deleted cases.
  5. AC5: Given a search of both test families (tests/skills and tests/hooks) and of the documentation for the helper names and files this change leaves unused, when it runs, then no reference to a removed Bats case or helper remains, and the shared helpers that other suites still load stay in place for child 24.
  6. AC6: Non-functional: the Bats suite wall time does not increase.

Scope and non-goals

Scope: the Bats files named in the problem, helpers loaded by them alone, and the ledger in the pull request description.

Non-goal: porting new behavior or editing production code. Non-goal: removing shared helpers other suites still load, which child 24 does. Non-goal: the other agent-merge-pr suites.

Approach

Meet AC1 to AC6 by building the ledger first, then deleting cases whose Go counterpart exists, then mutation-checking a sample and running the reference audit.

Dependencies

Blocked by: #874, #875, #876, #333.

Prerequisites: #333.

Size

S. One pull request, about 645 lines changed, almost all deletions.

Verify plan

  • AC1: the ledger lists all 50 cases, plus prerequisite additions, with a disposition each.
  • AC2, AC3: mise run test-go and mise run test-hooks tests/skills/agent-merge-pr at the merged head.
  • AC4: the sampled mutation runs, with the failing test names.
  • AC5: the search output, run over both test families and the documentation.
  • AC6: Bats wall times before and after.
  • 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 passes the per-post leak review before it is posted. D5 every commit is signed. D6 the documentation matches the change or the Verification says why there is no impact. D7 every commit has an Assisted-by trailer and the pull request has the provenance:agent-assisted label. D8 the pull request stays within the size limits. D9 follow-ups are filed as issues and listed under Related. D10 the merge closes the issue through a closing reference, the board shows Done, and the final Status: comment names the merge commit.

Risks

A deleted case with no true Go equivalent loses coverage of the gates that decide owner approval and protected paths. The ledger and the mutation sample are the controls. Undo: revert the deletion.

Operator authority

None.

Activity

  1. tbhb-agent commented on Oct 27, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: READY — The replacements and policy Bats cases are retired where Go covers them
    Model: gpt-6-astra

    Child 16 The replacements and policy Bats cases are retired where Go covers them

    Verdict: READY

    Checked: The broader coverage attribution, prerequisite-added cases and separate helper audit close the previous findings. Protection coverage includes #753 without duplicating its implementation. The suites contain 572 code lines; existing protection assertions appear at tests/skills/agent-merge-pr/policy.bats:340.

    Gaps: None.

    Split: None needed.

    Definition of done

    The definition of done for this issue (Chore). 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.

    Round: 3 of 3

  2. added theissue type on Oct 27, 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