refactor(turn-orchestrator): split state handlers into ports/process modules - #188
Merged
ytallo merged 2 commits intoMay 25, 2026
Conversation
…modules Reorganize the turn FSM into per-state directories that separate injected dependencies from pure transition logic, and consolidate the shared runtime behind typed ports. State handlers (flat states/*.ts -> per-state dirs): - assistant-streaming, function-execute, function-awaiting-approval, provisioning, steering-check each gain a ports.ts (I/O dependencies) and process.ts (transition logic); function-execute also splits out run.ts and types.ts New state-runtime/ layer: - store.ts all state::* turn-store I/O (was persistence/turn-state-write) - transcript.ts transcript idempotency helpers (was flat-messages) - turn-end.ts turn-end and FSM resume helpers (was finish) - ports.ts shared TurnStatePorts consumed by every state handler context-compaction: - handler-pipeline.ts holds the shared prune -> summarize -> flat-state path for the sync and async handlers (was emit.ts) Dead code removed (with tests): estimate, wake, subscriber-presence, turn-state-write, flat-messages, and the obsolete flat-state-key test. 945 tests pass; tsc -b clean.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
skill-check — worker0 verified, 12 skipped (no docs/).
Three for three. Nicely done. |
CI runs `biome ci` with biome 2.4.10 while the repo toolchain formats with 1.9.4, so newly written and edited files drift from CI's expected output and fail the lint gate. Apply 2.4.10 formatting across the affected harness files (no logic changes). Includes a few pre-existing base-branch files that had the same drift. 945 tests pass; tsc -b clean; `biome ci harness` reports no errors.
ytallo
added a commit
that referenced
this pull request
May 27, 2026
…modules (#188) * refactor(turn-orchestrator): split state handlers into ports/process modules Reorganize the turn FSM into per-state directories that separate injected dependencies from pure transition logic, and consolidate the shared runtime behind typed ports. State handlers (flat states/*.ts -> per-state dirs): - assistant-streaming, function-execute, function-awaiting-approval, provisioning, steering-check each gain a ports.ts (I/O dependencies) and process.ts (transition logic); function-execute also splits out run.ts and types.ts New state-runtime/ layer: - store.ts all state::* turn-store I/O (was persistence/turn-state-write) - transcript.ts transcript idempotency helpers (was flat-messages) - turn-end.ts turn-end and FSM resume helpers (was finish) - ports.ts shared TurnStatePorts consumed by every state handler context-compaction: - handler-pipeline.ts holds the shared prune -> summarize -> flat-state path for the sync and async handlers (was emit.ts) Dead code removed (with tests): estimate, wake, subscriber-presence, turn-state-write, flat-messages, and the obsolete flat-state-key test. 945 tests pass; tsc -b clean. * style: format harness to biome 2.4.10 CI runs `biome ci` with biome 2.4.10 while the repo toolchain formats with 1.9.4, so newly written and edited files drift from CI's expected output and fail the lint gate. Apply 2.4.10 formatting across the affected harness files (no logic changes). Includes a few pre-existing base-branch files that had the same drift. 945 tests pass; tsc -b clean; `biome ci harness` reports no errors.
sergiofilhowz
pushed a commit
that referenced
this pull request
May 28, 2026
…on teardown, readability) (#185) * refactor: remove isTerminal function and update state checks in tests - Eliminated the isTerminal function from the state module, simplifying the state management logic. - Updated tests to directly check the state property instead of using isTerminal, ensuring clarity and consistency in state validation. - Adjusted the function_id generation in wakeState to use a template string for better readability. * refactor(turn-orchestrator): add failed state, work field; own entry types * feat(turn-orchestrator): terminal-vs-transient error model with failed state * refactor(turn-orchestrator): function_execute owns batch via record.work * refactor(turn-orchestrator): emit lean TurnStateView to UI * refactor(turn-orchestrator): merge assistant_finished into assistant_streaming * feat(turn-orchestrator): legacy-record migration shim * refactor(turn-orchestrator): cap max_turns in steering_check * refactor(turn-orchestrator): drop staging keys, fold function_schemas into run_request * docs(turn-orchestrator): update for rewrite (states, work, error model, TurnStateView) * fix(turn-orchestrator): guard failed in shouldRunStep; no phantom start on re-entry * refactor(turn-orchestrator): extract skillIdFromUri, options-object system prompt - Add skillIdFromUri; remove the iii:// strip duplicated across system-prompt, provisioning, and bootstrap. - buildSystemPrompt takes a SystemPromptOptions object instead of four positional optionals, removing the unreadable bare-null call sites. - Delete the dead-in-production cwd path (no run-request source fed it). - Drop stale 'Mirrors *.rs' doc comments; use function-schema terminology. * refactor(turn-orchestrator): extract provider-stream from assistant-streaming - New provider-stream.ts owns channel creation, the concurrent provider trigger, and the read loop. A MessagePump bridges channel.onMessage to an async iterator, replacing the hand-rolled messageQueue/resolveNext/done. - assistant-streaming.ts drops 259->154 lines; handleStreaming is now linear orchestration over streamProviderTurn + finalizeAssistant. - Unify the two synthetic-error paths: createChannel failure now also emits a message_update, so the UI surfaces the error like the channel-closed path. - Add provider-stream.test.ts covering done/error frames, the per-delta callback, trigger rejection, create_channel failure, and bad-frame skips. * refactor(turn-orchestrator): simplify function_execute finalize + dedup - Replace the stale 55-line finalize comment and the convoluted tail-walk dedup (incomingIds/existingResultIds/unseen boundary) with a small persistedResultIds helper: a turn's results are the trailing run of function_result messages, so skip ids already present there. - Make augmentFunctionCall pure (it mutated the arguments object in place). - Extract applyAfterHook and toFunctionResultMessage from finalizeExecutedCalls. - Fix the stale 'handleFinalize:' log label. * refactor(turn-orchestrator): abort uses canonical agent-scope key helpers Replace the local STATE_SCOPE_AGENT constant and inlined abort_signal key string with AGENT_SCOPE and abortSignalKey() from state.ts. * refactor(turn-orchestrator): steering_check abort read uses AGENT_SCOPE * refactor(turn-orchestrator): one synthetic-assistant factory Collapse four hand-built synthetic AssistantMessage literals (assistant stream error, transition failure, steering abort, max_turns stop) into a single syntheticAssistant() helper built on emptyAssistant. The empty turn_end fallback in steering_check now uses emptyAssistant directly. * refactor(turn-orchestrator): drop dead pending_function_calls field The field was only ever written (= []) and never read. Remove it from TurnStateRecord, its schema, newRecord, the function_execute finalize assignment, and the test fixtures that set it. Also drop the stale 'Mirrors src/state.rs' header comment. * refactor(turn-orchestrator): drop stale rust-mirror comments, align terminology - Remove the last 'Mirrors *.rs' header comments (events, persistence). - events.ts uses AGENT_SCOPE instead of a local 'agent' constant. - 'tool call' -> 'function call' in agent-trigger/function-execute comments (the agent_trigger tool schema name stays: it is the LLM-facing tool). - Name the policy consult timeout (POLICY_TIMEOUT_MS) like HOOK_TIMEOUT_MS. * refactor(turn-orchestrator): unexport internal-only helpers and schemas decodeOrPassthrough, formatItemId, TurnStateRecordSchema, and SessionIdPayloadSchema are each used only within their own file (the public wrappers are triggerFunctionCall, emit, parseTurnStateRecord, and the derived payload schemas). Drop the unnecessary export to shrink the module surface. * wip: turn-orchestrator cleanup in progress Snapshot of uncommitted work moved off the harness-trigger PR branch. * feat(turn-orchestrator): add approval-decision state-event schema * feat(turn-orchestrator): add reactive approval state trigger * feat(turn-orchestrator): register approval trigger; wake parked sessions on boot * feat(approval-gate): resolve writes decision to approvals scope * feat(turn-orchestrator): abort writes aborted decisions to approvals scope * feat(turn-orchestrator): park without per-call resume registration * refactor(turn-orchestrator): remove per-call approval-resume machinery * feat(turn-orchestrator): add finishSession terminal helper * feat(turn-orchestrator): end turns via finishSession instead of tearing_down * refactor(turn-orchestrator): remove tearing_down state * test(turn-orchestrator): remove tautological and duplicated tests Drop tests that assert properties set on a literal (type-shape only) or duplicate behavior covered elsewhere: - persistence-prepared.test.ts: asserted PreparedEntry fields it just set; PreparedEntry is exercised for real in functions/awaiting-approval tests - agent-trigger.test.ts: DispatchResult shape block read back the discriminant it constructed; real behavior is covered by dispatchWithHook - state.test.ts: removed negative-tautology asserts, the AwaitingApprovalEntry shape test, the handleAwaitingApproval empty-queue case (dup of awaiting-approval.test.ts), and the duplicated newRecord test (work-undefined check folded into the kept one) No behavioral coverage lost: 228 -> 217 passing, tsc clean. * refactor(turn-orchestrator): cut reactive-trigger RPC overhead, remove dead abort path - After-function-call hook skips publish_collect when no durable subscriber is registered for the topic (subscriber-presence cache), removing a fixed ~500ms collect wait per executed tool-call result on the turn critical path. - Context compaction subscribes to a dedicated `agent::turn_end` stream (mirrored by the event producer) instead of the full `agent::events` firehose, so it wakes once per turn instead of on every agent event. - Session-create fanout rides a dedicated `session_index` scope marker written once at first persist; the trigger matches by scope alone, dropping the per-write `harness::session::is_create_event` condition RPC. - Remove the orphaned state-based abort subgraph (abort.ts, on-abort-signal.ts, the `turn_abort` scope, the steering_check abort route): it had no production producer (abort/cancel is handled at the ACP layer) and the `router::abort` entrypoint it depended on does not exist. Drops the dead `!router::abort` kernel-deny entry. * refactor(turn-orchestrator): split state handlers into ports/process modules (#188) * refactor(turn-orchestrator): split state handlers into ports/process modules Reorganize the turn FSM into per-state directories that separate injected dependencies from pure transition logic, and consolidate the shared runtime behind typed ports. State handlers (flat states/*.ts -> per-state dirs): - assistant-streaming, function-execute, function-awaiting-approval, provisioning, steering-check each gain a ports.ts (I/O dependencies) and process.ts (transition logic); function-execute also splits out run.ts and types.ts New state-runtime/ layer: - store.ts all state::* turn-store I/O (was persistence/turn-state-write) - transcript.ts transcript idempotency helpers (was flat-messages) - turn-end.ts turn-end and FSM resume helpers (was finish) - ports.ts shared TurnStatePorts consumed by every state handler context-compaction: - handler-pipeline.ts holds the shared prune -> summarize -> flat-state path for the sync and async handlers (was emit.ts) Dead code removed (with tests): estimate, wake, subscriber-presence, turn-state-write, flat-messages, and the obsolete flat-state-key test. 945 tests pass; tsc -b clean. * style: format harness to biome 2.4.10 CI runs `biome ci` with biome 2.4.10 while the repo toolchain formats with 1.9.4, so newly written and edited files drift from CI's expected output and fail the lint gate. Apply 2.4.10 formatting across the affected harness files (no logic changes). Includes a few pre-existing base-branch files that had the same drift. 945 tests pass; tsc -b clean; `biome ci harness` reports no errors. * refactor(turn-orchestrator): enhance function-awaiting-approval handling - Update the function-awaiting-approval state to execute resolved calls immediately as decisions arrive, improving responsiveness. - Modify the function-execute state to allow concurrent handling of pending approvals, ensuring that multiple calls can await approval without blocking. - Refactor related logic to streamline the transition between states, including clearer handling of batch completion and decision processing. - Remove outdated tests and add new integration tests to validate parallel approval flows and decision handling. This refactor aims to improve the efficiency and clarity of the approval process within the turn orchestrator, ensuring a more responsive and robust system. * refactor(turn-orchestrator): streamline approval handling and state transitions - Remove the wakeStep and wakeFromRecord methods from TurnStore, replacing them with inline enqueuing of state transitions directly from saveRecord. - Update the on-approval trigger to enqueue function_awaiting_approval, enhancing the responsiveness of the approval process. - Refactor the function-awaiting-approval logic to ensure that parked calls are handled more efficiently, allowing for better management of concurrent approvals. - Clean up related tests and remove outdated code to improve maintainability and clarity. These changes aim to enhance the efficiency and clarity of the turn orchestrator's approval handling, ensuring a more robust and responsive system. * refactor(turn-orchestrator): enhance approval handling and state management - Streamline the approval process by updating the `approval-gate` description to clarify its role in persisting decisions and enqueuing `turn::function_awaiting_approval`. - Refactor the handling of approval decisions to ensure that state transitions are triggered correctly when decisions are written to the `approvals` scope. - Remove the obsolete `on-approval.ts` file and integrate its functionality into the existing structure, improving maintainability. - Introduce a new `TurnStateInvariantError` class to handle validation errors related to turn state records. These changes aim to improve the clarity and efficiency of the turn orchestrator's approval handling, ensuring a more robust and responsive system. * chore(harness): apply biome 2.4.10 formatting Match the CI biome version on 11 files (3 src, 8 tests). * docs(context-compaction): note dedicated agent::turn_end subscription Header and registered-function blurb still said the subscriber fires on every agent::events message; the wire is agent::turn_end (one wake per turn). The function name kept its historical on_agent_event id. * docs: reconcile harness docs with code after FSM rewrite - architecture.md: worker count 11 → 15; add provider-config + provider-llamacpp rows; note context-compaction rides agent::turn_end. - approval-gate.md: replace dangling on-approval.ts link with function-awaiting-approval/process.ts. - context-compaction.md: use update_parts (batched) in prose + dependencies table; list handler-pipeline.ts and flat-state.ts in source layout. - session.md: add the four session-tree functions the list was missing (compactions, append_synthetic, update_part, update_parts); 11 → 15. - turn-orchestrator.md: drop misleading "in parallel" from the function_execute description — the dispatch loop is sequential, just non-blocking on pending calls. * docs(shell): remove outdated isolation boundary warning from README The README.md file has been updated to remove the section regarding the isolation boundary of `shell::exec`, clarifying that it is not an effective isolation mechanism. This change aims to improve the documentation's accuracy and guidance for users regarding the use of shell execution in untrusted environments.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Restructures the turn-orchestrator state machine for readability: each FSM state moves from a single flat file into its own directory that separates injected dependencies (
ports.ts) from pure transition logic (process.ts). A newstate-runtime/layer consolidates the shared store, transcript, and turn-end helpers behind one typedTurnStatePorts. Net change is a large deletion (dead code + duplication removed).State handlers (
states/*.ts→ per-state directories)assistant-streamingports.ts,process.tsfunction-executeports.ts,process.ts,run.ts,types.tsfunction-awaiting-approvalports.ts,process.tsprovisioningports.ts,process.ts,load-skills.tssteering-checkports.ts,process.tsNew
state-runtime/layerstore.ts— allstate::*turn-store I/O (replacespersistence.ts/turn-state-write.ts)transcript.ts— transcript idempotency helpers (replacesflat-messages.ts)turn-end.ts— turn-end and FSM resume helpers (replacesfinish.ts)ports.ts— sharedTurnStatePortsconsumed by every state handlercontext-compaction
handler-pipeline.tsholds the shared prune → summarize → flat-state rewrite path used by both the sync and async handlers (replacesemit.ts)Dead code removed (with tests)
estimate,wake,subscriber-presence,turn-state-write,flat-messages, and the obsoleteflat-state-keytest.Test plan
pnpm -C harness test— 945 passed across 115 filespnpm -C harness typecheck(tsc -b --noEmit) — clean