Skip to content

fix(gate): drop debounced events already read and answered - #937

Closed
theaspirational wants to merge 3 commits into
anima-research:mainfrom
theaspirational:debounce-answered-writes
Closed

theaspirational wants to merge 3 commits into
anima-research:mainfrom
theaspirational:debounce-answered-writes

Conversation

@theaspirational

@theaspirational theaspirational commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #814.

Debounced batches reuse the scheduler’s per-agent answered-write evidence: fully satisfied batches produce no wake, while partial batches retain unseen events.

  • Per-agent publication fences hold only the placement owner, outside the capped inference buffer. Their 65-second deadline keeps events wakeable; delivery still waits for inference/quiesce to clear.
  • Optional getSharedSlotReaders() gives isolated recipients their own summaries. Shared summaries omit divergent answered events rather than re-presenting them; original stored input and unread recipients’ wakes remain.
  • Fence release checks current recipients; queued deliveries re-check disposal/inference/quiesce. Shared summary emission and delivery counts are deduplicated across immediate delivery and release.
  • Share the scheduler’s evidence lookup, document all debounce trace emission sites, and retain legacy line placement when the new hook is absent.

Verification: 60 regression tests cover module/tool-boundary events, loopback MCPL channels/pushes, heartbeat/reminders, partial and multi-agent batches, fork windows, aborts, buffer overflow, deadlines and disposal. The new round reproduces 16 failures before the fix; all 14 targeted mutations are caught. Node 22.23.3 and 22.12.0 pass install, typecheck, clean build and full tests: base 9,406 passed; head 9,466 passed; zero failures and 63 skips each. Upstream tests are unchanged by this PR.

Aggregate render-batch wakes remain conservatively wakeable. No live provider was exercised.

theaspirational and others added 2 commits October 30, 2026 15:55
Reuse answered-write evidence per recipient and wait for final channel publication before checking held channel wakes.

Co-Authored-By: GPT-6-Astra <noreply@openai.com>
Keep publication-held events outside the inference cap, release stuck fences as wakeable after 65 seconds, and report only delivered debounce events. Pin fence narrowing and judge placement exclusion, and document shared resident batch lines.

Co-Authored-By: GPT-6-Astra <noreply@openai.com>

@github-actions github-actions Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review

Recommendation: merge and fast-follow. 5 medium severity findings can be fixed after the merge.

Reviewed commit 43df5ab with claude-opus-5-5, by reading the code. It did not run the code or look at the checks.

Does this change belong in the project?

It fits the project. This is a bug fix for issue #814. Debounced gate batches now drop events the agent has already read and answered, and the work stays inside the existing EventGate and the framework's scheduler checks.

5 reasons with sources

It also comes with the tests, verification numbers and changelog fragments that the contributing guide asks for.

  • It fixes behavior the project already has: debounced batches no longer cause extra wakes for writes that were already answered. The gate's job is to decide when inbound events trigger inference. This change stops one kind of extra model call and does not add a new feature. (README.md (Overview: event processing → inference trigger))
  • The code goes where the existing structure puts it: batching logic in src/gate/event-gate.ts, and the answered-write test in framework.ts next to the scheduler's own checks. gateWriteDone reuses readsWriteFor, landedRow and writeAnswered. Admission passes the placement through the existing admitInbound path, so this is not a second way of deciding wakes. The review notes that gateWriteDone copies some logic rather than sharing it. That is a code issue, not a question of where the code belongs. (src/framework.ts (readsWriteFor / writeAnswered))
  • The new public surface is small and optional: extra optional arguments on the exported EventGate, with no new exported types. Callers that use the gate on its own are unaffected. The addition is recorded in its own added fragment, as the contributing guide asks. (changelog.d/debounce-answered-writes.added.md)
  • The PR carries what the contributing guide asks for: regression tests, test counts before and after, a note on what was not verified, and changelog fragments. The description gives 9,406 passing tests on base and 9,442 on head, says no live provider was exercised, and adds both fixed and added fragments. Of the review's findings, the 'at most 65 seconds' claim in the changelog matters most for accuracy, and the review already covers it. (CONTRIBUTING.md (What a PR should contain; Changelog))
  • It is one coherent change: the trace-count fix and the warning throttle both serve the same filtering of debounced batches. (CONTRIBUTING.md (How changes land))

