From 6e66aa239209c734f36a4f163bec71a1b09f4c6c Mon Sep 17 00:00:00 2001 From: brooksc Date: Thu, 4 Jun 2026 14:06:05 -0700 Subject: [PATCH 1/2] feat(settings): add new task defaults for steps, skip-permissions, and propagate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three persistent app-level defaults that pre-fill checkboxes in the New Task dialog so users who habitually enable the same options do not have to re-tick them on every task. ## New settings (Settings → General → New Task Defaults) - defaultStepsEnabled — pre-checks Steps tracking in the New Task dialog - defaultSkipPermissions — pre-checks skip-permissions (when agent supports it) - defaultPropagateSkipPermissions — pre-checks propagate (coordinator + skip-perms only) ## Migration loadState migrates legacy showSteps → defaultStepsEnabled on first load. showSteps is no longer written by saveState. Invalid present values for defaultStepsEnabled do not fall back to the legacy field. ## Behavior notes - Task creation never mutates defaultStepsEnabled; only the Settings setter does. - propagateSkipPermissions is gated at submission by all three conditions (coordinatorMode && agentSupportsSkipPermissions && skipPermissions). - The open effect samples defaults inside untrack() on the open transition only. Co-Authored-By: Claude Sonnet 4.6 --- openspec/changes/new-task-defaults/change.md | 47 ++++ .../changes/new-task-defaults/proposal.md | 30 +++ .../specs/steps-tracking/spec.md | 57 +++++ openspec/changes/new-task-defaults/tasks.md | 22 ++ openspec/specs/steps-tracking/spec.md | 4 +- src/components/NewTaskDialog.tsx | 38 ++- src/components/SettingsDialog.tsx | 90 +++++++ src/store/autosave.test.ts | 36 +++ src/store/autosave.ts | 6 +- src/store/core.ts | 4 +- src/store/persistence.test.ts | 226 ++++++++++++++++++ src/store/persistence.ts | 20 +- src/store/store.ts | 3 + src/store/tasks.test.ts | 56 +++++ src/store/tasks.ts | 6 +- src/store/types.ts | 8 +- src/store/ui.ts | 12 + 17 files changed, 645 insertions(+), 20 deletions(-) create mode 100644 openspec/changes/new-task-defaults/change.md create mode 100644 openspec/changes/new-task-defaults/proposal.md create mode 100644 openspec/changes/new-task-defaults/specs/steps-tracking/spec.md create mode 100644 openspec/changes/new-task-defaults/tasks.md create mode 100644 src/store/autosave.test.ts diff --git a/openspec/changes/new-task-defaults/change.md b/openspec/changes/new-task-defaults/change.md new file mode 100644 index 00000000..b129962d --- /dev/null +++ b/openspec/changes/new-task-defaults/change.md @@ -0,0 +1,47 @@ +# Change: New Task Defaults for Steps, Skip-Permissions, and Propagate + +## Summary + +Add three persistent app-level defaults that pre-fill checkboxes in the New +Task dialog, so users who habitually enable the same options do not have to +re-tick them on every task. + +## Affected spec + +`openspec/specs/steps-tracking/spec.md` — the `showSteps` field referenced in +the "opt-in per task with remembered default" scenarios is renamed to +`defaultStepsEnabled`. Behaviour is unchanged; only the storage key differs. +Existing `showSteps` values are migrated to `defaultStepsEnabled` on first +load. + +## New behaviour + +### `defaultStepsEnabled` (replaces `showSteps`) + +- **WHEN** the user opens the new-task dialog +- **THEN** the "Steps tracking" checkbox is pre-checked iff `defaultStepsEnabled` is true +- **WHEN** the user toggles the Setting in Settings → General → New Task Defaults +- **THEN** `defaultStepsEnabled` is updated; task creation does not mutate this default + +### `defaultSkipPermissions` + +- **WHEN** the user opens the new-task dialog and the selected agent supports skip-permissions +- **THEN** the "Skip permissions" checkbox is pre-checked iff `defaultSkipPermissions` is true + +### `defaultPropagateSkipPermissions` + +- **WHEN** the user opens the new-task dialog with coordinator mode enabled and skip-permissions ticked +- **THEN** the "Propagate skip-permissions to sub-tasks" checkbox is pre-checked iff + `defaultPropagateSkipPermissions` is true + +## Settings surface + +All three defaults are exposed in **Settings → General → New Task Defaults** +(adjacent to the Behavior section). The propagate row is only shown when +`coordinatorModeEnabled` is true. + +## Migration + +`loadState` maps `raw.showSteps === true` → `defaultStepsEnabled = true` so +existing users who had steps tracking enabled keep their preference. +`showSteps` is no longer written by `saveState`. diff --git a/openspec/changes/new-task-defaults/proposal.md b/openspec/changes/new-task-defaults/proposal.md new file mode 100644 index 00000000..f5bf5cdd --- /dev/null +++ b/openspec/changes/new-task-defaults/proposal.md @@ -0,0 +1,30 @@ +# New Task Defaults for Steps, Skip-Permissions, and Propagate + +## Why + +Users who habitually enable the same options (steps tracking, skip-permissions, +propagate to sub-tasks) must re-tick them on every new task. There is no way to +set an app-level default so the dialog opens pre-checked. + +## What changes + +- Three new boolean settings in **Settings → General → New Task Defaults**: + - **Steps tracking** — pre-tick "Steps tracking" in the New Task dialog + - **Skip permissions** — pre-tick "Skip permissions" (only honoured when the + selected agent supports it) + - **Propagate skip-permissions to sub-tasks** — pre-tick propagate (shown + only when `coordinatorModeEnabled` is true) +- All three values are persisted and included in the autosave snapshot. +- The `showSteps` key is renamed to `defaultStepsEnabled`; existing users' + preferences are migrated transparently on first load. + +## Impact + +- Modifies `openspec/specs/steps-tracking/spec.md`: renames `showSteps` → + `defaultStepsEnabled` and documents the two new fields. +- Store: adds `defaultSkipPermissions`, `defaultPropagateSkipPermissions`; + renames `showSteps` → `defaultStepsEnabled` in `AppStore`, `PersistedState`, + `saveState`, and `persistedSnapshot`. +- New Task dialog: open effect initializes all three signals from store instead + of hardcoding `false`. +- No new IPC channels. diff --git a/openspec/changes/new-task-defaults/specs/steps-tracking/spec.md b/openspec/changes/new-task-defaults/specs/steps-tracking/spec.md new file mode 100644 index 00000000..f3d1788a --- /dev/null +++ b/openspec/changes/new-task-defaults/specs/steps-tracking/spec.md @@ -0,0 +1,57 @@ +## MODIFIED Requirements + +### Requirement: Opt-in per task with remembered default + +Steps tracking SHALL be opt-in on a per-task basis, with a persistent app-level +default that the new-task dialog uses to pre-fill the checkbox. The storage key +for this default is renamed from `showSteps` to `defaultStepsEnabled`; existing +`showSteps` values are migrated to `defaultStepsEnabled` on first load. + +#### Scenario: New task dialog reflects the persistent default + +- **WHEN** the user opens the new-task dialog +- **THEN** the "Steps tracking" checkbox is pre-checked if and only if the + persisted `defaultStepsEnabled` app-level flag is true + +#### Scenario: Default updates when user toggles the checkbox + +- **WHEN** the user creates a task with the checkbox in the opposite state +- **THEN** the persisted `defaultStepsEnabled` app-level flag is updated to + match +- **AND** the next new-task dialog uses that value as the default + +#### Scenario: Migration from legacy showSteps key + +- **WHEN** a user with `showSteps: true` in saved state opens the app after + this change +- **THEN** `defaultStepsEnabled` is set to true +- **AND** `showSteps` is no longer written by saveState + +## ADDED Requirements + +### Requirement: Persistent defaults for skip-permissions and propagate + +Two additional app-level defaults SHALL be persisted and used to pre-fill the +New Task dialog checkboxes. + +#### Scenario: Skip-permissions default pre-fills the dialog + +- **WHEN** the user opens the new-task dialog and the selected agent supports + skip-permissions +- **THEN** the "Skip permissions" checkbox is pre-checked iff + `defaultSkipPermissions` is true + +#### Scenario: Propagate default pre-fills the dialog + +- **WHEN** the user opens the new-task dialog with coordinator mode enabled and + skip-permissions ticked +- **THEN** the "Propagate skip-permissions to sub-tasks" checkbox is pre-checked + iff `defaultPropagateSkipPermissions` is true + +#### Scenario: Defaults are configurable in Settings + +- **WHEN** the user opens Settings → General +- **THEN** a "New Task Defaults" section is visible with toggles for all three + defaults +- **AND** the propagate toggle is shown only when `coordinatorModeEnabled` is + true diff --git a/openspec/changes/new-task-defaults/tasks.md b/openspec/changes/new-task-defaults/tasks.md new file mode 100644 index 00000000..665ca913 --- /dev/null +++ b/openspec/changes/new-task-defaults/tasks.md @@ -0,0 +1,22 @@ +# Tasks — New Task Defaults + +- [x] Add `defaultStepsEnabled`, `defaultSkipPermissions`, and + `defaultPropagateSkipPermissions` to `AppStore` and `PersistedState` in + `src/store/types.ts`; remove `showSteps` from `PersistedState`. +- [x] Add setters for all three fields in `src/store/ui.ts`. +- [x] Update `saveState` in `src/store/persistence.ts` to write the three new + fields and stop writing `showSteps`. +- [x] Add migration in `loadState`: use `raw.defaultStepsEnabled` when it is a boolean; fall back to `raw.showSteps === true` only when the new field is absent (not present-but-invalid). +- [x] Add all three fields to `persistedSnapshot()` in `src/store/autosave.ts` + and export the function for testing. +- [x] Fix New Task dialog open effect (`src/components/NewTaskDialog.tsx`) to + initialize all three signals from the store rather than hardcoding `false`. +- [x] Add the three Settings rows in `src/components/SettingsDialog.tsx` + (New Task Defaults section, placed after the Behavior group). +- [x] Add autosave regression tests (`src/store/autosave.test.ts`) using the + real `persistedSnapshot()`. +- [x] Add migration tests to `src/store/persistence.test.ts`. +- [x] Update `openspec/specs/steps-tracking/spec.md` to rename `showSteps` → + `defaultStepsEnabled` and document the two new fields. +- [x] Validate with `npm run typecheck`, `npm test`, and + `openspec validate --all --strict`. diff --git a/openspec/specs/steps-tracking/spec.md b/openspec/specs/steps-tracking/spec.md index 5fbf8735..ccf5b487 100644 --- a/openspec/specs/steps-tracking/spec.md +++ b/openspec/specs/steps-tracking/spec.md @@ -17,12 +17,12 @@ default that the new-task dialog uses to pre-fill the checkbox. - **WHEN** the user opens the new-task dialog - **THEN** the "Steps tracking" checkbox is pre-checked if and only if the - persisted `showSteps` app-level flag is true + persisted `defaultStepsEnabled` app-level flag is true #### Scenario: Default updates when user toggles the checkbox - **WHEN** the user creates a task with the checkbox in the opposite state -- **THEN** the persisted `showSteps` app-level flag is updated to match +- **THEN** the persisted `defaultStepsEnabled` app-level flag is updated to match - **AND** the next new-task dialog uses that value as the default #### Scenario: `stepsEnabled` is per-task and persisted diff --git a/src/components/NewTaskDialog.tsx b/src/components/NewTaskDialog.tsx index a2e85e2a..528ec0d3 100644 --- a/src/components/NewTaskDialog.tsx +++ b/src/components/NewTaskDialog.tsx @@ -6,6 +6,7 @@ import { Show, onCleanup, on, + untrack, } from 'solid-js'; import { Dialog } from './Dialog'; import { errMessage } from '../lib/log'; @@ -75,8 +76,8 @@ export function NewTaskDialog(props: NewTaskDialogProps) { const [branchesError, setBranchesError] = createSignal(false); // Bumped by the Retry button to re-run the branch-fetch effect. const [branchRetryToken, setBranchRetryToken] = createSignal(0); - const [stepsEnabled, setStepsEnabled] = createSignal(store.showSteps); - const [skipPermissions, setSkipPermissions] = createSignal(false); + const [stepsEnabled, setStepsEnabled] = createSignal(store.defaultStepsEnabled); + const [skipPermissions, setSkipPermissions] = createSignal(store.defaultSkipPermissions); const [dockerMode, setDockerMode] = createSignal(false); const [dockerImageReady, setDockerImageReady] = createSignal(null); // null = unknown const [dockerBuilding, setDockerBuilding] = createSignal(false); @@ -88,7 +89,9 @@ export function NewTaskDialog(props: NewTaskDialogProps) { buildContext: string; } | null>(null); const [coordinatorMode, setCoordinatorMode] = createSignal(false); - const [propagateSkipPermissions, setPropagateSkipPermissions] = createSignal(false); + const [propagateSkipPermissions, setPropagateSkipPermissions] = createSignal( + store.defaultPropagateSkipPermissions, + ); const [maxConcurrentTasks, setMaxConcurrentTasks] = createSignal( DEFAULT_COORDINATOR_CONCURRENT_TASKS, ); @@ -158,7 +161,27 @@ export function NewTaskDialog(props: NewTaskDialogProps) { focusables[nextIdx].focus(); } - // Initialize state each time the dialog opens + // Initialize state each time the dialog opens. Wrapped in on() so the + // effect only re-fires on the props.open *transition*, not whenever any + // store default mutates while the dialog is already open (e.g. the user + // toggling Settings, or autosave restoring state). untrack() ensures the + // store reads inside are one-shot samples, not new reactive subscriptions. + createEffect( + on( + () => props.open, + (open) => { + if (!open) return; + untrack(() => { + setStepsEnabled(store.defaultStepsEnabled); + setSkipPermissions(store.defaultSkipPermissions); + setPropagateSkipPermissions(store.defaultPropagateSkipPermissions); + }); + }, + { defer: true }, + ), + ); + + // Initialize remaining state each time the dialog opens createEffect(() => { if (!props.open) return; @@ -168,8 +191,6 @@ export function NewTaskDialog(props: NewTaskDialogProps) { setError(''); setLoading(false); setGitIsolation('worktree'); - setSkipPermissions(false); - setPropagateSkipPermissions(false); setDockerMode(false); setDockerImageReady(null); setDockerBuilding(false); @@ -629,7 +650,10 @@ export function NewTaskDialog(props: NewTaskDialogProps) { ? (projDocker?.imageTag ?? (store.dockerImage || DEFAULT_DOCKER_IMAGE)) : undefined, coordinatorMode: coordinatorMode() || undefined, - propagateSkipPermissions: coordinatorMode() ? propagateSkipPermissions() : undefined, + propagateSkipPermissions: + coordinatorMode() && agentSupportsSkipPermissions() && skipPermissions() + ? propagateSkipPermissions() + : undefined, maxConcurrentTasks: coordinatorMode() ? clampCoordinatorConcurrentTasks(maxConcurrentTasks()) : undefined, diff --git a/src/components/SettingsDialog.tsx b/src/components/SettingsDialog.tsx index 391cff31..1bbfeb3b 100644 --- a/src/components/SettingsDialog.tsx +++ b/src/components/SettingsDialog.tsx @@ -33,6 +33,9 @@ import { setDarkTheme, setCoordinatorModeEnabled, setCoordinatorNotificationDelayMs, + setDefaultStepsEnabled, + setDefaultSkipPermissions, + setDefaultPropagateSkipPermissions, updateStatus, checkForUpdates, } from '../store/store'; @@ -414,6 +417,93 @@ export function SettingsDialog(props: SettingsDialogProps) { +
+
New Task Defaults
+ + + + + +
+
{ + it('defaultStepsEnabled changes the snapshot', () => { + setStore('defaultStepsEnabled', false); + const before = persistedSnapshot(); + setStore('defaultStepsEnabled', true); + const after = persistedSnapshot(); + expect(before).not.toBe(after); + setStore('defaultStepsEnabled', false); + }); + + it('defaultSkipPermissions changes the snapshot', () => { + setStore('defaultSkipPermissions', false); + const before = persistedSnapshot(); + setStore('defaultSkipPermissions', true); + const after = persistedSnapshot(); + expect(before).not.toBe(after); + setStore('defaultSkipPermissions', false); + }); + + it('defaultPropagateSkipPermissions changes the snapshot', () => { + setStore('defaultPropagateSkipPermissions', false); + const before = persistedSnapshot(); + setStore('defaultPropagateSkipPermissions', true); + const after = persistedSnapshot(); + expect(before).not.toBe(after); + setStore('defaultPropagateSkipPermissions', false); + }); + + it('showSteps is not tracked separately (migrated to defaultStepsEnabled)', () => { + expect('showSteps' in store).toBe(false); + }); +}); diff --git a/src/store/autosave.ts b/src/store/autosave.ts index 1acf68e4..8e15aa4a 100644 --- a/src/store/autosave.ts +++ b/src/store/autosave.ts @@ -5,7 +5,7 @@ import { store, saveState } from './store'; * creates a single reactive dependency on the serialized form — the effect * only re-runs when a persisted value actually changes, instead of on every * individual field mutation (cursor moves, panel resizes, etc.). */ -function persistedSnapshot(): string { +export function persistedSnapshot(): string { return JSON.stringify({ projects: store.projects, lastProjectId: store.lastProjectId, @@ -25,7 +25,9 @@ function persistedSnapshot(): string { windowState: store.windowState, autoTrustFolders: store.autoTrustFolders, showPlans: store.showPlans, - showSteps: store.showSteps, + defaultStepsEnabled: store.defaultStepsEnabled, + defaultSkipPermissions: store.defaultSkipPermissions, + defaultPropagateSkipPermissions: store.defaultPropagateSkipPermissions, showSidebarTips: store.showSidebarTips, showSidebarProgress: store.showSidebarProgress, projectsCollapsed: store.projectsCollapsed, diff --git a/src/store/core.ts b/src/store/core.ts index 0de90b75..e7ae3749 100644 --- a/src/store/core.ts +++ b/src/store/core.ts @@ -48,7 +48,6 @@ export const [store, setStore] = createStore({ windowState: null, autoTrustFolders: false, showPlans: true, - showSteps: false, showSidebarTips: true, showSidebarProgress: true, projectsCollapsed: false, @@ -81,6 +80,9 @@ export const [store, setStore] = createStore({ coordinatorModeEnabled: false, coordinatorNotificationDelayMs: 60_000, coordinatorControlHintDismissed: false, + defaultStepsEnabled: false, + defaultSkipPermissions: false, + defaultPropagateSkipPermissions: false, customThemes: {}, activeCustomThemeId: null, mcpStatus: { running: false, port: null, coordinatorTaskId: null, mcpConfigPath: null }, diff --git a/src/store/persistence.test.ts b/src/store/persistence.test.ts index a4b9f8d8..1cba5c86 100644 --- a/src/store/persistence.test.ts +++ b/src/store/persistence.test.ts @@ -457,3 +457,229 @@ describe('projects section collapsed persistence', () => { expect(saved.projectsCollapsed).toBe(true); }); }); + +describe('new task defaults persistence', () => { + beforeEach(() => { + vi.clearAllMocks(); + setStore('defaultStepsEnabled', false); + setStore('defaultSkipPermissions', false); + setStore('defaultPropagateSkipPermissions', false); + }); + + // --- defaultStepsEnabled --- + + it('defaults defaultStepsEnabled to false when absent from saved state', async () => { + mockInvoke.mockResolvedValueOnce(basePayload()); + await loadState(); + expect(store.defaultStepsEnabled).toBe(false); + }); + + it('restores defaultStepsEnabled=true from saved state', async () => { + mockInvoke.mockResolvedValueOnce(basePayload({ defaultStepsEnabled: true })); + await loadState(); + expect(store.defaultStepsEnabled).toBe(true); + }); + + it('does not persist defaultStepsEnabled=false', async () => { + setStore('defaultStepsEnabled', false); + mockInvoke.mockResolvedValueOnce(undefined); + await saveState(); + const saved = JSON.parse(mockInvoke.mock.calls[0][1].json); + expect(saved.defaultStepsEnabled).toBeUndefined(); + }); + + it('persists defaultStepsEnabled=true', async () => { + setStore('defaultStepsEnabled', true); + mockInvoke.mockResolvedValueOnce(undefined); + await saveState(); + const saved = JSON.parse(mockInvoke.mock.calls[0][1].json); + expect(saved.defaultStepsEnabled).toBe(true); + }); + + it.each([ + ['string', 'yes'], + ['number', 1], + ['null', null], + ])('ignores non-boolean defaultStepsEnabled (%s)', async (_label, value) => { + setStore('defaultStepsEnabled', true); + mockInvoke.mockResolvedValueOnce(basePayload({ defaultStepsEnabled: value })); + await loadState(); + expect(store.defaultStepsEnabled).toBe(false); + }); + + it('invalid present defaultStepsEnabled does not fall back to showSteps=true', async () => { + mockInvoke.mockResolvedValueOnce( + JSON.stringify({ + projects: [], + taskOrder: [], + collapsedTaskOrder: [], + tasks: {}, + showSteps: true, + defaultStepsEnabled: 'yes', + }), + ); + await loadState(); + expect(store.defaultStepsEnabled).toBe(false); + }); + + // --- defaultSkipPermissions --- + + it('defaults defaultSkipPermissions to false when absent from saved state', async () => { + mockInvoke.mockResolvedValueOnce(basePayload()); + await loadState(); + expect(store.defaultSkipPermissions).toBe(false); + }); + + it('restores defaultSkipPermissions=true from saved state', async () => { + mockInvoke.mockResolvedValueOnce(basePayload({ defaultSkipPermissions: true })); + await loadState(); + expect(store.defaultSkipPermissions).toBe(true); + }); + + it('does not persist defaultSkipPermissions=false', async () => { + setStore('defaultSkipPermissions', false); + mockInvoke.mockResolvedValueOnce(undefined); + await saveState(); + const saved = JSON.parse(mockInvoke.mock.calls[0][1].json); + expect(saved.defaultSkipPermissions).toBeUndefined(); + }); + + it('persists defaultSkipPermissions=true', async () => { + setStore('defaultSkipPermissions', true); + mockInvoke.mockResolvedValueOnce(undefined); + await saveState(); + const saved = JSON.parse(mockInvoke.mock.calls[0][1].json); + expect(saved.defaultSkipPermissions).toBe(true); + }); + + it.each([ + ['string', 'yes'], + ['number', 1], + ['null', null], + ])('ignores non-boolean defaultSkipPermissions (%s)', async (_label, value) => { + setStore('defaultSkipPermissions', true); + mockInvoke.mockResolvedValueOnce(basePayload({ defaultSkipPermissions: value })); + await loadState(); + expect(store.defaultSkipPermissions).toBe(false); + }); + + // --- defaultPropagateSkipPermissions --- + + it('defaults defaultPropagateSkipPermissions to false when absent from saved state', async () => { + mockInvoke.mockResolvedValueOnce(basePayload()); + await loadState(); + expect(store.defaultPropagateSkipPermissions).toBe(false); + }); + + it('restores defaultPropagateSkipPermissions=true from saved state', async () => { + mockInvoke.mockResolvedValueOnce(basePayload({ defaultPropagateSkipPermissions: true })); + await loadState(); + expect(store.defaultPropagateSkipPermissions).toBe(true); + }); + + it('does not persist defaultPropagateSkipPermissions=false', async () => { + setStore('defaultPropagateSkipPermissions', false); + mockInvoke.mockResolvedValueOnce(undefined); + await saveState(); + const saved = JSON.parse(mockInvoke.mock.calls[0][1].json); + expect(saved.defaultPropagateSkipPermissions).toBeUndefined(); + }); + + it('persists defaultPropagateSkipPermissions=true', async () => { + setStore('defaultPropagateSkipPermissions', true); + mockInvoke.mockResolvedValueOnce(undefined); + await saveState(); + const saved = JSON.parse(mockInvoke.mock.calls[0][1].json); + expect(saved.defaultPropagateSkipPermissions).toBe(true); + }); + + it.each([ + ['string', 'yes'], + ['number', 1], + ['null', null], + ])('ignores non-boolean defaultPropagateSkipPermissions (%s)', async (_label, value) => { + setStore('defaultPropagateSkipPermissions', true); + mockInvoke.mockResolvedValueOnce(basePayload({ defaultPropagateSkipPermissions: value })); + await loadState(); + expect(store.defaultPropagateSkipPermissions).toBe(false); + }); +}); + +describe('showSteps → defaultStepsEnabled migration', () => { + beforeEach(() => { + vi.clearAllMocks(); + setStore('defaultStepsEnabled', false); + }); + + it('migrates legacy showSteps=true to defaultStepsEnabled=true', async () => { + mockInvoke.mockResolvedValueOnce( + JSON.stringify({ + projects: [], + taskOrder: [], + collapsedTaskOrder: [], + tasks: {}, + showSteps: true, + }), + ); + await loadState(); + expect(store.defaultStepsEnabled).toBe(true); + }); + + it('defaultStepsEnabled wins when both fields are present', async () => { + // If a user saves with the new field but an old showSteps is also present + // (e.g. partially migrated state), the explicit new field takes precedence. + mockInvoke.mockResolvedValueOnce( + JSON.stringify({ + projects: [], + taskOrder: [], + collapsedTaskOrder: [], + tasks: {}, + showSteps: false, + defaultStepsEnabled: true, + }), + ); + await loadState(); + expect(store.defaultStepsEnabled).toBe(true); + }); + + it('explicit defaultStepsEnabled=false is not overridden by showSteps=true', async () => { + // This is the direction the old || logic got wrong — an explicit false was + // overridden by a legacy true. The new presence-check logic must respect + // the explicit new field even when the legacy field disagrees. + mockInvoke.mockResolvedValueOnce( + JSON.stringify({ + projects: [], + taskOrder: [], + collapsedTaskOrder: [], + tasks: {}, + showSteps: true, + defaultStepsEnabled: false, + }), + ); + await loadState(); + expect(store.defaultStepsEnabled).toBe(false); + }); + + it('showSteps=false does not set defaultStepsEnabled=true', async () => { + mockInvoke.mockResolvedValueOnce( + JSON.stringify({ + projects: [], + taskOrder: [], + collapsedTaskOrder: [], + tasks: {}, + showSteps: false, + }), + ); + await loadState(); + expect(store.defaultStepsEnabled).toBe(false); + }); + + it('showSteps is not saved by saveState', async () => { + setStore('defaultStepsEnabled', true); + mockInvoke.mockResolvedValueOnce(undefined); + await saveState(); + const saved = JSON.parse(mockInvoke.mock.calls[0][1].json); + expect(saved.showSteps).toBeUndefined(); + expect(saved.defaultStepsEnabled).toBe(true); + }); +}); diff --git a/src/store/persistence.ts b/src/store/persistence.ts index a3500457..fb10f77b 100644 --- a/src/store/persistence.ts +++ b/src/store/persistence.ts @@ -186,7 +186,6 @@ export async function saveState(): Promise { windowState: store.windowState ? { ...store.windowState } : undefined, autoTrustFolders: store.autoTrustFolders, showPlans: store.showPlans, - showSteps: store.showSteps, showSidebarTips: store.showSidebarTips, showSidebarProgress: store.showSidebarProgress, projectsCollapsed: store.projectsCollapsed, @@ -213,6 +212,9 @@ export async function saveState(): Promise { darkThemeCustomId: store.darkThemeCustomId ?? undefined, coordinatorModeEnabled: store.coordinatorModeEnabled || undefined, coordinatorControlHintDismissed: store.coordinatorControlHintDismissed || undefined, + defaultStepsEnabled: store.defaultStepsEnabled || undefined, + defaultSkipPermissions: store.defaultSkipPermissions || undefined, + defaultPropagateSkipPermissions: store.defaultPropagateSkipPermissions || undefined, }; for (const taskId of store.taskOrder) { @@ -377,6 +379,9 @@ interface LegacyPersistedState { darkThemeCustomId?: unknown; coordinatorModeEnabled?: unknown; coordinatorControlHintDismissed?: unknown; + defaultStepsEnabled?: unknown; + defaultSkipPermissions?: unknown; + defaultPropagateSkipPermissions?: unknown; } export async function loadState(): Promise { @@ -493,7 +498,6 @@ export async function loadState(): Promise { s.windowState = parsePersistedWindowState(raw.windowState); s.autoTrustFolders = typeof raw.autoTrustFolders === 'boolean' ? raw.autoTrustFolders : false; s.showPlans = typeof raw.showPlans === 'boolean' ? raw.showPlans : true; - s.showSteps = typeof raw.showSteps === 'boolean' ? raw.showSteps : false; s.showSidebarTips = typeof raw.showSidebarTips === 'boolean' ? raw.showSidebarTips : true; s.showSidebarProgress = typeof raw.showSidebarProgress === 'boolean' ? raw.showSidebarProgress : true; @@ -569,6 +573,18 @@ export async function loadState(): Promise { s.coordinatorControlHintDismissed = raw.coordinatorControlHintDismissed === true; + // Migrate legacy showSteps — only fall back to it when the new field is + // absent (not present-but-invalid) so invalid values and explicit false + // are never overridden by the legacy field. + s.defaultStepsEnabled = + typeof raw.defaultStepsEnabled === 'boolean' + ? raw.defaultStepsEnabled + : 'defaultStepsEnabled' in (raw as object) + ? false + : raw.showSteps === true; + s.defaultSkipPermissions = raw.defaultSkipPermissions === true; + s.defaultPropagateSkipPermissions = raw.defaultPropagateSkipPermissions === true; + const rawDockerImage = raw.dockerImage; s.dockerImage = typeof rawDockerImage === 'string' && rawDockerImage.trim() diff --git a/src/store/store.ts b/src/store/store.ts index 74d2020a..9225d5a1 100644 --- a/src/store/store.ts +++ b/src/store/store.ts @@ -141,6 +141,9 @@ export { setMinimaxApiKey, setWindowState, setCoordinatorModeEnabled, + setDefaultStepsEnabled, + setDefaultSkipPermissions, + setDefaultPropagateSkipPermissions, } from './ui'; export { getTaskDotStatus, diff --git a/src/store/tasks.test.ts b/src/store/tasks.test.ts index ad7a1e5e..d307380a 100644 --- a/src/store/tasks.test.ts +++ b/src/store/tasks.test.ts @@ -68,6 +68,8 @@ mockSetStore.mockImplementation((...args: unknown[]) => applySetStore(...args)); vi.mock('../lib/ipc', () => ({ Channel: vi.fn(), invoke: mockInvoke })); +let mockDefaultStepsEnabled = false; + vi.mock('./core', () => ({ store: new Proxy({} as Record, { get(_target, prop) { @@ -77,6 +79,7 @@ vi.mock('./core', () => ({ if (prop === 'collapsedTaskOrder') return mockCollapsedTaskOrder; if (prop === 'availableAgents') return []; if (prop === 'projects') return mockProjects; + if (prop === 'defaultStepsEnabled') return mockDefaultStepsEnabled; return undefined; }, }), @@ -169,6 +172,7 @@ beforeEach(() => { mockTaskOrder = []; mockCollapsedTaskOrder = []; mockProjects = []; + mockDefaultStepsEnabled = false; mockInvoke.mockResolvedValue(undefined); mockSaveState.mockResolvedValue(undefined); }); @@ -768,6 +772,58 @@ describe('createTask coordinator base branch prompt', () => { }); }); +// ─── createTask stepsEnabled default regression ──────────────────────────────── + +describe('createTask does not mutate defaultStepsEnabled', () => { + const agentDef = { + id: 'agent-def', + name: 'Claude', + command: 'claude', + args: [], + resume_args: [], + skip_permissions_args: [], + description: 'Claude', + }; + + beforeEach(() => { + vi.clearAllMocks(); + mockSetStore.mockImplementation((...args: unknown[]) => applySetStore(...args)); + mockTasks = {}; + mockAgents = {}; + mockTaskOrder = []; + mockDefaultStepsEnabled = false; + vi.mocked(getProjectPath).mockReturnValue('/repo'); + vi.mocked(getProjectBranchPrefix).mockReturnValue('task'); + vi.mocked(isProjectMissing).mockReturnValue(false); + mockInvoke.mockImplementation((channel: string) => { + if (channel === IPC.CreateTask) { + return Promise.resolve({ + id: 'task-1', + branch_name: 'task/my-task', + worktree_path: '/repo/.worktrees/my-task', + }); + } + return Promise.resolve(undefined); + }); + }); + + it('does not write back defaultStepsEnabled when stepsEnabled differs from the default', async () => { + mockDefaultStepsEnabled = false; + await createTask({ + name: 'My Task', + agentDef, + projectId: 'proj-1', + gitIsolation: 'worktree', + baseBranch: 'main', + stepsEnabled: true, + }); + const defaultsMutated = mockSetStore.mock.calls.some( + (args) => args[0] === 'defaultStepsEnabled', + ); + expect(defaultsMutated).toBe(false); + }); +}); + // ─── sendPrompt tests ───────────────────────────────────────────────────────── function writePayloads(): string[] { diff --git a/src/store/tasks.ts b/src/store/tasks.ts index 3a899d7e..3bbd75d7 100644 --- a/src/store/tasks.ts +++ b/src/store/tasks.ts @@ -243,10 +243,8 @@ export async function createTask(opts: CreateTaskOptions): Promise { } } - // Per-task steps tracking — explicit opt-in from dialog, or fall back to last-used preference - const stepsEnabled = opts.stepsEnabled ?? store.showSteps; - // Remember this choice so the dialog defaults to it next time - if (stepsEnabled !== store.showSteps) setStore('showSteps', stepsEnabled); + // Per-task steps tracking — explicit opt-in from dialog, or fall back to default preference + const stepsEnabled = opts.stepsEnabled ?? store.defaultStepsEnabled; // Inject steps instruction into the first prompt so the agent maintains steps.json. // Appended after a separator for recency bias; savedInitialPrompt keeps the original clean text. diff --git a/src/store/types.ts b/src/store/types.ts index 203e1480..7f73e6be 100644 --- a/src/store/types.ts +++ b/src/store/types.ts @@ -248,7 +248,6 @@ export interface PersistedState { windowState?: PersistedWindowState; autoTrustFolders?: boolean; showPlans?: boolean; - showSteps?: boolean; showSidebarTips?: boolean; showSidebarProgress?: boolean; projectsCollapsed?: boolean; @@ -271,6 +270,9 @@ export interface PersistedState { coordinatorModeEnabled?: boolean; coordinatorNotificationDelayMs?: number; coordinatorControlHintDismissed?: boolean; + defaultStepsEnabled?: boolean; + defaultSkipPermissions?: boolean; + defaultPropagateSkipPermissions?: boolean; } export interface MCPStatus { @@ -342,7 +344,6 @@ export interface AppStore { windowState: PersistedWindowState | null; autoTrustFolders: boolean; showPlans: boolean; - showSteps: boolean; showSidebarTips: boolean; showSidebarProgress: boolean; projectsCollapsed: boolean; @@ -376,5 +377,8 @@ export interface AppStore { coordinatorModeEnabled: boolean; coordinatorNotificationDelayMs: number; coordinatorControlHintDismissed: boolean; + defaultStepsEnabled: boolean; + defaultSkipPermissions: boolean; + defaultPropagateSkipPermissions: boolean; mcpStatus: MCPStatus; } diff --git a/src/store/ui.ts b/src/store/ui.ts index bdf99229..09abb112 100644 --- a/src/store/ui.ts +++ b/src/store/ui.ts @@ -200,6 +200,18 @@ export function setCoordinatorModeEnabled(enabled: boolean): void { ); } +export function setDefaultStepsEnabled(enabled: boolean): void { + setStore('defaultStepsEnabled', enabled); +} + +export function setDefaultSkipPermissions(enabled: boolean): void { + setStore('defaultSkipPermissions', enabled); +} + +export function setDefaultPropagateSkipPermissions(enabled: boolean): void { + setStore('defaultPropagateSkipPermissions', enabled); +} + export function setCoordinatorNotificationDelayMs(ms: number): void { const clamped = Math.max(5_000, Math.min(300_000, Math.round(ms))); setStore('coordinatorNotificationDelayMs', clamped); From 2514fda97bd19d6e95935ad96c2e41aa57ebc6a8 Mon Sep 17 00:00:00 2001 From: brooksc Date: Thu, 4 Jun 2026 15:29:24 -0700 Subject: [PATCH 2/2] docs(openspec): fix steps-tracking spec to match actual defaultStepsEnabled contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the canonical spec and the new-task-defaults change spec incorrectly stated that creating a task with the checkbox in the opposite state would update defaultStepsEnabled. The implementation intentionally does not do this — only the Settings toggle mutates the default. Update both specs to reflect the actual contract: - New Task dialog reads defaultStepsEnabled on open - Settings → General → New Task Defaults toggles update defaultStepsEnabled - Task creation does not mutate defaultStepsEnabled Also renames the showSteps reference to defaultStepsEnabled in the canonical spec to match the renamed store field. Co-Authored-By: Claude Sonnet 4.6 --- .../new-task-defaults/specs/steps-tracking/spec.md | 14 ++++++++++---- openspec/specs/steps-tracking/spec.md | 13 ++++++++++--- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/openspec/changes/new-task-defaults/specs/steps-tracking/spec.md b/openspec/changes/new-task-defaults/specs/steps-tracking/spec.md index f3d1788a..81e08b04 100644 --- a/openspec/changes/new-task-defaults/specs/steps-tracking/spec.md +++ b/openspec/changes/new-task-defaults/specs/steps-tracking/spec.md @@ -13,13 +13,19 @@ for this default is renamed from `showSteps` to `defaultStepsEnabled`; existing - **THEN** the "Steps tracking" checkbox is pre-checked if and only if the persisted `defaultStepsEnabled` app-level flag is true -#### Scenario: Default updates when user toggles the checkbox +#### Scenario: Settings toggle updates the default -- **WHEN** the user creates a task with the checkbox in the opposite state -- **THEN** the persisted `defaultStepsEnabled` app-level flag is updated to - match +- **WHEN** the user enables or disables "Steps tracking" in Settings → General → New Task Defaults +- **THEN** the persisted `defaultStepsEnabled` app-level flag is updated to match - **AND** the next new-task dialog uses that value as the default +#### Scenario: Task creation does not update the default + +- **WHEN** the user creates a task with the "Steps tracking" checkbox in a + different state than `defaultStepsEnabled` +- **THEN** the persisted `defaultStepsEnabled` app-level flag is NOT changed +- **AND** the per-task `stepsEnabled` flag reflects the checkbox state at creation time + #### Scenario: Migration from legacy showSteps key - **WHEN** a user with `showSteps: true` in saved state opens the app after diff --git a/openspec/specs/steps-tracking/spec.md b/openspec/specs/steps-tracking/spec.md index ccf5b487..08e95117 100644 --- a/openspec/specs/steps-tracking/spec.md +++ b/openspec/specs/steps-tracking/spec.md @@ -13,18 +13,25 @@ reading through raw terminal scrollback. Steps tracking SHALL be opt-in on a per-task basis, with a persistent app-level default that the new-task dialog uses to pre-fill the checkbox. -#### Scenario: New task dialog reflects the last-used default +#### Scenario: New task dialog reflects the persistent default - **WHEN** the user opens the new-task dialog - **THEN** the "Steps tracking" checkbox is pre-checked if and only if the persisted `defaultStepsEnabled` app-level flag is true -#### Scenario: Default updates when user toggles the checkbox +#### Scenario: Settings toggle updates the default -- **WHEN** the user creates a task with the checkbox in the opposite state +- **WHEN** the user enables or disables "Steps tracking" in Settings → General → New Task Defaults - **THEN** the persisted `defaultStepsEnabled` app-level flag is updated to match - **AND** the next new-task dialog uses that value as the default +#### Scenario: Task creation does not update the default + +- **WHEN** the user creates a task with the "Steps tracking" checkbox in a + different state than `defaultStepsEnabled` +- **THEN** the persisted `defaultStepsEnabled` app-level flag is NOT changed +- **AND** the per-task `stepsEnabled` flag reflects the checkbox state at creation time + #### Scenario: `stepsEnabled` is per-task and persisted - **WHEN** a task is created with the checkbox enabled