Compare commits

...
Author SHA1 Message Date
KRATOS 227fac9ef6 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.
2026-09-12 17:18:43 +05:30
KRATOS 9515f381ff fix(acp): preserve replay and restore identity 2026-09-12 17:18:42 +05:30
KRATOS fb8f1e00b7 fix(acp): restore session lifecycle boundaries 2026-09-12 17:18:42 +05:30
KRATOS c0bf29a8c4 fix(acp): preserve session restore invariants 2026-09-12 17:18:42 +05:30
KRATOS 718ffc899a test(acp): bound replay notification waits 2026-09-12 17:18:42 +05:30
KRATOS 386aa2585c fix(acp): persist tool activity for session replay 2026-09-12 17:18:42 +05:30
KRATOS 07bb5820fe 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.
2026-09-12 17:18:42 +05:30
KRATOS 62672974f6 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 ad34dc8d. 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
2026-09-12 17:18:42 +05:30
KRATOS 3447fd773e 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
2026-09-12 17:18:42 +05:30
KRATOS 9e6b732e2d 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 f2c6fc9a, 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
2026-09-12 17:18:42 +05:30
KRATOS d3406f4c3b 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
2026-09-12 17:18:42 +05:30
KRATOS 76e2e283c4 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
2026-09-12 17:18:42 +05:30
KRATOS 7b906fca7b 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
2026-09-12 17:18:42 +05:30
KRATOS c41104dd67 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
2026-09-12 17:18:42 +05:30
KRATOS 5a3ee23e06 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
2026-09-12 17:18:42 +05:30
KRATOS 17cde6452a 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
2026-09-12 17:18:42 +05:30
KRATOS 12f0779424 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
2026-09-12 17:18:42 +05:30
9 changed files with 2445 additions and 93 deletions

No files matched your search

+3 -2
View File
@@ -302,8 +302,9 @@ Conceptually, one requested tool goes through these phases:
the model can inspect it and decide what to do next.
6. **Render and persist** — the result is surfaced to the TUI, stream-JSON
writer, or ACP client. TUI and exec record rich tool/session events for
resume/history; ACP currently persists conversational messages and streams
tool/permission updates to the client.
resume/history; ACP persists conversational messages and completed tool-call
starts/results for load replay. Live streaming details and permission
exchanges remain client notifications rather than durable history.
Tool success and failure are both informative context. A successful `read_file`
result gives the model file contents; a failed `edit_file` result gives the model
+547 -55
View File
@@ -2,10 +2,14 @@ package acp
import (
"context"
"crypto/sha256"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"log"
"os"
"path/filepath"
"strings"
"sync"
@@ -40,8 +44,12 @@ type Deps struct {
// ResolveWorkspaceRoot validates + normalizes a client-supplied cwd (must be an
// existing directory; never the bare root). It is the file-tool confinement root.
ResolveWorkspaceRoot func(cwd string) (string, error)
Store *sessions.Store
AgentInfo Implementation
// PersistEvents is an optional test seam for injecting event-store failures.
// It receives a COMPLETED TURN as one batch, never one event at a time: the
// batch is the unit the shared store keeps contiguous (see persistTurn).
PersistEvents func(sessionID string, inputs []sessions.AppendEventInput) error
Store *sessions.Store
AgentInfo Implementation
}
// Workspace is the per-turn execution environment passed to the agent. ACP does
@@ -106,6 +114,8 @@ func NewAgent(conn *Conn, deps Deps) *Agent {
conn.Handle(MethodInitialize, a.handleInitialize)
conn.Handle(MethodSessionNew, a.handleSessionNew)
conn.Handle(MethodSessionLoad, a.handleSessionLoad)
conn.Handle(MethodSessionList, a.handleSessionList)
conn.Handle(MethodSessionResume, a.handleSessionResume)
conn.Handle(MethodSessionPrompt, a.handleSessionPrompt)
conn.Handle(MethodSessionSetMode, a.handleSetMode)
conn.Handle(MethodSessionSetConfigOption, a.handleSetConfigOption)
@@ -134,11 +144,11 @@ func (a *Agent) handleInitialize(_ context.Context, params json.RawMessage) (any
return InitializeResult{
ProtocolVersion: negotiated,
AgentCapabilities: AgentCapabilities{
// Only advertise what ZERO actually implements: session/load (loadSession)
// and image prompts. session/resume + the session-capability sub-object
// are intentionally omitted since there is no resume handler yet.
LoadSession: true,
PromptCapabilities: PromptCapabilities{Image: true},
// ACP v1 optional methods are advertised as empty capability objects;
// clients must gate session/list and session/resume on their presence.
LoadSession: true,
PromptCapabilities: PromptCapabilities{Image: true},
SessionCapabilities: &SessionCapabilities{List: &struct{}{}, Resume: &struct{}{}},
},
AgentInfo: &info,
// ZERO owns credentials (BYOK) and does not delegate auth to the editor.
@@ -148,12 +158,41 @@ func (a *Agent) handleInitialize(_ context.Context, params json.RawMessage) (any
// ---- session lifecycle ----
// requestedWorkspace is the single rule for a cwd that arrived from the client,
// applied before the value can reach ResolveWorkspaceRoot.
//
// THE REQUEST'S OWN CWD IS THE ONLY THING ALLOWED TO PICK A ROOT. Every params
// type spells cwd as a plain string, so an absent field and an empty one decode
// to exactly the same "", and the resolver treats "" as "use the directory this
// process was started in" and joins a relative cwd onto it. Those two defaults
// meet in the middle: `{"sessionId":"known"}` used to activate a persisted
// session, and `{}` used to create one, both rooted at wherever the editor
// happened to spawn `zero acp` — which then becomes the sandbox and file-tool
// confinement root. The client never named that directory, so it is a root the
// server invented, and ACP requires an absolute cwd on every method carrying
// one. Rejecting here is what keeps a missing field from being answered with a
// guess.
func requestedWorkspace(cwd string) (string, error) {
trimmed := strings.TrimSpace(cwd)
if trimmed == "" {
return "", RPCError(codeInvalidParams, "cwd is required and must be an absolute path")
}
if !filepath.IsAbs(trimmed) {
return "", RPCError(codeInvalidParams, "cwd must be an absolute path: "+trimmed)
}
return trimmed, nil
}
func (a *Agent) handleSessionNew(ctx context.Context, params json.RawMessage) (any, error) {
var p NewSessionParams
if err := json.Unmarshal(params, &p); err != nil {
return nil, RPCError(codeInvalidParams, "invalid session/new params")
}
root, err := a.deps.ResolveWorkspaceRoot(p.Cwd)
cwd, err := requestedWorkspace(p.Cwd)
if err != nil {
return nil, err
}
root, err := a.deps.ResolveWorkspaceRoot(cwd)
if err != nil {
return nil, RPCError(codeInvalidParams, err.Error())
}
@@ -165,7 +204,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)
return NewSessionResult{
SessionID: sess.id,
ConfigOptions: a.configOptions(sess),
@@ -178,22 +217,90 @@ func (a *Agent) handleSessionLoad(ctx context.Context, params json.RawMessage) (
if err := json.Unmarshal(params, &p); err != nil {
return nil, RPCError(codeInvalidParams, "invalid session/load params")
}
return a.activatePersistedSession(ctx, p, persistedSessionLoad)
}
func (a *Agent) handleSessionResume(ctx context.Context, params json.RawMessage) (any, error) {
var p ResumeSessionParams
if err := json.Unmarshal(params, &p); err != nil {
return nil, RPCError(codeInvalidParams, "invalid session/resume params")
}
return a.activatePersistedSession(ctx, p, persistedSessionResume)
}
type persistedSessionOperation uint8
const (
persistedSessionLoad persistedSessionOperation = iota
persistedSessionResume
)
// activatePersistedSession restores the agent's internal conversation context
// for both lifecycle methods. session/load additionally replays user-visible
// history as ordered session/update notifications; session/resume deliberately
// does not, which makes it safe for an already-rendered desktop reconnect.
func (a *Agent) activatePersistedSession(ctx context.Context, p LoadSessionParams, operation persistedSessionOperation) (any, error) {
// BOTH METHODS, BEFORE ANYTHING IS LOOKED UP. session/resume shares this
// params type with session/load, so the wire cannot tell an omitted cwd from
// an empty one; the blank case used to be answered with the persisted cwd,
// which made {"sessionId":"known"} a complete, successful activation request.
// Validating here rather than in one handler is what keeps the two entry
// points from drifting apart.
cwd, err := requestedWorkspace(p.Cwd)
if err != nil {
return nil, err
}
meta, err := a.deps.Store.Get(p.SessionID)
if err != nil || meta == nil {
return nil, RPCError(codeInvalidParams, "session not found: "+p.SessionID)
}
cwdInput := p.Cwd
if strings.TrimSpace(cwdInput) == "" {
cwdInput = meta.Cwd
if operation == persistedSessionResume && !sessions.IsResumableKind(meta.SessionKind) {
return nil, RPCError(codeInvalidParams, "session is not resumable: "+p.SessionID)
}
root, err := a.deps.ResolveWorkspaceRoot(cwdInput)
if strings.TrimSpace(meta.Cwd) == "" {
return nil, RPCError(codeInvalidParams, "session has no persisted workspace: "+p.SessionID)
}
// SAME RULE ON THE WAY IN. Omitting a relative entry from the listing is not
// enough: a stored "." still resolves against this process's directory, so a
// client that hands back that same directory as its cwd would be sold the
// invented root as a match. A workspace that cannot be identified is not one
// this session can be restored into.
if !filepath.IsAbs(meta.Cwd) {
return nil, RPCError(codeInvalidParams, "session workspace is not an absolute path, so it cannot be identified: "+p.SessionID)
}
persistedRoot, err := a.deps.ResolveWorkspaceRoot(meta.Cwd)
if err != nil {
return nil, RPCError(codeInvalidParams, "persisted session workspace is unavailable: "+err.Error())
}
root, err := a.deps.ResolveWorkspaceRoot(cwd)
if err != nil {
return nil, RPCError(codeInvalidParams, err.Error())
}
// ACP session cwd is immutable. Loading history under a different root
// would give a conversation from one workspace access to another
// workspace's configuration, files, and tools.
if !sameWorkspace(root, persistedRoot) {
return nil, RPCError(codeInvalidParams, "session cwd does not match its persisted workspace")
}
// Load history BEFORE publishing the session so no concurrent prompt observes
// a half-initialized session (registerSession sets history under the lock and
// reuses an already-live session rather than orphaning its in-flight turn).
history, historyErr := a.loadHistory(meta.SessionID)
requireHistoryLog := operation == persistedSessionResume && meta.EventCount > 0
history, messages, historyWarning, historyErr := a.loadHistory(meta.SessionID, requireHistoryLog)
// RESUME PROMISES RESTORED CONTEXT, SO A FAILED RESTORATION IS A FAILED
// RESUME. historyErr used to do nothing but suppress replay and raise a
// warning: the session was registered and reported ready regardless, so a
// corrupt record or an unreadable events file left the caller holding a live,
// promptable session ID whose next prompt ran as a brand-new conversation
// under the old identity. Nothing in the response said so.
//
// Publication is now gated on restoration for resume. LOAD keeps the
// best-effort policy deliberately rather than by inheriting this helper: it
// replays what it has and warns about the rest, which is the right trade for
// a client rebuilding a transcript it can still scroll. Reported by @jatmn.
if operation == persistedSessionResume && historyErr != nil {
return nil, RPCError(codeInternalError, "restore session history: "+historyErr.Error())
}
model, models, restrictModels, err := a.resolveModelChoices(ctx, root)
if err != nil {
return nil, RPCError(codeInternalError, "config: "+err.Error())
@@ -204,12 +311,36 @@ func (a *Agent) handleSessionLoad(ctx context.Context, params json.RawMessage) (
models = append(models, SessionConfigOptionValue{Value: persistedModel, Name: persistedModel})
}
}
sess := a.registerSession(meta.SessionID, root, history, model, models, restrictModels)
sess, existed := a.registerSession(meta.SessionID, root, history, model, models, restrictModels)
if existed {
_, messages, historyWarning, historyErr = a.refreshSessionHistory(sess, requireHistoryLog)
if operation == persistedSessionResume && historyErr != nil {
return nil, RPCError(codeInternalError, "restore session history: "+historyErr.Error())
}
}
note := &notifier{conn: a.conn, sessionID: sess.id}
if operation == persistedSessionLoad && historyErr == nil {
for _, message := range messages {
if message.tool != nil {
note.send(*message.tool)
continue
}
note.send(replayMessageChunk(message.role, replayMessageID(message.eventID), message.content))
}
}
a.warnPersistence(
&notifier{conn: a.conn, sessionID: sess.id},
note,
"rehydrate session compaction",
"Could not apply session compaction. Raw session history was restored instead.",
historyWarning,
operation == persistedSessionLoad,
)
a.warnPersistence(
note,
"load session history",
"Could not load session history. The session is open, but earlier turns may be missing until storage recovers.",
historyErr,
operation == persistedSessionLoad,
)
return LoadSessionResult{
ConfigOptions: a.configOptions(sess),
@@ -217,6 +348,80 @@ func (a *Agent) handleSessionLoad(ctx context.Context, params json.RawMessage) (
}, nil
}
func (a *Agent) handleSessionList(_ context.Context, params json.RawMessage) (any, error) {
var p ListSessionsParams
if len(params) > 0 {
if err := json.Unmarshal(params, &p); err != nil {
return nil, RPCError(codeInvalidParams, "invalid session/list params")
}
}
if p.Cursor != "" {
return nil, RPCError(codeInvalidParams, "invalid session/list cursor")
}
items, err := a.deps.Store.ListResumable()
if err != nil {
return nil, RPCError(codeInternalError, "list sessions: "+err.Error())
}
// The filter is the one cwd ACP leaves optional, so a blank one means "no
// filter" rather than an error — but a relative one is rebased onto this
// process's directory exactly like the others, and would then silently answer
// about a workspace the client never named.
var cwd string
if strings.TrimSpace(p.Cwd) != "" {
filter, err := requestedWorkspace(p.Cwd)
if err != nil {
return nil, err
}
cwd, err = a.deps.ResolveWorkspaceRoot(filter)
if err != nil {
return nil, RPCError(codeInvalidParams, err.Error())
}
}
result := ListSessionsResult{Sessions: make([]SessionInfo, 0, len(items))}
for _, item := range items {
// EVERY ENTRY IS RESOLVED, FILTER OR NO FILTER. Listing is a menu, and
// every item on it has to be orderable: activatePersistedSession
// resolves the persisted workspace and refuses what it cannot reach, so
// anything this loop cannot resolve is something the client would be
// offered and then denied.
//
// Resolving only when a cwd filter was supplied left two shapes through —
// a session whose workspace has since been deleted, and a legacy entry
// holding a relative path, which was then reported as cwd "." even though
// ACP requires SessionInfo.cwd to be absolute.
if strings.TrimSpace(item.Cwd) == "" {
continue
}
// A RELATIVE PERSISTED CWD HAS NO RECOVERABLE IDENTITY. Resolving one
// rebases it onto wherever this ACP server happens to be running and
// advertises that invented absolute path as the session's workspace — so a
// conversation created for one project could be resumed against another
// project's configuration, files and tools. The original base is not
// knowable from the metadata, so the honest answer is to omit the entry
// rather than to guess at it.
if !filepath.IsAbs(item.Cwd) {
continue
}
itemRoot, err := a.deps.ResolveWorkspaceRoot(item.Cwd)
if err != nil {
continue
}
if cwd != "" && !sameWorkspace(itemRoot, cwd) {
continue
}
result.Sessions = append(result.Sessions, SessionInfo{
SessionID: item.SessionID,
Title: item.Title,
// The RESOLVED root, not the stored string: absolute as ACP
// requires, and the same value the client will hand back on resume.
Cwd: itemRoot,
UpdatedAt: item.UpdatedAt,
Meta: &SessionInfoMeta{ModelID: item.ModelID, CreatedAt: item.CreatedAt},
})
}
return result, nil
}
// ---- prompt turn ----
func (a *Agent) handleSessionPrompt(ctx context.Context, params json.RawMessage) (any, error) {
@@ -293,6 +498,15 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
note.text("\n\n[zero warning] " + text + "\n")
}
}
// Buffer the complete turn until RunAgent has a valid ACP outcome. A hard run
// failure is an aborted turn: neither live history nor durable history may
// retain its user/tool prefix under an RPC error. StopCancelled is a valid
// outcome and commits through the same boundary below.
pendingEvents := make([]sessions.AppendEventInput, 0, 4)
queue := func(input sessions.AppendEventInput) {
pendingEvents = append(pendingEvents, input)
}
queue(messageEvent("user", userText))
opts := agent.Options{
Cwd: sess.cwd,
@@ -307,8 +521,12 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
Images: images,
OnText: note.text,
OnReasoning: note.thought,
OnToolCall: note.toolCall,
OnToolCall: func(call agent.ToolCall) {
queue(toolCallEvent(call))
note.toolCall(call)
},
OnToolResult: func(result agent.ToolResult) {
queue(toolResultEvent(result))
note.toolResult(result)
if result.Name == "update_plan" {
a.emitPlan(registry, note)
@@ -321,19 +539,36 @@ func (a *Agent) runTurn(ctx context.Context, sess *acpSession, userText string,
agentPrompt := buildPrompt(sess.snapshotHistory(), userText)
result, runErr := a.deps.RunAgent(ctx, agentPrompt, provider, opts)
if result.FinalAnswer != "" {
queue(messageEvent("assistant", result.FinalAnswer))
}
reason, stopErr := stopReasonFor(result, runErr)
if stopErr != nil {
return "", RPCError(codeInternalError, stopErr.Error())
}
if err := a.persistTurn(sess, userText, result.FinalAnswer); err != nil {
a.warnPersistence(
note,
"save session history",
"Could not save session history. This turn is available in memory, but future resume may miss it until storage recovers.",
err,
)
}
// ONE BATCH, ONE LOCK. The outcome gate above decides WHETHER the turn is
// committed; the store batch decides WHICH RECORDS STAY TOGETHER relative to
// other writers, and these are different guarantees. Persisting the buffer
// one event per call took and released the store's session lock between
// events, so a second ACP instance holding the same session — its turnMu is
// its own, only the store is shared — could append its whole turn in the
// gap: 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,
// persistTurn handed the user and assistant events to AppendEvents together;
// buffering until the outcome is known was correct, splitting the commit
// into singleton writes was not. The store writes a batch under one lock and
// one file write, which is the contiguity restored here — not a disk
// transaction, and not a promise about crashes mid-fsync. Reported by @jatmn.
persistenceErr := a.persistTurn(sess.id, pendingEvents)
sess.appendHistory(turnRecord{user: userText, assistant: result.FinalAnswer})
a.warnPersistence(
note,
"save session history",
"Could not save session history. This turn is available in memory, but future resume may miss it until storage recovers.",
persistenceErr,
true,
)
return reason, nil
}
@@ -592,39 +827,159 @@ func (a *Agent) configOptions(s *acpSession) []SessionConfigOption {
// ---- persistence + continuity ----
func (a *Agent) persistTurn(sess *acpSession, user, assistant string) error {
defer sess.appendHistory(turnRecord{user: user, assistant: assistant})
// persistTurn commits a completed turn's buffered events as one store batch, so
// no other writer's records can land between them. A failure is the failure of
// the whole batch: nothing after a failed prerequisite is appended on its own.
func (a *Agent) persistTurn(sessionID string, inputs []sessions.AppendEventInput) error {
if len(inputs) == 0 {
return nil
}
if a.deps.PersistEvents != nil {
return a.deps.PersistEvents(sessionID, inputs)
}
if a.deps.Store == nil {
return nil
}
events := []sessions.AppendEventInput{
{
Type: sessions.EventMessage,
Payload: map[string]any{"role": "user", "content": user},
},
}
if assistant != "" {
events = append(events, sessions.AppendEventInput{
Type: sessions.EventMessage,
Payload: map[string]any{"role": "assistant", "content": assistant},
})
}
_, err := a.deps.Store.AppendEvents(sess.id, events)
_, err := a.deps.Store.AppendEvents(sessionID, inputs)
return err
}
func (a *Agent) loadHistory(sessionID string) ([]turnRecord, error) {
if a.deps.Store == nil {
return nil, nil
func messageEvent(role, content string) sessions.AppendEventInput {
return sessions.AppendEventInput{
Type: sessions.EventMessage,
Payload: map[string]any{"role": role, "content": content},
}
events, err := a.deps.Store.ReadEvents(sessionID)
}
func toolCallEvent(call agent.ToolCall) sessions.AppendEventInput {
return sessions.AppendEventInput{
Type: sessions.EventToolCall,
Payload: map[string]any{
"toolCallId": call.ID,
"name": call.Name,
"arguments": call.Arguments,
},
}
}
func toolResultEvent(result agent.ToolResult) sessions.AppendEventInput {
return sessions.AppendEventInput{
Type: sessions.EventToolResult,
Payload: map[string]any{
"toolCallId": result.ToolCallID,
"name": result.Name,
"status": result.Status,
"output": result.Output,
"changedFiles": append([]string(nil), result.ChangedFiles...),
},
}
}
// persistedMessage is one entry of the restored transcript in stored order. It
// is either a message (role/content) or a tool-call notification (tool), never
// both: keeping them in ONE ordered slice is what preserves the interleaving a
// client needs to redraw the conversation as it originally happened.
type persistedMessage struct {
eventID string
role string
content string
// tool is set for a replayed tool call or its result, and is already in wire
// shape — it comes from the same toolCallStart/toolCallResult mapping the
// live turn uses, so a replayed tool call renders identically to a fresh one
// and there is no second mapping to drift out of sync.
tool *ToolCallUpdate
}
func (a *Agent) loadHistory(sessionID string, requireHistoryLog bool) ([]turnRecord, []persistedMessage, error, error) {
if a.deps.Store == nil {
return nil, nil, nil, nil
}
// THE EFFECTIVE CONVERSATION, NOT THE RAW LOG. A compacted session keeps its
// original prefix on disk alongside an EventCompaction that names the events
// it replaced and carries their durable summary. ReadEvents hands back both,
// so restoring from it replayed superseded turns AND dropped the summary that
// replaced them — resume then seeded the next turn with context the
// conversation had already moved past, and load visibly resurrected
// transcript entries the user had seen compacted away.
//
// ReadRehydratedEvents is the projection the TUI and exec paths already use
// (internal/tui/session.go, internal/sessions/exec_session.go), so all three
// now agree on what the conversation IS. Switching readers alone is not
// enough: rehydration substitutes the compaction event in place of the events
// it replaced, so a loop that skips everything but EventMessage would drop the
// summary exactly as before. It is projected below. Reported by @jatmn.
events, eventLogPresent, err := a.deps.Store.ReadRehydratedEventsWithPresence(sessionID)
var rehydrateWarning error
if err != nil {
return nil, err
rehydrateWarning = err
events, eventLogPresent, err = a.deps.Store.ReadEventsWithPresence(sessionID)
if err != nil {
return nil, nil, nil, err
}
}
if requireHistoryLog && !eventLogPresent {
return nil, nil, nil, fmt.Errorf("zero session %s event log is missing", sessionID)
}
var records []turnRecord
var messages []persistedMessage
activeToolCalls := make(map[string]string)
var pendingUser string
havePending := false
for _, e := range events {
if e.Type == sessions.EventCompaction {
// The summary stands in for the turns it replaced, so it enters the
// history as an assistant message: it is prior context the next turn
// must see, and the wire has no separate role for it.
raw, marshalErr := json.Marshal(e.Payload)
if marshalErr != nil {
continue
}
var payload struct {
Summary string `json:"summary"`
}
if json.Unmarshal(raw, &payload) != nil || strings.TrimSpace(payload.Summary) == "" {
continue
}
messages = append(messages, persistedMessage{
eventID: persistedMessageIdentity(sessionID, e),
role: "assistant",
content: payload.Summary,
})
records = append(records, turnRecord{user: pendingUser, assistant: payload.Summary})
pendingUser = ""
havePending = false
continue
}
// TOOL ACTIVITY IS PART OF THE TRANSCRIPT, NOT DECORATION. A restored
// conversation that shows only the prose reads as if the agent asserted
// its edits and command output from nowhere: the user sees "I updated
// scope.go" with no record that any tool ran. These are collected for
// replay only. They deliberately do NOT enter turnRecord, so the prompt
// context a resumed turn receives stays byte-identical to what it was —
// load and resume continue to consume the SAME effective history, and
// only the wire rendering differs. Reported by @jatmn.
if e.Type == sessions.EventToolCall || e.Type == sessions.EventToolResult {
if upd := replayToolUpdate(e); upd != nil {
rawID := upd.ToolCallID
if e.Type == sessions.EventToolResult {
replayID, ok := activeToolCalls[rawID]
if !ok {
continue
}
upd.ToolCallID = replayID
delete(activeToolCalls, rawID)
} else {
replayID := replayToolCallID(persistedMessageIdentity(sessionID, e))
activeToolCalls[rawID] = replayID
upd.ToolCallID = replayID
}
messages = append(messages, persistedMessage{
eventID: persistedMessageIdentity(sessionID, e),
tool: upd,
})
}
continue
}
if e.Type != sessions.EventMessage {
continue
}
@@ -641,12 +996,14 @@ func (a *Agent) loadHistory(sessionID string) ([]turnRecord, error) {
}
switch msg.Role {
case "user":
messages = append(messages, persistedMessage{eventID: persistedMessageIdentity(sessionID, e), role: msg.Role, content: msg.Content})
if havePending {
records = append(records, turnRecord{user: pendingUser})
}
pendingUser = msg.Content
havePending = true
case "assistant":
messages = append(messages, persistedMessage{eventID: persistedMessageIdentity(sessionID, e), role: msg.Role, content: msg.Content})
records = append(records, turnRecord{user: pendingUser, assistant: msg.Content})
pendingUser = ""
havePending = false
@@ -655,10 +1012,89 @@ func (a *Agent) loadHistory(sessionID string) ([]turnRecord, error) {
if havePending {
records = append(records, turnRecord{user: pendingUser})
}
return records, nil
return records, messages, rehydrateWarning, nil
}
func (a *Agent) warnPersistence(note *notifier, action string, message string, err error) {
// replayToolUpdate rebuilds the ACP notification for one stored tool event.
//
// The stored toolCallId is returned here as a correlation key. loadHistory maps
// each call occurrence to a stable session-wide replay identity and applies that
// identity to its result. Older records wrote the field as "id", so both
// spellings are accepted — the TUI projection already does the same
// (internal/tui/session.go).
//
// A record that carries no usable id is skipped rather than replayed under a
// synthesized one: an unpairable update is worse than a missing row.
func replayToolUpdate(event sessions.Event) *ToolCallUpdate {
raw, err := json.Marshal(event.Payload)
if err != nil {
return nil
}
var payload struct {
Name string `json:"name"`
ToolCallID string `json:"toolCallId"`
ID string `json:"id"`
Arguments string `json:"arguments"`
Status string `json:"status"`
Output string `json:"output"`
ChangedFiles []string `json:"changedFiles"`
}
if json.Unmarshal(raw, &payload) != nil {
return nil
}
id := payload.ToolCallID
if id == "" {
id = payload.ID
}
if id == "" {
return nil
}
if event.Type == sessions.EventToolCall {
upd := toolCallStart(agent.ToolCall{ID: id, Name: payload.Name, Arguments: payload.Arguments})
return &upd
}
status := tools.Status(payload.Status)
if status == "" {
status = tools.StatusOK
}
upd := toolCallResult(agent.ToolResult{
ToolCallID: id,
Name: payload.Name,
Status: status,
Output: payload.Output,
ChangedFiles: append([]string(nil), payload.ChangedFiles...),
})
return &upd
}
func persistedMessageIdentity(sessionID string, event sessions.Event) string {
if event.ID != "" {
return event.ID
}
return fmt.Sprintf("%s:%d", sessionID, event.Sequence)
}
// replayMessageID maps ZERO's stable event identity to a standards-shaped UUID
// without leaking or parsing the event id on the wire. The same stored message
// receives the same opaque id across loads, which also gives clients an exact
// chunk boundary when two adjacent persisted messages have the same role.
func replayMessageID(eventID string) string {
return replayOpaqueID("message", eventID)
}
func replayToolCallID(eventID string) string {
return replayOpaqueID("tool-call", eventID)
}
func replayOpaqueID(kind, eventID string) string {
sum := sha256.Sum256([]byte("zero-acp-" + kind + ":" + eventID))
b := sum[:16]
b[6] = (b[6] & 0x0f) | 0x50
b[8] = (b[8] & 0x3f) | 0x80
return fmt.Sprintf("%x-%x-%x-%x-%x", b[:4], b[4:6], b[6:8], b[8:10], b[10:16])
}
func (a *Agent) warnPersistence(note *notifier, action string, message string, err error, notify bool) {
if err == nil {
return
}
@@ -667,7 +1103,7 @@ func (a *Agent) warnPersistence(note *notifier, action string, message string, e
sessionID = note.sessionID
}
log.Printf("zero acp: failed to %s for session %s: %v", action, sessionID, err)
if note != nil {
if note != nil && notify {
note.text("\n\n[zero warning] " + message + "\n")
}
}
@@ -714,18 +1150,34 @@ func promptImages(blocks []ContentBlock) []zeroruntime.ImageBlock {
// registerSession publishes a session under the agent's lock. If one is already
// registered for id (e.g. a re-load of an in-flight session) the existing live
// session is returned unchanged rather than orphaning its turn or resetting its
// mode/model. history is set BEFORE publishing 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 {
// session is returned rather than orphaning its turn or resetting its mode/model.
// 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) {
a.mu.Lock()
defer a.mu.Unlock()
if existing := a.sessions[id]; existing != nil {
return existing
return existing, true
}
sess := &acpSession{id: id, cwd: cwd, mode: agent.PermissionModeAuto, model: model, models: models, restrictModels: restrictModels, history: history}
a.sessions[id] = sess
return sess
return sess, false
}
// refreshSessionHistory is the activation apply boundary for an already-live
// session. The history is read while prompts are excluded, then applied to the
// same object the prompt path uses. Re-reading here avoids overwriting a turn
// that committed between activatePersistedSession's pre-publication read and
// discovering that another activation had already registered the session.
func (a *Agent) refreshSessionHistory(sess *acpSession, requireHistoryLog bool) ([]turnRecord, []persistedMessage, error, error) {
sess.turnMu.Lock()
defer sess.turnMu.Unlock()
history, messages, warning, err := a.loadHistory(sess.id, requireHistoryLog)
if err == nil {
sess.setHistory(history)
}
return history, messages, warning, err
}
func (a *Agent) session(id string) *acpSession {
@@ -808,8 +1260,48 @@ func (s *acpSession) appendHistory(rec turnRecord) {
s.mu.Unlock()
}
func (s *acpSession) setHistory(history []turnRecord) {
s.mu.Lock()
s.history = append([]turnRecord(nil), history...)
s.mu.Unlock()
}
func (s *acpSession) snapshotHistory() []turnRecord {
s.mu.Lock()
defer s.mu.Unlock()
return append([]turnRecord(nil), s.history...)
}
// sameWorkspace reports whether two resolved roots name the same directory.
//
// STRING EQUALITY IS NOT DIRECTORY IDENTITY. ResolveWorkspaceRoot is abs plus
// filepath.Clean plus a stat: it does not fold case and does not resolve
// junctions, so one directory reached by two spellings produces two different
// roots. A session persisted from the TUI was then unresumable from an editor
// holding a different spelling of the same project folder, and session/list
// filtered by the other spelling returned nothing — not a failed resume but an
// invisible one, on exactly the case this feature exists for.
//
// filepath.EvalSymlinks is NOT the fix on Windows: it normalises a drive letter
// but returns a junction path unchanged, so the alias survives it. Junctions need
// no privilege, so this is ordinary rather than exotic. os.SameFile compares the
// filesystem's own identity for the two directories, which is the question being
// asked.
//
// The string comparison 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.
func sameWorkspace(left, right string) bool {
if left == right {
return true
}
leftInfo, err := os.Stat(left)
if err != nil {
return false
}
rightInfo, err := os.Stat(right)
if err != nil {
return false
}
return os.SameFile(leftInfo, rightInfo)
}
+1684 -13
View File
File diff suppressed because it is too large. Load diff
+43 -10
View File
@@ -89,6 +89,13 @@ type Conn struct {
handlers map[string]HandlerFunc
notifiers map[string]NotifyFunc
// Notifications for one method are an ordered stream: session/update in
// particular is stateful, so dispatching every frame in an unrelated
// goroutine can make a later chunk overtake an earlier one. Different
// methods retain independent tails, keeping session/cancel responsive while
// an update handler is busy.
notifyMu sync.Mutex
notifyTails map[string]chan struct{}
mu sync.Mutex
nextID int64
@@ -106,11 +113,12 @@ type Conn struct {
// lifetime.
func NewConn(r io.Reader, w io.Writer) *Conn {
return &Conn{
rawReader: r,
w: w,
handlers: make(map[string]HandlerFunc),
notifiers: make(map[string]NotifyFunc),
pending: make(map[int64]chan rpcMessage),
rawReader: r,
w: w,
handlers: make(map[string]HandlerFunc),
notifiers: make(map[string]NotifyFunc),
notifyTails: make(map[string]chan struct{}),
pending: make(map[int64]chan rpcMessage),
}
}
@@ -303,11 +311,7 @@ func (c *Conn) handleLine(ctx context.Context, line []byte) {
}(msg)
case msg.isNotify():
if fn := c.notifiers[msg.Method]; fn != nil {
c.wg.Add(1)
go func(m rpcMessage) {
defer c.wg.Done()
fn(ctx, m.Params)
}(msg)
c.dispatchNotification(ctx, msg, fn)
}
default:
// Malformed frame; reply only if we can identify a request id.
@@ -317,6 +321,35 @@ func (c *Conn) handleLine(ctx context.Context, line []byte) {
}
}
// dispatchNotification preserves wire order within one method without making
// unrelated notification methods wait for each other. The read loop installs
// each tail before starting its goroutine, so goroutine scheduling cannot
// reorder the chain it observes.
func (c *Conn) dispatchNotification(ctx context.Context, msg rpcMessage, fn NotifyFunc) {
c.notifyMu.Lock()
previous := c.notifyTails[msg.Method]
done := make(chan struct{})
c.notifyTails[msg.Method] = done
c.notifyMu.Unlock()
c.wg.Add(1)
go func() {
defer c.wg.Done()
defer func() {
close(done)
c.notifyMu.Lock()
if c.notifyTails[msg.Method] == done {
delete(c.notifyTails, msg.Method)
}
c.notifyMu.Unlock()
}()
if previous != nil {
<-previous
}
fn(ctx, msg.Params)
}()
}
func (c *Conn) dispatchRequest(ctx context.Context, msg rpcMessage) {
fn := c.handlers[msg.Method]
if fn == nil {
+80
View File
@@ -360,6 +360,86 @@ func TestConnNotification(t *testing.T) {
}
}
func TestConnNotificationsForOneMethodStayInWireOrder(t *testing.T) {
a, b, stop := connPair(t)
defer stop()
firstStarted := make(chan struct{})
releaseFirst := make(chan struct{})
delivered := make(chan int, 2)
b.HandleNotify("update", func(_ context.Context, params json.RawMessage) {
var in struct{ Sequence int }
_ = json.Unmarshal(params, &in)
if in.Sequence == 1 {
close(firstStarted)
<-releaseFirst
}
delivered <- in.Sequence
})
if err := a.Notify("update", map[string]int{"Sequence": 1}); err != nil {
t.Fatalf("first notify: %v", err)
}
select {
case <-firstStarted:
case <-time.After(2 * time.Second):
t.Fatal("first notification did not start")
}
if err := a.Notify("update", map[string]int{"Sequence": 2}); err != nil {
t.Fatalf("second notify: %v", err)
}
select {
case sequence := <-delivered:
t.Fatalf("notification %d overtook the blocked first notification", sequence)
case <-time.After(25 * time.Millisecond):
}
close(releaseFirst)
for want := 1; want <= 2; want++ {
select {
case got := <-delivered:
if got != want {
t.Fatalf("notification order = ...,%d; want %d", got, want)
}
case <-time.After(2 * time.Second):
t.Fatalf("notification %d was not delivered", want)
}
}
}
func TestConnDifferentNotificationMethodsStayConcurrent(t *testing.T) {
a, b, stop := connPair(t)
defer stop()
updateStarted := make(chan struct{})
releaseUpdate := make(chan struct{})
cancelDelivered := make(chan struct{})
b.HandleNotify("update", func(context.Context, json.RawMessage) {
close(updateStarted)
<-releaseUpdate
})
b.HandleNotify("cancel", func(context.Context, json.RawMessage) {
close(cancelDelivered)
})
if err := a.Notify("update", nil); err != nil {
t.Fatalf("update notify: %v", err)
}
select {
case <-updateStarted:
case <-time.After(2 * time.Second):
t.Fatal("update notification did not start")
}
if err := a.Notify("cancel", nil); err != nil {
t.Fatalf("cancel notify: %v", err)
}
select {
case <-cancelDelivered:
case <-time.After(2 * time.Second):
t.Fatal("different notification method was blocked behind update")
}
close(releaseUpdate)
}
func TestConnMethodNotFound(t *testing.T) {
a, _, stop := connPair(t)
defer stop()
+8
View File
@@ -19,6 +19,14 @@ func agentMessageChunk(delta string) ContentChunk {
return ContentChunk{SessionUpdate: UpdateAgentMessageChunk, Content: TextBlock(delta)}
}
func replayMessageChunk(role, messageID, text string) ContentChunk {
update := UpdateAgentMessageChunk
if role == "user" {
update = UpdateUserMessageChunk
}
return ContentChunk{SessionUpdate: update, MessageID: messageID, Content: TextBlock(text)}
}
func agentThoughtChunk(delta string) ContentChunk {
return ContentChunk{SessionUpdate: UpdateAgentThoughtChunk, Content: TextBlock(delta)}
}
+54 -5
View File
@@ -12,6 +12,8 @@ const (
MethodAuthenticate = "authenticate"
MethodSessionNew = "session/new"
MethodSessionLoad = "session/load"
MethodSessionList = "session/list"
MethodSessionResume = "session/resume"
MethodSessionPrompt = "session/prompt"
MethodSessionCancel = "session/cancel" // notification
MethodSessionUpdate = "session/update" // notification (agent -> client)
@@ -62,8 +64,16 @@ type PromptCapabilities struct {
}
type AgentCapabilities struct {
LoadSession bool `json:"loadSession"`
PromptCapabilities PromptCapabilities `json:"promptCapabilities"`
LoadSession bool `json:"loadSession"`
PromptCapabilities PromptCapabilities `json:"promptCapabilities"`
SessionCapabilities *SessionCapabilities `json:"sessionCapabilities,omitempty"`
}
// Empty capability objects are presence flags in ACP v1. Pointers preserve
// the wire distinction between an advertised `{}` and an omitted capability.
type SessionCapabilities struct {
List *struct{} `json:"list,omitempty"`
Resume *struct{} `json:"resume,omitempty"`
}
type AuthMethod struct {
@@ -117,9 +127,14 @@ type McpServer struct {
}
type NewSessionParams struct {
Cwd string `json:"cwd"`
McpServers []McpServer `json:"mcpServers"`
AdditionalDirectories []string `json:"additionalDirectories,omitempty"`
Cwd string `json:"cwd"`
McpServers []McpServer `json:"mcpServers"`
// AdditionalDirectories is declared by the protocol and consumed nowhere in
// this repo yet. When it is wired up it must go through requestedWorkspace
// like Cwd does: these are client-supplied paths with no absoluteness rule of
// their own, which is the same shape as the resume-cwd hole that made
// {"sessionId":"known"} activate a session against this process's directory.
AdditionalDirectories []string `json:"additionalDirectories,omitempty"`
}
type NewSessionResult struct {
@@ -140,6 +155,39 @@ type LoadSessionResult struct {
Modes *SessionModeState `json:"modes,omitempty"`
}
// ListSessionsParams and the following types implement ACP v1 session/list.
// Transcript contents stay behind session/load; this method returns metadata
// only and leaves the optional pagination cursor opaque.
type ListSessionsParams struct {
Cwd string `json:"cwd,omitempty"`
Cursor string `json:"cursor,omitempty"`
}
type SessionInfoMeta struct {
ModelID string `json:"modelId,omitempty"`
CreatedAt string `json:"createdAt,omitempty"`
}
type SessionInfo struct {
SessionID string `json:"sessionId"`
Cwd string `json:"cwd"`
Title string `json:"title,omitempty"`
UpdatedAt string `json:"updatedAt,omitempty"`
Meta *SessionInfoMeta `json:"_meta,omitempty"`
}
type ListSessionsResult struct {
Sessions []SessionInfo `json:"sessions"`
NextCursor string `json:"nextCursor,omitempty"`
}
// session/resume takes session/load's wire shape, so its cwd is a plain string
// too and an omitted one decodes exactly like an empty one. Sharing the type is
// safe only because activatePersistedSession requires an absolute cwd from the
// request itself — nothing downstream may supply a default for either method.
type ResumeSessionParams = LoadSessionParams
type ResumeSessionResult = LoadSessionResult
// ---- prompt turn ----
type PromptParams struct {
@@ -174,6 +222,7 @@ type SessionNotification struct {
// ContentBlock under "content"; the variant is set via SessionUpdate.
type ContentChunk struct {
SessionUpdate string `json:"sessionUpdate"`
MessageID string `json:"messageId,omitempty"`
Content ContentBlock `json:"content"`
}
+12 -3
View File
@@ -226,11 +226,20 @@ func CompactionPayloadFromPlan(summary string, plan CompactionPlan) (CompactionP
}
func (store *Store) ReadRehydratedEvents(sessionID string) ([]Event, error) {
events, err := store.ReadEvents(sessionID)
events, _, err := store.ReadRehydratedEventsWithPresence(sessionID)
return events, err
}
// ReadRehydratedEventsWithPresence carries the underlying event-log presence
// through compaction projection without changing ReadRehydratedEvents' existing
// empty-on-missing contract.
func (store *Store) ReadRehydratedEventsWithPresence(sessionID string) ([]Event, bool, error) {
events, present, err := store.ReadEventsWithPresence(sessionID)
if err != nil {
return nil, err
return nil, present, err
}
return RehydrateEvents(events)
rehydrated, err := RehydrateEvents(events)
return rehydrated, present, err
}
func (store *Store) ReadReplayEvents(sessionID string) ([]Event, error) {
+14 -5
View File
@@ -813,15 +813,24 @@ func (store *Store) UpdateModel(sessionID string, modelID string) (Metadata, err
}
func (store *Store) ReadEvents(sessionID string) ([]Event, error) {
events, _, err := store.ReadEventsWithPresence(sessionID)
return events, err
}
// ReadEventsWithPresence preserves the distinction between an intentionally
// empty event log and a missing one. Most readers accept both as empty history,
// but activation protocols that promise to restore previously populated
// context need the stronger signal.
func (store *Store) ReadEventsWithPresence(sessionID string) ([]Event, bool, error) {
if !ValidSessionID(sessionID) {
return nil, fmt.Errorf("invalid zero session id %q", sessionID)
return nil, false, fmt.Errorf("invalid zero session id %q", sessionID)
}
data, err := os.ReadFile(store.eventsPath(sessionID))
if err != nil {
if errors.Is(err, os.ErrNotExist) {
return []Event{}, nil
return []Event{}, false, nil
}
return nil, fmt.Errorf("read zero session events: %w", err)
return nil, false, fmt.Errorf("read zero session events: %w", err)
}
// A genuine torn tail is an INCOMPLETE final write — a crash mid-append leaves
// 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) {
if index == lastNonEmpty && tornTailPossible {
break
}
return nil, fmt.Errorf("invalid json in zero session %s %s at line %d: %w", sessionID, EventsFile, index+1, err)
return nil, true, fmt.Errorf("invalid json in zero session %s %s at line %d: %w", sessionID, EventsFile, index+1, err)
}
events = append(events, event)
}
return events, nil
return events, true, nil
}
func (store *Store) timestamp() string {