fix(eventing): a rejected response must not act, only be recorded

[mrsabath]

Closes #885. M1 of the KubeCon MVP (KUBECON_NA_2026.md §5) — #888 and #889 landed in #902/#901, so this is the remainder and the last thing blocking #896 (M2).

Phase / scope

Phase	2 — the response-verification control in §4.4, made effective rather than advisory
What this changes	The acting paths behind response verification: the responses consumer, the group service, the store's conflict rule, and the group mirror. No wire change, no new flag, no schema break.
Default behaviour	Byte-identical. not ok is only reachable with a keyset loaded and EB_REQUIRE_RESPONSE_SIGNATURE=true, so the rule inherits those flags.

The bug, run rather than described

§8.6's rule. Same scenario both sides — a 2-agent batch, enforcement on, a forged terminal for member 1 and a forged group.completed, then the genuine signed answer:

BEFORE
  [kafka_in] unverified response on test-agent-0002: no ce_signature attribute
  [kafka_in] unverified response on test-agent-0001: expected kid 'eb-01', got 'runner-01'
  [groups]   duplicate terminal event for test-agent-0001/test-agent-0002 ignored
  batch completed_utc = 2026-10-09T22:33:15.727Z        <- settled, 1 of 2 agents terminal
  counts = {finished: 0, failed: 1, terminal: 1}        <- the agent had SUCCEEDED

AFTER
  [kafka_in] unverified response on test-agent-0002: no ce_signature attribute
  [kafka_in] unverified response on test-agent-0001: expected kid 'eb-01', got 'runner-01'
  rejected=2 rejected_not_stored=0
  transcript: seq 1 result 'Q1 revenue was flat.'       <- the genuine answer
  batch completed_utc = None                            <- still open
  counts = {finished: 1, failed: 0, terminal: 1}

Note the third log line in BEFORE. The forgery won mark_member_finished's finished_utc IS NULL race, so the genuine answer arriving afterwards was discarded as a duplicate — the forgery did not merely add a lie, it destroyed the truth, and the batch reported a failure for an agent that had succeeded.

One rule, three paths

A rejected envelope is evidence, not a fact. It is stored and it is rendered; it never advances state.

ce.is_rejected is the single predicate; it is read by the consumer, by GroupService.on_member_event/on_group_event, and by insert_response.

Why the marker is a top-level key and not phase or data — both obvious alternatives are wrong:

• phase == "error" is a value genuine failures carry. runner.py emits phase="error", final=True when an agent exits non-zero, and mark_member_finished maps it to failed. A guard keyed on phase would stop every real agent failure from finishing its member, hanging the batch to its deadline — failing as silence, which §21.6 names the worst outcome. test_a_genuine_error_phase_terminal_still_fails_its_member was written first and passes before and after.
• data arrives off the wire intact, so keying on data["signature_rejected"] is §8.5.4 answered the wrong way.

A correction worth flagging: the __ prefix was not sufficient on its own. A header named ce___rejected strips to exactly that key, so the marker was forgeable until from_kafka_binary started refusing __-prefixed names. The impact is the inverse of a bypass — a forger able to set it could mark every genuine answer rejected. Found by the test written to assert the opposite, which is why §8.5.4 asks the question instead of trusting the argument.

The store rule is asymmetric on purpose

INSERT OR REPLACE on (correlationid, sequence) is load-bearing: at-least-once redelivery must collapse onto one row, and emit.seed_seq resumes numbering from the stored maximum on a cold /continue turn — the fix for Phase 1's per-pod sequence restart that stored six rows for a three-turn conversation. Making the insert non-clobbering in general would re-open that bug under a new name.

So only a rejected event is refused a row it did not already own. It may create a row, and may replace another rejection (idempotent redelivery), never a row that is not marked rejected. test_a_verified_answer_can_still_replace_a_redelivered_duplicate and test_a_duplicate_delivery_still_collapses_onto_one_row guard the unchanged side. The refusal is counted (rejected_not_stored) and logged, because a silent drop is what §4.4 forbids.

The group mirror, scoped honestly

The mirror is a second, independent acting path that did no verification, so the consumer-side guard closed the hole only until the pod bounced: a forged group.completed sitting on the topic was applied unconditionally on the next restart.

