fix(probe): walk Windows service definition chains (Task Scheduler + WinSW) for ownership - #1154
Conversation
The Windows service-manager probe hardcoded `unknown` ("the Windows
definition chain is not inspected yet"), so every unattended Codex
write — the dashboard "Sync now" path included — refused on Windows.
Implement the chain walk: task XML -> VBS launcher -> batch wrapper,
extracting CODEX_HOME/OPENCODEX_HOME from the wrapper's `set` lines,
and one bounded `schtasks /query /xml` call for registration. Decode
the UTF-16LE on-disk assets (task XML and VBS) so the on-disk and
registered forms parse the same. Any broken link stays `unknown`
(fail closed); absence is only claimed when nothing is staged.
Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughWindows service-manager probing now inspects scheduled-task and WinSW definitions, follows launcher assets, extracts configured homes, decodes supported formats, classifies registration failures, and detects backend mismatches during ownership checks. ChangesWindows service-manager inspection
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ServiceManagerProbe
participant schtasks
participant TaskXML
participant LauncherVBS
participant CommandWrapper
participant WinSW
participant OwnershipPreflight
ServiceManagerProbe->>schtasks: query scheduled-task registration
schtasks-->>ServiceManagerProbe: registration status
ServiceManagerProbe->>TaskXML: read and decode task definition
TaskXML-->>ServiceManagerProbe: launcher path
ServiceManagerProbe->>LauncherVBS: read and decode launcher
LauncherVBS-->>ServiceManagerProbe: wrapper path
ServiceManagerProbe->>CommandWrapper: validate wrapper and extract homes
CommandWrapper-->>ServiceManagerProbe: expanded home claims
ServiceManagerProbe->>WinSW: inspect service definition and status
WinSW-->>ServiceManagerProbe: WinSW claim
ServiceManagerProbe->>OwnershipPreflight: compare detected and recorded backends
OwnershipPreflight-->>ServiceManagerProbe: ownership status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/service-manager-probe.ts`:
- Around line 369-372: Update windowsTaskRegistered to return "absent" only when
the schtasks result definitively indicates the task is not found; classify
access-denied, execution-error, signal-terminated, null-status, and all other
nonzero statuses as "unknown", while preserving "present" for status 0. Add a
regression test covering a nonzero access-denied response that expects
"unknown".
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 70fe5a5b-e1e3-4105-9be5-7c32c6a8ef22
📒 Files selected for processing (2)
src/service-manager-probe.tstests/codex-service-manager-probe.test.ts
…sk probe schtasks exits 1 for both "task not found" and "access denied"; only the stderr message distinguishes them. Keying absence on the exit code alone let a locked-down or wedged Task Scheduler read as a clean machine, so `windowsTaskRegistered` now returns `absent` only when the stderr states the task cannot be found, `present` only on exit 0, and `unknown` for every other nonzero status, access denied, execution errors, signal termination, null status, and spawn/timeout failures. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 153f8aab60
ℹ️ 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".
| if (task === "absent") { | ||
| return registered === "present" | ||
| ? unknown("Task Scheduler holds opencodex-proxy but its task XML is missing") | ||
| : { kind: "absent" }; |
There was a problem hiding this comment.
When the opt-in native backend is installed but its service-state mirror is missing or stale, this branch queries only Task Scheduler and returns absent. WinSW is a separate supported SCM registration (opencodex-proxy-native) whose ownership homes live in its XML (src/lib/winsw.ts:32-45, src/lib/winsw.ts:230-255); inspectNativeCodexOwnership then interprets no state plus this absent result as owned (src/integrations/native/ownership-preflight.ts:161-164), allowing unattended convergence to alter files still used by a foreign WinSW service. Query the SCM via the existing tri-state probe, parse the WinSW XML, and return conflict when both backends are present.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
|
|
||
| /** Registration state of the scheduled task. `unknown` when the query fails. */ | ||
| function windowsTaskRegistered(deps: Required<Pick<ProbeDeps, "run">>): "present" | "absent" | "unknown" { | ||
| const queried = deps.run("schtasks", ["/query", "/tn", windowsTaskName(), "/xml"]); |
There was a problem hiding this comment.
When OpenCodex starts from an attacker-controlled project directory or search path on Windows, spawning the bare schtasks name permits executable search to select a planted binary instead of the system utility. This repository already provides and uses resolveTrustedWindowsSchtasksExe() specifically to obtain a verified absolute System32 path (src/lib/windows-elevation.ts:198-205, src/service.ts:735-748); use that resolver here as well so this startup/ownership probe cannot execute project-controlled code.
Useful? React with 👍 / 👎.
| function windowsTaskRegistered(deps: Required<Pick<ProbeDeps, "run">>): "present" | "absent" | "unknown" { | ||
| const queried = deps.run("schtasks", ["/query", "/tn", windowsTaskName(), "/xml"]); | ||
| if (queried.spawnFailed || queried.timedOut) return "unknown"; | ||
| return queried.status === 0 ? "present" : "absent"; |
There was a problem hiding this comment.
When schtasks exits nonzero because of access denial, an unavailable RPC/service, or another query failure, this maps the result to proven absence. The existing scheduler wrapper explicitly says to “never treat a failed query as proven absence” and falls back to a CSV listing before returning unknown (src/service.ts:764-768, src/service.ts:856-881); its tests cover access-denied and RPC failures (tests/windows-scheduler-install-verification.test.ts:123-134). With no staged XML, the new path instead returns kind: "absent", and ownership can become owned, so reuse that tri-state fallback or accept absence only after it is independently proven.
AGENTS.md reference: src/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
| const queried = deps.run("schtasks", ["/query", "/tn", windowsTaskName(), "/xml"]); | ||
| if (queried.spawnFailed || queried.timedOut) return "unknown"; | ||
| return queried.status === 0 ? "present" : "absent"; |
There was a problem hiding this comment.
When the registered task still points at an old or foreign launcher while a reinstall has staged a new on-disk XML, /query /xml returns the authoritative registered action in stdout, but this helper discards it and later walks only the staging copy. The resulting claim can pair the new homes with registration: "present" even though Task Scheduler will launch the old wrapper, causing the ownership preflight to accept the wrong definition. Preserve and correctly decode the queried XML, walk both definitions, and return unknown when the registered and staged chains disagree.
Useful? React with 👍 / 👎.
| // The wrapper assets live under OPENCODEX_HOME. Defaulting to `~/.opencodex` | ||
| // mirrors service.ts defaultOpenCodexHome(); the caller can override `home` | ||
| // in tests. | ||
| return join(home, ".opencodex"); |
There was a problem hiding this comment.
When OPENCODEX_HOME is customized, service.ts writes the scheduler XML, launcher, and wrapper under getConfigDir() (src/service.ts:70-80, src/service.ts:1797-1805), while this probe always looks under <home>/.opencodex. A registered custom-home service is therefore reported as having missing XML, and an interrupted install staged only in the custom directory can be reported absent, defeating ownership detection for a supported configuration. Pass the effective/injected OpenCodex config directory into the probe and retain the default-home mirror only as an additional candidate.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| /** Pull one `set "NAME=value"` out of a batch wrapper. */ | ||
| function batchSetValue(body: string, name: string): string | null { | ||
| const match = new RegExp(`^\\s*set\\s+"${name}=([^"]*)"\\s*$`, "im").exec(body); | ||
| return match ? match[1]!.trim() : null; |
There was a problem hiding this comment.
When an explicit CODEX_HOME or OPENCODEX_HOME lies under USERPROFILE, APPDATA, or LOCALAPPDATA, the real builder intentionally emits values such as %USERPROFILE%\.codex (src/lib/win-paths.ts:44-55), but this parser returns that batch syntax verbatim. claimNamesDifferentHome then compares it with the resolved absolute current home and reports a false disagreement; literal percent signs are similarly returned doubled. Decode the known generated indirection and batch escaping before returning the home values, and cover the probe with a wrapper produced by buildWindowsServiceScript rather than only raw-path fixtures.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| return { | ||
| kind: "present", | ||
| claims: [{ | ||
| backend: "scheduler", |
There was a problem hiding this comment.
When a v2 service-state file records backend: "native" but the new Windows probe finds a scheduler definition whose homes agree, this claim is passed to inspectNativeCodexOwnership, which compares only the homes and returns owned. That state occurs during an interrupted or failed backend switch and does not prove which manager owns the installation; the repository's completed ownership design explicitly requires unknown when the manager backend disagrees with state.backend (devlog/_fin/260804_codex_write_substrate/041_wp12_closeout.md:763-769). Correlate each Windows manager claim with the recorded backend before admitting the write.
Useful? React with 👍 / 👎.
| codexHome: batchSetValue(wrapperBody, "CODEX_HOME"), | ||
| opencodexHome: batchSetValue(wrapperBody, "OPENCODEX_HOME"), |
There was a problem hiding this comment.
When the wrapper is empty, truncated, or replaced with an unrelated readable file, both calls here return null, which is indistinguishable from a legitimate install that deliberately omitted both variables. If a valid service-state file exists, inspectNativeCodexOwnership finds no disagreeing home and returns owned, so an interrupted non-atomic wrapper rewrite can authorize unattended writes despite the definition being malformed. Validate the generated wrapper's required structure and execution line before interpreting absent optional set lines; malformed content must use the existing unknown failure representation.
AGENTS.md reference: src/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
Three findings from the Codex connector review: - P1: a malformed wrapper (empty, truncated, or an unrelated readable file) previously read as a legitimate install that omitted both homes. The probe now validates the generated wrapper structure (`:loop` + `%OCX_BUN%`/ `%OCX_CLI%` launch tail) and returns `unknown` for anything that does not look generated. - P2: home values baked by `buildWindowsServiceScript` use `%USERPROFILE%`- style env tokens and double literal `%` as `%%`; the probe now decodes that indirection against the live environment before comparing, so an install under the user profile no longer reports a false home disagreement. - P2: a v2 service-state file recording backend `native` (WinSW) beside a scheduler task claim is an interrupted backend switch; ownership now returns `unknown` instead of `owned` when the manager backend disagrees with the recorded state. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/service-manager-probe.ts`:
- Around line 404-405: Update wrapperLooksGenerated to validate an actual
line-anchored %OCX_BUN% or %OCX_CLI% invocation using the start command after
the :loop label, rather than accepting any bun substring. Add a regression test
covering valid set lines followed by :loop and rem bun, asserting the probe
result remains unknown.
- Around line 379-395: Update decodeBatchPathValue to protect escaped percent
sequences with a sentinel before expanding environment tokens, then restore the
literal percent signs afterward so %%USERPROFILE%% remains %USERPROFILE%.
Normalize lookup names to uppercase for case-insensitive matching and use
resolved === undefined so defined empty variables expand correctly. Add
regression coverage using a controlled USERPROFILE value for both escaped and
lowercase token forms.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d5f3ad16-6c91-48d4-bc25-3fc70a7620c0
📒 Files selected for processing (3)
src/integrations/native/ownership-preflight.tssrc/service-manager-probe.tstests/codex-service-manager-probe.test.ts
CodeRabbit: - decodeBatchPathValue: protect `%%` with a sentinel before expanding env tokens (%%USERPROFILE%% stays literal), normalize token names to uppercase for case-insensitive lookup, and use `resolved === undefined` so defined empty vars expand. - wrapperLooksGenerated: require a line-anchored `"%OCX_BUN%" "%OCX_CLI%" start` invocation after `:loop` instead of any `bun` substring, so a `:loop` + `rem bun` shell is rejected as malformed. Codex connector: - Resolve schtasks through resolveTrustedWindowsSchtasksExe() so a planted binary on PATH cannot be executed. - Walk the REGISTERED task XML (from /query /xml stdout) in addition to the staged on-disk definition; return unknown when the registered and staged chains disagree (interrupted reinstall). - Locate scheduler assets under the effective OPENCODEX_HOME (new configDir probe dep) instead of always the default-home mirror. - Include the WinSW native backend: probe the SCM registration + parse the WinSW XML homes, and report a conflict when both the scheduler task and the native service are present. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
|
✅ Deterministic PR hygiene checks passed. |
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/service-manager-probe.ts`:
- Around line 464-467: Update walkWinswChain and its caller to route the WinSW
status check through the injected ProbeDeps.run, or introduce and use an
injectable winswStatus dependency instead of calling statusWinswRaw directly.
Ensure every Windows probe uses the bounded, observable dependency path, then
update the bounded-query test assertion to expect and verify both the schtasks
and WinSW queries.
- Around line 620-623: Update src/service-manager-probe.ts lines 620-623 to
validate the WinSW XML with the same generated-artifact check used by
wrapperLooksGenerated, returning unknown when validation fails; broaden envValue
to support single-quoted values and either attribute order. At lines 484-486,
call walkWindowsChain(deps, xml, taskXmlPath) before constructing the conflict
result and reuse its scheduler claim instead of hardcoded null homes.
- Around line 624-635: Update inspectWindows to treat a WinSW claim as installed
only when its registration is "present", rather than relying solely on
winsw.kind === "present". Apply this registration check to both the conflict
detection and the early return that selects winsw, so claims with registration
"absent" do not block ownership or count as an installed backend.
- Around line 442-446: Wrap the resolveTrustedWindowsSchtasksExe() call in the
inspectWindows probe with exception handling, returning the existing fail-closed
{ registered: "unknown", registeredXml: "" } result when resolution throws. Keep
the deps.run query and its existing spawnFailed/timedOut handling unchanged.
- Around line 484-486: Update the early conflict branch in the service-manager
probe to call walkWindowsChain before returning, then reuse its resulting
scheduler claim in the conflict result instead of constructing homes with null
values. Preserve the existing conflict kind and WinSW claim while ensuring the
staged definition’s actual home fields are retained.
- Around line 505-516: Update the registered-task validation around
walkWindowsChain so any result other than kind "present" returns unknown instead
of skipping the comparison; retain the existing homesDisagree check for
successfully walked chains. Add a regression test in the service-manager probe
tests using a registered task whose launcher path does not exist, asserting the
probe returns unknown.
- Around line 509-511: Normalize both registered and staged home paths before
the comparisons in the homesDisagree calculation: lowercase them, convert
forward slashes to backslashes, and remove trailing separators. Apply the same
normalization to codexHome and opencodexHome while preserving null handling and
the existing disagreement behavior.
- Around line 447-451: Update ProbeRunner/defaultProbeRunner to preserve
schtasks stdout and stderr as raw buffers instead of decoding them as UTF-8,
with an explicit UTF-16LE contract for Windows task output. In the
queried.status === 0 path, decode the raw schtasks /xml output using utf16le
before the existing registeredXml fallback and return behavior; do not pass
UTF-8-reconstructed strings through decodeWindowsText.
- Around line 487-496: Update the Windows probe around the winsw status handling
to return { kind: "unknown", reason: "cannot confirm native WinSW status" }
whenever winsw.kind is "unknown" and task is not "absent", before the scheduler
claim comparison can return present. Preserve the existing absent-task handling
and document the intentional tradeoff near the surrounding probe logic if
appropriate.
In `@tests/codex-service-manager-probe.test.ts`:
- Around line 465-492: Add a distinct decoy service chain under the default
<home>/.opencodex location in the scheduler asset test, including a different
CODEX_HOME value. Keep the existing custom chain and assert that
inspectServiceManagerInstallation with configDir: custom reports the custom
chain’s codexHome, proving customized OPENCODEX_HOME takes precedence over the
default mirror.
- Around line 356-359: Update the executable-path assertion in the schtasks
query test to verify the resolved path is absolute and located under the trusted
System32 directory, rather than merely containing “schtasks.” Reuse the trusted
system-directory expectation established by resolveTrustedWindowsSchtasksExe,
while preserving the existing call count and argument assertions.
- Around line 461-463: Update the test around inspectServiceManagerInstallation
to assert result.reason equals the specific reason returned by
wrapperLooksGenerated at service-manager-probe.ts, in addition to result.kind
being "unknown", so the test verifies the intended failure path rather than
another unknown outcome.
- Around line 488-492: Add focused tests in the service-manager probe test suite
covering the WinSW backend: assert kind "conflict" when both Task Scheduler and
WinSW are present, verify WinSW home paths parsed from <env .../> entries, and
assert kind "unknown" when statusWinswRaw() cannot verify SCM state. Reuse the
existing probe fixtures and helpers, and keep the current Task Scheduler
coverage unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2bdfdd34-b371-46e9-8a89-b67795128c94
📒 Files selected for processing (2)
src/service-manager-probe.tstests/codex-service-manager-probe.test.ts
…hains - Route the WinSW status check through an injectable winswStatus dep (defaults to statusWinswRaw) so every Windows probe uses the bounded, observable dependency path. - Validate the WinSW XML with a generated-artifact check (winswXmlLooksGenerated) and broaden envValue to single/double quotes and either attribute order. - Treat a WinSW claim as installed only when registration is "present", in both conflict detection and the early return. - Wrap resolveTrustedWindowsSchtasksExe() in exception handling; a resolution failure fails closed to unknown. - The conflict branch walks the staged scheduler definition first and reuses its real claim instead of null homes. - A registered-task walk that yields anything other than "present" now returns unknown (not silently skipped), with a regression test for a registered task whose launcher is missing. - Normalize registered vs staged homes (case, slash, trailing separator) before the disagreement comparison. - Preserve schtasks stdout/stderr as raw buffers (UTF-16LE contract) via a new raw probe runner; decode the registered XML from the raw bytes. - Return unknown when WinSW state is unverifiable while a scheduler task is also present. Tests: decoy default-home chain in the OPENCODEX_HOME test, absolute System32 schtasks path assertion, exact wrapper reason assertion, and new WinSW tests (conflict, homes parsed, unverifiable SCM). Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/service-manager-probe.ts (1)
690-697: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not use an unrelated
<env>element when the requested home is omitted.If a WinSW XML contains only
CODEX_HOME,envValue("OPENCODEX_HOME")fails at Line 693 and Line 694 selects the first<env>element. The function then reports theCODEX_HOMEvalue for both homes instead of reportingopencodexHome: null.Remove the fallback. The first expression already accepts either attribute order and both quote styles. Add a fixture with only one home variable and assert that the other home remains
null.Proposed fix
- const tag = new RegExp(`<env\\b[^>]*\\bname=["']${name}["'][^>]*>`, "i").exec(body) - ?? new RegExp(`<env\\b[^>]*>[^<]*`, "i").exec(body); + const tag = new RegExp(`<env\\b[^>]*\\bname=["']${name}["'][^>]*>`, "i").exec(body);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/service-manager-probe.ts` around lines 690 - 697, Remove the fallback RegExp in envValue so a missing requested name returns null instead of matching an unrelated env element; retain the existing name-matching expression for both attribute orders and quote styles, and add a fixture containing only one home variable that verifies the other home remains null.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/service-manager-probe.ts`:
- Around line 524-535: Update the winswInstalled && schedulerPresent branch to
return staged unchanged whenever walkWindowsChain returns a non-present result,
including unknown. For a present staged claim, build the conflict using that
claim with registration set to "present" rather than fabricating null homes;
preserve the WinSW claim. Add coverage for a broken scheduler chain and verify
registration on a valid conflict.
---
Duplicate comments:
In `@src/service-manager-probe.ts`:
- Around line 690-697: Remove the fallback RegExp in envValue so a missing
requested name returns null instead of matching an unrelated env element; retain
the existing name-matching expression for both attribute orders and quote
styles, and add a fixture containing only one home variable that verifies the
other home remains null.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ffeeca59-b7f8-4d83-b0c4-e4975c40bcb7
📒 Files selected for processing (2)
src/service-manager-probe.tstests/codex-service-manager-probe.test.ts
The WinSW XML without its paired exe cannot prove a verifiable SCM claim; add a regression test asserting the probe returns unknown rather than an owned WinSW backend. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
- When WinSW and a scheduler task are both present, a broken/malformed staged scheduler chain now returns unknown instead of fabricating a null-homes claim; a valid staged claim is a conflict with registration present (no invented placeholder homes). - WinSW absence now requires both the XML and the service exe to be missing, so a missing binary is unknown, not "absent". - The <env> value reader drops its loose fallback regex: an unmatched name stays null rather than grabbing a wrong element's value. - Tests: conflict asserts real (non-fabricated) scheduler claim homes + registration; new case covers a broken scheduler chain alongside WinSW failing closed as unknown. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
…ilable Windows without Developer Mode or elevated privileges cannot create symlinks (EPERM), so the dangling-plist test always failed there even though the code under test is fine. Probe symlink capability once and gate the test with test.skipIf, mirroring claude-agents-inject; the machine without symlinks now reports a visible skip instead of a spurious failure. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
The Windows chain-walk suites pass `platform: "win32"` as a parameter, but the trusted schtasks resolver keys off process.platform, which is "linux" on the ubuntu-latest test shards. It threw on every Windows test, so the probe returned "Task Scheduler could not be asked" and 18 tests failed. Set the trusted-system-directory resolver seam in beforeEach, pointing at a per-test fake System32 containing schtasks.exe (mirroring windows-elevation.test.ts), and reset it in afterEach. Reproduced the CI failure locally under a linux-platform simulation: 18 failed without the seam, 49 pass / 0 fail with it. Co-authored-by: CommandCodeBot <noreply@commandcode.ai>
Wibias
changed the title
1 participant
Summary
Fixes the Windows service-manager ownership probe, which previously hardcoded
unknown("the Windows definition chain is not inspected yet"). Because admission fails closed onunknown, every unattended Codex write on Windows — including the dashboard "Sync now" action — was refused with:The probe now walks the Windows service definition chains (Task Scheduler and WinSW) and reports real homes, so a genuine install resolves to
ownedand sync proceeds.What changed
src/service-manager-probe.tsnow walks the definition chains instead of refusing:opencodex-service-task.xml) → extract the launcher path from<Arguments>; VBS launcher → extract the batch wrapper path; batch wrapper → extractCODEX_HOME/OPENCODEX_HOMEfromset "NAME=..."lines (including%VAR%env-token decoding).unknown, notabsent.schtasks /query /tn opencodex-proxy /xmlcall.presentis exit 0;absentonly a definitive "task not found" message; everything else (access denied, timeout, spawn failure, null status) isunknown.unknown— never fabricated into a claim.Encoding
On-disk task XML and VBS launchers are UTF-16LE (often BOM-prefixed); they're now decoded before parsing so the on-disk and
/queryforms parse identically.Fail-closed behavior preserved
unknownschtasks/ WinSW that cannot be asked →unknownabsentis only claimed when nothing is staged and the service is confirmed not registeredsetline staysnull(not""), and the<env>value reader no longer falls back to a loose regex that could grab a wrong element's valueTesting
49 tests covering: full chain extraction, env-token decoding, UTF-16 decoding, unregistered-but-staged tasks, omitted homes, broken links, missing XML, unaskable
schtasks, WinSW exe-missing fail-closed, the WinSW+scheduler conflict, and ownership admission against every service-manager state. Two CI-specific test fixes:Verified on Windows against a real
ocx service install: the probe returnspresentwith the task registered, ownership resolves toowned, andocx sync --port 10100completes.