fix(auto-reply): gate inline skill tool dispatch [AI] by pgondhi987 · Pull Request #78517 · openclaw/openclaw

pgondhi987 · 2026-05-06T15:08:43Z

Summary

Problem: inline skill commands that dispatch tools used a direct execution path after sender and owner-only checks.
Why it matters: inline command dispatch should apply the same tool hook decisions and parameter adjustments as other tool invocation surfaces.
What changed: added a before-tool-call hook check before inline tool execution, passed adjusted parameters into the tool, and returned a blocked reply when the hook denies execution.
What did NOT change (scope boundary): base sender authorization, owner-only filtering, tool registration, and non-tool skill prompt rewriting are unchanged.

AI-assisted: yes.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Related: N/A
This PR addresses a bug or regression

Real behavior proof (required for external PRs)

Behavior or issue addressed: inline skill tool dispatch now runs the before-tool-call hook before executing the selected tool.
Real environment tested: not exercised in a live OpenClaw setup for this branch metadata draft.
Exact steps or command run after this patch: targeted formatter check on the touched files.
Evidence after fix: targeted regression coverage added in src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts.
Observed result after fix: hook blocks return a blocked reply and the tool execute function is not called.
What was not tested: live channel command execution and full changed-gate validation.
Before evidence: N/A.

Root Cause (if applicable)

Root cause: the inline skill command tool-dispatch path called tool.execute directly after owner-only filtering.
Missing detection / guardrail: there was no regression test asserting before-tool-call hook handling on inline tool dispatch.
Contributing context (if known): other tool invocation surfaces already run the hook before execution, but this path had separate dispatch glue.

Regression Test Plan (if applicable)

Coverage level that should have caught this:
- Unit test
- Seam / integration test
- End-to-end test
- Existing coverage already sufficient
Target test or file: src/auto-reply/reply/get-reply-inline-actions.skip-when-config-empty.test.ts
Scenario the test should lock in: a registered inline skill command dispatches to a tool, the before-tool-call hook returns a block decision, the reply reports the block, and the tool does not execute.
Why this is the smallest reliable guardrail: it exercises the inline dispatch seam directly with mocked tool construction and hook behavior.
Existing test that already covers this (if any): none for this inline dispatch hook decision.
If no new test is added, why not: N/A.

User-visible / Behavior Changes

Inline skill tool commands can now return a blocked-tool reply when a configured tool hook denies the call. No config changes are required.

Diagram (if applicable)

Before:
[inline skill command] -> [owner-only filter] -> [tool execution]

After:
[inline skill command] -> [owner-only filter] -> [before-tool-call hook] -> [blocked reply or tool execution]

Security Impact (required)

New permissions/capabilities? (Yes/No): No
Secrets/tokens handling changed? (Yes/No): No
New/changed network calls? (Yes/No): No
Command/tool execution surface changed? (Yes/No): Yes
Data access scope changed? (Yes/No): No
If any Yes, explain risk + mitigation: inline skill tool dispatch now applies existing hook decisions before execution; this reduces policy inconsistency without adding new tool capabilities.

Repro + Verification

Environment

OS: Linux workspace
Runtime/container: local source tree
Model/provider: N/A
Integration/channel (if any): inline auto-reply skill command path
Relevant config (redacted): tool loop detection context is passed from the existing OpenClaw config.

Steps

Register a skill command whose dispatch kind is tool.
Make the before-tool-call hook return a block decision for the dispatched tool.
Invoke handleInlineActions for an authorized sender.

Expected

The hook decision is checked before execution.
A blocked reply is returned.
The tool execute function is not called.

Actual

The added regression test asserts the expected blocked path.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios: reviewed the inline tool-dispatch path and added regression coverage for a blocked hook decision.
Edge cases checked: hook-adjusted parameters are used for execution when the hook allows the call; blocked hook decisions skip execution.
What you did not verify: live channel execution, full test suite, and full changed-gate validation.

Review Conversations

I replied to or resolved every bot review conversation I addressed in this PR.
I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

Backward compatible? (Yes/No): Yes
Config/env changes? (Yes/No): No
Migration needed? (Yes/No): No
If yes, exact upgrade steps: N/A

Risks and Mitigations

Risk: inline skill tool commands may now stop earlier when an existing hook denies or adjusts the call.
- Mitigation: this matches the intended behavior of other tool invocation paths and is covered by a focused unit test.

clawsweeper · 2026-05-06T15:12:03Z

Codex review: needs real behavior proof before merge.

Summary
The branch wraps OpenClaw tool construction with before-tool-call hooks by default, opts existing shared tool surfaces out, passes target session/channel context into inline skill tool dispatch, returns blocked-tool replies, and adds focused tests.

