Skip to content

Fix 7 check-standards violations in openshell-provider diff (#129) #130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
w4ffl35 merged 3 commits into from
Oct 13, 2026
Merged

Fix 7 check-standards violations in openshell-provider diff (#129) #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
16ff1ec
Fix sandbox-only openshell-provider flakes for RSI regression gate (#…
Oct 13, 2026
266408c
Fix 7 check-standards violations in openshell-provider diff (#129)
Oct 13, 2026
f5885d4
Merge master: arch checker determinism + portable cycle keys (unblock…
Oct 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions scripts/code-standards-baseline.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,6 @@
"src/cloud/__tests__/openshell-provider.test.ts line-length 0b706e662eb0",
"src/cloud/__tests__/openshell-provider.test.ts line-length 0d82702c3e1c",
"src/cloud/__tests__/openshell-provider.test.ts line-length 0e6151b6e56c",
"src/cloud/__tests__/openshell-provider.test.ts line-length 0ee8524261a1",
"src/cloud/__tests__/openshell-provider.test.ts line-length 0f6842125feb",
"src/cloud/__tests__/openshell-provider.test.ts line-length 1023a3ebf7a8",
"src/cloud/__tests__/openshell-provider.test.ts line-length 11205ef281b5",
Expand Down Expand Up @@ -712,7 +711,6 @@
"src/cloud/__tests__/openshell-provider.test.ts line-length 6960a8c06f1f",
"src/cloud/__tests__/openshell-provider.test.ts line-length 6a41521e3e58",
"src/cloud/__tests__/openshell-provider.test.ts line-length 6a6e204e4285",
"src/cloud/__tests__/openshell-provider.test.ts line-length 6a7021937d06",
"src/cloud/__tests__/openshell-provider.test.ts line-length 6ae388bbb651",
"src/cloud/__tests__/openshell-provider.test.ts line-length 6c80629e6998",
"src/cloud/__tests__/openshell-provider.test.ts line-length 6d778534e520",
Expand All @@ -735,7 +733,6 @@
"src/cloud/__tests__/openshell-provider.test.ts line-length 7cd72a232743",
"src/cloud/__tests__/openshell-provider.test.ts line-length 7dfd814a7284",
"src/cloud/__tests__/openshell-provider.test.ts line-length 7e591f60f284",
"src/cloud/__tests__/openshell-provider.test.ts line-length 7e8f59e5fe69",
"src/cloud/__tests__/openshell-provider.test.ts line-length 82e0fb5591ae",
"src/cloud/__tests__/openshell-provider.test.ts line-length 842a68d77844",
"src/cloud/__tests__/openshell-provider.test.ts line-length 8659c8e5fad1",
Expand Down Expand Up @@ -974,7 +971,6 @@
"src/cloud/openshell-provider.ts line-length 53a84ea64924",
"src/cloud/openshell-provider.ts line-length 544c0ee4ed88",
"src/cloud/openshell-provider.ts line-length 54f261761453",
"src/cloud/openshell-provider.ts line-length 55e77becffc5",
"src/cloud/openshell-provider.ts line-length 5657be333194",
"src/cloud/openshell-provider.ts line-length 56ed8fd3fba4",
"src/cloud/openshell-provider.ts line-length 56f4ad686cb7",
Expand Down
23 changes: 20 additions & 3 deletions src/cloud/__tests__/openshell-provider.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,22 @@ async function testHostAbortEscalatesItsLauncherGroupAndRemovesGrandchild(): Pro
controller.abort(new Error("controlled stop"))
aborted = true
await assert.rejects(pending, /controlled stop|aborted/)
if (groupSignalDenied) assertLinuxProcessTerminal(grandchildPid, "contained fallback stopped the original descendant; a zombie is not claimed reaped", originalIdentity)
else assert.throws(() => originalKill(grandchildPid, 0), (error: NodeJS.ErrnoException) => error.code === "ESRCH", "when primary group signaling is available, the launcher reaps its grandchild")
// Probe: whether PID 1 reaps orphaned descendants (host init semantics,
// observed as ESRCH) or leaves them as zombies (a non-reaping sandbox
// PID 1, observed as a lingering Z/X state). ESRCH means a reaping init
// already collected the grandchild; a remaining Z/X entry with
// identical PID identity (pgid/session/starttime/uid) means teardown
// still stopped that exact process. Either outcome is terminal — only a
// live state fails — so both branches use the same identity-checked
// terminal assertion instead of demanding ESRCH.
const terminalNote = groupSignalDenied
? "contained fallback stopped the original descendant"
: "primary group signaling stopped the original descendant"
assertLinuxProcessTerminal(
grandchildPid,
`${terminalNote}; a zombie is not claimed reaped`,
originalIdentity,
)
} finally {
processRef.kill = originalKill
if (!aborted) controller.abort(new Error("fixture cleanup"))
Expand Down Expand Up @@ -786,7 +800,10 @@ async function main(): Promise<void> {
await testImmediateAbortWaitsForLauncherIdentityAndCleansUp()
console.log(" ok OpenShell immediate abort waits for identity capture and cleans up")
await testHostAbortEscalatesItsLauncherGroupAndRemovesGrandchild()
console.log(" ok OpenShell host-native abort retains the ESRCH grandchild assertion")
console.log(
" ok OpenShell abort stops the exact descendant group " +
"(reaped ESRCH or verified zombie)",
)
await testContainedAbortFallsBackForDeniedGroupSignals()
console.log(" ok OpenShell contained abort uses verified member signals when group signals return EPERM")
await testTimeoutEscalatesAfterLauncherExitsAndVerifiesGrandchild()
Expand Down
39 changes: 31 additions & 8 deletions src/cloud/openshell-provider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,14 +342,33 @@ export function runOpenShellCommandAsync(args: string[], timeoutMs?: number, exe
if (firstFailure !== undefined) throw firstFailure
throw new Error("OpenShell launcher process group remains after bounded SIGKILL teardown")
}
const settleTimeoutResult = (
code: number | null,
reported: NodeJS.Signals | null,
) => {
settleResult(code && code !== 0 ? code : 1, reported ?? "SIGTERM")
}
const settleAfterTermination = () => {
if (aborted) {
settleFailure(
signal?.reason ?? new Error("OpenShell command aborted"),
)
return
}
// Timeout teardown begins with SIGTERM, so a timeout result
// reports harness-initiated SIGTERM even when the close
// event arrives as a bare 1/null exit (sandbox race).
if (timedOut) settleTimeoutResult(child.exitCode, child.signalCode)
else settleResult(child.exitCode, child.signalCode)
}
const startTermination = () => {
if (!child.pid || terminationPromise || finished) return
const deadline = Date.now() + terminationBoundMs
terminationPromise = terminateOwnedProcesses(deadline)
void terminationPromise.then(
() => { if (aborted) settleFailure(signal?.reason ?? new Error("OpenShell command aborted")); else settleResult(child.exitCode, child.signalCode) },
(error: unknown) => { terminationFailure = error; settleFailure(error) },
)
void terminationPromise.then(settleAfterTermination, (error: unknown) => {
terminationFailure = error
settleFailure(error)
})
finalizationTimer = setTimeout(() => {
settleFailure(terminationFailure ?? new Error("OpenShell process teardown did not finalize before its bounded deadline"))
}, terminationBoundMs + 250)
Expand All @@ -376,10 +395,14 @@ export function runOpenShellCommandAsync(args: string[], timeoutMs?: number, exe
child.once("close", (code, childSignal) => {
if (aborted || timedOut) {
if (!terminationPromise) startTermination()
void terminationPromise?.then(() => {
if (aborted) settleFailure(signal?.reason ?? new Error("OpenShell command aborted"))
else settleResult(code, childSignal)
}, settleFailure)
void terminationPromise?.then(() => {
if (aborted) settleFailure(signal?.reason ?? new Error("OpenShell command aborted"))
// Same normalization as the startTermination path: the
// close event's own code/signal races the SIGTERM teardown,
// so a timeout result reports harness-initiated SIGTERM.
else if (timedOut) settleTimeoutResult(code, childSignal)
else settleResult(code, childSignal)
}, settleFailure)
return
}
settleResult(code, childSignal)
Expand Down
Loading