Expose legacy cron store doctor lint findings by giodl73-repo · Pull Request #99211 · openclaw/openclaw

giodl73-repo · 2026-07-02T19:10:07Z

Summary

expose default-disabled core/doctor/legacy-cron-store findings for legacy cron stores, legacy run logs, quarantined rows, SQLite projection backfills, legacy notify fallback, and stale dreaming payload shape
use read-only state inspection paths for selected lint
keep all real cron mutation in the existing legacy cron doctor --fix contribution; this PR is lint-only and does not add a structured repair/dry-run hook

Contract / compatibility

No public SDK, plugin, config, or CLI contract changes.
Default doctor --lint is unchanged; --only core/doctor/legacy-cron-store and --all can select it.
Selected lint must not create or rewrite user cron state.

Validation on `9ca7491f8c`

node scripts/run-vitest.mjs src/commands/doctor/cron/index.test.ts src/flows/doctor-health-contributions.test.ts (119 tests)
changed-file oxfmt --check
changed-file oxlint --tsconfig config/tsconfig/oxlint.core.json
pnpm check:architecture
node scripts/plugin-sdk-surface-report.mjs --check
git diff --check

Type-check note

Hosted check-test-types on 56a56afdc5 failed on an untyped test mock; fixed on 9ca7491f8c by typing collectLegacyCronStoreHealthFindings as Promise<readonly HealthFinding[]>.
Local Windows pnpm check:test-types / pnpm tsgo:core:test timed out without diagnostics after the fix; hosted CI is the authoritative type-check proof for the refreshed head.

Real behavior proof on `56a56afdc5` (source behavior unchanged on `9ca7491f8c`)

Ran source CLI with isolated OPENCLAW_CONFIG_PATH and OPENCLAW_STATE_DIR against a temp legacy cron JSON array store:
node scripts/run-node.mjs doctor --lint --json --only core/doctor/legacy-cron-store
Result: exit code 1, checksRun: 1, checksSkipped: 44, four core/doctor/legacy-cron-store warning findings for the temp legacy store.
Read-only proof: <state>/openclaw-state.sqlite was not created and <state>/identity/device.json was not created.

Notes

The earlier structured dry-run preview text was removed after Omar's review because the core check repair hook was not reachable from a supported Doctor CLI path. Real mutation remains in the legacy cron doctor --fix path.

giodl73-repo · 2026-07-02T19:10:16Z

@clawsweeper review

clawsweeper · 2026-07-02T19:10:20Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

clawsweeper · 2026-07-02T19:11:11Z

Codex review: needs maintainer review before merge. Reviewed July 3, 2026, 5:18 PM ET / 21:18 UTC.

Summary
The branch adds a default-disabled core doctor lint check for legacy cron store state, read-only SQLite/cron inspection helpers, guardrail allowlist coverage, and focused tests while leaving repair in the existing doctor fix path.

PR surface: Source +247, Tests +158, Other +1. Total +406 across 8 files.

Reproducibility: not applicable. as a bug reproduction; this PR adds a new opt-in doctor lint diagnostic. The PR body provides source CLI real behavior proof for selected lint on isolated config/state, and focused tests cover the selection behavior.

Review metrics: 3 noteworthy metrics.

Doctor lint selection surface: 1 added, default-disabled. The new check is skipped by default but can change output and exit code for --all or explicit --only runs.
Read-only SQLite inspection paths: 2 added. The new cron store and migration receipt readers are the safety mechanism behind the no-mutation lint claim.
Structured repair hooks: 0 added. The latest head removed the unreachable dry-run repair hook, keeping mutation in the existing legacy cron repair contribution.

Stored data model
Persistent data-model change detected: migration/backfill/repair: scripts/check-kysely-guardrails.mjs, migration/backfill/repair: src/commands/doctor/cron/index.test.ts, migration/backfill/repair: src/commands/doctor/cron/index.ts, migration/backfill/repair: src/commands/doctor/cron/migration-ledger.ts, migration/backfill/repair: src/flows/doctor-core-checks.ts, migration/backfill/repair: src/flows/doctor-health-contributions.test.ts, and 7 more. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
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:

[P2] Get explicit maintainer acceptance of the new doctor --lint --all and --only core/doctor/legacy-cron-store behavior before merge.

Risk before merge

[P1] Merging can newly produce warning findings and a nonzero lint exit for users or automation that run doctor --lint --all or explicitly select core/doctor/legacy-cron-store, so maintainers need to accept that opt-in diagnostic behavior.

Maintainer options:

