mirror of
https://github.com/mvschwarz/openrig.git
synced 2026-11-07 05:56:21 -10:00
fix(daemon): let down settle seats after tmux server loss (#1999)
After a team's tmux server dies (a crash, OOM or kill) and leaves its socket file, `rig down` reported "Could not confirm session … is absent: no server running" for every seat and stopped nothing, so `rig archive` refused live sessions and `rig up` refused a running rig. `rig down` now treats tmux's own "no server running on <socket>" diagnostic from the daemon's probe as positive absence: it records the sessions as intentionally stopped and clears their bindings, without killing anything or starting a server, and archive and up then proceed. The rule lives in one shared predicate beside `SessionProbe`, used by teardown and the import guard, and matches only the start of tmux's diagnostic (stderr, or the line right after a `Command failed:` wrapper). A missing socket file, a permission failure, socket-path text and any later line stay uncertain, as before. The session-absence check uses the same start-of-diagnostic boundary, and the probe rethrows the original error so its reason stays readable. Scope: a reboot that clears the socket directory is not covered. Validation: focused teardown, adapter and guard suites pass, including path-text, newline-path and mixed permission controls that kept the rig and binding, and daemon typecheck passes. Native acceptance (kill a disposable team's server, then down, archive and up) is separate.
This commit is contained in:
1 parent
3623f4d648
commit
e9faaee2a9
7 files changed
+75
-20
No files matched your search
@@ -130,6 +130,16 @@ export type SessionProbe =
|
||||
| { state: "absent" }
|
||||
| { state: "transport_unavailable"; cause: string };
|
||||
|
||||
/** Positive server absence in this adapter's namespace; match the diagnostic line, never a path or command fragment. */
|
||||
export function probeShowsNoServer(probe: { state: string; cause?: string }): boolean {
|
||||
return probe.state === "transport_unavailable" && /^no server running on /.test(tmuxDiagnosticStart(probe.cause ?? ""));
|
||||
}
|
||||
|
||||
/** Injected executors may retain exec's one-line command wrapper instead of stderr. */
|
||||
function tmuxDiagnosticStart(message: string): string {
|
||||
return message.replace(/^Command failed:[^\r\n]*\r?\n/, "");
|
||||
}
|
||||
|
||||
const TMUX_FIELD_SEPARATOR = "|";
|
||||
const SESSION_FORMAT = [
|
||||
"#{session_name}",
|
||||
@@ -167,7 +177,8 @@ const CURSOR_FORMAT = [
|
||||
].join(TMUX_FIELD_SEPARATOR);
|
||||
|
||||
function isNoServerError(err: unknown): boolean {
|
||||
return err instanceof Error && err.message.includes("no server running");
|
||||
const error = tmuxReportedError(err);
|
||||
return error instanceof Error && probeShowsNoServer({ state: "transport_unavailable", cause: error.message });
|
||||
}
|
||||
|
||||
/** The error tmux reported, without the stdout an exec error carries: in a chain that stdout is the earlier panes'
|
||||
@@ -179,11 +190,8 @@ function tmuxReportedError(err: unknown): unknown {
|
||||
|
||||
function isSessionAbsenceError(err: unknown): boolean {
|
||||
if (!(err instanceof Error)) return false;
|
||||
const msg = err.message.toLowerCase();
|
||||
return msg.includes("session not found") ||
|
||||
msg.includes("can't find session") ||
|
||||
msg.includes("no current target") ||
|
||||
msg.includes("no session");
|
||||
const msg = tmuxDiagnosticStart(err.message).toLowerCase();
|
||||
return /^(?:session not found|can't find session|no current target|no sessions?)\b/.test(msg);
|
||||
}
|
||||
|
||||
function isPaneAbsenceError(err: unknown): boolean {
|
||||
@@ -653,11 +661,12 @@ export class TmuxAdapter {
|
||||
`tmux has-session -t ${shellQuote(target)}`);
|
||||
return { state: "present" }; // exit 0 = session exists
|
||||
} catch (err) {
|
||||
if (isSessionAbsenceError(err)) {
|
||||
const error = tmuxReportedError(err);
|
||||
if (isSessionAbsenceError(error)) {
|
||||
return { state: "absent" };
|
||||
}
|
||||
if (isNoServerError(err) || isTmuxTransportAbsentError(err)) {
|
||||
return { state: "transport_unavailable", cause: (err as Error).message };
|
||||
if (isNoServerError(error) || isTmuxTransportAbsentError(error)) {
|
||||
return { state: "transport_unavailable", cause: (error as Error).message };
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type Database from "better-sqlite3";
|
||||
import type { RigRepository } from "./rig-repository.js";
|
||||
import type { SessionRegistry } from "./session-registry.js";
|
||||
import type { TmuxAdapter, SessionProbe } from "../adapters/tmux.js";
|
||||
import { probeShowsNoServer, type TmuxAdapter, type SessionProbe } from "../adapters/tmux.js";
|
||||
import type { SnapshotCapture } from "./snapshot-capture.js";
|
||||
import type { EventBus } from "./event-bus.js";
|
||||
import { RigNotFoundError } from "./errors.js";
|
||||
@@ -188,7 +188,8 @@ export class RigTeardownOrchestrator {
|
||||
if (!absent && this.deps.tmuxAdapter.probeSession) {
|
||||
try {
|
||||
const probe = await this.deps.tmuxAdapter.probeSession(session.sessionName);
|
||||
absent = probe.state === "absent";
|
||||
// This is the daemon's own tmux namespace, as in the import guard; a missing socket file is not enough.
|
||||
absent = probe.state === "absent" || probeShowsNoServer(probe);
|
||||
if (probe.state !== "present" && !absent) {
|
||||
result.errors.push(`Could not confirm session '${session.sessionName}' is absent: ${probe.state === "transport_unavailable" ? probe.cause : probe.state}`);
|
||||
killFailures++;
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import type Database from "better-sqlite3";
|
||||
import { probeShowsNoServer } from "../adapters/tmux.js";
|
||||
|
||||
/**
|
||||
* S5b (OPR.0.5.4.11) — the running-name guard, the FLOOR against duplicate rig
|
||||
@@ -105,7 +106,7 @@ export async function confirmStoppedGenerations(
|
||||
return { ok: false, rigId, reason: `tmux could not check session ${name}: ${(err as Error).message}` };
|
||||
}
|
||||
if (result.state === "absent") continue;
|
||||
if (result.state === "transport_unavailable" && result.cause?.includes("no server running")) continue;
|
||||
if (probeShowsNoServer(result)) continue;
|
||||
return {
|
||||
ok: false,
|
||||
rigId,
|
||||
|
||||
@@ -7,7 +7,7 @@ import { RigRepository } from "../src/domain/rig-repository.js";
|
||||
import { SessionRegistry } from "../src/domain/session-registry.js";
|
||||
import { EventBus } from "../src/domain/event-bus.js";
|
||||
import { RigTeardownOrchestrator } from "../src/domain/rig-teardown.js";
|
||||
import type { TmuxAdapter } from "../src/adapters/tmux.js";
|
||||
import { TmuxAdapter } from "../src/adapters/tmux.js";
|
||||
import { SnapshotCapture } from "../src/domain/snapshot-capture.js";
|
||||
import { SnapshotRepository } from "../src/domain/snapshot-repository.js";
|
||||
import { CheckpointStore } from "../src/domain/checkpoint-store.js";
|
||||
@@ -15,6 +15,7 @@ import { ResumeMetadataRefresher } from "../src/domain/resume-metadata-refresher
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { checkRunningNameGuard, makeRunningSessionCounter, confirmStoppedGenerations } from "../src/domain/running-name-guard.js";
|
||||
import { ALL_MIGRATIONS } from "../src/db/all-migrations.js";
|
||||
import { managedBlockText, mergeManagedBlock } from "../src/domain/managed-blocks.js";
|
||||
import { SEAT_SKILLS_BOOTSTRAP_LINE } from "../src/domain/seat-skill-storage.js";
|
||||
@@ -740,11 +741,49 @@ describe("RigTeardownOrchestrator", () => {
|
||||
expect(rigRepo.getRig(rigId)).not.toBeNull();
|
||||
});
|
||||
|
||||
it("retains the binding when the tmux server is unreachable", async () => {
|
||||
it.each(["/tmp/tmux-501/default", "/team/tmux/tmux-501/default"])("stops a rig after its own server reports absence on %s", async (socket) => {
|
||||
const { rigId, nodeId, sessionId } = seedRig();
|
||||
sessionRegistry.updateBinding(nodeId, { tmuxSession: "r01-dev" });
|
||||
const terminal = noServerTerminal(`Command failed: tmux has-session -t =r01-dev\nno server running on ${socket}`);
|
||||
const tmux = mockTmux();
|
||||
tmux.probeSession = terminal.probeSession;
|
||||
|
||||
const result = await buildTeardown(tmux).teardown(rigId);
|
||||
|
||||
expect(result.errors).toEqual([]);
|
||||
expect(result.sessionsKilled).toBe(0);
|
||||
expect(tmux.killSession).not.toHaveBeenCalled();
|
||||
expect(sessionRegistry.getBindingForNode(nodeId)).toBeNull();
|
||||
expect(sessionRegistry.getSessionsForRig(rigId).find((session) => session.id === sessionId))
|
||||
.toMatchObject({ status: "exited" });
|
||||
expect(db.prepare("SELECT intentionally_stopped FROM sessions WHERE id = ?").get(sessionId))
|
||||
.toEqual({ intentionally_stopped: 1 });
|
||||
expect(checkRunningNameGuard({ findRigsByName: (name) => rigRepo.findRigsByName(name),
|
||||
countRunningSessions: makeRunningSessionCounter(db) }, "test-rig")).toEqual({ ok: true });
|
||||
expect(terminal.commands).toEqual(["tmux has-session -t '=r01-dev'"]);
|
||||
expect((await confirmStoppedGenerations(db, [rigId], tmux.probeSession.bind(tmux))).ok).toBe(true);
|
||||
rigRepo.archiveRig(rigId);
|
||||
expect(db.prepare("SELECT archived_at FROM rigs WHERE id = ?").get(rigId))
|
||||
.toEqual({ archived_at: expect.any(String) });
|
||||
});
|
||||
|
||||
it.each([
|
||||
"error connecting to /other-caller/tmux-501/default (No such file or directory)",
|
||||
// Synthetic Connection refused shape: real tmux 3.6a prints "no server running on ..." for a stale socket.
|
||||
"error connecting to /tmp/tmux-501/default (Connection refused)",
|
||||
"error connecting to /tmp/no server running/tmux-501/default (Connection refused)",
|
||||
"error connecting to /tmp/no server running/tmux-501/default (No such file or directory)",
|
||||
"error connecting to /tmp/\nno server running on x/tmux-501/default (Permission denied)",
|
||||
"error connecting to /tmp/no session/tmux-501/default (No such file or directory)",
|
||||
"permission denied: no server running on /tmp/tmux-501/default",
|
||||
])("retains the binding when the server probe is uncertain: %s", async (cause) => {
|
||||
const { rigId, nodeId, sessionId } = seedRig();
|
||||
sessionRegistry.updateBinding(nodeId, { tmuxSession: "r01-dev" });
|
||||
const tmux = mockTmux();
|
||||
tmux.probeSession = vi.fn(async () => ({ state: "transport_unavailable", cause: "no server running on /tmp/tmux-501/default" }));
|
||||
const adapter = new TmuxAdapter(async () => { throw new Error("unexpected legacy exec"); }, undefined, async () => {
|
||||
throw Object.assign(new Error(`Command failed: /fixture/tmux has-session -t =r01-dev\n${cause}`), { execStderr: cause });
|
||||
});
|
||||
tmux.probeSession = vi.fn((name) => adapter.probeSession(name));
|
||||
|
||||
const result = await buildTeardown(tmux).teardown(rigId, { delete: true });
|
||||
|
||||
@@ -753,6 +792,8 @@ describe("RigTeardownOrchestrator", () => {
|
||||
expect(rigRepo.getRig(rigId)).not.toBeNull();
|
||||
expect(sessionRegistry.getBindingForNode(nodeId)?.tmuxSession).toBe("r01-dev");
|
||||
expect(sessionRegistry.getSessionsForRig(rigId).find((session) => session.id === sessionId)?.status).toBe("running");
|
||||
sessionRegistry.updateStatus(sessionId, "detached");
|
||||
expect((await confirmStoppedGenerations(db, [rigId], tmux.probeSession.bind(tmux))).ok).toBe(false);
|
||||
});
|
||||
|
||||
it("retains the binding if the tmux server vanishes between probe and kill", async () => {
|
||||
|
||||
@@ -247,7 +247,7 @@ describe("TmuxAdapter", () => {
|
||||
|
||||
it("throws on generic unrecognized exec error", async () => {
|
||||
const adapter = new TmuxAdapter(mockExec({
|
||||
"has-session": { error: new Error("Command failed with exit code 127") },
|
||||
"has-session": { error: Object.assign(new Error("Command failed with exit code 127"), { execStderr: "" }) },
|
||||
}));
|
||||
await expect(adapter.hasSession("any-session")).rejects.toThrow("exit code 127");
|
||||
});
|
||||
|
||||
@@ -45,7 +45,7 @@ describe("TmuxAdapter.capturePanesContent (#308)", () => {
|
||||
const calls: string[][] = [];
|
||||
const run = (argv: string[]) => {
|
||||
calls.push(argv);
|
||||
if (opts.noServer) throw new Error("no server running");
|
||||
if (opts.noServer) throw new Error("no server running on /tmp/tmux-501/default");
|
||||
return runTmux(argv, panes, opts.vanished, opts.maxBuffer);
|
||||
};
|
||||
const exec = async (cmd: string) => run(shellToArgv(cmd));
|
||||
|
||||
@@ -116,14 +116,17 @@ describe.skipIf(!nativeAvailable)("tmux shell and argv transports against a real
|
||||
noServerSessions: [],
|
||||
});
|
||||
expect(shell.result.captured).toEqual(["openrig-native"]);
|
||||
// The first line names the command, which each transport renders its own way; the tmux stderr after it,
|
||||
// which every error classifier reads, is the same apart from each run's private directory.
|
||||
for (const run of [shell, argv]) {
|
||||
expect(run.result.noServerCause).toMatch(/^(no server running on|error connecting to) /);
|
||||
}
|
||||
// Write errors retain a command-wrapper line; a probe cause is already tmux's own stderr.
|
||||
// Compare both transports after normalizing only each run's private directory.
|
||||
const stderr = ({ result: r, root }: typeof shell) => {
|
||||
const tail = (message: string) => message.split("\n").slice(1).join("\n").replaceAll(root, "<root>").trim();
|
||||
return {
|
||||
missingKill: tail((r.missingKill as { message: string }).message),
|
||||
missingKeys: `${(r.missingKeys as { code: string }).code}: ${tail((r.missingKeys as { message: string }).message)}`,
|
||||
noServerCause: tail(r.noServerCause as string),
|
||||
noServerCause: (r.noServerCause as string).replaceAll(root, "<root>").trim(),
|
||||
};
|
||||
};
|
||||
expect(stderr(argv)).toEqual(stderr(shell));
|
||||
|
||||
Reference in new issue
Block a user