mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-03 09:39:25 +00:00
fix(auto-reply): show usage for invalid approval decisions (#137877)
## What Problem This Solves
`/approve abc constructor` and `/approve abc __proto__` returned a failed-submission error instead of usage. The parser read inherited object properties as decisions. It also misread `constructor` when used as an approval ID before a valid decision.
## Why This Change Was Made
Both supported argument layouts now check that a decision is a declared alias before reading its value. The ten aliases and downstream authorization and resolver behavior remain unchanged.
Downstream validation already rejected these values before making a Gateway request. This change gives the caller the normal usage reply at parsing.
## User Impact
Unknown decisions return the usage reply. Valid decisions still resolve pending exec and plugin approvals. An approval ID such as `constructor` also works when followed by a declared decision.
## Evidence
Verified candidate `b03792933d42978e969d9da25748f15fa0f79326` against pinned main `d3a2fb0296`, using separate installs and state on one isolated Linux Testbox.
- Real source Gateway and QA channel: baseline returns the invalid-decision submission error; candidate returns usage. The expanded matrix records 54 cases per pin, including every alias in both layouts, real pending exec/plugin records, duplicates, conflicts, unknown and expired IDs, whitespace, and prototype-like IDs.
- Real Telegram Test Server user and bot: the same before/after behavior, with one lease shared by both pins within each pair. Fast approval commands make zero provider requests. Owner records and resolution events confirm valid decisions take effect once and invalid inputs leave pending approvals unchanged.
- Separate controls cover the named account, unauthorized sender, disabled approval capability, disabled text/native commands, foreign-bot commands, and a real Gateway client missing approval scope. Disabled text handling follows normal deterministic model dispatch without resolving the pending approval.
- Fresh-pending syntax controls preserve whitespace and extra-token behavior. The earlier timed duplicate probe is retained separately because it crossed the existing resolved-entry grace window.
- All 71 focused approval/parser tests and 134 QA catalog tests passed. Changed-file formatting and lint passed. [Exact-head CI](https://github.com/openclaw/openclaw/actions/runs/33940306696) is green.
`toString` and `valueOf` were already rejected after lowercasing; they are preservation controls. Normal channel delivery may update message state. The approval invariant concerns decision and grant effects; audit-row absence is not treated as proof of absence. No execution-identity diagnostics or production approvals were enabled for this proof.
AI-assisted.
Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
parent
810e064067
commit
996c928205
3 changed files with 131 additions and 4 deletions
|
|
@ -0,0 +1,89 @@
|
|||
title: Approve command rejects Object.prototype names as decisions
|
||||
|
||||
scenario:
|
||||
id: approve-command-prototype-decision-usage
|
||||
surface: channels
|
||||
category: channels.channel-actions-commands-and-approvals
|
||||
coverage:
|
||||
primary:
|
||||
- channels.channel-native-commands
|
||||
regressionRefs:
|
||||
- openclaw/openclaw#137877
|
||||
objective: Verify a chat /approve whose decision token is an inherited Object.prototype name gets the usage reply instead of being submitted as a decision.
|
||||
successCriteria:
|
||||
- Sending /approve abc constructor replies with the /approve usage text.
|
||||
- The usage reply is produced without a model request.
|
||||
- Sending /approve abc deny still reaches the Gateway approval resolver.
|
||||
docsRefs:
|
||||
- docs/channels/qa-channel.md
|
||||
codeRefs:
|
||||
- src/auto-reply/reply/commands-approve.ts
|
||||
- src/auto-reply/reply/fast-approve.ts
|
||||
execution:
|
||||
kind: flow
|
||||
summary: Send /approve with a prototype-name decision over a DM and verify the usage reply, then send a real decision and verify it is submitted.
|
||||
config:
|
||||
conversationId: approve-prototype-dm
|
||||
prototypeCommandText: /approve abc constructor
|
||||
realCommandText: /approve abc deny
|
||||
usageNeedle: "Usage: /approve"
|
||||
|
||||
flow:
|
||||
steps:
|
||||
- name: prototype decision name gets the usage reply
|
||||
actions:
|
||||
- call: waitForGatewayHealthy
|
||||
args: [{ ref: env }, 60000]
|
||||
- call: waitForTransportReady
|
||||
args: [{ ref: env }, 60000]
|
||||
- resetTransport: true
|
||||
- set: requestCursorBefore
|
||||
value:
|
||||
expr: "env.mock ? (await fetchJson(`${env.mock.baseUrl}/debug/request-cursor`)).cursor : 0"
|
||||
- set: startIndex
|
||||
value:
|
||||
expr: "state.getSnapshot().messages.filter((message) => message.direction === 'outbound').length"
|
||||
- sendInbound:
|
||||
conversation: { id: { ref: config.conversationId }, kind: direct }
|
||||
senderId: qa-approver
|
||||
senderName: QA Approver
|
||||
text: { ref: config.prototypeCommandText }
|
||||
- waitForOutbound:
|
||||
conversation: { id: { ref: config.conversationId }, kind: direct }
|
||||
sinceIndex: { ref: startIndex }
|
||||
timeoutMs: 60000
|
||||
saveAs: reply
|
||||
- assert:
|
||||
expr: "reply.text.includes(config.usageNeedle)"
|
||||
message:
|
||||
expr: "`prototype decision was not rejected with usage: ${reply.text}`"
|
||||
- set: scenarioRequests
|
||||
value:
|
||||
expr: "env.mock ? await fetchJson(`${env.mock.baseUrl}/debug/requests?after=${requestCursorBefore}`) : []"
|
||||
- assert:
|
||||
expr: "!env.mock || scenarioRequests.length === 0"
|
||||
message:
|
||||
expr: "`/approve usage reply unexpectedly invoked the model ${String(scenarioRequests.length)} time(s)`"
|
||||
detailsExpr: reply.text
|
||||
|
||||
- name: real decision still reaches the approval resolver
|
||||
actions:
|
||||
- set: realStartIndex
|
||||
value:
|
||||
expr: "state.getSnapshot().messages.filter((message) => message.direction === 'outbound').length"
|
||||
- sendInbound:
|
||||
conversation: { id: { ref: config.conversationId }, kind: direct }
|
||||
senderId: qa-approver
|
||||
senderName: QA Approver
|
||||
text: { ref: config.realCommandText }
|
||||
- waitForOutbound:
|
||||
conversation: { id: { ref: config.conversationId }, kind: direct }
|
||||
sinceIndex: { ref: realStartIndex }
|
||||
textIncludes: Failed to submit approval
|
||||
timeoutMs: 60000
|
||||
saveAs: realReply
|
||||
- assert:
|
||||
expr: "!realReply.text.includes(config.usageNeedle)"
|
||||
message:
|
||||
expr: "`real decision was rejected as usage: ${realReply.text}`"
|
||||
detailsExpr: "formatTransportTranscript(state, { conversationId: config.conversationId })"
|
||||
Loading…
Add table
Add a link
Reference in a new issue