Skip to content

buzz-acp: resume channel sessions across a restart, and name the channel on session/new - #6088

Open
jhgaylor wants to merge 3 commits into
block:mainfrom
jhgaylor:buzz-acp/resume-sessions-across-restart
Open

buzz-acp: resume channel sessions across a restart, and name the channel on session/new#6088
jhgaylor wants to merge 3 commits into
block:mainfrom
jhgaylor:buzz-acp/resume-sessions-across-restart

Conversation

@jhgaylor

@jhgaylor jhgaylor commented Aug 17, 2026

Copy link
Copy Markdown

Problem

A harness restart — a desktop relaunch, a config-change restart, or a hosted harness moved between nodes on a deploy — empties the in-memory channel → session map (SessionState.sessions), and the next mention in every channel gets a fresh session/new. For an agent whose session is its workspace (a sandbox per session, as with the fountain acp gateway) that discards the channel's memory and files every time. The base prompt tells the agent to "resume silently after a session restart", which assumes the opposite.

Change

1. SessionStore + session/load on restart. A small JSON file (<state-dir>/sessions-<pubkey>.json) remembers, per channel, the session id plus the model and permission mode it was created under. Before opening a new session for a channel with no live one, the pool session/loads the remembered id when the agent advertised loadSession at initialize. Resumed sessions get fresh delivery state and no zero-usage baseline (their prior usage isn't zero).

The receipt is a scoped lifecycle record, not a cache:

  • Scope. The file records the (relay, agent pubkey) pair it belongs to and is ignored otherwise, so one agent identity serving several communities can't resume across them.
  • Compatibility. model and permissionMode are the two things the harness applies from the session/new response and cannot apply at session/load, so a receipt is refused when either has changed. Nothing else is fingerprinted — the system prompt, team instructions, MCP list and memory are delivered per turn, and invalidating on those would wipe every channel's history on a prompt tweak.
  • Lifecycle. invalidate_for_fresh_session is the single seam for "the next turn must not reuse this session": !rotate (idle and busy), a model switch (idle and busy), a token/turn-limit rotation, a cancel, and a failed cleanup clear RAM and disk together. Agent death (invalidate_all) deliberately does not — the harness still wants the conversation back when the agent respawns.
  • Failure classification. A JSON-RPC error is the agent answering: forget and open fresh. A timeout, broken pipe, or mid-load exit is not an answer — the receipt survives and the turn fails retryably rather than opening a session that could compete with one that loaded remotely. Bounded at three consecutive attempts.
  • Durability. Every write re-reads and merges under the lock, so two overlapping harnesses can't lose each other's channels. Directory 0700, file 0600, unique create_new temp file, fsync of file and parent, size and version bounds on parse.
  • Replay. session/updates replayed during session/load no longer reach the owner Activity feed as fresh output; everything else on the wire during a load still does.

A resume that cannot happen never costs the turn: every definitive failure falls back to session/new for that channel.

  • --state-dir / BUZZ_ACP_STATE_DIR (default <cwd>/.buzz-acp)
  • --no-resume-sessions / BUZZ_ACP_NO_RESUME_SESSIONS to opt out (previous behavior)

2. _meta.channelId / _meta.channelType on session/new (AcpClient::session_new_with_origin; session_new_full unchanged, delegates with None). The prompt names the channel only as text in [Context]; sessionTitle carries the channel name and is empty for DMs. An agent or gateway that wants to key sessions by channel — to resume the same conversation after this harness restarts even when its own state directory isn't persistent — had nothing machine-readable to key on. Absent for heartbeat sessions.

Tests

Causal coverage through the real run_prompt_task, driving a scripted ACP agent against an on-disk SessionStore:

  • happy restart — process 1 session/new → persist → process 2 with an empty SessionState → exactly one session/load for the same id and zero session/new;
  • origin parity — a resumed public stream carries the same BUZZ_GIT_ORIGIN_CHANNEL_ID as a fresh one;
  • failure classification — a definitive error replaces the receipt; an indeterminate one retains it and opens no competing session;
  • configuration change — a model change refuses the receipt without attempting the load;
  • lifecycle seam — rotate/switch/limit clear RAM and disk together; agent death clears only RAM.

Unit coverage: session_store (round-trip, remove, malformed, version and scope mismatch, the two-writer lost-update repro, 0600/0700 modes, compatibility matching); acp (channelId/channelType in _meta merging with sessionTitle, session_new_full sending no origin, loadSession capability recorded/left false, session/load request shape and replay tolerance, errors surfacing as Err, replay suppression on the observer feed); config (state-dir default, explicit, opt-out).

Verified by mutation: disabling persistence, the resume lookup, the origin wiring, or replay suppression each fails a test.

cargo test -p buzz-acp --lib 813/0, cargo fmt --all -- --check, workspace cargo clippy --all-targets clean.

Compatibility

Default-on, but strictly additive on the wire: agents that don't advertise loadSession never see session/load; _meta members are ignored by agents that don't read them (as sessionTitle already is).

Related work

Fixes #5342 (channel → session map is in-memory only; every restart opens
session/new and, on Codex, a new cloud thread).

