Skip to content

Forward the write-pr-description, pr check and agent-pr entry scripts to agentrepo pr #922

Description

@tbhb-agent

Problem

The write-pr-description context script, the pr skill's check.sh and the four agent-pr wrappers still run only the shell, and the agent-pr wrappers reach into the sibling pr skill directory (exec bash "$here/../../pr/scripts/preflight.sh"), so a deployment with only agent-pr installed cannot run its scripts. The agent-pr manifest also names the pr skill's guard hook, pr/scripts/guard-gh.sh, which is a protected path this change cannot copy or move. With children 10 and 11 in Go, these six entry points must forward to agentrepo pr directly when the installed binary has the behavior, and otherwise keep their frozen shell behavior through the N-2 window, as docs/compatibility.md allows and as the sibling areas under #588 (#321 and #324) do. The frozen shell gets no new logic, and its removal is a later cleanup issue under #588.

Part of #588 and #323. Changes packages/agents/source/skills/write-pr-description/shared/scripts/context.sh, packages/agents/source/skills/pr/shared/scripts/check.sh and packages/agents/source/skills/agent-pr/shared/scripts/ (check.sh, preflight.sh, publish.sh, review.sh), and regenerates their outputs. The frozen shell stays as the fallback and nothing is deleted.

Acceptance criteria

  1. AC1: Given a binary that has the matching agentrepo pr behavior, when the four agent-pr wrappers run, then each hands over to that behavior with the same arguments and unattended mode set as today, and the exit status, stdout and stderr are the Go command's; given context.sh or the pr skill's check.sh runs, then each hands over with the same arguments and the invocation mode it has today (neither sets unattended mode itself), and check.sh still accepts no argument and refuses any with its existing usage message.
  2. AC2: Given a consumer with only the agent-pr scripts deployed and a binary that has the behavior, when each wrapper runs, then it works without the pr skill's scripts beside it. This promise covers script execution only; the manifest's guard hook dependency is AC6.
  3. AC3: Given a binary without the matching behavior, when any of the six entry points runs, then its frozen shell body runs unchanged, including the wrappers' use of the sibling pr skill, and exactly one added line on stderr says the frozen shell is in use; that line is the only permitted difference from the entry point's behavior before this change.
  4. AC4: Given a binary that has the behavior and the Go command refuses or fails, when any of the six entry points exits, then it exits with the Go command's status and does not replay the operation through the frozen shell.
  5. AC5: Given each entry point, when its arguments are forwarded, then its existing argument contract is unchanged (the publication wrapper still refuses --confirmed; the preflight and review wrappers forward their arguments as today; check.sh takes none).
  6. AC6: Given the pr and agent-pr skill manifests, when read, then they name the forwarding entry points, and the agent-pr manifest's guard hook still references pr/scripts/guard-gh.sh byte for byte as before, so the skill as a whole keeps its dependency on the pr skill's guard until the guard cutover; the guard's allowlist behavior is unchanged.
  7. AC7: 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.
  8. AC8: Given the same fixtures, when the entry points as they were before this change and the forwarding entry points 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 entry points run with a binary lacking the behavior, then all output, files and exit codes equal the earlier ones' except the one stderr line of AC3; the comparison is recorded.
  9. AC9: 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.
  10. AC10: Non-functional: the writer's scoped permissions in the write-pr-description manifest are unchanged, and the optional writer still works when the helper is not deployed.

Scope and non-goals

