feat: minimal approval gate with fail-closed execution + resume stability - #150
Closed
ytallo wants to merge 4 commits into
Closed
feat: minimal approval gate with fail-closed execution + resume stability#150ytallo wants to merge 4 commits into
ytallo wants to merge 4 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughApproval gating moves to session-scoped pending records with created_at/expires_at. New APIs: consume (drain resolved entries) and sweep_session (timeout deny). approval::resolve emits events and triggers run::resume; turn-orchestrator consumes approvals, resumes idled turns, and UI surfaces wake-failure alerts. ChangesApproval Session Resumption
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
The approval gate must fail closed when its hook infrastructure is missing, pause write-capable calls until an operator resolves them, and resume execution without duplicating tool results in the provider transcript. This keeps the minimal approval path releaseable without carrying unrelated branch work. Constraint: Keep the delivery scoped to the minimal approval-gate path from origin/main Constraint: Approval-gate resolution must execute allowed calls through the existing function dispatch path Rejected: Execute the target function inside approval::resolve | would bypass orchestrator lifecycle and dispatch invariants Rejected: Publish run::resume for active turns | created duplicate step execution and repeated tool results Confidence: high Scope-risk: moderate Directive: Do not change approval resume behavior without rerunning the browser allow/deny E2E against a real provider Tested: git diff --check Tested: cargo test in approval-gate, hook-fanout, turn-orchestrator, provider-router Tested: npm test -- ApprovalRow.test.tsx reducer.test.ts in harness/web Tested: npm run build in harness/web Tested: npm run e2e -- tests/e2e/approval.spec.ts --output=/tmp/harness-approval-e2e-results with claude-haiku-4-5 and Anthropic env credential Not-tested: separate fresh-worktree patch replay after the final E2E-driven resume fixes
The iii-state session store was reconstructing entry order by UUID/id after state::list, but active_path(None) treats the last loaded entry as the current leaf. Approval resume can append the final assistant response with an id that sorts before earlier entries, so the backend state contains the response while the web console reload picks the wrong path. Sorting loaded entries by entry timestamp preserves append order and keeps the resumed response visible. Constraint: state::list returns values without storage keys or insertion order Rejected: Add web-console polling as the fix | the missing result was already present in state and the root issue was active-path reconstruction Confidence: high Scope-risk: narrow Directive: Do not change active_path(None) or state-store ordering without verifying approval resume reloads in the web console Tested: git diff --check Tested: cargo test --lib load_entries_lists_session_scope_and_sorts_by_timestamp -- --nocapture Tested: cargo test library suite passed as part of cargo test before integration tests Not-tested: full session cargo test; tests/inbox_integration.rs end_to_end_push_via_iii_sdk failed because session-inbox::push was not registered in the local iii runtime
ytallo
force-pushed
the
feat/approval-gate
branch
from
May 18, 2026 09:58
9490da5 to
55891f2
Compare
Contributor
skill-check — worker2 verified, 24 skipped (no docs/).
Three for three. Nicely done. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
approval-gate/src/lib.rs (2)
661-683: ⚖️ Poor tradeoffBusy-wait loop with fixed deadline may delay caller longer than necessary.
The
resume_sessionloop sleeps 250ms between attempts and runs for up to 30 seconds. Ifrun::resumereturnsresumed: falserepeatedly (e.g., session is stuck in a non-terminal state), this will block the caller for the full 30 seconds before returning an error.Consider adding a check for non-transient failure conditions (e.g., if the session no longer exists or is in an unexpected state) to fail fast rather than waiting the full timeout.
🤖 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 `@approval-gate/src/lib.rs` around lines 661 - 683, The busy-wait in resume_session (uses III.trigger with function_id "run::resume") should bail early on non-transient failures instead of always sleeping 250ms up to 30s: after calling iii.trigger and parsing response, inspect additional response fields (e.g., response.get("error"), response.get("state") or any "status"/"exists" indicators returned by the "run::resume" RPC) and return Err immediately for terminal/non-retryable conditions (session not found, closed, invalid state) rather than continuing the loop; also consider replacing the fixed 250ms sleep with a short backoff strategy (or at least a smaller initial sleep) so transient retries remain possible but caller isn’t forced to wait the full deadline when a non-transient condition is detected.
355-360: 💤 Low valuePotential truncation in
now_ms()on 32-bit targets or distant future.
as_millis()returnsu128, and casting tou64truncates silently. On 64-bit systems with current timestamps this is fine, but consider usingtry_into().unwrap_or(u64::MAX)for defensive clarity.🤖 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 `@approval-gate/src/lib.rs` around lines 355 - 360, The now_ms() function currently casts SystemTime::duration_since(...).as_millis() (u128) to u64 which can silently truncate on 32-bit targets or for far-future timestamps; change the conversion to use try_into() (std::convert::TryInto) and on failure return a safe sentinel such as u64::MAX (or propagate) so truncation cannot occur—update the now_ms() implementation to map the u128 result with try_into().unwrap_or(u64::MAX) (or equivalent error handling) instead of as_millis() as u64.turn-orchestrator/src/states/functions.rs (1)
329-338: 💤 Low valueUnused parameter
_has_last_assistantinnext_state_after_finalize.The parameter is prefixed with underscore indicating intentional non-use, but the function currently ignores it entirely. If future logic needs to differentiate behavior based on whether
last_assistantexists, this is ready. Otherwise, consider documenting the intent or removing the parameter if it won't be needed.🤖 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 `@turn-orchestrator/src/states/functions.rs` around lines 329 - 338, The function next_state_after_finalize currently takes the unused parameter _has_last_assistant; remove the parameter (and its underscore) from the signature and all call sites if it is not needed, or if intended for future logic, document its purpose in a short comment above next_state_after_finalize and use the non-underscored name has_last_assistant so it is clear and can be referenced later; ensure any callers that pass the extra argument are updated and keep the existing return logic using TurnState::TearingDown and TurnState::SteeringCheck unchanged.
🤖 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 `@harness/web/tests/e2e/approval.spec.ts`:
- Line 65: The one-shot assertion expect(existsSync(path)).toBe(false) can race
with late side-effects; change the test to poll for a short window (e.g.,
50–200ms) and assert the directory never appears during that window. Make the
test async, repeatedly call existsSync(path) on a small interval (e.g., 10–20ms)
for the chosen timeout and fail if any poll returns true; keep the same
expectation semantics (directory must remain absent). Use the existing path
variable and replace the single expect(existsSync(path)).toBe(false) with this
short-poll loop-based assertion.
In `@session/src/tree/store_iii_state.rs`:
- Around line 112-116: The sort tie-breaker uses entry id which can reorder
concurrent millisecond timestamps; add and persist a monotonic append-order
field (e.g., append_index or seq_no) on the entry type, set/increment it when
writing/appending entries, and change the comparator at entries.sort_by (the
closure using a.timestamp() and a.id()) to compare timestamp(), then
append_index(), then id() as a final fallback so replay always follows true
write order; ensure the write/append code path that constructs entries assigns
the monotonic value and that the field is included in persistence and
deserialization.
---
Nitpick comments:
In `@approval-gate/src/lib.rs`:
- Around line 661-683: The busy-wait in resume_session (uses III.trigger with
function_id "run::resume") should bail early on non-transient failures instead
of always sleeping 250ms up to 30s: after calling iii.trigger and parsing
response, inspect additional response fields (e.g., response.get("error"),
response.get("state") or any "status"/"exists" indicators returned by the
"run::resume" RPC) and return Err immediately for terminal/non-retryable
conditions (session not found, closed, invalid state) rather than continuing the
loop; also consider replacing the fixed 250ms sleep with a short backoff
strategy (or at least a smaller initial sleep) so transient retries remain
possible but caller isn’t forced to wait the full deadline when a non-transient
condition is detected.
- Around line 355-360: The now_ms() function currently casts
SystemTime::duration_since(...).as_millis() (u128) to u64 which can silently
truncate on 32-bit targets or for far-future timestamps; change the conversion
to use try_into() (std::convert::TryInto) and on failure return a safe sentinel
such as u64::MAX (or propagate) so truncation cannot occur—update the now_ms()
implementation to map the u128 result with try_into().unwrap_or(u64::MAX) (or
equivalent error handling) instead of as_millis() as u64.
In `@turn-orchestrator/src/states/functions.rs`:
- Around line 329-338: The function next_state_after_finalize currently takes
the unused parameter _has_last_assistant; remove the parameter (and its
underscore) from the signature and all call sites if it is not needed, or if
intended for future logic, document its purpose in a short comment above
next_state_after_finalize and use the non-underscored name has_last_assistant so
it is clear and can be referenced later; ensure any callers that pass the extra
argument are updated and keep the existing return logic using
TurnState::TearingDown and TurnState::SteeringCheck unchanged.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 07774515-4f61-4177-b2d0-2622d1ce6965
📒 Files selected for processing (22)
approval-gate/src/lib.rsapproval-gate/tests/integration.rsharness/web/src/App.tsxharness/web/src/components/ApprovalRow.test.tsxharness/web/src/components/ApprovalRow.tsxharness/web/src/reducer.test.tsharness/web/src/reducer.tsharness/web/src/types.tsharness/web/tests/e2e/approval.spec.tshook-fanout/src/handler.rshook-fanout/src/lib.rsprovider-router/src/register.rssession/src/tree/mod.rssession/src/tree/store_iii_state.rsturn-orchestrator/src/bootstrap.rsturn-orchestrator/src/manifest.rsturn-orchestrator/src/run_start.rsturn-orchestrator/src/states/assistant.rsturn-orchestrator/src/states/functions.rsturn-orchestrator/src/states/provisioning.rsturn-orchestrator/src/states/tearing_down.rsturn-orchestrator/src/system_prompt.rs
| const result = page.locator(".block-tool-result[data-error='true']"); | ||
| await expect(result).toBeVisible({ timeout: 30_000 }); | ||
| await expect(result).toBeVisible({ timeout: 90_000 }); | ||
| expect(existsSync(path)).toBe(false); |
There was a problem hiding this comment.
Harden deny-path filesystem assertion against delayed side effects.
On Line 65, the one-shot existsSync check can pass before an unintended late tool execution happens. Use a short poll window to assert the directory stays absent.
Suggested fix
- expect(existsSync(path)).toBe(false);
+ await expect.poll(() => existsSync(path), { timeout: 5_000 }).toBe(false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expect(existsSync(path)).toBe(false); | |
| await expect.poll(() => existsSync(path), { timeout: 5_000 }).toBe(false); |
🤖 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 `@harness/web/tests/e2e/approval.spec.ts` at line 65, The one-shot assertion
expect(existsSync(path)).toBe(false) can race with late side-effects; change the
test to poll for a short window (e.g., 50–200ms) and assert the directory never
appears during that window. Make the test async, repeatedly call
existsSync(path) on a small interval (e.g., 10–20ms) for the chosen timeout and
fail if any poll returns true; keep the same expectation semantics (directory
must remain absent). Use the existing path variable and replace the single
expect(existsSync(path)).toBe(false) with this short-poll loop-based assertion.
Comment on lines
+112
to
+116
| entries.sort_by(|a, b| { | ||
| a.timestamp() | ||
| .cmp(&b.timestamp()) | ||
| .then_with(|| a.id().cmp(b.id())) | ||
| }); |
There was a problem hiding this comment.
Timestamp collisions can still reorder entries and break append-order recovery.
Line 112 orders equal timestamps by id, which is not an append sequence. If two entries share a millisecond timestamp, replay order can still flip. Persist a monotonic append index (or equivalent write-order field) and use it as the tie-breaker instead of id.
🤖 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 `@session/src/tree/store_iii_state.rs` around lines 112 - 116, The sort
tie-breaker uses entry id which can reorder concurrent millisecond timestamps;
add and persist a monotonic append-order field (e.g., append_index or seq_no) on
the entry type, set/increment it when writing/appending entries, and change the
comparator at entries.sort_by (the closure using a.timestamp() and a.id()) to
compare timestamp(), then append_index(), then id() as a final fallback so
replay always follows true write order; ensure the write/append code path that
constructs entries assigns the monotonic value and that the field is included in
persistence and deserialization.
…ling The resolver's 250 ms `run::resume` retry loop racing the executor's park is replaced with an in-process `AwaitingApproval` notifier. The executor signals on terminal save in `subscriber::execute`; `execute_resume` does fast-path → arm Notified → recheck → bounded wait. The resolver fires one `run::resume` and waits server-side. Median resume latency drops from up-to-250ms quantized polling to the microseconds it takes Notify::notify_waiters to wake the parked task. The 30 s ceiling is unchanged from the old loop's deadline; it's the failure timeout, not the expected wait.
`bridge::trigger` was renamed to `harness::call` (and span name `harness.bridge.trigger` → `harness.call`) in a95f2ad, and `bridge::events` was removed entirely in 9b9b015. The trace_correlation tests still referenced the old names and failed with `function_not_found`. - Update the three remaining tests to call `harness::call` and assert the new span name `harness.call`. - Rename `bridge_trigger_*` test fn names to `harness_call_*` to match. - Delete `bridge_events_sse_emits_traceparent_and_message_id_headers` — the function it exercised no longer exists.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
turn-orchestrator/src/awaiting.rs (1)
1-42: 💤 Low valueConsider bounded session slot capacity or periodic cleanup.
The
innerHashMap grows unboundedly as sessions are added. Whileclear()exists for explicit removal, long-running processes could accumulate stale slots if callers forget to clear. This is low-priority sinceArc<Notify>is small, but worth noting for future observability or explicit TTL-based eviction.🤖 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 `@turn-orchestrator/src/awaiting.rs` around lines 1 - 42, The inner HashMap in AwaitingApproval can grow unbounded because slots are stored as Arc<Notify> and only removed if callers call clear(); to prevent leaks, change the storage to HashMap<String, Weak<Notify>> (or use an LRU/TTL cache) and update AwaitingApproval::slot to try upgrading the Weak to Arc and clean up dead Weak entries before inserting a new Arc, ensuring signal() and clear() continue to work with the new storage type; alternatively add a background cleanup task that periodically prunes stale entries from inner.approval-gate/src/lib.rs (1)
302-304: 💤 Low valueConsider logging consume write failures for observability.
Silent
let _ =on the status update means failures go unnoticed. If the write fails, the entry remains "resolved" and will be re-consumed on the next call. While this fail-safe behavior is reasonable (re-delivery over loss), logging the error would help diagnose state-bus issues in production.Suggested improvement
row["status"] = json!("consumed"); row["consumed_at"] = json!(now_ms()); - let _ = bus.set(state_scope, &key, row).await; + if let Err(err) = bus.set(state_scope, &key, row).await { + tracing::warn!( + "approval-gate: failed to mark {}/{} as consumed: {err}", + session_id, + function_call_id + ); + }🤖 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 `@approval-gate/src/lib.rs` around lines 302 - 304, The code silently drops the result of bus.set(state_scope, &key, row).await; capture its Result and log failures for observability instead of using let _ =. Replace that line with an explicit match or if let Err(e) = bus.set(state_scope, &key, row).await { ... } and call the crate's error logger (e.g., tracing::error! or log::error!) including context like the key, state_scope, and the error so we can diagnose write failures to the state bus; keep the same semantics (do not panic) so re-delivery still happens on error.
🤖 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.
Nitpick comments:
In `@approval-gate/src/lib.rs`:
- Around line 302-304: The code silently drops the result of
bus.set(state_scope, &key, row).await; capture its Result and log failures for
observability instead of using let _ =. Replace that line with an explicit match
or if let Err(e) = bus.set(state_scope, &key, row).await { ... } and call the
crate's error logger (e.g., tracing::error! or log::error!) including context
like the key, state_scope, and the error so we can diagnose write failures to
the state bus; keep the same semantics (do not panic) so re-delivery still
happens on error.
In `@turn-orchestrator/src/awaiting.rs`:
- Around line 1-42: The inner HashMap in AwaitingApproval can grow unbounded
because slots are stored as Arc<Notify> and only removed if callers call
clear(); to prevent leaks, change the storage to HashMap<String, Weak<Notify>>
(or use an LRU/TTL cache) and update AwaitingApproval::slot to try upgrading the
Weak to Arc and clean up dead Weak entries before inserting a new Arc, ensuring
signal() and clear() continue to work with the new storage type; alternatively
add a background cleanup task that periodically prunes stale entries from inner.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 96c820bf-116d-4671-be80-56e177f8377c
📒 Files selected for processing (26)
approval-gate/src/lib.rsapproval-gate/tests/integration.rsharness/web/src/App.tsxharness/web/src/components/ApprovalRow.test.tsxharness/web/src/components/ApprovalRow.tsxharness/web/src/reducer.test.tsharness/web/src/reducer.tsharness/web/src/types.tsharness/web/tests/e2e/approval.spec.tshook-fanout/src/handler.rshook-fanout/src/lib.rsprovider-router/src/register.rssession/src/tree/mod.rssession/src/tree/store_iii_state.rsturn-orchestrator/src/awaiting.rsturn-orchestrator/src/bootstrap.rsturn-orchestrator/src/lib.rsturn-orchestrator/src/manifest.rsturn-orchestrator/src/register.rsturn-orchestrator/src/run_start.rsturn-orchestrator/src/states/assistant.rsturn-orchestrator/src/states/functions.rsturn-orchestrator/src/states/provisioning.rsturn-orchestrator/src/states/tearing_down.rsturn-orchestrator/src/subscriber.rsturn-orchestrator/src/system_prompt.rs
✅ Files skipped from review due to trivial changes (4)
- turn-orchestrator/src/lib.rs
- turn-orchestrator/src/manifest.rs
- turn-orchestrator/src/states/provisioning.rs
- turn-orchestrator/src/system_prompt.rs
🚧 Files skipped from review as they are similar to previous changes (14)
- harness/web/src/reducer.ts
- harness/web/src/App.tsx
- harness/web/src/components/ApprovalRow.test.tsx
- session/src/tree/mod.rs
- turn-orchestrator/src/states/tearing_down.rs
- turn-orchestrator/src/run_start.rs
- turn-orchestrator/src/states/assistant.rs
- harness/web/src/reducer.test.ts
- hook-fanout/src/lib.rs
- harness/web/src/types.ts
- approval-gate/tests/integration.rs
- provider-router/src/register.rs
- harness/web/src/components/ApprovalRow.tsx
- harness/web/tests/e2e/approval.spec.ts
5 tasks
sergiofilhowz
pushed a commit
that referenced
this pull request
May 18, 2026
…152) * feat(console): drive model picker from harness-node models-catalog Replace the hardcoded `ModelId` union and four-entry `MODELS` constant with composite `provider::<catalog_model_id>` keys sourced from the harness-node models-catalog worker. - `lib/catalog-model-key.ts`: parse/format `provider::id` (uses `::` separator constant `CATALOG_MODEL_KEY_SEP`). - `lib/models-catalog.ts`: client wrapper around `models::list` / `models::get` for the picker. - `hooks/use-model-picker-source.ts`: live-updating picker source with a static fallback for when the engine is down or the mock backend is in use. - `types/chat.ts`: `ModelId` widened to `string`; `STATIC_MODEL_OPTIONS` + `DEFAULT_MODEL` replace the old enum-shaped `MODELS`. - `components/chat/ModelPicker.tsx`: accept `options` + `loading` props; fall back to `{id, label}` from the current value if the catalog is empty so the picker is never blank. - `components/chat/Composer.tsx`, `components/chat/ChatView.tsx`, `pages/Chat.tsx`, `hooks/use-conversations.ts`, `pages/Examples/sections/*`, `pages/Playground/index.tsx`: thread the catalog-shaped model ids through; legacy heuristic ids without `::` still get the old `claude*` / `gemini*` provider guess in `resolveRunParams`. - `lib/backend/real.ts`: `resolveRunParams` parses `provider::id` and falls back to the heuristic for legacy ids; doc comment updated. * refactor(real): remove debug instrumentation from session event handling This commit removes the debug instrumentation code that was previously used to log session events to a local ingestion endpoint. The removal streamlines the event handling process in the `realStream` function, improving code clarity and maintainability. * feat(approval-gate): port PR #150 with iii-native resume + console wiring Port the fail-closed approval-gate redesign from the Rust workspace to harness-node and finish wiring the console/web UI so chat sessions can approve/deny intercepted tool calls without polling. Backend: - intercept-at-gate: subscriber returns block_reply with status pending|denied; resolved decisions are drained later via approval::consume; pending entries can be force-resolved via approval::sweep_session - run::resume rebuilds terminal TurnStateRecord (preserving turn_count + max_turns) and publishes a fresh turn::step_requested - handle_resolve emits approval_resolved + asks the orchestrator to resume via a single iii::durable::publish on turn::step_requested; no polling loop and no explicit timeout (the bus default is the right backstop) - subscriber detects terminal records and rebuilds via build_resume_plan before stepping - hook-fanout merge_first_block_wins preserves the full blocking reply (status, function_call_id, subscriber, approval_gate, denial) - publish-collect emits publish.{ok,error} + publish_failed so the orchestrator can fail closed on bus unavailability - handle_awaiting + tearing_down each call approval::consume first; on non-empty result they stage the resolved entries as prepared calls and transition (or resurrect) into FunctionExecute - router::abort composes [state::set abort, approval::sweep_session] - session-tree sorts entries by (timestamp, id) so replies always appear in causal order Console/web: - approve/deny buttons call approval::resolve via the iii client - the ui::session::event stream stays subscribed for the chat lifetime (no auto-close on first agent_end) so multi-turn approvals continue to flow - approval prompts auto-scroll into view; submitting state shows disabled buttons + visible feedback while waiting for the agent to resume - fcall-start events deduplicate on functionCallId; pending_approval placeholders suppress their fcall-end until the real result lands Tests: TDD'd helpers, intercept/consume/sweep handlers, resume plumbing, the orchestrator integration points, and an end-to-end approval-resume.e2e covering intercept -> resolve -> publish -> subscriber wake -> rebuild -> consume -> execute. * feat(sessions-poll): implement reactive sessions fanout with state triggers Replaces the polling mechanism for session updates with a reactive approach using state triggers. The new implementation filters state events for session creation and directly pushes updates to subscribers without polling. This change enhances efficiency and reduces latency in session updates, streamlining the fanout process. Key components include condition and handler functions for session creation events, and a direct trigger registration for state changes. * Simplify iii-native approval gate flow (#154) * Make approval pause explicit in the turn state machine The iii-native approval flow needs a non-terminal state where the orchestrator can wait without tearing down the turn or rebuilding from provisioning. This adds the state value and locks its non-terminal behavior in the state test. Constraint: Plan task 1 targets harness-node TypeScript only; Rust workers remain untouched. Rejected: Keep using stopped/resume as the pause marker | later tasks remove that detour. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/state.test.ts Not-tested: Full harness-node suite deferred until the batch verification step * Let the turn record own approval waits The pause-in-place design needs the orchestrator record to carry the function calls that are waiting on human decisions. This adds the awaiting_approval entry shape without changing any behavior yet. Constraint: Field remains optional so fresh and existing records continue to deserialize without migration. Rejected: Store pending calls only in approval-gate state | the simplification moves ownership into the orchestrator FSM. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/state.test.ts Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until the batch verification step * Make prepared function calls extensible for approvals The upcoming pause-in-place approval handler needs to persist per-call metadata without overloading tuple positions. This converts prepared calls to an object shape and adds the optional pre_approved flag while preserving the current execution behavior. Constraint: Existing staged values may use function_call or legacy tool_call keys, so the loader keeps both forms. Rejected: Add a parallel pre-approved lookup table | it would split one prepared-call concern across two state keys. Confidence: medium Scope-risk: moderate Directive: Keep PreparedEntry as the single persisted shape for prepared-call execution metadata. Tested: pnpm test tests/turn-orchestrator/persistence-prepared.test.ts tests/turn-orchestrator/functions.test.ts tests/turn-orchestrator/assistant.test.ts tests/turn-orchestrator/tearing-down.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm test Tested: pnpm typecheck Tested: pnpm lint * Define dispatch outcomes before approval cutover The approval pause cutover needs dispatchWithHook to distinguish successful execution, hard denial, and pending approval. This commit introduces the discriminated result type before changing runtime dispatch behavior. Constraint: Task 4 is type-only; dispatchWithHook still returns FunctionResult until the later refactor task. Rejected: Change dispatch behavior now | the plan keeps the behavioral cutover staged. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/agent-call.test.ts Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until batch verification * Resume approved calls from an explicit wait state The new approval pause state needs a handler that stays idle until every awaited decision is present, then updates prepared calls with either pre_approved metadata or a denial result before returning to function execution. Constraint: Decisions are read from the approvals state scope using the existing session/function-call key shape. Rejected: Poll approval::consume from generic assistant or teardown states | the simplification makes awaiting approval an explicit FSM state. Confidence: medium Scope-risk: moderate Directive: Keep missing decisions as a no-op so durable turn::step_requested wakeups remain idempotent. Tested: pnpm test tests/turn-orchestrator/awaiting-approval.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until batch verification * Route approval waits through the turn transition switch The FSM can now enter function_awaiting_approval, so the central step dispatcher must route that state to the new handler instead of silently doing nothing. Constraint: This only wires the state; handleExecute has not yet been cut over to produce it. Rejected: Depend on fallthrough/no-op behavior | the wait state needs explicit wake handling from turn::step_requested. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/state.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until batch verification * Normalize awaiting-approval files for Biome The new wait-state handler passed behavior and type checks, but Biome required import ordering and line wrapping before the batch could be called clean. Constraint: Formatting-only follow-up after Task 5 and Task 6 verification. Confidence: high Scope-risk: narrow Tested: pnpm test Tested: pnpm typecheck Tested: pnpm lint * Return structured outcomes from hooked dispatch The approval cutover needs dispatchWithHook to report whether a call ran, was denied, or is waiting for human approval. This changes the dispatcher API to return DispatchResult and keeps the LLM-facing agent::call wrapper unwrapped as a FunctionResult. Constraint: handleExecute still consumes the old return shape until the next cutover task. Rejected: Keep pending as a placeholder FunctionResult | the new FSM state owns approval waiting explicitly. Confidence: medium Scope-risk: moderate Directive: Callers must branch on DispatchResult.kind before treating the value as a FunctionResult. Tested: pnpm test tests/turn-orchestrator/agent-call.test.ts tests/turn-orchestrator/functions.test.ts Not-tested: pnpm typecheck fails until handleExecute is cut over in Task 9 * Pause function execution in the approval wait state handleExecute now treats pending approval as a real FSM pause instead of manufacturing a placeholder function result. It also resumes pre-approved calls without re-entering the hook and emits denial results without dispatching blocked calls. Constraint: Old placeholder helper tests are skipped until the later cleanup task removes that machinery. Rejected: Keep pending approval as a terminating FunctionResult | that leaks an artificial result into the transcript and keeps the resume detour alive. Confidence: medium Scope-risk: moderate Directive: Preserve the existing executed-call check before dispatching so retrying a step does not duplicate approved function execution. Tested: pnpm test tests/turn-orchestrator/functions.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm typecheck Tested: pnpm test tests/integration/approval-resume.e2e.test.ts Tested: pnpm test Tested: pnpm lint * Keep approval gate pending state in the orchestrator The gate no longer writes a pending state-bus record when policy asks for human approval. It emits approval_requested and returns a pending hook reply; the turn orchestrator owns the pending-call list on its state record. Constraint: The approvals state bus now stores resolved decisions only. Rejected: Keep handleIntercept in the gate path | it preserves the obsolete pending-record lifecycle and duplicates orchestrator state. Confidence: medium Scope-risk: moderate Directive: Do not add state-bus pending writes back to policy::approval_gate; pending ownership belongs to TurnStateRecord.awaiting_approval. Tested: pnpm test tests/approval-gate/gate-subscriber.test.ts tests/approval-gate/events.test.ts Tested: pnpm typecheck * Drop approval request expiry from event types Approval prompts no longer time out through the gate, so the shared event types should not promise an expires_at timestamp. The console typecheck also needed the currently-unused mode parameter marked intentionally unused. Constraint: No timeout sweep behavior remains in the target approval-request event contract. Rejected: Keep expires_at as optional | it would preserve dead UI expectations around automatic expiry. Confidence: high Scope-risk: narrow Tested: pnpm typecheck in harness-node Tested: pnpm typecheck in console/web * Remove obsolete approval intercept lifecycle The gate-subscriber now handles needs_approval directly and the orchestrator owns pending approval state, so the old intercept helper and its state-writing tests are dead. The old approval-resume integration is paused until the new pause-in-place integration lands later in the plan. Constraint: Task 26 is responsible for replacing the legacy integration coverage. Rejected: Keep intercept.ts as compatibility shim | no production caller remains and it would preserve obsolete pending-record semantics. Confidence: medium Scope-risk: moderate Directive: Do not reintroduce approval-gate/intercept; route policy::approval_gate pending replies through gate-subscriber directly. Tested: pnpm typecheck in harness-node Tested: pnpm test tests/approval-gate tests/integration/approval-resume.e2e.test.ts * Normalize console backend imports Biome sorted the touched console backend imports after the approval event type cleanup. This keeps the batch verification clean without changing runtime behavior. Constraint: Formatting-only follow-up from console/web verification. Confidence: high Scope-risk: narrow Tested: pnpm typecheck in console/web Tested: pnpm exec biome check src/lib/backend/real.ts src/types/iii-agent-event.ts in console/web * Let approval resolution overwrite the wait record The orchestrator now pauses in a native awaiting-approval state and reads the final decision directly from the state bus, so resolve no longer needs to validate or mutate a pending lifecycle record. It writes the compact decision payload and wakes the turn subscriber once the write succeeds. Constraint: Keep approval::list_pending export until its registration is removed in the later registry cleanup task. Rejected: Preserve pending/status/resolved_at transitions | the paused turn only needs decision and reason, and stale lifecycle fields recreate the old consume path. Confidence: high Scope-risk: narrow Directive: Do not reintroduce preexisting pending-record checks here; the paused turn owns waiting semantics. Tested: pnpm --filter harness-node test tests/approval-gate/pending.test.ts * Retire wake-failed approval events Resolve now writes a decision and publishes the turn wake directly. There is no retry wrapper that converts wake errors into stream events, so keeping approval_wake_failed advertised a lifecycle branch that no longer exists. Constraint: Preserve approval_requested and approval_resolved event envelopes unchanged for existing consumers. Rejected: Keep an unused event helper for compatibility | no caller remains and the event would only describe obsolete resume failure handling. Confidence: high Scope-risk: narrow Directive: Add a new lifecycle event only when a real runtime branch emits it. Tested: rg -n "emitApprovalWakeFailed|approval_wake_failed" harness-node/src harness-node/tests Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/approval-gate * Remove approval placeholder resurrection helpers Approval waits now stay inside the function FSM, so the old placeholder, consume, and resurrection helpers no longer describe the runtime path. Finalize appends real function results directly, awaiting-assistant no longer polls approval::consume, and teardown no longer resurrects terminal turns. Constraint: Keep the paused-turn approval path in function_awaiting_approval; resolved decisions are read from state during function execution resumption. Rejected: Leave consume helpers until a later pass | deleting the helpers without their callers would break typecheck, and keeping them would preserve dead behavior. Confidence: high Scope-risk: moderate Directive: Do not add transcript placeholder replacement back unless function_awaiting_approval is removed or the transcript again stores synthetic pending results. Tested: pnpm --filter harness-node test tests/turn-orchestrator Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node lint * Drop obsolete approval list and consume APIs Approval decisions are now read directly by the paused function state, so the separate list_pending and consume endpoints are dead runtime surface. Removing them keeps the approval gate contract to resolve, sweep-on-abort, and the policy subscriber. Constraint: Keep approval::sweep_session registered until abort cleanup is converted to inline state cleanup. Rejected: Remove sweep in the same pass | router::abort still calls approval::sweep_session and that cleanup belongs to the abort task. Confidence: high Scope-risk: narrow Directive: Do not re-add list_pending or consume for resume; function_awaiting_approval is the resume boundary. Tested: pnpm --filter harness-node test tests/approval-gate tests/harness/policy.test.ts tests/turn-orchestrator/assistant.test.ts tests/turn-orchestrator/tearing-down.test.ts Tested: pnpm --filter harness-node lint && pnpm --filter harness-node typecheck && pnpm --filter harness-node test * Wake turns from approval state writes Approval decisions are now intended to be reactive: the state write is the event that wakes the orchestrator. This adds the adapter that turns an approvals-scope state trigger into the existing turn::step_requested durable publish. Constraint: State trigger payload keys use the approval key shape <session_id>/<function_call_id>. Rejected: Debounce approval decision writes | repeated wakes are harmless and debouncing would add state to a pure adapter. Confidence: high Scope-risk: narrow Directive: Keep this adapter thin; orchestration policy belongs in function_awaiting_approval. Tested: pnpm --filter harness-node test tests/approval-gate/on-decision-written.test.ts * Register approval decision state trigger The approval gate now exposes the on-decision adapter as a state trigger on the approvals scope. Any write to an approval decision key can wake the turn orchestrator without the writer knowing about turn::step_requested. Constraint: Register with scope only and no key filter so every approvals/<session>/<call> decision write is observed. Rejected: Keep wake-up only in approval::resolve | abort and future override writers would still need duplicated publish logic. Confidence: high Scope-risk: narrow Directive: Keep the trigger scope tied to loadApprovalGateConfig so custom approval scopes keep working. Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node test tests/approval-gate * Let approval resolve rely on reactive wakeups The approvals state trigger now owns turn wake-up, so approval::resolve should only persist the decision and emit approval_resolved. Removing resumeSession eliminates the duplicated explicit durable publish from the resolve path. Constraint: approval::on_decision_written is registered on the approvals scope and publishes turn::step_requested after the state write. Rejected: Keep resolve publishing as a fallback | it would double-wake sessions once the state trigger is active and blur responsibility for the wake-up. Confidence: high Scope-risk: narrow Directive: Writers to the approvals scope should not publish turn::step_requested directly; the state trigger is the boundary. Tested: pnpm --filter harness-node test tests/approval-gate/pending.test.ts Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/approval-gate && pnpm --filter harness-node lint * Abort approvals through state decisions The abort path now writes aborted approval decisions directly for turns paused in function_awaiting_approval. Those writes are enough to wake the orchestrator through the approvals state trigger, so abort side effects no longer call sweep_session or publish turn::step_requested. Constraint: This branch did not yet have performAbortSideEffects, so this applies the missing inline cleanup and the reactive no-publish behavior together. Rejected: Keep approval::sweep_session in abort side effects | it preserves the old lifecycle endpoint and bypasses the decision-write trigger boundary. Confidence: high Scope-risk: moderate Directive: Abort approval cleanup should write approvals/<session>/<call> decisions and let the state trigger wake the FSM. Tested: pnpm --filter harness-node test tests/turn-orchestrator/abort.test.ts Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/turn-orchestrator/abort.test.ts && pnpm --filter harness-node lint * Cover reactive approval wake integration The integration fixture now simulates the engine firing a state trigger after approvals-scope writes. A direct decision write proves the adapter publishes turn::step_requested without approval::resolve doing it explicitly. Constraint: The fake state trigger runs on a microtask to match the engine's async trigger behavior closely enough for this unit-level integration test. Rejected: Restore the skipped legacy approval-resume contract | it tested the old explicit resume path rather than the reactive state-trigger boundary. Confidence: high Scope-risk: narrow Directive: Approval resume integration should model approvals state writes as the wake source. Tested: pnpm --filter harness-node test tests/integration/approval-resume.e2e.test.ts Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/integration/approval-resume.e2e.test.ts && pnpm --filter harness-node lint * Remove terminal resume path from turn stepping Approval waits now pause inside function_awaiting_approval and wake through approvals state writes. Terminal-session resurrection via run::resume and buildResumePlan is dead behavior, so turn::step now treats terminal records as terminal and no longer republishes while waiting for approval. Constraint: Keep publishStep for the normal FSM pump after non-terminal transitions. Rejected: Preserve run::resume for compatibility | no active approval path calls it and it would revive the old stopped-session model. Confidence: high Scope-risk: moderate Directive: Do not reintroduce terminal resurrection unless approval waiting leaves the FSM again. Tested: pnpm --filter harness-node test tests/turn-orchestrator Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node lint * Drop approval sweep and timeout remnants The gate no longer owns pending-record lifecycles or timeouts. Abort writes aborted decisions inline and approval decisions wake through the state trigger, so approval::sweep_session, buildPendingRecord, and approval_gate.default_timeout_ms are dead surface. Constraint: Leave unrelated hook-fanout and turn-orchestrator timeout config intact. Rejected: Keep sweep as an internal fallback | no caller remains and it depends on the deleted pending record shape. Confidence: high Scope-risk: narrow Directive: Approval cleanup should write decision records directly; do not restore pending/resolved sweep state. Tested: pnpm --filter harness-node test tests/approval-gate tests/turn-orchestrator/abort.test.ts Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node lint * Remove stale approval placeholder branches The approval wait path no longer emits synthetic pending function results, so console translation does not need to suppress pending_approval function_execution_end events. The hook comment now describes the actual pending dispatch result instead of the removed placeholder flow. Constraint: Preserve approval_requested and approval_resolved UI event translation. Rejected: Keep placeholder suppression defensively | it preserves a deleted wire shape and hides unexpected function results. Confidence: high Scope-risk: narrow Directive: Pending approval UI should be driven by approval_requested, not synthetic function_result placeholders. Tested: pnpm --filter chat-app typecheck Tested: pnpm --filter chat-app exec biome check src/lib/backend/translate.ts Tested: pnpm --filter harness-node test tests/turn-orchestrator/agent-call.test.ts && pnpm --filter harness-node lint * Clean console approval lint drift Repo-wide lint surfaced small console issues around approval resolve handlers and import ordering. Removing non-null assertions and stale suppressions keeps the cleanup branch lintable without changing approval UI behavior. Constraint: Keep existing approval resolve callback contract unchanged. Rejected: Leave console lint failures as unrelated | the approval callback warnings are on this feature surface and are cheap to make safe. Confidence: high Scope-risk: narrow Directive: Keep approval IDs narrowed before invoking resolve callbacks instead of using non-null assertions. Tested: pnpm --filter chat-app lint && pnpm --filter chat-app typecheck Tested: pnpm -r lint * fix(approval-gate): match engine state:* event_type values * test(approval-gate): use engine state:* event_type values * test(integration): fake-iii emits engine-correct event_type for state writes * feat(approval-gate): filter approvals state trigger with condition function Add isDecisionWrite/CONDITION_FN_ID to on-decision-written.ts and wire condition_function_id into the state trigger config in register.ts so the iii engine skips the handler on state:deleted and writes lacking a decision field, without touching the in-handler guard (removed in Task 5). * refactor(approval-gate): handler relies on engine-side condition for filtering * feat(turn-orchestrator): adapter + condition for reactive abort wake * feat(turn-orchestrator): register reactive abort_signal state trigger * test(integration): abort_signal write triggers reactive wake * chore: final cleanup after reactive trigger follow-up Fix biome import-order lint: move `import type { ISdk }` before the value imports in on-abort-signal.test.ts. * Remove obsolete approval event handling and tests This commit deletes the `events.ts` file, which contained the `emitApprovalRequested` and `emitApprovalResolved` functions, as their functionality has been integrated directly into the `gate-subscriber` and `pending` modules. Corresponding test cases in `events.test.ts` have also been removed to reflect this change. The new implementation uses `streamSet` directly for emitting approval events, streamlining the approval gate flow.
ytallo
added a commit
that referenced
this pull request
May 18, 2026
…152) * feat(console): drive model picker from harness-node models-catalog Replace the hardcoded `ModelId` union and four-entry `MODELS` constant with composite `provider::<catalog_model_id>` keys sourced from the harness-node models-catalog worker. - `lib/catalog-model-key.ts`: parse/format `provider::id` (uses `::` separator constant `CATALOG_MODEL_KEY_SEP`). - `lib/models-catalog.ts`: client wrapper around `models::list` / `models::get` for the picker. - `hooks/use-model-picker-source.ts`: live-updating picker source with a static fallback for when the engine is down or the mock backend is in use. - `types/chat.ts`: `ModelId` widened to `string`; `STATIC_MODEL_OPTIONS` + `DEFAULT_MODEL` replace the old enum-shaped `MODELS`. - `components/chat/ModelPicker.tsx`: accept `options` + `loading` props; fall back to `{id, label}` from the current value if the catalog is empty so the picker is never blank. - `components/chat/Composer.tsx`, `components/chat/ChatView.tsx`, `pages/Chat.tsx`, `hooks/use-conversations.ts`, `pages/Examples/sections/*`, `pages/Playground/index.tsx`: thread the catalog-shaped model ids through; legacy heuristic ids without `::` still get the old `claude*` / `gemini*` provider guess in `resolveRunParams`. - `lib/backend/real.ts`: `resolveRunParams` parses `provider::id` and falls back to the heuristic for legacy ids; doc comment updated. * refactor(real): remove debug instrumentation from session event handling This commit removes the debug instrumentation code that was previously used to log session events to a local ingestion endpoint. The removal streamlines the event handling process in the `realStream` function, improving code clarity and maintainability. * feat(approval-gate): port PR #150 with iii-native resume + console wiring Port the fail-closed approval-gate redesign from the Rust workspace to harness-node and finish wiring the console/web UI so chat sessions can approve/deny intercepted tool calls without polling. Backend: - intercept-at-gate: subscriber returns block_reply with status pending|denied; resolved decisions are drained later via approval::consume; pending entries can be force-resolved via approval::sweep_session - run::resume rebuilds terminal TurnStateRecord (preserving turn_count + max_turns) and publishes a fresh turn::step_requested - handle_resolve emits approval_resolved + asks the orchestrator to resume via a single iii::durable::publish on turn::step_requested; no polling loop and no explicit timeout (the bus default is the right backstop) - subscriber detects terminal records and rebuilds via build_resume_plan before stepping - hook-fanout merge_first_block_wins preserves the full blocking reply (status, function_call_id, subscriber, approval_gate, denial) - publish-collect emits publish.{ok,error} + publish_failed so the orchestrator can fail closed on bus unavailability - handle_awaiting + tearing_down each call approval::consume first; on non-empty result they stage the resolved entries as prepared calls and transition (or resurrect) into FunctionExecute - router::abort composes [state::set abort, approval::sweep_session] - session-tree sorts entries by (timestamp, id) so replies always appear in causal order Console/web: - approve/deny buttons call approval::resolve via the iii client - the ui::session::event stream stays subscribed for the chat lifetime (no auto-close on first agent_end) so multi-turn approvals continue to flow - approval prompts auto-scroll into view; submitting state shows disabled buttons + visible feedback while waiting for the agent to resume - fcall-start events deduplicate on functionCallId; pending_approval placeholders suppress their fcall-end until the real result lands Tests: TDD'd helpers, intercept/consume/sweep handlers, resume plumbing, the orchestrator integration points, and an end-to-end approval-resume.e2e covering intercept -> resolve -> publish -> subscriber wake -> rebuild -> consume -> execute. * feat(sessions-poll): implement reactive sessions fanout with state triggers Replaces the polling mechanism for session updates with a reactive approach using state triggers. The new implementation filters state events for session creation and directly pushes updates to subscribers without polling. This change enhances efficiency and reduces latency in session updates, streamlining the fanout process. Key components include condition and handler functions for session creation events, and a direct trigger registration for state changes. * Simplify iii-native approval gate flow (#154) * Make approval pause explicit in the turn state machine The iii-native approval flow needs a non-terminal state where the orchestrator can wait without tearing down the turn or rebuilding from provisioning. This adds the state value and locks its non-terminal behavior in the state test. Constraint: Plan task 1 targets harness-node TypeScript only; Rust workers remain untouched. Rejected: Keep using stopped/resume as the pause marker | later tasks remove that detour. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/state.test.ts Not-tested: Full harness-node suite deferred until the batch verification step * Let the turn record own approval waits The pause-in-place design needs the orchestrator record to carry the function calls that are waiting on human decisions. This adds the awaiting_approval entry shape without changing any behavior yet. Constraint: Field remains optional so fresh and existing records continue to deserialize without migration. Rejected: Store pending calls only in approval-gate state | the simplification moves ownership into the orchestrator FSM. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/state.test.ts Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until the batch verification step * Make prepared function calls extensible for approvals The upcoming pause-in-place approval handler needs to persist per-call metadata without overloading tuple positions. This converts prepared calls to an object shape and adds the optional pre_approved flag while preserving the current execution behavior. Constraint: Existing staged values may use function_call or legacy tool_call keys, so the loader keeps both forms. Rejected: Add a parallel pre-approved lookup table | it would split one prepared-call concern across two state keys. Confidence: medium Scope-risk: moderate Directive: Keep PreparedEntry as the single persisted shape for prepared-call execution metadata. Tested: pnpm test tests/turn-orchestrator/persistence-prepared.test.ts tests/turn-orchestrator/functions.test.ts tests/turn-orchestrator/assistant.test.ts tests/turn-orchestrator/tearing-down.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm test Tested: pnpm typecheck Tested: pnpm lint * Define dispatch outcomes before approval cutover The approval pause cutover needs dispatchWithHook to distinguish successful execution, hard denial, and pending approval. This commit introduces the discriminated result type before changing runtime dispatch behavior. Constraint: Task 4 is type-only; dispatchWithHook still returns FunctionResult until the later refactor task. Rejected: Change dispatch behavior now | the plan keeps the behavioral cutover staged. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/agent-call.test.ts Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until batch verification * Resume approved calls from an explicit wait state The new approval pause state needs a handler that stays idle until every awaited decision is present, then updates prepared calls with either pre_approved metadata or a denial result before returning to function execution. Constraint: Decisions are read from the approvals state scope using the existing session/function-call key shape. Rejected: Poll approval::consume from generic assistant or teardown states | the simplification makes awaiting approval an explicit FSM state. Confidence: medium Scope-risk: moderate Directive: Keep missing decisions as a no-op so durable turn::step_requested wakeups remain idempotent. Tested: pnpm test tests/turn-orchestrator/awaiting-approval.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until batch verification * Route approval waits through the turn transition switch The FSM can now enter function_awaiting_approval, so the central step dispatcher must route that state to the new handler instead of silently doing nothing. Constraint: This only wires the state; handleExecute has not yet been cut over to produce it. Rejected: Depend on fallthrough/no-op behavior | the wait state needs explicit wake handling from turn::step_requested. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/state.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until batch verification * Normalize awaiting-approval files for Biome The new wait-state handler passed behavior and type checks, but Biome required import ordering and line wrapping before the batch could be called clean. Constraint: Formatting-only follow-up after Task 5 and Task 6 verification. Confidence: high Scope-risk: narrow Tested: pnpm test Tested: pnpm typecheck Tested: pnpm lint * Return structured outcomes from hooked dispatch The approval cutover needs dispatchWithHook to report whether a call ran, was denied, or is waiting for human approval. This changes the dispatcher API to return DispatchResult and keeps the LLM-facing agent::call wrapper unwrapped as a FunctionResult. Constraint: handleExecute still consumes the old return shape until the next cutover task. Rejected: Keep pending as a placeholder FunctionResult | the new FSM state owns approval waiting explicitly. Confidence: medium Scope-risk: moderate Directive: Callers must branch on DispatchResult.kind before treating the value as a FunctionResult. Tested: pnpm test tests/turn-orchestrator/agent-call.test.ts tests/turn-orchestrator/functions.test.ts Not-tested: pnpm typecheck fails until handleExecute is cut over in Task 9 * Pause function execution in the approval wait state handleExecute now treats pending approval as a real FSM pause instead of manufacturing a placeholder function result. It also resumes pre-approved calls without re-entering the hook and emits denial results without dispatching blocked calls. Constraint: Old placeholder helper tests are skipped until the later cleanup task removes that machinery. Rejected: Keep pending approval as a terminating FunctionResult | that leaks an artificial result into the transcript and keeps the resume detour alive. Confidence: medium Scope-risk: moderate Directive: Preserve the existing executed-call check before dispatching so retrying a step does not duplicate approved function execution. Tested: pnpm test tests/turn-orchestrator/functions.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm typecheck Tested: pnpm test tests/integration/approval-resume.e2e.test.ts Tested: pnpm test Tested: pnpm lint * Keep approval gate pending state in the orchestrator The gate no longer writes a pending state-bus record when policy asks for human approval. It emits approval_requested and returns a pending hook reply; the turn orchestrator owns the pending-call list on its state record. Constraint: The approvals state bus now stores resolved decisions only. Rejected: Keep handleIntercept in the gate path | it preserves the obsolete pending-record lifecycle and duplicates orchestrator state. Confidence: medium Scope-risk: moderate Directive: Do not add state-bus pending writes back to policy::approval_gate; pending ownership belongs to TurnStateRecord.awaiting_approval. Tested: pnpm test tests/approval-gate/gate-subscriber.test.ts tests/approval-gate/events.test.ts Tested: pnpm typecheck * Drop approval request expiry from event types Approval prompts no longer time out through the gate, so the shared event types should not promise an expires_at timestamp. The console typecheck also needed the currently-unused mode parameter marked intentionally unused. Constraint: No timeout sweep behavior remains in the target approval-request event contract. Rejected: Keep expires_at as optional | it would preserve dead UI expectations around automatic expiry. Confidence: high Scope-risk: narrow Tested: pnpm typecheck in harness-node Tested: pnpm typecheck in console/web * Remove obsolete approval intercept lifecycle The gate-subscriber now handles needs_approval directly and the orchestrator owns pending approval state, so the old intercept helper and its state-writing tests are dead. The old approval-resume integration is paused until the new pause-in-place integration lands later in the plan. Constraint: Task 26 is responsible for replacing the legacy integration coverage. Rejected: Keep intercept.ts as compatibility shim | no production caller remains and it would preserve obsolete pending-record semantics. Confidence: medium Scope-risk: moderate Directive: Do not reintroduce approval-gate/intercept; route policy::approval_gate pending replies through gate-subscriber directly. Tested: pnpm typecheck in harness-node Tested: pnpm test tests/approval-gate tests/integration/approval-resume.e2e.test.ts * Normalize console backend imports Biome sorted the touched console backend imports after the approval event type cleanup. This keeps the batch verification clean without changing runtime behavior. Constraint: Formatting-only follow-up from console/web verification. Confidence: high Scope-risk: narrow Tested: pnpm typecheck in console/web Tested: pnpm exec biome check src/lib/backend/real.ts src/types/iii-agent-event.ts in console/web * Let approval resolution overwrite the wait record The orchestrator now pauses in a native awaiting-approval state and reads the final decision directly from the state bus, so resolve no longer needs to validate or mutate a pending lifecycle record. It writes the compact decision payload and wakes the turn subscriber once the write succeeds. Constraint: Keep approval::list_pending export until its registration is removed in the later registry cleanup task. Rejected: Preserve pending/status/resolved_at transitions | the paused turn only needs decision and reason, and stale lifecycle fields recreate the old consume path. Confidence: high Scope-risk: narrow Directive: Do not reintroduce preexisting pending-record checks here; the paused turn owns waiting semantics. Tested: pnpm --filter harness-node test tests/approval-gate/pending.test.ts * Retire wake-failed approval events Resolve now writes a decision and publishes the turn wake directly. There is no retry wrapper that converts wake errors into stream events, so keeping approval_wake_failed advertised a lifecycle branch that no longer exists. Constraint: Preserve approval_requested and approval_resolved event envelopes unchanged for existing consumers. Rejected: Keep an unused event helper for compatibility | no caller remains and the event would only describe obsolete resume failure handling. Confidence: high Scope-risk: narrow Directive: Add a new lifecycle event only when a real runtime branch emits it. Tested: rg -n "emitApprovalWakeFailed|approval_wake_failed" harness-node/src harness-node/tests Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/approval-gate * Remove approval placeholder resurrection helpers Approval waits now stay inside the function FSM, so the old placeholder, consume, and resurrection helpers no longer describe the runtime path. Finalize appends real function results directly, awaiting-assistant no longer polls approval::consume, and teardown no longer resurrects terminal turns. Constraint: Keep the paused-turn approval path in function_awaiting_approval; resolved decisions are read from state during function execution resumption. Rejected: Leave consume helpers until a later pass | deleting the helpers without their callers would break typecheck, and keeping them would preserve dead behavior. Confidence: high Scope-risk: moderate Directive: Do not add transcript placeholder replacement back unless function_awaiting_approval is removed or the transcript again stores synthetic pending results. Tested: pnpm --filter harness-node test tests/turn-orchestrator Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node lint * Drop obsolete approval list and consume APIs Approval decisions are now read directly by the paused function state, so the separate list_pending and consume endpoints are dead runtime surface. Removing them keeps the approval gate contract to resolve, sweep-on-abort, and the policy subscriber. Constraint: Keep approval::sweep_session registered until abort cleanup is converted to inline state cleanup. Rejected: Remove sweep in the same pass | router::abort still calls approval::sweep_session and that cleanup belongs to the abort task. Confidence: high Scope-risk: narrow Directive: Do not re-add list_pending or consume for resume; function_awaiting_approval is the resume boundary. Tested: pnpm --filter harness-node test tests/approval-gate tests/harness/policy.test.ts tests/turn-orchestrator/assistant.test.ts tests/turn-orchestrator/tearing-down.test.ts Tested: pnpm --filter harness-node lint && pnpm --filter harness-node typecheck && pnpm --filter harness-node test * Wake turns from approval state writes Approval decisions are now intended to be reactive: the state write is the event that wakes the orchestrator. This adds the adapter that turns an approvals-scope state trigger into the existing turn::step_requested durable publish. Constraint: State trigger payload keys use the approval key shape <session_id>/<function_call_id>. Rejected: Debounce approval decision writes | repeated wakes are harmless and debouncing would add state to a pure adapter. Confidence: high Scope-risk: narrow Directive: Keep this adapter thin; orchestration policy belongs in function_awaiting_approval. Tested: pnpm --filter harness-node test tests/approval-gate/on-decision-written.test.ts * Register approval decision state trigger The approval gate now exposes the on-decision adapter as a state trigger on the approvals scope. Any write to an approval decision key can wake the turn orchestrator without the writer knowing about turn::step_requested. Constraint: Register with scope only and no key filter so every approvals/<session>/<call> decision write is observed. Rejected: Keep wake-up only in approval::resolve | abort and future override writers would still need duplicated publish logic. Confidence: high Scope-risk: narrow Directive: Keep the trigger scope tied to loadApprovalGateConfig so custom approval scopes keep working. Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node test tests/approval-gate * Let approval resolve rely on reactive wakeups The approvals state trigger now owns turn wake-up, so approval::resolve should only persist the decision and emit approval_resolved. Removing resumeSession eliminates the duplicated explicit durable publish from the resolve path. Constraint: approval::on_decision_written is registered on the approvals scope and publishes turn::step_requested after the state write. Rejected: Keep resolve publishing as a fallback | it would double-wake sessions once the state trigger is active and blur responsibility for the wake-up. Confidence: high Scope-risk: narrow Directive: Writers to the approvals scope should not publish turn::step_requested directly; the state trigger is the boundary. Tested: pnpm --filter harness-node test tests/approval-gate/pending.test.ts Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/approval-gate && pnpm --filter harness-node lint * Abort approvals through state decisions The abort path now writes aborted approval decisions directly for turns paused in function_awaiting_approval. Those writes are enough to wake the orchestrator through the approvals state trigger, so abort side effects no longer call sweep_session or publish turn::step_requested. Constraint: This branch did not yet have performAbortSideEffects, so this applies the missing inline cleanup and the reactive no-publish behavior together. Rejected: Keep approval::sweep_session in abort side effects | it preserves the old lifecycle endpoint and bypasses the decision-write trigger boundary. Confidence: high Scope-risk: moderate Directive: Abort approval cleanup should write approvals/<session>/<call> decisions and let the state trigger wake the FSM. Tested: pnpm --filter harness-node test tests/turn-orchestrator/abort.test.ts Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/turn-orchestrator/abort.test.ts && pnpm --filter harness-node lint * Cover reactive approval wake integration The integration fixture now simulates the engine firing a state trigger after approvals-scope writes. A direct decision write proves the adapter publishes turn::step_requested without approval::resolve doing it explicitly. Constraint: The fake state trigger runs on a microtask to match the engine's async trigger behavior closely enough for this unit-level integration test. Rejected: Restore the skipped legacy approval-resume contract | it tested the old explicit resume path rather than the reactive state-trigger boundary. Confidence: high Scope-risk: narrow Directive: Approval resume integration should model approvals state writes as the wake source. Tested: pnpm --filter harness-node test tests/integration/approval-resume.e2e.test.ts Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/integration/approval-resume.e2e.test.ts && pnpm --filter harness-node lint * Remove terminal resume path from turn stepping Approval waits now pause inside function_awaiting_approval and wake through approvals state writes. Terminal-session resurrection via run::resume and buildResumePlan is dead behavior, so turn::step now treats terminal records as terminal and no longer republishes while waiting for approval. Constraint: Keep publishStep for the normal FSM pump after non-terminal transitions. Rejected: Preserve run::resume for compatibility | no active approval path calls it and it would revive the old stopped-session model. Confidence: high Scope-risk: moderate Directive: Do not reintroduce terminal resurrection unless approval waiting leaves the FSM again. Tested: pnpm --filter harness-node test tests/turn-orchestrator Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node lint * Drop approval sweep and timeout remnants The gate no longer owns pending-record lifecycles or timeouts. Abort writes aborted decisions inline and approval decisions wake through the state trigger, so approval::sweep_session, buildPendingRecord, and approval_gate.default_timeout_ms are dead surface. Constraint: Leave unrelated hook-fanout and turn-orchestrator timeout config intact. Rejected: Keep sweep as an internal fallback | no caller remains and it depends on the deleted pending record shape. Confidence: high Scope-risk: narrow Directive: Approval cleanup should write decision records directly; do not restore pending/resolved sweep state. Tested: pnpm --filter harness-node test tests/approval-gate tests/turn-orchestrator/abort.test.ts Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node lint * Remove stale approval placeholder branches The approval wait path no longer emits synthetic pending function results, so console translation does not need to suppress pending_approval function_execution_end events. The hook comment now describes the actual pending dispatch result instead of the removed placeholder flow. Constraint: Preserve approval_requested and approval_resolved UI event translation. Rejected: Keep placeholder suppression defensively | it preserves a deleted wire shape and hides unexpected function results. Confidence: high Scope-risk: narrow Directive: Pending approval UI should be driven by approval_requested, not synthetic function_result placeholders. Tested: pnpm --filter chat-app typecheck Tested: pnpm --filter chat-app exec biome check src/lib/backend/translate.ts Tested: pnpm --filter harness-node test tests/turn-orchestrator/agent-call.test.ts && pnpm --filter harness-node lint * Clean console approval lint drift Repo-wide lint surfaced small console issues around approval resolve handlers and import ordering. Removing non-null assertions and stale suppressions keeps the cleanup branch lintable without changing approval UI behavior. Constraint: Keep existing approval resolve callback contract unchanged. Rejected: Leave console lint failures as unrelated | the approval callback warnings are on this feature surface and are cheap to make safe. Confidence: high Scope-risk: narrow Directive: Keep approval IDs narrowed before invoking resolve callbacks instead of using non-null assertions. Tested: pnpm --filter chat-app lint && pnpm --filter chat-app typecheck Tested: pnpm -r lint * fix(approval-gate): match engine state:* event_type values * test(approval-gate): use engine state:* event_type values * test(integration): fake-iii emits engine-correct event_type for state writes * feat(approval-gate): filter approvals state trigger with condition function Add isDecisionWrite/CONDITION_FN_ID to on-decision-written.ts and wire condition_function_id into the state trigger config in register.ts so the iii engine skips the handler on state:deleted and writes lacking a decision field, without touching the in-handler guard (removed in Task 5). * refactor(approval-gate): handler relies on engine-side condition for filtering * feat(turn-orchestrator): adapter + condition for reactive abort wake * feat(turn-orchestrator): register reactive abort_signal state trigger * test(integration): abort_signal write triggers reactive wake * chore: final cleanup after reactive trigger follow-up Fix biome import-order lint: move `import type { ISdk }` before the value imports in on-abort-signal.test.ts. * Remove obsolete approval event handling and tests This commit deletes the `events.ts` file, which contained the `emitApprovalRequested` and `emitApprovalResolved` functions, as their functionality has been integrated directly into the `gate-subscriber` and `pending` modules. Corresponding test cases in `events.test.ts` have also been removed to reflect this change. The new implementation uses `streamSet` directly for emitting approval events, streamlining the approval gate flow.
ytallo
added a commit
that referenced
this pull request
May 18, 2026
…156) * feat(approval-gate): iii-native pause-and-resume with reactive wakes (#152) * feat(console): drive model picker from harness-node models-catalog Replace the hardcoded `ModelId` union and four-entry `MODELS` constant with composite `provider::<catalog_model_id>` keys sourced from the harness-node models-catalog worker. - `lib/catalog-model-key.ts`: parse/format `provider::id` (uses `::` separator constant `CATALOG_MODEL_KEY_SEP`). - `lib/models-catalog.ts`: client wrapper around `models::list` / `models::get` for the picker. - `hooks/use-model-picker-source.ts`: live-updating picker source with a static fallback for when the engine is down or the mock backend is in use. - `types/chat.ts`: `ModelId` widened to `string`; `STATIC_MODEL_OPTIONS` + `DEFAULT_MODEL` replace the old enum-shaped `MODELS`. - `components/chat/ModelPicker.tsx`: accept `options` + `loading` props; fall back to `{id, label}` from the current value if the catalog is empty so the picker is never blank. - `components/chat/Composer.tsx`, `components/chat/ChatView.tsx`, `pages/Chat.tsx`, `hooks/use-conversations.ts`, `pages/Examples/sections/*`, `pages/Playground/index.tsx`: thread the catalog-shaped model ids through; legacy heuristic ids without `::` still get the old `claude*` / `gemini*` provider guess in `resolveRunParams`. - `lib/backend/real.ts`: `resolveRunParams` parses `provider::id` and falls back to the heuristic for legacy ids; doc comment updated. * refactor(real): remove debug instrumentation from session event handling This commit removes the debug instrumentation code that was previously used to log session events to a local ingestion endpoint. The removal streamlines the event handling process in the `realStream` function, improving code clarity and maintainability. * feat(approval-gate): port PR #150 with iii-native resume + console wiring Port the fail-closed approval-gate redesign from the Rust workspace to harness-node and finish wiring the console/web UI so chat sessions can approve/deny intercepted tool calls without polling. Backend: - intercept-at-gate: subscriber returns block_reply with status pending|denied; resolved decisions are drained later via approval::consume; pending entries can be force-resolved via approval::sweep_session - run::resume rebuilds terminal TurnStateRecord (preserving turn_count + max_turns) and publishes a fresh turn::step_requested - handle_resolve emits approval_resolved + asks the orchestrator to resume via a single iii::durable::publish on turn::step_requested; no polling loop and no explicit timeout (the bus default is the right backstop) - subscriber detects terminal records and rebuilds via build_resume_plan before stepping - hook-fanout merge_first_block_wins preserves the full blocking reply (status, function_call_id, subscriber, approval_gate, denial) - publish-collect emits publish.{ok,error} + publish_failed so the orchestrator can fail closed on bus unavailability - handle_awaiting + tearing_down each call approval::consume first; on non-empty result they stage the resolved entries as prepared calls and transition (or resurrect) into FunctionExecute - router::abort composes [state::set abort, approval::sweep_session] - session-tree sorts entries by (timestamp, id) so replies always appear in causal order Console/web: - approve/deny buttons call approval::resolve via the iii client - the ui::session::event stream stays subscribed for the chat lifetime (no auto-close on first agent_end) so multi-turn approvals continue to flow - approval prompts auto-scroll into view; submitting state shows disabled buttons + visible feedback while waiting for the agent to resume - fcall-start events deduplicate on functionCallId; pending_approval placeholders suppress their fcall-end until the real result lands Tests: TDD'd helpers, intercept/consume/sweep handlers, resume plumbing, the orchestrator integration points, and an end-to-end approval-resume.e2e covering intercept -> resolve -> publish -> subscriber wake -> rebuild -> consume -> execute. * feat(sessions-poll): implement reactive sessions fanout with state triggers Replaces the polling mechanism for session updates with a reactive approach using state triggers. The new implementation filters state events for session creation and directly pushes updates to subscribers without polling. This change enhances efficiency and reduces latency in session updates, streamlining the fanout process. Key components include condition and handler functions for session creation events, and a direct trigger registration for state changes. * Simplify iii-native approval gate flow (#154) * Make approval pause explicit in the turn state machine The iii-native approval flow needs a non-terminal state where the orchestrator can wait without tearing down the turn or rebuilding from provisioning. This adds the state value and locks its non-terminal behavior in the state test. Constraint: Plan task 1 targets harness-node TypeScript only; Rust workers remain untouched. Rejected: Keep using stopped/resume as the pause marker | later tasks remove that detour. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/state.test.ts Not-tested: Full harness-node suite deferred until the batch verification step * Let the turn record own approval waits The pause-in-place design needs the orchestrator record to carry the function calls that are waiting on human decisions. This adds the awaiting_approval entry shape without changing any behavior yet. Constraint: Field remains optional so fresh and existing records continue to deserialize without migration. Rejected: Store pending calls only in approval-gate state | the simplification moves ownership into the orchestrator FSM. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/state.test.ts Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until the batch verification step * Make prepared function calls extensible for approvals The upcoming pause-in-place approval handler needs to persist per-call metadata without overloading tuple positions. This converts prepared calls to an object shape and adds the optional pre_approved flag while preserving the current execution behavior. Constraint: Existing staged values may use function_call or legacy tool_call keys, so the loader keeps both forms. Rejected: Add a parallel pre-approved lookup table | it would split one prepared-call concern across two state keys. Confidence: medium Scope-risk: moderate Directive: Keep PreparedEntry as the single persisted shape for prepared-call execution metadata. Tested: pnpm test tests/turn-orchestrator/persistence-prepared.test.ts tests/turn-orchestrator/functions.test.ts tests/turn-orchestrator/assistant.test.ts tests/turn-orchestrator/tearing-down.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm test Tested: pnpm typecheck Tested: pnpm lint * Define dispatch outcomes before approval cutover The approval pause cutover needs dispatchWithHook to distinguish successful execution, hard denial, and pending approval. This commit introduces the discriminated result type before changing runtime dispatch behavior. Constraint: Task 4 is type-only; dispatchWithHook still returns FunctionResult until the later refactor task. Rejected: Change dispatch behavior now | the plan keeps the behavioral cutover staged. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/agent-call.test.ts Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until batch verification * Resume approved calls from an explicit wait state The new approval pause state needs a handler that stays idle until every awaited decision is present, then updates prepared calls with either pre_approved metadata or a denial result before returning to function execution. Constraint: Decisions are read from the approvals state scope using the existing session/function-call key shape. Rejected: Poll approval::consume from generic assistant or teardown states | the simplification makes awaiting approval an explicit FSM state. Confidence: medium Scope-risk: moderate Directive: Keep missing decisions as a no-op so durable turn::step_requested wakeups remain idempotent. Tested: pnpm test tests/turn-orchestrator/awaiting-approval.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until batch verification * Route approval waits through the turn transition switch The FSM can now enter function_awaiting_approval, so the central step dispatcher must route that state to the new handler instead of silently doing nothing. Constraint: This only wires the state; handleExecute has not yet been cut over to produce it. Rejected: Depend on fallthrough/no-op behavior | the wait state needs explicit wake handling from turn::step_requested. Confidence: high Scope-risk: narrow Tested: pnpm test tests/turn-orchestrator/state.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm typecheck Not-tested: Full harness-node suite deferred until batch verification * Normalize awaiting-approval files for Biome The new wait-state handler passed behavior and type checks, but Biome required import ordering and line wrapping before the batch could be called clean. Constraint: Formatting-only follow-up after Task 5 and Task 6 verification. Confidence: high Scope-risk: narrow Tested: pnpm test Tested: pnpm typecheck Tested: pnpm lint * Return structured outcomes from hooked dispatch The approval cutover needs dispatchWithHook to report whether a call ran, was denied, or is waiting for human approval. This changes the dispatcher API to return DispatchResult and keeps the LLM-facing agent::call wrapper unwrapped as a FunctionResult. Constraint: handleExecute still consumes the old return shape until the next cutover task. Rejected: Keep pending as a placeholder FunctionResult | the new FSM state owns approval waiting explicitly. Confidence: medium Scope-risk: moderate Directive: Callers must branch on DispatchResult.kind before treating the value as a FunctionResult. Tested: pnpm test tests/turn-orchestrator/agent-call.test.ts tests/turn-orchestrator/functions.test.ts Not-tested: pnpm typecheck fails until handleExecute is cut over in Task 9 * Pause function execution in the approval wait state handleExecute now treats pending approval as a real FSM pause instead of manufacturing a placeholder function result. It also resumes pre-approved calls without re-entering the hook and emits denial results without dispatching blocked calls. Constraint: Old placeholder helper tests are skipped until the later cleanup task removes that machinery. Rejected: Keep pending approval as a terminating FunctionResult | that leaks an artificial result into the transcript and keeps the resume detour alive. Confidence: medium Scope-risk: moderate Directive: Preserve the existing executed-call check before dispatching so retrying a step does not duplicate approved function execution. Tested: pnpm test tests/turn-orchestrator/functions.test.ts Tested: pnpm test tests/turn-orchestrator Tested: pnpm typecheck Tested: pnpm test tests/integration/approval-resume.e2e.test.ts Tested: pnpm test Tested: pnpm lint * Keep approval gate pending state in the orchestrator The gate no longer writes a pending state-bus record when policy asks for human approval. It emits approval_requested and returns a pending hook reply; the turn orchestrator owns the pending-call list on its state record. Constraint: The approvals state bus now stores resolved decisions only. Rejected: Keep handleIntercept in the gate path | it preserves the obsolete pending-record lifecycle and duplicates orchestrator state. Confidence: medium Scope-risk: moderate Directive: Do not add state-bus pending writes back to policy::approval_gate; pending ownership belongs to TurnStateRecord.awaiting_approval. Tested: pnpm test tests/approval-gate/gate-subscriber.test.ts tests/approval-gate/events.test.ts Tested: pnpm typecheck * Drop approval request expiry from event types Approval prompts no longer time out through the gate, so the shared event types should not promise an expires_at timestamp. The console typecheck also needed the currently-unused mode parameter marked intentionally unused. Constraint: No timeout sweep behavior remains in the target approval-request event contract. Rejected: Keep expires_at as optional | it would preserve dead UI expectations around automatic expiry. Confidence: high Scope-risk: narrow Tested: pnpm typecheck in harness-node Tested: pnpm typecheck in console/web * Remove obsolete approval intercept lifecycle The gate-subscriber now handles needs_approval directly and the orchestrator owns pending approval state, so the old intercept helper and its state-writing tests are dead. The old approval-resume integration is paused until the new pause-in-place integration lands later in the plan. Constraint: Task 26 is responsible for replacing the legacy integration coverage. Rejected: Keep intercept.ts as compatibility shim | no production caller remains and it would preserve obsolete pending-record semantics. Confidence: medium Scope-risk: moderate Directive: Do not reintroduce approval-gate/intercept; route policy::approval_gate pending replies through gate-subscriber directly. Tested: pnpm typecheck in harness-node Tested: pnpm test tests/approval-gate tests/integration/approval-resume.e2e.test.ts * Normalize console backend imports Biome sorted the touched console backend imports after the approval event type cleanup. This keeps the batch verification clean without changing runtime behavior. Constraint: Formatting-only follow-up from console/web verification. Confidence: high Scope-risk: narrow Tested: pnpm typecheck in console/web Tested: pnpm exec biome check src/lib/backend/real.ts src/types/iii-agent-event.ts in console/web * Let approval resolution overwrite the wait record The orchestrator now pauses in a native awaiting-approval state and reads the final decision directly from the state bus, so resolve no longer needs to validate or mutate a pending lifecycle record. It writes the compact decision payload and wakes the turn subscriber once the write succeeds. Constraint: Keep approval::list_pending export until its registration is removed in the later registry cleanup task. Rejected: Preserve pending/status/resolved_at transitions | the paused turn only needs decision and reason, and stale lifecycle fields recreate the old consume path. Confidence: high Scope-risk: narrow Directive: Do not reintroduce preexisting pending-record checks here; the paused turn owns waiting semantics. Tested: pnpm --filter harness-node test tests/approval-gate/pending.test.ts * Retire wake-failed approval events Resolve now writes a decision and publishes the turn wake directly. There is no retry wrapper that converts wake errors into stream events, so keeping approval_wake_failed advertised a lifecycle branch that no longer exists. Constraint: Preserve approval_requested and approval_resolved event envelopes unchanged for existing consumers. Rejected: Keep an unused event helper for compatibility | no caller remains and the event would only describe obsolete resume failure handling. Confidence: high Scope-risk: narrow Directive: Add a new lifecycle event only when a real runtime branch emits it. Tested: rg -n "emitApprovalWakeFailed|approval_wake_failed" harness-node/src harness-node/tests Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/approval-gate * Remove approval placeholder resurrection helpers Approval waits now stay inside the function FSM, so the old placeholder, consume, and resurrection helpers no longer describe the runtime path. Finalize appends real function results directly, awaiting-assistant no longer polls approval::consume, and teardown no longer resurrects terminal turns. Constraint: Keep the paused-turn approval path in function_awaiting_approval; resolved decisions are read from state during function execution resumption. Rejected: Leave consume helpers until a later pass | deleting the helpers without their callers would break typecheck, and keeping them would preserve dead behavior. Confidence: high Scope-risk: moderate Directive: Do not add transcript placeholder replacement back unless function_awaiting_approval is removed or the transcript again stores synthetic pending results. Tested: pnpm --filter harness-node test tests/turn-orchestrator Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node lint * Drop obsolete approval list and consume APIs Approval decisions are now read directly by the paused function state, so the separate list_pending and consume endpoints are dead runtime surface. Removing them keeps the approval gate contract to resolve, sweep-on-abort, and the policy subscriber. Constraint: Keep approval::sweep_session registered until abort cleanup is converted to inline state cleanup. Rejected: Remove sweep in the same pass | router::abort still calls approval::sweep_session and that cleanup belongs to the abort task. Confidence: high Scope-risk: narrow Directive: Do not re-add list_pending or consume for resume; function_awaiting_approval is the resume boundary. Tested: pnpm --filter harness-node test tests/approval-gate tests/harness/policy.test.ts tests/turn-orchestrator/assistant.test.ts tests/turn-orchestrator/tearing-down.test.ts Tested: pnpm --filter harness-node lint && pnpm --filter harness-node typecheck && pnpm --filter harness-node test * Wake turns from approval state writes Approval decisions are now intended to be reactive: the state write is the event that wakes the orchestrator. This adds the adapter that turns an approvals-scope state trigger into the existing turn::step_requested durable publish. Constraint: State trigger payload keys use the approval key shape <session_id>/<function_call_id>. Rejected: Debounce approval decision writes | repeated wakes are harmless and debouncing would add state to a pure adapter. Confidence: high Scope-risk: narrow Directive: Keep this adapter thin; orchestration policy belongs in function_awaiting_approval. Tested: pnpm --filter harness-node test tests/approval-gate/on-decision-written.test.ts * Register approval decision state trigger The approval gate now exposes the on-decision adapter as a state trigger on the approvals scope. Any write to an approval decision key can wake the turn orchestrator without the writer knowing about turn::step_requested. Constraint: Register with scope only and no key filter so every approvals/<session>/<call> decision write is observed. Rejected: Keep wake-up only in approval::resolve | abort and future override writers would still need duplicated publish logic. Confidence: high Scope-risk: narrow Directive: Keep the trigger scope tied to loadApprovalGateConfig so custom approval scopes keep working. Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node test tests/approval-gate * Let approval resolve rely on reactive wakeups The approvals state trigger now owns turn wake-up, so approval::resolve should only persist the decision and emit approval_resolved. Removing resumeSession eliminates the duplicated explicit durable publish from the resolve path. Constraint: approval::on_decision_written is registered on the approvals scope and publishes turn::step_requested after the state write. Rejected: Keep resolve publishing as a fallback | it would double-wake sessions once the state trigger is active and blur responsibility for the wake-up. Confidence: high Scope-risk: narrow Directive: Writers to the approvals scope should not publish turn::step_requested directly; the state trigger is the boundary. Tested: pnpm --filter harness-node test tests/approval-gate/pending.test.ts Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/approval-gate && pnpm --filter harness-node lint * Abort approvals through state decisions The abort path now writes aborted approval decisions directly for turns paused in function_awaiting_approval. Those writes are enough to wake the orchestrator through the approvals state trigger, so abort side effects no longer call sweep_session or publish turn::step_requested. Constraint: This branch did not yet have performAbortSideEffects, so this applies the missing inline cleanup and the reactive no-publish behavior together. Rejected: Keep approval::sweep_session in abort side effects | it preserves the old lifecycle endpoint and bypasses the decision-write trigger boundary. Confidence: high Scope-risk: moderate Directive: Abort approval cleanup should write approvals/<session>/<call> decisions and let the state trigger wake the FSM. Tested: pnpm --filter harness-node test tests/turn-orchestrator/abort.test.ts Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/turn-orchestrator/abort.test.ts && pnpm --filter harness-node lint * Cover reactive approval wake integration The integration fixture now simulates the engine firing a state trigger after approvals-scope writes. A direct decision write proves the adapter publishes turn::step_requested without approval::resolve doing it explicitly. Constraint: The fake state trigger runs on a microtask to match the engine's async trigger behavior closely enough for this unit-level integration test. Rejected: Restore the skipped legacy approval-resume contract | it tested the old explicit resume path rather than the reactive state-trigger boundary. Confidence: high Scope-risk: narrow Directive: Approval resume integration should model approvals state writes as the wake source. Tested: pnpm --filter harness-node test tests/integration/approval-resume.e2e.test.ts Tested: pnpm --filter harness-node typecheck && pnpm --filter harness-node test tests/integration/approval-resume.e2e.test.ts && pnpm --filter harness-node lint * Remove terminal resume path from turn stepping Approval waits now pause inside function_awaiting_approval and wake through approvals state writes. Terminal-session resurrection via run::resume and buildResumePlan is dead behavior, so turn::step now treats terminal records as terminal and no longer republishes while waiting for approval. Constraint: Keep publishStep for the normal FSM pump after non-terminal transitions. Rejected: Preserve run::resume for compatibility | no active approval path calls it and it would revive the old stopped-session model. Confidence: high Scope-risk: moderate Directive: Do not reintroduce terminal resurrection unless approval waiting leaves the FSM again. Tested: pnpm --filter harness-node test tests/turn-orchestrator Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node lint * Drop approval sweep and timeout remnants The gate no longer owns pending-record lifecycles or timeouts. Abort writes aborted decisions inline and approval decisions wake through the state trigger, so approval::sweep_session, buildPendingRecord, and approval_gate.default_timeout_ms are dead surface. Constraint: Leave unrelated hook-fanout and turn-orchestrator timeout config intact. Rejected: Keep sweep as an internal fallback | no caller remains and it depends on the deleted pending record shape. Confidence: high Scope-risk: narrow Directive: Approval cleanup should write decision records directly; do not restore pending/resolved sweep state. Tested: pnpm --filter harness-node test tests/approval-gate tests/turn-orchestrator/abort.test.ts Tested: pnpm --filter harness-node typecheck Tested: pnpm --filter harness-node lint * Remove stale approval placeholder branches The approval wait path no longer emits synthetic pending function results, so console translation does not need to suppress pending_approval function_execution_end events. The hook comment now describes the actual pending dispatch result instead of the removed placeholder flow. Constraint: Preserve approval_requested and approval_resolved UI event translation. Rejected: Keep placeholder suppression defensively | it preserves a deleted wire shape and hides unexpected function results. Confidence: high Scope-risk: narrow Directive: Pending approval UI should be driven by approval_requested, not synthetic function_result placeholders. Tested: pnpm --filter chat-app typecheck Tested: pnpm --filter chat-app exec biome check src/lib/backend/translate.ts Tested: pnpm --filter harness-node test tests/turn-orchestrator/agent-call.test.ts && pnpm --filter harness-node lint * Clean console approval lint drift Repo-wide lint surfaced small console issues around approval resolve handlers and import ordering. Removing non-null assertions and stale suppressions keeps the cleanup branch lintable without changing approval UI behavior. Constraint: Keep existing approval resolve callback contract unchanged. Rejected: Leave console lint failures as unrelated | the approval callback warnings are on this feature surface and are cheap to make safe. Confidence: high Scope-risk: narrow Directive: Keep approval IDs narrowed before invoking resolve callbacks instead of using non-null assertions. Tested: pnpm --filter chat-app lint && pnpm --filter chat-app typecheck Tested: pnpm -r lint * fix(approval-gate): match engine state:* event_type values * test(approval-gate): use engine state:* event_type values * test(integration): fake-iii emits engine-correct event_type for state writes * feat(approval-gate): filter approvals state trigger with condition function Add isDecisionWrite/CONDITION_FN_ID to on-decision-written.ts and wire condition_function_id into the state trigger config in register.ts so the iii engine skips the handler on state:deleted and writes lacking a decision field, without touching the in-handler guard (removed in Task 5). * refactor(approval-gate): handler relies on engine-side condition for filtering * feat(turn-orchestrator): adapter + condition for reactive abort wake * feat(turn-orchestrator): register reactive abort_signal state trigger * test(integration): abort_signal write triggers reactive wake * chore: final cleanup after reactive trigger follow-up Fix biome import-order lint: move `import type { ISdk }` before the value imports in on-abort-signal.test.ts. * Remove obsolete approval event handling and tests This commit deletes the `events.ts` file, which contained the `emitApprovalRequested` and `emitApprovalResolved` functions, as their functionality has been integrated directly into the `gate-subscriber` and `pending` modules. Corresponding test cases in `events.test.ts` have also been removed to reflect this change. The new implementation uses `streamSet` directly for emitting approval events, streamlining the approval gate flow. * docs(harness-node): refresh architecture for reactive approval resume Sync the harness-node docs introduced on main with this branch's pause-and-resume refactor: 11-state FSM with function_awaiting_approval, state-trigger wakes on the approvals and agent scopes, hook-fanout's stream-trigger reply path, and the harness sessions fanout moving from a 1 Hz state::list diff to a state trigger. Also drop poll_interval_ms from PublishCollectConfig — it's leftover from the old polling implementation and unused in the reactive path. * feat(turn-orchestrator): reactive run::start_and_wait via terminal state trigger executeSync now installs an in-process waiter keyed by session_id, kicks the run, and races the waiter against sync_default_timeout_ms instead of polling persistence every sync_poll_interval_ms. The waiter is resolved by a new `state` trigger on scope=agent gated by turn::is_terminal_state_write, which fires the moment the FSM lands `session/<sid>/turn_state` with state=stopped. Drops sync_poll_interval_ms from the config type, default loader, and config.yaml — the only consumer is gone. * feat(console): wire real backend to approval::resolve - Add realResolveApproval calling approval::resolve over the iii bus and expose it via ChatBackend.resolveApproval so the chat UI can approve or deny pending function calls. - Thread sessionId into translateAgentEvent so approval-related stream events carry their session context. - Drop the stale "Phase 3 adds approve/deny buttons" comment now that the flow is live end-to-end.
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
ApprovalRow+ reducer wired for approval state, with unit/E2E coverage (ApprovalRow.test.tsx,reducer.test.ts,tests/e2e/approval.spec.ts).Test plan
cargo testinapproval-gate,hook-fanout,turn-orchestrator,provider-router,sessionnpm test -- ApprovalRow.test.tsx reducer.test.tsinharness/webnpm run buildinharness/webnpm run e2e -- tests/e2e/approval.spec.tscargo test(inbox integration end-to-end) on CISummary by CodeRabbit
New Features
Bug Fixes
Tests