Skip to content

fix(repository): accept GitHub's spelling of the repository in native draft PR receipts - #3280

Merged
freeqaz merged 2 commits into
mainfrom
fix-2233-2236
Oct 23, 2026
Merged

freeqaz merged 2 commits into
mainfrom
fix-2233-2236

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The registry stores repository names in lowercase (Acme/Widgets becomes acme/widgets), but GitHub answers with the repository's own spelling.

  • Native draft PRs (finding 2236). The broker checks GitHub's draft reply, and each stored receipt it reads back, against https://github.com/<lowercased name>/pull/N exactly (validateDraftProjection). For a repository whose GitHub name has capitals, GitHub creates the draft, the check refuses its html_url, and the create never settles as created. The shipped oce-github pr-create compared the broker's receipt URL the same way. fix(repository): accept GitHub's spelling of the repository in draft PR replies #3278 fixed only the OpenShell client path.
  • OpenShell Git clone URL (finding 2233). Cloning or pushing the mixed-case URL GitHub shows (https://github.com/Acme/Widgets.git) gets no credential. The Sandbox's Git credential helper key and the OpenShell provider endpoint path both use the lowercase name, and Git and OpenShell match that path case-sensitively, so Git stops with could not read Username.

Change

  • Add isSameGitHubRepository and isGitHubPullUrl to client-contracts.ts, the module the controller and the isolated client bundle already share. Only the repository's ASCII letter case may differ; the host, path shape and PR number stay exact, and a non-ASCII letter that lowercases to ASCII (the Kelvin sign) is refused.
  • Use them in validateDraftProjection (GitHub's reply and stored receipts), in the client's native receipt check, and in the OpenShell client check from fix(repository): accept GitHub's spelling of the repository in draft PR replies #3278 (replacing its inline copy). The receipt keeps GitHub's spelling of the URL. Stored receipts carry the lowercase URL, which still matches.
  • 2233: OCE learns GitHub's own spelling only from the token reply, after the provider profile is registered, and the path match itself is in Git and OpenShell. So this documents the supported path in the protected OpenShell Git reference: with repository scope, use the lowercase URL OCE gives the Sandbox (https://github.com/$OCE_OPENSHELL_GIT_REPOSITORY.git, with .git).

Verification

  • New conformance case (repository-credentials-contracts.test.mjs): lowercase, mixed-case and uppercase URLs validate unchanged; another repository, owner, longer path, PR number, host, plain HTTP and a non-ASCII lookalike are refused, as are a trailing slash, a query and number zero. Fails on main (the mixed-case and uppercase cases).
  • New broker mode prepared draft pull: created in GitHub's spelling (postgres-prepared-repository-git-broker.test.mjs): the fake GitHub answers with an uppercase html_url; the shipping client prints that receipt and a repeated send returns the same stored receipt. Fails on main (the operation never settles).
  • Mutants killed: client native check left exact, ASCII check dropped, case-sensitive comparison, PR number not checked.
  • Internal review (high effort): approve with nits; applied (positive PR number in the shared helper, exact clone URL with .git in the docs, extra URL cases). Its one follow-up, the same exact full_name comparison in the metadata reads, is tracked separately.
  • repository-credentials-draft-pr-cli.test.mjs still passes (OpenShell client path).
  • Pinned 20/20 at 2 CPUs (taskset, local): the contracts conformance file, repository-credentials-draft-pr-cli.test.mjs and the full broker file against a prepared PostgreSQL. verify-repository-credentials-boundary.mjs passes.
  • tsc --build (controller), eslint and prettier on the touched files.

Risks

Low. The comparison only widens to ASCII case of the repository name, as GitHub names ignore case; IDs, refs, host and number are unchanged. Docs-only for 2233: a mixed-case clone URL still gets no credential.

… draft PR receipts

The native broker checks GitHub's draft reply, and every stored receipt it
re-reads, against https://github.com/<lowercased registry name>/pull/N. GitHub
answers with the repository's own spelling, so a draft created in a repository
with capitals could never settle as created (finding 2236). The shipped client
compared the broker's receipt URL the same way.

Both checks, and the OpenShell client check from #3278, now share
isGitHubPullUrl and isSameGitHubRepository in client-contracts.ts: only the
repository's ASCII letter case may differ; host, path shape and number stay
exact. Stored lowercase receipts still validate.

Finding 2233: the OpenShell Git route's credential path match is
case-sensitive in Git and OpenShell, and OCE only learns GitHub's spelling
from the token reply after the provider is registered. Document that agents
use the lowercase URL OCE gives the Sandbox.
@freeqaz
freeqaz requested a review from a team as a code owner October 23, 2026 06:42
@clawsweeper

clawsweeper Bot commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

…RL docs

isGitHubPullUrl now also requires a positive PR number, so the native
receipt check gets it too. The docs name the exact lowercase clone URL with
.git and both ways Git can fail. Tests pin a trailing slash, a query and
number zero.
@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. clawsweeper:intake:needs-repro Preliminary submission classification; full source review pending. labels Oct 23, 2026
@clawsweeper

clawsweeper Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes

This PR accepts ASCII case differences in GitHub draft receipt repository names and documents the lowercase clone URL required by protected OpenShell access.

Example: The registered repository is acme/kit and GitHub returns https://github.com/Acme/Kit/pull/8.

  • Before: GitHub creates the draft, but the native broker rejects its receipt and cannot report successful creation.
  • After: The broker validates, stores, and returns the receipt with GitHub's original URL spelling.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) A useful, focused repair with credible component coverage and no concrete defect found; confidence is limited by the absence of a live changed-path trace.
Proof confidence 🦐 gold shrimp (3/6) This collaborator-authored PR is outside the ordinary contributor proof gate; its reported PostgreSQL component coverage exercises production broker and client code against simulated GitHub, and no live-runtime artifact is supplied.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: An Agent receives an uncertain draft outcome even though GitHub successfully created its draft in a repository with capital letters.
Reason: This maintainer-authored repair restores valid native receipts with a narrow shared comparison; the separate mixed-case OpenShell clone limitation is accurately documented.

Merge readiness

✅ Ready for maintainer review

Keep open: main still rejects valid native draft receipts when GitHub capitalizes the repository name, and this PR provides a focused fix with no concrete introduced defect found.

Priority: P2
Reviewed head: fb91f85779412e2bec0ee8ccbd1bfb36f26850c3

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: The PR reports before/after PostgreSQL and client test results, but supplies no live installed oce-github creation trace against GitHub.
Agent review details

How this fits together

The repository credential broker authorizes an Agent's draft creation, validates GitHub's response, persists a safe receipt in PostgreSQL, and returns it to oce-github.

flowchart TB
  A[Agent oce-github client] --> B[Repository credential broker]
  B --> C[GitHub draft creation]
  C --> D[Draft projection validation]
  D --> E[PostgreSQL receipt]
  E --> F[Validated client output]
Loading

Technical review

Best possible solution:

Land the shared receipt comparison fix through the existing broker and client owners while retaining the documented OpenShell clone URL restriction.

Do we have a high-confidence way to reproduce the issue?

Current-main source deterministically rejects https://github.com/Acme/Kit/pull/8 for registered acme/kit; the added tests cover that input, but no target code was executed during this read-only review.

Is this the best way to solve the issue?

The fix uses the existing shared client module and updates both native validators without introducing a parallel receipt path or weakening repository ID and ref checks.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 2b11514b7a82.

Provenance checked

  • Native draft projection and client receipt validation changes intended behavior with a stated reason (3115679: The credential implementation validates and persists an allowlisted draft projection tied to the admitted repository and refs; exact URL spelling accidentally rejects GitHub's canonical capitalization.)
  • OpenShell draft response validation keeps the original intent (2b11514: The earlier fix intentionally accepts repository case differences while preserving host, number, state, and ref checks; this PR consolidates that behavior and explicitly restricts folding to ASCII.)
  • Prepared broker integration scenarios keeps the original intent (3115679: The existing scenarios prove committed receipts, uncertain outcomes, one-create behavior, and withdrawal; the added capitalization mode extends those same invariants.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Labels

Label changes:

  • add P2: Valid draft creation cannot settle for mixed-case repository names, but the failure is limited to the repository credential workflow.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Label justifications:

  • P2: Valid draft creation cannot settle for mixed-case repository names, but the failure is limited to the repository credential workflow.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 23, 2026, 3:02 AM ET / 07:02 UTC.

@freeqaz
freeqaz merged commit 4139972 into main Oct 23, 2026
80 of 81 checks passed
@freeqaz
freeqaz deleted the fix-2233-2236 branch October 23, 2026 07:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant