mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 07:10:34 +01:00
fix(server): keep reply recovery off web startup critical path (#19769)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The server restores retained native replies during startup. > - Fair reply recovery can admit more due work while an earlier group remains active. > - Waiting for that whole pass can hold web readiness on unrelated retained replies. > - This pull request tracks reply recovery with the existing background startup owner. > - The server can become ready while idle and shutdown still account for that work. ## Linked Issues or Issue Description Refs: #19726. **What happened?** The startup path awaited the native response worker's first pass. That pass can remain active while fair dispatch admits later replies. A controlled `startServer` regression stays in recovery until the held reply pass finishes. A deployed backlog supported investigating this boundary, but no captured runtime stack proves it caused the observed startup delay. **Expected behavior** Retained reply recovery must keep its authority checks, retry deadlines, and fair dispatch. Its completion must not be a condition for unrelated web readiness. Active recovery must still block idle and be joined at shutdown. **Steps to reproduce** Run the startup regression with a held native response worker promise. Before this change, `startServer` does not complete. After this change, startup reports ready, idle work remains active, and shutdown waits until the promise settles. ## What Changed - Track the initial native response pass with the existing heartbeat scheduler work owner. - Extend the actual startup wiring test to cover readiness, idle accounting, worker stop, and shutdown join. - Document the response recovery boundary for idle and readiness. ## Verification - Mandatory agent preflight passed on macOS arm64 with Node 26.4.0. - True RED: the new startup case failed because `startServer` had not completed while reply work was held. The same case passed after the change. - Final head `279e97587b` passed the full startup test file: 123/123 tests. - The unchanged real PostgreSQL later-page case passed. It holds 100 old publication rows, publishes the independent 101st reply once while the first pass remains pending, then verifies completion and no recurring idle SQL. - Final-head CI passed: 60 successful checks and 3 skipped checks, including `ci / verify`. Fresh Greptile is 5/5 with no open threads. - Server TypeScript check passed: `node node_modules/typescript/bin/tsc -p server/tsconfig.json --noEmit`. - Initial test-launch and missing generated SDK setup failures were resolved before the regression ran. They are not RED or pass evidence. - No deployment or live recovery action was performed for this PR. Focused commands, from `server/`: ```sh node ../node_modules/vitest/vitest.mjs run src/__tests__/server-startup-feedback-export.test.ts --maxWorkers=1 --no-file-parallelism node ../cli/node_modules/tsx/dist/cli.mjs ../packages/db/src/test-postgres-command.ts -- node ../node_modules/vitest/vitest.mjs run src/services/native-runtime/native-response-publication-retry.test.ts -t 'publishes a later-page reply while older publication groups remain contended' --maxWorkers=1 --no-file-parallelism --hookTimeout 90000 ``` ## Risks Web readiness can complete before retained reply recovery finishes. The existing scheduler still owns errors, retries, scope checks, and delivery authority. The existing idle tracker and shutdown join retain in-flight work. Other startup recovery gates stay in place. ## Model Used OpenAI Codex, `gpt-6.1-sol`, with reasoning, tool use, and code execution. The context window size is not exposed. ## 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 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:
1 parent
beb3e55072
commit
e2c7d81067
3 files changed
+15
-2
No files matched your search
@@ -1705,7 +1705,7 @@ describe("startServer feedback export wiring", () => {
|
||||
} finally { release(); await starting; timer.mockRestore(); }
|
||||
});
|
||||
|
||||
it.each(["retained cleanup", "workspace finalization", "environment settlement", "native run projection", "native result commit", "native recovery notices", "external conversation settlement"] as const)("becomes ready while %s is held but joins it at shutdown", async owner => {
|
||||
it.each(["retained cleanup", "workspace finalization", "environment settlement", "native run projection", "native result commit", "native recovery notices", "external conversation settlement", "native response presentation"] as const)("becomes ready while %s is held but joins it at shutdown", async owner => {
|
||||
loadConfigMock.mockReturnValue(buildTestConfig({ heartbeatSchedulerEnabled: true }));
|
||||
let releaseCleanup!: () => void;
|
||||
const ready = new Promise<void>(resolve => { releaseCleanup = resolve; });
|
||||
@@ -1733,11 +1733,13 @@ describe("startServer feedback export wiring", () => {
|
||||
: owner === "native run projection" ? heartbeatServiceMock.registerNativeRunProjectionWork
|
||||
: owner === "native result commit" ? heartbeatServiceMock.registerNativeResultCommitWork
|
||||
: owner === "native recovery notices" ? heartbeatServiceMock.registerNativeRecoveryNoticeWork
|
||||
: owner === "native response presentation" ? heartbeatServiceMock.registerNativeResponsePresentationWork
|
||||
: heartbeatServiceMock.registerExternalConversationSettlementWork;
|
||||
if (owner === "native run projection") heartbeatServiceMock.registerNativeRunProjectionWork.mockReturnValueOnce({ ready });
|
||||
else if (owner === "native result commit") heartbeatServiceMock.registerNativeResultCommitWork.mockReturnValueOnce({ ready });
|
||||
else if (owner === "native recovery notices") heartbeatServiceMock.registerNativeRecoveryNoticeWork.mockReturnValueOnce({ ready });
|
||||
else if (owner === "external conversation settlement") heartbeatServiceMock.registerExternalConversationSettlementWork.mockReturnValueOnce({ ready });
|
||||
else if (owner === "native response presentation") heartbeatServiceMock.registerNativeResponsePresentationWork.mockReturnValueOnce({ ready, stop });
|
||||
else (owner === "retained cleanup" ? heartbeatServiceMock.registerRetainedNativeSessionCleanupWork
|
||||
: owner === "workspace finalization" ? heartbeatServiceMock.registerNativeWorkspaceFinalizationWork
|
||||
: heartbeatServiceMock.registerNativeEnvironmentSettlementWork).mockReturnValueOnce({ ready, stop });
|
||||
@@ -1751,6 +1753,7 @@ describe("startServer feedback export wiring", () => {
|
||||
await settleStartupHousekeeping();
|
||||
expect(started).toBe(true);
|
||||
expect(getStartupRecoveryState().phase).toBe("ready");
|
||||
if (owner === "native response presentation") expect(idleWorkSnapshot().active).toBeGreaterThan(0);
|
||||
expect(heartbeatServiceMock.registerNativeStatusReassessmentWork).toHaveBeenCalledOnce();
|
||||
const server = await starting;
|
||||
stopping = server.shutdown().then(() => { stopped = true; });
|
||||
@@ -1761,9 +1764,14 @@ describe("startServer feedback export wiring", () => {
|
||||
expect(onRelease).toHaveBeenCalledOnce();
|
||||
}
|
||||
expect(stopped).toBe(false);
|
||||
if (owner === "native response presentation") {
|
||||
expect(stop).toHaveBeenCalled();
|
||||
expect(idleWorkSnapshot().active).toBeGreaterThan(0);
|
||||
}
|
||||
releaseCleanup();
|
||||
await stopping;
|
||||
expect(stopped).toBe(true);
|
||||
if (owner === "native response presentation") expect(idleWorkSnapshot().active).toBe(0);
|
||||
if (releaseNotification) {
|
||||
releaseNotification();
|
||||
expect(onRelease).toHaveBeenCalledOnce();
|
||||
|
||||
+1
-1
@@ -1812,7 +1812,7 @@ async function startServerWithDatabaseTeardown(
|
||||
() => !isIdleTaskDrainActive() && !heartbeatSchedulerStopped && !isWarmStandby());
|
||||
unsubscribeNativeResponsePresentation = worker.stop;
|
||||
server.on("close", worker.stop);
|
||||
await worker.ready;
|
||||
trackHeartbeatSchedulerWork(worker.ready);
|
||||
}
|
||||
if (config.heartbeatSchedulerEnabled) await heartbeat.registerNativePendingEffectWork(deliveryWork,
|
||||
() => !isIdleTaskDrainActive() && !heartbeatSchedulerStopped && !isWarmStandby()).ready;
|
||||
|
||||
Reference in new issue
Block a user