Skip to content

feat(core): AgentSession entry point (1.V) + pre-egress budget governor (1.AC) - #26

Merged
cemililik merged 14 commits into
mainfrom
development
Jun 16, 2026
Merged

feat(core): AgentSession entry point (1.V) + pre-egress budget governor (1.AC)#26
cemililik merged 14 commits into
mainfrom
development

Conversation

@cemililik

@cemililikcemililik commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Bundles two Phase-1 engine workstreams (both on the shared run-loop substrate) plus the review/hardening that followed. Supersedes the closed PR #25 (1.AC alone): this PR carries 1.AC's commits unchanged, plus 1.V and the cross-cutting fixes — development is a strict superset, no commit lost.

1.V — AgentSession entry point (ADR-0024)

The agent-first multi-turn entry point: start / sendMessage / cancel over the same turn core (runAgentTurn) a workflow agent node uses. In-memory transcript, session-wide cost, and a hard turn cap that fails loud with turn_limit (regression-pinned). Emits via an injected SessionEventSink (the RunEventBus wiring + SessionHandle is 1.W). No @relavium/shared changes — the session contracts landed at 1.L.0.

1.AC — pre-egress budget governor (ADR-0028)

BudgetGovernor + estimator: a pre-egress cost check before every LLM call, a run timeout_ms, and on_exceed = warn / fail / pause_for_approval (the pause reuses the human-gate seam). Emits budget:warning / budget:paused / run:timeout.

Review + hardening

A comprehensive multi-dimensional review (+ adversarial verification) of the consolidated branch found and fixed 4 HIGH budget-governor defects + the 1.V×1.AC session seam:

  • H1 — checkpoint: isBudgetGate survives the budget:paused + human_gate:paused pair on cross-process resume (a rejected budget gate now fails the run).
  • H2 — the cumulative cost is restored on resume from the durable budget:paused.spentMicrocents, and the governor is re-seeded.
  • H3 — approving a budget pause continues the deferred LLM call (one-shot per-re-dispatch bypass), not a {decision} short-circuit, and never infinite-loops.
  • H4 — an unpriced/custom-base-URL model degrades to allow (no uncaught UnknownModelError).
  • M1/M3 — a budgeted session feeds the governor (updateCost) and settles a pre-egress pause loudly as budget_exceeded.

Plus the PR-#25 bot/Sonar findings: max_cost_microcents: 0 = unbounded (also removes a /0 NaN), dispatcher builds handlers once, cognitive-complexity extractions (executeAgent, runAgentTurn), #budgetGovernor readonly, test-helper hoists + non-/tmp fixture path.

Validation

pnpm turbo run format:check lint typecheck test build — all green (674 core + 245 shared + 300 llm + 14 db tests). Leakwatch 0.

Deferred (tracked in docs/roadmap/deferred-tasks.md)

Full session pause/resume + per-session tool narrowing (1.V×1.AC); general cost-event persistence on resume (a human-gate/crash resume still loses cost — budget:paused is the only durable cost record today); the configurable budget warning threshold + estimate-accuracy watch items.

Refs: ADR-0024, ADR-0028, ADR-0011, ADR-0036

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • New Features
    • AgentSession: Multi-turn conversation sessions with a persistent transcript and configurable hard turn limits.
    • Budget governance: Pre-egress cost caps with warning events, approval-gate pauses/resumes, and budget-exceeded termination.
    • Run-level timeout protection: Automatic timeout failures for long-running workflows.
  • Changes
    • SSE event contracts expanded for budget and timeout events, including new payload details for reliable client handling.

cemililikand others added 12 commits June 15, 2026 16:09
PR #24 (ADR-0040 + 1.S Part A) is merged, so record it as Done and advance the
active pointer:
- CLAUDE.md: status line now notes node retry (1.S) landed (PR #24, ADR-0040 amending
ADR-0038), with retry-from-node (Part B) deferred to Phase-2; active work is now the
last 1.m4 workstream, the pre-egress budget governor (1.AC).
- current.md: active-now + follow-ups note updated the same way (1.S ✅ Done, 1.AC next).
- phase-1-engine-and-llm.md: 1.S section header → ✅ Done (PR #24); tasks marked
landed/deferred (retry-from-node Part B + optional input adjustment deferred — ADR-0040
Part B / A.9); acceptance marked met. Dependency table: 1.S → Done (PR #24), and the
stale already-merged rows brought into line (1.P → PR #20, 1.R/1.Q → PR #22).
ADR-0040 (decisions/README.md) and the deferred-tasks.md Part-B entry already landed in
PR #24 — no change. Docs-only; no code touched.
Refs: ADR-0040
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The agent-first entry point (ADR-0024): a multi-turn conversation bound to one agent +
its fallback chain, driving the SAME correlation-agnostic turn core (`runAgentTurn`) a
workflow `agent` node uses — so streaming, the tool-call loop, and provider fallback are
identical to a node; only the entry point and lifetime differ.
- `AgentSession` (start / sendMessage / cancel): an in-memory `LlmMessage[]` accumulator,
a session-wide cumulative cost total (overwrites the turn-core placeholder), and a
classified-failure → `session:turn_completed{error}` mapping.
- Hard turn cap (the 1.V deliverable): a finite session-level round cap (default 50,
distinct from `[chat].max_messages` trim and from the turn core's within-turn
`maxToolTurns`). A `sendMessage` past the cap fails LOUDLY with no egress —
`session:turn_completed{ stopReason:'error', error.code:'turn_limit' }`, never a silent
stop — pinned by a mandatory regression test so a turn-loop refactor cannot drop it.
- Emits through an injected `SessionEventSink` (no bus coupling): wiring it onto the shared
`RunEventBus` + a `SessionHandle` is 1.W; DB persistence + the durable `SessionMessage`
schema is 1.X; resume 1.Y; export 1.Z. Zero `@relavium/shared` changes — the `session:*`
union, `SessionContext`, and `turn_limit` already landed at 1.L.0.
- `preEgress` threaded through `SessionDeps` as a pass-through so the 1.AC budget governor
binds to a session with no second wiring path.
Decided no new ADR (agent-session-spec.md + ADR-0024/0026/0036 fully specify 1.V; both
adversarial reviewers concurred). Spec clarified: the shared substrate is the turn core
(the "thin wrapper over AgentRunner" prose predates the 1.O split), the hard-cap knob, and
the in-memory/1.X persistence boundary. Phase doc records the deferrals (transcript
content-only, per-session tool narrowing, session output_schema; 1.W also owns the bus
session-event gate + SessionHandle; 1.X authors SessionMessageSchema).
Engine purity holds (no platform imports; abort via the injected host controller). No
vendor type crosses the @relavium/llm seam. 9/9 unit tests green; typecheck/eslint clean
on the 1.V files; Leakwatch 0.
Refs: ADR-0024, ADR-0026, ADR-0036
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BudgetPausedEvent now carries nodeId (the paused vertex) and gateId
(the human-gate decision token) so surfaces can resume a paused run
and correlate the gate back to its source node.
Refs: ADR-0028
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
estimateMaxNextCost gives a worst-case micro-cent estimate before a
provider call. The FallbackChain now accepts a preAttempt hook that
receives model + maxTokens so the core can enforce a pre-egress
budget check before each real provider attempt.
Refs: ADR-0028
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds BudgetGovernor that evaluates worst-case cost before each LLM
call and emits budget:warning / budget:paused / human_gate:paused.
Implements pause_for_approval by parking a gate and resuming through
the existing engine.resume path; a rejected budget gate fails the run
with budget_exceeded. Also wires workflow timeout_ms to a run-level
timer that emits run:timeout and fails with run_timeout.
Refs: ADR-0028
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…events
Update the SSE event schema to describe budget:warning threshold
semantics, the BudgetPausedEvent shape with nodeId/gateId, and the
run:timeout -> run:failed{run_timeout} sequence. Adds a note about
deferring a configurable sub-100% warning threshold.
Refs: ADR-0028
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ck, hardening)
Verified each reviewer finding against the code; fixed the valid ones, kept the
established engine patterns where a finding was based on a misread.
- Cross-turn transcript (HIGH, refined): the reviewers' tool_use-400 mechanism does not
apply — runAgentTurn returns only the final non-tool_use content, so the appended
assistant message never carries an orphaned tool_use. But the instinct surfaced a real
subtler bug: result.content carries reasoning parts whose `signature` is a within-turn,
same-provider replay token (ADR-0030/0039) and must NOT span turns (a failed-over turn
would carry a fallback-provider signature into the next turn's primary request). Fix:
append the assistant reply TEXT-ONLY across turns. New capturing-provider test pins that
turn 2's outbound messages carry no tool_call/tool_result/reasoning part. Faithful
cross-turn tool/reasoning history stays the 1.X/1.Z deferral (needs the turn core to
expose intermediate messages, owned by concurrent 1.AC).
- User-message rollback (MEDIUM): a turn that does not complete now rolls the pushed user
message back, so the transcript holds only completed exchanges — no dangling user turn,
no two-consecutive-user-messages. This also prevents a future non-AgentTurnError the
session does not yet handle (a 1.AC pre-egress BudgetPauseError, thrown before egress)
from orphaning the user message. General fix; no dependency on 1.AC's type.
- buildLlmTools (LOW): ToolDefSchema.parse → safeParse, throwing a classified
AgentTurnError('internal') so a malformed granted-tool schema surfaces as a session
error turn instead of an uncaught ZodError.
- Cost accumulation (reviewed MAJOR — kept, not a bug): per-event accumulation of the
per-attempt cost increments mirrors the WorkflowEngine exactly (engine.ts #nodeEmit), is
robust to multiple cost events per turn, and the proposed "read cost from
AgentTurnResult.usage" is infeasible (usage is tokens, not microcents). Added a comment
+ strengthened the test to a tool-round-trip turn (two cost events) to pin no double-count.
- Tests: turn-cap regression now pins the EXACT sequence
[session:turn_started, session:turn_completed] for a blocked turn; added the
resolveProvider→undefined (internal-error) branch; annotated the intra-package
createAbortController import.
11/11 unit tests green; agent-session typecheck/eslint clean; Leakwatch 0. Only my 3 files
staged (concurrent 1.AC WIP left untouched).
Refs: ADR-0024, ADR-0030, ADR-0039
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pre-egress budget-governor commits (fa4f849..851ed30, workstream 1.AC) landed without
`prettier --write`, leaving budget-governor.ts/.test.ts, engine.ts, and run-plan.ts failing
the repo `format:check` gate on `development` after the 1.V/1.AC consolidation. Formatting
only — no logic change; the full suite (669 core + 245 shared + 300 llm + 14 db) stays green
and typecheck/lint are clean. Keeps the consolidated `development` gate-clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
While building 1.V the deferrals were noted in the phase doc's 1.V scope notes; now that the
1.AC work is committed and consolidated onto development (deferred-tasks.md is no longer a
concurrently-edited file), give them their canonical actionable home. Adds an "AgentSession
(1.V) follow-ups" section: faithful cross-turn transcript (tool/reasoning history → 1.X/1.Z),
session budget pause/resume (1.V × 1.AC), per-session tool narrowing (ADR-0029),
[chat].max_turns surface wiring, and session output_schema. The bus wiring/SessionHandle (1.W),
persistence + SessionMessage schema (1.X), resume (1.Y), export (1.Z) are scheduled workstreams
(tracked in the phase doc), not deferred items. Docs-only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…session seam (development review)
The comprehensive multi-dimensional review of the consolidated development branch confirmed
4 distinct HIGH defects (all in the 1.AC pre-egress budget governor) + the latent 1.V×1.AC
session-budget seam. Each fix lands with a regression test that fails before it.
HIGH (ADR-0028 budget governor):
- H1 — checkpoint.ts: a budget gate emits budget:paused THEN human_gate:paused with the same
gateId; applyGateEvent's Map.set let the second downgrade isBudgetGate→false on cross-process
reconstruction, so a resumed REJECTED budget gate would not fail the run. OR the flag so it
stays true. Test: fold [budget:paused, human_gate:paused] keeps isBudgetGate true.
- H2 — engine.ts: #seedFromCheckpoint restored the engine cumulative but never re-seeded the
governor, so a resumed budgeted run started at 0 and under-blocked by ~a full cap. Re-seed via
governor.updateCost(cp.cumulativeCostMicrocents). Test: a run that spends ~the cap, parks at a
gate, crashes, and resumes now blocks the downstream node's pre-egress.
- H3 — engine.ts: approving a budget pause completed the agent node with {decision:'approved'} —
the deferred LLM call never ran. Per the maintainer decision (continue the call), an approved
budget gate now arms a one-shot per-vertex pre-egress bypass and re-dispatches the node (reset
to pending → #claimReady), so the call actually issues; a later over-budget call pauses again.
Test (e2e): approve re-dispatches and node:completed carries the model output, not the decision.
- H4 — budget-governor.ts: evaluatePreEgress let an unpriced/custom-base-URL model's
UnknownModelError escape uncaught, hard-failing every budgeted call on it. Catch it and degrade
to allow (mirrors the FallbackChain's unpriced⇒no-cost policy). Test: unlisted model resolves.
MEDIUM (session seam — M1/M3, the 1.V×1.AC composition gap the completeness critic flagged):
- M3 — agent-session.ts: SessionDeps gains an updateCost seam, called per cost:updated, so a
host wiring preEgress to a governor also keeps the governor's running total current (else a
tool-looping chat never fails safe). Test: updateCost called with the cumulative.
- M1 — agent-session.ts: a pre-egress BudgetPauseError no longer escapes sendMessage raw; the
session has no pause/resume in 1.V, so it settles the turn loudly as budget_exceeded. Test.
674 core tests green (+5), full turbo gate clean (format/lint/typecheck/test/build), Leakwatch 0.
Full session pause/resume + per-session tool narrowing remain tracked in deferred-tasks.md.
Refs: ADR-0028, ADR-0024
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…mment + the deeper cost-persistence gap
An adversarial verification of e7c30c5 (8 Opus skeptics) confirmed H1/H4/M1/M3 sound and
surfaced two real issues; investigating H2 then exposed a deeper, previously-mischaracterised
gap. Acted on all three.
- H2 (was: "re-seed the governor") — the re-seed at #seedFromCheckpoint is correct, but its
source `cp.cumulativeCostMicrocents` is ALWAYS ~0 on a real resume: `cost:updated` is
streamed (`#nodeEmit` → bus), never persisted, so the checkpoint fold's cost handling is
inert. The old engine test (`maxTokens:10_000`, est. 15M ≫ 1M cap) was a tautology that
masked this — it passed even with the re-seed reverted. Fix: restore the cumulative from the
DURABLE `budget:paused.spentMicrocents` in the fold, so a run paused at a budget gate resumes
with the correct spend (+ the governor re-seed then blocks correctly). Pinned by a checkpoint
unit test (fold restores cumulative=900k); the tautological engine test is removed. The
general fix (make cost durable for a human-gate/crash resume) is filed in deferred-tasks.
- H3 (per-re-dispatch bypass) — verification confirmed it is CORRECT, not the regression first
flagged: the suggested "per-LLM-call" bypass would, given that resume re-runs the turn from
scratch, re-pause and loop forever (the exact "pauses forever" failure the chosen "re-run,
one-shot bypass" option avoids). Kept the per-re-dispatch design; fixed its genuine
sub-issues: clear `#budgetApprovedVertices` in `#settle` (no stale entry if a sibling
failure/cancel races the re-dispatch), corrected the inaccurate "later call pauses again"
comments to the true per-re-run semantics + the mid-tool-loop re-run caveat, and softened the
gate message (it authorises the step's completion past the cap, not literally one call).
673 core tests green; full turbo gate clean (format/lint/typecheck/test/build); Leakwatch 0.
Refs: ADR-0028
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ernor
PR #25 (the other agent's 1.AC branch) and SonarCloud flagged the budget governor + turn core.
Triaged against current `development` (which already carries the e7c30c5/53f96af review fixes
the PR branch lacks):
Already fixed on development (verified, no change):
- "consecutive user messages on a failed/cancelled turn" → the failed-turn user-message rollback
already landed (agent-session.ts catch); a cancelled session is terminal, so no next sendMessage.
- "approving a budget gate loops forever" → the per-re-dispatch one-shot bypass (H3) already lands
the call without re-pausing the approved step.
Fixed here:
- budget-governor.ts: treat `max_cost_microcents: 0` as UNBOUNDED (the [chat] "0 = unbounded"
semantics) — guard at the top of evaluatePreEgress, which also removes the `cumulative / 0` NaN
in thresholdPct. + test.
- dispatcher.ts + agent-runner.ts: the dispatcher rebuilt all 8 handlers on every execute() to
bridge `ctx.preEgress`; the agent runner now reads `ctx.preEgress ?? deps.preEgress`, so the
handler map is built ONCE (and the H3 ctx.preEgress=undefined bypass still works).
- agent-runner.ts: extract the turn-error→outcome mapping (executeAgent cognitive complexity 17→<15).
- agent-turn.ts: extract awaitPreEgress + dispatchToolUseTurn from runAgentTurn (cognitive complexity
24→~13); awaitPreEgress also drops the dead BudgetPauseError re-throw branch.
- engine.ts: mark `#budgetGovernor` readonly (assigned once in the constructor).
- tests: hoist budgetWorkflow/cheapProvider to module scope (Sonar); replace the `/tmp/session`
fixture path with `/workspace/session` (Sonar security hotspot — a fixture string, never touched).
Left (with reason): the agent-session.ts `DistributiveOmit` "never in union" (Minor) is the
idiomatic distributive-conditional `T extends unknown ? Omit<T,K> : never`; the same pattern is in
node-executor.ts. Not worth an awkward rewrite.
674 core tests green; full turbo gate clean (format/lint/typecheck/test/build); Leakwatch 0.
Refs: ADR-0028, ADR-0011
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@sourcery-aisourcery-aiBot 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.

Sorry @cemililik, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitaiBot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3dad1c3a-0af9-4c0f-9fdc-fc0fb44188c6

📥 Commits

Reviewing files that changed from the base of the PR and between 7dec842 and 3682314.

📒 Files selected for processing (13)
  • docs/reference/contracts/agent-session-spec.md
  • docs/reference/contracts/sse-event-schema.md
  • packages/core/src/engine/agent-runner.e2e.test.ts
  • packages/core/src/engine/agent-runner.ts
  • packages/core/src/engine/agent-session.test.ts
  • packages/core/src/engine/agent-session.ts
  • packages/core/src/engine/agent-turn.test.ts
  • packages/core/src/engine/agent-turn.ts
  • packages/core/src/engine/budget-governor.test.ts
  • packages/core/src/engine/engine.ts
  • packages/core/src/index.ts
  • packages/shared/src/run-event.test.ts
  • packages/shared/src/run-event.ts
🚧 Files skipped from review as they are similar to previous changes (7)
  • packages/core/src/engine/agent-runner.ts
  • docs/reference/contracts/sse-event-schema.md
  • packages/core/src/engine/agent-turn.ts
  • packages/core/src/engine/agent-runner.e2e.test.ts
  • packages/core/src/engine/budget-governor.test.ts
  • packages/core/src/engine/agent-session.ts
  • packages/core/src/engine/engine.ts

📝 Walkthrough

Walkthrough

This PR introduces a pre-egress BudgetGovernor that blocks LLM calls projected to exceed a configured micro-cent cost cap (warn/fail/pause modes), wired through FallbackChain, agent-turn, agent-runner, and WorkflowEngine with run-level timeout support and checkpoint reconstruction for budget gates. It also adds AgentSession—a new multi-turn in-memory session driver over the shared runAgentTurn core—with lifecycle events, cross-turn transcript, turn cap, cost accumulation, and cancellation. Event schemas, roadmap, and contract docs are updated accordingly.

Changes

Pre-egress Budget Governor and Run-level Timeout

Layer / File(s)Summary
LLM budget estimator and PreAttemptHook
packages/llm/src/budget-estimator.ts, packages/llm/src/budget-estimator.test.ts, packages/llm/src/fallback-chain.ts, packages/llm/src/index.ts
Adds estimateMaxNextCost (worst-case output cost in micro-cents) and a PreAttemptHook type wired as preAttempt on FallbackChainOptions, awaited before each non-skipped provider attempt.
BudgetGovernor, error types, and budget event schemas
packages/core/src/engine/budget-governor.ts, packages/core/src/engine/budget-governor.test.ts, packages/shared/src/run-event.ts, packages/shared/src/run-event.test.ts
Introduces BudgetExceededError, BudgetPauseError (with toGateRequest()), BudgetCheckResult, and the stateful BudgetGovernor; extends BudgetWarningEventSchema to require integer thresholdPct and BudgetPausedEventSchema to require nodeId and gateId.
PreEgressHook wiring through agent-turn and node execution context
packages/core/src/engine/agent-turn.ts, packages/core/src/engine/node-executor.ts
Extends PreEgressHook with maxTokens, adds awaitPreEgress/dispatchToolUseTurn helpers, maps budget errors in streamOneTurn, passes preEgress into FallbackChain as preAttempt, and adds budget-gate fields to GateRequest and NodeExecContext.
agent-runner budget-pause outcome mapping and tool schema validation
packages/core/src/engine/agent-runner.ts
Adds turnOutcomeForError mapping AgentTurnError→failed, BudgetPauseError→paused; wires ctx.preEgress ?? deps.preEgress precedence; upgrades tool LLM-visible schema validation to safeParse with internal error handling.
RunPlan budget/timeout and checkpoint budget-gate reconstruction
packages/core/src/run-plan.ts, packages/core/src/dag.ts, packages/core/src/engine/checkpoint.ts, packages/core/src/engine/checkpoint.test.ts
Extends RunPlan with budget and timeoutMs; propagates both from buildRunPlan. Adds isBudgetGate to CheckpointPendingGate, handles budget:paused in applyGateEvent to restore cumulativeCostMicrocents.
WorkflowEngine budget governance and run-level timeout
packages/core/src/engine/engine.ts, packages/core/src/engine/engine.test.ts, packages/core/src/engine/execution-host.test.ts, packages/core/src/engine/node-handlers/dispatcher.ts
Wires BudgetGovernor into RunExecution; arms/disarms run-level timeout on begin/resume, emitting run:timeout; handles budget gate approve (one-shot #budgetApprovedVertices bypass) and reject (budget_exceeded failure); re-seeds governor from checkpoint.
agent-runner e2e test refactor and resource governance tests
packages/core/src/engine/agent-runner.e2e.test.ts
Refactors existing tests to use shared agentExecutor helper; adds "resource governance" block covering budget warn, fail, pause/resume (approved + rejected), and run timeout.

AgentSession Multi-Turn Driver

Layer / File(s)Summary
AgentSession types, SessionDeps, and public exports
packages/core/src/engine/agent-session.ts, packages/core/src/index.ts
Defines SessionLifecycleEvent, SessionStreamEvent, SessionEventSink, SessionDeps, AgentSessionParams, SessionStateError, and DEFAULT_SESSION_MAX_TURNS; re-exports from package entry.
AgentSession core implementation
packages/core/src/engine/agent-session.ts
Implements start, sendMessage (state enforcement, hard turn cap, transcript management, error mapping), cancel, #runTurn, #onTurnEmit (cumulative cost), #resolvePlan (memoized provider plan), and buildLlmTools.
AgentSession test suite
packages/core/src/engine/agent-session.test.ts
Covers multi-turn lifecycle events, transcript integrity, cost accumulation, updateCost callback, budget pause, turn-limit cap, tool-loop limit, provider exhaustion, no-provider wiring, cancellation (between and during turns), and lifecycle guards.

Documentation Updates

Layer / File(s)Summary
Roadmap status and deferred tasks
CLAUDE.md, docs/roadmap/current.md, docs/roadmap/deferred-tasks.md, docs/roadmap/phases/phase-1-engine-and-llm.md
Marks node retry (1.S) complete (ADR-0040 Part A), positions 1.AC (pre-egress budget governor) as next, defers retry-from-node to Phase 2; adds deferred entries for cost-restore gap, warn_at_pct, and AgentSession follow-ups.
Agent-session spec and SSE event schema contracts
docs/reference/contracts/agent-session-spec.md, docs/reference/contracts/sse-event-schema.md
Adds Hard Turn Cap spec (default 50, turn_limit code, distinction from max_messages/maxToolTurns); adds BudgetWarningEvent, BudgetPausedEvent, RunTimeoutEvent interface shapes and revises governance table rows.

Sequence Diagrams

sequenceDiagram
participant Client
participant WorkflowEngine
participant BudgetGovernor
participant FallbackChain
participant LLMProvider
Client->>WorkflowEngine: start(runId, plan{budget, timeoutMs})
WorkflowEngine->>WorkflowEngine: armRunTimeout(timeoutMs)
WorkflowEngine->>WorkflowEngine: `#runAttempt` → makePreEgressHook()
WorkflowEngine->>FallbackChain: stream(request, {preAttempt: preEgressHook})
FallbackChain->>BudgetGovernor: preAttempt({ model, maxTokens })
BudgetGovernor->>BudgetGovernor: estimateMaxNextCost → evaluatePreEgress()
alt within cap
BudgetGovernor-->>FallbackChain: allow
FallbackChain->>LLMProvider: stream()
LLMProvider-->>WorkflowEngine: cost:updated → BudgetGovernor.updateCost()
else projected cost exceeds cap (pause_for_approval)
BudgetGovernor-->>FallbackChain: throw BudgetPauseError
FallbackChain-->>WorkflowEngine: paused outcome → settleGate()
WorkflowEngine->>WorkflowEngine: emit budget:paused{nodeId, gateId}
Client->>WorkflowEngine: resume(runId, gateId, approved)
WorkflowEngine->>WorkflowEngine: reset vertex, add to `#budgetApprovedVertices`
WorkflowEngine->>FallbackChain: re-dispatch (one-shot bypass)
end
Loading
sequenceDiagram
participant App
participant AgentSession
participant runAgentTurn
participant LLMProvider
App->>AgentSession: start()
AgentSession-->>App: emit session:started
App->>AgentSession: sendMessage("hello")
AgentSession-->>App: emit session:turn_started
AgentSession->>runAgentTurn: `#runTurn`(signal)
runAgentTurn->>LLMProvider: stream(transcript + user msg)
LLMProvider-->>AgentSession: token / cost:updated events
AgentSession->>AgentSession: `#onTurnEmit` → accumulate cumulativeCostMicrocents
runAgentTurn-->>AgentSession: StopReason
AgentSession-->>App: emit session:turn_completed{stopReason, usage}
App->>AgentSession: cancel()
AgentSession->>AgentSession: abort in-flight turn
AgentSession-->>App: emit session:cancelled
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

  • HodeTech/Relavium#18: Introduced the AgentRunner and agent-turn seam that this PR directly extends with the pre-egress budget hook, BudgetPauseError mapping, and ctx.preEgress propagation in agent-runner.ts and agent-turn.ts.
  • HodeTech/Relavium#17: Introduced the WorkflowEngine run-loop substrate that this PR extends with BudgetGovernor wiring, #budgetApprovedVertices, run-level timeout arming, and budget-gate resume handling in engine.ts.
  • HodeTech/Relavium#13: The main PR's pre-egress budgeting integration into packages/llm/src/fallback-chain.ts (via the preAttempt hook into the per-provider attempt flow) directly builds on this PR's FallbackChain runner changes.

Poem

🐇 Hoppity-hop through the budget gate,
A governor checks before tokens stream late,
"BudgetPauseError!" the rabbit cries,
AgentSession turns under moonlit skies—
Fifty turns max, then a loud turn_limit,
No silent failures—this PR's got spirit! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring coverage is 61.54% which is insufficient. The required threshold is 80.00%.Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe PR title 'feat(core): AgentSession entry point (1.V) + pre-egress budget governor (1.AC)' clearly summarizes the main changes: two Phase-1 workstreams (AgentSession and budget governor) added to the core engine with specific identifiers.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch development

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assistgemini-code-assistBot 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.

Code Review

This pull request implements the pre-egress budget governor (1.AC) and the multi-turn AgentSession entry point (1.V), along with run-level timeout support. It introduces the stateful BudgetGovernor to enforce cost caps, gates provider attempts before egress, and manages budget-paused states that can be resumed or rejected. The AgentSession driver manages multi-turn conversation state, hard turn caps, and cancellation. Feedback on the changes identifies a high-severity issue in AgentSession where cancelling a turn mid-flight (or when an error is thrown during cancellation) fails to roll back the user message, leaving a dangling user turn in the transcript.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +253 to +255
const result = await this.#runTurn(abort.signal);
// A cancel landed mid-turn — the cancel path owns the terminal session:cancelled; stay quiet.
if (this.#statusIs('cancelled')) return;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

If the turn is cancelled mid-flight, the user message pushed to this.#messages remains in the transcript because the early return bypasses the rollback (this.#messages.pop()). This leaves a dangling user turn at the end of the transcript, violating the design goal of keeping only completed exchanges in the transcript (which affects session exports and persistence).

Suggested change
constresult=awaitthis.#runTurn(abort.signal);
// A cancel landed mid-turn — the cancel path owns the terminal session:cancelled; stay quiet.
if(this.#statusIs('cancelled'))return;
constresult=awaitthis.#runTurn(abort.signal);
// A cancel landed mid-turn — the cancel path owns the terminal session:cancelled; stay quiet.
if(this.#statusIs('cancelled')){
this.#messages.pop();
return;
}

model: result.model,
});
} catch (err) {
if (this.#statusIs('cancelled')) return; // cancel-during-turn: session:cancelled is the terminal

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Similarly, if an error is thrown but the session has been cancelled, we should pop the user message to prevent a dangling user turn from remaining in the transcript.

Suggested change
if(this.#statusIs('cancelled'))return;// cancel-during-turn: session:cancelled is the terminal
if(this.#statusIs('cancelled')){
this.#messages.pop();
return;
}

@coderabbitaicoderabbitaiBot 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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/core/src/engine/agent-turn.ts (1)

544-551: ⚠️ Potential issue | 🔴 Critical

Remove the extra pre-egress check before chain.stream to avoid budget checks against skipped models.

With preAttempt already wired in Lines 548-551, the additional call on Line 568 pre-checks activeModelbefore the FallbackChain's skip decisions. On tool iterations after the first, if that model is later skipped due to rate-limit cooldown or capability mismatch, the budget hook has already run against a provider that will not be invoked, and the actual fallback provider receives no budget check.

Suggested patch
-/**- * Await the pre-egress budget hook before a provider call, mapping a budget-cap failure to the closed turn- * error taxonomy: a {`@link` BudgetExceededError} (`on_exceed: fail`) → `AgentTurnError('budget_exceeded')`;- * a {`@link` BudgetPauseError} (`pause_for_approval`) and any other error propagate as-is (the run path maps- * the pause to a `paused` node outcome). Extracted from the turn loop to keep its complexity in budget.- */-async function awaitPreEgress(params: AgentTurnParams, activeModel: string): Promise<void> {- try {- await params.preEgress?.({- model: activeModel,- ...(params.maxTokens === undefined ? {} : { maxTokens: params.maxTokens }),- });- } catch (err) {- if (err instanceof BudgetExceededError) {- throw new AgentTurnError('budget_exceeded', err.message, false);- }- throw err;- }-}
@@
- await awaitPreEgress(params, activeModel);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/engine/agent-turn.ts` around lines 544 - 551, Remove the
duplicate pre-egress budget check that occurs before the chain.stream call.
Since preAttempt is already properly configured in the FallbackChain constructor
(via the params.preEgress conditional assignment in the chain initialization),
the additional pre-egress check before chain.stream is redundant and causes the
budget hook to run against the activeModel before FallbackChain has a chance to
skip it due to rate-limit cooldown or capability mismatch. This leaves the
fallback provider without a budget check. Delete the extra pre-egress invocation
that pre-checks activeModel prior to calling chain.stream.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/engine/budget-governor.test.ts`:
- Around line 57-60: The code on line 59 uses an unsafe TypeScript cast (as
BudgetPauseError) when accessing the toGateRequest() method on the err variable.
Instead of using the as assertion, add a proper type guard check to narrow the
type before calling toGateRequest(). This can be done by checking if err is an
instance of BudgetPauseError using the instanceof operator before attempting to
call the method, which will satisfy TypeScript's strict type checking
requirements without unsafe assertions.
---
Outside diff comments:
In `@packages/core/src/engine/agent-turn.ts`:
- Around line 544-551: Remove the duplicate pre-egress budget check that occurs
before the chain.stream call. Since preAttempt is already properly configured in
the FallbackChain constructor (via the params.preEgress conditional assignment
in the chain initialization), the additional pre-egress check before
chain.stream is redundant and causes the budget hook to run against the
activeModel before FallbackChain has a chance to skip it due to rate-limit
cooldown or capability mismatch. This leaves the fallback provider without a
budget check. Delete the extra pre-egress invocation that pre-checks activeModel
prior to calling chain.stream.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 32d59184-5e49-4577-a84c-bd78c6087758

📥 Commits

Reviewing files that changed from the base of the PR and between 4bcb86c and 7dec842.

📒 Files selected for processing (29)
  • CLAUDE.md
  • docs/reference/contracts/agent-session-spec.md
  • docs/reference/contracts/sse-event-schema.md
  • docs/roadmap/current.md
  • docs/roadmap/deferred-tasks.md
  • docs/roadmap/phases/phase-1-engine-and-llm.md
  • packages/core/src/dag.ts
  • packages/core/src/engine/agent-runner.e2e.test.ts
  • packages/core/src/engine/agent-runner.ts
  • packages/core/src/engine/agent-session.test.ts
  • packages/core/src/engine/agent-session.ts
  • packages/core/src/engine/agent-turn.ts
  • packages/core/src/engine/budget-governor.test.ts
  • packages/core/src/engine/budget-governor.ts
  • packages/core/src/engine/checkpoint.test.ts
  • packages/core/src/engine/checkpoint.ts
  • packages/core/src/engine/engine.test.ts
  • packages/core/src/engine/engine.ts
  • packages/core/src/engine/execution-host.test.ts
  • packages/core/src/engine/node-executor.ts
  • packages/core/src/engine/node-handlers/dispatcher.ts
  • packages/core/src/index.ts
  • packages/core/src/run-plan.ts
  • packages/llm/src/budget-estimator.test.ts
  • packages/llm/src/budget-estimator.ts
  • packages/llm/src/fallback-chain.ts
  • packages/llm/src/index.ts
  • packages/shared/src/run-event.test.ts
  • packages/shared/src/run-event.ts

Comment threadpackages/core/src/engine/budget-governor.test.ts
cemililikand others added 2 commits June 16, 2026 00:19
…t, unused test collection)
Verified each finding against current code; fixed the valid ones, skipped two with reason.
Fixed:
- agent-session.ts: roll the pushed user message back on a cancelled turn too — moved
this.#messages.pop() before the catch's cancel-check and added it to the success-path cancel
return, so a cancel-during-turn leaves NO dangling user turn in the transcript (the "only
completed exchanges" invariant — matters for 1.X persistence / 1.Z export).
- budget-governor.test.ts: replace the unsafe `(err as BudgetPauseError)` cast with an
`instanceof` type guard (CLAUDE.md no-unsafe-`as`).
- agent-session.test.ts: drop the unused `events` collection in the content-only cross-turn test
(it inspects the provider's received messages via `seen`, never the event stream) — emit is now
a no-op (Sonar: use-or-remove).
Skipped (with reason):
- "Remove the duplicate pre-egress check before chain.stream" (agent-turn.ts): NOT redundant and
NOT safe to remove. The loop-top check + the abort re-check provide the zero-egress-on-cancel
guarantee (a cancel firing during the async budget check is caught BEFORE any provider egress;
the chain's preAttempt has no post-check before its provider.stream). Removing it broke the
existing test `runAgentTurn — re-checks the signal after the preEgress await — a cancel there
costs no provider egress`, so the change was reverted. The comment's "leaves the fallback
provider without a budget check" premise is also false — the chain's preAttempt gates every
actual attempt (incl. the fallback) against its real model. The prior adversarial review (L14)
reached the same "keep both" conclusion.
- agent-session.ts `'never' in union` (Minor): the idiomatic distributive-conditional
`T extends unknown ? Omit<T,K> : never` — the `never` false-branch is required TS syntax; the
same pattern is in node-executor.ts. Not worth an awkward rewrite; does not fail the gate.
674 core tests green; full turbo gate clean; Leakwatch 0.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Apply the verified findings from the comprehensive Sonnet-agent review of the
PR-26 scope (AgentSession 1.V + budget governor 1.AC).
Engine / behavior:
- [HIGH] thread the H3 budget-approval bypass through node retries: consume the
approval ONCE per dispatch and pass it to every `#runAttempt`, so an approved
over-budget node's transient-failure retry runs uncapped instead of re-pausing
(above-chain retry, ADR-0040). Regression test pins exactly one budget:paused.
- AgentSession: drop the spurious turn-count increment on a pre-egress
BudgetPauseError (it engaged no provider); add a synchronous cancel-guard right
after the turn_started emit; settle an unexpected (non-classified) error LOUDLY
with a terminal session:turn_completed{internal} before re-raising.
- agent-runner buildLlmTools: safeParse + classified AgentTurnError (parity with
AgentSession) instead of letting a raw ZodError escape.
Contracts:
- export BudgetGovernor / BudgetExceededError / BudgetPauseError /
DEFAULT_MAX_TOKENS_ESTIMATE / BudgetCheckResult from @relavium/core.
- tighten BudgetWarningEvent.thresholdPct to an integer (the governor already
rounds + clamps).
Docs:
- sse-event-schema: a cancellation emits session:cancelled (not turn_completed);
clarify the session envelope (sessionId/sequenceNumber/gap-resync) is the bus's
job (1.W), AgentSession emits envelope-free drafts.
- agent-session-spec: context.variables is plaintext echoed in session:started —
MUST NOT carry secrets.
- agent-turn: document the intentional double pre-egress gate (loop-top
zero-egress-on-cancel guard vs the chain's per-attempt enforcement).
- agent-session: document the memoized failed-plan resolution.
Tests: +6 (approved-retry no-re-pause regression; awaitPreEgress Budget* mapping;
BudgetExceededError fields; thresholdPct/nodeId schema rejects; session reuse
after failure; unexpected-error loud-terminal + key-free message).
Full gate (format/lint/typecheck/test/build) green; Leakwatch clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@cemililik
cemililik merged commit 998f633 into mainJun 16, 2026
7 checks passed
cemililik added a commit that referenced this pull request Jun 16, 2026
…to 1.U/M2
PR #26 merged the pre-egress budget governor (1.AC, ADR-0028) and the AgentSession
agent-first entry point (1.V, ADR-0024) together. Reconcile every status home to the
post-merge state.
- 1.AC was the last 1.m4 component → **1.m4 is complete** (the full engine stack:
node handlers, gate, checkpoint/resume, retry, tools, sandbox, budget governor).
- 1.V opens the Lane-C agent-first sub-spine (1.m5, still open: 1.W/1.X/1.Y/1.Z/1.AA).
- M2 is NOT promoted: it is gated solely by the end-to-end Node harness (1.U), which is
still open. 1.AC completing 1.m4 only unblocks 1.U — now the next critical-path task.
Files:
- phase-1-engine-and-llm.md: banner; 1.AC section header + Acceptance (Met); 1.V bullet
Done marker + scope-note past-tense; 1.m4 milestone row ✅; dep-matrix 1.AC/1.U/1.V rows.
- current.md: date bump; M2-next sentence (1.m4 done, only 1.U remains); next-steps tail
(1.AC+1.V Done, 1.U next + Lane C 1.W/1.X); PR-#12 follow-up note.
- CLAUDE.md / AGENTS.md / README.md: Status paragraphs brought current through PR #26.
- deferred-tasks.md: date; 1.V merge note; close the per-attempt pre-egress gate item
(landed with 1.AC); re-home the multi-tool-ordering item (1.V now reuses the core).
Leakwatch clean; no code touched.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for freeto 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

@cemililik