Skip to content

fix(sessions): seed the immediate-retry approval bypass for every approved scope - #1800

Merged
Aaronontheweb merged 3 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/shell-approval-redirect-candidate
Aug 7, 2026
Merged

fix(sessions): seed the immediate-retry approval bypass for every approved scope#1800
Aaronontheweb merged 3 commits into
netclaw-dev:devfrom
Aaronontheweb:fix/shell-approval-redirect-candidate

Conversation

@Aaronontheweb

@Aaronontheweb Aaronontheweb commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Closes #1802

The bug

A shell command the user approved could still fail with "I encountered an error executing a tool." This is the trigger behind the self-hosted/DeepSeek approval failures (the history-wedge amplifier was fixed separately in #1796).

Confirmed mechanism (not theorized)

For a piped command like gh api … 2>/dev/null | base64 -d | head, the approval gate extracts these candidates:

gh api | <cwd>-ish   ;   base64 | (no directory)   ;   head | (no directory)

The standalone pipeline verbs base64 / head have no path argument, so:

  • Always here (persistent, directory-scoped) cannot persist a grant for them — their directory defaults to cwd, and when cwd is the session scratch dir the dead-on-arrival guard drops them. This is by design (you can't fold curl/gh/base64 to a directory).
  • This chat (session-scoped) records them verb-only, so it works.

The one-time bypass that lets the just-approved call run this once was seeded only for ApprovedOnce (SessionToolExecutionPipeline.cs). So ApprovedSession / ApprovedAlways re-hit the gate on retry, the uncovered verbs read as unapproved, and the turn failed — for a command the user had just approved. In a parallel batch approved call-by-call this looked like one sibling running (session, verb-only match) and the other throwing (persistent grant didn't cover the standalone verbs), with no approval_near_miss logged (nothing was persisted to compare) — exactly what the production logs showed.

The fix

Seed the one-time bypass for the just-approved call regardless of scope, in both the live pipeline and the cold re-drive plan (LlmSessionActor.BuildApprovalRedrivePlan). This matches the sub-agent loop (SubAgentActor), which already seeds for every approved scope and documents why. The bypass is per-call, pattern-scoped, and cleared after the attempt, so it only authorizes the immediate retry the user approved; broader scopes still record their durable grant separately for future calls. It does not change what "Always here" persists (standalone verbs remaining non-persistable is intentional).

Reproduction & tests

A pipeline regression test drives ApprovedAlways against an executor that requires the bypass on retry (standing in for a command whose durable grant doesn't cover every verb): ToolExecutionFailed before, runs after. Full Sessions + Tools suites stay green (1331 passed), including the existing ApprovedOnce / ApprovedSession / ApprovedAlways pipeline tests, the shell-approval matrix, and all sub-agent tests.

…roved scope

A shell command the user approved could still throw ToolApprovalRequiredException
on its immediate retry. When the command is a pipeline, its standalone verbs
(base64, head, ...) have no path argument, so "Always here" cannot persist a
directory-scoped grant for them (by design). The one-time bypass that lets a
just-approved call run this once was seeded ONLY for ApprovedOnce, so
ApprovedSession / ApprovedAlways re-hit the gate on retry — the verbs the durable
grant could not cover read as unapproved — and the turn failed with
"I encountered an error executing a tool" for a command the user had approved.

This is the trigger behind the self-hosted/DeepSeek approval failures. In a
parallel batch approved call-by-call it presented as one sibling running and the
other throwing (session-scoped verb-only match worked; the persistent grant did
not cover the standalone verbs).

Seed the one-time bypass for the just-approved call regardless of scope, in both
the live pipeline (SessionToolExecutionPipeline) and the cold re-drive plan
(LlmSessionActor.BuildApprovalRedrivePlan). This matches the sub-agent loop, which
already seeds for every approved scope. The bypass is per-call, pattern-scoped,
and cleared after the attempt, so it only authorizes the immediate retry the user
approved; broader scopes still record their durable grant separately for future
calls.

Regression test drives ApprovedAlways through the pipeline against an executor
that requires the bypass on retry: ToolExecutionFailed before, runs after.
@Aaronontheweb Aaronontheweb added bug Something isn't working reliability Retries, resilience, graceful degradation sessions LLM session actor, turn lifecycle, pipelines shell Issues related to the shell tool, since it has the largest security perimeter. labels Aug 7, 2026
…dicate

Route the three "approval granted" seed sites through one predicate instead of
duplicating the four-scope list: the live pipeline retry
(SessionToolExecutionPipeline), the cold re-drive plan
(LlmSessionActor.BuildApprovalRedrivePlan), and the sub-agent loop
(SubAgentActor). Adds ApprovalDecision.IsApprovalGrant() and the
ParentApprovalDecision twin.

A future approval scope now updates one predicate, not three. Missing a site had
reintroduced exactly the "approved command still fails on retry" bug this branch
fixes, for the new scope (addresses the code-review maintainability finding).
Reference the tracking issue from the pipeline/session-actor fix comments and
the pipeline regression test, and trim the inline explanation now that the
issue carries the full analysis.

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

/// <summary>
/// Extensions over <see cref="ParentApprovalDecision"/>.
/// </summary>
public static class ParentApprovalDecisionExtensions

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

// Retry execution now that approval is granted
// (Approve-once is retried through transient context state; broader scopes
// are also recorded by the session actor into the shared approval service.)
if (decision == ApprovalDecision.ApprovedOnce)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Aaronontheweb
Aaronontheweb enabled auto-merge (squash) August 7, 2026 19:27
@Aaronontheweb
Aaronontheweb disabled auto-merge August 7, 2026 19:52
@Aaronontheweb
Aaronontheweb merged commit 20211fa into netclaw-dev:dev Aug 7, 2026
15 checks passed
@Aaronontheweb
Aaronontheweb deleted the fix/shell-approval-redirect-candidate branch August 7, 2026 19:52
@Aaronontheweb Aaronontheweb mentioned this pull request Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working reliability Retries, resilience, graceful degradation sessions LLM session actor, turn lifecycle, pipelines shell Issues related to the shell tool, since it has the largest security perimeter.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Approved shell command (This chat / Always) still fails with "error executing a tool" on retry

1 participant