diff --git a/CHANGELOG.md b/CHANGELOG.md index 577600b5..e8752071 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## [Unreleased] +### Changed + +- Replace permission tiers with Ask, Approve for Me (default), and Full Access. Full Access runs dangerous and unanalyzable commands after a session-only or remembered risk acknowledgment. Saved legacy permission modes reset to Ask for users to choose again; `--approval-mode strict` is no longer accepted. Both automatic tiers enable bounded model-error continuation by default; Ask does not. + ### Fixed +- Recognize `/exit` as an alias for `/quit`, including while the agent is running or compacting. - Recognize `search_files`, `find_files`, and `list_directory` when selecting default system-prompt file-exploration guidance. diff --git a/README.md b/README.md index 3e282981..a14e87d2 100644 --- a/README.md +++ b/README.md @@ -184,7 +184,7 @@ When running from the source tree, replace `step` with `pnpm step`\. ## Security -- Four permission modes \(Ask / Read Only / Bypass / Autopilot\), cycled with `Shift+Tab`; dangerous commands require a separate confirmation dialog in every mode\. +- Three permission tiers \(Ask / Approve for Me / Full Access\), cycled with `Shift+Tab`; dangerous commands require a separate confirmation dialog in Ask and Approve for Me, and run without one only in Full Access after its risk acknowledgment\. ## Uninstall diff --git a/README.zh-CN.md b/README.zh-CN.md index 5f404ca5..95da760e 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -154,7 +154,7 @@ pnpm step ## 安全 -- 权限四档（Ask / Read Only / Bypass / Autopilot），`Shift+Tab` 循环切换；危险命令在任何模式下都会单独弹窗确认。 +- 权限三档（Ask / Approve for Me / Full Access），`Shift+Tab` 循环切换；危险命令在 Ask 和 Approve for Me 下都会单独弹窗确认，只有 Full Access 在风险确认后不再拦截。 ## 卸载 diff --git a/apps/cli/src/ui/runtime/input-dispatch.ts b/apps/cli/src/ui/runtime/input-dispatch.ts index a9f942cd..a3aa07c2 100644 --- a/apps/cli/src/ui/runtime/input-dispatch.ts +++ b/apps/cli/src/ui/runtime/input-dispatch.ts @@ -394,7 +394,7 @@ export function wireSubmitHandler(ctx: RuntimeContext): void { ctx.editor.setText(""); return; } - if (text === "/quit") { + if (text === "/quit" || text === "/exit") { ctx.editor.setText(""); await ctx.shutdown(); return; diff --git a/apps/cli/src/ui/view/chrome/footer.ts b/apps/cli/src/ui/view/chrome/footer.ts index f0d11dc8..c25ea6e6 100644 --- a/apps/cli/src/ui/view/chrome/footer.ts +++ b/apps/cli/src/ui/view/chrome/footer.ts @@ -311,9 +311,10 @@ export class FooterComponent implements Component { const extensionStatuses = this.footerData.getExtensionStatuses(); const permissionStatus = extensionStatuses.get("step-permission"); const statusPreset = permissionStatus?.match(/^Mode:\s*([^()]+?)(?:\s*\(auto-resume\))?$/u)?.[1]?.trim(); - // Extension statuses use human-readable labels (for example "Read Only"), - // while embedded hosts may expose the corresponding id ("read-only" or - // "readOnly"). Normalize the vocabulary before selecting the footer color. + // Extension statuses use human-readable labels (for example "Approve for + // Me"), while embedded hosts may expose the corresponding id + // ("approve-for-me"). Normalize the vocabulary before selecting the footer + // color. const normalizedStatusPreset = statusPreset?.toLowerCase().replace(/[\s_-]+/gu, ""); const rawMode = String( (this.session as unknown as { approvalMode?: string }).approvalMode ?? @@ -324,18 +325,13 @@ export class FooterComponent implements Component { .trim() .toLowerCase() .replace(/[\s_-]+/gu, ""); + const effectiveMode = normalizedStatusPreset ?? normalizedRawMode; const mode = - normalizedStatusPreset === "autopilot" - ? { label: "Autopilot", paint: warning } - : normalizedStatusPreset === "bypass" - ? { label: "Bypass", paint: warning } - : normalizedRawMode === "auto" || normalizedRawMode === "bypasspermissions" - ? { label: "Bypass", paint: warning } - : normalizedStatusPreset === "readonly" || - normalizedRawMode === "strict" || - normalizedRawMode === "readonly" - ? { label: "Read-only", paint: muted } - : { label: "Ask", paint: accent }; + effectiveMode === "fullaccess" + ? { label: "Full Access", paint: error } + : effectiveMode === "approveforme" || effectiveMode === "auto" || effectiveMode === "bypasspermissions" + ? { label: "Approve for Me", paint: warning } + : { label: "Ask", paint: accent }; const displayMode = this.bashMode ? { label: "Shell", paint: (text: string) => theme.fg("error", text) } : mode; // The permission cycle was reachable but invisible: the footer named the // mode and nothing said how to change it. Feedback issue-b39a464025061aa5. diff --git a/apps/cli/test/footer-width.test.ts b/apps/cli/test/footer-width.test.ts index 7b91511c..296b8cf0 100644 --- a/apps/cli/test/footer-width.test.ts +++ b/apps/cli/test/footer-width.test.ts @@ -296,7 +296,7 @@ describe("FooterComponent width handling", () => { }); const plain = stripAnsi(footer.render(120)[0] ?? ""); - expect(plain.startsWith("⏵ Bypass (shift+tab)")).toBe(true); + expect(plain.startsWith("⏵ Approve for Me (shift+tab)")).toBe(true); }); it("omits the permission cycle key on a narrow terminal", () => { @@ -308,7 +308,7 @@ describe("FooterComponent width handling", () => { const plain = stripAnsi(footer.render(50)[0] ?? ""); expect(plain).not.toContain("shift+tab"); - expect(plain).toContain("⏵ Bypass"); + expect(plain).toContain("⏵ Approve for Me"); }); it("omits the permission cycle key when the session has no cycle", () => { @@ -318,19 +318,35 @@ describe("FooterComponent width handling", () => { expect(stripAnsi(footer.render(120)[0] ?? "")).not.toContain("shift+tab"); }); - it("renders the read-only permission status instead of falling back to ask", () => { - const session = createSession({ sessionName: "", approvalMode: "strict" }); + it("renders the full access permission status", () => { + const session = createSession({ sessionName: "", approvalMode: "auto" }); const footer = new FooterComponent( session, - createFooterData(1, new Map([["step-permission", "Mode: Read Only"]])), + createFooterData(1, new Map([["step-permission", "Mode: Full Access"]])), { presentation: "step" }, ); const [line = ""] = footer.render(120); const plain = stripAnsi(line); - expect(plain.startsWith("⏵ Read-only")).toBe(true); - expect(plain.startsWith("⏵ Ask")).toBe(false); - expect(plain).not.toContain("Mode: Read Only"); + expect(plain.startsWith("⏵ Full Access")).toBe(true); + expect(plain.startsWith("⏵ Approve for Me")).toBe(false); + expect(plain).not.toContain("Mode: Full Access"); + }); + + it("prefers the active Ask status over a stale automatic raw mode", () => { + const session = createSession({ sessionName: "", approvalMode: "auto" }); + const footer = new FooterComponent( + session, + createFooterData(1, new Map([["step-permission", "Mode: Ask"]])), + { presentation: "step" }, + ); + expect(stripAnsi(footer.render(120)[0] ?? "").startsWith("⏵ Ask")).toBe(true); + }); + + it.each(["approve-for-me", "full-access"])("renders the raw %s tier when no status is present", (preset) => { + const session = createSession({ sessionName: "", approvalMode: preset }); + const footer = new FooterComponent(session, createFooterData(1), { presentation: "step" }); + expect(stripAnsi(footer.render(120)[0] ?? "")).toContain(preset === "full-access" ? "Full Access" : "Approve for Me"); }); it("does not duplicate the permission status while retaining other statuses", () => { @@ -340,7 +356,7 @@ describe("FooterComponent width handling", () => { createFooterData( 1, new Map([ - ["step-permission", "Mode: Autopilot (auto-resume)"], + ["step-permission", "Mode: Approve for Me (auto-resume)"], ["plan-mode", "plan 1/2"], ]), ), @@ -349,8 +365,8 @@ describe("FooterComponent width handling", () => { const [line = ""] = footer.render(120); const plain = stripAnsi(line); - expect(plain.startsWith("⏵ Autopilot")).toBe(true); - expect(plain).not.toContain("Mode: Autopilot"); + expect(plain.startsWith("⏵ Approve for Me")).toBe(true); + expect(plain).not.toContain("Mode: Approve for Me"); expect(plain).toContain("plan 1/2"); }); diff --git a/apps/cli/test/interactive-exit-command.test.ts b/apps/cli/test/interactive-exit-command.test.ts new file mode 100644 index 00000000..622c570d --- /dev/null +++ b/apps/cli/test/interactive-exit-command.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it, vi } from "vitest"; +import { BUILTIN_SLASH_COMMANDS } from "../../../packages/coding-agent/src/core/slash-commands.ts"; +import type { RuntimeContext } from "../src/ui/runtime/context.ts"; +import { wireSubmitHandler } from "../src/ui/runtime/input-dispatch.ts"; + +describe("interactive exit commands", () => { + it("advertises /exit in slash-command discovery", () => { + expect(BUILTIN_SLASH_COMMANDS.some((command) => command.name === "exit")).toBe(true); + }); + + it.each([ + { name: "idle", isStreaming: false, isCompacting: false }, + { name: "streaming", isStreaming: true, isCompacting: false }, + { name: "compacting", isStreaming: false, isCompacting: true }, + ])("exits immediately while $name instead of queuing a prompt", async (state) => { + for (const command of ["/exit", "/quit", " /exit "]) { + const editor = { setText: vi.fn(), onSubmit: undefined as ((text: string) => Promise) | undefined }; + const shutdown = vi.fn(async () => {}); + const prompt = vi.fn(); + const unknown = vi.fn(() => "exit"); + const showError = vi.fn(); + const queueCompactionMessage = vi.fn(); + const context = { + defaultEditor: editor, editor, shutdown, + session: { ...state, prompt }, + getUnknownSlashCommandName: unknown, showError, queueCompactionMessage, + } as unknown as RuntimeContext; + wireSubmitHandler(context); + await editor.onSubmit!(command); + expect(shutdown).toHaveBeenCalledOnce(); + expect(editor.setText).toHaveBeenCalledWith(""); + expect(unknown).not.toHaveBeenCalled(); + expect(showError).not.toHaveBeenCalled(); + expect(prompt).not.toHaveBeenCalled(); + expect(queueCompactionMessage).not.toHaveBeenCalled(); + } + }); +}); diff --git a/docs/command-permissions.md b/docs/command-permissions.md index d37bdd08..9ec86960 100644 --- a/docs/command-permissions.md +++ b/docs/command-permissions.md @@ -1,17 +1,21 @@ # Command permissions -Explicit per-tool denial takes precedence. Otherwise, shared command analysis +Explicit per-tool overrides remain effective, including `deny` and `confirm` +under Full Access. Otherwise, shared command analysis returns one of three outcomes: -| Analysis | Ask / Bypass / Autopilot | Read-only | Without an approval channel | +| Analysis | Ask / Approve for Me | Full Access (acknowledged) | Ask / Approve for Me without an approval channel | | --- | --- | --- | --- | -| A built-in dangerous rule matched | Confirm each call | Deny | Deny | -| Syntax or executable input could not be fully analyzed | Confirm each call, explaining the uncertainty | Deny | Deny | -| Analysis completed without a rule match | Ordinary preset/tool policy | Ordinary read-only policy | Ordinary unattended policy | - -Unresolved analysis is not reported as a detected dangerous command. +| A built-in dangerous rule matched | Confirm each call | Run without confirmation | Deny | +| Syntax or executable input could not be fully analyzed | Confirm each call, explaining the uncertainty | Run without confirmation | Deny | +| Analysis completed without a rule match | Ordinary tier/tool policy | Run without confirmation | Ordinary unattended policy | + +Full Access is the only tier that runs a detected dangerous command or an +unanalyzable one; it requires the one-time risk acknowledgment and is the sole +behavioral difference from Approve for Me. Unresolved analysis is not reported +as a detected dangerous command. `StepToolDecision.analysisIncomplete` distinguishes it from `hazardous`. -Neither outcome can use an automatic tool override or unattended `allow`. +Outside Full Access, neither outcome can use an automatic tool override or unattended `allow`. Explicit user approval applies only to that call. Without an approval channel, a deny ends the run by default. The opt-in @@ -20,7 +24,7 @@ env `STEP_NON_INTERACTIVE_DENIAL=continue`) keeps a recoverable confirmation blocked but returns it as a failed tool result so the agent can continue. Unsupported shells and shell-configuration failures still terminate because a model retry cannot repair the execution environment. Explicit per-tool denial -and read-only mode still terminate, and dangerous commands remain blocked. +still terminates, and dangerous commands remain blocked outside Full Access. The product policy in `packages/coding-agent/src/step/permissions.ts` runs through the existing `tool_call` hook, before foreground/background execution. Clients diff --git a/docs/step-configuration.md b/docs/step-configuration.md index 4e277635..c75e74b7 100644 --- a/docs/step-configuration.md +++ b/docs/step-configuration.md @@ -21,12 +21,13 @@ by an older release is still repaired in place at startup. ## Mandatory command approval -Permission presets and tool overrides cannot automatically approve commands -matched by the built-in command rules. In particular, `rm` with both recursive -and force options requires confirmation for every target, including `./build` -and `/tmp/cache`. Bypass, auto, and autopilot still ask for each call. Read-only -mode blocks it, and runs without an approval channel cannot execute it even -with `nonInteractiveApproval = "allow"`. +Ask and Approve for Me cannot automatically approve commands matched by the +built-in command rules. In particular, `rm` with both recursive and force +options requires confirmation for every target, including `./build` and +`/tmp/cache`. Full Access is the only tier that runs them without confirmation, +and it requires the one-time risk acknowledgment; a run without an approval +channel cannot execute a dangerous command under the other tiers even with +`nonInteractiveApproval = "allow"`. By default, a run without an approval channel terminates after such a block. `--non-interactive-denial continue` (or `STEP_NON_INTERACTIVE_DENIAL=continue`) @@ -34,12 +35,40 @@ keeps a recoverable confirmation blocked but reports it as a failed tool result, so an unattended agent can take a safer route instead of ending the run. Unsupported shells and shell-configuration failures still terminate because the agent cannot repair the execution environment by retrying. Explicit denials — -read-only mode and `deny` tool overrides — still terminate, and dangerous -commands remain blocked. +`deny` tool overrides — still terminate, and dangerous commands remain blocked +outside Full Access. See [command permissions](command-permissions.md) for matching behavior and how to extend the built-in rules. +### Full Access confirmation + +Select Full Access using `/permissions full-access` or `Shift+Tab`: + +- **Yes, continue anyway** enables Full Access for this session without saving the tier. +- **Yes, and don't ask again** saves Full Access and its risk acknowledgment for future launches. +- **Cancel** keeps the current tier. + +A session-only acknowledgment expires when the session is replaced or closed. +Without an interactive UI, an unacknowledged Full Access selection falls back to +Approve for Me. An acknowledged Full Access selection also runs dangerous +commands unattended. Explicit per-tool `deny` and `confirm` overrides still apply. + +### Upgrading permission settings and automatic continuation + +Saved Bypass, Autopilot, and Read Only policies reset to Ask on upgrade, including +their old `mode`, unattended-approval and auto-resume fields. Saved policies with +only the old low-level approval mode also reset to Ask. Select a new tier through +`/permissions` or `Shift+Tab`; Full Access requires the risk acknowledgment above. +Once a new tier is saved, later launches retain it. + +Both Approve for Me and Full Access enable the bounded model-error continuation +by default; Ask does not resume automatically. Selecting either automatic tier +enables continuation even if the old tier had saved `autoResume = false`. +The existing retry limits and backoff remain unchanged. `STEP_AUTO_RESUME` or an +explicit `autoResume` config value can override continuation at startup without +granting extra permissions. + ## Environment and shell commands `STEP_CODING_AGENT_DIR` selects the agent directory for CLI and SDK callers. diff --git a/docs/step-unified-config-and-mcp.md b/docs/step-unified-config-and-mcp.md index d36c4d0f..d34c99a2 100644 --- a/docs/step-unified-config-and-mcp.md +++ b/docs/step-unified-config-and-mcp.md @@ -334,7 +334,9 @@ agent 目录本身,而不是把凭据写到宿主指定的命名空间之外。 theme = "step-dark" defaultProvider = "step" defaultModel = "step-2" -permissionPreset = "standard" + +[tools.approval] +preset = "approve-for-me" autoResume = true [telemetry] diff --git a/packages/coding-agent/docs/environment-variables.md b/packages/coding-agent/docs/environment-variables.md index 9817e723..cc218afe 100644 --- a/packages/coding-agent/docs/environment-variables.md +++ b/packages/coding-agent/docs/environment-variables.md @@ -8,6 +8,10 @@ Step reads the variables below. Configuration shared across launches belongs in | `STEP_CODING_AGENT_DIR` | Override the agent directory; default is `~/.stepcode/agent` | | `STEP_CODING_AGENT_SESSION_DIR` | Override session storage; `--session-dir` takes precedence | | `STEP_CODING_AGENT_PLAN_DIR` | Directory for newly selected native Markdown plan paths; defaults to `/.stepcode/plans` | +| `STEP_PERMISSION_PRESET` | Permission tier: `ask`, `approve-for-me`, or `full-access`. Retired `bypass`/`autopilot`/`read-only` tiers reset to `ask` so the user can choose again; `full-access` requires risk acknowledgment | +| `STEP_APPROVAL_MODE` | Low-level approval mode: `confirm` or `auto` (retired `strict` maps to `confirm`) | +| `STEP_AUTO_RESUME` | Model-error continuation override (`1` enables, `0` disables). Enabled by default in Approve for Me and Full Access, inactive in Ask. Does not grant permissions | +| `STEP_AUTOPILOT` | Retired tier selector; resets to Ask. Select a new tier instead | | `STEP_API_KEY` | StepFun API credential | | `STEP_BASE_URL` | Override the StepFun API endpoint | | `STEP_PROVIDER`, `STEP_MODEL` | Default provider and model selection | diff --git a/packages/coding-agent/docs/step-integration.md b/packages/coding-agent/docs/step-integration.md index 968e42d8..d2c4f1c0 100644 --- a/packages/coding-agent/docs/step-integration.md +++ b/packages/coding-agent/docs/step-integration.md @@ -452,8 +452,8 @@ Pi before loading the Step product adapter. This layer does not reimplement editor key handling, UTF-8 buffering, render throttling, selectors, or agent scheduling. Step's permission extension only -defines the product presets and dangerous-command policy; it invokes Pi's -native `tool_call` confirmation and `ui.select` flows. Autopilot likewise +defines the product tiers and dangerous-command policy; it invokes Pi's +native `tool_call` confirmation and `ui.select` flows. Auto-resume likewise toggles Pi's native retry setting and adds only a bounded continuation after a settled retryable failure. Changes to the underlying interaction or scheduling behavior belong in Pi's shared packages and should be consumed here through diff --git a/packages/coding-agent/src/cli/args.ts b/packages/coding-agent/src/cli/args.ts index a95942bc..9a5cac61 100644 --- a/packages/coding-agent/src/cli/args.ts +++ b/packages/coding-agent/src/cli/args.ts @@ -68,7 +68,7 @@ export interface Args { /** Request-time lightweight context projection mode (step.compaction.contextProjection). */ contextProjection?: "off" | "lightweight-v1"; projectTrustOverride?: boolean; - /** Step tool approval mode (confirm, auto, or strict). */ + /** Step tool approval mode (confirm or auto). */ approvalMode?: StepPermissionMode; /** Fallback for approval requests when no interactive UI is available. */ nonInteractiveApproval?: StepNonInteractiveApproval; @@ -245,15 +245,15 @@ export function parseArgs(args: string[]): Args { } else if (arg === "--approval-mode" || arg.startsWith("--approval-mode=")) { const value = arg === "--approval-mode" ? args[i + 1] : arg.slice("--approval-mode=".length); if (arg === "--approval-mode" && (value === undefined || value.startsWith("-"))) { - result.diagnostics.push({ type: "error", message: "--approval-mode requires confirm, auto, or strict" }); + result.diagnostics.push({ type: "error", message: "--approval-mode requires confirm or auto" }); } else { if (arg === "--approval-mode") i++; - if (value === "confirm" || value === "auto" || value === "strict") { + if (value === "confirm" || value === "auto") { result.approvalMode = value; } else { result.diagnostics.push({ type: "error", - message: `Invalid approval mode "${value}". Valid values: confirm, auto, strict`, + message: `Invalid approval mode "${value}". Valid values: confirm, auto`, }); } } @@ -595,14 +595,15 @@ export function printHelp(extensionFlags?: ExtensionFlag[]): void { " STEP_BASE_URL Step provider API base URL", " STEPCODE_DEFAULT_THEME Default interactive theme for step", " STEPCODE_DISABLE_PI_SERVICES Disable upstream update/catalog services (enabled by step)", - " STEP_APPROVAL_MODE Default tool approval mode (confirm|auto|strict)", + " STEP_APPROVAL_MODE Default tool approval mode (confirm|auto)", " STEP_NON_INTERACTIVE_APPROVAL Fallback when no approval UI is available (allow|deny)", " STEP_NON_INTERACTIVE_DENIAL Blocked call without a UI (terminate|continue)", " STEP_AUTOPILOT Enable bounded model-error auto-resume", + " STEP_PERMISSION_PRESET Permission tier (ask|approve-for-me|full-access)", ].join("\n") : ""; const stepPermissionOptionsText = IS_STEP_ENTRYPOINT - ? "\n --approval-mode Tool approval mode: confirm, auto, or strict\n --non-interactive-approval Fallback without a UI: allow or deny\n --non-interactive-denial Blocked call without a UI: terminate the run or continue\n --tool-override Per-tool override (repeatable; mode: allow, confirm, deny)" + ? "\n --approval-mode Tool approval mode: confirm or auto\n --non-interactive-approval Fallback without a UI: allow or deny\n --non-interactive-denial Blocked call without a UI: terminate the run or continue\n --tool-override Per-tool override (repeatable; mode: allow, confirm, deny)" : ""; const stepAuthCommandsText = IS_STEP_ENTRYPOINT ? `\n ${APP_NAME} login Sign in with the Step account (OAuth)\n ${APP_NAME} logout Remove the stored Step credential` diff --git a/packages/coding-agent/src/core/slash-commands.ts b/packages/coding-agent/src/core/slash-commands.ts index 6cab5dfa..9a921555 100644 --- a/packages/coding-agent/src/core/slash-commands.ts +++ b/packages/coding-agent/src/core/slash-commands.ts @@ -72,4 +72,5 @@ export const BUILTIN_SLASH_COMMANDS: ReadonlyArray = [ description: "Reload keybindings, extensions, skills, prompts, themes, and context files", }, { name: "quit", description: `Quit ${APP_NAME}` }, + { name: "exit", description: `Quit ${APP_NAME} (alias for /quit)` }, ]; diff --git a/packages/coding-agent/src/features/step.ts b/packages/coding-agent/src/features/step.ts index 72f9568c..3d670edc 100644 --- a/packages/coding-agent/src/features/step.ts +++ b/packages/coding-agent/src/features/step.ts @@ -5,6 +5,7 @@ import type { ExtensionCommandContext, ExtensionContext, ExtensionFactory, + ExtensionUIContext, InlineExtension, } from "../core/extensions/types.ts"; import { BUILTIN_SLASH_COMMANDS } from "../core/slash-commands.ts"; @@ -13,11 +14,17 @@ import { STEP_INIT_PROMPT } from "../step/init-prompt.ts"; import { createStepMcpExtension } from "../step/mcp.ts"; import { AUTO_RESUME_PROMPT, + FULL_ACCESS_RISK_MESSAGE, + FULL_ACCESS_RISK_OPTIONS, + FULL_ACCESS_RISK_TITLE, getStepPermissionPreset, + normalizeStepPermissionPresetId, + parseFullAccessRiskChoice, publishStepPermissionStatus, StepAutoResumeController, StepPermissionController, type StepPermissionControllerOptions, + type StepPermissionState, } from "../step/permissions.ts"; import { createStepPluginResourcesExtension } from "../step/plugins.ts"; import type { StepSettingsManager } from "../step/settings-manager.ts"; @@ -131,6 +138,69 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi let autoResumeAllowed = false; let nativeRetryPreference: boolean | undefined; let nativeRetryOverridden = false; + /** + * Session-scoped Full Access grant. The persisted "don't ask again" choice + * lives in Step settings and is consulted alongside this flag; the flag is + * cleared on session shutdown so a "this session" grant does not leak into + * a replacement session. + */ + let fullAccessAcknowledged = false; + /** + * Incremented on every session_start. An async risk-dialog grant only + * applies while its generation is current, so a prompt answered after a + * trust re-probe or /new replaced the session cannot flip the new + * session's policy through a stale callback. + */ + let permissionSessionGeneration = 0; + + const isFullAccessAcknowledged = (): boolean => + fullAccessAcknowledged || options.stepSettings?.()?.getStepSettings().fullAccessAcknowledged === true; + + const isFullAccessState = (state: StepPermissionState): boolean => state.skipCommandPolicy === true; + + /** + * Ask the user to accept the Full Access risk. Resolves true for both + * grant options ("always" persists the acknowledgment) and false when the + * dialog is dismissed, so callers keep the previous tier. + */ + const confirmFullAccessRisk = async (ui: ExtensionUIContext): Promise => { + const generation = permissionSessionGeneration; + if (typeof ui.select !== "function") return false; + const selected = await ui.select(`${FULL_ACCESS_RISK_TITLE}\n\n${FULL_ACCESS_RISK_MESSAGE}`, [ + ...FULL_ACCESS_RISK_OPTIONS, + ]); + if (generation !== permissionSessionGeneration) return false; + const choice = parseFullAccessRiskChoice(selected); + if (choice === "cancel") return false; + fullAccessAcknowledged = true; + if (choice === "always") { + try { + options.stepSettings?.()?.setEffectiveStepSettings({ + permissionPreset: "full-access", + approvalMode: "auto", + nonInteractiveApproval: "allow", + autoResume: true, + fullAccessAcknowledged: true, + }); + } catch { + // Persisting the acknowledgment must not block the current grant. + } + } + return true; + }; + + /** Apply a resolved policy state: retry switch, footer, telemetry, persistence. */ + const applyPermissionState = ( + ctx: ExtensionContext, + state: StepPermissionState, + apply: { source: "startup" | "shortcut" | "command"; persist: boolean }, + ): void => { + syncNativeRetry(ctx, state.autoResume); + publishStepPermissionStatus(ctx.ui, state); + trackPermissionMode(options.telemetry, state.preset, apply.source === "startup" ? "command" : apply.source); + if (apply.persist) persistPermissionState(options.stepSettings, state); + ctx.ui.notify(`Permission mode: ${getStepPermissionPreset(state.preset)?.label ?? state.preset}`, "info"); + }; /** * Let Pi own provider retries. Step's Autopilot only changes the native @@ -211,26 +281,61 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi }); pi.on("session_start", (_event, ctx) => { + const generation = ++permissionSessionGeneration; + fullAccessAcknowledged = false; notify = ctx.ui.notify; // During project-trust probing Pi loads inline extensions while the // project scope is hidden. Rehydrate once the final trust decision has // been applied so project-level Step policy is not silently ignored. const persistedOptions = resolvePermissionOptions(options); if (persistedOptions) permissions = new StepPermissionController(persistedOptions); - const state = permissions.getState(); + let state = permissions.getState(); // A replacement session gets its own context. The previous session's - // shutdown handler restores any temporary Autopilot override before this + // shutdown handler restores any temporary auto-resume override before this // callback runs, so this snapshot is the user's real native preference. syncNativeRetry(ctx, state.autoResume, true); + // A persisted or env-selected Full Access tier still owes the risk + // acknowledgment. Prompt once; without a grant the session falls back to + // Approve for Me instead of silently executing dangerous commands. + if (isFullAccessState(state) && !isFullAccessAcknowledged()) { + const requestedPermissions = permissions; + if (ctx.hasUI) { + void confirmFullAccessRisk(ctx.ui) + .then((granted) => { + // A newer session_start (trust re-probe, /new, reload) owns the + // policy now; its own dialog decides the outcome. + if (!granted || generation !== permissionSessionGeneration) return; + permissions = requestedPermissions; + applyPermissionState(ctx, permissions.getState(), { source: "startup", persist: false }); + }) + .catch(() => { + // A UI torn down mid-dialog must not surface as an unhandled + // rejection; the session stays on Approve for Me. + }); + } else { + notify?.( + "Full access needs its risk acknowledgment, which requires an interactive session; using Approve for Me.", + "warning", + ); + } + // Startup fallback changes only the tier, preserving explicit overrides + // such as unattended denial and the requested auto-resume preference. + permissions = new StepPermissionController({ ...persistedOptions, initialPreset: "approve-for-me" }); + state = permissions.getState(); + } publishStepPermissionStatus(ctx.ui, state); }); pi.on("session_shutdown", (_event, ctx) => { + permissionSessionGeneration++; // Timers and UI callbacks are scoped to the old session. Cancel them before // Pi tears down its extension runner, and restore the native retry setting - // if Step temporarily enabled it for Autopilot. + // if Step temporarily enabled it for auto-resume. autoResumeAllowed = false; autoResume.reset(); + // A "this session" Full Access grant must not carry into the next + // session; only the persisted acknowledgment does. + fullAccessAcknowledged = false; if (nativeRetryOverridden && nativeRetryPreference !== undefined && ctx.setAutoRetryEnabled) { try { ctx.setAutoRetryEnabled(nativeRetryPreference); @@ -319,20 +424,29 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi }); const handlePermissionCommand = async (args: string, ctx: ExtensionCommandContext): Promise => { + const generation = permissionSessionGeneration; const requested = args.trim().toLowerCase(); if (requested === "--cycle" || requested === "cycle") { + // Full Access is gated by the risk acknowledgment; a canceled dialog + // keeps the current tier instead of moving. + const nextPreset = permissions.nextPresetId(); + if (nextPreset === "full-access" && !isFullAccessAcknowledged()) { + const granted = await confirmFullAccessRisk(ctx.ui); + if (generation !== permissionSessionGeneration) return; + if (!granted) { + ctx.ui.notify("Full access was not enabled.", "warning"); + return; + } + } const state = permissions.cycle(); - trackPermissionMode(options.telemetry, state.preset, "shortcut"); // Deliberately not persisted. The shortcut means "stop asking me right // now, I am watching", but persisting it wrote the whole policy triple - // — including `nonInteractiveApproval: "allow"` under Bypass — to - // config.toml, so one keypress silently granted every later unattended - // `--print` run in that project permission to write and execute. Use + // — including `nonInteractiveApproval: "allow"` — to config.toml, so + // one keypress silently granted every later unattended `--print` run in + // that project permission to write and execute. Use // `/permissions ` for a durable choice. autoResume.reset(); - syncNativeRetry(ctx, state.autoResume); - publishStepPermissionStatus(ctx.ui, state); - ctx.ui.notify(`Permission mode: ${getStepPermissionPreset(state.preset)?.label ?? state.preset}`, "info"); + applyPermissionState(ctx, state, { source: "shortcut", persist: false }); return; } @@ -340,18 +454,28 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi if (!selected) { const options = permissionsPresetsForSelector(); selected = (await ctx.ui.select("Permission mode", options))?.trim().toLowerCase() ?? ""; + if (!selected || generation !== permissionSessionGeneration) return; + } + if (normalizeStepPermissionPresetId(selected) === "full-access" && !isFullAccessAcknowledged()) { + const granted = await confirmFullAccessRisk(ctx.ui); + if (generation !== permissionSessionGeneration) return; + if (!granted) { + ctx.ui.notify("Full access was not enabled.", "warning"); + return; + } } const state = permissions.setPreset(selected); if (!state) { - ctx.ui.notify("Unknown permission mode. Use ask, read-only, bypass, or autopilot.", "warning"); + ctx.ui.notify("Unknown permission mode. Use ask, approve-for-me, or full-access.", "warning"); return; } autoResume.reset(); - trackPermissionMode(options.telemetry, state.preset, "command"); - persistPermissionState(options.stepSettings, state); - syncNativeRetry(ctx, state.autoResume); - publishStepPermissionStatus(ctx.ui, state); - ctx.ui.notify(`Permission mode: ${getStepPermissionPreset(state.preset)?.label ?? state.preset}`, "info"); + applyPermissionState(ctx, state, { + source: "command", + persist: + state.preset !== "full-access" || + options.stepSettings?.()?.getStepSettings().fullAccessAcknowledged === true, + }); }; // Keep only the plural product command. The old singular alias was never a @@ -397,10 +521,8 @@ function resolvePermissionOptions(options: StepExtensionOptions): StepPermission ...explicit, ...(explicit?.initialPreset === undefined && explicit?.approvalMode === undefined && - explicit?.autoResume === undefined && !hasEnvPreset && !hasEnvMode && - !hasEnvAutoResume && persisted.permissionPreset ? { initialPreset: persisted.permissionPreset } : {}), @@ -439,7 +561,7 @@ function persistPermissionState( export const stepExtension: ExtensionFactory = createStepExtension(); function permissionsPresetsForSelector(): string[] { - return ["ask", "read-only", "bypass", "autopilot"]; + return ["ask", "approve-for-me", "full-access"]; } function trackPermissionMode( diff --git a/packages/coding-agent/src/step/permissions.ts b/packages/coding-agent/src/step/permissions.ts index ea1dc11d..c5db2317 100644 --- a/packages/coding-agent/src/step/permissions.ts +++ b/packages/coding-agent/src/step/permissions.ts @@ -21,8 +21,8 @@ import { getThenRunCommand } from "./then-run.ts"; export { containsDangerousLifecycleCommand, isDangerousCommand } from "./command-policy.ts"; -export type StepPermissionPresetId = "ask" | "read-only" | "bypass" | "autopilot"; -export type StepPermissionMode = "confirm" | "strict" | "auto"; +export type StepPermissionPresetId = "ask" | "approve-for-me" | "full-access"; +export type StepPermissionMode = "confirm" | "auto"; export type StepNonInteractiveApproval = "allow" | "deny"; export type StepNonInteractiveDenial = "terminate" | "continue"; export type StepToolPermissionMode = "allow" | "confirm" | "deny"; @@ -34,40 +34,41 @@ export interface StepPermissionPreset { mode: StepPermissionMode; nonInteractiveApproval: StepNonInteractiveApproval; autoResume: boolean; + /** + * Full Access only: detected dangerous commands and commands whose analysis + * is incomplete run without confirmation. This flag is the entire behavioral + * difference between Approve for Me and Full Access; every other policy + * (ordinary approvals, per-tool overrides, unattended fallback) is shared. + */ + skipCommandPolicy?: boolean; } export const STEP_PERMISSION_PRESETS: readonly StepPermissionPreset[] = [ { id: "ask", label: "Ask", - description: "Safe tools run; writes and commands ask first", + description: "Ask before modifying files or running commands", mode: "confirm", nonInteractiveApproval: "deny", autoResume: false, }, { - id: "read-only", - label: "Read Only", - description: "Read and discovery tools only", - mode: "strict", - nonInteractiveApproval: "deny", - autoResume: false, - }, - { - id: "bypass", - label: "Bypass", - description: "Run ordinary tools without approval; dangerous commands still ask", + id: "approve-for-me", + label: "Approve for Me", + description: + "Routine actions run automatically; only actions detected as potentially unsafe and key decisions ask", mode: "auto", nonInteractiveApproval: "allow", - autoResume: false, + autoResume: true, }, { - id: "autopilot", - label: "Autopilot", - description: "Bypass ordinary approvals and resume transient model failures", + id: "full-access", + label: "Full Access", + description: "Every action runs without confirmation, including dangerous commands; trusted environments only", mode: "auto", nonInteractiveApproval: "allow", autoResume: true, + skipCommandPolicy: true, }, ]; @@ -115,6 +116,12 @@ export interface StepPermissionState { * permissive, so a run without a UI must not inherit it. */ defaulted?: boolean; + /** + * Full Access only: dangerous-command and incomplete-analysis gates no + * longer confirm or deny. Always accompanied by an acknowledged risk + * confirmation (see confirmFullAccessRisk); never set by a default. + */ + skipCommandPolicy?: boolean; /** Per-tool policy overrides supplied by the StepCode or an embedding host. */ toolOverrides?: Readonly>; } @@ -162,22 +169,38 @@ export function getStepPermissionPreset(id: string | undefined): StepPermissionP return STEP_PERMISSION_PRESETS.find((preset) => preset.id === normalized); } -/** Normalize the mode vocabulary used by older Step clients and pi hosts. */ +/** + * Normalize the mode vocabulary used by older Step clients and pi hosts. + * The retired read-only tier maps onto Ask, the strongest remaining tier. + */ export function normalizeStepPermissionPresetId(value: string | undefined): StepPermissionPresetId | undefined { switch (value?.trim().toLowerCase()) { case "ask": case "confirm": return "ask"; - case "read-only": - case "readonly": - case "strict": - return "read-only"; + case "approve-for-me": + case "approve": + case "approveforme": + return "approve-for-me"; + // Retired tiers require the user to choose a new permission tier. case "bypass": case "auto": - case "bypasspermissions": - return "bypass"; case "autopilot": - return "autopilot"; + case "bypasspermissions": + return "ask"; + case "full-access": + case "fullaccess": + case "full": + case "yolo": + case "never-ask": + return "full-access"; + // The retired Read Only tier degrades to Ask rather than to a tier that + // would silently execute writes. + case "read-only": + case "readonly": + case "strict": + case "plan": + return "ask"; default: return undefined; } @@ -190,12 +213,12 @@ export function normalizeStepPermissionMode(value: string | undefined): StepPerm case "ask": case "default": case "acceptedits": - return "confirm"; + // Retired with the read-only tier; treated as the nearest remaining tier. case "strict": case "read-only": case "readonly": case "plan": - return "strict"; + return "confirm"; case "auto": case "bypass": case "bypasspermissions": @@ -222,7 +245,7 @@ function resolveStepPermissionPresetWithProvenance( options: StepPermissionControllerOptions = {}, ): ResolvedStepPermissionPreset { if (options.initialPreset && getStepPermissionPreset(options.initialPreset)) { - return { preset: options.initialPreset, defaulted: false }; + return { preset: getStepPermissionPreset(options.initialPreset)!.id, defaulted: false }; } const env = options.env ?? process.env; @@ -230,44 +253,39 @@ function resolveStepPermissionPresetWithProvenance( // existing preset-first behavior for the older `STEP_PERMISSION_MODE` alias. const explicitMode = options.approvalMode ?? normalizeStepPermissionMode(env.STEP_APPROVAL_MODE); if (explicitMode) { - const nonInteractive = - options.nonInteractiveApproval ?? - (env.STEP_NON_INTERACTIVE_APPROVAL ?? env.STEP_NONINTERACTIVE_APPROVAL)?.trim().toLowerCase(); - const autoResume = options.autoResume ?? (isTruthy(env.STEP_AUTOPILOT) || isTruthy(env.STEP_AUTO_RESUME)); - if (explicitMode === "auto") { - return { - preset: autoResume && nonInteractive !== "deny" ? "autopilot" : "bypass", - defaulted: false, - }; - } + // Low-level modes select Approve for Me at most: Full Access is only + // reachable through an explicit preset id, because it additionally + // requires the risk acknowledgment. + if (explicitMode === "auto") return { preset: "approve-for-me", defaulted: false }; + return { preset: "ask", defaulted: false }; + } + const explicitPreset = env.STEP_PERMISSION_PRESET?.trim().toLowerCase(); + const normalizedExplicitPreset = normalizeStepPermissionPresetId(explicitPreset); + if (normalizedExplicitPreset) { return { - preset: explicitMode === "strict" ? "read-only" : "ask", + preset: normalizedExplicitPreset, defaulted: false, }; } - const explicitPreset = env.STEP_PERMISSION_PRESET?.trim().toLowerCase(); - const normalizedExplicitPreset = normalizeStepPermissionPresetId(explicitPreset); - if (normalizedExplicitPreset) return { preset: normalizedExplicitPreset, defaulted: false }; const mode = env.STEP_PERMISSION_MODE?.trim().toLowerCase(); - if (mode === "strict" || mode === "read-only" || mode === "readonly") { - return { preset: "read-only", defaulted: false }; + if (mode === "strict" || mode === "read-only" || mode === "readonly" || mode === "plan") { + return { preset: "ask", defaulted: false }; } if (mode === "auto" || mode === "bypass" || mode === "bypasspermissions") { - return { - preset: isTruthy(env.STEP_AUTOPILOT) ? "autopilot" : "bypass", - defaulted: false, - }; + return { preset: "ask", defaulted: false }; } if (mode === "confirm" || mode === "ask") return { preset: "ask", defaulted: false }; - if (isTruthy(env.STEP_AUTOPILOT)) return { preset: "autopilot", defaulted: false }; - // Default to bypass: tools run without approval prompts. Explicit CLI flags, - // STEP_* env vars, and any persisted preset are all resolved above this line, - // so they continue to override the default. Dangerous commands (see - // decideStepToolCall / isDangerousCommand) still require confirmation even - // under bypass, and a run with no UI refuses the defaulted policy outright - // (see StepPermissionController.handleToolCall). - return { preset: "bypass", defaulted: true }; + if (isTruthy(env.STEP_AUTOPILOT)) { + return { preset: "ask", defaulted: false }; + } + // Default to Approve for Me: routine tools run without approval prompts. + // Explicit CLI flags, STEP_* env vars, and any persisted preset are all + // resolved above this line, so they continue to override the default. + // Dangerous commands (see decideStepToolCall / isDangerousCommand) still + // require confirmation, and a run with no UI refuses the defaulted policy + // outright (see StepPermissionController.handleToolCall). + return { preset: "approve-for-me", defaulted: true }; } export function stepPermissionStateForPreset(presetId: StepPermissionPresetId): StepPermissionState { @@ -277,6 +295,7 @@ export function stepPermissionStateForPreset(presetId: StepPermissionPresetId): mode: preset.mode, nonInteractiveApproval: preset.nonInteractiveApproval, autoResume: preset.autoResume, + ...(preset.skipCommandPolicy ? { skipCommandPolicy: true } : {}), }; } @@ -289,6 +308,15 @@ export function normalizeAutoResume( return mode === "auto" && nonInteractiveApproval === "allow" && autoResume === true; } +/** + * Both automatic tiers enable model-error continuation by default. Explicit + * options or STEP_AUTO_RESUME can disable it; this never selects a permission tier. + */ +export function resolveAutoResumeRequest(options: StepPermissionControllerOptions = {}): boolean { + const env = options.env ?? process.env; + return options.autoResume ?? (env.STEP_AUTO_RESUME?.trim() ? isTruthy(env.STEP_AUTO_RESUME) : true); +} + /** * Resolve the complete policy triple. The old Step runtime accepted the mode, * non-interactive fallback, and auto-resume flag independently; keep that @@ -305,22 +333,34 @@ export function resolveInitialStepPermissionState(options: StepPermissionControl const nonInteractiveApproval: StepNonInteractiveApproval = rawNonInteractive === "allow" || rawNonInteractive === "deny" ? rawNonInteractive : preset.nonInteractiveApproval; const effectiveMode = mode ?? preset.mode; - const requestedAutoResume = - options.autoResume ?? (isTruthy(env.STEP_AUTOPILOT) || isTruthy(env.STEP_AUTO_RESUME) || preset.autoResume); + const requestedAutoResume = resolveAutoResumeRequest(options); const autoResume = normalizeAutoResume(effectiveMode, nonInteractiveApproval, requestedAutoResume); const resolved: StepPermissionState = { preset: preset.preset, mode: effectiveMode, nonInteractiveApproval, autoResume, + ...(preset.skipCommandPolicy ? { skipCommandPolicy: true } : {}), }; + // Approve for Me and Full Access share the (mode, approval, resume) triple, + // so skipCommandPolicy must participate in the relabeling match; otherwise a + // Full Access state with a requested auto-resume (or an explicit unattended + // deny) would be renamed to Approve for Me while still skipping the command + // policy, and the footer would understate the active tier. The mode-only + // fallback keeps the legacy mixed-triple relabeling, still tier-aware. const matchingPreset = STEP_PERMISSION_PRESETS.find( (candidate) => candidate.mode === resolved.mode && candidate.nonInteractiveApproval === resolved.nonInteractiveApproval && - candidate.autoResume === resolved.autoResume, - ) ?? STEP_PERMISSION_PRESETS.find((candidate) => candidate.mode === resolved.mode); + candidate.autoResume === resolved.autoResume && + (candidate.skipCommandPolicy ?? false) === (resolved.skipCommandPolicy ?? false), + ) ?? + STEP_PERMISSION_PRESETS.find( + (candidate) => + candidate.mode === resolved.mode && + (candidate.skipCommandPolicy ?? false) === (resolved.skipCommandPolicy ?? false), + ); if (matchingPreset) resolved.preset = matchingPreset.id; // Only a grant counts as configuring the policy. An explicit `deny` asks for // less access, so it must not move the policy out of the defaulted bucket and @@ -364,8 +404,9 @@ function combineThenRunDecisions( /** * Decide a tool call without involving the terminal. This is intentionally - * conservative for unknown tools: ask mode confirms them, read-only blocks - * them, and bypass permits them unless the call is hazardous. + * conservative for unknown tools: ask mode confirms them, Approve for Me + * permits them unless the call is hazardous or its analysis is incomplete, and + * Full Access permits everything (see skipCommandPolicy). */ export function decideStepToolCall( toolName: string, @@ -400,6 +441,11 @@ export function decideStepToolCall( } const commandRule = analysis?.kind === "matched" ? analysis.ruleId : undefined; const override = findToolOverride(normalizedName, overrides); + // Full Access only: the dangerous-command and incomplete-analysis gates are + // the sole difference from Approve for Me, so they are skipped together. + // Explicit per-tool denials are user-configured blocks, not confirmations, + // and still apply below. + const enforceCommandPolicy = state.skipCommandPolicy !== true; if (override === "deny") { return { @@ -409,17 +455,17 @@ export function decideStepToolCall( }; } - if (commandRule) { + if (commandRule && enforceCommandPolicy) { return { - action: state.mode === "strict" ? "deny" : "confirm", + action: "confirm", hazardous: true, reason: `Dangerous command requires confirmation (${commandRule}): ${summarizeToolInput(toolName, input)}`, }; } - if (analysis?.kind === "unresolved") { + if (analysis?.kind === "unresolved" && enforceCommandPolicy) { return { - action: state.mode === "strict" ? "deny" : "confirm", + action: "confirm", hazardous: false, analysisIncomplete: true, analysisReason: analysis.reason, @@ -436,18 +482,11 @@ export function decideStepToolCall( } const mutating = WRITE_OR_EXECUTE_TOOLS.has(normalizedName) || !READ_ONLY_TOOLS.has(normalizedName); - if (state.mode === "strict" && mutating) { - return { - action: "deny", - hazardous: false, - reason: `Read-only mode blocks ${toolName}`, - }; - } if (state.mode === "auto") { return { action: "allow", hazardous: false, - reason: "Bypass approval mode is enabled", + reason: "Automatic approval mode is enabled", }; } if (!mutating) { @@ -497,15 +536,21 @@ export class StepPermissionController { setPreset(presetId: string): StepPermissionState | undefined { const preset = getStepPermissionPreset(presetId); if (!preset) return undefined; - this.state = stepPermissionStateForPreset(preset.id); + const next = stepPermissionStateForPreset(preset.id); + // A deliberate tier selection uses its defaults, rather than carrying + // the disabled autoResume field saved by an older Ask/Bypass preset. + this.state = next; return this.getState(); } cycle(): StepPermissionState { + return this.setPreset(this.nextPresetId())!; + } + + /** The preset `cycle()` would move to, without applying it. */ + nextPresetId(): StepPermissionPresetId { const index = STEP_PERMISSION_PRESETS.findIndex((preset) => preset.id === this.state.preset); - const next = STEP_PERMISSION_PRESETS[(index + 1) % STEP_PERMISSION_PRESETS.length]!; - this.state = stepPermissionStateForPreset(next.id); - return this.getState(); + return STEP_PERMISSION_PRESETS[(index + 1) % STEP_PERMISSION_PRESETS.length]!.id; } /** @@ -541,7 +586,7 @@ export class StepPermissionController { * Why a call may not run unattended. Two cases qualify: * * - nothing selected the policy at all, so the call would inherit the - * interactive Bypass default with nobody watching; + * interactive Approve for Me default with nobody watching; * - the caller explicitly refused unattended approvals. * * Feedback issue-287bfff1a5fe7668. @@ -561,8 +606,7 @@ export class StepPermissionController { * fallback consulted, because `auto` decides every call as `allow` and * returns before the fallback is reached. Without this, * `--non-interactive-approval deny` silently does nothing. Only `auto` needs - * it: `confirm` and `strict` already route through the fallback with their - * own reasons, and demoting `strict` would weaken read-only mode. + * it: `confirm` already routes through the fallback with its own reason. */ private effectiveState(cause: UnattendedCause): StepPermissionState { if (this.state.mode !== "auto" || !(cause.refused || cause.unconfigured)) return this.state; @@ -764,7 +808,7 @@ export interface StepAutoResumeTelemetry { } /** - * Bounded, abortable continuation scheduler for Step's autopilot tier. + * Bounded, abortable continuation scheduler for Step's auto-resume tier. * Pi's own retry loop runs first; this controller handles a final settled * transport/model failure and resumes with a context-aware instruction. */ @@ -921,3 +965,34 @@ export function publishStepPermissionStatus(ui: ExtensionUIContext, state: StepP const preset = getStepPermissionPreset(state.preset) ?? STEP_PERMISSION_PRESETS[0]!; ui.setStatus("step-permission", `Mode: ${preset.label}${state.autoResume ? " (auto-resume)" : ""}`); } + +// ============================================================================= +// Full Access risk acknowledgment +// ============================================================================= + +/** + * Full Access executes dangerous commands without any confirmation, so + * entering the tier requires an explicit risk acknowledgment. The dialog + * mirrors the Codex/Claude wording: one session-scoped grant, one remembered + * grant, and a cancel that keeps the previous tier. + */ +export const FULL_ACCESS_RISK_TITLE = "Enable full access?"; + +export const FULL_ACCESS_RISK_MESSAGE = + "When Step runs with full access, it can edit any file on your computer and run commands with network, without your approval. Exercise caution when enabling full access. This significantly increases the risk of data loss, leaks, or unexpected behavior."; + +/** Dialog options in display order; the selected label maps back to a choice. */ +export const FULL_ACCESS_RISK_OPTIONS = [ + "Yes, continue anyway — apply full access for this session", + "Yes, and don't ask again — enable full access and remember this choice", + "Cancel — go back without enabling full access", +] as const; + +export type FullAccessRiskChoice = "session" | "always" | "cancel"; + +/** Map a selected option label (or a dismissed dialog) to a risk choice. */ +export function parseFullAccessRiskChoice(selected: string | undefined): FullAccessRiskChoice { + if (selected === FULL_ACCESS_RISK_OPTIONS[0]) return "session"; + if (selected === FULL_ACCESS_RISK_OPTIONS[1]) return "always"; + return "cancel"; +} diff --git a/packages/coding-agent/src/step/settings-manager.ts b/packages/coding-agent/src/step/settings-manager.ts index 933ffdd0..b3657895 100644 --- a/packages/coding-agent/src/step/settings-manager.ts +++ b/packages/coding-agent/src/step/settings-manager.ts @@ -3,7 +3,7 @@ * * Pi owns the canonical settings schema and its global/project merge rules. * Step adds a small product-owned namespace in a sidecar file so product - * policy (for example `ask`/`autopilot`) does not leak into Pi's settings + * policy (for example `ask`/`approve-for-me`) does not leak into Pi's settings * schema. The returned object is a transparent decorator: every Pi method is * still available and is invoked against the original SettingsManager. */ @@ -33,6 +33,12 @@ export interface StepSettings { nonInteractiveApproval?: StepNonInteractiveApproval; /** Enables the bounded model-error continuation ladder. */ autoResume?: boolean; + /** + * The user accepted the Full Access risk with "don't ask again". Recorded + * separately from the active preset so leaving Full Access does not force a + * second acknowledgment when it is selected again. + */ + fullAccessAcknowledged?: boolean; /** Enables the user feedback submission flow. Defaults to enabled. */ feedbackEnabled?: boolean; } @@ -132,7 +138,19 @@ function merge(base: JsonObject, overrides: JsonObject): JsonObject { return result; } -function normalizeSettings(value: JsonObject): StepSettings { +/** Old saved policies must not silently authorize one of the new automatic tiers. */ +export function resetLegacyPermissionSettings(settings: StepSettings, rawPreset?: string, rawMode?: string): void { + const preset = rawPreset?.trim().toLowerCase(); + const retired = ["bypass", "auto", "autopilot", "bypasspermissions", "read-only", "readonly", "strict", "plan"]; + if (!(preset && retired.includes(preset)) && !(rawPreset === undefined && rawMode !== undefined)) return; + settings.permissionPreset = "ask"; + settings.approvalMode = "confirm"; + settings.nonInteractiveApproval = "deny"; + settings.autoResume = false; + delete settings.fullAccessAcknowledged; +} + +function normalizeSettings(value: JsonObject, resetLegacy = true): StepSettings { const result: StepSettings = {}; // Older Step configs used both a top-level `approval` object and a nested // `tools.approval` object. Keep both candidates in precedence order, but do @@ -140,21 +158,25 @@ function normalizeSettings(value: JsonObject): StepSettings { const approval = asObject(value.approval); const toolsApproval = asObject(asObject(value.tools)?.approval); const presetCandidates = [value.permissionPreset, value.permissionMode, approval?.preset, toolsApproval?.preset]; + let winningPresetCandidate: string | undefined; for (const candidate of presetCandidates) { if (typeof candidate !== "string") continue; const preset = getStepPermissionPreset(candidate); if (preset) { result.permissionPreset = preset.id; + winningPresetCandidate = candidate; break; } } const modeCandidates = [value.approvalMode, approval?.mode, toolsApproval?.mode]; + let winningModeCandidate: string | undefined; for (const candidate of modeCandidates) { if (typeof candidate !== "string") continue; const mode = normalizeStepPermissionMode(candidate); if (mode) { result.approvalMode = mode; + winningModeCandidate = candidate; break; } } @@ -191,6 +213,18 @@ function normalizeSettings(value: JsonObject): StepSettings { } } + const fullAccessAcknowledgedCandidates = [ + value.fullAccessAcknowledged, + approval?.fullAccessAcknowledged, + toolsApproval?.fullAccessAcknowledged, + ]; + for (const candidate of fullAccessAcknowledgedCandidates) { + if (typeof candidate === "boolean") { + result.fullAccessAcknowledged = candidate; + break; + } + } + const feedbackEnabledCandidates = [value.feedbackEnabled, asObject(value.feedback)?.enabled]; for (const candidate of feedbackEnabledCandidates) { if (typeof candidate === "boolean") { @@ -198,14 +232,30 @@ function normalizeSettings(value: JsonObject): StepSettings { break; } } + if (resetLegacy) resetLegacyPermissionSettings(result, winningPresetCandidate, winningModeCandidate); return result; } +function savedPermissionSelection(value: JsonObject): { preset?: string; mode?: string } { + const approval = asObject(value.approval); + const nested = asObject(asObject(value.tools)?.approval); + const preset = [value.permissionPreset, value.permissionMode, approval?.preset, nested?.preset].find( + (candidate): candidate is string => + typeof candidate === "string" && getStepPermissionPreset(candidate) !== undefined, + ); + const mode = [value.approvalMode, approval?.mode, nested?.mode].find( + (candidate): candidate is string => + typeof candidate === "string" && normalizeStepPermissionMode(candidate) !== undefined, + ); + return { preset, mode }; +} + const STEP_SETTING_ALIASES: Record = { permissionPreset: ["permissionMode"], approvalMode: [], nonInteractiveApproval: ["noninteractiveApproval"], autoResume: ["autopilot"], + fullAccessAcknowledged: [], feedbackEnabled: [], }; @@ -230,6 +280,10 @@ const STEP_SETTING_NESTED_ALIASES: Record): Partial { if (settings.permissionPreset !== undefined && !getStepPermissionPreset(settings.permissionPreset)) { throw new Error(`Invalid Step permission preset: ${String(settings.permissionPreset)}`); } - if ( - settings.approvalMode !== undefined && - settings.approvalMode !== "confirm" && - settings.approvalMode !== "strict" && - settings.approvalMode !== "auto" - ) { + if (settings.approvalMode !== undefined && settings.approvalMode !== "confirm" && settings.approvalMode !== "auto") { throw new Error(`Invalid Step approval mode: ${String(settings.approvalMode)}`); } if ( @@ -278,6 +327,9 @@ function validatePatch(settings: Partial): Partial { if (settings.autoResume !== undefined && typeof settings.autoResume !== "boolean") { throw new Error(`Invalid Step autoResume setting: ${String(settings.autoResume)}`); } + if (settings.fullAccessAcknowledged !== undefined && typeof settings.fullAccessAcknowledged !== "boolean") { + throw new Error(`Invalid Step fullAccessAcknowledged setting: ${String(settings.fullAccessAcknowledged)}`); + } if (settings.feedbackEnabled !== undefined && typeof settings.feedbackEnabled !== "boolean") { throw new Error(`Invalid Step feedbackEnabled setting: ${String(settings.feedbackEnabled)}`); } @@ -484,10 +536,17 @@ class StepSettingsDecorator { // global canonical field (for example `feedbackEnabled`) survives the raw // object merge and outranks the equivalent project alias // (`feedback.enabled`) during normalization. - return { - ...normalizeSettings(this.store.getGlobal()), - ...normalizeSettings(this.store.getProject()), - }; + const global = this.store.getGlobal(); + const project = this.store.getProject(); + const settings = { ...normalizeSettings(global, false), ...normalizeSettings(project, false) }; + const localPolicy = savedPermissionSelection(project); + const policy = + localPolicy.preset !== undefined || localPolicy.mode !== undefined + ? localPolicy + : savedPermissionSelection(global); + // A partial project setting must not reactivate a migrated legacy grant. + resetLegacyPermissionSettings(settings, policy.preset, policy.mode); + return settings; } getStepGlobalSettings(): StepSettings { @@ -540,7 +599,7 @@ class StepSettingsDecorator { } setStepApprovalMode(mode: StepPermissionMode | undefined): void { - if (mode !== undefined && mode !== "confirm" && mode !== "strict" && mode !== "auto") { + if (mode !== undefined && mode !== "confirm" && mode !== "auto") { throw new Error(`Invalid Step approval mode: ${String(mode)}`); } this.setEffectiveStepSettings({ approvalMode: mode }); diff --git a/packages/coding-agent/src/step/slash-commands.ts b/packages/coding-agent/src/step/slash-commands.ts index 3440581f..130f0cfa 100644 --- a/packages/coding-agent/src/step/slash-commands.ts +++ b/packages/coding-agent/src/step/slash-commands.ts @@ -69,18 +69,6 @@ export function registerStepPiCommandAdapters( telemetry, ); - registerTrackedCommand( - pi, - "exit", - { - description: "Exit the interactive shell", - handler: async (_args, ctx) => { - ctx.shutdown(); - }, - }, - telemetry, - ); - registerTrackedCommand( pi, "theme", diff --git a/packages/coding-agent/src/step/stepcode-config.ts b/packages/coding-agent/src/step/stepcode-config.ts index 04b05ea1..25659c84 100644 --- a/packages/coding-agent/src/step/stepcode-config.ts +++ b/packages/coding-agent/src/step/stepcode-config.ts @@ -15,7 +15,12 @@ import { type StepPermissionMode, type StepPermissionPresetId, } from "./permissions.ts"; -import type { StepSettings, StepSettingsManager, StepSettingsPaths } from "./settings-manager.ts"; +import { + resetLegacyPermissionSettings, + type StepSettings, + type StepSettingsManager, + type StepSettingsPaths, +} from "./settings-manager.ts"; export const STEPCODE_CONFIG_ENV_NAME = "STEPCODE_CONFIG_PATH"; @@ -336,15 +341,19 @@ function readStepCodeSettings(root: JsonObject): StepSettings { const nonInteractiveApproval = rawNonInteractive === "allow" || rawNonInteractive === "deny" ? rawNonInteractive : undefined; const autoResume = readBoolean(approval?.autoResume ?? approval?.autopilot); + const fullAccessAcknowledged = readBoolean(approval?.fullAccessAcknowledged); const feedback = asObject(root.feedback); const feedbackEnabled = readBoolean(feedback?.enabled); - return { + const settings: StepSettings = { ...(preset ? { permissionPreset: preset } : {}), ...(mode ? { approvalMode: mode } : {}), ...(nonInteractiveApproval ? { nonInteractiveApproval } : {}), ...(autoResume !== undefined ? { autoResume } : {}), + ...(fullAccessAcknowledged !== undefined ? { fullAccessAcknowledged } : {}), ...(feedbackEnabled !== undefined ? { feedbackEnabled } : {}), }; + resetLegacyPermissionSettings(settings, readString(approval?.preset), readString(approval?.mode)); + return settings; } function writeStepCodeSettings(root: Record, patch: Partial): void { @@ -354,6 +363,7 @@ function writeStepCodeSettings(root: Record, patch: Partial): void { if (settings.permissionPreset !== undefined && !getStepPermissionPreset(settings.permissionPreset)) { throw new Error(`Invalid Step permission preset: ${String(settings.permissionPreset)}`); } - if ( - settings.approvalMode !== undefined && - settings.approvalMode !== "confirm" && - settings.approvalMode !== "strict" && - settings.approvalMode !== "auto" - ) { + if (settings.approvalMode !== undefined && settings.approvalMode !== "confirm" && settings.approvalMode !== "auto") { throw new Error(`Invalid Step approval mode: ${String(settings.approvalMode)}`); } if ( @@ -398,6 +403,9 @@ function validateStepSettings(settings: Partial): void { if (settings.autoResume !== undefined && typeof settings.autoResume !== "boolean") { throw new Error(`Invalid Step autoResume setting: ${String(settings.autoResume)}`); } + if (settings.fullAccessAcknowledged !== undefined && typeof settings.fullAccessAcknowledged !== "boolean") { + throw new Error(`Invalid Step fullAccessAcknowledged setting: ${String(settings.fullAccessAcknowledged)}`); + } } function findRawModelHandle(root: JsonObject, providerId: string, modelId: string): string | undefined { diff --git a/packages/coding-agent/test/args.test.ts b/packages/coding-agent/test/args.test.ts index 6e966ebc..2a267af1 100644 --- a/packages/coding-agent/test/args.test.ts +++ b/packages/coding-agent/test/args.test.ts @@ -396,25 +396,34 @@ describe("parseArgs", () => { test("supports equals syntax and repeated per-tool overrides", () => { expect( parseArgs([ - "--approval-mode=strict", + "--approval-mode=confirm", "--non-interactive-approval=deny", "--tool-override", "write_file=deny", "--tool-override=run_command=confirm", ]), ).toMatchObject({ - approvalMode: "strict", + approvalMode: "confirm", nonInteractiveApproval: "deny", toolOverride: { write_file: "deny", run_command: "confirm" }, toolOverrides: { write_file: "deny", run_command: "confirm" }, }); }); + test("rejects the retired strict approval mode", () => { + const result = parseArgs(["--approval-mode", "strict"]); + expect(result.approvalMode).toBeUndefined(); + expect(result.diagnostics).toContainEqual({ + type: "error", + message: 'Invalid approval mode "strict". Valid values: confirm, auto', + }); + }); + test("reports invalid approval values without treating them as messages", () => { const result = parseArgs(["--approval-mode", "wat", "--tool-override", "bash=wat"]); expect(result.messages).toEqual([]); expect(result.diagnostics).toEqual([ - { type: "error", message: 'Invalid approval mode "wat". Valid values: confirm, auto, strict' }, + { type: "error", message: 'Invalid approval mode "wat". Valid values: confirm, auto' }, { type: "error", message: 'Invalid --tool-override "bash=wat". Expected ' }, ]); }); diff --git a/packages/coding-agent/test/step-command-policy-case.test.ts b/packages/coding-agent/test/step-command-policy-case.test.ts index 4d2d10c7..08147beb 100644 --- a/packages/coding-agent/test/step-command-policy-case.test.ts +++ b/packages/coding-agent/test/step-command-policy-case.test.ts @@ -36,7 +36,7 @@ describe.each(["darwin", "linux", "win32"] as const)("executable case on %s", (p it.each(mixedCaseCommands)("keeps platform-specific approval for %s", async (command) => { const hazardous = platform !== "linux"; expect(analyzeCommandPolicy(command)).toMatchObject({ kind: hazardous ? "matched" : "ordinary" }); - for (const preset of ["bypass", "autopilot"] as const) { + for (const preset of ["ask", "approve-for-me"] as const) { expect( decideStepToolCall("run_command", { command }, stepPermissionStateForPreset(preset), { run_command: "allow", @@ -122,7 +122,7 @@ describe.skipIf(actualPlatform !== "darwin")("case-insensitive executable lookup expect(result.status, result.stderr).toBe(0); expect(result.stderr).toBe(""); expect(readFileSync(callLog, "utf8")).toBe(`${call}\n`); - for (const preset of ["bypass", "autopilot"] as const) { + for (const preset of ["ask", "approve-for-me"] as const) { expect(decideStepToolCall("run_command", { command }, stepPermissionStateForPreset(preset))).toMatchObject({ action: "confirm", hazardous: true, diff --git a/packages/coding-agent/test/step-command-policy-shell-semantics.test.ts b/packages/coding-agent/test/step-command-policy-shell-semantics.test.ts index 9ccc4dbe..470afdb7 100644 --- a/packages/coding-agent/test/step-command-policy-shell-semantics.test.ts +++ b/packages/coding-agent/test/step-command-policy-shell-semantics.test.ts @@ -519,11 +519,14 @@ describe.skipIf(process.platform === "win32")("command policy agrees with harmle const hazardous = calls.length > 0 && !analysisIncomplete; expect.soft(isDangerousCommand(command)).toBe(hazardous); - for (const preset of ["bypass", "autopilot"] as const) { + for (const preset of ["ask", "approve-for-me"] as const) { + // Ask confirms every mutating call; the automatic tiers only confirm + // hazardous or incompletely analyzed commands. + const confirmExpected = hazardous || analysisIncomplete || preset === "ask"; expect .soft(decideStepToolCall("run_command", { command }, stepPermissionStateForPreset(preset))) .toMatchObject({ - action: hazardous || analysisIncomplete ? "confirm" : "allow", + action: confirmExpected ? "confirm" : "allow", hazardous, ...(analysisIncomplete ? { analysisIncomplete: true } : {}), }); diff --git a/packages/coding-agent/test/step-command-policy-uncertainty.test.ts b/packages/coding-agent/test/step-command-policy-uncertainty.test.ts index 048d4aa8..884df6ed 100644 --- a/packages/coding-agent/test/step-command-policy-uncertainty.test.ts +++ b/packages/coding-agent/test/step-command-policy-uncertainty.test.ts @@ -6,17 +6,17 @@ describe("incomplete command analysis requires an explicit decision", () => { it.each(['[ -n "$PATH" ]', 'export PATH="$PATH"', `export FOO="\${BAR:-safe}"`])( "keeps ordinary prefixes eligible for unattended execution: %s", async (commandPrefix) => { - for (const initialPreset of ["bypass", "autopilot"] as const) { - const controller = new StepPermissionController({ - env: {}, - initialPreset, - shellContext: () => ({ commandPrefix }), - }); - const input = { command: "printf safe" }; - expect(controller.decide("run_command", input)).toMatchObject({ action: "allow", hazardous: false }); - const event: ToolCallEvent = { type: "tool_call", toolName: "run_command", toolCallId: "prefix", input }; - expect(await controller.handleToolCall(event, { hasUI: false } as ExtensionContext)).toBeUndefined(); - } + // Only the automatic tiers run ordinary confirmations unattended; Ask + // confirms every mutating call by design. + const controller = new StepPermissionController({ + env: {}, + initialPreset: "approve-for-me", + shellContext: () => ({ commandPrefix }), + }); + const input = { command: "printf safe" }; + expect(controller.decide("run_command", input)).toMatchObject({ action: "allow", hazardous: false }); + const event: ToolCallEvent = { type: "tool_call", toolName: "run_command", toolCallId: "prefix", input }; + expect(await controller.handleToolCall(event, { hasUI: false } as ExtensionContext)).toBeUndefined(); }, ); @@ -28,7 +28,7 @@ describe("incomplete command analysis requires an explicit decision", () => { ])("checks the executed prefix for $toolName input $input", async ({ toolName, input }) => { const controller = new StepPermissionController({ env: {}, - initialPreset: "bypass", + initialPreset: "approve-for-me", nonInteractiveApproval: "allow", shellContext: () => ({ commandPrefix: "rm -rf ./build" }), }); @@ -43,7 +43,7 @@ describe("incomplete command analysis requires an explicit decision", () => { it("omits the prefix only for the background tool that omits it during execution", () => { const controller = new StepPermissionController({ env: {}, - initialPreset: "bypass", + initialPreset: "approve-for-me", shellContext: () => ({ commandPrefix: "rm -rf ./build" }), }); expect(controller.decide("run_command", { command: ":", run_in_background: true })).toMatchObject({ @@ -75,7 +75,7 @@ describe("incomplete command analysis requires an explicit decision", () => { 'local -I value="$VALUE"', "readarray -C 'rm -rf ./build; #' -c 1 <<< ok", ])("does not turn an incomplete analysis into unattended permission: %s", async (command) => { - for (const initialPreset of ["bypass", "autopilot"] as const) { + for (const initialPreset of ["ask", "approve-for-me"] as const) { const controller = new StepPermissionController({ env: {}, initialPreset, @@ -109,21 +109,20 @@ describe("incomplete command analysis requires an explicit decision", () => { } }); - it("permits uncertain input only after explicit approval and keeps strict/deny precedence", async () => { + it("permits uncertain input only after explicit approval and keeps deny precedence", async () => { const input = { command: 'bash -c "$SCRIPT"' }; const event: ToolCallEvent = { type: "tool_call", toolName: "run_command", toolCallId: "review", input }; const confirm = vi.fn(async () => true); const context = { hasUI: true, ui: { confirm } } as unknown as ExtensionContext; - const controller = new StepPermissionController({ env: {}, initialPreset: "bypass" }); + const controller = new StepPermissionController({ env: {}, initialPreset: "approve-for-me" }); expect(await controller.handleToolCall(event, context)).toBeUndefined(); expect(confirm).toHaveBeenCalledOnce(); - for (const options of [ - { initialPreset: "read-only" as const }, - { initialPreset: "bypass" as const, toolOverrides: { run_command: "deny" as const } }, - ]) { - const denied = new StepPermissionController({ env: {}, ...options }); - expect(await denied.handleToolCall(event, context)).toMatchObject({ block: true, terminate: true }); - } + const denied = new StepPermissionController({ + env: {}, + initialPreset: "approve-for-me", + toolOverrides: { run_command: "deny" }, + }); + expect(await denied.handleToolCall(event, context)).toMatchObject({ block: true, terminate: true }); expect(confirm).toHaveBeenCalledOnce(); }); }); diff --git a/packages/coding-agent/test/step-command-policy-wrappers.test.ts b/packages/coding-agent/test/step-command-policy-wrappers.test.ts index 5cdb899c..66d2bd42 100644 --- a/packages/coding-agent/test/step-command-policy-wrappers.test.ts +++ b/packages/coding-agent/test/step-command-policy-wrappers.test.ts @@ -11,7 +11,7 @@ vi.mock("../src/utils/shell.ts", () => ({ getShellConfig: () => ({ shell: "bash" async function checkDecision(command: string, kind: CommandPolicyAnalysis["kind"]): Promise { expect(analyzeCommandPolicy(command)).toMatchObject({ kind }); - for (const preset of ["bypass", "autopilot"] as const) { + for (const preset of ["ask", "approve-for-me"] as const) { expect( decideStepToolCall("run_command", { command }, stepPermissionStateForPreset(preset), { run_command: "allow" }), ).toMatchObject({ diff --git a/packages/coding-agent/test/step-command-policy.test.ts b/packages/coding-agent/test/step-command-policy.test.ts index ef6e64d2..8f7d57d0 100644 --- a/packages/coding-agent/test/step-command-policy.test.ts +++ b/packages/coding-agent/test/step-command-policy.test.ts @@ -126,7 +126,7 @@ describe("mandatory command approval", () => { }); describe.each(STEP_PERMISSION_PRESETS)("$id policy", (preset) => { - it.each(["run_command", "bash", "powershell"])("cannot auto-approve %s with an allow override", (toolName) => { + it.each(["run_command", "bash", "powershell"])("resolves an allow override for %s under its tier", (toolName) => { const decision = decideStepToolCall( toolName, { command: "rm -rf ./build" }, @@ -134,8 +134,10 @@ describe("mandatory command approval", () => { { [toolName]: "allow" }, ); expect(decision).toMatchObject({ - action: preset.mode === "strict" ? "deny" : "confirm", - hazardous: true, + // Full Access is the only tier that lets a detected dangerous + // command run; every other tier confirms it. + action: preset.skipCommandPolicy ? "allow" : "confirm", + ...(preset.skipCommandPolicy ? {} : { hazardous: true }), }); }); @@ -153,7 +155,8 @@ describe("mandatory command approval", () => { input: { command: "rm -rf ./build", run_in_background: true }, }; const result = await controller.handleToolCall(event, { hasUI: false } as ExtensionContext); - expect(result).toMatchObject({ block: true, terminate: true }); + if (preset.skipCommandPolicy) expect(result).toBeUndefined(); + else expect(result).toMatchObject({ block: true, terminate: true }); }); it("preserves explicit tool denial without offering approval", async () => { @@ -189,10 +192,10 @@ describe("mandatory command approval", () => { input: { command: "rm -rf ./build" }, }; const result = await controller.handleToolCall(event, context); - if (preset.mode !== "strict" && approved) expect(result).toBeUndefined(); + if (preset.skipCommandPolicy || approved) expect(result).toBeUndefined(); else expect(result).toMatchObject({ block: true }); } - expect(confirm).toHaveBeenCalledTimes(preset.mode === "strict" ? 0 : 2); + expect(confirm).toHaveBeenCalledTimes(preset.skipCommandPolicy ? 0 : 2); }); }); diff --git a/packages/coding-agent/test/step-extension.test.ts b/packages/coding-agent/test/step-extension.test.ts index b762c00b..c7b21491 100644 --- a/packages/coding-agent/test/step-extension.test.ts +++ b/packages/coding-agent/test/step-extension.test.ts @@ -8,21 +8,61 @@ import type { import { createStepExtension, stepExtension } from "../src/features/step.ts"; import { STEP_INIT_PROMPT } from "../src/step/init-prompt.ts"; -function commandContext(isIdle: boolean): ExtensionCommandContext { +function commandContext( + isIdle: boolean, + select?: (title: string, options: string[]) => Promise, +): ExtensionCommandContext { return { isIdle: () => isIdle, - ui: { notify: vi.fn(), setStatus: vi.fn() }, + ui: { notify: vi.fn(), setStatus: vi.fn(), select: select ?? (async () => undefined) }, } as unknown as ExtensionCommandContext; } describe("Step extension", () => { + test.each(["ask", "approve-for-me", "full-access"] as const)( + "resumes model errors by default only in automatic tiers: %s", + async (initialPreset) => { + vi.useFakeTimers(); + try { + const on = vi.fn(); + const sendUserMessage = vi.fn(); + createStepExtension({ + permission: { env: {}, initialPreset }, + stepSettings: () => ({ + getStepSettings: () => ({ fullAccessAcknowledged: true }), + setEffectiveStepSettings: vi.fn(), + }), + })({ on, sendUserMessage, registerProvider: vi.fn(), registerCommand: vi.fn() } as unknown as ExtensionAPI); + const handler = (name: string) => on.mock.calls.filter(([event]) => event === name).at(-1)![1]; + const ctx = { + hasUI: false, + ui: { notify: vi.fn(), setStatus: vi.fn() }, + isIdle: () => true, + hasPendingMessages: () => false, + autoRetryEnabled: false, + setAutoRetryEnabled: vi.fn(), + }; + handler("session_start")({}, ctx); + handler("agent_end")({ + messages: [{ role: "assistant", stopReason: "error", errorMessage: "network down" }], + }); + handler("agent_settled")({}, ctx); + await vi.advanceTimersByTimeAsync(5000); + expect(sendUserMessage).toHaveBeenCalledTimes(initialPreset === "ask" ? 0 : 1); + if (initialPreset !== "ask") expect(ctx.setAutoRetryEnabled).toHaveBeenCalledWith(true); + handler("session_shutdown")({}, ctx); + } finally { + vi.useRealTimers(); + } + }, + ); test.each([ { name: "selected non-Bash interpreter", shellPath: process.execPath, prefix: undefined }, { name: "executed command prefix", shellPath: undefined, prefix: "rm -rf ./build" }, ])("uses the real shell settings for approval: $name", async ({ shellPath, prefix }) => { const on = vi.fn(); createStepExtension({ - permission: { env: {}, initialPreset: "bypass", nonInteractiveApproval: "allow" }, + permission: { env: {}, initialPreset: "approve-for-me", nonInteractiveApproval: "allow" }, stepSettings: () => ({ getStepSettings: () => ({}), setEffectiveStepSettings: vi.fn(), @@ -134,7 +174,7 @@ describe("Step extension", () => { const command = registerCommand.mock.calls.find(([name]) => name === "permissions")?.[1] as { handler: (args: string, ctx: ExtensionCommandContext) => Promise; }; - await command.handler("bypass", commandContext(true)); + await command.handler("approve-for-me", commandContext(true)); const inputHandler = on.mock.calls.find(([name]) => name === "input")?.[1] as ( event: { text: string }, @@ -147,7 +187,11 @@ describe("Step extension", () => { { command: "/permissions", recognized: true }, undefined, ); - expect(track).toHaveBeenCalledWith("permission_mode_toggled", { mode: "bypass", source: "command" }, undefined); + expect(track).toHaveBeenCalledWith( + "permission_mode_toggled", + { mode: "approve-for-me", source: "command" }, + undefined, + ); expect(track).toHaveBeenCalledWith("slash_command_used", { command: "/unknown", recognized: false }, undefined); }); @@ -180,7 +224,7 @@ describe("Step extension", () => { const setEffectiveStepSettings = vi.fn(); const settings = { getStepSettings: () => ({ - permissionPreset: "bypass" as const, + permissionPreset: "approve-for-me" as const, approvalMode: "auto" as const, nonInteractiveApproval: "allow" as const, autoResume: false, @@ -205,16 +249,16 @@ describe("Step extension", () => { const command = registerCommand.mock.calls.find(([name]) => name === "permissions")?.[1] as { handler: (args: string, ctx: ExtensionCommandContext) => Promise; }; - await command.handler("autopilot", commandContext(true)); + await command.handler("approve-for-me", commandContext(true)); expect(setEffectiveStepSettings).toHaveBeenCalledWith({ - permissionPreset: "autopilot", + permissionPreset: "approve-for-me", approvalMode: "auto", nonInteractiveApproval: "allow", autoResume: true, }); }); - test.each(["bypass", "autopilot"] as const)( + test.each(["ask", "approve-for-me"] as const)( "keeps recursive forced removal gated through the %s tool-call hook", async (initialPreset) => { const on = vi.fn(); @@ -250,6 +294,7 @@ describe("Step extension", () => { const on = vi.fn(); const registerCommand = vi.fn(); const setEffectiveStepSettings = vi.fn(); + const setStatus = vi.fn(); const settings = { getStepSettings: () => ({}), setEffectiveStepSettings }; createStepExtension({ stepSettings: () => settings })({ registerProvider: vi.fn(), @@ -261,12 +306,358 @@ describe("Step extension", () => { const command = registerCommand.mock.calls.find(([name]) => name === "permissions")?.[1] as { handler: (args: string, ctx: ExtensionCommandContext) => Promise; }; - await command.handler("--cycle", commandContext(true)); + // The default tier is Approve for Me, so the first cycle lands on Full + // Access. A canceled dialog keeps the current tier instead of moving. + const cancel = vi.fn(async (): Promise => undefined); + const notify = vi.fn(); + await command.handler("--cycle", { + isIdle: () => true, + ui: { notify, setStatus, select: cancel }, + } as unknown as ExtensionCommandContext); + expect(cancel).toHaveBeenCalledOnce(); + expect(notify).toHaveBeenCalledWith("Full access was not enabled.", "warning"); + expect(setStatus).not.toHaveBeenCalledWith("step-permission", expect.stringContaining("Full Access")); + expect(setEffectiveStepSettings).not.toHaveBeenCalled(); + + // A granted dialog moves the tier to Full Access and still persists nothing. + const grant = vi.fn(async () => "Yes, continue anyway — apply full access for this session"); + const grantContext = commandContext(true, grant); + grantContext.ui.setStatus = setStatus; + await command.handler("--cycle", grantContext); + expect(grant).toHaveBeenCalledOnce(); + expect(setStatus).toHaveBeenCalledWith("step-permission", "Mode: Full Access (auto-resume)"); expect(setEffectiveStepSettings).not.toHaveBeenCalled(); + // Cycling again leaves Full Access for Ask without a dialog. + const noPrompt = vi.fn(async (): Promise => undefined); + const cycleContext = commandContext(true, noPrompt); + cycleContext.ui.setStatus = setStatus; + await command.handler("--cycle", cycleContext); + expect(noPrompt).not.toHaveBeenCalled(); + expect(setStatus).toHaveBeenCalledWith("step-permission", "Mode: Ask"); + // A named preset is a deliberate choice and still persists. - await command.handler("read-only", commandContext(true)); - expect(setEffectiveStepSettings).toHaveBeenCalledWith(expect.objectContaining({ permissionPreset: "read-only" })); + await command.handler("approve-for-me", commandContext(true)); + expect(setEffectiveStepSettings).toHaveBeenCalledWith( + expect.objectContaining({ permissionPreset: "approve-for-me" }), + ); + }); + + test("requires the Full Access risk acknowledgment before /permissions enables it", async () => { + const on = vi.fn(); + const registerCommand = vi.fn(); + const setEffectiveStepSettings = vi.fn(); + const select = vi.fn(async (): Promise => undefined); + createStepExtension({ + stepSettings: () => ({ getStepSettings: () => ({}), setEffectiveStepSettings }), + })({ + registerProvider: vi.fn(), + on, + registerCommand, + sendUserMessage: vi.fn(), + } as unknown as ExtensionAPI); + + const command = registerCommand.mock.calls.find(([name]) => name === "permissions")?.[1] as { + handler: (args: string, ctx: ExtensionCommandContext) => Promise; + }; + const notify = vi.fn(); + const dismissed = { + isIdle: () => true, + ui: { notify, setStatus: vi.fn(), select }, + } as unknown as ExtensionCommandContext; + + // A dismissed dialog keeps the previous tier and persists nothing. + await command.handler("full-access", dismissed); + expect(select).toHaveBeenCalledWith( + expect.stringContaining("Enable full access?"), + expect.arrayContaining([expect.stringContaining("apply full access for this session")]), + ); + expect(select).toHaveBeenCalledWith( + expect.stringContaining("Enable full access?"), + expect.arrayContaining([expect.stringContaining("remember this choice")]), + ); + expect(setEffectiveStepSettings).not.toHaveBeenCalled(); + expect(notify).toHaveBeenCalledWith("Full access was not enabled.", "warning"); + // The tier never moved: a dangerous command still asks. + const blockedHandler = on.mock.calls.find(([name]) => name === "tool_call")?.[1] as ( + event: unknown, + ctx: unknown, + ) => Promise; + expect( + await blockedHandler( + { + type: "tool_call", + toolName: "run_command", + toolCallId: "dismissed-removal", + input: { command: "rm -rf ./build" }, + }, + { hasUI: true, ui: { confirm: vi.fn(async () => false) } }, + ), + ).toMatchObject({ block: true }); + + // A session-scoped grant enables the tier and runs dangerous commands. + select.mockImplementation(async () => "Yes, continue anyway — apply full access for this session"); + await command.handler("full-access", commandContext(true, select)); + expect(setEffectiveStepSettings).not.toHaveBeenCalled(); + const toolHandler = on.mock.calls.find(([name]) => name === "tool_call")?.[1] as ( + event: unknown, + ctx: unknown, + ) => Promise; + expect( + await toolHandler( + { + type: "tool_call", + toolName: "run_command", + toolCallId: "full-access-removal", + input: { command: "rm -rf ./build" }, + }, + { hasUI: false }, + ), + ).toBeUndefined(); + + // The session-scoped grant suppresses a second prompt for the same tier. + select.mockClear(); + await command.handler("full-access", commandContext(true, select)); + expect(select).not.toHaveBeenCalled(); + }); + + test("remembers the Full Access acknowledgment when the user asks it to", async () => { + const on = vi.fn(); + const registerCommand = vi.fn(); + const setEffectiveStepSettings = vi.fn(); + const select = vi.fn(async () => "Yes, and don't ask again — enable full access and remember this choice"); + createStepExtension({ + stepSettings: () => ({ getStepSettings: () => ({}), setEffectiveStepSettings }), + })({ + registerProvider: vi.fn(), + on, + registerCommand, + sendUserMessage: vi.fn(), + } as unknown as ExtensionAPI); + + const command = registerCommand.mock.calls.find(([name]) => name === "permissions")?.[1] as { + handler: (args: string, ctx: ExtensionCommandContext) => Promise; + }; + await command.handler("full-access", commandContext(true, select)); + expect(setEffectiveStepSettings).toHaveBeenCalledWith(expect.objectContaining({ fullAccessAcknowledged: true })); + }); + + test.each([true, false])("applies the startup Full Access decision (approved=%s)", async (approved) => { + const on = vi.fn(); + const setStatus = vi.fn(); + let answer!: (value: string | undefined) => void; + const select = vi.fn( + () => + new Promise((resolve) => { + answer = resolve; + }), + ); + createStepExtension({ + permission: { env: {}, initialPreset: "full-access" }, + stepSettings: () => ({ getStepSettings: () => ({}), setEffectiveStepSettings: vi.fn() }), + })({ + registerProvider: vi.fn(), + on, + registerCommand: vi.fn(), + sendUserMessage: vi.fn(), + } as unknown as ExtensionAPI); + + const sessionStart = on.mock.calls.filter(([name]) => name === "session_start").at(-1)?.[1] as ( + event: unknown, + ctx: unknown, + ) => void; + sessionStart( + { type: "session_start", reason: "new" }, + { + hasUI: true, + ui: { notify: vi.fn(), setStatus, select }, + autoRetryEnabled: false, + setAutoRetryEnabled: vi.fn(), + }, + ); + // The session starts on Approve for Me while the dialog is pending. + expect(setStatus).toHaveBeenCalledWith("step-permission", "Mode: Approve for Me (auto-resume)"); + const toolHandler = on.mock.calls.find(([name]) => name === "tool_call")?.[1] as ( + event: unknown, + ctx: unknown, + ) => Promise; + const event = { + type: "tool_call", + toolName: "run_command", + toolCallId: "startup", + input: { command: "rm -rf ./x" }, + }; + expect(await toolHandler(event, { hasUI: false })).toMatchObject({ block: true }); + answer(approved ? "Yes, continue anyway — apply full access for this session" : undefined); + await Promise.resolve(); + await Promise.resolve(); + const confirm = vi.fn(async () => false); + const result = await toolHandler(event, { hasUI: true, ui: { confirm } }); + if (approved) { + expect(setStatus).toHaveBeenCalledWith("step-permission", "Mode: Full Access (auto-resume)"); + expect(result).toBeUndefined(); + expect(confirm).not.toHaveBeenCalled(); + } else { + expect(setStatus).not.toHaveBeenCalledWith("step-permission", "Mode: Full Access (auto-resume)"); + expect(result).toMatchObject({ block: true }); + expect(confirm).toHaveBeenCalledOnce(); + } + }); + + test("clears the session-scoped Full Access grant on shutdown", async () => { + const on = vi.fn(); + const registerCommand = vi.fn(); + const select = vi.fn(async () => "Yes, continue anyway — apply full access for this session"); + createStepExtension({ + stepSettings: () => ({ getStepSettings: () => ({}), setEffectiveStepSettings: vi.fn() }), + })({ + registerProvider: vi.fn(), + on, + registerCommand, + sendUserMessage: vi.fn(), + } as unknown as ExtensionAPI); + + const command = registerCommand.mock.calls.find(([name]) => name === "permissions")?.[1] as { + handler: (args: string, ctx: ExtensionCommandContext) => Promise; + }; + await command.handler("full-access", commandContext(true, select)); + select.mockClear(); + + const sessionShutdown = on.mock.calls.filter(([name]) => name === "session_shutdown").at(-1)?.[1] as ( + event: unknown, + ctx: unknown, + ) => void; + sessionShutdown({ type: "session_shutdown", reason: "quit" }, { setAutoRetryEnabled: vi.fn() }); + + // The next session owes its own prompt for a session-scoped grant. + await command.handler("full-access", commandContext(true, select)); + expect(select).toHaveBeenCalledOnce(); + }); + + test.each(["session_start", "session_shutdown"] as const)( + "ignores a stale risk-dialog answer after %s, including its remembered grant", + async (replacement) => { + const on = vi.fn(); + const registerCommand = vi.fn(); + const persist = vi.fn(); + createStepExtension({ + permission: { env: {}, initialPreset: "full-access" }, + stepSettings: () => ({ getStepSettings: () => ({}), setEffectiveStepSettings: persist }), + })({ on, registerCommand, registerProvider: vi.fn(), sendUserMessage: vi.fn() } as unknown as ExtensionAPI); + const start = on.mock.calls.filter(([name]) => name === "session_start").at(-1)![1]; + const shutdown = on.mock.calls.filter(([name]) => name === "session_shutdown").at(-1)![1]; + let answer!: (value: string) => void; + const oldStatus = vi.fn(); + start( + {}, + { + hasUI: true, + ui: { + notify: vi.fn(), + setStatus: oldStatus, + select: () => + new Promise((resolve) => { + answer = resolve; + }), + }, + }, + ); + if (replacement === "session_start") { + start({}, { hasUI: false, ui: { notify: vi.fn(), setStatus: vi.fn() } }); + } else { + shutdown({}, {}); + } + answer("Yes, and don't ask again — enable full access and remember this choice"); + await Promise.resolve(); + await Promise.resolve(); + expect(persist).not.toHaveBeenCalled(); + expect(oldStatus).not.toHaveBeenCalledWith("step-permission", "Mode: Full Access (auto-resume)"); + // A stale answer must not even cache the acknowledgment for a later command. + const command = registerCommand.mock.calls.find(([name]) => name === "permissions")![1]; + const select = vi.fn(async () => undefined); + await command.handler("full-access", commandContext(true, select)); + expect(select).toHaveBeenCalledOnce(); + }, + ); + + test("remembering Full Access through the cycle persists both the tier and acknowledgment", async () => { + const registerCommand = vi.fn(); + const persist = vi.fn(); + createStepExtension({ + permission: { env: {} }, + stepSettings: () => ({ getStepSettings: () => ({}), setEffectiveStepSettings: persist }), + })({ + on: vi.fn(), + registerCommand, + registerProvider: vi.fn(), + sendUserMessage: vi.fn(), + } as unknown as ExtensionAPI); + const command = registerCommand.mock.calls.find(([name]) => name === "permissions")![1]; + await command.handler( + "--cycle", + commandContext(true, async () => "Yes, and don't ask again — enable full access and remember this choice"), + ); + expect(persist).toHaveBeenCalledWith({ + permissionPreset: "full-access", + approvalMode: "auto", + nonInteractiveApproval: "allow", + autoResume: true, + fullAccessAcknowledged: true, + }); + }); + + test.each([false, true])( + "preserves startup unattended denial when Full Access consent is granted=%s", + async (granted) => { + const on = vi.fn(); + createStepExtension({ + permission: { env: {}, initialPreset: "full-access", nonInteractiveApproval: "deny" }, + })({ + on, + registerCommand: vi.fn(), + registerProvider: vi.fn(), + sendUserMessage: vi.fn(), + } as unknown as ExtensionAPI); + const start = on.mock.calls.filter(([name]) => name === "session_start").at(-1)![1]; + const status = vi.fn(); + start( + {}, + { + hasUI: granted, + ui: { + notify: vi.fn(), + setStatus: status, + select: async () => "Yes, continue anyway — apply full access for this session", + }, + }, + ); + if (granted) + await vi.waitFor(() => expect(status).toHaveBeenCalledWith("step-permission", "Mode: Full Access")); + const tool = on.mock.calls.find(([name]) => name === "tool_call")![1]; + expect( + await tool({ toolName: "write_file", input: { path: "file", content: "x" } }, { hasUI: false }), + ).toMatchObject({ block: true, terminate: true }); + }, + ); + + test.each(["option", "environment"] as const)("keeps remembered Full Access with %s auto-resume", (source) => { + const on = vi.fn(); + createStepExtension({ + permission: source === "option" ? { env: {}, autoResume: true } : { env: { STEP_AUTO_RESUME: "1" } }, + stepSettings: () => ({ + getStepSettings: () => ({ permissionPreset: "full-access", fullAccessAcknowledged: true }), + setEffectiveStepSettings: vi.fn(), + }), + })({ + on, + registerCommand: vi.fn(), + registerProvider: vi.fn(), + sendUserMessage: vi.fn(), + } as unknown as ExtensionAPI); + const start = on.mock.calls.filter(([name]) => name === "session_start").at(-1)![1]; + const status = vi.fn(); + start({}, { hasUI: false, ui: { notify: vi.fn(), setStatus: status } }); + expect(status).toHaveBeenCalledWith("step-permission", "Mode: Full Access (auto-resume)"); }); test("keeps explicit environment policy ahead of persisted Step settings", async () => { @@ -276,7 +667,7 @@ describe("Step extension", () => { const on = vi.fn(); const settings = { getStepSettings: () => ({ - permissionPreset: "autopilot" as const, + permissionPreset: "approve-for-me" as const, approvalMode: "auto" as const, nonInteractiveApproval: "allow" as const, autoResume: true, @@ -302,14 +693,14 @@ describe("Step extension", () => { } }); - test("keeps an explicit false auto-resume value ahead of persisted autopilot", () => { + test("keeps an explicit false auto-resume value ahead of persisted auto-resume", () => { const on = vi.fn(); const setStatus = vi.fn(); createStepExtension({ permission: { autoResume: false }, stepSettings: () => ({ getStepSettings: () => ({ - permissionPreset: "autopilot", + permissionPreset: "approve-for-me", approvalMode: "auto", nonInteractiveApproval: "allow", autoResume: true, @@ -335,12 +726,12 @@ describe("Step extension", () => { setAutoRetryEnabled: vi.fn(), }, ); - expect(setStatus).toHaveBeenCalledWith("step-permission", "Mode: Bypass"); + expect(setStatus).toHaveBeenCalledWith("step-permission", "Mode: Approve for Me"); }); test("rehydrates project policy after the trust probe", async () => { const on = vi.fn(); - let persisted: "ask" | "autopilot" = "ask"; + let persisted: "ask" | "approve-for-me" = "ask"; const settings = { getStepSettings: () => persisted === "ask" @@ -351,7 +742,7 @@ describe("Step extension", () => { autoResume: false, } : { - permissionPreset: "autopilot" as const, + permissionPreset: "approve-for-me" as const, approvalMode: "auto" as const, nonInteractiveApproval: "allow" as const, autoResume: true, @@ -381,7 +772,7 @@ describe("Step extension", () => { block: true, terminate: true, }); - persisted = "autopilot"; + persisted = "approve-for-me"; sessionStart( { type: "session_start", reason: "new" }, { diff --git a/packages/coding-agent/test/step-permissions.test.ts b/packages/coding-agent/test/step-permissions.test.ts index 10565500..42c25373 100644 --- a/packages/coding-agent/test/step-permissions.test.ts +++ b/packages/coding-agent/test/step-permissions.test.ts @@ -14,14 +14,14 @@ import { } from "../src/step/permissions.ts"; describe("Step permission presets", () => { - it("exposes the four stable presets in cycle order", () => { - expect(STEP_PERMISSION_PRESETS.map((preset) => preset.id)).toEqual(["ask", "read-only", "bypass", "autopilot"]); + it("exposes the three stable presets in cycle order", () => { + expect(STEP_PERMISSION_PRESETS.map((preset) => preset.id)).toEqual(["ask", "approve-for-me", "full-access"]); const controller = new StepPermissionController({ env: {} }); - expect(controller.getState().preset).toBe("bypass"); - expect(controller.cycle().preset).toBe("autopilot"); + expect(controller.getState().preset).toBe("approve-for-me"); + expect(controller.cycle().preset).toBe("full-access"); expect(controller.cycle().preset).toBe("ask"); - expect(controller.cycle().preset).toBe("read-only"); - expect(controller.cycle().preset).toBe("bypass"); + expect(controller.cycle().preset).toBe("approve-for-me"); + expect(controller.nextPresetId()).toBe("full-access"); expect( new StepPermissionController({ env: { STEP_PERMISSION_PRESET: "confirm" }, @@ -31,7 +31,20 @@ describe("Step permission presets", () => { new StepPermissionController({ env: { STEP_PERMISSION_PRESET: "strict" }, }).getState().preset, - ).toBe("read-only"); + ).toBe("ask"); + }); + + it("normalizes retired preset ids onto the remaining tiers", () => { + // Every retired tier returns to Ask until the user chooses a new tier. + for (const legacy of ["bypass", "auto", "autopilot", "bypasspermissions"]) { + expect(resolveInitialStepPermissionPreset({ env: { STEP_PERMISSION_PRESET: legacy } })).toBe("ask"); + } + for (const legacy of ["read-only", "readonly", "strict"]) { + expect(resolveInitialStepPermissionPreset({ env: { STEP_PERMISSION_PRESET: legacy } })).toBe("ask"); + } + for (const alias of ["full-access", "fullaccess", "full", "yolo", "never-ask"]) { + expect(resolveInitialStepPermissionPreset({ env: { STEP_PERMISSION_PRESET: alias } })).toBe("full-access"); + } }); it("resolves explicit preset and autopilot environment settings", () => { @@ -39,13 +52,86 @@ describe("Step permission presets", () => { resolveInitialStepPermissionPreset({ env: { STEP_PERMISSION_PRESET: "read-only" }, }), - ).toBe("read-only"); + ).toBe("ask"); expect( resolveInitialStepPermissionPreset({ env: { STEP_PERMISSION_MODE: "auto" }, }), - ).toBe("bypass"); - expect(resolveInitialStepPermissionPreset({ env: { STEP_AUTOPILOT: "1" } })).toBe("autopilot"); + ).toBe("ask"); + expect(resolveInitialStepPermissionPreset({ env: { STEP_AUTOPILOT: "1" } })).toBe("ask"); + }); + + it("resets legacy autopilot to Ask and enables auto-resume for newly selected automatic tiers", () => { + expect( + resolveInitialStepPermissionState({ + env: { STEP_PERMISSION_PRESET: "autopilot" }, + }), + ).toMatchObject({ preset: "ask", mode: "confirm", autoResume: false }); + expect(resolveInitialStepPermissionState({ env: { STEP_AUTOPILOT: "1" } })).toMatchObject({ + preset: "ask", + autoResume: false, + }); + expect(resolveInitialStepPermissionState({ env: { STEP_AUTO_RESUME: "1" } })).toMatchObject({ + preset: "approve-for-me", + autoResume: true, + }); + }); + + it("keeps Full Access labeled Full Access when auto-resume or an unattended deny is also set", () => { + // Approve for Me and Full Access share the (mode, approval) pair, so the + // relabeling must not rename a Full Access state that also requests + // auto-resume or refuses unattended approvals. + for (const env of [ + { STEP_PERMISSION_PRESET: "full-access", STEP_AUTO_RESUME: "1" }, + { STEP_PERMISSION_PRESET: "full-access", STEP_NON_INTERACTIVE_APPROVAL: "deny" }, + ]) { + const state = resolveInitialStepPermissionState({ env }); + expect(state).toMatchObject({ preset: "full-access", skipCommandPolicy: true }); + expect(decideStepToolCall("run_command", { command: "rm -rf /etc" }, state).action).toBe("allow"); + } + expect( + resolveInitialStepPermissionState({ + initialPreset: "full-access", + autoResume: true, + env: {}, + }), + ).toMatchObject({ preset: "full-access", skipCommandPolicy: true, autoResume: true }); + }); + + it("maps the retired plan vocabulary onto Ask", () => { + expect(resolveInitialStepPermissionPreset({ env: { STEP_PERMISSION_PRESET: "plan" } })).toBe("ask"); + expect(resolveInitialStepPermissionPreset({ env: { STEP_PERMISSION_MODE: "plan" } })).toBe("ask"); + expect(resolveInitialStepPermissionState({ env: { STEP_APPROVAL_MODE: "acceptedits" } })).toMatchObject({ + preset: "ask", + mode: "confirm", + }); + }); + + it("does not grant unattended writes just because STEP_AUTO_RESUME is enabled", async () => { + const controller = new StepPermissionController({ env: { STEP_AUTO_RESUME: "1" } }); + expect(controller.getState().defaulted).toBe(true); + expect( + await controller.handleToolCall( + { + type: "tool_call", + toolName: "write_file", + toolCallId: "resume-only", + input: { path: "x", content: "x" }, + }, + { hasUI: false } as ExtensionContext, + ), + ).toMatchObject({ block: true, terminate: true }); + }); + + it("uses automatic tier defaults on deliberate selection after a legacy disabled auto-resume", () => { + const controller = new StepPermissionController({ + env: { STEP_PERMISSION_PRESET: "autopilot" }, + autoResume: false, + }); + expect(controller.getState().autoResume).toBe(false); + expect(controller.setPreset("full-access")?.autoResume).toBe(true); + const legacy = new StepPermissionController({ env: { STEP_PERMISSION_PRESET: "autopilot" } }); + expect(legacy.setPreset("full-access")?.autoResume).toBe(true); }); it("resolves the old Step approval triple and per-tool overrides", () => { @@ -57,7 +143,7 @@ describe("Step permission presets", () => { toolOverrides: { write_file: "deny" }, }), ).toMatchObject({ - preset: "autopilot", + preset: "approve-for-me", mode: "auto", nonInteractiveApproval: "allow", autoResume: true, @@ -77,7 +163,7 @@ describe("Step permission presets", () => { STEP_PERMISSION_MODE: "strict", }, }), - ).toMatchObject({ preset: "autopilot", mode: "auto", autoResume: true }); + ).toMatchObject({ preset: "ask", mode: "confirm", autoResume: false }); expect( resolveInitialStepPermissionState({ env: { @@ -85,21 +171,20 @@ describe("Step permission presets", () => { STEP_PERMISSION_PRESET: "autopilot", }, }), - ).toMatchObject({ preset: "read-only", mode: "strict", autoResume: false }); + ).toMatchObject({ preset: "ask", mode: "confirm", autoResume: false }); }); - it("allows read tools, confirms writes, and blocks writes in read-only mode", () => { + it("allows read tools and confirms writes under Ask", () => { const ask = stepPermissionStateForPreset("ask"); expect(decideStepToolCall("read_file", { path: "a.txt" }, ask).action).toBe("allow"); expect(decideStepToolCall("search_web", { query: "current news" }, ask).action).toBe("allow"); expect(decideStepToolCall("write_file", { path: "a.txt", content: "x" }, ask).action).toBe("confirm"); - const readOnly = stepPermissionStateForPreset("read-only"); - expect(decideStepToolCall("search_web", { query: "current news" }, readOnly).action).toBe("allow"); - expect(decideStepToolCall("write_file", { path: "a.txt", content: "x" }, readOnly).action).toBe("deny"); + const approveForMe = stepPermissionStateForPreset("approve-for-me"); + expect(decideStepToolCall("write_file", { path: "a.txt", content: "x" }, approveForMe).action).toBe("allow"); }); - it("keeps dangerous commands behind confirmation even in bypass/autopilot", () => { - for (const preset of ["bypass", "autopilot"] as const) { + it("keeps dangerous commands behind confirmation except under Full Access", () => { + for (const preset of ["ask", "approve-for-me"] as const) { const decision = decideStepToolCall( "run_command", { command: "rm -rf /etc" }, @@ -111,16 +196,42 @@ describe("Step permission presets", () => { const fused = decideStepToolCall( "write_file", { path: "a.txt", content: "x", then_run: "rm -rf /etc" }, - stepPermissionStateForPreset("bypass"), + stepPermissionStateForPreset("approve-for-me"), ); expect(fused.action).toBe("confirm"); expect(fused.hazardous).toBe(true); expect(fused.reason).toContain("then_run: rm -rf /etc"); + // Full Access is the only tier that runs detected dangerous commands, and + // the only behavioral difference from Approve for Me. + const fullAccess = stepPermissionStateForPreset("full-access"); + expect(fullAccess.skipCommandPolicy).toBe(true); + expect(decideStepToolCall("run_command", { command: "rm -rf /etc" }, fullAccess).action).toBe("allow"); + expect( + decideStepToolCall("write_file", { path: "a.txt", content: "x", then_run: "rm -rf /etc" }, fullAccess).action, + ).toBe("allow"); expect(isDangerousCommand("sudo -n reboot")).toBe(true); expect(isDangerousCommand("qemu-system-x86_64 -no-reboot -no-shutdown")).toBe(false); expect(containsDangerousLifecycleCommand("sh -c 'systemctl reboot'")).toBe(true); }); + it("keeps explicit tool denials and unresolved analysis distinct under Full Access", () => { + const fullAccess = stepPermissionStateForPreset("full-access"); + // An explicit per-tool deny is a user-configured block, not a confirmation. + expect( + decideStepToolCall("run_command", { command: "rm -rf /etc" }, fullAccess, { run_command: "deny" }).action, + ).toBe("deny"); + // Incomplete analysis runs too: Full Access asks nothing. + expect(decideStepToolCall("run_command", { command: '"$HOME/x" sync' }, fullAccess).action).toBe("allow"); + // Approve for Me still confirms the same unanalyzable command. + expect( + decideStepToolCall( + "run_command", + { command: '"$HOME/x" sync' }, + stepPermissionStateForPreset("approve-for-me"), + ).action, + ).toBe("confirm"); + }); + it.each([true, false])("identifies each approval without changing the decision (approved=%s)", async (approved) => { const controller = new StepPermissionController({ env: {}, @@ -163,7 +274,7 @@ describe("Step permission presets", () => { it("keeps the hazard warning in an identifiable approval prompt", async () => { const controller = new StepPermissionController({ env: {}, - initialPreset: "bypass", + initialPreset: "approve-for-me", }); const confirm = vi.fn(async () => false); const context = { @@ -284,19 +395,8 @@ describe("Step permission presets", () => { }); it("keeps explicit denials terminating even with continue", async () => { - const readOnly = new StepPermissionController({ - initialPreset: "read-only", - nonInteractiveDenial: "continue", - env: {}, - }); - expect( - await readOnly.handleToolCall( - { toolName: "write_file", input: { path: "x", content: "y" } } as never, - noUI, - ), - ).toMatchObject({ block: true, terminate: true }); const overridden = new StepPermissionController({ - initialPreset: "bypass", + initialPreset: "approve-for-me", nonInteractiveDenial: "continue", toolOverrides: { run_command: "deny" }, env: {}, @@ -354,7 +454,7 @@ describe("Step permission presets", () => { it("marks an unselected policy as defaulted", () => { const state = resolveInitialStepPermissionState({ env: {} }); - expect(state.preset).toBe("bypass"); + expect(state.preset).toBe("approve-for-me"); expect(state.defaulted).toBe(true); }); @@ -374,7 +474,7 @@ describe("Step permission presets", () => { expect(result).toBeUndefined(); }); - it("keeps the interactive Bypass default when a UI exists", async () => { + it("keeps the interactive automatic default when a UI exists", async () => { const controller = new StepPermissionController({ env: {} }); const confirm = vi.fn(async () => true); const context = { @@ -386,18 +486,27 @@ describe("Step permission presets", () => { expect(confirm).not.toHaveBeenCalled(); }); - it("honors an explicit STEP_PERMISSION_PRESET=bypass", async () => { + it("honors an explicit STEP_PERMISSION_PRESET=approve-for-me", async () => { const controller = new StepPermissionController({ - env: { STEP_PERMISSION_PRESET: "bypass" }, + env: { STEP_PERMISSION_PRESET: "approve-for-me" }, }); expect(controller.getState().defaulted).toBeUndefined(); expect(await controller.handleToolCall(writeCall, noUI)).toBeUndefined(); }); + it("requires a new selection for legacy STEP_PERMISSION_PRESET=bypass", async () => { + const controller = new StepPermissionController({ + env: { STEP_PERMISSION_PRESET: "bypass" }, + }); + expect(controller.getState().preset).toBe("ask"); + expect(controller.getState().defaulted).toBeUndefined(); + expect(await controller.handleToolCall(writeCall, noUI)).toMatchObject({ block: true }); + }); + it("honors a trusted project preset injected as initialPreset", async () => { const controller = new StepPermissionController({ env: {}, - initialPreset: "bypass", + initialPreset: "approve-for-me", }); expect(controller.getState().defaulted).toBeUndefined(); expect(await controller.handleToolCall(writeCall, noUI)).toBeUndefined(); @@ -415,7 +524,7 @@ describe("Step permission presets", () => { // An explicit `--non-interactive-approval deny` used to be worse than // passing nothing: it cleared the defaulted marker and the mode stayed // `auto`, where the fallback is never consulted, so the call ran. - it("honors an explicit non-interactive deny under the bypass default", async () => { + it("honors an explicit non-interactive deny under the automatic default", async () => { const controller = new StepPermissionController({ env: {}, nonInteractiveApproval: "deny", @@ -424,10 +533,10 @@ describe("Step permission presets", () => { expect(result).toMatchObject({ block: true, terminate: true }); }); - it("keeps an explicit deny blocking even with an explicit bypass preset", async () => { + it("keeps an explicit deny blocking even with an explicit approve-for-me preset", async () => { const controller = new StepPermissionController({ env: {}, - initialPreset: "bypass", + initialPreset: "approve-for-me", nonInteractiveApproval: "deny", }); expect(await controller.handleToolCall(writeCall, noUI)).toMatchObject({ @@ -443,13 +552,13 @@ describe("Step permission presets", () => { expect(await controller.handleToolCall(writeCall, noUI)).toBeUndefined(); }); - it("keeps read-only mode denying with its own reason", async () => { + it("maps the retired strict approval mode onto Ask", async () => { const controller = new StepPermissionController({ - env: {}, - approvalMode: "strict", + env: { STEP_APPROVAL_MODE: "strict" }, }); + expect(controller.getState()).toMatchObject({ preset: "ask", mode: "confirm" }); const result = await controller.handleToolCall(writeCall, noUI); - expect((result as { reason: string }).reason).toContain("Read-only mode blocks"); + expect(result).toMatchObject({ block: true, terminate: true }); }); // A block that ends the run is the only thing a non-interactive caller sees, @@ -495,9 +604,25 @@ describe("Step permission presets", () => { it("drops the defaulted marker once a preset is chosen", () => { const controller = new StepPermissionController({ env: {} }); expect(controller.getState().defaulted).toBe(true); - controller.setPreset("bypass"); + controller.setPreset("approve-for-me"); expect(controller.getState().defaulted).toBeUndefined(); }); + + it("enables auto-resume in both automatic tiers without an environment flag", () => { + const controller = new StepPermissionController({ env: {}, initialPreset: "approve-for-me" }); + expect(controller.getState()).toMatchObject({ preset: "approve-for-me", autoResume: true }); + // Ask makes the flag inert but must not forget the request. + expect(controller.setPreset("ask")).toMatchObject({ preset: "ask", autoResume: false }); + expect(controller.setPreset("approve-for-me")).toMatchObject({ + preset: "approve-for-me", + autoResume: true, + }); + expect(controller.setPreset("full-access")).toMatchObject({ + preset: "full-access", + skipCommandPolicy: true, + autoResume: true, + }); + }); }); }); @@ -618,11 +743,11 @@ describe("Step autopilot continuation", () => { }); it("gates edit_file then_run as an embedded run_command call", () => { - const bypass = stepPermissionStateForPreset("bypass"); + const approveForMe = stepPermissionStateForPreset("approve-for-me"); const input = { path: "a.txt", search: "a", replace: "b", then_run: "npm test" }; - expect(decideStepToolCall("edit_file", input, bypass).action).toBe("allow"); + expect(decideStepToolCall("edit_file", input, approveForMe).action).toBe("allow"); - const denied = decideStepToolCall("edit_file", input, bypass, { run_command: "deny" }); + const denied = decideStepToolCall("edit_file", input, approveForMe, { run_command: "deny" }); expect(denied.action).toBe("deny"); expect(denied.reason).toContain("then_run (run_command)"); @@ -630,9 +755,13 @@ describe("Step autopilot continuation", () => { expect(ask.action).toBe("confirm"); expect(ask.reason).toContain("then_run: npm test"); - const readOnly = stepPermissionStateForPreset("read-only"); - expect(decideStepToolCall("edit_file", input, readOnly)).toEqual( - decideStepToolCall("edit_file", { path: "a.txt", search: "a", replace: "b" }, readOnly), - ); + // Full Access runs the fused call, dangerous verification command included. + expect( + decideStepToolCall( + "edit_file", + { ...input, then_run: "rm -rf ./build" }, + stepPermissionStateForPreset("full-access"), + ).action, + ).toBe("allow"); }); }); diff --git a/packages/coding-agent/test/step-settings-manager.test.ts b/packages/coding-agent/test/step-settings-manager.test.ts index 57c9ca5f..065b014d 100644 --- a/packages/coding-agent/test/step-settings-manager.test.ts +++ b/packages/coding-agent/test/step-settings-manager.test.ts @@ -72,39 +72,39 @@ describe("Step settings manager decorator", () => { const { manager, fixture } = makeManager(); manager.setStepSettings({ permissionPreset: "ask", autoResume: false }); - manager.setProjectStepSettings({ permissionPreset: "autopilot" }); + manager.setProjectStepSettings({ permissionPreset: "approve-for-me" }); await manager.flush(); expect(manager.getStepGlobalSettings()).toEqual({ permissionPreset: "ask", autoResume: false }); - expect(manager.getStepProjectSettings()).toEqual({ permissionPreset: "autopilot" }); - expect(manager.getStepSettings()).toEqual({ permissionPreset: "autopilot", autoResume: false }); + expect(manager.getStepProjectSettings()).toEqual({ permissionPreset: "approve-for-me" }); + expect(manager.getStepSettings()).toEqual({ permissionPreset: "approve-for-me", autoResume: false }); expect(JSON.parse(readFileSync(fixture.paths.global, "utf8"))).toEqual({ permissionPreset: "ask", autoResume: false, }); - expect(JSON.parse(readFileSync(fixture.paths.project, "utf8"))).toEqual({ permissionPreset: "autopilot" }); + expect(JSON.parse(readFileSync(fixture.paths.project, "utf8"))).toEqual({ permissionPreset: "approve-for-me" }); expect(existsSync(join(fixture.projectDir, ".pi", "step-settings.json"))).toBe(false); }); it("writes effective fields back to the scope currently overriding them", () => { const { manager, fixture } = makeManager(); manager.setStepSettings({ permissionPreset: "ask", autoResume: false }); - manager.setProjectStepSettings({ permissionPreset: "bypass" }); + manager.setProjectStepSettings({ permissionPreset: "approve-for-me" }); - manager.setEffectiveStepSettings({ permissionPreset: "autopilot", autoResume: true }); + manager.setEffectiveStepSettings({ permissionPreset: "full-access", autoResume: true }); expect(JSON.parse(readFileSync(fixture.paths.global, "utf8"))).toEqual({ permissionPreset: "ask", autoResume: true, }); expect(JSON.parse(readFileSync(fixture.paths.project, "utf8"))).toEqual({ - permissionPreset: "autopilot", + permissionPreset: "full-access", }); }); it("honors project trust for the Step sidecar just like Pi", () => { const fixture = makeRoot(); - writeFileSync(fixture.paths.project, JSON.stringify({ permissionPreset: "autopilot" })); + writeFileSync(fixture.paths.project, JSON.stringify({ permissionPreset: "approve-for-me" })); const { manager } = makeManager(fixture, false); expect(manager.getStepProjectSettings()).toEqual({}); @@ -114,10 +114,10 @@ describe("Step settings manager decorator", () => { ); manager.setProjectTrusted(true); - expect(manager.getStepPermissionPreset()).toBe("autopilot"); + expect(manager.getStepPermissionPreset()).toBe("approve-for-me"); }); - it("normalizes legacy aliases and supports clearing a persisted field", () => { + it("resets the legacy approval triple instead of inheriting unattended access", () => { const fixture = makeRoot(); writeFileSync( fixture.paths.global, @@ -126,14 +126,142 @@ describe("Step settings manager decorator", () => { const { manager } = makeManager(fixture); expect(manager.getStepSettings()).toEqual({ - permissionPreset: "bypass", - autoResume: true, - nonInteractiveApproval: "allow", + permissionPreset: "ask", + approvalMode: "confirm", + autoResume: false, + nonInteractiveApproval: "deny", }); manager.setStepAutoResume(undefined); const persisted = JSON.parse(readFileSync(fixture.paths.global, "utf8")); expect(persisted.autopilot).toBeUndefined(); - expect(manager.getStepAutoResume()).toBeUndefined(); + expect(manager.getStepAutoResume()).toBe(false); + }); + + it.each(["autopilot", "bypass", "read-only"])("resets the saved %s tier to Ask", (preset) => { + const fixture = makeRoot(); + writeFileSync( + fixture.paths.global, + JSON.stringify({ + permissionPreset: preset, + approvalMode: "auto", + nonInteractiveApproval: "allow", + autoResume: true, + }), + ); + const { manager } = makeManager(fixture); + + expect(manager.getStepSettings()).toEqual({ + permissionPreset: "ask", + approvalMode: "confirm", + nonInteractiveApproval: "deny", + autoResume: false, + }); + }); + + it("persists the Full Access risk acknowledgment in the sidecar", () => { + const fixture = makeRoot(); + writeFileSync(fixture.paths.global, JSON.stringify({ permissionPreset: "full-access" })); + const { manager } = makeManager(fixture); + expect(manager.getStepSettings().fullAccessAcknowledged).toBeUndefined(); + + manager.setStepSettings({ fullAccessAcknowledged: true }); + expect(manager.getStepSettings()).toEqual({ + permissionPreset: "full-access", + fullAccessAcknowledged: true, + }); + expect(JSON.parse(readFileSync(fixture.paths.global, "utf8"))).toMatchObject({ + fullAccessAcknowledged: true, + }); + + manager.setStepSettings({ fullAccessAcknowledged: false }); + expect(manager.getStepSettings().fullAccessAcknowledged).toBe(false); + // Clearing the acknowledgment keeps the selected tier. + expect(manager.getStepPermissionPreset()).toBe("full-access"); + }); + + it("remembers a newly chosen automatic tier after resetting old nested permission settings", () => { + const fixture = makeRoot(); + writeFileSync( + fixture.paths.global, + JSON.stringify({ + tools: { approval: { preset: "autopilot", mode: "auto", nonInteractive: "allow", autoResume: true } }, + }), + ); + const { manager } = makeManager(fixture); + expect(manager.getStepPermissionPreset()).toBe("ask"); + manager.setEffectiveStepSettings({ + permissionPreset: "approve-for-me", + approvalMode: "auto", + nonInteractiveApproval: "allow", + autoResume: true, + }); + const restarted = makeManager(fixture).manager; + expect(restarted.getStepSettings()).toEqual({ + permissionPreset: "approve-for-me", + approvalMode: "auto", + nonInteractiveApproval: "allow", + autoResume: true, + }); + }); + + it("retains the new selection when legacy policy fields were split across scopes", () => { + const fixture = makeRoot(); + writeFileSync( + fixture.paths.global, + JSON.stringify({ + permissionPreset: "bypass", + approvalMode: "auto", + nonInteractiveApproval: "allow", + autoResume: false, + }), + ); + writeFileSync(fixture.paths.project, JSON.stringify({ permissionPreset: "autopilot" })); + const { manager } = makeManager(fixture); + expect(manager.getStepSettings()).toEqual({ + permissionPreset: "ask", + approvalMode: "confirm", + nonInteractiveApproval: "deny", + autoResume: false, + }); + manager.setEffectiveStepSettings({ + permissionPreset: "approve-for-me", + approvalMode: "auto", + nonInteractiveApproval: "allow", + autoResume: true, + }); + expect(makeManager(fixture).manager.getStepSettings()).toEqual({ + permissionPreset: "approve-for-me", + approvalMode: "auto", + nonInteractiveApproval: "allow", + autoResume: true, + }); + }); + + it("reads the Full Access acknowledgment from the legacy approval object", () => { + const fixture = makeRoot(); + writeFileSync( + fixture.paths.global, + JSON.stringify({ approval: { preset: "full-access", fullAccessAcknowledged: true } }), + ); + const { manager } = makeManager(fixture); + + expect(manager.getStepSettings()).toEqual({ + permissionPreset: "full-access", + fullAccessAcknowledged: true, + }); + }); + + it("does not enable auto-resume from a losing legacy autopilot candidate", () => { + const fixture = makeRoot(); + // Within one scope the higher-priority candidate wins the preset; the + // losing legacy autopilot spelling must not switch auto-resume on. + writeFileSync( + fixture.paths.global, + JSON.stringify({ permissionPreset: "ask", approval: { preset: "autopilot" } }), + ); + const { manager } = makeManager(fixture); + + expect(manager.getStepSettings()).toEqual({ permissionPreset: "ask" }); }); it("reads and clears the feedback setting through the Step sidecar", () => { @@ -167,16 +295,16 @@ describe("Step settings manager decorator", () => { writeFileSync( fixture.paths.global, JSON.stringify({ - approval: { preset: "autopilot", mode: "auto", nonInteractive: "allow", autoResume: true }, + approval: { preset: "bypass", mode: "auto", nonInteractive: "allow", autoResume: true }, }), ); const { manager } = makeManager(fixture); expect(manager.getStepSettings()).toEqual({ - permissionPreset: "autopilot", - approvalMode: "auto", - nonInteractiveApproval: "allow", - autoResume: true, + permissionPreset: "ask", + approvalMode: "confirm", + nonInteractiveApproval: "deny", + autoResume: false, }); manager.setStepSettings({ permissionPreset: undefined, @@ -195,35 +323,38 @@ describe("Step settings manager decorator", () => { fixture.paths.global, JSON.stringify({ permissionPreset: "unknown", - permissionMode: "read-only", + permissionMode: "bypass", approvalMode: "unknown", - approval: { mode: "strict" }, + approval: { mode: "auto" }, }), ); const { manager } = makeManager(fixture); expect(manager.getStepSettings()).toEqual({ - permissionPreset: "read-only", - approvalMode: "strict", + permissionPreset: "ask", + approvalMode: "confirm", + nonInteractiveApproval: "deny", + autoResume: false, }); }); - it("deep-merges legacy approval objects across global and project scopes", () => { + it("does not reactivate a migrated global policy through partial project settings", () => { const fixture = makeRoot(); writeFileSync(fixture.paths.global, JSON.stringify({ approval: { mode: "auto", autoResume: true } })); writeFileSync(fixture.paths.project, JSON.stringify({ approval: { nonInteractive: "allow" } })); const { manager } = makeManager(fixture); expect(manager.getStepSettings()).toEqual({ - approvalMode: "auto", - nonInteractiveApproval: "allow", - autoResume: true, + permissionPreset: "ask", + approvalMode: "confirm", + nonInteractiveApproval: "deny", + autoResume: false, }); }); it("writes effective updates to a project scope selected through a nested legacy alias", () => { const fixture = makeRoot(); - writeFileSync(fixture.paths.project, JSON.stringify({ tools: { approval: { mode: "strict" } } })); + writeFileSync(fixture.paths.project, JSON.stringify({ tools: { approval: { mode: "auto" } } })); const { manager } = makeManager(fixture); manager.setEffectiveStepSettings({ approvalMode: "auto" }); @@ -273,11 +404,11 @@ describe("Step settings manager decorator", () => { expect(existsSync(fixture.paths.project)).toBe(false); expect(existsSync(join(fixture.projectDir, ".stepcode"))).toBe(false); - manager.setProjectStepSettings({ permissionPreset: "read-only" }); + manager.setProjectStepSettings({ permissionPreset: "full-access" }); expect(manager.drainErrors()).toEqual([]); - expect(readStepConfig(fixture.paths.project)).toEqual({ permissionPreset: "read-only" }); + expect(readStepConfig(fixture.paths.project)).toEqual({ permissionPreset: "full-access" }); await manager.reload(); - expect(manager.getStepSettings()).toEqual({ permissionPreset: "read-only" }); + expect(manager.getStepSettings()).toEqual({ permissionPreset: "full-access" }); expect(manager.drainErrors()).toEqual([]); }); diff --git a/packages/coding-agent/test/step-slash-commands.test.ts b/packages/coding-agent/test/step-slash-commands.test.ts index 3df0b8a2..b26886a0 100644 --- a/packages/coding-agent/test/step-slash-commands.test.ts +++ b/packages/coding-agent/test/step-slash-commands.test.ts @@ -113,7 +113,7 @@ describe("Step Pi slash command adapters", () => { test("registers product adapters without replacing Pi's native commands", () => { const { commands } = registerCommands(); - expect([...commands.keys()]).toEqual(expect.arrayContaining(["plugin", "clear", "exit", "theme", "status"])); + expect([...commands.keys()]).toEqual(expect.arrayContaining(["plugin", "clear", "theme", "status"])); expect(commands.has("effort")).toBe(false); }); @@ -146,29 +146,24 @@ describe("Step Pi slash command adapters", () => { { track }, ); - await getCommand(commands, "exit").handler("secret argument", { - shutdown: vi.fn(), + await getCommand(commands, "clear").handler("secret argument", { + newSession: vi.fn().mockResolvedValue({ cancelled: false }), } as never); expect(track).toHaveBeenCalledWith( "slash_command_used", { - command: "/exit", + command: "/clear", recognized: true, }, undefined, ); }); - test("routes /exit to Pi's graceful shutdown action", async () => { + test("leaves /exit to the built-in dispatcher without an extension command conflict", () => { const { commands } = registerCommands(); - const shutdown = vi.fn(); - - await getCommand(commands, "exit").handler("ignored arguments", { - shutdown, - } as unknown as ExtensionCommandContext); - - expect(shutdown).toHaveBeenCalledOnce(); + expect(BUILTIN_SLASH_COMMANDS.some((command) => command.name === "exit")).toBe(true); + expect(commands.has("exit")).toBe(false); }); test("uses Pi's UI selector and theme setter for /theme", async () => { diff --git a/packages/coding-agent/test/stepcode-config.test.ts b/packages/coding-agent/test/stepcode-config.test.ts index 6e2c2fd9..5cb1d79f 100644 --- a/packages/coding-agent/test/stepcode-config.test.ts +++ b/packages/coding-agent/test/stepcode-config.test.ts @@ -212,8 +212,9 @@ describe("StepCode config compatibility", () => { expect(manager.getDefaultProvider()).toBe("stepcode-anthropic"); expect(manager.getDefaultModel()).toBe("water18-new"); expect(manager.getStepSettings()).toEqual({ - approvalMode: "auto", - nonInteractiveApproval: "allow", + permissionPreset: "ask", + approvalMode: "confirm", + nonInteractiveApproval: "deny", autoResume: false, }); @@ -259,6 +260,99 @@ describe("StepCode config compatibility", () => { }); }); + test.each(["bypass", "autopilot", "read-only"])( + "resets legacy %s in an injected config before a new selection", + async (preset) => { + const root = await mkdtemp(join(tmpdir(), "stepcode-permission-migration-")); + roots.push(root); + const configPath = join(root, "config.json"); + await writeFile( + configPath, + JSON.stringify({ + providers: { + test: { api: "openai-completions", baseUrl: "https://example.invalid/v1", models: [{ id: "test" }] }, + }, + tools: { + approval: { + preset, + mode: "auto", + nonInteractive: "allow", + autoResume: true, + fullAccessAcknowledged: true, + }, + }, + }), + ); + const config = await loadStepCodeConfig({ STEPCODE_CONFIG_PATH: configPath }, root); + const manager = decorateStepCodeSettingsManager(createStepSettingsManager(root, join(root, "agent")), config!); + expect(manager.getStepSettings()).toEqual({ + permissionPreset: "ask", + approvalMode: "confirm", + nonInteractiveApproval: "deny", + autoResume: false, + }); + manager.setEffectiveStepSettings({ + permissionPreset: "full-access", + approvalMode: "auto", + nonInteractiveApproval: "allow", + autoResume: true, + fullAccessAcknowledged: true, + }); + const reloaded = await loadStepCodeConfig({ STEPCODE_CONFIG_PATH: configPath }, root); + const next = decorateStepCodeSettingsManager(createStepSettingsManager(root, join(root, "agent")), reloaded!); + expect(next.getStepSettings()).toEqual({ + permissionPreset: "full-access", + approvalMode: "auto", + nonInteractiveApproval: "allow", + autoResume: true, + fullAccessAcknowledged: true, + }); + }, + ); + + test("round-trips the Full Access risk acknowledgment through the injected config", async () => { + const root = await mkdtemp(join(tmpdir(), "stepcode-config-")); + roots.push(root); + const projectDir = join(root, "project"); + const agentDir = join(root, "agent"); + await Promise.all([mkdir(projectDir, { recursive: true }), mkdir(agentDir, { recursive: true })]); + const configPath = join(root, "config.json"); + await writeFile( + configPath, + JSON.stringify({ + providers: { + "stepcode-anthropic": { + api: "anthropic-messages", + baseUrl: "https://config.example/v1", + models: [{ id: "water18" }], + }, + }, + activeModel: "water18", + tools: { approval: { preset: "full-access", fullAccessAcknowledged: true } }, + }), + ); + + const config = await loadStepCodeConfig({ STEPCODE_CONFIG_PATH: configPath }, root); + const manager = decorateStepCodeSettingsManager(createStepSettingsManager(projectDir, agentDir), config!); + expect(manager.getStepSettings()).toEqual({ + permissionPreset: "full-access", + fullAccessAcknowledged: true, + }); + + manager.setEffectiveStepSettings({ fullAccessAcknowledged: false }); + const saved = JSON.parse(readFileSync(configPath, "utf8")) as { + tools: { approval: Record }; + }; + expect(saved.tools.approval).toMatchObject({ fullAccessAcknowledged: false }); + + const restarted = await loadStepCodeConfig({ STEPCODE_CONFIG_PATH: configPath }, root); + const restartedManager = decorateStepCodeSettingsManager( + createStepSettingsManager(projectDir, agentDir), + restarted!, + ); + expect(restartedManager.getStepSettings().fullAccessAcknowledged).toBe(false); + }); + test("defaults omitted vision metadata to image input and honors an explicit opt-out", async () => { const root = await mkdtemp(join(tmpdir(), "stepcode-config-")); roots.push(root); diff --git a/packages/coding-agent/test/suite/agent-session-tool-output.test.ts b/packages/coding-agent/test/suite/agent-session-tool-output.test.ts index c8568a67..10d6d049 100644 --- a/packages/coding-agent/test/suite/agent-session-tool-output.test.ts +++ b/packages/coding-agent/test/suite/agent-session-tool-output.test.ts @@ -247,7 +247,7 @@ describe("AgentSession final tool output", () => { settings: { compaction: { enabled: false } }, extensionFactories: [ createStepExtension({ - permission: { env: {}, initialPreset: "bypass", toolOverrides: { run_command: "allow" } }, + permission: { env: {}, initialPreset: "approve-for-me", toolOverrides: { run_command: "allow" } }, }), (pi) => { const tool = createStepToolProfile(sandbox, { agentDir: join(sandbox, "agent") }).find( diff --git a/packages/coding-agent/test/suite/regressions/step-run-command-timeout.test.ts b/packages/coding-agent/test/suite/regressions/step-run-command-timeout.test.ts index c9f18217..0486ecbc 100644 --- a/packages/coding-agent/test/suite/regressions/step-run-command-timeout.test.ts +++ b/packages/coding-agent/test/suite/regressions/step-run-command-timeout.test.ts @@ -166,7 +166,7 @@ describe("Step run_command timeout recovery through the agent loop", () => { createStepExtension({ permission: { env: {}, - initialPreset: "bypass", + initialPreset: "approve-for-me", toolOverrides: { run_command: "deny" }, }, }), diff --git a/packages/coding-agent/test/suite/step-command-approval.test.ts b/packages/coding-agent/test/suite/step-command-approval.test.ts index cc63659e..3ca7a38e 100644 --- a/packages/coding-agent/test/suite/step-command-approval.test.ts +++ b/packages/coding-agent/test/suite/step-command-approval.test.ts @@ -10,9 +10,8 @@ import { createHarness, getAssistantTexts, type Harness } from "./harness.ts"; const modes: Array<{ name: string; permission: StepPermissionControllerOptions }> = [ { name: "ask", permission: { initialPreset: "ask" } }, - { name: "bypass", permission: { initialPreset: "bypass" } }, + { name: "approve-for-me", permission: { initialPreset: "approve-for-me" } }, { name: "auto", permission: { approvalMode: "auto" } }, - { name: "autopilot", permission: { initialPreset: "autopilot" } }, ]; describe("Step command approval through the agent loop", () => { @@ -33,12 +32,18 @@ describe("Step command approval through the agent loop", () => { vi.unstubAllEnvs(); }); - async function setup(permission: StepPermissionControllerOptions): Promise { + async function setup( + permission: StepPermissionControllerOptions, + stepSettings?: () => { + getStepSettings: () => Record; + setEffectiveStepSettings: (settings: Record) => void; + }, + ): Promise { harness = await createHarness({ tools: [], initialActiveToolNames: ["run_command"], extensionFactories: [ - createStepExtension({ permission: { env: {}, ...permission } }), + createStepExtension({ permission: { env: {}, ...permission }, stepSettings }), (pi) => { const tool = createStepToolProfile(sandbox, { agentDir: join(sandbox, "agent") }).find( (candidate) => candidate.name === "run_command", @@ -113,6 +118,67 @@ describe("Step command approval through the agent loop", () => { }); }); + it("full access runs the real deletion unattended without prompting", async () => { + // The risk acknowledgment is what admits Full Access to a headless run; + // without it the session falls back to Approve for Me (see below). + const session = await setup( + { + initialPreset: "full-access", + nonInteractiveApproval: "allow", + toolOverrides: { run_command: "allow" }, + }, + () => ({ + getStepSettings: () => ({ fullAccessAcknowledged: true }), + setEffectiveStepSettings: vi.fn(), + }), + ); + await session.session.bindExtensions({ mode: "print" }); + const marker = prepareRemoval(session, "full-access-removal"); + await session.session.prompt("Remove the test directory"); + expect(existsSync(marker)).toBe(false); + expect(session.session.messages.find((message) => message.role === "toolResult")).toMatchObject({ + isError: false, + }); + }); + + it("full access falls back to Approve for Me in a headless run without the acknowledgment", async () => { + const session = await setup({ + initialPreset: "full-access", + nonInteractiveApproval: "allow", + toolOverrides: { run_command: "allow" }, + }); + await session.session.bindExtensions({ mode: "print" }); + const marker = prepareRemoval(session, "full-access-unacknowledged"); + await session.session.prompt("Remove the test directory"); + // Approve for Me still confirms the dangerous command, and no approval + // channel exists in print mode. + expect(existsSync(marker)).toBe(true); + expect(session.session.messages.find((message) => message.role === "toolResult")).toMatchObject({ + isError: true, + }); + }); + + it("full access still honors an explicit tool denial", async () => { + const session = await setup( + { + initialPreset: "full-access", + nonInteractiveApproval: "allow", + toolOverrides: { run_command: "deny" }, + }, + () => ({ + getStepSettings: () => ({ fullAccessAcknowledged: true }), + setEffectiveStepSettings: vi.fn(), + }), + ); + await session.session.bindExtensions({ mode: "print" }); + const marker = prepareRemoval(session, "full-access-denied"); + await session.session.prompt("Remove the test directory"); + expect(existsSync(marker)).toBe(true); + expect(session.session.messages.find((message) => message.role === "toolResult")).toMatchObject({ + isError: true, + }); + }); + it("continues past a refused unattended deletion when denial recovery is enabled", async () => { const session = await setup({ approvalMode: "auto", @@ -133,25 +199,22 @@ describe("Step command approval through the agent loop", () => { expect(getAssistantTexts(session)).toContain("done"); }); - it.each([ - { name: "read-only", permission: { initialPreset: "read-only" } }, - { name: "explicit deny", permission: { initialPreset: "bypass", toolOverrides: { run_command: "deny" } } }, - ] satisfies Array<{ name: string; permission: StepPermissionControllerOptions }>)( - "$name blocks the real tool without prompting", - async ({ permission }) => { - const session = await setup(permission); - const confirm = vi.fn(async () => true); - await session.session.bindExtensions({ - mode: "tui", - uiContext: { ...session.session.extensionRunner.getUIContext(), confirm }, - }); - const marker = prepareRemoval(session, "denied-removal"); - await session.session.prompt("Remove the test directory"); - expect(existsSync(marker)).toBe(true); - expect(confirm).not.toHaveBeenCalled(); - expect(session.session.messages.find((message) => message.role === "toolResult")).toMatchObject({ - isError: true, - }); - }, - ); + it("an explicit deny blocks the real tool without prompting", async () => { + const session = await setup({ + initialPreset: "approve-for-me", + toolOverrides: { run_command: "deny" }, + }); + const confirm = vi.fn(async () => true); + await session.session.bindExtensions({ + mode: "tui", + uiContext: { ...session.session.extensionRunner.getUIContext(), confirm }, + }); + const marker = prepareRemoval(session, "denied-removal"); + await session.session.prompt("Remove the test directory"); + expect(existsSync(marker)).toBe(true); + expect(confirm).not.toHaveBeenCalled(); + expect(session.session.messages.find((message) => message.role === "toolResult")).toMatchObject({ + isError: true, + }); + }); }); diff --git a/packages/coding-agent/test/suite/step-stream-recovery.test.ts b/packages/coding-agent/test/suite/step-stream-recovery.test.ts index 6479c41a..205efc5e 100644 --- a/packages/coding-agent/test/suite/step-stream-recovery.test.ts +++ b/packages/coding-agent/test/suite/step-stream-recovery.test.ts @@ -31,7 +31,7 @@ describe("Step incomplete stream recovery", () => { }; const harness = await createHarness({ tools: [writeTool], - extensionFactories: [createStepExtension({ permission: { initialPreset: "bypass" } })], + extensionFactories: [createStepExtension({ permission: { initialPreset: "approve-for-me" } })], settings: { retry: { enabled: true, maxRetries: 2, baseDelayMs: 1 } }, }); harnesses.push(harness);