fix: audit fallout — tunnel port, orphaned dev processes, scrollback clear

Auditing for repeats of the `pnpm dev --tunnel` bugs turned up the same
mistakes elsewhere.

Remote tunnels assumed 4040. ProjectEngine's Cloudflare quick tunnel
hardcoded http://localhost:4040, so a dashboard on an explicit --port, a PORT
override, or runDashboard's EADDRINUSE rebind published a PUBLIC tunnel to
whatever else held 4040. The dashboard now records its bound port
(setLocalDashboardPort, from both runDashboard and headless serve) and the
tunnel reads it, keeping 4040 only as the pre-report default.
register-discovery-routes already derived its port from req.socket.localPort
and is untouched.

Stopping the dev wrapper orphaned everything it started. It installed no
signal handlers, so teardown only ran from the child's close handler:
signalling the wrapper left the dev server AND its cloudflared alive —
observed twice, four surviving processes each time, including a public
trycloudflare URL still serving a dev server believed to be down. Ctrl-C hid
it by signalling the whole process group.

SessionTerminal appended scrollback instead of clearing first, though the
server sends it as a separate frame precisely so the client can clear. Latent
today because every reattach builds a fresh xterm; a duplicated-history bug
the moment an in-place reconnect appears.

And BackupManager's centralDbPath is gone: written, never read, and a
leftover of the removed SQLite backup — the same class of stale artifact that
onboarding was using as evidence about a Postgres install.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-08-18 21:09:03 -07:00
parent b67e3aa8bc
commit 16e63462cc
14 changed files with 257 additions and 13 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Stopping `pnpm dev` now also stops its dev server and tunnel instead of orphaning them.
category: fix
dev: `scripts/dev-with-memory.mjs` installed no signal handlers; teardown lived only in the child's `close` handler. Signalling the wrapper directly (`kill <pid>`, or any supervisor-style stop) killed it and left the dev server and its `cloudflared` running — observed twice while debugging, four surviving processes each time, including a live public trycloudflare URL still serving the dev server after it was believed down. Interactive Ctrl-C masked this because the terminal signals the whole process group. SIGINT/SIGTERM/SIGHUP now stop the tunnel, forward the signal to the child, and exit on its close with a 10s cap so a wedged child cannot pin the terminal.

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Remote tunnels now target the port the dashboard is really on, instead of assuming 4040.
category: fix
dev: An audit for repeats of the `pnpm dev --tunnel` port bug found the same mistake shipped in remote access: `ProjectEngine`'s Cloudflare quick tunnel hardcoded `http://localhost:4040`, so a dashboard started with `--port`, with a `PORT` override, or rebound to an ephemeral port by `runDashboard`'s EADDRINUSE path published a public tunnel to whatever else owned 4040 — another app, another Fusion, or nothing. `setLocalDashboardPort()` records the bound port (from both `runDashboard` and headless `serve`) and `getLocalDashboardPort()` supplies the tunnel target, defaulting to 4040 only while nothing has reported. `register-discovery-routes` already derived its port from `req.socket.localPort` and is unchanged.

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: The agent session terminal clears before replaying scrollback, as its protocol intended.
category: internal
dev: `cli-session-ws.ts` sends scrollback as its own frame explicitly "so the client can clear before replay", but `SessionTerminal` handled `scrollback` identically to `data` and appended. Latent rather than live — every reattach path there rebuilds a fresh xterm via `reattachEpoch` — but it becomes the duplicated-history bug just fixed in the PTY terminal the moment an in-place reconnect is added. Also drops dead `centralDbPath` plumbing in `BackupManager`/`createBackupManager`: it was written, never read (PgBackupManager takes only `includeCentral`), and a leftover of the removed SQLite file-copy backup — the same kind of stale artifact whose presence was being used as evidence about a Postgres install in onboarding.

View File

