Skip to content

fix(cli): publish with a refused login refreshes or asks you to log in again - #4964

Merged
miguel-heygen merged 11 commits into
mainfrom
fix/cli-publish-expired-login
Oct 4, 2026
Merged

miguel-heygen merged 11 commits into
mainfrom
fix/cli-publish-expired-login

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Oct 3, 2026 •

Copy link
Copy Markdown
Collaborator

What

hyperframes publish with a credential the server refuses now stops before anything is created and says what to do:

  • a login (or a saved API key): "Your login expired. Run hyperframes auth login, then publish again."
  • an API key from HEYGEN_API_KEY / HYPERFRAMES_API_KEY: "HEYGEN_API_KEY was rejected. Fix or unset it, then publish again." (logging in would not help: the environment key outranks a login)

A login that expired and has no refresh token is no longer treated as "not signed in": publish stops with the same re-login message and sends nothing, and hyperframes auth status says "Your HeyGen login expired" instead of "Not signed in". A publish with a login that the server still answers with an unowned (unclaimed) project fails the same way. Only an owned result is saved as this directory's link. API-key publishes are unchanged: the server never makes them owned.

--update and --space need a login, because only an owned project can be updated in place or put in a team space. With an API key they now stop before uploading and say so ("--update requires a login; HEYGEN_API_KEY cannot own a project. Unset it and run 'hyperframes auth login'.", or "an API key cannot own a project" for a saved key), instead of publishing a new unowned project and ignoring the flag.

publish now looks up the credential once and uploads with that same credential. Before, the upload looked it up again after baking proxies, so a login that expired during the bake fell back to a saved API key and the flags were dropped after all. A login that expires during the bake is refreshed just before the upload, never swapped for another credential; a login that cannot be refreshed stops with the re-login message before anything is sent.

A token refresh never writes over a different login. If you log in as someone else while a publish is running, the old login's refresh leaves the new login exactly as it is, and publish stops with "Your login changed during publish. Run publish again."

An expired login that has a refresh token is refreshed first, as the cloud commands already do, so the common case of a stale access token publishes normally instead of failing. The warning for the old behaviour ("Your login looks expired or invalid, so --update was ignored and a NEW url was created above") is gone.

Why

The publish API is changing so that a request carrying a credential it rejects gets HTTP 401 instead of silently publishing an anonymous project with a claim URL. Without this change the CLI would print the bare server message ("Unauthorized"), and it would send a stale access token it could have refreshed.

