feat(agent): signal-based HITL approval gate for client-side tools#112
Closed
runyaga wants to merge 3 commits intosoliplex:mainfrom
Closed
feat(agent): signal-based HITL approval gate for client-side tools#112runyaga wants to merge 3 commits intosoliplex:mainfrom
runyaga wants to merge 3 commits intosoliplex:mainfrom
Conversation
Replace AgentUiDelegate (callback-based, single-tool) with a signal-driven Human-in-the-Loop gate that scales to concurrent tool calls and integrates cleanly with the signals reactive layer. ## What changed **ClientTool API** (`tool_registry.dart`): - `requiresApproval: bool` — when true, AgentSession suspends execution before the tool executor runs and emits a PendingApprovalRequest; the UI must call approveToolCall or denyToolCall to resume. - `platformConsentNote: String? Function()?` — optional callback for tools that trigger an OS-level permission dialog (e.g. clipboard read on web). Returns a human-readable note; AgentSession emits PlatformConsentNotice (non-blocking, informational). **New types**: - `PendingApprovalRequest` — immutable data class (toolCallId, toolName, arguments) emitted on pendingApproval signal. - `PlatformConsentNotice` / `AwaitingApproval` — ExecutionEvent subclasses for consent/approval lifecycle. **AgentSession** (`agent_session.dart`): - `pendingApproval: ReadonlySignal<PendingApprovalRequest?>` — UI watches this to render Allow/Deny UI. - `approveToolCall(String) / denyToolCall(String)` — resolves the Completer gating the suspended tool call. - `_awaitApproval()` — internal gate; stores Completer per toolCallId in _pendingApprovals, signals UI, awaits resolution. Auto-denies on session cancel. **Deleted**: - `AgentUiDelegate` (58 lines) + its 457-line test file. Replaced entirely by the signal approach above. **session_extension.dart**: `onDispose()` → `dispose()` rename for consistency with Dart disposal conventions. **Tests** (`hitl_test.dart`): 12 tests covering requiresApproval defaults, three approval tiers (agent-gate / OS-gate / ungated), PendingApprovalRequest fields, platformConsentNote callbacks, and PlatformConsentNotice equality. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The HITL commit renamed SessionExtension.onDispose() to dispose() but left ScriptEnvironmentExtension and its tests pointing at the old name. Three targeted fixes to compile cleanly without the scripting refactor: - script_environment.dart: onDispose() → dispose() in the ScriptEnvironmentExtension adapter (keeps it implementing the renamed SessionExtension interface). - agent_session_test.dart: revert 'ScriptEnvironment satisfies SessionExtension' back to 'ScriptEnvironmentExtension adapter lifecycle' (scripting refactor is a separate PR). - concurrency_experiments_test.dart: restore ScriptEnvironmentExtension wrappers around _FakeBridgeScriptEnvironment at all 6 usage sites (fakes implement ScriptEnvironment, not SessionExtension directly). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
ExecutionEvent is a sealed class — the switch in ExecutionTracker must be exhaustive. PlatformConsentNotice (added in the HITL commit) was missing from the no-op arm. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
Replaces
AgentUiDelegate(callback-based, single-tool approval) witha signal-driven Human-in-the-Loop gate that scales to concurrent tool
calls and integrates cleanly with the existing
signalsreactive layer.ClientTool.requiresApproval: bool— whentrue,AgentSessionsuspends execution before the tool executor runs, emits a
PendingApprovalRequeston a signal, and waits forapproveToolCallor
denyToolCallto resume. Intended for high-risk operations likeexecute_python.ClientTool.platformConsentNote: String? Function()?— optionalcallback for tools that trigger an OS-level permission dialog (e.g.
clipboard read on web triggers a browser prompt; silent on native).
Returns a human-readable note;
AgentSessionemits aPlatformConsentNoticeevent (non-blocking, purely informational).AgentSessiongainspendingApproval(ReadonlySignal<PendingApprovalRequest?>),approveToolCall(String), anddenyToolCall(String). The gate uses aCompleter<bool>pertoolCallId; auto-denies on session cancel.PendingApprovalRequest— new immutable data class(
toolCallId,toolName,arguments).PlatformConsentNotice/AwaitingApproval— newExecutionEventsubclasses for the consent/approval lifecycle.session_extension.dart:onDispose()renamed todispose()forconsistency with Dart disposal conventions.
Deleted:
AgentUiDelegate(58 lines) and its 457-line test.Tests
hitl_test.dart— 12 new tests:requiresApprovaldefaults for all three tool constructorsPendingApprovalRequestfield valuesplatformConsentNotedefaults, non-null return, null returnPlatformConsentNoticefield values and equalityAll 461 existing unit tests continue to pass. DCM clean.
Test plan
dart analyze --fatal-infospasses (no warnings)dcm analyzepasses (no issues)dart test --exclude-tags integration --reporter failures-only— 461 passtool_registry.dartdoc comments for the three approval tiersagent_session.dart_awaitApprovalfor cancel/deny edge cases🤖 Generated with Claude Code