Fix what the relay loop does each tick: binding, durability, fairness, diagnostics, supersession - #9
thisisjun786 wants to merge 31 commits into
Conversation
…d enqueue Two pre-existing relay defects, each with a regression test captured failing against the unmodified source before the fix. The only automatic anchor binding ran after the daemon's own new-dispatch branch. A revision that reached dispatched any other way -- the deliver command, either reconcile promotion, a dispatch committed in the last tick before shutdown -- left its generation anchor_pending, and by I-06 every later receipt for that generation was refused as unbound. The suite missed it because its one binding test called bind_dispatched_revision by hand. Binding is now recovery over state rather than a hook on one path. bind_pending_anchors finds every generation still pending whose revision actually dispatched, and runs first in the tick so a receipt arriving in the same tick is accepted. It also repairs a generation stranded before this existed. bind_anchor is idempotent for the same turn and refuses a conflicting rebind, so the scan cannot move an anchor that is already bound. Separately, _settle_turn recorded the terminal observation before queuing what it implied, and swallowed the enqueue error. A relationship paused between selection and queuing left a final event with no delivery row, and the next tick skipped the turn because it had already been observed -- so resuming the relationship never helped and the event reached nobody. Finalizing, recording and queuing are now one commit, through transaction-aware siblings of resolve_staged and record_observation. The two kinds of failure are no longer treated alike: a durable refusal is a legitimate answer, so the observation stands and _requeue_missing picks the event up once the refusal lifts, while anything else rolls the whole transaction back so the next tick re-observes cleanly. The recovery obligation is derived from state rather than kept in a queue, so events stranded before this change are recovered too.
…ally running Reproduced in operation as JUN-100 generation 11 and JUN-101 generation 13: the daemon was alive, inside its time bound, and the child turns were completed, yet the events sat staged with no delivery and never moved. _turns_to_poll collected every generation anchor oldest-first, sliced to the per-tick budget, and _observe discarded the already-observed ones after that slice. Past eight generations the slice was permanently the first eight, every one of them already observed, so the current generation was never selected again. Raising the cap would only move where that happens. Three changes make it a schedule rather than a prefix. Candidates are filtered before the budget, so an anchor with nothing left to learn consumes no opportunity. The current anchor is reserved. The rest rotate through a cursor persisted in discovery_cursors, so a backlog larger than the share is covered in a finite number of ticks and a restart resumes the rotation instead of starting from the same end. The relationship set is bounded and rotated too. Promising every current anchor a read stops being possible once the relationship count passes the budget, so a tick serves a rotating subset properly rather than promising everyone something it cannot deliver. Where the share is one, the anchor and the ring alternate: advancing a cursor past a candidate without reading it would be skipping work, not scheduling it. _observe also stops treating an observation as the end of a turn. A receipt written just after the completion was seen still has to be resolved, so the turn is skipped only when it has been observed AND has no unresolved staged claim.
Three defects an independent review of the previous two commits found, one of them mine. resolve_staged selects staged claims by thread and turn, and two assignments can share a child. The settlement rewrite queued every resolved event using the relationship the loop happened to be polling, so with overlapping artifact roots an event belonging to B could be queued to A's parent. Ownership now comes from each event's own relationship. The cross-delivery guard added earlier did not catch this because it compared the delivery row's relationship against itself, which is a tautology; it now reads the event's relationship. The relationship cursor advanced only when a tick actually read something, so a window of relationships with nothing to do pinned it and everything behind them waited forever. That is the same starvation the scheduler exists to remove, one level up. It now advances after the window either way. A transient failure inside daemon_observation was swallowed and settlement continued, which recorded the observation and suppressed the staged claim. The turn then never looked new again and nothing was left for recovery to find, so the failure notification was lost permanently. A refusal is still a decision and settlement proceeds, but a transient failure now leaves the turn untouched for the next tick.
The four remaining findings from the review of this branch. Recovery was derived from "final event with no delivery row", which is not the question it was asked. That shape also matches a receipt emitted with --no-enqueue and an event stranded by a generation that has since moved on, so the scan would have sent reports nobody wanted. Intent is now recorded explicitly, in the same transaction as the observation that produced the event, and recovery reads that instead of guessing from what is missing. Each refusal backs its own retry off, so four permanently unqueueable events can no longer hold every recovery slot against events that would succeed. Writing the intent exposed a real hole the derived scan had hidden: enqueue_in does not validate and enqueue does, so moving settlement onto enqueue_in had quietly dropped the require_active and recipient checks the old path got for free. Settlement now asks for that authorization itself, inside the same transaction, which is also what makes the injected refusal in the test model a branch that exists in production. Bulk deliver binds pending anchors for the same reason the single-event path does. The two tests that called bind_pending_anchors themselves are renamed to say they cover the helper; the route evidence is the tick test, which calls nothing but tick. Six scheduling tests cover what one relationship could not: more relationships than the read budget, a served window with nothing to do, a backlog deeper than the share, rotation across a restart, and a turn that never answers. The empty-window case is the one that would have caught the cursor only advancing after a read.
delivery.eligible was one ORDER BY created_at LIMIT across every relationship, so a parent with a large older backlog filled the window by itself and a parent with one newer row was never seen. The review measured it: with forty thousand older rows for A, eligible(limit=4) returned only A. Selection now starts from the question that cannot be crowded out - which PARENTS are eligible - and then takes a bounded share from each, dealt one at a time rather than in contiguous blocks. Blocks leave the last parent short whenever the budget is not a multiple of the share, which showed up as an 18/9/9 split over nine windows. _reconcile had the same shape one layer over, and there a gate skip still consumed its place in the prefix, so another parent's revision never reached dispatched and its anchor never bound. open_attempts gains a bounded, parent-filtered form while keeping its exhaustive no-argument form, because recover_on_start has to see everything. It is deliberately not filtered on active status: an unresolved send belonging to a cancelled assignment still needs its evidence settled. The rotation advances by one position per window, not by the parent count, which wraps to the same head and hands the odd slot to the same parent forever. My own test caught that. Error isolation is per parent and lasts one tick. It keys on parent_task_id rather than the recipient, since revisions target children, and it triggers on returned outcomes as well as exceptions - a busy parent is a deferral, not a raise, and that was the case consuming whole ticks. A skip reserves no capacity, opens no attempt and creates no hold. Transport execution isolation is not claimed here: the adapter serialises on one worker, so a stalled call still blocks the one behind it. What this guarantees is that the scheduler stops handing a struggling parent the rest of the budget.
Reproduced on 2026-09-16: JUN-119 generation 2 events delivered after generation 3 opened, and JUN-100 generation 5 delivered after generation 7. Every one was rejected downstream as disposition_conflict: stale_generation. They had been waiting on a busy parent, and the wait ending was treated as permission to send. A generation that has moved on now invalidates every outcome of the previous one — ready, blocked, failed, manifest or not — whether or not the new generation has produced a revision yet. That last part is the case the reproduction turned on: an empty new generation is not a reason to send the old one. The decision is not a preflight. A preflight can be overtaken, because the generation can advance while the lifecycle and turn-list reads are in flight, which is precisely how a superseded event went out after its busy wait. The claim statement itself now refuses a delivery whose event belongs to an older generation, so there is no window to lose. Terminal suppression is restricted to the three states that are provably unsent. An outstanding send is annotated instead: reconciliation refuses to promote a terminal superseded aggregate, so rewriting one would turn a lost response into something that can never be resolved. mark_superseded's guard moved into its UPDATE for the same reason a preflight was wrong there. A staged successor does not suppress anything. It is a claim, not a replacement, and if it later fails the older revision is the only finished one there is — destroying its delivery chance on the strength of a claim would be permanent. Suppression writes state and journals and makes no transport call at all, so a stale event cannot wake the parent or open a generation.
… looking withheld_pre_send meant five different things at once: the receipt had not been collected, the parent was mid-turn, the host would not confirm the authorized settings, a turn had started, or the acknowledgement was outstanding. The cause is the only part that suggests an action, and it was the part nothing recorded. failed_operations keeps the most recent cause per subject and operation, fed from returned failure values as well as exceptions. That distinction is the whole point for the case that matters most: a settings rejection never raises. The host answers with a failed receipt carrying settingsFindings, and classify_operation_receipt keeps only the error code, so the field-level difference is read from the raw receipt before classification discards it. Observation health answers a question a live process cannot. observations records terminal turns only, so a healthy long-running anchor has no row there at all and would read as stale forever; poll_observations records that we looked. A failed read updates the attempt time and never the success time, because an anchor whose first read failed has never been polled and saying otherwise is the one lie that matters here. That is the shape of the JUN-100 and JUN-101 incident: a live pid, inside its time bound, polling nothing useful and delivering nothing. Liveness is reported separately and is never counted as health.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
The pre-send supersession check ran before the claim, and the claim's own predicate rejected only an older execution generation. A newer final revision of the SAME generation committed between the two would still be claimed and sent, which is reachable when a CLI emission races the daemon. The check is now repeated inside the claim transaction, and the discovery there is recorded through the same path and reported identically. The phase taxonomy replaced one vague word with a confident wrong one. held_uncertain means the transport gave no usable answer, which is not a turn having been accepted, and a settled withheld_pre_send can be an ordinary thread/read failure rather than a settings mismatch. Both now come from the attempt record's failedOperation rather than the delivery state, and an unanswered send reports outcome_unknown. Observation health was built from poll_observations, so an anchor with no poll row was absent from the set instead of counted as never polled — precisely the relationship a rotating scheduler has not reached yet. It is now built from the active current generations and left-joined to their polls, so an untouched anchor reads as stalled rather than vanishing.
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 1 new potential issue.
🐛 1 issue in files not directly in the diff
🐛 Upgrades bypass existing scope locks
When an existing daemon uses a symlinked socket path, upgraded key hashes a different identity than that daemon. The upgraded daemon acquires a second lock and serves the same App Server concurrently.
There was a problem hiding this comment.
Note
This report is out of date. Scroll down for Devin Review's latest report on this PR.
Devin Review found 1 new potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ Supervisor deadline overrun
When deadline has under 100 ms remaining, segment_seconds expands the worker allowance to 100 ms. The worker can start another tick past the supervisor’s bound.
status --relationship filtered the delivery list but left the observation block global, so an operator asking about one assignment saw every assignment's staged backlog, anchors and poll ages beside it. A filter that narrows one half of a payload and not the other is worse than no filter: the numbers read as if they belong to the assignment that was asked about. observation_health now takes an optional relationship_id and applies it to all three queries it runs, the staged events, the current anchors and the per-relationship backlog, and cmd_status passes the same --relationship it already passes to snapshot. Unscoped callers are unchanged. The CLI test registers two assignments, stages a receipt under each, and reads status both ways. Against the previous cli.py it fails with both event ids in the scoped list.
Five findings on what the tick reports and what it can reach. A dispatched revision was reported as awaiting_ack, but contract v1 defines an acknowledgement for the child-to-parent direction only and AckService refuses one for a revision. The child answers a revision request with its next completion receipt, so every successfully dispatched revision sat on an obligation nothing was allowed to meet. It now reports awaiting_child_receipt, and the completion direction still reports awaiting_ack. A delivery withheld because its authorized settings are missing or unusable is refused before anything is claimed, so no attempt record exists - and the phase taxonomy reads the cause from the attempt. Status therefore called the most actionable failure in the set awaiting_receipt, which points at the child instead of the settings nobody recorded. The refusal is now written to failed_operations with its reason and next retry, and the phase falls back to that when there is no attempt. observation_health aged every anchor, but _worth_polling deliberately stops scheduling a turn once it is terminal with nothing staged behind it. Its last poll can never advance again, so a quiet, fully observed assignment reported stalled forever once stale_after had elapsed - the same word this exists to reserve for a loop that has actually stopped looking. Such anchors are now marked settled and excluded from freshness, and a late staged receipt on that turn un-settles it. Reconciliation had a cursor over parents and none over the attempts inside a parent. _gate skips an attempt whose fingerprint is unchanged, but the skipped attempt still held its place in the share, so a parent with more unresolved attempts than its share re-read the same leading ones every tick and never reached the rest - the same starvation the parent cursor was added to fix, one level down. open_attempts takes an offset, and each parent now has its own persisted cursor that wraps. The pre-send backoff computed base * 2 ** (attempts - 1) and clamped afterwards. An intent that stays legitimately unqueueable has no cap on its attempt count, and at the 1025th refusal the product is an integer too large to convert to a float. The OverflowError escaped the refusal handler, the transaction rolled back with the intent still due, and every later tick failed identically. The regression test reproduces it end to end: OverflowError: int too large to convert to float at the 1025th refusal. Also removed two docstring lines duplicated by an earlier patch, in _message_status and _worth_polling. Relay suite 588 passed, 30 skipped. validate.py, contracts.py, check_operations_contract.py, the scripts/ci/tests unittest suite, secrets.sh and git diff --check all exit 0.
…utstanding sends A failed turn shared by two assignments only reached one parent. _already_observed and _worth_polling both asked globally - thread and turn, no assignment - so the first assignment's settlement closed the turn for every other one watching it. The second never reached _synthesize, and a failed turn SUPPRESSES the staged claim rather than finalizing it, so its parent was left waiting on a verdict that could never arrive. Both questions are now asked per assignment. The regression test registers two parents on one child turn, fails it, and asserts both get a terminal outcome and that neither is settled twice. A delivery that was sending or held_uncertain when a newer generation opened never reached the pre-send supersession check, because attempt() rejects a non-claimable state and nothing else called it. It could reconcile to dispatched with no supersession row at all and be presented as an ordinary current delivery rather than as history. open_generation_in now annotates outstanding deliveries of earlier generations in the same transaction that advances the generation. Annotated, never rewritten: reconciliation cannot promote a terminal superseded aggregate, so rewriting one would make a lost response permanently unresolvable. A generation whose anchor is still pending has no dispatch turn - which is the normal state between a needs_changes verdict and the revision being dispatched - and observation_health counted it as an anchor that had never been polled, so a relay behaving exactly as designed reported stalled. Pending anchors are reported as such and excluded from freshness, and are held to the same freshness the moment they bind. Also removed a stray pass_placeholder that preceded _verify_acks's docstring and left __doc__ None. Relay suite 601 passed, 30 skipped. validate.py, contracts.py, check_operations_contract.py, the scripts/ci/tests unittest suite, secrets.sh and git diff --check all exit 0.
…ing predecessors Four findings on the tick, three of them defects introduced by the previous two rounds. The per-parent reconciliation cursor advanced inside the selection, before the budget decided what was actually dealt. With more parents than the budget every parent was selected and had its cursor moved, and only the first budgeted queues were handled - so the parents that got nothing had their leading attempts stepped over unread. The cursor now advances by what was dealt. The regression test shows twelve cursors moving for eight reconciled parents. I could not construct a case where the old behaviour skipped an attempt PERMANENTLY rather than repeatedly delaying it; the parent rotation and the attempt cursors drift against each other and eventually cover everything. What is demonstrably wrong is the advance itself, and that is what the test pins, together with the invariant that every unresolved attempt is reached in a finite number of ticks. The observed subquery behind the settled-anchor check asked only by thread and turn, so one assignment's observation marked another settled on a shared child anchor - excluding an assignment whose own settlement was still outstanding from the freshness check that would have surfaced it. Scoped per assignment, like the scheduler's own check. The generation-advance annotation excluded dispatched. Its acknowledgement will be refused as stale_generation, so status reported awaiting_ack for an obligation that can no longer be met, with nothing to say why. Annotating it does not rewrite what was sent. An older revision already in flight when its successor became final within the SAME generation was never annotated either: the pre-send check cannot reach it, because attempt() returns early for a non-claimable state, and the registry hook only fires on generation advance. Finalizing an event now annotates the predecessors it replaces, in the same transaction, asking _supersession_reason per candidate rather than assuming. The test declares the lineage the way a revision that replaces another does when it is emitted; without that declaration the generation has two unsuperseded revisions and no head at all, which is the separate ambiguity question recorded on the other thread. Relay suite 610 passed, 30 skipped. validate.py, contracts.py, check_operations_contract.py, the scripts/ci/tests unittest suite, secrets.sh and git diff --check all exit 0.
Five findings, two of them defects the last round introduced. The generation-advance annotation was written and then ignored by the thing that reads it. An outstanding send whose generation has moved on keeps its state on purpose, so a lost response stays reconcilable - but status went on reporting awaiting_ack or outcome_unknown for it. The phase now reports superseded with the reason, and the delivery state is still left alone. staged_here counted by thread and turn while the observation beside it counts per assignment, so a shared anchor let one assignment's staged claim unsettle another that has nothing of its own outstanding - which then ages into a stall with nothing wrong. Scoped to match. annotate_predecessors_in hung off settlement alone. A receipt the host already reports as terminal never goes through settlement; it is accepted and enqueued directly, so a successor arriving that way annotated nothing. The call moved to enqueue_in, which every route by which an event becomes deliverable passes through. turn_accepted was reported for a failed turn/start with no turn id. That is a call that was REFUSED, not one whose answer was lost, and the phase claimed a turn exists on no affirmative evidence at all - pointing an operator at a turn nobody can find. A turn id is now required; the started-then-lost case still reports turn_accepted. settings_rejected was reported for any failed thread/resume, including ordinary connectivity and internal failures, which hands an operator a remediation that cannot work. The recorded failure is the discriminator: _settle writes settings_check only when the receipt actually carried field-level findings. The pre-send backoff bound is derived from its own policy ratio now, matching the same correction in restart_delay_for. Known limitation, recorded rather than fixed: the observations primary key is thread, turn and terminal status, with no relationship. Two assignments sharing a child turn can each settle it and each get their own receipt - that is what the shared-turn fix restored - but only the first records an observation row, so the other re-settles idempotently on later ticks and never reads as settled in the health block. Giving observations a per-assignment key is a schema change with no migration path in this store, and it is not this PR's surface. Relay suite 621 passed, 30 skipped. validate.py, contracts.py, check_operations_contract.py, the scripts/ci/tests unittest suite, secrets.sh and git diff --check all exit 0.
The observations table is keyed by thread, turn and terminal status, with no relationship, so when two assignments watch the same child turn only the first records a row. The per-assignment scoping added in the last two rounds asked a question that storage could not answer: every other assignment on that turn looked permanently unsettled, was re-selected on every round, settled again idempotently, reported the tick non-quiet and spent observation budget forever. assignment_settlements records which assignment has settled which turn. It is a new table rather than a re-keyed observations, because this store has no migration path and CREATE TABLE IF NOT EXISTS would silently leave an existing database on the old key - so the fix would work on a fresh store and not on the one that needed it. A new table appears on the next open of either. The scheduler's two questions and the health block's settled calculation read it. observations is untouched and still holds one row per turn, which is what the contract describes. The test that previously pinned this as a limitation now asserts the behaviour: both assignments settle a shared turn for themselves, the turn table still holds exactly one row, health reports both settled, and the next tick observes nothing. Relay suite 624 passed, 30 skipped. validate.py, contracts.py, check_operations_contract.py, the scripts/ci/tests unittest suite, secrets.sh and git diff --check all exit 0.
test_cli.py conflicted: both sides are wanted, the scoped-status tests from this branch and the service exit-code tests from PR-A.
…too little assignment_settlements arrives empty on an existing store, and the scheduler and the health block ask it instead of observations. Every historical terminal turn therefore looked unsettled, so a current turn that can no longer be read would leave a previously settled assignment stalled forever and spending polling budget - a regression introduced by the fix that removed the previous one. It is backfilled on open from the observation rows that name their relationship. Rows written before that column existed name nobody and cannot be attributed to one. awaiting_receipt was the fallback for everything the taxonomy did not name, which made it the answer for two states it is wrong about. A delivery row exists only because the receipt was collected and accepted, so a queued delivery is waiting on this relay reaching the recipient, not on the child: awaiting_send. A sending delivery whose process stopped after committing the claim has an attempt that may already need reconciliation: in_flight. Both previously sent an operator to the child for a delay that was never the child's. Two existing tests asserted awaiting_receipt for a queued delivery. They encoded the mislabel rather than a requirement, and they now assert the stage that is actually outstanding. Relay suite 639 passed, 30 skipped. validate.py, contracts.py, check_operations_contract.py, the scripts/ci/tests unittest suite, secrets.sh and git diff --check all exit 0.
| # stale_generation, so leaving it unannotated meant status showed awaiting_ack | ||
| # for an obligation that can no longer be met. Annotating does not rewrite the | ||
| # delivery, so the history of what was actually sent is untouched. | ||
| " AND d.state IN ('sending','held_uncertain','dispatched')" |
There was a problem hiding this comment.
🟡 Capped stale deliveries hide supersession
When a generation advances, capped deferred_busy and withheld_pre_send deliveries receive no supersession annotation. Their hold bypasses attempt() forever, so status keeps reporting an obsolete cap.
Learn more
A delivery can reach its busy or pre-send attempt cap while its state is deferred_busy or withheld_pre_send. The cap sets hold_reason, and attempt returns before checking supersession whenever that field is set. Generation advancement therefore provides the only automatic opportunity to annotate these held rows. The current query covers unresolved sends and dispatched history, but omits both capped retryable states. Their status remains a current-looking cap even though the generation has invalidated them.
Example: A generation-2 completion reaches busy_cap in deferred_busy. Generation 3 opens. The delivery remains held:busy_cap; it never becomes superseded:stale_generation because no later attempt can pass the hold guard.
Recommended fix: Include deferred_busy and withheld_pre_send in the generation-advance annotation query. Keep the delivery state and hold unchanged; the existing supersession sidecar then makes _phase report the stale-generation reason.
| " AND d.state IN ('sending','held_uncertain','dispatched')" | |
| " AND d.state IN ('sending','held_uncertain','dispatched'," | |
| " 'deferred_busy','withheld_pre_send')" |
Was this helpful? React with 👍 or 👎 to provide feedback.
An event whose delivery was wanted and refused has no deliveries row by design, and snapshot was built only from deliveries. A permanently paused or unauthorized assignment therefore had no status entry, no phase and no retry time at all while the daemon went on retrying it - the most stuck state in the system was the one status could not show. status reports pendingIntents beside the deliveries now, scoped by the same --relationship. A paused, cancelled, archived or superseded assignment is dropped by the scheduler and its staged claim will never be settled, but the health block went on ageing it, so one such claim held the whole block at degraded indefinitely while every active assignment was fine. The staged query joins through relationships with the same active predicate the anchors already use. A predecessor that settled as inbox_only is terminal and attempt() cannot revisit it, so it stayed reported as channel_closed with no supersession note even though acknowledgement currency already rejects it. Added to the annotation candidates. emit --no-enqueue accepts a terminal successor and deliberately never queues it, and the annotation was riding on enqueue, so a predecessor already in flight kept being presented as current. Whatever an event replaces stops being current when that event becomes final, whether or not anyone asked to deliver it. A suppressed delivery - superseded before any transport call - was counted as delivered, which is the opposite of what that counter is read for. It counts as skipped. Relay suite 643 passed, 30 skipped. validate.py, contracts.py, check_operations_contract.py, the scripts/ci/tests unittest suite, secrets.sh and git diff --check all exit 0.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 342c95839b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if parent in struggling: | ||
| # Skipped for the REST OF THIS TICK only. It reserves no capacity, opens no |
There was a problem hiding this comment.
Rotate past a parent's persistently failing delivery
When a parent's oldest selected delivery repeatedly raises before changing its state, this skip suppresses every later delivery selected for that parent, while eligible_for_parent() returns the same oldest prefix on every tick because delivery has no per-parent row cursor. This can permanently starve valid later work—for example, two assignments sharing a parent can have revision requests targeting different children, and an exception while observing the first child prevents the second child's request from ever being attempted. Rotate within each parent's delivery backlog or continue scanning after the failing candidate.
Useful? React with 👍 / 👎.
| current = turn_id | ||
| else: | ||
| history.append(turn_id) | ||
| staged = [row["turn_id"] for row in self.intake.staged_events(thread_id=thread)] |
There was a problem hiding this comment.
Exclude inactive assignments from shared-child settlement
When paused or cancelled assignment A has a staged receipt and active assignment B shares its child thread, this unscoped lookup adds A's turn to B's polling ring. B then calls the globally scoped resolve_staged_in(), which finalizes A's event and creates a delivery intent that is retried despite A being absent from _active_relationships(); a paused assignment therefore continues being processed through another assignment. Scope staged candidates and settlement to the relationship being served.
Useful? React with 👍 / 👎.
| staged = [row["turn_id"] for row in self.intake.staged_events(thread_id=thread)] | ||
| ring = [ | ||
| turn_id for turn_id in dict.fromkeys(staged + history) | ||
| if turn_id != current and self._worth_polling(thread, turn_id, rid) |
There was a problem hiding this comment.
🔴 Shared-child failure loses terminal outcome
When assignments share a child, _turns_to_poll schedules staged continuation turns without checking their owner. A failed turn is suppressed globally before its owner can synthesize a failure receipt. The owner’s parent never receives the terminal outcome.
Learn more
The observation ring combines staged turns by child thread, but a child thread can serve several assignments. A staged continuation turn can belong only to assignment B and be unadmitted to assignment A. If A polls that failed turn first, daemon_observation refuses to synthesize A's receipt, while resolve_staged_in suppresses B's staged event. The turn then disappears from B's staged ring unless it is also a generation anchor, so B never synthesizes its own failure event.
Example: Assignments A and B share child child-1. B stages a receipt on admitted continuation turn-b2, then that turn fails. A selects turn-b2 from the global staged list, suppresses B's claim, and records no B failure receipt; B's parent remains waiting.
Recommended fix: Scope staged candidates in _turns_to_poll to relationship_id, or carry event ownership through selection and settle only the owning assignment. Add a regression with a shared child and a failed staged continuation that is not either assignment's anchor.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for row in self.store.all( | ||
| "SELECT relationship_id, COUNT(*) AS n FROM events WHERE stage = 'staged'" | ||
| + (" AND relationship_id = ?" if relationship_id else "") | ||
| + " GROUP BY relationship_id", | ||
| (relationship_id,) if relationship_id else (), |
There was a problem hiding this comment.
🟡 Inactive work remains in backlog
For an inactive assignment, observation_health still includes staged rows in backlog. The staged-events query filters active relationships, but the backlog query does not. Status reports work the scheduler will never process.
Learn more
The scheduler only observes active, non-superseded relationships. The stagedEvents query uses that same predicate, but the backlog aggregation reads directly from events. As a result, the two fields disagree after an assignment is paused, cancelled, archived, or superseded.
Example: Assignment A has one staged event and is then cancelled. stagedEvents becomes empty and health can be healthy, while backlog still reports A: 1.
Recommended fix: Join relationships in the backlog query and apply status = 'active' AND superseded_by IS NULL, preserving the optional relationship filter.
Was this helpful? React with 👍 or 👎 to provide feedback.
Dependent on #8. Base is
codex/jun-103-shared-relay, so review this on top of that branch.Problem
PR #8 gave the relay a shared store and an owner. This one fixes what the loop does each
tick. Every defect below was reproduced in real operation before it was fixed here.
An anchor bound on only one route
The single automatic binding hook ran after the daemon's own new-dispatch branch. A revision
that reached
dispatchedany other way, thedelivercommand, either reconcile promotion, or adispatch committed in the last tick before shutdown, left its generation
anchor_pending, andby I-06 every later receipt for that generation was refused as unbound. The child's work had
nowhere to land.
The existing suite missed it because its one binding test calls
bind_dispatched_revisionbyhand. Binding is now recovery over state:
bind_pending_anchorsfinds every generation stillpending whose revision actually dispatched, runs first in the tick so a receipt arriving in
the same tick is accepted, and repairs generations stranded before this existed.
A refused enqueue lost the event permanently
_settle_turnrecorded the terminal observation before queuing what it implied, and swallowedthe enqueue error. A relationship paused between selection and queuing left a final event with
no delivery row, and the next tick skipped the turn through
_already_observed, so resumingthe relationship never helped.
Finalizing, recording and queuing are now one commit. The two kinds of failure are no longer
treated alike: a durable refusal (paused relationship, unauthorized recipient) is a legitimate
answer, so the observation stands and
_requeue_missingpicks the event up once the refusallifts; anything else rolls the whole transaction back so the next tick re-observes cleanly.
The obligation is derived from state rather than kept in a queue, so a refusal that lifts is
retried without the turn ever having to look new again. It is scoped on purpose: recovery
reads the recorded intent, so an event stranded before this existed has no intent row and is
not discovered. The alternative rule, a final event with no delivery row, also matches an
event emitted with --no-enqueue, and sending something nobody asked to send is the worse
failure. Anchor binding IS repaired retroactively, because it derives from state alone.
An observation budget that never reached the ninth turn
_turns_to_pollcollected turns oldest generation first and kept only the firstmax_reconciles_per_tick, and_observefiltered already-observed turns after that slice.Every tick re-read the same completed prefix, so a relationship past eight generations never
had its current anchor looked at again. Two live assignments sat staged with
delivery=nullbehind a healthy-looking daemon.
Selection now runs a rotating, bounded scheduler: unresolved work is chosen ahead of settled
work, the cursor advances so no relationship holds the window, and turns that are already
observed and have nothing staged do not consume budget. A late receipt staged after the
completion observation is picked up rather than skipped, which
_already_observedalone gotwrong.
One busy parent taking every other parent's turn
eligiblewas a singleORDER BY created_at LIMITacross all relationships, so a parent witha large older backlog filled the window by itself, and
_reconcilehad the same shape with agate skip still consuming its place. Both now allocate per parent, so a parent that is busy,
erroring or backed up cannot starve a parent that is ready. Ending or cancelling one
assignment does not stop the shared service.
A stale receipt sent and then rejected
Receipts that waited behind a busy parent were sent after a newer generation had already
opened, and were refused downstream as
stale_generation. The check now happens before thesend and again inside the delivery claim, so a supersession that lands mid-flight cannot slip
through the gap. Every outcome of a superseded generation is suppressed, including
blocked_needs_inputwith no manifest, and including the case where the current generation hasno revision yet. Attempts already delivered stay in the history; they are simply not counted
as the current generation being verified.
Diagnostics that name the stage
withheld_pre_sendcovered a receipt not yet collected, a parent mid-turn, a rejected settingscheck, a started turn and an outstanding acknowledgement. Deliveries now carry a phase, the
most recent
failedOperationwith its error code, the specific settings difference and the nextretry time. A closed delivery channel is queryable rather than hidden.
observation_healthreports staged-event ages, when each current anchor was last polled and per-relationship
backlog, so a live pid is never counted as the loop working.
status --relationshipscopes thathealth block to the assignment asked about.
A launch reported as running after it finished
Carried in from #8 through the merge:
startmatched the child's record on the launch id andthe daemon lock, and both stay true through supervision's cleanup, so a launch that finished
during startup briefly looked healthy. A recorded pid is now required as well.
Red before green
Every fix has a regression test captured failing against the unmodified source. The two
absorbed JUN-167 defects:
The enqueue test asserts the intermediate defect state explicitly, observation recorded, event
final, no delivery row, so it cannot pass for the wrong reason.
Reporting a stage nothing can leave
A dispatched revision reported
awaiting_ack, but contract v1 defines an acknowledgement forthe child-to-parent direction only and
AckServicerefuses one for a revision, so everydispatched revision sat on an obligation nothing was allowed to meet. It now reports
awaiting_child_receipt. A delivery withheld because its authorized settings are missing isrefused before an attempt exists, and the phase taxonomy reads its cause from the attempt, so
the most actionable failure in the set reported
awaiting_receiptand pointed at the child.The refusal is now persisted and the phase falls back to it.
observation_healthaged every anchor, including the ones the scheduler deliberately stopsreading once their turn is terminal with nothing staged behind it. Those reported
stalledforever with nothing wrong. They are marked settled and excluded from freshness now, and new
staged work on the same turn un-settles them.
Reconciliation had a cursor over parents and none over the attempts inside a parent, so a
parent with more unresolved attempts than its share re-read the same leading ones every tick
and never reached the rest. Each parent now has its own wrapping cursor.
The pre-send backoff built
base * 2 ** (attempts - 1)and clamped afterwards, so the 1025threfusal of a permanently unqueueable intent raised
OverflowError, rolled the transaction backwith the intent still due, and failed identically on every later tick. It is bounded before
the exponent is evaluated.
Shared turns, pending anchors and outstanding sends
A failed turn shared by two assignments only reached one parent:
_already_observedand_worth_pollingboth asked globally, so the first assignment's settlement closed the turn forthe other, which never synthesized its own receipt and was left waiting on a verdict that
could never arrive. Both are scoped per assignment now.
A delivery that was
sendingorheld_uncertainwhen a newer generation opened never reachedthe pre-send check, so it could reconcile to
dispatchedwith no supersession row and bepresented as current. Opening a generation now annotates them in the same transaction.
A generation whose anchor is still pending has no turn to poll, which is the normal state
between a verdict and the revision being dispatched, and it was being counted as an anchor
nothing had ever read.
Verification
The packaged gate is the one that counts: packages.py exits 0 with codex-thread-bridge 119
tests and codex-session-relay 650 tests, and it rejects any skipped case. A direct pytest run
in this working tree reports 601 passed / 30 skipped; those 30 are cases the packaged gate
does not exercise, the bridge-adapter suite without a live App Server and schema conformance
without its fixtures.
packages.pybuilds and installs both packagesand runs both suites.
validate.py,contracts.py, thescripts/ci/testsunittest suite,secrets.shandgit diff --checkall exit 0.No daemon was registered or enabled. Every probe ran under a temporary
CODEX_HOME, an explicittemporary
--state-dirand a matching temporaryCODEX_SESSION_RELAY_STATE; the real~/.codex/hooks.jsonsha256 is unchanged.Not in scope
The two remaining JUN-167 findings in
ack.py, the re-review verdict deadlock and the ACKrace, are deliberately untouched.