fix(sdk-message-processor): preserve activeTaskToolIds for a pending sub-agent permission on task_notification (lr-b75006)

[clagentic-builder]

What changed

A sibling guard in lib/sdk-message-processor.js task_notification handler was incomplete. The handler deleted session.activeTaskToolIds[parentId] unconditionally, while a nearby guard (added by lr-9d4b) protects session.subagentToolOwners against the identical race: task_notification can arrive while a sub-agent own nested permission (e.g. its last Bash call) is still awaiting the operator click.

partitionSubagentOwnedPermissions (lib/sdk-permission-ownership.js), which result and processQueryStream finally rely on to decide whether to preserve a sub-agent-owned pending permission across turn-boundary cleanup, requires BOTH owningTaskId AND activeTaskToolIds[owningTaskId] to hold. Guarding only subagentToolOwners left the other half false, so the entry was force-resolved to deny and purged from session.pendingPermissions and sm.permissionRequestIndex by sweepClearedPermissionIndex -- while the browser permission card was never told to update, so it stayed rendered and clickable. Every subsequent click (Allow Once / Allow for Session / Deny) then hit the same dead pendingPermissions lookup in project-sessions.js permission_response handler and silently no-opped -- matching the reported symptom of none of the three buttons sticking.

Fix: compute the same live-ownership snapshot task_notification already uses to guard subagentToolOwners, and use it to also defer releasing activeTaskToolIds[parentId] and the activity token while a live permission still references this Task. Deferring the activity-token release is safe -- the token own tool_result release path, per-turn generation bump, and idle-sweep backstop all still apply once the permission resolves.

Why (investigation summary)

This is lr-b75006, reported as a P1: Allow, Allow-for-Session, and Deny all failed to stop a re-firing Bash permission prompt during crew-manifest orchestration work. Per the mandated investigation order:

• Key-mismatch hypothesis (checked first, refuted): write and read both compute the grant key via the same permissionGrantKey(toolName, input) helper (lib/utils.js) -- no shape divergence between write and lookup sides.
• Key granularity for Bash: keyed on bare tool name (Bash), not the full command string.
• Durable-storage gap (lr-8b2e): ruled out -- allowedTools persistence/flush-on-grant machinery is intact and unchanged.
• Grant landing on a different session object (engram 7494706): ruled out directly -- there is no separate session object for a sub-agent; a Task sub-agent tool calls flow through the SAME top-level session object and the SAME handleCanUseTool/permissionRequestIndex as the parent turn.
• Which of (a) click never delivered / (b) click delivered but did not persist / (c) grant persisted but not consulted: none of these exactly -- the mechanism is a distinct fourth shape: the grant resolver is force-cleared by server-side turn-boundary bookkeeping BEFORE the operator ever clicks, while the UI card is never told the request died. This presents identically to a broken click for all three buttons because they all resolve through the same now-dead entry.

Named predecessor: this is not a fresh independent cause and not a literal repeat of lr-8b2e, lr-f969dc, or lr-9d4b -- it is an incomplete application of lr-9d4b own fix. lr-9d4b added a guard so session.subagentToolOwners survives task_notification while a nested permission is pending, but did not mirror the same guard onto session.activeTaskToolIds, the other half of the AND-check partitionSubagentOwnedPermissions uses to decide preservation.

Verification (demonstrated failure then pass)

1. Wrote the regression test first, driving the real message-processor sequence: Task starts, sub-agent issues a Bash tool call, its permission is registered as pending, task_notification (Task completion) arrives while that permission is still pending, then later the parent turn result arrives.
2. Ran it against pre-fix code (fix temporarily stashed): failed exactly as predicted on the activeTaskToolIds survival assertion, full suite otherwise green (1583/1584, only the new test failing).
3. Restored the fix, reran: full suite green (1584/1584), including the new test and all pre-existing lr-9d4b and lr-f940 sibling coverage.

Regression coverage: test/sdk-message-processor-activetasktoolids-notification-race-lr-b75006.test.js. The message-processor layer is the right layer because it can directly observe the ordering between task_notification, a still-pending permission, and a later turn-boundary cleanup without a full daemon/WS/SDK harness.

Test status

npm test: 1584/1584 pass, 0 fail (full suite, post-fix).

TASK: lr-b75006

[clagentic-reviewer]

PEACHES — clean (0 findings)

Root-Cause Verification

PR claim: partitionSubagentOwnedPermissions (sdk-permission-ownership.js:47) requires BOTH owningTaskId AND activeTaskToolIds[owningTaskId] to be true. Pre-fix, only one half was guarded.

VERIFIED TRUE. Pre-fix sdk-message-processor.js line 947 had unconditional delete session.activeTaskToolIds[parentId] with NO guard. Existing lr-9d4b guard protected subagentToolOwners (lines 1015-1021) only. AND-check was failing its second half when task_notification fired before result.

Fix Completeness

Fix defers activeTaskToolIds deletion by computing liveOwnedToolIds (lines 944-948) and checking parentOwnsLivePermission (950-957), guarding the delete (986-989). Precomputed snapshot is reused by subagentToolOwners guard (1015-1021).

SYMMETRIC AND COMPLETE. Both halves of AND-check now defer. All ownership fields accounted for:

• pendingPermissions (held), subagentToolOwners (guarded), activeTaskToolIds (NOW guarded), taskIdMap (preserved via retainPreservedTaskBookkeeping at line 652).

