fix: reduce opencode session list churn (#1478)
* fix: reduce opencode session list churn * fix: make bun-tmp-janitor cross-platform and move to process-level boot - Extend platform support from Linux-only to Linux + macOS (win32 skipped since opencode ships no Windows binary and the kernel disallows unlinking mapped files there) - Use os.tmpdir() instead of hardcoded /tmp to handle macOS $TMPDIR paths - Extend file pattern from \.so to \.(so|dylib) to cover macOS dylib leaks - Move startBunTmpJanitor() from ensureLifecycleWorker() (per-project) to the process-level boot in registerStart() immediately after register(), where the single-instance contract is already in force - Drop the project-observer-bound onSweep closure that incorrectly attributed janitor health to whichever project happened to start first; replaced with a simple stderr warn on errors (no project context needed for a process-wide sweep of /tmp) - Move stopBunTmpJanitor() into the SIGINT/SIGTERM shutdown handler in registerStart() alongside stopAllLifecycleWorkers() - Remove startBunTmpJanitor/stopBunTmpJanitor from lifecycle-service.ts entirely; lifecycle workers have no business knowing about a process-wide OS resource * fix(opencode): address PR #1478 review (TMPDIR isolation, shared cache, janitor cleanup) Implements all seven findings from the PR #1478 review: Core / agent-opencode: - New @aoagents/ao-core/opencode-shared module owns the single TTL cache + in-flight dedup for 'opencode session list' (was duplicated across core and the plugin, doubling spawns per poll cycle). - TTL dropped from 3s to 500ms so the send-confirmation loop's updatedAt > baselineUpdatedAt delivery signal can actually fire. - New invalidateOpenCodeSessionListCache() called by deleteOpenCodeSession so reuse / remap / restore code paths cannot observe a deleted id. - New getOpenCodeChildEnv() / getOpenCodeTmpDir(): every opencode child spawned by core, the plugin, or the agent runtime points TMPDIR/TMP/TEMP at ~/.agent-orchestrator/.bun-tmp. Bounds the janitor's blast radius to AO-owned files even on shared hosts. CLI janitor: - Sweeps only the AO-owned tmp dir (not the system /tmp). - Filters synchronously before spawning per-entry stat/unlink work. - stopBunTmpJanitor() is now async and awaits any in-flight sweep so SIGTERM cannot exit while unlink() is mid-flight; start.ts shutdown handler awaits it. - onSweep callback in start.ts now logs successful reclaims, not just errors, so operators can confirm the janitor is doing useful work. Tests: - packages/core/__tests__/opencode-shared.test.ts (TTL contract, TMPDIR location, env merge semantics). - packages/cli/__tests__/lib/bun-tmp-janitor.test.ts (sweep behavior, stop-awaits-in-flight, pattern matching, missing-dir tolerance). * chore: remove review postmortem artifact * fix(cli): remove start command non-null assertions
This commit is contained in:
Madhav Kumar
GitHub
parent
5f671ee3c7
commit
4701122342
|
|
@ -0,0 +1,37 @@
|
|||
---
|
||||
"@aoagents/ao-core": patch
|
||||
"@aoagents/ao-cli": patch
|
||||
"@aoagents/ao-plugin-agent-opencode": patch
|
||||
---
|
||||
|
||||
opencode: bound /tmp blast radius and consolidate session-list cache
|
||||
|
||||
Addresses review feedback on PR #1478:
|
||||
|
||||
- **TMPDIR isolation.** Every `opencode` child we spawn now points at
|
||||
`~/.agent-orchestrator/.bun-tmp/` via `TMPDIR`/`TMP`/`TEMP`. Bun's
|
||||
embedded shared-library extraction lands there instead of the system
|
||||
`/tmp`, so the cli janitor only ever sweeps AO-owned files. Other
|
||||
users' or other applications' Bun artifacts on a shared host can no
|
||||
longer be touched by the regex.
|
||||
- **Single shared session-list cache.** Core and the agent-opencode
|
||||
plugin previously kept independent caches; per poll cycle the system
|
||||
spawned at least two `opencode session list` processes instead of
|
||||
one. Both consumers now use the shared cache exported from
|
||||
`@aoagents/ao-core` (`getCachedOpenCodeSessionList`).
|
||||
- **TTL no longer covers the send-confirmation loop.** The cache TTL
|
||||
dropped from 3s to 500ms so the
|
||||
`updatedAt > baselineUpdatedAt` delivery signal in
|
||||
`sendWithConfirmation` actually fires. Concurrent callers still
|
||||
share the in-flight promise.
|
||||
- **Delete invalidates the cache.** `deleteOpenCodeSession` now calls
|
||||
`invalidateOpenCodeSessionListCache()` on success so reuse, remap,
|
||||
and restore code paths cannot observe a deleted session id within
|
||||
the TTL window.
|
||||
- **Janitor reliability.** `sweepOnce` now filters synchronously
|
||||
before allocating per-file promises (matters on hosts with thousands
|
||||
of `/tmp` entries), and `stopBunTmpJanitor()` is now async and awaits
|
||||
any in-flight sweep so SIGTERM cannot exit while `unlink` is mid-flight.
|
||||
- **Janitor observability.** The sweep callback in `ao start` now logs
|
||||
successful reclaims, not just errors, so operators can confirm the
|
||||
janitor is doing useful work.
|
||||
|
|
@ -0,0 +1,122 @@
|
|||
import { describe, expect, it, beforeEach, afterEach, vi } from "vitest";
|
||||
import { mkdtempSync, writeFileSync, existsSync, statSync, utimesSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
|
||||
import type * as AoCore from "@aoagents/ao-core";
|
||||
|
||||
// We point the shared module at a fresh temp dir per test by mocking the
|
||||
// AO base path resolver. The janitor pulls the dir from
|
||||
// @aoagents/ao-core's getOpenCodeTmpDir.
|
||||
let mockedDir = "";
|
||||
vi.mock("@aoagents/ao-core", async () => {
|
||||
const actual = await vi.importActual<typeof AoCore>("@aoagents/ao-core");
|
||||
return {
|
||||
...actual,
|
||||
getOpenCodeTmpDir: () => mockedDir,
|
||||
};
|
||||
});
|
||||
|
||||
import {
|
||||
isBunTmpJanitorRunning,
|
||||
startBunTmpJanitor,
|
||||
stopBunTmpJanitor,
|
||||
} from "../../src/lib/bun-tmp-janitor.js";
|
||||
|
||||
const PATTERN_NAME = ".fcb8efb7fbaad77d-00000000.so";
|
||||
|
||||
function setMtime(path: string, ageMs: number): void {
|
||||
const t = (Date.now() - ageMs) / 1000;
|
||||
utimesSync(path, t, t);
|
||||
}
|
||||
|
||||
describe("bun-tmp-janitor", () => {
|
||||
beforeEach(() => {
|
||||
mockedDir = mkdtempSync(join(tmpdir(), "ao-bun-janitor-test-"));
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await stopBunTmpJanitor();
|
||||
});
|
||||
|
||||
it("sweeps matching files older than ageMs in the AO-owned dir only", async () => {
|
||||
const oldFile = join(mockedDir, PATTERN_NAME);
|
||||
const youngFile = join(mockedDir, ".aaaaaaaa-bbbbbbbb.so");
|
||||
const unrelated = join(mockedDir, "regular-file.txt");
|
||||
writeFileSync(oldFile, "x".repeat(1024));
|
||||
writeFileSync(youngFile, "x");
|
||||
writeFileSync(unrelated, "x");
|
||||
setMtime(oldFile, 120_000); // 2 minutes old
|
||||
|
||||
const sweeps: { removed: number; freedBytes: number; errors: number }[] = [];
|
||||
startBunTmpJanitor({
|
||||
intervalMs: 60_000,
|
||||
ageMs: 60_000,
|
||||
onSweep: (r) => sweeps.push(r),
|
||||
});
|
||||
|
||||
// Wait for the immediate sweep to complete.
|
||||
await stopBunTmpJanitor();
|
||||
|
||||
expect(sweeps.length).toBe(1);
|
||||
expect(sweeps[0]?.removed).toBe(1);
|
||||
expect(sweeps[0]?.freedBytes).toBe(1024);
|
||||
expect(existsSync(oldFile)).toBe(false);
|
||||
expect(existsSync(youngFile)).toBe(true);
|
||||
expect(existsSync(unrelated)).toBe(true);
|
||||
});
|
||||
|
||||
it("treats a missing AO tmp dir as empty (no error)", async () => {
|
||||
mockedDir = join(tmpdir(), `ao-bun-janitor-missing-${Date.now()}`);
|
||||
expect(existsSync(mockedDir)).toBe(false);
|
||||
|
||||
const sweeps: { removed: number; freedBytes: number; errors: number }[] = [];
|
||||
startBunTmpJanitor({ onSweep: (r) => sweeps.push(r) });
|
||||
await stopBunTmpJanitor();
|
||||
|
||||
// No callback fires when removed=0 and errors=0.
|
||||
expect(sweeps).toEqual([]);
|
||||
});
|
||||
|
||||
it("stopBunTmpJanitor awaits the in-flight sweep", async () => {
|
||||
// Drop a matching file so the immediate sweep does real work.
|
||||
const path = join(mockedDir, PATTERN_NAME);
|
||||
writeFileSync(path, "x");
|
||||
setMtime(path, 120_000);
|
||||
|
||||
startBunTmpJanitor({ ageMs: 60_000 });
|
||||
expect(isBunTmpJanitorRunning()).toBe(true);
|
||||
|
||||
await stopBunTmpJanitor();
|
||||
|
||||
// After awaiting stop, the sweep must have run to completion: the
|
||||
// file must be gone, and the timer cleared.
|
||||
expect(isBunTmpJanitorRunning()).toBe(false);
|
||||
expect(existsSync(path)).toBe(false);
|
||||
});
|
||||
|
||||
it("does not double-start", async () => {
|
||||
const a = startBunTmpJanitor();
|
||||
const b = startBunTmpJanitor();
|
||||
expect(a).toBe(true);
|
||||
expect(b).toBe(false);
|
||||
await stopBunTmpJanitor();
|
||||
});
|
||||
|
||||
it("ignores files that do not match the Bun tmp library pattern", async () => {
|
||||
const matching = join(mockedDir, ".1234567890abcdef-12345678.dylib");
|
||||
const nonMatching = join(mockedDir, "libopentui.so");
|
||||
writeFileSync(matching, "x");
|
||||
writeFileSync(nonMatching, "x");
|
||||
setMtime(matching, 120_000);
|
||||
setMtime(nonMatching, 120_000);
|
||||
|
||||
startBunTmpJanitor({ ageMs: 60_000 });
|
||||
await stopBunTmpJanitor();
|
||||
|
||||
expect(existsSync(matching)).toBe(false);
|
||||
expect(existsSync(nonMatching)).toBe(true);
|
||||
// Sanity: stat works (file exists), proves we did not unlink it.
|
||||
expect(statSync(nonMatching).size).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
|
@ -48,6 +48,7 @@ import { parse as yamlParse, stringify as yamlStringify } from "yaml";
|
|||
import { exec, execSilent, git } from "../lib/shell.js";
|
||||
import { getSessionManager } from "../lib/create-session-manager.js";
|
||||
import { ensureLifecycleWorker, stopAllLifecycleWorkers } from "../lib/lifecycle-service.js";
|
||||
import { startBunTmpJanitor, stopBunTmpJanitor } from "../lib/bun-tmp-janitor.js";
|
||||
import {
|
||||
findWebDir,
|
||||
buildDashboardEnv,
|
||||
|
|
@ -1636,6 +1637,10 @@ export function registerStart(program: Command): void {
|
|||
// Non-TTY callers (scripts/agents) keep the old "AO is already
|
||||
// running" message and do NOT mutate config behind the user's back.
|
||||
if (running && projectArgIsUrlOrPath && isHumanCaller()) {
|
||||
const requestedProjectArg = projectArg;
|
||||
if (!requestedProjectArg) {
|
||||
throw new Error("Expected project path or URL argument");
|
||||
}
|
||||
// Always register against the GLOBAL config — never the cwd's
|
||||
// local config. Cross-project visibility lives in the global
|
||||
// registry, and addProjectToConfig only routes to global when
|
||||
|
|
@ -1646,9 +1651,9 @@ export function registerStart(program: Command): void {
|
|||
: loadConfig();
|
||||
|
||||
let existingId: string | null = null;
|
||||
if (isRepoUrl(projectArg!)) {
|
||||
if (isRepoUrl(requestedProjectArg)) {
|
||||
try {
|
||||
const parsed = parseRepoUrl(projectArg!);
|
||||
const parsed = parseRepoUrl(requestedProjectArg);
|
||||
for (const [id, p] of Object.entries(globalCfg.projects)) {
|
||||
if (p.repo === parsed.ownerRepo) {
|
||||
existingId = id;
|
||||
|
|
@ -1660,7 +1665,7 @@ export function registerStart(program: Command): void {
|
|||
}
|
||||
} else {
|
||||
const resolvedPath = resolve(
|
||||
projectArg!.replace(/^~/, process.env["HOME"] || ""),
|
||||
requestedProjectArg.replace(/^~/, process.env["HOME"] || ""),
|
||||
);
|
||||
let canonicalTarget: string;
|
||||
try {
|
||||
|
|
@ -1697,14 +1702,14 @@ export function registerStart(program: Command): void {
|
|||
let resolvedId: string;
|
||||
if (existingId) {
|
||||
resolvedId = existingId;
|
||||
} else if (isRepoUrl(projectArg!)) {
|
||||
} else if (isRepoUrl(requestedProjectArg)) {
|
||||
// Clone + register inline. We DO NOT call handleUrlStart —
|
||||
// that helper writes a legacy wrapped (`projects:`) local
|
||||
// config that the new resolver rejects, requiring a repair
|
||||
// pass after the fact. Instead, we write a flat local config
|
||||
// here so the global registry + repo can be loaded cleanly
|
||||
// on the very first read.
|
||||
const parsed = parseRepoUrl(projectArg!);
|
||||
const parsed = parseRepoUrl(requestedProjectArg);
|
||||
console.log(
|
||||
chalk.bold.cyan(`\n Cloning ${parsed.ownerRepo} (${parsed.host})\n`),
|
||||
);
|
||||
|
|
@ -1736,7 +1741,7 @@ export function registerStart(program: Command): void {
|
|||
throw new Error(
|
||||
`Repository "${parsed.ownerRepo}" appears to be empty (no commits or refs).\n` +
|
||||
` AO needs at least one commit on the default branch to spawn an orchestrator.\n` +
|
||||
` Push an initial commit, then re-run \`ao start ${projectArg}\`.`,
|
||||
` Push an initial commit, then re-run \`ao start ${requestedProjectArg}\`.`,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -1775,7 +1780,7 @@ export function registerStart(program: Command): void {
|
|||
}
|
||||
} else {
|
||||
const resolvedPath = resolve(
|
||||
projectArg!.replace(/^~/, process.env["HOME"] || ""),
|
||||
requestedProjectArg.replace(/^~/, process.env["HOME"] || ""),
|
||||
);
|
||||
resolvedId = await addProjectToConfig(globalCfg, resolvedPath);
|
||||
}
|
||||
|
|
@ -2221,6 +2226,25 @@ export function registerStart(program: Command): void {
|
|||
});
|
||||
unlockStartup();
|
||||
|
||||
// Start the Bun-extracted /tmp/.*.{so,dylib} janitor once per AO
|
||||
// process. Single-instance is enforced by running.json + the
|
||||
// startup lock above, so this call site is reached at most once
|
||||
// per process. The janitor uses an unref'd interval timer, so it
|
||||
// does not keep the event loop alive on its own and dies with the
|
||||
// process on SIGTERM/SIGINT.
|
||||
startBunTmpJanitor({
|
||||
onSweep: ({ removed, freedBytes, errors }) => {
|
||||
if (removed > 0) {
|
||||
console.info(
|
||||
`[bun-tmp-janitor] reclaimed ${removed} file(s) / ${freedBytes} bytes`,
|
||||
);
|
||||
}
|
||||
if (errors > 0) {
|
||||
console.warn(`[bun-tmp-janitor] sweep had ${errors} error(s)`);
|
||||
}
|
||||
},
|
||||
});
|
||||
|
||||
// Install shutdown handlers so Ctrl+C and `ao stop` (which sends
|
||||
// SIGTERM) perform a full graceful shutdown: kill sessions, record
|
||||
// last-stop state for restore, unregister, then exit.
|
||||
|
|
@ -2290,11 +2314,22 @@ export function registerStart(program: Command): void {
|
|||
} catch {
|
||||
// Best-effort — always exit even if cleanup fails
|
||||
}
|
||||
try {
|
||||
// Await any in-flight sweep so shutdown does not exit while
|
||||
// unlink() calls are still mid-flight against the filesystem.
|
||||
await stopBunTmpJanitor();
|
||||
} catch {
|
||||
// Best-effort cleanup.
|
||||
}
|
||||
process.exit(exitCode);
|
||||
})();
|
||||
};
|
||||
process.once("SIGINT", shutdown);
|
||||
process.once("SIGTERM", shutdown);
|
||||
process.once("SIGINT", (sig) => {
|
||||
void shutdown(sig);
|
||||
});
|
||||
process.once("SIGTERM", (sig) => {
|
||||
void shutdown(sig);
|
||||
});
|
||||
} catch (err) {
|
||||
if (err instanceof Error) {
|
||||
console.error(chalk.red("\nError:"), err.message);
|
||||
|
|
|
|||
|
|
@ -0,0 +1,145 @@
|
|||
import { readdir, stat, unlink } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { getOpenCodeTmpDir } from "@aoagents/ao-core";
|
||||
|
||||
// Bun-bundled binaries (opencode, etc.) extract embedded shared libraries to
|
||||
// `TMPDIR` on startup and never unlink them on exit — this is a known upstream
|
||||
// Bun bug that leaks ~4.3 MB per process invocation. Files look like
|
||||
// `.{16hex}-{8hex}.{so|dylib}` (e.g. `.fcb8efb7fbaad77d-00000000.so`).
|
||||
//
|
||||
// We point every `opencode` child we spawn at an AO-owned subdirectory via
|
||||
// `TMPDIR` (see `getOpenCodeChildEnv` in `@aoagents/ao-core`). The janitor
|
||||
// then sweeps **only that directory**, which keeps the blast radius bounded:
|
||||
// no other user's or other application's Bun artifacts can ever be touched
|
||||
// even if AO runs as root on a shared host.
|
||||
//
|
||||
// Deleting these files is safe even while a live process has them mmap'd: on
|
||||
// POSIX systems, `unlink` removes the directory entry but the kernel keeps
|
||||
// the inode alive until the last mapping is torn down, at which point the
|
||||
// space is reclaimed. For already-exited processes the unlink frees disk
|
||||
// immediately. Windows does not allow unlinking mapped files, and opencode
|
||||
// does not ship a Windows binary, so the janitor is a no-op there.
|
||||
//
|
||||
// This janitor runs once per `ao start` process and sweeps matching files
|
||||
// older than `ageMs` at every interval.
|
||||
|
||||
const BUN_TMP_LIB_PATTERN = /^\.[0-9a-f]{8,}-[0-9a-f]{6,}\.(so|dylib)$/i;
|
||||
const DEFAULT_INTERVAL_MS = 60_000;
|
||||
const DEFAULT_AGE_MS = 60_000;
|
||||
|
||||
export interface BunTmpJanitorOptions {
|
||||
intervalMs?: number;
|
||||
ageMs?: number;
|
||||
onSweep?: (result: { removed: number; freedBytes: number; errors: number }) => void;
|
||||
}
|
||||
|
||||
let timer: NodeJS.Timeout | null = null;
|
||||
let inFlightTick: Promise<void> | null = null;
|
||||
|
||||
async function sweepOnce(
|
||||
dir: string,
|
||||
ageMs: number,
|
||||
): Promise<{ removed: number; freedBytes: number; errors: number }> {
|
||||
let removed = 0;
|
||||
let freedBytes = 0;
|
||||
let errors = 0;
|
||||
|
||||
let entries: string[];
|
||||
try {
|
||||
entries = await readdir(dir);
|
||||
} catch {
|
||||
// Directory may not exist yet (no opencode child has run). That is not
|
||||
// an error condition — there is nothing to sweep.
|
||||
return { removed, freedBytes, errors: 0 };
|
||||
}
|
||||
|
||||
// Filter synchronously *before* spawning per-entry stat/unlink work. On a
|
||||
// host with thousands of /tmp entries this avoids allocating one promise
|
||||
// per file we are about to discard. (Belt-and-suspenders: TMPDIR isolation
|
||||
// already bounds the directory contents to AO's own children.)
|
||||
const matches = entries.filter((name) => BUN_TMP_LIB_PATTERN.test(name));
|
||||
if (matches.length === 0) {
|
||||
return { removed, freedBytes, errors: 0 };
|
||||
}
|
||||
|
||||
const cutoff = Date.now() - ageMs;
|
||||
|
||||
await Promise.all(
|
||||
matches.map(async (name) => {
|
||||
const path = join(dir, name);
|
||||
try {
|
||||
const st = await stat(path);
|
||||
if (!st.isFile() || st.mtimeMs > cutoff) return;
|
||||
await unlink(path);
|
||||
removed += 1;
|
||||
freedBytes += st.size;
|
||||
} catch {
|
||||
// File may have been deleted by another sweeper, or stat raced
|
||||
// with an unlink, or we lack permission. Best-effort — don't throw.
|
||||
errors += 1;
|
||||
}
|
||||
}),
|
||||
);
|
||||
|
||||
return { removed, freedBytes, errors };
|
||||
}
|
||||
|
||||
export function startBunTmpJanitor(options: BunTmpJanitorOptions = {}): boolean {
|
||||
// Windows: opencode ships no win32 binary and unlinking mapped files is
|
||||
// disallowed by the kernel, so the janitor would be both unnecessary and
|
||||
// potentially error-prone. Skip.
|
||||
if (process.platform === "win32") return false;
|
||||
if (timer) return false;
|
||||
|
||||
const intervalMs = options.intervalMs ?? DEFAULT_INTERVAL_MS;
|
||||
const ageMs = options.ageMs ?? DEFAULT_AGE_MS;
|
||||
const { onSweep } = options;
|
||||
const dir = getOpenCodeTmpDir();
|
||||
|
||||
const tick = async (): Promise<void> => {
|
||||
// Single-flight: if a previous tick is still running, skip this one.
|
||||
if (inFlightTick) return;
|
||||
const promise = (async () => {
|
||||
try {
|
||||
const result = await sweepOnce(dir, ageMs);
|
||||
if (onSweep && (result.removed > 0 || result.errors > 0)) {
|
||||
onSweep(result);
|
||||
}
|
||||
} finally {
|
||||
inFlightTick = null;
|
||||
}
|
||||
})();
|
||||
inFlightTick = promise;
|
||||
await promise;
|
||||
};
|
||||
|
||||
// Run an immediate sweep to clear any backlog, then on an interval.
|
||||
void tick();
|
||||
timer = setInterval(() => void tick(), intervalMs);
|
||||
timer.unref();
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Stop the janitor and await any sweep currently in flight. The shutdown
|
||||
* handler in `start.ts` awaits this so the process never exits while
|
||||
* `unlink` calls are still mid-flight against the filesystem.
|
||||
*/
|
||||
export async function stopBunTmpJanitor(): Promise<void> {
|
||||
if (timer) {
|
||||
clearInterval(timer);
|
||||
timer = null;
|
||||
}
|
||||
if (inFlightTick) {
|
||||
try {
|
||||
await inFlightTick;
|
||||
} catch {
|
||||
// Best-effort — don't block shutdown on an in-flight sweep error.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function isBunTmpJanitorRunning(): boolean {
|
||||
return timer !== null;
|
||||
}
|
||||
|
|
@ -0,0 +1,68 @@
|
|||
import { describe, expect, it, beforeEach } from "vitest";
|
||||
|
||||
import {
|
||||
ensureOpenCodeTmpDir,
|
||||
getOpenCodeChildEnv,
|
||||
getOpenCodeTmpDir,
|
||||
invalidateOpenCodeSessionListCache,
|
||||
resetOpenCodeSessionListCache,
|
||||
OPENCODE_SESSION_LIST_CACHE_TTL_MS,
|
||||
} from "../opencode-shared.js";
|
||||
|
||||
describe("opencode-shared", () => {
|
||||
beforeEach(() => {
|
||||
resetOpenCodeSessionListCache();
|
||||
});
|
||||
|
||||
describe("OPENCODE_SESSION_LIST_CACHE_TTL_MS", () => {
|
||||
it("does not cover the 500ms send-confirmation poll interval", () => {
|
||||
// The send-confirmation loop in session-manager polls 6× at 500ms
|
||||
// intervals (~3s total). The TTL must be ≤500ms so each loop
|
||||
// iteration sees fresh data and the
|
||||
// `updatedAt > baselineUpdatedAt` delivery signal can fire.
|
||||
expect(OPENCODE_SESSION_LIST_CACHE_TTL_MS).toBeLessThanOrEqual(500);
|
||||
expect(OPENCODE_SESSION_LIST_CACHE_TTL_MS).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe("getOpenCodeTmpDir / ensureOpenCodeTmpDir", () => {
|
||||
it("lives under the AO base dir, not the system /tmp", () => {
|
||||
const dir = getOpenCodeTmpDir();
|
||||
expect(dir).toMatch(/\.agent-orchestrator[\\/]\.bun-tmp$/);
|
||||
expect(dir.startsWith("/tmp")).toBe(false);
|
||||
});
|
||||
|
||||
it("creates the directory and tolerates pre-existing", () => {
|
||||
const a = ensureOpenCodeTmpDir();
|
||||
const b = ensureOpenCodeTmpDir();
|
||||
expect(a).toBe(b);
|
||||
});
|
||||
});
|
||||
|
||||
describe("getOpenCodeChildEnv", () => {
|
||||
it("sets TMPDIR/TMP/TEMP to the AO-owned dir", () => {
|
||||
const env = getOpenCodeChildEnv();
|
||||
const dir = getOpenCodeTmpDir();
|
||||
expect(env["TMPDIR"]).toBe(dir);
|
||||
expect(env["TMP"]).toBe(dir);
|
||||
expect(env["TEMP"]).toBe(dir);
|
||||
});
|
||||
|
||||
it("merges extra env vars on top, but keeps TMPDIR pointed at AO dir", () => {
|
||||
const env = getOpenCodeChildEnv({ FOO: "bar" });
|
||||
expect(env["FOO"]).toBe("bar");
|
||||
expect(env["TMPDIR"]).toBe(getOpenCodeTmpDir());
|
||||
});
|
||||
|
||||
it("allows extra env to override TMPDIR for explicit opt-out", () => {
|
||||
const env = getOpenCodeChildEnv({ TMPDIR: "/elsewhere" });
|
||||
expect(env["TMPDIR"]).toBe("/elsewhere");
|
||||
});
|
||||
});
|
||||
|
||||
describe("invalidateOpenCodeSessionListCache", () => {
|
||||
it("is a no-throw on an empty cache", () => {
|
||||
expect(() => invalidateOpenCodeSessionListCache()).not.toThrow();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -4,6 +4,7 @@ import { join } from "node:path";
|
|||
import { tmpdir } from "node:os";
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { getProjectSessionsDir, getProjectDir } from "../paths.js";
|
||||
import { resetOpenCodeSessionListCache } from "../session-manager.js";
|
||||
import { createInitialCanonicalLifecycle, deriveLegacyStatus } from "../lifecycle-state.js";
|
||||
import { createActivitySignal } from "../activity-signal.js";
|
||||
import type {
|
||||
|
|
@ -364,6 +365,7 @@ export interface TestContext {
|
|||
}
|
||||
|
||||
export function setupTestContext(): TestContext {
|
||||
resetOpenCodeSessionListCache();
|
||||
const originalPath = process.env.PATH;
|
||||
const originalHome = process.env["HOME"];
|
||||
const tmpDir = join(tmpdir(), `ao-test-session-mgr-${randomUUID()}`);
|
||||
|
|
|
|||
|
|
@ -151,6 +151,16 @@ export {
|
|||
parseWebhookBranchRef,
|
||||
} from "./scm-webhook-utils.js";
|
||||
export { asValidOpenCodeSessionId } from "./opencode-session-id.js";
|
||||
export {
|
||||
OPENCODE_SESSION_LIST_CACHE_TTL_MS,
|
||||
getOpenCodeTmpDir,
|
||||
ensureOpenCodeTmpDir,
|
||||
getOpenCodeChildEnv,
|
||||
getCachedOpenCodeSessionList,
|
||||
invalidateOpenCodeSessionListCache,
|
||||
resetOpenCodeSessionListCache,
|
||||
} from "./opencode-shared.js";
|
||||
export type { OpenCodeSessionListEntry } from "./opencode-shared.js";
|
||||
export {
|
||||
getWorkspaceAgentsMdPath,
|
||||
writeWorkspaceOpenCodeAgentsMd,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,235 @@
|
|||
/**
|
||||
* Shared opencode-child plumbing.
|
||||
*
|
||||
* This module exists for two reasons:
|
||||
*
|
||||
* 1. **Bounded /tmp blast radius** (issue #1046, PR #1478 review).
|
||||
* Bun-bundled binaries leak `.so`/`.dylib` files into the system temp
|
||||
* directory and never unlink them. Rather than sweeping all of `/tmp`
|
||||
* with a regex (which would touch other users' or other apps' Bun
|
||||
* artifacts), we point every `opencode` child at an AO-owned temp dir
|
||||
* via `TMPDIR`. The cli-side janitor then sweeps only that directory.
|
||||
*
|
||||
* 2. **Single shared cache for `opencode session list`.**
|
||||
* Both `@aoagents/ao-core` and the `agent-opencode` plugin previously
|
||||
* kept independent module-level caches. Per poll cycle the system
|
||||
* therefore spawned at least two `opencode session list` processes
|
||||
* instead of one. A single cache exported from core collapses them.
|
||||
*/
|
||||
|
||||
import { execFile } from "node:child_process";
|
||||
import { mkdirSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
import { promisify } from "node:util";
|
||||
|
||||
import { getAoBaseDir } from "./paths.js";
|
||||
import { safeJsonParse } from "./utils/validation.js";
|
||||
import { asValidOpenCodeSessionId } from "./opencode-session-id.js";
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// AO-owned temp dir for opencode children
|
||||
// -----------------------------------------------------------------------------
|
||||
|
||||
let cachedOpenCodeTmpDir: string | null = null;
|
||||
|
||||
/**
|
||||
* Path that opencode children should treat as `TMPDIR`. Lives under the AO
|
||||
* base dir so a stray sweep cannot touch unrelated files.
|
||||
*/
|
||||
export function getOpenCodeTmpDir(): string {
|
||||
if (cachedOpenCodeTmpDir) return cachedOpenCodeTmpDir;
|
||||
cachedOpenCodeTmpDir = join(getAoBaseDir(), ".bun-tmp");
|
||||
return cachedOpenCodeTmpDir;
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-effort: create the AO-owned temp dir. Spawn paths call this before
|
||||
* launching opencode so the child sees a real directory at `TMPDIR`.
|
||||
*
|
||||
* Synchronous because the spawn helpers are themselves synchronous in their
|
||||
* env construction. Failures are swallowed — opencode will fall back to the
|
||||
* OS default temp dir, which is the pre-PR behavior.
|
||||
*/
|
||||
export function ensureOpenCodeTmpDir(): string {
|
||||
const dir = getOpenCodeTmpDir();
|
||||
try {
|
||||
mkdirSync(dir, { recursive: true });
|
||||
} catch {
|
||||
// Best-effort. If creation fails opencode still works; we just leak to
|
||||
// the system temp dir as before.
|
||||
}
|
||||
return dir;
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the env passed to every spawned `opencode` child.
|
||||
*
|
||||
* Setting both `TMPDIR` and `TMP`/`TEMP` covers POSIX (TMPDIR) and Windows
|
||||
* fallbacks. Bun honors `TMPDIR` for its embedded shared-library extraction.
|
||||
*/
|
||||
export function getOpenCodeChildEnv(
|
||||
extra?: NodeJS.ProcessEnv,
|
||||
): NodeJS.ProcessEnv {
|
||||
const dir = ensureOpenCodeTmpDir();
|
||||
return {
|
||||
...process.env,
|
||||
TMPDIR: dir,
|
||||
TMP: dir,
|
||||
TEMP: dir,
|
||||
...extra,
|
||||
};
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------------
|
||||
// Shared `opencode session list` cache
|
||||
// -----------------------------------------------------------------------------
|
||||
|
||||
export interface OpenCodeSessionListEntry {
|
||||
id: string;
|
||||
title: string;
|
||||
/** Raw `updated` field as emitted by opencode (string or number). */
|
||||
updated?: string | number;
|
||||
/** Normalized epoch ms, parsed from `updated`. */
|
||||
updatedAt?: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* TTL for the `opencode session list` cache.
|
||||
*
|
||||
* **Why 500ms.** The send-confirmation loop in `session-manager.sendMessage`
|
||||
* polls at 500ms intervals up to 6 times (~3s total). The original PR sized
|
||||
* the TTL to *cover* that window (3s), which made every loop iteration return
|
||||
* the same cached snapshot — the `updatedAt > baselineUpdatedAt` delivery
|
||||
* signal could not fire by construction. Sizing the TTL at 500ms means each
|
||||
* poll iteration sees fresh data while still collapsing tight bursts (e.g.
|
||||
* lifecycle poll + UI enrichment in the same tick) onto a single child.
|
||||
* Concurrent callers always share the in-flight promise regardless of TTL.
|
||||
*/
|
||||
export const OPENCODE_SESSION_LIST_CACHE_TTL_MS = 500;
|
||||
|
||||
const OPENCODE_SESSION_LIST_DEFAULT_TIMEOUT_MS = 30_000;
|
||||
|
||||
interface OpenCodeSessionListCache {
|
||||
entries: OpenCodeSessionListEntry[];
|
||||
timestamp: number;
|
||||
promise?: Promise<OpenCodeSessionListEntry[]>;
|
||||
}
|
||||
|
||||
let sessionListCache: OpenCodeSessionListCache | null = null;
|
||||
|
||||
function parseUpdatedToEpochMs(value: unknown): number | undefined {
|
||||
if (typeof value === "number" && Number.isFinite(value)) return value;
|
||||
if (typeof value !== "string") return undefined;
|
||||
const trimmed = value.trim();
|
||||
if (trimmed.length === 0) return undefined;
|
||||
if (/^\d+$/.test(trimmed)) {
|
||||
const n = Number(trimmed);
|
||||
return Number.isFinite(n) ? n : undefined;
|
||||
}
|
||||
const parsed = Date.parse(trimmed);
|
||||
return Number.isNaN(parsed) ? undefined : parsed;
|
||||
}
|
||||
|
||||
function parseSessionListStdout(stdout: string): OpenCodeSessionListEntry[] {
|
||||
const parsed = safeJsonParse<unknown>(stdout);
|
||||
if (!Array.isArray(parsed)) return [];
|
||||
|
||||
return parsed.flatMap((entry) => {
|
||||
if (!entry || typeof entry !== "object") return [];
|
||||
const record = entry as Record<string, unknown>;
|
||||
const id = asValidOpenCodeSessionId(record["id"]);
|
||||
if (!id) return [];
|
||||
const title = typeof record["title"] === "string" ? record["title"] : "";
|
||||
const rawUpdated = record["updated"];
|
||||
const updated =
|
||||
typeof rawUpdated === "string" || typeof rawUpdated === "number"
|
||||
? rawUpdated
|
||||
: undefined;
|
||||
const updatedAt = parseUpdatedToEpochMs(rawUpdated);
|
||||
return [
|
||||
{
|
||||
id,
|
||||
title,
|
||||
...(updated !== undefined ? { updated } : {}),
|
||||
...(updatedAt !== undefined ? { updatedAt } : {}),
|
||||
},
|
||||
];
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetch the opencode session list, sharing both the cached snapshot and any
|
||||
* in-flight request across all callers in core and plugins.
|
||||
*
|
||||
* Pass `forceRefresh: true` to bypass the TTL when a write is known to have
|
||||
* just landed. Concurrent callers still collapse onto the in-flight promise.
|
||||
*/
|
||||
export async function getCachedOpenCodeSessionList(options?: {
|
||||
timeoutMs?: number;
|
||||
forceRefresh?: boolean;
|
||||
}): Promise<OpenCodeSessionListEntry[]> {
|
||||
const timeoutMs = options?.timeoutMs ?? OPENCODE_SESSION_LIST_DEFAULT_TIMEOUT_MS;
|
||||
const forceRefresh = options?.forceRefresh ?? false;
|
||||
const now = Date.now();
|
||||
|
||||
if (sessionListCache) {
|
||||
if (sessionListCache.promise) {
|
||||
// A fetch is already in flight — every caller waits on it, even if
|
||||
// they wanted a refresh.
|
||||
return sessionListCache.promise;
|
||||
}
|
||||
if (
|
||||
!forceRefresh &&
|
||||
now - sessionListCache.timestamp < OPENCODE_SESSION_LIST_CACHE_TTL_MS
|
||||
) {
|
||||
return sessionListCache.entries;
|
||||
}
|
||||
}
|
||||
|
||||
const promise: Promise<OpenCodeSessionListEntry[]> = execFileAsync(
|
||||
"opencode",
|
||||
["session", "list", "--format", "json"],
|
||||
{ timeout: timeoutMs, env: getOpenCodeChildEnv() },
|
||||
)
|
||||
.then(({ stdout }) => {
|
||||
const entries = parseSessionListStdout(stdout);
|
||||
if (sessionListCache?.promise === promise) {
|
||||
sessionListCache = { entries, timestamp: Date.now() };
|
||||
}
|
||||
return entries;
|
||||
})
|
||||
.catch(() => {
|
||||
if (sessionListCache?.promise === promise) {
|
||||
sessionListCache = null;
|
||||
}
|
||||
return [] as OpenCodeSessionListEntry[];
|
||||
});
|
||||
|
||||
sessionListCache = { entries: [], timestamp: now, promise };
|
||||
return promise;
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop any cached snapshot. Call this immediately after any code path that
|
||||
* mutates opencode session state (delete, create) so that the next reader
|
||||
* does not observe a stale entry.
|
||||
*/
|
||||
export function invalidateOpenCodeSessionListCache(): void {
|
||||
// If a fetch is currently in flight we leave it alone — its result is
|
||||
// about to land and a fresh fetch on top of it would be wasted work.
|
||||
// Subsequent callers will see a stale snapshot for at most one tick;
|
||||
// this is acceptable because the in-flight result already reflects state
|
||||
// captured after the mutation began.
|
||||
if (sessionListCache?.promise) {
|
||||
sessionListCache = { ...sessionListCache, timestamp: 0 };
|
||||
return;
|
||||
}
|
||||
sessionListCache = null;
|
||||
}
|
||||
|
||||
/** Test-only: clear the cache including any in-flight promise. */
|
||||
export function resetOpenCodeSessionListCache(): void {
|
||||
sessionListCache = null;
|
||||
}
|
||||
|
|
@ -73,6 +73,13 @@ import {
|
|||
generateSessionName,
|
||||
} from "./paths.js";
|
||||
import { asValidOpenCodeSessionId } from "./opencode-session-id.js";
|
||||
import {
|
||||
getCachedOpenCodeSessionList,
|
||||
getOpenCodeChildEnv,
|
||||
invalidateOpenCodeSessionListCache,
|
||||
resetOpenCodeSessionListCache as resetSharedOpenCodeSessionListCache,
|
||||
type OpenCodeSessionListEntry,
|
||||
} from "./opencode-shared.js";
|
||||
import {
|
||||
writeWorkspaceOpenCodeAgentsMd,
|
||||
} from "./opencode-agents-md.js";
|
||||
|
|
@ -114,10 +121,15 @@ async function deleteOpenCodeSession(sessionId: string): Promise<void> {
|
|||
try {
|
||||
await execFileAsync("opencode", ["session", "delete", validatedSessionId], {
|
||||
timeout: 30_000,
|
||||
env: getOpenCodeChildEnv(),
|
||||
});
|
||||
// Drop cached list immediately so reuse / remap / restore call sites
|
||||
// do not observe the deleted id for the remainder of the TTL window.
|
||||
invalidateOpenCodeSessionListCache();
|
||||
return;
|
||||
} catch (err) {
|
||||
if (errorIncludesSessionNotFound(err)) {
|
||||
invalidateOpenCodeSessionListCache();
|
||||
return;
|
||||
}
|
||||
lastError = err;
|
||||
|
|
@ -126,42 +138,15 @@ async function deleteOpenCodeSession(sessionId: string): Promise<void> {
|
|||
throw lastError instanceof Error ? lastError : new Error(String(lastError));
|
||||
}
|
||||
|
||||
interface OpenCodeSessionListEntry {
|
||||
id: string;
|
||||
title: string;
|
||||
updatedAt?: number;
|
||||
/** Re-export so existing core test-utils + session-manager call sites keep working. */
|
||||
export function resetOpenCodeSessionListCache(): void {
|
||||
resetSharedOpenCodeSessionListCache();
|
||||
}
|
||||
|
||||
async function fetchOpenCodeSessionList(
|
||||
timeoutMs = OPENCODE_DISCOVERY_TIMEOUT_MS,
|
||||
timeoutMs: number = OPENCODE_DISCOVERY_TIMEOUT_MS,
|
||||
): Promise<OpenCodeSessionListEntry[]> {
|
||||
try {
|
||||
const { stdout } = await execFileAsync("opencode", ["session", "list", "--format", "json"], {
|
||||
timeout: timeoutMs,
|
||||
});
|
||||
const parsed = safeJsonParse<unknown>(stdout);
|
||||
if (!Array.isArray(parsed)) return [];
|
||||
|
||||
return parsed.flatMap((entry) => {
|
||||
if (!entry || typeof entry !== "object") return [];
|
||||
const title = typeof entry["title"] === "string" ? entry["title"] : "";
|
||||
const id = asValidOpenCodeSessionId(entry["id"]);
|
||||
if (!id) return [];
|
||||
const rawUpdated = entry["updated"];
|
||||
let updatedAt: number | undefined;
|
||||
if (typeof rawUpdated === "number" && Number.isFinite(rawUpdated)) {
|
||||
updatedAt = rawUpdated;
|
||||
} else if (typeof rawUpdated === "string") {
|
||||
const parsedUpdated = Date.parse(rawUpdated);
|
||||
if (!Number.isNaN(parsedUpdated)) {
|
||||
updatedAt = parsedUpdated;
|
||||
}
|
||||
}
|
||||
return [{ id, title, ...(updatedAt !== undefined ? { updatedAt } : {}) }];
|
||||
});
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
return getCachedOpenCodeSessionList({ timeoutMs });
|
||||
}
|
||||
|
||||
async function discoverOpenCodeSessionIdsByTitle(
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ vi.mock("node:child_process", () => ({
|
|||
},
|
||||
}));
|
||||
|
||||
import { create, manifest, default as defaultExport } from "./index.js";
|
||||
import { create, manifest, default as defaultExport, resetOpenCodeSessionListCache } from "./index.js";
|
||||
|
||||
function makeSession(overrides: Partial<Session> = {}): Session {
|
||||
return {
|
||||
|
|
@ -95,6 +95,7 @@ function mockTmuxWithProcess(processName: string, found = true) {
|
|||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
resetOpenCodeSessionListCache();
|
||||
});
|
||||
|
||||
describe("plugin manifest & exports", () => {
|
||||
|
|
|
|||
|
|
@ -7,6 +7,10 @@ import {
|
|||
getActivityFallbackState,
|
||||
recordTerminalActivity,
|
||||
asValidOpenCodeSessionId,
|
||||
getCachedOpenCodeSessionList,
|
||||
getOpenCodeChildEnv,
|
||||
ensureOpenCodeTmpDir,
|
||||
resetOpenCodeSessionListCache,
|
||||
type Agent,
|
||||
type AgentSessionInfo,
|
||||
type AgentLaunchConfig,
|
||||
|
|
@ -18,18 +22,13 @@ import {
|
|||
type Session,
|
||||
type WorkspaceHooksConfig,
|
||||
type OpenCodeAgentConfig,
|
||||
type OpenCodeSessionListEntry,
|
||||
} from "@aoagents/ao-core";
|
||||
import { execFile, execFileSync } from "node:child_process";
|
||||
import { promisify } from "node:util";
|
||||
|
||||
const execFileAsync = promisify(execFile);
|
||||
|
||||
interface OpenCodeSessionListEntry {
|
||||
id: string;
|
||||
title?: string;
|
||||
updated?: string | number;
|
||||
}
|
||||
|
||||
function parseUpdatedTimestamp(updated: string | number | undefined): Date | null {
|
||||
if (typeof updated === "number") {
|
||||
if (!Number.isFinite(updated)) return null;
|
||||
|
|
@ -54,20 +53,8 @@ function parseUpdatedTimestamp(updated: string | number | undefined): Date | nul
|
|||
return new Date(parsedMs);
|
||||
}
|
||||
|
||||
function parseSessionList(raw: string): OpenCodeSessionListEntry[] {
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(raw);
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
if (!Array.isArray(parsed)) return [];
|
||||
return parsed.filter((item): item is OpenCodeSessionListEntry => {
|
||||
if (!item || typeof item !== "object") return false;
|
||||
const record = item as Record<string, unknown>;
|
||||
return asValidOpenCodeSessionId(record["id"]) !== undefined;
|
||||
});
|
||||
}
|
||||
// Re-export for backward compat — see @aoagents/ao-core/opencode-shared.
|
||||
export { resetOpenCodeSessionListCache };
|
||||
|
||||
/**
|
||||
* Parse JSON stream lines from `opencode run --format json` output.
|
||||
|
|
@ -158,13 +145,7 @@ async function findOpenCodeSession(
|
|||
session: Session,
|
||||
): Promise<OpenCodeSessionListEntry | null> {
|
||||
try {
|
||||
const { stdout } = await execFileAsync(
|
||||
"opencode",
|
||||
["session", "list", "--format", "json"],
|
||||
{ timeout: 30_000 },
|
||||
);
|
||||
|
||||
const sessions = parseSessionList(stdout);
|
||||
const sessions = await getCachedOpenCodeSessionList();
|
||||
|
||||
// Prefer exact ID match from metadata
|
||||
if (session.metadata?.opencodeSessionId) {
|
||||
|
|
@ -273,6 +254,15 @@ function createOpenCodeAgent(): Agent {
|
|||
env["AO_ISSUE_ID"] = config.issueId;
|
||||
}
|
||||
|
||||
// Point Bun's embedded shared-library extraction at an AO-owned temp
|
||||
// dir so the cli-side janitor only needs to sweep our own files
|
||||
// (issue #1046). Setting all three keys covers POSIX (TMPDIR) and
|
||||
// Windows fallbacks; opencode itself ships POSIX-only today.
|
||||
const tmpDir = ensureOpenCodeTmpDir();
|
||||
env["TMPDIR"] = tmpDir;
|
||||
env["TMP"] = tmpDir;
|
||||
env["TEMP"] = tmpDir;
|
||||
|
||||
// PATH and GH_PATH are injected by session-manager for all agents.
|
||||
|
||||
return env;
|
||||
|
|
@ -452,7 +442,7 @@ export function create(): Agent {
|
|||
|
||||
export function detect(): boolean {
|
||||
try {
|
||||
execFileSync("opencode", ["version"], { stdio: "ignore" });
|
||||
execFileSync("opencode", ["version"], { stdio: "ignore", env: getOpenCodeChildEnv() });
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
|
|
|
|||
Loading…
Reference in New Issue