policy: classify doctor fix recommendations#99686
Conversation
|
Codex review: needs maintainer review before merge. Reviewed July 4, 2026, 10:33 PM ET / 02:33 UTC. Summary PR surface: Source +480, Tests +143. Total +623 across 5 files. Reproducibility: not applicable. this is a policy JSON metadata feature PR, not a bug report. The changed behavior is supported by source inspection, targeted tests, and contributor live-output proof for policy JSON and attestation hashing. Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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: Land the policy-owned metadata surface after maintainer signoff on the classification taxonomy, while keeping generic Do we have a high-confidence way to reproduce the issue? Not applicable: this is a policy JSON metadata feature PR, not a bug report. The changed behavior is supported by source inspection, targeted tests, and contributor live-output proof for policy JSON and attestation hashing. Is this the best way to solve the issue? Yes as an implementation shape: keeping recommendations policy-owned and out of generic doctor lint and attestation hashes is the narrow maintainable boundary. The remaining question is maintainer acceptance of the classification taxonomy. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against eb2f9bece730. Label changesLabel justifications:
Evidence reviewedPR surface: Source +480, Tests +143. Total +623 across 5 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
Review history (14 earlier review cycles; latest 8 shown)
|
|
@clawsweeper re-review I restacked the policy series onto current Fresh proof from the top of the stack (
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
Codex review of the Fix Metadata PR. Findings:
Best-fix verdict: not the best fix yet. The useful metadata needs to reach the public JSON surfaces, and the SDK shape should stay generic unless the PR intentionally defines a shared recommendation contract. Evidence checked: |
|
Addressed Omar's JSON-surface finding in |
|
@clawsweeper re-review Reviewer-requested fixes are pushed on this updated head. Fresh local proof before push: focused policy/core/CLI/tooling Vitest passed, |
|
🦞🧹 I asked ClawSweeper to review this item again. |
076a5cb to
2466c7a
Compare
|
@clawsweeper re-review Restacked this PR directly onto current I also refreshed the PR body with real JSON behavior proof:
Fresh local validation on the restacked head passed:
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
2a20d12 to
1946575
Compare
1946575 to
ef4face
Compare
|
Merged via rebase.
|
Summary
automatic,reviewRequired,manual,validateOnly, orunsupportedpolicy.fixRecommendationHealthFinding/ genericdoctor --lint --jsoncontract unchangedValidation
node scripts/run-vitest.mjs extensions/policy/src/doctor/metadata.test.ts extensions/policy/src/doctor/register.test.ts extensions/policy/src/doctor/strictness.test.ts src/flows/doctor-repair-flow.test.ts extensions/policy/src/cli.test.ts src/commands/doctor-lint.test.ts test/scripts/plugin-sdk-surface-report.test.ts --reporter=dotpassed: 4 shards / 367 tests.node scripts/run-tsgo.mjs -p tsconfig.extensions.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/extensions.tsbuildinfopassed.OPENCLAW_OXLINT_SKIP_PREPARE=1 node scripts/run-oxlint.mjs --tsconfig config/tsconfig/oxlint.extensions.json extensionspassed.git diff --checkpassed.node --import tsx scripts/check-policy-config-coverage.ts --check --jsonpassed with no unclassified, unmatched, or stale monitored paths.Note: the normal local
run-oxlint.mjswrapper is blocked on this Windows checkout before linting by a new-base Rolldown boundary root shim error,TypeError: Cannot convert undefined or null to object; the actual extension oxlint pass is clean when using the repo-supported prepare skip.Stack
Restacked directly onto current
openclaw/openclaw@mainafter #99121 merged. Current PR head:2c05bf83e875f5d7a7683fb9e405f6ba4bdb271e.Downstream repair PRs consume this classification metadata in separate slices; this PR only introduces the metadata and policy-owned JSON exposure.
Real behavior proof
Behavior or issue addressed:
Policy findings expose structured recommendation metadata in public policy JSON output, while generic doctor lint and policy attestation hashing remain recommendation-free.
Real environment tested:
Windows checkout
C:\src\openclaw-forkon current #99686 head2c05bf83e875f5d7a7683fb9e405f6ba4bdb271e, with an isolated temp workspace under%TEMP%\openclaw-policy-proof-*. The temp config enabled the source policy plugin path, enabled the Telegram channel, and authored apolicy.jsoncdeny rule for providertelegram. No live LLM or network provider calls were made.Exact steps or command run after this patch:
Evidence after fix:
policyCheckCommand({ json: true })exited1as expected for the seeded policy violation and emitted this public finding shape:{ "checkId": "policy/channels-denied-provider", "severity": "error", "ocPath": "oc://openclaw.config/channels/telegram", "target": "oc://openclaw.config/channels/telegram", "requirement": "oc://policy.jsonc/channels/denyRules/#0", "policy": { "fixRecommendation": { "fixClass": "automatic", "policyPath": ["channels", "denyRules"], "configTargets": ["channels"], "summary": "Disable product-managed channels matching the denied provider." } } }The same run emitted this attestation object, with no recommendation payload in the attestation itself:
{ "checkedAt": "2026-07-05T00:53:54.236Z", "policy": { "path": "policy.jsonc", "hash": "sha256:f7bea32f53401bb152a32836107514220ab6282b5f5c7149787f4c5ee3c685bb" }, "workspace": { "scope": "policy", "hash": "sha256:a03faac8463be7dbad38177af758315a5d39438a2e338118fa1a847adb93ac36" }, "findingsHash": "sha256:66792d3024b94eeea5769cb13fc07cebad84247704006fa05928741676011936", "attestationHash": "sha256:8ba8818628c7e36a6b76e5fa5692fe1ba88923b3530c7ad64c540a42dcea0cf7" }Hash check from the same current-head run:
{ "attestationFindingsHash": "sha256:66792d3024b94eeea5769cb13fc07cebad84247704006fa05928741676011936", "strippedFindingsHash": "sha256:66792d3024b94eeea5769cb13fc07cebad84247704006fa05928741676011936", "publicFindingsHash": "sha256:6a0a798fd82057422c891d75b4ea359ce1e728862e3cba83b81c82da72d74229", "matchesStripped": true, "differsFromPublic": true }Observed result after fix:
The public policy JSON finding includes
policy.fixRecommendation, but the attestationfindingsHashmatches the recommendation-stripped finding and differs from the public finding hash. This proves recommendation copy/classification changes do not perturb policy attestation hashes.What was not tested:
No packaged release binary, live provider run, or live LLM call was run. The proof used the source policy command implementation from the current PR head.