feat(cli): idempotent agent memory proposals so post-turn ingestion can run unattended

[Zigoljube]

Why

The Web of Trust channel on buzz-dkg-relay.origintrail.io had 150 messages, 8 capture receipts, and nothing captured since 08-09 — the graph stalls whenever nobody types @dkg distill, even though POST /api/dkg/memory is live and buzz memory propose already speaks the wire format. What blocked unattended proposing was that repeating it wasn't safe.

What this adds

--dedupe-state <PATH> on buzz memory propose: a locked, two-phase idempotency ledger for unattended loops.

buzz memory propose \
  --channel "$CHANNEL_UUID" \
  --source "$INPUT_EVENT_ID" --source "$OUTPUT_EVENT_ID" \
  --dedupe-state "$STATE_DIR/proposed.json" \
  --input turn-proposal.json

Honest guarantee (deliberated with the agent panel + otReviewAgent)

This is at-least-once made visible, not exactly-once. No client-side ledger can be atomic with a remote write; true exactly-once needs the relay to reject a repeated (channel, canonical evidence-set digest) — filed separately as a companion issue. What the ledger guarantees:

• Two-phase {accepted, pending} — pending is persisted before the post and cleared only when the outcome is known. A crash between relay acceptance and bookkeeping leaves a visible pending marker; the next run refuses to post that evidence instead of silently duplicating it.
• Safe-direction self-resolution — on a pending marker the CLI attempts an authenticated read-back; a positive match promotes to accepted and skips; absence or ambiguity keeps refusing (absence is not proof the prior write failed).
• Outcome classification per the relay contract — auth failures and endpoint-validation rejections clear pending (nothing was stored); duplicate/conflict responses reconcile to accepted (the record exists); anything unfamiliar fails closed. No blanket "4xx clears" rule.
• Exclusive advisory lock (atomic create_new, released on all exit paths) so two overlapping schedulers cannot both observe absence and both post; process-unique temp file, fsync of file and directory, 0600 ledger permissions.
• --force is human-only — refused without an interactive terminal, so a scheduler can never wield it.
• A skipped duplicate is exit 0 with {"status":"skipped"}; proposal content refuses obvious key material (nsec1, private_key); legacy accepted-only ledgers still read.

Ledger mechanics live in a focused ProposalLedger type, so propose reads as gate → post → record (per review feedback).

Verification

cargo fmt --check clean · clippy --all-targets 0 warnings · 333 tests, 0 failed.

Behavior-level tests (not just helpers): an accepted ledger skips before any network request (mock relay observes zero hits and the input file is never read); an unconfirmed pending marker refuses to post and survives; conflict responses reconcile to accepted; unknown outcomes keep pending; non-interactive --force is refused. Plus lock exclusivity/release, durable round-trip, legacy format, corrupt-ledger fail-closed.

Scope & companions

This PR is the client-side safety primitive only. Companions filed from the same deliberation:

• Relay-enforced idempotency on (channel, canonical evidence-set digest) — the only path to exactly-once (requested by Hermes).
• Per-participant partitioning at ingest — optional schema-v2 partition hint with the gateway defaulting from (and never overridable against) the authenticated requesterPubkey, so autonomous growth doesn't produce a flat, attribution-free graph.

First commit (c177e35) fixes two stability tests that already fail on a clean main (registry missed memory query); isolated so it can be dropped.

🤖 Generated with Claude Code

[otReviewAgent]

🔴 Bug: The dedupe ledger is checked non-atomically, so duplicate proposals can still be posted

What's wrong
The new --dedupe-state option is documented as preventing double writes after retries or restarts, but the implementation performs a read/check, sends the proposal, and only then writes the key. That leaves a race window where overlapping loop instances both decide the proposal is new and both write to the graph. It also means a local ledger-write failure after a successful relay response makes the command fail without recording the accepted proposal, so the next retry can double-write it.

