Skip to content

fix: mock plugin blob stores#47

Merged
steipete merged 1 commit into
mainfrom
fix/mock-plugin-blob-store
Jul 18, 2026
Merged

fix: mock plugin blob stores#47
steipete merged 1 commit into
mainfrom
fix/mock-plugin-blob-store

Conversation

@steipete

Copy link
Copy Markdown
Contributor

Summary

  • add an in-memory mock for the current OpenClaw runtime.state.openBlobStore contract
  • preserve blob-store byte-copy and expired-key reservation semantics used by trusted plugins
  • cover direct capture and TypeScript runtime-capture paths

Proof

  • npm test (206 tests)
  • AutoReview clean
  • Crabpot Diffs fixture isolated execution: 7/7 steps passed; synthetic probes 0 failures

@steipete
steipete requested a review from a team as a code owner July 18, 2026 05:21
@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. labels Jul 18, 2026
@clawsweeper

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 18, 2026, 1:24 AM ET / 05:24 UTC.

Summary
The PR adds an in-memory runtime.state.openBlobStore mock to plugin capture, with tests for byte-copy isolation, expiration reservation, and TypeScript runtime capture.

Reproducibility: no. high-confidence live reproduction was supplied. The source and tests clearly exercise the new mock path, but they do not demonstrate a real plugin capture using the host blob-store contract after the change.

Review metrics: 2 noteworthy metrics.

  • Patch surface: 4 files affected; 137 added, 0 removed. The change is narrowly concentrated in the capture mock and its direct/TypeScript coverage, which makes the remaining runtime-proof gap easy to isolate.
  • Automated checks: 4 reported successful checks. Node 22, CodeQL, and both analysis checks support code quality but do not replace after-fix behavior proof from a real plugin setup.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
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:

  • [P1] Run the crabpot plugin-inspector smoke required by AGENTS.md.
  • [P1] Add redacted terminal output or logs from a real blob-store-using plugin capture.
  • Update the PR body with the proof; if no new review appears, ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR provides unit-test, CI, and synthetic-fixture claims, but no redacted after-fix output from a real plugin capture exercising openBlobStore; add that evidence and the required crabpot smoke before merge. 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] Merging changes the capture mock’s compatibility surface for trusted plugins using blob stores, but the available proof does not show an after-fix capture from a real plugin setup.
  • [P1] The repository-required crabpot plugin-inspector smoke is not evidenced by the supplied PR body; the listed isolated Diffs fixture run is useful supplemental validation but is not the same check.

Maintainer options:

  1. Prove the runtime contract before merge (recommended)
    Run the required crabpot plugin-inspector smoke and add redacted output from a real blob-store-using plugin capture so the compatibility bridge is demonstrated outside mocks.
  2. Pause for host-contract confirmation
    If a real plugin fixture is not currently available, pause until a maintainer confirms that the mock’s supported method and expiration semantics match the intended host contract.

Next step before merge

  • [P1] No automated repair is appropriate: the remaining blockers are contributor-owned real behavior evidence and required validation, not a concrete code defect.

Security
Cleared: The patch is limited to in-memory test/capture runtime behavior and tests; it adds no dependency, workflow, publishing, secret, artifact-download, or execution-path change.

Review details

Best possible solution:

Keep the mock narrowly scoped, run the repository-required crabpot plugin-inspector smoke, and attach redacted real-plugin capture output showing a blob-store-using plugin completes under the capture harness.

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

No high-confidence live reproduction was supplied. The source and tests clearly exercise the new mock path, but they do not demonstrate a real plugin capture using the host blob-store contract after the change.

Is this the best way to solve the issue?

Unclear. The focused in-memory mock is a plausible narrow solution, but a real capture and the required crabpot smoke are needed to confirm it models the supported host contract rather than only its new unit tests.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a bounded capture-runtime compatibility change with normal user impact and a clear proof requirement before merge.
  • add merge-risk: 🚨 compatibility: The PR expands the mocked plugin runtime contract, so an inaccurate semantic match could change which trusted plugins can be captured successfully.
  • 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 provides unit-test, CI, and synthetic-fixture claims, but no redacted after-fix output from a real plugin capture exercising openBlobStore; add that evidence and the required crabpot smoke before merge. 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 bounded capture-runtime compatibility change with normal user impact and a clear proof requirement before merge.
  • merge-risk: 🚨 compatibility: The PR expands the mocked plugin runtime contract, so an inaccurate semantic match could change which trusted plugins can be captured successfully.
  • 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 provides unit-test, CI, and synthetic-fixture claims, but no redacted after-fix output from a real plugin capture exercising openBlobStore; add that evidence and the required crabpot smoke before merge. 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

What I checked:

  • Focused capture-runtime implementation: The proposed implementation adds a per-namespace blob-store mock under runtime.state, returning copied byte arrays on reads and retaining expired keys until explicit deletion, which matches the behavior described in the PR summary. (src/capture-api.js:205, 927609712515)
  • Targeted regression coverage: The direct capture test covers input and output byte copying plus expired-key reservation/deletion, while the TypeScript fixture calls the new runtime method. (test/capture-api.test.js:88, 927609712515)
  • Automated validation reported: The supplied PR context reports four successful checks, including Node 22 and CodeQL, and the PR body reports npm test with 206 tests plus isolated synthetic probes. (927609712515)
  • Repository policy applied: The repository policy requires plugin-inspector behavior changes to receive the crabpot plugin-inspector smoke before the work is called complete; the supplied proof names an isolated Diffs fixture run, not that smoke. (AGENTS.md:12)

Likely related people:

  • steipete: The only available provenance identifies this person as the author of the sole commit adding the capture-runtime blob-store support; no independent current-main ownership trail was available because the restricted sandbox rejected read-only git commands before execution. (role: recent contributor; confidence: low; commits: 927609712515; files: src/capture-api.js, test/capture-api.test.js, test/runtime-capture-report.test.js)
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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9276097125

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/capture-api.js
Comment on lines +253 to +254
values.set(key, {
bytes: Uint8Array.from(bytes),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Enforce blob store limits in the mock

When a plugin creates blobs during registration or synthetic probes, this mock stores every entry unconditionally, so blobs larger than maxBytesPerEntry or writes that exceed maxEntries/maxBytesPerNamespace still succeed even though the real OpenClaw blob store rejects or evicts according to those options. That can make runtime capture report a trusted plugin as compatible when the same registration path would fail under the host runtime; please simulate the configured byte/row limits before inserting.

Useful? React with 👍 / 👎.

Comment thread src/capture-api.js
const ttlMs = registerOptions?.ttlMs ?? options.defaultTtlMs;
values.set(key, {
bytes: Uint8Array.from(bytes),
metadata,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clone blob metadata at store boundaries

If a plugin mutates the metadata object after register()/registerIfAbsent() or passes metadata that JSON persistence would reject, the mock keeps that original object reference and later lookup(), entries(), or deleteExpired() observes the mutated/invalid value. The real blob store persists metadata as JSON and parses it on reads, so metadata is copied and validated at the API boundary; this divergence can let capture pass for plugins that would fail or clean up the wrong artifact under OpenClaw.

Useful? React with 👍 / 👎.

@steipete
steipete merged commit f165eaf into main Jul 18, 2026
4 checks passed
@steipete
steipete deleted the fix/mock-plugin-blob-store branch July 18, 2026 05:29
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. 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