Skip to content

fix(repository): accept GitHub's spelling of the repository in draft PR replies - #3278

Merged
freeqaz merged 2 commits into
mainfrom
fix-2232-draft-case
Oct 23, 2026
Merged

freeqaz merged 2 commits into
mainfrom
fix-2232-draft-case

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

The repository registry stores registered names in lowercase (registry.ts: .toLowerCase()), so the OpenShell Git route gives the Sandbox OCE_OPENSHELL_GIT_REPOSITORY=acme/widgets for a repository that GitHub spells Acme/Widgets.

oce-github pr-create --draft posts to /repos/acme/widgets/pulls. GitHub ignores case, creates the draft and answers 201 with its own spelling: html_url: https://github.com/Acme/Widgets/pull/N and head.repo.full_name: Acme/Widgets. The client compared both case-sensitively. The created draft was therefore reported as "Draft not confirmed ... do not submit another create", and the Agent never got the URL or number. Every OpenShell-mode draft create in a registered repository with a capital letter in the owner or name hit this. (Board finding 2232.)

Change

  • client/draft-pr.ts (OpenShell mode): compare the repository part of html_url and both full_name fields without case. Everything else in the reply check is unchanged: status 201, open, draft, number, the /pull/<number> suffix, head ref and base ref. Token issuance already compares names the same way (acquisition-response.ts).
  • repository-credentials-draft-pr-cli.test.mjs: a reply in GitHub's own spelling is confirmed and printed with GitHub's URL. A reply naming another repository in the URL, the head or the base, another host or another pull number is still reported as not confirmed.

Verification

  • The new assertions fail on main (exit 1, "Draft not confirmed") and pass with the fix.
  • Mutants, each killed by the test: drop the URL host, number or repository check; drop the head full_name check; drop the base full_name check; compare case-sensitively.
  • node --test tests/integration/repository-credentials-draft-pr-cli.test.mjs: 20/20 at 2 CPUs.
  • tsc --build, eslint and prettier are clean on the touched files.
  • Internal review: approve with nits. Applied: host and number cases in the test, firmer wording below. Declined: an ASCII-only fold, since the reply comes from GitHub over TLS and GitHub names are ASCII.

Risks

  • Low. The check stays as strict as before about which repository answered; only letter case is ignored, and GitHub repository names are case-insensitive.
  • Out of scope, still open on the board: the Git remote helper key and the provider endpoint path also use the lowercased name, so a clone of the mixed-case URL gets no credential (finding 2233). The native (broker) prepared draft path has the same URL mismatch (prepared-draft-pull.ts validateDraftProjection, and the broker reply check in draft-pr.ts); it is tracked separately (finding 2236), because the fix there must keep stored projections valid.

…PR replies

The repository registry lowercases registered names, so the OpenShell Git
route delivers OCE_OPENSHELL_GIT_REPOSITORY=acme/widgets for a repository
GitHub spells Acme/Widgets. GitHub creates the draft (201) and answers with
its own spelling in html_url and head/base full_name. The oce-github client
compared those case-sensitively, so a created draft was reported as
"Draft not confirmed" and the Agent never got its URL or number.

Compare the repository part of the URL and both full names without case,
as GitHub does and as token issuance already does. Another repository is
still refused.
@freeqaz
freeqaz requested a review from a team as a code owner October 23, 2026 06:13
@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.

Internal review nit: a reply whose URL has another host of the same length,
or another pull number, must still be reported as not confirmed.
@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 case differences in GitHub repository names when confirming an OpenShell draft pull request.

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

  • Before: The created draft is reported as not confirmed.
  • After: The client prints GitHub’s URL and pull request number.

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The patch is focused and appears correct, with useful CLI regression coverage; the available behavior evidence remains harness-based.
Proof confidence 🦐 gold shrimp (3/6) This collaborator-authored PR is outside the ordinary contributor proof gate; supplied verification exercises the real CLI against a simulated TLS GitHub endpoint, without establishing installed OpenShell or live GitHub behavior.
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 does not receive the URL or number of a successfully created draft when GitHub returns capital letters in the repository name.
Reason: This restores the existing draft-confirmation contract, and the collaborator-authored PR establishes owner direction for the narrow repair.

Merge readiness

✅ Ready for maintainer review

Keep this PR open: it narrowly fixes a current-main response-validation bug, and no introduced defect was found.

Priority: P2
Reviewed head: 7613dda94f9bb58795f29bd3e2bd4af2b4860004

Before merge

None.

Findings

None.

Tests

  • Missing end-to-end proof: No supplied evidence shows the changed CLI confirming a real mixed-case GitHub draft through an installed OpenShell route.
Agent review details

How this fits together

The repository CLI sends an admitted draft-create request through OpenShell, validates GitHub’s response, and returns the confirmed draft URL to the Agent.

flowchart LR
Registry[Repository registry] --> Sandbox[Sandbox repository binding]
Sandbox --> CLI[oce-github CLI]
CLI --> OpenShell[OpenShell mediation]
OpenShell --> GitHub[GitHub API]
GitHub --> Validation[Draft response validation]
Validation --> Agent[Agent receives URL]
Loading

Technical review

Best possible solution:

Keep the correction in the existing OpenShell response validator and retain the focused CLI regression cases.

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

Main’s lowercase registry value necessarily fails its exact comparisons against a mixed-case GitHub response; the PR reports a failing-base/passing-head CLI regression, but no tests were run during this read-only review.

Is this the best way to solve the issue?

The patch is a small, complete correction for the stated OpenShell confirmation bug and follows existing case-insensitive repository matching without changing authorization or the separate native receipt path.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 7297355a48bd.

Provenance checked

  • OpenShell draft response validation keeps the original intent (3115679: The merged Credential Proxy work requires scoped grants and conservative confirmation of uncertain draft writes.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

What I checked:

Labels

Label changes:

  • add P2: Mixed-case repository names cause a successful draft creation to be reported as unconfirmed, with limited impact on the repository 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: Mixed-case repository names cause a successful draft creation to be reported as unconfirmed, with limited impact on the repository 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, 2:29 AM ET / 06:29 UTC.

@freeqaz
freeqaz merged commit 2b11514 into main Oct 23, 2026
78 of 79 checks passed
@freeqaz
freeqaz deleted the fix-2232-draft-case branch October 23, 2026 06:33
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