Skip to content

fix(cli): tolerate invalid optional package cache manifests - #5349

Merged
jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/optional-cache-manifest-read
Oct 24, 2026
Merged

jrusso1020 merged 1 commit into
heygen-com:mainfrom
user-github-me:fix/optional-cache-manifest-read

Conversation

@user-github-me

Copy link
Copy Markdown
Contributor

A malformed cached optional-package manifest makes hyperframes doctor --json exit with an error and no report. The version reader also returns non-string and absent versions despite its string | null contract, which can make dependency availability checks report a package as installed.

Read the cache manifest as unknown, return a non-empty string version only, and return null for missing, unreadable, or invalid manifests. A pinned package beside the CLI retains priority. This change only reads metadata; it does not load package entries or repair the cache.

Validation:

  • Ten regression cases fail on main; all 112 optional-package, doctor, local-embedder, local-search, and local-model tests pass with the fix.
  • Built CLI in an isolated production dependency layout: malformed JSON and null change from exit 1 with no JSON to exit 0 with the complete doctor report. A numeric version is reported as unavailable; a valid cached version remains 1.52.0 installed. No optional package was downloaded or installed during these checks.
  • CLI build/typecheck, workspace lint, formatting, comment checks, test reachability, and normal commit hooks pass.

Related: #4156 introduced the cache and #4507 added pinned packages beside the CLI. Both are merged; this corrects the remaining cached-manifest reader.

@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.

Main still does an unguarded JSON.parse(...).version cast in installedOptionalPackageVersion, so a truncated cached manifest throws out of checkOptionalPackage and doctor --json dies, while {} returns undefined and passes the !== null check in localEmbedder.isLocalRuntimeInstalled. Dropping the isInstalled pre-check is safe, since that function is only an existsSync on the same manifest path and a missing file now lands in the catch. The pinned-beside-CLI path still takes priority. The tests cover malformed JSON, null, non-object, missing, numeric and empty versions through both the reader and the doctor check. Need: yes, still broken on main · Size: ok · Reuse: n/a · Simpler: already minimal.

— Rames

@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
@jrusso1020
jrusso1020 removed this pull request from the merge queue due to a manual request Oct 24, 2026
@jrusso1020
jrusso1020 added this pull request to the merge queue Oct 24, 2026
Merged via the queue into heygen-com:main with commit 1e6a2f0 Oct 24, 2026
81 checks passed
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.

2 participants