fix(cli): align env/ + constants/ + build-script error messages with 4-ingredient strategy#1258
Open
John-David Dalton (jdalton) wants to merge 5 commits intomainfrom
Open
fix(cli): align env/ + constants/ + build-script error messages with 4-ingredient strategy#1258John-David Dalton (jdalton) wants to merge 5 commits intomainfrom
John-David Dalton (jdalton) wants to merge 5 commits intomainfrom
Conversation
…4-ingredient strategy Rewrites runtime and build-time error messages for the build-inlined version/checksum pipeline to follow the What / Where / Saw vs. wanted / Fix strategy from CLAUDE.md. Sources (runtime): - env/coana-version.mts, env/sfw-version.mts (2 getters), env/socket-basics-version.mts, env/socket-patch-version.mts, env/trufflehog-version.mts, env/trivy-version.mts, env/opengrep-version.mts, env/pycli-version.mts — 9 "INLINED_X not found" errors. Each now names the exact env var, the bundle-tools.json path it comes from, and how to rebuild (`pnpm run build:cli`). - env/checksum-utils.mts — parseChecksums() and requireChecksum() now show the exact JSON.parse error or the list of known assets so you can see what was in vs. out of the map. - constants/paths.mts — getSocketRegistryPath() now enumerates every env var the app-data lookup checks (HOME, USERPROFILE, LOCALAPPDATA, XDG_DATA_HOME) so a cold environment tells you which to set. Sources (build-time scripts, same message style for consistency): - scripts/sea-build-utils/downloads.mts — 3 checksum-missing errors in the SEA build path, each now names the bundle-tools.json key and tells you to run `pnpm run sync-checksums`. No tests pinned these messages (only dist/cli.js — unchecked-in build output). Follows strategy from #1254. Continues #1255, #1256, #1257.
3 tasks
Switch `(e as Error).message` to `e instanceof Error ? e.message : String(e)` so that when a non-Error value is thrown (strings, objects, null) the error message stays informative instead of becoming 'undefined'. Same fix as applied to #1260 (iocraft.mts) after Cursor bugbot flagged the pattern on that PR.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Two issues flagged by Cursor bugbot on #1258: 1. (Low) parseChecksums() built the env var name as `INLINED_${toolName.toUpperCase()}_CHECKSUMS`. When toolName has spaces (e.g. 'Socket Patch'), toUpperCase() produces 'SOCKET PATCH' → 'INLINED_SOCKET PATCH_CHECKSUMS' — not a valid env var name. The real env var is INLINED_SOCKET_PATCH_CHECKSUMS. 2. (Low) Both parseChecksums() and requireChecksum() embedded `tools.${toolName}.checksums` to reference bundle-tools.json paths, but toolName is the display name (PyCLI, OpenGrep, Socket Patch) not the case-sensitive JSON key (socketsecurity, opengrep, socket-patch). Both came from the same root cause: I treated the display-name parameter as if it were a canonical identifier. Fix: reword the messages to just name the tool in prose ('inlined checksums for X', 'X has no SHA-256 for Y') and point at the 'matching entry in bundle-tools.json' instead of inventing a wrong path. Keeps the 4-ingredient structure (what/where/saw/fix) without claiming identifiers that don't exist. Caught by #1258 bugbot review.
Switch the 4 `Object.keys(x).join(', ')` calls in error messages on
this branch to `joinAnd(Object.keys(x))` so they render as human
prose (e.g. 'a, b, and c') instead of machine-y comma-joins.
Sites:
- src/env/checksum-utils.mts: requireChecksum known-assets list
- scripts/sea-build-utils/downloads.mts: 3 missing-checksum errors
(external tools, socketsecurity wheel, socket-basics archive)
No behavior change — just uses the fleet helper consistently.
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7381d9a. Configure here.
| throw new Error( | ||
| `Missing SHA-256 checksum for ${toolName} asset: ${assetName}. ` + | ||
| 'This is a security requirement. Please update bundle-tools.json with the correct checksum.', | ||
| `bundle-tools.json tools.${toolName}.checksums has no entry for "${assetName}" (seen: ${joinAnd(Object.keys(toolConfig?.checksums ?? {})) || '<empty>'}); run \`pnpm run sync-checksums\` to populate — builds must verify every external download`, |
There was a problem hiding this comment.
Invalid dot-notation JSON path for hyphenated tool names
Low Severity
The generic error message at line 336 uses tools.${toolName}.checksums with dot notation, but toolName can be socket-patch (a hyphenated key from PLATFORM_MAP_TOOLS), producing the invalid JSON path tools.socket-patch.checksums. The hardcoded socket-basics error at line 546 correctly uses bracket notation (tools["socket-basics"].checksums), showing the author was aware of this for some cases but missed it in the generic path. The old error message didn't include JSON paths, so this is a new inaccuracy.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 7381d9a. Configure here.
Cursor flagged the checksum-missing error in downloads.mts: it used
\`tools.\${toolName}.checksums\` (dot notation) which produces an
invalid JSONPath like \`tools.socket-patch.checksums\` when toolName
is hyphenated. The socket-basics site a few hundred lines down already
uses bracket notation for the same reason; make this one match.
Reported on PR #1258.
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
1 participant
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.
Summary
PR 5 of the error-message series. Covers the build-inlined version + checksum pipeline:
packages/cli/src/env/,packages/cli/src/constants/paths.mts, and the matching build-time helpers underpackages/cli/scripts/sea-build-utils/.~14 messages total. Zero tests pinned on these strings, so this PR is source-only.
What's fixed
Version getters (9 throws, near-identical pattern)
Every tool has an
INLINED_X_VERSIONenv var that esbuild inlines frombundle-tools.jsonat build time. Before, each getter said "INLINED_X_VERSION not found. Please ensure Y is properly configured in bundle-tools.json." which didn't help you figure out whether your build step ran, what file to edit, or what key in that file.Before:
INLINED_SFW_VERSION not found. Please ensure sfw is properly configured in bundle-tools.json.After:
process.env.INLINED_SFW_VERSION is empty at runtime; this value should be inlined at build time from bundle-tools.json tools.sfw.version (GitHub release tag) — rebuild socket-cli (`pnpm run build:cli`) or check that esbuild's define step ranApplied to:
coana,sfw(both GitHub and npm variants),socket-basics,socket-patch,trufflehog,trivy,opengrep,pycli.Checksum utilities (2 throws)
parseChecksums()now surfaces the actualJSON.parseerror.requireChecksum()now lists the available asset keys so you can tell whether the asset name is wrong or just missing from the map.App-data path (1 throw)
getSocketRegistryPath()now enumerates every env var the lookup checks (HOME,USERPROFILE,LOCALAPPDATA,XDG_DATA_HOME) so a user running in a stripped environment knows which to set.Build scripts (3 throws, same pipeline)
scripts/sea-build-utils/downloads.mtshad the same "Missing SHA-256 checksum" wording. Updated to match so you see the available-keys list and get pointed atpnpm run sync-checksums.Tests
None affected.
grepconfirmed notest/**/*.test.mtsfile pinned any of these substrings; onlydist/cli.js(stale build artifact, not checked in) had hits.Test plan
unset INLINED_COANA_VERSION && node dist/cli.js fix --help(or similar) should now show the rebuild hintNote
Low Risk
Low risk: changes are limited to error-message text and minor error formatting in build/download and env parsing paths, without altering download/verification behavior.
Overview
Improves CLI/build diagnostics for the build-inlined version + checksum pipeline by rewriting multiple
throw new Error(...)messages to be actionable (points to the exactbundle-tools.jsonkey, suggestspnpm run sync-checksums/pnpm run build:cli, and includes known asset keys when a checksum is missing).Also enhances checksum parsing errors to surface the underlying
JSON.parsefailure, and updatesgetSocketRegistryPath()to explicitly list the environment variables required to resolve the app-data directory when it can’t be determined.Reviewed by Cursor Bugbot for commit 7381d9a. Configure here.