Skip to content

chore(agents): delete agent.sh and signing.sh after the Go port; cutover 2 under #588 #998

Open

Description

@tbhb-agent

Problem

packages/agents/source/lib/agent.sh and packages/agents/source/lib/signing.sh, with their generated copies, stay in the tree after #321 ports the shared skill library to Go. Epic #321 lists this deletion as the second of two cutover issues under #588, and no issue tracks it. agent.sh is an owner and golden path and signing.sh is a constitutional path (.agents/agent-skills.conf), so the deletion needs the owner's exact-head approval at merge.

Acceptance criteria

  1. AC1: Given the prerequisite set under Dependencies, when the pull request is opened, then each prerequisite has a merged deliverable (or a named superseding one), the warning deliverable declared Compatibility: deprecates agent.sh since <version> (and the same for signing.sh), and the pull request lists a release ledger of the warning-start release, two published releases whose notes carry that warning text, the eligible removal release and the linked merge evidence; both source files are then deleted.
  2. AC2: Every generated copy of either file under the claude and codex packages and the deployed .claude/skills and .agents/skills trees is deleted, the tokens.json entries and apm.lock.yaml entries for them are gone, and running mise run generate-skills, mise run skill-tokens and apm install in that order, then mise run generate-skills and mise run check-generated-skills again, leaves no agent.sh or signing.sh file or entry and no diff.
  3. AC3: No remaining script, skill manifest, task, workflow, generator or test sources or invokes either file as an executable dependency: the generators' copy lists (internal/skills/generate.go, tools/generate-skills.sh) and their tests drop both names through tested Go and deletion or thin-exec changes to shell only, and a boundary-anchored git grep shows only the allowed exceptions listed under Verify plan.
  4. AC4: Given a case-level retirement ledger in the pull request, every existing test case that exercises agent.sh or signing.sh is either deleted with a named Go test that covers it, or retained because it covers other behavior; no suite is deleted wholesale, and unrelated coverage stays.
  5. AC5: Non-functional: mise run check, mise run lint, mise run test, mise run cover-go-check and mise run cover-py-check pass at the head, and no gate is loosened.
  6. AC6: Non-functional: the pull request declares its consumer impact under Risk with a validator-compatible Compatibility: line, and records the removal release and the AC1 ledger separately in Verification.
  7. AC7: Non-functional: the pull request's Verification cites each AC ID with evidence on the exact head, the leak scans (gitleaks, check-leaks.sh with the private-strings list, and the leak review of each post) are clean, each commit and the squash are signed with Assisted-by trailers and the provenance:agent-assisted label, docs/agent-skills-migration.md carries the consumer steps (or Verification says "No consumer impact" and why), an independent reviewer approved the exact head with each finding resolved or filed, the owner approved the exact head, each follow-up is an issue with a Problem and numbered ACs listed under Related and any deferred work left in code links its issue, and after the merge the issue closes through a closing reference, the board shows Done and a final Status: comment names the merge commit.
  8. AC8: Non-functional: the non-generated code delta stays under 800 lines, measured on the head and recorded in Verification; no new behavior or shell logic is added, and a behavior gap found here returns to its port owner as a follow-up.

Scope and non-goals

In scope: deleting agent.sh, signing.sh, their generated copies, token and lock entries, the generators' copy-list entries, references, and the test cases AC4 retires. Non-goals: porting behavior (the #321 children, #850 and #856 own that), deleting lib.sh (#997), shortening any N-2 window, and editing the owner, golden and constitutional patterns in .agents/agent-skills.conf, which stay as they are so no gate is loosened.

Approach

Delete after the last prerequisite merges and the window closes. Remove both names from the generator copy lists, delete the source and the copies, then run mise run generate-skills, mise run skill-tokens and apm install in that order and commit what they rewrite. Run the generation and check a second time to prove it is stable. Resolve the remaining references: tests/tools/generate-skills.bats, internal/skills/generate_internal_test.go, the cp line in tests/skills/source/lib/masked-status.bats that copies both helpers for an unrelated case, and the agent.sh loop in tests/skills/agent-merge-pr/policy.bats. The history sentence in docs/agent-skills-migration.md may name the files.

