Skip to content

feat: add semantic Windows exec approval prompts#1027

Draft
ruben2000de wants to merge 1 commit into
openclaw:mainfrom
ruben2000de:agent/semantic-approval-prompts
Draft

feat: add semantic Windows exec approval prompts#1027
ruben2000de wants to merge 1 commit into
openclaw:mainfrom
ruben2000de:agent/semantic-approval-prompts

Conversation

@ruben2000de

Copy link
Copy Markdown

What changed

  • carry OpenClaw's canonical commandPreview through system.run.prepare and system.run
  • show a plain-language approval summary before the exact technical command
  • localize the native approval prompt and actions for German Windows locales
  • keep Deny as the default-focused action and keep policy/sandbox enforcement authoritative
  • strip control and bidirectional override characters from approval display text
  • preserve compatibility with legacy prompt presenters while retaining the exact command separately

Why

The Windows approval prompt currently exposes a raw shell or PowerShell command as the primary decision surface. That is not meaningful consent for operators who can judge the requested purpose and impact but cannot safely audit shell syntax.

OpenClaw already defines commandPreview as the human-readable summary for an execution plan, but the Windows node did not carry that field into its approval presenter. This change turns the prompt into a semantic decision card while keeping the exact command visible for technical inspection and keeping the host policy as the actual security boundary.

User impact

German users see:

  • what the command is for (agent-supplied context)
  • a reminder that policy and sandbox checks remain active
  • the exact technical command and policy reason
  • localized Einmal erlauben, Dauerhaft erlauben, and Ablehnen actions

If no preview was provided, the prompt tells the operator to deny and request a clearer explanation.

Validation

  • four focused approval tests pass, including canonical preview propagation, exact-command visibility, missing-preview guidance, and BiDi sanitization
  • OpenClaw.Shared Release build passes with zero warnings/errors
  • broader Linux test run: 2898 passed, 31 skipped, 31 Windows/platform-dependent failures
  • full WinUI build cannot run in the Linux development container because MakePri.exe and XamlCompiler.exe are Windows-only; this draft intentionally relies on repository Windows CI for that validation

@ruben2000de
ruben2000de force-pushed the agent/semantic-approval-prompts branch from 3b0c7fb to 48ece7e Compare July 19, 2026 18:17
@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 priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jul 19, 2026
@clawsweeper

clawsweeper Bot commented Jul 19, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 19, 2026, 2:20 PM ET / 18:20 UTC.

Summary
The PR propagates commandPreview through Windows system.run approvals, adds semantic/localized prompt text, and adds focused approval-prompt tests.

Reproducibility: yes. from source inspection: submit a system.run request with commandPreview to a legacy IExecApprovalPromptHandler; the PR’s own test demonstrates that its existing Command field becomes the preview rather than the exact command.

Review metrics: 1 noteworthy metric.

  • Patch surface: 6 files affected; 268 added, 16 removed. The change crosses shared execution approval contracts, native WinUI presentation, and tests, so compatibility evidence matters beyond the focused unit cases.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • Preserve Command as the exact command and use a separate preview field only in preview-aware prompt rendering.
  • Post a redacted current-head Windows approval capture or runtime transcript that visibly shows preview, exact command, localized labels, and default Deny focus.
  • [P1] Run and report ./build.ps1, the shared and tray test projects, and ./scripts/validate-mxc-e2e.ps1 from a Windows-capable environment.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body reports focused tests and a Linux build, but it provides no after-fix Windows prompt capture, live output, or redacted runtime log from the current head; native Windows and MXC validation remain required. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Existing prompt handlers may treat Command as the authoritative technical command; replacing it with agent-supplied explanatory text can produce misleading approval, audit, or integration behavior even though host policy still executes against the exact command.
  • [P1] The PR changes security-sensitive operator-consent UI but lacks after-fix native Windows evidence and the repository-required Windows/MXC validation.

Maintainer options:

  1. Preserve the existing request contract (recommended)
    Keep Command as the exact executable command, add the preview as an independent presentation field, and update the native presenter and tests to show both clearly.
  2. Pause the prompt redesign
    Do not merge the new prompt path if maintainers intentionally want legacy handlers to receive semantic text instead of the historical exact-command contract.

Next step before merge

  • [P1] The code defects are narrow, but the required native Windows and MXC proof must come from a real capable setup and cannot be established by a read-only review.

Security
Needs attention: The patch does not alter policy enforcement, but it weakens the human-consent presentation contract for legacy handlers by substituting untrusted preview text for the exact command.

Review findings

  • [P1] Keep Command as the exact command for legacy presenters — src/OpenClaw.Shared/Capabilities/SystemCapability.cs:499-505
  • [P2] Use the supplied display name in the prompt copy — src/OpenClaw.Shared/ExecApprovalPromptText.cs:28-30
Review details

Best possible solution:

