Skip to content

fix(plugins): isolate bundled capability capture fields#90068

Closed
vincentkoc wants to merge 1 commit into
mainfrom
fuzz-tool-schema-next158-20260604
Closed

fix(plugins): isolate bundled capability capture fields#90068
vincentkoc wants to merge 1 commit into
mainfrom
fuzz-tool-schema-next158-20260604

Conversation

@vincentkoc

@vincentkoc vincentkoc commented Jun 4, 2026

Copy link
Copy Markdown
Member

Summary

  • Hardens bundled capability runtime capture so malformed captured provider/backend/harness id fields or tool name fields are diagnosed and skipped per row.
  • Preserves healthy sibling captured registrations instead of failing the whole bundled runtime snapshot.
  • Carries manifest contracts into the runtime-created plugin record so captured tools use the same manifest-declared contract enforcement as the normal plugin registry path.
  • Out of scope: broad provider behavior changes, external plugin API changes, and compatibility shims for malformed non-string ids/names beyond diagnostic isolation.

Reviewers should focus on the captured registration boundary in src/plugins/bundled-capability-runtime.ts and the temp bundled plugin regression in src/plugins/bundled-capability-runtime.test.ts.

Linked context

Related to the ongoing tool-schema/plugin SDK fuzz hardening sweep.

Real behavior proof

  • Behavior addressed: a malformed bundled capability registration with an unreadable captured id or tool name field can no longer poison the whole bundled capability runtime snapshot; healthy sibling registrations continue.
  • Real environment tested: local OpenClaw linked worktree on Node/Vitest using the repo scripts/run-vitest.mjs wrapper.
  • Exact steps or command run after this patch: node scripts/run-vitest.mjs run src/plugins/bundled-capability-runtime.test.ts
  • Evidence after fix: passed 1 Vitest shard, 1 file, 2 tests after the final rebase.
  • Observed result after fix: the regression plugin stays loaded, emits a diagnostic for the unreadable captured tool name, and still registers healthy_tool.
  • What was not tested: full pnpm check:changed did not run.
  • Proof limitations or environment constraints: Crabbox changed gate failed before lease with coordinator HTTP 401 for provider azure; blacksmith testbox list --all also failed with not authenticated, so broad remote proof is blocked by runner auth.
  • Before evidence: the focused regression initially showed the healthy captured tool was not present in registry.tools.

Tests and validation

  • ./node_modules/.bin/oxfmt --write --threads=1 src/plugins/bundled-capability-runtime.ts src/plugins/bundled-capability-runtime.test.ts
  • git diff --check
  • node scripts/run-vitest.mjs run src/plugins/bundled-capability-runtime.test.ts
  • $autoreview local dirty-patch review: clean, overall: patch is correct (0.86)
  • Crabbox changed gate attempted with node scripts/crabbox-wrapper.mjs run --shell -- env OPENCLAW_CHECK_CHANGED_REMOTE_CHILD=1 OPENCLAW_CHANGED_LANES_RAW_SYNC=1 corepack pnpm check:changed; blocked by coordinator HTTP 401 on /v1/leases, latest slug pearl-krill.

Regression coverage added in src/plugins/bundled-capability-runtime.test.ts for a temp bundled plugin with one captured tool whose name getter throws followed by a healthy manifest-declared tool.

Risk checklist

  • Did user-visible behavior change? No
  • Did config, environment, or migration behavior change? No
  • Did security, auth, secrets, network, or tool execution behavior change? Yes

Highest-risk area: plugin capability registration projection. The mitigation is that the change only filters malformed captured rows at the runtime boundary, keeps healthy rows intact, and leaves existing manifest contract validation in place.

Current review state

Next action: maintainer review and CI.

Waiting on: CI and, if desired, maintainer-side Crabbox/Testbox changed-gate proof once runner auth is available.

Bot/reviewer comments addressed: none yet.

@vincentkoc vincentkoc self-assigned this Jun 4, 2026
@openclaw-barnacle openclaw-barnacle Bot added size: M maintainer Maintainer-authored PR labels Jun 4, 2026
@clawsweeper

clawsweeper Bot commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed June 30, 2026, 8:42 AM ET / 12:42 UTC.

Summary
The PR preserves bundled manifest contracts on synthesized capability PluginRecords and snapshots captured provider/backend/harness ids plus tool names per row so malformed captured rows are diagnosed without dropping healthy siblings.

PR surface: Source +133, Tests +117. Total +250 across 2 files.

Reproducibility: yes. source-level. Current main and v2026.6.10 build bundled capability records without manifest contracts and read captured tool names inside the registration projection path that this PR isolates.

Review metrics: 1 noteworthy metric.

  • Captured identity families: 17 gated. The readable-string snapshot path now controls provider, backend, harness, and tool capture families, which is the compatibility-sensitive runtime projection surface before merge.

Stored data model
Persistent data-model change detected: serialized state: src/plugins/bundled-capability-runtime.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: partial_overlap
Canonical: #77982
Summary: This PR partially overlaps the canonical bundled manifest-contract propagation bug while also adding distinct malformed captured-row isolation; the focused open PR remains the safer canonical candidate for the contract-only bug.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

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

Rank-up moves:

  • Resolve the draft/conflict state against current main.
  • Choose whether the focused manifest-contract PR lands first or is folded into this branch.
  • Rerun the focused bundled-runtime test plus build or changed-gate proof after refresh.

Risk before merge

