Code Quality: PR #391 #1126
codeql
on: dynamic
Matrix: analyze
Annotations
9 warnings
|
Analyze (python)
Starting April 2026, the CodeQL Action will skip computing file coverage information on pull requests to improve analysis performance. File coverage information will still be computed on non-PR analyses.
To opt out of this change, create a custom repository property with the name `github-codeql-file-coverage-on-prs` and the type "True/false", then set this property to `true` in the repository's settings.
|
|
Analyze (javascript-typescript)
Starting April 2026, the CodeQL Action will skip computing file coverage information on pull requests to improve analysis performance. File coverage information will still be computed on non-PR analyses.
To opt out of this change, create a custom repository property with the name `github-codeql-file-coverage-on-prs` and the type "True/false", then set this property to `true` in the repository's settings.
|
|
Discovered test total bumped from 52 to 57 without matching items list change:
src/snapshot-tests/__fixtures__/json/device/test--failure.json#L34
The fixture's discovered.total increased from 52 to 57, but the visible portion of the items array in this hunk shows no new entries to justify five additional discovered tests. Per the skill guardrails, fixture updates must map to intentional behavior changes and must not be patched to make tests pass; if the discovered count truly changed due to workspace cleanup work, the items array should be updated in lockstep, otherwise this looks like an ad hoc fixture patch that breaks the structured output envelope contract.
|
|
isPidAlive treats EPERM as 'not alive', inverting liveness semantics:
src/utils/process-liveness.ts#L11
When `process.kill(pid, 0)` throws EPERM, it means the process exists but the caller lacks permission to signal it — i.e., the PID is alive. The current logic returns `false` for EPERM (`code !== 'ESRCH' && code !== 'EPERM'`), so live processes owned by another user are reported as dead. Given this utility is used to gate workspace-scoped cleanup and ownership checks, this can cause one process to delete another live process's artifacts — exactly the regression the PR description warns about.
|
|
[4TC-W63] isPidAlive treats EPERM as 'not alive', inverting liveness semantics (additional location):
src/daemon.ts#L336
When `process.kill(pid, 0)` throws EPERM, it means the process exists but the caller lacks permission to signal it — i.e., the PID is alive. The current logic returns `false` for EPERM (`code !== 'ESRCH' && code !== 'EPERM'`), so live processes owned by another user are reported as dead. Given this utility is used to gate workspace-scoped cleanup and ownership checks, this can cause one process to delete another live process's artifacts — exactly the regression the PR description warns about.
|
|
Forced shutdown timer and server.close callback can race, running cleanup twice and calling process.exit concurrently:
src/daemon.ts#L336
When the 5-second forced shutdown timer fires it sets `forcedShutdownTimer = null` and starts an async `cleanupArtifacts()` chain that ends in `process.exit(1)`. If `server.close()` then completes (or was already in flight), its callback observes `forcedShutdownTimer` as null, skips cancellation, and runs its own `cleanupArtifacts()` and `process.exit(exitCode)` chain. The two async chains can execute `cleanupOwnedWorkspaceFilesystemArtifacts` concurrently against the same workspace and race on `process.exit`, undermining the ownership/locking guarantees the PR is trying to add and yielding non-deterministic exit codes.
|
|
[NZY-D9Q] Forced shutdown timer and server.close callback can race, running cleanup twice and calling process.exit concurrently (additional location):
src/utils/workspace-filesystem-lifecycle.ts#L410
When the 5-second forced shutdown timer fires it sets `forcedShutdownTimer = null` and starts an async `cleanupArtifacts()` chain that ends in `process.exit(1)`. If `server.close()` then completes (or was already in flight), its callback observes `forcedShutdownTimer` as null, skips cancellation, and runs its own `cleanupArtifacts()` and `process.exit(exitCode)` chain. The two async chains can execute `cleanupOwnedWorkspaceFilesystemArtifacts` concurrently against the same workspace and race on `process.exit`, undermining the ownership/locking guarantees the PR is trying to add and yielding non-deterministic exit codes.
|
|
isPidAlive returns false for live processes owned by other users (EPERM):
src/utils/process-liveness.ts#L11
`process.kill(pid, 0)` returns EPERM when the target PID exists but is owned by a different user (or is otherwise not signalable by this process). Treating EPERM as 'not alive' will misclassify live processes — the function returns `code !== 'ESRCH' && code !== 'EPERM'`, so EPERM yields false. In a multi-process workspace cleanup context this means another user's live daemon/simulator helper could be considered dead and its artifacts could be reclaimed/deleted, which is exactly the cross-process safety boundary this PR is meant to protect.
|
|
[7FE-H6X] isPidAlive returns false for live processes owned by other users (EPERM) (additional location):
src/utils/workspace-filesystem-lifecycle.ts#L468
`process.kill(pid, 0)` returns EPERM when the target PID exists but is owned by a different user (or is otherwise not signalable by this process). Treating EPERM as 'not alive' will misclassify live processes — the function returns `code !== 'ESRCH' && code !== 'EPERM'`, so EPERM yields false. In a multi-process workspace cleanup context this means another user's live daemon/simulator helper could be considered dead and its artifacts could be reclaimed/deleted, which is exactly the cross-process safety boundary this PR is meant to protect.
|