Skip to content

Plugin-owned external verification approvals#15

Draft
Guardiola31337 wants to merge 1 commit into
openclaw:mainfrom
Guardiola31337:rfc/plugin-owned-external-verification-approvals
Draft

Plugin-owned external verification approvals#15
Guardiola31337 wants to merge 1 commit into
openclaw:mainfrom
Guardiola31337:rfc/plugin-owned-external-verification-approvals

Conversation

@Guardiola31337

Copy link
Copy Markdown

Summary

Adds a draft RFC for plugin-owned external verification of approval resolution. The proposal keeps OpenClaw core as the approval owner, avoids arbitrary plugin-defined approval actions, and introduces a narrow externalResolution command-template route plus an ownership-checked verified resolver.

Motivation

This captures the AgentKit / World verification use case as a ClawHub plugin path instead of bundled core code, while addressing maintainer feedback that the earlier custom-actions shape expanded the API/UI/channel surface too much.

Validation

  • git diff --check --no-index -- /dev/null rfcs/0011-plugin-owned-external-verification-approvals.md
  • Draft includes the latest local AgentKit/OpenClaw proof matrix for deny, verify once, verify and trust for session, and the follow-up session-grant path.

Notes

The RFC references the AgentKit plugin as the concrete reference implementation and calls out the interaction with auto approvals / auto review from the safer auto-mode design.

@Guardiola31337 Guardiola31337 force-pushed the rfc/plugin-owned-external-verification-approvals branch from 636a8b2 to 49f2895 Compare June 15, 2026 14:58
@clawsweeper

clawsweeper Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 18, 2026, 9:47 PM ET / 01:47 UTC.

Summary
Adds a draft RFC for plugin-owned external verification approvals using an externalResolution command template and an ownership-checked plugin.approval.resolveVerified resolver.

Reproducibility: not applicable. this is a design RFC PR, not a bug report. The review checked the added RFC content, current main, GitHub discussion, and related RFC history instead of reproducing a runtime failure.

Review metrics: 2 noteworthy metrics.

  • Diff Scope: 1 added RFC, 799 inserted lines. The PR is Markdown-only but large enough to need focused maintainer review of the proposed API contract.
  • New API Concepts: 2 proposed surfaces. The RFC centers on externalResolution and plugin.approval.resolveVerified, both compatibility-sensitive plugin approval contracts.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until real behavior proof is added.

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

Rank-up moves:

  • Attach redacted proof output or a short recording/log artifact for the claimed AgentKit/OpenClaw flow, and update the PR body so ClawSweeper can re-review it.
  • Record the maintainer-discussion outcome for resolver authority, session grants, audit fields, and channel fallback before moving the RFC out of draft.
  • Keep status: draft until the RFC is accepted, then fill the implementation issue per the repository lifecycle.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR describes local tests and a manual TUI run, but I found no inspectable after-fix artifact; redacted terminal output, logs, screenshot, recording, or linked proof should be attached before merge if maintainers require this external RFC to prove the reference flow. 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

  • [P2] Merging the draft would bless a new plugin SDK/Gateway approval API before maintainers settle resolver authorization scope, session-grant semantics, audit fields, and channel fallback behavior.
  • [P1] The RFC proposal affects the approval security boundary because an external plugin would be able to resolve its own pending approvals after proof; the ownership checks are promising but still need explicit maintainer agreement.
  • [P1] The PR body and RFC describe local tests and a manual TUI run, but I did not find an inspectable attached artifact, terminal output, recording, screenshot, or log for real behavior proof.

Maintainer options:

  1. Resolve RFC Acceptance Gates (recommended)
    Before merge, maintainers should record the resolver authority scope, session-grant semantics, audit expectations, channel fallback decision, and proof expectations in the RFC or discussion thread.
  2. Accept With Explicit Deferrals
    Maintainers may merge the RFC with some questions deferred only if the deferrals are written as implementation-phase risks rather than implied acceptance.
  3. Keep Draft Open
    If the approval boundary is not ready for acceptance, leave the draft open rather than closing it because the proposal is coherent and not superseded.

Next step before merge

  • [P1] This PR needs maintainer product/security review and RFC lifecycle handling, not an automated repair branch.

Security
Needs attention: The diff adds no executable code or supply-chain changes, but the RFC proposes a security-sensitive approval resolver that needs explicit maintainer boundary review.

Review details

Best possible solution:

Keep the RFC open as a draft until maintainers record the security/API decisions and discussion-thread outcome, add inspectable redacted proof where appropriate, then update the frontmatter and implementation tracking before merge.

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

Not applicable; this is a design RFC PR, not a bug report. The review checked the added RFC content, current main, GitHub discussion, and related RFC history instead of reproducing a runtime failure.

Is this the best way to solve the issue?

Unclear until maintainer review; the command-template plus ownership-checked resolver is a coherent narrow design, but the RFC itself leaves security and compatibility decisions that should be settled before acceptance.

AGENTS.md: not found in the target repository.

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

Label changes

