Skip to content

Commit eefc9f2

Browse files
authored
fix(core): shared sidecar must not hang one-shot scripts on exit (#1551)
The default shared sidecar pool kept its child process + stdio referenced after vm.dispose() released the last VM lease, so every standalone script (all the quickstart examples) hung on exit and had to be SIGINT'd. Fix: a counted event-loop hold on the shared sidecar, taken for the WHOLE create→use→dispose lifetime of each VM lease. Child + stdio are ref'd while holds > 0 and unref'd at 0, so in-flight VM work keeps the host alive while an idle program can exit. unref != kill: the sidecar stays pooled/reusable and is re-ref'd on the next lease. A one-time synchronous process 'exit' hook SIGKILLs pooled sidecars so a clean exit reaps them instantly. No SIGINT/SIGTERM handlers — the host owns signals. Hardened after adversarial review: - counted (not boolean) hold survives concurrent create/dispose - child handle cached BEFORE the handshake await so a failed authenticateAndOpenSession() can still reap it (no orphan / pinned loop) - rejected spawn clears nativeProcess so a later create() retries - dispose no longer force-zeros the hold counter; unbalanced release warns - loud warning if the secure-exec child handle can't be resolved Also: - tests/shared-sidecar-clean-exit.test.ts: regression for the hang - tests/agent-pkg-matrix.e2e.test.ts: skipped-by-default 4 pkg mgr × 4 agent real-API streaming matrix (AGENTOS_MATRIX_E2E) - fix(agentos-sidecar): embed AGENTOS_SYSTEM_PROMPT.md in the crate so cargo publish can package it; drop now-empty fixtures dir from agentos-core files - docs(agents): document OpenCode model config on its per-agent page; Sessions points to the per-agent docs for agent-specific configuration
1 parent 0a43031 commit eefc9f2

12 files changed

Lines changed: 659 additions & 9 deletions

File tree

File renamed without changes.

‎crates/agentos-sidecar/src/acp_extension.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,10 @@ const OPENCODE_DEFAULT_CONTEXT_PATHS: [&str; 11] = [
6868
"OPENCODE.md",
6969
"OPENCODE.local.md",
7070
];
71-
const AGENTOS_SYSTEM_PROMPT: &str = include_str!(concat!(
72-
env!("CARGO_MANIFEST_DIR"),
73-
"/../../packages/core/fixtures/AGENTOS_SYSTEM_PROMPT.md"
74-
));
71+
// Embedded next to this source so `cargo publish` packages it (an out-of-crate
72+
// `include_str!` path breaks the isolated package-verify build). The TypeScript
73+
// side reads the same file from this location for its sanity check.
74+
const AGENTOS_SYSTEM_PROMPT: &str = include_str!("AGENTOS_SYSTEM_PROMPT.md");
7575
/// Hard ceiling on the `stdout_buffer` retained on an `AcpSessionRecord` between
7676
/// requests. The buffer only ever holds the partial trailing line not yet parsed
7777
/// into a complete JSON-RPC message, so this also bounds the per-session record

‎packages/core/package.json‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
"main": "./dist/index.js",
77
"types": "./dist/index.d.ts",
88
"files": [
9-
"dist",
10-
"fixtures"
9+
"dist"
1110
],
1211
"exports": {
1312
".": {

‎packages/core/src/agent-os.ts‎

Lines changed: 169 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5527,11 +5527,121 @@ interface AgentOsSidecarState {
55275527
* are cheap incremental tenants of one process rather than one-process-each.
55285528
*/
55295529
nativeProcess?: Promise<SharedSidecarNativeProcess>;
5530+
/**
5531+
* The shared sidecar's child process + stdio, cached for synchronous
5532+
* ref/unref. Unref'd when no VM leases are active so a one-shot host process
5533+
* can exit after `dispose()`; re-ref'd while leases are live.
5534+
*/
5535+
sharedChild?: SidecarEventLoopHandle;
5536+
/**
5537+
* Number of live "holds" on the shared sidecar's event-loop reference. A hold
5538+
* is taken for the WHOLE create→use→dispose lifetime of every VM lease (not
5539+
* just while it sits in `activeLeases`), so a VM that is still mid-creation
5540+
* still counts. The child + stdio are ref'd while this is >0 and unref'd at 0.
5541+
* A counter (not a boolean) so concurrent create/dispose cannot clobber each
5542+
* other — Node ref/unref is not itself counted.
5543+
*/
5544+
eventLoopHolds?: number;
55305545
}
55315546

55325547
const sidecarStates = new WeakMap<AgentOsSidecar, AgentOsSidecarState>();
55335548
const sharedSidecars = new Map<string, AgentOsSidecar>();
55345549

5550+
interface RefCountableHandle {
5551+
ref?(): unknown;
5552+
unref?(): unknown;
5553+
}
5554+
5555+
interface SidecarEventLoopHandle extends RefCountableHandle {
5556+
stdin?: RefCountableHandle | null;
5557+
stdout?: RefCountableHandle | null;
5558+
stderr?: RefCountableHandle | null;
5559+
kill?(signal?: string | number): unknown;
5560+
}
5561+
5562+
let sidecarProcessExitHookInstalled = false;
5563+
5564+
/**
5565+
* Install a one-time, synchronous `process.on("exit")` hook that SIGKILLs any
5566+
* pooled shared sidecar child. Once a one-shot host process is allowed to exit
5567+
* (its sidecar handles are unref'd at 0 leases), this reaps the sidecar
5568+
* immediately instead of waiting for its stdin-EOF grace window — no orphan, no
5569+
* delay. We deliberately do NOT install SIGINT/SIGTERM handlers: a library
5570+
* should not hijack the host's signal handling. SIGINT still reaches the sidecar
5571+
* via the process group, and SIGTERM-driven exit still closes its stdin.
5572+
*/
5573+
function ensureSidecarProcessExitCleanup(): void {
5574+
if (sidecarProcessExitHookInstalled) return;
5575+
sidecarProcessExitHookInstalled = true;
5576+
process.on("exit", () => {
5577+
for (const sidecar of sharedSidecars.values()) {
5578+
try {
5579+
sidecarStates.get(sidecar)?.sharedChild?.kill?.("SIGKILL");
5580+
} catch {
5581+
// best-effort reap; the process is exiting regardless
5582+
}
5583+
}
5584+
});
5585+
}
5586+
5587+
function sidecarChildHandle(client: unknown): SidecarEventLoopHandle | undefined {
5588+
// SidecarProcess -> StdioSidecarProtocolClient.child (the spawned ChildProcess).
5589+
const protocolClient = (
5590+
client as { protocolClient?: { child?: SidecarEventLoopHandle } } | undefined
5591+
)?.protocolClient;
5592+
return protocolClient?.child ?? undefined;
5593+
}
5594+
5595+
/**
5596+
* Apply the current hold state to the shared sidecar's child + stdio: ref them
5597+
* while ≥1 hold is live so in-flight VM work keeps the host process alive; unref
5598+
* them at 0 so a one-shot script exits on its own after `dispose()`. The sidecar
5599+
* process itself stays running (reusable) and self-exits on stdin EOF when the
5600+
* host finally goes away. Best-effort: never let ref/unref break VM lifecycle.
5601+
*/
5602+
function applySharedSidecarHold(state: AgentOsSidecarState): void {
5603+
const child = state.sharedChild;
5604+
if (!child) return;
5605+
const hold = (state.eventLoopHolds ?? 0) > 0;
5606+
for (const handle of [child, child.stdin, child.stdout, child.stderr]) {
5607+
if (!handle) continue;
5608+
try {
5609+
if (hold) handle.ref?.();
5610+
else handle.unref?.();
5611+
} catch {
5612+
// ref/unref is an optimization, not correctness-critical
5613+
}
5614+
}
5615+
}
5616+
5617+
/**
5618+
* Take a hold for the entire create→use→dispose lifetime of one VM lease. Taken
5619+
* BEFORE VM creation starts (not when the lease lands in `activeLeases`) so a VM
5620+
* that is still mid-creation keeps the sidecar ref'd and a concurrent dispose
5621+
* cannot unref it out from under the in-flight create.
5622+
*/
5623+
function acquireSharedSidecarHold(state: AgentOsSidecarState): void {
5624+
state.eventLoopHolds = (state.eventLoopHolds ?? 0) + 1;
5625+
if (state.eventLoopHolds === 1) applySharedSidecarHold(state);
5626+
}
5627+
5628+
/** Release a hold taken by {@link acquireSharedSidecarHold}; unref at 0. */
5629+
function releaseSharedSidecarHold(state: AgentOsSidecarState): void {
5630+
const current = state.eventLoopHolds ?? 0;
5631+
if (current <= 0) {
5632+
// The `holdReleased` guard makes each lease release exactly once, so this
5633+
// should be unreachable. Warn rather than silently floor, per the repo's
5634+
// no-silent-masking rule, so an accounting bug surfaces instead of hiding.
5635+
state.eventLoopHolds = 0;
5636+
console.warn(
5637+
"[agentos] shared sidecar event-loop hold released more than acquired",
5638+
);
5639+
return;
5640+
}
5641+
state.eventLoopHolds = current - 1;
5642+
if (state.eventLoopHolds === 0) applySharedSidecarHold(state);
5643+
}
5644+
55355645
/**
55365646
* Spawn-once accessor for a sidecar handle's shared native process. Concurrent
55375647
* callers await the same promise, so one `AgentOsSidecar` maps to exactly one
@@ -5542,15 +5652,47 @@ function ensureSharedSidecarNativeProcess(
55425652
): Promise<SharedSidecarNativeProcess> {
55435653
const state = getSidecarState(sidecar);
55445654
if (!state.nativeProcess) {
5655+
ensureSidecarProcessExitCleanup();
55455656
state.nativeProcess = (async () => {
55465657
const client = SidecarProcess.spawn({
55475658
cwd: REPO_ROOT,
55485659
command: ensureNativeSidecarBinary(),
55495660
args: [],
55505661
frameTimeoutMs: NATIVE_SIDECAR_FRAME_TIMEOUT_MS,
55515662
});
5552-
const session = await client.authenticateAndOpenSession();
5553-
return { client, session };
5663+
// Track the child immediately — BEFORE the handshake await — so a
5664+
// failed `authenticateAndOpenSession()` can still reap it (otherwise
5665+
// the spawned child is untracked, unreapable, and pins the loop).
5666+
state.sharedChild = sidecarChildHandle(client);
5667+
if (!state.sharedChild) {
5668+
// We reached into @secure-exec/core internals to get the child for
5669+
// idle-unref. If that shape ever changes this returns undefined and
5670+
// the optimization silently stops working (one-shot scripts would
5671+
// hang again). Make it loud rather than a silent regression.
5672+
console.warn(
5673+
"[agentos] could not resolve the shared sidecar child handle; " +
5674+
"standalone scripts may not exit cleanly after dispose(). " +
5675+
"This usually means @secure-exec/core internals changed.",
5676+
);
5677+
}
5678+
// Apply the current hold state to the just-spawned child.
5679+
applySharedSidecarHold(state);
5680+
try {
5681+
const session = await client.authenticateAndOpenSession();
5682+
return { client, session };
5683+
} catch (error) {
5684+
// Spawn/handshake failed: reap the child, drop the cached handle,
5685+
// and CLEAR the rejected promise so the next create() retries
5686+
// instead of permanently wedging on a rejected `nativeProcess`.
5687+
try {
5688+
state.sharedChild?.kill?.("SIGKILL");
5689+
} catch {
5690+
// already gone
5691+
}
5692+
state.sharedChild = undefined;
5693+
state.nativeProcess = undefined;
5694+
throw error;
5695+
}
55545696
})();
55555697
}
55565698
return state.nativeProcess;
@@ -5565,6 +5707,14 @@ async function disposeSharedSidecarNativeProcess(
55655707
return;
55665708
}
55675709
state.nativeProcess = undefined;
5710+
// The cached child is now dead; drop it (symmetric with the assignment in
5711+
// ensureSharedSidecarNativeProcess). We deliberately do NOT zero
5712+
// `eventLoopHolds` here: this runs only from `AgentOsSidecar.dispose()`, which
5713+
// has already set the handle to `disposing` (so no new lease can acquire) and
5714+
// drained `activeLeases`; the disposed handle's state is then abandoned. Force-
5715+
// zeroing a shared counter could clobber a hold on a freshly re-acquired
5716+
// process generation, so it is left to the balanced acquire/release pairs.
5717+
state.sharedChild = undefined;
55685718
try {
55695719
const { client } = await pending;
55705720
await client.dispose();
@@ -5700,6 +5850,18 @@ async function leaseAgentOsSidecarVm<TVmAdmin extends InProcessSidecarVmAdmin>(
57005850
},
57015851
});
57025852

5853+
// Hold the shared sidecar's event-loop ref for this lease's WHOLE lifetime —
5854+
// taken now, before VM creation, so a concurrent dispose cannot unref the
5855+
// sidecar while this create is still in flight. Released exactly once on
5856+
// dispose or on a failed create.
5857+
acquireSharedSidecarHold(state);
5858+
let holdReleased = false;
5859+
const releaseHold = () => {
5860+
if (holdReleased) return;
5861+
holdReleased = true;
5862+
releaseSharedSidecarHold(state);
5863+
};
5864+
57035865
let disposed = false;
57045866
let leaseRecord: AgentOsSidecarLeaseRecord | undefined;
57055867

@@ -5726,6 +5888,10 @@ async function leaseAgentOsSidecarVm<TVmAdmin extends InProcessSidecarVmAdmin>(
57265888
state.activeLeases.delete(leaseRecord!);
57275889
state.description.activeVmCount = state.activeLeases.size;
57285890
await client.dispose();
5891+
// Release this lease's hold; the shared sidecar is unref'd only
5892+
// once the last hold (across all in-flight + active leases) drops,
5893+
// so a one-shot host process can then exit on its own.
5894+
releaseHold();
57295895
},
57305896
};
57315897

@@ -5737,6 +5903,7 @@ async function leaseAgentOsSidecarVm<TVmAdmin extends InProcessSidecarVmAdmin>(
57375903
return lease;
57385904
} catch (error) {
57395905
await client.dispose().catch(() => {});
5906+
releaseHold();
57405907
throw error;
57415908
}
57425909
}

0 commit comments

Comments
 (0)