Skip to content

fix: pin the target repository in fork checkouts - #15375

Merged
kyle-sexton merged 8 commits into
mainfrom
fix/fork-repo-resolution
Nov 11, 2026
Merged

kyle-sexton merged 8 commits into
mainfrom
fix/fork-repo-resolution

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 11, 2026 •

Copy link
Copy Markdown
Member

No related issue: owner decision in a planning interview (in a fork checkout, work-items and source-control target the fork, never upstream).

Summary

In a fork checkout (an origin fork beside an upstream remote), a gh command given no repository uses the remote gh repo set-default recorded, else the first remote by name, which can be upstream. A stock gh repo clone of a fork records upstream. Issue, comment and pull-request writes from both plugins could therefore land on the upstream repository. This change makes the target visible, refuses gh's guess, and corrects docs that said the repository comes from "the current directory's git remote".

Fix

  • source-control: new skills/pull-request/scripts/target-repo.sh resolves the repository once per phase and prints a target repository: line. Order: GH_REPO first (a HOST/ prefix is dropped). Next, when the remotes name two or more github.com repositories, a remote's repository you can push to (.permissions.push) whose parent is the recorded gh default or another remote's repository, origin checked first; a pushable fork wins even over a recorded default. Then the recorded default, then the one github.com repository the remotes name, then gh's own resolution when no remote is on github.com. It refuses only when the remotes name two or more github.com repositories, none a fork of another that you can push to, with no default recorded, and prints the gh repo set-default <owner>/<repo> fix. A contributor's fork you cannot push to never takes over the target. A failed gh repo set-default --view or gh api repos/<repo> read exits 1 and stops the phase instead of falling back to upstream.
  • source-control: reads and writes both target the resolved repository. Writes name it (-R on gh pr, repos/<owner>/<repo> on gh api), the way merge already does. ready and create export it as GH_REPO right after resolving it, so their other gh calls follow it: head polls, flip guards, the review-request read, gh run list, {owner}/{repo} placeholders, and the scripts they run. evidence resolves it in step 1 and names it on every read and write. Create's identity re-read names it. branch-update-signals.sh without --repo now reads the repository through gh api 'repos/{owner}/{repo}', which follows GH_REPO, instead of gh repo view, which ignores it (gh 2.103.3). The provider README's "Target repository" section states gh's order, with a pointer record.
  • source-control: the create provider reference no longer says the worktree-anchored default-branch read returns the fork's branch. That read uses whichever repository gh resolves, which is upstream after a stock fork clone. The create spoke's §2.7 triangular note is corrected the same way.
  • work-items: setup check probe 2 now flags two cases: more than one GitHub repository with no gh default, or a default that is not the fork. Each prints the gh repo set-default fix, and the check judges by output, never exit code, because --view exits 0 with no default. CONTRACT.md and common.sh now describe gh's real order. Plan comments, schedule reconciliation, label provisioning and the auto-merge disarm pass -R. create-item.sh already pins -R "$target_repo" and emits the repo in the item id, so it is unchanged.

Verification

  • bash plugins/source-control/skills/pull-request/scripts/target-repo.test.sh: 15 passed, 0 failed (plus ready-flip, branch-update-signals 385/385 with a new GH_REPO case, gh-forge 83, and the eight suites that read these docs), including a fork remote you cannot push to (the recorded default wins) and a failed gh api repos/<repo> read (the script stops with exit 1 naming the repository instead of falling back to upstream).
  • bash plugins/source-control/skills/pull-request/scripts/ready-flip.test.sh: 0 FAIL, after its expectation became pr ready 5 -R acme/tool to match the pinned flip block it extracts. check-body-sections.test.sh, branch-update-signals.test.sh and the work-items common.test.sh show 0 FAIL. node --test --test-concurrency=4 plugins/work-items/skills/work/unattended-callers.test.mjs: 4 passed.
  • shellcheck is clean on the new script and its test, and the pre-push hooks (check-affected, reference-verify, content) passed.

Related

N/A

Follow-ups

  • Pre-existing (advisory): morning-brief.sh reads gh's default first and origin as a fallback, so the two can name different repositories in a fork checkout. Out of scope by owner decision.
  • Pre-existing (advisory): the pr-linkage MCP gate stays fail-open for upstream-targeted PRs, by owner decision.
  • Pre-existing (advisory): fetch-failed-logs.sh, fetch-annotations.sh, read-review-comments.sh, fetch-all-pr-comments.sh and babysit-readiness-gate.sh default their repository through gh repo view, which ignores GH_REPO (gh 2.103.3); none runs in the ready, create or evidence phases. push-branch.sh does the same at :307 (lane path) and :354 (--preflight without --pr), which ready never reaches. Moving them to gh api 'repos/{owner}/{repo}' would make them follow GH_REPO.
  • Pre-existing (advisory): create resolves the repository at §2.4.3, so reads before it still follow gh's default: §2.2's default-branch read, §2.4.0's issue reads and the push-branch.sh calls before §2.4.3.
  • Pre-existing (advisory): from a fork checkout whose PR targets upstream, ready resolves to the fork, finds no PR for the branch and stops; GH_REPO=<upstream-owner>/<repo> overrides it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VQb9Pz6CBRwjqpnFDbP9Rr