Maintainer options:

  1. Refresh And Sequence Before Merge (recommended)
    Resolve the draft/conflict state, choose how this branch relates to the focused manifest-contract PR, then rerun focused bundled-runtime proof plus build or changed-gate proof on the refreshed head.
  2. Land The Focused Contract Fix First
    Land fix(plugins): preserve bundled capability manifest contracts #81312 first if maintainers want manifest-contract propagation separated, then rebase this PR to carry only malformed-capture isolation.
  3. Pause The Broader Branch
    Pause or close this PR if maintainers decide the focused manifest-contract fix is the only desired bundled runtime change for now.

Next step before merge

  • [P2] The remaining action is maintainer sequencing and conflict refresh for a protected compatibility-sensitive runtime PR, not a narrow automated repair.

Security
Cleared: No concrete security or supply-chain regression was found; the diff changes in-memory bundled plugin registration projection and tests without new dependencies, workflows, secrets, permissions, downloads, or package resolution.

Review details

Best possible solution:

Refresh the branch against current main, explicitly sequence or fold the focused manifest-contract fix, and land the malformed-capture row isolation only after focused runtime proof plus build or changed-gate proof on the refreshed merge result.

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

Yes, source-level. Current main and v2026.6.10 build bundled capability records without manifest contracts and read captured tool names inside the registration projection path that this PR isolates.

Is this the best way to solve the issue?

Yes for the malformed-capture boundary, with a sequencing caveat. Snapshotting readable identity fields once at the bundled runtime boundary is narrower than plugin-specific shims, but the manifest-contract overlap should be coordinated with the focused open PR.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: The PR is a bounded plugin-runtime bugfix and hardening change with limited blast radius, not an emergency or active core-runtime outage.
  • merge-risk: 🚨 compatibility: The diff changes bundled plugin registration projection and manifest contract handling, which can affect plugin tool/provider availability and diagnostics after upgrade.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit 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 external-contributor proof gate does not apply to this maintainer-labeled MEMBER PR; the body still includes focused runtime test output and notes the blocked broad changed gate.
Evidence reviewed

PR surface:

Source +133, Tests +117. Total +250 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 169 36 +133
Tests 1 119 2 +117
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 288 38 +250

What I checked:

  • Repository policy: Root AGENTS.md and src/plugins/AGENTS.md were read fully; plugin-runtime compatibility and manifest-first loader guidance apply to this review. (AGENTS.md:1, 6cb82eaab865)
  • Live PR state: Live GitHub metadata reports this PR open, draft, maintainer-labeled, head 81be743, mergeable=CONFLICTING, and mergeStateStatus=DIRTY. (81be743a5eef)
  • Current main still lacks bundled contracts propagation: Current main creates bundled capability PluginRecords without a contracts parameter or contracts field. (src/plugins/bundled-capability-runtime.ts:133, 6cb82eaab865)
  • Current main reads captured identities inside the whole-plugin projection try block: Current main validates captured tools against record.contracts and reads tool.name during projection, so a bad captured name can poison the same registration block the PR isolates. (src/plugins/bundled-capability-runtime.ts:511, 6cb82eaab865)
  • Latest release still has the same bundled runtime shape: Tag v2026.6.10 has the same createCapabilityPluginRecord shape without contracts and the same captured tool validation against record.contracts. (src/plugins/bundled-capability-runtime.ts:133, aa69b12d0086)
  • PR implementation: The PR diff adds contracts to createCapabilityPluginRecord and introduces snapshotCapturedStringField to diagnose unreadable or non-string captured id/name fields per row before registry projection. (src/plugins/bundled-capability-runtime.ts:137, 81be743a5eef)

Likely related people:

  • vincentkoc: This account authored the PR and also appears in recent merged bundled capability/plugin runtime history, including capability Vitest shim and bundled cache work. (role: current branch owner and recent area contributor; confidence: high; commits: 81be743a5eef, 60ea8e9a1cd8, b4e38a7eb090; files: src/plugins/bundled-capability-runtime.ts, src/plugins/bundled-capability-runtime.test.ts)
  • steipete: GitHub history shows repeated prior work on the bundled capability runtime, plugin module loading, plugin contracts docs, and related registry boundaries. (role: feature-history contributor; confidence: high; commits: 040ebadfc52f, 23fd8a90f9bb, 629baf5fa7f6; files: src/plugins/bundled-capability-runtime.ts, src/plugins/loader-records.ts, docs/plugins/manifest.md)
  • shakkernerd: Commit 7641783 added plugin tool manifest contract enforcement across the bundled runtime and registry surfaces that make missing bundled contracts visible. (role: introduced related contract enforcement; confidence: high; commits: 7641783d6b0e; files: src/plugins/bundled-capability-runtime.ts, src/plugins/registry.ts, src/plugins/tool-contracts.ts)
  • dutifulbob: Commit ae4806e changed the same bundled runtime and loader-record contract projection area while adding embedding provider contracts. (role: adjacent contract surface contributor; confidence: medium; commits: ae4806ed9adf; files: src/plugins/bundled-capability-runtime.ts, src/plugins/loader-records.ts, src/plugins/loader-records.test.ts)
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. P2 Normal backlog priority with limited blast radius. labels Jun 4, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 18, 2026
Open
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jun 19, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 26, 2026
Open
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Closing this fuzz-only hardening PR. The only failing state is a synthetic fixture using hostile in-process objects, throwing getters or callbacks, or similarly malformed metadata; there is no linked production report or supported external contract requiring that state.

This adds broad fail-soft behavior and substantial regression surface across consumers. If a real plugin or provider demonstrates the failure, please open a focused issue with the observed payload and stack so validation can live at the single owning registration/parsing boundary. Thanks for exploring it.

@steipete steipete closed this Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vincentkoc vincentkoc

Labels

maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vincentkoc @steipete