Skip to content

fix(skills): Resolve the push target from Git remotes - #10

Merged
lyfne123 merged 2 commits into
hw-native-sys:mainfrom
Hzfengsy:resolve-the-push-target-from-git-remotes
Aug 12, 2026
Merged

lyfne123 merged 2 commits into
hw-native-sys:mainfrom
Hzfengsy:resolve-the-push-target-from-git-remotes

Conversation

@Hzfengsy

@Hzfengsy Hzfengsy commented Aug 12, 2026 •

Copy link
Copy Markdown
Member

Summary

Repository identity for the pull-request workflows came from gh repo view,
which resolves the GitHub CLI's base repository rather than the repository a
checkout pushes to. In a fork checkout that also has an upstream remote gh
prefers the parent, so LOCAL_REPO became the upstream, PUSH_REMOTE pointed
at it, ROLE stayed owner, and the fork disappeared from publication,
lookup, and repair alike; when several remotes qualify and no default is set,
gh stops to ask which base repository to use, which turns an unattended run
into a prompt. Identity now comes from the Git remotes through one resolver
shared by the pull-request and issue workflows: a fork checkout pushes to its
fork and targets the parent, and a checkout with nothing writable stops during
setup with the remedy named instead of continuing until Git refuses the push.

Changes

  • lib/github/scripts/repo-identity.sh: new resolver. resolve [--require-push] reads every remote fetch and push URL, pins one host,
    validates each candidate through host-pinned REST metadata, and reports
    github_host, local_repo, base_repo, is_fork, local_can_push, and
    default_branch. With --require-push the write target is chosen by
    evidence: a fork the authenticated account both owns and can push to wins.
    Write access is not ownership — an account can be a collaborator on somebody
    else's fork — so a fork owned by another account is rejected whether it is
    the only writable one or one of several. More than one owned writable fork is
    an error rather than a guess, the base repository wins only when the account
    can push to it, and with nothing writable the resolver fails, naming the base
    repository and every fork remote it rejected.
  • lib/github/setup.md: section 2 consumes that resolver with --require-push
    instead of gh repo view, and no longer derives the host, fork state, or
    default branch itself. LOCAL_REPO is now the verified writable repository,
    so section 3 selects the fork remote as PUSH_REMOTE when a fork exists.
  • lib/github/scripts/issue-context.sh: repository delegates to the same
    resolver and reshapes its output to the unchanged github_host,
    local_repo, issue_repo, default_branch contract, so issue and
    pull-request identity cannot drift. Without push evidence the read-only rule
    is unchanged: a fork remote still identifies the checkout.
  • lib/github/issue-context.md, lib/github/common-issues.md: document the
    delegation and add diagnostics for a checkout with no writable repository and
    for several writable fork remotes.
  • tests/test_repo_identity.py: 15 executable cases over a fake gh and real
    Git remotes — fork beside upstream, upstream-only, unwritable upstream, an
    unwritable foreign fork remote, a sole writable fork owned by another
    account both with and without a writable base, nothing writable, two owned
    writable forks, one owned among several, none owned among several, read-only
    resolution, unrelated hosts, unrelated repositories, no remotes, and usage
    errors.
  • tests/test_portability.py: setup.md may now consume REPO_IDENTITY_HELPER;
    new assertions pin the resolver's presence and executability, the
    --require-push call, the documented selection rule, the issue-side
    delegation, and a repository-wide invariant that no deployable shell calls
    gh repo view.

No interface changes for callers of the skills: setup.md produces the same
context variables, and issue-context.sh repository returns the same JSON
shape. Consumers pick this up when the plugin version they have installed is
updated; this change does not bump the plugin version.

Review feedback addressed in f037c85: ownership is verified in the single-writable-fork case too, so write access to somebody else's fork never selects it as the push target.

Verification

  • python -m ruff check tests: 0, All checks passed
  • python -m ruff format --check tests: 0, 15 files already formatted
  • python -m pyright: 0, 0 errors, 0 warnings, 0 informations
  • git ls-files -z -- '*.sh' | xargs -0 -r -n 1 bash -n: 0
  • python -m unittest discover -s tests: 0, 230 tests OK
  • The same set ran again inside each push transaction's validation runner at
    the pushed commit, and passed both times.
  • repo-identity.sh resolve --require-push in three live checkouts: a fork
    checkout with an upstream remote resolved to the fork with the parent as
    base, a second fork checkout did the same, and a checkout with only an
    upstream remote resolved to that repository with is_fork false.

Setup read repository identity from `gh repo view`, which answers "what is the
GitHub CLI's base repository here" rather than "which repository does this
checkout push to". The two differ exactly where it matters: in a fork checkout
that also has an upstream remote, `gh` prefers the parent, so LOCAL_REPO became
the upstream, PUSH_REMOTE pointed at it, ROLE stayed `owner`, and the fork
disappeared from every pull-request workflow. With no default set and several
remotes qualifying, `gh` instead stops to ask which base repository to use, so
an unattended run turned into a prompt.

Add lib/github/scripts/repo-identity.sh as the single identity resolver for the
pull-request and issue workflows. It reads every remote fetch and push URL,
pins one host, validates each candidate through host-pinned REST metadata, and
selects the write target by evidence: a fork the authenticated account can
actually push to wins, so a remote for somebody else's fork never becomes the
push target; several writable forks resolve only when exactly one belongs to
that account; the base repository wins only when the account can push to it;
and with nothing writable the resolver fails, naming the base repository and
every fork remote it rejected. Setup consumes that result and no longer derives
the host, fork state, or default branch itself.

Delegate `issue-context.sh repository` to the same resolver so issue and
pull-request identity cannot drift, and keep its read-only preference rule:
without push evidence a fork remote still identifies the checkout.

A fork checkout now pushes to its fork and opens the pull request against the
parent. A checkout with no writable repository stops during setup with the
remedy named, instead of continuing until Git refuses the push.

@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: 7465e68680

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

Comment thread plugins/pypto-developer/lib/github/scripts/repo-identity.sh Outdated
The resolver checked ownership only when several writable forks were present.
A single writable fork was selected on push permission alone, so an account
that is a collaborator on somebody else's fork — a maintainer with write
access to a teammate's fork and a writable upstream remote — resolved that
fork as LOCAL_REPO and would have published to the wrong repository, against
the documented guarantee that a remote for somebody else's fork never becomes
the push target.

Resolve the authenticated login whenever any writable fork is present and
require ownership in every case, not only in the ambiguous one. A writable
fork owned by another account is now rejected: the base repository wins when
the account can push to it, and otherwise the failure names the rejected fork
and the account that does not own it. More than one owned writable fork stays
an error rather than a guess.
@lyfne123
lyfne123 merged commit d8d853c into hw-native-sys:main Aug 12, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants