fix(doctor): recover stale managed daemon configuration

[bcdonadio]

Summary

Fixes #669.

• let lcm doctor delegate only an exact authenticated stale-config refusal from ensureDaemon to the existing safe restartDaemon repair path
• preserve identical lifecycle options, all non-stale fail-closed behavior, exact refusal remediation, and version-mismatch behavior
• report successful repair as a warning with fixApplied: true, backed by regression-first TDD
• document the recovery behavior and add a patch changeset

TDD and verification

• RED: 2 stale-config regression cases failed because restartDaemon was called 0 times; non-stale control passed
• GREEN: focused doctor tests 170 passed
• npm run typecheck passed
• ESLint on changed TypeScript passed
• canonical npm run test:ci: 260 passed / 2 skipped files; 6,333 passed / 12 skipped tests; 100% statements (26,820/26,820), branches (19,380/19,380), functions (4,251/4,251), and lines (24,483/24,483)

MoM review

• Plan: gpt-5.6-luna max — APPROVED
• Adversarial: cortex-hq/zai-org-GLM-5.2 max + xai/grok-4.5 max — APPROVED
• Second pass: anthropic/claude-opus-5 medium with both reports — APPROVED

Changeset

.changeset/steady-doctors-repair.md — patch

[qodo-code-review]

PR Summary by Qodo

Fix lcm doctor by repairing authenticated stale managed daemon config

🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Retry daemon lifecycle once via authenticated restart when ensureDaemon refuses with exact
 stale-config.
• Preserve fail-closed behavior for all other refusal reasons and version-mismatch handling.
• Report successful repair as warn with fixApplied: true, and document the recovery behavior.

Diagram

sequenceDiagram
  participant D as "Doctor (runDoctor)"
  participant L as "Lifecycle module"
  participant DM as "Managed daemon"
  participant H as "Health endpoint"

  D->>L: ensureDaemon(options)
  alt version mismatch
    D->>L: restartDaemon(options)
  else ensure refused: stale-config
    D->>L: restartDaemon(options)
  end
  D->>H: fetch /health (auth)
  H-->>D: {status, version}
  D-->>D: report pass | warn(fixApplied) | fail

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Move stale-config repair into lifecycle.ensureDaemon

• ➕ Keeps runDoctor simpler; a single call covers validate-or-repair semantics
• ➕ Centralizes decision logic so other callers can benefit consistently
• ➖ Risk of broadening ensureDaemon semantics beyond “non-destructive validation”
• ➖ Harder to guarantee doctor-only behavior (single retry, exact reporting) without new flags/options

2. Introduce a small helper in doctor for lifecycle attempts

• ➕ Improves readability/testability (encapsulate: options + attempt + conditional retry)
• ➕ Makes it harder to accidentally diverge options between ensure/restart calls
• ➖ Extra indirection for a currently small conditional flow
• ➖ May be premature unless more refusal-specific handling is expected

Recommendation: Current approach is appropriate because it keeps ensureDaemon fail-closed semantics intact and limits the repair to an exact, authenticated stale-config case within lcm doctor. If additional refusal-specific recovery paths are expected in the future, consider extracting a small helper to keep runDoctor from accumulating conditional complexity.

Files changed (4) +148 / -5

Bug fix (1) +30 / -5

doctor.tsRetry daemon restart only on exact stale-config refusal from ensureDaemon +30/-5

Retry daemon restart only on exact stale-config refusal from ensureDaemon

• Refactors lifecycle invocation to reuse identical options for 'ensureDaemon'/'restartDaemon', and adds a single retry path when a matching-version daemon returns 'refusalReason: "stale-config"' without connecting. Reports successful repair as a warning with 'fixApplied: true', while keeping non-stale refusals and restart refusal remediation unchanged.

src/doctor/doctor.ts

Tests (1) +94 / -0

doctor.test.tsAdd regression tests for stale-config repair and reporting +94/-0

