fix(server): release idle tracking after aborted static UI files (#16685)

## Thinking Path

> - Paperclip runs work for AI agents.
> - Hosts must prove that accepted work has finished before idle sleep.
> - HTTP request tracking keeps unfinished writes active after a client
leaves.
> - Static file middleware has no returned completion promise.
> - An aborted UI download can leave its request active forever.
> - This PR completes only a matched file response and keeps other work
tracked.

## Linked Issues or Issue Description

Refs #15522. Found while qualifying #16679. A search found no duplicate
static-download idle fix.

**What happened?**

Closing a client during a static UI download leaves one tracked request
active. The response has closed, but the file middleware never calls the
wrapped response end method. The instance cannot report idle after that
request.

**Expected behavior**

A closed, matched static file response must not leave a permanent work
hold. Unfinished application writes must remain tracked. A missing file
must keep the normal fallback and error paths.

**Steps to reproduce**

1. Serve a large local file through the static UI middleware with idle
admission enabled.
2. Start the download over HTTP.
3. Disconnect the client after its first data chunk.
4. Observe that the tracked request remains active with the original
middleware.

**Paperclip version or commit**

Reproduced on the behavior in
`d30648dee89720358271c276b9fc28cf83f18be7`. This branch starts at
`cfa9dce295970a85b3d8ee14cab0eccf50bc72e2`.

## What Changed

- Mark the completion of a known read-only response without dropping
pending middleware promises.
- Wrap static UI file serving. A successful file match is required
before a closed response can complete its tracking. A late match
rechecks an already closed response. Missing files keep the normal
fallback.
- Use the wrapper for hashed assets, other static UI files, and public
development files. Preserve caller headers and cache options.
- Test both HTTP abort timings, nested routers, conservative
mounted-application holds, parent writes, mutating GET fallbacks, and
callback-only work.
- Update the idle contract and its verification map. Correct the
paused-message recipe from hosted qualification: an explicit Pause
cancels older queued work, so test new accepted input after the pause
and await the normal resume lifecycle.

## Verification

- The original two real stream-abort regressions fail with the original
Express static behavior.
- Three review regressions fail on
`e260f4694afab8447b0cb203f439cb3ccad254e8`: a client closing before file
lookup, an unfinished parent in a mounted application, and an unfinished
parent in a root without installed handler tracking. Fixed sources were
restored byte for byte after the comparison.
- Two focused suites: 16 passed. They cover nested routers, both abort
timings, hidden parent writes, late missing-file fallbacks, contents,
HEAD, conditional responses, and caller headers.
- Module and map checks pass. Map checks are structural only.
- `pnpm -r typecheck` and `pnpm build`: passed on
`f242ee739344484995d3addf81635512370778c8`.
- Full CI matrix: 54 successful checks and 2 intentional Storybook
skips.
- Apex: 5/5 on the exact current head, with zero unresolved threads.
- Full local maintained test groups are still running before final
handoff.
- The initial local broad run was stopped before changing source for the
P1 review findings. Two file-mode fixture failures in that run came from
the scratch runner's inherited `0077` mask; the same suite passes with
the normal child test mask `0022`, without source or host configuration
changes. The replacement run uses `0022` for test children and
command-scoped `init.defaultBranch=master` for existing Git fixture
assumptions.
- This PR has not been deployed to a hosted environment.

## Risks

Only a matched file served by the admitted application with installed
handler tracking can complete this way. Routers share that scope.
Mounted Express applications and roots without handler tracking keep the
existing conservative response hold; this change does not claim their
hidden writes are tracked. An unfinished tracked parent promise still
blocks sleep after a file client disconnects. Redirects, missing files,
application callbacks, and other GET requests retain their guards. No
authentication, company scope, provider cleanup, or scheduling gate is
removed. No schema or API response changes.

## Model Used

OpenAI GPT-6 through Codex, with reasoning, source editing, and shell
tools. The session does not expose a more specific serving model ID or
its context window size.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Devin FoleyandPaperclip authored and GitHub committed 2026-10-17 20:28:57 -07:00
1 parent 1f27427ffd
commit 63fbb714e6
7 files changed
+386 -9

No files matched your search

+295
View File
@@ -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<void>(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<void>(resolve => listener.close(() => resolve()));
}
async function abortDownload(listener: Server, url: string) {
const address = listener.address() as { port: number };
await new Promise<void>((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 });
}
});
});
+4 -3
View File
@@ -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)) {
+11 -2
View File
@@ -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<Request, RequestWork>();
const trackedApplications = new WeakSet<Application>();
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();
}
+46
View File
@@ -0,0 +1,46 @@
import express, { type RequestHandler } from "express";
import { completeIdleReadOnlyResponse } from "./idle-admission.js";
export function staticUiFiles(root: string, options: Parameters<typeof express.static>[1] = {}): RequestHandler {
const matches = new WeakMap<object, () => 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);
}
};
}