fix(acp): retain transiently rejected observer frames for resend (PR27 salvage)

[mfethe1]

Summary

Focused salvage of PR #27's ACP rejected-observer-frame retransmit fix only: cherry-pick of commit 84c014c1459a78b83bf85c394bf3722f97574f3f (merge 2516efe112 carries the identical first-parent delta; its other parent's proactive-resubscribe/recovery changes are out of scope here).

The bug: the mid-session OK handler ends with an unconditional state.acknowledge_observer_frame(&event_id) after the classify_ok match. That trailing call retires the in-flight frame on every OK — defeating the Retriable arm, so a transiently refused observer event (e.g. error: database unavailable) is dropped from the resend queue exactly as though it had been stored.

The fix: remove the two trailing lines so each classify_ok arm controls its own acknowledgement (Stored/Permanent retire; Retriable stays in flight for resend), and replace the state-only unit test with a real-socket regression test (test_ws_pair + execute_connected_command + handle_ws_message) asserting: a transient refusal retains the frame, the resend publishes identical signed bytes, and a terminal OK retires it.

Need verification (done before implementation)

• product/main @ bcd3e1c96e (includes [lenny] fix(test): deterministic fake_llm steer/cancel assertions #46): trailing unconditional ack still present (relay.rs OK handler); only the weak unit test exists. Needed.
• PR [lenny] integrate upstream block/main bfc3848558 (8 commits) #28 @ 4c1d2d17: OK handler identical to trunk — no coverage, no conflict. Needed.
• Fix direction aligns with upstream (110ff873, 2f3dd850 introduced the OkDisposition machinery this completes).

Regression proof (deterministic)

With the two-line fix reverted and only the new test kept, the test fails exactly as the bug predicts:

assertion `left == right` failed: "error: database unavailable" must not acknowledge an uncommitted observer frame
  left: []  right: [EventId(a828ada...)]

With the fix: passes.

Gates (local, hermit cargo 1.95.0)

• cargo test -p buzz-acp — 986 passed / 0 failed / 1 ignored (lib) + 9 + 9 (integration) — all green
• cargo clippy -p buzz-acp --all-targets --locked — 0 warnings, 0 errors
• cargo fmt --all --check — clean
• cargo metadata --locked — clean

Scope: crates/buzz-acp/src/relay.rs only (+93/−28). No migrations, CI, compose, credentials, shell fixtures, or process-reaping changes. Source SHA preserved: PR27 commit 84c014c145 cherry-picked verbatim with original DCO sign-off; branch head dc85e296810356b9858d96c528189c5c8a982917 on product/main bcd3e1c96e.

Parent owns independent review and merge.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 9f12e5f2-575b-4819-9203-f49fcf751d36

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}