Skip to content

fix(runtime,verify): make simulation delivery single-owner (ARN-236) - #387

Open
rita-aga wants to merge 10 commits into
mainfrom
codex/arn-236-simulation-delayed-delivery
Open

fix(runtime,verify): make simulation delivery single-owner (ARN-236)#387
rita-aga wants to merge 10 commits into
mainfrom
codex/arn-236-simulation-delayed-delivery

Conversation

@rita-aga

@rita-aga rita-aga commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes ARN-236: delayed simulation messages could be removed from pending scheduler ownership without being consumed by the verifier/runtime driver, causing silent loss at the simulation horizon.

The implementation establishes one deterministic ownership path:

  • tick() advances logical time and enqueues due messages into bounded scheduler mailboxes.
  • cyclic, budgeted drain_ready() transfers each message exactly once.
  • terminal-tick flushing drains every already-due batch without advancing time.
  • completed + in-flight reservations bound random action admission.
  • callback cascades share a bounded reaction budget and terminalize the run on rejection/exhaustion, preventing stale work from resuming through either public execution API.
  • runtime and verifier now use the same delivery model.

ADR: docs/adrs/0171-single-owner-simulation-delivery.md.

Evidence

Scope

No files under crates/temper-actor-runtime are modified. Do not merge as part of the ARN-165 arena; this PR is submitted for review only.

Linear: ARN-236

Greptile Summary

This PR fixes ARN-236 by establishing a deterministic single-owner delivery path for simulation messages: tick() enqueues due messages into bounded per-actor mailboxes, and drain_ready() is the sole transfer point to simulation drivers. Both SimActorSystem (runtime) and run_simulation_impl (verifier) now use the same drain-loop pattern with terminal-tick flushing and shared reaction budgets.

  • SimScheduler gains a bounded mailbox per actor, drain_ready() for cyclic starvation-free bulk draining, and with_mailbox_budget() constructor; tick() no longer returns messages directly.
  • SimActorSystem::run_random() drives a drain_ready inner loop with terminal-tick flushing, budget-tracked integration callbacks, and ensure_execution_active() guards that prevent stale work after any callback rejection or budget exhaustion.
  • temper-verify/src/simulation.rs is aligned to the same drain loop model, with per-actor reservation accounting via actor_in_flight_actions and assert-backed single-delivery guarantees.

Confidence Score: 5/5

Safe to merge — the single-owner delivery contract is correctly implemented across both crates, the previous callback-cascade and budget-exhaustion findings are addressed, and the RED/GREEN regression suites cover the exact failure modes from ARN-236.

The drain-loop, terminal-tick flushing, reservation accounting, and invalidation path all behave correctly. Test coverage spans single-seed, multi-seed, cross-mode (step to run_random), callback rejection, budget exhaustion, and exact-once delivery across 16 seeds. The only finding is a cosmetic naming inconsistency on the loop variable in run_random().

No files require special attention.

Important Files Changed

Filename Overview
crates/temper-runtime/src/scheduler/core.rs Adds bounded per-actor mailboxes, cyclic drain_ready(), and with_mailbox_budget() constructor. tick() now enqueues to mailboxes (single owner). Logic is correct; run_until_quiescent() correctly documents that callers must drain separately.
crates/temper-runtime/src/scheduler/sim_actor_system.rs Core of the fix: run_random() drives a drain-loop with terminal-tick flushing; ensure_execution_active() guards prevent stale resumption. One minor style issue: the loop variable is named _tick despite being actively used in the break condition.
crates/temper-runtime/src/scheduler/sim_actor_system/callbacks.rs Callback cascade termalization via invalidate_callback_cascade() correctly clears pending callbacks and records the error; both rejection and budget exhaustion call this path. No issues found.
crates/temper-verify/src/simulation.rs Aligned to single-owner drain model with per-actor actor_in_flight_actions accounting, assert-backed delivery invariants, and identical terminal-tick flushing logic. Uses tick (correct) as the loop variable name, consistent with Rust conventions.
crates/temper-runtime/src/scheduler/sim_actor_system/random_budget.rs Clean extraction of reserve/release helpers with assert-backed invariant (reservation must be owned before release). Correct.
crates/temper-runtime/src/scheduler/core/tests.rs Good coverage of drain ownership, starvation-free ordering, mailbox budget fast-fail, exact-once delivery across 16 seeds, and send_at semantics.
crates/temper-runtime/src/scheduler/sim_actor_system/tests.rs Tests cover callback rejection, budget exhaustion, terminal-tick flushing, step-to-step and step-to-run_random cross-mode contamination, and run_record equality. Solid regression suite.
crates/temper-verify/src/simulation/tests.rs RED regression for delayed final-tick delivery and multi-message terminal-tick flushing are clearly structured and cover the exact failure mode from ARN-236.
crates/temper-runtime/tests/public_api_compat.rs Exhaustive struct-literal test guards SimActorResult against unintentional public field additions. Correct and sufficient.
docs/adrs/0171-single-owner-simulation-delivery.md ADR present as required by team policy for multi-crate architectural changes.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Driver as Sim Driver
    participant Sched as SimScheduler
    participant Mailbox as Actor Mailboxes
    participant Handler as SimActorHandler

    Driver->>Sched: send + reserve()
    note over Sched: message in pending heap
    Driver->>Sched: tick()
    Sched->>Mailbox: enqueue due messages
    Driver->>Sched: drain_ready(batch_budget)
    Sched->>Mailbox: pop_front() cyclic
    Sched-->>Driver: Vec SimMessage (owned once)
    loop each message
        Driver->>Driver: release(in_flight)
        Driver->>Handler: apply_action()
        Handler->>Driver: schedule_integration_callbacks()
    end
    Driver->>Driver: deliver_integration_callbacks
    alt budget exhausted or rejected
        Driver->>Driver: invalidate and break
    else terminal tick with ready messages
        Driver->>Sched: drain_ready again
    else normal
        Driver->>Driver: break inner loop
    end
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Driver as Sim Driver
    participant Sched as SimScheduler
    participant Mailbox as Actor Mailboxes
    participant Handler as SimActorHandler

    Driver->>Sched: send + reserve()
    note over Sched: message in pending heap
    Driver->>Sched: tick()
    Sched->>Mailbox: enqueue due messages
    Driver->>Sched: drain_ready(batch_budget)
    Sched->>Mailbox: pop_front() cyclic
    Sched-->>Driver: Vec SimMessage (owned once)
    loop each message
        Driver->>Driver: release(in_flight)
        Driver->>Handler: apply_action()
        Handler->>Driver: schedule_integration_callbacks()
    end
    Driver->>Driver: deliver_integration_callbacks
    alt budget exhausted or rejected
        Driver->>Driver: invalidate and break
    else terminal tick with ready messages
        Driver->>Sched: drain_ready again
    else normal
        Driver->>Driver: break inner loop
    end
Loading

Reviews (5): Last reviewed commit: "fix(runtime): preserve result API compat..." | Re-trigger Greptile

Copy link
Copy Markdown
Collaborator Author

Independent GPT-5.6 re-review of PR #387 at head 53dc74a8.

No blocking findings.

Reviewed the open GitHub PR diff and current-head files for correctness, durability, deterministic-simulation behavior, budgets, compatibility, and regression coverage.

  • Single ownership: tick() only advances logical time and moves due messages into scheduler mailboxes; drain_ready() consumes them. There is no retained clone/parallel delivery path, and scheduler tests verify a message cannot be returned twice.
  • Final-tick batch defect: both runtime and verifier drivers repeat bounded drains only on the terminal tick until has_ready_messages() is false. The new regressions use a batch budget of 1 with two messages due at the horizon and require both transitions, covering the previously missed “more due messages than one batch” case.
  • Durability/budgets: undrained work remains mailbox-owned; per-actor mailbox retention is explicitly bounded and fails fast. The derived mailbox budget is sufficient for the maximum one-driver-action-per-tick admission pattern. Callback cascades are iterative, share one reaction budget across terminal batches, retain the pending callback on exhaustion, and surface an execution error rather than silently succeeding.
  • Action admission: completed plus in-flight reservations enforce max_actions_per_actor; reservations are released on both scheduler delivery and fault-drop, preventing delayed actions from over-enqueueing while allowing dropped actions to free capacity.
  • DST/replay: ordering uses BTreeMap plus FIFO VecDeque, with a persistent cyclic cursor for deterministic fairness. No wall clock, OS randomness, threads, or nondeterministic collections are introduced. Multi-seed delayed-delivery coverage checks exact message IDs for loss/duplication.
  • Compatibility: actor-specific receive() and run_until_quiescent() remain available; in-repository config literals and both simulation drivers are migrated to the explicit budget fields.
  • Truthfulness: callback rejection and reaction-budget exhaustion invalidate runtime simulation results, with regression coverage for scripted partial completion and random execution.

The architectural decision and tests align with the implementation. I found no correctness, durability, DST, or compatibility issue that should block ARN-236.

Verdict: PASS

