Repository navigation
fix(cli): prefer OAuth session over HEYGEN_API_KEY - #7051
Conversation
A signed-in `hyperframes auth login` session now wins over an API key in the environment, so a paid plan on the login is used even when the shell also exports HEYGEN_API_KEY. New order: HEYGEN_ACCESS_TOKEN (host app), the stored login (refreshed when expired), HEYGEN_API_KEY, HYPERFRAMES_API_KEY, the saved API key. The env key still comes first when HEYGEN_API_URL or HEYGEN_API_BASE names a host app's gateway, and takes over for the rest of the process when the login cannot be refreshed. The media-use audio helper follows the same order. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
Edit accuracy: accurate 267 (base branch 267), smooth 111 of thoseThe gate passes. Quarantined, measured but not gated (8)
Unstable (1)
|
|
@miguel-heygen FYI: this is the fix for the env key winning over a fresh OAuth login that Terence hit in the reverse-any-video bug bash. |
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 1 nit · head 2bea9f69
The new order and the gateway exception hold up. The blocker is the refresh-failure fallback (see inline): it switches on any refresh error and then stays switched for the whole process, so a single transient failure brings back the bug this PR fixes.
Scope
- Read all of resolver.ts, cloud/auth.ts, media-environment.ts, publishProject's credential path, status.ts and heygen.mjs. Checked every
resolveCredential/tryResolveCredentialcaller: all of them refresh before signing, so an expired-but-refreshable login never goes out unrefreshed. - Gateway: HFI's agent env (
heygenAgentEnv.mjs) setsHEYGEN_API_URLto the loopback gateway and blanksHEYGEN_ACCESS_TOKEN, so its key still goes first. voiceFix andwithMediaAuthEnvironmenteach set the token and delete the keys. No known host sets both a token and a key on a HeyGen host, so movingHEYGEN_ACCESS_TOKENfirst changes nothing for them. - Not from this PR: with a gateway URL and no env key, the stored login still goes to the gateway through the cloud client, so the resolver comment saying it "must never receive an OAuth token" is only enforced for media.
- Confirmed with a throwaway vitest at the head (not committed). CI is green.
A 503, a network error or an overlapping refresh no longer switches the process to HEYGEN_API_KEY. The fallback now needs a refused grant or an expired login with no refresh token. A login another process stored since is re-read first, and refreshes that overlap in one process share one request. The media-use helper no longer treats HeyGen's own API base as a gateway. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Independent review at ede3c3d8e9765a067b581fe9f96531dce3dff993, additive to heygengenesis's refresh-failure finding. The new order preserves the stored login ahead of shell keys, and the current change repairs the 503/network and overlapping-refresh cases. I audited all 23 changed paths plus cloud, media, publish, status and audio credential consumers.
Blocker — malformed token replies still permanently select another credential: packages/cli/src/auth/resolver.ts:154–158 treats every REFRESH_FAILED as a refused grant. However, refreshOnce also calls parseTokenResponse on successful HTTP responses (packages/cli/src/auth/oauth.ts:400–401), and that parser throws the same code for {}, null, missing access_token or unsafe token fields (:542–557). With an expired stored login and HEYGEN_API_KEY set, HTTP 200 {} therefore returns the shell key and sets loginRefreshFailed=true; subsequent calls skip the login even if the endpoint recovers. A malformed response did not establish that the grant was refused, and this restores the wrong-account/billing selection the PR is fixing.
Distinguish actual grant rejection from token-response parsing/protocol errors before enabling sticky fallback. Add a malformed-200 → successful-refresh regression asserting the first call throws without selecting the key and the next call signs with the refreshed login. This finding follows the exact source/error path; no live login, token refresh or paid request was made.
Validation limit: current-head CI run 41555665808, attempt 1, was still in progress at 00:55:54 UTC. The fresh guard also still has checks in progress (CodeQL, Windows test shards and Flake gate); no failure conclusion was returned. No whole-job bodies or completed-suite result are claimed; no local tests were run. The blocker is independently established by the source error path.
— Magi (cutroom-dev)
Verdict: REQUEST CHANGES
Reasoning: Credential precedence and transient network handling improve, but a token parser error still permanently switches from the stored login to an environment API key without a refused grant.
Only a 400/401 from the token endpoint marks the grant as refused and switches to the env key. A 200 whose body can't be parsed now throws and leaves the login in place, so the next call refreshes again. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
There was a problem hiding this comment.
Verdict: APPROVE · 2 resolved, 0 open, 0 new · 2bea9f69 → 6605e03f
Magi's malformed-200 blocker is also fixed: only a 400/401 (refused) or LOGIN_EXPIRED switches to the env key, and there's a test for it. Required CI is green. The optional Comments ratchet is red: errors.ts share 2.2% → 3.0% from the new refused doc line.
miguel-heygen
left a comment
miguel-heygen
left a comment
There was a problem hiding this comment.
Re-review at 6605e03f1cc4c0b6142908adc86775f5feb7146b: my malformed-success-response blocker in review5745627713 is resolved; I found no further code blocker.
packages/cli/src/auth/oauth.ts:393 explicitly marks only HTTP400/401 refresh refusals; packages/cli/src/auth/resolver.ts:154 preserves the login on malformed200, network and server errors, while retaining concurrent-login recovery. packages/cli/src/cloud/auth.test.ts:86 pins malformed200 followed by a healthy login refresh. I also traced the shared cloud/media/publish callers and retained gateway behavior; this agrees with heygengenesis's current source review.
Audited: the six-path repair since my prior23-path review, OAuth parsing/refusal classification, resolver and affected callers; unchanged code reuses that prior audit. Original hosted CI41557164722 tested this head through synthetic6aaa689 into eb35629. Both CLI shards passed (4009+3360 cases;23+89 skipped); the entire12-case cloud-auth file and26-case resolver file passed. Build, Lint, Typecheck, Format and required CLI smoke executed successfully. The final guard finds all 12 required contexts successful; GitHub still reports blocked. Empty combined commit statuses do not represent an additional executing job. No live login refresh or paid identity probe was performed.
Nit: packages/cli/src/auth/errors.ts:20 raises the optional Comments ratchet from2/91 to3/100 comment lines; original job125658753861 fails for that reason. Retain this failure rather than calling all checks green.
Verdict: APPROVE
Reasoning: Explicit refusal classification closes the sticky-env identity regression, and the original hosted regression file passes. The optional comment-density failure is not a correctness blocker.
— Magi (cutroom-dev)
What
A signed-in
hyperframes auth loginsession now wins over an API key inHEYGEN_API_KEY/HYPERFRAMES_API_KEY.Order before:
HEYGEN_API_KEY>HYPERFRAMES_API_KEY>HEYGEN_ACCESS_TOKEN>~/.hyperframes/credentials(login, else saved key).Order after:
HEYGEN_ACCESS_TOKEN(a host app's injected OAuth token)HEYGEN_API_KEYHYPERFRAMES_API_KEYauth login --api-keyThe env key still comes first in two cases:
HEYGEN_API_URLorHEYGEN_API_BASEnames a host other than HeyGen's. A host app's gateway takes the key it set, and an OAuth token must never be sent to it.An expired login with no env key still asks you to sign in, and still never switches to the saved API key.
Why
A Pro feature (
hyperframes reverse start) refused a user on a paid plan as Free. Their login carried the plan, but aHEYGEN_API_KEYexported in the shell outranked it. An ambient env key should not silently replace the account you signed in with.Related work
None.
How
All CLI paths already go through
resolveCredentialinpackages/cli/src/auth/resolver.ts, so the order changes there once. The refresh sites fall back to the env key when a refresh fails:refreshIfNeededandforceRefreshCredentialsinpackages/cli/src/cloud/auth.ts(cloud render/get/list/delete, the 401 retry, reverse and other v3 jobs, audio analysis, publish)withMediaAuthEnvironmentinpackages/cli/src/auth/media-environment.ts(media-use helpers)auth statusrefreshes first, so it reports the credential commands actually usepublishkeeps no fallback for a login it already checked mid-publish, so one publish never switches accounts halfway--update/--spaceerror no longer says to unset the env key; signing in is enough nowThe standalone media-use audio helper (
skills/media-use/audio/scripts/lib/heygen.mjs) uses the same order. It cannot refresh, so an expired login gives way to an env key.Explicit override: no command takes a per-command
--api-keyflag (--api-keyexists only onauth login, where it saves a key), so there was nothing explicit to keep ahead of the login, and no new flag is added. To use an env key while signed in, runhyperframes auth logout. If a per-run switch turns out to be needed, it would be a flag on the HeyGen commands that picks the env key, added in its own PR.Test plan
New tests:
generate.test.tsnow setsHEYGEN_API_URLto the host its mocks answer for. With an env key the resolver now reads the credentials file, and that real file read raced the test's fake timers."login + env key -> login" fails on main (resolves
source: "env",type: "api_key").Ran locally:
bun run lint,oxfmt --check, CLItsc --noEmit, the CLI unit suite (6373 passed), the media-use audio node tests,fallow audit, and the comment checks.🤖 Generated with Claude Code