How

  • resolvePublishCredential resolves the credential and refreshes it with refreshIfNeeded from cloud/auth.ts (now exported, not copied). A refused refresh (REFRESH_FAILED) is the "log in again" case and stops before any request.
  • Only the requests that carry the credential (/publish/upload, /publish/complete, the legacy /publish) turn a 401 into CredentialRejectedError; publishProjectArchive maps it to the message for that credential. A 401 from the presigned storage upload carries no credential and keeps its own error.
  • The first credentialed request (/publish/upload) creates nothing, so a refused credential stops before the archive upload. The existing catch in publish.ts prints the message under "Publish failed" and exits 1.
  • The credential resolver (auth/resolver.ts) throws LOGIN_EXPIRED when the stored login is expired with no refresh token and there is no saved API key to fall back to; NOT_CONFIGURED now means only "never signed in". resolvePublishCredential maps LOGIN_EXPIRED like REFRESH_FAILED, and the --update / --space preflight uses it too.
  • The server owns a publish only for a bearer login (every owned publish returns claimed: true) and drops a bearer it cannot verify, publishing anonymously (claimed: false). An API key (x-api-key) always publishes unowned. So a login publish whose result is unclaimed raises CredentialRejectedError and goes through the same message mapping, and writeProjectLink runs only for a claimed result.
  • publish.ts resolves the credential once, before the bake, and passes it to publishProjectArchive (new optional credential; null publishes anonymously, omitted resolves inside as before, which feedback still uses). The --update / --space preflight checks that same credential and accepts only a login (type === "oauth"). resolvePublishCredential(checked) refreshes a passed login whose expiry has passed (the resolver's isTokenExpired, same 60 s skew) and never resolves again.
  • persistOAuth (auth/oauth.ts) takes the refresh token that was exchanged ({ refreshed }, replacing the preserveMissing flag). If the stored login no longer holds that refresh token, it writes nothing and throws LOGIN_CHANGED; resolvePublishCredential maps it to the message above. This applies to every refresh caller, not only publish. A refresh that cannot read the credentials file now reports that error instead of treating the file as empty.
  • Anonymous publishing sends no credential and is unchanged.

Rollout

Release this CLI before or with the API change (an older CLI shows "Unauthorized" on the 401). Before the API deploy, a token the server silently drops still creates an unowned project on the server, but the CLI now reports it as a failed publish with the re-login message instead of showing a claim URL or saving the link.

Test plan

New in publishProject.test.ts:

  • 401 on the first request: re-login message after exactly one request, which carried the bearer.

  • 401 on /publish/complete and on the legacy multipart publish: same message.

  • 401 from the storage upload: the storage error, not the login message.

  • 401 with HEYGEN_API_KEY: names the variable.

  • An expired, refreshable login is refreshed and the request carries the new token; a refused refresh asks for a new login and sends nothing.

  • A stored login that is expired with no refresh token, through the real resolver and a temp credentials file: re-login message, no request sent (publishProject.test.ts), and LOGIN_EXPIRED from the resolver (resolver.test.ts).

  • A login publish answered with an unclaimed project, staged and legacy: re-login message, no link written.

  • An API-key publish answered with an unclaimed project: succeeds with its claim token, no link written. Checking every credential, or linking on any credential, fails it.

  • publish.test.ts, through the real command and resolver: --update and --space with HEYGEN_API_KEY, and --update with a saved API key, exit 1 with the login message and never call publish; --update with a login publishes. The three refusals fail with the previous "any credential" check. A command test for each flag runs the real upload code with the stored login expiring mid-publish next to a saved API key: every request carries the login and the command exits 1 without a claim URL. It fails if the upload resolves again or if publish.ts does not pass its credential. A passed login that expired after the check is refreshed and the upload carries the new token, with no second lookup (publishProject.test.ts); it fails without the expiry recompute. A passed login that expired with no refresh token stops before any request; it fails without that check. A command test swaps the stored login to another account during the bake and answers the refresh without a refresh token: the other login, its refresh token and its user stay untouched, only the token request is sent, exit 1. With the guard removed it fails, because the store ends up with the old access token on the new refresh token; the oauth.test.ts unit test fails the same way. A refresh against a corrupt credentials file reports INVALID_STORE and leaves the file alone. Real CLI on Linux with HEYGEN_API_KEY set: both flags print the message and exit 1. src/auth, src/cloud, src/commands/auth, feedback, publishProject.test.ts, publish.test.ts: 369 pass, 3 runs in a row; tsc --noEmit, oxlint, oxfmt clean.

The three new tests fail on the previous head (anonymous publish, NOT_CONFIGURED); dropping the LOGIN_EXPIRED mapping fails the end-to-end test. Real CLI on Linux with an expired, no-refresh credentials file and the API pointed at a dead port: before, auth status said "Not signed in" and publish --yes tried an anonymous upload; after, auth status prints "Your HeyGen login expired" and publish prints the re-login message and exits 1, for plain publish and --update. src/auth, src/commands/auth, src/cloud, publishProject.test.ts, publish.test.ts: 355 pass, 3 runs in a row (with the feedback tests); tsc --noEmit clean; oxlint clean.

The first test fails on main (Unauthorized). Breaking the code three ways (no refresh, mapping the storage 401, dropping the API-key message) fails 2, 1 and 1 tests. publishProject.test.ts, publish.test.ts and the cloud tests: 152 pass, 3 runs in a row; CLI tsc --noEmit clean; oxlint clean; comment ratchet at or under base.

No visible change

CLI text output only; nothing under Studio or the player.

Size

Credential refresh reuse, two error types, two messages, one unclaimed-result check, one deleted branch, a login-only --update / --space check, one credential lookup per publish (refreshed, never replaced), a refresh that only writes to the login it refreshed, twenty-two tests.

…again

The server now answers 401 to a rejected credential instead of publishing
anonymously, so the CLI says so and drops the warning for the old fallback.
@github-actions

github-actions Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Edit accuracy: accurate 1556 (base branch 1556), smooth 1436 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (0)

…ted API key

An expired access token with a refresh token is refreshed (the cloud commands
already do) instead of being sent and refused. Only the requests that carry the
credential map a 401; an environment API key is named, since logging in would
not replace it.
@miguel-heygen miguel-heygen changed the title fix(cli): publish with an expired login stops and asks you to log in again fix(cli): publish with a refused login refreshes or asks you to log in again Oct 3, 2026
@miguel-heygen
miguel-heygen marked this pull request as ready for review October 3, 2026 18:30

@jrusso1020 jrusso1020 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approving at 5d6a6d1f.

What I checked

  • Refresh before publish. The resolver marks a login refreshable only when it has expired and has a refresh token. refreshTokens saves the new tokens itself, including a rotated refresh token, so the refresh here doesn't use up the stored one. A refused refresh (REFRESH_FAILED) stops with the log-in-again message before any request goes out. Any other refresh error now ends the publish, the same way the cloud commands behave.
  • Which 401s get the message. Only the three requests that carry the credential (/publish/upload, /publish/complete and the legacy /publish) are mapped. A 401 from the presigned storage upload keeps its own error. The upload endpoint's 404/405 fallback to the legacy path still runs before the 401 check. An expired login with no refresh token still sends the stale token, and the 401 then gets the login message.
  • Tests. publishProject.test.ts and publish.test.ts pass 47/47 locally. I reverted two changes in turn. Removing the refresh makes 2 tests fail: "refreshes the login and publishes with the new token" and "asks for a new login, without sending anything, when the refresh is refused". Disabling the CredentialRejectedError mapping makes 4 fail: the first-request, complete, legacy and environment-key tests.

Non-blocking. Deleting the "--update was ignored and a NEW url was created" warning is only safe once the server returns 401. Until that deploy, an expired login that can't be refreshed and uses --update gets a new anonymous URL with no warning. writeProjectLink then points the directory at that anonymous project, which is existing behaviour. The PR body already describes this window, so the fix is just to ship this right before or with the server change.

— Rames

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At 5d6a6d1f, the eager refresh and credentialed-401 mapping are well scoped (packages/cli/src/utils/publishProject.ts:161–189,708–732): the presigned storage PUT keeps its own error. I found two paths where the promised “expired login refreshes or asks you to log in again” behavior still fails.

Blocker — an expired login without a refresh token becomes anonymous before the server can reject it. packages/cli/src/auth/resolver.ts:97–105 discards that OAuth credential; tryResolveCredential() returns null, and packages/cli/src/utils/publishProject.ts:761–769 sends no authentication at all. A normal hyperframes publish --yes then creates an anonymous claim URL rather than asking for a new login; the planned server 401-on-rejected credential cannot fix a request with no credential. (Explicit --update/--space do fail at the command preflight, but plain publish does not.) Refresh tokens are optional in the stored/token-response shape (auth/oauth.ts:542–562). The prior review says this case sends the stale token; the resolver contradicts that specific claim. Please distinguish a known expired login from a genuinely unsigned-in user and test this path end to end.

Blocker — a credentialed HTTP 200 with claimed: false is still accepted and linked. packages/cli/src/utils/publishProject.ts:70–93,789–799 parses an anonymous response as success and writes its URL into the local project link whenever a credential was resolved. The current server repository's optional-owner resolver still returns None for a rejected bearer, its staged-upload endpoint does not authenticate, and /publish/complete can return that unclaimed result (experiment-framework/heygen/services/heygen_server/hyperframes/routes/render_routes.py:70–88,126–151, current master 6c76f68b; I have not verified the deployed server). This PR also removes the --update/--space fallback warning in packages/cli/src/commands/publish.ts:286–301; until server rejection actually ships, an attempted update can mint an anonymous project without that explicit warning. The new authenticated refresh test (publishProject.test.ts:1159–1174) even uses a successful stagedFetch() fixture whose response has a claim token and no claimed: true (:125–149). Please fail closed on an unclaimed response when credentials were supplied, and do not persist it as an owned stable link; pin the 200-anonymous case in a regression test. This guard also protects against server policy drift after rollout.

Important, not independently blocking: a locally unexpired but server-revoked token gets the re-login error despite a valid refresh token; the cloud client already force-refreshes and retries once on 401 (packages/cli/src/cloud/index.ts:18–65). Consider parity for publish, especially if uploads take long enough for a token to expire between preparation and completion.

I reviewed the exact-head source and current server-repository source; I did not run the CLI tests locally. All 11 required checks pass at this head, but the new 401 fixtures do not exercise either anonymous-success path.

Verdict: REQUEST CHANGES
Reasoning: Neither a missing refresh token nor a successful anonymous fallback is stopped by the new 401 handling, so publishing can still create an unclaimed project where the user expected an authenticated publish.

— Review by tai (pr-review)

… again

A stored login past expiry with no refresh token now resolves as
LOGIN_EXPIRED instead of "not signed in", so publish stops with the
re-login message before sending anything. A signed-in publish that the
server answers with an unclaimed project fails the same way and links nothing.

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At 866c9b5b, both blockers from my prior review are fixed: auth/resolver.ts:73–91 now preserves expired-login state instead of treating an OAuth login without a refresh token as anonymous, and utils/publishProject.ts:790–803 refuses an OAuth publish answered claimed: false before writing a project link. The new real-resolver and staged/direct unclaimed tests exercise those paths. The API-key unclaimed publish remains an intentional success.

Blocker — explicit --update / --space can still succeed while being ignored with a valid API key (packages/cli/src/commands/publish.ts:166–179,235–302). The new preflight only checks that some credential exists; an HEYGEN_API_KEY or saved API key passes. But this PR states API-key publishes are unowned: publishProjectArchive accepts their claimed: false response and does not link the project (utils/publishProject.ts:790–804), while the server's owner resolution requires a bearer login. With either explicit flag, the CLI now reports “Project published” and a generic Claim URL, without saying the requested existing-project update or team-space publish was ignored. The PR base had an explicit --update/--space was ignored and a NEW url was created warning in this branch (commands/publish.ts:292–301); this PR deletes it. This is reachable with a valid API key, so the planned server 401 for rejected credentials cannot fix it, and an environment API key can outrank a saved login. Please require an owner-capable login for those flags, or preserve an explicit ignored-flag warning on any unclaimed result, and pin both flags with a valid API key in a command-level test. The normal API-key claim-URL publish can stay unchanged.

All 11 active required checks passed at this head. I did not run local CLI tests. The prior concern about a locally fresh but server-revoked bearer not force-refreshing remains a nonblocking follow-up, not a new regression.

Verdict: REQUEST CHANGES
Reasoning: The two authentication blockers are resolved, but removing the warning leaves an explicit update/space request reporting success without performing that request when a valid API key is selected.

— Review by tai (pr-review)

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At fcfd1510, the stable valid-API-key case from my previous review is fixed: commands/publish.ts:169–190 now resolves the selected credential and rejects API-key --update / --space before building or uploading. The command tests cover environment-key versions of both flags, a saved-key --update, and successful OAuth --update. The earlier expired-login and OAuth claimed:false blockers remain fixed; ordinary API-key claim-URL publishing remains allowed.

Blocker — an OAuth-to-API-key expiry transition can still silently ignore explicit flags. The command checks OAuth at commands/publish.ts:170, then awaits proxy baking at :217–220. publishProjectArchive independently calls resolvePublishCredential() at utils/publishProject.ts:762. With stored OAuth lacking a refresh token and a saved valid API key, start when the OAuth token is just outside the resolver's 60-second expiry skew; let the bake cross that boundary. auth/resolver.ts:73–78,100–109 first selects OAuth, then falls back to the saved API key without any external credential change. The upload utility treats any credential as sufficient to send the requested project ID/space (publishProject.ts:766–770), but an API key cannot own that project; the unclaimed response is only rejected for OAuth (:790–796). commands/publish.ts:298–314 then prints “Project published” and a generic claim URL with no warning that --update or --space was ignored. This recreates the silent-flag outcome behind the new preflight and is reachable by time passing during the bake, not only a concurrent logout. Please enforce owner-capable credentials at the upload boundary, or pass/pin a suitable credential through the publish call and reject an unclaimed explicit-flag result; pin the expiry-to-key fallback with a command-level test for both flags.

Active checks at this head were still running when reviewed; I did not run local tests. I am not lifting the earlier changes-requested verdict on this head.

— Review by tai (pr-review)

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Fixed at e6dd31c (c3c8b3a, e673b22, e6dd31c): publish now resolves the credential once (commands/publish.ts), the --update / --space check runs on it, and the same credential is passed into publishProjectArchive through a new credential option, so the upload never looks it up again. A login that expires during the bake is refreshed just before the upload (resolvePublishCredential(checked) in utils/publishProject.ts), never replaced; one that cannot be refreshed stops with the re-login message before anything is sent. In neither case does it fall back to the saved API key.

New command test for both flags (publish.test.ts, "uploads with the checked login even if it expires mid-publish"): stored login + saved API key, the login is expired between the check and the upload, and the real publishProjectArchive runs against a stubbed server that answers unowned. Every request carries the bearer, exit code is 1, no claim URL. It fails if the upload resolves again or if the command does not pass its credential.

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At e6dd31c6, the prior silent explicit-flag blocker is fixed for the expiry-to-API-key transition: commands/publish.ts:172,238–244 passes the preflight-selected OAuth credential through proxy baking to the upload, and utils/publishProject.ts:181–192,772 refreshes that same credential rather than falling back to the saved API key. The earlier expired-login and OAuth claimed:false guards remain intact.

Blocker — the pinned credential can overwrite a newer login during refresh. Start publish --update or --space under OAuth login A while proxy baking awaits (commands/publish.ts:230–244). Log in as B from another CLI during the bake, then let A cross the expiry-skew boundary. resolvePublishCredential(checked) refreshes A (utils/publishProject.ts:186–192) and refreshTokens(A.refresh_token) persists the response (auth/oauth.ts:383–415) by rereading the current store and merging { ...existing.oauth, ...tokens } (auth/oauth.ts:610–631). That store is now B's. A refresh response can legitimately omit refresh_token (auth/oauth.ts:542–576,605–607): the durable result becomes A's access token paired with B's refresh token and B's user metadata. Even when A rotates its refresh token, this overwrites B's newer login. On the prior head, the publish path re-resolved credentials after baking and would have selected B instead; pinning A across the bake introduced the long-window stale refresh. Please protect refresh persistence when the stored login has changed (and fail the publish rather than mutating B's session), with a test that swaps logins while the bake is pending and exercises a refresh response without refresh_token.

