diff --git a/.changeset/dev-wrapper-signal-teardown.md b/.changeset/dev-wrapper-signal-teardown.md new file mode 100644 index 0000000000..0091039fc9 --- /dev/null +++ b/.changeset/dev-wrapper-signal-teardown.md @@ -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 `, 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. diff --git a/.changeset/remote-tunnel-actual-port.md b/.changeset/remote-tunnel-actual-port.md new file mode 100644 index 0000000000..4ec4671f83 --- /dev/null +++ b/.changeset/remote-tunnel-actual-port.md @@ -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. diff --git a/.changeset/session-terminal-scrollback-clear.md b/.changeset/session-terminal-scrollback-clear.md new file mode 100644 index 0000000000..c951ddf6c3 --- /dev/null +++ b/.changeset/session-terminal-scrollback-clear.md @@ -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. diff --git a/packages/cli/src/commands/dashboard.ts b/packages/cli/src/commands/dashboard.ts index d33597cd51..7560bdadbf 100644 --- a/packages/cli/src/commands/dashboard.ts +++ b/packages/cli/src/commands/dashboard.ts @@ -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. diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index d76ae8dbdb..2b0b678f04 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -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`); /* diff --git a/packages/core/src/backup/backup.ts b/packages/core/src/backup/backup.ts index a05c6fe0be..65cd522322 100644 --- a/packages/core/src/backup/backup.ts +++ b/packages/core/src/backup/backup.ts @@ -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, 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, }); diff --git a/packages/dashboard/app/components/SessionTerminal.tsx b/packages/dashboard/app/components/SessionTerminal.tsx index f8d63a555a..ff0722edcf 100644 --- a/packages/dashboard/app/components/SessionTerminal.tsx +++ b/packages/dashboard/app/components/SessionTerminal.tsx @@ -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); diff --git a/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx b/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx index 8d0358cd1f..3b3143d22e 100644 --- a/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx +++ b/packages/dashboard/app/components/__tests__/SessionTerminal.test.tsx @@ -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(); + 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 }], diff --git a/packages/engine/src/__tests__/local-dashboard-port.test.ts b/packages/engine/src/__tests__/local-dashboard-port.test.ts new file mode 100644 index 0000000000..3d5073e90d --- /dev/null +++ b/packages/engine/src/__tests__/local-dashboard-port.test.ts @@ -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); + }); +}); diff --git a/packages/engine/src/__tests__/project-engine.test.ts b/packages/engine/src/__tests__/project-engine.test.ts index f35c0095cf..bf136ae54f 100644 --- a/packages/engine/src/__tests__/project-engine.test.ts +++ b/packages/engine/src/__tests__/project-engine.test.ts @@ -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, diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index c417dc971e..40bbecd6b3 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -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, diff --git a/packages/engine/src/local-dashboard-port.ts b/packages/engine/src/local-dashboard-port.ts new file mode 100644 index 0000000000..f7c5fa1d4e --- /dev/null +++ b/packages/engine/src/local-dashboard-port.ts @@ -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; +} diff --git a/packages/engine/src/project-engine.ts b/packages/engine/src/project-engine.ts index 4ac7d80b87..bf0cdae6b9 100644 --- a/packages/engine/src/project-engine.ts +++ b/packages/engine/src/project-engine.ts @@ -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()}`], }, }; } diff --git a/scripts/dev-with-memory.mjs b/scripts/dev-with-memory.mjs index 27fca59de3..6e522a6761 100644 --- a/scripts/dev-with-memory.mjs +++ b/scripts/dev-with-memory.mjs @@ -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 ` (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) {