From 3b9c457ae2844a29b9d5b0616322b5154fdb5afd Mon Sep 17 00:00:00 2001 From: theFong Date: Sun, 4 Oct 2026 19:27:22 -0700 Subject: [PATCH] fix(server): keep bb connect's account status reads available during a server move freeze The server move freeze rejected every non-GET request outside /api/v1/server/move with 503 server_moving, including plugin RPCs, which are always POST. bb connect long-polls bb-account.v1.waitForStatusChange (returning at least every 25 s) and reads bb-account.v1.status and bb-account.v1.connectCredential. Its account client maps any 503 to AccountUnavailableError, so AccountLink treated the frozen server as signed out and closed the tunnel, dropping every daemon session that reaches the server through bb connect, including the move target. The move then failed at "Send data to : Host is not connected". Exempt exactly those three read-only bb-account RPC paths from the freeze. Writable bb-account RPCs (fetch, adoptConnectCredential, confirmRefusedCredential, login, redeem, sign-out) stay frozen. Fixes #4880 --- .../server/src/services/server-move/freeze.ts | 13 +++- .../test/server-move/coordinator.test.ts | 75 ++++++++++++++++++- apps/server/test/server-move/freeze.test.ts | 64 +++++++++++++++- docs/server-move-plan.md | 5 +- 4 files changed, 153 insertions(+), 4 deletions(-) diff --git a/apps/server/src/services/server-move/freeze.ts b/apps/server/src/services/server-move/freeze.ts index 5206d890362..ee570e73dc8 100644 --- a/apps/server/src/services/server-move/freeze.ts +++ b/apps/server/src/services/server-move/freeze.ts @@ -3,6 +3,13 @@ import { serverMovingError } from "./freeze-state.js"; const READ_METHODS: ReadonlySet = new Set(["GET", "HEAD", "OPTIONS"]); const SERVER_MOVE_ROUTE_PATTERN = /^\/api\/v1\/server\/move(?:\/|$)/u; +const CONNECT_ACCOUNT_STATUS_RPC_PATHS: ReadonlySet = new Set( + [ + "bb-account.v1.status", + "bb-account.v1.waitForStatusChange", + "bb-account.v1.connectCredential", + ].map((method) => `/api/v1/plugins/bb-account/rpc/${method}`), +); export interface ServerMoveFreezeState { isFrozen(): boolean; @@ -12,7 +19,11 @@ export function isServerMoveFreezeExempt( method: string, path: string, ): boolean { - return READ_METHODS.has(method) || SERVER_MOVE_ROUTE_PATTERN.test(path); + return ( + READ_METHODS.has(method) || + SERVER_MOVE_ROUTE_PATTERN.test(path) || + CONNECT_ACCOUNT_STATUS_RPC_PATHS.has(path) + ); } export function serverMoveFreezeMiddleware( diff --git a/apps/server/test/server-move/coordinator.test.ts b/apps/server/test/server-move/coordinator.test.ts index 941e81ba06d..cf75492541d 100644 --- a/apps/server/test/server-move/coordinator.test.ts +++ b/apps/server/test/server-move/coordinator.test.ts @@ -13,8 +13,11 @@ import { } from "@bb/server-archive"; import type { ServerMoveStatus } from "@bb/server-contract"; import { createDeferredPromise } from "@bb/test-helpers"; +import { Hono } from "hono"; import { describe, expect, it, vi } from "vitest"; +import { createServerErrorHandler } from "../../src/errors.js"; import { createServerMoveCoordinator } from "../../src/services/server-move/coordinator.js"; +import { serverMoveFreezeMiddleware } from "../../src/services/server-move/freeze.js"; import { isServerMoveFrozen, isServerMoveSnapshotFenced, @@ -32,7 +35,11 @@ import { type FakeDaemonReply, } from "../helpers/server-move.js"; import { seedHost, seedPrimaryHost } from "../helpers/seed.js"; -import { withTestHarness, type TestAppHarness } from "../helpers/test-app.js"; +import { + testLogger, + withTestHarness, + type TestAppHarness, +} from "../helpers/test-app.js"; const OLD = "host-old"; const NEW = "host-new"; @@ -634,6 +641,72 @@ describe("server move coordinator", () => { ).toEqual({ serverUrl: CONNECT_URL, serverHeaders: grantHeaders }); })); + it("keeps bb connect's bb account status reads open while a bb connect move is frozen", () => + withTestHarness(async (harness) => { + seedTopology(harness); + const base = createTestServerMoveEnvironment(harness); + const { events } = base; + const accountRpc = new Hono(); + accountRpc.onError(createServerErrorHandler(testLogger)); + accountRpc.use( + "/api/v1/*", + serverMoveFreezeMiddleware({ isFrozen: () => coordinator.isFrozen() }), + ); + accountRpc.post("/api/v1/plugins/:id/rpc/:method", (context) => + context.json({ ok: true, result: null }), + ); + const whileFrozen: Array<[string, boolean, number]> = []; + const coordinator = createServerMoveCoordinator({ + ...base.environment, + exportArchive: async (args) => { + for (const method of [ + "bb-account.v1.waitForStatusChange", + "bb-account.v1.status", + "bb-account.v1.connectCredential", + "bb-account.v1.fetch", + ]) { + const response = await accountRpc.request( + `/api/v1/plugins/bb-account/rpc/${method}`, + { method: "POST", body: JSON.stringify({ afterRevision: 1 }) }, + ); + whileFrozen.push([method, coordinator.isFrozen(), response.status]); + } + return base.environment.exportArchive(args); + }, + resolveMode: async () => ({ + mode: "connect", + connectHandle: "laptop", + serverUrl: CONNECT_URL, + }), + resolveServerHostGrant: async () => ({ + serverUrl: CONNECT_URL, + headers: { "x-bb-connect-machine": "bbcm_laptop" }, + }), + }); + registerFakeDaemon(harness, { events, hostId: OLD, handle: probeReply }); + registerFakeDaemon(harness, { + events, + hostId: WORKER, + handle: probeReply, + }); + registerFakeDaemon(harness, { + events, + hostId: NEW, + handle: targetReply, + }); + + await coordinator.start({ ...START_DIRECT, serverUrl: null }); + await expect.poll(() => events.includes("retire")).toBe(true); + + expect(whileFrozen).toEqual([ + ["bb-account.v1.waitForStatusChange", true, 200], + ["bb-account.v1.status", true, 200], + ["bb-account.v1.connectCredential", true, 200], + ["bb-account.v1.fetch", true, 503], + ]); + expect(coordinator.getStatus()?.state).toBe("completed"); + })); + it.each([ { name: "the target inspect during update-target", diff --git a/apps/server/test/server-move/freeze.test.ts b/apps/server/test/server-move/freeze.test.ts index 3c0b53fc754..004e0a09466 100644 --- a/apps/server/test/server-move/freeze.test.ts +++ b/apps/server/test/server-move/freeze.test.ts @@ -1,10 +1,15 @@ import { Hono } from "hono"; import { describe, expect, it } from "vitest"; import { createServerErrorHandler } from "../../src/errors.js"; -import { serverMoveFreezeMiddleware } from "../../src/services/server-move/freeze.js"; +import { + isServerMoveFreezeExempt, + serverMoveFreezeMiddleware, +} from "../../src/services/server-move/freeze.js"; import { readJson } from "../helpers/json.js"; import { testLogger } from "../helpers/test-app.js"; +const BB_ACCOUNT_RPC = "/api/v1/plugins/bb-account/rpc"; + function frozenApp(state: { frozen: boolean }): Hono { const app = new Hono(); app.onError(createServerErrorHandler(testLogger)); @@ -55,4 +60,61 @@ describe("server move freeze middleware", () => { (await app.request("/api/v1/threads", { method: "POST" })).status, ).toBe(200); }); + + it("keeps bb connect's bb account status reads open while frozen so the tunnel stays up", async () => { + const app = frozenApp({ frozen: true }); + + for (const method of [ + "bb-account.v1.status", + "bb-account.v1.waitForStatusChange", + "bb-account.v1.connectCredential", + ]) { + const response = await app.request(`${BB_ACCOUNT_RPC}/${method}`, { + method: "POST", + body: JSON.stringify({ afterRevision: 3 }), + }); + expect(response.status, method).toBe(200); + } + for (const method of [ + "bb-account.v1.fetch", + "bb-account.v1.adoptConnectCredential", + "bb-account.v1.confirmRefusedCredential", + ]) { + const response = await app.request(`${BB_ACCOUNT_RPC}/${method}`, { + method: "POST", + }); + expect(response.status, method).toBe(503); + expect(await readJson(response)).toMatchObject({ + code: "server_moving", + }); + } + }); + + it("exempts only the read-only bb account RPCs that bb connect polls", () => { + for (const path of [ + `${BB_ACCOUNT_RPC}/bb-account.v1.status`, + `${BB_ACCOUNT_RPC}/bb-account.v1.waitForStatusChange`, + `${BB_ACCOUNT_RPC}/bb-account.v1.connectCredential`, + ]) { + expect(isServerMoveFreezeExempt("POST", path), path).toBe(true); + } + for (const path of [ + `${BB_ACCOUNT_RPC}/bb-account.v1.fetch`, + `${BB_ACCOUNT_RPC}/bb-account.v1.adoptConnectCredential`, + `${BB_ACCOUNT_RPC}/bb-account.v1.confirmRefusedCredential`, + `${BB_ACCOUNT_RPC}/login.start`, + `${BB_ACCOUNT_RPC}/redeemCode`, + `${BB_ACCOUNT_RPC}/signOut`, + `${BB_ACCOUNT_RPC}/bb-account.v1.statusX`, + `${BB_ACCOUNT_RPC}/bb-account.v1.status/extra`, + `${BB_ACCOUNT_RPC}/bb-account.v1.waitForStatusChange/`, + "/api/v1/plugins/connect/rpc/bb-account.v1.status", + "/api/v1/plugins/bb-account-evil/rpc/bb-account.v1.status", + "/api/v1/plugins/bb-account/cli", + `/prefix${BB_ACCOUNT_RPC}/bb-account.v1.status`, + "/api/v1/threads", + ]) { + expect(isServerMoveFreezeExempt("POST", path), path).toBe(false); + } + }); }); diff --git a/docs/server-move-plan.md b/docs/server-move-plan.md index 24eb2a9eb8d..f42b087a706 100644 --- a/docs/server-move-plan.md +++ b/docs/server-move-plan.md @@ -224,7 +224,10 @@ All refuse requests authenticated by a machine credential. the imported files it created. - **At the freeze:** running turns stop, plugin schedules pause, and every plugin except bb connect is suspended before the target is updated, so - nothing writes during the update or after the snapshot. Suspending and the + nothing writes during the update or after the snapshot. The read-only bb + account RPCs that bb connect polls (`bb-account.v1.status`, + `waitForStatusChange`, and `connectCredential`) stay available while frozen, + so the tunnel stays up. Suspending and the later plugin shutdown are each time-boxed at 30 seconds; on expiry the move logs a warning and continues, and retirement's forced exit covers a hung tunnel. A failed, cancelled, or abandoned move lifts the freeze at once, so