Skip to content

feat(settings): add create-worktree-by-default toggle for new tasks and fix run restart after stop intent#1329

Merged
arnestrickmann merged 2 commits intogeneralaction:mainfrom
isEmmanuelOlowe:main
Mar 6, 2026
Merged

feat(settings): add create-worktree-by-default toggle for new tasks and fix run restart after stop intent#1329
arnestrickmann merged 2 commits intogeneralaction:mainfrom
isEmmanuelOlowe:main

Conversation

@isEmmanuelOlowe
Copy link
Contributor

Summary

This PR adds a new task setting to reduce friction when creating many parallel tasks on the same branch.
Users can now disable worktree creation by default from Settings, and the New Task modal respects that saved preference.
It also includes a small lifecycle fix so a run can restart correctly while a stop intent is active.

Why

Some users working with capable parallel agents (for example Kimi 2.5, GPT-5.*, Claude) prefer creating many simultaneous tasks on a shared branch. This setting makes that workflow one-click instead of repeatedly disabling worktree creation.

Fixes

N/A

Snapshot

N/A (settings toggle + behavior change)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Mandatory Tasks

  • I have self-reviewed the code
  • A decent size PR without self-review might be rejected

Checklist

  • I have read the contributing guide
  • My code follows the style guidelines of this project (pnpm run format)
  • I have checked if my changes generate no new warnings (pnpm run lint)

@vercel
Copy link

vercel bot commented Mar 6, 2026

@isEmmanuelOlowe is attempting to deploy a commit to the General Action Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 6, 2026

Greptile Summary

This PR adds a createWorktreeByDefault boolean setting (default true) so users can opt out of automatic worktree creation from the Settings page, and the New Task modal now reads and applies that preference on open. It also attempts to fix a lifecycle bug where calling startRun while a stop intent was active would be silently skipped.

Key changes:

  • src/main/settings.ts — new createWorktreeByDefault field added to AppSettings, defaults, and normalizeSettings.
  • src/renderer/hooks/useTaskSettings.ts — model and hook extended with the new field and updater.
  • src/renderer/components/TaskSettingsRows.tsx / SettingsPage.tsx — new CreateWorktreeByDefaultRow toggle wired into the Settings UI.
  • src/renderer/components/TaskModal.tsx — reads the setting asynchronously and applies it via setUseWorktree.
  • src/main/services/TaskLifecycleService.ts — bypass the "already running" guard when a stop intent is active so a restart is not silently dropped.

Issues found:

  • Logic bug in TaskLifecycleService.ts: the stop intent is not cleared before the new process is spawned. When the restarted process later exits normally, the lingering stop intent causes its status to be set to 'idle' instead of 'succeeded' or 'failed', producing incorrect state in the UI. See inline comment for the suggested fix.
  • Minor UX flicker in TaskModal.tsx: setUseWorktree(true) is hardcoded as the synchronous reset value on modal open. Users who have createWorktreeByDefault: false saved will see the toggle briefly flip to the "on" state before the async settings fetch corrects it.

Confidence Score: 2/5

  • Not safe to merge — the lifecycle restart fix introduces a stale stop-intent bug that will misreport run exit status as idle.
  • The settings plumbing and UI changes are clean and follow existing patterns. However, the core bug fix in TaskLifecycleService.startRunInternal has a correctness issue: clearing the stop intent before spawning the replacement process is omitted, so the new process's eventual exit is always classified as a user-initiated stop ('idle') regardless of its actual exit code. This corrupts lifecycle state and will surface as incorrect UI status every time a user restarts a run while a stop is in flight.
  • src/main/services/TaskLifecycleService.ts requires a one-line fix to clear stopIntents before the replacement process is spawned.

Important Files Changed

Filename Overview
src/main/services/TaskLifecycleService.ts Adds !this.stopIntents.has(taskId) guard to allow run restart during an active stop intent, but fails to clear the lingering stop intent before spawning the new process — causing the new process's exit to be misclassified as 'idle' instead of 'succeeded'.
src/main/settings.ts Adds createWorktreeByDefault: boolean to the AppSettings interface, sets a sensible default of true, and normalises it correctly in normalizeSettings. Pattern is consistent with existing boolean settings.
src/renderer/components/SettingsPage.tsx Imports and inserts CreateWorktreeByDefaultRow between the auto-approve and auto-trust-worktrees rows. Placement is semantically appropriate; change is minimal and correct.
src/renderer/components/TaskModal.tsx Reads createWorktreeByDefault from async-loaded settings and applies it via setUseWorktree. The hardcoded setUseWorktree(true) synchronous reset on modal open will cause a visible flicker for users who have the setting disabled.
src/renderer/components/TaskSettingsRows.tsx New CreateWorktreeByDefaultRow component follows the exact same structure as peer rows (AutoApproveByDefaultRow, AutoTrustWorktreesRow). No issues.
src/renderer/hooks/useTaskSettings.ts Extends TaskSettingsModel with createWorktreeByDefault field and updateCreateWorktreeByDefault handler. Default fallback (?? true) matches the backend default. No issues.