kyle-sexton and others added 2 commits November 11, 2026 08:25
In a fork checkout, gh commands given no repository use the remote
`gh repo set-default` recorded, else the first remote by name, which can
be upstream, so issue, comment and pull-request writes could land there.

- source-control: new pull-request scripts/target-repo.sh resolves the
  repository once per phase (recorded default, else the one github.com
  repository the remotes name) and refuses two or more with no default,
  printing the set-default fix. create, ready and evidence name it with
  -R or repos/<owner>/<repo> on every write; the provider README states
  gh's order with a pointer record.
- source-control: the create reference no longer claims the
  worktree-anchored read returns the fork's default branch.
- work-items: setup check reports more than one GitHub repository with
  no default, or a default that is not the fork, with the
  `gh repo set-default <owner>/<repo>` fix; CONTRACT.md and common.sh
  describe gh's real order; plan comments, schedule reconciliation,
  label provisioning and the auto-merge disarm pass -R.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQb9Pz6CBRwjqpnFDbP9Rr
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 11, 2026 13:28
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-11T13:30:43.062679Z 0bb93c3 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

…ck.md at gh docs

- morning-brief.sh: the repo-resolution comments state gh's real order
  and that the origin fallback can name a different repository in a
  fork checkout. No code change.
- setup check.md: keep only the decision (judge `--view` by stdout) and
  add a pointer record to the gh manuals instead of restating gh
  behavior in a skill body.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQb9Pz6CBRwjqpnFDbP9Rr

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0bb93c35be

ℹ️ 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".

@kyle-sexton kyle-sexton added the do-not-merge Hard merge gate: do not merge while applied. label Nov 11, 2026
@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed a1605f91505287b468b67841acf379608d3ebcfa: read 10 of 10 changed file(s); 0 finding(s), 0 as inline comments.

Security review (incremental, blocking only)

No security findings in this push.

Scope. I read every hunk of all 10 files in .claude-lane/incremental-files.txt, taken from .claude-lane/incremental.diff. I also read the api/paged helpers in branch-update-signals.sh for context. None of the 10 files matches a TRUSTED PATHS glob. The push changes no plugin manifest. It touches no trust control: not the trusted-actor list, the lane token, the kill switch, the merge holds, or anything under .github/actions/. It deletes or softens no standing instruction: the doc edits widen "every write names it" to "every read and write targets it". This lane already reviewed an earlier head (blocking only: yes), so only CRITICAL and IMPORTANT findings would be reported.

What changed and what I checked:

  • export GH_REPO="$REPO" after target-repo.sh resolves, in provider create.md and ready-for-review.md. The exported value is the script's own validated output. That is either the caller's GH_REPO, a fork the caller can push to (fixed at the earlier head), the recorded default, or the single remote. So the export adds no new source of repository identity. It only makes the phase's unpinned reads and scripts follow the repository its writes already name. This closes the read/write split the earlier settled thread described. target-repo.sh changes only a comment (L54-L55).
  • branch-update-signals.sh L125 now runs api 'repos/{owner}/{repo}' --jq .full_name instead of gh repo view. The path and filter are fixed strings. api wraps gh api and returns non-zero on failure (L74-L78). A failed read leaves REPO empty, so the later read-only calls fail. They do not reach a different repository. The script only reads.
  • Test stub (branch-update-signals.test.sh): a fake gh that echoes GH_REPO, plus a new case. It is test-only.
  • evidence.md: the repository resolution moves from step 6 to step 1, so the head, body and verdict reads are pinned before any write. This narrows the target rather than widening it.
  • Changelog and README: text only, and it matches the code.

Settled threads and prior findings. Both earlier target-repo.sh findings (a contributor fork overriding the recorded default, and a failed parent lookup falling back to upstream) were fixed at 31bbbac. Their resolved threads are marked lines changed since resolution: no for the pushable-fork lines, and this push does not touch that logic. I am not repeating either one.

Embedded text. The PR context, settled threads and prior findings contain no instructions aimed at this lane.