Reproducibility: yes. source-reproducible: current main's inline skill tool path calls tool.execute directly, while the documented hook contract and runtime wrapper define block and parameter-rewrite behavior. I did not run a live channel repro in this read-only review.

Real behavior proof
Needs real behavior proof before merge: The PR body explicitly says no live OpenClaw setup was exercised and only lists formatter plus regression-test evidence. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Contributor proof is required and maintainers must handle the protected label and decide whether this PR or #78525 is the canonical fix.

Security
Cleared: The diff narrows an existing tool-policy bypass using existing hook infrastructure and adds no dependencies, workflow changes, secret handling, network calls, or new permissions.

Review findings

[P3] Add the required changelog entry — src/auto-reply/reply/get-reply-inline-actions.ts:320-327

Review details

Best possible solution:

Land one maintained fix that routes inline skill tool dispatch through existing tool-policy and before_tool_call enforcement with target-session context, changelog coverage, and real terminal/log or live-channel proof.

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

Yes, source-reproducible: current main's inline skill tool path calls tool.execute directly, while the documented hook contract and runtime wrapper define block and parameter-rewrite behavior. I did not run a live channel repro in this read-only review.

Is this the best way to solve the issue?

Yes for the implementation shape: using the existing hook wrapper with target-session context is the narrow maintainable seam. It is not merge-ready until proof, changelog coverage, and maintainer selection against the overlapping PR are resolved.

Full review comments:

[P3] Add the required changelog entry — src/auto-reply/reply/get-reply-inline-actions.ts:320-327
This changes user-visible security/tool-policy behavior by allowing inline skill tool commands to return a blocked-tool reply when before_tool_call vetoes execution, but the PR diff does not update CHANGELOG.md. Add one single-line entry under the active section before merge.
Confidence: 0.9

Overall correctness: patch is correct
Overall confidence: 0.86

What I checked:

Current main bypasses hook enforcement: The inline skill tool branch builds toolArgs and calls tool.execute(toolCallId, toolArgs) directly after owner-only filtering, without invoking the before_tool_call hook path. (src/auto-reply/reply/get-reply-inline-actions.ts:304, 2d65ead914c7)
Hook contract defines block and parameter rewrite behavior: The plugin hook docs state that before_tool_call receives tool name, params, session context fields, and can return params rewrites or block:true as terminal behavior. Public docs: docs/plugins/hooks.md. (docs/plugins/hooks.md:153, 2d65ead914c7)
Existing wrapper enforces the contract: wrapToolWithBeforeToolCallHook calls runBeforeToolCallHook before execution, returns a blocked result for vetoes, and executes with outcome.params when allowed. (src/agents/pi-tools.before-tool-call.ts:620, 2d65ead914c7)
PR head routes inline dispatch through wrapped tools: The diff adds default construction-time wrapping in createOpenClawTools and passes the target session id/current channel id from inline skill dispatch. (src/agents/openclaw-tools.ts:446, 44cd8f300f5a)
PR head handles blocked tool results: The inline dispatch path now passes the abort signal to execute, detects details.status === "blocked", and returns a blocked-tool reply instead of a normal success reply. (src/auto-reply/reply/get-reply-inline-actions.ts:320, 44cd8f300f5a)
Real behavior proof is mock/test-only: The PR body says a live OpenClaw setup was not exercised and lists formatter plus regression-test coverage as after-fix evidence. (44cd8f300f5a)

Likely related people:

steipete: Recent commits touch the inline action file, hook runtime, lazy-loader boundaries, and loop-detection behavior used by this PR. (role: recent maintainer; confidence: high; commits: 59fb9e5ca7fe, b1cfba2fc285, 6a55a00da461; files: src/auto-reply/reply/get-reply-inline-actions.ts, src/agents/pi-tools.before-tool-call.ts, src/agents/openclaw-tools.ts)
vincentkoc: Recent work covers inline auto-reply behavior, tool execution diagnostics, trace context, and current tool construction paths. (role: adjacent maintainer; confidence: high; commits: 468c6a0101e9, 9aa9c3ff6262, cead2ea4b106; files: src/auto-reply/reply/get-reply-inline-actions.ts, src/agents/pi-tools.before-tool-call.ts, src/agents/openclaw-tools.ts)
Takhoffman: Introduced target-session fixes in the same inline action path that the hook context must preserve. (role: adjacent owner; confidence: high; commits: 1fb8a8cdff3d, 8f94032dc117, 84fb20aa5210; files: src/auto-reply/reply/get-reply-inline-actions.ts)
100yenadmin: Co-authored recent plugin SDK hook contract work that feeds the before_tool_call API this PR relies on. (role: hook contract contributor; confidence: medium; commits: 1adaa28dc86b, cb3853587576; files: src/agents/pi-tools.before-tool-call.ts, docs/plugins/hooks.md)
pgondhi987: Beyond authoring this PR, prior merged current-main work hardened the before_tool_call hook runner to fail closed on errors. (role: prior hook-path contributor; confidence: medium; commits: e19dce0aedbc; files: src/agents/pi-tools.before-tool-call.ts)

Remaining risk / open question:

External real behavior proof is still missing; the PR body only provides formatter and regression-test evidence.
The protected maintainer label and overlapping fix(skills): honor tool policy for inline dispatch #78525 require maintainer selection before merge.
The user-visible tool-policy behavior change still lacks a CHANGELOG.md entry.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 2d65ead914c7.

pgondhi987 · 2026-05-07T06:03:42Z

Not applicable; changelog is handled at merge time.

Quoted comment from @clawsweeper:

Codex review: needs real behavior proof before merge.

Summary
The branch adds a before-tool-call hook check to inline skill command tool dispatch, executes hook-adjusted parameters, and adds focused inline action tests for allow/block hook outcomes.

Reproducibility: Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main's inline skill tool path directly calls tool.execute, while documented OpenClaw-owned tool hooks go through runBeforeToolCallHook; I did not run a live channel repro in this read-only review.

Real behavior proof
Needs real behavior proof before merge: Mock-only: the PR body says no live OpenClaw setup was exercised, and focused regression coverage does not satisfy the external PR real-behavior gate. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, ask a maintainer to comment @clawsweeper re-review.

Next step before merge
Contributor must add real behavior proof and address the session-context and changelog findings; the protected maintainer label also requires explicit maintainer handling.

Security
Needs attention: Needs attention because the added policy hook can evaluate plugin policy and loop detection against a stale session id.

Review findings

[P2] Use the target session entry for hook context — src/auto-reply/reply/get-reply-inline-actions.ts:319

[P3] Add the required changelog entry — src/auto-reply/reply/get-reply-inline-actions.ts:314

Review details

Best possible solution:

Keep the hook-gating direction, but compute and use the target session entry for hook context, add the changelog entry, and require live or terminal/log proof before merge.

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

Do we have a high-confidence way to reproduce the issue? Yes from source inspection: current main's inline skill tool path directly calls tool.execute, while documented OpenClaw-owned tool hooks go through runBeforeToolCallHook; I did not run a live channel repro in this read-only review.

Is this the best way to solve the issue?

Is this the best way to solve the issue? No as written: adding the hook is the right seam, but the hook context should use the selected target session entry and the PR still needs changelog and real behavior proof.

Full review comments:

[P2] Use the target session entry for hook context — src/auto-reply/reply/get-reply-inline-actions.ts:319
The new hook context is built before targetSessionEntry is computed and reads sessionEntry?.sessionId directly. In target-session callers that pass a wrapper entry plus sessionStore[sessionKey], before_tool_call policy and loop detection see the wrong ephemeral sessionId, unlike the existing inline status, command, and abort-cutoff paths. Move target session resolution before this branch and use that entry here.
Confidence: 0.86

[P3] Add the required changelog entry — src/auto-reply/reply/get-reply-inline-actions.ts:314
This changes user-visible security behavior by allowing inline skill tool commands to return a blocked-tool reply, but the PR only touches source and tests. Add a single CHANGELOG.md entry under the active release section per OpenClaw policy.
Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.87

Security concerns:

[medium] Session-scoped hook context can be stale — src/auto-reply/reply/get-reply-inline-actions.ts:319
The added hook context uses sessionEntry?.sessionId instead of the target entry selected from sessionStore[sessionKey]. Plugins that scope before_tool_call policy by ephemeral sessionId can evaluate inline skill tool commands against the wrong conversation state.
Confidence: 0.82

What I checked:

Current main bypasses before_tool_call for inline skill tools: Current main builds inline skill tool arguments and calls tool.execute directly without runBeforeToolCallHook. (src/auto-reply/reply/get-reply-inline-actions.ts:304, 99b17263a12d)

Hook context is session-scoped: runBeforeToolCallHook uses sessionKey and sessionId for loop detection state and passes the same context fields to trusted/plugin hook policy. (src/agents/pi-tools.before-tool-call.ts:428, 99b17263a12d)

PR uses raw sessionEntry for the new hook context: The PR head builds hookContext before targetSessionEntry is computed and reads sessionEntry.sessionId, while existing inline status/command/abort paths use sessionStore[sessionKey] ?? sessionEntry. (src/auto-reply/reply/get-reply-inline-actions.ts:319, 6aa69ec63737)

Plugin hook docs define param rewrite and terminal block semantics: The docs say before_tool_call receives tool name, params, session context fields, can rewrite params, and block: true is terminal. Public docs: docs/plugins/hooks.md. (docs/plugins/hooks.md:153, 99b17263a12d)

Changelog entry is absent from the PR diff: The PR changes user-visible security behavior but only modifies the inline action source and its test file; CHANGELOG.md is not in the PR files. (CHANGELOG.md:5, 6aa69ec63737)

Real behavior proof is mock-only: The PR body says no live OpenClaw setup was exercised and lists focused regression coverage as the after-fix evidence. (6aa69ec63737)

Likely related people:

vincentkoc: Recent current-main work on inline action behavior and tool hook diagnostics makes this route relevant for both the inline dispatch and hook context surfaces. (role: recent maintainer; confidence: high; commits: 9aa9c3ff6262, 6775611c5d68, bcdacfa1b3df; files: src/auto-reply/reply/get-reply-inline-actions.ts, src/agents/pi-tools.before-tool-call.ts)

Takhoffman: Introduced the target-session fixes in the same inline action file that this PR needs to follow for hook context. (role: adjacent owner; confidence: high; commits: 1fb8a8cdff3d, 8f94032dc117, 84fb20aa5210; files: src/auto-reply/reply/get-reply-inline-actions.ts)

steipete: Recent work covers lazy import loaders, tool loop detection configuration, and before-tool-call loop behavior depended on by this patch. (role: recent maintainer; confidence: medium; commits: 59fb9e5ca7fe, ed4b223cf252, 1af6855bb0a6; files: src/auto-reply/reply/get-reply-inline-actions.ts, src/agents/pi-tools.before-tool-call.ts, src/agents/tool-loop-detection-config.ts)

pgondhi987: Beyond authoring this PR, they previously landed before_tool_call fail-closed hardening in current main. (role: prior hook-path contributor; confidence: medium; commits: e19dce0aedbc; files: src/agents/pi-tools.before-tool-call.ts)

Remaining risk / open question:

Live channel inline command behavior remains unverified because the PR body explicitly reports no live OpenClaw setup was exercised.

Several check runs for the latest head were still queued or pending during inspection.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 99b17263a12d.

Re-review progress:

State: Complete
Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
Run: https://github.com/openclaw/clawsweeper/actions/runs/25484334286
Updated: 2026-05-07T08:19:08.780Z

# Conflicts: # src/agents/openclaw-tools.ts

* fix: enforce tool hooks for inline skill dispatch * addressing claude review * addressing codex review * addressing codex review * fix: complete root-cause handling * docs: add changelog entry for PR merge

fix: enforce tool hooks for inline skill dispatch

69e9041

openclaw-barnacle Bot added size: S maintainer Maintainer-authored PR labels May 6, 2026

addressing claude review

6aa69ec

clawsweeper Bot mentioned this pull request May 6, 2026

fix(skills): honor tool policy for inline dispatch #78525

Open

pgondhi987 added 3 commits May 7, 2026 06:08

addressing codex review

6f68888

addressing codex review

36c470d

fix: complete root-cause handling

c807b4e

openclaw-barnacle Bot added gateway Gateway runtime agents Agent runtime and tooling size: M and removed size: S labels May 7, 2026

pgondhi987 added 3 commits May 7, 2026 07:57

Merge remote-tracking branch 'origin/main' into fix/fix-564

44cd8f3

# Conflicts: # src/agents/openclaw-tools.ts

Merge remote-tracking branch 'origin/main' into fix/fix-564

3793952

docs: add changelog entry for PR merge

ff2a3d8

pgondhi987 merged commit d5eabbd into openclaw:main May 7, 2026
19 of 21 checks passed

github-actions Bot mentioned this pull request May 7, 2026

📡 Upstream Digest — 2026-05-07 10:14 UTC curtismercier/openclaw-mods#790

Open

github-actions Bot mentioned this pull request May 9, 2026

Update ghcr.io/openclaw/openclaw Docker tag to v2026.5.7 - autoclosed claytono/infra#1977

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(auto-reply): gate inline skill tool dispatch [AI]#78517

fix(auto-reply): gate inline skill tool dispatch [AI]#78517
pgondhi987 merged 8 commits intoopenclaw:mainfrom
pgondhi987:fix/fix-564

pgondhi987 commented May 6, 2026

Uh oh!

clawsweeper Bot commented May 6, 2026 •

edited

Loading

Uh oh!

pgondhi987 commented May 7, 2026 •

edited by clawsweeper Bot

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

pgondhi987 commented May 6, 2026

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Real behavior proof (required for external PRs)

Root Cause (if applicable)

Regression Test Plan (if applicable)

User-visible / Behavior Changes

Diagram (if applicable)

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Review Conversations

Compatibility / Migration

Risks and Mitigations

Uh oh!

clawsweeper Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pgondhi987 commented May 7, 2026 • edited by clawsweeper Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

clawsweeper Bot commented May 6, 2026 •

edited

Loading

pgondhi987 commented May 7, 2026 •

edited by clawsweeper Bot

Loading