Keep ExecApprovalPromptRequest.Command as the exact command for existing handlers, have the native prompt render CommandPreview as separately labelled context, use AppIdentity.DisplayName rather than a fixed name, then provide a redacted Windows approval capture plus the required validation results.

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

Yes, from source inspection: submit a system.run request with commandPreview to a legacy IExecApprovalPromptHandler; the PR’s own test demonstrates that its existing Command field becomes the preview rather than the exact command.

Is this the best way to solve the issue?

No. The semantic preview should be additive: preserve the exact-command contract for existing handlers and let only preview-aware presenters render the additional context before the technical command.

Full review comments:

  • [P1] Keep Command as the exact command for legacy presenters — src/OpenClaw.Shared/Capabilities/SystemCapability.cs:499-505
    This path replaces the established ExecApprovalPromptRequest.Command value with agent-supplied preview text. Existing handlers only know Command, not the newly added TechnicalCommand, so they can display or record a semantic claim without the command they are approving. Preserve Command as the technical command and render CommandPreview separately in preview-aware presenters.
    Confidence: 0.94
  • [P2] Use the supplied display name in the prompt copy — src/OpenClaw.Shared/ExecApprovalPromptText.cs:28-30
    Build accepts displayName, but the new German copy hardcodes “Otti”. Installations whose AppIdentity.DisplayName differs are misidentified in the consent dialog. Interpolate the supplied name and add a non-default-name assertion.
    Confidence: 0.98

Overall correctness: patch is incorrect
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against bd6c4bf8e575.

Label changes

Label changes:

  • add P2: This is a meaningful but bounded operator-approval and compatibility change rather than a demonstrated current production outage.
  • add merge-risk: 🚨 compatibility: The patch changes the meaning of the pre-existing ExecApprovalPromptRequest.Command field for legacy prompt handlers.
  • add merge-risk: 🚨 security-boundary: The approval display is part of the operator-consent boundary, and legacy presenters can lose visible exact-command context under the proposed field substitution.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports focused tests and a Linux build, but it provides no after-fix Windows prompt capture, live output, or redacted runtime log from the current head; native Windows and MXC validation remain required. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P2: This is a meaningful but bounded operator-approval and compatibility change rather than a demonstrated current production outage.
  • merge-risk: 🚨 compatibility: The patch changes the meaning of the pre-existing ExecApprovalPromptRequest.Command field for legacy prompt handlers.
  • merge-risk: 🚨 security-boundary: The approval display is part of the operator-consent boundary, and legacy presenters can lose visible exact-command context under the proposed field substitution.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports focused tests and a Linux build, but it provides no after-fix Windows prompt capture, live output, or redacted runtime log from the current head; native Windows and MXC validation remain required. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

Security concerns:

  • [medium] Legacy handlers can lose exact-command visibility — src/OpenClaw.Shared/Capabilities/SystemCapability.cs:499
    The proposed field substitution means pre-existing handlers that render Command can show only agent-provided context while the approval applies to a separate technical command; TechnicalCommand does not preserve compatibility for handlers that do not know it.
    Confidence: 0.94

What I checked:

  • Current approval contract: Current main passes the exact command in ExecApprovalPromptRequest.Command, and the native presenter renders that field as the technical command shown to the operator. (src/OpenClaw.Shared/Capabilities/SystemCapability.cs:22, bd6c4bf8e575)
  • Legacy compatibility regression: The proposed focused test asserts that a legacy capturing handler receives the semantic preview in Request.Command; handlers compiled against the existing request shape will not consume the new TechnicalCommand field and can therefore display, record, or evaluate the preview instead of the exact command. (tests/OpenClaw.Shared.Tests/ExecApprovalPolicyTests.cs:1288, 48ece7eda134)
  • Branding input is unused: The new prompt-text builder receives displayName, but the German copy shown in the diff hardcodes “Otti”; the supplied test passes a different display name without asserting the rendered identity. (src/OpenClaw.Shared/ExecApprovalPromptText.cs:28, 48ece7eda134)
  • Repository validation policy: The repository policy requires the full Windows build, shared and tray test projects after code changes; because this changes system.run, it additionally requires the MXC E2E validation path. The PR body reports focused/Linux validation but no current-head required Windows validation or native prompt proof. (AGENTS.md:1, bd6c4bf8e575)

Likely related people:

  • unknown: The relevant current-main code is concentrated in the shared approval flow and native tray presenter, but read-only shell history inspection was unavailable in this review environment, so no individual ownership attribution can be made safely. (role: current-main approval-flow owner; confidence: low; commits: bd6c4bf8e575; files: src/OpenClaw.Shared/Capabilities/SystemCapability.cs, src/OpenClaw.Tray.WinUI/Services/ExecApprovalPromptService.cs)
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.

@ruben2000de

Copy link
Copy Markdown
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant