fix(subagents): harden approval lifecycle by Aaronontheweb · Pull Request #1217 · netclaw-dev/netclaw

Aaronontheweb · 2026-05-28T15:45:38Z

Summary

Add the redesign-subagent-approval-lifecycle OpenSpec change for Redesign sub-agent approval lifecycle #1212.
Fail missing parent approval bridges closed as terminal failed subagent runs without executing gated tools.
Harden subagent approval waits for watchdog pause/resume, approve-once isolation, denied/timed-out tool results, cancellation, external stop, parallel waits, and late approval expiry.
Update the subagents runbook with user-visible approval behavior.

Validation

dotnet test "src/Netclaw.Actors.Tests/Netclaw.Actors.Tests.csproj" --filter "FullyQualifiedName~SubAgentActorTests|FullyQualifiedName~ParentSessionApprovalBridgeTests|FullyQualifiedName~ToolApprovalGateTests"
openspec validate "redesign-subagent-approval-lifecycle" --strict
dotnet slopwatch analyze
pwsh "./scripts/Add-FileHeaders.ps1" -Verify
git diff --check

Fixes #1212

Refs #1212

Aaronontheweb · 2026-05-28T20:14:00Z

    // path arguments the agent originally passed, rather than collapsing to cwd.
    IReadOnlyList<ApprovalCandidate> Candidates) : INoSerializationVerificationNeeded;

+internal sealed record ToolInteractionRequestDispatch(


This wrapper separates the approval event from the persistence decision. Parent-session approvals can be journaled and redriven after restart; sub-agent approvals are only valid while the child actor is alive. ToolInteractionRequest stays as the UI/client-facing prompt, while ToolInteractionRequestDispatch tells the session actor whether this specific prompt should become durable approval state.

Aaronontheweb · 2026-05-28T20:14:01Z

-        var tcs = _pending.GetOrAdd(callId, _ => new TaskCompletionSource<ApprovalDecision>(TaskCreationOptions.RunContinuationsAsynchronously));
+        var tcs = new TaskCompletionSource<ApprovalDecision>(TaskCreationOptions.RunContinuationsAsynchronously);
+        if (!_pending.TryAdd(callId, tcs))
+            throw new InvalidOperationException($"Approval wait for call '{callId}' is already pending.");


This is a fail-fast guard for call-id collisions. The old GetOrAdd behavior could let two waits share the same approval TCS, meaning one user click could release more than one blocked tool call. Duplicate waits should be treated as a bug, not merged.

Aaronontheweb · 2026-05-28T20:14:02Z

+    /// Returns false when the prompt is no longer backed by a live wait.
    /// </summary>
-    void Complete(ToolCallId callId, ApprovalDecision decision);
+    bool TryClaim(ToolCallId callId, out ClaimedApprovalWait wait);


TryClaim makes “is this prompt still live?” and “remove it from the pending map” one atomic step. We need that before writing any broader approval grant, otherwise a stale or double-clicked prompt could still persist an approval even though no live tool call is waiting for it.

Aaronontheweb · 2026-05-28T20:14:03Z

-        var waitTask = _channel.WaitForApprovalAsync(callId, Timeout.InfiniteTimeSpan, ct);
+        EnsureAuthorityContext();
+
+        var parentCallId = CreateParentCallId(callId);


Sub-agent tool call IDs are child-local, so two sub-agents can both produce the same call ID. The parent approval channel is shared at the session level, so the approval prompt needs a parent-scoped ID that includes the spawning call and a sequence number.

Aaronontheweb · 2026-05-28T20:14:04Z

+        return new ToolCallId($"{_approvalScopeId}/subagent-approval/{requestId}/{childCallId.Value}");
+    }
+
+    private void EnsureAuthorityContext()


A sub-agent approval has to be bound back to the parent requester. If we do not have enough parent authority context, there is no safe user or principal to authorize against, so this fails closed instead of emitting an approval prompt that might be approved by the wrong person.

Aaronontheweb · 2026-05-28T20:14:05Z

                return;
            }

+            if (!_approvalChannel.TryClaim(msg.CallId, out var approvalWait))


The live path now validates the response first, then claims the live wait, then persists broader grants. That order is intentional: authorization alone is not enough to write a grant. The prompt also has to still correspond to a currently blocked tool call.

Aaronontheweb · 2026-05-28T20:14:07Z

+            RequestedAtMs = NowMs()
+        };
+
+        if (!dispatch.PersistApprovalState)


PersistApprovalState: false means this pending entry is only in-memory routing state for a live sub-agent prompt. Once the live wait is claimed, we clear it instead of journaling ToolApprovalResolved, because there is no durable child actor or tool batch to redrive after restart.