Sequence Diagram

sequenceDiagram
    participant User
    participant TaskModal
    participant SettingsStore
    participant TaskLifecycleService

    User->>TaskModal: Open "New Task" modal
    TaskModal->>SettingsStore: rpc.appSettings.get()
    SettingsStore-->>TaskModal: { createWorktreeByDefault: bool }
    TaskModal->>TaskModal: setUseWorktree(createWorktreeByDefault)

    User->>TaskModal: Submit task (useWorktree=false)
    TaskModal->>TaskLifecycleService: createTask(useWorktree=false)

    Note over TaskLifecycleService: Task runs on shared branch

    User->>TaskLifecycleService: stopRun(taskId)
    TaskLifecycleService->>TaskLifecycleService: stopIntents.add(taskId)
    TaskLifecycleService->>TaskLifecycleService: SIGTERM → old process

    User->>TaskLifecycleService: startRun(taskId) [restart]
    TaskLifecycleService->>TaskLifecycleService: stopIntents.has(taskId)=true → skip guard
    TaskLifecycleService->>TaskLifecycleService: spawn new process
    Note over TaskLifecycleService: ⚠️ stopIntents NOT cleared

    TaskLifecycleService->>TaskLifecycleService: old process exits → early return (no cleanup)
    TaskLifecycleService->>TaskLifecycleService: new process exits (code=0)
    TaskLifecycleService->>TaskLifecycleService: wasStopped=true (stale intent!) → status='idle' ❌
Loading

Comments Outside Diff (1)

  1. src/renderer/components/TaskModal.tsx, line 219 (link)

    Hardcoded reset causes flicker when setting is false

    On every modal open, setUseWorktree(true) is called unconditionally before the async settings fetch resolves. If the user has saved createWorktreeByDefault: false, the toggle will briefly flash as checked (true) before being corrected once the rpc.appSettings.get() promise resolves.

    The other two async-loaded booleans (autoApprove, autoGenerateName) start from safe false/true defaults that match their actual saved values for most users, so the flicker is rarely visible. For useWorktree, users who disabled the default will see the flash every time they open the modal.

    Consider initialising the state from the already-available useAppSettings context (used by useTaskSettings) instead of a hard-coded literal, or at minimum drop the synchronous reset line so the previous value is preserved until the fetch completes.

    → could be removed or replaced with the value from the synchronously available settings context if the context is accessible here.

Last reviewed commit: 97cfff9

Comment on lines +336 to 343
if (
existing &&
existing.exitCode === null &&
!existing.killed &&
!this.stopIntents.has(taskId)
) {
return { ok: true, skipped: true };
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale stop intent corrupts new run's exit status

When startRun is called while a stop intent is active (e.g. the user clicks "restart" immediately after clicking "stop"), the new guard correctly bypasses the skip check and spawns a fresh process. However, the stop intent for taskId is never cleared before the new child is registered in runProcesses.

Here is what happens step by step:

  1. stopRun is called → stopIntents.add(taskId), SIGTERM sent to old process.
  2. startRun is called before the old process exits.
  3. startRunInternal passes the new guard (stop intent present), spawns a new child, and calls this.runProcesses.set(taskId, newChild).
  4. Old process exits → runProcesses.get(taskId) !== oldChild, so the handler returns early without calling this.stopIntents.delete(taskId).
  5. New process runs to completion (exit code 0).
  6. New process's exit handler fires: wasStopped = this.stopIntents.has(taskId) evaluates to true (the leftover stop intent), so status is set to 'idle' instead of 'succeeded'.

The fix is to clear the stop intent immediately after the guard is bypassed, before spawning the new process:

// Clear any residual stop intent so the new process's exit is not misclassified.
if (this.stopIntents.has(taskId)) {
  this.stopIntents.delete(taskId);
}

const state = this.ensureState(taskId);
state.run = {
  status: 'running',
  ...

@arnestrickmann
Copy link
Contributor

I see a PR from the SSH King

I am happy

Without this, restarting a run while a stop is in flight leaves the old
stop intent in place. When the new process exits, it sees the leftover
intent and misreports its status as 'idle' instead of 'succeeded'/'failed'.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@arnestrickmann
Copy link
Contributor

Added a one-line fix for the stale stop-intent bug in TaskLifecycleService.ts: when restarting a run while a stop is in flight, the old stop intent was never cleared before spawning the new process.

@arnestrickmann arnestrickmann merged commit 6529720 into generalaction:main Mar 6, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants