fix(core): report shell exit only after output is captured (#56321)

This commit is contained in:
Aiden Cline
2026-10-17 20:34:28 -05:00
committed by GitHub
Unverified
parent 3099ec2c05
commit 3bdab634eb
+65 -51
View File
@@ -51,6 +51,13 @@ type Active = {
timeout?: (duration: number) => Effect.Effect<void>
}
type Ended = {
status: "exited" | "timeout"
exit?: number
signal?: string
completed: number
}
/**
* Location-owned non-interactive shell command process service.
*
@@ -72,7 +79,8 @@ export interface Interface {
readonly wait: (id: Shell.ID) => Effect.Effect<Shell.Info, NotFoundError>
// A known shell's terminal state and bounded tail. Missing capture remains distinct from its exit status.
readonly result: (started: Shell.Info) => Effect.Effect<ShellResult.Result>
// Replaces the running command's timeout from now; zero clears it.
// Replaces the running command's timeout from now; zero clears it. Ignored once the command has
// ended or timed out, even while its process is still being killed or its output captured.
readonly timeout: (id: Shell.ID, duration: number) => Effect.Effect<Shell.Info, NotFoundError>
readonly output: (id: Shell.ID, input?: Shell.OutputInput) => Effect.Effect<Shell.Output, NotFoundError>
readonly remove: (id: Shell.ID) => Effect.Effect<void, NotFoundError>
@@ -353,53 +361,28 @@ const layer = () =>
}),
)
const finish = (status: Info["status"], exit?: number, beforeWait = Effect.void, signal?: string) =>
Effect.gen(function* () {
if (command.info.status !== "running") return
command.info = produce(command.info, (draft) => {
draft.status = status
if (exit !== undefined) draft.exit = exit
if (signal !== undefined) draft.signal = signal
draft.time.completed = Date.now()
})
yield* beforeWait
yield* outputDone.await
// Resolve waiters with the terminal Info before any retention eviction, so an evicted
// command still reports success rather than the removal NotFoundError. This runs before
// the timeout-fiber interrupt below, which on the timeout path would otherwise cancel
// this very fiber (finish is invoked by the timeout fiber) before waiters are resolved.
yield* Deferred.succeed(command.done, command.info)
yield* bus.publish(Shell.Event.Exited, {
id,
...(exit !== undefined ? { exit } : {}),
status,
})
exitOrder.push(id)
while (exitOrder.length > EXITED_LIMIT) {
const oldest = exitOrder[0]
if (!oldest) break
yield* removeCommand(oldest)
}
// Keep exited history data-only. Interrupt last because finish may run on the timeout fiber.
const timeoutFiber = command.timeoutFiber
command.timeout = undefined
command.timeoutFiber = undefined
if (timeoutFiber) yield* Fiber.interrupt(timeoutFiber)
})
// The process exit and the timeout race to record how the command ended; the first wins.
const ended = Deferred.makeUnsafe<Ended>()
const end = (outcome: Omit<Ended, "completed">) =>
Effect.suspend(() => Deferred.succeed(ended, { ...outcome, completed: Date.now() }))
command.timeout = (duration) =>
Effect.gen(function* () {
if (Deferred.isDoneUnsafe(ended)) return
if (command.timeoutFiber) yield* Fiber.interrupt(command.timeoutFiber)
command.timeoutFiber = undefined
if (duration === 0 || command.info.status !== "running") return
if (duration === 0) return
command.timeoutFiber = runFork(
Effect.sleep(Duration.millis(duration)).pipe(
Effect.flatMap(() =>
finish(
"timeout",
undefined,
handle.kill({ forceKillAfter: Duration.seconds(3) }).pipe(Effect.catch(() => Effect.void)),
),
// A recorded timeout must always kill. The kill runs detached, so interrupting this
// timer (retime, remove, teardown) neither cancels it nor waits for the process to die.
Effect.andThen(
Effect.gen(function* () {
if (!(yield* end({ status: "timeout" }))) return
runFork(
handle.kill({ forceKillAfter: Duration.seconds(3) }).pipe(Effect.catch(() => Effect.void)),
)
}).pipe(Effect.uninterruptible),
),
),
)
@@ -407,18 +390,49 @@ const layer = () =>
yield* command.timeout(invocation.timeout)
// The process exits even after a timeout kill, so this fiber always publishes the outcome.
runFork(
handle.exitCode.pipe(
Effect.flatMap((code) => finish("exited", code)),
Effect.catch((error) =>
finish(
"exited",
undefined,
Effect.void,
error.cause instanceof CrossSpawnSpawner.KilledBySignal ? error.cause.signal : undefined,
Effect.gen(function* () {
yield* handle.exitCode.pipe(
Effect.flatMap((code) => end({ status: "exited", exit: code })),
Effect.catch((error) =>
end({
status: "exited",
signal:
error.cause instanceof CrossSpawnSpawner.KilledBySignal ? error.cause.signal : undefined,
}),
),
),
),
)
const outcome = yield* Deferred.await(ended)
// Report the terminal status only once captured output is complete, so a reader that
// sees it never reads a partial file.
yield* outputDone.await
command.info = produce(command.info, (draft) => {
draft.status = outcome.status
if (outcome.exit !== undefined) draft.exit = outcome.exit
if (outcome.signal !== undefined) draft.signal = outcome.signal
draft.time.completed = outcome.completed
})
// Resolve waiters before retention eviction, so an evicted command still reports its
// terminal Info rather than the removal NotFoundError.
yield* Deferred.succeed(command.done, command.info)
yield* bus.publish(Shell.Event.Exited, {
id,
...(outcome.exit !== undefined ? { exit: outcome.exit } : {}),
status: outcome.status,
})
exitOrder.push(id)
while (exitOrder.length > EXITED_LIMIT) {
const oldest = exitOrder[0]
if (!oldest) break
yield* removeCommand(oldest)
}
// Keep exited history data-only.
const timeoutFiber = command.timeoutFiber
command.timeout = undefined
command.timeoutFiber = undefined
if (timeoutFiber) yield* Fiber.interrupt(timeoutFiber)
}),
)
yield* bus.publish(Shell.Event.Created, { info })