It now verifies group.completed — and only that, and only when a keyset and bridge kid are both configured. Verifying everything would be catastrophic rather than strict: the mirror exists to replay history that predates signing, it runs on every start, and the test/kind overlays keep /data on an emptyDir, so refusing unsigned records would lose every group's state on every restart — the exact failure the module was written to prevent. group.completed is also the only mirror path that can settle a batch; on_member_event(replay=True) returns before maybe_complete.

To be explicit, since claiming otherwise would be §8.5.1 again: the GroupService guard protects the mirror against nothing on its own. The marker is written into SQLite by the live consumer and is not on the wire. The mirror's safety comes from its own bridge_kid check.

A test that claimed an outcome and checked a verdict

test_an_approved_runner_cannot_forge_a_group_event asserted groups[0]["phase"] == "error" with the message "rather than settling the group" — while wiring a list's append as the callback, so nothing in it could tell whether the group settled. §8.5.1 exactly: a claim about an outcome has to follow the path to that outcome. It keeps its name, now drives a real GroupService, and fails without the fix (verified by reverting the guards).

Checks

• 947 pass, 7 skipped, 1 xfailed (924 before — 23 new, nothing weakened or removed).
• ruff check clean.
• Every new test was confirmed to fail without its fix, including by reverting each guard individually.
• Verified the rejected column's idempotent ALTER against a database created without it, mirroring the prompts.submitter precedent.

Out of scope, documented

final_seen still counts a rejected terminal, so the "final" field on two read endpoints can report an answered correlation when the only terminal event was refused. Presentational — it advances no state — so it is left to a follow-up with a VERIFY comment naming what should change, per §8.5.5.

🤖 Generated with Claude Code

[aslom]

The core of this is right, and the hard part is right for the right reason. Keying the marker on a top-level __rejected rather than on phase or data is correct, and the two rejected alternatives are rejected on accurate grounds — runner.py does emit phase="error", final=True for a genuine failure and mark_member_finished does map it to failed, so a phase-keyed guard really would hang every batch containing a failed agent. Finding that ce___rejected strips to __rejected and closing it in the codec, then recording that the prefix argument was wrong, is §8.5.4 working as designed.

I ran the store's new conflict rule against sqlite directly rather than reading it, since all of item 3 rests on rowcount:

rejected into an empty slot      rowcount=1   stored
rejected over a rejected row     rowcount=1   replaced (idempotent redelivery)
genuine over a rejected row      rowcount=1   replaced
rejected over a GENUINE row      rowcount=0   refused, genuine row intact
genuine redelivery               rowcount=1   collapses onto one row

All four quadrants behave as the docstring says, INSERT OR REPLACE is preserved on the path seed_seq depends on, and returning False rather than raising keeps the consume thread alive. I also checked insert_response is called from exactly one place, so there is no HTTP path — including the unauthenticated PUT /transcript — that can reach it with a forged marker.

The half-configured case is genuinely closed, not just asserted: keyset.load_if_set returns None when unset, so keyset=ks is None and the mirror's self._keyset is not None gate holds; and a keyset without EB_SIGNING_KID already SystemExits at startup, so the mirror cannot be reached in the state that would refuse every replayed completion. Test coverage is 23 new tests that line up one-to-one with the claims, and test_audit_mode_still_lets_an_unverified_event_act pins the thing that actually matters about the guard.

Approving. The enforcement logic is correct where it acts, the default path is unchanged, and
neither note below is a defect in this change — the first is a residual the fix does not reach
and the second is comment-only. Both are worth a follow-up rather than another round here.

The mirror still re-applies forged member terminals, and that is the same damage the BEFORE table shows. The PR correctly identifies the mirror as a second acting path and fixes group.completed. But on_member_event(replay=True) runs add_group_member and mark_member_finished before its if replay: return, and no marker exists on the wire, so on every start a forged terminal earlier in the topic than the genuine one wins the finished_utc IS NULL race again. On the test/kind overlays — the emptyDir case the mirror exists for — the replay is authoritative, so counts goes back to failed: 1 for an agent that succeeded. The scoping decision looks right to me; the justification for it ("group.completed is the only mirror path that can settle a batch") answers settling but not member status, and that gap deserves naming the way final_seen was named.

Three new comments state the opposite of what response_decision returns. They say an unsigned non-terminal and a no-keyset deployment "both report verified=False"; both return verified=True, with inline comments in signing.py explaining why. No behaviour is affected — the guards read ok — but the stated reason for reading ok describes a failure mode that cannot occur, and the real reason is the one test_audit_mode_still_lets_an_unverified_event_act already pins.

Smaller: to_kafka_binary has no __ filter to match the inbound one. Nothing re-serializes a marked envelope today, so this is §8.5.5 rather than a live hole.

On the scoping of final_seen — the VERIFY comment that names the exact edit and then says which sentence not to delete is the best version of §8.7 I have seen in this repo. That is the pattern the mirror note above should follow.

I could not re-run the suite (no venv and kafka is absent in my environment), so the 947/7/1 figures rest on the eventing-test check, which passed along with the other eleven.

[mrsabath]

All three addressed in a3882e4. Thanks for running the four conflict quadrants against sqlite directly rather than reading the docstring — rowcount is the whole of item 3, and checking insert_response has exactly one caller is the check I would have wanted someone to make on my behalf.

	Note	Fix
high	Mirror paragraph answered settling, read as covering membership	Scoped the claim with the residual stated and a VERIFY naming the store-level fix
medium	verified described backwards in three comments	Corrected; the real argument for reading ok is stronger
low	to_kafka_binary had no counterpart to the inbound __ filter	Filtered both sides, with a test

Two things worth saying plainly:

The verified note is a straight error on my part — I confirmed response_decision returns verified=True for both cases earlier in this work, then wrote False in three comments. Comment-only, since every site keys on ok, but it would have pointed the next reader at a flag that does not behave that way.

The member-replay residual is real and I had not seen it. I had checked that on_member_event(replay=True) returns before maybe_complete and stopped there — which is the same shape as the §8.5.1 mistake this PR fixes in a test, so it is a fair catch on a PR whose whole subject is following a verdict to its outcome. It is narrower than the original bug (the batch stays open) and conditional in the unlucky direction you describe. Scoped rather than widened, for the reason you gave.

Happy to file the store-level follow-up now, or fold it into M2 (#896) — the forged-event demo stage is going to want a restart beat anyway, which is exactly where this would show.

[mrsabath]

Closing the one loop your review left open: you noted you could not re-run the suite, so the figures rested on the eventing-test check alone. Re-ran locally against the pushed head a3882e4:

948 passed, 7 skipped, 1 xfailed in 60.41s

924 on main before this branch, so 24 new — the 23 you counted plus test_the_rejection_marker_is_never_emitted_onto_the_wire from the outbound-filter fix. ruff check clean.

Everything else in the review was already in a3882e4, which is still the head; the three threads are replied to and resolved. Verified in the tree rather than from memory:

• no verified=False remains anywhere in eventbridge/ or shared/
• the mirror residual is named at kafka_group_mirror.py:88 with the VERIFY naming the store-level fix
• __ is filtered on both codec directions (ce.py:183 outbound, ce.py:212 inbound)

Nothing further outstanding from my side — approved, green, and mergeable whenever you want it in.

[mrsabath]

Merged as d7d2c5f7. Follow-up bookkeeping:

• 🐛 eventing: response-signature enforcement is advisory — rejected events still end batches and overwrite verified answers #885 auto-closed; 🐛 eventing: MVP M1 — make response-signature enforcement actually refuse (#888, #885) #895 (M1) closed with the before/after measurement against its own exit criteria.
• The member-replay residual you found is filed as 🐛 eventing: the group mirror re-applies forged member terminals on replay #956, with your reachability analysis, the PVC-vs-emptyDir asymmetry, and why widening the mirror's verification is the wrong fix. It also notes the one thing that changed in its favour: the rejected column this PR added means the store now holds the information the wire lacks, which is probably where it gets resolved.
• M2 (🐛 eventing: MVP M2 — run the signed path end to end, and make the demo able to drive it #896) is unblocked and next.