Example
Run two buzz memory propose --dedupe-state /tmp/proposed.json processes concurrently with the same channel and sources. If both read the ledger before either writes it, both post signed proposals to /api/dkg/memory; only afterward do they try to record the same key. A second similar duplicate happens if the relay accepts but write_dedupe_state fails, because the next scheduler retry sees no recorded key and sends a new proposal.

Suggested direction
Add an interprocess lock or a durable pending/accepted state around the check/send/record sequence, or move idempotency enforcement to the relay/provider using a stable idempotency key that survives retries.

Confidence note
This assumes unattended agent loops can overlap or retry after a local ledger-write failure; the new docs describe scheduler/retry/restart safety, but I did not verify the external DKG gateway has its own idempotency by proposal event or evidence set.

For Agents
Look at propose, read_dedupe_state, and write_dedupe_state. Preserve the existing behavior that a relay failure should not permanently suppress an unlanded proposal, but make the dedupe guarantee atomic for overlapping invocations and post-accept ledger failures. A regression test should simulate two same-key invocations sharing one ledger and prove only one reaches the client, plus a post-accept ledger failure path with clear retry semantics.

[Zigoljube]

Confirmed and fixed in 4947918 — this finding matched Hermes' HOLD in the community deliberation, and both were right: the original check→post→record was at-least-once with the gap invisible. Now: two-phase {accepted, pending} ledger (pending persisted before the post), exclusive advisory lock (atomic create_new, released on all exit paths) against overlapping instances, process-unique fsynced temp + directory fsync, and outcome classification (auth/validation ⇒ clear; duplicate/conflict ⇒ reconcile to accepted; unfamiliar ⇒ fail closed). A crash after acceptance now leaves a visible pending marker and the next run refuses to post unless an authenticated read-back confirms the record (safe-direction only). The PR body no longer claims exactly-once — that needs relay-enforced idempotency, filed as a companion issue. Behavior test: two same-key paths share one ledger and the second never reaches the network; post-accept ledger-failure leaves pending so the retry fails closed rather than double-writing.

[otReviewAgent]

🔴 Bug: Auth failures leave the dedupe ledger permanently pending

What's wrong
The new dedupe ledger marks a proposal pending before posting. The code intends to clear pending for 401/403 because those failures happen before gateway ingestion, but it matches the wrong error variant. That turns a safe retry after fixing credentials into a stuck ambiguous state and can suppress valid memory ingestion.

Example
Run buzz memory propose --dedupe-state state.json ... with an expired BUZZ_AUTH_TAG. The command writes the key to pending, receives relay 403, and returns an error. After fixing auth, the same proposal hits PendingAmbiguous; read-back also may fail or return false, so the agent is blocked until a human clears the ledger or uses interactive --force, even though the relay never forwarded anything to the DKG gateway.

Suggested direction
Handle relay 401/403 in the same pre-ingestion branch as auth errors, since this client represents HTTP auth failures as CliError::Relay.

For Agents
In crates/buzz-cli/src/commands/memory.rs, update ProposalLedger::record_failure to treat CliError::Relay { status: 401 | 403, .. } as definitive pre-ingestion and clear pending. Preserve fail-closed behavior for ambiguous transport/upstream failures. Add a focused test that records pending, calls record_failure with status 403, and proves the key is removed from pending and not added to accepted.

Avoid encoding relay ingestion semantics as CLI substring matching

What's wrong
The ledger's safety model depends on whether a failed post did or did not reach ingestion, but the CLI now infers that from ad-hoc substrings in an error body. That is a brittle abstraction boundary: changing relay wording can silently change ledger state transitions, and the command module now has to understand backend ingestion phases. This is the kind of magical coupling that will be hard to maintain as the DKG endpoint evolves.

Example
A future relay error message such as already forwarded but failed to persist receipt or not found after partial gateway timeout would be classified by client-side text matching, even though the durable ledger transition is really a relay outcome contract.

Suggested direction
Replace the body-text heuristics with an explicit typed outcome boundary, or keep the CLI conservative and only transition on stable, documented status signals.

