Repository navigation
refactor(source-control): move pull-request and worktree spoke GitHub commands to provider files - #7170
Conversation
… commands to provider files The pull-request and worktree spokes now name each forge operation and link the GitHub command, which lives in a per-spoke provider file under reference/providers/github/pull-request/ or a row in the provider README (ADR 0060, containment part 3). The four GitHub scripts carry a portability-scope declaration. No command an agent runs is dropped. Refs: #6941 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 3m 0s —— View job Security reviewRan
No security issues found. This PR is a pure documentation reorganization: it moves Because this is exactly the shape of change the instruction-surface-deletion lens exists for (prose rewritten, commands relocated), I diffed the highest-stakes blocks directly rather than trusting the PR's own verification claims:
No new injection surface, credential handling, or trust-boundary change: the moved |
Reviewing PR #7170
|
|
Claude finished @kyle-sexton's task in 3m 38s —— View job Code review complete
No blocking findings. This is a mechanical refactor (move GitHub commands out of spoke files into provider files / README rows, replace them with neutral descriptions + links), and it holds up under inspection:
One minor, non-blocking observation: Not independently re-verified (author-claimed in the PR body, not rechecked here): the gh-pattern lint counts, the 370-span behavior-preservation script, the plugin-eval before/after comparison, and the various test-suite pass counts. Take those as author-reported. |
|
claude-security-review has reviewed this pull request through de1d4b1; a later push is reviewed from there. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ec19e3dc67
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Claude finished @kyle-sexton's task in 3m 4s —— View job Security review complete
No security issues found. This PR is a pure documentation/reference reorganization: GitHub commands, REST scripts, and
Scope note: GitHub Actions workflow hardening (triggers, |
|
claude-review has reviewed this pull request through de1d4b1; a later push is reviewed from there. |
Resolve merge.md against #7170, which moved the phase's gh commands into the GitHub provider file: the phase prose stays provider-neutral and the HEAD_SHA pin, --match-head-commit and the confirm-queue-entry script move to reference/providers/github/pull-request/merge.md. Also, per the owner's direction on #6953: poll mergeStateStatus while it reads UNKNOWN before any merge call in 4.2 and 4.2.1, merging or enqueueing only on CLEAN; and split the enqueue into a read that prints the PR node id and a separate enqueuePullRequest call with the node id and head written as literals, so the guardrails merge guard can resolve the PR. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVW9hAwnSBrnFsR3kzXcB4
1 participant
Refs: #6941
Summary
The
pull-requestandworktreespoke files now name each forge operation and link its GitHub command instead of carrying it inline (ADR 0060, containment part 3, spokes half). The commands, REST scripts andghfacts moved into per-spoke provider files underplugins/source-control/reference/providers/github/pull-request/, or into rows of the provider README. Every rule, threshold, ordering and prohibition stays in the spoke. Hits of the stagedghpattern across the 18 in-scope files went from 154 to 0, and none needed aportability-ok. The issue stays open for the lint decision.Fix
Provider files mirror spokes. New
reference/providers/github/pull-request/{create,ready-for-review,monitor,readiness,merge}.md. Each one opens with a line linking back to its spoke and uses stable headings that the spoke links by anchor. Blocks that are GitHub procedures moved byte-for-byte, with only leading indentation removed where they sat inside a list: the Monitor REST poll script, the queued/running jobs read, the merge-after-AI-review script, the merge-queue poll, the async merge API call, the no-trackerCloses #Nblock, the REST PR-create forms (same-repo, out-of-tree, triangular), and the base-refresh block. CLI and API facts moved with their commands and kept their as-of and recheck lines: GraphQL vs REST rate budgets,--watchinterval,--log-failedtruncation, the cli/cli#14007 linked-worktree delete, the template override, and the sandboxed-session proxy evidence.Call sites. Each spoke step now says what the operation reads or does and links
[GitHub](...#anchor). Where a moved block ran as a script (the Monitor poll script, the merge-after-AI-review script), the spoke states its contract: inputs, what it emits, and when it holds or exits. Command-naming prohibitions are now neutral ("never a blocking checks watch", "never a bare PR view, checks or merge call"), and the literal flags are in the provider file. Flags that are themselves the rule (--auto,--admin,--match-head-commit) stay in the spoke, because they do not match the pattern.README rows instead of files for the one-line operations: default branch (CLI and REST), "a branch's PR merged" (worktree cleanup),
delete_branch_on_mergewith its command. Existing rows cover the worktree status batch read, the PR checkout inwatch-handoff.md, and reply/reaction surfaces. The README paragraph that said the spokes "still carry their full GitHub procedures" now describes the per-spoke provider files. It says the worktree spokes need only the rows, and that babysit-prsloop.md/safety.mdstill carry theirs (another PR owns those).Declared, not moved:
fetch-failed-logs.sh,fetch-annotations.sh,read-pr.shandpush-branch.share GitHub scripts end to end, so each gets one# portability-scope:comment line and no behavior change. The line sits after the header block's first blank line rather than on line 2:--helpprints lines 2 through that blank line, so line 2 would have changed the help output.Registry pin satisfied without a literal. The B2 lane-productivity clause in
readiness.mdrequiresgh pr checks|checks roster. It now says "the checks roster (the PR's checks list)", which matches the second alternative, so noportability-okwas needed.Verification
Staged gh pattern (
SKILL_PORTABILITY_TOKENS=.work/gh-token.txt bash scripts/check-skill-portability.sh --paths …), before → after:monitor.md39 → 0,merge.md33 → 0,create.md30 → 0,readiness.md16 → 0,ready-for-review.md13 → 0,watch-handoff.md1 → 0,reviewer-shapes.md1 → 0,native-surfaces.md1 → 0,templates/checklist.md4 → 0,fetch-failed-logs.sh10 → 0,fetch-annotations.sh10 → 0,read-pr.sh6 → 0,push-branch.sh2 → 0, worktreecleanup.md3 → 0,status.md1 → 0,audit.md1 → 0. Total 154 → 0.Behavior preservation (scripted): 370 distinct
gh <subcommand> …spans underplugins/source-control/onorigin/main. All but 4 still appear verbatim (whitespace-normalized) underplugins/source-control/. The 4 that do not: two prose fragments, not commands ("gh release adds an option to", the "via direct gh api" comment in the Tier 2 log-fetch comment); a prose mention ofgh pr view --json mergeable, whose command formgh pr view <N> --json mergeable,mergeStateStatusis in the provider file; and--jq .delete_branch_on_merge, now the README's equivalent quoted form--jq '.delete_branch_on_merge'.Every new relative link and
#anchorresolves (scripted against GitHub heading slugs): 0 bad.python3 scripts/check-contract-clause-coverage.py: passed.bash scripts/check-skill-portability.sh origin/main: no unexcused coupling tokens.bash scripts/check-changed-skills.sh origin/main: 0 failed.bash scripts/affected-tests.sh --run: shell suites passed;test_guards.pyis reported as its own lane and was run separately.babysit-prs
tests/test_skill_contract.py+tests/test_guards.py: 50 passed, 214 subtests.fetch-annotations,fetch-failed-logs,read-pr,push-branchandcheck-body-sections.test.sh: pass.scripts/check-loop-lane-floor-drift.sh --check(watch-handoff is a registered consumer): 7 consumers match.markdownlint-cli2 and typos on the 19 changed markdown files, shellcheck on the 4 scripts,
check-purged-em-dashes.sh: clean.Before/after
claude plugin evalon the full source-control suite: before =origin/main(ec78703), after = this PR merged onto that main, run at the same time with identical flags (--allow-tools Bash Write Edit --ablation none --runs 3 --judge-model opus --scaffold --keep-temp), Claude Code 2.1.296, modelclaude-opus-5-5in 75 of 75 traces per version. Paired over 25 cases: change +0.01, 95% interval -0.01 to +0.04,compare verdict: non-inferior at margin 0.05. Mean case score 0.785 before, 0.797 after. No case dropped by a third or more; the largest drop,resolve-conflicts-refuses-bulk-ours-and-abort0.53 to 0.40, is in a skill this PR does not touch.Limits: the validity gate marks both versions INVALID for the same reasons as on refactor(source-control): neutral operation wording, GitHub commands under reference/providers/github #7042 (the skill did not fire in 17 with-arm runs before and 15 after, in cases whose descriptions this PR does not change; 2 and 3 permission denials), so the absolute scores are not a valid measurement of the skills; the paired comparison ran under identical conditions.
pull-request-ready-hands-pr-to-explain-changedid not load in either version: itsplugins: ../../../reviewentry resolves outside the CLI's containment root when the plugin is evaluated from a copy, soready-for-review.md's hand-off to explain-change is not measured. Every judge vote was unanimous. A first pair run without--allow-toolswas discarded: Bash was denied at the skills' first!pre-computed line in both arms, so no case reached a spoke.Related
merge.md) and fix(source-control): read pull-request facts and diffs over REST in view #7081 (read-pr.sh) touch the same files; expect merge conflicts there.loop.md/safety.mdstill carry their procedures.🤖 Generated with Claude Code