Skip to content

fix(plugins): name the selected driver when a plugin id is not offered - #1404

Merged
freeqaz merged 2 commits into
mainfrom
plugin-unknown-id-message
Oct 5, 2026
Merged

freeqaz merged 2 commits into
mainfrom
plugin-unknown-id-message

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Dogfood finding D108, seen again in round 22. While the Installation selects codex-plugin, the plugins-configure.md CLI example on a dedicated Codex Agent ("occ-plugin:diffs") answers 400 INVALID_REQUEST: The supplied plugin policies are invalid. The message names neither the plugin nor the selected Driver. An Installation selects one Plugin Driver, so the same thing happens on update or deploy after a Driver switch.

Change

  • PluginPolicyValidationError gains an unknownPlugin field. Its message names the selected Plugin Driver ID, which comes from trusted Installation configuration and never from the request, and says an Installation selects one Driver.
  • The plugin runtime translator tags its two unknown-ID errors (OpenClaw catalog miss; Codex prefix or marketplace mismatch) with policyField: "pluginId". It now resolves the plugin ID before any policy field check, so a Codex selection with toolDefaults.reviewer under occ-plugin reports the ID mismatch, not the reviewer field.
  • BundledPluginDriverBase.validate maps that field to the new error. Deployment admission uses the same path.
  • Conformance test; one line in docs/reference/agent-plugins.md "Failures and boundaries".

Other policy errors keep their current messages.

Verification

  • node --test tests/conformance/plugin-driver.test.mjs: 29/29 pass, including the new test.
  • node --test --test-name-pattern '[Pp]lugin' tests/integration/occ-api.test.mjs: 5/5.
  • tsc --build, eslint and prettier on the touched files; docs word count and docs-site link check.
  • Reproduced live on oce-dogfood at main dd934c39b before the change.

Risks

Low. This changes only the message text of a 400 that was already returned. No admission decision changes: the moved ID check runs on input that was already rejected later in the same loop. Clients matching the old generic text for unknown IDs would see the new one; no test or console code does.

Conflict gate (no new conflicts)

git merge-tree --write-tree of each open PR touching these files, against origin/main and against this head:

PR Conflicted files vs main vs this head Hunks in shared files (main → head)
#913 none none n/a
#378 same set (21 files) same set drivers/plugin/index.ts 2 → 2
#307 same set same set packages/occ/src/errors.ts 1 → 1
#299 same set same set n/a
#489 same set same set drivers/plugin/index.ts 1 → 1, agent-plugins.md 1 → 1
#508 same set same set n/a
#115 same set same set n/a

Selecting occ-plugin:diffs while the Installation selects codex-plugin (or a
Codex app under occ-plugin) answered 400 'The supplied plugin policies are
invalid.' with no hint. Unknown plugin IDs now name the selected Plugin Driver
and say an Installation selects one Driver. Other policy errors are unchanged.
… nits)

A Codex selection with toolDefaults.reviewer under occ-plugin reported the
unsupported reviewer instead of the plugin the Driver does not offer. Resolve
the ID first, cover a Codex ID without the marketplace suffix, and say in the
reference that Codex admission checks only the prefix and marketplace.
@freeqaz
freeqaz requested a review from a team as a code owner October 5, 2026 18:19
@clawsweeper

clawsweeper Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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 Oct 5, 2026
@clawsweeper

clawsweeper Bot commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed October 5, 2026, 2:25 PM ET / 18:25 UTC.

ClawSweeper review

What this changes

The PR makes rejected plugin selections name the Installation’s selected Plugin Driver, prioritizes plugin-ID errors over unsupported policy fields, and adds regression coverage and reference documentation.

Merge readiness

✅ Ready for maintainer review

Keep open for maintainer triage: current main still lacks the clearer diagnostic, and the patch has no identified correctness or security blocker.

Priority: P3
Reviewed head: e7121e5adcf65c1e6ab8ebff5f5656d7198c501d

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused diagnostic improvement with meaningful regression coverage and no identified blocking defect.
Proof confidence 🌊 off-meta tidepool Not applicable: The workflow’s MEMBER classification supplies the trusted-author exemption. The changed Driver diagnostic has focused conformance coverage and reported API checks; the live dogfood observation is before-fix evidence. No stored-data contract changes.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The workflow’s MEMBER classification supplies the trusted-author exemption. The changed Driver diagnostic has focused conformance coverage and reported API checks; the live dogfood observation is before-fix evidence. No stored-data contract changes.
Evidence reviewed 7 items Pinned introduced change: The merge-base-to-head diff changes five files. The validation wrapper maps tagged plugin-ID failures to the new diagnostic; existing rejection checks remain in place.
Current main still needs the diagnostic: Fetched main retains the generic policy-error mapping without the pluginId branch. This establishes that the proposed diagnostic remains useful; no fixing release or merged replacement was established.
Production callers and HTTP mapping: Agent policy validation and locked deployment admission invoke the selected Driver. The existing HTTP mapper returns PluginPolicyValidationError.message as 400 INVALID_REQUEST. Driver identity is constrained to bundled identities, and the message does not echo submitted plugin values.
Findings None None.
Security None None.

