feat(cli): --auto-approve / --yes / --force capability matrix + write-gate#254
Merged
feat(cli): --auto-approve / --yes / --force capability matrix + write-gate#254
Conversation
Contributor
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Results will be posted here when complete. |
7 tasks
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
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: Write tools ignore
allowDestructivedespite documented contract- Added an
allowDestructivecheck inevaluateWriteGatefor write tools by accepting an optionalcontextparameter withtargetFileExists, so when the upstream PreToolUse hook provides filesystem context, write-tool clobber attempts are denied whenallowDestructiveis false.
- Added an
Or push these changes by commenting:
@cursor push f866e2ee43
Preview (f866e2ee43)
diff --git a/src/lib/__tests__/mode-config.test.ts b/src/lib/__tests__/mode-config.test.ts
--- a/src/lib/__tests__/mode-config.test.ts
+++ b/src/lib/__tests__/mode-config.test.ts
@@ -205,12 +205,42 @@
}
});
- it('allows Edit / Write when allowWrites is true', () => {
+ it('allows Edit / Write when allowWrites is true and no existing file context', () => {
for (const tool of ['Edit', 'Write', 'MultiEdit']) {
expect(evaluateWriteGate(tool, {}, writesOnly).kind).toBe('allow');
}
});
+ it('denies write tools when target file exists and allowDestructive is false', () => {
+ for (const tool of ['Edit', 'Write', 'MultiEdit', 'NotebookEdit']) {
+ const decision = evaluateWriteGate(tool, {}, writesOnly, {
+ targetFileExists: true,
+ });
+ expect(decision.kind).toBe('deny');
+ if (decision.kind === 'deny') {
+ expect(decision.resumeFlag).toBe('--force');
+ expect(decision.reason).toMatch(tool);
+ }
+ }
+ });
+
+ it('allows write tools when target file exists and allowDestructive is true', () => {
+ for (const tool of ['Edit', 'Write', 'MultiEdit', 'NotebookEdit']) {
+ expect(
+ evaluateWriteGate(tool, {}, allow, { targetFileExists: true }).kind,
+ ).toBe('allow');
+ }
+ });
+
+ it('allows write tools when targetFileExists is false regardless of allowDestructive', () => {
+ for (const tool of ['Edit', 'Write', 'MultiEdit', 'NotebookEdit']) {
+ expect(
+ evaluateWriteGate(tool, {}, writesOnly, { targetFileExists: false })
+ .kind,
+ ).toBe('allow');
+ }
+ });
+
it('denies destructive Bash patterns when allowDestructive is false', () => {
const dangerous = [
'rm -rf node_modules',
diff --git a/src/lib/mode-config.ts b/src/lib/mode-config.ts
--- a/src/lib/mode-config.ts
+++ b/src/lib/mode-config.ts
@@ -157,13 +157,19 @@
* capability grants. Pure function — no I/O, easy to unit test.
*
* - Write tools (Edit/Write/MultiEdit/NotebookEdit) require `allowWrites`.
+ * - Write tools targeting an existing file also require `allowDestructive`.
* - Bash commands matching destructive patterns require `allowDestructive`.
* - Everything else is allowed.
+ *
+ * The caller (PreToolUse hook) is responsible for checking the filesystem
+ * and passing `context.targetFileExists` so this function can enforce the
+ * `allowDestructive` contract without performing I/O itself.
*/
export function evaluateWriteGate(
toolName: string,
toolInput: unknown,
caps: CapabilityFlags,
+ context?: { targetFileExists?: boolean },
): WriteGateDecision {
if (WRITE_TOOLS.has(toolName)) {
if (!caps.allowWrites) {
@@ -173,12 +179,13 @@
resumeFlag: '--yes',
};
}
- // Best-effort destructive detection for write tools: if the input has a
- // `path` or `file_path` that refers to an existing file, that's a
- // potential overwrite. The hook doesn't have filesystem access from
- // here, so we lean conservative — block writes that look like full
- // file replacements when --force is not set. The PreToolUse hook
- // upstream can do an fs.statSync check before calling this.
+ if (!caps.allowDestructive && context?.targetFileExists) {
+ return {
+ kind: 'deny',
+ reason: `Tool "${toolName}" would overwrite an existing file and --force was not provided.`,
+ resumeFlag: '--force',
+ };
+ }
return { kind: 'allow' };
}You can send follow-ups to the cloud agent here.
7 tasks
Collaborator
Author
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed:
--forcedoesn't imply--yesin command handler- Added
|| options.forceto the CI-mode branch condition at line 981 and!options.forceto the non-interactive auto-detection guard at lines 940–941 so--forcecorrectly routes to CI mode on a TTY.
- Added
- ✅ Fixed:
requireExplicitWritessilently suppressesautoApprovefrom--agent- Updated the JSDoc for
requireExplicitWritesto explicitly document that bothautoApproveandallowWritesare suppressed when the option is true, preventing developer confusion.
- Updated the JSDoc for
Or push these changes by commenting:
@cursor push 271ceafd24
Preview (271ceafd24)
diff --git a/bin.ts b/bin.ts
--- a/bin.ts
+++ b/bin.ts
@@ -939,6 +939,7 @@
process.env.AMPLITUDE_WIZARD_AGENT === '1' ||
(!options.ci &&
!options.yes &&
+ !options.force &&
!options.classic &&
process.env.AMPLITUDE_WIZARD_CLASSIC !== '1' &&
isNonInteractiveEnvironment())
@@ -978,7 +979,7 @@
() => session.additionalFeatureQueue,
);
})();
- } else if (options.ci || options.yes) {
+ } else if (options.ci || options.yes || options.force) {
// CI mode: no prompts, auto-select first environment
setUI(new LoggingUI());
if (!options.installDir) options.installDir = process.cwd();
diff --git a/src/lib/mode-config.ts b/src/lib/mode-config.ts
--- a/src/lib/mode-config.ts
+++ b/src/lib/mode-config.ts
@@ -49,10 +49,12 @@
human?: boolean;
isTTY: boolean;
/**
- * When true, write capability must be granted explicitly via `--yes` /
- * `--ci` / `--force`. Used by the `apply` subcommand and any other
- * command that wants strict opt-in. Default `false` preserves today's
- * `--agent` behavior (auto-approve + writes implied).
+ * When true, both `autoApprove` and `allowWrites` must be granted
+ * explicitly via `--auto-approve` / `--yes` / `--ci` / `--force`.
+ * The bare `--agent` flag will NOT imply either capability. Used by
+ * the `apply` subcommand and any other command that wants strict
+ * opt-in. Default `false` preserves today's `--agent` behavior
+ * (auto-approve + writes implied).
*/
requireExplicitWrites?: boolean;
}You can send follow-ups to the cloud agent here.
Collaborator
Author
Contributor
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: Regex matches safe
--force-with-leaseas destructive- Added negative lookahead
(?!-)to the--forceregex pattern so--force-with-leaseis no longer matched as destructive.
- Added negative lookahead
Or push these changes by commenting:
@cursor push dd4d2f6e18
Preview (dd4d2f6e18)
diff --git a/src/lib/mode-config.ts b/src/lib/mode-config.ts
--- a/src/lib/mode-config.ts
+++ b/src/lib/mode-config.ts
@@ -145,7 +145,7 @@
/\bgit\s+checkout\s+--\s/i,
/\bgit\s+clean\s+-/i,
/\bgit\s+restore\s+\./i,
- /\bgit\s+push\s+.*--force\b/i,
+ /\bgit\s+push\s+.*--force(?!-)\b/i,
/\bdrop\s+(table|database)\b/i,
/\btruncate\s+table\b/i,
];You can send follow-ups to the cloud agent here.
…-gate Splits the implicit "agent-mode auto-approves everything" behavior into three explicit, composable capabilities so plan/apply/verify can layer on without ambiguity: --auto-approve → silently pick `recommended` on `needs_input` (no writes) --yes (-y) → autoApprove + allowWrites (today's --yes / --ci semantics) --force → autoApprove + allowWrites + allowDestructive Back-compat preserved: `--agent` alone still implies `autoApprove + allowWrites`. The upcoming `apply` command will pass `requireExplicitWrites: true` to `resolveMode`, which forces writes to be requested by name. - `ModeConfig` extends new `CapabilityFlags` interface - `resolveMode` builds capabilities additively from the flag set - `evaluateWriteGate(toolName, toolInput, caps)` is a pure function the PreToolUse hook can call to decide allow/deny — gates Edit/Write/ MultiEdit/NotebookEdit on `allowWrites`, and gates a curated set of destructive Bash patterns (rm -rf, git reset --hard, git push --force, DROP TABLE, etc.) on `allowDestructive` - New `--auto-approve` and `--force` global flags in bin.ts - `--yes` consolidated to a single global declaration with `-y` alias - `ExitCode.WRITE_REFUSED = 13` for clean re-invocation by outer agents Tests: +16 in mode-config.test.ts (35 total). Suite green (1257 pass). Part of the wizard sub-agent design — Gap 4 of 4. Stacked on #253. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Applied via @cursor push command
…icitWrites JSDoc Applied via @cursor push command
kelsonpw
force-pushed
the
kelsonpw/agent-flag-matrix
branch
from
296868e to
cf68cb0
Compare
…licitWrites docs Two Bugbot follow-ups on the capability matrix: 1. The regex `\bgit\s+push\s+.*--force\b/i` matched `git push --force-with-lease` because `\b` recognizes a word boundary between `e` (word char) and `-` (non-word). That flagged the explicitly safer push variant as destructive and would have pushed users toward the broader CLI `--force` flag as a workaround. Fix: use a negative lookahead `--force(?!-)` so `--force-with-lease` passes through. Pinned with a regression test. 2. The `requireExplicitWrites` JSDoc only mentioned writes, but the flag also gates `autoApprove`. A future subcommand using this flag needs to know that `--auto-approve` (or higher) is required even for prompt selection — not just for filesystem writes. Updated the doc to call this out explicitly. The two MEDIUM-severity Bugbot findings on this PR (allowDestructive contract, --force in command handler) are false positives against the current code: the JSDoc on evaluateWriteGate documents the contextual contract for write tools (lines 161-162), enforced when callers pass `targetFileExists`; and `--force` IS included in both the auto-detect guard (line 960) and the CI branch (line 1000) of bin.ts via the follow-up commit on this branch. Co-Authored-By: Cursor Bugbot <bugbot@cursor.com>
Contributor
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: Agent back-compat grants writes despite
--auto-approve"no writes"- Added a check for explicit capability flags (autoApprove/yes/ci/force) so the back-compat block in resolveMode() only fires when --agent is used alone with no other flags, and added --auto-approve to bin.ts's non-interactive exclusion list to prevent auto-detecting agent mode when the user explicitly chose --auto-approve.
Or push these changes by commenting:
@cursor push 785347b4e0
Preview (785347b4e0)
diff --git a/bin.ts b/bin.ts
--- a/bin.ts
+++ b/bin.ts
@@ -958,6 +958,7 @@
(!options.ci &&
!options.yes &&
!options.force &&
+ !options.autoApprove &&
!options.classic &&
process.env.AMPLITUDE_WIZARD_CLASSIC !== '1' &&
isNonInteractiveEnvironment())
diff --git a/src/lib/__tests__/mode-config.test.ts b/src/lib/__tests__/mode-config.test.ts
--- a/src/lib/__tests__/mode-config.test.ts
+++ b/src/lib/__tests__/mode-config.test.ts
@@ -130,6 +130,20 @@
expect(r.allowDestructive).toBe(false);
});
+ it('--agent + --auto-approve does NOT escalate to allowWrites', () => {
+ const r = resolveMode({ agent: true, autoApprove: true, isTTY: true });
+ expect(r.autoApprove).toBe(true);
+ expect(r.allowWrites).toBe(false);
+ expect(r.allowDestructive).toBe(false);
+ });
+
+ it('--agent + --yes still grants autoApprove + allowWrites', () => {
+ const r = resolveMode({ agent: true, yes: true, isTTY: true });
+ expect(r.autoApprove).toBe(true);
+ expect(r.allowWrites).toBe(true);
+ expect(r.allowDestructive).toBe(false);
+ });
+
it('--agent + requireExplicitWrites does NOT grant writes', () => {
const r = resolveMode({
agent: true,
diff --git a/src/lib/mode-config.ts b/src/lib/mode-config.ts
--- a/src/lib/mode-config.ts
+++ b/src/lib/mode-config.ts
@@ -106,8 +106,13 @@
}
// Back-compat: today's `--agent` (no other flags) implies auto-approve
// and writes. New scoped commands (`apply`, `verify`) opt out via
- // `requireExplicitWrites: true`.
- if (isAgent && !requireExplicitWrites) {
+ // `requireExplicitWrites: true`. Skip the implicit grant when any
+ // explicit capability flag was provided — the caller chose their own
+ // permission level and we must not silently escalate it.
+ const hasExplicitCapFlags = Boolean(
+ opts.autoApprove || opts.yes || opts.ci || opts.force,
+ );
+ if (isAgent && !requireExplicitWrites && !hasExplicitCapFlags) {
autoApprove = true;
allowWrites = true;
} else if (isAgent) {You can send follow-ups to the cloud agent here.
…mpat
Bugbot caught: the back-compat path on isAgent && !requireExplicitWrites
fired whenever --agent was set, even when an explicit capability flag
(--auto-approve) was also passed. So `--auto-approve --agent` silently
upgraded to allowWrites=true, contradicting --auto-approve's documented
"no writes" contract.
This is compounded by bin.ts's non-interactive auto-detect: passing
just --auto-approve in a piped run set options.agent=true, then
resolveMode's back-compat path granted writes the user never asked for.
Two fixes:
1. resolveMode: only apply --agent back-compat when NO explicit
capability flag is set (autoApprove / yes / ci / force).
2. bin.ts: exclude --auto-approve from the non-interactive
auto-detect, so the user's "no writes" intent isn't silently
promoted to "writes" via auto-detected --agent.
Pinned with a regression test: `{agent: true, autoApprove: true,
isTTY: false}` must give autoApprove=true, allowWrites=false.
Co-Authored-By: Cursor Bugbot <bugbot@cursor.com>
Contributor
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: Overly broad
rmregex matches package manager commands- Replaced
/\brm\s+-/iwith/(?<!\w)rm\s+(-[^\s]*[rfRF]|--recursive|--force)\b/to only match standalonermwith dangerous-r/-fflags, excluding package manager subcommands likenpm rm -D.
- Replaced
Or push these changes by commenting:
@cursor push 3be7cb2968
Preview (3be7cb2968)
diff --git a/src/lib/__tests__/mode-config.test.ts b/src/lib/__tests__/mode-config.test.ts
--- a/src/lib/__tests__/mode-config.test.ts
+++ b/src/lib/__tests__/mode-config.test.ts
@@ -283,6 +283,18 @@
).toBe('allow');
});
+ it('does NOT flag package manager `rm` subcommands as destructive', () => {
+ const safe = [
+ 'npm rm -D express',
+ 'pnpm rm -D some-package',
+ 'npm rm -S lodash',
+ ];
+ for (const cmd of safe) {
+ const decision = evaluateWriteGate('Bash', { command: cmd }, writesOnly);
+ expect(decision.kind).toBe('allow');
+ }
+ });
+
it('does NOT flag `git push --force-with-lease` as destructive', () => {
// Regression: the regex used `--force\b`, and the word boundary between
// `e` (word char) and `-` (non-word) matched mid-token, so the safer
diff --git a/src/lib/mode-config.ts b/src/lib/mode-config.ts
--- a/src/lib/mode-config.ts
+++ b/src/lib/mode-config.ts
@@ -150,7 +150,7 @@
const WRITE_TOOLS = new Set(['Edit', 'Write', 'MultiEdit', 'NotebookEdit']);
const DESTRUCTIVE_BASH_PATTERNS: RegExp[] = [
- /\brm\s+-/i, // rm -rf, rm -r, rm -f
+ /(?<!\w)rm\s+(-[^\s]*[rfRF]|--recursive|--force)\b/, // rm -rf, rm -r, rm -f (not npm rm -D)
/\bgit\s+reset\s+--hard\b/i,
/\bgit\s+checkout\s+--\s/i,
/\bgit\s+clean\s+-/i,You can send follow-ups to the cloud agent here.
…tructive contract Two more Bugbot follow-ups: 1. The regex /\brm\s+-/i matched ANY rm after a word boundary, so package-manager uninstalls like `pnpm rm -D foo` were treated as destructive filesystem operations and required --force to proceed. Routine dependency management would have demanded an unrelated capability flag. Fix: anchor to start-of-string or chain operators so standalone rm is caught but `pnpm rm` / `npm rm` / `yarn rm` / `bun rm` pass through. Pinned with regression tests for both directions (allow pnpm rm, still deny `rm -rf node_modules`). 2. The CapabilityFlags JSDoc for allowDestructive promised "write tools may still create new files but can't clobber" as an unconditional contract, but evaluateWriteGate only enforces this when the caller passes targetFileExists. Clarified the JSDoc to match reality: the existence check is delegated to the PreToolUse hook, not done by the gate function. Co-Authored-By: Cursor Bugbot <bugbot@cursor.com>
| // filesystem ops. Anchor to start-of-string or chain operators so | ||
| // standalone `rm` is caught but `pnpm rm` / `npm rm` / `yarn rm` / | ||
| // `bun rm` aren't. | ||
| /(?:^|[;&|]\s*)rm\s+-/i, |
Contributor
There was a problem hiding this comment.
Destructive rm regex bypassed by multi-line Bash commands
Medium Severity
The rm destructive-pattern regex /(?:^|[;&|]\s*)rm\s+-/i doesn't match rm on subsequent lines of a multi-line Bash command. Without the m flag, ^ only matches start-of-string, and \n isn't in the [;&|] character class. A command like "echo hello\nrm -rf /" bypasses the gate entirely. The other DESTRUCTIVE_BASH_PATTERNS use \b and don't have this problem — only the rm pattern is affected. The codebase already shows multi-line Bash commands in tests (e.g. 'pnpm install\nsleep 60' in agent-interface.test.ts), confirming agents send them.
Reviewed by Cursor Bugbot for commit c297e7b. Configure here.
| // user who passes only --auto-approve in a non-TTY env should | ||
| // NOT be auto-promoted to agent mode (which would otherwise | ||
| // route through resolveMode's --agent back-compat path). | ||
| !options['auto-approve'] && |
Contributor
There was a problem hiding this comment.
--auto-approve alone in non-TTY falls to TUI
Low Severity
When --auto-approve is the only flag in a non-TTY environment, the new !options['auto-approve'] guard correctly prevents agent-mode auto-detection (to avoid granting implicit writes). However, --auto-approve is also absent from the CI-mode check on line 1005 (options.ci || options.yes || options.force), so the routing falls through to the interactive TUI code path — which writes raw ANSI escape sequences and launches Ink in a pipe. The resolveMode function correctly resolves --auto-approve alone to mode: 'ci', but bin.ts routing doesn't match that.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c297e7b. Configure here.
Belt-and-suspenders for the persistent Bugbot finding about "--force doesn't imply --yes". The bin.ts CI-branch check at line 1004 already includes options.force; this test pins the resolveMode side so a future refactor of bin.ts can't silently un-pair the implication.
kelsonpw
added a commit
that referenced
this pull request
Apr 26, 2026
Splits the wizard's run-everything-at-once flow into three explicit
phases so outer agents (Claude Code, Cursor, Codex) can inspect a plan
before any writes happen:
npx wizard plan
→ emits a `plan` NDJSON event with planId + framework + sdk +
resumeFlags. No writes. Persists the plan to
$TMPDIR/amplitude-wizard-plans/<planId>.json with a 24h TTL.
npx wizard apply --plan-id <id> --yes
→ loads + validates the plan, then runs the wizard. Refuses without
--yes (exits WRITE_REFUSED=13 with a clear resume hint). Refuses
stale or unknown plan IDs (exits INVALID_ARGS=2).
npx wizard verify
→ cheap, no-network check that SDK is installed + API key is
configured + framework is detectable. Emits a structured
`verification_result` event.
Implementation:
- `src/lib/agent-plans.ts` — typed plan persistence layer.
Zod-validated WizardPlan schema (v1), atomic JSON writes (0o600
perms), TTL-based expiry, `pruneStalePlans` for cleanup.
- `src/lib/agent-ops.ts` — adds `runPlan` and `runVerify` business
logic alongside the existing `runDetect` / `runStatus`. No UI, no
process.exit; thin yargs handlers compose them.
- `bin.ts` — three new yargs `.command()` entries that all pass
`requireExplicitWrites: true` to `resolveMode` so the new commands
opt out of the agent-implies-writes back-compat.
Smoke tests (manual, in this directory):
$ wizard plan --json → emits plan envelope, exit 0
$ wizard apply --plan-id X --json → exits 13, no writes
$ wizard apply --plan-id X --yes --json → executes
$ wizard verify --json → emits verification_result, exit reflects pass/fail
Tests: +10 in agent-plans.test.ts (1267 total). Suite green.
Part of the wizard sub-agent design — Gap 3 of 4. Stacked on #254.
Note: the `apply` handler currently spawns the existing `--agent --yes`
wizard run with the planId in the env. Wiring the plan into the agent
prompt (so the inner agent reads `WizardPlan` and reports back) is a
follow-up that lands cleanly when #180 (three-phase handoff schemas)
ships.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Reviewed by Cursor Bugbot for commit 0b69de6. Configure here.
| // filesystem ops. Anchor to start-of-string or chain operators so | ||
| // standalone `rm` is caught but `pnpm rm` / `npm rm` / `yarn rm` / | ||
| // `bun rm` aren't. | ||
| /(?:^|[;&|]\s*)rm\s+-/i, |
Contributor
There was a problem hiding this comment.
Destructive rm regex misses sudo rm -rf commands
Medium Severity
The rm regex /(?:^|[;&|]\s*)rm\s+-/i anchors to start-of-string or shell chain operators, which correctly avoids false positives on pnpm rm but also stops detecting sudo rm -rf, env rm -rf, and similar prefixed destructive commands. The previous \brm\s+- word-boundary approach caught these. Common patterns like sudo rm -rf / now bypass the destructive gate entirely.
Reviewed by Cursor Bugbot for commit 0b69de6. Configure here.
4 tasks
kelsonpw
added a commit
that referenced
this pull request
Apr 26, 2026
) * feat(cli): plan / apply / verify subcommands with plan persistence Splits the wizard's run-everything-at-once flow into three explicit phases so outer agents (Claude Code, Cursor, Codex) can inspect a plan before any writes happen: npx wizard plan → emits a `plan` NDJSON event with planId + framework + sdk + resumeFlags. No writes. Persists the plan to $TMPDIR/amplitude-wizard-plans/<planId>.json with a 24h TTL. npx wizard apply --plan-id <id> --yes → loads + validates the plan, then runs the wizard. Refuses without --yes (exits WRITE_REFUSED=13 with a clear resume hint). Refuses stale or unknown plan IDs (exits INVALID_ARGS=2). npx wizard verify → cheap, no-network check that SDK is installed + API key is configured + framework is detectable. Emits a structured `verification_result` event. Implementation: - `src/lib/agent-plans.ts` — typed plan persistence layer. Zod-validated WizardPlan schema (v1), atomic JSON writes (0o600 perms), TTL-based expiry, `pruneStalePlans` for cleanup. - `src/lib/agent-ops.ts` — adds `runPlan` and `runVerify` business logic alongside the existing `runDetect` / `runStatus`. No UI, no process.exit; thin yargs handlers compose them. - `bin.ts` — three new yargs `.command()` entries that all pass `requireExplicitWrites: true` to `resolveMode` so the new commands opt out of the agent-implies-writes back-compat. Smoke tests (manual, in this directory): $ wizard plan --json → emits plan envelope, exit 0 $ wizard apply --plan-id X --json → exits 13, no writes $ wizard apply --plan-id X --yes --json → executes $ wizard verify --json → emits verification_result, exit reflects pass/fail Tests: +10 in agent-plans.test.ts (1267 total). Suite green. Part of the wizard sub-agent design — Gap 3 of 4. Stacked on #254. Note: the `apply` handler currently spawns the existing `--agent --yes` wizard run with the planId in the env. Wiring the plan into the agent prompt (so the inner agent reads `WizardPlan` and reports back) is a follow-up that lands cleanly when #180 (three-phase handoff schemas) ships. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: include --install-dir in plan resumeFlags and remove dead frameworkHintsFromDetect code Applied via @cursor push command * fix: apply honors plan's stored installDir when --install-dir is not passed Bugbot caught: apply read installDir from argv['install-dir'] ?? cwd but ignored result.plan.installDir, even though the plan persists it. Combined with plan's resumeFlags conditionally omitting --install-dir when installDir === cwd at plan time, an outer agent whose cwd shifted between plan and apply would silently run wizard against the wrong dir. Fix: argv (explicit user override) → plan.installDir (correctness when argv missing) → cwd (last resort). Honoring the plan's stored directory is what makes plan → apply work across cwd shifts. Co-Authored-By: Cursor Bugbot <bugbot@cursor.com> * fix: runPlan resolves installDir to absolute before persisting Bugbot caught: a relative installDir (e.g. `.` or `./my-project`) was persisted verbatim. apply later resolves the persisted path against *apply-time* cwd, not plan-time cwd — so a cwd shift between plan and apply silently runs wizard in the wrong directory, defeating the plan→apply contract this PR is supposed to enable. Fix: path.resolve() the input at the top of runPlan so the persisted plan always carries an absolute path. Also use the resolved path for runDetect so detection and persistence agree on the same target. Co-Authored-By: Cursor Bugbot <bugbot@cursor.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Cursor Bugbot <bugbot@cursor.com>
This was referenced Apr 26, 2026
kelsonpw
added a commit
that referenced
this pull request
Apr 26, 2026
Replaces closed PR #246, redone off current main. Pure mechanical refactor. bin.ts has been the conflict magnet of the repo — every CLI PR edits the same 2700+ line file. Per-command files isolate ownership: a new flag, a new mcp subcommand, a new whoami enhancement — none of those should require touching the same file as everything else. Every flag, env-var passthrough, help text, exit code, strict-mode rejection, and .check() validator is byte-identical post-split. bin.ts: 2768 → 473 lines. Structure: src/commands/ context.ts CLI_INVOCATION, WIZARD_VERSION, IS_WIZARD_DEV helpers.ts buildSessionFromOptions, resolveNonInteractiveCredentials, runDirectSignupIfRequested, lazyRunWizard index.ts barrel export default.ts $0 — interactive/CI/agent/classic dispatch (849 lines) login.ts OAuth login flow logout.ts Clear stored credentials whoami.ts, feedback.ts, slack.ts, region.ts, detect.ts, status.ts plan.ts, apply.ts, verify.ts (sub-agent commands from #269) auth.ts Subcommand dispatcher: status / token mcp.ts Subcommand dispatcher: add / remove / serve manifest.ts Print agent-mode manifest Critical preserve-list (independently verified): In default.ts: - 4-way handler dispatch order: agent → ci|yes|force → classic → TUI - --auto-approve does NOT promote to agent mode - --env deprecation warning paths - Fire-and-forget fetchAmplitudeUser org/workspace/appId hydration - Crash-recovery checkpoint with three onEnterScreen save/clear hooks - Direct-signup → fall-through-to-OAuth with signupTokensObtained fallback - Pre-detection branch (detectAmplitudeInProject + setAmplitudePreDetected + waitForPreDetectedChoice + resetForAgentAfterPreDetected) - SIGINT handler installed immediately after startTUI() In apply.ts: - spawn(process.execPath, [process.argv[1] ?? '', '--agent', '--yes', ...]) - AMPLITUDE_WIZARD_PLAN_ID env passthrough - 4-way switch on resolvePlan result.kind - mode.allowWrites gate exits with WRITE_REFUSED (13) In bin.ts: - sanitizeNestedClaudeEnv() runs first - NODE_ENV path-based detection from #249 - Node >=20 version gate - All 21 flags: --auto-approve and --force from #254 preserved - .check() for --app-id numeric on root yargs - .env('AMPLITUDE_WIZARD') for env-var passthrough shadows Verification: - 20/20 --help outputs byte-identical - 4/4 strict-mode rejections byte-identical - 2/2 env-var passthroughs byte-identical - pnpm tsc --noEmit: clean - pnpm lint: clean (1 pre-existing warning, unrelated) - pnpm test: 1497 passed, 17 skipped, 0 failed - pnpm build smoke: passes src/lib/__tests__/zone-resolution.invariants.test.ts holds a hardcoded file-path allowlist for session.region reads. The two new paths (default.ts, helpers.ts) inherit the read from bin.ts. Structural accommodation; no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 26, 2026
kelsonpw
added a commit
that referenced
this pull request
Apr 27, 2026
Replaces closed PR #246, redone off current main. Pure mechanical refactor. bin.ts has been the conflict magnet of the repo — every CLI PR edits the same 2700+ line file. Per-command files isolate ownership: a new flag, a new mcp subcommand, a new whoami enhancement — none of those should require touching the same file as everything else. Every flag, env-var passthrough, help text, exit code, strict-mode rejection, and .check() validator is byte-identical post-split. bin.ts: 2768 → 473 lines. Structure: src/commands/ context.ts CLI_INVOCATION, WIZARD_VERSION, IS_WIZARD_DEV helpers.ts buildSessionFromOptions, resolveNonInteractiveCredentials, runDirectSignupIfRequested, lazyRunWizard index.ts barrel export default.ts $0 — interactive/CI/agent/classic dispatch (849 lines) login.ts OAuth login flow logout.ts Clear stored credentials whoami.ts, feedback.ts, slack.ts, region.ts, detect.ts, status.ts plan.ts, apply.ts, verify.ts (sub-agent commands from #269) auth.ts Subcommand dispatcher: status / token mcp.ts Subcommand dispatcher: add / remove / serve manifest.ts Print agent-mode manifest Critical preserve-list (independently verified): In default.ts: - 4-way handler dispatch order: agent → ci|yes|force → classic → TUI - --auto-approve does NOT promote to agent mode - --env deprecation warning paths - Fire-and-forget fetchAmplitudeUser org/workspace/appId hydration - Crash-recovery checkpoint with three onEnterScreen save/clear hooks - Direct-signup → fall-through-to-OAuth with signupTokensObtained fallback - Pre-detection branch (detectAmplitudeInProject + setAmplitudePreDetected + waitForPreDetectedChoice + resetForAgentAfterPreDetected) - SIGINT handler installed immediately after startTUI() In apply.ts: - spawn(process.execPath, [process.argv[1] ?? '', '--agent', '--yes', ...]) - AMPLITUDE_WIZARD_PLAN_ID env passthrough - 4-way switch on resolvePlan result.kind - mode.allowWrites gate exits with WRITE_REFUSED (13) In bin.ts: - sanitizeNestedClaudeEnv() runs first - NODE_ENV path-based detection from #249 - Node >=20 version gate - All 21 flags: --auto-approve and --force from #254 preserved - .check() for --app-id numeric on root yargs - .env('AMPLITUDE_WIZARD') for env-var passthrough shadows Verification: - 20/20 --help outputs byte-identical - 4/4 strict-mode rejections byte-identical - 2/2 env-var passthroughs byte-identical - pnpm tsc --noEmit: clean - pnpm lint: clean (1 pre-existing warning, unrelated) - pnpm test: 1497 passed, 17 skipped, 0 failed - pnpm build smoke: passes src/lib/__tests__/zone-resolution.invariants.test.ts holds a hardcoded file-path allowlist for session.region reads. The two new paths (default.ts, helpers.ts) inherit the read from bin.ts. Structural accommodation; no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3 tasks
kelsonpw
added a commit
that referenced
this pull request
Apr 29, 2026
…ify + capability-matrix landings (#356) The hand-maintained agent-manifest.ts had drifted from the actual CLI surface that bin.ts implements. Agents calling `amplitude-wizard manifest` to discover the available verbs would not see plan/apply/ verify, the capability-matrix flags, or AMPLITUDE_WIZARD_MAX_TURNS. - Add plan, apply, verify to commands (with --plan-id on apply). - Add --yes, --auto-approve, --force to globalFlags. Reword --ci so it no longer mis-claims --yes as an alias — they're now distinct capability-gate flags per the matrix from #254. - Add AMPLITUDE_WIZARD_MAX_TURNS to env (#291). - Extend agent-manifest.test.ts with three new assertions covering the added entries so future drift fails fast. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kelsonpw
added a commit
that referenced
this pull request
Apr 29, 2026
Replaces closed PR #246, redone off current main. Pure mechanical refactor. bin.ts has been the conflict magnet of the repo — every CLI PR edits the same 2700+ line file. Per-command files isolate ownership: a new flag, a new mcp subcommand, a new whoami enhancement — none of those should require touching the same file as everything else. Every flag, env-var passthrough, help text, exit code, strict-mode rejection, and .check() validator is byte-identical post-split. bin.ts: 2768 → 473 lines. Structure: src/commands/ context.ts CLI_INVOCATION, WIZARD_VERSION, IS_WIZARD_DEV helpers.ts buildSessionFromOptions, resolveNonInteractiveCredentials, runDirectSignupIfRequested, lazyRunWizard index.ts barrel export default.ts $0 — interactive/CI/agent/classic dispatch (849 lines) login.ts OAuth login flow logout.ts Clear stored credentials whoami.ts, feedback.ts, slack.ts, region.ts, detect.ts, status.ts plan.ts, apply.ts, verify.ts (sub-agent commands from #269) auth.ts Subcommand dispatcher: status / token mcp.ts Subcommand dispatcher: add / remove / serve manifest.ts Print agent-mode manifest Critical preserve-list (independently verified): In default.ts: - 4-way handler dispatch order: agent → ci|yes|force → classic → TUI - --auto-approve does NOT promote to agent mode - --env deprecation warning paths - Fire-and-forget fetchAmplitudeUser org/workspace/appId hydration - Crash-recovery checkpoint with three onEnterScreen save/clear hooks - Direct-signup → fall-through-to-OAuth with signupTokensObtained fallback - Pre-detection branch (detectAmplitudeInProject + setAmplitudePreDetected + waitForPreDetectedChoice + resetForAgentAfterPreDetected) - SIGINT handler installed immediately after startTUI() In apply.ts: - spawn(process.execPath, [process.argv[1] ?? '', '--agent', '--yes', ...]) - AMPLITUDE_WIZARD_PLAN_ID env passthrough - 4-way switch on resolvePlan result.kind - mode.allowWrites gate exits with WRITE_REFUSED (13) In bin.ts: - sanitizeNestedClaudeEnv() runs first - NODE_ENV path-based detection from #249 - Node >=20 version gate - All 21 flags: --auto-approve and --force from #254 preserved - .check() for --app-id numeric on root yargs - .env('AMPLITUDE_WIZARD') for env-var passthrough shadows Verification: - 20/20 --help outputs byte-identical - 4/4 strict-mode rejections byte-identical - 2/2 env-var passthroughs byte-identical - pnpm tsc --noEmit: clean - pnpm lint: clean (1 pre-existing warning, unrelated) - pnpm test: 1497 passed, 17 skipped, 0 failed - pnpm build smoke: passes src/lib/__tests__/zone-resolution.invariants.test.ts holds a hardcoded file-path allowlist for session.region reads. The two new paths (default.ts, helpers.ts) inherit the read from bin.ts. Structural accommodation; no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
kelsonpw
added a commit
that referenced
this pull request
Apr 29, 2026
* refactor(cli): split bin.ts into per-command CommandModule files Replaces closed PR #246, redone off current main. Pure mechanical refactor. bin.ts has been the conflict magnet of the repo — every CLI PR edits the same 2700+ line file. Per-command files isolate ownership: a new flag, a new mcp subcommand, a new whoami enhancement — none of those should require touching the same file as everything else. Every flag, env-var passthrough, help text, exit code, strict-mode rejection, and .check() validator is byte-identical post-split. bin.ts: 2768 → 473 lines. Structure: src/commands/ context.ts CLI_INVOCATION, WIZARD_VERSION, IS_WIZARD_DEV helpers.ts buildSessionFromOptions, resolveNonInteractiveCredentials, runDirectSignupIfRequested, lazyRunWizard index.ts barrel export default.ts $0 — interactive/CI/agent/classic dispatch (849 lines) login.ts OAuth login flow logout.ts Clear stored credentials whoami.ts, feedback.ts, slack.ts, region.ts, detect.ts, status.ts plan.ts, apply.ts, verify.ts (sub-agent commands from #269) auth.ts Subcommand dispatcher: status / token mcp.ts Subcommand dispatcher: add / remove / serve manifest.ts Print agent-mode manifest Critical preserve-list (independently verified): In default.ts: - 4-way handler dispatch order: agent → ci|yes|force → classic → TUI - --auto-approve does NOT promote to agent mode - --env deprecation warning paths - Fire-and-forget fetchAmplitudeUser org/workspace/appId hydration - Crash-recovery checkpoint with three onEnterScreen save/clear hooks - Direct-signup → fall-through-to-OAuth with signupTokensObtained fallback - Pre-detection branch (detectAmplitudeInProject + setAmplitudePreDetected + waitForPreDetectedChoice + resetForAgentAfterPreDetected) - SIGINT handler installed immediately after startTUI() In apply.ts: - spawn(process.execPath, [process.argv[1] ?? '', '--agent', '--yes', ...]) - AMPLITUDE_WIZARD_PLAN_ID env passthrough - 4-way switch on resolvePlan result.kind - mode.allowWrites gate exits with WRITE_REFUSED (13) In bin.ts: - sanitizeNestedClaudeEnv() runs first - NODE_ENV path-based detection from #249 - Node >=20 version gate - All 21 flags: --auto-approve and --force from #254 preserved - .check() for --app-id numeric on root yargs - .env('AMPLITUDE_WIZARD') for env-var passthrough shadows Verification: - 20/20 --help outputs byte-identical - 4/4 strict-mode rejections byte-identical - 2/2 env-var passthroughs byte-identical - pnpm tsc --noEmit: clean - pnpm lint: clean (1 pre-existing warning, unrelated) - pnpm test: 1497 passed, 17 skipped, 0 failed - pnpm build smoke: passes src/lib/__tests__/zone-resolution.invariants.test.ts holds a hardcoded file-path allowlist for session.region reads. The two new paths (default.ts, helpers.ts) inherit the read from bin.ts. Structural accommodation; no behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(commands): import parseEventPlanContent from event-plan-parser Address Cursor Bugbot finding: the dynamic import in `default.ts` was sourcing `parseEventPlanContent` from `agent-interface.js`, which defeats the stated purpose of keeping the Claude Agent SDK / UI singleton out of the bin.ts load path. The lightweight `event-plan-parser.js` module was extracted specifically for CLI/ops callers — use it directly here. Behavior unchanged (`agent-interface.ts` still re-exports for back-compat). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
2 participants
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
Splits the implicit "agent-mode auto-approves everything" behavior into three explicit, composable capabilities so the upcoming
plan/apply/verifycommands can layer on without ambiguity:Back-compat preserved:
--agentalone still impliesautoApprove + allowWrites. The upcomingapplycommand will passrequireExplicitWrites: truetoresolveMode, which forces writes to be requested by name.This is Gap 4 of 4 in the wizard sub-agent contract. Stacked on #253.
What's in
ModeConfigextends newCapabilityFlags— three orthogonal booleans the rest of the system can branch on without re-deriving from raw flags every timeresolveMode(opts)builds capabilities additively from the flag setrequireExplicitWrites: trueopt-in disables the--agent-implies-writes back-compat for scoped commandsevaluateWriteGate(toolName, toolInput, caps)— pure function for the PreToolUse hookEdit / Write / MultiEdit / NotebookEditwhenallowWritesis falseBash(rm -rf, git reset --hard, git push --force, DROP TABLE, TRUNCATE, etc.) whenallowDestructiveis false{ kind: 'deny', reason, resumeFlag: '--yes' | '--force' }so the wizard can emit a clean error and the outer agent knows exactly which flag to add--auto-approveand--forceglobal flags inbin.ts--yesconsolidated to a single global declaration with-yalias (was duplicated at command level)ExitCode.WRITE_REFUSED = 13for clean re-invocation by outer agentsWire-up
The hook integration is deliberately not in this PR — it lives in Gap 3 (
plan/apply/verify), where the PreToolUse hook will callevaluateWriteGateand surface deny decisions as NDJSONerrorevents withresumeFlaghints.Test plan
pnpm test— 1257 passed, 17 skipped (16 new tests in mode-config.test.ts, 35 total)pnpm tsc --noEmitcleanpnpm lintcleannpx wizard --helpshows the new global flagsnpx wizard --ci(should still auto-approve + write — back-compat)npx wizard --auto-approve(no-op for default\$0command since it's TTY-interactive without --ci/--agent)Out of scope
evaluateWriteGateinto the PreToolUse hook — Gap 3 (next PR in stack)WRITE_REFUSEDexits — Gap 3plan/apply/verifysubcommands themselves — Gap 3cc @amplitude/growth
🤖 Generated with Claude Code
Note
Medium Risk
Changes CLI flag semantics and non-interactive mode selection, which can affect automation and when writes/destructive actions are permitted. Risk is mitigated by isolating the logic in
resolveMode/evaluateWriteGateand adding extensive unit tests.Overview
Adds a capability matrix for non-interactive runs by introducing global
--auto-approve,--yes/-y, and--forceflags and updatingbin.tsmode routing so--forcebehaves like CI and--auto-approvedoes not implicitly enable agent mode.Refactors
resolveModeto compute three explicit capabilities (autoApprove,allowWrites,allowDestructive), addsrequireExplicitWritesto disable--agentback-compat auto-grants for scoped commands, and fixes the edge case where--agentcould previously upgrade--auto-approveinto write permission.Introduces
evaluateWriteGate(with newExitCode.WRITE_REFUSED = 13) to let a PreToolUse hook deny write/destructive tool calls, including refined destructive Bash pattern matching (avoids flaggingpnpm/npm/yarn/bun rmandgit push --force-with-lease), and adds comprehensive tests covering the new behavior.Reviewed by Cursor Bugbot for commit 0b69de6. Bugbot is set up for automated code reviews on this repo. Configure here.