RomanNum3ral
/
paperclip
mirror of https://github.com/paperclipai/paperclip.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(adapter-utils): close remaining SSH env-lab fixture orphan windows 

The start-failure catch block sent one SIGTERM and removed the root
directory at once, without waiting for sshd to exit. Because the state
file is written only after readiness succeeds, a slow-to-exit sshd on
this path became an orphan with no state file to target it. Reuse the
bounded SIGTERM-to-SIGKILL escalation before the removal, and keep the
root directory when the listener survives it.

The test drain swallowed a failed stop and deleted the root directory
anyway, which erased the state file stopSshEnvLabFixture relies on for
a later kill attempt. The drain now removes the root directory only
after the stop call succeeds, and logs the pid and port when it fails.

The per-test teardown entry was pushed after several setup `await`
calls (mkdir, writeFile, git init), so a throw among them leaked the
temporary directory with no entry to clean it up. A new helper creates
the root directory and registers its teardown entry in the same step.

Add a regression test that forces a fixture to fail readiness (via an
unreachable RFC 5737 host and a short, test-only readiness timeout)
and proves it leaves no live listener and no root directory behind.

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Priya Raman 2026-08-26 19:01:04 +00:00
parent 6497ae6478
commit 7edb4843bf
No known key found for this signature in database
GPG Key ID: 4861541D36B2037E
2 changed files with 178 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

160
packages/adapter-utils/src/ssh-fixture.test.ts
View File