Accept the opt-in cron diagnostic (recommended)
Maintain the current lint-only design and merge after protected-label maintainer review confirms the new --all/--only warnings and exit behavior are intended.
Narrow broad lint selection
If maintainers do not want doctor --lint --all to fail on legacy cron state, adjust selection behavior before merge while keeping explicit --only support.
Pause the core diagnostic
If legacy cron upgrade diagnostics should stay only in the existing interactive doctor path, pause or close the PR instead of adding a core lint id.

Next step before merge

[P2] Human maintainer review is required because the PR has a protected maintainer label and the remaining question is product acceptance of a compatibility-sensitive opt-in diagnostic, not an automation-repairable defect.

Security
Cleared: The diff adds local doctor lint/read-only SQLite inspection and tests, with no workflow, dependency, package-resolution, secret, permission, or external code-execution surface change found.

Review details

Best possible solution:

Merge the lint-only, read-only diagnostic after maintainers explicitly accept the opt-in --all and --only behavior, while keeping all cron mutation in the existing doctor --fix path.

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

Not applicable as a bug reproduction; this PR adds a new opt-in doctor lint diagnostic. The PR body provides source CLI real behavior proof for selected lint on isolated config/state, and focused tests cover the selection behavior.

Is this the best way to solve the issue?

Yes for the implementation shape: the read-only lint check uses the existing doctor lint path and leaves repair in the legacy cron doctor --fix contribution. Product acceptance of the new opt-in diagnostic output remains the maintainer decision.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7d593bb9ae54.

Label changes

Label changes:

add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied source CLI proof for the selected lint path on isolated config/state, including exit code, finding count, and no state DB or device identity creation; the latest head only changes guardrail/test typing around that behavior.

Label justifications:

P3: This is a low-priority opt-in doctor diagnostic improvement with limited blast radius, not an urgent broken runtime or channel workflow.
merge-risk: 🚨 compatibility: The PR can newly produce warnings and nonzero lint exits for users or automation that run broad or explicit legacy cron doctor lint selection.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied source CLI proof for the selected lint path on isolated config/state, including exit code, finding count, and no state DB or device identity creation; the latest head only changes guardrail/test typing around that behavior.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied source CLI proof for the selected lint path on isolated config/state, including exit code, finding count, and no state DB or device identity creation; the latest head only changes guardrail/test typing around that behavior.

Evidence reviewed

PR surface:

Source +247, Tests +158, Other +1. Total +406 across 8 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	5	250	3	+247
Tests	2	159	1	+158
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	1	1	0	+1
Total	8	410	4	+406

What I checked:

Repository policy read and applied: Root AGENTS.md and scripts/AGENTS.md were read fully; the review applied the doctor/state compatibility guidance, protected-label handling, read-beyond-diff guidance, and script guardrail guidance. (AGENTS.md:1, 7d593bb9ae54)
Current main lacks the new lint id: Current main's legacy cron contribution lists only core/doctor/legacy-whatsapp-crontab, so the central diagnostic is not already implemented on main. (src/flows/doctor-health-contributions.ts:1736, 7d593bb9ae54)
PR registers the default-disabled check: PR head adds core/doctor/legacy-cron-store as a default-disabled SplitHealthCheckInput that only detects findings through the cron collector. (src/flows/doctor-core-checks.ts:681, 9ca7491f8c61)
Collector uses read-only repair-state loading: The new collector calls loadLegacyCronRepairState with readOnly: true and emits HealthFinding records rather than invoking the existing mutation path. (src/commands/doctor/cron/index.ts:314, 9ca7491f8c61)
Read-only SQLite helper avoids state DB creation: The helper returns an empty cron store when the state DB is absent and opens an existing DB with readOnly: true before loading cron rows. (src/cron/store.ts:102, 9ca7491f8c61)
Lint selection is opt-in for default-disabled checks: The existing lint runner skips default-disabled checks unless --all is set or --only explicitly selects the check, bounding the user-visible change to broad or explicit lint selection. (src/flows/doctor-lint-flow.ts:38, 7d593bb9ae54)

Likely related people:

