From 21cd7ae480183d6226e41c49428e7c7f3e607310 Mon Sep 17 00:00:00 2001 From: Vasanthdev2004 Date: Wed, 11 Nov 2026 18:41:00 +0530 Subject: [PATCH] fix(specialist): let Task resume continue its own specialist child Task resume runs `zero exec --resume ` on the child session the fresh run created. Since #748, exec refuses every child session, once in the CLI preflight and again in sessions.PrepareExec, so every Task resume failed with "Zero session is not resumable". The specialist tests launch a fake binary, so nothing caught it. The resume now passes --calling-session-id and --calling-tool-use-id the way the fresh run does, and sessions.ExecResumable lets exec continue a child only when the calling session is the child's recorded parent and the tag is the child's own. Both exec checks call it. A plain resume, another parent, another tag and side sessions such as /btw are still refused. Fixes #1209 --- internal/cli/exec_sessions.go | 2 +- internal/cli/exec_task_resume_test.go | 127 ++++++++++++++++++++ internal/sessions/exec_resume_owner_test.go | 56 +++++++++ internal/sessions/exec_session.go | 25 +++- internal/specialist/exec.go | 13 ++ 5 files changed, 221 insertions(+), 2 deletions(-) create mode 100644 internal/cli/exec_task_resume_test.go create mode 100644 internal/sessions/exec_resume_owner_test.go diff --git a/internal/cli/exec_sessions.go b/internal/cli/exec_sessions.go index 06aebcec0..a5a5ab492 100644 --- a/internal/cli/exec_sessions.go +++ b/internal/cli/exec_sessions.go @@ -67,7 +67,7 @@ func preflightExecSession(options execOptions) error { if session == nil { return execUsageError{"Zero session not found: " + options.resume} } - if !sessions.IsResumableKind(session.SessionKind) { + if !sessions.ExecResumable(*session, options.callingSessionID, options.tag) { return execUsageError{"Zero session is not resumable: " + options.resume} } case options.resumeLatest: diff --git a/internal/cli/exec_task_resume_test.go b/internal/cli/exec_task_resume_test.go new file mode 100644 index 000000000..fc1b4536b --- /dev/null +++ b/internal/cli/exec_task_resume_test.go @@ -0,0 +1,127 @@ +package cli + +import ( + "bytes" + "context" + "encoding/json" + "strings" + "testing" + + "github.com/Twigpine/zero/internal/config" + "github.com/Twigpine/zero/internal/sessions" + "github.com/Twigpine/zero/internal/specialist" + "github.com/Twigpine/zero/internal/streamjson" + "github.com/Twigpine/zero/internal/tools" + "github.com/Twigpine/zero/internal/zeroruntime" +) + +// TestTaskResumeContinuesTheSpecialistChildThroughExec runs the Task tool's own +// arguments through the real exec path, for the fresh run and for the resume. +// The fresh run creates a child session (kind child, tag specialist), and exec +// refused to resume that kind, so every Task resume failed with "Zero session +// is not resumable" while the specialist tests, which launch a fake binary, +// stayed green. +func TestTaskResumeContinuesTheSpecialistChildThroughExec(t *testing.T) { + isolateCLIUserState(t) + cwd := t.TempDir() + store := sessions.NewStore(sessions.StoreOptions{}) + parent, err := store.Create(sessions.CreateInput{SessionID: "parent_task", Cwd: cwd}) + if err != nil { + t.Fatalf("create parent: %v", err) + } + + provider := &capturingImageProvider{} + deps := appDeps{ + stdin: strings.NewReader(""), + getwd: func() (string, error) { return cwd, nil }, + resolveConfig: func(string, config.Overrides) (config.ResolvedConfig, error) { + return execResolvedConfig(), nil + }, + newProvider: func(config.ProviderProfile) (zeroruntime.Provider, error) { return provider, nil }, + } + var childStderr []string + executor := specialist.Executor{ + BinaryPath: "zero", + SessionStore: store, + NewSessionID: func() (string, error) { return "child_task", nil }, + Load: func(specialist.LoadOptions) (specialist.LoadResult, error) { + return specialist.LoadResult{Specialists: []specialist.Manifest{{ + Metadata: specialist.Metadata{ + Name: "skim", + Description: "Bounded read-only lookups.", + Tools: []string{"read_file"}, + }, + SystemPrompt: "Find the thing and stop.", + ResolvedTools: []string{"read_file"}, + }}}, nil + }, + // The child is the real exec entry point, run in process with the + // arguments the executor built, instead of a fake binary. + RunChild: func(_ context.Context, _ string, args []string, _ func(streamjson.Event)) (specialist.ChildRunResult, error) { + var stdout, stderr bytes.Buffer + code := runWithDeps(args, &stdout, &stderr, deps) + childStderr = append(childStderr, stderr.String()) + events := []streamjson.Event{} + for _, line := range strings.Split(stdout.String(), "\n") { + if line = strings.TrimSpace(line); line == "" { + continue + } + var event streamjson.Event + if err := json.Unmarshal([]byte(line), &event); err != nil { + t.Fatalf("child stream-json line %q: %v", line, err) + } + events = append(events, event) + } + return specialist.ChildRunResult{Events: events, Stderr: stderr.String(), ExitCode: code, Started: true}, nil + }, + } + options := specialist.TaskRunOptions{ParentSessionID: parent.SessionID, ToolCallID: "call_1", Cwd: cwd} + + fresh, err := executor.Run(context.Background(), specialist.TaskParameters{Name: "skim", Prompt: "find the config"}, options) + if err != nil || fresh.Result.Status != tools.StatusOK { + t.Fatalf("SETUP: fresh Task run = %+v, %v; child stderr %q", fresh.Result, err, childStderr) + } + child, err := store.Get("child_task") + if err != nil || child == nil { + t.Fatalf("SETUP: child session = %v, %v", child, err) + } + // SETUP: the fresh run made exactly the kind of session the resume is about. + if child.SessionKind != sessions.SessionKindChild || child.Tag != specialist.SessionTagSpecialist || child.ParentSessionID != parent.SessionID { + t.Fatalf("SETUP: child kind %q tag %q parent %q, want child/specialist/%s", child.SessionKind, child.Tag, child.ParentSessionID, parent.SessionID) + } + + options.ToolCallID = "call_2" + resumed, err := executor.Run(context.Background(), specialist.TaskParameters{Name: "skim", Prompt: "now check the tests", Resume: "child_task"}, options) + if err != nil || resumed.Result.Status != tools.StatusOK { + t.Fatalf("Task resume = %+v, %v; child stderr %q", resumed.Result, err, childStderr) + } + provider.mu.Lock() + prompt := provider.prompt + provider.mu.Unlock() + if !strings.Contains(prompt, "Continuing Zero session child_task.") || !strings.Contains(prompt, "now check the tests") { + t.Fatalf("resumed child prompt = %q, want the child's session continued with the follow-up", prompt) + } + sessionsAfter, err := store.List() + if err != nil { + t.Fatal(err) + } + if len(sessionsAfter) != 2 { + t.Fatalf("sessions after resume = %d, want the parent and the one child, not a new session", len(sessionsAfter)) + } + + // The child is still not resumable by hand, or from another session. + other, err := store.Create(sessions.CreateInput{SessionID: "other_parent", Cwd: cwd}) + if err != nil { + t.Fatalf("create other parent: %v", err) + } + for _, args := range [][]string{ + {"exec", "--resume", "child_task", "keep going"}, + {"exec", "--resume", "child_task", "--tag", "specialist", "--calling-session-id", other.SessionID, "keep going"}, + } { + var stdout, stderr bytes.Buffer + code := runWithDeps(args, &stdout, &stderr, deps) + if code == exitSuccess || !strings.Contains(stderr.String(), "Zero session is not resumable: child_task") { + t.Errorf("%q exit=%d stderr=%q, want the not-resumable refusal", args, code, stderr.String()) + } + } +} diff --git a/internal/sessions/exec_resume_owner_test.go b/internal/sessions/exec_resume_owner_test.go new file mode 100644 index 000000000..5547d819d --- /dev/null +++ b/internal/sessions/exec_resume_owner_test.go @@ -0,0 +1,56 @@ +package sessions + +import ( + "strings" + "testing" +) + +// A child session stays out of user-facing resume, but the run that started it +// can continue it: the Task tool resumes a specialist through `zero exec +// --resume`, naming the child's parent as its calling session and passing the +// child's tag. Every other combination is refused as before. +func TestPrepareExecResumesAChildOnlyForItsOwner(t *testing.T) { + store := NewStore(StoreOptions{RootDir: t.TempDir()}) + for _, input := range []CreateInput{ + {SessionID: "parent"}, + {SessionID: "other_parent"}, + {SessionID: "child_task", SessionKind: SessionKindChild, ParentSessionID: "parent", Tag: "specialist", Depth: 1}, + {SessionID: "side_task", SessionKind: SessionKindSide, ParentSessionID: "parent", Tag: "specialist"}, + } { + if _, err := store.Create(input); err != nil { + t.Fatalf("create %s: %v", input.SessionID, err) + } + } + if _, err := store.AppendEvent("child_task", AppendEventInput{Type: EventMessage, Payload: map[string]any{"role": "assistant", "content": "found the config"}}); err != nil { + t.Fatal(err) + } + + prepared, err := PrepareExec(PrepareExecOptions{Store: store, Resume: "child_task", CallingSessionID: "parent", Tag: "specialist"}) + if err != nil { + t.Fatalf("owner resume of a child: %v", err) + } + if prepared.Mode != ModeResume || prepared.Session.SessionID != "child_task" || len(prepared.ContextEvents) != 1 { + t.Fatalf("owner resume = mode %q session %q with %d context events, want the child resumed with its history", prepared.Mode, prepared.Session.SessionID, len(prepared.ContextEvents)) + } + + for _, tc := range []struct { + name string + options PrepareExecOptions + }{ + {name: "plain resume", options: PrepareExecOptions{Resume: "child_task"}}, + {name: "tag without a caller", options: PrepareExecOptions{Resume: "child_task", Tag: "specialist"}}, + {name: "caller without the tag", options: PrepareExecOptions{Resume: "child_task", CallingSessionID: "parent"}}, + {name: "another parent", options: PrepareExecOptions{Resume: "child_task", CallingSessionID: "other_parent", Tag: "specialist"}}, + {name: "another tag", options: PrepareExecOptions{Resume: "child_task", CallingSessionID: "parent", Tag: "spec"}}, + {name: "side session", options: PrepareExecOptions{Resume: "side_task", CallingSessionID: "parent", Tag: "specialist"}}, + } { + t.Run(tc.name, func(t *testing.T) { + options := tc.options + options.Store = store + _, err := PrepareExec(options) + if err == nil || !strings.Contains(err.Error(), "Zero session is not resumable: "+options.Resume) { + t.Fatalf("PrepareExec(%+v) error = %v, want the not-resumable refusal", tc.options, err) + } + }) + } +} diff --git a/internal/sessions/exec_session.go b/internal/sessions/exec_session.go index ec45e37a3..3c4d0a6ae 100644 --- a/internal/sessions/exec_session.go +++ b/internal/sessions/exec_session.go @@ -109,7 +109,7 @@ func PrepareExec(options PrepareExecOptions) (PreparedExec, error) { if session == nil { return PreparedExec{}, ExecError{"Zero session not found: " + sessionID} } - if !IsResumableKind(session.SessionKind) { + if !ExecResumable(*session, options.CallingSessionID, options.Tag) { return PreparedExec{}, ExecError{"Zero session is not resumable: " + sessionID} } contextEvents, err := readExecContextEvents(store, session.SessionID) @@ -151,6 +151,29 @@ func PrepareExec(options PrepareExecOptions) (PreparedExec, error) { return PreparedExec{Mode: ModeNew, Session: session, ContextEvents: []Event{}, Store: store}, nil } +// ExecResumable reports whether `zero exec --resume` may continue session when +// it runs with the given --calling-session-id and --tag. The exec preflight and +// PrepareExec both ask this, so they cannot disagree. +// +// Any kind IsResumableKind accepts is resumable. A child session is hidden from +// user-facing resume, but the Task tool continues a specialist by resuming the +// child it created, so refusing every child made Task resume fail every time. +// A child is resumable only when the resume names the child's recorded parent +// as its calling session and carries the child's own tag. The Task tool passes +// both. A plain `zero exec --resume ` passes neither and is still refused, +// and side sessions such as /btw are never children, so they stay out too. +func ExecResumable(session Metadata, callingSessionID string, tag string) bool { + if IsResumableKind(session.SessionKind) { + return true + } + if session.SessionKind != SessionKindChild { + return false + } + parent := strings.TrimSpace(callingSessionID) + tag = strings.TrimSpace(tag) + return parent != "" && parent == session.ParentSessionID && tag != "" && tag == session.Tag +} + func readExecContextEvents(store *Store, sessionID string) ([]Event, error) { // The session was picked from its metadata a moment ago. Make sure it is // still there, and held, before reading it to continue: see HoldToContinue. diff --git a/internal/specialist/exec.go b/internal/specialist/exec.go index 04a4deeec..00b7e5567 100644 --- a/internal/specialist/exec.go +++ b/internal/specialist/exec.go @@ -151,6 +151,11 @@ type BuildResumeArgsInput struct { ParentModel string ParentReasoningEffort string PermissionMode string + // ParentSessionID is the session resuming the child. exec resumes a child + // session only for its recorded parent (sessions.PrepareExec), so without + // it every Task resume is refused as "not resumable". + ParentSessionID string + ParentToolUseID string } type BuildArgsResult struct { @@ -440,6 +445,12 @@ func (executor Executor) BuildResumeArgs(input BuildResumeArgsInput) (BuildArgsR } args = append(args, "--enabled-tools", strings.Join(toolAllowlist, ",")) args = append(args, "--depth", strconv.Itoa(input.CurrentDepth+1), "--tag", sessionTagSpecialist) + if parentSessionID := strings.TrimSpace(input.ParentSessionID); parentSessionID != "" { + args = append(args, "--calling-session-id", parentSessionID) + } + if parentToolUseID := strings.TrimSpace(input.ParentToolUseID); parentToolUseID != "" { + args = append(args, "--calling-tool-use-id", parentToolUseID) + } if cwd := strings.TrimSpace(input.Cwd); cwd != "" { args = append(args, "--cwd", cwd) } @@ -513,6 +524,8 @@ func (executor Executor) runResume(ctx context.Context, params TaskParameters, o ParentModel: options.ParentModel, ParentReasoningEffort: options.ParentReasoningEffort, PermissionMode: options.PermissionMode, + ParentSessionID: options.ParentSessionID, + ParentToolUseID: options.ToolCallID, }) if err != nil { return ExecResult{}, err