Repository navigation
Turn the commit skill scripts into wrappers over agentrepo commit #856
Description
Problem
After children 01 to 06 the commit logic exists in Go, but the commit and agent-commit skill entry points still run only the shell implementations, agent-commit still runs the sibling commit skill's scripts, and the skill text and PRECONDITIONS.md describe the old layout. Switching the callers is one change, made once, so no entry point is half migrated. The shell is kept as a frozen fallback with no new logic, because devtools bumps must need zero configuration changes through the N-2 compatibility window, and is deleted afterwards.
Part of #322, within #588. Child 07 in #322's Children list. Turns the preflight.sh, commit.sh and review.sh entry points of packages/agents/source/skills/commit/shared/scripts and the three agent-commit scripts into short wrappers that exec agentrepo commit and otherwise run the frozen shell.
Acceptance criteria
- AC1: Given the
commitandagent-commitskill entry-point scripts, when generation runs, then each is a short wrapper that execs the matchingagentrepo commitsubcommand when the binary is present and new enough, and otherwise runs the frozen shell implementation; the frozen shell receives no new logic. - AC2: Given the
agent-commitwrappers and a binary that is present and new enough, when they run, then they callagentrepo commit --unattended, work with onlyagent-commitdeployed (nocommitskill beside it), and do not setDEVTOOLS_UNATTENDEDorDT_UNATTENDED. - AC3: Given the binary is absent or older than the first release that has the subcommand, when a wrapper runs, then it runs the frozen shell and behaves exactly as the scripts did before this change, including the environment variables the shell sets today.
- AC4: Given each affected generated artifact (skill packages, local deployments, token records and lock output), when the change lands, then it is current when produced by the documented generate, measure and deploy sequence, and the generated-skills check passes.
- AC5: Given old and new implementations on the same fixtures, when exit codes, standard output and error, files written and refusals are compared, then they match across preflight, review and commit, attended and unattended, including
--dry-runoutput. Parity compares stdout, stderr and exit status of the shell script and the Go command on the same inputs, except for exactly one warning line on stderr, emitted once per invocation by the Go command and defined in AC10; stdout and exit status stay identical. No other input or behavior difference is authorized, and any difference found is a defect. - AC6: Given the skill text and
PRECONDITIONS.md, when the change lands, then they name theagentrepo commitcommands and the retained fallback, and no longer point at deleted script paths. - AC7: Given the
commitguard hook, when the wrappers change, then the guard still recognizes the wrapper paths and refuses whole-tree staging and hand-written commits, and no guard hook or shared-library source file is edited. - AC8: Given the shared libraries
lib.shandagent.sh, when generation runs after the wrappers change, then the generated copies of both in every skill package and deployment are byte-identical to the copies before this change. - AC9: Non-functional: the Go tests for the commit packages pass on macOS, Linux and Windows in CI.
- AC10: Given the change switches the callers away from
agent.shandsigning.sh, when the pull request is published, then its## Risksection declaresCompatibility: deprecates agent.sh since <version>andCompatibility: deprecates signing.sh since <version>, where<version>is the release that first ships the switch, in the formdocs/compatibility.mdaccepts; and the released callers print a one-line deprecation warning naming the replacement. The warning contract: its fixed text lives in one Go constant in theagentrepopackage, naming the deprecated script,since <release>and the replacement command; the Go command writes it to stderr only, once per invocation, before any other output, with no flag or env switch.
Scope and non-goals
Scope: the skill entry-point sources, their generated output, and the skill text and docs for the commit area. This is the only child that switches callers.
Non-goal: porting or editing guard-git.sh, which the guard-hook migration in #588 owns. Non-goal: deleting the frozen shell, lib.sh, agent.sh or their generated copies; one final cutover issue directly under #588, after the N-2 window and as its own pull request, does that, and because those are constitutional paths that pull request gets the owner exact-head review. Non-goal: deleting Bats cases, which children 08 and 09 own.
Approach
Meet AC1 to AC9 by editing canonical sources only, then running generation, token measurement and deployment in the documented order. Never repair generated copies by hand. Release availability of the binary comes first. Keeping the frozen shell means the generator still finds the shared-library references in the scripts and keeps including the libraries.
Dependencies
Blocked by: #851, #854, #855, #333, #544 (and so #850, #852, #853, and #753 through #850).
Prerequisites: the release carrying every new subcommand is published (#333) and installable on consumers with diagnostics (#544), before any caller is switched.
Size
M. One pull request. Estimated code SLOC with scc: about 45 added wrapper lines plus about 300 lines of skill and reference prose; generated paths excluded by the size rule are the token files and skill copies. The Bats deletions are split into 08 and 09 to stay under the 800 limit.
Verify plan
- AC1, AC2: a test greps each generated entry point for the exec line, the fallback path and the absence of new logic, and a Bats case deploys only
agent-commitwith the binary present and runs it, checking neither variable is set. - AC3: a Bats case with an absent-binary stub and an old-version stub, comparing the run with the pre-change scripts.
- AC4: the documented generate, measure and deploy sequence, then the generated-skills check, with each affected artifact compared after the run.
- AC5: a Go test in
internal/agentrepo, next to the command, runs a parity comparison against the shell on shared fixtures from the earlier children, including--dry-run; it strips exactly the one AC10 warning line from stderr and asserts the remainder, stdout and exit status are byte-identical to the shell output, so any other stderr difference still fails. No Bats case covers it. - AC6: a test reads the skill text for the commands and the fallback, and for dead paths.
- AC7: the existing guard tests run against the new wrappers, and a diff check lists no guard hook or shared-library source path.
- AC8: a checksum comparison of every generated
lib.shandagent.shcopy before and after generation. - AC9: the CI matrix.
- AC10:
mise run lint-pr-descriptionaccepts the declarations, and a Go test ininternal/agentrepo, next to the command, asserts the constant's exact bytes on stderr and its single occurrence. No Bats case covers it. - 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-bytrailer and the pull request has theprovenance:agent-assistedlabel. 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 finalStatus:comment names the merge commit.
Risks
A wrapper against a consumer without the new binary falls back to the frozen shell, so commits there keep working; the fallback cases are the control. The generated output is large and a hand edit fails the check. Undo: revert the change; the previous scripts and generated copies return.
Operator authority
None. This child edits no guard hook, shared-library or other owner-tier path, because the fallback keeps those libraries in place; their deletion is the separate cutover issue named above.
Activity
tbhb-agent commentedon Oct 27, 2026
Refinement: READY — Turn the commit skill scripts into wrappers over agentrepo commit
Model: gpt-6-astra
Child 07 Turn the commit skill scripts into wrappers over agentrepo commit
Verdict: READY
Checked: The binding frozen-fallback decision resolves the earlier compatibility and generated-library conflict. Retained scripts can preserve the references that cause generation to include the libraries; recursive reference detection is at
tools/generate-skills.sh:111. AC8 now requires generatedlib.shandagent.shcopies to remain byte-identical, with checksum verification.The six entry points have one switching owner. Standalone unattended operation is correctly limited to a sufficiently new binary; fallback retains existing behavior. The release/install prerequisites, generation sequence and artifact checks are explicit, consistent with
mise.toml:469and live #333/#544.AC1–AC9 have checks. No fallback-file layout is prescribed. Guard and protected-library edits are excluded, so “None” under authority is consistent with
.agents/agent-skills.conf:9.Gaps: None.
Split: None. M remains plausible with frozen code retained and Bats retirement separated into 08 and 09.
Definition of done
The definition of done for this issue (Feature). Tick each item and give its evidence.
check-leaks.shwith the repository's private-strings list, and the leak review of each post.docs/, CHANGELOG or help text), or the Verification section says "No doc impact" and why.provenance:agent-assistedlabel.Status:comment names the merge commit.mise run checkis green, and so are the coverage floorsmise run cover-go-checkandmise run cover-py-check, whichcheckdoesn't run.packages/agents/source/commits whatmise run generate-skills, thenmise run skill-tokens, thenapm installrewrite, in that order, andcheck-generated-skills(part oflint) passes.docs/agent-skills-migration.mdor the adoption docs, or the PR's Verification says "No consumer impact" and why.Round: 3 of 3
tbhb-dev-reviews commentedon Oct 31, 2026
Refinement: NOT READY — reconcile strict stream parity with the new deprecation warning
Checked:
Gaps:
Split: None required. Resolve the warning contract within this bounded change; retain separate Bats-retirement and post-N-2 deletion issues.
Definition of done
Ticks mean the issue's ACs/Verify plan cover the requirement, not that implementation or merge has occurred.
Round: 2 of 3
tbhb-dev-reviews commentedon Oct 31, 2026
Refinement: READY — the single warning-line exception resolves AC5/AC10
Checked:
internal/agentrepo, so the amendment adds no shell behavior. The six entry points remain thin dispatch wrappers over tested Go, with the existing shell frozen through N-2. This fits Replace complex shell with tested Go, starting with gates and merging #588 and the warning period in docs/compatibility.md:5; declarations use the form at docs/compatibility.md:13.Gaps: None blocking readiness. The warning is the only permitted stream difference; its constant and Go tests are part of this issue's explicit AC10 deliverable.
Split: None. One M-sized PR remains plausible with existing fixture reuse and a small Go warning addition. Keep Bats retirement in #857/#858 and frozen-shell/library deletion in the separate post-N-2 cutover.
Definition of done
Ticks mean the ACs and Verify plan cover these requirements; they do not claim implementation or merge has occurred. The base requirements also apply to the eventual squash.
Round: 3 of 3