Skip to content

Doctor: expose shell completion health findings#85566

Merged
giodl73-repo merged 2 commits into
openclaw:mainfrom
giodl73-repo:doctor-core-run-contract-migration
May 25, 2026
Merged

Doctor: expose shell completion health findings#85566
giodl73-repo merged 2 commits into
openclaw:mainfrom
giodl73-repo:doctor-core-run-contract-migration

Conversation

@giodl73-repo

@giodl73-repo giodl73-repo commented May 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds structured doctor visibility for shell-completion health without changing public doctor/plugin SDK contracts:

  • reports slow dynamic completion and missing completion caches as core/doctor/shell-completion findings
  • keeps those findings informational so default doctor --lint compatibility is preserved
  • adds dry-run repair effects only for the cheap previewable cases
  • keeps real shell-completion repair on the existing doctorShellCompletion path, outside the broad structured repair pass

This is the small repeatable doctor pattern: expose structured findings first, add effects where honest, and avoid rewriting legacy repair behavior.

Real behavior proof

Behavior or issue addressed:
Doctor lint and structured repair preview plumbing can now see shell-completion issues as core/doctor/shell-completion findings/effects while normal doctor --fix still uses the existing shell-completion repair flow. The ClawSweeper compatibility ask is addressed by making these findings informational, so default lint does not newly fail for existing slow or missing-cache shell-completion profiles.

Real environment tested:
WSL Ubuntu 24.04 worktree /root/src/openclaw-doctor-shell-main, branch doctor-core-run-contract-migration, source harness on head 1344b6d3c1, Node 24.15.0, pnpm workspace dependencies installed. The terminal proof uses redacted synthetic shell-completion status values and imports the real branch code paths.

Exact steps or command run after this patch:

  1. git diff --check origin/main...HEAD
  2. node_modules/.bin/oxfmt --check --threads=1 src/commands/doctor-completion.ts src/commands/doctor-completion.test.ts src/flows/doctor-lint-flow.test.ts src/flows/doctor-core-checks.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  3. node_modules/.bin/oxlint --tsconfig config/tsconfig/oxlint.core.json src/commands/doctor-completion.ts src/commands/doctor-completion.test.ts src/flows/doctor-lint-flow.test.ts src/flows/doctor-core-checks.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts
  4. node scripts/run-vitest.mjs src/commands/doctor-completion.test.ts src/flows/doctor-lint-flow.test.ts src/flows/doctor-core-checks.test.ts src/flows/doctor-health-contributions.test.ts --reporter=dot
  5. node scripts/run-tsgo.mjs -p tsconfig.core.json --noEmit --incremental false --pretty false
  6. node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --noEmit --incremental false --pretty false
  7. node --import tsx /tmp/85566-proof.mjs
  8. bash /mnt/c/src/claws-hapi/.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main

Evidence after fix:
Focused validation passed:

git diff --check origin/main...HEAD -> pass
oxfmt -> All matched files use the correct format.
oxlint -> Found 0 warnings and 0 errors.
Vitest -> Test Files 4 passed (4); Tests 35 passed (35)
tsgo core -> pass
tsgo core tests -> pass
autoreview -> clean: no accepted/actionable findings reported

Terminal source-harness output from the changed shell-completion mapping and lint exit-code path:

{
  "name": "slowDynamic",
  "findings": [
    {
      "checkId": "core/doctor/shell-completion",
      "severity": "info",
      "message": "Your zsh profile uses slow dynamic completion (source <(...)).",
      "path": "shellCompletion.zsh",
      "fixHint": "Run `openclaw doctor --fix` to upgrade to cached completion."
    }
  ],
  "defaultLintExitCode": 0,
  "effects": [
    {
      "kind": "state",
      "action": "would-generate-completion-cache",
      "target": "/redacted/home/.cache/openclaw/completion.zsh",
      "dryRunSafe": true
    },
    {
      "kind": "file",
      "action": "would-upgrade-shell-profile-completion",
      "target": "zsh",
      "dryRunSafe": false
    }
  ]
}
{
  "name": "missingCache",
  "findings": [
    {
      "checkId": "core/doctor/shell-completion",
      "severity": "info",
      "message": "Shell completion is configured in your zsh profile but the cache is missing.",
      "path": "shellCompletion.zsh",
      "fixHint": "Run `openclaw completion --write-state` or `openclaw doctor --fix` to regenerate /redacted/home/.cache/openclaw/completion.zsh."
    }
  ],
  "defaultLintExitCode": 0,
  "effects": [
    {
      "kind": "state",
      "action": "would-regenerate-completion-cache",
      "target": "/redacted/home/.cache/openclaw/completion.zsh",
      "dryRunSafe": true
    }
  ]
}

Observed result after fix:
Both slow dynamic completion and missing cache states produce structured shell-completion findings and dry-run effects, but the findings are info and exitCodeFromFindings(findings) returns 0, preserving default lint compatibility. Real mutation remains delegated to the existing positional doctorShellCompletion repair path.

