Visitar URL original
improve(gateway): leave legacy worktree scans to Doctor by steipete · Pull Request #152802 · openclaw/openclaw · GitHub
Skip to content

improve(gateway): leave legacy worktree scans to Doctor - #152802

Merged
steipete merged 1 commit into
mainfrom
codex/doctor-only-worktree-startup-v1
Sep 19, 2026
Merged

steipete merged 1 commit into
mainfrom
codex/doctor-only-worktree-startup-v1

Conversation

@steipete

@steipete steipete commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

What Problem This Solves

Gateway startup scans session stores for legacy managed-worktree workspace repairs solely to emit a warning, even though Doctor owns detection and repair.

User Impact

Gateway startup no longer performs this legacy workspace scan or prints its automatic openclaw doctor --fix warning. Operators can run openclaw doctor to detect missing canonical workspace metadata and openclaw doctor --fix to repair eligible sessions.

Canonical session readiness, schema opening, admission checks, database cleanup, and runtime handoff remain unchanged. Doctor's existing migrations, including its update-time path, remain available.

Why This Change Was Made

Keep legacy migration diagnostics with Doctor and remove the warning-only scan from normal startup. The existing Doctor owner still repairs workspace metadata after session-key repairs settle; no replacement migration reader or cache is introduced.

Evidence

  • Historical exact-patch proof on 2e053: 66 tests across seven files passed, covering startup-to-Doctor detection/repair, canonical readiness, concurrency, registry recovery, handoff, and cleanup. Selected type/lint/static checks, docs formatting/MDX, and the canonical full-profile build passed; static/build Node was 24.19.0. Fresh P2 review was scoped-clean. The five candidate blobs and canonical patch were preserved by the refresh to 30701b8 on 88e037f.
  • Current-head CI passed for 30701b8. The earlier combined343 integration passed 118 tests across 13 files plus selected static checks; the combined88 union passed 23 tests across the complete model-auth and worker-preparation suites. These are distinct integration scopes, not reruns of every historical test on the refreshed head.
  • Canonical Package Acceptance passed for signed combined package source 0c8e7ed, containing these exact five D5 blobs. Its published openclaw@2026.9.2 missing-configured-plugin-migration cell passed all 17 required phases, including installed update, deferred startup/Doctor behavior, and migration resumption.
  • Same-artifact projects qualification passed all 35 required phases for published openclaw@2026.9.4 to the identical candidate tar and original plugin companions. It verified update-time repair, then a separate retained specimen's first-start preservation, explicit Doctor repair, second-start preservation, retained data, and shutdown. The collector verified package identity and every required phase exactly once in order; the fixture publishes phase receipts and sanitized summaries, not raw private snapshots. The earlier run exposed a fixture expectation that omitted Doctor's existing title repair; correcting that expectation left the product package unchanged.
  • Historical Linux A–B–B–A screen with 10,002 synthetic retained session records: mean readiness 6.03s → 5.82s (214ms lower), two measured samples per version after priming. This does not establish installed/cold-start performance, statistical significance, refreshed-head timing, or the isolated cost of the removed scan.

Native prepare/merge will revalidate current integration and required gates before landing. The upgrade proofs establish behavior, not startup latency.

@clawsweeper

clawsweeper Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime commands Command implementations size: S maintainer Maintainer-authored PR labels Sep 19, 2026
@clawsweeper clawsweeper Bot added P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. 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. labels Sep 19, 2026
@clawsweeper

clawsweeper Bot commented Sep 19, 2026 •

Copy link
Copy Markdown
Contributor

Codex review: blocked before merge. Reviewed September 19, 2026, 2:09 PM ET / 18:09 UTC (Revision 4).

ClawSweeper review

What this changes

Removes Gateway startup’s legacy worktree metadata scan and warning, leaving detection and repair to Doctor and updating the corresponding tests and documentation.

Merge readiness

⛔ Blocked before merge - 1 item remains

No actionable defects found. The new package and upgrade evidence addresses the previous review’s qualification blocker. Current main still performs the scan, so this protected, maintainer-authored PR remains useful.

Priority: P3
Reviewed head: 30701b8c96f23304e78a5753f7d41c76105e4de6

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused simplification with meaningful retained-state coverage and package upgrade evidence resolving the earlier blocker.
Proof confidence 🦞 diamond lobster (5/6) ✨ media proof bonus Sufficient (linked_artifact): The captured package qualification reports exercise installed update and Gateway startup with retained worktree sessions, observe preservation before explicit Doctor repair, and verify preservation afterward. All five changed blobs match the tested package source, and GitHub confirms acceptance succeeded; raw phase logs were inaccessible to this reviewer.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Verified Sufficient (linked_artifact): The captured package qualification reports exercise installed update and Gateway startup with retained worktree sessions, observe preservation before explicit Doctor repair, and verify preservation afterward. All five changed blobs match the tested package source, and GitHub confirms acceptance succeeded; raw phase logs were inaccessible to this reviewer.
Evidence reviewed 7 items Introduced production change: The pinned parent-to-head diff removes only the worktree detection call, its injection seam, and its aggregate warning. Canonical validation, current-authority checks, runtime handoff, concurrency limits, and connection cleanup remain.
Doctor retains the repair owner: Doctor still invokes workspace repair after session-key repairs, defers writes while legacy-main claims remain incomplete, and reports detected or repaired entries. The migration’s detect mode returns before workspace resolution or writes.
Preservation and repair coverage: The retained boundary test checks unchanged entries after startup and read-only Doctor, followed by explicit repair across two stores and database closure. Concurrency tests now observe the existing runtime handoff while retaining readiness, refusal, drainage, and lease checks. Tests were inspected, not executed in this read-only review.
Findings None None.
Security None None.

How this fits together

Gateway startup prepares existing session databases before handing them to runtime services. Doctor separately detects and repairs legacy session metadata, including during updates.

flowchart TD
  A[Existing session databases] --> B[Gateway startup]
  B --> C[Admission and readiness checks]
  C --> D[Runtime session services]
  A --> E[Doctor inspection]
  E --> F{Repair requested}
  F -->|No| G[Report missing metadata]
  F -->|Yes| H[Repair eligible workspace metadata]
Loading

Before merge

  • Add data-model compatibility proof - Confirm migration or upgrade compatibility proof before merge.
Agent review details

Security

None.

PR surface

Source -26, Tests -31, Docs 0. Total -57 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 1 2 28 -26
Tests 3 40 71 -31
Docs 1 1 1 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 43 100 -57

Review metrics

None.

Stored data model

Persistent data-model change detected: migration/backfill/repair: src/config/sessions/startup-migration.ts, unknown-data-model-change: src/config/sessions/startup-migration.ts. Confirm migration or upgrade compatibility proof before merge.

Technical review

Best possible solution:

Keep legacy workspace inspection and repair in Doctor while startup retains database readiness, admission, and runtime recovery responsibilities.

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

Not applicable as a defect reproduction: source confirms the existing warning-only scan, and the PR removes it intentionally; no local runtime execution was performed.

Is this the best way to solve the issue?

Yes. Removing the detection-only startup call uses the existing Doctor owner without adding another reader, configuration option, or migration.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against baec505bffcf.

Labels

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The captured package qualification reports exercise installed update and Gateway startup with retained worktree sessions, observe preservation before explicit Doctor repair, and verify preservation afterward. All five changed blobs match the tested package source, and GitHub confirms acceptance succeeded; raw phase logs were inaccessible to this reviewer.

Label justifications:

  • P3: This is a bounded startup cleanup that removes advisory scanning without changing repair capability or readiness requirements.
  • 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 (linked_artifact): The captured package qualification reports exercise installed update and Gateway startup with retained worktree sessions, observe preservation before explicit Doctor repair, and verify preservation afterward. All five changed blobs match the tested package source, and GitHub confirms acceptance succeeded; raw phase logs were inaccessible to this reviewer.
  • proof: sufficient: Contributor real behavior proof is sufficient. The captured package qualification reports exercise installed update and Gateway startup with retained worktree sessions, observe preservation before explicit Doctor repair, and verify preservation afterward. All five changed blobs match the tested package source, and GitHub confirms acceptance succeeded; raw phase logs were inaccessible to this reviewer.

Evidence

What I checked:

  • Introduced production change: The pinned parent-to-head diff removes only the worktree detection call, its injection seam, and its aggregate warning. Canonical validation, current-authority checks, runtime handoff, concurrency limits, and connection cleanup remain. (src/config/sessions/startup-migration.ts:243, 30701b8c96f2)
  • Doctor retains the repair owner: Doctor still invokes workspace repair after session-key repairs, defers writes while legacy-main claims remain incomplete, and reports detected or repaired entries. The migration’s detect mode returns before workspace resolution or writes. (src/commands/doctor-session-transcripts.ts:321, 30701b8c96f2)
  • Preservation and repair coverage: The retained boundary test checks unchanged entries after startup and read-only Doctor, followed by explicit repair across two stores and database closure. Concurrency tests now observe the existing runtime handoff while retaining readiness, refusal, drainage, and lease checks. Tests were inspected, not executed in this read-only review. (src/commands/doctor-session-worktree-workspace.test.ts:103, 30701b8c96f2)
  • Still necessary on main and latest release: Fetched main still calls the worktree migration in detect mode and emits the pending-repair warning. A separate GitHub contents read confirmed the same behavior in v2026.9.5; the initial local release-blob read failed and was replaced by that successful API inspection. (src/config/sessions/startup-migration.ts:260, baec505bffcf)
  • Package source matches the reviewed change: GitHub contents API blob IDs matched local HEAD for all five changed files in combined package source 0c8e7ed. The production file matched blob 0ea0ea84621ca9aa360fa9c1a308c3bd7b20cf02. This verifies source identity separately from behavioral evidence. (src/config/sessions/startup-migration.ts, 0c8e7edc2f68)
  • Previous qualification blocker addressed: The captured body, sourceRevision 4685e5d0e7796608f62d9b351c1c1f86fa193d9267011efba161f694759189a4, reports 35 successful phases covering published 2026.9.4 update, first-start preservation, explicit Doctor repair, second-start preservation, retained data, and shutdown against the same package. GitHub independently confirms the targeted Docker lane, npm installation, and acceptance verifier succeeded in https://github.com/openclaw/openclaw/actions/runs/35458812919. The separate 2026.9.2 qualification and exact-head CI also succeeded. Raw job-log retrieval was blocked by the environment’s storage-host allowlist, so individual receipts were not independently inspected. (0c8e7edc2f68)

Likely related people:

  • steipete: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

  • 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.

History

Review history (3 earlier review cycles)
  • reviewed 2026-09-19T11:01:08.627Z sha bc876cf :: blocked before merge. :: none
  • reviewed 2026-09-19T11:47:36.774Z sha bc876cf :: blocked before merge. :: none
  • reviewed 2026-09-19T12:44:04.860Z sha 30701b8 :: blocked before merge. :: none

@steipete
steipete marked this pull request as ready for review September 19, 2026 11:38
@steipete
steipete force-pushed the codex/doctor-only-worktree-startup-v1 branch from bc876cf to 30701b8 Compare September 19, 2026 12:37
@fuller-stack-dev fuller-stack-dev self-assigned this Sep 19, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Sep 19, 2026
@steipete
steipete merged commit 2a9266e into main Sep 19, 2026
215 of 216 checks passed
@steipete
steipete deleted the codex/doctor-only-worktree-startup-v1 branch September 19, 2026 18:34
@steipete

Copy link
Copy Markdown
Contributor Author

Merged via squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands Command implementations docs Improvements or additions to documentation gateway Gateway runtime maintainer Maintainer-authored PR P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants