fix(acp): fail closed on indeterminate sessions

[OpenSpek]

Summary

This is a focused cooperative follow-up to block/buzz#2633 for the durable ACP session-restore failure path.

Authored and DCO-signed by Luke Westlake; published through the OpenSpek GitHub account and fork.

• represent ambiguous session/load and session/new outcomes as safety-blocked rather than falling through to replacement creation
• commit per-binding write-ahead Creating / Restoring guards before stateful ACP wire access
• retain an exclusive per-binding OS lease through restore and active in-process ownership
• require exact binding, guard, returned-session-ID, and durable commit checks before observer, prompt, history, or result publication
• fail closed on store, lock, parse, guard, persistence, and commit failures without queue retry, binding deletion, or overwrite
• keep restore guards separate from the legacy version-1 whole-file binding map so an older writer cannot erase corrected-process quarantine metadata
• emit an actionable best-effort notice naming BUZZ_ACP_SESSION_STORE

Verification

• cargo test -p buzz-acp — 614 unit tests, 9 integration tests, and doc tests passed
• cargo fmt --all -- --check
• cargo clippy -p buzz-acp --all-targets -- -D warnings
• cargo clippy --workspace --all-targets -- -D warnings
• git diff --check

The desktop Biome gate remains blocked in this Windows checkout by 1,640 pre-existing CRLF formatting diagnostics. This patch changes no desktop, web, or mobile files.

Scope and threat-model boundary

This patch is intentionally limited to durable channel-session resolution. It does not include broader session listing/selection, provenance, workspace, or privacy changes.

Corrected processes coordinate through the new guard and lease files. An already-running older Buzz binary cannot be forced to honor those files, so operators must stop every Buzz ACP process sharing the store before deploying or manually reconciling the sidecar. This does not claim safety against malicious/admin guard deletion, path aliases that select different store identities, network filesystems with unreliable lock/rename/sync semantics, ACP provider protocol violations, or storage hardware that falsely acknowledges durability.

Built against Joel's exact feat/hermes-native-profile head 7b76647e7dff22b59896f56e22b6ec1eba85a5bf.

[joelbrilliant]

Thanks for this, and for the DCO and clean history. It is green here too: 614 tests, clippy clean at -D warnings, fmt clean.

The contract logic is right. SessionRestoreFailure, session_resolution_after_load_failure and the two regressions are what we agreed, and every_session_load_failure_blocks_new_session_fallback closes the caller path I got wrong.

I do not want to merge it into block#2633 as one piece though, and I would rather say why than sit on it.

It is +1308/-181. block#2633 is currently +701/-7, so this is close to twice the PR it patches. What is in it goes past the bounded contract: a per-binding guard file family with its own RESTORE_GUARD_VERSION and write-ahead Creating/Restoring/Ready states, a new on-disk layout with hashed filenames, and an exclusive per-channel OS lease taken with try_lock_exclusive and held for the process lifetime.

That last one is what I most want reviewed on its own. block#2633's store takes its lock per operation on purpose so several buzz-acp processes can share it. An exclusive per-channel lease held for the life of the process is a different concurrency model. It may well be the better one and the fencing argument is real, but it changes a documented property of the module, and it should be argued for in a PR that is about that rather than arriving inside a persistence PR.

Same for the guards. The write-ahead approach is more crash-safe than anything I would have written, and your own note that rolling mixed-version safety cannot be enforced by the module is exactly what reviewers should be looking at directly.

The practical reason: block#2633 sat with no reviewer for five days at +1051 alongside three competing PRs, and only picked up review interest once it came down to doing one thing. Merging this makes it a ~2000 line PR spanning persistence, a write-ahead log and a new locking model, and I think that puts it straight back where it was.

So what I would ask, if you are willing:

1. Into feat(acp): Hermes runtime discovery and durable session load block/buzz#2633: the minimal contract only. The failure enum, the resolution function, the lib.rs error surface, the two regressions. The honest catch is that mark_indeterminate_if_equals currently rides on the guard files, so a bounded version needs the marker done more simply, a field on the existing v1 StoreFile rather than a new artefact family. Less crash-safe than yours, which is fine as an interim because the guards then upgrade it.
2. Your own PR: guards, leases, state machine, observer restructuring. I will review it, and I would rather it landed on its own merits than by association with mine.

If reworking the marker is annoying, say so and I will do that part and credit you for the contract design. I would just rather not be the reason your locking work gets rubber-stamped inside someone else's PR.

[OpenSpek]

Thanks — I split the bounded contract out as requested and did not open a competing PR.

Minimal branch: https://github.com/OpenSpek/buzz/tree/fix/acp-indeterminate-minimal
Commit: OpenSpek@85eda79

This contains only the four-file contract for the #2633 route:

• strict fail-closed session/load result validation;
• SessionRestoreFailure and binding-first resolution;
• a simple indeterminate marker in the existing v1 StoreFile;
• notice/drop/no-replay handling with synchronized-process reuse vs wire-ambiguity respawn;
• focused persistence, malformed-response, no-fallback, and metadata-safety regressions.

It excludes guards, process-lifetime leases, the broader state machine/new disk layout, and general observer restructuring.

A fail-closed independent review initially found that legacy store filenames could encode agent args and that restore observer context inherited the channel UUID. Both were corrected: relay-facing surfaces now use only a SHA-256-derived opaque store reference, exact paths remain local-only diagnostics, and the restore turn_error has no channel/session IDs. The corrected snapshot received a CLEAN independent re-review.

Verification on the exact tree:

• cargo fmt --all --check
• cargo test -p buzz-acp — 610 library + 9 integration tests passed
• cargo clippy -p buzz-acp --all-targets -- -D warnings
• git diff --check
• added-line secret/injection scan clean

The commit is DCO-signed and ready to cherry-pick into the feature branch if this route suits you. PR #1 remains the separate review vehicle for the broader guards/leases/state-machine work.