Skip to content

refactor(core): simplify session runner loop and pending input scopes - #38602

Merged
kitlangton merged 2 commits into
v2from
runner-cleanup
Jul 24, 2026
Merged

kitlangton merged 2 commits into
v2from
runner-cleanup

Conversation

@kitlangton

Copy link
Copy Markdown
Contributor

What

Readability refactor of the V2 session runner and the SessionPending surface. The runner hierarchy now reads top-down as drain → runSteps → runStep → callModel, and the pending-input module exposes two deep combinators instead of five overlapping ones.

flowchart TD
  W[wake / resume] --> D[drain]
  D -->|settle stale tools, run pending compaction| S[runSteps]
  S -->|one logical step at a time| R[runStep]
  R -->|"retry / compaction-restart loop"| C[callModel]
  C -->|"Completed | Retry | Restart"| R
Loading

How

SessionPending (packages/core/src/session/pending.ts)

  • has(db, sessionID, scope) with scope "any" | "input" | Delivery replaces hasWork, hasEligible, and the delivery-only has. "any" counts every row including compaction; delivery scopes are blocked behind a pending compaction barrier.
  • promote(db, events, sessionID, "input" | "steer") replaces promoteEligible({ boundary }) and promoteSteers; promoteNextQueued deleted (zero callers). PromotionBoundary becomes Promotable, retiring the boundary: "step" / step counter name collision.

Runner (packages/core/src/session/runner/llm.ts)

  • drain reads as its phases: dismiss spurious wake → settleStaleToolCalls → runPendingCompaction → run steps until no promotable input. Stale tool settlement now happens before compaction reads history.
  • runStep retry restructured: Schedule.toStepWithSleep + a waitForRetry loop arm replace Effect.suspend + tapError closure mutation + retryOrElse. Outcomes are a Data.taggedEnum (Completed | Retry | Restart), collapsing the two ad hoc restart tags.
  • callModel: recoverOverflow is a plain boolean instead of a function-typed one-shot flag; step-end helpers regrouped so the stream definition sits directly against the settlement mask that runs it; phase comments added ("make room", settlement contract).
  • Title generation extracted to startTitleOnce.

Retry (packages/core/src/session/runner/retry.ts)

  • Schedule input narrowed to RetryableFailure, dropping the setInputType union, passthrough, Schedule.while, and both instanceof hedges that existed only because retryOrElse funneled every error through it.

Semantics

Two deliberate behavior changes, both small:

  1. Retry budget scope: the 4-retry allowance now spans one logical step including compaction restarts. Previously each restart rebuilt a fresh schedule and reset the budget.
  2. Drain entry: a spurious wake with no pending work now returns before settling stale tool calls; previously stale-tool repair ran whenever eligible input existed but not on compaction-only wakes. Stale tools are now always settled before any compaction or model call in the same drain.

Also removed: the Effect.yieldNow between restart attempts (no observable dependency; suites pass repeatedly) and a redundant post-loop runPendingCompaction.

Scope

Not covered, planned as follow-ups:

  • Extracting pure settlement judgments (classifyToolExits etc.) from callModel's uninterruptible block
  • Top-down declaration reordering of llm.ts (drain first)
  • The loaded.model.model resolved-model naming in model.ts/context.ts

Testing

  • bun typecheck (packages/core) — clean
  • bun run test test/session-runner.test.ts test/session-runner-recorded.test.ts test/session-prompt.test.ts test/session-create.test.ts test/tool-subagent.test.ts — 206 pass, 0 fail
  • Retry tests with --rerun-each 3 — 30 pass; previously timing-sensitive steering test hardened to await real session idleness and passed 5 consecutive runs
  • Full packages/core suite: 1415 pass / 5 fail — the same 5 failures (Mistral provider ×4, LocationServiceMap ×1) reproduce on the clean v2 base and are unrelated

@kitlangton
kitlangton enabled auto-merge (squash) July 24, 2026 03:17
@3ugen

3ugen commented Jul 24, 2026

Copy link
Copy Markdown

While exercising the new retry-budget scope, I found a pre-existing history anchor issue across retry + overflow compaction.

A deterministic sequence is:

  1. a retryable pre-output failure creates assistant A and schedules attempt 2;
  2. attempt 2 reports context overflow, so compaction completes and the step restarts;
  3. a later attempt succeeds and durably ends A;
  4. the next prompt's model request contains the compaction checkpoint and the new user input, but not A's successful text.

StepStarted inserts A before the compaction row. The restart reuses A's assistantMessageID, and later step/text events update that row without moving its message sequence. SessionHistory then filters at the newer compaction sequence, so the successful response remains in the full message/event log but falls out of active model context.

I reproduced this at 8547e6f2 and on base 7456598c, so it is not introduced by this refactor. The PR's shared retry schedule does work: the combined trace keeps attempt 2 at 2s and attempt 3 at 6s, and a schedule-reset mutation makes the test fail. Would a compaction restart need a post-anchor assistant identity or an explicit re-anchoring transition?

@kitlangton
kitlangton merged commit 0f3c301 into v2 Jul 24, 2026
9 of 11 checks passed
@kitlangton
kitlangton deleted the runner-cleanup branch July 24, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants