Skip to content

fix(cli): bundle pty-daemon.js + pass OAuth JWT through to relay#4054

Merged
saddlepaddle merged 1 commit into
mainfrom
fix/cli-pty-daemon-bundling-and-oauth-jwt
May 4, 2026
Merged

fix(cli): bundle pty-daemon.js + pass OAuth JWT through to relay#4054
saddlepaddle merged 1 commit into
mainfrom
fix/cli-pty-daemon-bundling-and-oauth-jwt

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented May 4, 2026

Why

Two bugs reported on cli-v0.2.4 after install. Both reproduce on linux-x64 and darwin-arm64 (universal regressions in the CLI host-service path):

[supervisor] bootstrap failed for org=…: Error:
  [pty-daemon:…] script not found at /home/pty-daemon/dist/pty-daemon.js
  — has the daemon binary been bundled?
[host-service] failed to register/connect relay:
  TRPCClientError2: Failed to mint JWT: 401

Root causes

#1 — pty-daemon never bundled. packages/cli/scripts/build-dist.ts builds host-service.js but no equivalent step for pty-daemon.js. The host-service's resolveSupervisorScriptPath() (packages/host-service/src/daemon/singleton.ts:22-47) looks side-by-side first, falls through to the workspace path ../../../pty-daemon/dist/pty-daemon.js resolved against <install>/lib, which on a user machine like /home/avi/superset/lib becomes the nonsense /home/pty-daemon/dist/pty-daemon.js. Confirmed with tar -tzf superset-darwin-arm64.tar.gz | grep pty-daemon against the released tarball — only host-service.js is present.

#2 — OAuth access token rejected by /api/auth/token. superset auth login does code+PKCE and stores result.accessToken (packages/cli/src/commands/auth/login/command.ts:57), which is an OAuth access token issued by better-auth's oauthProvider. JwtApiAuthProvider.getJwt() blindly POSTs it to better-auth's /api/auth/token (the JWT plugin endpoint), which only accepts session tokens / api keys — OAuth bearer tokens get 401.

But that exchange is unnecessary. The OAuth access token is already a JWT: signed by the same JWKS the relay verifies against (apps/relay/src/auth.ts:13), with organizationIds baked in via customAccessTokenClaims (packages/auth/src/server.ts:228-251). We can pass it straight through.

What changes

packages/cli/scripts/build-dist.ts

Add buildPtyDaemon() and copy the output next to host-service.js:

console.log("[build-dist] building pty-daemon bundle");
const ptyDaemonBundle = await buildPtyDaemon();
cpSync(ptyDaemonBundle, join(stagingRoot, "lib", "pty-daemon.js"));

packages/host-service/src/providers/auth/JwtAuthProvider/JwtAuthProvider.ts

  • If sessionToken is already a 3-segment JWT, return it as-is.
  • For sk_live_… API keys, send as x-api-key (mirrors what the CLI's tRPC client already does in packages/cli/src/lib/api-client.ts:24). This was previously fixed on the unmerged satya-patel/api-key-headless-cli branch and never landed.
  • Otherwise (session tokens) keep the old Authorization: Bearer exchange.

Smoke-test guard rails

Both the GH Actions workflow (.github/workflows/build-cli.yml) and the local Docker harness (packages/cli/scripts/build-dist-linux-docker.sh) now test -f both host-service.js and pty-daemon.js after build. So pty-daemon ever going missing again will fail the build, not slip through.

Verification

  • bun run build:dist --target=darwin-arm64 produces a tarball where tar -tzf | grep pty-daemon shows ./lib/pty-daemon.js (29KB).
  • bash packages/cli/scripts/build-dist-linux-docker.sh linux-arm64 ran end-to-end inside the oven/bun:1.3.11 container: bun install → npm rebuild → build-dist → smoke test all green, tarball produced (191MB), pty-daemon.js assertion passed, require('better-sqlite3' | 'node-pty' | '@parcel/watcher' | 'libsql') all OK against the bundled Node.
  • ✅ Typecheck + lint clean on both packages.

JWT pass-through hasn't been live-tested against the relay since I can't easily replay the user's OAuth token on my machine — but the relay-side validation is jwtVerify(token, jwks, { issuer, audience }) and the OAuth access token has iss = NEXT_PUBLIC_API_URL, aud ⊇ NEXT_PUBLIC_API_URL, and organizationIds claim, so it satisfies the relay's contract.

Test plan

  • CI green
  • After merge, cut cli-v0.2.5 (bump packages/cli/package.json + cli.config.ts + bun.lock; tag) — confirm the released tarball contains lib/pty-daemon.js
  • On a fresh machine, superset auth loginsuperset start → host-service comes up, relay connects (no Failed to mint JWT: 401), pty-daemon spawns (no script not found error)

Summary by cubic

Fixes supervisor startup and relay auth in the CLI by bundling pty-daemon.js and passing OAuth JWTs straight through to the relay.

  • Bug Fixes
    • Build now compiles and ships lib/pty-daemon.js alongside host-service.js.
    • JwtAuthProvider.getJwt() returns 3-part JWTs as-is and sends sk_live_… keys via x-api-key; session tokens still use Authorization: Bearer.
    • CI and Docker smoke tests assert both files exist to prevent regressions.

Written for commit 136d505. Summary will update on new commits.

Summary by CodeRabbit

Release Notes

  • New Features

    • pty-daemon bundle now included in the CLI distribution
    • Improved JWT authentication to support both API key and bearer token methods
  • Chores

    • Enhanced build process with validation checks to ensure all required artifacts are present

Two bugs reported on cli-v0.2.4 across both linux-x64 and darwin-arm64:

1) [supervisor] bootstrap failed: script not found at
   /home/pty-daemon/dist/pty-daemon.js — has the daemon binary been bundled?

   build-dist.ts only built host-service.js, never pty-daemon.js. The
   host-service's resolveSupervisorScriptPath() looked for pty-daemon.js
   side-by-side with host-service.js, missed it, and fell back to the
   workspace path packages/pty-daemon/dist/pty-daemon.js — which on a
   user's machine resolved to a nonsense /home/pty-daemon/... path.

   Fix: add buildPtyDaemon() and copy the output next to host-service.js
   in the staged bundle. Add a smoke-test assertion (CI workflow + docker
   harness) that both files exist in the tarball, so this can't ship
   broken again.

2) [host-service] failed to register/connect relay:
   Failed to mint JWT: 401

   The CLI's `superset auth login` stores an OAuth access token from the
   code+PKCE flow. JwtApiAuthProvider.getJwt() blindly POSTed it to
   better-auth's /api/auth/token, which only accepts session tokens and
   API keys (not OAuth bearer tokens) and 401s.

   But the OAuth access token *is already a JWT*, signed by the same
   JWKS the relay verifies against, with `organizationIds` baked in via
   customAccessTokenClaims (packages/auth/src/server.ts:228-251). So we
   can pass it straight through — no exchange needed.

   Fix: detect three-segment JWTs and return as-is. Restore the
   sk_live_ → x-api-key behavior for api-key auth (mirrors what the
   CLI's tRPC client already does in api-client.ts:24).
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 4, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR introduces a pty-daemon bundle to the CLI distribution pipeline and enhances JWT authentication handling. The build scripts now compile and stage pty-daemon.js alongside host-service.js, with explicit existence checks added to smoke tests. The JWT provider detects tokens that are already valid JWTs and passes them through directly, while conditionally routing token exchange requests via x-api-key headers for API key formats or bearer tokens for other formats.

Changes

PTY Daemon Build Integration

Layer / File(s) Summary
Core Build Logic
packages/cli/scripts/build-dist.ts
New buildPtyDaemon() helper runs bun run build:daemon and returns the built artifact path; main() invokes it and writes pty-daemon.js to the staging lib/ directory.
Build Verification
.github/workflows/build-cli.yml, packages/cli/scripts/build-dist-linux-docker.sh
Smoke test and Docker build steps now verify that both $DIST/lib/host-service.js and $DIST/lib/pty-daemon.js exist, failing fast if either artifact is missing.

JWT Authentication Enhancement

Layer / File(s) Summary
Token Detection
packages/host-service/src/providers/auth/JwtAuthProvider/JwtAuthProvider.ts
New looksLikeJwt() helper validates JWT shape (three dot-separated parts); getJwt() returns the token immediately if it already looks like a valid JWT.
Request Header Conditioning
packages/host-service/src/providers/auth/JwtAuthProvider/JwtAuthProvider.ts
Token exchange requests now conditionally send x-api-key header for tokens starting with sk_live_, or Authorization: Bearer for other formats; caching behavior and error handling remain unchanged.

Sequence Diagram

sequenceDiagram
    participant Client
    participant JwtAuthProvider
    participant API

    Client->>JwtAuthProvider: getJwt(sessionToken)
    activate JwtAuthProvider
    
    alt Token looks like JWT
        JwtAuthProvider->>JwtAuthProvider: looksLikeJwt(sessionToken)
        JwtAuthProvider-->>Client: return sessionToken directly
    else Token is not a JWT
        JwtAuthProvider->>JwtAuthProvider: check cachedJwt validity
        alt Cached JWT still valid
            JwtAuthProvider-->>Client: return cachedJwt
        else Need new JWT
            alt Token starts with sk_live_
                JwtAuthProvider->>API: POST /api/auth/token<br>(x-api-key header)
            else Other token format
                JwtAuthProvider->>API: POST /api/auth/token<br>(Authorization: Bearer header)
            end
            API-->>JwtAuthProvider: minted JWT
            JwtAuthProvider->>JwtAuthProvider: cache JWT with expiry
            JwtAuthProvider-->>Client: return new JWT
        end
    end
    deactivate JwtAuthProvider
Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A daemon joins the CLI quest,
With PTY bundled to the rest,
And tokens now skip needless trade—
When JWTs are truly made!
The build scripts verify, no fuss,
Two artifacts journey with us.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the two main changes: bundling pty-daemon.js and passing OAuth JWT through to the relay, matching the core fixes in the changeset.
Description check ✅ Passed The description comprehensively covers the 'Why' (root causes), 'What changes' (detailed changes per file), verification steps, and test plan, exceeding template requirements despite following a custom structure.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cli-pty-daemon-bundling-and-oauth-jwt

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 4, 2026

Greptile Summary

This PR fixes two regressions in the CLI v0.2.4 host-service path: pty-daemon.js was never included in the distribution tarball, and the OAuth access token was being POSTed to an endpoint that only accepts session/API-key tokens. The pty-daemon bundling fix is solid; the JWT auth change correctly short-circuits the broken /api/auth/token exchange, but introduces a new silent-failure mode described in the inline comment.

Confidence Score: 3/5

Safe for short-lived sessions; will silently break relay connectivity for long-running daemons once the OAuth access token's exp passes.

The pty-daemon bundling and smoke-test changes are clean. The JWT passthrough fixes the immediate 401 regression, but the code returns the raw OAuth token on every getJwt() call with no expiry check — unlike the session/api-key paths that re-fetch every 55 minutes. For a daemon running longer than the token lifetime, all relay connections will permanently fail with 401 until the user manually re-authenticates, with no error message explaining why.

packages/host-service/src/providers/auth/JwtAuthProvider/JwtAuthProvider.ts — the JWT passthrough path needs an exp-based expiry guard before this is safe for production daemon use.

Important Files Changed

Filename Overview
packages/host-service/src/providers/auth/JwtAuthProvider/JwtAuthProvider.ts Adds JWT passthrough for OAuth access tokens and x-api-key header for sk_live_ keys; the JWT path bypasses all expiry/caching logic, leaving long-running daemons silently broken once the token's exp passes.
packages/cli/scripts/build-dist.ts Adds buildPtyDaemon() following the same pattern as buildHostService(), and copies the output to lib/pty-daemon.js in the staging directory — correctly fixes the missing-bundle bug.
.github/workflows/build-cli.yml Adds smoke-test assertions for both host-service.js and pty-daemon.js presence; straightforward guard rail.
packages/cli/scripts/build-dist-linux-docker.sh Mirrors the CI smoke-test assertions inside the Docker build harness; matches the workflow change exactly.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["getJwt() called"] --> B{"looksLikeJwt(sessionToken)?"}
    B -- "yes (OAuth JWT)" --> C["Return sessionToken as-is\n⚠️ No exp check — token\nmay already be expired"]
    B -- "no" --> D{"cachedJwt &&\nDate.now() < expiresAt - buffer?"}
    D -- "yes" --> E["Return cachedJwt"]
    D -- "no" --> F{"sessionToken starts\nwith 'sk_live_'?"}
    F -- "yes" --> G["POST /api/auth/token\nheader: x-api-key"]
    F -- "no" --> H["POST /api/auth/token\nheader: Authorization: Bearer"]
    G --> I["Cache JWT for 55 min\nReturn token"]
    H --> I
Loading
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
packages/host-service/src/providers/auth/JwtAuthProvider/JwtAuthProvider.ts:34-36
**JWT passthrough has no expiry check**

When `looksLikeJwt()` returns `true`, the stored token is returned unconditionally on every single call, completely bypassing the `cachedJwtExpiresAt` guard. OAuth access tokens carry an `exp` claim that the relay's `jwtVerify` actively enforces. Once that claim passes, every relay request will return 401 forever — there's no retry, no re-fetch, and no signal to the user — until they manually re-run `auth login`. The non-JWT path at least re-fetches from `/api/auth/token` every 55 minutes; the JWT passthrough never does.

A minimal guard: decode the payload, extract `exp`, and throw a descriptive error when the token is near or past expiry:

```ts
if (looksLikeJwt(this.sessionToken)) {
    try {
        const payload = JSON.parse(
            Buffer.from(this.sessionToken.split(".")[1], "base64url").toString(),
        );
        if (
            typeof payload.exp === "number" &&
            Date.now() / 1000 >= payload.exp - JWT_REFRESH_BUFFER_MS / 1000
        ) {
            throw new Error(
                "OAuth access token has expired — please re-run `superset auth login`",
            );
        }
    } catch (e) {
        if (!(e instanceof SyntaxError)) throw e;
    }
    return this.sessionToken;
}
```

Reviews (1): Last reviewed commit: "fix(cli): bundle pty-daemon.js + pass th..." | Re-trigger Greptile

Comment on lines +34 to +36
if (looksLikeJwt(this.sessionToken)) {
return this.sessionToken;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 JWT passthrough has no expiry check

When looksLikeJwt() returns true, the stored token is returned unconditionally on every single call, completely bypassing the cachedJwtExpiresAt guard. OAuth access tokens carry an exp claim that the relay's jwtVerify actively enforces. Once that claim passes, every relay request will return 401 forever — there's no retry, no re-fetch, and no signal to the user — until they manually re-run auth login. The non-JWT path at least re-fetches from /api/auth/token every 55 minutes; the JWT passthrough never does.

A minimal guard: decode the payload, extract exp, and throw a descriptive error when the token is near or past expiry:

if (looksLikeJwt(this.sessionToken)) {
    try {
        const payload = JSON.parse(
            Buffer.from(this.sessionToken.split(".")[1], "base64url").toString(),
        );
        if (
            typeof payload.exp === "number" &&
            Date.now() / 1000 >= payload.exp - JWT_REFRESH_BUFFER_MS / 1000
        ) {
            throw new Error(
                "OAuth access token has expired — please re-run `superset auth login`",
            );
        }
    } catch (e) {
        if (!(e instanceof SyntaxError)) throw e;
    }
    return this.sessionToken;
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/providers/auth/JwtAuthProvider/JwtAuthProvider.ts
Line: 34-36

Comment:
**JWT passthrough has no expiry check**

When `looksLikeJwt()` returns `true`, the stored token is returned unconditionally on every single call, completely bypassing the `cachedJwtExpiresAt` guard. OAuth access tokens carry an `exp` claim that the relay's `jwtVerify` actively enforces. Once that claim passes, every relay request will return 401 forever — there's no retry, no re-fetch, and no signal to the user — until they manually re-run `auth login`. The non-JWT path at least re-fetches from `/api/auth/token` every 55 minutes; the JWT passthrough never does.

A minimal guard: decode the payload, extract `exp`, and throw a descriptive error when the token is near or past expiry:

```ts
if (looksLikeJwt(this.sessionToken)) {
    try {
        const payload = JSON.parse(
            Buffer.from(this.sessionToken.split(".")[1], "base64url").toString(),
        );
        if (
            typeof payload.exp === "number" &&
            Date.now() / 1000 >= payload.exp - JWT_REFRESH_BUFFER_MS / 1000
        ) {
            throw new Error(
                "OAuth access token has expired — please re-run `superset auth login`",
            );
        }
    } catch (e) {
        if (!(e instanceof SyntaxError)) throw e;
    }
    return this.sessionToken;
}
```

How can I resolve this? If you propose a fix, please make it concise.

@saddlepaddle saddlepaddle merged commit 1d297d4 into main May 4, 2026
13 of 14 checks passed
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

🚀 Preview Deployment

🔗 Preview Links

Service Status Link
Neon Database (Neon) View Branch
Vercel API (Vercel) Open Preview
Vercel Web (Vercel) Open Preview
Vercel Marketing (Vercel) Open Preview
Vercel Admin (Vercel) Open Preview
Vercel Docs (Vercel) Open Preview

Preview updates automatically with new commits

@saddlepaddle saddlepaddle mentioned this pull request May 4, 2026
3 tasks
saddlepaddle added a commit that referenced this pull request May 4, 2026
Two cli-v0.2.4 regressions fixed:
- pty-daemon.js never bundled into the dist tarball — supervisor bricked
  at spawn with `script not found at /home/pty-daemon/dist/pty-daemon.js`
  on every fresh install (#4054).
- OAuth access tokens (CLI auth login default) sent to better-auth's
  /api/auth/token endpoint, which only handles sessions/api keys → 401
  on relay JWT mint (#4054).

Push cli-v0.2.5 after this lands to fire the release pipeline.
@Kitenite Kitenite deleted the fix/cli-pty-daemon-bundling-and-oauth-jwt branch May 6, 2026 04:51
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request May 8, 2026
Recorded as integrated via -s ours after batch PRs #455-#464.

Taken via individual PRs:
- PR  1 (#455): v2 polish 前半 safe set (9 commits)
- PR  2 (#456): v2/host-service polish 中盤 (12 commits)
- PR  3 (#457): sidebar polish + jwt API (5 commits)
- PR  4 (#458): host-service tRPC retry/cache/timeout (3 commits)
- PR  5 (#459): v2 diff pane / file pane polish (2 commits)
- PR  7 (#462): host-service v2 canonical workspace.create + attachment store (PR1 superset-sh#3893 + PR2 superset-sh#3916)
- PR 11 (#463): agents API + onboarding (7 commits + 1 cleanup)
- PR 12 (#464): v1→v2 import flow rewrite (11 commits + 2 follow-ups)
- PR 13 (#460): host-service shell env probe + typo (2 commits)
- PR 16 (#461): marketplace 19 themes (1 commit)

Skipped / deferred (recorded as integrated for behind=0):
- PR  6: CLI v1 launch (superset-sh#3898 + 30+ CLI/SDK followups) — defer to dedicated migration
- PR  9: v2 PR3 (superset-sh#3940) + revert (superset-sh#4017) — net-zero pair
- PR 10: pty-daemon (superset-sh#3896, superset-sh#3971, superset-sh#4054) — fork keeps its terminal-host
- PR 14: Slack MCP-v2 (superset-sh#4197, superset-sh#4208) — depends on mcp-v2/sdk divergence
- PR 15: onboarding remaining (superset-sh#4115, superset-sh#4125, superset-sh#4214, superset-sh#4213, superset-sh#4222, superset-sh#4225) — depends on fork's deleted setup pages

Behind: 0 after this merge.
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