Skip to content

Sign in through the GitHub App - #244

Merged
kixelated merged 7 commits into
mainfrom
quest/m0/kix/app-sign-in
Oct 28, 2026
Merged

kixelated merged 7 commits into
mainfrom
quest/m0/kix/app-sign-in

Conversation

@kixelated

@kixelated kixelated commented Oct 28, 2026 •

Copy link
Copy Markdown
Collaborator

GitHub sign-in now uses the registered GitHub App's client credentials, removing the separate OAuth app from operator setup. The manifest declares the production callback, authorization during installation, and Email addresses read permission required for private-email sign-in. The seeded GitHub response models an App token with no OAuth scopes.

Choices made: suppress OAuth-app scopes, and preserve Better Auth's state/cookie checks. A code returned directly from GitHub installation without a sign-in state is discarded; the visitor reaches the existing sign-in form and completes a fresh state-bound OAuth flow. This adds a sign-in click for that entry path. Repository authority still comes from live maintain/admin checks; user tokens never publish work.

Validation: just check and just test passed in the pinned Nix shell (CLI 538 passed, 2 platform skips; Worker 362 passed, including seeded local login). Regression coverage includes private email, expiring App tokens, configured client credentials, empty OAuth scopes, and an installation callback that never exchanges an unbound code. No production secrets, App settings, or deployment were changed. Live installation verification remains in the existing operator setup quest.

(written by Codex (GPT-6))

@kixelated
kixelated marked this pull request as ready for review October 28, 2026 04:36
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T17:26:43.894567Z 9e9ced3 New commits
🔒 Security Review ✅ Completed 2026-10-28T17:29:17.763616Z 9e9ced3 New commits
ℹ️ 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.

@kixelated kixelated left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Automated review by review (OpenAI)

Reviewed commit: b6e9f68.

No actionable bugs found in the changed authentication, manifest, fixtures, and surrounding sign-in/authorization paths. Direction is sound: one App replaces the separate OAuth registration, while repository operations still use scoped installation tokens and live maintainer checks. Discarding an installation callback's unbound code and restarting Better Auth's state-bound flow is a simpler, safer trade-off than adding a second login mechanism; the extra sign-in click is documented.

The regression tests cover empty OAuth scopes, private email, expiring-token responses, configured client credentials, and refusing to exchange a state-less installation code. Verification was static only: I did not run tests or verify real App registration, installation, token expiry/refresh, or production configuration.

@kixelated

Copy link
Copy Markdown
Collaborator Author

Merge summary for head b6e9f68:

  • CI: 7/7 required checks passed.
  • Reviews: the Codex code and security reviews of this head finished with no findings. The automated review posted from the maintainer's account also found nothing actionable.
  • quest check --base origin/main passed (59 documents ok). The PR deletes quest/m0/kix/app-sign-in.md and its links.
  • Decisions from the body were accepted as is: OAuth-app scopes are suppressed, and Better Auth's state/cookie checks stay. An installation callback without a sign-in state drops its code and restarts sign-in, which costs one extra click.
  • Live App installation is still verified in the existing operator setup quest.

No changes were made during merge. Queuing for quest land.

(written by Claude Opus 5.5)

kixelated and others added 2 commits October 28, 2026 09:34
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Merged main into this branch at 4a0ac2e. main has no quest land gate, and #253 refuses to land a head that's behind its base in that case.

  • cloudflare/GITHUB.md conflicted and was resolved by hand. The paragraph keeps main's review-thread and inline-comment webhook subscriptions and adds this PR's Email addresses: read user permission. It drops main's "no user permissions" claim, which this PR makes false.
  • quest/m0/kix/README.md: both sides removed a finished quest link (Merge holds and App sign-in), so both lines are dropped.
  • quest/m0/kix/deploy.md (new commit): the deploy step now configures the production GitHub App's homepage and callback, not the OAuth app this PR removes.

quest check --base origin/main and just check pass. just test passes except two test/agent.test.ts cases, which fail only when the runner's own CLAUDE_CONFIG_DIR leaks in. They pass with it unset, and this PR doesn't touch them. The conflict was resolved by hand, so this head needs a fresh review before it lands.

(written by Claude Opus 5.5)

kixelated and others added 2 commits October 28, 2026 09:41
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Merge summary for head 9e9ced3:

  • Correction: the earlier summary said "Queuing for quest land". That was wrong. main has no quest land gate, so this PR lands through squash auto-merge bound to its head SHA.
  • Auto-merge on 9d0e8ed was disabled after main moved and the PR started conflicting. That way no unreviewed head could merge.
  • Merged origin/main (e542b69) locally. The only conflict was two adjacent link deletions in quest/m0/kix/README.md: this PR removes App sign-in and main removes Context pages. The repository's quest resolve merge driver resolved it, with no hand edits. Since this merge is conflict-free, Codex's no-findings review of 9d0e8ed still covers it.
  • just check and just test pass with CLAUDE_CONFIG_DIR unset.
  • quest check --base origin/main --branch quest/m0/kix/app-sign-in passes (48 documents ok). Its build came from current main and ran outside the head checkout.
  • The decisions from the body still stand as accepted earlier.

(written by Claude Opus 5.5)

@kixelated
kixelated enabled auto-merge (squash) October 28, 2026 17:26
@kixelated
kixelated merged commit c22bd74 into main Oct 28, 2026
6 of 7 checks passed
@kixelated
kixelated deleted the quest/m0/kix/app-sign-in branch October 28, 2026 17:57

This branch was previously deployed

1 inactive deployment
land — 9e9ced3c Deployed Oct 28, 2026 by kixelated via land #6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant