Skip to content

Forward the pr skill entry scripts to agentrepo pr #921

Description

@tbhb-agent

Problem

After children 01 to 11 the Go code carries the pull request behavior, but nothing calls it. The five pr skill entry scripts (validate-description.sh, scaffold-description.sh, preflight.sh, review.sh and create-pr.sh; publish.sh is sourced by create-pr.sh and is not an entry point) still run only the shell. Each must forward to agentrepo pr when the installed binary has the matching behavior, and otherwise run its existing shell body unchanged for the N-2 window, as docs/compatibility.md allows, as the sibling areas under #588 (#321 and #324) do. The shell body is frozen: no new logic goes into it, and its removal after the N-2 window is a later cleanup issue under #588 (owner-tier paths go with the single cutover issue there).

Part of #588 and #323. Changes packages/agents/source/skills/pr/shared/scripts/ entry scripts and regenerates their outputs. The frozen shell, including description.sh, pr-checks.sh, pr-agent.sh, properties.sh and the shared libraries, stays in place as the fallback through the N-2 window, so nothing is deleted and no shared-library copy changes.

Acceptance criteria

  1. AC1: Given a binary that has the matching agentrepo pr behavior, when validate-description.sh, scaffold-description.sh, preflight.sh, review.sh or create-pr.sh runs, then it hands over to that behavior with the same arguments, and the exit status, stdout and stderr are the Go command's.
  2. AC2: Given a binary without the matching behavior, when one of those scripts runs, then the frozen shell body runs unchanged and exactly one added line on stderr says the frozen shell is in use; that line is the only permitted difference from the script's behavior before this change.
  3. AC3: Given a binary that has the behavior and the Go command refuses or fails, when the script exits, then it exits with the Go command's status and does not replay the operation through the frozen shell.
  4. AC4: Given the same fixtures, when the scripts as they were before this change and the forwarding scripts run with the built agentrepo binary that has the behavior, then exit codes, stdout, files written and refusal messages match and stderr matches in its message text; given the forwarding scripts run with a binary lacking the behavior, then all output, files and exit codes equal the earlier scripts' except the one stderr line of AC2; the comparison is recorded.
  5. AC5: Given mise run lint-pr-description on the same draft, when it runs before and after, then its output is unchanged.
  6. AC6: Given mise run regen-generated followed by mise run check-generated-skills, when they run, then both harness packages and both deployed copies are current, the token files and lock are refreshed, and the check passes.
  7. AC7: Given the pull request's diff, when its paths are compared with the golden_paths, owner_paths and constitutional_paths patterns in .agents/agent-skills.conf, then no path matches: no lib.sh, agent.sh, signing.sh, merge-agent.sh, merge-policy.py or guard*.sh source or generated copy is changed.
  8. AC8: Given the documentation, when read, then docs/script-inventory.md and docs/agent-skills-migration.md describe the forwarding scripts, the frozen fallback and the condition for removing it.
  9. AC9: Non-functional: the pr skill guard hooks keep their allowlist behavior unchanged.

Scope and non-goals

Scope: forwarding for the five entry points above and the regeneration of their outputs, plus the two documents.

Non-goal: forwarding for write-pr-description and agent-pr, which child 13 does. Non-goal: removing any shell body, library, generated shared-library copy or the fallback; the removal after the N-2 window is a later cleanup issue under #588 and is not part of this epic. Non-goal: adding logic to the frozen shell beyond the one stderr line of AC2. Non-goal: deleting Bats suites, which children 14 to 16 do. Non-goal: guard-gh.sh or other guard scripts.

Approach

Meet AC1 to AC9 after a parity run: run the old scripts and the forwarding scripts over the Bats fixtures and compare before the forwarders land. Edit the sources and run the documented regeneration sequence; never hand edit the generated copies. Size is counted with the repository's line counter (scc) excluding only the generated output under packages/agents/claude/.apm/skills/, packages/agents/codex/.apm/skills/, .claude/skills/ and .agents/skills/, and tokens.json; the source copy under packages/agents/source/, the Bats cases, apm.lock.yaml and every other path count. The count is recorded in the pull request and stays under 800 code lines and 2,000 prose lines.

Dependencies

Blocked by: #911, #912, #913, #915, #916, #917, #918, #333 (binary distribution reaching consumers), #753 (protection of new Go enforcement paths).

Prerequisite: #321.

Size

S. One pull request. Measured with scc under the exclusions above against the files at origin/main: the five entry scripts hold 978 code lines today, of which only about 10 lines each at the top change (about 50 code lines); the forwarding Bats cases and stub binary are estimated at about 180 lines (three cases per script and the stub); the apm.lock.yaml hash lines for five scripts are about 30; the two documents are about 80 prose lines. The total is about 260 code lines and 80 prose lines, well under 800 and 2,000 (the generated copies are excluded and would add about 250). If the recorded count passes 800 code lines the pull request is split by entry script before it is opened.

Verify plan

  • AC1, AC3: a Bats case with a binary stub that has the behavior, records its arguments and exits non-zero, for each script.
  • AC2: a Bats case with a binary stub lacking the behavior, for each script, checking that exactly one added stderr line appears and the golden-path result of the frozen body is otherwise unchanged.
  • AC4: the recorded parity run over every fixture, cited in the pull request. The supported path runs the earlier scripts and the forwarding scripts with the built agentrepo binary (not a stub) through the real validation, review and publication chain, comparing exit codes, stdout, files written and refusal messages, and stderr message text; the unsupported path runs the forwarding scripts with a binary lacking the behavior, and its diff against the earlier scripts' output is exactly the AC2 line. Capability stubs are used only for the forwarding, argument and status-propagation checks of AC1 to AC3.
  • AC5: mise run lint-pr-description output captured before and after.
  • AC6: mise run regen-generated followed by mise run check-generated-skills, with the lock and deployed diff reviewed.
  • AC7: a diff-path listing, including the generated paths, checked against the three patterns in .agents/agent-skills.conf at origin/main, with no match.
  • AC8: a read of the two documents.
  • AC9: the guard hook tests still pass.
  • 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 deployment that gets a forwarding script before the binary would fall back to the frozen shell, which AC2 and #333 cover; a stub binary that reports the behavior but misbehaves is caught by AC3. Undo: revert the change; the scripts run the shell as before.

Operator authority

None. The shell bodies and the shared-library copies stay, so no path the patterns in .agents/agent-skills.conf protect is changed (AC7). If regeneration were to change one, the change does not proceed in this pull request and goes to the single cutover issue under #588 instead.

Activity

  1. tbhb-agent commented on Oct 27, 2026

    @tbhb-agent
    ContributorAuthor

    Refinement: READY — Forward the pr skill entry scripts to agentrepo pr
    Model: gpt-6-astra

    Child 12 Forward the pr skill entry scripts to agentrepo pr

    Verdict: READY

    Checked: AC4 and its verify plan now require the built agentrepo binary for supported-path parity through the actual validation, review and publication chain. Stubs remain limited to forwarding and capability/failure checks. This covers the behavior exercised by packages/agents/source/skills/pr/shared/scripts/review.sh:67 and packages/agents/source/skills/pr/shared/scripts/create-pr.sh:194.

    The fallback diagnostic exception, N-2 retention, dependencies, per-AC checks, regeneration, protected-path exclusion and undo remain coherent. Retaining shell library references preserves generated library copies under tools/generate-skills.sh:111.

    Gaps: None.

    Split: S remains plausible under the decided generated-output exclusions. The corrected parity oracle adds no implementation scope.

    Definition of done

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

    Round: 1 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:readyPassed the Definition of Ready; may be picked up

    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