Skip to content

fix(windows): preserve foreign resources during host-agent lifecycle cleanup - #7266

Closed
tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/windows-host-agent-owned-stop
Closed

tang-vu wants to merge 1 commit into
Osmantic:mainfrom
tang-vu:fix/windows-host-agent-owned-stop

Conversation

@tang-vu

@tang-vu tang-vu commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Inspect ownership before stopping receipt/listener processes or task/Startup persistence. Retained process handles prevent numeric PID reuse during termination; failures preserve recovery receipts.

Reconstructed only the reviewed #7266 scope on current upstream main b4f015a921c59eeb20317c5f48adf2dfb43b1430. Local candidate a581a5105c1827e3b1ae7c75254fc966312df351; previous published head a0e9316a578ed875460152304ece507532a488b2 is retained locally under refs/archive/ods-20261004/pr-7266. No behavioral changes were added during reconstruction; current-main changes outside this scope are preserved.

Issue association: Closes #7296.

AI Assistance

The human contributor and Codex collaborated on this PR. The contributor provided project direction, scope decisions and environment setup; Codex assisted with analysis, implementation or reconstruction, tests, review and PR preparation. The validation commands were executed by Codex, with results and limitations documented in this PR. The human contributor remains responsible for the final changes and validation claims. Maintainer review and applicable release gates remain required before merge. Earlier independent AI review receipts apply only to their recorded heads; current-candidate validation is identified separately in this PR.

Release Lane

  • Mainline change targeting main

Changed Surface

  • Installer / bootstrap / lifecycle

Risk And Validation

  • Risk level: High

  • git diff --check

  • Focused tests

  • Commit identity gate against exact current-main base/candidate

19 native ownership checks, 52 uninstall contracts and runtime recovery passed.

The focused results were run on the exact candidate file tree before committing; the commit only recorded that tested tree. Original-head/full native and combined integration receipts remain archived, and are inherited evidence rather than fresh install or new-head hosted CI results.

CI execution: lint-powershell.yml explicitly runs ownership on PowerShell 5.1 and 7.

Operational Change Check

  • Operational validation intentionally deferred: full fresh install, live model/GPU, Portal, hardware/release fleet and platform-specific lanes not explicitly exercised above. Root's separate current-main installation is not yet candidate validation.

Notes For Reviewers

Real Windows processes/CIM/TCP are exercised; scheduler failure fixtures do not certify a full installed login-persistence lifecycle.

Rollback: revert this scope; preserve existing recovery receipts and configured data. Local native tests and green original-head CI do not replace human review or deployed-runtime gates.

@Lightheartdevs

Copy link
Copy Markdown
Collaborator

Thanks for the thorough PR. The concern, killing an unrelated process through a stale PID, is fair. This version fails closed, though: a stale PID file pointing at a reused PID makes agent start and restart refuse and makes reinstall throw. That would lock owners out of their own agent, which is the outcome we most want to avoid. A smaller fix would only stop a process whose command line runs <InstallDir>\bin\ods-host-agent.py, and otherwise drop the stale PID file and carry on. Test-ODSNativeProcessExecutable already does that kind of check. We're closing this one, and a PR along those lines would be welcome.

@tang-vu

tang-vu commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

Following the requested narrower approach, #7390 checks only the installed Python host-agent script before stopping PID-file or port-listener processes. Stale receipts are discarded and start/restart/reinstall continue; the previous refusal/framework is not carried forward.

Native regression fails on current-main source because the foreign fixture is terminated; the replacement passes 19 checks on PowerShell 5.1 and 7, plus affected runtime/uninstall contracts. The independent agent review also verified the actual py -3 child command line. Task Scheduler/Startup/health portions use fixtures, so full installed login persistence and release-fleet validation remain explicit deferred gates.

The contributor and Codex collaborated on direction, implementation, tests and review; these command receipts were produced by Codex. The original head and remote branch remain preserved. #7390 references #7296 for the process concern without claiming its broader task/Startup scope is resolved.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Windows agent lifecycle can terminate unrelated processes from stale receipts or port collisions

2 participants