From 2aa8d73b2d330e54731f627dff27ea99b9022505 Mon Sep 17 00:00:00 2001 From: David Kaya Date: Tue, 24 Mar 2026 19:48:58 +0100 Subject: [PATCH] feat: add approval checkpoints backend Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- HANDOVER.md | 306 ++++++++++++++++++ .../Contracts/ProtocolModels.cs | 31 ++ .../Services/CopilotWorkflowRunner.cs | 188 ++++++++++- .../Services/ITurnWorkflowRunner.cs | 5 + .../Services/SidecarProtocolHost.cs | 10 + .../SidecarProtocolHostTests.cs | 115 ++++++- src/main/EryxAppService.ts | 259 +++++++++++++++ src/main/ipc/registerIpcHandlers.ts | 4 + src/main/persistence/workspaceRepository.ts | 10 +- src/main/sidecar/runTurnPending.ts | 3 +- src/main/sidecar/sidecarProcess.ts | 39 ++- src/preload/index.ts | 1 + src/renderer/components/RunTimeline.tsx | 2 + src/renderer/lib/runTimelineFormatting.ts | 15 +- src/shared/contracts/channels.ts | 1 + src/shared/contracts/ipc.ts | 8 + src/shared/contracts/sidecar.ts | 29 +- src/shared/domain/approval.ts | 212 ++++++++++++ src/shared/domain/pattern.ts | 17 + src/shared/domain/runTimeline.ts | 83 +++++ src/shared/domain/session.ts | 2 + src/shared/domain/sessionLibrary.ts | 1 + tests/main/runTurnPending.test.ts | 2 + tests/renderer/runTimelineFormatting.test.ts | 10 + tests/shared/approval.test.ts | 67 ++++ tests/shared/pattern.test.ts | 24 ++ tests/shared/runTimeline.test.ts | 42 +++ tests/shared/sessionLibrary.test.ts | 8 + 28 files changed, 1471 insertions(+), 23 deletions(-) create mode 100644 HANDOVER.md create mode 100644 src/shared/domain/approval.ts create mode 100644 tests/shared/approval.test.ts diff --git a/HANDOVER.md b/HANDOVER.md new file mode 100644 index 0000000..0e3408f --- /dev/null +++ b/HANDOVER.md @@ -0,0 +1,306 @@ +# Approval checkpoints frontend handover + +## What is implemented in the backend + +The backend now supports approval checkpoints in two places: + +1. `tool-call` +2. `final-response` + +`tool-call` approvals are enforced inside the .NET sidecar through the Copilot permission callback. + +`final-response` approvals are enforced in the Electron main process after the sidecar finishes generating assistant messages but before those messages are published into the session transcript. + +The roadmap item **"pause before handing off outside the original working set"** is **not implemented** in this change. There is still no working-set model in the backend that can support that rule correctly. + +## Important behavioral semantics + +- A session **stays** `status: 'running'` while waiting for approval. +- The backend exposes the paused state through `session.pendingApproval`. +- The frontend should treat `session.pendingApproval` as "awaiting approval" even though `session.status` is still `running`. +- Resolving an approval clears `session.pendingApproval`. +- Approving a checkpoint resumes the run. +- Rejecting a `final-response` checkpoint causes the run to fail with an explicit error after the backend resumes the waiting flow. +- Rejecting a `tool-call` checkpoint sends a denial back to the Copilot runtime. The eventual run outcome depends on the runtime response, but it should be treated as a rejected risky action. +- If Eryx restarts while an approval is pending, the backend marks that session as errored and converts the pending approval into a rejected/interrupted run event. + +## New shared model shapes + +### Pattern approval policy + +File: `src/shared/domain/approval.ts` + +Patterns can now carry an optional `approvalPolicy`: + +```ts +type ApprovalCheckpointKind = 'tool-call' | 'final-response'; + +interface ApprovalCheckpointRule { + kind: ApprovalCheckpointKind; + agentIds?: string[]; // omitted or empty = all agents in the pattern +} + +interface ApprovalPolicy { + rules: ApprovalCheckpointRule[]; +} +``` + +This is attached to `PatternDefinition` as: + +```ts +approvalPolicy?: ApprovalPolicy; +``` + +Backend validation already rejects agent IDs that do not exist in the pattern. + +### Session pending approval + +File: `src/shared/domain/approval.ts` + +Sessions can now expose a single active pending approval: + +```ts +interface PendingApprovalMessageRecord { + id: string; + authorName: string; + content: string; +} + +interface PendingApprovalRecord { + id: string; + kind: 'tool-call' | 'final-response'; + status: 'pending' | 'approved' | 'rejected'; + requestedAt: string; + resolvedAt?: string; + agentId?: string; + agentName?: string; + toolName?: string; + permissionKind?: string; + title: string; + detail?: string; + messages?: PendingApprovalMessageRecord[]; +} +``` + +This is attached to `SessionRecord` as: + +```ts +pendingApproval?: PendingApprovalRecord; +``` + +For `final-response`, `pendingApproval.messages` contains the unpublished assistant output that the reviewer should inspect before publication. + +For `tool-call`, `pendingApproval.messages` is typically absent. + +### Run timeline event + +File: `src/shared/domain/runTimeline.ts` + +There is a new run timeline event kind: + +```ts +kind: 'approval' +``` + +The event is updated in place over time rather than creating separate requested/approved/rejected entries. + +Relevant fields: + +```ts +approvalId?: string; +approvalKind?: 'tool-call' | 'final-response'; +approvalTitle?: string; +approvalDetail?: string; +permissionKind?: string; +decision?: 'approved' | 'rejected'; +status: 'running' | 'completed' | 'error'; +``` + +Interpretation: + +- `status: 'running'` => approval requested and still pending +- `status: 'completed'` + `decision: 'approved'` => approval granted +- `status: 'error'` + `decision: 'rejected'` => approval rejected + +## New IPC surface + +Files: + +- `src/shared/contracts/ipc.ts` +- `src/shared/contracts/channels.ts` +- `src/preload/index.ts` +- `src/main/ipc/registerIpcHandlers.ts` + +New preload/API call: + +```ts +resolveSessionApproval({ + sessionId: string; + approvalId: string; + decision: 'approved' | 'rejected'; +}): Promise +``` + +This is the frontend action to approve or reject the active checkpoint. + +## Internal sidecar protocol additions + +These are backend-only, but useful context for debugging: + +Files: + +- `src/shared/contracts/sidecar.ts` +- `sidecar/src/Eryx.AgentHost/Contracts/ProtocolModels.cs` +- `sidecar/src/Eryx.AgentHost/Services/SidecarProtocolHost.cs` +- `sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs` + +New sidecar event: + +```ts +type: 'approval-requested' +``` + +New sidecar command: + +```ts +type: 'resolve-approval' +``` + +The Electron main process already handles this handshake. The frontend should not talk to the sidecar directly. + +## Files that already understand approval data + +Backend and shared code: + +- `src/shared/domain/approval.ts` +- `src/shared/domain/pattern.ts` +- `src/shared/domain/session.ts` +- `src/shared/domain/runTimeline.ts` +- `src/main/EryxAppService.ts` +- `src/main/sidecar/sidecarProcess.ts` + +Minimal renderer compile-support only: + +- `src/renderer/lib/runTimelineFormatting.ts` +- `src/renderer/components/RunTimeline.tsx` + +Those renderer changes are intentionally minimal. They only keep the current app build/typecheck working with the new timeline event kind. + +## Frontend work still needed + +### 1. Pattern editor support for approval policy + +Files to inspect: + +- `src/renderer/components/PatternEditor.tsx` +- `src/renderer/App.tsx` + +Needed UI: + +- Allow enabling `tool-call` approvals. +- Allow enabling `final-response` approvals. +- Allow scoping each checkpoint to: + - all agents + - selected agents + +Recommended representation: + +- one section named "Approval checkpoints" +- one row per checkpoint kind +- a toggle +- an "all agents / selected agents" selector +- multi-select agent chips when scoped + +### 2. Pending approval UI in chat/session surfaces + +Files to inspect: + +- `src/renderer/components/ChatPane.tsx` +- `src/renderer/components/ActivityPanel.tsx` +- `src/renderer/components/Sidebar.tsx` +- `src/renderer/lib/sessionWorkspace.ts` + +Needed behavior: + +- Show a visible checkpoint card/banner whenever `session.pendingApproval` exists. +- Present: + - title + - detail + - agent name + - permission kind (for tool-call) + - preview messages (for final-response) +- Add **Approve** and **Reject** actions that call `api.resolveSessionApproval(...)`. + +### 3. Final-response review UI + +Use `session.pendingApproval.messages` when `kind === 'final-response'`. + +Recommended UX: + +- Render the candidate assistant messages exactly as they would appear in chat. +- Make it clear they are pending publication, not yet committed to transcript history. +- On approval, let the backend publish them. +- On rejection, expect the run to fail and show an error state after the backend resumes the blocked flow. + +### 4. Run timeline UI + +Files to inspect: + +- `src/renderer/components/RunTimeline.tsx` +- `src/renderer/lib/runTimelineFormatting.ts` + +Needed improvements: + +- Add a dedicated approval event treatment instead of the current minimal fallback. +- Show distinct visuals for: + - requested + - approved + - rejected +- Include checkpoint kind and detail text. +- Include message preview affordances for final-response if desired. + +### 5. Session activity / status treatment + +Files to inspect: + +- `src/renderer/lib/sessionActivity.ts` +- `src/renderer/components/ActivityPanel.tsx` +- `src/renderer/components/ChatPane.tsx` + +Important rule: + +- Do **not** rely on `session.status` alone to decide whether the session is actively streaming versus blocked on approval. +- Use `session.pendingApproval` as the signal for "paused awaiting human action." + +### 6. Error / stale checkpoint handling + +The frontend should handle two backend-produced edge cases cleanly: + +1. `resolveSessionApproval(...)` can fail if the approval is no longer active. +2. A pending approval can disappear on reload because the backend converted it into an interrupted error after restart. + +Recommended UX: + +- show a toast or inline error if approval resolution fails +- refresh from workspace state afterward +- show the session error banner when the run has been interrupted + +## Suggested frontend implementation order + +1. Add read-only pending approval banner/card in `ChatPane`. +2. Wire approve/reject buttons to `resolveSessionApproval`. +3. Render final-response preview from `pendingApproval.messages`. +4. Add timeline rendering for `approval` events. +5. Add pattern editor controls for configuring `approvalPolicy`. +6. Refine sidebar/activity badges for "awaiting approval" state. + +## Validation commands + +Backend changes already pass: + +- `bun run typecheck` +- `bun test` +- `bun run sidecar:test` +- `bun run build` + +After the frontend agent finishes, rerun the same four commands. diff --git a/sidecar/src/Eryx.AgentHost/Contracts/ProtocolModels.cs b/sidecar/src/Eryx.AgentHost/Contracts/ProtocolModels.cs index 32a57a3..a0b3698 100644 --- a/sidecar/src/Eryx.AgentHost/Contracts/ProtocolModels.cs +++ b/sidecar/src/Eryx.AgentHost/Contracts/ProtocolModels.cs @@ -21,11 +21,23 @@ public sealed class PatternDefinitionDto public string Availability { get; init; } = "available"; public string? UnavailabilityReason { get; init; } public int MaxIterations { get; init; } + public ApprovalPolicyDto? ApprovalPolicy { get; init; } public IReadOnlyList Agents { get; init; } = []; public string CreatedAt { get; init; } = string.Empty; public string UpdatedAt { get; init; } = string.Empty; } +public sealed class ApprovalPolicyDto +{ + public IReadOnlyList Rules { get; init; } = []; +} + +public sealed class ApprovalCheckpointRuleDto +{ + public string Kind { get; init; } = string.Empty; + public IReadOnlyList AgentIds { get; init; } = []; +} + public sealed class ChatMessageDto { public string Id { get; init; } = string.Empty; @@ -116,6 +128,12 @@ public sealed class RunTurnCommandDto : SidecarCommandEnvelope public RunTurnToolingConfigDto? Tooling { get; init; } } +public sealed class ResolveApprovalCommandDto : SidecarCommandEnvelope +{ + public string ApprovalId { get; init; } = string.Empty; + public string Decision { get; init; } = string.Empty; +} + public sealed class RunTurnToolingConfigDto { public IReadOnlyList McpServers { get; init; } = []; @@ -187,6 +205,19 @@ public sealed class AgentActivityEventDto : SidecarEventDto public string? ToolName { get; init; } } +public sealed class ApprovalRequestedEventDto : SidecarEventDto +{ + public string SessionId { get; init; } = string.Empty; + public string ApprovalId { get; init; } = string.Empty; + public string ApprovalKind { get; init; } = string.Empty; + public string? AgentId { get; init; } + public string? AgentName { get; init; } + public string? ToolName { get; init; } + public string? PermissionKind { get; init; } + public string Title { get; init; } = string.Empty; + public string? Detail { get; init; } +} + public sealed class CommandErrorEventDto : SidecarEventDto { public string Message { get; init; } = string.Empty; diff --git a/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs b/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs index 7b36a25..2992f0c 100644 --- a/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs +++ b/sidecar/src/Eryx.AgentHost/Services/CopilotWorkflowRunner.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using System.Text; using GitHub.Copilot.SDK; using Eryx.AgentHost.Contracts; @@ -22,6 +23,7 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner private static readonly Type? ImageGenerationToolCallContentType = LoadType( "Microsoft.Extensions.AI.ImageGenerationToolCallContent, Microsoft.Extensions.AI.Abstractions"); private readonly PatternValidator _patternValidator; + private readonly ConcurrentDictionary _pendingApprovals = new(StringComparer.Ordinal); public CopilotWorkflowRunner(PatternValidator patternValidator) { @@ -32,6 +34,7 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner RunTurnCommandDto command, Func onDelta, Func onActivity, + Func onApproval, CancellationToken cancellationToken) { PatternValidationIssueDto? validationError = _patternValidator.Validate(command.Pattern).FirstOrDefault(); @@ -40,7 +43,16 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner throw new InvalidOperationException(validationError.Message); } - await using AgentBundle bundle = await AgentBundle.CreateAsync(command, cancellationToken); + await using AgentBundle bundle = await AgentBundle.CreateAsync( + command, + (agent, request, invocation) => RequestApprovalAsync( + command, + agent, + request, + invocation, + onApproval, + cancellationToken), + cancellationToken); Workflow workflow = bundle.BuildWorkflow(command.Pattern); List inputMessages = command.Messages.Select(ToChatMessage).ToList(); @@ -169,6 +181,38 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner return completedMessages; } + public Task ResolveApprovalAsync( + ResolveApprovalCommandDto command, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(command); + + if (string.IsNullOrWhiteSpace(command.ApprovalId)) + { + throw new InvalidOperationException("Approval ID is required."); + } + + if (!_pendingApprovals.TryGetValue(command.ApprovalId, out PendingApprovalRequest? pending)) + { + throw new InvalidOperationException($"Approval \"{command.ApprovalId}\" is not pending."); + } + + PermissionRequestResultKind decision = command.Decision.Trim().ToLowerInvariant() switch + { + "approved" => PermissionRequestResultKind.Approved, + "rejected" => PermissionRequestResultKind.DeniedInteractivelyByUser, + _ => throw new InvalidOperationException( + $"Unsupported approval decision \"{command.Decision}\"."), + }; + + if (!pending.Decision.TrySetResult(decision)) + { + throw new InvalidOperationException($"Approval \"{command.ApprovalId}\" is no longer pending."); + } + + return Task.CompletedTask; + } + private static AgentActivityEventDto CreateActivityEvent( RunTurnCommandDto command, string activityType, @@ -190,6 +234,61 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner }; } + private async Task RequestApprovalAsync( + RunTurnCommandDto command, + PatternAgentDefinitionDto agent, + PermissionRequest request, + PermissionInvocation invocation, + Func onApproval, + CancellationToken cancellationToken) + { + if (!RequiresApproval(command.Pattern.ApprovalPolicy, "tool-call", agent.Id)) + { + return new PermissionRequestResult + { + Kind = PermissionRequestResultKind.Approved, + }; + } + + string approvalId = CreateApprovalRequestId(); + TaskCompletionSource decisionSource = + new(TaskCreationOptions.RunContinuationsAsynchronously); + PendingApprovalRequest pending = new( + command.RequestId, + command.SessionId, + approvalId, + decisionSource); + + if (!_pendingApprovals.TryAdd(approvalId, pending)) + { + throw new InvalidOperationException($"Approval \"{approvalId}\" is already pending."); + } + + try + { + await onApproval(BuildPermissionApprovalEvent(command, agent, request, invocation, approvalId)) + .ConfigureAwait(false); + + using CancellationTokenRegistration registration = cancellationToken.Register( + static state => + { + ((TaskCompletionSource)state!) + .TrySetCanceled(); + }, + decisionSource); + + PermissionRequestResultKind decision = await decisionSource.Task.ConfigureAwait(false); + return new PermissionRequestResult + { + Kind = decision, + }; + } + finally + { + _pendingApprovals.TryRemove(approvalId, out _); + } + } + private static AgentActivityEventDto? TryCreateActivityFromRequest( RunTurnCommandDto command, RequestInfoEvent requestInfo, @@ -284,6 +383,75 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner return false; } + private static ApprovalRequestedEventDto BuildPermissionApprovalEvent( + RunTurnCommandDto command, + PatternAgentDefinitionDto agent, + PermissionRequest request, + PermissionInvocation invocation, + string approvalId) + { + string permissionKind = string.IsNullOrWhiteSpace(request.Kind) + ? "tool access" + : request.Kind.Trim(); + string agentName = string.IsNullOrWhiteSpace(agent.Name) ? agent.Id : agent.Name; + string? sessionId = string.IsNullOrWhiteSpace(invocation.SessionId) + ? null + : invocation.SessionId.Trim(); + + return new ApprovalRequestedEventDto + { + Type = "approval-requested", + RequestId = command.RequestId, + SessionId = command.SessionId, + ApprovalId = approvalId, + ApprovalKind = "tool-call", + AgentId = string.IsNullOrWhiteSpace(agent.Id) ? null : agent.Id, + AgentName = string.IsNullOrWhiteSpace(agentName) ? null : agentName, + PermissionKind = permissionKind, + Title = $"Approve {permissionKind}", + Detail = sessionId is null + ? $"{agentName} requested {permissionKind} permission." + : $"{agentName} requested {permissionKind} permission for Copilot session {sessionId}.", + }; + } + + private static bool RequiresApproval( + ApprovalPolicyDto? approvalPolicy, + string checkpointKind, + string agentId) + { + if (approvalPolicy?.Rules is null || approvalPolicy.Rules.Count == 0) + { + return false; + } + + foreach (ApprovalCheckpointRuleDto rule in approvalPolicy.Rules) + { + if (!string.Equals(rule.Kind, checkpointKind, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + if (rule.AgentIds.Count == 0) + { + return true; + } + + if (rule.AgentIds.Any(candidate => + string.Equals(candidate, agentId, StringComparison.OrdinalIgnoreCase))) + { + return true; + } + } + + return false; + } + + private static string CreateApprovalRequestId() + { + return $"approval-{Guid.NewGuid():N}"; + } + private static Type? LoadType(string assemblyQualifiedName) { return Type.GetType(assemblyQualifiedName, throwOnError: false); @@ -512,6 +680,7 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner public static async Task CreateAsync( RunTurnCommandDto command, + Func> onPermissionRequest, CancellationToken cancellationToken) { List disposables = []; @@ -542,7 +711,7 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner Content = AgentInstructionComposer.Compose(command.Pattern, definition, agentIndex, command.WorkspaceKind), }, WorkingDirectory = command.ProjectPath, - OnPermissionRequest = ApprovePermissionAsync, + OnPermissionRequest = (request, invocation) => onPermissionRequest(definition, request, invocation), Streaming = true, }; @@ -649,14 +818,11 @@ public sealed class CopilotWorkflowRunner : ITurnWorkflowRunner .Build(); } - private static Task ApprovePermissionAsync( - PermissionRequest request, - PermissionInvocation invocation) - { - return Task.FromResult(new PermissionRequestResult - { - Kind = PermissionRequestResultKind.Approved, - }); - } } + + private sealed record PendingApprovalRequest( + string RequestId, + string SessionId, + string ApprovalId, + TaskCompletionSource Decision); } diff --git a/sidecar/src/Eryx.AgentHost/Services/ITurnWorkflowRunner.cs b/sidecar/src/Eryx.AgentHost/Services/ITurnWorkflowRunner.cs index 7d551d3..a752623 100644 --- a/sidecar/src/Eryx.AgentHost/Services/ITurnWorkflowRunner.cs +++ b/sidecar/src/Eryx.AgentHost/Services/ITurnWorkflowRunner.cs @@ -8,5 +8,10 @@ public interface ITurnWorkflowRunner RunTurnCommandDto command, Func onDelta, Func onActivity, + Func onApproval, + CancellationToken cancellationToken); + + Task ResolveApprovalAsync( + ResolveApprovalCommandDto command, CancellationToken cancellationToken); } diff --git a/sidecar/src/Eryx.AgentHost/Services/SidecarProtocolHost.cs b/sidecar/src/Eryx.AgentHost/Services/SidecarProtocolHost.cs index 327ce4c..985eb83 100644 --- a/sidecar/src/Eryx.AgentHost/Services/SidecarProtocolHost.cs +++ b/sidecar/src/Eryx.AgentHost/Services/SidecarProtocolHost.cs @@ -127,6 +127,7 @@ public sealed class SidecarProtocolHost runTurnCommand, delta => WriteAsync(output, delta, cancellationToken), activity => WriteAsync(output, activity, cancellationToken), + approval => WriteAsync(output, approval, cancellationToken), cancellationToken).ConfigureAwait(false); await WriteAsync(output, new TurnCompleteEventDto @@ -138,6 +139,15 @@ public sealed class SidecarProtocolHost }, cancellationToken).ConfigureAwait(false); break; + case "resolve-approval": + ResolveApprovalCommandDto resolveApprovalCommand = + JsonSerializer.Deserialize(rawCommand, _jsonOptions) + ?? throw new InvalidOperationException("Could not deserialize resolve-approval command."); + + await _workflowRunner.ResolveApprovalAsync(resolveApprovalCommand, cancellationToken) + .ConfigureAwait(false); + break; + default: throw new NotSupportedException($"Unknown sidecar command type '{envelope.Type}'."); } diff --git a/sidecar/tests/Eryx.AgentHost.Tests/SidecarProtocolHostTests.cs b/sidecar/tests/Eryx.AgentHost.Tests/SidecarProtocolHostTests.cs index 3c8f3eb..eae745d 100644 --- a/sidecar/tests/Eryx.AgentHost.Tests/SidecarProtocolHostTests.cs +++ b/sidecar/tests/Eryx.AgentHost.Tests/SidecarProtocolHostTests.cs @@ -108,7 +108,7 @@ public sealed class SidecarProtocolHostTests { SidecarProtocolHost host = new( new PatternValidator(), - new FakeWorkflowRunner(async (command, onDelta, onActivity, cancellationToken) => + new FakeWorkflowRunner(async (command, onDelta, onActivity, onApproval, cancellationToken) => { await onActivity(new AgentActivityEventDto { @@ -226,6 +226,102 @@ public sealed class SidecarProtocolHostTests }); } + [Fact] + public async Task RunTurnCommand_ReturnsApprovalEvents() + { + SidecarProtocolHost host = new( + new PatternValidator(), + new FakeWorkflowRunner(async (command, onDelta, onActivity, onApproval, cancellationToken) => + { + await onApproval(new ApprovalRequestedEventDto + { + Type = "approval-requested", + RequestId = command.RequestId, + SessionId = command.SessionId, + ApprovalId = "approval-1", + ApprovalKind = "tool-call", + AgentId = "agent-1", + AgentName = "Primary", + PermissionKind = "tool access", + Title = "Approve tool access", + }); + + return []; + })); + + IReadOnlyList events = await RunHostAsync( + new RunTurnCommandDto + { + Type = "run-turn", + RequestId = "turn-approval", + SessionId = "session-1", + ProjectPath = "C:\\workspace\\project", + Pattern = new PatternDefinitionDto + { + Id = "pattern-1", + Name = "Single Agent", + Mode = "single", + Availability = "available", + Agents = + [ + CreateAgent(name: "Primary"), + ], + }, + }, + host); + + Assert.Collection( + events, + approvalEvent => + { + Assert.Equal("approval-requested", approvalEvent.GetProperty("type").GetString()); + Assert.Equal("turn-approval", approvalEvent.GetProperty("requestId").GetString()); + Assert.Equal("approval-1", approvalEvent.GetProperty("approvalId").GetString()); + Assert.Equal("tool-call", approvalEvent.GetProperty("approvalKind").GetString()); + Assert.Equal("Approve tool access", approvalEvent.GetProperty("title").GetString()); + }, + completionEvent => + { + Assert.Equal("turn-complete", completionEvent.GetProperty("type").GetString()); + }, + commandCompleteEvent => + { + Assert.Equal("command-complete", commandCompleteEvent.GetProperty("type").GetString()); + Assert.Equal("turn-approval", commandCompleteEvent.GetProperty("requestId").GetString()); + }); + } + + [Fact] + public async Task ResolveApprovalCommand_DelegatesToWorkflowRunnerAndCompletes() + { + ResolveApprovalCommandDto? captured = null; + SidecarProtocolHost host = new( + new PatternValidator(), + new FakeWorkflowRunner( + handler: async (command, onDelta, onActivity, onApproval, cancellationToken) => [], + resolveApprovalHandler: (command, cancellationToken) => + { + captured = command; + return Task.CompletedTask; + })); + + IReadOnlyList events = await RunHostAsync( + new ResolveApprovalCommandDto + { + Type = "resolve-approval", + RequestId = "approval-command-1", + ApprovalId = "approval-1", + Decision = "approved", + }, + host); + + JsonElement completionEvent = Assert.Single(events); + Assert.Equal("command-complete", completionEvent.GetProperty("type").GetString()); + Assert.Equal("approval-command-1", completionEvent.GetProperty("requestId").GetString()); + Assert.Equal("approval-1", captured?.ApprovalId); + Assert.Equal("approved", captured?.Decision); + } + [Fact] public void ClassifyConnectionStatus_ReturnsAuthRequiredForLoginFailures() { @@ -371,27 +467,40 @@ public sealed class SidecarProtocolHostTests RunTurnCommandDto, Func, Func, + Func, CancellationToken, Task>> _handler; + private readonly Func _resolveApprovalHandler; public FakeWorkflowRunner( Func< RunTurnCommandDto, Func, Func, + Func, CancellationToken, - Task>> handler) + Task>> handler, + Func? resolveApprovalHandler = null) { _handler = handler; + _resolveApprovalHandler = resolveApprovalHandler ?? ((_, _) => Task.CompletedTask); } public Task> RunTurnAsync( RunTurnCommandDto command, Func onDelta, Func onActivity, + Func onApproval, CancellationToken cancellationToken) { - return _handler(command, onDelta, onActivity, cancellationToken); + return _handler(command, onDelta, onActivity, onApproval, cancellationToken); + } + + public Task ResolveApprovalAsync( + ResolveApprovalCommandDto command, + CancellationToken cancellationToken) + { + return _resolveApprovalHandler(command, cancellationToken); } } } diff --git a/src/main/EryxAppService.ts b/src/main/EryxAppService.ts index 3519a3f..7a525c0 100644 --- a/src/main/EryxAppService.ts +++ b/src/main/EryxAppService.ts @@ -5,6 +5,7 @@ import { dialog } from 'electron'; import type { AgentActivityEvent, + ApprovalRequestedEvent, RunTurnLspProfileConfig, RunTurnMcpServerConfig, RunTurnToolingConfig, @@ -23,6 +24,14 @@ import { type ReasoningEffort, validatePatternDefinition, } from '@shared/domain/pattern'; +import { + approvalPolicyRequiresCheckpoint, + normalizeApprovalPolicy, + resolvePendingApproval, + type ApprovalDecision, + type PendingApprovalMessageRecord, + type PendingApprovalRecord, +} from '@shared/domain/approval'; import { isScratchpadProject, type ProjectRecord } from '@shared/domain/project'; import { duplicateSessionRecord, @@ -45,6 +54,7 @@ import { completeSessionRunRecord, createSessionRunRecord, failSessionRunRecord, + upsertRunApprovalEvent, upsertRunMessageEvent, upsertSessionRunRecord, type SessionRunRecord, @@ -75,6 +85,12 @@ type AppServiceEvents = { 'session-event': [SessionEventRecord]; }; +type PendingApprovalHandle = { + sessionId: string; + requestId: string; + resolve: (decision: ApprovalDecision) => void | Promise; +}; + function isBuiltinPattern(patternId: string): boolean { return patternId.startsWith('pattern-'); } @@ -84,6 +100,7 @@ export class EryxAppService extends EventEmitter { private readonly sidecar = new SidecarClient(); private readonly secretStore = new SecretStore(); private readonly gitService = new GitService(); + private readonly pendingApprovalHandles = new Map(); private workspace?: WorkspaceState; private sidecarCapabilities?: SidecarCapabilities; private didScheduleInitialProjectGitRefresh = false; @@ -99,6 +116,9 @@ export class EryxAppService extends EventEmitter { async loadWorkspace(): Promise { if (!this.workspace) { this.workspace = await this.workspaceRepository.load(); + if (this.failInterruptedPendingApprovals(this.workspace)) { + await this.workspaceRepository.save(this.workspace); + } } if (!this.didScheduleInitialProjectGitRefresh) { @@ -180,6 +200,7 @@ export class EryxAppService extends EventEmitter { const existingIndex = workspace.patterns.findIndex((current) => current.id === pattern.id); const candidate: PatternDefinition = { ...pattern, + approvalPolicy: normalizeApprovalPolicy(pattern.approvalPolicy), isFavorite: pattern.isFavorite ?? workspace.patterns[existingIndex]?.isFavorite, createdAt: existingIndex >= 0 ? workspace.patterns[existingIndex].createdAt : nowIso(), updatedAt: nowIso(), @@ -385,6 +406,9 @@ export class EryxAppService extends EventEmitter { async sendSessionMessage(sessionId: string, content: string): Promise { const workspace = await this.loadWorkspace(); const session = this.requireSession(workspace, sessionId); + if (session.status === 'running') { + throw new Error('Wait for the current response or approval checkpoint to finish before sending another message.'); + } const project = this.requireProject(workspace, session.projectId); const pattern = this.requirePattern(workspace, session.patternId); const effectivePattern = await this.buildEffectivePattern(project, pattern, session); @@ -447,8 +471,13 @@ export class EryxAppService extends EventEmitter { async (event) => { await this.applyAgentActivity(workspace, session.id, requestId, event); }, + async (event) => { + await this.handleApprovalRequested(workspace, session.id, requestId, event, (decision) => + this.sidecar.resolveApproval(event.approvalId, decision)); + }, ); + await this.awaitFinalResponseApproval(workspace, session.id, requestId, effectivePattern, responseMessages); this.finalizeTurn(workspace, session.id, requestId, responseMessages); await this.persistAndBroadcast(workspace); } catch (error) { @@ -474,6 +503,66 @@ export class EryxAppService extends EventEmitter { } } + async resolveSessionApproval( + sessionId: string, + approvalId: string, + decision: ApprovalDecision, + ): Promise { + const workspace = await this.loadWorkspace(); + const session = this.requireSession(workspace, sessionId); + const approval = session.pendingApproval; + if (!approval || approval.id !== approvalId) { + throw new Error(`Approval "${approvalId}" is not pending for session "${sessionId}".`); + } + + const handle = this.pendingApprovalHandles.get(approvalId); + if (!handle || handle.sessionId !== sessionId) { + throw new Error(`Approval "${approvalId}" is no longer active. Restart the run and try again.`); + } + + const resolvedAt = nowIso(); + const resolvedApproval = resolvePendingApproval(approval, decision, resolvedAt); + session.pendingApproval = undefined; + session.updatedAt = resolvedAt; + + const updatedRun = this.updateSessionRun(session, handle.requestId, (run) => + upsertRunApprovalEvent(run, resolvedApproval)); + + const result = await this.persistAndBroadcast(workspace); + if (updatedRun) { + this.emitRunUpdated(sessionId, resolvedAt, updatedRun); + } + + this.pendingApprovalHandles.delete(approvalId); + + try { + await Promise.resolve(handle.resolve(decision)); + } catch (error) { + const failedAt = nowIso(); + session.status = 'error'; + session.lastError = error instanceof Error ? error.message : String(error); + session.updatedAt = failedAt; + + const failedRun = this.updateSessionRun(session, handle.requestId, (run) => + failSessionRunRecord(run, failedAt, session.lastError ?? 'Unknown error.')); + + this.emitSessionEvent({ + sessionId, + kind: 'error', + occurredAt: failedAt, + error: session.lastError, + }); + if (failedRun) { + this.emitRunUpdated(sessionId, failedAt, failedRun); + } + + await this.persistAndBroadcast(workspace); + throw error; + } + + return result; + } + async updateScratchpadSessionConfig( sessionId: string, model: string, @@ -834,6 +923,131 @@ export class EryxAppService extends EventEmitter { } } + private async handleApprovalRequested( + workspace: WorkspaceState, + sessionId: string, + requestId: string, + approval: ApprovalRequestedEvent | PendingApprovalRecord, + resolve: (decision: ApprovalDecision) => void | Promise, + ): Promise { + const session = this.requireSession(workspace, sessionId); + if (session.pendingApproval) { + throw new Error(`Session "${sessionId}" already has a pending approval.`); + } + + const pendingApproval = + 'type' in approval ? this.createPendingApprovalFromSidecarEvent(approval) : approval; + + session.pendingApproval = pendingApproval; + session.updatedAt = pendingApproval.requestedAt; + + const updatedRun = this.updateSessionRun(session, requestId, (run) => + upsertRunApprovalEvent(run, pendingApproval)); + + this.pendingApprovalHandles.set(pendingApproval.id, { + sessionId, + requestId, + resolve, + }); + + await this.persistAndBroadcast(workspace); + if (updatedRun) { + this.emitRunUpdated(sessionId, pendingApproval.requestedAt, updatedRun); + } + } + + private createPendingApprovalFromSidecarEvent(event: ApprovalRequestedEvent): PendingApprovalRecord { + return { + id: event.approvalId, + kind: event.approvalKind, + status: 'pending', + requestedAt: nowIso(), + agentId: event.agentId, + agentName: event.agentName, + toolName: event.toolName, + permissionKind: event.permissionKind, + title: event.title, + detail: event.detail, + }; + } + + private async awaitFinalResponseApproval( + workspace: WorkspaceState, + sessionId: string, + requestId: string, + pattern: PatternDefinition, + messages: ChatMessageRecord[], + ): Promise { + const pendingApproval = this.buildFinalResponseApproval(pattern, messages); + if (!pendingApproval) { + return; + } + + let resolveDecision: ((decision: ApprovalDecision) => void) | undefined; + const decisionPromise = new Promise((resolve) => { + resolveDecision = resolve; + }); + + await this.handleApprovalRequested( + workspace, + sessionId, + requestId, + pendingApproval, + (decision) => { + resolveDecision?.(decision); + }, + ); + + const decision = await decisionPromise; + if (decision === 'rejected') { + throw new Error('Final response approval was rejected.'); + } + } + + private buildFinalResponseApproval( + pattern: PatternDefinition, + messages: ChatMessageRecord[], + ): PendingApprovalRecord | undefined { + const assistantMessages = messages.filter((message) => message.role === 'assistant'); + if (assistantMessages.length === 0) { + return undefined; + } + + const previewMessages: PendingApprovalMessageRecord[] = assistantMessages.map((message) => ({ + id: message.id, + authorName: message.authorName, + content: message.content, + })); + + for (let index = assistantMessages.length - 1; index >= 0; index -= 1) { + const message = assistantMessages[index]; + if (!message) { + continue; + } + + const agent = pattern.agents.find((candidate) => + candidate.id === message.authorName || candidate.name === message.authorName); + if (!approvalPolicyRequiresCheckpoint(pattern.approvalPolicy, 'final-response', agent?.id)) { + continue; + } + + const agentName = agent?.name ?? message.authorName; + return { + id: createId('approval'), + kind: 'final-response', + status: 'pending', + requestedAt: nowIso(), + agentId: agent?.id, + agentName, + title: agentName ? `Approve final response from ${agentName}` : 'Approve final response', + detail: 'Review the pending assistant response before it is added to the session transcript.', + messages: previewMessages, + }; + } + + return undefined; + } + private async persistAndBroadcast(workspace: WorkspaceState): Promise { await this.workspaceRepository.save(workspace); this.emit('workspace-updated', workspace); @@ -974,6 +1188,51 @@ export class EryxAppService extends EventEmitter { this.emit('session-event', event); } + private failInterruptedPendingApprovals(workspace: WorkspaceState): boolean { + let changed = false; + + for (const session of workspace.sessions) { + const pendingApproval = session.pendingApproval; + if (!pendingApproval) { + continue; + } + + changed = true; + const failedAt = nowIso(); + const error = 'Pending approval was interrupted because Eryx restarted before a decision was recorded.'; + const requestId = this.findApprovalRequestId(session, pendingApproval.id); + const rejectedApproval = resolvePendingApproval(pendingApproval, 'rejected', failedAt, error); + + session.pendingApproval = undefined; + session.status = 'error'; + session.lastError = error; + session.updatedAt = failedAt; + + if (!requestId) { + continue; + } + + this.updateSessionRun(session, requestId, (run) => + failSessionRunRecord( + upsertRunApprovalEvent(run, rejectedApproval), + failedAt, + error, + )); + } + + return changed; + } + + private findApprovalRequestId(session: SessionRecord, approvalId: string): string | undefined { + const matchingRun = session.runs.find((run) => + run.events.some((event) => event.kind === 'approval' && event.approvalId === approvalId)); + if (matchingRun) { + return matchingRun.requestId; + } + + return session.runs.find((run) => run.status === 'running')?.requestId; + } + private async loadSidecarCapabilities(forceRefresh = false): Promise { if (forceRefresh || !this.sidecarCapabilities) { this.sidecarCapabilities = await this.sidecar.describeCapabilities(); diff --git a/src/main/ipc/registerIpcHandlers.ts b/src/main/ipc/registerIpcHandlers.ts index 9d96c35..8a24477 100644 --- a/src/main/ipc/registerIpcHandlers.ts +++ b/src/main/ipc/registerIpcHandlers.ts @@ -5,6 +5,7 @@ import type { CreateSessionInput, DuplicateSessionInput, RenameSessionInput, + ResolveSessionApprovalInput, SaveLspProfileInput, SaveMcpServerInput, SavePatternInput, @@ -77,6 +78,9 @@ export function registerIpcHandlers(window: BrowserWindow, service: EryxAppServi ipcMain.handle(ipcChannels.sendSessionMessage, (_event, input: SendSessionMessageInput) => service.sendSessionMessage(input.sessionId, input.content), ); + ipcMain.handle(ipcChannels.resolveSessionApproval, (_event, input: ResolveSessionApprovalInput) => + service.resolveSessionApproval(input.sessionId, input.approvalId, input.decision), + ); ipcMain.handle( ipcChannels.updateScratchpadSessionConfig, (_event, input: UpdateScratchpadSessionConfigInput) => diff --git a/src/main/persistence/workspaceRepository.ts b/src/main/persistence/workspaceRepository.ts index 56aeb76..ae43430 100644 --- a/src/main/persistence/workspaceRepository.ts +++ b/src/main/persistence/workspaceRepository.ts @@ -5,6 +5,10 @@ import type { PatternDefinition } from '@shared/domain/pattern'; import { mergeScratchpadProject } from '@shared/domain/project'; import { normalizeSessionRunRecords } from '@shared/domain/runTimeline'; import { normalizeSessionToolingSelection, normalizeWorkspaceSettings } from '@shared/domain/tooling'; +import { + normalizeApprovalPolicy, + normalizePendingApproval, +} from '@shared/domain/approval'; import { createWorkspaceSeed, type WorkspaceState } from '@shared/domain/workspace'; import { nowIso } from '@shared/utils/ids'; @@ -59,12 +63,16 @@ export class WorkspaceRepository { const workspace: WorkspaceState = { ...stored, - patterns: mergePatterns(stored.patterns ?? []), + patterns: mergePatterns(stored.patterns ?? []).map((pattern) => ({ + ...pattern, + approvalPolicy: normalizeApprovalPolicy(pattern.approvalPolicy), + })), projects, sessions: (stored.sessions ?? []).map((session) => ({ ...session, runs: normalizeSessionRunRecords(session.runs), tooling: normalizeSessionToolingSelection(session.tooling), + pendingApproval: normalizePendingApproval(session.pendingApproval), })), settings: normalizeWorkspaceSettings(stored.settings), selectedProjectId: projects.some((project) => project.id === stored.selectedProjectId) diff --git a/src/main/sidecar/runTurnPending.ts b/src/main/sidecar/runTurnPending.ts index 594a9d2..0b1477d 100644 --- a/src/main/sidecar/runTurnPending.ts +++ b/src/main/sidecar/runTurnPending.ts @@ -1,4 +1,4 @@ -import type { AgentActivityEvent, TurnDeltaEvent } from '@shared/contracts/sidecar'; +import type { AgentActivityEvent, ApprovalRequestedEvent, TurnDeltaEvent } from '@shared/contracts/sidecar'; import type { ChatMessageRecord } from '@shared/domain/session'; export interface RunTurnPendingCommand { @@ -7,6 +7,7 @@ export interface RunTurnPendingCommand { reject: (error: Error) => void; onDelta: (event: TurnDeltaEvent) => void | Promise; onActivity: (event: AgentActivityEvent) => void | Promise; + onApproval: (event: ApprovalRequestedEvent) => void | Promise; errored: boolean; } diff --git a/src/main/sidecar/sidecarProcess.ts b/src/main/sidecar/sidecarProcess.ts index 42913c8..6a77172 100644 --- a/src/main/sidecar/sidecarProcess.ts +++ b/src/main/sidecar/sidecarProcess.ts @@ -3,13 +3,15 @@ import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; import type { AgentActivityEvent, - SidecarCapabilities, + ApprovalRequestedEvent, SidecarCommand, + SidecarCapabilities, SidecarEvent, TurnDeltaEvent, ValidatePatternCommand, RunTurnCommand, } from '@shared/contracts/sidecar'; +import type { ApprovalDecision } from '@shared/domain/approval'; import type { ChatMessageRecord } from '@shared/domain/session'; import { createSidecarEnvironment } from '@main/sidecar/sidecarEnvironment'; import { @@ -30,6 +32,11 @@ type PendingCommand = resolve: (issues: ValidatePatternCommand['pattern'] extends never ? never : unknown) => void; reject: (error: Error) => void; } + | { + kind: 'resolve-approval'; + resolve: () => void; + reject: (error: Error) => void; + } | RunTurnPendingCommand; export class SidecarClient { @@ -58,8 +65,18 @@ export class SidecarClient { command: RunTurnCommand, onDelta: (event: TurnDeltaEvent) => void | Promise, onActivity: (event: AgentActivityEvent) => void | Promise, + onApproval: (event: ApprovalRequestedEvent) => void | Promise, ): Promise { - return this.dispatch(command, onDelta, onActivity); + return this.dispatch(command, onDelta, onActivity, onApproval); + } + + async resolveApproval(approvalId: string, decision: ApprovalDecision): Promise { + return this.dispatch({ + type: 'resolve-approval', + requestId: `approval-${Date.now()}`, + approvalId, + decision, + }); } async dispose(): Promise { @@ -118,6 +135,7 @@ export class SidecarClient { command: SidecarCommand, onDelta?: (event: TurnDeltaEvent) => void | Promise, onActivity?: (event: AgentActivityEvent) => void | Promise, + onApproval?: (event: ApprovalRequestedEvent) => void | Promise, ): Promise { const process = await this.ensureProcess(); @@ -129,6 +147,7 @@ export class SidecarClient { reject, onDelta: onDelta ?? (() => undefined), onActivity: onActivity ?? (() => undefined), + onApproval: onApproval ?? (() => undefined), errored: false, }); } else if (command.type === 'validate-pattern') { @@ -137,6 +156,12 @@ export class SidecarClient { resolve: resolve as (issues: unknown) => void, reject, }); + } else if (command.type === 'resolve-approval') { + this.pending.set(command.requestId, { + kind: 'resolve-approval', + resolve: resolve as () => void, + reject, + }); } else { this.pending.set(command.requestId, { kind: 'capabilities', @@ -193,6 +218,11 @@ export class SidecarClient { this.invokeRunTurnHandler(event.requestId, pending, () => pending.onActivity(event)); } return; + case 'approval-requested': + if (pending.kind === 'run-turn' && shouldHandleRunTurnEvent(pending)) { + this.invokeRunTurnHandler(event.requestId, pending, () => pending.onApproval(event)); + } + return; case 'turn-complete': if (pending.kind === 'run-turn') { if (shouldHandleRunTurnEvent(pending)) { @@ -210,7 +240,10 @@ export class SidecarClient { this.pending.delete(event.requestId); return; case 'command-complete': - if (pending.kind !== 'run-turn' || pending.errored) { + if (pending.kind === 'resolve-approval') { + pending.resolve(); + this.pending.delete(event.requestId); + } else if (pending.kind !== 'run-turn' || pending.errored) { this.pending.delete(event.requestId); } return; diff --git a/src/preload/index.ts b/src/preload/index.ts index 98cc286..cc52507 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -25,6 +25,7 @@ const api: ElectronApi = { setSessionPinned: (input) => ipcRenderer.invoke(ipcChannels.setSessionPinned, input), setSessionArchived: (input) => ipcRenderer.invoke(ipcChannels.setSessionArchived, input), sendSessionMessage: (input) => ipcRenderer.invoke(ipcChannels.sendSessionMessage, input), + resolveSessionApproval: (input) => ipcRenderer.invoke(ipcChannels.resolveSessionApproval, input), updateScratchpadSessionConfig: (input) => ipcRenderer.invoke(ipcChannels.updateScratchpadSessionConfig, input), querySessions: (input) => ipcRenderer.invoke(ipcChannels.querySessions, input), diff --git a/src/renderer/components/RunTimeline.tsx b/src/renderer/components/RunTimeline.tsx index edc9ae0..361bb4d 100644 --- a/src/renderer/components/RunTimeline.tsx +++ b/src/renderer/components/RunTimeline.tsx @@ -59,6 +59,8 @@ function EventIcon({ kind, status }: { kind: RunTimelineEventRecord['kind']; sta return ; case 'tool-call': return ; + case 'approval': + return ; case 'message': return ; case 'run-completed': diff --git a/src/renderer/lib/runTimelineFormatting.ts b/src/renderer/lib/runTimelineFormatting.ts index 63e65e5..a082e32 100644 --- a/src/renderer/lib/runTimelineFormatting.ts +++ b/src/renderer/lib/runTimelineFormatting.ts @@ -78,6 +78,14 @@ export function formatEventLabel(event: RunTimelineEventRecord): string { return event.toolName ? `${event.agentName ?? 'Agent'} used ${event.toolName}` : `${event.agentName ?? 'Agent'} tool call`; + case 'approval': + if (event.status === 'completed') { + return event.approvalTitle ? `${event.approvalTitle} approved` : 'Approval granted'; + } + if (event.status === 'error') { + return event.approvalTitle ? `${event.approvalTitle} rejected` : 'Approval rejected'; + } + return event.approvalTitle ?? 'Approval requested'; case 'message': return event.agentName ?? 'Response'; case 'run-completed': @@ -132,9 +140,10 @@ const eventKindOrder: Record = { 'thinking': 1, 'handoff': 2, 'tool-call': 3, - 'message': 4, - 'run-completed': 5, - 'run-failed': 5, + 'approval': 4, + 'message': 5, + 'run-completed': 6, + 'run-failed': 6, }; export function isTerminalEvent(kind: RunTimelineEventKind): boolean { diff --git a/src/shared/contracts/channels.ts b/src/shared/contracts/channels.ts index a049117..f854049 100644 --- a/src/shared/contracts/channels.ts +++ b/src/shared/contracts/channels.ts @@ -20,6 +20,7 @@ export const ipcChannels = { setSessionPinned: 'sessions:set-pinned', setSessionArchived: 'sessions:set-archived', sendSessionMessage: 'sessions:send-message', + resolveSessionApproval: 'sessions:resolve-approval', querySessions: 'sessions:query', updateScratchpadSessionConfig: 'sessions:update-scratchpad-config', selectProject: 'selection:project', diff --git a/src/shared/contracts/ipc.ts b/src/shared/contracts/ipc.ts index 8a6ad84..11a956f 100644 --- a/src/shared/contracts/ipc.ts +++ b/src/shared/contracts/ipc.ts @@ -1,3 +1,4 @@ +import type { ApprovalDecision } from '@shared/domain/approval'; import type { SidecarCapabilities } from '@shared/contracts/sidecar'; import type { PatternDefinition, ReasoningEffort } from '@shared/domain/pattern'; import type { ProjectRecord } from '@shared/domain/project'; @@ -25,6 +26,12 @@ export interface SendSessionMessageInput { content: string; } +export interface ResolveSessionApprovalInput { + sessionId: string; + approvalId: string; + decision: ApprovalDecision; +} + export interface UpdateScratchpadSessionConfigInput { sessionId: string; model: string; @@ -87,6 +94,7 @@ export interface ElectronApi { setSessionPinned(input: SetSessionPinnedInput): Promise; setSessionArchived(input: SetSessionArchivedInput): Promise; sendSessionMessage(input: SendSessionMessageInput): Promise; + resolveSessionApproval(input: ResolveSessionApprovalInput): Promise; updateScratchpadSessionConfig(input: UpdateScratchpadSessionConfigInput): Promise; querySessions(input: QuerySessionsInput): Promise; selectProject(projectId?: string): Promise; diff --git a/src/shared/contracts/sidecar.ts b/src/shared/contracts/sidecar.ts index b9028b8..bfc7a4a 100644 --- a/src/shared/contracts/sidecar.ts +++ b/src/shared/contracts/sidecar.ts @@ -1,4 +1,5 @@ import type { PatternDefinition, PatternValidationIssue, ReasoningEffort } from '@shared/domain/pattern'; +import type { ApprovalCheckpointKind, ApprovalDecision } from '@shared/domain/approval'; import type { ChatMessageRecord } from '@shared/domain/session'; export interface SidecarModeCapability { @@ -76,7 +77,18 @@ export interface RunTurnCommand { tooling?: RunTurnToolingConfig; } -export type SidecarCommand = DescribeCapabilitiesCommand | ValidatePatternCommand | RunTurnCommand; +export interface ResolveApprovalCommand { + type: 'resolve-approval'; + requestId: string; + approvalId: string; + decision: ApprovalDecision; +} + +export type SidecarCommand = + | DescribeCapabilitiesCommand + | ValidatePatternCommand + | RunTurnCommand + | ResolveApprovalCommand; export interface RunTurnLocalMcpServerConfig { id: string; @@ -157,6 +169,20 @@ export interface AgentActivityEvent { toolName?: string; } +export interface ApprovalRequestedEvent { + type: 'approval-requested'; + requestId: string; + sessionId: string; + approvalId: string; + approvalKind: ApprovalCheckpointKind; + agentId?: string; + agentName?: string; + toolName?: string; + permissionKind?: string; + title: string; + detail?: string; +} + export interface CommandErrorEvent { type: 'command-error'; requestId: string; @@ -174,5 +200,6 @@ export type SidecarEvent = | TurnDeltaEvent | TurnCompleteEvent | AgentActivityEvent + | ApprovalRequestedEvent | CommandErrorEvent | CommandCompleteEvent; diff --git a/src/shared/domain/approval.ts b/src/shared/domain/approval.ts new file mode 100644 index 0000000..1be43f0 --- /dev/null +++ b/src/shared/domain/approval.ts @@ -0,0 +1,212 @@ +export type ApprovalCheckpointKind = 'tool-call' | 'final-response'; +export type ApprovalStatus = 'pending' | 'approved' | 'rejected'; +export type ApprovalDecision = Exclude; + +export interface ApprovalCheckpointRule { + kind: ApprovalCheckpointKind; + agentIds?: string[]; +} + +export interface ApprovalPolicy { + rules: ApprovalCheckpointRule[]; +} + +export interface PendingApprovalMessageRecord { + id: string; + authorName: string; + content: string; +} + +export interface PendingApprovalRecord { + id: string; + kind: ApprovalCheckpointKind; + status: ApprovalStatus; + requestedAt: string; + resolvedAt?: string; + agentId?: string; + agentName?: string; + toolName?: string; + permissionKind?: string; + title: string; + detail?: string; + messages?: PendingApprovalMessageRecord[]; +} + +const approvalCheckpointKinds: ApprovalCheckpointKind[] = ['tool-call', 'final-response']; +const approvalCheckpointKindSet = new Set(approvalCheckpointKinds); +const approvalStatusSet = new Set(['pending', 'approved', 'rejected']); + +export function isApprovalCheckpointKind(value: string | undefined): value is ApprovalCheckpointKind { + return value !== undefined && approvalCheckpointKindSet.has(value as ApprovalCheckpointKind); +} + +export function isApprovalStatus(value: string | undefined): value is ApprovalStatus { + return value !== undefined && approvalStatusSet.has(value as ApprovalStatus); +} + +export function normalizeApprovalPolicy(policy?: Partial): ApprovalPolicy | undefined { + const rules = Array.isArray(policy?.rules) ? policy.rules : []; + const selectedAgents = new Map>(); + const appliesToAllAgents = new Set(); + + for (const rule of rules) { + if (!isApprovalCheckpointKind(rule?.kind)) { + continue; + } + + const normalizedAgentIds = normalizeStringArray(rule.agentIds); + if (normalizedAgentIds.length === 0) { + appliesToAllAgents.add(rule.kind); + selectedAgents.delete(rule.kind); + continue; + } + + if (appliesToAllAgents.has(rule.kind)) { + continue; + } + + const existing = selectedAgents.get(rule.kind) ?? new Set(); + for (const agentId of normalizedAgentIds) { + existing.add(agentId); + } + selectedAgents.set(rule.kind, existing); + } + + const normalizedRules = approvalCheckpointKinds.flatMap((kind): ApprovalCheckpointRule[] => { + if (appliesToAllAgents.has(kind)) { + return [{ kind }]; + } + + const agentIds = [...(selectedAgents.get(kind) ?? [])]; + if (agentIds.length === 0) { + return []; + } + + return [{ kind, agentIds }]; + }); + + return normalizedRules.length > 0 ? { rules: normalizedRules } : undefined; +} + +export function validateApprovalPolicy( + policy: ApprovalPolicy | undefined, + knownAgentIds: readonly string[], +): string[] { + if (!policy) { + return []; + } + + const knownAgents = new Set(normalizeStringArray(knownAgentIds)); + const issues: string[] = []; + + for (const rule of policy.rules) { + for (const agentId of rule.agentIds ?? []) { + if (!knownAgents.has(agentId)) { + issues.push(`Approval checkpoint "${rule.kind}" references unknown agent "${agentId}".`); + } + } + } + + return issues; +} + +export function approvalPolicyRequiresCheckpoint( + policy: ApprovalPolicy | undefined, + kind: ApprovalCheckpointKind, + agentId?: string, +): boolean { + const rule = policy?.rules.find((candidate) => candidate.kind === kind); + if (!rule) { + return false; + } + + if (!rule.agentIds || rule.agentIds.length === 0) { + return true; + } + + const normalizedAgentId = normalizeOptionalString(agentId); + if (!normalizedAgentId) { + return false; + } + + return rule.agentIds.includes(normalizedAgentId); +} + +export function normalizePendingApproval( + approval?: Partial, +): PendingApprovalRecord | undefined { + const id = normalizeOptionalString(approval?.id); + const kind = isApprovalCheckpointKind(approval?.kind) ? approval.kind : undefined; + const status = isApprovalStatus(approval?.status) ? approval.status : undefined; + const requestedAt = normalizeOptionalString(approval?.requestedAt); + const title = normalizeOptionalString(approval?.title); + if (!id || !kind || !status || !requestedAt || !title) { + return undefined; + } + + return { + id, + kind, + status, + requestedAt, + resolvedAt: status === 'pending' ? undefined : normalizeOptionalString(approval?.resolvedAt), + agentId: normalizeOptionalString(approval?.agentId), + agentName: normalizeOptionalString(approval?.agentName), + toolName: normalizeOptionalString(approval?.toolName), + permissionKind: normalizeOptionalString(approval?.permissionKind), + title, + detail: normalizeOptionalString(approval?.detail), + messages: normalizePendingApprovalMessages(approval?.messages), + }; +} + +export function resolvePendingApproval( + approval: PendingApprovalRecord, + decision: ApprovalDecision, + resolvedAt: string, + detail?: string, +): PendingApprovalRecord { + return { + ...approval, + status: decision, + resolvedAt, + detail: normalizeOptionalString(detail) ?? approval.detail, + }; +} + +function normalizePendingApprovalMessages( + messages?: ReadonlyArray>, +): PendingApprovalMessageRecord[] | undefined { + if (!messages || messages.length === 0) { + return undefined; + } + + const normalized = messages.flatMap((message) => { + const id = normalizeOptionalString(message.id); + const authorName = normalizeOptionalString(message.authorName); + if (!id || !authorName) { + return []; + } + + return [{ + id, + authorName, + content: message.content ?? '', + }]; + }); + + return normalized.length > 0 ? normalized : undefined; +} + +function normalizeOptionalString(value: string | undefined): string | undefined { + const trimmed = value?.trim(); + return trimmed ? trimmed : undefined; +} + +function normalizeStringArray(values?: ReadonlyArray): string[] { + if (!values) { + return []; + } + + return [...new Set(values.map((value) => value.trim()).filter((value) => value.length > 0))]; +} diff --git a/src/shared/domain/pattern.ts b/src/shared/domain/pattern.ts index 8da7239..785d7e1 100644 --- a/src/shared/domain/pattern.ts +++ b/src/shared/domain/pattern.ts @@ -1,4 +1,9 @@ import type { ChatMessageRecord } from '@shared/domain/session'; +import { + normalizeApprovalPolicy, + type ApprovalPolicy, + validateApprovalPolicy, +} from '@shared/domain/approval'; export type OrchestrationMode = | 'single' @@ -36,6 +41,7 @@ export interface PatternDefinition { availability: PatternAvailability; unavailabilityReason?: string; maxIterations: number; + approvalPolicy?: ApprovalPolicy; agents: PatternAgentDefinition[]; createdAt: string; updatedAt: string; @@ -315,6 +321,17 @@ export function validatePatternDefinition(pattern: PatternDefinition): PatternVa } } + for (const message of validateApprovalPolicy( + normalizeApprovalPolicy(pattern.approvalPolicy), + pattern.agents.map((agent) => agent.id), + )) { + issues.push({ + level: 'error', + field: 'approvalPolicy', + message, + }); + } + return issues; } diff --git a/src/shared/domain/runTimeline.ts b/src/shared/domain/runTimeline.ts index 7860062..d65f3b1 100644 --- a/src/shared/domain/runTimeline.ts +++ b/src/shared/domain/runTimeline.ts @@ -1,3 +1,8 @@ +import type { + ApprovalCheckpointKind, + ApprovalDecision, + PendingApprovalRecord, +} from '@shared/domain/approval'; import type { PatternDefinition, ReasoningEffort } from '@shared/domain/pattern'; import type { ProjectRecord } from '@shared/domain/project'; import { createId } from '@shared/utils/ids'; @@ -9,6 +14,7 @@ export type RunTimelineEventKind = | 'thinking' | 'handoff' | 'tool-call' + | 'approval' | 'message' | 'run-completed' | 'run-failed'; @@ -34,6 +40,12 @@ export interface RunTimelineEventRecord { targetAgentId?: string; targetAgentName?: string; toolName?: string; + approvalId?: string; + approvalKind?: ApprovalCheckpointKind; + approvalTitle?: string; + approvalDetail?: string; + permissionKind?: string; + decision?: ApprovalDecision; messageId?: string; content?: string; error?: string; @@ -84,6 +96,17 @@ export interface UpsertRunMessageEventInput { error?: string; } +function approvalStatusToRunStatus(status: PendingApprovalRecord['status']): RunTimelineEventStatus { + switch (status) { + case 'approved': + return 'completed'; + case 'rejected': + return 'error'; + default: + return 'running'; + } +} + function normalizeOptionalString(value: string | undefined): string | undefined { const trimmed = value?.trim(); return trimmed ? trimmed : undefined; @@ -129,6 +152,12 @@ function normalizeRunTimelineEvent( targetAgentId: normalizeOptionalString(event.targetAgentId), targetAgentName: normalizeOptionalString(event.targetAgentName), toolName: normalizeOptionalString(event.toolName), + approvalId: normalizeOptionalString(event.approvalId), + approvalKind: event.approvalKind, + approvalTitle: normalizeOptionalString(event.approvalTitle), + approvalDetail: normalizeOptionalString(event.approvalDetail), + permissionKind: normalizeOptionalString(event.permissionKind), + decision: event.decision, messageId: normalizeOptionalString(event.messageId), content: event.content, error: normalizeOptionalString(event.error), @@ -317,6 +346,60 @@ export function upsertSessionRunRecord( return nextRuns; } +export function upsertRunApprovalEvent( + run: SessionRunRecord, + approval: PendingApprovalRecord, +): SessionRunRecord { + const existingIndex = run.events.findIndex( + (event) => event.kind === 'approval' && event.approvalId === approval.id, + ); + const nextStatus = approvalStatusToRunStatus(approval.status); + const nextEvent: RunTimelineEventRecord = { + id: existingIndex >= 0 ? run.events[existingIndex].id : createId('run-event'), + kind: 'approval', + occurredAt: + existingIndex >= 0 ? run.events[existingIndex].occurredAt : approval.requestedAt, + updatedAt: approval.status === 'pending' ? undefined : approval.resolvedAt, + status: nextStatus, + agentId: normalizeOptionalString(approval.agentId), + agentName: normalizeOptionalString(approval.agentName), + toolName: normalizeOptionalString(approval.toolName), + approvalId: approval.id, + approvalKind: approval.kind, + approvalTitle: approval.title, + approvalDetail: normalizeOptionalString(approval.detail), + permissionKind: normalizeOptionalString(approval.permissionKind), + decision: approval.status === 'pending' ? undefined : approval.status, + }; + + if (existingIndex < 0) { + return appendRunTimelineEvent(run, nextEvent); + } + + const existingEvent = run.events[existingIndex]; + if ( + existingEvent.updatedAt === nextEvent.updatedAt + && existingEvent.status === nextEvent.status + && existingEvent.agentId === nextEvent.agentId + && existingEvent.agentName === nextEvent.agentName + && existingEvent.toolName === nextEvent.toolName + && existingEvent.approvalKind === nextEvent.approvalKind + && existingEvent.approvalTitle === nextEvent.approvalTitle + && existingEvent.approvalDetail === nextEvent.approvalDetail + && existingEvent.permissionKind === nextEvent.permissionKind + && existingEvent.decision === nextEvent.decision + ) { + return run; + } + + const nextEvents = run.events.slice(); + nextEvents[existingIndex] = nextEvent; + return { + ...run, + events: nextEvents, + }; +} + export function appendRunActivityEvent( run: SessionRunRecord, input: AppendRunActivityEventInput, diff --git a/src/shared/domain/session.ts b/src/shared/domain/session.ts index a44294f..61483ab 100644 --- a/src/shared/domain/session.ts +++ b/src/shared/domain/session.ts @@ -4,6 +4,7 @@ import { normalizeSessionToolingSelection, type SessionToolingSelection, } from '@shared/domain/tooling'; +import type { PendingApprovalRecord } from '@shared/domain/approval'; import type { SessionRunRecord } from '@shared/domain/runTimeline'; export type ChatRole = 'system' | 'user' | 'assistant'; @@ -39,6 +40,7 @@ export interface SessionRecord { lastError?: string; scratchpadConfig?: ScratchpadSessionConfig; tooling?: SessionToolingSelection; + pendingApproval?: PendingApprovalRecord; runs: SessionRunRecord[]; } diff --git a/src/shared/domain/sessionLibrary.ts b/src/shared/domain/sessionLibrary.ts index 681eeb9..2714cff 100644 --- a/src/shared/domain/sessionLibrary.ts +++ b/src/shared/domain/sessionLibrary.ts @@ -181,6 +181,7 @@ export function duplicateSessionRecord( enabledLspProfileIds: [...session.tooling.enabledLspProfileIds], } : undefined, + pendingApproval: undefined, runs: [], messages: session.messages.map((message): ChatMessageRecord => ({ ...message, diff --git a/tests/main/runTurnPending.test.ts b/tests/main/runTurnPending.test.ts index 97dfa40..a0642f5 100644 --- a/tests/main/runTurnPending.test.ts +++ b/tests/main/runTurnPending.test.ts @@ -15,6 +15,7 @@ describe('run turn pending helpers', () => { reject: (error) => rejected.push(error), onDelta: () => undefined, onActivity: () => undefined, + onApproval: () => undefined, errored: false, }; @@ -36,6 +37,7 @@ describe('run turn pending helpers', () => { reject: () => undefined, onDelta: () => undefined, onActivity: () => undefined, + onApproval: () => undefined, errored: false, }; diff --git a/tests/renderer/runTimelineFormatting.test.ts b/tests/renderer/runTimelineFormatting.test.ts index 8e7e0c7..99fc606 100644 --- a/tests/renderer/runTimelineFormatting.test.ts +++ b/tests/renderer/runTimelineFormatting.test.ts @@ -66,6 +66,16 @@ describe('run timeline formatting', () => { agentName: 'Writer', toolName: 'file_search', }))).toBe('Writer used file_search'); + expect(formatEventLabel(createEvent({ + kind: 'approval', + status: 'running', + approvalTitle: 'Approve tool access', + }))).toBe('Approve tool access'); + expect(formatEventLabel(createEvent({ + kind: 'approval', + status: 'completed', + approvalTitle: 'Approve final response', + }))).toBe('Approve final response approved'); expect(formatEventLabel(createEvent({ kind: 'message', agentName: 'Reviewer' }))).toBe('Reviewer'); expect(formatEventLabel(createEvent({ kind: 'run-completed' }))).toBe('Completed'); expect(formatEventLabel(createEvent({ kind: 'run-failed' }))).toBe('Failed'); diff --git a/tests/shared/approval.test.ts b/tests/shared/approval.test.ts new file mode 100644 index 0000000..6bbd19a --- /dev/null +++ b/tests/shared/approval.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, test } from 'bun:test'; + +import { + approvalPolicyRequiresCheckpoint, + normalizeApprovalPolicy, + normalizePendingApproval, +} from '@shared/domain/approval'; + +describe('approval helpers', () => { + test('normalizes duplicate checkpoint rules into stable agent-scoped policy entries', () => { + expect(normalizeApprovalPolicy({ + rules: [ + { kind: 'tool-call', agentIds: ['agent-1', ' agent-1 ', 'agent-2'] }, + { kind: 'tool-call', agentIds: ['agent-2', 'agent-3'] }, + { kind: 'final-response', agentIds: [] }, + ], + })).toEqual({ + rules: [ + { kind: 'tool-call', agentIds: ['agent-1', 'agent-2', 'agent-3'] }, + { kind: 'final-response' }, + ], + }); + }); + + test('matches approval requirements for all-agent and agent-specific rules', () => { + const policy = normalizeApprovalPolicy({ + rules: [ + { kind: 'tool-call', agentIds: ['agent-1'] }, + { kind: 'final-response', agentIds: [] }, + ], + }); + + expect(approvalPolicyRequiresCheckpoint(policy, 'tool-call', 'agent-1')).toBe(true); + expect(approvalPolicyRequiresCheckpoint(policy, 'tool-call', 'agent-2')).toBe(false); + expect(approvalPolicyRequiresCheckpoint(policy, 'final-response', 'agent-2')).toBe(true); + }); + + test('normalizes pending approvals with optional message previews', () => { + expect(normalizePendingApproval({ + id: 'approval-1', + kind: 'final-response', + status: 'pending', + requestedAt: '2026-03-24T10:00:00.000Z', + title: 'Approve final response', + messages: [ + { + id: 'msg-1', + authorName: 'Primary Agent', + content: 'Draft answer', + }, + ], + })).toEqual({ + id: 'approval-1', + kind: 'final-response', + status: 'pending', + requestedAt: '2026-03-24T10:00:00.000Z', + title: 'Approve final response', + messages: [ + { + id: 'msg-1', + authorName: 'Primary Agent', + content: 'Draft answer', + }, + ], + }); + }); +}); diff --git a/tests/shared/pattern.test.ts b/tests/shared/pattern.test.ts index 26d448d..70d80f0 100644 --- a/tests/shared/pattern.test.ts +++ b/tests/shared/pattern.test.ts @@ -101,4 +101,28 @@ describe('pattern validation', () => { expect(groupChat?.agents[1].instructions).toContain('specific improvements'); expect(groupChat?.agents[1].instructions).toContain('instead of restarting the conversation'); }); + + test('approval policy rejects unknown agent references', () => { + const singlePattern = createBuiltinPatterns(BUILTIN_TIMESTAMP).find( + (pattern) => pattern.mode === 'single', + ); + + expect(singlePattern).toBeDefined(); + + const issues = validatePatternDefinition({ + ...singlePattern!, + approvalPolicy: { + rules: [ + { + kind: 'tool-call', + agentIds: ['agent-missing'], + }, + ], + }, + }); + + expect(issues.find((issue) => issue.field === 'approvalPolicy')?.message).toBe( + 'Approval checkpoint "tool-call" references unknown agent "agent-missing".', + ); + }); }); diff --git a/tests/shared/runTimeline.test.ts b/tests/shared/runTimeline.test.ts index e1801ba..4e7e8a9 100644 --- a/tests/shared/runTimeline.test.ts +++ b/tests/shared/runTimeline.test.ts @@ -6,9 +6,11 @@ import { completeSessionRunRecord, createSessionRunRecord, normalizeSessionRunRecords, + upsertRunApprovalEvent, upsertRunMessageEvent, } from '@shared/domain/runTimeline'; import type { ProjectRecord } from '@shared/domain/project'; +import type { PendingApprovalRecord } from '@shared/domain/approval'; function createPattern(): PatternDefinition { return { @@ -168,4 +170,44 @@ describe('run timeline helpers', () => { test('normalizes missing run collections to an empty array', () => { expect(normalizeSessionRunRecords(undefined)).toEqual([]); }); + + test('tracks approval checkpoints as a single timeline event that can be resolved later', () => { + const baseRun = createSessionRunRecord({ + requestId: 'turn-1', + project: createProject(), + workspaceKind: 'project', + pattern: createPattern(), + triggerMessageId: 'msg-user-1', + startedAt: '2026-03-23T00:00:01.000Z', + }); + + const pendingApproval: PendingApprovalRecord = { + id: 'approval-1', + kind: 'tool-call', + status: 'pending', + requestedAt: '2026-03-23T00:00:02.000Z', + agentId: 'agent-writer', + agentName: 'Writer', + title: 'Approve tool access', + permissionKind: 'tool access', + }; + + const pendingRun = upsertRunApprovalEvent(baseRun, pendingApproval); + const resolvedRun = upsertRunApprovalEvent(pendingRun, { + ...pendingApproval, + status: 'approved', + resolvedAt: '2026-03-23T00:00:03.000Z', + }); + + const approvalEvent = resolvedRun.events.find((event) => event.kind === 'approval'); + expect(approvalEvent).toMatchObject({ + approvalId: 'approval-1', + approvalKind: 'tool-call', + approvalTitle: 'Approve tool access', + permissionKind: 'tool access', + status: 'completed', + decision: 'approved', + updatedAt: '2026-03-23T00:00:03.000Z', + }); + }); }); diff --git a/tests/shared/sessionLibrary.test.ts b/tests/shared/sessionLibrary.test.ts index d03f0a8..b4fc26c 100644 --- a/tests/shared/sessionLibrary.test.ts +++ b/tests/shared/sessionLibrary.test.ts @@ -122,6 +122,13 @@ describe('session library helpers', () => { isPinned: true, isArchived: true, lastError: 'sidecar crashed', + pendingApproval: { + id: 'approval-1', + kind: 'tool-call', + status: 'pending', + requestedAt: '2026-03-23T00:01:00.000Z', + title: 'Approve tool access', + }, messages: [ { id: 'msg-1', @@ -149,6 +156,7 @@ describe('session library helpers', () => { updatedAt: '2026-03-23T00:07:00.000Z', }); expect(session.messages[0]?.pending).toBe(false); + expect(session.pendingApproval).toBeUndefined(); expect(session.runs).toEqual([]); });