@rita-aga
rita-aga marked this pull request as ready for review July 14, 2026 11:02
Comment thread crates/temper-runtime/src/scheduler/sim_actor_system/callbacks.rs Outdated
Comment thread crates/temper-verify/src/simulation.rs Outdated

Copy link
Copy Markdown
Collaborator Author

Fresh independent GPT-5.6 review of hosted head 2f53851410e4916b1977adde9093a63322c3291c.

Blocking finding:

  • crates/temper-runtime/src/scheduler/sim_actor_system/callbacks.rs:78-105 leaves callback work live after reporting a terminal execution error. On reaction-budget exhaustion, the popped callback is pushed back before Err is returned; on callback failure, any later queued callbacks likewise remain. A subsequent public step() starts with a fresh reaction counter and silently executes that stale work after the newly requested primary action. This can apply an extra transition after the caller was told the prior cascade failed, and it makes scripted replay depend on whether the caller happens to continue using an already-invalid run. Clear/terminalize the pending cascade on failure (or expose an explicit resumable state) and add a regression that calls step() again after both budget exhaustion and callback rejection.

I inspected the complete hosted PR file set, including scheduler ownership transfer, cyclic bounded draining, final-tick flushing, verifier/runtime action reservations, callback cascades, deterministic collections/time, ADR, and downstream config updates. I found no other blocking issue in those paths.

Verdict: FAIL

Copy link
Copy Markdown
Collaborator Author

Fresh independent GPT-5.6 review of hosted head 0dc906b998bb3841e0c569aafe8548bf626872c1.

No blocking findings.

I re-reviewed the complete PR diff and specifically re-audited the callback-lifecycle failure reported in #387 (comment):

  • Failure terminalization: callback rejection and reaction-budget exhaustion both flow through invalidate_callback_cascade, which clears the remaining callback queue, records one SimExecutionError, and returns the terminal error. Stale callback work cannot survive for a later public call.
  • step() → step(): step() calls ensure_execution_active() before advancing the clock, incrementing messages, or applying an action. The regression asserts that a post-failure step returns “simulation run is invalid” with no extra transition.
  • step() → run_random(): run_random() checks the same terminal state before entering the tick loop and returns an immutable result snapshot. Tests pin unchanged tick, transition, and message counts.
  • run_random() → run_random(): a random run invalidated by callback rejection cannot resume. The second invocation returns the same transition/message counts and does not advance the clock.
  • The guard is shared by callback rejection and budget exhaustion, so both failure causes have the same non-resumable lifecycle rather than separate partially resumable behavior.

I also verified the scheduler/durability design:

  • tick() only moves due messages into bounded mailboxes; drain_ready() is the sole consuming ownership transfer, eliminating the prior mailbox clone/returned-clone split.
  • Cyclic BTreeMap actor order plus per-mailbox VecDeque FIFO preserves deterministic, starvation-free delivery; the cursor persists across bounded drains.
  • The terminal tick drains every already-due batch without advancing time, and one reaction counter spans those batches. In-flight action reservations are released on both delivery and fault-drop.
  • Mailbox and callback budgets fail explicitly rather than dropping work silently. No wall-clock time, OS randomness, threads, or nondeterministic collections were introduced.
  • Focused validation passed: 10/10 actor-system tests, 15/15 scheduler tests, and 9/9 verifier simulation tests, including final-tick delivery, exact-once multi-seed replay, bounded drain, callback rejection, and reaction-budget exhaustion regressions.

Verdict: PASS

Copy link
Copy Markdown
Collaborator Author

ARN-236 live local E2E and validation evidence

Head: 0dc906b9

RED (before the fix)

cargo test -p temper-verify delayed_message_due_on_final_tick_is_delivered -- --nocapture

thread 'simulation::tests::delayed_message_due_on_final_tick_is_delivered' panicked:
assertion left == right failed: a message due on the final simulation tick must be delivered
  left: 0
 right: 1
total_dropped: 0

This reproduced durable loss without a fault drop: tick() removed the due message from pending ownership, while the verifier ignored the returned delivery.

GREEN (current head)

CARGO_INCREMENTAL=0 RUSTFLAGS='-C debuginfo=0' cargo test -p temper-verify delayed_message_due_on_final_tick_is_delivered -- --nocapture
running 1 test
test simulation::tests::delayed_message_due_on_final_tick_is_delivered ... ok
test result: ok. 1 passed; 0 failed

CARGO_INCREMENTAL=0 RUSTFLAGS='-C debuginfo=0' cargo test -p temper-verify final_tick_drains_every_due_message_when_batch_exceeds_budget -- --nocapture
running 1 test
test simulation::tests::final_tick_drains_every_due_message_when_batch_exceeds_budget ... ok
test result: ok. 1 passed; 0 failed