What was not tested:
No live shell startup file was modified in this proof. The proof uses a source harness with redacted synthetic shell-completion statuses to exercise the real mapping/effect/exit-code functions; existing shell profile mutation remains covered by the legacy doctorShellCompletion path and focused unit coverage.

Validation

  • Local WSL focused format, lint, Vitest, core typecheck, core test typecheck, and autoreview clean on 1344b6d3c1
  • ClawSweeper P1 compatibility ask addressed: shell-completion findings are informational and default lint exit remains zero
  • Public plugin SDK contract unchanged by this PR

Doctor stack

Order PR Area Status
1 #85566 Shell completion health findings Ready after compatibility/proof update; checks pending on latest push
2 #84290 UI freshness health findings Ready; checks passing after latest fix
3 #84326 Sandbox registry findings Ready/checks clean; stale labels may remain
4 #84340 Extra gateway service findings Ready/checks clean; stale labels may remain
5 #84366 Session lock findings Draft; checks/proof passing, keep draft until reviewed
6 #84450 Config audit scrub findings Draft; checks/proof passing, keep draft until reviewed
7 #84472 Dry-run preview reports Draft; broad preview surface, likely last/deferred

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation commands Command implementations size: XL maintainer Maintainer-authored PR labels May 23, 2026
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

Re-review progress:

@clawsweeper

clawsweeper Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 25, 2026, 11:18 AM ET / 15:18 UTC.

Summary
The branch adds a core shell-completion doctor health check that maps slow dynamic completion and missing completion caches to structured findings/effects while keeping real mutation on the legacy repair path.

PR surface: Source +99, Tests +150. Total +249 across 6 files.

Reproducibility: yes. for the blocking regression from source: the PR adds info findings, while the CLI default severity threshold is also info, so those findings would make default lint nonzero. I did not run the CLI because this review is read-only.

Review metrics: 1 noteworthy metric.

  • Default lint-affecting states: 2 shell-completion states added. Both new states flow into the doctor lint exit threshold, so they matter for existing automation before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Fix default doctor lint compatibility through the CLI path and add a runDoctorLintCli regression.
  • Add redacted terminal output or logs from the real openclaw doctor --lint --only core/doctor/shell-completion --json path showing the fixed exit behavior.

Proof guidance:
Needs stronger real behavior proof before merge: The terminal proof is useful but insufficient because it exercises a helper/source harness rather than the real openclaw doctor --lint default threshold path; add redacted CLI output or logs after fixing the compatibility path. 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

  • Default openclaw doctor --lint can start exiting 1 for existing slow dynamic or missing-cache shell-completion profiles because the CLI's default threshold is info.
  • The posted proof exercises a helper/source harness default, not the actual runDoctorLintCli default threshold path that users and automation run.

Maintainer options:

  1. Fix the default lint path before merge (recommended)
    Update the branch so default openclaw doctor --lint remains zero for shell-completion info findings, with a runDoctorLintCli regression covering slow dynamic and missing-cache statuses.
  2. Accept stricter lint intentionally
    Maintainers can choose to make shell-completion info findings fail default lint, but that should be documented as an intentional compatibility change.

Next step before merge
Needs contributor/maintainer follow-up because the proof and code disagree on default lint compatibility; do not route to automation until the PR has CLI-level fix/proof.

Security
Cleared: No concrete security or supply-chain concern was found in the diff; the patch changes doctor reporting and dry-run effects without adding dependencies or new code execution sources.

Review findings

  • [P1] Preserve default lint exit for completion info — src/commands/doctor-completion.ts:98-109
Review details

Best possible solution:

Keep the structured detector, but land it only after default lint compatibility is preserved and proven through the actual CLI path.

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

Yes for the blocking regression from source: the PR adds info findings, while the CLI default severity threshold is also info, so those findings would make default lint nonzero. I did not run the CLI because this review is read-only.

Is this the best way to solve the issue?

No. The structured check shape is reasonable, but the compatibility fix must be proven against runDoctorLintCli or the real openclaw doctor --lint path rather than the helper default.

Full review comments:

  • [P1] Preserve default lint exit for completion info — src/commands/doctor-completion.ts:98-109
    These new findings are info, but runDoctorLintCli defaults severityMin to "info" and uses that same threshold for ok and the returned exit code. That means a user with slow dynamic completion or a missing cache will now get exit 1 from default openclaw doctor --lint, contrary to the PR's compatibility goal; please cover the real CLI path and keep default lint non-failing for these informational states.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 8da8bc4aadfc.

Label changes

Label changes:

  • add merge-risk: 🚨 automation: The affected surface is the automation-friendly doctor lint command used by CI, preflight, and review gates.

Label justifications:

  • P2: This is a normal doctor improvement with a limited but real compatibility blocker in the lint path.
  • merge-risk: 🚨 compatibility: Existing users with slow dynamic completion or a missing completion cache may see default doctor --lint fail after upgrade.
  • merge-risk: 🚨 automation: The affected surface is the automation-friendly doctor lint command used by CI, preflight, and review gates.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The terminal proof is useful but insufficient because it exercises a helper/source harness rather than the real openclaw doctor --lint default threshold path; add redacted CLI output or logs after fixing the compatibility path. 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

