Skip to content

fix(exec): downgrade ask policy for autonomous sessions (#94599)#94654

Closed
mmyzwl wants to merge 4 commits into
openclaw:mainfrom
mmyzwl:fix/issue-94599-approvals
Closed

fix(exec): downgrade ask policy for autonomous sessions (#94599)#94654
mmyzwl wants to merge 4 commits into
openclaw:mainfrom
mmyzwl:fix/issue-94599-approvals

Conversation

@mmyzwl

@mmyzwl mmyzwl commented Jun 18, 2026

Copy link
Copy Markdown
Contributor

Summary

When agents execute explicitly-assigned tasks (cron jobs, subagents), they currently request manual approval for every command, even when exec.ask is set to "always". This defeats the purpose of autonomous task execution — users who explicitly schedule a cron job or spawn a subagent expect those tasks to execute without repeated approval prompts.

This fix adds an autonomousSession parameter to requiresExecApproval() that downgrades the effective ask policy from "always" to "on-miss" for cron and subagent sessions. The allowlist security boundary is fully preserved — commands that fail allowlist analysis still require approval regardless of session type.

Fixes #94599

Real behavior proof

Behavior addressed: Agents performing explicitly-assigned tasks (cron jobs, subagents) stop and request manual approval for commands that are clearly within the scope of the assigned task.

Real setup tested:

  • Runtime: Linux 4.19.112-2.el8.x86_64, Node 24.x, Vitest 4.1.8
  • Repo: openclaw/openclaw main branch

Exact steps or command run after fix:

cd openclaw
pnpm install
pnpm run test -- src/infra/exec-approvals-policy.test.ts
pnpm run test -- src/node-host/exec-policy.test.ts
pnpm run test -- src/infra/exec-approvals-allow-always.test.ts
pnpm run test -- src/agents/bash-tools.exec-host-gateway.test.ts

After-fix evidence:

 ✓ exec-approvals-policy.test.ts   72/72 passed (includes 7 new autonomousSession tests)
 ✓ exec-policy.test.ts             18/18 passed (includes 4 new autonomousSession tests)
 ✓ exec-approvals-allow-always     45/45 passed (no regression)
 ✓ exec-host-gateway.test.ts       29/29 passed (no regression)

Observed result after the fix: Autonomous sessions with ask="always" now have their effective ask policy downgraded to "on-miss". Commands that pass allowlist analysis execute without approval. Commands that fail allowlist analysis still require approval — the security boundary is preserved.

What was not tested: End-to-end cron job execution with a running gateway, interactive session behavior with real agent turns.

Tests and validation

Test Suite Result Details
exec-approvals-policy.test.ts 72/72 ✅ 7 new tests for autonomousSession parameter
exec-policy.test.ts 18/18 ✅ 4 new tests for autonomousSession in system run policy
exec-approvals-allow-always.test.ts 45/45 ✅ No regression
exec-host-gateway.test.ts 29/29 ✅ No regression
TypeScript compilation ✅ No errors on modified files

Risk checklist

Did user-visible behavior change? No — the change only affects autonomous sessions (cron/subagent) with ask="always". Interactive sessions are completely unaffected, and the default behavior is preserved.

Did config, environment, or migration behavior change? No — no new config keys, no migration needed. Uses existing isCronSessionKey()/isSubagentSessionKey() detection.

Did security, auth, secrets, network, or tool execution behavior change? Yes — autonomous sessions with ask="always" now downgrade to "on-miss" behavior. The allowlist security boundary is preserved: commands that fail allowlist analysis still require approval.

What is the highest-risk area?

  • Downgrading ask="always" to "on-miss" for autonomous sessions means the allowlist becomes the sole gate for those sessions, rather than having both ask="always" AND the allowlist as gates.

How is that risk mitigated?

  • The allowlist boundary is fully preserved — commands that fail allowlist analysis still require human approval
  • The change is minimal (1 parameter added to an existing function)
  • Only affects sessions already detected as cron/subagent by existing, well-tested utilities
  • All existing tests continue to pass

Current review state

What is the next action?

  • Maintainer review requested

⚠️ ClawSweeper Advisory

This issue has ClawSweeper labels that should be considered:

  • clawsweeper:no-new-fix-pr — automated fix PR was not recommended
  • clawsweeper:needs-product-decision — product behavior decision needed
  • clawsweeper:needs-security-review — security review needed
  • clawsweeper:needs-maintainer-review — maintainer review needed

This PR implements a minimal, opt-in approach that preserves all security defaults. Please review the product and security implications before merging.

mmyzwl and others added 3 commits June 18, 2026 00:10
@mmyzwl @claude
perf(mcp): parallelize MCP server connections in getCatalog to reduce…
3e3dd40 
… prep latency

Every agent request incurred 6-7s of prep latency because bundle-tools
connected to configured MCP servers sequentially, one at a time. With
4-5 MCP servers at ~1.5s each (default tools/list timeout), the total
was the sum of all servers' connection times.

Fix: split getCatalog() into two phases:
1. Synchronous pre-computation of safe server names (fast, sequential)
2. Async connection + tool listing (parallelized via Promise.allSettled)

Now MCP servers connect and list tools concurrently, reducing the total
latency from the sum of all servers to roughly the slowest single server.
Each server still has its own error handling — individual failures are
gracefully demoted to diagnostics, not fatal to the catalog.

Prep stage timing change:
  Before: bundle-tools = sum(connection + listTools) for each server
  After:  bundle-tools = max(connection + listTools) across all servers

Closes openclaw#94162

Co-Authored-By: Claude <noreply@anthropic.com>
@mmyzwl @claude
fix(mcp): add missing braces for eslint curly rule
0d2f1cd 
Two if-statements lacked braces, failing the CI check-lint job.

Co-Authored-By: Claude <noreply@anthropic.com>
@mmyzwl @claude
fix(exec): downgrade ask policy for autonomous sessions
a9e82a0 
- Add autonomousSession parameter to requiresExecApproval() so cron jobs and
  subagents don't require repeated manual approval for explicitly assigned tasks
- When autonomousSession is true, downgrade ask="always" to "on-miss" so the
  allowlist security boundary is still enforced for all sessions
- Detect autonomous sessions from sessionKey using existing isCronSessionKey()
  and isSubagentSessionKey() utilities across gateway, node-host, and system-run paths
- Add comprehensive tests verifying allowlist boundary is preserved and
  backward compatibility is maintained for interactive sessions

Fixes openclaw#94599

Co-Authored-By: Claude <noreply@anthropic.com>
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: M proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 18, 2026
@clawsweeper

clawsweeper Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 18, 2026, 1:17 PM ET / 17:17 UTC.

Summary
The PR changes exec approval policy so cron/subagent session keys downgrade ask="always" to on-miss, threads that flag through gateway and node-host exec paths, adds tests, and also includes an MCP catalog parallelization rewrite from a separate PR.

PR surface: Source +97, Tests +159. Total +256 across 8 files.

Reproducibility: Partly/source-level: current main and docs show that ask="always" prompts on every command, including durable trust cases. I did not establish a high-confidence real cron/subagent reproduction showing the unwanted prompt sequence inside an autonomous run.

Review metrics: 3 noteworthy metrics.

  • Effective ask policy downgrade: 1 added. The PR adds one path that overrides documented ask="always" behavior for autonomous sessions, which maintainers should approve before merge.
  • Unrelated runtime overlap: 1 open PR duplicated. The branch carries MCP catalog parallelization that is already under review separately, so this PR is not a clean exec-only landing path.
  • MCP startup fan-out: 1 sequential loop changed to concurrent fan-out. The unrelated MCP change can affect peak subprocess/network activity and availability characteristics, so it needs separate proof and review.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94599
Summary: This PR is a candidate fix for the approval-policy issue, while the MCP catalog rewrite carried in the branch belongs to a separate adjacent PR.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Remove the MCP catalog commits and leave that work to the focused MCP performance PR.
  • [P1] Add redacted real cron or subagent proof showing in-scope commands proceed and unsafe commands still require approval.
  • Get maintainer product and security approval before changing ask="always" semantics.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies test output only and explicitly says no end-to-end cron job, gateway, subagent, or real agent-turn behavior was tested after the change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Existing users who deliberately configured ask="always" for cron or subagent sessions would silently lose the every-command approval prompt in those sessions.
  • [P1] Session-key classification would become implicit permission to reuse allowlist trust, which broadens a command-execution security boundary before maintainers have approved that policy.
  • [P1] The supplied proof is test output only and does not show a real cron or subagent run where allowed commands proceed and unsafe commands still require approval.
  • [P1] The branch carries an unrelated MCP catalog concurrency rewrite that duplicates #94162: Performance: bundle-tools loading adds 6-7s latency on every agent request #94230 and adds separate availability/proof risk to this exec-policy PR.

Maintainer options:

  1. Restore explicit approval semantics before merge (recommended)
    Keep ask="always" strict by default, remove the unrelated MCP commits, and move any autonomous no-prompt behavior behind a maintainer-approved explicit policy with docs and proof.
  2. Sponsor the autonomous trust policy
    Maintainers can intentionally accept the ask="always" downgrade, but that should include a visible security decision, upgrade notes, and real cron/subagent behavior proof.
  3. Replace with a narrow branch
    If the mixed branch is too costly to repair, pause or close it and ask for a new PR containing only the approved exec approval policy change.

Next step before merge

  • [P1] The remaining blockers are maintainer product/security direction, branch scope, and contributor real-behavior proof rather than a narrow automated repair.

Security
Needs attention: The diff weakens a command-execution approval boundary by silently downgrading ask="always" for autonomous sessions.

Review findings

  • [P1] Keep ask=always strict unless maintainers approve the policy — src/infra/exec-approvals.ts:1229
  • [P1] Drop the unrelated MCP runtime rewrite — src/agents/agent-bundle-mcp-runtime.ts:570-600
Review details

Best possible solution:

Keep ask="always" strict by default, split the MCP work back to its focused PR, and only add autonomous approval suppression through a maintainer-approved explicit policy with docs, upgrade notes, and real cron/subagent proof.

Do we have a high-confidence way to reproduce the issue?

Partly/source-level: current main and docs show that ask="always" prompts on every command, including durable trust cases. I did not establish a high-confidence real cron/subagent reproduction showing the unwanted prompt sequence inside an autonomous run.

Is this the best way to solve the issue?

No. The patch is a plausible shortcut, but the better fix is a maintainer-approved explicit autonomous approval policy that preserves strict ask="always" by default and documents the security and upgrade behavior.

Full review comments:

  • [P1] Keep ask=always strict unless maintainers approve the policy — src/infra/exec-approvals.ts:1229
    This rewrites explicit ask="always" into on-miss for cron/subagent sessions. Current docs and tests define always as prompting on every command, so existing users who chose that stricter mode would silently lose an approval gate unless this becomes an approved opt-in policy.
    Confidence: 0.89
  • [P1] Drop the unrelated MCP runtime rewrite — src/agents/agent-bundle-mcp-runtime.ts:570-600
    The branch also rewrites SessionMcpRuntime.getCatalog() for MCP catalog concurrency, which is unrelated to the approval-policy fix and is already under review at #94162: Performance: bundle-tools loading adds 6-7s latency on every agent request #94230. Keeping it here makes this a mixed-purpose landing candidate with separate availability and proof requirements.
    Confidence: 0.94

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8b34b8537a90.

Label changes

Label changes:

  • add merge-risk: 🚨 availability: The bundled MCP rewrite changes server startup/listing from serialized work to concurrent fan-out, which can affect stalls or resource pressure in configured environments.

Label justifications:

  • P2: This is a normal-priority approval-policy change with meaningful autonomous workflow impact, but it is not a proven urgent regression or emergency.
  • merge-risk: 🚨 compatibility: Merging would change existing ask="always" behavior for cron/subagent sessions without an opt-in setting, migration, or upgrade proof.
  • merge-risk: 🚨 security-boundary: Merging would reduce manual approval gates for command execution in autonomous sessions, which is a security-sensitive boundary change.
  • merge-risk: 🚨 availability: The bundled MCP rewrite changes server startup/listing from serialized work to concurrent fan-out, which can affect stalls or resource pressure in configured environments.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies test output only and explicitly says no end-to-end cron job, gateway, subagent, or real agent-turn behavior was tested after the change. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +97, Tests +159. Total +256 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 6 218 121 +97
Tests 2 159 0 +159
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 8 377 121 +256

Security concerns:

  • [medium] Implicitly downgrades an explicit approval policy — src/infra/exec-approvals.ts:1229
    Users who configured ask="always" currently get a prompt for every command; changing that to on-miss for cron/subagent sessions reduces operator review without an explicit opt-in or maintainer-approved security policy.
    Confidence: 0.9

What I checked:

  • Repository policy applied: Root AGENTS.md and the scoped src/agents guide were read fully; their compatibility, security-boundary, agent-runtime, and real-behavior-proof guidance applies to this PR. (AGENTS.md:7, 8b34b8537a90)
  • Current ask=always source contract: Current main returns true immediately for requiresExecApproval() when ask is always, before durable approval or allowlist state can suppress the prompt. (src/infra/exec-approvals.ts:1228, 8b34b8537a90)
  • Current ask=always docs contract: The exec approvals docs state that always prompts on every command and durable allow-always trust does not suppress prompts in effective always mode. Public docs: docs/tools/exec-approvals.md. (docs/tools/exec-approvals.md:163, 8b34b8537a90)
  • Current tests cover strict ask=always: Node-host policy tests assert that ask=always still requires approval even when durable trust is present. (src/node-host/exec-policy.test.ts:95, 8b34b8537a90)
  • PR changes approval boundary: At the PR head, the added effectiveAsk expression converts autonomous ask="always" sessions to on-miss, changing a documented approval boundary. (src/infra/exec-approvals.ts:1229, a9e82a05e9e9)
  • PR proof is test-only: The PR body lists unit test output and explicitly says end-to-end cron job execution, interactive session behavior, and real agent turns were not tested. (a9e82a05e9e9)

Likely related people:

  • jesse-merhi: Recent GitHub history shows this person authored the Tree-sitter command authorization rebuild touching src/infra/exec-approvals.ts and both gateway/node exec paths used by this PR. (role: recent exec authorization contributor; confidence: high; commits: c9707ab635b9; files: src/infra/exec-approvals.ts, src/agents/bash-tools.exec-host-gateway.ts, src/agents/bash-tools.exec-host-node.ts)
  • vincentkoc: Recent commits touched state-dir approvals, inline exec approval decisions, exec approval security events, and exec helper exports in the same approval surface. (role: recent approval-policy contributor; confidence: high; commits: adad27d7448e, df86f36a5728, 481fd10988f6; files: src/infra/exec-approvals.ts, src/agents/bash-tools.exec-host-gateway.ts, src/agents/bash-tools.exec-host-node.ts)
  • joshavant: The normalized exec auto mode and adjacent node/gateway auto-review commits are directly related to reducing human prompts without redefining strict ask="always". (role: feature-history contributor; confidence: medium; commits: 80227005a016, 2bfc7350505e, 49ab52894a57; files: src/infra/exec-approvals.ts, src/agents/exec-defaults.ts, src/node-host/exec-policy.ts)
  • steipete: Recent history shows exec/node approval docs work and several MCP runtime/operator workflow commits, so this person is relevant to both the approval contract and the unrelated MCP rewrite carried by the branch. (role: adjacent docs and MCP runtime contributor; confidence: medium; commits: 5a869eea5a28, bbdfba569440, 99ce71ddbbdb; files: src/node-host/exec-policy.ts, docs/tools/exec-approvals.md, src/agents/agent-bundle-mcp-runtime.ts)
  • openperf: Recent MCP lease lifecycle work touched the same runtime manager that this PR rewrites in its unrelated MCP commit. (role: recent MCP lifecycle contributor; confidence: medium; commits: f2530de8320e; files: src/agents/agent-bundle-mcp-runtime.ts, src/agents/agent-bundle-mcp-runtime.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jun 18, 2026
@mmyzwl @claude
chore: resolve merge conflict in exec-host-node.ts
c8f4c74 
Co-Authored-By: Claude <noreply@anthropic.com>
@mmyzwl
mmyzwl requested a review from a team as a code owner June 18, 2026 17:23
@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime cli CLI command changes scripts Repository scripts commands Command implementations docker Docker and sandbox tooling channel: qqbot extensions: qa-lab extensions: codex extensions: copilot size: XL and removed size: M labels Jun 18, 2026
@mmyzwl mmyzwl closed this Jun 18, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 20, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling app: web-ui App: web-ui channel: qqbot cli CLI command changes commands Command implementations docker Docker and sandbox tooling extensions: codex extensions: copilot extensions: qa-lab gateway Gateway runtime merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. scripts Repository scripts size: XL status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: approvals

1 participant

@mmyzwl