All 10 active required checks passed at this head when checked, including the Windows rerun. I did not run local tests. I cannot lift the earlier changes-requested review while this durable auth-store regression remains.

— Review by tai (pr-review)

@miguel-heygen

Copy link
Copy Markdown
Collaborator Author

Fixed at 5ab36e9 (48e5e6a, plus 5ab36e9 so an unreadable credentials file during a refresh reports itself instead of a changed login), at the persistence boundary so every refresh caller is covered, not only publish. persistOAuth (auth/oauth.ts) now receives the refresh token it exchanged. If the stored login no longer holds that refresh token (B replaced A), it writes nothing and throws LOGIN_CHANGED. Publish maps that to "Your login changed during publish. Run publish again." and exits 1 before any upload.

Tests:

  • publish.test.ts, "fails without touching a login that replaced the checked one mid-publish": publish --update under A; the store is swapped to B while the bake is pending; the clock moves past A's expiry; the token endpoint answers without refresh_token. B's access token, refresh token and user stay as they were, only the token request is sent, and the exit code is 1.
  • oauth.test.ts, "leaves a login that replaced the refreshed one untouched": the same check at the refresh call itself.

With the guard removed, both fail. The store then ends up with A's new access token on B's refresh token, which is the state you described.

@terencecho terencecho left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approving 5ab36e94. My blocker from e6dd31c6 is fixed: a pinned-login refresh can no longer write A's tokens over a newer login B.

