fix(cli): align utils/dlx/ error messages with 4-ingredient strategy#1256
Open
John-David Dalton (jdalton) wants to merge 4 commits intomainfrom
Open
fix(cli): align utils/dlx/ error messages with 4-ingredient strategy#1256John-David Dalton (jdalton) wants to merge 4 commits intomainfrom
John-David Dalton (jdalton) wants to merge 4 commits intomainfrom
Conversation
Rewrites error messages across packages/cli/src/utils/dlx/ to follow
the What / Where / Saw vs. wanted / Fix strategy from CLAUDE.md.
Sources:
- spawn.mts: 27 messages
- 6x 'Unexpected resolution type for <tool>' — now name the resolver
function and the actual resolution.type seen
- Archive/platform errors name the supported formats/platforms
- Python DLX errors surface the lock file path and cache dir
- PyPI fetch errors include the URL that failed
- Security errors (zip-slip, symlink escape) tell user to delete
the cached asset and report upstream
- resolve-binary.mts: 4 messages (socket-patch, trivy, trufflehog,
opengrep platform support) — each now lists supported platforms
and suggests how to install the tool manually
- vfs-extract.mts: 5 messages (SEA VFS extraction failures) — each
names what went wrong with the bundle and how to recover
(usually: rebuild SEA)
Internal invariant errors stay as plain Error (not InputError) but
are informative enough that if they ever fire, the user can open
a useful bug report.
Tests updated: test/unit/utils/dlx/resolve-binary.test.mts (1
substring match switched to regex).
Follows strategy from #1254. Part of the multi-PR series started
by #1255 (commands/).
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Error references variables before their declaration
- Moved the declarations of pythonDir, pythonBin, and lockFile above the retryCount >= MAX_RETRIES guard so they are initialized before the error message interpolates them, avoiding the temporal dead zone ReferenceError.
Or push these changes by commenting:
@cursor push 2a82947a44
Preview (2a82947a44)
diff --git a/packages/cli/src/utils/dlx/spawn.mts b/packages/cli/src/utils/dlx/spawn.mts
--- a/packages/cli/src/utils/dlx/spawn.mts
+++ b/packages/cli/src/utils/dlx/spawn.mts
@@ -1054,6 +1054,9 @@
*/
export async function ensurePythonDlx(retryCount = 0): Promise<string> {
const MAX_RETRIES = 3
+ const pythonDir = getPythonCachePath()
+ const pythonBin = getPythonBinPath(pythonDir)
+ const lockFile = path.join(pythonDir, '.downloading')
if (retryCount >= MAX_RETRIES) {
throw new InputError(
@@ -1061,10 +1064,6 @@
)
}
- const pythonDir = getPythonCachePath()
- const pythonBin = getPythonBinPath(pythonDir)
- const lockFile = path.join(pythonDir, '.downloading')
-
if (!existsSync(pythonBin)) {
await safeMkdir(pythonDir, { recursive: true })You can send follow-ups to the cloud agent here.
The previous commit referenced `${lockFile}` and `${pythonDir}` in
the MAX_RETRIES error message, but those consts were declared AFTER
the retry check. Hitting the retry path threw ReferenceError from
the temporal dead zone instead of the intended InputError.
Fix: move the three const declarations (pythonDir, pythonBin,
lockFile) above the MAX_RETRIES guard. Caught by Cursor bugbot
(#1256 (comment))
and confirmed by the type-check job.
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.
Switch to shared fleet helpers so error lists render as human prose
('a, b, and c') and error-cause stringification works safely for
non-Error throws (falls back to 'Unknown error' instead of crashing
or producing 'undefined').
- resolve-binary.mts: SOCKET_PATCH_ASSETS + OPENGREP_ASSETS
platform lists now use `joinAnd(Object.keys(...))`.
- vfs-extract.mts: missingTools list uses joinAnd; extract-failure
error now uses getErrorCause(e) — equivalent to the inline
'e instanceof Error ? e.message : String(e)' with the standard
UNKNOWN_ERROR fallback.
- spawn.mts: output-map listing uses joinAnd.
No behavior change for Error throws; non-Error throws now produce
'Unknown error' instead of '[object Object]' or similar.
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7c72686. Configure here.
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.

Summary
PR 3 in the error-message cleanup series started by #1254 (strategy) and #1255 (commands/). This one covers
packages/cli/src/utils/dlx/— the downloaded-tool runner stack.~35 error messages across 3 files. Tests updated.
What's fixed
spawn.mts(27 messages)6× "Unexpected resolution type for X" (coana, cdxgen, sfw, trivy, trufflehog, opengrep) — these are internal contract checks between
spawnX()andresolveX(). The old message said nothing. The new one names the resolver function and the actualresolution.typeseen, so a bug report is actionable:Before:
Unexpected resolution type for coanaAfter:
internal: resolveCoana returned resolution.type="local" (expected "dlx"); this is a resolver contract bug — re-run with --debug and report the outputArchive handling — the "Unsupported archive format", "Binary not found after extraction", and the two security rejections (zip-slip, unsafe symlink) now all name the file, the cache dir, and what to do about it. The security ones specifically tell you to stop trusting the release asset and delete the cache.
Platform detection —
Unsupported platform: linux→python-build-standalone does not ship a prebuilt for os.platform()="linux" (supported: darwin, linux, win32); install Python manually and point socket at it via PATH.PyPI / Python DLX — errors now include the HTTP status, the URL that was fetched, the exact package spec (
packageName==version), and the lock file path for retry lock failures.Package name validation — the two InputErrors at the top of the file now state the actual rule (regex) instead of prose.
resolve-binary.mts(4 messages)Platform-support errors for socket-patch / trivy / trufflehog / opengrep each list supported platforms and suggest how to install the tool manually as a fallback.
vfs-extract.mts(5 messages)SEA VFS extraction errors now all suggest "rebuild the SEA binary" and state what specifically went wrong (bundle missing tool, layout changed, etc.).
Error type
Left all these as
throw new Error(notInputError) because they're internal-invariant / system-level failures — the user didn't pass bad input, something deeper failed. But the messages now give enough context to file a useful bug.Tests
Only one existing test pinned on an old substring:
test/unit/utils/dlx/resolve-binary.test.mts— swapped to regex on the new phrasing.All 30 tests in
test/unit/utils/dlx/pass.Test plan
socket scan --reachon an unsupported platform prints a clearly-actionable platform errorsocket scan --reach— mismatch error names the asset and the tagNote
Low Risk
Low risk: changes are almost entirely error-message rewrites (plus minor refactor of local variables) with no intended behavior change, but they touch tool download/extraction paths so regressions would mainly affect failure-mode diagnostics.
Overview
Improves
utils/dlxfailure-mode diagnostics by rewriting ~35 errors across binary resolution, GitHub-release downloads/extraction, Python/PyPI installs, and SEA VFS extraction to follow a consistent, actionable format (include platform/asset/tool details, expected vs actual resolver contracts, and concrete remediation steps).Adds
joinAndfor nicer supported/missing lists, enhances internal invariant errors (e.g.,resolveXreturning unexpectedresolution.type), and updates the one unit test assertion to match the newresolveSocketPatchunsupported-platform wording.Reviewed by Cursor Bugbot for commit 7c72686. Configure here.