Repository navigation
Conversation
Detached startup on Windows reported every failure as "the host may prohibit Job Object breakaway", so a partial breakaway under a nested Job, a direct refusal, and unrelated spawn errors all got the same guidance. - Report Job refusal only when CreateProcessW returns access denied and this process is in a Job; other spawn errors name the executable and keep the OS error. - Add PartialBreakaway for a child that was created but is still in an outer Job. - Split Job membership checks from resuming the suspended thread, and add ResumeFailed for a child that left every Job but could not be resumed. - Give each failure its own recovery hint.
yaou-li
changed the title
iuyo5678
left a comment
iuyo5678
left a comment
There was a problem hiding this comment.
Thanks for improving the Windows startup diagnostics. Separating process creation, partial breakaway, and thread resumption makes the failure paths clearer, while preserving the existing check-before-resume behavior and child cleanup.
I would like to address the diagnostic accuracy and Windows test coverage points below before merging. These should be small, focused changes; a full Job-chain enumeration implementation does not need to be part of this PR.
Validation on this revision: cargo fmt --all -- --check passed, and cargo test -p bsk --lib --locked --offline startup_ passed all three selected tests on macOS. This does not validate the Windows-only paths. The Windows cross-check stopped in a native dependency because the local Windows SDK headers were unavailable. The GitHub CI run is currently action_required with no jobs executed, so we still need successful Windows regression results before merging.
One small metadata correction: the title references #391, but the Windows nested-Job diagnostics issue is #392.
| /// when the immediate Job forbids breakaway. Any other error, or one raised | ||
| /// outside every Job, has a different cause. | ||
| fn classify_spawn_failure(err: io::Error, in_job: bool, exe: &Path) -> anyhow::Error { | ||
| let refused = in_job && err.raw_os_error() == Some(ERROR_ACCESS_DENIED as i32); |
There was a problem hiding this comment.
Being in a Job and receiving ERROR_ACCESS_DENIED does not establish that the Job forbids breakaway. For example, a process in a breakaway-enabled Job can still be denied access to the target executable. This branch would then report that the host Job definitely forbids breakaway and direct the user toward a workaround that does not address the actual cause. Could we keep this diagnosis tentative (Job restrictions or process/executable access permissions), retain the executable path and original OS error, and only use definitive wording when the restriction has actually been verified? Please cover access denied in a breakaway-enabled Job as well as a genuinely restrictive Job.
| "The immediate Job Object allows breakaway, but a nested outer Job Object \ | ||
| (typically held by a Windows system service such as BrokerInfrastructure, \ | ||
| Appinfo or PcaSvc) blocked full breakaway — this is the default Windows 11 \ | ||
| nested-Job layout. Switching terminals or shells usually does not help. \ |
There was a problem hiding this comment.
The membership check establishes that the child is still in a Job, but it does not inspect the Job owner or identify the host's Job hierarchy. An IDE or another launcher can also supply the restrictive outer Job. Presenting this as the default Windows 11 system-service layout, and saying that switching terminals usually does not help, goes beyond what was detected and may discourage a useful recovery path. Could we describe the observed partial breakaway, present the named services only as possible examples if needed, and keep trying an independent host/terminal as a conditional alternative to a persistent foreground task?
| } | ||
|
|
||
| #[test] | ||
| fn only_access_denied_inside_a_job_is_a_breakaway_refusal() { |
There was a problem hiding this comment.
Please wire this new Windows-only unit test into CI. The Windows workflow runs filtered library tests, and the lifecycle script selects daemon::start::tests, which does not match daemon::start::windows::tests. The Linux full-suite job excludes this module, so this test will not run under the current configuration even after CI is approved. An explicit Windows step such as cargo test -p bsk --lib --locked daemon::start::windows::tests would cover it. It would also help to extend the existing restrictive/nested Job integration tests to assert the distinct diagnostics; their shared refusal assertions currently only check generic Job/recovery text.
Access denied from CreateProcessW inside a Job no longer proves that the Job forbids breakaway. Read the immediate Job's breakaway policy and report a Job refusal only when it forbids breakaway; report denied access to the executable or process when breakaway is permitted, and keep the diagnosis tentative when the policy cannot be read. Every case keeps the executable path and the original OS error. Describe a partial breakaway as observed, without asserting which Job holds the child, and keep an independent host or terminal as a conditional alternative to a persistent foreground task. Run daemon::start::windows::tests in the Windows CI job and assert the distinct diagnostics in the restrictive and nested Job integration tests. Refs Tencent#392
yaou-li
changed the title
Detached startup on Windows reported every failure as "the host may prohibit Job Object breakaway", so a partial breakaway under a nested Job, a direct refusal, and unrelated spawn errors all got the same guidance.