[SebTardif]

summarize daemon uninstall on Windows runs schtasks /Delete and ignores the exit code. If delete is denied, the command still deletes the launcher files and prints that the task was removed, while Task Scheduler keeps the task.

/Delete now fails uninstall on a real schtasks error, and the launcher files stay. A task that is already gone (The system cannot find the file specified, or a message that the task does not exist) still cleans those files, so running uninstall again does not stop early. /End failing because the task is not running is still ignored.

This is the absence case called out when #525 was closed. That pull treated every nonzero /Delete as failure, including a task that was already gone.

pnpm exec vitest run tests/daemon.schtasks.test.ts passed (13 tests). This was not run against Windows Task Scheduler. The absent and denied cases are the English schtasks strings.

[clawsweeper]

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

[clawsweeper]

Codex review: needs real behavior proof before merge.

What this changes

This PR preserves Windows daemon launcher files when Scheduled Task deletion fails while allowing cleanup when an English diagnostic indicates the task is absent.

Example: Run summarize daemon uninstall when deleting Summarize Daemon returns ERROR: Access is denied.

• Before: Uninstall removes launcher files and reports success while the scheduled task remains registered.
• After: Uninstall reports failure and preserves the launcher files.

Review scores

Measure	Result	What it means
Overall readiness	🧂 unranked krab (1/6)	The repair has clear user value and owner direction, but two actionable defects remain and supplied validation is limited to mocks.
Proof confidence	🧂 unranked krab (1/6)	Real behavior proof is necessary before merge. See Before merge.
Patch quality	🦪 silver shellfish (2/6)	2 actionable review findings remain.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: Windows daemon uninstall can claim success and remove its launchers while leaving the autostart task registered.
Reason: The failure is present in current source, and the owner explicitly requested a corrected Windows-only follow-up in #525 (comment).

Merge readiness

⛔ Blocked before merge - 5 items remain

Keep this PR open: it addresses a current Windows uninstall bug and follows recorded owner direction, but localized absent-task handling and actionable deletion errors need correction.

Priority: P2
Reviewed head: b6b2bca2998dbd92941a02f8d1d9bbade6eb54fd

Before merge

• Add real behavior proof - The supplied evidence exercises the Scheduled Task adapter with mocked subprocess results, and the contributor explicitly reports no native Task Scheduler run. Provide redacted terminal output or terminal screenshots from the shipped CLI on Windows showing denied deletion preserves launchers, successful deletion removes them, and repeated uninstall handles an absent task on English and non-English Windows; include the owner-requested upgrade run from v0.26.0 with settings intact. Redact API keys, IPs, and non-public endpoints. After adding proof, update the PR body, then request a new review with @clawsweeper re-review. The PR author or someone with repository write access can request it.
• Preserve absent-task cleanup on localized Windows (P2) - When the task is already absent on non-English Windows, schtasks returns a localized missing-file diagnostic, such as German Das System kann die angegebene Datei nicht finden. Neither English regex matches, so the new deletion guard throws and skips removal of daemon.cmd, daemon-launch.vbs, daemon-task.xml, and legacy artifacts. Main previously completed this cleanup. Distinguish confirmed absence without relying exclusively on English display text, and cover a localized absent task alongside access denial.
• Give deletion failures actionable uninstall guidance (P2) - For the access-denied scenario covered by the new test, this call renders only schtasks delete failed: ERROR: Access is denied. It does not tell the user that uninstall stopped with launcher files retained or to retry uninstall from an elevated PowerShell/cmd. Add localized uninstall-specific recovery guidance; enabling the current needsAdmin flag alone would incorrectly recommend daemon install.
• Add data-model compatibility proof - The review found that existing stored data may not work after upgrade. Show that existing data still loads and works with this change.
• Explain or restore the original intent of Generated launcher and legacy-artifact cleanup - 6e56049: fix(daemon): repair Windows Scheduled Task launcher and document elevation #192 preserves cleanup of generated launchers and obsolete PID artifacts across upgraded installations; English absent-task handling preserves this intent, while localized absence does not.

Findings

• [P2] Preserve absent-task cleanup on localized Windows — src/daemon/schtasks.ts:236-241
• [P2] Give deletion failures actionable uninstall guidance — src/daemon/schtasks.ts:366

Provenance