@@ -100,6 +100,7 @@ import {
createFusionAuthStorage,
createFusionModelRegistry,
refreshFusionModelRegistry,
setLocalDashboardPort,
} from "@fusion/engine";
import { setHostTaskStore, clearHostTaskStores } from "../extension.js";
import { DefaultPackageManager, SettingsManager, discoverAndLoadExtensions, createExtensionRuntime } from "@earendil-works/pi-coding-agent";
@@ -2964,6 +2965,14 @@ export async function runDashboard(port: number, opts: { paused?: boolean; dev?:
logSink.warn(`Port ${selectedPort} in use, using ${actualPort} instead`, "dashboard");
}
/*
FNXC:RemoteAccess 2026-08-19-04:00:
Publish the bound port to the engine so remote tunnels target THIS dashboard. Before this they
pointed at a hardcoded localhost:4040, so a dashboard on any other port (explicit --port, PORT,
or the EADDRINUSE rebind just above) tunnelled whatever else owned 4040.
*/
setLocalDashboardPort(actualPort);
/*
FNXC:DevTunnel 2026-08-19-02:05: report the REAL port to the dev supervisor (no-op without an
IPC channel, i.e. every non-`pnpm dev` launch). See DEV_SERVER_LISTENING_MESSAGE.

View File

@@ -35,6 +35,7 @@ import {
createFusionAuthStorage,
createFusionModelRegistry,
refreshFusionModelRegistry,
setLocalDashboardPort,
} from "@fusion/engine";
import { setHostTaskStore, clearHostTaskStores } from "../extension.js";
import { resolveServeDaemonToken } from "./serve-daemon-token.js";
@@ -1105,6 +1106,9 @@ export async function runServe(
});
const actualPort = (server.address() as AddressInfo).port;
// FNXC:RemoteAccess 2026-08-19-04:00: headless serve must publish its bound port too, or a remote
// tunnel started from it targets a hardcoded 4040. See local-dashboard-port.
setLocalDashboardPort(actualPort);
logPhase(`startup phase time-to-listen: ${Date.now() - serveStartedAt}ms`);
/*

View File

@@ -1,7 +1,6 @@
import { join } from "node:path";
import { resolveGlobalDir } from "../config/global-settings.js";
import { CronExpressionParser } from "cron-parser";
import { getDefaultCentralDbPath } from "../central/central-db.js";
import { PgBackupManager, type PgBackupPair, type PgDumpResult } from "../postgres/pg-backup.js";
import { resolveBackend } from "../postgres/backend-resolver.js";
import { getActiveEmbeddedRuntimeUrl } from "../postgres/active-backend-registry.js";
@@ -45,7 +44,6 @@ export interface BackupPairInfo {
export interface BackupOptions {
backupDir?: string;
retention?: number;
centralDbPath?: string;
includeCentralDb?: boolean;
/**
* FNXC:SqliteFinalRemoval 2026-06-26-00:15:
@@ -69,7 +67,6 @@ export class BackupManager {
private fusionDir: string;
private backupDir: string;
private retention: number;
private centralDbPath: string;
private includeCentralDb: boolean;
private readonly pgManager: PgBackupManager;
@@ -77,7 +74,6 @@ export class BackupManager {
this.fusionDir = fusionDir;
this.backupDir = options?.backupDir ?? ".fusion/backups";
this.retention = options?.retention ?? 7;
this.centralDbPath = options?.centralDbPath ?? join(this.fusionDir, "..", ".fusion", "fusion-central.db");
this.includeCentralDb = options?.includeCentralDb ?? true;
const connectionString = options?.connectionString ?? resolveBackendConnectionString();
if (!connectionString) {
@@ -233,12 +229,13 @@ export function createBackupManager(
settings?: Partial<Settings>,
connectionString?: string,
): BackupManager {
let centralDbPath: string;
try {
centralDbPath = getDefaultCentralDbPath();
} catch {
centralDbPath = join(fusionDir, "..", ".fusion", "fusion-central.db");
}
/*
FNXC:SqliteFinalRemoval 2026-08-19-04:00:
The `fusion-central.db` path this used to compute and pass through was never read: PgBackupManager
takes only the includeCentral flag. It was a leftover of the SQLite file-copy backup that
VAL-REMOVAL-003 deleted, and keeping it invited the mistake that shipped elsewhere — treating that
file's presence as evidence about a Postgres install (see onboard-autolaunch).
*/
/*
* FNXC:SqliteFinalRemoval 2026-06-26:
@@ -253,7 +250,6 @@ export function createBackupManager(
return new BackupManager(fusionDir, {
backupDir: canonicalizeBackupDir(settings?.autoBackupDir),
retention: settings?.autoBackupRetention,
centralDbPath,
includeCentralDb: true,
connectionString: resolvedConnectionString,
});

View File

@@ -718,7 +718,23 @@ export function SessionTerminal({
return;
}
switch (msg.type) {
case "scrollback":
case "scrollback": {
if (typeof msg.data !== "string") return;
/*
FNXC:TerminalSharing 2026-08-19-04:00:
Clear before replaying. The server sends scrollback as its own frame precisely so the
client can (see cli-session-ws.ts), but this handler used to treat it exactly like
`data` and append. That is safe only because every reattach path here rebuilds a fresh
xterm via reattachEpoch — the moment anyone adds an in-place reconnect, appending a full
replay onto a terminal that still shows that history duplicates it, which is precisely
the duplicated-prompt bug fixed in the PTY terminal.
*/
term.reset();
const text = decodeBase64ToString(msg.data);
const byteLen = text.length;
term.write(text, () => ackBytes(byteLen));
break;
}
case "data": {
if (typeof msg.data !== "string") return;
const text = decodeBase64ToString(msg.data);

View File

@@ -114,6 +114,8 @@ const mockTerm = {
hasSelection: vi.fn(() => false),
getSelection: vi.fn(() => ""),
write: vi.fn((_data: string, cb?: () => void) => cb?.()),
// xterm's Terminal has reset(); the scrollback handler clears with it before replaying.
reset: vi.fn(),
refresh: vi.fn(),
dispose: vi.fn(),
unicode: { activeVersion: "6" },
@@ -247,6 +249,28 @@ describe("SessionTerminal", () => {
await waitFor(() => expect(mockTerm.write).toHaveBeenCalledWith("hello", expect.any(Function)));
});
/*
FNXC:TerminalSharing 2026-08-19-04:00:
The server sends scrollback as its own frame so the client can CLEAR before replaying it; this
handler used to append it exactly like `data`. That is only harmless while every reattach builds a
fresh xterm — add an in-place reconnect and a full replay lands on top of history the terminal
still shows, which is the duplicated-prompt bug fixed in the PTY terminal.
*/
it("clears before replaying scrollback, but never on live data", async () => {
render(<SessionTerminal sessionId="s1" />);
await waitFor(() => expect(FakeWS.instances.length).toBe(1));
const ws = FakeWS.instances[0];
ws.onmessage?.({ data: JSON.stringify({ type: "scrollback", data: Buffer.from("history", "utf8").toString("base64") }) });
await waitFor(() => expect(mockTerm.reset).toHaveBeenCalledTimes(1));
mockTerm.reset.mockClear();
ws.onmessage?.({ data: JSON.stringify({ type: "data", data: Buffer.from("live", "utf8").toString("base64") }) });
await waitFor(() => expect(mockTerm.write).toHaveBeenCalledWith("live", expect.any(Function)));
// Live output must never wipe the screen.
expect(mockTerm.reset).not.toHaveBeenCalled();
});
it.each([
["read-only", { readOnly: true }],
["idle", { mode: "idle" as const }],

View File

@@ -0,0 +1,45 @@
import { describe, it, expect, beforeEach } from "vitest";
import {
DEFAULT_DASHBOARD_PORT,
getLocalDashboardPort,
setLocalDashboardPort,
resetLocalDashboardPortForTests,
} from "../local-dashboard-port.js";
/*
FNXC:RemoteAccess 2026-08-19-04:00:
Remote tunnels targeted a hardcoded http://localhost:4040, so a dashboard on any other port —
`--port`, a PORT override, or the EADDRINUSE rebind to an ephemeral port — published whatever ELSE
owned 4040 under a URL the operator believed was theirs. The identical mistake in `pnpm dev
--tunnel` published a container's own Fusion instead of the dev server.
*/
describe("local dashboard port", () => {
beforeEach(() => {
resetLocalDashboardPortForTests();
});
it("falls back to the historical default before anything reports", () => {
expect(getLocalDashboardPort()).toBe(DEFAULT_DASHBOARD_PORT);
expect(DEFAULT_DASHBOARD_PORT).toBe(4040);
});
it("returns the port the dashboard actually bound", () => {
setLocalDashboardPort(51234);
expect(getLocalDashboardPort()).toBe(51234);
});
it("ignores values that cannot be a bound port", () => {
setLocalDashboardPort(4041);
for (const bogus of [0, -1, Number.NaN, Number.POSITIVE_INFINITY]) {
setLocalDashboardPort(bogus);
// A bad report must never erase a good one, or the tunnel silently reverts to 4040.
expect(getLocalDashboardPort()).toBe(4041);
}
});
it("takes the latest report, so a restart onto a new port is followed", () => {
setLocalDashboardPort(4041);
setLocalDashboardPort(51234);
expect(getLocalDashboardPort()).toBe(51234);
});
});

View File

@@ -17,6 +17,7 @@ import {
} from "../merge/merger-ai.js";
import { runtimeLog } from "../logger.js";
import { TunnelProcessManager } from "../remote-access/tunnel-process-manager.js";
import { setLocalDashboardPort, resetLocalDashboardPortForTests } from "../local-dashboard-port.js";
import { NtfyNotifier } from "../util/notifier.js";
import { NotificationService, OAuthAlertStateStore, OAuthExpiryMonitor, OAuthValidityLogger } from "../notification/index.js";
@@ -1233,6 +1234,7 @@ describe("ProjectEngine remote lifecycle quick tunnel mode", () => {
provider: "cloudflare",
quickTunnel: true,
executablePath: "cloudflared",
// Nothing reported a port, so the historical default stands.
args: ["tunnel", "--url", "http://localhost:4040"],
}),
);
@@ -1241,6 +1243,55 @@ describe("ProjectEngine remote lifecycle quick tunnel mode", () => {
startSpy.mockRestore();
});
/*
FNXC:RemoteAccess 2026-08-19-04:00:
The target was hardcoded to 4040, so a dashboard on any other port — an explicit --port, a PORT
override, or runDashboard's EADDRINUSE rebind to an ephemeral port — published a public tunnel to
whatever ELSE owned 4040 (another Fusion, another app, or nothing). The dashboard reports its
bound port and the tunnel must follow it.
*/
it("targets the port the dashboard actually bound", async () => {
const quickTunnelSettings = {
...baseSettings,
remoteAccess: {
...baseRemoteAccess,
providers: {
...baseRemoteAccess.providers,
cloudflare: {
...baseRemoteAccess.providers.cloudflare,
quickTunnel: true,
tunnelName: "",
tunnelToken: null,
ingressUrl: "",
},
},
},
};
const mockStore = createMockStore(quickTunnelSettings);
mocks.currentStore = mockStore.store;
const startSpy = vi.spyOn(TunnelProcessManager.prototype, "start").mockResolvedValue(undefined);
setLocalDashboardPort(51234);
try {
const engine = createEngine();
await engine.start();
await engine.startRemoteTunnel();
expect(startSpy).toHaveBeenCalledWith(
"cloudflare",
expect.objectContaining({
args: ["tunnel", "--url", "http://localhost:51234"],
}),
);
await engine.stop();
} finally {
resetLocalDashboardPortForTests();
startSpy.mockRestore();
}
});
it("surfaces runtime prerequisite missing when cloudflared is unavailable in quick tunnel mode", async () => {
mocks.execFile.mockImplementation((
_file: string,

View File

@@ -8,6 +8,12 @@ export {
export { reloadExemptTools, addToExemptTools, getExemptToolNames, evaluateAgentActionGate, resolveGateOutcome } from "./agents/agent-action-gate.js";
export type { AgentActionGateContext, AgentActionGateDecision } from "./agents/agent-action-gate.js";
export { createFusionAuthStorage, createFusionModelRegistry } from "./auth/auth-storage.js";
export {
DEFAULT_DASHBOARD_PORT,
getLocalDashboardPort,
setLocalDashboardPort,
resetLocalDashboardPortForTests,
} from "./local-dashboard-port.js";
export {
DEFAULT_MODEL_REGISTRY_REFRESH_TIMEOUT_MS,
boundExistingModelRegistryRefresh,

View File

@@ -0,0 +1,38 @@
/*
FNXC:RemoteAccess 2026-08-19-04:00:
The port this process's dashboard is actually serving on.
Remote tunnels used to point at a hardcoded `http://localhost:4040`. That is only correct when the
dashboard happens to hold 4040: `fn dashboard --port`, a `PORT` override, or the EADDRINUSE path in
runDashboard (which rebinds to an ephemeral port) all move it, and the tunnel then published
whatever ELSE owned 4040 — another Fusion, another app, or nothing — under a URL the operator
believes is theirs. The identical mistake in `pnpm dev --tunnel` published a container's own Fusion
instead of the dev server, which is what made it worth hunting down here.
The dashboard records its bound port here as soon as it is listening; the engine reads it when
building tunnel arguments. Same process in every shipping configuration (the dashboard route calls
`engine.startRemoteTunnel()` in-process), so a module-scoped value is the whole mechanism. The 4040
default only applies before anything has reported, which preserves the previous behaviour rather
than inventing a new failure.
*/
/** Port assumed when nothing has reported one — the historical dashboard default. */
export const DEFAULT_DASHBOARD_PORT = 4040;
let reportedPort: number | undefined;
/** Record the port the dashboard is listening on. Called once the server is bound. */
export function setLocalDashboardPort(port: number): void {
if (!Number.isFinite(port) || port <= 0) return;
reportedPort = Math.floor(port);
}
/** The dashboard's reported port, or the historical default when it has not reported yet. */
export function getLocalDashboardPort(): number {
return reportedPort ?? DEFAULT_DASHBOARD_PORT;
}
/** Test seam: forget any reported port. */
export function resetLocalDashboardPortForTests(): void {
reportedPort = undefined;
}

View File

@@ -126,6 +126,7 @@ import { finalizeProvenAutoMergeTask } from "./merge/auto-merge-finalization.js"
import { isTransientError } from "./errors/transient-error-detector.js";
import { classifyTransientMergeError, MAX_AUTO_MERGE_TRANSIENT_RETRIES } from "./errors/transient-merge-error-classifier.js";
import { TunnelProcessManager } from "./remote-access/tunnel-process-manager.js";
import { getLocalDashboardPort } from "./local-dashboard-port.js";
import {
deliverPostgresMigrationCompleteNoticeIfNeeded,
deliverPostgresMigrationNoticeIfNeeded,
@@ -2908,7 +2909,9 @@ export class ProjectEngine {
provider: "cloudflare",
quickTunnel: true,
executablePath: "cloudflared",
args: ["tunnel", "--url", "http://localhost:4040"],
// FNXC:RemoteAccess 2026-08-19-04:00: target the port the dashboard actually bound, not a
// hardcoded 4040 that publishes whatever else happens to own it. See local-dashboard-port.
args: ["tunnel", "--url", `http://localhost:${getLocalDashboardPort()}`],
},
};
}

View File

@@ -315,6 +315,37 @@ async function warnIfDistStale() {
}
}
/*
FNXC:DevWorkflow 2026-08-19-04:00:
Stop the dev server AND the tunnel when this supervisor is signalled. Teardown used to live only in
the child's `close` handler, so `kill <wrapper-pid>` (or any supervisor-style stop) killed the
wrapper and left the dev server and its cloudflared running as orphans — observed twice, four
processes surviving each time. Interactive Ctrl-C hid it because the terminal signals the whole
process group; anything that signals only this process did not.
An orphaned tunnel is the dangerous half: a public trycloudflare URL keeps serving the dev server
after the operator believes it is down. Forward the signal, give the child a moment to exit on its
own, then leave.
*/
let shuttingDown = false;
for (const signal of ["SIGINT", "SIGTERM", "SIGHUP"]) {
process.on(signal, () => {
if (shuttingDown) return;
shuttingDown = true;
devTunnel?.stop?.();
if (appChild && !appChild.killed) {
appChild.kill(signal === "SIGHUP" ? "SIGTERM" : signal);
// The child owns a graceful shutdown path (draining agents, stopping Postgres); give it room,
// then stop waiting so a wedged child cannot pin the terminal open.
const forceExit = setTimeout(() => process.exit(0), 10_000);
forceExit.unref?.();
appChild.once("close", () => process.exit(0));
return;
}
process.exit(0);
});
}
await warnIfDistStale();
if (!prebuildCommand) {