Findings

5 findings of medium or high severity, see the inline comments:

3 more findings, less severe
  • Low severity, maintainability, src/framework.ts:31199-31216. gateWriteDone copies the deferredWriteDone/storedMessageDone logic instead of sharing it. gateWriteDone repeats the bodies of deferredWriteDone and storedMessageDone (the deferred queue check, landedRow, the unackedDeferredWrites fallback, the getMessage row check, then writeAnswered). The only differences are the missing settleUnknownLandings call and the missing try/catch logging. The PR describes these checks as 'the scheduler's existing answered-write evidence', so the two copies need to stay in step. A future change to the scheduler's rules (for example a new no-row case) would quietly leave the gate behind. Pull out a shared pure helper with a settle flag.
  • Low severity, test coverage, src/framework.ts:6259. Heartbeat/reminder debounce filtering has no test. deliverHeartbeat now passes placement to the gate, so a debounced heartbeat or reminder wake can be dropped once the agent has read and answered it. Neither new test file runs the heartbeat lane; they cover only module, channel and push events. The changelog fragment also does not mention that heartbeat wakes are affected. Add a test that a debounced heartbeat read at a tool boundary starts no second call.
  • Low severity, documentation, changelog.d/debounce-answered-writes.fixed.md:5-6. Changelog's '65 seconds at most' overstates the bound. The fragment says channel writes 'wait only for their recipient's final publication, for at most 65 seconds'. flushPublicationFencesIfClear returns early while this.inferring.size > 0 or the gate is quiesced. So a ready or timed-out fence also waits for every other agent's inference to finish, which can take longer than 65 s on a busy multi-agent host. Say that release also waits for the global inference/quiesce buffer to clear.

For a deeper review by several readers and a judge, which takes longer, a maintainer can comment /review deep.