Deferral Safety

Operator resolving, still-pending at result, sub-agent errors — all paths eventually clear the deferred entries. Ownership invariant maintained; cleanup paths unbroken.

Three Buttons Restored

All route through permission_response handler reading session.pendingPermissions[requestId]. Pre-fix entry was force-denied and PURGED while card stayed rendered; clicks no-op'd. Post-fix entry survives, Deny works.

Test Quality

Third instance of this class (lr-8b2e, lr-f969dc, lr-9d4b). New variant risk LOW. This PR adds missing end-to-end test: sdk-message-processor-activetasktoolids-notification-race-lr-b75006.test.js. Exercises REACHABILITY (task_notification → result → permission_response), not plumbing. Key assertion would FAIL on pre-fix code (activeTaskToolIds deletion causes partition to purge entry via sweepClearedPermissionIndex). Addresses tome #845 lesson 1.

{"reviewer": "peaches", "review_status": "clean", "head_sha": "68aa87d42e624a46f55bd08d6560463cf4c317a9", "pr_number": 419}

[clagentic-security]

BOBBIE -- clean

Security audit of PR #419 (lr-b75006), scope b4811a2..68aa87d (lib/sdk-message-processor.js task_notification handler plus new regression test). Focus per assignment: fail-open risk, implicit-authorization window, grant scope, cross-ownership, resource exhaustion.

1. FAIL-CLOSED DEFAULT PRESERVED. sdk-permission-ownership.js:105 (sweepClearedPermissionIndex, unmodified by this diff) still force-resolves any dropped pending permission to a deny behavior with message Session turn ended. The second test in the new file (Deny still works normally for an ordinary non-subagent permission after a result cleanup, lines 230-262) asserts the top-level no-live-sub-agent case is still fully cleared -- no new bypass introduced.

2. NO IMPLICIT-AUTHORIZATION WINDOW. Traced every consumer of activeTaskToolIds. It is read in exactly one place as a gate: sdk-permission-ownership.js:47, owningTaskId and activeTaskToolIds[owningTaskId], inside partitionSubagentOwnedPermissions, which only decides whether a pendingPermissions entry (and its resolver) survives turn-boundary cleanup -- never whether a tool call is authorized. The actual grant path is project-sessions.js:1358-1506 (permission_response handler), keyed purely by requestId via sm.permissionRequestIndex/session.pendingPermissions, entirely independent of activeTaskToolIds. sdk-message-processor.js:934-957 computes parentOwnsLivePermission from session.pendingPermissions/session.subagentToolOwners BEFORE any cleanup, then at line 986 only skips the release/delete when a live permission is still outstanding -- the operator click is still required to resolve it. Extending the bookkeeping lifetime does not extend any authorization.

3. GRANT SCOPE UNCHANGED. utils.permissionGrantKey(pending.toolName, pending.toolInput) at project-sessions.js:1489, the lr-f969dc fine-grained fix, is untouched -- this diff touches only lib/sdk-message-processor.js and a new test file, no lines in project-sessions.js or utils.js. lr-f969dc over-granting shape (bare toolName key) is not reintroduced or widened.

4. NO CROSS-OWNERSHIP LEAKAGE. parentOwnsLivePermission (sdk-message-processor.js:949-957) matches only session.subagentToolOwners[polk] === parentId for the specific parentId being notified -- it cannot attribute a sibling sub-agent pending permission to a different Task, and all state is session-scoped (no cross-session reads introduced).

5. RESOURCE EXHAUSTION: bounded, not a new leak. Deferring the activeTaskToolIds[parentId] delete mirrors the existing lr-9d4b bounded exception already applied to subagentToolOwners -- one entry held per outstanding backgrounded-sub-agent permission, cleared as soon as the permission resolves (click) or the session is deleted. Same pre-existing architectural shape, not a new unbounded-growth path introduced by this PR.

6. Test file test/sdk-message-processor-activetasktoolids-notification-race-lr-b75006.test.js reviewed in full: reproduces the exact race end-to-end through the real processor (task_notification then result), asserts survival of pendingPermissions/subagentToolOwners/activeTaskToolIds, and includes an explicit sanity-check that the ordinary top-level deny-on-cleanup path is unchanged.

7. Tome 845 recurrence check (reports success while nothing happened): no new variant found. This fix does not introduce a success-shaped no-op; it symmetrizes an existing exception (subagentToolOwners) onto a second map (activeTaskToolIds) using the identical live-ownership computation, and the new test would fail on the incomplete pre-fix shape.

Scanners: semgrep --config auto against the 2 changed files -- 0 findings (200 rules, clean parse). gitleaks detect over b4811a2..68aa87d -- no leaks found. osv-scanner: not applicable, no dependency manifest or lockfile changed in this diff.

No blocking or nit findings.

{"reviewer": "bobbie", "review_status": "clean", "head_sha": "68aa87d42e624a46f55bd08d6560463cf4c317a9", "pr_number": 419}

[clagentic-merger]

Merged via clagentic-loadout v0.2.0

Field	Value
Gated HEAD SHA	68aa87d42e624a46f55bd08d6560463cf4c317a9
Merged SHA	68aa87d42e624a46f55bd08d6560463cf4c317a9
Reviews	clagentic-reviewer[bot], clagentic-security[bot]
CI status	no-runner-by-design (0 commit-status entries at HEAD)
task_id	lr-b75006