CARGO_INCREMENTAL=0 RUSTFLAGS='-C debuginfo=0' cargo test -p temper-runtime final_tick_drains_every_due_message_when_batch_exceeds_budget -- --nocapture
running 1 test
test scheduler::sim_actor_system::tests::final_tick_drains_every_due_message_when_batch_exceeds_budget ... ok
test result: ok. 1 passed; 0 failed

CARGO_INCREMENTAL=0 RUSTFLAGS='-C debuginfo=0' cargo test -p temper-runtime test_delayed_delivery_is_exactly_once_across_replay_seeds -- --nocapture
running 1 test
test scheduler::core::tests::test_delayed_delivery_is_exactly_once_across_replay_seeds ... ok
test result: ok. 1 passed; 0 failed

CARGO_INCREMENTAL=0 RUSTFLAGS='-C debuginfo=0' cargo test -p temper-runtime callback_ -- --nocapture
running 2 tests
test scheduler::sim_actor_system::tests::callback_cascade_fails_when_reaction_budget_is_exhausted ... ok
test scheduler::sim_actor_system::tests::callback_failure_is_returned_and_invalidates_random_run ... ok
test result: ok. 2 passed; 0 failed

Live server

HOME=/tmp/arn236-temper-home target/debug/temper serve --port 39036 --no-observe
Storage: turso (file:/tmp/arn236-temper-home/.local/share/temper/agents.db)
Local sandbox: http://127.0.0.1:39046 (auto-started)
Temper Data API: http://localhost:39036/tdata
Listening on http://0.0.0.0:39036

curl -sS -i http://127.0.0.1:39036/healthz
HTTP/1.1 200 OK
content-length: 0
date: Tue, 14 Jul 2026 11:44:03 GMT

The server was then stopped cleanly with Ctrl-C.

Full gates

  • CARGO_INCREMENTAL=0 RUSTFLAGS='-C debuginfo=0' cargo test --workspace: PASS outside the filesystem/network sandbox, including the 100-seed randomized platform DST run.
  • Strict cargo clippy with -D warnings for all touched crates/reference apps: PASS.
  • cargo fmt --all -- --check: PASS.
  • git diff --check: PASS.
  • ./scripts/readability-ratchet.sh: PASS.
  • Runtime suite: 105 passed.
  • Mandatory code-quality review: PASS.
  • Mandatory DST review: PASS, Architecture CLEAN, DST-READY.

No files under crates/temper-actor-runtime are modified by this PR.

Copy link
Copy Markdown
Collaborator Author

CI is fully green at head 0dc906b998bb3841e0c569aafe8548bf626872c1.

Run: https://github.com/nerdsane/temper/actions/runs/29329902725

The first Tests attempt was terminated by GitHub runner infrastructure (No space left on device while writing the runner diagnostic log). I reran that failed job on a fresh runner; cargo test --workspace -- --skip dst_ passed, and the workflow completed with conclusion success.

Copy link
Copy Markdown
Collaborator Author

@greptile review

Copy link
Copy Markdown
Collaborator Author

Independent GPT-5.6 review of the complete open PR diff at d5eccfaf17bc0176b05af7df39cb5be96f052c24, including the resolved Greptile P1 callback-lifecycle thread and P2 config-compatibility thread.

Finding:

  • P2 — the compatibility remediation is incomplete for SimActorResult. The current diff removes the newly added fields from SimConfig and SimActorSystemConfig, correctly preserving downstream struct literals, but it still adds the public execution_errors field to the existing public, exhaustive SimActorResult struct in crates/temper-runtime/src/scheduler/sim_actor_system.rs. Any downstream crate constructing SimActorResult { ... } against the current API will fail to compile for the same reason Greptile identified on the config structs. Preserve the existing result shape (for example, keep explicit error evidence behind the already-added SimActorSystem::execution_errors() accessor while all_invariants_held reflects driver failure), and update tests to query the system accessor.

The callback remediation itself is sound: callback rejection and reaction-budget exhaustion clear the remaining cascade, record one terminal execution error, and prevent both step() and run_random() from advancing afterward. The scheduler’s single-owner tick() → cyclic FIFO drain_ready() path, terminal-tick bounded flush, shared reaction counter, and delivery/drop reservation release are deterministic and covered by exact-once/final-tick/fault/replay tests. No nondeterministic collections, wall-clock inputs, OS randomness, or threads were introduced.

Verdict: FAIL

Copy link
Copy Markdown
Collaborator Author

Fresh independent GPT-5.6 review of the complete hosted open PR diff at b7ef993b469e33314288c3c2870d08ae22f59c14.

No blocking findings.

I re-audited the current diff after the prior compatibility failure at #387 (comment) and the fixes in 1e89545dd18caf2e43c3470ca0f43378c65ea9c9 and b7ef993b469e33314288c3c2870d08ae22f59c14.

  • Public API compatibility: SimConfig and SimActorSystemConfig have the same exhaustive public field sets as origin/main. The current head also restores SimActorResult to its original exhaustive field set. The downstream integration regression constructs SimActorResult with the original struct literal and passes. Detailed callback/driver failures remain available through the additive SimActorSystem::execution_errors() accessor, while all_invariants_held becomes false on such failures.
  • Callback failure lifecycle: callback rejection and reaction-budget exhaustion both clear the remaining cascade, append one deterministic SimExecutionError, and terminalize the simulator. step() checks that terminal state before advancing time, messages, or transitions; run_random() returns an unchanged snapshot when called after failure. The regressions cover step → step, step → run_random, and run_random → run_random.
  • Single-owner delayed delivery: tick() moves due messages from the pending heap into bounded actor mailboxes and does not expose a second processing copy. drain_ready() is the consuming transfer, uses persistent cyclic BTreeMap actor order and per-actor VecDeque FIFO, and cannot return a drained message twice. Runtime and verifier both use this path.
  • Budgets and horizon durability: per-actor mailboxes fail fast at a derived bound; action admission counts completed plus in-flight work; delivery and fault-drop both release exactly one reservation. The terminal tick drains all already-due batches without advancing logical time, and runtime callback batches share one reaction counter across that flush.
  • Determinism and coverage: no wall-clock source, OS randomness, threads, or nondeterministic collection was introduced. Exact-once multi-seed replay, bounded cyclic draining, final-tick one- and multi-message delivery, fault/drop reservation release, callback rejection, budget exhaustion, and post-failure non-resumption are covered.
  • Greptile remediation: both Greptile threads are resolved with concrete evidence. The P1 stale-callback finding is fixed by terminal invalidation; the P2 exhaustive-config compatibility finding is fixed without adding #[non_exhaustive]; and the subsequent independent SimActorResult compatibility finding is fixed on the current head.

Independent focused validation at this exact commit passed: public API compatibility 1/1, actor-system regressions 10/10, and verifier simulation regressions 9/9.

No files under crates/temper-actor-runtime are changed.

Verdict: PASS

Copy link
Copy Markdown
Collaborator Author

ARN-236 final-head verification

Head: b7ef993b469e33314288c3c2870d08ae22f59c14

Live server

CARGO_INCREMENTAL=0 RUSTFLAGS='-C debuginfo=0' cargo build -p temper-cli
Finished dev profile

HOME=/tmp/arn236-temper-home target/debug/temper serve --port 39036 --no-observe
Storage: turso (file:/tmp/arn236-temper-home/.local/share/temper/agents.db)
Local sandbox: http://127.0.0.1:39046 (auto-started)
Temper Data API: http://localhost:39036/tdata
Listening on http://0.0.0.0:39036

curl -sS -i http://127.0.0.1:39036/healthz
HTTP/1.1 200 OK
content-length: 0
date: Sat, 18 Jul 2026 19:58:46 GMT

The server was stopped with Ctrl-C after the health check.

Exact current-head regressions

  • verifier delayed message due on final tick: 1/1 PASS
  • verifier final tick drains all due batches: 1/1 PASS
  • runtime final tick drains all due batches: 1/1 PASS
  • runtime multi-seed delayed-delivery exact-once replay: 1/1 PASS
  • callback rejection/budget exhaustion terminalization: 2/2 PASS
  • external exhaustive SimActorResult struct-literal compatibility: 1/1 PASS
  • full runtime suite: 105/105 PASS plus compatibility integration test
  • full verifier suite: 115/115 PASS
  • strict all-target/all-feature clippy for touched crates and reference apps: PASS
  • fmt, diff check, readability ratchet: PASS
  • mandatory code-quality and DST reviews: PASS

Review remediation

No files under crates/temper-actor-runtime are modified.

rita-aga commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator Author

ARENA SHIPPABLE · GPT-5.6 · 2026-07-18T20:12:02Z

Current head: b7ef993b469e33314288c3c2870d08ae22f59c14

Ordered review gate evidence

Validation evidence

PR remains open and unmerged.

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.

1 participant