Add regression tests for stale-config repair and reporting

• Adds test coverage ensuring doctor restarts only for 'stale-config', uses identical lifecycle options, re-checks health, and reports 'warn' with 'fixApplied: true' (including optional warnings). Adds controls confirming non-stale refusals do not trigger restarts and that stale-config restart refusals keep the exact remediation.

test/doctor/doctor.test.ts

Documentation (1) +19 / -0

daemon-restart-recovery.mdDocument doctor recovery flow for stale daemon configuration +19/-0

Document doctor recovery flow for stale daemon configuration

• Documents that 'lcm doctor' performs normal validation first, then runs a single authenticated restart only for an exact 'stale-config' refusal, followed by a health re-check. Clarifies reporting as a warning with 'fixApplied: true' and that other refusals remain fail-closed with their original remediation guidance.

docs/daemon-restart-recovery.md

Other (1) +5 / -0

steady-doctors-repair.mdAdd patch changeset for stale-config repair behavior +5/-0

Add patch changeset for stale-config repair behavior

• Adds a patch changeset describing 'lcm doctor' repairing an authenticated managed daemon when its service configuration is 'stale-config', while preserving fail-closed refusals and reporting the repaired restart.

.changeset/steady-doctors-repair.md

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1e6ac0617c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

This PR updates lcm doctor so that when ensureDaemon() returns an exact authenticated stale-config refusal (and the running daemon version matches the installed LCM version), doctor performs a single authenticated restartDaemon() attempt using identical lifecycle options, then reports the outcome as a warning with fixApplied: true on success. It also documents the recovery behavior and adds a patch Changeset.

Changes:

• Add stale-config-specific doctor recovery: retry with restartDaemon() only for matching-version stale-config refusals from ensureDaemon().
• Add regression-first doctor tests covering successful repair, non-stale refusals, and failed stale-config repair with exact remediation preserved.
• Document the recovery behavior and include a patch changeset.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/doctor/doctor.ts	Adds targeted stale-config recovery logic that reuses the exact validated lifecycle options and reports repaired restarts as warn with fixApplied: true.
test/doctor/doctor.test.ts	Adds TDD regressions asserting the restart delegation is narrow (stale-config only), option-identical, and preserves exact remediation on failure.
docs/daemon-restart-recovery.md	Documents the new doctor-driven stale-config recovery behavior and its reporting semantics.
.changeset/steady-doctors-repair.md	Patch changeset for the user-facing doctor recovery behavior change.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Stale repair skips auth-check ✓ Resolved 🐞 Bug ≡ Correctness

Description

runDoctor() reports “stale configuration repaired” with fixApplied:true and sets daemonHealthy=true
as soon as restartDaemon() returns connected, even if the subsequent token-authenticated /health
probe fails or cannot run (e.g., missing token). This can clear the remediation marker and let
downstream checks proceed under an unverified daemon state.

Code

src/doctor/doctor.ts[R1073-1076]