How this fits together

OpenClaw Control Plane validates Agent plugin selections through the Installation’s selected Plugin Driver before saving or admitting a deployment. Validation failures become HTTP error responses for console, CLI, and API callers.

flowchart TD
  A[Agent plugin selections] --> C[Selected Plugin Driver]
  B[Installation configuration] --> C
  C --> D[Validate ID and policy]
  D --> E[Reject invalid selection]
  E --> F[HTTP 400 with Driver guidance]
  D --> G[Save or admit deployment]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Production and test LOC production +41/-10; tests +33/-0; documentation +4/-1 Production growth supports a focused diagnostic and shared catalog lookup, with no new dependency or platform capability.

Technical review

Best possible solution:

Keep plugin-ID diagnostics in the existing Driver validation and HTTP error path while preserving current admission decisions.

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

Yes, from source: submitting occ-plugin:diffs while codex-plugin is selected reaches the generic validation error on current main. No live reproduction was executed during this review.

Is this the best way to solve the issue?

Yes. Tagging the existing ID failures and mapping them through the existing error owner is a narrow repair that preserves rejection behavior.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 3f882c6ebb4a.

Labels

Label changes:

  • add P3: This improves guidance for an already rejected request without changing supported workflows or admission outcomes.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The workflow’s MEMBER classification supplies the trusted-author exemption. The changed Driver diagnostic has focused conformance coverage and reported API checks; the live dogfood observation is before-fix evidence. No stored-data contract changes.

Label justifications:

  • P3: This improves guidance for an already rejected request without changing supported workflows or admission outcomes.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The workflow’s MEMBER classification supplies the trusted-author exemption. The changed Driver diagnostic has focused conformance coverage and reported API checks; the live dogfood observation is before-fix evidence. No stored-data contract changes.

Evidence

What I checked:

  • Pinned introduced change: The merge-base-to-head diff changes five files. The validation wrapper maps tagged plugin-ID failures to the new diagnostic; existing rejection checks remain in place. (apps/controller/src/drivers/plugin/index.ts:321, e7121e5adcf6)
  • Current main still needs the diagnostic: Fetched main retains the generic policy-error mapping without the pluginId branch. This establishes that the proposed diagnostic remains useful; no fixing release or merged replacement was established. (apps/controller/src/drivers/plugin/index.ts, 3f882c6ebb4a)
  • Production callers and HTTP mapping: Agent policy validation and locked deployment admission invoke the selected Driver. The existing HTTP mapper returns PluginPolicyValidationError.message as 400 INVALID_REQUEST. Driver identity is constrained to bundled identities, and the message does not echo submitted plugin values. (apps/controller/src/http/errors.ts:659, e7121e5adcf6)
  • Focused regression coverage: The added test exercises real bundled Drivers for cross-Driver selections, an unknown OpenClaw entry, a Codex ID lacking its marketplace, error precedence, and preservation of an unrelated policy message. Existing API tests cover rejected writes, unchanged state, authorization, and audit boundaries. Tests were inspected, not executed. (tests/conformance/plugin-driver.test.mjs:305, e7121e5adcf6)
  • Area history and routing: Current-main path history records recent plugin admission work by freeqaz, shared policy translation by stevenlee-oai, and dedicated-policy maintenance by Kimi Yu. Deeper blame and introduction searches encountered unavailable historical objects; these are routing signals, not verified line-introduction claims. (apps/controller/src/drivers/plugin/runtime-translator.ts, 3f882c6ebb4a)
  • Proof and author context: Repository State identifies the author as MEMBER, although the supplied GitHub snapshot and live pull endpoint report CONTRIBUTOR. The trusted-author classification supplied by the owning workflow is used for this review. The body reports before-fix dogfood reproduction and passing conformance/API checks, without claiming an after-fix live run. Prepared media belongs to the unrelated creation-retry PR. (e7121e5adcf6)

Likely related people:

  • freeqaz: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • stevenlee-oai: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
  • Kimi Yu: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

@freeqaz
freeqaz merged commit ed6ade5 into main Oct 5, 2026
47 checks passed
@freeqaz
freeqaz deleted the plugin-unknown-id-message branch October 5, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant