feat!: consent-gated agent setup and flat CLI output#200
Conversation
Greptile SummaryThis PR adds consent-based coding-agent setup and simplifies CLI output. The main changes are:
Confidence Score: 5/5The latest changes appear safe to merge.
Important Files Changed
Reviews (6): Last reviewed commit: "refactor: dedup UI palette, lazy-load in..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| } else if (!isPromptAllowed() && !opts.assumeYes) { | ||
| // Explicit `workos setup` in a non-interactive context needs --yes. | ||
| exitWithError({ | ||
| code: 'confirmation_required', | ||
| message: `Interactive setup needs a TTY. Re-run \`${formatWorkOSCommand('setup --yes')}\` to install non-interactively.`, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🔍 workos setup --json on a TTY prompts interactively and corrupts JSON stdout
In the command trigger path, gating only blocks non-interactive contexts: else if (!isPromptAllowed() && !opts.assumeYes) errors, but there is no isJsonMode() guard (src/commands/setup.ts:114-120). If a user runs workos setup --json on an interactive TTY, output mode is JSON while interaction mode stays human, so isPromptAllowed() is true and assumeYes is false. The flow then falls through to the offer block (src/commands/setup.ts:143-160) and calls ui.heading/ui.note (which console.log human text) and ui.confirm (an interactive prompt), polluting the machine-readable stdout stream. The automatic (login/install) path explicitly guards isJsonMode() (src/commands/setup.ts:112) and the api command errors in JSON mode rather than prompting, so this is an inconsistency. Note that connection/directory delete share the same TTY-prompt-without-JSON-guard pattern, so this may be an accepted repo convention; worth confirming whether workos setup --json on a TTY should instead require --yes.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const [skillResult, mcpResults] = await Promise.all([ | ||
| skillAgents.length > 0 ? refreshWorkOSSkills({ agents: skillAgents }) : Promise.resolve(null), | ||
| Promise.all(mcpTargets.map((target) => target.add())), | ||
| ]); | ||
| const skillAgentNames = skillResult?.agents.map((a) => a.displayName) ?? []; | ||
|
|
||
| recordSetupCompleted(); | ||
|
|
||
| const mcpInstalled = mcpResults | ||
| .filter((r) => r.outcome === 'installed' || r.outcome === 'already-installed') | ||
| .map((r) => r.agent); | ||
| const mcpFailed = mcpResults.filter((r) => r.outcome === 'failed').map((r) => r.agent); | ||
|
|
||
| emitSetupEvent(opts.trigger, startedAt, true, { | ||
| skills: skillResult?.agents.map((a) => a.name) ?? [], | ||
| mcpInstalled, | ||
| mcpFailed, | ||
| }); | ||
|
|
||
| reportResults(skillResult ? { agents: skillAgentNames, count: skillResult.skills.length } : null, mcpResults); |
There was a problem hiding this comment.
🔍 recordSetupCompleted() fires even when every install fails
installAndReport calls recordSetupCompleted() (src/commands/setup.ts:182) unconditionally after the install attempt, before checking outcomes. If the user consented but every MCP add() returned failed (and/or refreshWorkOSSkills returned null), completion is still persisted. Because isSetupCompleted() suppresses all future automatic offers (src/commands/setup.ts:113), a user whose setup failed transiently during a post-login/post-install offer will never be auto-offered again; they'd have to run workos setup manually. The old standalone MCP offer only recorded a decline on an explicit "no", never marking completion on failure. This is a behavioral trade-off (treat consent as completion) rather than a clear defect, but may not be the intended UX for failed installs.
Was this helpful? React with 👍 or 👎 to provide feedback.
Two problems, one change: 1. Consent — a customer called the CLI's skill auto-install "prompt injection malware": `workos auth login` / `workos install` silently and unconditionally installed all bundled skills to every detected coding agent. 2. Aesthetic — clack's gutter/box styling buried the important asks and the post-login sequence felt spammy. What changed: - One consented setup moment: a new `workos setup` owns skills + MCP together. Automatic offers after login/install are gated (silent in agent/CI/non-TTY/ JSON, never re-ask after a decline/completion) and nothing is written until the user confirms (or passes --yes). Replaces the silent auto-install. - Flat, gutterless output: swapped the clack engine for @inquirer/prompts behind the single UI facade (src/utils/ui.ts). Hand-styled: branded intro, aligned key/value rows, nested sub-steps, one accent color. De-boxed notices (telemetry = flat line, unclaimed-env = WARN pill). - Security fix: installer tool calls route through canUseTool (allowedTools: []) so installerCanUseTool's Bash allowlist actually runs — bare tool names had been auto-approving Bash and bypassing the gate (also silenced the SDK CLAUDE_SDK_CAN_USE_TOOL_SHADOWED warning). BREAKING CHANGE: `workos auth login` and `workos install` no longer auto-install skills. Skills + MCP are installed only through the consented setup flow (`workos setup`, or the post-login/install offer on a human TTY). Agent / CI / non-TTY / JSON runs write nothing.
9fa0ca7 to
4c577c2
Compare
…etry Follow-up UX + reliability pass on the CLI overhaul (PR #200). Prompts - Serialize all prompts through a single-flight mutex in the ui facade and pause any active spinner around a prompt. Fixes the concurrent git-dirty + protected-branch prompts that opened on one stdin (the "asks a question but moves on" bug), and stops a spinner's redraw from clobbering a prompt. - Guard prompts against JSON / non-TTY contexts; route JSON output to the headless adapter and have the CLI adapter fail fast on an unanswerable prompt instead of hanging or silently crashing. - Abort queued sibling prompts when a run is cancelled, so a cancelled run can't leave a now-moot question open. - Remove the 30s deadline that auto-dismissed the "Set up now?" confirm. Output - Replace the block-letter banner with a compact lock brand mark, flatten the completion summary and the provisioned/unclaimed stderr notices, and make the unclaimed-environment warning bolder with a clear claim CTA. Errors + telemetry - Rewrite install errors for humans with recovery hints and word-boundary matching (no more "author" reading as "auth"). - Emit telemetry for login staging/refresh/auth failures, setup cancel/errors, and MCP install failure reasons. - Fix abortIfCancelled flushing a "cancelled" session on every prompt (bogus session end plus ~3s of dead-time per prompt).
Address Devin review findings on the consent-gated setup flow. - `workos setup --json` on a TTY no longer prompts. Output mode is JSON while interaction mode stays human, so isPromptAllowed() is true and the command path fell through to ui.confirm/ui.note, corrupting the machine-readable stdout stream. Require --yes under --json (mirrors the api command), consistent with the automatic path's isJsonMode() guard. - recordSetupCompleted() now fires only when something actually installed. A run where every install failed (e.g. transient `claude mcp add` timeouts, no skills to fall back on) previously persisted completion, and isSetupCompleted() suppresses all future automatic offers -- so a transient failure permanently stranded the user. Leave the pref unset on total failure so the next login/install re-offers.
… lifecycle Quality cleanups from a /simplify pass on the CLI UX refactor: - Move the WorkOS brand palette to a single `palette` in cli-symbols.ts; ui.ts and summary-box.ts now share it instead of redefining the hexes. - Lazy-load the @inquirer/prompts barrel inside the four prompt fns so its ten widgets stay off every non-interactive path (--json, --version, resource commands) — most invocations. import() is module-cached. - Route stray blank-line writes through a guarded blank() sink and drop the redundant per-function dashboard-mode guards in intro/outro/heading/note. - Extract withPromptActive() in the CLI adapter, collapsing the copy-pasted isPromptActive/flushPendingLogs dance across six prompt handlers into one exception-safe try/finally. - Remove dead log.message and the never-passed note() title param.
1 participant
Why
Two problems, one branch:
workos auth loginandworkos installsilently and unconditionally installed all bundled skills to every detected coding agent, with no consent and no opt-out.What changed
workos setupcommand owns skills + MCP together. Automatic offers afterlogin/installare gated: silent in agent/CI/non-TTY/JSON, never re-ask after a decline or completion, and nothing is written to a coding agent until the user confirms (or passes--yes). This replaces the silent auto-install.@inquirer/promptsbehind the existing single UI facade (src/utils/ui.ts). Output is hand-styled: branded intro (WorkOS · AuthKit installer), aligned key/value rows, nested sub-steps, minimal glyphs, one accent color.ℹline; the unclaimed-environment warning uses aWARNpill (newrenderStderrNotice).allowedTools: []so installer tool calls route throughcanUseTool— this re-arms the Bash safety gate that bare tool names were bypassing, and silences the SDK'sCLAUDE_SDK_CAN_USE_TOOL_SHADOWEDwarning.BREAKING CHANGE
workos auth loginandworkos installno longer auto-install skills. Skills + MCP are installed only through the consented setup flow (workos setup, or the post-login/install offer on a human TTY). Agent / CI / non-TTY / JSON runs write nothing.Test plan
pnpm buildpnpm typecheckpnpm lintpnpm test(2175 tests pass)Screenshots