+        results.push({
+          name: "daemon", category: "Daemon", status: "warn",
+          message: `localhost:${config.port} — stale configuration repaired; daemon restarted${warning}`,
+          fixApplied: true,

Relevance

●●● Strong

Team often accepts doctor fail-closed hardening; marking healthy/fixed without auth post-check
likely flagged as correctness bug.

PR-#355
PR-#25

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code computes postRestartOk from a token-authenticated health probe, but the stale-config
“success” branch does not use it and marks the daemon healthy/fixed solely based on
lifecycleResult.connected. This is unsafe because lifecycle’s health polling is unauthenticated
(public /health), and the authenticated health helper explicitly returns null when token/auth fails;
the newly added docs also state authenticated health is checked again before reporting.

src/doctor/doctor.ts[1025-1037]
src/doctor/doctor.ts[1070-1080]
src/doctor/doctor.ts[189-240]
src/daemon/lifecycle.ts[1383-1393]
src/daemon/lifecycle.ts[3195-3215]
docs/daemon-restart-recovery.md[102-119]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`runDoctor()` computes a post-restart token-authenticated health result (`postRestartOk`) but the new stale-config recovery success branch ignores it. The code currently treats `restartDaemon(...).connected === true` as sufficient to:
- clear the daemon remediation marker,
- report a successful repair (`fixApplied: true`), and
- set `daemonHealthy = true`.

This is incorrect because lifecycle connectivity is based on public health polling; it does not prove authenticated health/access is working after the repair.

### Issue Context
- `restartDaemon()` polls public `/health` without a token.
- `readDoctorAuthenticatedHealth()` can return `null` when the token is missing or the authenticated health check fails.
- The docs added in this PR state that doctor re-checks authenticated daemon health before reporting the repair result.

### Fix approach
- Require authenticated post-restart health success before reporting a successful stale-config repair.
 - e.g., change the success condition to `repairedStaleConfiguration && lifecycleResult.connected && postRestartOk`.
- If `repairedStaleConfiguration && lifecycleResult.connected` but `postRestartOk === false`, report a failure (or at least a warning with `fixApplied:false`) indicating that the restart occurred but authenticated verification failed.
 - Do **not** call `clearRemediationMarker()` in this case.
 - Set `daemonHealthy` conservatively (likely `false`) to avoid downstream checks assuming a verified daemon.

### Fix Focus Areas
- src/doctor/doctor.ts[1003-1117]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Missing config notes in doc ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

The new Doctor recovery for stale daemon configuration documentation explains purpose/behavior but
does not list relevant configuration options or explicitly state that there are none. This violates
the requirement that feature docs under docs/ cover purpose, usage, and configuration for end
users.

Code

docs/daemon-restart-recovery.md[R102-105]

+### Doctor recovery for stale daemon configuration
+
+When the daemon is healthy and its version matches the installed LCM version,
+`lcm doctor` first performs the normal non-destructive daemon validation. If

Relevance

●●● Strong

Docs config/no-config coverage is routinely enforced; same file had accepted “state if
configurable/none” request.

PR-#666
PR-#554
PR-#592

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 2446384 requires that changed feature docs include purpose, usage, and
configuration options (or explicitly state there are none). The added section describes behavior and
usage of lcm doctor for stale-config, but contains no explicit configuration-options section or
explicit 'no configuration' statement.

Rule 2446384: Feature docs in docs/ must cover purpose, usage, and configuration for end users
docs/daemon-restart-recovery.md[102-120]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The updated feature documentation for stale-config recovery does not include a configuration section (or an explicit statement that there are no new configuration options), which is required for end-user feature docs.

## Issue Context
A new section was added describing how `lcm doctor` repairs `stale-config`. The compliance rule requires each changed feature-related doc to include: purpose, usage, and configuration options (or explicitly state none).

## Fix Focus Areas
- docs/daemon-restart-recovery.md[102-120]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context

✅ Compliance rules (platform): 140 rules

Review mode: ⚖️ Balanced: This changes authenticated daemon lifecycle and fail-closed recovery behavior, so it carries real operational risk; the logic is localized enough for a careful single-pass review rather than extended redundancy.

Tip of the day

💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/doctor/doctor.ts:1022

• The second restartDaemon() call can throw (e.g. when the PID file points at an unverified/non-stoppable process). Because this happens inside the broad try { … } catch { … }, a thrown restart during stale-config repair is currently converted into a warn (“daemon validation failed”) rather than a fail-closed daemon result with stale-config remediation. Consider catching restart throws locally and converting them into a disconnected RestartDaemonResult so doctor reports a failure with the correct guidance.

        && !lifecycleResult.connected
        && lifecycleResult.refusalReason === "stale-config";
      if (repairedStaleConfiguration) {
        lifecycleResult = await restartDaemon(lifecycleOptions);
      }