Skip to content

fix(windows): scope scheduler ownership by OpenCodex home - #2857

Merged
lidge-jun merged 1 commit into
devfrom
codex/lane-f-multihome-scheduler
Aug 28, 2026
Merged

lidge-jun merged 1 commit into
devfrom
codex/lane-f-multihome-scheduler

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Aug 28, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Scope the machine-global opencodex-proxy Task Scheduler registration to the effective OPENCODEX_HOME before it participates in write ownership.
  • Treat a registered launcher outside the current config directory as another home's manager claim, allowing the current home to reach the existing Codex write coordinator.
  • Keep current-home missing XML, malformed launcher arguments, unreadable registration XML, WinSW overlap, and manager-query failures fail-closed.

Closes #2800

Verification

  • RED: bun test tests/codex-service-manager-probe-hardening.test.ts returned ownership: unknown with Task Scheduler holds opencodex-proxy but its task XML is missing for the second-home fixture.
  • Mutation RED: restoring the previous machine-global unknown decision failed only the new Windows: a second OpenCodex home can never pass admitCodexWrite once Task Scheduler holds opencodex-proxy #2800 ownership regression while the current-home negative remained green.
  • GREEN: bun test tests/codex-service-manager-probe-hardening.test.ts tests/codex-service-manager-probe.test.ts tests/codex-admission.test.ts — 83 pass, 0 fail, 201 assertions.
  • bun x tsc --noEmit — exit 0.
  • bun run privacy:scan — passed.
  • Real Windows Task Scheduler execution was not available on this macOS host. The focused tests inject scheduler and service-state evidence and verify the exact ownership decisions; they do not claim a Windows 11 end-to-end ocx sync run.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed. No user-facing contract changed; this restores per-home behavior for an existing supported setup.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults. The probe parses only the registered launcher path, never reads a foreign path, and retains fail-closed behavior for current-home or malformed evidence.

Summary by CodeRabbit

  • Bug Fixes

    • Improved Windows scheduled-task detection to verify that a registered task belongs to the current installation.
    • Reports an indeterminate status when task details are missing, malformed, or point outside the configured installation directory.
    • Prevents tasks belonging to another installation from being incorrectly recognized.
  • Tests

    • Added coverage for task ownership validation and missing local task definitions.

Treat a machine-global scheduled task that points outside the effective
OpenCodex home as another home's manager claim. Keep missing or malformed
current-home scheduler evidence fail-closed.
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner August 28, 2026 23:01
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 28, 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-08-28T23:05:29.920066Z 51998e3 PR opened
ℹ️ 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.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the bug Something isn't working label Aug 28, 2026
@coderabbitai

coderabbitai Bot commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2d6deaed-36d0-4a68-b89b-78115a68647a

📥 Commits

Reviewing files that changed from the base of the PR and between 577bce6 and 51998e3.

📒 Files selected for processing (2)
  • src/service-manager-probe.ts
  • tests/codex-service-manager-probe-hardening.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.


📝 Walkthrough

Walkthrough

The Windows service-manager probe now validates a registered scheduled task’s launcher path against the effective OPENCODEX_HOME when local task XML is absent. New tests cover foreign-home ownership and missing local XML.

Changes

Windows scheduled-task ownership

Layer / File(s) Summary
Validate registered task ownership
src/service-manager-probe.ts
When local task XML is absent, the probe reads the registered task XML. It returns present only for a quoted launcher path inside the effective OPENCODEX_HOME; otherwise, it returns unknown.
Cover ownership regression cases
tests/codex-service-manager-probe-hardening.test.ts
Tests verify that a task from another OpenCodex home does not claim the current home and that a task for the current home remains unknown when local task XML is missing.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 51998

The change scopes Windows scheduler ownership by OpenCodex home while preserving fail-closed handling for ambiguous or malformed registrations. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: ingwannu

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: scoping Windows scheduler ownership by the effective OpenCodex home.
Linked Issues check ✅ Passed The changes address issue #2800. In src/service-manager-probe.ts, registered task ownership is checked against the effective OPENCODEX_HOME when local task XML is absent. A task launcher outside the c…
Out of Scope Changes check ✅ Passed The changes are limited to the Windows scheduler ownership probe and focused regression tests. They directly support the linked issue and do not introduce unrelated production behavior or unrelated te…
Full details: Linked Issues check

Explanation

The changes address issue #2800. In src/service-manager-probe.ts, registered task ownership is checked against the effective OPENCODEX_HOME when local task XML is absent. A task launcher outside the current home no longer claims the current installation, while missing or invalid current-home ownership remains fail-closed. tests/codex-service-manager-probe-hardening.test.ts covers both ownership cases.

Full details: Out of Scope Changes check

Explanation

The changes are limited to the Windows scheduler ownership probe and focused regression tests. They directly support the linked issue and do not introduce unrelated production behavior or unrelated test coverage.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/lane-f-multihome-scheduler

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.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 74 / 80

이 PR은 Windows에서 기계 전역으로 등록되는 Task Scheduler 작업 opencodex-proxy가 어느 OpenCodex 홈(OPENCODEX_HOME) 소속인지를 따지지 않던 구멍(#2800)을 막습니다. 지금 dev(HEAD 577bce695)의 src/service-manager-probe.ts inspectWindows는 로컬 스테이징 파일 opencodex-service-task.xml가 없고(task === "absent") 스케줄러만 등록돼 있으면 무조건 unknown("Task Scheduler holds opencodex-proxy but its task XML is missing")를 돌려줍니다. 스케줄러 등록은 머신 하나당 하나이고, 작업 XML·런처는 홈마다 따로 있으니, 이미 한 홈이 스케줄러를 잡은 뒤 두 번째 홈에서 ocx sync를 하면 "스케줄러는 있는데 내 XML은 없다"가 되어 inspectNativeCodexOwnership이 ownership: "unknown"으로 멈추고 admitCodexWrite가 영구 거절합니다. 레이스나 잠금이 아니라 소유권 모델 자체 버그입니다.

고치는 방식은 단순합니다. task === "absent"이어도 스케줄러가 등록돼 있으면 schtasks /query /xml로 받은 registration.registeredXml에서 런처 경로만 뽑고(windowsTaskArguments + 따옴표 경로 정규식), 그 경로가 현재 configDir(유효 OPENCODEX_HOME) 안인지 windowsPathInsideConfigDir로만 봅니다. 현재 홈 안이면 예전처럼 fail-closed unknown(내 XML이 진짜 없거나 읽기 실패), 현재 홈 밖이면 다른 홈의 매니저 클레임으로 보고 { kind: "absent" }를 줍니다. 그러면 ownership-preflight.ts의 manager.kind === "absent" 갈래로 내려가 상태 파일이 없으면 ownership: "owned" / no service state and no service manager claim이 되어 두 번째 홈도 기존 Codex write coordinator까지 갈 수 있습니다. 외부가 런처 파일을 읽지 않고 경로 소속만 보는 점은 walkWindowsChain이 이미 쓰던 "configDir 밖은 따라가지 않는다" 규칙과 같고, 보안상도 맞습니다.

테스트는 tests/codex-service-manager-probe-hardening.test.ts에 두 개를 더합니다. (1) 스케줄러 XML이 foreign-opencodex 런처를 가리키면 현재 홈은 owned가 되어야 하고, (2) 현재 홈 런처를 가리키는데 로컬 task XML이 없으면 여전히 unknown + 기존 문구여야 합니다. PR 본문의 RED→mutation RED→GREEN(관련 스위트 83 pass)과 tsc/privacy:scan 서술은 이 회귀를 고정하려는 의도와 맞습니다. WinSW 겹침·등록 XML 읽기 실패·매니저 질의 실패 경로는 이 블록 앞에서 그대로 fail-closed로 남습니다.

라인 681–698 (PR diff 기준 inspectWindows의 task === "absent" 분기) - 런처 인자 파싱 실패·따옴표 경로 추출 실패·현재 홈 소속인데 로컬 XML 부재, 세 경우가 모두 같은 문구 Task Scheduler holds opencodex-proxy but its task XML is missing를 씁니다. XML을 아예 못 읽은 경우만 definition XML could not be read로 갈라져 있어서, 현장에서 "XML이 없다"와 "Arguments/경로가 깨졌다"를 구분하기 어렵습니다. 동작(fail-closed)은 맞지만 진단 문자열은 갈라 두는 편이 낫습니다.

windowsTaskArguments / 첫 따옴표 경로 추출 - walkWindowsChain과 같은 파서라 일관성은 좋습니다. 다만 Arguments에 따옴표 없는 경로·추가 스위치가 섞인 비정상 XML이면 현재 홈인데도 unknown이 아니라 파서 실패 문구로만 남거나, 외국 홈으로 오인될 여지는 거의 없고 대체로 fail-closed 쪽으로 기울습니다. 실기기 Windows 11에서 두 홈 ocx sync E2E는 이 호스트에서 돌리지 않았고, 주입 픽스처 검증만 한 점은 PR이 스스로 밝힌 한계입니다.

메인테이너의 판단이 필요한 지점

너의 추천
dev에 그대로 merge하세요. #2800 재현 경로를 정확히 끊는 작은 소유권 수정이고, 외국 홈 런처를 읽지 않으며 현재 홈·WinSW·질의 실패는 fail-closed를 유지합니다. merge 후 #2800 닫고, 가능하면 Windows 박스에서 홈 두 개 ocx sync 한 번만 smoke 하면 됩니다. 진단 문구 분리는 필수는 아니고 follow-up으로 미뤄도 됩니다.

이 댓글은 grok-bot이 작성했습니다

@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: 51998e3f39

ℹ️ 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 (!registration.registeredXml.trim()) {
return unknown("Task Scheduler is registered but its definition XML could not be read");
}
const launcherArg = windowsTaskArguments(registration.registeredXml);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Reject multiple task actions before declaring the task foreign

When the on-disk task XML is absent and the registered task contains multiple valid <Exec> actions, this reads only the first <Arguments> element. If that first action names a launcher outside configDir while a later action launches the current home's wrapper, the branch returns absent; inspectNativeCodexOwnership can then authorize unattended Codex writes despite an unaccounted current-home scheduler registration. Fail closed unless the registered XML has exactly one structurally valid executable action (as windowsTaskRegistrationHealthy already requires), then classify its launcher path.

Useful? React with 👍 / 👎.

@lidge-jun
lidge-jun merged commit 878d986 into dev Aug 28, 2026
27 checks passed
@lidge-jun
lidge-jun deleted the codex/lane-f-multihome-scheduler branch August 28, 2026 23:14
tarunravi pushed a commit to tarunravi/opencodex that referenced this pull request Sep 14, 2026
Treat a machine-global scheduled task that points outside the effective
OpenCodex home as another home's manager claim. Keep missing or malformed
current-home scheduler evidence fail-closed.
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
Treat a machine-global scheduled task that points outside the effective
OpenCodex home as another home's manager claim. Keep missing or malformed
current-home scheduler evidence fail-closed.
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