diff --git a/internal/acp/agent.go b/internal/acp/agent.go index 7c301044c..003a2c1cb 100644 --- a/internal/acp/agent.go +++ b/internal/acp/agent.go @@ -106,6 +106,14 @@ type acpSession struct { restrictModels bool cancel context.CancelFunc history []turnRecord + + // readOnly marks a session published by session/load for a kind that is + // not resumable (a child, side or spec sub-run). Load still replays its + // transcript, which is the render-only use a client needs, but a prompt + // would continue the sub-run as a standalone conversation, which every + // other resume path refuses (sessions.IsResumableKind). Set once when the + // session is first registered and never changed, so it is read without mu. + readOnly bool } // NewAgent builds the ACP server and registers its method handlers on conn. @@ -204,7 +212,7 @@ func (a *Agent) handleSessionNew(ctx context.Context, params json.RawMessage) (a if err != nil { return nil, RPCError(codeInternalError, "create session: "+err.Error()) } - sess, _ := a.registerSession(meta.SessionID, root, nil, model, models, restrictModels) + sess, _ := a.registerSession(meta.SessionID, root, nil, model, models, restrictModels, false) return NewSessionResult{ SessionID: sess.id, ConfigOptions: a.configOptions(sess), @@ -311,7 +319,10 @@ func (a *Agent) activatePersistedSession(ctx context.Context, p LoadSessionParam models = append(models, SessionConfigOptionValue{Value: persistedModel, Name: persistedModel}) } } - sess, existed := a.registerSession(meta.SessionID, root, history, model, models, restrictModels) + // Resume has already refused a non-resumable kind above, so only load can + // reach here with one, and it publishes that session for viewing only. + readOnly := !sessions.IsResumableKind(meta.SessionKind) + sess, existed := a.registerSession(meta.SessionID, root, history, model, models, restrictModels, readOnly) if existed { _, messages, historyWarning, historyErr = a.refreshSessionHistory(sess, requireHistoryLog) if operation == persistedSessionResume && historyErr != nil { @@ -433,6 +444,9 @@ func (a *Agent) handleSessionPrompt(ctx context.Context, params json.RawMessage) if sess == nil { return nil, RPCError(codeInvalidParams, "unknown session: "+p.SessionID) } + if sess.readOnly { + return nil, RPCError(codeInvalidParams, "session is not resumable, so it was loaded for viewing only and cannot be prompted: "+p.SessionID) + } // Serialize turns for this session so two prompts can't interleave history or // fight over the single cancel slot. session/cancel still works concurrently @@ -1154,13 +1168,13 @@ func promptImages(blocks []ContentBlock) []zeroruntime.ImageBlock { // The bool reports that case so persisted activation can re-read and apply disk // history under turnMu. history is set BEFORE first publication so no concurrent // prompt can read a half-initialized session. -func (a *Agent) registerSession(id, cwd string, history []turnRecord, model string, models []SessionConfigOptionValue, restrictModels bool) (*acpSession, bool) { +func (a *Agent) registerSession(id, cwd string, history []turnRecord, model string, models []SessionConfigOptionValue, restrictModels bool, readOnly bool) (*acpSession, bool) { a.mu.Lock() defer a.mu.Unlock() if existing := a.sessions[id]; existing != nil { return existing, true } - sess := &acpSession{id: id, cwd: cwd, mode: agent.PermissionModeAuto, model: model, models: models, restrictModels: restrictModels, history: history} + sess := &acpSession{id: id, cwd: cwd, mode: agent.PermissionModeAuto, model: model, models: models, restrictModels: restrictModels, history: history, readOnly: readOnly} a.sessions[id] = sess return sess, false } diff --git a/internal/acp/agent_test.go b/internal/acp/agent_test.go index 41457549e..88635d99e 100644 --- a/internal/acp/agent_test.go +++ b/internal/acp/agent_test.go @@ -1251,6 +1251,85 @@ func TestACPResumeAppliesStandaloneSessionKindPolicy(t *testing.T) { }, &LoadSessionResult{}); err != nil { t.Fatalf("session/load should retain render-only access: %v", err) } + // The check above only showed the refused resume never registered the + // session. Load does register it, so the prompt that matters is the one + // AFTER the load: render-only means the transcript can be read and the + // sub-run cannot be continued. + err = h.client.Call(ctx, MethodSessionPrompt, PromptParams{ + SessionID: created.SessionID, Prompt: []ContentBlock{TextBlock("must stay closed after load")}, + }, &PromptResult{}) + if !errors.As(err, &rpcErr) || rpcErr.Code != codeInvalidParams { + t.Fatalf("session/prompt after load of a %s session = %v, want invalid params", tc.name, err) + } + }) + } +} + +// LOAD PUBLISHES A SUB-RUN FOR VIEWING, NOT FOR CONTINUING. Every other resume +// path (the TUI, exec sessions, the exec CLI, and ACP's own session/resume) +// refuses a kind sessions.IsResumableKind rejects. session/load has a real +// render-only use, a client rebuilding a transcript it can scroll, so it keeps +// loading those sessions; what it must not do is publish one that accepts a +// prompt, because the prompt would carry on a child, side or spec sub-run as a +// standalone conversation. Checked in both directions, so a gate that refused +// everything would fail on the resumable kinds. +func TestACPLoadPublishesSubRunsReadOnly(t *testing.T) { + for _, tc := range []struct { + name string + kind sessions.SessionKind + readOnly bool + }{ + {name: "regular", kind: ""}, + {name: "fork", kind: sessions.SessionKindFork}, + {name: "child", kind: sessions.SessionKindChild, readOnly: true}, + {name: "side", kind: sessions.SessionKindSide, readOnly: true}, + {name: "spec draft", kind: sessions.SessionKindSpecDraft, readOnly: true}, + {name: "spec impl", kind: sessions.SessionKindSpecImpl, readOnly: true}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := !sessions.IsResumableKind(tc.kind); got != tc.readOnly { + t.Fatalf("SETUP INVALID: IsResumableKind(%q) disagrees with this table (readOnly=%v), so the cases no longer mean what they say", tc.kind, tc.readOnly) + } + deps := testDeps(t) + workspace := t.TempDir() + created, err := deps.Store.Create(sessions.CreateInput{ + SessionID: "load-" + strings.ReplaceAll(tc.name, " ", "-"), Cwd: workspace, SessionKind: tc.kind, + }) + if err != nil { + t.Fatal(err) + } + h := newHarness(t, deps) + defer h.stop() + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + if err := h.client.Call(ctx, MethodSessionLoad, LoadSessionParams{ + SessionID: created.SessionID, Cwd: workspace, + }, &LoadSessionResult{}); err != nil { + t.Fatalf("session/load of a %s session: %v", tc.name, err) + } + err = h.client.Call(ctx, MethodSessionPrompt, PromptParams{ + SessionID: created.SessionID, Prompt: []ContentBlock{TextBlock("continue")}, + }, &PromptResult{}) + // A resumable session has to be genuinely promptable, not merely "not + // refused as view-only": any other failure would hide a broken + // writable path behind a passing test. + if !tc.readOnly { + if err != nil { + t.Fatalf("session/prompt after load of a %s session = %v, want success", tc.name, err) + } + return + } + // The decision is the RPC code; the message only distinguishes the + // view-only refusal from other invalid-params errors such as an + // unknown session id. + var rpcErr *rpcError + if !errors.As(err, &rpcErr) || rpcErr.Code != codeInvalidParams { + t.Fatalf("session/prompt after load of a %s session = %v, want invalid params", tc.name, err) + } + if !strings.Contains(rpcErr.Message, "loaded for viewing only") { + t.Fatalf("session/prompt after load of a %s session was refused for another reason: %v", tc.name, err) + } }) } }