Skip to content

Commit c1937df

Browse files
authored
feat(acp): add standard session list and resume (#914)
* feat(acp): add standard session list and resume Origin-Session: local-abff1c | Claude Code | 7 prompts Origin-Snapshot: b7d0806d49f9 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b Origin-Session: local-c962d7 | Claude Code | 3 prompts Origin-Snapshot: d2b8f44a9abc * fix(acp): preserve notification wire order Origin-Session: local-abff1c | Claude Code | 7 prompts Origin-Snapshot: b7d0806d49f9 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b Origin-Session: local-c962d7 | Claude Code | 3 prompts Origin-Snapshot: d2b8f44a9abc * fix(acp): bind sessions to persisted workspaces Origin-Session: local-abff1c | Claude Code | 7 prompts Origin-Snapshot: b7d0806d49f9 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b Origin-Session: local-c962d7 | Claude Code | 3 prompts Origin-Snapshot: d2b8f44a9abc * fix(acp): compare workspaces by filesystem identity, not by spelling @Vasanthdev2004's three findings, all reproduced before changing anything. TWO SPELLINGS OF ONE DIRECTORY WERE TWO WORKSPACES. Both comparisons were string equality on ResolveWorkspaceRoot output, and that resolver is abs plus filepath.Clean plus a stat — it does not fold case and does not resolve junctions. A session persisted from the TUI was unresumable from an editor holding a different spelling of the same project folder, and session/list filtered by the other spelling returned nothing, which makes it an invisible failure rather than a reported one. It lands on exactly the case this feature exists for, and on the two processes most likely to disagree about spelling. os.SameFile asks the filesystem which directories these are, which is the question. filepath.EvalSymlinks is NOT the fix on Windows — it normalises a drive letter and returns a junction path unchanged, so the alias survives it, and junctions need no privilege. String equality stays as the fast path, and a stat failure falls back to it rather than widening the match: this gate refuses access to another workspace's files and configuration, so an unanswerable comparison denies. THE TEST COULD NOT SEE ANY OF IT. testDeps resolves with the identity function, so the guard degenerated to "are these two strings different" fed two unrelated temp directories — it could only ever answer yes. The rejection direction was pinned and the acceptance direction was asserted nowhere. The new test uses a resolver reproducing the production normalisation and drives the ACCEPTANCE direction through an alias, skipping if the filesystem folds the alias away so it never passes vacuously. Reverting to string equality fails it three ways: load, resume, and a list that returns zero. THE WIRE KEYS ARE AN EXTERNAL CONTRACT. Nothing pinned sessionId, cwd, title, updatedAt, _meta, modelId, createdAt, sessions, nextCursor, cursor, loadSession, promptCapabilities, sessionCapabilities, list or resume, so renaming a Go field would break every client and leave the suite green. Renaming modelId to model_id now fails. Origin-Session: local-79d7a0 | Claude Code | 5 prompts Origin-Snapshot: c175cabb9d50 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b Origin-Session: local-c962d7 | Claude Code | 3 prompts Origin-Snapshot: d2b8f44a9abc * fix(acp): test the alias guard where the bug lives, and stop listing unusable sessions THE TEST COULD NOT RUN ON THE PLATFORM THE BUG IS FROM. @Vasanthdev2004's point, and he is right that it is not a nit. The alias test built its second name with os.Symlink, which needs a privilege an ordinary Windows session does not hold, so it SKIPPED there — and Windows is where junctions exist and where this defect came from. The identity guard was verified by CI on the two platforms that never had the problem. It now builds the alias with mklink /J on Windows, which needs no privilege and is how he found the defect in the first place, and keeps the symlink arm elsewhere. This is the second time in this series a correct fix shipped with a test that could not exercise it: the same helper shape was added to internal/memory for the same reason a day earlier. A SESSION WITH NO PERSISTED WORKSPACE IS NOT RESUMABLE, SO IT IS NOT LISTED. CodeRabbit's finding. activatePersistedSession refuses an empty Cwd, but the listing advertised it anyway — a menu entry that only fails when taken. The test asserts both halves, because the listing is only correct relative to what resume will accept: the omitted session is checked to really fail on resume, and a usable session is checked to survive the filter. Mutation-checked: removing the filter advertises the unusable session again. Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b Origin-Session: local-c962d7 | Claude Code | 3 prompts Origin-Snapshot: d2b8f44a9abc * fix(acp): resolve every listed workspace, not only when a filter was supplied @anandh8x's P1, both halves reproduced before changing anything. The loop resolved item.Cwd only when a cwd filter was present, and skipped only a blank one. Two shapes stayed on the menu that session/resume then refuses: - a session whose persisted workspace has since been deleted, advertised as resumable - a legacy entry holding a relative path, reported as cwd "." although ACP requires SessionInfo.cwd to be absolute Every entry is now resolved unconditionally, anything that cannot resolve is omitted, and the RESOLVED root is what SessionInfo carries — absolute as the contract requires, and the same value the client hands back on resume. The optional identity filter then applies to the resolved roots, which is also where it belonged. Listing is a menu: activatePersistedSession resolves and refuses what it cannot reach, so anything this loop cannot resolve is something a client would be offered and then denied. Mutation-checked: restoring the resolve-only-when-filtered shape re-advertises the deleted workspace and re-emits the relative cwd. Origin-Session: local-76c8d7 | Claude Code | 6 prompts Origin-Snapshot: 259b715cf0fd Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b Origin-Session: local-c962d7 | Claude Code | 3 prompts Origin-Snapshot: d2b8f44a9abc * test(acp): assert the relative entry is retained, not merely absolute CodeRabbit's catch, and the test was genuinely weaker than it looked. It checked that the deleted workspace was gone, the live one kept, and every listed cwd absolute — all of which a "fix" that simply DISCARDED any non-absolute entry would satisfy, while losing a resumable session. Presence is now asserted separately from spelling: the relative entry must still be listed, and listed with an absolute path. Mutation-checked: skipping non-absolute entries instead of resolving them now fails with "a session with a resolvable relative workspace was dropped rather than normalised". The first attempt at that mutation did not compile, so it proved nothing until it was rewritten — worth saying, because a mutation that fails to build looks exactly like a test that passes. Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b Origin-Session: local-c962d7 | Claude Code | 3 prompts Origin-Snapshot: d2b8f44a9abc * fix(acp): a relative persisted workspace is omitted, not rebased Reported by @anandh8x. Resolving a stored relative cwd does not recover the session's workspace, it invents one: ResolveWorkspaceRoot joins it against whatever directory the ACP server happens to be running in, and that invented absolute path was then advertised as the session's workspace and accepted as its home on resume. Reproduced on f2c6fc9, a session persisted with cwd ".": LISTED legacy-rel as cwd="/Users/kratos/dev/f914/internal/acp" resume with an UNRELATED workspace -> err=... cwd does not match its persisted workspace resume with NO cwd (falls back to ".") -> err=<nil> The mismatch check does its job when the client names a workspace, so the only opening was the fallback path, where the rebased value was compared against itself and always agreed. A conversation created for one project could be resumed against another project's files, configuration and tools. Both doors now take the same guard: handleSessionList omits an entry whose persisted cwd is not absolute, and activatePersistedSession refuses one rather than resolving it. The original base is not knowable from the metadata, so guessing at it is not an option a fix can take. This reverses an earlier assertion in TestSessionListResolvesEveryWorkspace, which expected the relative entry to be normalised and retained. That was requested in review on the grounds that dropping it loses a resumable session. It does, but the entry was never resumable into its own workspace, only into this process's. The test now asserts it is dropped, and a new TestResumeRefusesARelativePersistedWorkspace covers the fallback path that the listing filter alone leaves open. Both guards mutation-checked: removing either one fails its test. Pre-existing on this branch and on its merge-base, unrelated to this change: TestRunDoctorFormatsRedactedProviderDiagnostics and TestRunDoctorConnectivityProbesProvider both exit 3 in this environment. Origin-Session: local-8cd239 | Claude Code | 11 prompts Origin-Snapshot: 365efe3045f2 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b Origin-Session: local-c962d7 | Claude Code | 3 prompts Origin-Snapshot: d2b8f44a9abc * test(acp): the resume guard is asserted through both activating methods Raised by CodeRabbit: the test is named for resume and called session/load. session/load and session/resume are separate entry points that today share activatePersistedSession, so an assertion through either one passes while the guard holds — but the name promised a surface it was not touching. Both are now named explicitly, which keeps that true: if resume is ever given its own path, this fails rather than quietly covering half of what it claims to. With the guard removed, both methods accept a relative persisted workspace on the no-cwd fallback. The named-workspace case was already refused by the existing mismatch check; the fallback was the only door open, and it is open on both. Not taken in this PR, from the same review: threading a rooted directory handle through ResolveWorkspaceRoot and workspace construction so a root rename or link swap cannot redirect later file operations. That is a real question and a pre-existing one — this change adds a refusal and no path handling — but it is a capability refactor across workspace and tool access with its own race test, not something to fold into a session-list fix. Worth its own issue. Origin-Session: local-8cd239 | Claude Code | 11 prompts Origin-Snapshot: 365efe3045f2 Origin-Session: local-13d543 | Claude Code | 5 prompts Origin-Snapshot: 6b5eb8ba4e5b Origin-Session: local-c962d7 | Claude Code | 3 prompts Origin-Snapshot: d2b8f44a9abc * fix(acp): activation requires an absolute cwd in the REQUEST, not just the record Reported by @jatmn. ResumeSessionParams is a type alias for LoadSessionParams, so JSON decoding turns an OMITTED resume cwd into an empty string. That blank reached the shared activation path, whose blank-cwd fallback substitutes meta.Cwd — so {"sessionId":"known"} activated a persisted session, even though ACP v1 requires session/resume to carry an absolute working directory. This is a different hole from the persisted-cwd one fixed earlier on this branch. That guard asks whether the STORED workspace is identifiable; this asks whether the CALLER named one at all. The earlier fix does not cover it, because a stored absolute cwd passes that check and the blank request then silently inherits it. requestedWorkspace validates the request's own cwd before anything reaches the fallback: absent, empty and whitespace-only are all invalid params, and so is a relative path. Applied to both activating methods rather than to resume alone — load's omitted-cwd fallback was inheriting the same way, and leaving one door open is how this class survived the last fix. Mutation-checked at the wire, which is where the defect lives: making the blank case return no error compiles and fails the regression on four separate requests — session/load and session/resume, each with cwd omitted and with cwd blank. A Go-level test would not have caught it, since the defect is in decoding an absent field. Two notes from a verification pass, neither a defect: Error precedence changed: a blank cwd with an UNKNOWN session id now reports the cwd problem instead of "session not found". Kept deliberately and pinned — it stops the server confirming whether a session exists to a request that named no workspace. AdditionalDirectories is declared on two params structs and consumed nowhere in the repo. Not a hole today, but it is the same shape — client-supplied paths with no absoluteness rule — so the field now carries a note saying it must go through requestedWorkspace when wired up. Rebased onto ad34dc8. go test -race ./internal/acp/ -count=5: clean. Pre-existing here and on main: TestRunDoctorFormatsRedactedProviderDiagnostics and TestRunDoctorConnectivityProbesProvider exit 3 in this environment. Origin-Session: local-c962d7 | Claude Code | 3 prompts Origin-Snapshot: d2b8f44a9abc * fix(acp): resume the conversation a session became, and fail closed when it cannot Three defects in ACP session restoration, all reported by @jatmn. loadHistory read the raw event log and kept only EventMessage. A compacted session stores its original prefix alongside an EventCompaction naming the events it replaced and carrying their summary, so restoring from the raw log replayed superseded turns AND dropped the summary that replaced them. It now reads the same rehydrated view the TUI and exec paths use, and explicitly projects the compaction summary -- switching readers alone would still drop it, because rehydration substitutes the compaction event in place of what it replaced. historyErr only suppressed replay and raised a warning: the session was registered and reported ready regardless, so an unreadable events file left the caller holding a live, promptable session ID whose next prompt ran as a fresh conversation under the old identity. Resume now fails. Load keeps the best-effort policy deliberately rather than by inheriting the shared helper. Tool calls and their results were dropped from session/load, so a restored transcript showed prose asserting edits with no record that any tool ran. They now replay through the same toolCallStart/toolCallResult mapping a live turn uses, keyed on the stored toolCallId so results pair with their calls. They do not enter turnRecord, so load and resume still consume the same effective history. Resume stays replay-free. Also asserts that an unset sessionCapabilities is omitted rather than serialized as null, raised by CodeRabbit. * fix(acp): persist tool activity for session replay * test(acp): bound replay notification waits * fix(acp): preserve session restore invariants * fix(acp): restore session lifecycle boundaries * fix(acp): preserve replay and restore identity * fix(acp): commit a completed turn as one store batch A completed turn was buffered until its outcome was known and then persisted one event per call. Each call took and released the store's session lock, so a second ACP instance holding the same session -- its turnMu is its own, only the store is shared -- could append its whole turn between two of them: user A, user B, answer B, answer A. Every write was individually locked and the log was valid, but a fresh load pairs prose by order and reconstructed three wrong turns from two right ones. Before this branch the user and assistant events went to AppendEvents together; buffering until the outcome is known was the right change and splitting the commit into singleton writes was not. The whole buffer now goes through one Store.AppendEvents call, which writes the batch under one lock and one file write. That is the contiguity being restored: not a disk transaction, and not a promise about a crash mid-fsync. The outcome gate still runs first; a hard failure still commits nothing; cancellation still commits through the same boundary. A persistence failure is now the failure of the whole batch, so there is no dependent suffix to protect and the rewritten regression asserts that nothing of the turn is durable, the warning is raised, and a fresh load replays none of it. The two-instance regression holds both writers at the commit boundary until each has arrived and then delegates to the real batch API under the real lock; it observes what production hands it rather than batching anything itself, so reverting to singleton writes fails it before any interleaving has to be provoked. The restored prompt keeps both prose pairs and no historical tool output. Also normalizes the expected workspace in the listing test through filepath.Clean, matching the resolver's output contract, so a TMPDIR spelled with a ".." component no longer fails a correct result. Rebased onto main to pick up the ACP browser metadata, specialist model restoration and worktree pointer protection changes. Both reported by @jatmn.
1 parent 9001421 commit c1937df

9 files changed

Lines changed: 2443 additions & 91 deletions

File tree

‎docs/HOW_ZERO_WORKS.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,8 +302,9 @@ Conceptually, one requested tool goes through these phases:
302302
the model can inspect it and decide what to do next.
303303
6. **Render and persist** — the result is surfaced to the TUI, stream-JSON
304304
writer, or ACP client. TUI and exec record rich tool/session events for
305-
resume/history; ACP currently persists conversational messages and streams
306-
tool/permission updates to the client.
305+
resume/history; ACP persists conversational messages and completed tool-call
306+
starts/results for load replay. Live streaming details and permission
307+
exchanges remain client notifications rather than durable history.
307308

308309
Tool success and failure are both informative context. A successful `read_file`
309310
result gives the model file contents; a failed `edit_file` result gives the model

‎internal/acp/agent.go‎

Lines changed: 545 additions & 53 deletions
Large diffs are not rendered by default.

‎internal/acp/agent_test.go‎

Lines changed: 1684 additions & 13 deletions
Large diffs are not rendered by default.

‎internal/acp/jsonrpc.go‎

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,13 @@ type Conn struct {
8989

9090
handlers map[string]HandlerFunc
9191
notifiers map[string]NotifyFunc
92+
// Notifications for one method are an ordered stream: session/update in
93+
// particular is stateful, so dispatching every frame in an unrelated
94+
// goroutine can make a later chunk overtake an earlier one. Different
95+
// methods retain independent tails, keeping session/cancel responsive while
96+
// an update handler is busy.
97+
notifyMu sync.Mutex
98+
notifyTails map[string]chan struct{}
9299

93100
mu sync.Mutex
94101
nextID int64
@@ -106,11 +113,12 @@ type Conn struct {
106113
// lifetime.
107114
func NewConn(r io.Reader, w io.Writer) *Conn {
108115
return &Conn{
109-
rawReader: r,
110-
w: w,
111-
handlers: make(map[string]HandlerFunc),
112-
notifiers: make(map[string]NotifyFunc),
113-
pending: make(map[int64]chan rpcMessage),
116+
rawReader: r,
117+
w: w,
118+
handlers: make(map[string]HandlerFunc),
119+
notifiers: make(map[string]NotifyFunc),
120+
notifyTails: make(map[string]chan struct{}),
121+
pending: make(map[int64]chan rpcMessage),
114122
}
115123
}
116124

@@ -303,11 +311,7 @@ func (c *Conn) handleLine(ctx context.Context, line []byte) {
303311
}(msg)
304312
case msg.isNotify():
305313
if fn := c.notifiers[msg.Method]; fn != nil {
306-
c.wg.Add(1)
307-
go func(m rpcMessage) {
308-
defer c.wg.Done()
309-
fn(ctx, m.Params)
310-
}(msg)
314+
c.dispatchNotification(ctx, msg, fn)
311315
}
312316
default:
313317
// Malformed frame; reply only if we can identify a request id.
@@ -317,6 +321,35 @@ func (c *Conn) handleLine(ctx context.Context, line []byte) {
317321
}
318322
}
319323

324+
// dispatchNotification preserves wire order within one method without making
325+
// unrelated notification methods wait for each other. The read loop installs
326+
// each tail before starting its goroutine, so goroutine scheduling cannot
327+
// reorder the chain it observes.
328+
func (c *Conn) dispatchNotification(ctx context.Context, msg rpcMessage, fn NotifyFunc) {
329+
c.notifyMu.Lock()
330+
previous := c.notifyTails[msg.Method]
331+
done := make(chan struct{})
332+
c.notifyTails[msg.Method] = done
333+
c.notifyMu.Unlock()
334+
335+
c.wg.Add(1)
336+
go func() {
337+
defer c.wg.Done()
338+
defer func() {
339+
close(done)
340+
c.notifyMu.Lock()
341+
if c.notifyTails[msg.Method] == done {
342+
delete(c.notifyTails, msg.Method)
343+
}
344+
c.notifyMu.Unlock()
345+
}()
346+
if previous != nil {
347+
<-previous
348+
}
349+
fn(ctx, msg.Params)
350+
}()
351+
}
352+
320353
func (c *Conn) dispatchRequest(ctx context.Context, msg rpcMessage) {
321354
fn := c.handlers[msg.Method]
322355
if fn == nil {

‎internal/acp/jsonrpc_test.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,6 +360,86 @@ func TestConnNotification(t *testing.T) {
360360
}
361361
}
362362

363+
func TestConnNotificationsForOneMethodStayInWireOrder(t *testing.T) {
364+
a, b, stop := connPair(t)
365+
defer stop()
366+
367+
firstStarted := make(chan struct{})
368+
releaseFirst := make(chan struct{})
369+
delivered := make(chan int, 2)
370+
b.HandleNotify("update", func(_ context.Context, params json.RawMessage) {
371+
var in struct{ Sequence int }
372+
_ = json.Unmarshal(params, &in)
373+
if in.Sequence == 1 {
374+
close(firstStarted)
375+
<-releaseFirst
376+
}
377+
delivered <- in.Sequence
378+
})
379+
380+
if err := a.Notify("update", map[string]int{"Sequence": 1}); err != nil {
381+
t.Fatalf("first notify: %v", err)
382+
}
383+
select {
384+
case <-firstStarted:
385+
case <-time.After(2 * time.Second):
386+
t.Fatal("first notification did not start")
387+
}
388+
if err := a.Notify("update", map[string]int{"Sequence": 2}); err != nil {
389+
t.Fatalf("second notify: %v", err)
390+
}
391+
select {
392+
case sequence := <-delivered:
393+
t.Fatalf("notification %d overtook the blocked first notification", sequence)
394+
case <-time.After(25 * time.Millisecond):
395+
}
396+
close(releaseFirst)
397+
for want := 1; want <= 2; want++ {
398+
select {
399+
case got := <-delivered:
400+
if got != want {
401+
t.Fatalf("notification order = ...,%d; want %d", got, want)
402+
}
403+
case <-time.After(2 * time.Second):
404+
t.Fatalf("notification %d was not delivered", want)
405+
}
406+
}
407+
}
408+
409+
func TestConnDifferentNotificationMethodsStayConcurrent(t *testing.T) {
410+
a, b, stop := connPair(t)
411+
defer stop()
412+
413+
updateStarted := make(chan struct{})
414+
releaseUpdate := make(chan struct{})
415+
cancelDelivered := make(chan struct{})
416+
b.HandleNotify("update", func(context.Context, json.RawMessage) {
417+
close(updateStarted)
418+
<-releaseUpdate
419+
})
420+
b.HandleNotify("cancel", func(context.Context, json.RawMessage) {
421+
close(cancelDelivered)
422+
})
423+
424+
if err := a.Notify("update", nil); err != nil {
425+
t.Fatalf("update notify: %v", err)
426+
}
427+
select {
428+
case <-updateStarted:
429+
case <-time.After(2 * time.Second):
430+
t.Fatal("update notification did not start")
431+
}
432+
if err := a.Notify("cancel", nil); err != nil {
433+
t.Fatalf("cancel notify: %v", err)
434+
}
435+
select {
436+
case <-cancelDelivered:
437+
case <-time.After(2 * time.Second):
438+
t.Fatal("different notification method was blocked behind update")
439+
}
440+
close(releaseUpdate)
441+
}
442+
363443
func TestConnMethodNotFound(t *testing.T) {
364444
a, _, stop := connPair(t)
365445
defer stop()

‎internal/acp/translate.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@ func agentMessageChunk(delta string) ContentChunk {
1919
return ContentChunk{SessionUpdate: UpdateAgentMessageChunk, Content: TextBlock(delta)}
2020
}
2121

22+
func replayMessageChunk(role, messageID, text string) ContentChunk {
23+
update := UpdateAgentMessageChunk
24+
if role == "user" {
25+
update = UpdateUserMessageChunk
26+
}
27+
return ContentChunk{SessionUpdate: update, MessageID: messageID, Content: TextBlock(text)}
28+
}
29+
2230
func agentThoughtChunk(delta string) ContentChunk {
2331
return ContentChunk{SessionUpdate: UpdateAgentThoughtChunk, Content: TextBlock(delta)}
2432
}

‎internal/acp/types.go‎

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ const (
1212
MethodAuthenticate = "authenticate"
1313
MethodSessionNew = "session/new"
1414
MethodSessionLoad = "session/load"
15+
MethodSessionList = "session/list"
16+
MethodSessionResume = "session/resume"
1517
MethodSessionPrompt = "session/prompt"
1618
MethodSessionCancel = "session/cancel" // notification
1719
MethodSessionUpdate = "session/update" // notification (agent -> client)
@@ -62,8 +64,16 @@ type PromptCapabilities struct {
6264
}
6365

6466
type AgentCapabilities struct {
65-
LoadSession bool `json:"loadSession"`
66-
PromptCapabilities PromptCapabilities `json:"promptCapabilities"`
67+
LoadSession bool `json:"loadSession"`
68+
PromptCapabilities PromptCapabilities `json:"promptCapabilities"`
69+
SessionCapabilities *SessionCapabilities `json:"sessionCapabilities,omitempty"`
70+
}
71+
72+
// Empty capability objects are presence flags in ACP v1. Pointers preserve
73+
// the wire distinction between an advertised `{}` and an omitted capability.
74+
type SessionCapabilities struct {
75+
List *struct{} `json:"list,omitempty"`
76+
Resume *struct{} `json:"resume,omitempty"`
6777
}
6878

6979
type AuthMethod struct {
@@ -117,9 +127,14 @@ type McpServer struct {
117127
}
118128

119129
type NewSessionParams struct {
120-
Cwd string `json:"cwd"`
121-
McpServers []McpServer `json:"mcpServers"`
122-
AdditionalDirectories []string `json:"additionalDirectories,omitempty"`
130+
Cwd string `json:"cwd"`
131+
McpServers []McpServer `json:"mcpServers"`
132+
// AdditionalDirectories is declared by the protocol and consumed nowhere in
133+
// this repo yet. When it is wired up it must go through requestedWorkspace
134+
// like Cwd does: these are client-supplied paths with no absoluteness rule of
135+
// their own, which is the same shape as the resume-cwd hole that made
136+
// {"sessionId":"known"} activate a session against this process's directory.
137+
AdditionalDirectories []string `json:"additionalDirectories,omitempty"`
123138
}
124139

125140
type NewSessionResult struct {
@@ -140,6 +155,39 @@ type LoadSessionResult struct {
140155
Modes *SessionModeState `json:"modes,omitempty"`
141156
}
142157

158+
// ListSessionsParams and the following types implement ACP v1 session/list.
159+
// Transcript contents stay behind session/load; this method returns metadata
160+
// only and leaves the optional pagination cursor opaque.
161+
type ListSessionsParams struct {
162+
Cwd string `json:"cwd,omitempty"`
163+
Cursor string `json:"cursor,omitempty"`
164+
}
165+
166+
type SessionInfoMeta struct {
167+
ModelID string `json:"modelId,omitempty"`
168+
CreatedAt string `json:"createdAt,omitempty"`
169+
}
170+
171+
type SessionInfo struct {
172+
SessionID string `json:"sessionId"`
173+
Cwd string `json:"cwd"`
174+
Title string `json:"title,omitempty"`
175+
UpdatedAt string `json:"updatedAt,omitempty"`
176+
Meta *SessionInfoMeta `json:"_meta,omitempty"`
177+
}
178+
179+
type ListSessionsResult struct {
180+
Sessions []SessionInfo `json:"sessions"`
181+
NextCursor string `json:"nextCursor,omitempty"`
182+
}
183+
184+
// session/resume takes session/load's wire shape, so its cwd is a plain string
185+
// too and an omitted one decodes exactly like an empty one. Sharing the type is
186+
// safe only because activatePersistedSession requires an absolute cwd from the
187+
// request itself — nothing downstream may supply a default for either method.
188+
type ResumeSessionParams = LoadSessionParams
189+
type ResumeSessionResult = LoadSessionResult
190+
143191
// ---- prompt turn ----
144192

145193
type PromptParams struct {
@@ -174,6 +222,7 @@ type SessionNotification struct {
174222
// ContentBlock under "content"; the variant is set via SessionUpdate.
175223
type ContentChunk struct {
176224
SessionUpdate string `json:"sessionUpdate"`
225+
MessageID string `json:"messageId,omitempty"`
177226
Content ContentBlock `json:"content"`
178227
}
179228

‎internal/sessions/replay.go‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,20 @@ func CompactionPayloadFromPlan(summary string, plan CompactionPlan) (CompactionP
226226
}
227227

228228
func (store *Store) ReadRehydratedEvents(sessionID string) ([]Event, error) {
229-
events, err := store.ReadEvents(sessionID)
229+
events, _, err := store.ReadRehydratedEventsWithPresence(sessionID)
230+
return events, err
231+
}
232+
233+
// ReadRehydratedEventsWithPresence carries the underlying event-log presence
234+
// through compaction projection without changing ReadRehydratedEvents' existing
235+
// empty-on-missing contract.
236+
func (store *Store) ReadRehydratedEventsWithPresence(sessionID string) ([]Event, bool, error) {
237+
events, present, err := store.ReadEventsWithPresence(sessionID)
230238
if err != nil {
231-
return nil, err
239+
return nil, present, err
232240
}
233-
return RehydrateEvents(events)
241+
rehydrated, err := RehydrateEvents(events)
242+
return rehydrated, present, err
234243
}
235244

236245
func (store *Store) ReadReplayEvents(sessionID string) ([]Event, error) {

‎internal/sessions/store.go‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -813,15 +813,24 @@ func (store *Store) UpdateModel(sessionID string, modelID string) (Metadata, err
813813
}
814814

815815
func (store *Store) ReadEvents(sessionID string) ([]Event, error) {
816+
events, _, err := store.ReadEventsWithPresence(sessionID)
817+
return events, err
818+
}
819+
820+
// ReadEventsWithPresence preserves the distinction between an intentionally
821+
// empty event log and a missing one. Most readers accept both as empty history,
822+
// but activation protocols that promise to restore previously populated
823+
// context need the stronger signal.
824+
func (store *Store) ReadEventsWithPresence(sessionID string) ([]Event, bool, error) {
816825
if !ValidSessionID(sessionID) {
817-
return nil, fmt.Errorf("invalid zero session id %q", sessionID)
826+
return nil, false, fmt.Errorf("invalid zero session id %q", sessionID)
818827
}
819828
data, err := os.ReadFile(store.eventsPath(sessionID))
820829
if err != nil {
821830
if errors.Is(err, os.ErrNotExist) {
822-
return []Event{}, nil
831+
return []Event{}, false, nil
823832
}
824-
return nil, fmt.Errorf("read zero session events: %w", err)
833+
return nil, false, fmt.Errorf("read zero session events: %w", err)
825834
}
826835
// A genuine torn tail is an INCOMPLETE final write — a crash mid-append leaves
827836
// the last line without its terminating newline. If the file ends with a
@@ -851,11 +860,11 @@ func (store *Store) ReadEvents(sessionID string) ([]Event, error) {
851860
if index == lastNonEmpty && tornTailPossible {
852861
break
853862
}
854-
return nil, fmt.Errorf("invalid json in zero session %s %s at line %d: %w", sessionID, EventsFile, index+1, err)
863+
return nil, true, fmt.Errorf("invalid json in zero session %s %s at line %d: %w", sessionID, EventsFile, index+1, err)
855864
}
856865
events = append(events, event)
857866
}
858-
return events, nil
867+
return events, true, nil
859868
}
860869

861870
func (store *Store) timestamp() string {

0 commit comments

Comments
 (0)