Skip to content

fix(service): scope Windows scheduler probes to the root task and re-probe deleted registrations - #6401

Merged
lidge-jun merged 4 commits into
devfrom
devin/1790860555-win-scheduler-probe-scope
Oct 2, 2026
Merged

lidge-jun merged 4 commits into
devfrom
devin/1790860555-win-scheduler-probe-scope

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Scope Scheduler CSV and PowerShell state probes to the root \opencodex-proxy task. A same-named task in another folder must not supply presence or state for root-targeted stop operations.
  • Read registration XML only for a running manager. When state/XML becomes unreadable, re-probe presence once: proven deletion allows the existing stray-wrapper checks; present or unknown registrations continue to refuse.
  • Native Windows probes reproduced the folder-name mismatch before the fix. Unknown registrations and surviving/unreadable wrappers remain fail-closed.

Verification

  • Refreshed from dev at 06cc3815c06a10120d89ef986c46be95be37f28a without rewriting branch history, including the accepted upstream role-route and owner-registry ACL fixture corrections. Targeted verification: role routes 7 passed; owner-registry ACL 6 passed. Required CI passed at 3dbd9d8d6ef231729a34113754fbf626008b93b1: zero failed and zero pending checks (platform/path-inapplicable checks remain skipped).

  • Focused Windows tests: 65/65 passed; five regressions failed against the original behavior.

  • bun run typecheck, bun run structure:check: passed.

  • Full test:changed exceeded the local Windows 900-second cap; CI remains the full-suite gate.

  • Real registered-Scheduler CLI lifecycle passed on combined d50be3427 with this PR, fix(cli): stabilize guarded-stop re-verification and report the failing guard fact #6402, and fix(service): bound every Windows manager command the guarded stop depends on #6403. On a separate Windows VM with both managers initially absent: installed the root task in an isolated home, verified the wscript → cmd → Bun chain and supported approval, then guarded-stop returned runtimeDown: true in 49.4s. The task became Ready, the port closed, and all three approved processes exited. Repair/start succeeded; uninstall plus explicit ocx stop restored the absent-manager baseline.

  • WinSW account/password-dependent lifecycle remains untested. This CLI test does not claim native Tauri registered-service takeover coverage.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

Link to Devin session: https://app.devin.ai/sessions/a8795b27d4114aa6be37a88565e404b2
Open in Devin Desktop: https://app.devin.ai/desktop/session/a8795b27d4114aa6be37a88565e404b2?variant=devin

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • Required local validation passed; commands, results, and any full-suite exception are documented.

  • I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

…probe deleted registrations

Three probe-scoping defects in the guarded Windows manager proof turned a
provable absent verdict into a permanent unknown (approval-changed) or a
permanent failed stop:

- windowsSchedulerCsvIncludesTask treated a backslash as a name boundary, so a
  same-named task inside a task folder (\Tools\opencodex-proxy) read as present
  while every /tn operation resolves a bare name in the root folder only — the
  bound read/stop then failed on a task nothing could reach.
- windowsScheduledTaskState ran Get-ScheduledTask -TaskName without -TaskPath,
  a folder-agnostic filter that returns foreign task state and arrays.
- inspectWindowsSchedulerManager read the registration XML before the state
  check, so an unreadable registration blocked the absent verdict an inert
  task does not need; and a task deleted between probes was indistinguishable
  from an unreadable one. The XML now loads only on the running path, and the
  unanswerable state/XML reads re-probe presence: proven-gone gets the
  stray-wrapper inert verdict, persisting or unanswerable stays unknown.
  observeWindowsGuardedManagerStopped re-probes the same way before failing
  closed, and proven absence still owes the unreadable former-manager check.

Verified live on Windows Server 2022: \DevinProbe\opencodex-proxy exists while
schtasks /query /tn, /end and /xml all fail root-scoped, the CSV listing emits
"\DevinProbe\opencodex-proxy", and Get-ScheduledTask -TaskName returns the
foldered task's state plus arrays for duplicate names. Regression tests pin
each failure and the preserved fail-closed cases.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: ba00cf03-ba33-4723-b87c-597ddfdd512e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/0 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/0).

Review readiness checklist

  • ⬜ Required local validation passed; commands, results, and any full-suite exception are documented.
  • ⬜ I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/0 boxes ticked.

Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked.

@github-actions
github-actions Bot marked this pull request as draft October 1, 2026 15:03
@lidge-jun
lidge-jun marked this pull request as ready for review October 2, 2026 00:09
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer integration record (MAINTAINERS.md, dev-only)

@lidge-jun
lidge-jun merged commit cfde167 into dev Oct 2, 2026
39 of 40 checks passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-02T00:13:29.647264Z 914c052 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@lidge-jun
lidge-jun deleted the devin/1790860555-win-scheduler-probe-scope branch October 2, 2026 00:09

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 914c05241e

ℹ️ 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".

Comment on lines +423 to +424
return io.winTaskProbe().status === "absent"
? inertVerdict()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Refresh the process snapshot after proving task deletion

When the task starts after the winProcs() snapshot, reports running, and is then deleted before the XML read, this branch returns inertVerdict() using the stale strays array. A deleted registration can still leave its wrapper alive, so the guarded stop can classify the manager as absent, signal only the approved child, and let the surviving wrapper respawn it. Re-read the process list after the absence re-probe before returning absent; the state-unknown re-probe and post-stop paths have the same stale-snapshot issue.

Useful? React with 👍 / 👎.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant