Skip to content

fix(cli): error messages in env/ + constants/ + sea-build scripts#1258

Merged
John-David Dalton (jdalton) merged 6 commits intomainfrom
jdd/error-msg-env-constants
Apr 24, 2026
Merged

fix(cli): error messages in env/ + constants/ + sea-build scripts#1258
John-David Dalton (jdalton) merged 6 commits intomainfrom
jdd/error-msg-env-constants

Conversation

@jdalton
Copy link
Copy Markdown
Contributor

@jdalton John-David Dalton (jdalton) commented Apr 22, 2026

Summary

Align error messages across tool-version env/ modules, constants/paths.mts, and the sea-build download helper with the 4-ingredient strategy from CLAUDE.md (#1254).

Scope

  • packages/cli/src/constants/paths.mts
  • packages/cli/src/env/checksum-utils.mts
  • packages/cli/src/env/{coana,opengrep,pycli,sfw,socket-basics,socket-patch,trivy,trufflehog}-version.mts — 8 tool-version modules
  • packages/cli/scripts/sea-build-utils/downloads.mts

Smallest of the error-message batches (+22/-20).

Related PRs (sibling error-message batches)

Test plan

  • Type / lint green
  • No unit tests directly exercise these paths; SEA build exercises the download helper end-to-end

…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.
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.
Comment thread packages/cli/src/env/checksum-utils.mts Outdated
Comment thread packages/cli/src/env/checksum-utils.mts Outdated
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.
@jdalton
Copy link
Copy Markdown
Contributor Author

bugbot run

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

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.

Comment thread packages/cli/scripts/sea-build-utils/downloads.mts Outdated
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.
John-David Dalton (jdalton) added a commit that referenced this pull request Apr 24, 2026
* fix(cli): align test/ error messages with 4-ingredient strategy

Rewrites the Socket-JSON contract validator and auth-flow mocks under
packages/cli/src/test/ and packages/cli/test/ to follow the What /
Where / Saw vs. wanted / Fix strategy from CLAUDE.md.

Sources:
- src/test/json-output-validation.mts (6 throws): each violation now
  spells out the full Socket-JSON contract, the received value, and
  a concrete fix ("add ok:true", "return empty object", etc.).
  Long output payloads are truncated to 200 chars in the message so
  errors stay readable.
- src/test/mocks/socket-auth.mts (2 throws): "Authentication failed"
  and "OAuth timeout" now call out that they come from a test
  fixture and point at the configuration flag to change.
- test/json-output-validation.mts (2 non-throwing returns): message
  values now include the exit code / parse error and a stdout
  preview so failing tests diagnose themselves.
- test/smoke.sh (6 labels): updated to mirror the TS validator so
  the bash and JS harnesses produce the same wording.

Tests: full suite (343 files / 5225 tests) still passes. No
assertions touched — the unrelated "Authentication failed" hits
in other tests are test fixtures constructing their own Errors,
not references to the mock.

Follows strategy from #1254. Continues #1255-#1258.

* chore(cli): harden (e as Error) casts to safe stringify

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.
@jdalton John-David Dalton (jdalton) changed the title fix(cli): align env/ + constants/ + build-script error messages with 4-ingredient strategy fix(cli): error messages in env/ + constants/ + sea-build scripts Apr 24, 2026
John-David Dalton (jdalton) added a commit that referenced this pull request Apr 24, 2026
…, terminal) (#1260)

* fix(cli): align utils/ miscellaneous error messages with 4-ingredient strategy

Final PR in the error-message series. Covers everything not already
touched by #1255-#1259: utils/basics, utils/config, utils/fs,
utils/git, utils/npm, utils/promise, utils/terminal, and the flags
module at the root of the CLI tree.

Sources:
- flags.mts: 2 throws (--max-old-space-size, --max-semi-space-size)
  — name the flag, show the offending value, suggest a concrete
  megabyte value.
- utils/config.mts: 1 throw (SOCKET_CLI_CONFIG base64 decode) —
  explains the replacement-character symptom and how to re-encode.
- utils/basics/vfs-extract.mts: 4 throws (SEA VFS extraction for
  Python + security tools) — name the missing paths, the exit
  codes, and point at the "rebuild the SEA binary" fix.
- utils/promise/queue.mts: 1 throw (PromiseQueue concurrency guard)
  — show the offending value and suggest 4/8.
- utils/npm/spec.mts: 1 throw (PURL conversion) — show the input,
  state what a valid npm spec looks like.
- utils/git/operations.mts: 1 throw (git-not-on-PATH) — point at
  install and the local-path env-var override.
- utils/git/gitlab-provider.mts: 2 throws (no token, PR creation
  after retries) — name the token scope, the retry count, the
  repo/head refs.
- utils/fs/path-resolve.mts: 1 throw (npm path-walk iteration cap)
  — name the start path, current directory, and what usually
  causes the cycle (symlinks).
- utils/terminal/iocraft.mts: 1 throw (native-module load failure)
  — show the underlying error and the offending platform/arch
  triple.

Skipped (already informative):
- github-provider.mts pass-through errors (forward inner CResult
  cause/message)
- gitlab-provider.mts try/catch wrappers that call
  formatErrorWithDetail (inner error has context)
- 'process.exit called' sentinel throws in npm/pnpm/yarn/with-
  subcommands paths (test harness re-raise markers, not user-facing)

Tests updated:
- test/unit/utils/promise/queue.test.mts (2 assertions)
- test/unit/utils/npm/spec.test.mts (2 assertions)
- test/unit/utils/git/gitlab-provider.test.mts (3 assertions)

Full suite (343 files / 5225 tests) passes.

Completes the series: #1255 (commands/) → #1256 (utils/dlx/) →
#1257 (utils/update + utils/command/) → #1258 (env/ + constants/) →
#1259 (test/) → this.

* fix(cli): address Cursor bugbot findings on error messages

Four issues flagged by Cursor bugbot on #1260:

1. (Medium) gitlab-provider.mts: error said 'check GL_TOKEN permissions'
   but the actual env var is GITLAB_TOKEN (as the same file's getGitLabToken
   confirms). Fixed to GITLAB_TOKEN.

2. (Medium) git/operations.mts: error suggested 'set SOCKET_CLI_GIT_PATH
   to point at a specific binary' — that env var is not read anywhere.
   Removed the false suggestion; kept the real fix (install git and put
   it on PATH) with package-manager examples.

3. (Low) terminal/iocraft.mts: '(e as Error).message' evaluates to
   undefined when a non-Error is thrown. Switched to
   'e instanceof Error ? e.message : String(e)' for safe stringification.

4. (Low) gitlab-provider.mts: error said 'after ${retries} retries' but
   the loop runs attempt 1..retries inclusive — retries is the total
   attempt count, not retries beyond the first. Reworded to 'attempts'.
   Matching test assertions updated.

Caught by #1260 bugbot review.

* chore(cli): use joinAnd + getErrorCause helpers in utils/ misc

- basics/vfs-extract.mts: missingTools list now renders as prose
  via joinAnd('a, b, and c').
- terminal/iocraft.mts: inline `e instanceof Error ? e.message :
  String(e)` swapped for getErrorCause(e). require() of a native
  binding can throw non-Error values, so the safe-stringify with
  UNKNOWN_ERROR fallback is correct here.

No behavior change for Error throws.
@jdalton John-David Dalton (jdalton) merged commit fb46e04 into main Apr 24, 2026
20 of 21 checks passed
@jdalton John-David Dalton (jdalton) deleted the jdd/error-msg-env-constants branch April 24, 2026 20:55
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