Test retirement: tests/skills/source/lib/ holds lib.bats, gates.bats and masked-status.bats, and no separate suite for either file. Signing cases live in tests/skills/agent-commit/workflow.bats ("preflight refuses missing signing and missing hooks", "signing failure holds without an unsigned fallback"); each is kept or replaced by a named Go test per AC4.

Dependencies

Blocked by (native links on this issue; each is a prerequisite and needs a merged deliverable, not only a closed issue):

Part of #588.

Size

S. A deletion of two source files, their generated copies and token, lock and copy-list entries, plus the case-level test retirement. The 800-line bound is measured in AC8.

Verify plan

  • AC1: read the blocked-by list through GraphQL and assert every Dependencies entry is closed, and for each open the merged pull request (or named superseding deliverable) that delivers it; git tag --contains <deprecation merge commit> lists at least two published releases before the removal release, and each of those releases' notes (gh release view or the changelog entry) contains the deprecation warning text; the ledger in the pull request links each merge, tag and warning, and keeps the Compatibility: declaration separate from the removal-release row.
  • AC2: after git ls-files | grep -E '(^|/)(agent|signing)\.sh$' prints nothing, run mise run generate-skills, mise run skill-tokens and apm install in order, inspect both packages and both deployments, repeat mise run generate-skills and mise run check-generated-skills, and assert git status --porcelain is empty and no agent.sh or signing.sh reappears or appears in tokens.json or apm.lock.yaml.
  • AC3: git grep -nE '(^|[^A-Za-z0-9_-])(agent|signing)\.sh([^A-Za-z0-9_-]|$)' prints nothing except the allowed exceptions: history sentences in docs/agent-skills-migration.md, absence-test literals, and the unchanged owner, golden and constitutional patterns in .agents/agent-skills.conf; the boundaries exclude pr-agent.sh and merge-agent.sh. Each remaining hit is classified in the pull request.
  • AC4: the ledger names each existing case, its disposition and its replacement Go test; run the replacements before deleting the case.
  • AC5: run each listed task and assert exit 0; diff gate configuration for loosening.
  • AC6: read the pull request's Risk section and run mise run lint-pr-description.
  • AC7: read the Verification section against this list; read the leak scan output, git log --show-signature and the labels; read the independent reviewer's approval of the exact head and the disposition of each finding, and the owner's approval of the exact head; open each follow-up issue and assert a Problem and numbered ACs, and git grep for deferred-work markers linking an issue; after merge read the merge commit, the closing reference, the board Status (Done) and the final Status: comment.
  • AC8: count non-generated added and removed lines with git diff --stat excluding generated paths.

Risks

An installed consumer that still sources either file breaks when it disappears, which is why the warning window must close first. A merge ahead of #997 or #856 leaves lib.sh or a commit wrapper sourcing a missing file. Reverting the commit restores both files, their copies and the token and lock entries.

Operator authority

A standing rule names the operator: agent.sh is an owner and golden path and signing.sh is a constitutional path, so the merge needs exact-head approval from the configured merge owner (machine merge_owner). This issue grants no new authority.