Scope: forwarding for the six entry points above (context.sh, the pr skill's check.sh and the four agent-pr wrappers), script-level standalone execution of agent-pr for binaries that have the behavior, manifests and regeneration.

Non-goal: making the agent-pr skill manifest independent of the pr skill's guard hook: copying, moving or editing guard-gh.sh is a protected-path change that goes to the single cutover under #588 with the owner exact-head review, and its handoff follows #753 (heightened agent review, not owner paths). Non-goal: the five other pr entry points, which child 12 forwards. Non-goal: removing any shell body, the sibling-skill path in the frozen wrappers, the fallback, or any shared-library copy; 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 AC3. Non-goal: new unattended behavior, which child 10 owns, or any unattended mode for the read-only writer context. Non-goal: deleting Bats suites, which children 14 to 16 do.

Approach

Meet AC1 to AC10 after a parity run over the Bats fixtures. For AC2 and AC3 the fixtures deploy only the agent-pr output, once with a binary stub that has the behavior and once with a stub that lacks it. Edit the sources and run the documented regeneration sequence; never hand edit 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, the manifests 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: #919, #920, #921, #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 six entry scripts hold 146 code lines today, of which only about 10 lines each change (about 60 code lines); the forwarding Bats cases, the stub binary and the standalone agent-pr fixture are estimated at about 220 lines; the apm.lock.yaml hash lines for six scripts are about 36; the manifest edits are about 40 prose lines. The total is about 320 code lines and 40 prose lines, well under 800 and 2,000 (the generated copies are excluded and would add about 300). If the recorded count passes 800 code lines the pull request is split between the agent-pr wrappers and the other two entry points before it is opened.

Verify plan

  • AC1, AC4, AC5: a Bats case with a binary stub that has the behavior, records its arguments and the unattended setting and exits non-zero, for each of the six entry points, including --confirmed on publication, an argument on check.sh, and an assertion that context.sh and check.sh do not set unattended mode.
  • AC2: a Bats case deploys only the agent-pr scripts with that stub and runs each wrapper.
  • AC3: a Bats case deploys agent-pr with the pr skill and a stub lacking the behavior, for each of the six entry points, checking that exactly one added stderr line appears and the frozen result is otherwise unchanged.
  • AC6: a read of the manifests, plus a diff showing the guard hook line of the agent-pr manifest and the guard test results are unchanged.
  • AC7: mise run regen-generated followed by mise run check-generated-skills, with the lock and deployed diff reviewed.
  • AC8: the recorded parity run over every fixture, cited in the pull request. The supported path runs the earlier entry points and the forwarding entry points with the built agentrepo binary (not a stub), so context output and the unattended workflow are compared for real; the unsupported path runs the forwarding entry points with a binary lacking the behavior, and its diff against the earlier entry points' output is exactly the AC3 line. Stubs are used only for the forwarding, argument, unattended-setting and failure checks of AC1 to AC5.
  • AC9: a diff-path listing, including the generated paths, checked against the three patterns in .agents/agent-skills.conf at origin/main, with no match.
  • AC10: tests/skills/writers and the manifest permission checks 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 wrapper before the binary falls back to the frozen shell, which still needs the sibling pr skill; AC3 shows that case works, and #333 covers delivery. Until the guard cutover a consumer that deploys the agent-pr skill still needs the pr skill's guard hook, even though the scripts run alone (AC2, AC6). Undo: revert the change; the entry points 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 (AC9). 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 write-pr-description, pr check and agent-pr entry scripts to agentrepo pr
    Model: gpt-6-astra

    Child 13 Forward the write-pr-description, pr check and agent-pr entry scripts to agentrepo pr

    Verdict: READY

    Checked: AC8 now requires the built binary to compare actual context output and unattended workflow behavior. Relevant behavior remains grounded in packages/agents/source/skills/write-pr-description/shared/scripts/context.sh:87 and packages/agents/source/skills/pr/shared/scripts/pr-agent.sh:45.

    AC2 explicitly limits independence to script execution. AC6, scope and risks retain the manifest’s guard dependency until cutover, with verification of the unchanged hook reference and guard behavior. This matches packages/agents/source/skills/agent-pr/SKILL.claude.md:10 and packages/agents/source/skills/pr/claude/scripts/guard-gh.sh:54. Guard work stays outside this child’s protected-path exclusion and size estimate.

    Dependencies, argument contracts, verification coverage and rollback remain adequate.

    Gaps: None.

    Split: S remains plausible for the six forwarders and manifest updates; guard cutover remains separate.

    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