feat: add first-class CLI interaction modes#143
Conversation
📝 WalkthroughWalkthroughAdds interaction-mode resolution and CLI --mode wiring, structured error/recovery hints and output compatibility, a host-capability probe with observeHostFailure, integrations into credential/config stores and proxies, updated command auth/confirmation flows, doctor/reporting changes, and extensive tests and docs. ChangesInteraction Mode, Output & Recovery
Host Probe and Observability
Commands, run-with-core & ensure-auth
Doctor & Reporting
Possible related PRs:
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
Greptile SummaryThis PR separates output formatting from interaction control by introducing a first-class
Confidence Score: 4/5Safe to merge after addressing the sandbox warning suppression in JSON mode. The sandbox/host-execution warning system is the headline feature for agent visibility, but logVisibleWarn guards its console.error with !isJsonMode() and writeLog also skips output in JSON mode. In the typical agent invocation that auto-enables JSON output, the warning is completely silent. All other changes are well-constructed and previously-flagged issues are addressed. src/utils/debug.ts — the logVisibleWarn implementation is missing a JSON-mode branch to emit a structured warning to stderr. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
When the CLI runs inside an AI agent sandbox (Claude Code, Codex, Cursor), the keyring and home directory may be unavailable. Instead of letting opaque EPERM errors confuse the agent, we now: 1. Proactively probe home-fs and keychain on first auth check in non-interactive mode (warnIfSandboxed in ensure-auth) 2. Reactively observe permission errors in keyring read/write calls in both credential-store and config-store (observeHostFailure) 3. Emit a single actionable warning per session pointing the user to re-run on the host shell
- probeKeychain() no longer reports the keychain as failed when the
probe entry is simply absent. A 'not found' / 'No such' error from
@napi-rs/keyring now indicates a healthy keychain (matches the
existing pattern in deleteFromKeyring in config-store and
credential-store), so non-interactive runs on healthy hosts no longer
emit false-positive sandbox warnings.
- Replace require('@napi-rs/keyring') with a static ES import. The
package has 'type: module', so the previous require() threw
ReferenceError at runtime and caused probeKeychain to always fail.
- Switch probeHomeFs to node:fs/promises and make runHostProbe and
warnIfSandboxed async, per the project's no-sync-fs guideline.
- Tighten the /sandbox/i permission pattern to /\bsandboxd?\b/i so
unrelated error messages containing 'sandbox' as a substring don't
trigger sandbox warnings.
Updates the spec to drive the async API and adds coverage for the
healthy-keychain (entry-absent) and substring-collision cases.
Co-Authored-By: nick.nisi@workos.com <nick.nisi@workos.com>
Co-Authored-By: nick.nisi@workos.com <nick.nisi@workos.com>
probeHomeFs previously treated every fs error as a sandbox indicator, so transient errors like ENOSPC or EIO would emit a misleading "sandboxed environment" warning. Gate the catch block on isPermissionError so it stays consistent with observeHostFailure. Co-Authored-By: nick.nisi@workos.com <nick.nisi@workos.com>
probeKeychain previously treated any non-missing-entry keychain error as a sandbox indicator. A user-canceled macOS keychain prompt or a transient keyring daemon error would therefore produce a misleading "sandboxed environment" warning on healthy hosts. Mirror probeHomeFs and observeHostFailure by ignoring non-permission errors. Co-Authored-By: nick.nisi@workos.com <nick.nisi@workos.com>
nicknisi
force-pushed
the
nick/host-capability-probe
branch
from
1046ad3 to
a9b122a
Compare
Move fs.unlink into a finally block with best-effort error handling so a successful writeFile never leaves an orphan probe file in ~/.workos when unlink itself fails. The probe is checking write access; cleanup should not affect the result. Co-Authored-By: nick.nisi@workos.com <nick.nisi@workos.com>
nicknisi
changed the title
nicknisi
changed the title
There was a problem hiding this comment.
Actionable comments posted: 6
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ff7cc87c-3f3a-46a9-92dd-6473aaee1be5
📒 Files selected for processing (51)
DEVELOPMENT.mdREADME.mdsrc/bin.tssrc/commands/api/index.spec.tssrc/commands/api/index.tssrc/commands/claim.spec.tssrc/commands/claim.tssrc/commands/connection.spec.tssrc/commands/connection.tssrc/commands/debug.spec.tssrc/commands/debug.tssrc/commands/directory.spec.tssrc/commands/directory.tssrc/commands/env.spec.tssrc/commands/env.tssrc/commands/login.spec.tssrc/commands/login.tssrc/doctor/checks/auth-patterns.spec.tssrc/doctor/checks/auth-patterns.tssrc/doctor/checks/host-execution.spec.tssrc/doctor/checks/host-execution.tssrc/doctor/index.tssrc/doctor/issues.spec.tssrc/doctor/issues.tssrc/doctor/json-output.spec.tssrc/doctor/output.spec.tssrc/doctor/output.tssrc/doctor/types.tssrc/lib/config-store.tssrc/lib/credential-proxy.tssrc/lib/credential-store.tssrc/lib/ensure-auth.spec.tssrc/lib/ensure-auth.tssrc/lib/host-probe.spec.tssrc/lib/host-probe.tssrc/lib/run-with-core.tssrc/utils/debug.spec.tssrc/utils/debug.tssrc/utils/environment.spec.tssrc/utils/environment.tssrc/utils/exit-codes.spec.tssrc/utils/exit-codes.tssrc/utils/help-json.spec.tssrc/utils/help-json.tssrc/utils/interaction-mode.spec.tssrc/utils/interaction-mode.tssrc/utils/mode-compatibility.spec.tssrc/utils/output.spec.tssrc/utils/output.tssrc/utils/recovery-hints.spec.tssrc/utils/recovery-hints.ts
✅ Files skipped from review due to trivial changes (1)
- src/doctor/json-output.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/host-probe.spec.ts
| it('does not inspect WORKOS_FORCE_TTY directly', () => { | ||
| process.env.WORKOS_FORCE_TTY = '1'; | ||
| setInteractionMode({ mode: 'agent', source: 'non_tty' }); | ||
| expect(isNonInteractiveEnvironment()).toBe(true); | ||
| delete process.env.WORKOS_FORCE_TTY; | ||
| }); |
There was a problem hiding this comment.
Restore previous WORKOS_FORCE_TTY value instead of always deleting it.
Line 30 can leak state when this env var is already set in the test process.
Suggested fix
it('does not inspect WORKOS_FORCE_TTY directly', () => {
+ const prevForceTty = process.env.WORKOS_FORCE_TTY;
process.env.WORKOS_FORCE_TTY = '1';
setInteractionMode({ mode: 'agent', source: 'non_tty' });
expect(isNonInteractiveEnvironment()).toBe(true);
- delete process.env.WORKOS_FORCE_TTY;
+ if (prevForceTty === undefined) {
+ delete process.env.WORKOS_FORCE_TTY;
+ } else {
+ process.env.WORKOS_FORCE_TTY = prevForceTty;
+ }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('does not inspect WORKOS_FORCE_TTY directly', () => { | |
| process.env.WORKOS_FORCE_TTY = '1'; | |
| setInteractionMode({ mode: 'agent', source: 'non_tty' }); | |
| expect(isNonInteractiveEnvironment()).toBe(true); | |
| delete process.env.WORKOS_FORCE_TTY; | |
| }); | |
| it('does not inspect WORKOS_FORCE_TTY directly', () => { | |
| const prevForceTty = process.env.WORKOS_FORCE_TTY; | |
| process.env.WORKOS_FORCE_TTY = '1'; | |
| setInteractionMode({ mode: 'agent', source: 'non_tty' }); | |
| expect(isNonInteractiveEnvironment()).toBe(true); | |
| if (prevForceTty === undefined) { | |
| delete process.env.WORKOS_FORCE_TTY; | |
| } else { | |
| process.env.WORKOS_FORCE_TTY = prevForceTty; | |
| } | |
| }); |
- Use `{ wait: false }` for opn() in device auth to avoid blocking poll
- Disambiguate error messages for CI vs generic non-prompt scenarios
- Skip --mode values in help-json command extraction
- Fix contradictory WORKOS_NO_PROMPT docs in README
| import type { InstallerOptions } from '../utils/types.js'; | ||
| import { isNonInteractiveEnvironment } from '../utils/environment.js'; | ||
| import { getInteractionMode, isAgentMode, isCiMode } from '../utils/interaction-mode.js'; | ||
| import { getOutputMode, isJsonMode, resolveEffectiveOutputMode, setOutputMode } from '../utils/output.js'; |
There was a problem hiding this comment.
resolveEffectiveOutputMode does not exist in output.ts
resolveEffectiveOutputMode is imported from ../utils/output.js here but is never defined or exported from that module. With strict: true and "module": "nodenext" in tsconfig.json, TypeScript will reject this as a compile-time error, and Node.js ESM will throw a hard link-time SyntaxError at startup when this module is first loaded — breaking the entire installer flow for any runWithCore invocation. The function needs to be defined and exported from output.ts.
| : isCiMode() | ||
| ? 'Name and API key required in CI mode.' | ||
| : 'Name and API key required when prompting is unavailable.', | ||
| recovery: missingArgsRecovery(undefined, 'Provide environment name and API key as positional arguments.'), |
There was a problem hiding this comment.
undefined passed to command: string parameter
missingArgsRecovery is declared as (command: string, description: string), but undefined is passed as the first argument. With strictNullChecks: true this is a TypeScript compile error. Since the intent is a description-only hint, the signature of missingArgsRecovery should accept string | undefined, or the call site should pass a concrete example command.
| recovery: missingArgsRecovery(undefined, 'Provide environment name and API key as positional arguments.'), | |
| recovery: missingArgsRecovery( | |
| formatWorkOSCommand('env add <name> <api-key> [--client-id <id>]'), | |
| 'Provide environment name and API key as positional arguments.', | |
| ), |
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
src/utils/environment.spec.ts (1)
26-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRestore prior
WORKOS_FORCE_TTYvalue instead of always deleting it.Line 30 can clobber an existing test-process env value and leak state across tests.
Suggested fix
it('does not inspect WORKOS_FORCE_TTY directly', () => { + const prevForceTty = process.env.WORKOS_FORCE_TTY; process.env.WORKOS_FORCE_TTY = '1'; - setInteractionMode({ mode: 'agent', source: 'non_tty' }); - expect(isNonInteractiveEnvironment()).toBe(true); - delete process.env.WORKOS_FORCE_TTY; + try { + setInteractionMode({ mode: 'agent', source: 'non_tty' }); + expect(isNonInteractiveEnvironment()).toBe(true); + } finally { + if (prevForceTty === undefined) delete process.env.WORKOS_FORCE_TTY; + else process.env.WORKOS_FORCE_TTY = prevForceTty; + } });
🧹 Nitpick comments (1)
src/commands/api/index.spec.ts (1)
367-375: ⚡ Quick winMake the agent-mode stderr assertion less brittle.
Line 374 hard-codes a typo-prone literal (
agent mode require --yes). Prefer intent-based matching so copy tweaks don’t cause test churn.♻️ Suggested change
- expect(stderrOutput.some((l) => l.includes('agent mode require --yes'))).toBe(true); + expect(stderrOutput.some((l) => /agent mode.*--yes/i.test(l))).toBe(true);
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b3a843be-a7d5-428c-bbe3-ae407f3305c3
📒 Files selected for processing (51)
DEVELOPMENT.mdREADME.mdsrc/bin.tssrc/commands/api/index.spec.tssrc/commands/api/index.tssrc/commands/claim.spec.tssrc/commands/claim.tssrc/commands/connection.spec.tssrc/commands/connection.tssrc/commands/debug.spec.tssrc/commands/debug.tssrc/commands/directory.spec.tssrc/commands/directory.tssrc/commands/env.spec.tssrc/commands/env.tssrc/commands/login.spec.tssrc/commands/login.tssrc/doctor/checks/auth-patterns.spec.tssrc/doctor/checks/auth-patterns.tssrc/doctor/checks/host-execution.spec.tssrc/doctor/checks/host-execution.tssrc/doctor/index.tssrc/doctor/issues.spec.tssrc/doctor/issues.tssrc/doctor/json-output.spec.tssrc/doctor/output.spec.tssrc/doctor/output.tssrc/doctor/types.tssrc/lib/config-store.tssrc/lib/credential-proxy.tssrc/lib/credential-store.tssrc/lib/ensure-auth.spec.tssrc/lib/ensure-auth.tssrc/lib/host-probe.spec.tssrc/lib/host-probe.tssrc/lib/run-with-core.tssrc/utils/debug.spec.tssrc/utils/debug.tssrc/utils/environment.spec.tssrc/utils/environment.tssrc/utils/exit-codes.spec.tssrc/utils/exit-codes.tssrc/utils/help-json.spec.tssrc/utils/help-json.tssrc/utils/interaction-mode.spec.tssrc/utils/interaction-mode.tssrc/utils/mode-compatibility.spec.tssrc/utils/output.spec.tssrc/utils/output.tssrc/utils/recovery-hints.spec.tssrc/utils/recovery-hints.ts
✅ Files skipped from review due to trivial changes (2)
- src/utils/interaction-mode.spec.ts
- src/utils/recovery-hints.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/host-probe.spec.ts
| : isCiMode() | ||
| ? 'Name and API key required in CI mode.' | ||
| : 'Name and API key required when prompting is unavailable.', | ||
| recovery: missingArgsRecovery(undefined, 'Provide environment name and API key as positional arguments.'), |
There was a problem hiding this comment.
Fix blocking type error in missingArgsRecovery call
Line 46 passes undefined where a string is required, which is breaking typecheck in CI.
Suggested patch
} else if (!isPromptAllowed()) {
+ const recoveryCommand = formatWorkOSCommand('env add <name> <api-key> --client-id <client-id>');
exitWithError({
code: 'missing_args',
message: isAgentMode()
? `Name and API key required in agent mode. Example: ${formatWorkOSCommand('env add staging sk_test_xxx --client-id client_xxx')}`
: isCiMode()
? 'Name and API key required in CI mode.'
: 'Name and API key required when prompting is unavailable.',
- recovery: missingArgsRecovery(undefined, 'Provide environment name and API key as positional arguments.'),
+ recovery: missingArgsRecovery(recoveryCommand, 'Provide environment name and API key as positional arguments.'),
});
} else {🧰 Tools
🪛 GitHub Actions: CI / 0_Node 24.txt
[error] 46-46: TypeScript (tsc --noEmit) failed: TS2345 - Argument of type 'undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Actions: CI / 1_Node 20.txt
[error] 46-46: TypeScript (tsc) typecheck failed: TS2345: Argument of type 'undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Actions: CI / 2_Node 22.txt
[error] 46-46: TypeScript (tsc) error TS2345: Argument of type 'undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Actions: CI / Node 20
[error] 46-46: TypeScript typecheck failed (pnpm typecheck). TS2345: Argument of type 'undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Actions: CI / Node 22
[error] 46-46: TypeScript (tsc) error TS2345: Argument of type 'undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Actions: CI / Node 24
[error] 46-46: TypeScript typecheck failed (tsc --noEmit). TS2345: Argument of type 'undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Check: Node 20
[failure] 46-46:
Argument of type 'undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Check: Node 22
[failure] 46-46:
Argument of type 'undefined' is not assignable to parameter of type 'string'.
🪛 GitHub Check: Node 24
[failure] 46-46:
Argument of type 'undefined' is not assignable to parameter of type 'string'.
| try { | ||
| const content = fs.readFileSync(getConfigFilePath(), 'utf-8'); | ||
| const content = fs.readFileSync(filePath, 'utf-8'); | ||
| return JSON.parse(content); | ||
| } catch (error) { | ||
| observeHostFailure('home-fs', error, { | ||
| operation: 'read', | ||
| target: filePath, | ||
| label: 'config fallback file', | ||
| }); |
There was a problem hiding this comment.
Do not classify JSON parse failures as host capability failures
Line 81 and Line 139 currently report home-fs/keychain host failures for any exception, including malformed JSON. That can produce false HOST_EXECUTION_UNTRUSTED diagnostics.
Suggested fix (separate IO/keychain read from JSON parsing)
function readFromFile(): CliConfig | null {
if (!fileExists()) return null;
const filePath = getConfigFilePath();
+ let content: string;
try {
- const content = fs.readFileSync(filePath, 'utf-8');
- return JSON.parse(content);
+ content = fs.readFileSync(filePath, 'utf-8');
} catch (error) {
observeHostFailure('home-fs', error, {
operation: 'read',
target: filePath,
label: 'config fallback file',
});
logWarn('Failed to read config file:', error);
return null;
}
+ try {
+ return JSON.parse(content);
+ } catch (error) {
+ logWarn('Failed to parse config file:', error);
+ return null;
+ }
}
function readFromKeyring(): CliConfig | null {
+ let data: string | null;
try {
const entry = getKeyringEntry();
- const data = entry.getPassword();
- if (!data) return null;
- return JSON.parse(data);
+ data = entry.getPassword();
} catch (error) {
logWarn('Failed to read config from keyring:', error);
observeHostFailure('keychain', error, {
operation: 'read',
target: `${SERVICE_NAME}/${ACCOUNT_NAME}`,
label: 'config keychain entry',
});
return null;
}
+ if (!data) return null;
+ try {
+ return JSON.parse(data);
+ } catch (error) {
+ logWarn('Failed to parse config from keyring:', error);
+ return null;
+ }
}Also applies to: 133-143
| function readFromFile(): Credentials | null { | ||
| if (!fileExists()) return null; | ||
| const filePath = getCredentialsPath(); | ||
| try { | ||
| const content = fs.readFileSync(getCredentialsPath(), 'utf-8'); | ||
| const content = fs.readFileSync(filePath, 'utf-8'); | ||
| return JSON.parse(content); | ||
| } catch (error) { | ||
| observeHostFailure('home-fs', error, { | ||
| operation: 'read', | ||
| target: filePath, | ||
| label: 'credential fallback file', | ||
| }); |
There was a problem hiding this comment.
Don’t report JSON parse failures as host capability failures.
The new observeHostFailure(...) calls are inside catch blocks that also capture JSON.parse errors. Corrupt credential payloads are data-integrity issues, not host/sandbox failures, and this can produce false hostExecution trust warnings.
Suggested split of I/O vs parse error handling
function readFromFile(): Credentials | null {
if (!fileExists()) return null;
const filePath = getCredentialsPath();
+ let content: string;
try {
- const content = fs.readFileSync(filePath, 'utf-8');
- return JSON.parse(content);
+ content = fs.readFileSync(filePath, 'utf-8');
} catch (error) {
observeHostFailure('home-fs', error, {
operation: 'read',
target: filePath,
label: 'credential fallback file',
});
logWarn('Failed to read credentials file:', error);
return null;
}
+ try {
+ return JSON.parse(content);
+ } catch (error) {
+ logWarn('Failed to parse credentials file:', error);
+ return null;
+ }
}
function readFromKeyring(): Credentials | null {
try {
const entry = getKeyringEntry();
const data = entry.getPassword();
if (!data) {
logWarn('[credential-store] keyring: entry exists but data is null/empty');
return null;
}
- return JSON.parse(data);
+ try {
+ return JSON.parse(data);
+ } catch (error) {
+ logWarn('[credential-store] keyring payload parse failed:', error);
+ return null;
+ }
} catch (error) {
const msg = error instanceof Error ? error.message : String(error);
logWarn(`[credential-store] keyring read failed: ${msg}`);
observeHostFailure('keychain', error, {
operation: 'read',
target: `${SERVICE_NAME}/${ACCOUNT_NAME}`,
label: 'credential keychain entry',
});
return null;
}
}Also applies to: 112-129
| function isLikelyHostFailure(capability: HostCapability, error: unknown): boolean { | ||
| if (capability === 'browser-launch' || capability === 'localhost-bind') { | ||
| return true; | ||
| } | ||
|
|
||
| return isPermissionError(error); | ||
| } |
There was a problem hiding this comment.
Unconditional host-failure classification causes false sandbox warnings.
Line 58 currently treats all browser-launch/localhost-bind errors as host-trust failures. This can mislabel ordinary runtime issues (like address-in-use) as sandbox problems.
🐛 Suggested narrowing
function isLikelyHostFailure(capability: HostCapability, error: unknown): boolean {
- if (capability === 'browser-launch' || capability === 'localhost-bind') {
- return true;
- }
+ const msg = error instanceof Error ? error.message : String(error);
+ if (capability === 'localhost-bind') {
+ if (/\bEADDRINUSE\b|address already in use/i.test(msg)) return false;
+ return isPermissionError(error);
+ }
+ if (capability === 'browser-launch') {
+ return isPermissionError(error) || /\bsandboxd?\b|interaction is not allowed/i.test(msg);
+ }
return isPermissionError(error);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function isLikelyHostFailure(capability: HostCapability, error: unknown): boolean { | |
| if (capability === 'browser-launch' || capability === 'localhost-bind') { | |
| return true; | |
| } | |
| return isPermissionError(error); | |
| } | |
| function isLikelyHostFailure(capability: HostCapability, error: unknown): boolean { | |
| const msg = error instanceof Error ? error.message : String(error); | |
| if (capability === 'localhost-bind') { | |
| if (/\bEADDRINUSE\b|address already in use/i.test(msg)) return false; | |
| return isPermissionError(error); | |
| } | |
| if (capability === 'browser-launch') { | |
| return isPermissionError(error) || /\bsandboxd?\b|interaction is not allowed/i.test(msg); | |
| } | |
| return isPermissionError(error); | |
| } |
| observeHostFailure('browser-launch', error, { | ||
| operation: 'open', | ||
| target: deviceAuth.verification_uri_complete, | ||
| label: 'installer device auth browser', |
There was a problem hiding this comment.
Avoid storing full browser URLs in host-failure details
Line 385 and Line 512 pass full URLs to observeHostFailure. These URLs can include sensitive query params (auth/device codes), which may leak via diagnostics.
Suggested fix (redact query/fragment before reporting)
+function redactUrlForDiagnostics(rawUrl: string): string {
+ try {
+ const u = new URL(rawUrl);
+ return `${u.origin}${u.pathname}`;
+ } catch {
+ return rawUrl;
+ }
+}
observeHostFailure('browser-launch', error, {
operation: 'open',
- target: deviceAuth.verification_uri_complete,
+ target: redactUrlForDiagnostics(deviceAuth.verification_uri_complete),
label: 'installer device auth browser',
});
observeHostFailure('browser-launch', error, {
operation: 'open',
- target: inspectUrl,
+ target: redactUrlForDiagnostics(inspectUrl),
label: 'XState inspector browser',
});Also applies to: 510-513
| const helpJsonCommandNames = new Set([ | ||
| ...commands.map((command) => command.name.split(' ')[0]), | ||
| ...Object.keys(commandAliases), | ||
| ]); | ||
|
|
||
| /** | ||
| * Extract the requested command from raw argv before yargs parses --help. | ||
| * | ||
| * This intentionally matches only known command names so option values from | ||
| * global flags like `--mode agent` are not mistaken for commands. | ||
| */ | ||
| export function extractHelpJsonCommand(argv: string[]): string | undefined { | ||
| for (let i = 0; i < argv.length; i++) { | ||
| const arg = argv[i]; | ||
| if (arg === '--mode') { | ||
| i += 1; | ||
| continue; | ||
| } | ||
| if (arg.startsWith('--mode=')) { | ||
| continue; | ||
| } | ||
| if (!arg.startsWith('-') && helpJsonCommandNames.has(arg)) { | ||
| return commandAliases[arg] ?? arg; | ||
| } | ||
| } | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
extractHelpJsonCommand loses multi-token commands (e.g. auth login).
Line 1332 only indexes first tokens, so ['auth','login','--help','--json'] resolves to auth, which misses buildCommandTree() exact matching and returns the full tree.
Suggested fix
-const helpJsonCommandNames = new Set([
- ...commands.map((command) => command.name.split(' ')[0]),
- ...Object.keys(commandAliases),
-]);
+const helpJsonCommandNames = new Set(commands.map((command) => command.name));
export function extractHelpJsonCommand(argv: string[]): string | undefined {
+ const tokens: string[] = [];
for (let i = 0; i < argv.length; i++) {
const arg = argv[i];
if (arg === '--mode') {
i += 1;
continue;
}
if (arg.startsWith('--mode=')) {
continue;
}
- if (!arg.startsWith('-') && helpJsonCommandNames.has(arg)) {
- return commandAliases[arg] ?? arg;
+ if (!arg.startsWith('-')) {
+ tokens.push(commandAliases[arg] ?? arg);
}
}
+ for (let len = tokens.length; len > 0; len--) {
+ const candidate = tokens.slice(0, len).join(' ');
+ if (helpJsonCommandNames.has(candidate)) return candidate;
+ }
return undefined;
}…covery hints - resolveEffectiveOutputMode forces JSON when WORKOS_MODE is explicitly agent or ci, preventing human-formatted noise in machine streams - Shell-safe confirmation commands via buildConfirmationCommand with proper quoting for --data JSON values - Omit recovery command when it cannot be deterministically rebuilt (stdin body, explicit API key) - Suppress human warnings (emoji/chalk) in JSON output mode - Skip update check when prompting is disallowed
| writeLog('WARN', '⚠️ ', args); | ||
| } | ||
|
|
||
| export function logVisibleWarn(...args: unknown[]): void { | ||
| const msg = writeLog('WARN', '⚠️ ', args); | ||
| if (!debugEnabled && !isJsonMode()) { | ||
| console.error(chalk.yellow(`⚠️ ${msg}`)); | ||
| } |
There was a problem hiding this comment.
logVisibleWarn is always silent in JSON mode — sandbox warning never reaches agents
logVisibleWarn guards its only console.error call with !isJsonMode(), and writeLog also skips its clack output when isJsonMode() is true. So in agent/CI mode with JSON output — the exact context this feature targets — both warnIfSandboxed() and observeHostFailure() silently swallow the warning. An agent running inside a sandbox receives auth_required errors with no indication that the host filesystem or keychain is unreachable. The warning should be emitted to stderr as a structured JSON object in JSON mode so agents can parse it.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/commands/api/index.spec.ts (1)
2-4: ⚡ Quick winReplace sync fs APIs with async alternatives in test setup and cleanup.
The test functions are already async, so
mkdtempSync,writeFileSync, andrmSync(lines 2, 417, 419, 436) can be replaced with their async equivalents fromfs/promiseswithout changing test behavior.Proposed refactor
-import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; @@ - const dir = mkdtempSync(join(tmpdir(), 'workos-api-')); + const dir = await mkdtemp(join(tmpdir(), 'workos-api-')); @@ - writeFileSync(file, '{"name":"Acme"}'); + await writeFile(file, '{"name":"Acme"}'); @@ - rmSync(dir, { recursive: true, force: true }); + await rm(dir, { recursive: true, force: true });Per coding guidelines:
src/**/*.{ts,tsx,js}: Avoid Node-specific sync APIs (crypto, fs sync) unless necessary
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d45bdf39-6e34-4440-a274-20ab8a82bb7d
📒 Files selected for processing (13)
src/bin.tssrc/commands/api/index.spec.tssrc/commands/api/index.tssrc/commands/env.spec.tssrc/utils/command-invocation.spec.tssrc/utils/command-invocation.tssrc/utils/debug.spec.tssrc/utils/debug.tssrc/utils/mode-compatibility.spec.tssrc/utils/output.spec.tssrc/utils/output.tssrc/utils/recovery-hints.spec.tssrc/utils/recovery-hints.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/utils/recovery-hints.spec.ts
- src/commands/env.spec.ts
- src/utils/recovery-hints.ts
- src/commands/api/index.ts
- src/bin.ts
- src/utils/debug.ts
* feat(workos): add CLI agent-mode guidance to skill Coding agents now have explicit instructions for invoking the WorkOS CLI: - Recommend `WORKOS_MODE=agent workos doctor --json --skip-ai` as the setup/debugging preflight. - Document the two-axis model: `--json` is output formatting only; `WORKOS_MODE=agent` controls prompts, browser fallback, and host trust. - Treat `HOST_EXECUTION_UNTRUSTED` from doctor as a hard sandbox trust boundary that requires re-running on the host shell before trusting auth/config/API failures. - Note legacy compatibility for `WORKOS_NO_PROMPT` and `WORKOS_FORCE_TTY`. - Update workos-widgets install prompts to use `WORKOS_MODE=agent` when asking the user to run `npx workos@latest install`. The CLI surface (mode resolver, doctor visibility, recovery hints) is landing in the workos/cli repo (workos/cli#143); this skill update teaches agents to use it. * fix(workos): tighten agent-mode guidance for precision and rot-resistance Reframe guardrail as prohibition, explain --skip-ai flag, add --help fallback for confirmation flags, clarify WORKOS_NO_PROMPT migration path, and remove redundant parenthetical from widgets workflow. * fix(workos): address second-pass review findings - Fix broken WORKOS_NO_PROMPT migration syntax (was not a runnable fragment) - Add CLI/agent-mode/doctor to skill description for triggering - Drop "machine-readable" framing from widgets to avoid implying agent mode replaces --json - Add --skip-ai version fallback to cli-upgrade reference - Soften doctor JSON shape claims to resist schema rot * fix(workos): align agent-mode guidance with shipped CLI implementation - Soften guardrail from "never run without" to "prefer" since the CLI auto-detects CLAUDECODE, CLAUDE_CODE, CURSOR_AGENT, CODEX_SANDBOX, CURSOR_TRACE_ID, and non-TTY - Document the two-axis model (interaction mode vs output mode) with the three possible interaction values (human/agent/ci) - Restore precise doctor field names (interactionMode, hostExecution) since they're stable type definitions in the CLI - Fix recovery hints shape: describe per-hint fields (description, command, hostShellRequired) and note JSON is emitted on stderr - Enumerate all known confirmation flags including debug reset
1 participant
Summary
--mode human|agent|ciandWORKOS_MODE, while keeping--jsonscoped to output formatting.WORKOS_NO_PROMPT, non-TTY JSON defaults, andWORKOS_FORCE_TTYas an output-only override.interactionModeinworkos doctorand runs host-execution probing for agent/CI mode so sandboxed runs are visible.Why
Coding agents and CI need deterministic non-prompt behavior, but
--jsonis only an output contract. Before this, output formatting, TTY detection, prompts, browser launches, and sandbox trust were coupled. That made pseudo-TTY agents and scripts hard to reason about and made host-sandbox failures look like real WorkOS configuration or auth failures.This PR separates the two axes: output mode controls formatting; interaction mode controls who is driving the CLI and which behaviors are allowed.
Risk
WORKOS_FORCE_TTYnow only forces human output, not human interaction. UseWORKOS_MODE=humanfor that.Validation
pnpm test(138 files, 1776 tests)pnpm typecheckpnpm lintpnpm format:checkpnpm buildpnpm exec tsx src/bin.ts doctor --json --skip-ai --skip-api --install-dir "$PWD"-> inferredagent / non_ttyreport; exits 1 here because this repo intentionally lacks WorkOS env varsWORKOS_MODE=agent pnpm exec tsx src/bin.ts doctor --json --skip-ai --skip-api --install-dir "$PWD"->agent / envpnpm exec tsx src/bin.ts --mode agent doctor --json --skip-ai --skip-api --install-dir "$PWD"->agent / flagWORKOS_MODE=ci pnpm exec tsx src/bin.ts doctor --json --skip-ai --skip-api --install-dir "$PWD"->ci / envWORKOS_MODE=human pnpm exec tsx src/bin.ts doctor --json --skip-ai --skip-api --install-dir "$PWD"->human / env, proving JSON output and interaction mode are separatepnpm exec tsx src/bin.ts --mode agent doctor --help --json-> doctor command subtreeSummary by CodeRabbit
New Features
User-Visible Improvements
Documentation