Run PortOS's App Management update through the detached launcher

client/src/components/apps/tabs/UpdateTab.jsx

@@ -22,13 +22,17 @@ const STEP_LABELS = {
   build: 'Building client',
   restart: 'Restarting PortOS',
   restarting: 'Restarting PortOS',
+  verify: 'Verifying PortOS is back',
   complete: 'Complete'
 };
 
 function StepIndicator({ status }) {
   if (status === 'running') return <Loader size={14} className="text-port-accent animate-spin" />;
   if (status === 'done') return <Check size={14} className="text-port-success" />;
   if (status === 'error') return <XCircle size={14} className="text-port-error" />;
+  // The post-restart health check reports 'warning' when it could not confirm
+  // the server came back — the update finished, but the install may be down.
+  if (status === 'warning') return <AlertTriangle size={14} className="text-port-warning" />;
   return <span className="w-3.5 h-3.5 rounded-full border border-gray-600 inline-block" />;
 }
 

docs/MANAGED_APP_UPDATES.md

@@ -26,3 +26,18 @@ database migrations, generated assets, and build. Use the dedicated
 package-manager behavior is never invoked merely because PortOS updated it.
 When more than one is present, the configured **Update Command** wins, then
 `portos:update`, then the conventional script.
+
+## PortOS is itself a managed app
+
+The PortOS record appears in App Management like any other app, so **Update**
+there runs the same `appUpdater` flow described above. It is the one app whose
+update routine deletes the process running that flow, so it takes a different
+launcher: the conventional-script branch delegates to `executeUpdate()` in
+`server/services/updateExecutor.js`, whose double-fork keeps `update.sh` alive
+through its own `pm2 delete` step, and the trailing PM2 restart is skipped
+because the script starts the ecosystem itself. See
+[Self-Update Flow](SELF_UPDATE.md#every-portos-update-goes-through-the-detached-launcher).
+
+A custom **Update Command** on the PortOS record keeps the ordinary attached
+path, since delegating would silently run `update.sh` instead of the configured
+command.

docs/SELF_UPDATE.md

@@ -2,7 +2,7 @@
 
 How PortOS notices a new release and updates itself. PortOS is distributed software — many people run it, and a large share run it from a **personal fork**, so every step here is fork-aware. Breaking that assumption produces silent no-op updates.
 
-Code: `server/services/updateChecker.js`, `server/routes/update.js`, `server/lib/gitRemote.js`, `update.sh` / `update.ps1`, `client/src/components/apps/tabs/UpdateTab.jsx`.
+Code: `server/services/updateChecker.js`, `server/services/updateExecutor.js`, `server/services/appUpdater.js`, `server/routes/update.js`, `server/lib/gitRemote.js`, `server/lib/detachedSpawn.js`, `update.sh` / `update.ps1`, `scripts/verify-server-health.js`, `client/src/components/apps/tabs/UpdateTab.jsx`.
 
 ## Release polling always targets upstream
 
@@ -63,6 +63,24 @@ To prevent that confusion, `POST /api/update/execute` rejects fork runs with **4
 - the request body sets `acknowledgeFork: true`, or
 - `lastForkSync.fullName` matches `remoteInfo.fullName` (compared case-insensitively — GitHub owner/repo names are) and is less than 10 minutes old. The service computes this once as `status.forkSyncFresh` from `FORK_SYNC_FRESHNESS_MS`; the route and the UI both read that flag rather than re-implementing the time math.
 
+## Every PortOS update goes through the detached launcher
+
+`update.sh` deletes and restarts every PortOS PM2 entry. PM2's TreeKill walks **PPID**, so a script left attached to `portos-server` is killed by its own `pm2 delete` step — mid-list, before it can run the closing `pm2 start` — and the install is left headless. `spawnDetached`'s double-fork (`server/lib/detachedSpawn.js`) is what reparents the script to init so it survives; `executeUpdate()` in `server/services/updateExecutor.js` is the single launcher that applies it, along with the `STEP:` progress parsing, the still-running-script guard, and `recordUpdateResult()`.
+
+**PortOS is also a managed app**, so an update started from **App Management** reaches `update.sh` through `appUpdater.js` rather than `routes/update.js`. That path delegates to `executeUpdate()` for the PortOS record instead of spawning the script itself — a second detached-spawn implementation would be one more thing to keep in sync, and the attached one it replaced produced exactly the headless failure above (#5976). `appUpdater` also **skips its own `restart` step** for that case: the script runs `pm2 start ecosystem.config.cjs` itself, so restarting on top of it would be redundant and would race the script.
+
+Both entry points take the same atomic `setUpdateInProgress(true)` lock before launching, so they cannot run `update.sh` concurrently — and because that flag is what `subAgentSpawner`, `agentLifecycle` and `persistentMindSupervisor` gate on, holding it also stops a CoS agent from being spawned into a process the script is about to `pm2 delete` (#4124).
+
+A PortOS record carrying a custom `updateCommand`, or a `repoPath` that is not this checkout, keeps the ordinary attached path — delegating there would silently run `update.sh` instead of the configured command. That decision is logged rather than silent, since the attached path is the one that failed. The `repoPath` comparison resolves symlinks and case-folds on macOS/Windows: `repoPath` is user-editable and not force-synced, so a trailing slash or a different spelling must not be mistaken for a different checkout. Non-PortOS managed apps are unaffected and still get their own PM2 restart from `appUpdater`. The dashboard handoff it starts before that restart is PortOS-only — it opens the PortOS dashboard, so it would be meaningless after another app's update.
+
+## Post-update health verification
+
+`pm2 start` exiting 0 is not proof the server came back, and the process that would notice is the one that did not. Both platform scripts therefore close with a `verify` step that polls `/api/system/health` (`scripts/verify-server-health.js`) until it reports `ok` or the budget — `PORTOS_HEALTH_WAIT_MS`, default 120s — runs out. On failure they spend one more `pm2 start ecosystem.config.cjs` and then log the outcome loudly, with the manual recovery command.
+
+The probe tries the loopback HTTP mirror (`:5553`) first, then the API port over HTTP and HTTPS, because the listening scheme depends on whether a cert is provisioned; `/api/system/health` is in the always-public set, so it works with the optional instance password on. The recovery only fires when the probe fails, so it cannot make a healthy update worse.
+
+**When the probe still fails after the recovery, the scripts say so and exit non-zero** — the closing banner reads "Update applied, but PortOS is DOWN" instead of "Update Complete". The script outlives the server it restarts, so its exit status and the tail of `data/update.log` are the only signals a wrapper, a CI job, or an operator still has; printing a success banner over a confirmed-headless install is how the failure went unnoticed for hours in the first place.
+
 ## Syncing a fork
 
 `POST /api/update/sync-fork` shells out to:

scripts/verify-server-health.js

@@ -0,0 +1,167 @@
+#!/usr/bin/env node
+/**
+ * "Did portos-server actually come back after the update restarted it?"
+ *
+ * `update.sh` / `update.ps1` delete every PortOS PM2 entry and start it again.
+ * When that bracket half-fails, the install is left headless — and nothing else
+ * on the machine notices, because the thing that would have noticed is the
+ * server that did not come back (#5976: a no-op update left the install down
+ * for hours). The update script is the last PortOS process still running at
+ * that point, so the check has to live here.
+ *
+ * Usage as a CLI (what update.sh and update.ps1 call):
+ *   node scripts/verify-server-health.js
+ *
+ * Exit 0  → the server answered /api/system/health with status "ok".
+ * Exit 1  → it did not, within the budget. The caller re-runs `pm2 start`.
+ *
+ * Fails CLOSED, unlike `pm2-daemon-refresh.js`: an unreachable server is
+ * exactly the condition being detected, so anything short of a positive "ok"
+ * is reported as unhealthy. The recovery it triggers is one extra `pm2 start`,
+ * which cannot make an already-healthy install worse.
+ *
+ * `/api/system/health` is in the always-public set (`PUBLIC_API_PATHS`), so
+ * this works with the optional instance password on. All three candidate URLs
+ * are probed because the listening scheme/port depends on whether a cert is
+ * provisioned: HTTPS on :5555 plus the loopback HTTP mirror on :5553, or plain
+ * HTTP on :5555. Probing beats re-deriving the cert state — the answer we want
+ * is "is something serving", not "which URL should we advertise".
+ */
+
+import http from 'node:http';
+import https from 'node:https';
+import { PORTS } from '../server/lib/ports.js';
+import { isDirectlyInvoked } from './lib/directInvocation.js';
+
+const HEALTH_PATH = '/api/system/health';
+const DEFAULT_TIMEOUT_MS = 120_000;
+const DEFAULT_INTERVAL_MS = 2_000;
+const PROBE_TIMEOUT_MS = 5_000;
+
+/**
+ * Read a non-negative millisecond budget from the environment. `|| DEFAULT`
+ * would be wrong here: it collapses "unset" and "not a number" together with a
+ * deliberate `0` (fail fast, one pass and out), which `waitForHealthy`
+ * explicitly supports.
+ *
+ * @param {string|undefined} raw
+ * @param {number} fallback
+ * @returns {number}
+ */
+export function parseTimeoutMs(raw, fallback = DEFAULT_TIMEOUT_MS) {
+  if (raw === undefined || raw === null || String(raw).trim() === '') return fallback;
+  const parsed = Number(raw);
+  return Number.isFinite(parsed) && parsed >= 0 ? parsed : fallback;
+}
+
+/**
+ * The loopback URLs a healthy PortOS could be answering on, in the order worth
+ * trying: the plain-HTTP mirror first (always cert-free), then the API port
+ * over each scheme. Deduped so a plain-HTTP install (mirror port unbound,
+ * API port serving HTTP) does not probe the same URL twice.
+ *
+ * @param {{apiPort: number, mirrorPort: number}} ports
+ * @returns {string[]}
+ */
+export function healthProbeUrls({ apiPort, mirrorPort }) {
+  const urls = [
+    `http://127.0.0.1:${mirrorPort}${HEALTH_PATH}`,
+    `http://127.0.0.1:${apiPort}${HEALTH_PATH}`,
+    `https://127.0.0.1:${apiPort}${HEALTH_PATH}`,
+  ];
+  return [...new Set(urls)];
+}
+
+/**
+ * One request. Resolves true only on a 200 whose JSON body says status "ok" —
+ * a 502 from something else on the port, a hung socket, or a half-booted
+ * server that answers but not with "ok" all count as not-yet-healthy.
+ *
+ * `rejectUnauthorized: false` matches the rest of PortOS's loopback probing:
+ * the cert is issued for the Tailscale hostname, so 127.0.0.1 never validates,
+ * and there is no trust boundary to cross on loopback.
+ *
+ * @param {string} url
+ * @param {number} timeoutMs
+ * @returns {Promise<boolean>}
+ */
+export function probeHealth(url, timeoutMs = PROBE_TIMEOUT_MS) {
+  return new Promise((resolve) => {
+    const transport = url.startsWith('https:') ? https : http;
+    const req = transport.get(url, { timeout: timeoutMs, rejectUnauthorized: false }, (res) => {
+      if (res.statusCode !== 200) {
+        res.resume();
+        resolve(false);
+        return;
+      }
+      let body = '';
+      res.setEncoding('utf8');
+      res.on('data', (chunk) => { body += chunk; });
+      res.on('end', () => {
+        // A response that is not the health payload — a proxy error page, a
+        // truncated body — is not a healthy server.
+        try {
+          resolve(JSON.parse(body)?.status === 'ok');
+        } catch {
+          resolve(false);
+        }
+      });
+      res.on('error', () => resolve(false));
+    });
+    req.on('timeout', () => { req.destroy(); resolve(false); });
+    req.on('error', () => resolve(false));
+  });
+}
+
+/**
+ * Poll the candidate URLs until one reports healthy or the budget runs out.
+ * Clock and probe are injected so the timeout contract is testable without
+ * real sleeps or a real server.
+ *
+ * @param {object} options
+ * @param {string[]} options.urls
+ * @param {number} [options.timeoutMs] - total budget across all attempts
+ * @param {number} [options.intervalMs] - pause between full passes
+ * @param {(url: string) => Promise<boolean>} [options.probe]
+ * @param {() => number} [options.now]
+ * @param {(ms: number) => Promise<void>} [options.sleep]
+ * @returns {Promise<{healthy: boolean, url: string|null, attempts: number}>}
+ */
+export async function waitForHealthy({
+  urls,
+  timeoutMs = DEFAULT_TIMEOUT_MS,
+  intervalMs = DEFAULT_INTERVAL_MS,
+  probe = probeHealth,
+  now = Date.now,
+  sleep = (ms) => new Promise((r) => setTimeout(r, ms)),
+}) {
+  const deadline = now() + timeoutMs;
+  let attempts = 0;
+  // Always make one full pass, even with a zero/expired budget — the check is
+  // worthless if it can report "unhealthy" without having asked.
+  for (;;) {
+    for (const url of urls) {
+      attempts += 1;
+      if (await probe(url)) return { healthy: true, url, attempts };
+    }
+    if (now() >= deadline) return { healthy: false, url: null, attempts };
+    await sleep(intervalMs);
+  }
+}
+
+async function runCli() {
+  const apiPort = Number(process.env.PORT) || PORTS.API;
+  const mirrorPort = Number(process.env.PORTOS_HTTP_PORT) || PORTS.API_LOCAL;
+  const timeoutMs = parseTimeoutMs(process.env.PORTOS_HEALTH_WAIT_MS);
+  const urls = healthProbeUrls({ apiPort, mirrorPort });
+
+  const result = await waitForHealthy({ urls, timeoutMs });
+  if (result.healthy) {
+    console.log(`✅ PortOS is serving ${HEALTH_PATH} (${result.url})`);
+    return 0;
+  }
+  console.error(`❌ PortOS did not answer ${HEALTH_PATH} within ${Math.round(timeoutMs / 1000)}s (${result.attempts} attempts)`);
+  return 1;
+}
+
+if (isDirectlyInvoked(import.meta.url)) process.exit(await runCli());

scripts/verify-server-health.test.js

@@ -0,0 +1,132 @@
+import { describe, expect, it, vi } from 'vitest';
+import { createServer } from 'node:http';
+import { healthProbeUrls, parseTimeoutMs, probeHealth, waitForHealthy } from './verify-server-health.js';
+
+/** Start a loopback server that answers one canned response, and return its URL. */
+async function withServer(handler, run) {
+  const server = createServer(handler);
+  await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve));
+  try {
+    return await run(`http://127.0.0.1:${server.address().port}/api/system/health`);
+  } finally {
+    await new Promise((resolve) => server.close(resolve));
+  }
+}
+
+describe('post-update server health verification', () => {
+  it('probes the loopback mirror before the API port, and dedupes a plain-HTTP install', () => {
+    expect(healthProbeUrls({ apiPort: 5555, mirrorPort: 5553 })).toEqual([
+      'http://127.0.0.1:5553/api/system/health',
+      'http://127.0.0.1:5555/api/system/health',
+      'https://127.0.0.1:5555/api/system/health',
+    ]);
+    // No cert provisioned: the mirror never binds and the API port serves HTTP,
+    // so the two http candidates collapse into one.
+    expect(healthProbeUrls({ apiPort: 5555, mirrorPort: 5555 })).toEqual([
+      'http://127.0.0.1:5555/api/system/health',
+      'https://127.0.0.1:5555/api/system/health',
+    ]);
+  });
+
+  it('accepts only a 200 that actually reports status "ok"', async () => {
+    const ok = await withServer((_req, res) => {
+      res.writeHead(200, { 'content-type': 'application/json' });
+      res.end(JSON.stringify({ status: 'ok', version: '0.0.0-test' }));
+    }, (url) => probeHealth(url));
+    expect(ok).toBe(true);
+
+    // A half-booted server, or something else squatting the port, answers —
+    // treating that as healthy would skip the recovery the caller exists for.
+    const degraded = await withServer((_req, res) => {
+      res.writeHead(200, { 'content-type': 'application/json' });
+      res.end(JSON.stringify({ status: 'degraded' }));
+    }, (url) => probeHealth(url));
+    expect(degraded).toBe(false);
+
+    const notJson = await withServer((_req, res) => {
+      res.writeHead(200, { 'content-type': 'text/html' });
+      res.end('<html>proxy error</html>');
+    }, (url) => probeHealth(url));
+    expect(notJson).toBe(false);
+
+    const serverError = await withServer((_req, res) => {
+      res.writeHead(503);
+      res.end('');
+    }, (url) => probeHealth(url));
+    expect(serverError).toBe(false);
+  });
+
+  it('reports unhealthy for a port nothing is listening on', async () => {
+    // Bind then release so the port is known-free rather than guessed.
+    const port = await withServer(() => {}, (url) => Number(new URL(url).port));
+    expect(await probeHealth(`http://127.0.0.1:${port}/api/system/health`, 1_000)).toBe(false);
+  });
+
+  it('keeps polling a booting server until it answers, without spending the whole budget', async () => {
+    let clock = 0;
+    const probe = vi.fn()
+      .mockResolvedValueOnce(false)
+      .mockResolvedValueOnce(false)
+      .mockResolvedValueOnce(true);
+
+    const result = await waitForHealthy({
+      urls: ['http://127.0.0.1:5553/api/system/health'],
+      timeoutMs: 120_000,
+      intervalMs: 2_000,
+      probe,
+      now: () => clock,
+      sleep: async (ms) => { clock += ms; },
+    });
+
+    expect(result).toEqual({ healthy: true, url: 'http://127.0.0.1:5553/api/system/health', attempts: 3 });
+    expect(clock).toBe(4_000);
+  });
+
+  it('gives up once the budget is spent, after asking at least once', async () => {
+    let clock = 0;
+    const probe = vi.fn().mockResolvedValue(false);
+
+    const result = await waitForHealthy({
+      urls: ['http://a/health', 'http://b/health'],
+      timeoutMs: 5_000,
+      intervalMs: 2_000,
+      probe,
+      now: () => clock,
+      sleep: async (ms) => { clock += ms; },
+    });
+
+    expect(result.healthy).toBe(false);
+    expect(result.url).toBe(null);
+    // The contract is "kept asking for the whole budget, then stopped", not a
+    // particular pass schedule: every candidate is asked on every pass, at
+    // least one full pass happened, and it only gave up past the deadline.
+    expect(probe.mock.calls.length % 2).toBe(0);
+    expect(probe.mock.calls.length).toBeGreaterThanOrEqual(2);
+    expect(clock).toBeGreaterThanOrEqual(5_000);
+  });
+
+  it('reads a deliberate zero budget from the environment instead of falling back', () => {
+    // `|| DEFAULT` would turn a fail-fast 0 — and a typo — into a silent 120s.
+    expect(parseTimeoutMs('0')).toBe(0);
+    expect(parseTimeoutMs('30000')).toBe(30_000);
+    expect(parseTimeoutMs(undefined)).toBe(120_000);
+    expect(parseTimeoutMs('')).toBe(120_000);
+    expect(parseTimeoutMs('12O')).toBe(120_000);
+    expect(parseTimeoutMs('-1')).toBe(120_000);
+  });
+
+  it('still makes one full pass when the budget is already exhausted', async () => {
+    const probe = vi.fn().mockResolvedValue(false);
+
+    const result = await waitForHealthy({
+      urls: ['http://a/health'],
+      timeoutMs: 0,
+      probe,
+      now: () => 0,
+      sleep: async () => {},
+    });
+
+    expect(result.healthy).toBe(false);
+    expect(probe).toHaveBeenCalledTimes(1);
+  });
+});