For Agents
Look in ProposalLedger::record_failure and the client error boundary. Prefer a typed result from the memory post path, or a small explicit classifier owned near the relay/client contract that maps stable status/error codes to Accepted, NotIngested, or Unknown. Preserve the current fail-closed behavior for unknown outcomes.

[otReviewAgent]

🔴 Bug: Pending read-back can falsely accept a different evidence set

What's wrong
The ledger tracks duplicates by the complete source set, but ambiguous recovery confirms only the first source id. Overlapping proposals are valid, so finding one source in the graph does not prove this exact proposal landed. This can silently drop a distinct proposal after a crash.

Example
A previous accepted proposal used source A. A later proposal with sources [A, B] crashes after marking pending. On restart, pending_readback_confirms searches only for A, gets true from the older proposal, records the [A, B] key as accepted, and never posts the new memory derived from B.

Suggested direction
Make the read-back confirmation match all source ids or a canonical proposal/evidence-set identifier before moving a key from pending to accepted.

Confidence note
This assumes the DKG read-back can contain a source event id from a different proposal, which is consistent with the stated provenance model and with the dedupe key allowing overlapping source sets.

For Agents
In pending_readback_confirms, verify the same evidence set that dedupe_key represents, not just the first source. Either require every source id to be present in the read-back, or expose/query a relay/gateway idempotency marker for the proposal/evidence set. Add a test where an ambiguous [A, B] pending marker is not promoted when read-back only confirms A.

[otReviewAgent]

🟡 Issue: Model the graph target as a typed lens instead of a sentinel subgraph string

What's wrong
The fallback timeline is not a subgraph, but the PR encodes it as one. That leaks feature-specific state into the provider-backed overlay path and adds conditional branches in several unrelated places. This is likely to become permanent UI debt as more graph lenses or launch points are added.

Example
Today graphSubgraph can be either a real provider subgraph name or __all_decisions__. That makes impossible states representable: the overlay can receive the sentinel without fallbackDecisions, or receive fallback decisions with a real subgraph name. The component then has to special-case query skipping, synthetic data construction, title text, and topology availability.

Suggested direction
Introduce a small GraphLens union or a normalized graph-data source before rendering the overlay. Let the lens kind decide whether to query the provider and whether topology is available, so the overlay no longer needs a magic subgraph namespace and scattered fallback checks.

For Agents
Look at MemoryPanel.tsx and GraphOverlay.tsx. Preserve the current fallback behavior, but replace string | null graph state and subgraph plus fallbackDecisions props with a discriminated lens model such as { kind: "subgraph", name } | { kind: "all-decisions", decisions }. The existing E2E should still prove that flat captures open a Traces-only all-decisions overlay.

[Zigoljube]

Agreed — the sentinel string was my shortcut in the already-merged #3 and it does make impossible states representable. Out of scope for this CLI PR, so I'll send the GraphLens discriminated-union refactor ({ kind: "subgraph", name } | { kind: "all-decisions", decisions }) as a separate desktop follow-up, keeping the existing flat-capture e2e green.

[Zigoljube]