steipete: Authored the merged cron SQLite persistence work that established the SQLite-backed cron store this diagnostic inspects. (role: cron SQLite persistence history owner; confidence: high; commits: 005da57957c9; files: src/cron/store.ts)
MonkeyLeeT: Co-authored the merged legacy cron auto-migration work whose migration semantics this diagnostic reports on. (role: legacy cron migration introducer; confidence: high; commits: 21aa297434e4; files: src/commands/doctor/cron/index.ts, src/commands/doctor/cron/migration-ledger.ts)
jalehman: Reviewed the merged legacy cron auto-migration PR and is a useful routing candidate for migration-contract questions. (role: legacy cron migration reviewer; confidence: medium; commits: 21aa297434e4; files: src/commands/doctor/cron/index.ts, src/commands/doctor/cron/migration-ledger.ts)
giodl73-repo: Beyond this PR, authored the recently merged default-disabled WhatsApp doctor lint surface that shares the selected-lint pattern used here. (role: recent adjacent structured doctor lint contributor; confidence: medium; commits: 6011c9e16d21; files: src/flows/doctor-core-checks.ts, src/flows/doctor-health-contributions.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.

giodl73-repo · 2026-07-02T20:20:20Z

@clawsweeper review

clawsweeper · 2026-07-02T20:20:23Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

giodl73-repo · 2026-07-02T20:28:01Z

@clawsweeper review

clawsweeper · 2026-07-02T20:28:04Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

omarshahine · 2026-07-02T22:36:49Z

Codex review finding:

The structured dry-run repair effects for core/doctor/legacy-cron-store are not reachable from the supported Doctor CLI. The check defines repair() in CORE_HEALTH_CHECKS (src/flows/doctor-core-checks.ts:681-704), but the contribution only references that id and continues to run runLegacyCronHealth (src/flows/doctor-health-contributions.ts:1685-1688). Core checks are used by lint selection, but the generic structured repair pass filters to extension checks (src/flows/doctor-health-contributions.ts:440-453, src/flows/health-check-registry.ts:29-40), and --only is lint-only in the CLI (src/cli/program/register.maintenance.ts:42-83). So the PR body's structured dry-run preview cannot be invoked by openclaw doctor; real mutation still only happens through the legacy cron doctor contribution.

Best fix: either add a real core structured repair path for this check, or keep the cron-store work lint-only and remove the unreachable repair()/dry-run-effect tests and PR claim.

giodl73-repo · 2026-07-02T23:23:41Z

Addressed Omar's reachability finding by making this PR lint-only again.

What changed:

Removed the structured repair() hook from core/doctor/legacy-cron-store.
Removed the unreachable dry-run repair-effect helper/tests and associated structured preview claim.
Kept the default-disabled legacy cron store lint findings and the read-only inspection helpers; real mutation remains owned by the existing legacy cron doctor --fix contribution.

Validation on 588fbc26f0:

node scripts/run-vitest.mjs src/commands/doctor/cron/index.test.ts src/flows/doctor-health-contributions.test.ts (113 passed)
changed-file pnpm exec oxfmt --check
changed-file pnpm exec oxlint
node scripts/plugin-sdk-surface-report.mjs --check
git diff --check

@clawsweeper review

clawsweeper · 2026-07-02T23:23:45Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

giodl73-repo · 2026-07-02T23:31:13Z

@clawsweeper review final lint-only wording/body synced after Omar fix

giodl73-repo · 2026-07-03T20:30:13Z

Refreshed after #98406/main and addressed the remaining type-surface issue before asking for re-review.

Current head: 56a56afdc5f3117b9f499e96aa77d2e8225579be

Changes since the prior review:

Rebasing onto current main was clean.
Kept the PR lint-only after Omar's reachability finding; no structured repair/dry-run hook is exposed.
Changed legacyCronStoreCheck to SplitHealthCheckInput so defaultEnabled: false is carried on the intended metadata type.

Validation:

Focused cron/contribution Vitest passed (119 tests).
Changed-file oxfmt passed.
Changed-file oxlint with core tsconfig passed.
SDK surface report passed.
git diff --check passed.

Real behavior proof:

Source CLI with isolated config/state and a temp legacy cron JSON array store ran doctor --lint --json --only core/doctor/legacy-cron-store.
It exited 1 with checksRun: 1 and four core/doctor/legacy-cron-store warning findings.
It did not create <state>/openclaw-state.sqlite or <state>/identity/device.json.

@clawsweeper review refreshed lint-only head with real behavior proof

clawsweeper · 2026-07-03T20:30:15Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

giodl73-repo · 2026-07-03T20:36:03Z

Maintainer note before marking ready:

I accept the opt-in core/doctor/legacy-cron-store diagnostic surface for maintainer review.

The check is default-disabled; default doctor --lint remains unchanged.
It is only selected by doctor --lint --all or --only core/doctor/legacy-cron-store.
The PR is lint-only: no structured repair/dry-run hook and no new real mutation path.
Real mutation remains in the existing legacy cron doctor --fix contribution.
Selected lint uses read-only inspection paths; source CLI proof on isolated state found the expected warnings and did not create the state DB or device identity.
No public SDK/plugin/config/CLI contract change.

giodl73-repo · 2026-07-03T21:01:58Z

Refreshed to 9ca7491f8c612f40f20c1440811e7de4a7312e3f after the ready-for-review CI pass found two branch issues.

Fixes:

Added src/commands/doctor/cron/migration-ledger.ts to the explicit raw SQLite/Kysely guardrail allowlist for doctor legacy state migration.
Typed the legacy cron lint mock as Promise<readonly HealthFinding[]> so check-test-types does not infer never.

Validation after fixes:

Focused cron/contribution Vitest passed (119 tests).
Changed-file oxfmt passed.
Changed-file oxlint with core tsconfig passed.
pnpm check:architecture passed.
SDK surface report passed.
git diff --check passed.

Local Windows pnpm check:test-types and pnpm tsgo:core:test timed out without diagnostics after the mock typing fix; hosted check-test-types is the authoritative proof for this refreshed head.

@clawsweeper review refreshed after CI fixes

clawsweeper · 2026-07-03T21:02:01Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

giodl73-repo · 2026-07-03T22:28:53Z

Merged cleanly.

Source head: 9ca7491f8c612f40f20c1440811e7de4a7312e3f
Merge commit: 219f27a5ec6b6ed058c95653ac604bc2a25a26d2
Verification: focused cron/contribution Vitest (119 tests), changed-file oxfmt, changed-file oxlint, pnpm check:architecture, SDK surface report, git diff --check, isolated source CLI proof for doctor --lint --json --only core/doctor/legacy-cron-store, and exact-head hosted checks clean/skipped with only superseded cancelled runs.
ClawSweeper: proof sufficient, platinum hermit, exact-head reviewed.
Maintainer accepted the opt-in diagnostic surface before merge.

Adds a default-disabled Doctor lint check for legacy cron store state while keeping real repair in the existing doctor --fix path. Verification: - Focused cron/contribution Vitest passed (119 tests). - Changed-file oxfmt passed. - Changed-file oxlint with core tsconfig passed. - pnpm check:architecture passed. - SDK surface report passed. - git diff --check passed. - Source CLI proof on isolated config/state passed for doctor --lint --json --only core/doctor/legacy-cron-store. - Exact-head hosted checks are clean/skipped; only superseded auto-response/proof runs were cancelled. Maintainer notes: - ClawSweeper: proof sufficient, platinum hermit, exact-head reviewed. - No public SDK/plugin/config/CLI contract change; default doctor --lint remains unchanged.

openclaw-barnacle Bot added commands Command implementations size: M maintainer Maintainer-authored PR labels Jul 2, 2026

openclaw-barnacle Bot added size: L and removed size: M labels Jul 2, 2026

openclaw-barnacle Bot added size: M and removed size: L labels Jul 2, 2026

giodl73-repo added 3 commits July 3, 2026 13:21

Expose legacy cron store doctor lint findings

a0b4e45

Preview legacy cron store doctor repair effects

b11487e

Keep legacy cron doctor lint read-only

a9f1220

giodl73-repo force-pushed the doctor-legacy-cron-store-lint-findings branch from 588fbc2 to 56a56af Compare July 3, 2026 20:28

giodl73-repo marked this pull request as ready for review July 3, 2026 20:36

Keep legacy cron store doctor check lint-only

9ca7491

giodl73-repo force-pushed the doctor-legacy-cron-store-lint-findings branch from 56a56af to 9ca7491 Compare July 3, 2026 21:01

openclaw-barnacle Bot added the scripts Repository scripts label Jul 3, 2026

clawsweeper Bot added the status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. label Jul 3, 2026

giodl73-repo merged commit 219f27a into openclaw:main Jul 3, 2026
126 of 132 checks passed

github-actions Bot mentioned this pull request Jul 3, 2026

📡 Upstream Digest — 2026-07-03 22:55 UTC curtismercier/openclaw-mods#1200

Open

giodl73-repo mentioned this pull request Jul 4, 2026

Keep legacy WhatsApp crontab lint opt-in #99250

Merged

Uh oh!

Uh oh!

Conversation

giodl73-repo commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Contract / compatibility

Validation on 9ca7491f8c

Type-check note

Real behavior proof on 56a56afdc5 (source behavior unchanged on 9ca7491f8c)

Notes

Uh oh!

giodl73-repo commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

giodl73-repo commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

giodl73-repo commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

omarshahine commented Jul 2, 2026

Uh oh!

giodl73-repo commented Jul 2, 2026

Uh oh!

clawsweeper Bot commented Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

giodl73-repo commented Jul 2, 2026

Uh oh!

giodl73-repo commented Jul 3, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

giodl73-repo commented Jul 3, 2026

Uh oh!

giodl73-repo commented Jul 3, 2026

Uh oh!

clawsweeper Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

giodl73-repo commented Jul 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

giodl73-repo commented Jul 2, 2026 •

edited

Loading

Validation on `9ca7491f8c`

Real behavior proof on `56a56afdc5` (source behavior unchanged on `9ca7491f8c`)

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 2, 2026 •

edited

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading

clawsweeper Bot commented Jul 3, 2026 •

edited

Loading