Skip to content

fix(agents): surface real plugin approval rejection reason to agent#100337

Merged
steipete merged 2 commits into
openclaw:mainfrom
tzy-17:fix/100212-plugin-approval-rejection-reason
Jul 5, 2026
Merged

fix(agents): surface real plugin approval rejection reason to agent#100337
steipete merged 2 commits into
openclaw:mainfrom
tzy-17:fix/100212-plugin-approval-rejection-reason

Conversation

@tzy-17

@tzy-17 tzy-17 commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #100212: When the gateway rejects a plugin approval request (e.g. schema validation → INVALID_REQUEST), the agent receives a hardcoded "Plugin approval required (gateway unavailable)" message. The gateway is actually healthy and actively rejected the request — the misleading message points debugging in the wrong direction.

Real-world impact: This misled operators at Clawnify.com during a production incident where an overlong approval title (98 chars, cap is 80) caused every mutating tool call to fail. The agent, seeing "gateway unavailable," concluded the target app was broken — while the gateway was perfectly healthy.

Why This Change Was Made

The catch block around the plugin.approval.request gateway call in src/agents/agent-tools.before-tool-call.ts returned a fixed string regardless of error type. Now we distinguish:

  • GatewayClientRequestError (gateway code present) → "Plugin approval request rejected: <real message>" — gateway is up, request was invalid
  • Other errors (connection refused, timeout, etc.) → "Plugin approval required (gateway unavailable)" — genuine transport failure

User Impact

  • Agents and operators now see the real rejection reason, enabling faster root-cause analysis
  • Gateway-unavailability detection still works correctly for actual transport failures
  • Backward compatible: all existing tests pass unchanged

Evidence

  • Source-repro: agent-tools.before-tool-call.ts:931-935 verified as the single hardcoded-message site
  • pnpm test src/agents/agent-tools.before-tool-call.embedded-mode.test.ts — 15 tests pass
  • pnpm test src/agents/agent-tools.before-tool-call.e2e.test.ts — 58 tests pass
  • oxlint passes on changed file

🤖 Generated with Claude Code

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS labels Jul 5, 2026
@clawsweeper

clawsweeper Bot commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed July 5, 2026, 3:27 PM ET / 19:27 UTC.

Summary
The branch imports gateway request-error handling, surfaces INVALID_REQUEST plugin approval validation messages to the agent, and adds before_tool_call e2e coverage for INVALID_REQUEST versus UNAVAILABLE.

PR surface: Source +9, Tests +38. Total +47 across 2 files.

Reproducibility: yes. by source inspection. Current main sends overlong plugin approval titles to plugin.approval.request, the gateway schema returns INVALID_REQUEST, and the current catch path maps that to the unavailable fallback.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #100212
Summary: This PR is a candidate fix for the canonical plugin approval INVALID_REQUEST diagnostic issue; several sibling PRs target the same root cause, and the broad draft batch partially overlaps with a phase-aware variant.

Members:

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

  • [P2] Separate request-phase and wait-decision INVALID_REQUEST handling and add focused e2e coverage for the wait-decision case.

Risk before merge

  • [P1] As written, the branch can still mislead agents for wait-decision INVALID_REQUEST errors by reporting them as request payload rejections.

Maintainer options:

  1. Decide the mitigation before merge
    Use one phase-aware approval error classifier: request-phase INVALID_REQUEST should surface validation text, wait-phase missing or expired approvals should get a separate no-longer-available or unavailable reason, and sibling PRs should close after one canonical fix lands.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P2] The remaining blocker is a narrow mechanical repair in the approval error classifier and its focused regression coverage.

Security
Cleared: No concrete security or supply-chain regression was found; the diff changes local error classification and tests, and surfaced messages use the shared redacting formatter.

Review findings

  • [P2] Keep wait-decision errors out of the request-rejected branch — src/agents/agent-tools.before-tool-call.ts:935-938
Review details

Best possible solution:

Use one phase-aware approval error classifier: request-phase INVALID_REQUEST should surface validation text, wait-phase missing or expired approvals should get a separate no-longer-available or unavailable reason, and sibling PRs should close after one canonical fix lands.

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

Yes, by source inspection. Current main sends overlong plugin approval titles to plugin.approval.request, the gateway schema returns INVALID_REQUEST, and the current catch path maps that to the unavailable fallback.