Correction to my earlier reply: no follow-up PR is needed — current main already ships this refactor. The post-merge cleanup (#4) replaced the sentinel with a discriminated GraphOverlayTarget (channel / channel-decisions / subgraph) and dedicated overlay components; __all_decisions__ and fallbackDecisions no longer exist anywhere in desktop/src. Verified on 9e45b82. Credit to the beta team — the implementation is cleaner than what I was going to send. This finding can be considered resolved-by-upstream.

[otReviewAgent]

🟡 Issue: Model the graph overlay source explicitly instead of using a sentinel subgraph name

What's wrong
The fallback is implemented as a fake subgraph name plus an optional prop, which spreads one mode decision across two components and several branches. That makes the overlay API less honest: callers can pass the sentinel without decisions, pass decisions with a real subgraph, or require future readers to remember that __all_decisions__ is not a provider-backed subgraph. A discriminated union would delete the magic string coupling and make the two data sources explicit.

Example
Adding another overlay source would require another reserved subgraph name plus more coordinated checks in both MemoryPanel and GraphOverlay, even though the real model is simply subgraph query versus client-provided graph data.

Suggested direction
Use a typed source model for the overlay so fallback behavior is centralized and impossible to represent inconsistently.

For Agents
Replace subgraph: string plus fallbackDecisions?: DecisionEntry[] with a discriminated prop such as source: { kind: 'subgraph'; name: string } | { kind: 'decisions'; title: string; decisions: DecisionEntry[] }. Derive nodes/data from the source once, and make topology availability a property of the source instead of a sentinel comparison. Preserve the existing Traces-only fallback behavior and topic-subgraph behavior.

[otReviewAgent]

🟡 Issue: Keep the durable dedupe ledger out of the core propose flow

What's wrong
The PR adds a second responsibility, durable idempotency bookkeeping, directly into propose, which already validates input, reads stdin, signs an event, posts to the relay, and prints the response. The result is not large enough to be unworkable, but it is structurally heavier than it needs to be and makes future changes to either proposing or deduping more coupled.

Example
Reading propose now requires tracking ledger_path, key, mutable proposed, force, the early skip path, the relay write, and the post-accept persistence path in one function. The actual proposal operation is only a few lines, but the idempotency mechanics dominate the control flow.

Suggested direction
Move the ledger read/write/key/skip orchestration into a focused abstraction so propose reads as validate sources, maybe skip duplicate, build/sign/post event, then record accepted proposal. This would delete the ad-hoc mutable state from the command path without changing behavior.

Confidence note
This is a maintainability finding, not a behavioral claim; the current implementation appears intentionally defensive, but the proposal flow now has to carry the ledger orchestration details directly.

For Agents
In crates/buzz-cli/src/commands/memory.rs, preserve the skip-before-stdin and record-after-accept behavior. Extract the dedupe mechanics behind a small local type or helper, for example a no-op/active DedupeLedger with load, skip_response, and record_success. Keep the existing unit coverage around key normalization, missing/corrupt ledgers, and round-tripping.

[Zigoljube]

Done in 4947918: mechanics extracted into ProposalLedger (open/gate, mark_pending, record_accepted, record_failure) + a LedgerGate enum, so propose reads as gate → post → record.

[otReviewAgent]

🟡 Issue: Split the dedupe ledger out before letting memory.rs sprawl past 1k lines

What's wrong
This change pushes an already broad CLI command file over the 1000-line threshold and mixes several distinct responsibilities into one module. The new code is not just local command glue; it is a durable two-phase ledger with locking and reconciliation behavior, which deserves an ownership boundary of its own. Keeping it inline makes future changes to memory proposal validation or query handling harder to review safely because the file now carries unrelated persistence mechanics too.

Example
A reader opening memory.rs now has to scan command dispatch, proposal validation, query handling, filesystem locking, crash-recovery semantics, relay error classification, and async read-back reconciliation in one file before getting back to the actual propose flow.

Suggested direction
Treat the dedupe ledger as its own small component with a narrow API, instead of embedding durable storage and crash-recovery mechanics inside the command handler file.

For Agents
Move the new ledger implementation and its unit tests into a focused module such as crates/buzz-cli/src/commands/memory_dedupe.rs or memory/ledger.rs. Keep propose responsible for validating inputs, building the event, and calling a small ledger API like ProposalLedger::gate/mark_pending/record_result. Preserve the same stdout and exit behavior; run the existing memory command tests after the extraction.

[otReviewAgent]

🟡 Issue: The new dedupe contract is only tested through helpers, not through memory propose behavior

What's wrong
The risky behavior here is preventing duplicate graph writes in unattended loops. The tests verify the ledger file can round-trip, but they do not verify the changed CLI control flow that is supposed to stop duplicate submissions or handle --force. That leaves the main user-facing/data-integrity contract unprotected.

Example
A regression could move the post_authed_json("/api/dkg/memory", ...) call before the ledger check, or ignore force, and the current helper-only tests would still pass. A behavioral test should seed a ledger with the computed key, run memory propose --dedupe-state, and assert no HTTP request is made and stdout reports status: skipped; a second case should pass --force and assert the request is made.

Suggested direction
Add an integration-style or command-level unit test for the idempotency flow rather than only the serialization helpers.

For Agents
Look in crates/buzz-cli/src/commands/memory.rs and the CLI test strategy. Add a boundary test around propose/dispatch using a local mock relay or a thin injectable client so the test proves: duplicate ledger entry skips before network/stdin work, --force bypasses the skip, and failed relay responses do not persist the key.

[Zigoljube]

Added in 4947918 — behavior tests around propose itself with a mock axum relay: an accepted ledger skips before any network request (zero hits observed, input path is unreadable on purpose so a regression that reads stdin first fails); an unconfirmed pending marker refuses to post and the marker survives; conflict reconciles to accepted; unknown outcome keeps pending; non-interactive --force is refused (it's human-only per the agent panel).

[otReviewAgent]

🟡 Issue: Successful dedupe proposal flow lacks end-to-end coverage

What's wrong
The core contract of --dedupe-state is that a first successful proposal is recorded and later replays are skipped. The added tests verify pieces of the ledger and several edge paths, but they do not verify that the actual network success path performs the pending-to-accepted transition.

Example
A regression that accidentally removes ledger.record_accepted()? after a 200 response would still pass the current new tests: helper tests call record_accepted/record_failure directly or seed accepted/pending state, but no test proves a real first successful post becomes an accepted ledger entry.

Suggested direction
Exercise the primary new behavior through propose rather than only testing ledger helper methods and pre-seeded states.

Confidence note
I reviewed the added tests in the diff and surrounding file; they cover helper transitions and skip/fail-closed paths, but I did not run the suite in this read-only sandbox.

For Agents
Add an async behavior test in crates/buzz-cli/src/commands/memory.rs that uses a mock /api/dkg/memory server, a real valid proposal input file, and a fresh --dedupe-state; assert one POST occurs, the ledger contains the dedupe key in accepted with no pending, and a second call with the same channel/sources returns success without another POST.

[otReviewAgent]

🟡 Issue: Key-material rejection is untested

What's wrong
This change adds a security-facing tripwire that refuses memory proposals containing private key markers, but the test suite does not exercise it. A regression that removes or narrows the guard would still leave the new tests green.

Example
A focused regression test could call validate_proposal_content(r#"{"schemaVersion":1,"summary":"x","items":[{"kind":"note","text":"nsec1..."}]}"#) and assert it returns CliError::Usage with the key-material refusal message.

Suggested direction
Add a regression test for the new structural safety guard so a future change cannot silently allow private key material into memory proposals.

For Agents
Add coverage in crates/buzz-cli/src/commands/memory.rs tests near proposal_content_requires_version_summary_and_items. Preserve valid proposal schema coverage, and add explicit rejection cases for each new key-material sentinel plus at least one normal valid proposal to prove the guard is not bypassed or accidentally removed.

[otReviewAgent]

🟡 Issue: Fallback test does not verify that provider graph queries are skipped

What's wrong
One of the changed behaviors is that the fallback lens is client-only and must not issue a provider query for the sentinel subgraph. The test verifies the visible overlay, but its catch-all provider stub would hide an accidental network call for the non-existent sentinel graph.

Example
If a future refactor changed line 63 back to useSubgraphGraph(channelId, cg, subgraph), the spec could still pass because the route handler at desktop/tests/e2e/dkg-memory-fallback.spec.ts:70 masks the bogus provider request with a successful empty response while the fallback data still renders the cards.

Suggested direction
Tighten the Playwright route to fail or count unexpected subgraph requests while still stubbing channel-memory deterministically.

For Agents
In desktop/tests/e2e/dkg-memory-fallback.spec.ts, count or collect provider requests after opening the All decisions lens and assert that no subgraph/graph endpoint is called for __all_decisions__; keep the existing assertions for chip visibility, card count, and hidden topology toggle.