fix(exec): add concrete approval recovery hints#53347
Conversation
|
@codex review |
Greptile SummaryThis PR replaces three separate copies of a vague "approve from Web UI or terminal UI" fallback message with a single shared helper ( The change is clean and well-tested — tests were updated to assert every new hint line appears across all three Confidence Score: 5/5
Reviews (1): Last reviewed commit: "fix(exec): add concrete approval recover..." | Re-trigger Greptile |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64c127da61
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| function appendExecApprovalRecoveryHints(lines: string[]): void { | ||
| lines.push("Try one of these:"); | ||
| lines.push("- Open the Control UI: run `openclaw dashboard --no-open` to get a tokenized URL."); | ||
| lines.push("- Inspect pending approvals from the CLI: `openclaw approvals get`."); |
There was a problem hiding this comment.
The new fallback text says operators can “Inspect pending approvals” with openclaw approvals get, but this command does not show pending approval requests—it renders the exec approvals snapshot/allowlist (src/cli/exec-approvals-cli.ts, get command). It also defaults to the local approvals file when neither --gateway nor --node is provided (loadSnapshotTarget), so in common remote-gateway/node flows this advice points to the wrong target and does not help resolve the blocked approval. Please update the hint to describe what this command actually does or point to the correct pending-approval resolution path.
Useful? React with 👍 / 👎.
|
I rechecked the remaining failures and they look unrelated to this PR.\n\nLocal targeted verification still passes:\n- \pnpm test -- src/infra/exec-approval-reply.test.ts\n- \pnpm exec oxfmt --check src/infra/exec-approval-reply.ts src/infra/exec-approval-reply.test.ts\n\nThis PR only touches:\n- \src/infra/exec-approval-reply.ts\n- \src/infra/exec-approval-reply.test.ts\n\nThe remaining failing jobs are currently in:\n- \src/infra/exec-command-resolution.test.ts\ (/bin/sh\ vs /usr/bin/dash\ assertion)\n- extension runtime loading under \matrix\ / \whatsapp\ paths\n\nSo from the current logs, the red checks appear broader than the files touched here. |
|
Closing this to reduce my active PR queue and focus on the two smallest, highest-probability merge candidates. Happy to reopen if this specific path becomes active again. |
1 participant
Summary
Closes #53250
Verification