Label changes:

  • add P2: This is a meaningful plugin approval API proposal with security and compatibility implications, but it is not an urgent runtime regression.
  • add merge-risk: 🚨 compatibility: Accepting the RFC would set direction for new plugin SDK/Gateway approval surfaces that future plugins may depend on.
  • add merge-risk: 🚨 security-boundary: The proposed verified resolver changes who may resolve pending approvals after external proof, which needs explicit approval-boundary agreement before merge.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • 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 describes local tests and a manual TUI run, but I found no inspectable after-fix artifact; redacted terminal output, logs, screenshot, recording, or linked proof should be attached before merge if maintainers require this external RFC to prove the reference flow. 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.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.
  • remove status: 👀 ready for maintainer look: Current PR status label is status: 📣 needs proof.

Label justifications:

  • P2: This is a meaningful plugin approval API proposal with security and compatibility implications, but it is not an urgent runtime regression.
  • merge-risk: 🚨 compatibility: Accepting the RFC would set direction for new plugin SDK/Gateway approval surfaces that future plugins may depend on.
  • merge-risk: 🚨 security-boundary: The proposed verified resolver changes who may resolve pending approvals after external proof, which needs explicit approval-boundary agreement before merge.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR describes local tests and a manual TUI run, but I found no inspectable after-fix artifact; redacted terminal output, logs, screenshot, recording, or linked proof should be attached before merge if maintainers require this external RFC to prove the reference flow. 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] Settle verified resolver authority before acceptance — rfcs/0011-plugin-owned-external-verification-approvals.md:299
    The RFC would bless a plugin-owned resolver that mutates pending approval state, while the final authority scope is still unresolved; that approval boundary needs maintainer agreement before merge.
    Confidence: 0.87

What I checked:

  • Target policy check: No AGENTS.md exists in the target openclaw/rfcs checkout; the only AGENTS.md found was in a sibling review-harness checkout, so no target-specific AGENTS policy applied. (e7555fdda445)
  • Current main does not implement the RFC content: Searches of current main found no externalResolution, resolveVerified, AgentKit, or World ID RFC content, so the central proposal is not already implemented in this repository. (e7555fdda445)
  • Merge-result scope: The clean merge result adds one RFC Markdown file with 799 insertions and no runtime, workflow, dependency, or generated-file changes. (rfcs/0011-plugin-owned-external-verification-approvals.md:1, 58c49dc55430)
  • RFC lifecycle gate: The README says new RFCs start as PRs, remain unmerged while status: draft, and need a maintainer-discussion thread before acceptance, issue assignment, and merge. (README.md:80, e366ea9825a4)
  • Security-sensitive resolver design: The draft proposes a resolver that mutates pending approval state after external proof and names ownership, exposed-decision, and authority checks, including avoiding broad admin authority for external plugins. (rfcs/0011-plugin-owned-external-verification-approvals.md:283, 49f2895a4dbb)
  • Acceptance criteria and open questions: The RFC includes host API tests and AgentKit e2e proof expectations, while leaving authority scope, session grant semantics, audit fields, and channel support as unresolved questions. (rfcs/0011-plugin-owned-external-verification-approvals.md:571, 49f2895a4dbb)

Likely related people:

  • kevinlin-openai: Authored and updated the README/template lifecycle guidance that controls draft RFC merge readiness and maintainer-discussion flow. (role: RFC process contributor; confidence: high; commits: e366ea9825a4, f4fdf38f4717; files: README.md, rfcs/0000-template.md)
  • omarshahine: Authored the merged approval prompt markdown RFC that this proposal references as nearby approval-rendering direction. (role: adjacent approval RFC author; confidence: medium; commits: f346050b2878; files: rfcs/0005-approval-prompt-markdown.md)
  • jalehman: Recently authored and advanced the plugin SDK session/transcript storage RFC, a nearby public plugin SDK compatibility area. (role: recent plugin SDK RFC contributor; confidence: medium; commits: a9cf8cc4d427, 86183ca73c4c; files: rfcs/0007-plugin-sdk-session-transcript-storage.md)
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.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. 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. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 15, 2026

@kevinslin kevinslin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rfcs should be high signal - please take a human pass through it and remove noise that doesn't need to be in here

6. AgentKit resolves its own pending approval.
7. OpenClaw continues the original blocked tool call.

The first AgentKit attempt put World ID HITL directly in OpenClaw core:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we don't need this in the rfc - motivation should just be that certain plugins like worldcoin need ability to handle approvals

approval flows.
- Letting auto review approve an external-proof-required approval without the
external proof.
- Designing durable chat status cards in this RFC. Those are useful but should

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unrelated to plugin owning verification. we don't need it here. could we do a human pass and avoid slop in rfcs as much as possible?

- it can classify whether the underlying command/tool call is risky
- it can explain why the action should be denied or escalated
- it can decline and fall back to the human operator
- it can include `externalVerificationRequired: true` in any bounded review

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean auto review can move an approval to the external verification lane? that would require a change to autoreview

- channel-delivered approval prompts
- opt-in auto mode for lower-risk exec approval review

The safer-auto-mode blog frames the current direction well:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lets not reference a blog without any links. its also not necessary to call this out in the rfc since this is about plugin approval, not auto review


## Unresolved Questions

- Should verified resolution use the existing `operator.approvals` scope, or

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should answer these questions instead of leaving them open unless they are truly unresolvable rigth now

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.

2 participants