Two open PRs also add a session_store.rs; naming them up front:

# Overlap How this differs
#2633 Durable channel→session map + session/load behind loadSession — same shape as part 1 here Rescoped from a Hermes PR; keyed by (agent command, args); currently in conflict with main and fork CI never ran there. This one is smaller, default-on with --no-resume-sessions, keyed per agent pubkey, and adds part 2 (_meta.channelId/channelType). If #2633 lands first I'll drop the store and rebase part 2 onto it.
#5154 "durable ACP continuity" is one item in a 36-file desktop+acp omnibus This is the buzz-acp-only slice with tests.

Adjacent, not overlapping: #4480 (session/resume for live MCP swaps on the
same session id — argues resume over load to dodge the 60s replay timeout on
long histories; worth weighing as the resume verb here), #6045 (durable
pending-event journal across restart — complementary; happy to align the
state-dir convention if it lands first), #4721 (identity receipts, explicitly
excludes restart continuity).

Points raised in #2633's review, and in @ravarora2's review of this PR, are
addressed in 313d5fd4c: replayed session/updates are suppressed on the
observer feed for the duration of a session/load; load failures are split
into definitive (forget) and indeterminate (retain, retry, bounded); and the
store re-reads and merges under the lock so overlapping harness processes
cannot lose each other's channels. See that
comment
for
the full response, including the one item — a full configuration fingerprint —
where I'm arguing for a narrower scope.

🤖 Generated with Claude Code

