typebasedio/ctask
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

fix: only remove provisional workspace when child exits non-zero

Browse Source 
Adds a child-exit-code guard to handleProvisional so a `ctask new`
workspace is reclaimed only when the agent was actually canceled
before real work (trust prompt rejected, Esc during startup, Ctrl+C
mid-launch — all confirmed empirically as exit code 1). A zero exit
means the user entered the agent and exited cleanly, which is a
legitimate workflow that must preserve the workspace even with an
empty manifest diff — for example when the user wants the workspace
directory established so they can populate context/ before resuming.

The exit code reaches handleProvisional via a new childExitCode
helper that unwraps *exec.ExitError from cmd.Run's return. Non-exit
errors (agent not found, OS-level failure) map to -1 so the
workspace is still cleaned up — the child never actually ran.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
typebasedio
2026-04-22 19:03:17 -04:00
parent 35d3b24e09
commit ba8b3a19f9
3 changed files with 61 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/session/run.go
+21 -5
View File
@@ -3,6 +3,7 @@ package session
import (
"fmt"
"os"
"os/exec"
"path/filepath"
"time"
@@ -153,11 +154,12 @@ func Run(opts LaunchOpts) error {
}
// ---- Provisional-workspace cleanup ----
// If `ctask new` launched this session and the child exited without
// changing any files, remove the workspace entirely and skip finalize.
// Nothing to log, nothing to summarize, and the .ctask state files die
// with the directory.
if handleProvisional(opts, startManifest) {
// If `ctask new` launched this session, the child exited non-zero
// (canceled before real work), and no files changed, remove the workspace
// entirely and skip finalize. A zero child exit means the user entered
// the agent and exited normally — those workspaces are preserved even
// with an empty diff.
if handleProvisional(opts, startManifest, childExitCode(childErr)) {
return childErr
}
@@ -173,6 +175,20 @@ func Run(opts LaunchOpts) error {
return childErr
}
// childExitCode extracts the exit code from the error returned by cmd.Run().
// Returns 0 when err is nil (clean exit). Returns the reported code for
// *exec.ExitError. Returns -1 for any other error (agent not found, OS-level
// failure) so provisional cleanup still kicks in — the child never ran.
func childExitCode(err error) int {
if err == nil {
return 0
}
if ee, ok := err.(*exec.ExitError); ok {
return ee.ExitCode()
}
return -1
}
// finalize runs all end-of-session metadata writes under one write-lock
// acquisition: session log append (best-effort), last-session-summary.json
// (best-effort), lease removal (best-effort, if we owned it), and
internal/session/run_provisional.go
+16 -4
View File
@@ -6,9 +6,15 @@ import (
)
// handleProvisional removes a workspace that was created by this `ctask new`
// invocation but saw no real work. "No real work" is defined strictly as an
// empty manifest diff — zero added, zero modified, zero deleted. No other
// heuristic is used.
// invocation but was canceled before producing any real work. All three
// conditions must hold:
//
// - NewlyCreated is true (so the workspace is ours to reclaim)
// - the child process exited non-zero (trust prompt rejected, Esc during
// startup, Ctrl+C mid-launch — confirmed empirically; a zero exit means
// the user entered the agent and exited cleanly, which is a legitimate
// workflow that must preserve the workspace even with no file changes)
// - the manifest diff is empty (nothing to preserve)
//
// Removing the workspace directory also removes every v0.4 state file inside
// .ctask/ (session.json, manifest-start.json, last-session-summary.json,
@@ -16,10 +22,16 @@ import (
//
// Returns true iff the workspace was removed (the caller must then skip the
// normal finalize path, since there is nothing to log or summarize).
func handleProvisional(opts LaunchOpts, startManifest *Manifest) bool {
func handleProvisional(opts LaunchOpts, startManifest *Manifest, childExitCode int) bool {
if !opts.NewlyCreated {
return false
}
// A clean child exit (code 0) means the user reached the agent and exited
// normally — they may want the workspace preserved for context files even
// if the session produced no manifest changes. Keep it.
if childExitCode == 0 {
return false
}
// If the start manifest failed to capture we cannot prove the diff is
// empty, so err on the side of keeping the workspace.
if startManifest == nil {
internal/session/run_provisional_test.go
+24 -8
View File
@@ -25,11 +25,11 @@ func newWorkspace(t *testing.T) (string, *Manifest) {
return wsDir, m
}
func TestHandleProvisionalRemovesWhenNewlyCreatedAndEmptyDiff(t *testing.T) {
func TestHandleProvisionalRemovesWhenNewlyCreatedAndEmptyDiffAndExitNonZero(t *testing.T) {
wsDir, startManifest := newWorkspace(t)
opts := LaunchOpts{WsDir: wsDir, NewlyCreated: true}
if !handleProvisional(opts, startManifest) {
if !handleProvisional(opts, startManifest, 1) {
t.Fatal("expected handleProvisional to return true (workspace removed)")
}
if _, err := os.Stat(wsDir); !errors.Is(err, os.ErrNotExist) {
@@ -41,7 +41,7 @@ func TestHandleProvisionalKeepsWhenNotNewlyCreated(t *testing.T) {
wsDir, startManifest := newWorkspace(t)
opts := LaunchOpts{WsDir: wsDir, NewlyCreated: false}
if handleProvisional(opts, startManifest) {
if handleProvisional(opts, startManifest, 1) {
t.Fatal("expected handleProvisional to return false when NewlyCreated is false")
}
if _, err := os.Stat(wsDir); err != nil {
@@ -58,7 +58,7 @@ func TestHandleProvisionalKeepsWhenDiffNonEmpty(t *testing.T) {
}
opts := LaunchOpts{WsDir: wsDir, NewlyCreated: true}
if handleProvisional(opts, startManifest) {
if handleProvisional(opts, startManifest, 1) {
t.Fatal("expected handleProvisional to return false when diff has changes")
}
if _, err := os.Stat(wsDir); err != nil {
@@ -73,7 +73,7 @@ func TestHandleProvisionalKeepsWhenStartManifestNil(t *testing.T) {
}
opts := LaunchOpts{WsDir: wsDir, NewlyCreated: true}
if handleProvisional(opts, nil) {
if handleProvisional(opts, nil, 1) {
t.Fatal("expected handleProvisional to return false when startManifest is nil")
}
if _, err := os.Stat(wsDir); err != nil {
@@ -81,13 +81,29 @@ func TestHandleProvisionalKeepsWhenStartManifestNil(t *testing.T) {
}
}
// A zero exit code means the user accepted the trust prompt and exited the
// agent normally — a legitimate workflow (e.g. the user wanted the workspace
// directory established to populate context/ later). The workspace must be
// preserved even when the manifest diff is empty.
func TestHandleProvisionalKeepsWhenExitCodeZero(t *testing.T) {
wsDir, startManifest := newWorkspace(t)
opts := LaunchOpts{WsDir: wsDir, NewlyCreated: true}
if handleProvisional(opts, startManifest, 0) {
t.Fatal("expected handleProvisional to return false when child exited zero")
}
if _, err := os.Stat(wsDir); err != nil {
t.Errorf("expected wsDir to still exist, got err=%v", err)
}
}
func TestHandleProvisionalRemovesShellSession(t *testing.T) {
// The fix is not agent-specific: a --shell launch with no file changes
// must also be cleaned up.
// and a non-zero exit (Ctrl+C during shell startup) must be cleaned up.
wsDir, startManifest := newWorkspace(t)
opts := LaunchOpts{WsDir: wsDir, NewlyCreated: true, Shell: true}
if !handleProvisional(opts, startManifest) {
if !handleProvisional(opts, startManifest, 1) {
t.Fatal("expected handleProvisional to remove workspace for shell session with no changes")
}
if _, err := os.Stat(wsDir); !errors.Is(err, os.ErrNotExist) {
@@ -118,7 +134,7 @@ func TestHandleProvisionalRemovalWipesV04State(t *testing.T) {
}
opts := LaunchOpts{WsDir: wsDir, NewlyCreated: true}
if !handleProvisional(opts, startManifest) {
if !handleProvisional(opts, startManifest, 1) {
t.Fatal("expected handleProvisional to remove workspace")
}
for _, name := range stateFiles {