fix(media): guard document extractor metadata#89789
Conversation
|
Codex review: needs maintainer review before merge. Reviewed June 21, 2026, 6:25 AM ET / 10:25 UTC. Summary PR surface: Source +25, Tests +47. Total +72 across 2 files. Reproducibility: yes. by source inspection. Current main reads Review metrics: none identified. Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Refresh the branch against current main, keep the dispatcher guard if the merge result is still narrow, and land it only with focused document-extractor tests plus the appropriate changed-surface gate. Do we have a high-confidence way to reproduce the issue? Yes by source inspection. Current main reads Is this the best way to solve the issue? Yes for the narrow runtime-boundary hardening. Loader-level artifact isolation, inspect taxonomy, and installed-package build fixes remain separate related work for the broader document-extract report. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 72b9bc730370. Label changesLabel justifications:
Evidence reviewedPR surface: Source +25, Tests +47. Total +72 across 2 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
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
|
|
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. |
Summary
thisbinding when invoking valid extractors.Verification
document extractor mimeTypes getter explodedbefore the healthy PDF extractor ran.node_modules/.bin/oxfmt --check src/media/document-extractors.runtime.ts src/media/document-extractors.runtime.test.tsnode_modules/.bin/oxlint src/media/document-extractors.runtime.ts src/media/document-extractors.runtime.test.tsgit diff --checkOPENCLAW_VITEST_FS_MODULE_CACHE_PATH=/tmp/openclaw-vitest-next90 node_modules/.bin/vitest run src/media/document-extractors.runtime.test.ts -t "keeps trying healthy document extractors after unreadable metadata" --pool=forks --maxWorkers=1 --maxConcurrency=1-> 1 file / 1 test, 2 skippedOPENCLAW_VITEST_FS_MODULE_CACHE_PATH=/tmp/openclaw-vitest-next90 node_modules/.bin/vitest run src/media/document-extractors.runtime.test.ts --pool=forks --maxWorkers=1 --maxConcurrency=1-> 1 file / 3 tests.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main-> clean, no accepted/actionable findingsBehavior addressed: malformed document extractor metadata no longer crashes document extraction before healthy plugin extractors can run.
Real environment tested: local macOS worktree on branch
fuzz-tool-schema-next90-20260603, direct Vitest binary with one fork for the focused and full document-extractor runtime tests.Exact steps or command run after this patch:
OPENCLAW_VITEST_FS_MODULE_CACHE_PATH=/tmp/openclaw-vitest-next90 node_modules/.bin/vitest run src/media/document-extractors.runtime.test.ts --pool=forks --maxWorkers=1 --maxConcurrency=1.Evidence after fix: full
src/media/document-extractors.runtime.test.tspassed with 1 file / 3 tests; focused regression passed with 1 file / 1 test, 2 skipped.Observed result after fix: unreadable extractor metadata is skipped, the healthy PDF extractor still runs, and method-style extractor receiver binding is preserved.
What was not tested: full
pnpm check:changed; Blacksmith Testbox is unauthenticated locally, and AWS Crabbox changed-gate setup failed before lease creation with coordinator HTTP 401 on/v1/runsand/v1/leases.