@jhgaylor
jhgaylor requested a review from a team as a code owner August 17, 2026 00:07
jhgaylor added a commit to managoat/fountain that referenced this pull request Aug 17, 2026
…id, forwarded from ACP _meta (#775)

A chat harness like buzz-acp keeps its channel→session map in memory and
opens `session/new` again for every channel after a restart; through the
`fountain acp` gateway that was a new conversation and a new sandbox per
restart — every hosted deploy (#774).

- `conversations.channel_id` (opaque, client-supplied). `POST
  /api/conversations` with `channel_id` resumes the latest live conversation
  for the same user + agent + vault + channel (200, `meta.resumed: true`)
  instead of opening a new one (201); terminated/failed ones are past
  resuming and a new one takes over the binding. Without `channel_id`
  nothing changes.
- `fountain acp` reads `_meta.channelId` on `session/new`, forwards it, and
  hands the resumed conversation back as the session id.
- The hosted buzz-acp is built from jhgaylor/buzz@83eda69 (block/buzz#6088:
  `_meta.channelId` on session/new + session store) via a new optional
  `buzz-acp.source` the publish workflow honours; release name
  buzz-acp-v0.5.14-fountain.1. Delete the file and repin to an upstream tag
  once it merges.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@akeemjenkins

Copy link
Copy Markdown

Operator +1 on this. We run a fleet of single tenant Buzz relays (hachiflow) with managed agents supervised by Buzz Desktop, and harness restarts are not an edge case at that layer, they are routine ops: config changes, desktop relaunches, host reboots, moving a harness between nodes. In one four day stretch our telemetry showed three agents restarting in lockstep seven times, and channel sessions averaging barely more than one turn before being discarded. Every one of those restarts was an agent re-learning its channel from a small context window because the channel to session map lived only in process memory.

The forget-and-fall-back rule is the right shape: a resume that cannot happen never costs the turn, so the worst case is exactly today's behavior, and the best case removes the single biggest source of agent amnesia we see in practice.

The _meta.channelId addition quietly matters for hosted harnesses too. In a container or pod the state dir is ephemeral unless someone thinks to mount it, so a gateway that keys sessions by (agent pubkey, channelId) in its own store can resume across a node move even when sessions-.json did not survive. One small ask from the containerized side: a README note that BUZZ_ACP_STATE_DIR should point at a mounted volume when the harness does not live on a laptop, since the /.buzz-acp default will silently vanish on every deploy otherwise.

Would love to see this land.

@ScaleLeanChris

Copy link
Copy Markdown

Tested the current head 83eda69a9 end to end on macOS arm64 with a local relay and Hermes Agent ACP v0.20.1.

Live restart result

  1. Created a fresh private stream and fresh sender/agent identities.
  2. Started this PR's buzz-acp against the real hermes-acp binary.
  3. Sent a first mention containing a private codeword. Buzz opened Hermes session 16963d89-2fda-422b-a08a-dfbc474ff7c1, completed one turn, and persisted that exact channel → session mapping.
  4. Terminated the entire buzz-acp process, then started a new one with the same identity and state directory.
  5. Sent a second mention that did not contain the codeword.
  6. The new harness loaded one remembered mapping and issued session/load for the same session ID. Hermes logged Loaded session 16963d89-2fda-422b-a08a-dfbc474ff7c1; this PR logged exactly one session resumed event; the second process completed exactly one agent turn.
  7. Hermes' persisted session contained two user turns and answered CODEWORD:SILVERBEE after restart.

That verifies the production buzz-acp → real Hermes ACP session/new → persisted map → full process restart → session/load path, not just the store/helper tests.

Automated verification

  • cargo fmt --all -- --check: passed
  • cargo clippy -p buzz-acp --all-targets -- -D warnings: passed
  • cargo test -p buzz-acp --lib: 788 passed, 0 failed

Separate existing reply-path blocker

The first live turn also reproduced the known Hermes/Buzz publication gap: Hermes attempted the correct buzz messages send, but its terminal child did not receive BUZZ_PRIVATE_KEY and the CLI returned auth exit 3. That is tracked in #4923 and NousResearch/hermes-agent#78026; it is independent of this PR's session-resume behavior. The ACP response itself completed normally, which is what allowed the restart/resume assertion above.

Two remaining author actions before merge:

  • DCO Check is currently failing for the PR's commit.
  • Please add the requested README/operator note that BUZZ_ACP_STATE_DIR must point at mounted persistent storage in containers. The CLI help explains it, but the crates/buzz-acp/README.md configuration table does not yet list either new setting.

@jhgaylor

jhgaylor commented Aug 17, 2026

Copy link
Copy Markdown
Author

Thanks both — addressed in 9d5c53c:

  • DCO: the original commit is re-signed (af535a6), and the new one carries the trailer.
  • README: added a Session Persistence table under Configuration listing --state-dir / BUZZ_ACP_STATE_DIR and --no-resume-sessions / BUZZ_ACP_NO_RESUME_SESSIONS, the forget-and-fall-back rule, and the operator callout @akeemjenkins asked for: the <cwd>/.buzz-acp default is ephemeral in containers, so point BUZZ_ACP_STATE_DIR at mounted persistent storage (and gateways can key on _meta.channelId in their own store to survive a node move). Also a one-line Restart step in How It Works.

@ScaleLeanChris thanks for the end-to-end run against real Hermes. I was using Openclaw so that's more data

@jhgaylor
jhgaylor force-pushed the buzz-acp/resume-sessions-across-restart branch from 9d5c53c to 9390060 Compare August 17, 2026 04:12
@jhgaylor

Copy link
Copy Markdown
Author

(Re-signed the docs commit — now 9390060; DCO is green.)

jhgaylor added a commit to managoat/fountain that referenced this pull request Aug 17, 2026
jhgaylor added a commit to managoat/fountain that referenced this pull request Aug 17, 2026
…heck and the sync (#782)

* chore(buzz-acp): repin the fork to the DCO-signed commit of block/buzz#6088

The PR's single commit was amended with Signed-off-by to satisfy Block's DCO
check (83eda69a -> af535a64, identical tree). The old sha is now unreachable
from any branch on the fork, so the pin follows it before GitHub can GC it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* chore(buzz-acp): follow the fork to 9390060 (README docs commit on block/buzz#6088)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(acp): settle/1 tolerates a peer that stopped between the alive check and the sync

The server stops the peer the moment it reports {:done, _}; :sys.get_state
on it can hit noproc. That is the settled state, not a failure — it flaked
partition 2 on 2026-08-17.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@ravarora2 ravarora2 added the triage-ready Appropriate for agentic review label Aug 17, 2026
jhgaylor and others added 2 commits August 17, 2026 22:45
…nel on session/new

A harness restart — a desktop relaunch, a config-change restart, or a hosted
harness moved between nodes on a deploy — emptied the in-memory channel →
session map, and the next mention in every channel got a fresh session/new.
For an agent whose session *is* its workspace (a sandbox per session, as with
the `fountain acp` gateway) that discarded the channel's memory and files each
time; the base prompt tells the agent to "resume silently after a session
restart", which assumes the opposite.

Two changes:

* `SessionStore`: a small JSON file (`<state-dir>/sessions-<pubkey>.json`,
  atomic writes) remembering channel → session id. Before opening a new
  session for a channel with no live one, the pool `session/load`s the
  remembered id when the agent advertised `loadSession` at initialize; any
  failure forgets the id and falls back to `session/new`, so a resume that
  cannot happen never costs the turn. Leaving a channel and `!rotate` forget
  the entry. `--state-dir` / `BUZZ_ACP_STATE_DIR` (default `<cwd>/.buzz-acp`),
  `--no-resume-sessions` / `BUZZ_ACP_NO_RESUME_SESSIONS` to opt out.

* `session/new` now carries `_meta.channelId` and `_meta.channelType`
  (`AcpClient::session_new_with_origin`; `session_new_full` is unchanged and
  delegates). The prompt names the channel only as text; an agent or gateway
  that wants to key sessions by channel — to resume the same conversation
  after this harness restarts even when its own state directory is not
  persistent — had nothing machine-readable to key on. Absent for heartbeat
  sessions.

Tests: store round-trip/malformed/disabled; client sends channelId/type and
merges into existing _meta; loadSession capability recorded; session/load
request shape and tolerance of the pre-response replay; error surfaces;
config defaults and opt-out. Not covered here: a scripted end-to-end through
the pool's get-or-create path (the pool has no scripted-agent test harness
today).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Jake Gaylor <jhgaylor@gmail.com>
…container volume note

Reviewers asked for the README configuration table to list the two settings
introduced with session resumption, and for an operator note that the default
<cwd>/.buzz-acp state dir is ephemeral in containers and must be pointed at
mounted persistent storage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Jake Gaylor <jhgaylor@gmail.com>
@jhgaylor
jhgaylor force-pushed the buzz-acp/resume-sessions-across-restart branch from 9390060 to 40c06e7 Compare August 18, 2026 02:47
@ravarora2

Copy link
Copy Markdown
Contributor

🤖

Thanks for taking on #5342. Durable ACP continuity is useful and needed, and I verified that the basic production path works:

first process:  initialize -> session/new -> session/prompt
persisted:      channel -> session ID
second process: initialize -> session/load(same ID) -> session/prompt

The resumed turn used the same session ID and did not send a second session/new.

I am requesting changes because the durable receipt is currently underspecified and is not kept consistent with the session lifecycle. This creates security/configuration regressions and new stale/corrupt-state failure modes on the exact restart path the PR introduces.

1. Scope the receipt to the complete session identity

The current coordinate is effectively:

file: sessions-<agent-pubkey>.json
key:  channel UUID
value: ACP session ID

It does not bind the session to:

  • canonical relay/community;
  • owner identity;
  • runtime/provider identity;
  • agent command/arguments;
  • model;
  • permission mode;
  • system-prompt/persona version;
  • other session-affecting configuration.

The same agent identity can run in multiple communities. With a colliding channel UUID and shared state directory, one community can select another community's remembered session ID. session/load sends only that opaque ID to the provider, so the provider has no community identity with which to reject the selection.

A restart after a model, prompt, runtime, or permission change can also load an incompatible old session. The load branch does not run the fresh-session path that applies the desired model and permission mode.

Please use a versioned receipt with a full namespace and compatibility fingerprint, for example:

{
  "version": 1,
  "scope": {
    "relay": "canonical relay/community identity",
    "ownerPubkey": "...",
    "agentPubkey": "...",
    "channelId": "..."
  },
  "sessionId": "...",
  "compatibility": {
    "runtime": "...",
    "agentCommandHash": "...",
    "model": "...",
    "permissionMode": "...",
    "systemPromptHash": "...",
    "mcpConfigHash": "..."
  }
}

Hash sensitive or large configuration rather than storing prompt contents. If scope or compatibility does not match, do not load the old session.

2. Make RAM and durable state one lifecycle operation

Several transitions mean “the next turn must use a fresh session.” The current code often invalidates only the in-memory map while leaving the receipt on disk. The next dispatch can therefore reload the session that was intentionally discarded.

Idle !rotate and channel leave remove the receipt, but busy/racy rotation, model switches, automatic token/turn rotation, and damaged-session recovery paths can retain it.

Please centralize lifecycle operations:

remember_new(channel, session, compatibility)
resume(channel, compatibility)
invalidate_for_fresh_session(channel, reason)
replace_after_success(channel, new_session)

Every intentional fresh-session transition must atomically invalidate both RAM and durable state through the same API.

3. Distinguish definitive and indeterminate load failures

The current resume path removes the receipt for every session/load error and immediately falls back to session/new.

I reproduced this through the real run_prompt_task path with a scripted provider returning:

temporary provider overload; retry later

Production sent:

initialize -> session/load -> session/new -> session/prompt

and replaced still-valid-session with replacement-session.

A timeout, broken pipe, process exit, or provider overload is not proof that the remembered session is gone. The load may even have succeeded remotely while Buzz timed out locally, creating two competing sessions.

Please use a typed result:

Loaded
DefinitivelyMissingOrIncompatible
IndeterminateFailure
  • Definitive not-found/incompatible may create a replacement.
  • Indeterminate failure must retain the existing receipt and avoid creating a competing session. Return a retryable error or retry with a bounded policy.
  • Commit the replacement receipt only after session/new succeeds.

4. Preserve stream origin on session/load

The fresh path resolves a public channel as channel_type="stream" and injects:

BUZZ_GIT_ORIGIN_CHANNEL_ID=<channel UUID>

into the session's MCP tool environment.

The resume helper currently calls:

mcp_servers_with_git_origin(
    &ctx.mcp_servers,
    Some(channel_id),
    None,
    ctx.session_title.as_deref(),
)

so a resumed public stream is treated as private/unknown provenance.

I reproduced this through the actual session/load request: the session loaded and prompted successfully, but its MCP environment lacked the required channel-origin variable. Git issues, patches, statuses, and PR metadata created after restart can therefore lose or misstate the link back to the originating Buzz channel.

Pass the already-resolved channel type into the resume path and use one origin-context builder for both session/new and session/load.

5. Make the store safe for overlapping processes

Each SessionStore instance snapshots the whole JSON map and rewrites the whole file without a cross-process lock or compare-and-swap.

I reproduced a deterministic lost update:

store A opens empty file
store B opens empty file
A writes channel A
B writes channel B from its stale snapshot
reopen -> channel B exists, channel A is gone

This matters for the advertised node-move/rolling-deployment scenario, where old and new harnesses can overlap.

Use either:

  • a singleton process lease for each store namespace; or
  • transactional per-record storage; or
  • a cross-process lock with re-read/merge/write under the lock.

Writes should also use a unique temporary file, fsync the file and parent directory where durability is promised, and define crash recovery.

6. Add causal production-path tests

The committed tests cover SessionStore and ACP helpers separately, but none drives:

session/new -> persist -> process restart/empty RAM
-> session/load same ID -> prompt

Mutation testing showed the session-focused tests still passed when production resume, production persistence, or production origin wiring was disabled.

Please commit a scripted-ACP fixture around the real run_prompt_task path and cover:

  1. Happy restart: same session ID, exactly one session/load, no second session/new.
  2. Community isolation: same agent pubkey and channel UUID, different relay/owner scope, shared state directory.
  3. Configuration change: runtime/model/prompt/permission incompatibility starts fresh or safely reconciles before prompting.
  4. Rotation matrix: busy !rotate, model change, automatic limits, and damaged-session recovery never reload the discarded ID.
  5. Failure classification: definitive not-found replaces; timeout, IO, process exit, and provider overload retain the receipt and do not create a competing session.
  6. Stream origin: fresh and resumed MCP environments carry the same channel UUID.
  7. Real concurrent processes: two writers retain both records.
  8. Causal mutations: disabling persistence, resume, or origin makes the relevant test fail.

Also cover the documented replay behavior: session/update messages replayed during session/load should not appear in the owner Activity feed as fresh output.

Hardening

Under a normal umask 022, the current receipt file is 0644, and the predictable .json.tmp path can be symlinked. Default Desktop placement may be protected by an owner-only parent directory, so I do not treat that alone as the primary Desktop blocker. However, the PR explicitly documents custom container/PVC state directories. For that deployment contract, create an owner-only directory/file, bound and version parsing, reject unsafe symlink paths, and use unique atomic temporary files.

Bottom line

The feature's happy path is real, but this head can:

  • select a session outside the intended community/configuration scope;
  • reload a session that rotation or safety logic discarded;
  • destroy a valid receipt after a transient failure;
  • lose public-channel Git provenance after restart;
  • lose unrelated channel mappings under overlapping processes;
  • pass all committed tests when production wiring is removed.

Please keep the feature, but make the durable receipt a scoped, versioned lifecycle record rather than a channel-to-string cache.

Current PR head checked: 40c06e7cb0166e36dae6c3e8e1a27d404c2371b5.

Addresses review feedback on block#6088. The durable channel -> session map was
a plain cache, and several paths that deliberately abandon a session left
the receipt behind for the next turn to resume.

Lifecycle. `invalidate_for_fresh_session` is now the single seam for "the
next turn must not reuse this session": `!rotate` (idle and busy), a model
switch (idle and busy), a token/turn-limit rotation, a cancel, and a failed
cleanup all clear RAM and disk together. Agent death (`invalidate_all`)
deliberately does not — the harness still wants the conversation back when
the agent respawns, and a session the agent truly lost is forgotten by
`session/load` failing. Without this a `!rotate` during an in-flight turn,
or any MaxTokens rotation, resumed the very session that was discarded, and
an idle model switch silently no-oped.

Scope and compatibility. The file records the (relay, agent pubkey) pair it
belongs to and is ignored otherwise, so one agent identity serving several
communities cannot resume across them. Each receipt records the model and
permission mode the session was created under and is refused when either
changed: both are resolved from the `session/new` response, which
`session/load` does not return, so resuming across such a change would
silently run the old configuration. Nothing else is fingerprinted — the
system prompt, team instructions, MCP list and memory are delivered per
turn, and invalidating on those would wipe every channel on a prompt tweak.

Failure classification. A JSON-RPC error is the agent answering, so the
receipt is dropped and a fresh session opened. A timeout, broken pipe, or
mid-load exit is not an answer: the receipt survives and the turn fails
retryably rather than opening a session that could compete with one that
loaded remotely. Bounded at three consecutive attempts.

Stream origin. The resume path passed `None` for channel type, so a resumed
public stream got `BUZZ_GIT_ORIGIN_AGENT_NAME` instead of
`BUZZ_GIT_ORIGIN_CHANNEL_ID` and lost git provenance across a restart. It
now uses the same resolved type as `session/new`.

Store durability. Every write re-reads and merges under the lock, so two
overlapping harnesses cannot lose each other's channels to a stale
snapshot. Directory 0700, file 0600, unique `create_new` temp file, fsync
of file and parent, size and version bounds on parse.

Observer replay. `session/update`s replayed during `session/load` no longer
reach the owner Activity feed as fresh output; everything else on the wire
during a load still does.

Tests. Causal coverage through the real `run_prompt_task` against a
scripted ACP agent and an on-disk store: happy restart (one `session/load`,
no second `session/new`), definitive vs indeterminate failure, a
configuration change refusing the receipt, origin parity between fresh and
resumed, and the lifecycle seam. Verified by mutation — disabling
persistence, resume, origin wiring, or replay suppression each fails a test.

Signed-off-by: Jake Gaylor <jhgaylor@gmail.com>
@jhgaylor

Copy link
Copy Markdown
Author

Thanks @ravarora2 — this was a genuinely useful review. Most of it is now addressed in 313d5fd4c; one item I'm pushing back on, with reasoning.

Taken

2. RAM and durable state as one lifecycle operation. You were right, and it was worse than "the receipt can go stale." Concretely, on the old head:

  • !rotate on a busy channel took the signal_in_flight_task branch, which never removed the receipt — the next turn resumed the very session the owner rotated away from.
  • switch_idle_agent_model invalidated RAM only, and the resume path never applied desired_model (only create_session_and_apply_model does), so an idle model switch silently no-opped.
  • A MaxTokens / max_turns_per_session rotation (pool.rs, the should_rotate block) had the same shape — the maxed-out session came straight back.

There is now a single seam, invalidate_for_fresh_session(state, store, source), and every state.invalidate(&source) in run_prompt_task routes through it, along with the two idle paths in lib.rs. The rule is stated in its doc comment: the store exists to survive a process restart, never an in-process decision to abandon a session.

One deliberate exception: the invalidate_all paths (agent exited, hard timeout) keep the receipt. Those are agent death, not a decision to start over — the harness still wants the conversation back when the agent respawns, and a session the agent truly lost gets forgotten by session/load failing definitively. Flagging it explicitly since your list included "damaged-session recovery."

3. Definitive vs indeterminate. Adopted, and your framing ("the load may have succeeded remotely while Buzz timed out locally") is the part that convinced me — the risk isn't just a destroyed receipt, it's two live sessions. ResumeAttempt is now Resumed / StartFresh / Indeterminate. A JSON-RPC error is the agent answering → forget and open fresh. A timeout, broken pipe, or mid-load exit → keep the receipt, and the turn fails retryably rather than opening a competing session. Bounded at three consecutive indeterminate attempts (MAX_INDETERMINATE_RESUMES, in-memory) so a channel can't stall forever.

4. Stream origin. Confirmed and fixed. mcp_servers_with_git_origin only emits BUZZ_GIT_ORIGIN_CHANNEL_ID for channel_type == Some("stream"), so passing None dropped a resumed stream into the BUZZ_GIT_ORIGIN_AGENT_NAME arm. The resume path now takes the already-resolved origin_channel_type, and there's a test asserting parity between the fresh session/new and the resumed session/load rather than just asserting the var is present.

5. Overlapping processes. Fixed, though not with a lease. Every write now re-reads the file inside the lock, applies the single operation to what's actually on disk, and renames a fresh temp file over it — so your A/B repro keeps both records. Last-writer-wins on a single channel is retained deliberately: that's the correct resolution, since it's the session most recently created for it. A lease or per-record storage would buy nothing more for this failure mode; say the word if you disagree.

Hardening. Directory 0700, file 0600, uniquely-named temp opened create_new (which is what makes the symlink-preplacement path unusable), fsync of both file and parent directory, and size + version bounds on parse. Agreed this matters specifically because the PR invites operators to point BUZZ_ACP_STATE_DIR at container storage.

6. Causal tests — and a correction. My PR body said "the pool has no scripted-agent test harness today." That was wrong; pool.rs already drives the real run_prompt_task against a bash-scripted ACP agent with a request capture file. So there was no obstacle and I've added the coverage:

  • happy restart: process 1 session/new → persist → process 2 with empty SessionState → exactly one session/load for the same id, zero session/new;
  • definitive failure replaces, indeterminate failure retains and opens nothing;
  • a configuration change refuses the receipt without even attempting the load;
  • origin parity between fresh and resumed;
  • the lifecycle seam clearing RAM and disk together, and agent death not clearing it;
  • community isolation and the lost-update repro (store-level).

On your mutation point: I checked these the same way. Reverting the origin wiring, disabling the put after session/new, stubbing out the resume lookup, and disabling replay suppression each fail a test.

Replay and the observer feed. Fixed here rather than deferred. session/update notifications replayed while a session/load is outstanding no longer emit acp_read; the load's own response and any agent-initiated request still do, and the flag clears on every exit path so real output after the load is observed normally.

Partly taken — item 1

I've taken the scoping half. The file records the (relay, agent pubkey) pair it was written for and is treated as empty otherwise, so the shared-state-directory case you describe can't select another community's session.

I've taken a two-field compatibility check rather than the full fingerprint, on a principle I'd like to argue for: the receipt records exactly what the harness applies at session/new and cannot apply at session/loadmodel and permissionMode. Both are resolved from the session/new response (resolve_model_switch_method reads its configOptions/models; agent_supports_mode reads its modes), which session/load does not return. A resumed session silently keeps what it was created with, so resuming across a change to either would run the old configuration while the operator believes the new one is live. That's load-bearing, and it's why your model-change concern is real.

The other fields I'm not including: systemPromptHash, mcpConfigHash, agentCommandHash, runtime. Those are all delivered per turn or re-sent on the resumed session — a change to them doesn't require abandoning the conversation. Including them would mean any prompt or MCP tweak wipes every channel's history on the next restart, which defeats the feature for the operators asking for it (see @akeemjenkins above — seven lockstep restarts in four days). The failure mode we'd be trading into is worse and far more frequent than the one we'd be preventing.

If you think a specific one of those does change behavior in a way session/load can't reconcile, name it and I'll add it — that's the test I'd apply to any new field. The format is versioned (version: 1) precisely so adding one later is cheap.

Docs updated to describe all of the above. cargo test -p buzz-acp --lib 813/0, cargo fmt --all --check, workspace clippy clean.

mfethe1 added a commit to mfethe1/buzz that referenced this pull request Aug 24, 2026
…hardening)

Cleaned integration branch, not a raw PR head:
- base: block#6682 SQLite/WAL store + durable processed_events dedupe
- ported: block#6088 owner-only 0600/0700 perms + repair-on-open
- 2 new regression tests assert file MODE (negative control: 7/2 fail without)

Source branch: integration/session-store (based on origin/main f6e6617)
mfethe1 added a commit to mfethe1/buzz that referenced this pull request Sep 2, 2026
…WAL + processed_events)

Base chosen over block#6088 on verified grounds:
- block#6682 merges clean vs origin/main (merge-tree rc=0); block#6088 conflicts (rc=1)
- block#6682 durably dedupes relay events (processed_events table); block#6088 leaves
  duplicate-after-restart unguarded (queue.rs untouched)
- block#6682 has worker-slot keying (ContextKey::for_worker); block#6088 has a flat
  channel->session map with no worker discriminator

Upstream base: f6e6617. PR head: c11e582

Signed-off-by: Michael Fethe <mfethe1@gmail.com>
mfethe1 pushed a commit to mfethe1/buzz that referenced this pull request Sep 2, 2026
…ed from block#6088)

block#6682 opens the SQLite store with a bare Connection::open, leaving the DB at
the process umask (commonly 0644 = world-readable). Bindings are not secrets
but they name channels, agent pubkeys and workspace paths.

Ported block#6088's permission discipline onto block#6682's SQLite base:
- store directory created 0700
- database created 0600 BEFORE SQLite opens it, so its bytes are never
  briefly world-readable (SQLite derives -wal/-shm modes from the main file)
- an existing loose-permission store is REPAIRED on open, so the fix reaches
  deployments created before this landed

Two regression tests assert the MODE, not the content. block#6682's shipped
session_store_file_contains_ids_only passes even when the file is
world-readable, so it cannot catch this class of regression.

Negative control: removing the hardening fails exactly these 2 tests
(7 pass / 2 fail); restoring returns 9/9.

Upstream base: f6e6617
block#6682 head: c11e582
block#6088 reference: session_store.rs:324 (0o600), :350 (0o700), :520-521 (test)

Signed-off-by: Michael Fethe <mfethe1@gmail.com>
mfethe1 added a commit to mfethe1/buzz that referenced this pull request Sep 2, 2026
…hardening)

Cleaned integration branch, not a raw PR head:
- base: block#6682 SQLite/WAL store + durable processed_events dedupe
- ported: block#6088 owner-only 0600/0700 perms + repair-on-open
- 2 new regression tests assert file MODE (negative control: 7/2 fail without)

Source branch: integration/session-store (based on origin/main f6e6617)

Signed-off-by: Michael Fethe <mfethe1@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-ready Appropriate for agentic review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] buzz-acp loses channel sessions on restart and creates duplicate Codex cloud threads

4 participants