• Generated launcher and legacy-artifact cleanup changes intended behavior without addressing why it exists (6e56049: fix(daemon): repair Windows Scheduled Task launcher and document elevation #192 preserves cleanup of generated launchers and obsolete PID artifacts across upgraded installations; English absent-task handling preserves this intent, while localized absence does not.)

Tests

• Missing end-to-end proof: No shipped-CLI Windows run demonstrates denied deletion, successful deletion, localized absent-task cleanup, or an upgrade from v0.26.0 with configuration intact; no base-fail/head-pass regression run is recorded.

Agent review details

How this fits together

The daemon CLI dispatches Windows lifecycle operations to the Scheduled Task adapter, which runs Windows service commands and manages generated launcher files.

flowchart TD
  A[Daemon CLI] --> B[Windows service adapter]
  B --> C[Shared command runner]
  C --> D[Windows Task Scheduler]
  D --> E[Deletion result]
  E --> F[Preserve or remove launcher files]

Loading

Technical review

Best possible solution:

Keep deletion handling in the existing Windows adapter, distinguish absence independently of English diagnostics, and provide localized uninstall recovery guidance.

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

Current-main source directly shows that a nonzero /Delete result is ignored before launcher removal and CLI success; no target code or tests were executed.

Is this the best way to solve the issue?

Checking deletion before cleanup fixes the underlying ordering problem, but the absence distinction must cover localized Windows and the failure message must explain recovery.

Full review comments:

• [P2] Preserve absent-task cleanup on localized Windows — src/daemon/schtasks.ts:236-241
  When the task is already absent on non-English Windows, schtasks returns a localized missing-file diagnostic, such as German Das System kann die angegebene Datei nicht finden. Neither English regex matches, so the new deletion guard throws and skips removal of daemon.cmd, daemon-launch.vbs, daemon-task.xml, and legacy artifacts. Main previously completed this cleanup. Distinguish confirmed absence without relying exclusively on English display text, and cover a localized absent task alongside access denial.
  Confidence: 0.97
• [P2] Give deletion failures actionable uninstall guidance — src/daemon/schtasks.ts:366
  For the access-denied scenario covered by the new test, this call renders only schtasks delete failed: ERROR: Access is denied. It does not tell the user that uninstall stopped with launcher files retained or to retry uninstall from an elevated PowerShell/cmd. Add localized uninstall-specific recovery guidance; enabling the current needsAdmin flag alone would incorrectly recommend daemon install.
  Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.97

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Windows uninstall failure policy changes intended behavior with a stated reason (unknown: The provided provenance identifies formatting and lifecycle repairs but no rationale for ignoring deletion failures; refactor(runtime): share service commands and executable policy #428 preserved existing policy. Local creation-history inspection reached unavailable historical objects, so the original rationale remains unknown.)
• Typed localized service errors keeps the original intent (05d786f: feat(localization): render typed messages across CLI and extension #479 introduced typed service errors while preserving native diagnostics; this PR uses the existing error owner.)

Testing

Proof path: unit tests only.

Security

None.

Evidence

Acceptance criteria:

• pnpm exec vitest run tests/daemon.schtasks.test.ts tests/daemon.service-localization.test.ts tests/daemon.cli.test.ts
• pnpm check:localization
• pnpm run check

What I checked:

• Verified introduced change: The pinned diff checks the deletion result before launcher cleanup, adds an English absence matcher, and adds four mocked lifecycle scenarios. (src/daemon/schtasks.ts:365, b6b2bca2998d)
• Current release still ignores deletion failure: The current main revision, also tagged v0.26.0 in the supplied release context, ignores the /Delete result and proceeds to remove generated task files. (src/daemon/schtasks.ts:354, aaccb7c9c30e)
• Recorded owner direction: In fix: fail schtasks uninstall when delete fails and match Spotify hosts #525 (comment), steipete requested a corrected absence/error distinction and native Windows upgrade proof before the Windows change could land.
• Deletion recovery message: The new throw uses the generic service error without recovery guidance; its existing administrator hint specifically recommends daemon install and cannot correctly guide uninstall. (src/localization/en.json:251, b6b2bca2998d)
• Contributor evidence: The full PR body explicitly says the change was not run against Windows Task Scheduler and identifies the fixtures as English strings; current comments provide no additional behavior evidence. (tests/daemon.schtasks.test.ts:359, b6b2bca2998d)
• Windows lifecycle history: fix(daemon): repair Windows Scheduled Task launcher and document elevation #192 establishes detached-process termination and generated-file cleanup; refactor(runtime): share service commands and executable policy #428 deliberately preserved platform failure policy while sharing subprocess normalization. (src/daemon/schtasks.ts:264, aaccb7c9c30e)

Likely related people:

• Peter Steinberger: Raw commit 05d786f adds src/daemon/command.ts:26 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 05d786f37b07; files: src/daemon/command.ts)
• Aaron Meese: Raw commit 6e56049 adds src/daemon/schtasks.ts:161 relative to its recorded parents. This identifies author metadata, not feature responsibility or a PR merger. (role: source-line author; confidence: high; commits: 6e560498ad7e; files: src/daemon/schtasks.ts)

Preflight source

Source unknown. Source comparison at the observed PR snapshot only. Completion is self-reported; body, policy, tests, execution, and report bytes are not verified.

Labels

Label changes:

• add P2: The bug and proposed corrections affect Windows daemon removal with a limited platform and lifecycle scope.
• add merge-risk: 🚨 compatibility: The new English-only absence check makes previously successful leftover-file cleanup fail on localized Windows installations.
• add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
• add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Label justifications:

• P2: The bug and proposed corrections affect Windows daemon removal with a limited platform and lifecycle scope.
• merge-risk: 🚨 compatibility: The new English-only absence check makes previously successful leftover-file cleanup fail on localized Windows installations.
• rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦪 silver shellfish.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Rating scale

6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed November 6, 2026, 4:30 PM ET / 21:30 UTC.