fix: enable private repo subscriptions via OAuth token

[shuhuiluo]

Fixes the issue where users couldn't subscribe to private org repos because validateRepository() was called before checking GitHub App installation. Private repos require the app to be installed before the user's OAuth token can access them.

Root cause:

• validateRepository() with user OAuth token fails (404) for private repos
• This happened before installation check, causing incorrect error flow
• User couldn't complete subscription even after installing the app

Solution - Remove OAuth scope field (GitHub Apps don't use scopes):

• Remove scope column from github_user_tokens schema
• Remove scope parameter from OAuth authorization (caused TypeScript errors)
• Remove scope storage logic (was checking non-existent field)
• Update docs: "OAuth app" → "GitHub App"
• Generate migration with proper snapshots (0000, 0001, 0002)

Solution - Fix installation URL for private repos:

• Add getOwnerIdFromUsername() to fetch owner ID from public profile
• Use real owner ID instead of hardcoded 0 in installation URLs
• Fall back to /installations/new if lookup fails (manual account selection)
• Reorder validation: check installation before user access
• Improve error messages (remove duplication)

Result:

• Private org repo subscriptions now work correctly
• Installation URLs no longer show 404 errors
• Better error handling and user feedback
• Proper GitHub App permission model implemented

Database changes:

• Migration 0002: DROP COLUMN scope from github_user_tokens
• Retroactively created snapshots for migrations 0000 and 0001

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Clearer repository validation errors (distinct 403 vs 404) and improved guidance for org-approval scenarios.

• Improvements

  • Installation-aware subscription flow that picks delivery mode based on installation status and repo visibility.
  • Improved onboarding by surfacing install URLs when installation is required.

• New Features

  • Owner-ID lookup to better direct users to the correct installation target.

• Documentation

  • Clarified GitHub OAuth guidance and permission messaging.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This PR removes the scope column from github_user_tokens via a migration and updates the local schema declaration. It adds three new drizzle meta snapshot files and a journal entry. The OAuth client (src/api/user-oauth-client.ts) now distinguishes 404 vs 403 repo validation errors and exports getOwnerIdFromUsername to resolve owner IDs (org then user). The subscription service (src/services/subscription-service.ts) becomes installation-aware: it records installationId during validation, branches on installation presence/permissions for delivery-mode and installUrl-generation, and changes generateInstallUrl to accept an optional targetId. Minor docs/log text adjustments appear in the GitHub OAuth service.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

• Heterogeneous changes across SQL migrations, multiple large snapshot JSON files, schema types, and non-trivial service logic.
• Areas needing extra attention:
  • src/services/subscription-service.ts: new branching for installation presence, installUrl generation change, and error-path correctness.
  • src/api/user-oauth-client.ts: getOwnerIdFromUsername behavior (org→user lookup) and revised validateRepository error mapping.
  • Database migration & snapshots: ensure removal of scope is consistent with application logic and no lingering references; verify snapshots/journal reflect intended migration ordering.

Possibly related PRs

• refactor: use discriminated union for SubscribeResult type safety #50 — Overlaps changes to src/services/subscription-service.ts around subscription flow and install URL/installation-required handling.
• fix: handle OAuth token revocation and improve error handling #54 — Touches auth and repository validation code (src/api/user-oauth-client.ts and subscription flows) related to owner ID resolution and validation changes.
• feat: add OAuth infrastructure for GitHub user authentication #29 — Directly related to the github_user_tokens schema field scope (previously added/modified in that PR), so it likely intersects with this migration.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'fix: enable private repo subscriptions via OAuth token' directly aligns with the primary objective of fixing private repository subscriptions, which is the main change across all modified files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/private-repo-subscription

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/services/github-oauth-service.ts (1)

45-63: Updated GitHub App token docs look good; minor wording nit around 403 logging

The new docstring accurately describes the GitHub App user access vs installation tokens and how this service uses them, which is helpful given the mixed auth model.

One small follow‑up you might consider (non‑blocking): validateToken’s 403 log still says “insufficient scope, rate limit, or lockout”, but scopes are no longer central here; renaming that to something like “insufficient permissions, rate limit, or lockout” would better match the GitHub App framing you describe above.

src/api/user-oauth-client.ts (1)

103-132: getOwnerIdFromUsername helper is robust with a safe fallback

The org‑first then user fallback strategy and outer try/catch give you a good chance of resolving the owner ID, while returning undefined on failure cleanly supports the “no target_id” install URL path. The console warning on failure should be enough for debugging without impacting user flow.

If this ends up on a hot path, you might later consider caching owner IDs to avoid repeated lookups, but it’s not necessary for correctness.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f9a13a4 and 20842ac.

📒 Files selected for processing (9)

• drizzle/0002_zippy_celestials.sql (1 hunks)
• drizzle/meta/0000_snapshot.json (1 hunks)
• drizzle/meta/0001_snapshot.json (1 hunks)
• drizzle/meta/0002_snapshot.json (1 hunks)
• drizzle/meta/_journal.json (1 hunks)
• src/api/user-oauth-client.ts (2 hunks)
• src/db/schema.ts (0 hunks)
• src/services/github-oauth-service.ts (1 hunks)
• src/services/subscription-service.ts (3 hunks)

💤 Files with no reviewable changes (1)

• src/db/schema.ts

🧰 Additional context used

🧬 Code graph analysis (1)

src/services/subscription-service.ts (1)

src/api/user-oauth-client.ts (3)

• RepositoryInfo (6-14)
• validateRepository (32-76)
• getOwnerIdFromUsername (110-132)

🔇 Additional comments (8)

drizzle/0002_zippy_celestials.sql (1)

1-1: Migration dropping github_user_tokens.scope is consistent with schema changes

The migration cleanly removes the scope column and aligns with the updated model and snapshots; no dependent indexes/constraints are present, so this looks safe.

drizzle/meta/0002_snapshot.json (1)

245-333: Snapshot 0002 correctly reflects removal of github_user_tokens.scope

The 0002 snapshot’s github_user_tokens definition matches the migration (no scope column) and keeps existing PK/unique index structure intact, so the snapshot looks consistent.

drizzle/meta/_journal.json (1)

18-24: Journal entry for 0002_zippy_celestials is well-formed

The added journal entry (idx 2 / tag 0002_zippy_celestials) is consistent with the new migration and snapshots and keeps the index sequence contiguous.

drizzle/meta/0000_snapshot.json (1)

1-624: Baseline snapshot 0000 is consistent and self-contained

This snapshot cleanly captures the initial schema (including github_user_tokens.scope) and matches the evolution implied by later snapshots and migrations.

drizzle/meta/0001_snapshot.json (1)

1-655: Intermediate snapshot 0001 matches the expected schema evolution

The 0001 snapshot accurately reflects the schema (including scope) with the additional constraints/indexes and forms a coherent step between 0000 and 0002.

src/api/user-oauth-client.ts (1)

54-72: Wrapped 404/403 errors with cause improve upstream diagnostics

Using new Error(message, { cause: error }) for 404/403 along with more specific messages is a solid improvement and enables callers (like the subscription service) to inspect cause.status without losing the original error.

Just make sure your runtime/TS target supports Error options and cause (e.g., Node >= 16 and appropriate lib settings), so this compiles and behaves as expected.

src/services/subscription-service.ts (2)

136-183: Reordered install check and refined 404/403 handling correctly align with GitHub App behavior

Checking installationId before calling validateRepository, then using error.cause?.status lets you distinguish “private repo without installation” (404 + no installation) from other failures and return a requiresInstallation result with a targeted install URL via getOwnerIdFromUsername. The additional 403/org detection based on repoIdentifier vs githubUser.login also reads well.

One minor thing to keep in mind: for a genuinely non‑existent repo you’ll still surface a “might need installation” path on 404 + no installation, but that ambiguity already exists at the API level and your generic error message reflects it. Overall the flow looks sound for the private‑org‑repo case this PR is fixing.

---

457-467: Optional targetId install URL generation matches new call sites

Allowing generateInstallUrl to omit targetId and fall back to the base installations/new URL fits the new error path where getOwnerIdFromUsername can return undefined. The existing call sites that pass a concrete owner ID still get the .../permissions?target_id=... variant, so behavior remains correct while adding a safe fallback.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/services/github-oauth-service.ts (1)

46-63: Doc updates correctly reflect GitHub App model; consider clarifying the “does NOT read repository contents” guarantee

The expanded comment nicely distinguishes user access vs installation tokens and aligns with the new permission model. To avoid suggesting a GitHub‑wide guarantee, you might want to phrase that bullet as an app‑level usage guarantee rather than a capability guarantee, e.g.:

- * - Does NOT read repository contents or receive webhook events
+ * - In this app, not used to read repository contents or receive webhook events

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20842ac and ba12209.

📒 Files selected for processing (1)

• src/services/github-oauth-service.ts (2 hunks)

🔇 Additional comments (1)

src/services/github-oauth-service.ts (1)

364-369: 403 handling message now correctly avoids “scope” terminology

Updating the 403 comment and log from “insufficient scope” to “insufficient permissions” keeps the behavior the same while matching the new GitHub App permission model and removal of scopes from storage. No further changes needed here.