Activity

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

    @tbhb-dev-reviews

    Refinement: NOT READY — cutover prerequisites and removal evidence are incomplete

    Checked

    Gaps

    1. Complete the prerequisite graph and AC1. The Port the shared skill library (lib.sh, agent.sh, signing.sh, gates.py) to Go #321-only condition misses dt_commit_context and dt_commit_merge_head (packages/agents/source/lib/agent.sh:62, packages/agents/source/lib/agent.sh:72): Read commit workflow state in Go instead of shell #850 owns their port under Move the commit and agent-commit scripts into agentrepo commit #322 and leaves callers frozen until the later wrapper cutover. Add native blocked-by links for missing port/caller cutovers, including Read commit workflow state in Go instead of shell #850 and the applicable caller-switch child (Turn the commit skill scripts into wrappers over agentrepo commit #856). Add The generator copies the shared libraries from an explicit list #926 as the generator prerequisite, or its superseding Go deliverable if ownership changes. Name the actual prerequisite set in Dependencies and AC1. Record coordination with chore(agents): delete lib.sh after the Go port; cutover 1 under #588 #997 so retaining lib.sh cannot leave its unconditional sources broken (packages/agents/source/lib/lib.sh:935, packages/agents/source/lib/lib.sh:937).
    2. Prove compatibility eligibility. AC1's Verify command proves deletion, not merged prerequisites or a closed warning window. Add a release ledger/check identifying warning-start releases, two published warning releases, the eligible removal release, and linked merge/release evidence. docs/compatibility.md:5 requires warnings before file removal. AC6's deletion-release line alone does not establish that history; retain a validator-compatible Compatibility declaration and record removal timing separately where necessary (docs/compatibility.md:13, docs/compatibility.md:20).
    3. Make AC2/AC3 verification prove the intended outcome. Run generation, token refresh and APM installation in order, inspect packages and both deployments, repeat generation/check-generated-skills, and assert no reappearance or drift. git ls-files alone does not prove regeneration succeeds. Both generators explicitly add these helpers (internal/skills/generate.go:207, tools/generate-skills.sh:114). Include copy-list retirement in scope, using tested Go and only deletion/thin exec changes to shell under Replace complex shell with tested Go, starting with gates and merging #588. Replace AC3's unanchored pattern: it also matches unrelated pr-agent.sh and merge-agent.sh references (packages/agents/source/skills/pr/shared/scripts/check.sh:8, packages/agents/source/skills/merge-pr/shared/scripts/pull-request.sh:165). Use filename boundaries and distinguish executable dependencies from history and absence-test literals; enumerate allowed exceptions consistently with AC3.
    4. Correct AC4's retirement scope. No separate agent.sh/signing.sh suites exist under tests/skills/source/lib/: it contains lib.bats, gates.bats and masked-status.bats. Signing coverage also lives in tests/skills/agent-commit/workflow.bats:176 and tests/skills/agent-commit/workflow.bats:212; tests/skills/source/lib/masked-status.bats:190 copies both helpers for an unrelated retrospective case. Require a case-level retirement ledger naming existing cases and replacement Go tests, retaining unrelated coverage and coordinating shared-file ownership with chore(agents): delete lib.sh after the Go port; cutover 1 under #588 #997. Verify replacements cover those cases before deletion, rather than only naming tests.
    5. Complete Verify with the effective DoD below: exact-head AC evidence, leak scans, signing/provenance, docs and consumer migration steps, coverage floors, size measurement, follow-up disposition and closure. AC5 covers D3; other obligations are absent or partial. Size S is plausible after prerequisite ports; measure the non-generated code delta against the under-800-line bound. No new behavior or shell logic belongs in this cutover.

    Split

    No split required if prerequisite ports and generator work remain in their existing issues. Keep this PR limited to retirement, references, generation metadata and evidence; return newly discovered behavior gaps to the port owners. This is S with authority/compatibility implications, not an XS mechanical exception.

    Definition of done

    Ticks mean the current ACs and Verify plan cover the obligation, not that implementation has completed. Partial coverage remains unchecked.

    • D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link.
    • 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. Covered by AC5 and its Verify row, which prohibit 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.
    • 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 — release-proof and completion checks still need tightening

    Checked

    Gaps

    1. Tighten AC1's Verify to prove merged prerequisites and consumer warnings. The current assertion that dependencies are closed also accepts closure without implementation; check each merged deliverable or explicit superseding deliverable. git tag --contains <merge commit> proves inclusion, not that two published releases actually warned consumers. Link and inspect the warning text in each published release, alongside its tag and eligible removal release. docs/compatibility.md:5 requires warnings for at least two releases. The chosen clock owner Turn the commit skill scripts into wrappers over agentrepo commit #856 currently promises frozen fallbacks but has no AC requiring the two helper deprecation declarations or their release warning; align that prerequisite's contract with AC1, or name a separate warning deliverable. Keep the validator-compatible declaration separate from the removal-release ledger (docs/compatibility.md:13, docs/compatibility.md:20).
    2. Finish the remaining DoD verification in AC7 and its Verify row: inspect independent approval of the exact head and the disposition of each finding (D2); require follow-up issues to contain a Problem and numbered AC and require issue links for deferred work left in code (D9); check that the board shows Done after merge (D10). The current text covers these obligations only partially. Standing owner approval can supply the exact-head approval evidence, but Verify must actually inspect it and finding dispositions.
    3. Replace None under Operator authority with the standing rule requiring the owner's exact-head approval at merge, and include checking it in Verify. The following sentence already recognizes the rule; the leading None contradicts it. agent.sh is golden/owner and signing.sh is constitutional (.agents/agent-skills.conf:4, .agents/agent-skills.conf:5, .agents/agent-skills.conf:9). This issue grants no new authority.

    Split

    No split required. Keep behavior ports and generator redesign in their existing prerequisites. Limit this PR to retirement, necessary references/tests, regenerated metadata and evidence; return discovered behavior gaps to port owners. This is S, not an XS mechanical exception.

    Definition of done

    Ticks mean the current ACs and Verify plan cover the obligation, not that implementation is complete. Partial coverage remains unchecked.

    • D1 The author verified each AC on the merged head, and the Verification section of the pull request cites each AC ID with its evidence: the command and its result, or a link. Covered by AC7 and Verify.
    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up. Add explicit approval and disposition checks.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head. AC5 prohibits loosening and checks gates.
    • 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. Covered by AC7 and Verify.
    • D5 The author signed each commit, the squash included. Covered by AC7 and signature inspection.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why. AC7 requires migration steps or a reasoned no-impact declaration.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label. Covered by AC7 and Verify.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit. AC8 requires a measured non-generated code delta under 800 lines.
    • 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. AC7 omits issue content and code links.
    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit. AC7 omits the board check.
    • 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. Covered by AC5 and Verify.
    • 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. Covered by AC2, Approach and Verify.
    • 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. Covered by AC7 and Verify.

    Round: 2 of 3

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

    @tbhb-dev-reviews

    Refinement: READY — retirement contract covers prerequisites, warning releases and completion evidence

    Checked

    Gaps

    None blocking in the current specification. Merged prerequisite evidence, the release ledger and exact-head approvals remain mandatory execution gates.

    Split

    No split required. Keep this PR to retirement, references, case-level test retirement and regenerated metadata. Behavior ports and generator redesign stay with prerequisite owners. This is S with compatibility and authority implications; the XS single-pass exception does not apply.

    Definition of done

    Ticks mean the current ACs and Verify plan cover the obligation; they do not claim implementation is complete.

    • 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. Covered by AC7 and Verify.
    • D2 An independent reviewer approved the exact merged head, and the author resolved each finding or filed it as a follow-up. Covered by AC7 and approval/finding checks.
    • D3 The required checks pass on that head, and any loosened gate has the owner's approval of that exact head. Covered by AC5 and gate-diff verification; loosening is prohibited.
    • 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. Covered by AC7 and leak-output inspection.
    • D5 The author signed each commit, the squash included. Covered by AC7 and signature inspection, including the squash.
    • D6 The docs match the change (README, docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why. Covered by AC7 requires migration steps or a reasoned no-impact declaration.
    • D7 The commits and the squash have Assisted-by trailers, and the pull request has the provenance:agent-assisted label. Covered by AC7 and trailer/label inspection.
    • D8 The pull request stays within the size limits, or the owner approved its exact head over the hard limit. Covered by AC8 and the measured under-800 bound.
    • 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. Covered by AC7 and follow-up-content/code-link checks.
    • D10 The merge closes the issue through a closing reference and the board shows Done. The final Status: comment names the merge commit. Covered by AC7 and post-merge closure, board and Status checks.
    • 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. Covered by AC5 and Verify.
    • 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. Covered by AC2 and Verify.
    • 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. Covered by AC7 and Verify.

    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