Skip to content

Commit be54862

Browse files
committed
fix(task): close isolated launch ownership races
1 parent 11b5876 commit be54862

7 files changed

Lines changed: 461 additions & 87 deletions

cli/index.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3806,6 +3806,22 @@ program
38063806
}
38073807
});
38083808

3809+
// Resolve the durable task ownership receipt for an in-flight detached launch.
3810+
program
3811+
.command('get-task-id-by-spawn-token <token>')
3812+
.description('Output task ID for an internal spawn ownership token (machine-readable)')
3813+
.action(async (token) => {
3814+
try {
3815+
const { getTaskIdBySpawnToken } = await import(
3816+
'../task-lib/commands/get-task-id-by-spawn-token.js'
3817+
);
3818+
getTaskIdBySpawnToken(token);
3819+
} catch (error) {
3820+
console.error('Error resolving task spawn ownership:', error.message);
3821+
process.exit(1);
3822+
}
3823+
});
3824+
38093825
function failTuiUnavailable() {
38103826
console.error(
38113827
'The TUI is not included in this Zeroshot release. Use `zeroshot logs -f`, `zeroshot logs -w`, or `zeroshot list` instead.'

src/agent/agent-task-executor.js

Lines changed: 165 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1525,10 +1525,14 @@ async function spawnClaudeTaskIsolated(agent, context) {
15251525

15261526
// STEP 1: Spawn task and extract task ID (same as non-isolated mode)
15271527
// Timeout for spawn phase - if CLI hangs during init (e.g., opencode 429 bug), kill it
1528-
const SPAWN_TIMEOUT_MS = 30000; // 30 seconds to spawn task
1529-
// Note: Auth env vars are injected by IsolationManager, we only need model mapping here
1530-
const isolatedEnv =
1531-
providerName === 'claude' ? buildClaudeEnv(modelSpec, { includeAuth: false }) : {};
1528+
const SPAWN_TIMEOUT_MS = agent.spawnTimeoutMs ?? 30000;
1529+
const ownershipToken = createTaskSpawnOwnershipToken();
1530+
// Auth env vars are injected by IsolationManager; the launch token is the
1531+
// only authoritative bridge back to the detached task row in the container.
1532+
const isolatedEnv = {
1533+
...(providerName === 'claude' ? buildClaudeEnv(modelSpec, { includeAuth: false }) : {}),
1534+
[TASK_SPAWN_OWNERSHIP_TOKEN_ENV]: ownershipToken,
1535+
};
15321536

15331537
let isolatedPendingLaunch = null;
15341538
const taskId = await new Promise((resolve, reject) => {
@@ -1539,6 +1543,12 @@ async function spawnClaudeTaskIsolated(agent, context) {
15391543
let isolatedTaskId = null;
15401544
let wrapperClosed = false;
15411545
let resolveWrapperClose;
1546+
let stdout = '';
1547+
let stderr = '';
1548+
let resolved = false;
1549+
let timeoutError = null;
1550+
let spawnTimeout = null;
1551+
let cancellation = null;
15421552
const waitForWrapperClose = new Promise((resolveClose) => {
15431553
resolveWrapperClose = resolveClose;
15441554
});
@@ -1548,28 +1558,85 @@ async function spawnClaudeTaskIsolated(agent, context) {
15481558
resolveWrapperClose();
15491559
}
15501560
};
1561+
const findPersistedTaskId = async () => {
1562+
const persistedTaskId = await resolveIsolatedTaskIdBySpawnToken(
1563+
manager,
1564+
clusterId,
1565+
ownershipToken
1566+
);
1567+
if (persistedTaskId) {
1568+
isolatedTaskId = persistedTaskId;
1569+
assignDurableTaskId(agent, persistedTaskId);
1570+
}
1571+
return persistedTaskId;
1572+
};
1573+
const rejectLaunch = (error, { retainHandle = false } = {}) => {
1574+
if (resolved) return;
1575+
resolved = true;
1576+
clearTimeout(spawnTimeout);
1577+
if (!retainHandle && agent.currentTask === isolatedPendingLaunch) {
1578+
agent.currentTask = null;
1579+
}
1580+
reject(error);
1581+
};
1582+
15511583
proc.once('close', markWrapperClosed);
15521584
proc.once('error', markWrapperClosed);
15531585
isolatedPendingLaunch = {
15541586
pendingLaunch: true,
15551587
cancelled: false,
15561588
async kill(reason = 'Task killed') {
15571589
isolatedPendingLaunch.cancelled = true;
1558-
let termination = null;
1559-
if (isolatedTaskId) {
1560-
termination = await terminateIsolatedTask(manager, clusterId, isolatedTaskId);
1561-
if (termination?.forced === false) return termination;
1562-
}
1563-
if (!wrapperClosed) {
1564-
proc.kill('SIGKILL');
1565-
await waitForWrapperClose;
1566-
}
1567-
if (isolatedTaskId && !termination) {
1568-
termination = await terminateIsolatedTask(manager, clusterId, isolatedTaskId);
1590+
if (cancellation) return cancellation;
1591+
cancellation = (async () => {
1592+
let termination = null;
1593+
let commandError = null;
1594+
try {
1595+
const persistedTaskId = isolatedTaskId || (await findPersistedTaskId());
1596+
if (persistedTaskId) {
1597+
termination = await terminateIsolatedTask(manager, clusterId, persistedTaskId);
1598+
}
1599+
} catch (error) {
1600+
commandError = error;
1601+
}
1602+
1603+
if (!wrapperClosed) {
1604+
proc.kill('SIGKILL');
1605+
await waitForWrapperClose;
1606+
}
1607+
1608+
try {
1609+
const persistedTaskId = isolatedTaskId || (await findPersistedTaskId());
1610+
if (persistedTaskId && !termination) {
1611+
termination = await terminateIsolatedTask(manager, clusterId, persistedTaskId);
1612+
commandError = null;
1613+
}
1614+
} catch (error) {
1615+
commandError ||= error;
1616+
}
1617+
1618+
if (commandError) {
1619+
return { forced: false, reason: commandError.message };
1620+
}
1621+
agent._log?.(`Cancelled pending isolated task launch: ${reason}`);
1622+
return termination
1623+
? { ...termination, forced: true, taskId: isolatedTaskId }
1624+
: { forced: true, taskId: null };
1625+
})();
1626+
1627+
const termination = await cancellation;
1628+
if (termination?.forced === false) cancellation = null;
1629+
if (!resolved) {
1630+
const error =
1631+
timeoutError ||
1632+
new Error(
1633+
termination?.forced === false
1634+
? `Task launch cancellation was not confirmed: ${termination.reason}`
1635+
: `Task launch cancelled: ${isolatedTaskId || 'before persistence'}`
1636+
);
1637+
rejectLaunch(error, { retainHandle: termination?.forced === false });
15691638
}
1570-
if (termination?.forced === false) return termination;
1571-
agent._log?.(`Cancelled pending isolated task launch: ${reason}`);
1572-
return termination || { forced: true };
1639+
return termination;
15731640
},
15741641
};
15751642
agent.currentTask = isolatedPendingLaunch;
@@ -1578,21 +1645,17 @@ async function spawnClaudeTaskIsolated(agent, context) {
15781645
agent.processPid = proc.pid;
15791646
agent._publishLifecycle('PROCESS_SPAWNED', { pid: proc.pid });
15801647

1581-
let stdout = '';
1582-
let stderr = '';
1583-
let resolved = false;
1584-
1585-
// CRITICAL: Timeout to prevent infinite hang if provider CLI hangs
1586-
const spawnTimeout = setTimeout(() => {
1648+
// CRITICAL: Timeout to prevent infinite hang if provider CLI hangs. Timeout
1649+
// uses the same cancellation path so a durable child cannot outlive it.
1650+
spawnTimeout = setTimeout(() => {
15871651
if (resolved) return;
1588-
resolved = true;
1589-
proc.kill('SIGKILL');
1590-
reject(
1591-
new Error(
1592-
`Spawn timeout after ${SPAWN_TIMEOUT_MS / 1000}s - provider CLI hung. ` +
1593-
`stdout: ${stdout.slice(-500)}, stderr: ${stderr.slice(-500)}`
1594-
)
1652+
timeoutError = new Error(
1653+
`Spawn timeout after ${SPAWN_TIMEOUT_MS / 1000}s - provider CLI hung. ` +
1654+
`stdout: ${stdout.slice(-500)}, stderr: ${stderr.slice(-500)}`
15951655
);
1656+
isolatedPendingLaunch.kill(timeoutError.message).catch((error) => {
1657+
rejectLaunch(error, { retainHandle: true });
1658+
});
15961659
}, SPAWN_TIMEOUT_MS);
15971660

15981661
proc.stdout.on('data', (data) => {
@@ -1603,40 +1666,41 @@ async function spawnClaudeTaskIsolated(agent, context) {
16031666
stderr += data.toString();
16041667
});
16051668

1606-
proc.on('close', (code, signal) => {
1669+
proc.on('close', async (code, signal) => {
16071670
clearTimeout(spawnTimeout);
1608-
if (resolved) return;
1609-
resolved = true;
1610-
// Handle process killed by signal
1671+
if (resolved || isolatedPendingLaunch.cancelled) return;
16111672
if (signal) {
1612-
reject(new Error(`Process killed by signal ${signal}${stderr ? `: ${stderr}` : ''}`));
1673+
await isolatedPendingLaunch.kill(
1674+
`Process killed by signal ${signal}${stderr ? `: ${stderr}` : ''}`
1675+
);
16131676
return;
16141677
}
16151678

1616-
if (code === 0) {
1617-
// Parse task ID from output: "✓ Task spawned: xxx-yyy-nn"
1618-
const spawnedTaskId = parseTaskIdFromOutput(stdout);
1619-
if (spawnedTaskId) {
1620-
isolatedTaskId = spawnedTaskId;
1621-
assignDurableTaskId(agent, spawnedTaskId);
1622-
1623-
resolve(spawnedTaskId);
1624-
} else {
1625-
reject(new Error(`Could not parse task ID from output: ${stdout}`));
1626-
}
1627-
} else {
1628-
reject(new Error(`zeroshot task run failed with code ${code}: ${stderr}`));
1679+
try {
1680+
const persistedTaskId = await findPersistedTaskId();
1681+
const spawnedTaskId = requireTaskIdFromWrapperResult({
1682+
code,
1683+
stdout,
1684+
stderr,
1685+
parseTaskId: parseTaskIdFromOutput,
1686+
persistedTaskId,
1687+
});
1688+
isolatedTaskId = spawnedTaskId;
1689+
assignDurableTaskId(agent, spawnedTaskId);
1690+
resolved = true;
1691+
resolve(spawnedTaskId);
1692+
} catch (error) {
1693+
rejectLaunch(error, { retainHandle: Boolean(isolatedTaskId) });
16291694
}
16301695
});
16311696

1632-
proc.on('error', (error) => {
1633-
if (!isolatedTaskId && agent.currentTask === isolatedPendingLaunch) {
1634-
agent.currentTask = null;
1635-
}
1697+
proc.on('error', async (error) => {
16361698
clearTimeout(spawnTimeout);
1637-
if (resolved) return;
1638-
resolved = true;
1639-
reject(error);
1699+
if (resolved || isolatedPendingLaunch.cancelled) return;
1700+
const termination = await isolatedPendingLaunch.kill(error.message);
1701+
if (termination?.forced === false && !resolved) {
1702+
rejectLaunch(error, { retainHandle: true });
1703+
}
16401704
});
16411705
});
16421706
if (isolatedPendingLaunch?.cancelled) throw new Error(`Task launch cancelled: ${taskId}`);
@@ -1738,29 +1802,52 @@ function rejectIsolatedFollower({ agent, state, cleanup, reject, error }) {
17381802
reject(error);
17391803
}
17401804

1805+
async function resolveIsolatedTaskIdBySpawnToken(manager, clusterId, ownershipToken) {
1806+
const result = await manager.execInContainer(clusterId, [
1807+
'zeroshot',
1808+
'get-task-id-by-spawn-token',
1809+
ownershipToken,
1810+
]);
1811+
if (result.code === 2) return null;
1812+
if (result.code !== 0) {
1813+
throw new Error(
1814+
`Failed to resolve isolated task ownership: ${result.stderr || result.stdout || `exit ${result.code}`}`
1815+
);
1816+
}
1817+
const taskId = result.stdout.trim();
1818+
if (!taskId) {
1819+
throw new Error('Isolated task ownership lookup returned an empty task ID');
1820+
}
1821+
return taskId;
1822+
}
1823+
17411824
function parseIsolatedStatus(output) {
17421825
return output.match(/Status:\s+(completed|failed|killed|stale|cancelled)/i)?.[1].toLowerCase();
17431826
}
17441827

17451828
async function terminateIsolatedTask(manager, clusterId, taskId) {
17461829
const before = await manager.execInContainer(clusterId, ['zeroshot', 'status', taskId]);
17471830
const beforeStatus = before.code === 0 ? parseIsolatedStatus(before.stdout) : null;
1748-
if (beforeStatus) {
1749-
return { alreadyTerminal: true, forced: false, status: beforeStatus };
1831+
const result = await manager.execInContainer(clusterId, ['zeroshot', 'kill', taskId]);
1832+
if (result.code !== 0) {
1833+
throw new Error(
1834+
`Failed to terminate isolated task ${taskId}: ${result.stderr || result.stdout || `exit ${result.code}`}`
1835+
);
17501836
}
17511837

1752-
const result = await manager.execInContainer(clusterId, ['zeroshot', 'kill', taskId]);
17531838
const status = await manager.execInContainer(clusterId, ['zeroshot', 'status', taskId]);
17541839
const afterStatus = status.code === 0 ? parseIsolatedStatus(status.stdout) : null;
17551840
if (!afterStatus) {
17561841
throw new Error(
1757-
`Failed to terminate isolated task ${taskId}: ${result.stderr || result.stdout || `exit ${result.code}`}`
1842+
`Failed to confirm isolated task ${taskId} after cleanup recovery: ${
1843+
status.stderr || status.stdout || `exit ${status.code}`
1844+
}`
17581845
);
17591846
}
17601847

17611848
return {
1762-
alreadyTerminal: false,
1763-
forced: afterStatus === 'killed',
1849+
alreadyTerminal: Boolean(beforeStatus),
1850+
forced: !beforeStatus && afterStatus === 'killed',
17641851
status: afterStatus,
17651852
};
17661853
}
@@ -2377,24 +2464,28 @@ async function killTask(agent, termination = 'Task killed') {
23772464

23782465
async function killIsolatedTask(agent, currentTask, taskId, reason, code) {
23792466
let termination;
2380-
if (currentTask && typeof currentTask.terminate === 'function') {
2381-
termination = await currentTask.terminate(reason, { code });
2382-
} else {
2383-
termination = await terminateIsolatedTask(
2384-
agent.isolation.manager,
2385-
agent.isolation.clusterId,
2386-
taskId
2387-
);
2388-
if (currentTask && typeof currentTask.kill === 'function') {
2389-
currentTask.kill(reason, { code });
2467+
try {
2468+
if (currentTask && typeof currentTask.terminate === 'function') {
2469+
termination = await currentTask.terminate(reason, { code });
2470+
} else {
2471+
termination = await terminateIsolatedTask(
2472+
agent.isolation.manager,
2473+
agent.isolation.clusterId,
2474+
taskId
2475+
);
2476+
if (currentTask && typeof currentTask.kill === 'function') {
2477+
currentTask.kill(reason, { code });
2478+
}
23902479
}
2480+
} catch (error) {
2481+
return { forced: false, reason: error.message };
23912482
}
23922483

2393-
agent._stopLivenessCheck?.();
2394-
if (termination?.forced === false) {
2484+
if (termination?.forced === false && !termination.alreadyTerminal) {
23952485
return termination;
23962486
}
23972487

2488+
agent._stopLivenessCheck?.();
23982489
agent.currentTask = null;
23992490
agent.currentTaskId = null;
24002491
agent.processPid = null;

0 commit comments

Comments
 (0)