Repository navigation
fix(cli): the voice a film uses is listed from the HyperFrames account, never the heygen sign-in - #7144
Merged
Conversation
…t, never the heygen sign-in
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
Contributor
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 1 important · 0 nits · head 86fffcb0
The code change looks right to me. The only blocker is in the PR body.
- blocker (PR body): the
Fails without the fix:line doesn't name a test. It says "listing public instead of private turns the scoping test red", which describes a mutation of the fix rather than naming a test. Please name the test by file and title. Both of these qualify, and I confirmed each locally:packages/cli/src/heygen/v3.test.ts› "lists the clones of the HyperFrames sign-in's workspace only, never another account's"scripts/check-core-skills.test.mjs› "style-sampling picks a voice from the HyperFrames account, never the separate heygen sign-in". It fails againstmain'sremix.md(not ok 10) and passes at head.
- important:
packages/cli/src/heygen/v3.ts:308: the optional Comments ratchet fails. The file's comment share went from 6.4% to 6.5% because of 1 added comment line, the newownVoicesdocstring. Fold it into the name, or cut it, to go green.
What I checked
- Tests at head:
v3.test.ts26/26,generate.test.ts54/54 andcheck-core-skills.test.mjs10/10. - Request types:
listVoicesV3already typestype?: "public" | "private"(cloud/_gen/client.ts:404), so the sharedlistVoicesadds no new request shape. A 401/403 now gives theauth loginfix, and there is no fallback to theheygenbinary. - Other
heygen voice listcallers (Rule 2):media-use/lib/voice-provider.mjs:32andheygen-video-provider.mjs:38still pick a default starfish voice through theheygenbinary. That isn't this defect, because both list and speak on the sameheygensign-in. I'm leaving them as pre-existing, same as theoperations.mdclone note the body already leaves out. --limitrange: the help says 1–100, butnum()doesn't enforce it. An out-of-range value would come back as a "HeyGen 400 … run the same command again" error. This is minor and doesn't block.- CI at the time of review:
regression,Player perf,PR captures,preview-regressionandDocswere green.CI,CodeQLand Windows were still running, andCommentshad failed.
3 of 4 tasks
Merged
6 of 7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
To pick a voice, the style-sampling skill told the agent to list voices with
heygen voice list. That is the separateheygenCLI sign-in, which can be a different account from the HyperFrames one making the film. On one machine it listed 50 clones for the heygen sign-in while the HyperFrames account had none, so the agent offered another account's clones by name and choosing one would have voiced the film with someone else's clone.How
hyperframes media-use voices: lists the clones of the HyperFrames sign-in's own workspace (/v3/voices?type=privatethrough the HyperFrames credential,heygen/v3.ts, next topublicVoices). It never runs theheygenbinary. Signed out or refused (401/403) it stops withnpx hyperframes auth login; it does not fall back to any other sign-in.skills/style-sampling/references/remix.mdsends the agent to that command and says neverheygen voice list; the CLI docs list it.media-use generate --type voice --voice <id>,/v3/voices/speech). I checked that the server refuses another account's clone there: speaking the heygen sign-in's private voice with the HyperFrames token answered 404 "Voice ... not found in this workspace" and returned no audio.Tests
v3.test.ts: the list asks fortype=privatewith the HyperFrames credential only, and returns only that account's voices even when another sign-in's key would list others; a refused sign-in gives theauth loginfix.check-core-skills.test.mjs: the skill namesmedia-use voicesand neverheygen voice listas the way to list.packages/cli/src/heygen/v3.test.ts> "lists the clones of the HyperFrames sign-in's workspace only, never another account's" (red when the list asks for public instead of private: 1 failed, 25 passed), andscripts/check-core-skills.test.mjs> "style-sampling picks a voice from the HyperFrames account, never the separate heygen sign-in" (fails against main'sremix.md, passes here). With the change: 118 passed acrosssrc/heygenandmedia-use.test.ts, the skills scripts pass, formatter and manifest check clean.Not covered
skills/media-use/references/operations.mdstill listsheygen voice clone create; a clone made there lands on the heygen sign-in's account. Left as it documents the separate-credential escape hatch; worth its own change.Size
A lone fix for one defect (the wrong account listed for voices) that spans the CLI command, its docs and the skill text; it cannot be smaller.