PR surface:

Source +99, Tests +150. Total +249 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 3 106 7 +99
Tests 3 150 0 +150
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 256 7 +249

What I checked:

Likely related people:

  • giodl73-repo: Authored the recently merged doctor repair-runner normalization that this PR builds on, including doctor-repair-flow.ts and related tests. (role: recent area contributor; confidence: high; commits: d139d39e8840, 056378efd582; files: src/flows/doctor-repair-flow.ts, src/flows/doctor-lint-flow.test.ts, src/flows/health-check-adapter.ts)
  • steipete: Current-main blame for the doctor lint threshold and existing shell-completion doctor code points to commit f950132. (role: recent area contributor; confidence: medium; commits: f950132207f8; files: src/commands/doctor-lint.ts, src/commands/doctor-completion.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.

@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 backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels May 23, 2026
@clawsweeper

clawsweeper Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@giodl73-repo
giodl73-repo force-pushed the doctor-core-run-contract-migration branch from c23c553 to d64ff36 Compare May 23, 2026 05:20
@openclaw-barnacle openclaw-barnacle Bot removed the docs Improvements or additions to documentation label May 23, 2026
@giodl73-repo
giodl73-repo changed the base branch from main to doctor-detection-interactive-maintenance May 23, 2026 05:21
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated PR2 onto current #83753 (d139d39) and refreshed the proof/body. The branch now keeps run(ctx) internal to the core runner path and leaves the public plugin health contract as split detect(ctx, scope?) plus optional repair(ctx, findings).

@clawsweeper

clawsweeper Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

Re-review progress:

@giodl73-repo
giodl73-repo force-pushed the doctor-detection-interactive-maintenance branch from 9c35ac5 to d139d39 Compare May 23, 2026 05:22
@giodl73-repo
giodl73-repo force-pushed the doctor-core-run-contract-migration branch from d64ff36 to 6a4e5ad Compare May 23, 2026 16:31
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation channel: discord Channel integration: discord channel: signal Channel integration: signal channel: slack Channel integration: slack channel: telegram Channel integration: telegram channel: whatsapp-web Channel integration: whatsapp-web labels May 23, 2026
@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

Re-review progress:

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated this PR on current main and addressed the SDK boundary finding.

What changed:

  • registerHealthCheck, listHealthChecks, and getHealthCheck now remain public split-contract APIs typed around HealthCheck.
  • Core doctor paths use internal registry helpers for HealthCheckInput / RegisteredHealthCheck so runnable checks stay core-only.
  • Added a regression test through openclaw/plugin-sdk/health proving the public shell-completion check does not expose run or sourceContract, while the internal registry still can run it.

Validation run locally in WSL from /root/src/openclaw-85566 after rebase:

  • node scripts/run-vitest.mjs src/flows/doctor-core-checks.test.ts src/flows/doctor-health-contributions.test.ts src/flows/doctor-lint-flow.test.ts src/flows/doctor-repair-flow.test.ts --reporter=dot -> 4 files, 45 tests passed
  • pnpm exec oxfmt --check src/flows/health-check-registry.ts src/flows/doctor-lint-flow.ts src/flows/doctor-repair-flow.ts src/flows/doctor-core-checks.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts src/flows/doctor-core-checks.test.ts
  • pnpm exec oxlint src/flows/health-check-registry.ts src/flows/doctor-lint-flow.ts src/flows/doctor-repair-flow.ts src/flows/doctor-core-checks.ts src/flows/doctor-health-contributions.ts src/flows/doctor-health-contributions.test.ts src/flows/doctor-core-checks.test.ts
  • node scripts/run-tsgo.mjs -p tsconfig.core.json --noEmit --incremental false --pretty false
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --noEmit --incremental false --pretty false
  • git diff --check FETCH_HEAD...HEAD

No merge performed.

@clawsweeper

clawsweeper Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

Re-review progress:

@giodl73-repo

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Updated #85566 to follow the latest ClawSweeper asks:

  • Preserved default doctor --lint compatibility by making shell-completion structured findings informational; the added regression proves informational findings keep the default lint exit code at 0.
  • Added terminal source-harness output to the PR body showing slow dynamic completion and missing-cache states produce structured findings/effects while defaultLintExitCode remains 0.
  • Re-ran focused WSL proof: git diff --check, oxfmt, oxlint, focused Vitest, core typecheck, core test typecheck, and autoreview all clean.

No merge performed.

@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Rebased #85566 onto current main and force-pushed signed head 1344b6d3c1.

ClawSweeper asks remain addressed on the new head:

  • Shell-completion findings are informational, preserving default doctor --lint compatibility.
  • PR body includes terminal source-harness proof showing defaultLintExitCode: 0 for slow dynamic completion and missing-cache states, plus structured dry-run effects.
  • Local WSL proof after rebase: git diff --check, oxfmt, oxlint, focused Vitest, core typecheck, core test typecheck, source harness, and autoreview all clean.

No merge performed.

@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
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.

Re-review progress:

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

Labels

commands Command implementations maintainer Maintainer-authored PR merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: M 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