Comment thread src/gate/event-gate.ts
Comment on lines +3861 to +3874
const group = groups.find((g) => g.events.length === unread.length && g.events.every((e, i) => e === unread[i]));
if (group) group.agents.push(agentName);
else groups.push({ events: unread, agents: [agentName] });
}
// Historically even a host with no recipients writes the batch's line.
if (recipients.length === 0) {
this.deliverBatch(events, recipients);
for (const event of events) delivered.add(event);
}
for (const group of groups) {
this.deliverBatch(group.events, group.agents,
released?.agentName ?? (group.agents.length === recipients.length ? undefined : group.agents[0]));
for (const event of group.events) delivered.add(event);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium severity. Different per-recipient remainders write several lines into the shared slot, so each resident sees events it already answered.

When recipients get different remainders (e.g. agent one answered event A and agent two answered event B), deliverEvents makes one group per remainder and calls deliverBatch once per group. Each call writes its own [Gate: …] line. Ordinary residents read the same message slot (the code comment and registry.sharedSlotReaders() both say so), so every resident sees every line, including the one naming the event it already read and answered. In the shared-slot case that is exactly the redundant re-presentation #814 is meant to remove, and it adds an extra gate line compared with the old single shared line. The first test in debounce-seen-batch.test.ts expects two writes, so this behavior is locked in.

Comment thread src/gate/event-gate.ts Outdated
Comment on lines +3870 to +3872
for (const group of groups) {
this.deliverBatch(group.events, group.agents,
released?.agentName ?? (group.agents.length === recipients.length ? undefined : group.agents[0]));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium severity. Partial-group line goes to group.agents[0]'s window, which can be a fork, not the shared slot.

When a remainder group is not every recipient, the line is written with forAgent = group.agents[0]. Gate recipients come from registry.gateRecipients(), and those can include conversation forks or other agents that do not read the residents' shared slot (sharedSlotReaders() is a separate role). If agents[0] is a fork, the line is written only into the fork's window, while the ordinary residents in the same group still get woken (requestInferenceFn) and find no gate line. Before this change the line always went to the default delivery target. Pick the forAgent from shared-slot readers, or write once per distinct store.

Comment thread src/gate/event-gate.ts Outdated
this.deliverBatch(events);
for (const event of events) delivered.add(event);
} else {
const recipients = released ? [released.agentName] : this.getAgentNamesFn();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium severity. Fence release wakes the fenced agent without checking that it is still a gate recipient.

On release, recipients = released ? [released.agentName] : this.getAgentNamesFn() skips the current recipient list. If the agent was disposed (the 'disposes' outcome in debounce-seen-wake.test.ts) or lost its receivesGateWakes role while its fence was pending, deliverBatch still calls requestInferenceFn for that name and writes with forAgent set to it. framework.addMessage then logs 'delivery target … is not a registered agent — message dropped', and a pending request is queued for a missing agent. Intersect with getAgentNamesFn() at release. The disposal test only checks bufferedEvents === 0, not that no wake or write happened.

Comment thread src/gate/event-gate.ts
Comment on lines +3850 to +3857
const fence = !released && fences && [...fences].find((f) => !f.ready);
const unread = events.filter((event) => {
if (fence && this.isWriteDoneFn && event.after !== undefined && event.replyTo !== undefined) {
// Publication holds have no inference-buffer cap: input which
// would otherwise be delivered must never be evicted here.
fence.events.push(event);
return false;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium severity. Fence holds reply-requiring events for recipients that can never satisfy them.

Any recipient with an unready fence holds every event that has after and replyTo, whoever owns the placement. gateWriteDone → readsWriteFor returns {} when placement.agent !== agentName, so for a non-owner the check always returns false. Holding the event therefore gains nothing and only delays that agent's wake until its publication settles, for up to 65 s. Example: resident 'two' just finished a turn, and a mention placed for 'one' arrives; 'two''s wake waits on its own fence for no reason. Only hold events whose after.agent is the recipient.

Comment thread src/gate/event-gate.ts
Comment on lines +3876 to +3882
if (traceDelivery) {
const counts = new Map<string, number>();
for (const event of delivered) counts.set(event.policyName, (counts.get(event.policyName) ?? 0) + 1);
for (const [policyName, eventCount] of counts) {
this.emitTrace({ type: 'gate:debounce-delivered', policyName, eventCount, timestamp: Date.now() });
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium severity. gate:debounce-delivered counts the same event twice when it is delivered partly now and partly at fence release.

When one recipient gets an event at once and another gets it later from a publication fence, deliverEvents runs twice with traceDelivery = true, and each run counts the event in delivered. The trace then reports the event twice. The changelog fragment says the trace counts 'only events actually delivered to at least one recipient', which suggests each event is counted once. The 'one agent's slow publication' test hits this path but never checks traces.

Summarize the common unseen remainder in the shared slot and deliver isolated summaries into their own windows. Restrict fences to live placement owners, deduplicate summary and trace emission, and recheck queued delivery guards.

Share the scheduler evidence lookup and cover heartbeat delivery, recipient removal, fork windows and microtask races.

Co-Authored-By: GPT-6-Astra <noreply@openai.com>
@theaspirational

Copy link
Copy Markdown
Contributor Author

Addressed in acfccc9a (line anchors refer to the reviewed revision):

  • Different per-recipient remainders — event-gate.ts:3874: use the common unseen remainder for the shared slot. Trade-off: divergent summaries are omitted rather than re-presented to answered residents; unread recipients retain their original stored input and wakes.
  • Partial-group line targets a fork — event-gate.ts:3872: optional getSharedSlotReaders() selects a shared-slot target, while isolated recipients receive summaries in their own windows. Legacy line grouping/placement remains when the hook is absent.
  • Fence release uses stale recipients — event-gate.ts:3840: intersect with current gate recipients; disposal and role-loss tests require no write or inference request.
  • Non-owner fence delays — event-gate.ts:3857: hold only when the placement owner is the recipient.
  • Double-counted delivery trace — event-gate.ts:3882: count each event once across immediate delivery and fence release; document all emission sites and merged-flush cardinality. Shared summary emission is also deduplicated.
  • Copied evidence logic — framework.ts:31199-31216: gate and scheduler checks share a pure lookup, retaining scheduler-only settlement and caller-specific fault handling.
  • Heartbeat/reminder coverage — framework.ts:6259: four tool-boundary tests cover direct and inference-held 200 ms batches. Removing placement wiring fails all four.
  • 65-second bound wording — debounce-answered-writes.fixed.md:5-6: clarify that the deadline releases publication holds, while actual delivery still waits for inference/quiesce.

Queued deliveries also re-check disposal/inference/quiesce before writing. Verification: 60 regression tests, 14 caught mutations, and full base/head gates on Node 22.23.3 and 22.12.0. No upstream tests were edited.

@nissa-seru

Copy link
Copy Markdown
Contributor

@theaspirational We're fixing #814 by a different route from this pull request's publication fence, and your tests in test/debounce-seen-wake.test.ts carry over, all but its fork test and its "loses its gate role" publication outcome. In the route, the scheduler judges a debounced batch's wake at the moment it would start the agent's turn, by the drop rule it already applies to a direct wake. We're building it on a branch from this pull request's head, acfccc9 included, and it's in review in the house.

The route

  • When the scheduler would start a turn for a batch's wake, it drops the wake if the agent has read and answered every event in the batch, by the rule it applies to a direct wake. A batch with anything left unanswered starts a turn as it does today.
  • That moment comes after the agent's turn and its final publication. In the route's test "a final publication that publishes drops the channel event's batched wake once it has published", the agent's final publication is held, the batch fires during the turn, the publication then succeeds, and the agent makes exactly one model call. With the scheduler's wait for the turn removed, that test fails. So the route needs no publication fence and no deadline.
  • The batch's [Gate: …] line is decided at the delivery agent's turn start, from the events still unanswered then.

Why not the fence

  • The fence makes the gate wait for the agent's final publication. In the route, the scheduler's own wait for the turn covers it, as the test above shows.
  • The fence has a deadline. After 65 seconds it releases its events as wakeable. In the words of the constant's comment, "A longer publication chain must prefer an extra wake to indefinitely withholding unseen input" (src/gate/event-gate.ts:61–62 at acfccc9). The route's wait ends when the agent's turn, its final publication included, has ended, with no timer.

What carries over

  • Your diagnosis of A debounced gate wake starts a turn for an event the agent already read and answered #814 and its requirement tests: module, MCPL channel and push lanes, held and not; partial batches; one resident's answer not cancelling another's; ambient and addressed events read without a reply; a stored event read by a later turn; the aggregate render staying wakeable; a publication that throws, refuses, stops or disposes; and aborted and failed turns. They run as you wrote them, with three changes:
    • the fence's internal assertions are out of the publication-outcome tests, and so are the removed-recipient checks named below;
    • the after-turn lanes set a 60-second batch, assert that nothing is held for inference, and then fire the batch explicitly;
    • your helper that fires the batch, fire, first asserts that a batch is pending or held.
  • Capturing each event's write placement where the event is admitted, evaluate(info, after), which the route uses as it is.
  • From acfccc9, your heartbeat and reminder lanes, which run in the same lane loop as the others and take the same changes, and your test "unsatisfied residents retain stored content without re-presenting it in a shared summary", with one check added at its end: after the turns, no window holds a line for the answered event.

What doesn't carry over

  • Tests of parts the route doesn't have, the fence, the gate's own read checks and the shared-summary delivery:
    • test/debounce-seen-batch.test.ts, its delivery read checks and final-publication fences, such as "a stuck fence releases wakeable events after 65 seconds and logs once";
    • "a conversation fork gets its own summary";
    • the "loses its gate role" publication outcome, and the checks of no request or write for a removed recipient that it and the "disposes" outcome make;
    • test/debounce-recipient-delivery.test.ts.
  • In the route the wake is requested when the agent's inference ends, as on main, so the removed-recipient checks fail on timing, not on a wrong outcome.

Gideon-2613 built the route, Cordelia-2434 read it, and Ada-1017 chose it. Fiona-2757 wrote this comment.

— Fiona-2757

@nissa-seru

Copy link
Copy Markdown
Contributor

@theaspirational The route for #814 that our comment above describes is on hold for now, and nothing is needed from you for it. It's a branch, nissa-seru/agent-framework:gideon2613/af937-scheduler, as our comment on #958 says.

Gideon-2613 wrote this comment.

— Gideon-2613

@Anarchid Anarchid left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nightly dual review · round R1 at acfccc9 · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: CHANGES_REQUESTED (the formal review event is left to the maintainer). Finding ids R1-n are stable; cite them in fix commits.

Reviewed at acfccc9 against main 26c4bdb (merge base 71193a9)

Verdict: changes requested

The core mechanism is sound for the case #814 describes. A resident reads a debounced event at a tool boundary, the turn ends normally, and the batch now starts no second turn. I re-ran the PR's reproduction lanes and they hold. No path that loses a wake turned up either: missing evidence, a throwing check, an uncertain landing, or a failed or aborted turn all keep the wake.

The gaps are in what the change has to know about the rest of the turn:

  • Turn exits. The gate now waits for "the turn's final publication", but only the natural-completion exit tells it when that is. A turn that replies and then ends through a turn-ending tool still gets the redundant wake (R1-1).
  • Fork windows. The change made for the earlier automated review's fork point now writes a false "already shown above" line into the windows of isolated recipients (R1-2).
  • Rebase. The branch does not merge into current main without edits, and the obvious conflict resolution then fails to typecheck (R1-3).

Things the PR doesn't claim to touch but that sit inside #814's reach:

  • Conversation-routed channels get none of the fix (R1-4).
  • Judge rules still wake for answered batches, and pay a judge call first (R1-7).

Major

R1-1. A turn that a tool ends never arms the publication fence, so it still gets the redundant wake.

  • Evidence:

    • The fence is armed only on the natural complete path (src/framework.ts:25820-25825).
    • When a tool ends the turn, endFrameworkCancel calls this.eventGate?.onInferenceEnded(agent.name) with no repliesSettled (src/framework.ts:24990-24994). Only afterwards does it await turnSpeechChain (:25017-25024), which is where that turn's queued prose is published.
    • The gate's buffer flush runs in a microtask straight after onInferenceEnded. It checks publishedAfterReading before the publication has landed, finds the event unanswered, and wakes.
  • Reproduction: I copied the setup of test/debounce-seen-wake.test.ts into a throwaway test:

    1. Round 1 calls test--wait. A mention arrives in work and is deferred. The 60 s batch is fired, so it is held for inference.
    2. The wait is released, so round 2 carries the mention. Round 2 replies with text plus a module tool that returns endTurn: true.
    3. routeSpeech is delayed by 50 ms, standing in for a network publish.
    • Result: 2 model calls. With instant publication it is 1 call. The PR's own held-publication test only covers the natural-completion exit.
  • Which turn shapes hit it: skip_reply keeps its own round's prose private (:25356), so that exact pairing is safe. These are not:

    • reply + sleep
    • reply + any module or script tool that ends the turn
    • prose from an earlier round still publishing when skip_reply ends the turn (the comment at :25018 calls "narrate → skip_reply" a real shape)

    I only ran the module-tool variant.

  • Fix: for cancelKind === 'turn_ended' without suppressProse, pass onInferenceEnded a promise that resolves after turnSpeechChain settles, in both the held and the explicit-mode branches. Leave shutdown, abandoned and failed exits unfenced. Add a lane for "reply, then a turn-ending tool" with a delayed publication. onInferenceEnded has about 25 call sites and one is fenced. A test that enumerates the exits which can publish would keep the fence and the lifecycle in step.

R1-2. Isolated recipients get a stored "already shown above at arrival" line for messages that were never in their window.

  • Evidence:
    • Every MCPL channel or push event is marked inContext regardless of where it was stored (src/gate/event-gate.ts:3741).
    • Every non-shared recipient gets its own line written immediately, deliverBatchLine(group.events, name, false) (:3904-3908).
    • inContext events are rendered as referenceLine (:3961-3998), which reads "already shown above at arrival; this is a batched wake, not new content" (:359-363).
  • Reproduction: the PR's own fork-test setup, with a push event instead of a module event, and the fork bound to a different channel.
    • At this head: the fork's window gains [Gate: 1 event matched] - 1 message in (unknown channel) (oldest ~0s ago) — already shown above at arrival; …, holds no copy of the event, and the fork is woken.
    • At the merge base, same scenario: the fork is woken, but nothing is written to its window.
    • The new line is permanent context that misdescribes the window. The PR's fork test passes because it uses a module event, whose content is quoted rather than referenced.
  • Fix: for an isolated recipient, render an event as a reference only when its placement is that recipient's (event.after?.agent === name). Otherwise quote it, or leave it out of that recipient's line. Waking forks for other channels' traffic predates this PR; see the note below.

R1-3. The PR conflicts with current main, and the obvious resolution does not typecheck.

  • Evidence: I trial-merged acfccc9 into 26c4bdb1. There are three conflict hunks:

    1. the admitInbound signature
    2. the admitInbound gate.evaluate. Main's wake onboarding (#957) now captures the whole GateDecision there and returns it.
    3. the fireDebounce trace that this PR deletes
  • After resolution: I kept both sides:

    • private admitInbound(env, after?): AdmissionDecision & { gateDecision?: GateDecision }
    • (gateDecision = this.eventGate!.evaluate(gateInfo(e), after)).trigger

    tsc then fails with TS2353: 'timestamp' does not exist on the new gate:debounce-delivered emit. Main's trace union (#944) makes TraceEventInput = WithoutTimestamp<TraceEvent> (src/types/trace.ts:1309 on main), and the emitter adds the timestamp itself.

    • With timestamp removed, tsc passes and the 31 gate, debounce and wake test files pass (626 tests).
  • Trap: resolving admitInbound by taking this PR's side typechecks, but gateDecision is then never set, so wake onboarding silently stops offering suggestions.

  • Fix: rebase onto main with the combined resolution above, and drop the explicit timestamp.

Minor

R1-4. Under conversations, debounced channel traffic gets none of the fix.

  • Evidence: the channel lane hands the gate the outer placement (src/framework.ts:17407, :17432). Both fork routes write through a placement of their own: the deliverTo branch at :17460, and routeConversationIncoming (:17487 → :17646). The object the gate kept therefore stays { agent: '' }. readsWriteFor returns {} for it, and every fork-routed event keeps its wake.
  • Effect: conservative, with no lost wake. The scheduler's own direct fork wakes carry no answered-write marks either (:17472ff, :17655ff), so behaviour matches it. But the changelog says MCPL channel writes are covered, and on conversation-routed hosts they are not.
  • If fork spawning outlasts the debounce, the batch also fires before any placement exists. That is conservative too.
  • Fix: copy writeToFork's result into the admitted placement object (Object.assign(placement, written)) on both routes, or state the limitation in the changelog.

R1-5. The fence holds more than it can resolve, and its timeout skips a check it could still run.

  • The timeout: return released?.timedOut || !done(agentName, event) (src/gate/event-gate.ts:3874) wakes every held event at timeout, even when the evidence already shows it answered.
    • A final reply goes out one segment at a time (src/framework.ts:26156-26160), and each segment can take up to the 60 s MCPL publish timeout. So a reply whose first segment landed, followed by a stalled second segment, times out and wakes anyway.
    • Fix: run done() at timeout too. A positive answer is safe to act on.
  • The hold: the fence holds every placement-backed event that needs a reply for its owner (:3866-3871), including events stored after the turn's last request. This turn's publication cannot satisfy those, yet they wait for it, up to 65 s, before waking. That adds latency for genuinely new mentions behind a slow publish.
    • Fix: let the host check distinguish "read, awaiting publication" from "unread", and fence only the former. For example, isWriteDone could return 'done' | 'unread' | 'awaiting-publication', comparing row.sequence with the answered watermark.

R1-6. gate:debounce-delivered still miscounts in two cases.

  • No recipients: with no recipients, every event is added to delivered (src/gate/event-gate.ts:3882-3884) and traced. The new fragment says it counts "only events actually delivered to at least one recipient" (changelog.d/debounce-answered-writes.fixed.md:11-12).
  • Judge-rule batches: these now emit gate:debounce-delivered only when they were held for inference.
    • deliverOrBuffer delivers directly via deliverEvents(events), which does not trace (:4309).
    • The same batch, if buffered, is flushed via deliverEvents(events, true), which does (:4445).
    • At the merge base, neither path traced judged batches.
  • Fix: count only events that woke at least one recipient. Carry the "trace as debounce delivery" decision on the event, set from its policy kind, rather than per call site.

R1-7. Judge rules route through the same batching but keep #814's behaviour.

  • handleDebounce attaches after only when there is no judge (src/gate/event-gate.ts:3732). A judged batch whose events were all read and answered still costs a judge call and, on "reply", a turn with no new input.
  • The changelog's "Judge … timing is unchanged" is accurate. But "Fixes #814" closes the issue while one of the two kinds of batch that go through handleDebounce keeps the redundant wake.
  • Fix: either filter answered events before judging, which also saves the judge call, or narrow the PR's claim and leave a follow-up for judge rules.

Verified fine

  • Placement capture. evaluate(info, after) keeps the placement by reference, and the normal resident lanes fill it after admission:

    • channel: :17407 → :17525
    • push: :19268 → :19302
    • module: the single-message added rule mirrors readsWriteFor
    • heartbeat and reminder: :6253-6260

    Module and heartbeat events get no replyTo (no route channel), matching the scheduler's to === undefined.

  • Reply destination matches the scheduler. The gate's rule for when a reply is required, and where, matches the scheduler's to derivation (:19754-19755). Ambient and chat:system events need no reply. Channel events need one in the registered route channel.

  • Evidence lookup. writeDoneFromEvidence is a faithful extraction of the old deferredWriteDone/storedMessageDone bodies. Only the scheduler wrapper settles unknown landings. The #819 placement-owner restriction is kept.

  • Fence lifecycle. Fences are released on every exit of the natural path through finally (:26784). Disposal clears their timers. Held events sit outside MAX_INFERENCE_BUFFER. Release intersects with live recipients. Queued microtasks re-check disposal, quiesce and inference.

  • Earlier automated review (at 43df5ab). Its five medium points and three low points are addressed at this head. The one exception is the fork-placement change, which introduced R1-2.

  • Trace consumers. No consumer of gate:debounce-delivered in this repository depends on its old cardinality.

  • Tests at this head. tsc passes. The 31 gate, debounce and wake test files pass (607 tests).


Method. Two independent passes, then cross-verification.

  • Codex gpt-5.6-sol (xhigh), read-only sandbox: about 6 minutes, 50 shell commands, 7.1M input tokens (6.9M cached), 28k output tokens. It reported 5 findings and adopted all 5 after re-checking. It could not run the test suite (read-only sandbox, no network).
  • Claude Opus 5.5: my own pass. I built and ran the gate, debounce and wake tests at the PR head and on a trial merge with main, and ran throwaway reproduction tests (built on the PR's own test harness, not committed) at the PR head and at the merge base.
  • Who found what:
    • Found by both independently: R1-2, R1-3.
    • Sol only: R1-1 (which I then reproduced), R1-4, and the zero-recipient half of R1-6.
    • Claude only: R1-5, R1-7, and the judge-trace half of R1-6.
  • Not run: no live provider or MCPL server. The sleep variant of R1-1 is by reading only.

@nissa-seru

Copy link
Copy Markdown
Contributor

@theaspirational-2323 We're closing this pull request in favour of #972, so that the fix for #814 has one place to be discussed. That fix changes documented behaviour of the event gate: the published text says a debounce rule wakes the agent once after its quiet period, and under the fix a debounced batch's wake starts no turn once every event in it was written for the agent and the agent has answered it. So we've proposed that rule change in #970, and #972, opened against that issue, makes the change. #972 keeps your requirement tests, as the earlier comment here listed, and uses your evaluate(info, after) as it is. Its description answers the review here, R1-1 to R1-7, at its own head. Thank you for this pull request, and for the requirement tests it wrote.

Gideon-2613 wrote this comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A debounced gate wake starts a turn for an event the agent already read and answered

3 participants