Consolidate Coana launcher env vars into SOCKET_CLI_COANA_LAUNCHER#1360
Consolidate Coana launcher env vars into SOCKET_CLI_COANA_LAUNCHER#1360Martin Torp (mtorp) wants to merge 2 commits into
Conversation
…A_LAUNCHER The npm-install launcher path could previously be tuned via two boolean env vars: SOCKET_CLI_COANA_FORCE_NPM_INSTALL (skip npx, always npm install + node) and SOCKET_CLI_COANA_DISABLE_NPM_FALLBACK (npx only, never fall back). They really express three modes of one setting, so replace them with a single SOCKET_CLI_COANA_LAUNCHER variable taking auto (default), npx, or npm-install. The legacy variables remain supported for backward compatibility when the new variable is unset, but are intentionally left undocumented. Unrecognized values warn and behave as auto. Follow-up from the review discussion on SocketDev/socket-python-cli#230; the Python CLI is getting the same change.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort 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 db44ab5. Configure here.
| finalEnv, | ||
| { cwd: dlxOptions.cwd }, | ||
| spawnExtra, | ||
| ) |
There was a problem hiding this comment.
Options stdio ignored npm-install
Medium Severity
spawnCoanaDlx now promotes stdio from the options argument into the dlx launcher, but the local-path, npm-install, and auto-mode fallback branches still only pass the fourth spawnExtra argument into spawnCoanaScriptViaNode. Callers such as socket fix set stdio on options (e.g. pipe when --silence is used), so those launcher modes ignore the requested stdio and default to inherit.
Additional Locations (1)
Triggered by learned rule: shadowNpmBase stdio goes in options, not spawnExtra — don't muzzle dlx launchers
Reviewed by Cursor Bugbot for commit db44ab5. Configure here.
The dlx branch resolves the caller's requested stdio from both the options and spawnExtra arguments, but spawnCoanaScriptViaNode only read spawnExtra. The local-path, forced npm-install, and auto-mode fallback branches therefore dropped stdio passed via options and defaulted to inherit — `socket fix --silence` requests stdio 'pipe' via options, so those paths leaked Coana output to the terminal. Resolve the requested stdio once and thread it through every launch path.
Martin Torp (mtorp)
requested a review
from Oskar Haarklou Veileborg (BarrensZeppelin)
Oskar Haarklou Veileborg (BarrensZeppelin)
left a comment
Oskar Haarklou Veileborg (BarrensZeppelin)
left a comment
There was a problem hiding this comment.
LGTM 🎉
2 participants
Why
Follow-up from this review discussion on the Python CLI: the two env vars tuning how the pinned
@coana-tech/cliengine is launched —SOCKET_CLI_COANA_FORCE_NPM_INSTALL(skip npx, alwaysnpm install+node) andSOCKET_CLI_COANA_DISABLE_NPM_FALLBACK(npx only, never fall back) — really express three modes of one setting, so they're better off as a single variable. A matching PR updates the Python CLI.What changed
SOCKET_CLI_COANA_LAUNCHERvariable:auto(default / unset) — try npx/dlx first, fall back tonpm install+nodeon launcher-level failures.npx— never fall back; surface the dlx failure directly.npm-install— skip dlx entirely; alwaysnpm install+node.auto.SOCKET_CLI_COANA_LAUNCHERis documented (README).SOCKET_CLI_COANA_LOCAL_PATHis unrelated (takes a path) and is unchanged.Testing
src/utils/dlx.test.mtscovernpm-installmode,npxmode, precedence over the legacy variables, and unrecognized-value handling; existing legacy-var tests are kept as back-compat coverage.pnpm test:unit src/utils/dlx.test.mts— 26/26 passing;pnpm run checkclean.Note
Low Risk
Behavioral change is limited to Coana spawn strategy and preserves legacy env vars when the new one is unset; mis-set values only warn and default to auto.
Overview
Replaces two undocumented Coana launch toggles with a single
SOCKET_CLI_COANA_LAUNCHERenv var that selects how@coana-tech/cliis started:auto(try npx/dlx, thennpm install+nodeon launcher failure),npx(dlx only, no fallback), ornpm-install(skip dlx entirely).When the new variable is set, it overrides
SOCKET_CLI_COANA_FORCE_NPM_INSTALLandSOCKET_CLI_COANA_DISABLE_NPM_FALLBACK; those legacy flags still work if the new var is unset. Invalid launcher values log a warning and behave likeauto.SOCKET_CLI_COANA_LOCAL_PATHis unchanged.spawnCoanaDlxinsrc/utils/dlx.mtsroutes throughgetCoanaLauncherMode(); README documents only the new variable. Unit tests insrc/utils/dlx.test.mtscover each mode, precedence, and bad values.Reviewed by Cursor Bugbot for commit db44ab5. Configure here.
Also fixed
Cursor Bugbot flagged (correctly) that the local-path, forced npm-install, and auto-mode fallback branches only honored
stdiopassed via thespawnExtraargument, droppingstdioset on the options argument (e.g.socket fix --silencerequestsstdio: 'pipe'via options) and defaulting toinherit. This predates this PR (#1353 only fixed the dlx branch), but since it's the same plumbing, the requested stdio is now resolved once and threaded through every launch path, with regression tests for the npm-install and fallback paths.