Is this the best way to solve the issue?

No. The PR is close, but the best fix must preserve request versus wait-decision phase so INVALID_REQUEST from plugin.approval.waitDecision is not mislabeled as a rejected request payload.

Full review comments:

  • [P2] Keep wait-decision errors out of the request-rejected branch — src/agents/agent-tools.before-tool-call.ts:935-938
    This catch wraps both plugin.approval.request and the later plugin.approval.waitDecision call. The gateway wait-decision handler also returns INVALID_REQUEST for approval expired or not found after registration, so this branch would tell the agent the approval request was rejected even when the wait phase failed. Track the request/wait phase, or otherwise scope this new message to the request call, and add coverage for waitDecision INVALID_REQUEST. Late discovery: the affected code is unchanged since the previous reviewed head cd0d64d7cc8b761c7832b4194203194eb966e9e2.
    Confidence: 0.88
    Late finding: first raised on code an earlier review cycle already covered.

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 f141408e7749.

Label changes

Label changes:

  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR discussion includes maintainer-reported sanitized AWS focused tests, real loopback Gateway proof through the production hook path, and a proof: sufficient label for the current head.
  • remove rating: 🦞 diamond lobster: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: ⏳ waiting on author.

Label justifications:

  • P2: This is a normal-priority agent/plugin approval diagnostic bug fix with limited blast radius.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR discussion includes maintainer-reported sanitized AWS focused tests, real loopback Gateway proof through the production hook path, and a proof: sufficient label for the current head.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR discussion includes maintainer-reported sanitized AWS focused tests, real loopback Gateway proof through the production hook path, and a proof: sufficient label for the current head.
Evidence reviewed

PR surface:

Source +9, Tests +38. Total +47 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 10 1 +9
Tests 1 38 0 +38
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 48 1 +47

Acceptance criteria:

  • [P1] pnpm test src/agents/agent-tools.before-tool-call.e2e.test.ts.
  • [P1] pnpm test src/agents/agent-tools.before-tool-call.embedded-mode.test.ts.
  • [P1] git diff --check.

What I checked:

Likely related people:

  • joshavant: They authored and merged the async requireApproval PR that added the plugin approval gateway method family and approval behavior. (role: introduced plugin approval surface; confidence: high; commits: 6ade9c474cf1; files: src/agents/agent-tools.before-tool-call.ts, src/gateway/server-methods/plugin-approval.ts, packages/gateway-protocol/src/schema/plugin-approvals.ts)
  • steipete: Current-main blame on the central approval files points to recent work by Peter Steinberger, and they supplied the current PR verification plus a phase-aware draft batch touching the same path. (role: recent area contributor and verifier; confidence: high; commits: 862de9f1a1c2, bdead2e003f4, 5c3366996a8a; files: src/agents/agent-tools.before-tool-call.ts, src/gateway/server-methods/plugin-approval.ts, packages/gateway-protocol/src/schema/plugin-approvals.ts)
  • vincentkoc: Recent history shows release-tag work touching the same approval source, gateway handler, and protocol schema files, though the shallow history limits deeper attribution. (role: recent release-area contributor; confidence: medium; commits: e085fa1a3ffd; files: src/agents/agent-tools.before-tool-call.ts, src/gateway/server-methods/plugin-approval.ts, packages/gateway-protocol/src/schema/plugin-approvals.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.
Review history (5 earlier review cycles)
  • reviewed 2026-07-05T14:16:08.039Z sha ed9b345 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-05T15:22:12.319Z sha e4844b9 :: needs real behavior proof before merge. :: [P2] Narrow the structured gateway error branch
  • reviewed 2026-07-05T18:16:27.932Z sha 7f74140 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T18:35:14.655Z sha 074c219 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-05T19:01:35.746Z sha cd0d64d :: needs maintainer review before merge. :: none

@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. labels Jul 5, 2026
@tzy-17
tzy-17 force-pushed the fix/100212-plugin-approval-rejection-reason branch from ed9b345 to e4844b9 Compare July 5, 2026 15:09
@harjothkhara

harjothkhara commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Source comparison for the #100212 cluster: I would not treat this branch as the best current fix yet. The runtime change is in the right catch block (requestPluginToolApproval in src/agents/agent-tools.before-tool-call.ts), and distinguishing a structured GatewayClientRequestError from a transport failure matches the gateway client contract. But this branch changes only that source file, so the exact regression remains unproved in the checked-in harness.

The stronger current lane is #100231 (with #100228 close behind) because it exercises both affected approval seams: src/agents/agent-tools.before-tool-call.e2e.test.ts for the gateway-call path and src/agents/agent-tools.before-tool-call.embedded-mode.test.ts for the embedded/deferred approval path. Those tests reject with real GatewayClientRequestError instances and prove INVALID_REQUEST becomes Plugin approval request rejected: ... while the generic transport-error case still returns Plugin approval required (gateway unavailable). #100231 also has green Real behavior proof and green relevant CI on its current head.

Best-fix verdict: #100337 is a plausible source patch, but #100231 is the better merge candidate for #100212 unless this branch adds equivalent regression coverage. I did not find a source reason to prefer the source-only branch over the tested one.

@steipete steipete self-assigned this Jul 5, 2026
@steipete

steipete commented Jul 5, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Maintainer verification for final rebased head bdead2e003f434d1e0eef38ad9c923b5685d5add:

  • Reworked the broad structured-error check: only Gateway INVALID_REQUEST means the approval payload was rejected; structured UNAVAILABLE and transport failures retain the fail-closed availability message.
  • Uses the shared error formatter, so the actionable validation reason reaches the agent without bypassing normal error redaction.
  • Sanitized AWS Crabbox run_2d57166e1404: 60 focused before-tool-call tests passed.
  • Sanitized AWS Crabbox run_c83e9120621b: real loopback Gateway plus the production hook path reproduced an overlong approval title and returned the exact validation rejection.
  • Sanitized AWS Crabbox run_d36622bf4cc1: scoped changed typecheck, lint, and guards passed.
  • Fresh Codex autoreview: clean, correctness 0.95. Repository review artifacts validate with zero findings.
  • Exact-head hosted CI: run 28752019380, passed.

Known gap: this proves the real Gateway RPC and agent hook path, not a browser/plugin UI interaction. The structured rejection and unavailable paths are both covered at the production hook boundary.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed 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. labels Jul 5, 2026
@steipete
steipete requested a review from a team as a code owner July 5, 2026 18:19
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: discord Channel integration: discord channel: imessage Channel integration: imessage channel: telegram Channel integration: telegram channel: voice-call Channel integration: voice-call channel: whatsapp-web Channel integration: whatsapp-web channel: zalo Channel integration: zalo app: android App: android app: ios App: ios app: macos App: macos app: web-ui App: web-ui gateway Gateway runtime extensions: memory-core Extension: memory-core cli CLI command changes labels Jul 5, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 5, 2026
@steipete
steipete force-pushed the fix/100212-plugin-approval-rejection-reason branch 2 times, most recently from d2534cf to cd0d64d Compare July 5, 2026 18:51
@steipete steipete mentioned this pull request Jul 5, 2026
Merged
tzy-17 and others added 2 commits July 5, 2026 20:20
@tzy-17 @steipete
fix(agents): surface real plugin approval rejection reason to agent
26e998c 
Distinguish gateway-rejection errors from transport failures in the
plugin approval catch block. When the error is a GatewayClientRequestError
(with a structured gatewayCode), the gateway is reachable and actively
rejected the request — surface the real rejection reason. Otherwise keep
the existing "gateway unavailable" message for genuine transport failures.

Previously every approval gateway error was reported as "gateway
unavailable", even when the gateway was healthy and returned a
structured rejection like INVALID_REQUEST. This misdirected operators
and agents toward connectivity debugging instead of the actual schema
or policy violation.

Fixes openclaw#100212
@steipete @tzy-17
fix(agents): report plugin approval rejections accurately
bdead2e 
Co-authored-by: 唐梓夷0668001293 <tang.ziyi@xydigit.com>
@steipete steipete mentioned this pull request Jul 5, 2026
Merged
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Merged via squash.

This was referenced Jul 5, 2026
Closed
Closed
Closed
@github-actions github-actions Bot mentioned this pull request Jul 5, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@steipete steipete

Labels

agents Agent runtime and tooling P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: XS status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Plugin approval gateway rejection reported to agent as misleading "gateway unavailable"

3 participants

@tzy-17 @harjothkhara @steipete