@ -1,5 +1,5 @@
import { execFile } from "node:child_process";
import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promises";
import { mkdir, mkdtemp, readFile, rm, stat, symlink, writeFile } from "node:fs/promises";
import net from "node:net";
import os from "node:os";
import path from "node:path";
@ -23,11 +23,12 @@ import { prepareRemoteManagedRuntime } from "./remote-managed-runtime.js";
const SSH_FIXTURE_TEST_TIMEOUT_MS = 30_000;
let sshEnvLabUnsupportedReason: string | null = null;
// One entry per fixture-start attempt, registered at start time so teardown
// survives an assertion failure, a thrown error, or an early return on skip.
// `state` stays null until the fixture actually starts; a caller that stops
// the fixture itself still leaves the entry in the stack, so the drain below
// must be idempotent (stopSshEnvLabFixture is).
// One entry per fixture root directory, registered at creation time so
// teardown survives a setup call that throws before the fixture starts, an
// assertion failure, or an early return on skip. `state` stays null until
// the fixture actually starts; a caller that stops the fixture itself still
// leaves the entry in the stack, so the drain below must be idempotent
// (stopSshEnvLabFixture is).
interface FixtureTeardownEntry {
rootDir: string;
state: SshEnvLabFixtureState | null;
@ -35,12 +36,36 @@ interface FixtureTeardownEntry {
const fixtureTeardowns: FixtureTeardownEntry[] = [];
// Creates the fixture root directory and registers its teardown entry in
// the same step, so a setup call that throws between here and the fixture
// start (mkdir, writeFile, git init) still leaves the root directory queued
// for removal.
async function createFixtureRootDir(): Promise<string> {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
fixtureTeardowns.push({ rootDir, state: null });
return rootDir;
}
async function drainFixtureTeardowns(): Promise<void> {
while (fixtureTeardowns.length > 0) {
const entry = fixtureTeardowns.pop();
if (!entry) continue;
if (entry.state) {
await stopSshEnvLabFixture(entry.state).catch(() => undefined);
try {
await stopSshEnvLabFixture(entry.state);
} catch (error) {
// stopSshEnvLabFixture throws only when the listener survives
// SIGKILL, and it deliberately keeps the root directory so a later
// stop call can still find and signal it through the state file.
// Report the failure but keep the root directory; do not remove it,
// and do not rethrow, so a throw here cannot strand the entries
// still left on the stack.
console.error(
`SSH env-lab fixture teardown failed for pid ${entry.state.pid} on port ${entry.state.port}:`,
error,
);
continue;
}
}
await rm(entry.rootDir, { recursive: true, force: true }).catch(() => undefined);
}
@ -58,12 +83,39 @@ async function git(cwd: string, args: string[]): Promise<string> {
});
}
// Finds the pid of a running sshd process by its config file path, the same
// way isSshEnvLabFixtureProcess identifies a fixture internally. Used by the
// readiness-failure regression test, which needs the pid of a fixture that
// startSshEnvLabFixture never returns because it throws before returning it.
async function findSshdPidByConfigPath(sshdConfigPath: string): Promise<number | null> {
const stdout = await new Promise<string>((resolve) => {
execFile("ps", ["-eo", "pid=,args="], (error, out) => resolve(error ? "" : out));
});
for (const line of stdout.split("\n")) {
const trimmed = line.trim();
const spaceIndex = trimmed.indexOf(" ");
if (spaceIndex === -1) continue;
const pid = Number.parseInt(trimmed.slice(0, spaceIndex), 10);
const args = trimmed.slice(spaceIndex + 1);
if (Number.isFinite(pid) && args.includes(sshdConfigPath)) {
return pid;
}
}
return null;
}
async function startSshEnvLabFixtureOrSkip(statePath: string, label: string) {
// Register the teardown entry before the first await, so a fixture that
// starts and then throws later in the calling test still gets its root
// directory removed.
const entry: FixtureTeardownEntry = { rootDir: path.dirname(statePath), state: null };
fixtureTeardowns.push(entry);
// The teardown entry for this root directory must already exist: callers
// create it with createFixtureRootDir() before they derive statePath, so
// this only attaches the state to that entry instead of pushing a new
// one (a root directory must never get two entries).
const rootDir = path.dirname(statePath);
const entry = fixtureTeardowns.find((candidate) => candidate.rootDir === rootDir);
if (!entry) {
throw new Error(
`No fixture teardown entry for ${rootDir}. Call createFixtureRootDir() before starting a fixture.`,
);
}
if (sshEnvLabUnsupportedReason) {
console.warn(`Skipping ${label}: ${sshEnvLabUnsupportedReason}`);
@ -120,7 +172,7 @@ describe("ssh env-lab fixture", () => {
afterAll(drainFixtureTeardowns);
it("starts an isolated sshd fixture and executes commands through it", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const started = await startSshEnvLabFixtureOrSkip(statePath, "SSH env-lab fixture test");
@ -143,7 +195,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("forwards stdin to remote SSH commands", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const started = await startSshEnvLabFixtureOrSkip(statePath, "SSH stdin forwarding test");
@ -171,7 +223,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("does not treat an unrelated reused pid as the running fixture", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const started = await startSshEnvLabFixtureOrSkip(statePath, "SSH env-lab fixture test");
@ -196,7 +248,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("stops the fixture listener and frees its loopback port", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const started = await startSshEnvLabFixtureOrSkip(statePath, "SSH teardown regression test");
@ -224,6 +276,60 @@ describe("ssh env-lab fixture", () => {
});
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("leaves no live listener and no root directory when the fixture fails readiness", async () => {
if (sshEnvLabUnsupportedReason) {
console.warn(`Skipping SSH readiness-failure cleanup test: ${sshEnvLabUnsupportedReason}`);
return;
}
const support = await getSshEnvLabSupport();
if (!support.supported) {
sshEnvLabUnsupportedReason = support.reason ?? "unsupported environment";
console.warn(`Skipping SSH readiness-failure cleanup test: ${sshEnvLabUnsupportedReason}`);
return;
}
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const sshdConfigPath = path.join(rootDir, "sshd_config");
// sshd binds through bindHost (127.0.0.1) and stays alive; the readiness
// check targets an unreachable RFC 5737 TEST-NET-3 address instead, so it
// fails on every attempt without ever reaching a real host. Poll for the
// resulting sshd process concurrently, since startSshEnvLabFixture never
// returns a state on this path (it throws before writing one).
let capturedPid: number | null = null;
const pollDeadline = Date.now() + 5_000;
const pollForPid = (async () => {
while (capturedPid === null && Date.now() < pollDeadline) {
capturedPid = await findSshdPidByConfigPath(sshdConfigPath);
if (capturedPid === null) {
await new Promise((resolve) => setTimeout(resolve, 25));
}
}
})();
await expect(
startSshEnvLabFixture({
statePath,
host: "203.0.113.1",
readinessTimeoutMs: 1_000,
}),
).rejects.toThrow();
await pollForPid;
expect(capturedPid).not.toBeNull();
let pidStillRunning = true;
try {
process.kill(capturedPid!, 0);
} catch {
pidStillRunning = false;
}
expect(pidStillRunning).toBe(false);
await expect(stat(rootDir)).rejects.toThrow();
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("builds a remote script that sources login profiles but no nvm", async () => {
const target = await buildSshSpawnTarget({
spec: {
@ -290,7 +396,7 @@ describe("ssh env-lab fixture", () => {
});
it("syncs a local directory into the remote fixture workspace", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const localDir = path.join(rootDir, "local-overlay");
@ -322,7 +428,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("reports throttled upload progress with a clamped percent and terminal 100% line", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const localDir = path.join(rootDir, "local-overlay");
@ -369,7 +475,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("reports restore progress with a terminal completion line", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const localDir = path.join(rootDir, "local-overlay");
const restoreDir = path.join(rootDir, "restore-target");
@ -414,7 +520,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("reports exact git-history import percentage from the known bundle size", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const localRepo = path.join(rootDir, "local-workspace");
@ -455,7 +561,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("can dereference local symlinks while syncing to the remote fixture", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const sourceDir = path.join(rootDir, "source");
const localDir = path.join(rootDir, "local-overlay");
@ -490,7 +596,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("round-trips a git workspace through the SSH fixture", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const localRepo = path.join(rootDir, "local-workspace");
@ -549,7 +655,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("preserves both concurrent SSH restores in a shared git workspace", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const localRepo = path.join(rootDir, "local-workspace");
@ -606,7 +712,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("preserves nested per-run files across sequential SSH restores with stale baselines", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const localRepo = path.join(rootDir, "local-workspace");
@ -661,7 +767,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("round-trips remote git commits through the managed runtime restore path", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const localRepo = path.join(rootDir, "local-workspace");
@ -706,7 +812,7 @@ describe("ssh env-lab fixture", () => {
// packages/adapter-utils/README.md and packages/adapters/AUTHORING.md:
// the local execution-workspace cwd is the only persistence boundary
// across runs. No adapter may depend on a git remote for cross-run state.
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const localRepo = path.join(rootDir, "local-workspace");
@ -763,7 +869,7 @@ describe("ssh env-lab fixture", () => {
}, SSH_FIXTURE_TEST_TIMEOUT_MS);
it("merges concurrent remote commits through the managed runtime restore path", async () => {
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-ssh-fixture-"));
const rootDir = await createFixtureRootDir();
const statePath = path.join(rootDir, "state.json");
const localRepo = path.join(rootDir, "local-workspace");

65
packages/adapter-utils/src/ssh.ts
View File

@ -1729,6 +1729,25 @@ async function waitUntilFixtureProcessExits(
}
}
// Bounded shutdown escalation shared by every caller that must stop a
// fixture process: send SIGTERM, wait, re-check process identity (the pid
// can be reused in the gap between two signals), then SIGKILL, then wait
// again. Returns true only when the listener is confirmed gone.
async function escalateSshEnvLabFixtureShutdown(
state: Pick<SshEnvLabFixtureState, "pid" | "sshdConfigPath">,
): Promise<boolean> {
if (!(await isSshEnvLabFixtureProcess(state))) return true;
process.kill(state.pid, "SIGTERM");
if (await waitUntilFixtureProcessExits(state, 5_000)) return true;
if (!(await isSshEnvLabFixtureProcess(state))) return true;
process.kill(state.pid, "SIGKILL");
if (await waitUntilFixtureProcessExits(state, 2_000)) return true;
return !(await isSshEnvLabFixtureProcess(state));
}
// Accepts a state path or an already-read state so a caller that already
// holds the fixture state in memory does not have to depend on the state
// file, which a teardown step may have already removed.
@ -1740,22 +1759,10 @@ export async function stopSshEnvLabFixture(
: stateOrPath;
if (!state) return false;
if (await isSshEnvLabFixtureProcess(state)) {
process.kill(state.pid, "SIGTERM");
if (!(await waitUntilFixtureProcessExits(state, 5_000))) {
// Re-check identity immediately before SIGKILL: the pid can be reused
// in the gap between the two signals.
if (await isSshEnvLabFixtureProcess(state)) {
process.kill(state.pid, "SIGKILL");
if (!(await waitUntilFixtureProcessExits(state, 2_000))) {
if (await isSshEnvLabFixtureProcess(state)) {
throw new Error(
`SSH env-lab fixture did not stop: pid ${state.pid} on port ${state.port} is still running after SIGKILL.`,
);
}
}
}
}
if (!(await escalateSshEnvLabFixtureShutdown(state))) {
throw new Error(
`SSH env-lab fixture did not stop: pid ${state.pid} on port ${state.port} is still running after SIGKILL.`,
);
}
// Remove the root directory only after the listener process is confirmed
@ -1769,6 +1776,10 @@ export async function startSshEnvLabFixture(input: {
statePath: string;
bindHost?: string;
host?: string;
// Test-only. Shortens the readiness wait below its 10 second default, so
// a regression test can force the start-failure cleanup path without a
// real 10 second wait.
readinessTimeoutMs?: number;
}): Promise<SshEnvLabFixtureState> {
const existing = await readSshEnvLabFixtureState(input.statePath);
if (existing && await isSshEnvLabFixtureProcess(existing)) {
@ -1886,14 +1897,28 @@ export async function startSshEnvLabFixture(input: {
}
const config = await buildSshEnvLabFixtureConfig(state);
await ensureSshWorkspaceReady(config);
}, { timeoutMs: 10_000, intervalMs: 250 });
}, { timeoutMs: input.readinessTimeoutMs ?? 10_000, intervalMs: 250 });
await fs.writeFile(input.statePath, JSON.stringify(state, null, 2), { mode: 0o600 });
return state;
} catch (error) {
if (await isPidRunning(state.pid)) {
process.kill(state.pid, "SIGTERM");
// No state file exists on this path yet, so a later stopSshEnvLabFixture
// call can never find this pid. Escalate and wait for exit here, the
// same way stopSshEnvLabFixture does, before the root directory goes
// away — otherwise a slow-to-exit sshd survives as an orphan with its
// root directory already gone.
const stopped = await escalateSshEnvLabFixtureShutdown(state);
if (stopped) {
await fs.rm(rootDir, { recursive: true, force: true }).catch(() => undefined);
} else {
const survivalNote =
`SSH env-lab fixture pid ${state.pid} on port ${state.port} is still running after SIGKILL. ` +
`Kept ${rootDir} for inspection; no state file exists to target it with a later stop call.`;
if (error instanceof Error) {
error.message = `${error.message}\n${survivalNote}`;
} else {
console.error(survivalNote);
}
}
await fs.rm(rootDir, { recursive: true, force: true }).catch(() => undefined);
throw error;
}
}