Aaronontheweb · 2026-05-28T20:14:08Z

        PrincipalClassification? requesterPrincipalOverride = null,
-        IReadOnlyDictionary<string, ApprovalDecision>? decisionOverride = null)
+        IReadOnlyDictionary<string, ApprovalDecision>? decisionOverride = null,
+        CancellationToken ct = default)


Tool execution now uses a session-owned cancellation token. Previously, direct approval waits used CancellationToken.None, so a session stop, restart, or failed turn could leave a tool task blocked until some external timeout. The session actor now owns cancellation for the whole live tool-execution lifecycle.

Aaronontheweb · 2026-05-28T20:14:09Z

+    /// human approval prompt. The watchdog resumes on the next non-suspending
+    /// activity item or terminal completion.
+    /// </summary>
+    public bool SuspendsInactivityWatchdog { get; init; }


Waiting on a human approval is not a stalled tool. This flag lets the parent streaming watchdog pause while the sub-agent is legitimately blocked on approval, then resume when normal activity or completion comes through.

Aaronontheweb · 2026-05-28T20:14:11Z

                }
+                catch (ToolApprovalRequiredException)
+                {
+                    throw new ParentApprovalUnavailableException(


Missing parent approval plumbing is a wiring/security failure, not a normal tool error. Throwing ParentApprovalUnavailableException fails the sub-agent run loudly instead of returning an Error: tool result that the LLM might treat as recoverable and continue past.

Aaronontheweb · 2026-05-28T20:14:24Z

    {
        _executionCts?.Cancel();
        _externalCts?.Cancel();
+        if (!_completed)


If the actor is stopped externally while the parent is waiting on an Ask, the caller still needs a terminal result. The _completed guard prevents double replies when normal completion and PostStop race.

Aaronontheweb · 2026-05-28T20:21:47Z

+
+    private ToolCallId CreateParentCallId(ToolCallId childCallId)
+    {
+        var requestId = Interlocked.Increment(ref _nextApprovalRequestId);


This is intentionally not actor-confined state. The bridge is created by the session actor, then used by sub-agent/tool tasks on the thread pool, and multiple child tool calls can request approval concurrently. Interlocked.Increment keeps the parent-visible approval IDs unique without adding an actor round trip just to allocate a sequence number.

Aaronontheweb

LGTM

Aaronontheweb · 2026-05-28T20:39:05Z

+        // Child call ids are only unique inside the sub-agent's tool loop. The
+        // parent approval channel is session-wide, so include the spawning tool
+        // call scope plus a per-bridge sequence before the child-local id.
+        return new ToolCallId($"{_approvalScopeId}/subagent-approval/{requestId}/{childCallId.Value}");


makes sense

Aaronontheweb · 2026-05-28T20:39:46Z

+        // Approval responses are authorized against the parent requester. If we
+        // cannot reconstruct that authority context, emitting a prompt would let
+        // the channel decide without a safe requester binding.
+        if (_requesterPrincipal is null)


this can happen in instances like skills that request a specific sub-agent for execution. It's a headless session without a parent, so there is no one to run the approval bridge.

Aaronontheweb · 2026-05-28T20:40:15Z

+    private void EmitActivity(string phase, bool suspendsInactivityWatchdog = false)
+        => _activitySink?.TryWrite(new ToolActivityUpdate(phase)
+        {
+            SuspendsInactivityWatchdog = suspendsInactivityWatchdog


signal that we are waiting for a human, not timing out due to LLM inference provider issues

Aaronontheweb added the subagents spawn_agent, SubAgentActor, definition loader, discovery context layer, and related features label May 28, 2026

Aaronontheweb added 5 commits May 28, 2026 19:28

fix(subagents): harden approval lifecycle

bee52eb

Refs #1212

fix(subagents): fail missing approval bridge closed

1519c71

Refs #1212

fix(subagents): complete approval lifecycle

6b3cbc1

fix(subagents): close approval race gaps

7ae6cb9

refactor(approvals): trim live wait state

16998a2

Aaronontheweb force-pushed the fix/1212-subagent-approval-lifecycle branch from b3af96a to 16998a2 Compare May 28, 2026 19:33

Aaronontheweb commented May 28, 2026

View reviewed changes

docs(approvals): document lifecycle invariants

dad264a

Aaronontheweb commented May 28, 2026

View reviewed changes

Aaronontheweb merged commit d50eb9c into dev May 28, 2026
20 checks passed

Aaronontheweb deleted the fix/1212-subagent-approval-lifecycle branch May 28, 2026 20:40

Aaronontheweb mentioned this pull request May 31, 2026

Prepare release v0.22.0 #1256

Merged

Uh oh!

Conversation

Aaronontheweb commented May 28, 2026

Summary

Validation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aaronontheweb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant