diff --git a/doc/idle-sleep-safety.md b/doc/idle-sleep-safety.md index e1d6dd1711e..74629708703 100644 --- a/doc/idle-sleep-safety.md +++ b/doc/idle-sleep-safety.md @@ -36,6 +36,16 @@ signed bootstrap writes. Only the task-drain read skips request tracking. Its authentication finishes under a separate work token before entering the report, so it counts concurrent writes without blocking on its own scan. +A matched static UI file served by the admitted, handler-tracked application finishes its request +tracking when its response ends or closes, including a match found after the +client has already disconnected. Unfinished middleware promises remain tracked. +Routers share this application scope. A mounted Express application has hidden +handler promises, so it keeps the existing conservative hold after an aborted +download until the application itself completes the response. An application +without installed handler tracking keeps the same conservative hold. Missing files +continue through the normal fallback and error handlers; they do not exempt +other GET requests or callback-only application work. + ## Scheduled routine wake protocol Historical failed ephemeral leases and terminal custom-image setup sessions can diff --git a/maps/feature-map/instance-operations.md b/maps/feature-map/instance-operations.md index df83b1a8fde..8812efe61e3 100644 --- a/maps/feature-map/instance-operations.md +++ b/maps/feature-map/instance-operations.md @@ -44,6 +44,16 @@ It remains reachable during an idle drain. Use `/api/health` for explicit databa readiness, signed bootstrap, and authorized diagnostics; liveness does not establish database health or claim a workspace. +Automated: [static UI idle completion](../../server/src/__tests__/static-ui-idle.test.ts) +covers real aborted file streams, disconnects before file lookup, nested routers, +and unfinished parent/fallback writes. Mounted Express applications retain their +conservative hold. It does not qualify provider cleanup or a hosted sleep/wake cycle. + +Manual: On an otherwise idle disposable instance, close a static UI download +served by the admitted application before it finishes. Repeat with a disconnect +before headers arrive. Confirm that neither leaves a permanent request hold. An +unfinished application write must still prevent sleep after its client leaves. + ### `settings-backup` Automated: [experimental settings](../../ui/src/pages/InstanceExperimentalSettings.test.tsx) and [database backup routes](../../server/src/__tests__/instance-database-backups-routes.test.ts) cover their UI/API contracts. diff --git a/maps/feature-map/steering.md b/maps/feature-map/steering.md index e35c184eb0a..3d7be84eae0 100644 --- a/maps/feature-map/steering.md +++ b/maps/feature-map/steering.md @@ -100,10 +100,16 @@ the task (and separately its ancestor), preserve a draft, then resume the actual hold; no hidden send should occur while paused. Check both task presentations when available; the named component tests do not prove every host integration. -Pause an agent after a follow-up is accepted but before the prior turn finishes. -Let release and periodic recovery run. Confirm the saved request remains pending, -then resume the agent and verify one delivery. A paused request must not prevent -another eligible recipient's queued work from proceeding. +Fixture-backed recovery check: start with an agent in its completed paused +lifecycle state, then insert a new accepted `deferred_issue_execution` request +through the [PostgreSQL recovery fixture](../../server/src/modules/wake-queue/adapters/postgres.test.ts). +Normal wake admission +rejects paused agents, so this pending state cannot be created by sending a new +message through the ordinary UI or API. Let release and periodic recovery run. +Confirm that the saved request remains pending. Resume the agent, wait for its +lifecycle to become ready, and verify one delivery. An explicit Pause cancels +older queued work; those cancelled receipts must not be replayed. A paused +request must not prevent another eligible recipient's queued work from proceeding. ### `agent-chat` diff --git a/server/src/__tests__/static-ui-idle.test.ts b/server/src/__tests__/static-ui-idle.test.ts new file mode 100644 index 00000000000..2014448cb4c --- /dev/null +++ b/server/src/__tests__/static-ui-idle.test.ts @@ -0,0 +1,295 @@ +import express, { type Response } from "express"; +import request from "supertest"; +import { request as httpRequest, type Server } from "node:http"; +import { once } from "node:events"; +import { mkdtemp, open, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { idleAdmissionMiddleware, trackIdleRequestHandlers } from "../middleware/idle-admission.js"; +import { staticUiFiles } from "../middleware/static-ui.js"; +import { idleWorkSnapshot, startTaskDrain, stopTaskDrain } from "../services/task-admission.js"; + +function deferred() { + let resolve!: () => void; + const promise = new Promise(done => { resolve = done; }); + return { promise, resolve }; +} + +async function files() { + const root = await mkdtemp(path.join(os.tmpdir(), "paperclip-static-ui-")); + const handle = await open(path.join(root, "large.bin"), "w"); + await handle.truncate(32 * 1024 * 1024); + await handle.close(); + await writeFile(path.join(root, "hello.txt"), "hello"); + return root; +} + +async function listen(app: express.Application) { + const listener = app.listen(0, "127.0.0.1"); + await once(listener, "listening"); + return listener; +} + +async function close(listener: Server) { + listener.closeAllConnections(); + await new Promise(resolve => listener.close(() => resolve())); +} + +async function abortDownload(listener: Server, url: string) { + const address = listener.address() as { port: number }; + await new Promise((resolve, reject) => { + const client = httpRequest({ host: "127.0.0.1", port: address.port, path: url }, res => { + if (res.statusCode !== 200) { + res.resume(); + reject(new Error(`Unexpected static response: ${res.statusCode}`)); + return; + } + res.on("error", () => undefined); + res.once("data", () => { res.destroy(); client.destroy(); resolve(); }); + }); + client.on("error", error => { if ((error as NodeJS.ErrnoException).code !== "ECONNRESET") reject(error); }); + client.setTimeout(5000, () => { client.destroy(); reject(new Error("Static response timed out")); }); + client.end(); + }); +} + +afterEach(() => stopTaskDrain()); + +describe("static UI idle completion", () => { + it("releases an aborted file in a nested router", async () => { + const root = await files(), before = idleWorkSnapshot().active, ended = deferred(); + const app = express(), nested = express.Router(); + app.use(idleAdmissionMiddleware); + app.use((_req, res, next) => { res.once("close", ended.resolve); next(); }); + nested.use(staticUiFiles(root)); + app.use("/ui", nested); + trackIdleRequestHandlers(app); + const listener = await listen(app); + try { + await abortDownload(listener, "/ui/large.bin"); + await ended.promise; + expect(idleWorkSnapshot().active).toBe(before); + startTaskDrain({ purpose: "idle", ttlMs: 60_000 }); + await request(app).get("/ui/hello.txt").expect(503); + } finally { + await close(listener); + await rm(root, { recursive: true, force: true }); + } + }); + + it("releases a matched file after the client closes before lookup", async () => { + const root = await files(), before = idleWorkSnapshot().active; + const app = express(), entered = deferred(), ended = deferred(), proceed = deferred(), matched = deferred(); + app.use(idleAdmissionMiddleware); + app.use(async (_req, res, next) => { + res.once("close", ended.resolve); + entered.resolve(); + await proceed.promise; + next(); + }); + app.use(staticUiFiles(root, { setHeaders: () => matched.resolve() })); + trackIdleRequestHandlers(app); + const listener = await listen(app), address = listener.address() as { port: number }; + const client = httpRequest({ host: "127.0.0.1", port: address.port, path: "/large.bin" }); + client.on("error", () => undefined); + client.end(); + try { + await entered.promise; + client.destroy(); + await ended.promise; + expect(idleWorkSnapshot().active).toBe(before + 1); + proceed.resolve(); + await matched.promise; + await new Promise(resolve => setImmediate(resolve)); + expect(idleWorkSnapshot().active).toBe(before); + } finally { + proceed.resolve(); + client.destroy(); + await close(listener); + await rm(root, { recursive: true, force: true }); + } + }); + + it("keeps a late missing-file fallback tracked after the client closes", async () => { + const root = await files(), before = idleWorkSnapshot().active; + const app = express(), entered = deferred(), ended = deferred(), proceed = deferred(), writing = deferred(), done = deferred(); + let committed = false; + app.use(idleAdmissionMiddleware); + app.use(async (_req, res, next) => { + res.once("close", ended.resolve); + entered.resolve(); + await proceed.promise; + next(); + }); + app.use(staticUiFiles(root)); + app.get("/missing", async () => { writing.resolve(); await done.promise; committed = true; }); + trackIdleRequestHandlers(app); + const listener = await listen(app), address = listener.address() as { port: number }; + const client = httpRequest({ host: "127.0.0.1", port: address.port, path: "/missing" }); + client.on("error", () => undefined); + client.end(); + try { + await entered.promise; + client.destroy(); + await ended.promise; + proceed.resolve(); + await writing.promise; + expect(idleWorkSnapshot().active).toBe(before + 1); + expect(committed).toBe(false); + done.resolve(); + await new Promise(resolve => setImmediate(resolve)); + expect(committed).toBe(true); + expect(idleWorkSnapshot().active).toBe(before); + } finally { + proceed.resolve(); + done.resolve(); + client.destroy(); + await close(listener); + await rm(root, { recursive: true, force: true }); + } + }); + + it.each(["mounted application", "untracked root"])("does not release a hidden parent write in a %s", async scope => { + const root = await files(), before = idleWorkSnapshot().active; + const app = express(), nested = scope === "mounted application" ? express() : app, ended = deferred(), done = deferred(); + let committed = false, response!: Response; + app.use(idleAdmissionMiddleware); + nested.use(async (_req, res, next) => { + response = res; + res.once("close", ended.resolve); + next(); + await done.promise; + committed = true; + }); + nested.use(staticUiFiles(root)); + if (scope === "mounted application") { + app.use("/ui", nested); + trackIdleRequestHandlers(app); + } + const listener = await listen(app); + try { + await abortDownload(listener, scope === "mounted application" ? "/ui/large.bin" : "/large.bin"); + await ended.promise; + expect(committed).toBe(false); + expect(idleWorkSnapshot().active).toBe(before + 1); + done.resolve(); + await new Promise(resolve => setImmediate(resolve)); + expect(committed).toBe(true); + expect(idleWorkSnapshot().active).toBe(before + 1); + response.end(); + expect(idleWorkSnapshot().active).toBe(before); + } finally { + done.resolve(); + response?.end(); + await close(listener); + await rm(root, { recursive: true, force: true }); + } + }); + + it("preserves file contents, HEAD, conditional responses, and caller headers", async () => { + const root = await files(), app = express(), before = idleWorkSnapshot().active; + app.use(idleAdmissionMiddleware); + app.use(staticUiFiles(root, { maxAge: "1h", setHeaders: res => { res.setHeader("X-Static-Test", "kept"); } })); + trackIdleRequestHandlers(app); + try { + const first = await request(app).get("/hello.txt").expect(200).expect("X-Static-Test", "kept"); + expect(first.text).toBe("hello"); + await request(app).head("/hello.txt").expect(200).expect("X-Static-Test", "kept"); + await request(app).get("/hello.txt").set("If-None-Match", first.headers.etag).expect(304); + expect(idleWorkSnapshot().active).toBe(before); + } finally { await rm(root, { recursive: true, force: true }); } + }); + + it("keeps a parent write tracked after a static client leaves", async () => { + const root = await files(), before = idleWorkSnapshot().active; + const app = express(), ended = deferred(), done = deferred(); + let committed = false; + app.use(idleAdmissionMiddleware); + app.use(async (_req, res, next) => { + res.once("close", ended.resolve); + next(); + await done.promise; + committed = true; + }); + app.use(staticUiFiles(root)); + trackIdleRequestHandlers(app); + const listener = await listen(app); + try { + await abortDownload(listener, "/large.bin"); + await ended.promise; + expect(committed).toBe(false); + expect(idleWorkSnapshot().active).toBe(before + 1); + done.resolve(); + await new Promise(resolve => setImmediate(resolve)); + expect(committed).toBe(true); + expect(idleWorkSnapshot().active).toBe(before); + } finally { + done.resolve(); + await close(listener); + await rm(root, { recursive: true, force: true }); + } + }); + + it("keeps an unmatched mutating GET tracked after disconnect", async () => { + const root = await files(), before = idleWorkSnapshot().active; + const app = express(), entered = deferred(), ended = deferred(), done = deferred(); + let committed = false; + app.use(idleAdmissionMiddleware); + app.use(staticUiFiles(root)); + app.get("/callback", async (_req, res) => { + res.once("close", ended.resolve); + entered.resolve(); + await done.promise; + committed = true; + }); + trackIdleRequestHandlers(app); + const listener = await listen(app), address = listener.address() as { port: number }; + const client = httpRequest({ host: "127.0.0.1", port: address.port, path: "/callback" }); + client.on("error", () => undefined); + client.end(); + try { + await entered.promise; + client.destroy(); + await ended.promise; + expect(idleWorkSnapshot().active).toBe(before + 1); + expect(committed).toBe(false); + done.resolve(); + await new Promise(resolve => setImmediate(resolve)); + expect(committed).toBe(true); + expect(idleWorkSnapshot().active).toBe(before); + } finally { + done.resolve(); + client.destroy(); + await close(listener); + await rm(root, { recursive: true, force: true }); + } + }); + + it("retains an unmatched callback handler until the application ends it", async () => { + const root = await files(), before = idleWorkSnapshot().active; + const app = express(), entered = deferred(), ended = deferred(); + let response!: Response; + app.use(idleAdmissionMiddleware); + app.use(staticUiFiles(root)); + app.get("/callback", (_req, res) => { response = res; res.once("close", ended.resolve); entered.resolve(); }); + trackIdleRequestHandlers(app); + const listener = await listen(app), address = listener.address() as { port: number }; + const client = httpRequest({ host: "127.0.0.1", port: address.port, path: "/callback" }); + client.on("error", () => undefined); + client.end(); + try { + await entered.promise; + client.destroy(); + await ended.promise; + expect(idleWorkSnapshot().active).toBe(before + 1); + response.end(); + expect(idleWorkSnapshot().active).toBe(before); + } finally { + response?.end(); + client.destroy(); + await close(listener); + await rm(root, { recursive: true, force: true }); + } + }); +}); diff --git a/server/src/app.ts b/server/src/app.ts index e6bfec9b6e5..7e60da3a520 100644 --- a/server/src/app.ts +++ b/server/src/app.ts @@ -8,6 +8,7 @@ import { DELIVERY_QUEUES } from "./services/delivery-work-notifications.js"; import { createLifecycleDriver } from "./services/agent-lifecycle-driver.js"; import { startAgentLifecycle } from "./services/agent-lifecycle.js"; import { idleAdmissionMiddleware, trackIdleRequestHandlers } from "./middleware/idle-admission.js"; +import { staticUiFiles } from "./middleware/static-ui.js"; import { isIdleTaskDrainActive, trackIdleWork } from "./services/task-admission.js"; import { customerSuccessRoutes } from "./routes/customer-success.js"; import { cloudWarmStandbyMiddleware } from "./middleware/cloud-warm-standby.js"; @@ -1113,7 +1114,7 @@ export async function createApp( // never change once built, so they can be cached aggressively. staticUi.use( "/assets", - express.static(path.join(uiDist, "assets"), { + staticUiFiles(path.join(uiDist, "assets"), { maxAge: "1y", immutable: true, }), @@ -1127,7 +1128,7 @@ export async function createApp( // reasonably fast, with must-revalidate overrides for index.html and // sw.js (see staticUiCacheControl for why those two). staticUi.use( - express.static(uiDist, { + staticUiFiles(uiDist, { maxAge: "1h", setHeaders(res, filePath) { const override = staticUiCacheControl(filePath); @@ -1235,7 +1236,7 @@ export async function createApp( const renderViteHtml = viteHtmlRenderer; if (fs.existsSync(publicUiRoot)) { - staticUi.use(express.static(publicUiRoot, { index: false })); + staticUi.use(staticUiFiles(publicUiRoot, { index: false })); } staticUi.get(/.*/, async (req, res, next) => { if (!shouldServeViteDevHtml(req)) { diff --git a/server/src/middleware/idle-admission.ts b/server/src/middleware/idle-admission.ts index 96958837330..bc6a28d35c2 100644 --- a/server/src/middleware/idle-admission.ts +++ b/server/src/middleware/idle-admission.ts @@ -2,8 +2,9 @@ import { markIdleIngressTracked } from "../services/idle-local-work.js"; import type { Application, Request, RequestHandler } from "express"; import { beginIdleTrackedWork, isIdleTaskDrainActive } from "../services/task-admission.js"; -type RequestWork = { pending: number; ended: boolean; closed: boolean; asyncRoute: boolean; routeSequence: number; active: boolean; finish: () => void }; +type RequestWork = { application: Application; pending: number; ended: boolean; closed: boolean; asyncRoute: boolean; routeSequence: number; active: boolean; finish: () => void }; const requests = new WeakMap(); +const trackedApplications = new WeakSet(); function settle(work: RequestWork) { if (work.active && (work.ended || (work.closed && work.asyncRoute)) && work.pending === 0) { work.active = false; @@ -11,6 +12,13 @@ function settle(work: RequestWork) { } } +export function completeIdleReadOnlyResponse(req: Request, application: Application): void { + const work = requests.get(req); + if (!work || work.application !== application || !trackedApplications.has(application)) return; + work.ended = true; + settle(work); +} + // Only the exact control endpoint and health probes bypass admission. The // control route still authenticates each operation. GET is otherwise tracked: // OAuth callbacks and tool streams can create work too. @@ -40,7 +48,7 @@ export const idleAdmissionMiddleware: RequestHandler = (req, res, next) => { next(); return; } - const work: RequestWork = { pending: 0, ended: false, closed: false, asyncRoute: false, routeSequence: 0, active: true, finish: beginIdleTrackedWork() }; + const work: RequestWork = { application: req.app, pending: 0, ended: false, closed: false, asyncRoute: false, routeSequence: 0, active: true, finish: beginIdleTrackedWork() }; requests.set(req, work); res.once("close", () => { work.closed = true; settle(work); }); // close/aborted are NOT completion: an async handler can still be committing @@ -103,5 +111,6 @@ export function trackIdleRequestHandlers(app: Application): void { } }; visit(app.router.stack as Layer[]); + trackedApplications.add(app); markIdleIngressTracked(); } diff --git a/server/src/middleware/static-ui.ts b/server/src/middleware/static-ui.ts new file mode 100644 index 00000000000..6c9a558404e --- /dev/null +++ b/server/src/middleware/static-ui.ts @@ -0,0 +1,46 @@ +import express, { type RequestHandler } from "express"; +import { completeIdleReadOnlyResponse } from "./idle-admission.js"; + +export function staticUiFiles(root: string, options: Parameters[1] = {}): RequestHandler { + const matches = new WeakMap void>(); + const serve = express.static(root, { + ...options, + setHeaders(res, filePath, stat) { + options?.setHeaders?.(res, filePath, stat); + matches.get(res)?.(); + }, + }); + return (req, res, next) => { + const application = req.app; + let matched = false; + const cleanup = () => { + matches.delete(res); + res.off("finish", complete); + res.off("close", complete); + }; + const complete = () => { + if (!matched) return; + cleanup(); + completeIdleReadOnlyResponse(req, application); + }; + const forward = (error?: unknown) => { + cleanup(); + if (matched && res.destroyed) { + completeIdleReadOnlyResponse(req, application); + return; + } + next(error); + }; + matches.set(res, () => { + matched = true; + if (res.destroyed) complete(); + }); + res.once("finish", complete); + res.once("close", complete); + try { + serve(req, res, forward); + } catch (error) { + forward(error); + } + }; +}