Skip to content

feat(worker): Phase-2 signed-in tier — provision + fix OAuth callback prefix#7

Merged
OriNachum merged 2 commits into
mainfrom
feat/learn-signin-phase2
Jul 11, 2026
Merged

feat(worker): Phase-2 signed-in tier — provision + fix OAuth callback prefix#7
OriNachum merged 2 commits into
mainfrom
feat/learn-signin-phase2

Conversation

@OriNachum

Copy link
Copy Markdown
Contributor

What this is

Phase 2 — the signed-in tier of agentculture.org/learn. Provisioned and
deployed onto the same worker + /learn/* route that Phase 1 stood up, so
the signed-out experience is unchanged and sign-in now lights up.

Provisioned (with the now-broadened .env Cloudflare token — +Workers KV, +D1):

  • KV namespace SESSIONS (device-flow + session-revocation).
  • D1 learn-ledger with the schema applied to the remote DB (learners +
    records).
  • GITHUB_CLIENT_ID (public) in wrangler.toml; SESSION_SECRET (I
    generated) + GITHUB_CLIENT_SECRET set via wrangler secret put — never
    committed.

The GitHub app is a GitHub App (client id Ov23li…); the worker's OAuth
module already uses the shared /login/oauth/* + api.github.com/user
endpoints, so no code change was needed for that — confirmed by a live
device-flow start returning a real user_code.

Real bug this caught

handleLogin built redirect_uri from the bare origin
https://agentculture.org/api/auth/callback — dropping the /learn
zone-mount prefix. Two failures:

  1. GitHub rejects the web flow (redirect_uri ≠ the app's registered
    …/learn/api/auth/callback).
  2. Even if it didn't, that bare callback path isn't covered by this worker's
    route (agentculture.org/learn/*) — it would hit org's Pages site, not the
    API.

Fix: new callbackUrl() derives the callback from APP_URL (the mount root),
matching the callback handler's own dest logic. Regression test asserts
redirect_uri == https://agentculture.org/learn/api/auth/callback.

Verification

  • Worker suite 40/40 (was 39 + the new regression).

  • Live, post-deploy, against production:

    • signed-out intact (/learn/ 200, /api/health 200);
    • /api/me with no session → 401 (not 500 — KV/D1 bindings resolve);
    • /api/auth/login → 302 with redirect_uri = …/learn/api/auth/callback;
    • POST /api/auth/device {start} → real GitHub user_code.
  • A human end-to-end sign-in (web flow) + org Learn-nav merge are the last
    cutover steps, tracked outside this PR.

  • learn-cli (Claude)

Provisions and deploys the signed-in half of agentculture.org/learn on
the same worker/route:

- wrangler.toml: real KV (SESSIONS) + D1 (learn-ledger) ids and the
  GitHub client id (public). Secrets (SESSION_SECRET,
  GITHUB_CLIENT_SECRET) are set via `wrangler secret put`, never here.
- D1 schema applied to the remote ledger (learners + records).

Fixes a real OAuth bug the live smoke test caught:

- handleLogin built redirect_uri from the bare origin
  (https://agentculture.org/api/auth/callback), dropping the /learn
  zone-mount prefix. GitHub rejects the web flow on redirect_uri
  mismatch, and that bare callback wouldn't even route back to this
  worker (agentculture.org/learn/*) — it would hit org's Pages site.
  New callbackUrl() derives the callback from APP_URL (the mount root),
  matching the callback handler's own dest logic. Regression test
  asserts redirect_uri == https://agentculture.org/learn/api/auth/callback.

Device flow verified live end-to-end (GitHub returned a real user_code);
web login redirect_uri now correct in production. 40/40 worker tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jkvh4uxSi6AsBJYPjiurze
@OriNachum

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Provision signed-in Phase-2 learn worker and fix /learn OAuth callback URL

✨ Enhancement 🐞 Bug fix ⚙️ Configuration changes 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Provision Phase-2 signed-in tier on the existing /learn/* worker route (KV + D1).
• Fix GitHub OAuth redirect_uri to include the /learn mount prefix via APP_URL.
• Add regression test and bump CLI version + changelog entry.
Diagram

graph TD
  B(("Browser")) -->|"/learn/api/auth/login"| W["learn-api Worker"] -->|"OAuth authorize"| GH{{"GitHub OAuth"}}
  B(("Browser")) -->|"/learn/* (site)"| W["learn-api Worker"] -->|"proxy"| P[["Cloudflare Pages" همچ]]
  W["learn-api Worker"] --> KV[("KV: SESSIONS")]
  W["learn-api Worker"] --> D1[("D1: learn-ledger")]

  subgraph Legend
    direction LR
    _u(("User/Client")) ~~~ _svc["Service/Worker"] ~~~ _ext{{"External"}} ~~~ _db[("Data store")] ~~~ _static[["Static origin"]]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Derive callback from request URL + known mount prefix
  • ➕ Eliminates dependence on APP_URL for correctness
  • ➕ Works automatically across preview/staging domains
  • ➖ Requires carefully handling multiple hostnames/proxies and enforcing canonical public host
  • ➖ More risk of accidentally generating an unregistered redirect_uri in production
2. Register a bare-origin callback and add a second worker route
  • ➕ Avoids /learn prefix complexity in OAuth redirect_uri
  • ➕ Callback path is shorter and conventional
  • ➖ Requires owning an additional zone route outside /learn/*
  • ➖ Increases surface area and routing coupling with Pages vs Worker split

Recommendation: Keep the PR’s approach: deriving the callback URL from APP_URL (with an origin+/learn fallback) matches the worker’s zone-mount routing model and the post-callback redirect logic already uses APP_URL. The added regression test directly guards the production failure mode (missing /learn prefix) with minimal additional complexity.

Files changed (5) +39 / -8

Bug fix (1) +11 / -1
index.jsFix OAuth redirect_uri to preserve /learn mount prefix +11/-1

Fix OAuth redirect_uri to preserve /learn mount prefix

• Replaces redirect_uri construction based on the bare origin with a dedicated callbackUrl() helper. callbackUrl() derives the callback path from APP_URL (or origin+/learn in dev) so the URL both matches GitHub app settings and routes back to the same /learn/* worker.

workers/learn-api/src/index.js

Tests (1) +11 / -0
worker.test.jsAdd regression test for /learn-prefixed OAuth redirect_uri +11/-0

Add regression test for /learn-prefixed OAuth redirect_uri

• Adds a test asserting that /api/auth/login produces a Location with redirect_uri set to https://agentculture.org/learn/api/auth/callback when APP_URL is configured. Prevents reintroducing the bare-origin callback bug.

workers/learn-api/test/worker.test.js

Documentation (1) +10 / -0
CHANGELOG.mdDocument Phase-2 signed-in provisioning and OAuth callback fix +10/-0

Document Phase-2 signed-in provisioning and OAuth callback fix

• Adds a 0.5.3 entry describing KV+D1 provisioning and deployment details. Documents the OAuth redirect_uri bug and the /learn-prefix fix.

CHANGELOG.md

Other (2) +7 / -7
pyproject.tomlBump learn-cli version to 0.5.3 +1/-1

Bump learn-cli version to 0.5.3

• Updates the project version from 0.5.2 to 0.5.3 to reflect the release.

pyproject.toml

wrangler.tomlWire production KV+D1 bindings and GitHub client ID +6/-6

Wire production KV+D1 bindings and GitHub client ID

• Replaces placeholder identifiers with real Cloudflare KV namespace and D1 database IDs and sets the public GITHUB_CLIENT_ID. Keeps secrets (SESSION_SECRET, GITHUB_CLIENT_SECRET) out of git, to be provided via wrangler secrets.

workers/learn-api/wrangler.toml

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Jkvh4uxSi6AsBJYPjiurze
@sonarqubecloud

Copy link
Copy Markdown

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@OriNachum OriNachum merged commit f9cea51 into main Jul 11, 2026
8 checks passed
@OriNachum OriNachum deleted the feat/learn-signin-phase2 branch July 11, 2026 05:11
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.

1 participant