What I verified

  • persistOAuth (auth/oauth.ts:611–631) now takes the exchanged refresh_token and throws LOGIN_CHANGED before writing unless the stored oauth.refresh_token still equals it. That is a sound identity key: the only OAuth credential source is the store (auth/resolver.ts:68–74), a different login or re-login mints a different refresh token, and a logout mid-bake (no stored oauth) also mismatches rather than resurrecting the old login. A non-rotating refresh by the same login still matches and keeps its refresh token via the merge.
  • The case I flagged is covered end to end: publish.test.ts logs in as B during the bake, expires A, refreshes with a response that omits refresh_token, and asserts B's oauth and user.email are untouched, only the token endpoint was called (no upload), exit code 1, and the "login changed" message. oauth.test.ts pins the same at the refreshTokens layer and keeps the refresh-preserve tests meaningful by seeding the stored login they now require.
  • resolvePublishCredential maps LOGIN_CHANGED to a publish-level error (publishProject.ts:198) so --update / --space fail loudly instead of continuing under the wrong login; other refresh callers (cloud/auth.ts, client.ts retry) let it propagate, and an unreadable store now fails the refresh instead of being overwritten (tested). Fresh-login paths still overwrite the block cleanly.

Non-blocking: the check and the write are still separate steps (readStore then writeStore, no lock), so a login that lands inside that millisecond window could still be overwritten. It needs an interactive login to complete in that window, so I would not hold on it; an optimistic re-read before the rename would close it.

I read the code and tests but did not run them locally. At review time the 9 required checks present had passed; the lint/format check, Windows studio shard, and the edit-accuracy shards were still running, so this approval is on code merit, not on CI completion. It is not authorization to merge or deploy beyond what the gate already does.

— Review by tai (pr-review)

@miguel-heygen
miguel-heygen added this pull request to the merge queue Oct 4, 2026
Merged via the queue into main with commit de9bf48 Oct 4, 2026
169 checks passed
@miguel-heygen
miguel-heygen deleted the fix/cli-publish-expired-login branch October 4, 2026 05:07
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.

3 participants