mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(self-update): stalled status and hung shutdown on launchd-daemon installs (#478)
* fix(self-update): stop a stalled status from blocking every later update A Homebrew node upgrade under a long-running server deletes the versioned Cellar path the server passes as --node, so every status write from the updater failed. The update itself still built and restarted (npm and the build use node from PATH), but update-status.json stayed "queued" forever. The boot reconcile ran one minute after the restart, inside its 15 min window, and isInFlight() had no age limit, so "An update is already in progress." blocked every later update until the next server restart. - self-update.sh falls back to node on PATH when --node is not executable. - expireStalledStatus() (pure) fails an in-flight status whose last write is older than the stale window; applied on every read (start + status poll) and persisted. The live updater heartbeats every few seconds, so a running update never trips it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(self-update): a hung graceful shutdown no longer leaves a LaunchDaemon install down On a KeepAlive LaunchDaemon (headless macOS) the updater restarts by sending the server SIGTERM and letting launchd respawn it. launchd only respawns once the process EXITS, and nothing escalates a stuck stop (systemd would SIGKILL after TimeoutStopSec). Observed after an update to 1.32.1: the server closed port 3000, server.stop() never resolved, the process stayed alive and the service stayed down until it was killed by hand. - cli.ts: the signal handler arms an unref'd 10s timer that force-exits if server.stop() hangs. - self-update.sh (launchd-daemon): wait up to 30s for the server pid to exit, then SIGKILL it. tmux sessions live outside the server and survive. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Codeman maintainer <noreply@anthropic.com>
This commit is contained in:
+20
-1
@@ -74,6 +74,15 @@ echo "[self-update] $(date) start tag=$TAG supervisor=$SUPERVISOR repo=$REPO"
|
||||
export PATH="$(dirname "$NODE"):$HOME/.local/bin:$HOME/.npm-global/bin:/usr/local/bin:/opt/homebrew/bin:$PATH"
|
||||
export GIT_TERMINAL_PROMPT=0
|
||||
|
||||
# --node is the server's process.execPath, a VERSIONED path (Homebrew resolves it
|
||||
# into Cellar/node/<ver>/). A `brew upgrade node` under a long-running server
|
||||
# deletes it, and every status write then failed, so the status stayed "queued"
|
||||
# forever. Fall back to whatever node is on PATH.
|
||||
if [ ! -x "$NODE" ]; then
|
||||
echo "[self-update] WARN: $NODE is not executable, falling back to node on PATH"
|
||||
NODE="$(command -v node || echo node)"
|
||||
fi
|
||||
|
||||
TO_VERSION="${TAG##*@}" # codeman@0.9.4 → 0.9.4 (tag is validated upstream)
|
||||
STASH_REF=""
|
||||
MANUAL_CMD=""
|
||||
@@ -276,7 +285,17 @@ case "$SUPERVISOR" in
|
||||
# domain needs root, but we don't need it — kill the server and launchd
|
||||
# respawns it on the new dist/ within ThrottleInterval seconds.
|
||||
if [[ -n "$SERVER_PID" ]] && kill "$SERVER_PID" 2>/dev/null; then
|
||||
: # respawn is launchd's job from here
|
||||
# Respawn is launchd's job, but only once the old process EXITS. A graceful
|
||||
# shutdown that hangs leaves the port closed and the service down, so
|
||||
# escalate to SIGKILL (tmux sessions live outside the server and survive).
|
||||
for _ in $(seq 1 30); do
|
||||
kill -0 "$SERVER_PID" 2>/dev/null || break
|
||||
sleep 1
|
||||
done
|
||||
if kill -0 "$SERVER_PID" 2>/dev/null; then
|
||||
echo "[self-update] server pid $SERVER_PID still alive 30s after SIGTERM, sending SIGKILL"
|
||||
kill -9 "$SERVER_PID" 2>/dev/null || true
|
||||
fi
|
||||
else
|
||||
MANUAL_CMD="sudo launchctl kickstart -k system/com.codeman.web"
|
||||
write_status "completed-needs-manual-restart" "Update staged — restart Codeman to apply v$TO_VERSION."
|
||||
|
||||
+10
@@ -129,6 +129,8 @@ program
|
||||
|
||||
/** Same registry the server resolves case names through (mirrors `case-routes.ts`). */
|
||||
const LINKED_CASES_FILE = dataPath('linked-cases.json');
|
||||
/** Graceful shutdown budget before the process force-exits (see the SIGTERM handler). */
|
||||
const SHUTDOWN_FORCE_EXIT_MS = 10_000;
|
||||
|
||||
/**
|
||||
* Case name to directory, checking `linked-cases.json` FIRST and falling back to the
|
||||
@@ -1002,6 +1004,14 @@ webCmd.action(async (options) => {
|
||||
if (shuttingDown) return;
|
||||
shuttingDown = true;
|
||||
console.log(palette.warn(`\n${signal} received, shutting down gracefully...`));
|
||||
// A hung stop() must not keep the process alive: the listener is already
|
||||
// closed by then, and a KeepAlive LaunchDaemon only respawns the server once
|
||||
// it EXITS (systemd would SIGKILL after TimeoutStopSec; launchd does not).
|
||||
// Seen after a self-update on macOS: port closed, process alive, service down.
|
||||
setTimeout(() => {
|
||||
console.error(palette.err(`Shutdown did not finish in ${SHUTDOWN_FORCE_EXIT_MS / 1000}s, forcing exit`));
|
||||
process.exit(1);
|
||||
}, SHUTDOWN_FORCE_EXIT_MS).unref();
|
||||
try {
|
||||
await server.stop();
|
||||
} catch (err) {
|
||||
|
||||
+38
-2
@@ -226,6 +226,29 @@ export function reconcileStatusDecision(
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* PURE runtime staleness check, applied whenever the status is READ (not only on
|
||||
* boot). The live updater heartbeats `updatedAt` every few seconds, so an
|
||||
* in-flight status whose last write is older than the window has no updater
|
||||
* behind it. Without this, a status that never advanced (e.g. the updater could
|
||||
* not run its `--node` binary because Homebrew upgraded node under a long-running
|
||||
* server, so every status write failed) blocked every later update with "An
|
||||
* update is already in progress." until the server happened to restart.
|
||||
* Returns the failed status to persist, or null to leave the status untouched.
|
||||
*/
|
||||
export function expireStalledStatus(status: UpdateStatus | null, now: number): UpdateStatus | null {
|
||||
if (!status || !IN_FLIGHT_PHASES.has(status.phase)) return null;
|
||||
const lastWrite = status.updatedAt || status.startedAt;
|
||||
if (now - lastWrite <= RECONCILE_STALE_MS) return null;
|
||||
return {
|
||||
...status,
|
||||
phase: 'failed',
|
||||
message: 'Update stopped reporting progress',
|
||||
error: `no status update for ${Math.round((now - lastWrite) / 60_000)} min during "${status.phase}"`,
|
||||
updatedAt: now,
|
||||
};
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// PURE helpers — the container environment gate
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
@@ -380,6 +403,19 @@ export function writeUpdateStatusAtomic(status: UpdateStatus): void {
|
||||
renameSync(tmp, STATUS_FILE);
|
||||
}
|
||||
|
||||
/** Read the status, first failing (and persisting) an in-flight one that stopped heartbeating. */
|
||||
function readCurrentUpdateStatus(now = Date.now()): UpdateStatus | null {
|
||||
const status = readUpdateStatus();
|
||||
const expired = expireStalledStatus(status, now);
|
||||
if (!expired) return status;
|
||||
try {
|
||||
writeUpdateStatusAtomic(expired);
|
||||
} catch {
|
||||
// Still report the expired view; the next read retries the write.
|
||||
}
|
||||
return expired;
|
||||
}
|
||||
|
||||
/** Reconcile the status file on server boot (call once, early in start()). */
|
||||
export function reconcileUpdateOnBoot(now = Date.now()): void {
|
||||
const status = readUpdateStatus();
|
||||
@@ -796,7 +832,7 @@ export async function startUpdate(): Promise<StartUpdateResult> {
|
||||
: 'This is not a git install. Update with: npm i -g aicodeman@latest',
|
||||
};
|
||||
}
|
||||
const existing = readUpdateStatus();
|
||||
const existing = readCurrentUpdateStatus();
|
||||
if (isInFlight(existing)) {
|
||||
return { ok: false, code: 'in-flight', message: 'An update is already in progress.' };
|
||||
}
|
||||
@@ -885,7 +921,7 @@ export async function startUpdate(): Promise<StartUpdateResult> {
|
||||
|
||||
/** Current status for the polling endpoint; null collapses to an explicit idle. */
|
||||
export function getUpdateStatusForApi(): UpdateStatus {
|
||||
const status = readUpdateStatus();
|
||||
const status = readCurrentUpdateStatus();
|
||||
if (status) return status;
|
||||
return {
|
||||
updateId: '',
|
||||
|
||||
@@ -15,6 +15,7 @@ import {
|
||||
isValidReleaseTag,
|
||||
parseGitHubRepo,
|
||||
reconcileStatusDecision,
|
||||
expireStalledStatus,
|
||||
} from '../src/web/self-update.js';
|
||||
import type { UpdateStatus } from '../src/types/update.js';
|
||||
|
||||
@@ -167,3 +168,40 @@ describe('reconcileStatusDecision (boot handoff state machine)', () => {
|
||||
expect(reconcileStatusDecision(noTarget, '0.9.4', NOW)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('expireStalledStatus (runtime staleness backstop)', () => {
|
||||
const NOW = 1_000_000_000_000;
|
||||
const MIN = 60 * 1000;
|
||||
const base = (over: Partial<UpdateStatus>): UpdateStatus => ({
|
||||
updateId: 'u1',
|
||||
phase: 'queued',
|
||||
message: '',
|
||||
fromVersion: '1.24.7',
|
||||
toVersion: '1.29.0',
|
||||
startedAt: NOW - 5_000,
|
||||
updatedAt: NOW - 5_000,
|
||||
...over,
|
||||
});
|
||||
|
||||
it('leaves a heartbeating update alone', () => {
|
||||
expect(
|
||||
expireStalledStatus(base({ phase: 'installing', startedAt: NOW - 60 * MIN, updatedAt: NOW - 3_000 }), NOW)
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('fails a status that stopped heartbeating (queued forever: status writes were failing)', () => {
|
||||
const out = expireStalledStatus(
|
||||
base({ startedAt: NOW - 8 * 24 * 60 * MIN, updatedAt: NOW - 8 * 24 * 60 * MIN }),
|
||||
NOW
|
||||
);
|
||||
expect(out?.phase).toBe('failed');
|
||||
expect(out?.error).toContain('queued');
|
||||
expect(out?.updatedAt).toBe(NOW);
|
||||
});
|
||||
|
||||
it('never touches terminal phases or a missing status', () => {
|
||||
expect(expireStalledStatus(null, NOW)).toBeNull();
|
||||
expect(expireStalledStatus(base({ phase: 'completed', updatedAt: NOW - 60 * MIN }), NOW)).toBeNull();
|
||||
expect(expireStalledStatus(base({ phase: 'failed', updatedAt: NOW - 60 * MIN }), NOW)).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user