From 90119181e766fd9722450729e058eaee4e3c5e19 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Mon, 28 Sep 2026 18:08:57 -0700 Subject: [PATCH] test: control Telegram subscription retry timing (#14501) ## Thinking Path > - Paperclip manages AI agents and their work. > - Telegram delivery recovers subscription changes after a restart. > - The recovery test leaves a failed action on the real one-second retry timer. > - A slow runner can cross that deadline before the test checks that no retry occurred. > - This pull request holds the fixture deadline until the explicit restart transition. > - The test still checks that recovery uses fresh provider options. ## Linked Issues or Issue Description **What happened?** The Telegram subscription recovery test expected one `setWebhook` request but saw two. A 1.5-second delay after the first failed request reproduces the failure. **Expected behavior** The test controls when the failed request becomes eligible for retry. Host speed does not change its result. **Steps to reproduce** Run the test named `retries an unknown subscription mutation after restart` with a 1.5-second delay after the first failed-action assertion. The old fixture retries too early. The updated fixture passes with the same delay. Related: #13952 fixes a separate Telegram fixture cleanup problem. This change addresses retry timing. ## What Changed - Set the stored retry deadline to 2099 before the pre-restart assertions. - Keep the existing explicit epoch deadline after restart and all provider request assertions. - Report the current test phase only when this test fails, to diagnose an observed intermittent CI timeout. - Leave production retry code unchanged. Temporary delay and per-step console tracing are not included. ## Verification - Delayed regression: failed before the change with two requests instead of one; passed after the change. - Focused Telegram durable private draft Stop group: 34 tests passed. - `pnpm -r typecheck`: passed. - `pnpm build`: passed. - Full local chat shard: 355 tests passed twice. - `pnpm test:run`: general-server phase completed with 13,861 passed, 86 skipped, and 14 failures in unchanged macOS skills-cache and Git long-path tests. The same failures reproduce on unmodified base code. The command stops at that phase, so no full local pass is claimed. - CI exposed a separate 15-second timeout. A diagnostic run passed all 355 shard tests, with the affected test completing in under one second. Its cause remains unproven. Normal step logging is removed; a failure-only phase report remains for a recurrence. The final commit also passes the 355-test chat shard, and Greptile rates it 5/5. All 52 final-commit checks pass, with two intentional skips. There are no unresolved review comments or merge conflicts. ## Risks Low risk. This changes only the fixture deadline. It does not disable a test, extend a timeout, or change production retry behavior. Existing assertions still verify the failed action, the pending state, restart recovery, and fresh provider options. The intermittent CI timeout is not claimed fixed; phase diagnostics narrow the next occurrence without changing the timeout. No documentation change is needed for a test fixture correction. ## Model Used OpenAI GPT-6 (Codex), with reasoning, terminal tools, and code execution. The context window size is not exposed in this session. ## 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 references) - [x] My branch name describes the change and contains no internal Paperclip ticket id or instance-derived details - [x] I have run focused tests locally and they pass; full-suite status is recorded above - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes, or explained why none is needed - [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 --- .../chat-channels.integration.test.ts | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/server/src/__tests__/chat-channels.integration.test.ts b/server/src/__tests__/chat-channels.integration.test.ts index d04154ee977..39d9e150fe8 100644 --- a/server/src/__tests__/chat-channels.integration.test.ts +++ b/server/src/__tests__/chat-channels.integration.test.ts @@ -69063,7 +69063,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { }, ); - it("retries an unknown subscription mutation after restart using freshly observed options, not a recovered confirmation flag", async () => { + it("retries an unknown subscription mutation after restart using freshly observed options, not a recovered confirmation flag", async ({ onTestFailed }) => { + let phase = "create fixture"; + onTestFailed(() => { + console.error(`Telegram subscription recovery failed during: ${phase}`); + }); const lane = await draftFixture(); try { await db @@ -69080,6 +69084,7 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { throw new Error("Synthetic unknown subscription response"); return undefined; }); + phase = "first subscription attempt"; await lane.processSubscriptionAttempt(); const [action] = await db .select() @@ -69095,17 +69100,28 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { status: "failed", result: { retryable: true, providerConfirmed: false }, }); + // Keep the retry pending until the explicit post-restart transition below. + // A busy runner can otherwise exhaust the real one-second backoff here. + await db + .update(chatActions) + .set({ + result: { ...action!.result, retryAt: "2099-01-01T00:00:00.000Z" }, + }) + .where(eq(chatActions.id, action!.id)); + phase = "ordinary publication before restart"; expect((await lane.send("unknown-subscription"))?.state).toBe( "published", ); expect(lane.requests).toHaveLength(1); expect(lane.requests[0]!.method.endsWith("Draft")).toBe(false); + phase = "pending recovery before restart"; await lane.context.service.processPendingDeliveries(); expect( lane.maintenanceRequests.filter( ({ method }) => method === "setWebhook", ), ).toHaveLength(1); + phase = "restart"; await lane.restart(); lane.setSubscriptionInfo({ allowed_updates: ["message", "chat_member"], @@ -69121,10 +69137,12 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { }, }) .where(eq(chatActions.id, action!.id)); + phase = "concurrent recovery after restart"; await Promise.all([ lane.context.service.processPendingDeliveries(), lane.context.service.processPendingDeliveries(), ]); + phase = "verify recovered subscription"; const mutations = lane.maintenanceRequests.filter( ({ method }) => method === "setWebhook", ); @@ -69154,9 +69172,11 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => { 200, ); }); + phase = "publication after subscription recovery"; expect((await lane.send("repaired-after-unknown"))?.state).toBe( "cancelled", ); + phase = "close fixture"; } finally { await lane.close(); }