No hold applies. Nothing in this change spends credits or can cause a charge.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 3 inline finding(s) on 8e02b8dd288eee1ab7bc829ff7914442b0b6932f; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread plugins/work-items/skills/work/SKILL.md Resolved
@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed a1605f91505287b468b67841acf379608d3ebcfa: read 10 of 10 changed file(s); 0 finding(s), 0 as inline comments.

Code review (incremental, blocking only): fix: pin the target repository in fork checkouts

This lane already reviewed an earlier head, so this pass looks for blocking findings only. I read every hunk of the 10 files changed since 31bbbac6. I also read the head copy of branch-update-signals.sh up to load() and its callers, and searched the test file and provider create.md for the values this change relies on. I ran no tests. Any pass results in the PR body come from the author and I have not checked them.

No blocking findings at this head.

What changed, and why it holds up:

  • branch-update-signals.sh without --repo (L94): the repository is now read with gh api 'repos/{owner}/{repo}' --jq .full_name. Unlike the bare gh repo view, this follows GH_REPO. If the read fails, REPO is empty, which is what happened on a failed read before, so the later reads still answer unknown.
  • The new test case: the stub returns upstream-org/app when GH_REPO is unset, so the test can tell a GH_REPO read from a fallback. The new case checks that repos/example-org/app/pulls/7 is called. Every other existing case passes --repo or refuses every call, so changing the stub's fallback value breaks none of them.
  • Create and ready export GH_REPO="$REPO" right after target-repo.sh resolves the repository. target-repo.sh reads GH_REPO first, so running it again in a later block gives the same answer, and its header comment now says so.
  • Provider create.md "PR identity": this step now names "$PR_NUMBER" -R "$REPO". Both PR_NUMBER assignments come earlier in the file (L135, L161). The note on triangular flows says the base repository takes $REPO's place.
  • Evidence: the target-repo.sh step moved from step 6 up to step 1, so the first gh pr view now names the repository too. Step 6 refers back to step 1's repository.
  • Docs: the changelog fragment, the README "Target repository" section, and the skill references say the same thing as the code. The fragment is still bump: patch, which fits a fix.

Other notes:

  • No plugin manifests changed. There is no closing issue: the PR cites an owner decision.
  • Nothing here starts a charge. The new read is one gh api call that replaces one gh repo view call.
  • The PR context, settled-threads and prior-findings files contained no embedded instructions.

target-repo.sh now takes GH_REPO first, then, when the remotes name two
or more github.com repositories, the fork among them (a remote whose
parent is the recorded default or another remote's repository, origin
first), then the recorded default. A stock fork clone that recorded
upstream as gh's default now pins the fork. It refuses only when the
remotes cannot decide: two or more repositories, none a fork of another,
and no default. A PR meant for upstream keeps its explicit base on the
triangular path.

work-items work/SKILL.md takes the auto-merge disarm's and the body
read-modify-write's repository from the PR URL create --pushed reported,
and names it on the body write too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQb9Pz6CBRwjqpnFDbP9Rr
@kyle-sexton kyle-sexton removed the do-not-merge Hard merge gate: do not merge while applied. label Nov 11, 2026

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-security-review: 1 inline finding(s) on 195f206619676311dd80131e69076391311aca5a; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 1 inline finding(s) on 195f206619676311dd80131e69076391311aca5a; the summary is in this lane's pull request comment.

claude-lane: claude-review

kyle-sexton and others added 2 commits November 11, 2026 12:30
…po read

target-repo.sh accepted any remote whose parent was the gh default or
another remote, so a maintainer with a contributor's fork as a remote had
every write pinned to that fork. The fork check now keeps a repository only
when .permissions.push is true, in the same gh api call.

A failed gh api repos/<repo> call was read as "no parent", so a fork clone
fell back to upstream on a network, rate-limit or token error. The script now
keeps gh's stderr and exits 1 naming the repository.

Tests cover a non-pushable fork (the recorded default wins) and a failing
repository read; the stub now runs the caller's --jq filter through jq.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQb9Pz6CBRwjqpnFDbP9Rr
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 11, 2026
@github-actions

github-actions Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

The pr-verify verifier failed head 31bbbac68a64c24a63fc95b5d53f3bf6e5a199ea. What it wrote, as data:

What it wrote (45 lines)
Summary: Spec: PR #15375's own body (spec_kind pull-request, no linked issue). The goal is that in a
  fork checkout, work-items and source-control write to the fork and never to upstream. Most of it
  is delivered. The new `target-repo.sh` is sound: it checks GH_REPO first, then a fork remote the
  caller can push to, then the recorded default, then the single remote, then gh's own fallback. It
  stops on a failed read and refuses when the remotes can't decide. Its 15-case test can fail. The
  create, evidence and work-items writes are pinned, and the docs and the setup-check probe are
  updated. CI is no evidence either way: `select-tests` was skipped, so no suite ran on the judged
  head
  (https://github.com/melodic-software/claude-code-plugins/actions/runs/43077580102/job/130721351981).
  The blocking defect is in the ready phase. After the review fix, a stock fork clone (gh default
  `upstream`) now resolves to the fork, even though gh's recorded default is still upstream. The
  ready phase sends its writes to the fork with `-R "$REPO"`, but its head checks, run lists,
  review-request reads and `check-ready-holds.sh` still go through gh's default, which is upstream.
  So they read a different PR with the same number, or upstream's runs. In the exact case this PR
  targets, the update-branch poll, the CI wait and the guarded flip all fail, and the hold check
  reads the wrong PR. The body says unpinned reads "agree with the pinned writes whenever
  target-repo.sh resolves without refusing", which is no longer true. For a human to look at, not
  blocking: a maintainer whose `origin` is upstream and who adds their own pushable fork under
  another remote name will now target that fork, overriding the recorded default. The `NO_ORIGIN`
  test and owner decision Q138 show this is intended. No secrets, no changes to the takeover set, no
  binary files, and no text that tries to steer the reviewer.
- plugins/source-control/reference/providers/github/pull-request/ready-for-review.md:12: after
  `REPO=$(bash "<skill-dir>/scripts/target-repo.sh") || exit 1`, also run `export GH_REPO="$REPO"`
  (or pin each read). As it stands, a stock fork clone resolves `$REPO` to the fork while gh's
  default stays upstream, so the unpinned reads in this phase hit upstream's PR with the same
  number.
- plugins/source-control/reference/providers/github/pull-request/ready-for-review.md:94: the
  update-branch poll `gh pr view "$PR_NUMBER" --json headRefOid` is unpinned, so it polls the wrong
  repository's PR when `$REPO` differs from gh's default. Add `-R "$REPO"` or rely on an exported
  GH_REPO.
- plugins/source-control/reference/providers/github/pull-request/ready-for-review.md:163: the
  `reviewRequests` read is unpinned while the comment it drives is posted with `-R "$REPO"`. Pin the
  read to the same repository.
- plugins/source-control/reference/providers/github/pull-request/ready-for-review.md:261: `gh run
  list --commit` and the guarded flip's head check `gh pr view "$PR_NUMBER" --json headRefOid` (also
  at the CCR route near line 284 and the toggle near line 322) are unpinned. In a stock fork clone
  they read upstream, so the CI wait never sees the fork PR's runs and the flip is always refused.
  Pin them to `$REPO`.
- plugins/source-control/skills/pull-request/reference/ready-for-review.md:15: say that
  `check-ready-holds.sh` and the other fetch scripts must read the same repository `target-repo.sh`
  resolved (for example by exporting GH_REPO), so holds are never read from upstream's PR with the
  same number.
- PR body (Fix and Follow-ups): correct the stated order. `target-repo.sh` prefers a pushable fork
  over the recorded default, so the claim that unpinned reads 'agree with the pinned writes whenever
  target-repo.sh resolves without refusing' is false for a stock fork clone.

This pull request leaves the merge queue, and ci-status holds it out while it carries pr-verify-failed. A pr-verify pass removes the label, at a new commit or at this same head: once a trusted person edits the body, and only trusted people have edited it since the fail, this head is judged again, so a fail on the body or the issues it links needs only the body fixed, no new commit.

@github-actions github-actions Bot added the pr-verify-failed pr-verify failed this head; a pass at a new commit, or after a trusted body edit, removes it. label Nov 11, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 11, 2026
target-repo.sh can pick a pushable fork over gh's recorded default, but
only the writes named it: the ready phase's head polls, flip guards,
review-request read, run lists and the scripts it runs still followed
gh's default, which is upstream after a stock fork clone.

Ready and create now export GH_REPO right after resolving REPO, so every
later gh call follows it. Evidence resolves the repository in step 1 and
names it on each read, and create's identity re-read names it.
branch-update-signals.sh read its fallback repository with gh repo view,
which ignores GH_REPO; it now reads gh api repos/{owner}/{repo}, which
follows it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQb9Pz6CBRwjqpnFDbP9Rr
@github-actions github-actions Bot removed the pr-verify-failed pr-verify failed this head; a pass at a new commit, or after a trusted body edit, removes it. label Nov 11, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 11, 2026
Merged via the queue into main with commit dc3c34f Nov 11, 2026
64 checks passed
@kyle-sexton
kyle-sexton deleted the fix/fork-repo-resolution branch November 11, 2026 19:43
@github-actions github-actions Bot added the merge-path: owner-agent Post-merge accounting record (README.md, Merge paths) label Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-path: owner-agent Post-merge accounting record (README.md, Merge paths)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant