feat: add semantic Windows exec approval prompts - #1027
Conversation
3b0c7fb to
48ece7e
Compare
|
Codex review: needs real behavior proof before merge. Reviewed August 2, 2026, 1:58 PM ET / 17:58 UTC. ClawSweeper reviewWhat this changesThe PR carries agent-supplied command-preview text through Windows exec approvals and displays it as separately labeled, sanitized context after the exact command. Merge readiness⛔ Blocked until real behavior proof is added - 6 items remain Keep this PR open, but it is not merge-ready. The feature is still absent from current main, while the branch also edits the legacy approval path that the merged V2 cutover retired; it needs a V2-only rebase and current-head Windows proof before review can pass. Priority: P2 Review scores
Verification
How this fits togetherWindows node flowchart LR
A[Gateway system.run request] --> B[Windows node validation]
B --> C[V2 approval coordinator]
C --> D[WinUI approval dialog]
D --> E[Operator allow or deny]
E --> F[Contained command execution]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Rebase the useful preview and sanitizer design onto the current V2-only validation, coordinator, UI-handler, and dialog seam; preserve canonical argv as the sole authorization and execution identity; then publish redacted Windows proof showing the exact command, separate request context, localized actions, and Deny-focused default behavior. Do we have a high-confidence way to reproduce the issue? Not applicable. This PR adds a new approval-dialog context field rather than reporting a broken current-main behavior; source inspection confirms the V2 dialog currently has no preview field. Is this the best way to solve the issue? No. The preview design is reasonable, but this branch is not the best solution because its legacy-path edits conflict with the V2-only approval architecture now on main. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8ac00c5f4977. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
|
|
Windows CI is needed for the final WinUI validation because the relevant MakePri/XAML tooling is Windows-only. The focused approval tests and the Shared Release build pass locally; when convenient, please approve the fork workflow runs for this draft PR. |
48ece7e to
a3c3fba
Compare
|
Rebased onto current main and pushed a new head ( |
What changed
commandPreviewthroughsystem.run.prepare, the legacy approval path, and the current V2 approval pipelineCommand/DisplayCommandcontract as the exact authoritative commandWhy
The Windows approval prompt currently makes the raw command the operator's primary decision surface. OpenClaw already provides
commandPreviewas a human-readable explanation, but the Windows node did not preserve it through both approval pipelines.The preview must remain presentation-only. It can help the operator understand purpose, impact, risk, and the recommended decision, but it must never replace the exact command used for policy, audit, or execution. This revision therefore keeps the exact command contract unchanged and adds the preview as a separate field and UI block.
Security and compatibility
ExecApprovalPromptRequest.CommandDisplayCommandfrom canonical argvValidation
Passed locally on the rebased current head:
dotnet build src/OpenClaw.Shared/OpenClaw.Shared.csproj --no-restore -c Release— 0 warnings, 0 errorsReal behavior proof
Not verified yet on the rebased head. The Linux container cannot execute
MakePri.exeorXamlCompiler.exe, and the paired Windows validation node was offline during this revision.Still required before marking ready for review:
./build.ps1./scripts/validate-mxc-e2e.ps1