Flaky test: "stops working once it expires" signs a URL that is valid for 0–1 s #97

New Issue

2026-10-14T03:56:29Z

ekklesia-bot commented

2026-10-14 03:56:29 +00:00

What happened

migration-smoke failed on main in run #234, the push of 437720f (the 0.2.5 release merge). Every other job in that run passed, and the same commit had passed on its PR run. The failure:

FAIL core/tests/module-delivery.integration.ts > live direct object delivery > stops working once it expires
AssertionError: expected 403 to be 200
  at core/tests/module-delivery.integration.ts:195:48

Line 195 is the first fetch, the one that should still succeed. The URL had already expired before it was used once.

Why: the URL is valid for 0–1 s, not 1 s

The test signs a URL with expiresInSeconds: 1 at the provider seam (core/tests/module-delivery.integration.ts:184-198), immediately expects 200, waits 2.5 s, and expects 403.

createReadUrl in core/src/storage/s3-provider.ts doesn't sign at "now". readUrlWindow() (line 202) floors the signing time to a stability window, so repeated refreshes produce the same URL and browsers can cache it:

const windowSeconds = Math.max(1, Math.floor(expiresInSeconds / READ_URL_STABILITY_DIVISOR)); // divisor = 3
const signedAtMs   = Math.floor(nowMs / windowMs) * windowMs;
expiresAt          = signedAtMs + expiresInSeconds * 1000;
guaranteedSeconds  = expiresInSeconds - windowSeconds;

For expiresInSeconds: 1 the window is 1 s, so the URL expires at the next whole second. Its real lifetime is whatever is left of the current second, anywhere from ~1000 ms down to ~0 ms. The provider's own guaranteedSeconds is 1 − 1 = 0.

So the test fails whenever signing → fetch → SeaweedFS's check takes longer than the rest of the current second. On an idle machine that's rare. On the shared runner, under load from parallel jobs, it happens.

This only affects the test. Production lifetimes are clamped to a much longer minimum by module-storage.ts (resolveExpirySeconds), where the window is a small fraction of the lifetime.

How to reproduce

The failure depends on timing, not on any particular commit. To show the window directly: sign with expiresInSeconds: 1 when Date.now() % 1000 > 900, then fetch after ~150 ms. The response is 403.

Suggested fix (test only)

Use a lifetime whose guaranteed part covers the first fetch, and wait for the expiry the provider reports instead of a fixed sleep:

const delivery = await sharedProvider().createReadUrl(bucket, "fleeting.txt", { expiresInSeconds: 3 });
// window = max(1, floor(3/3)) = 1 s, so at least 2 s are guaranteed
expect((await fetch(delivery.url)).status).toBe(200);
await new Promise((r) => setTimeout(r, Math.max(0, delivery.expiresAt.getTime() - Date.now()) + 1_500));
expect((await fetch(delivery.url)).status).toBe(403);

That costs about 2 s of extra test time at most. It still proves the server enforces expiry and doesn't depend on runner speed. Alternatively, assert on guaranteedSeconds > 0 for the chosen lifetime so a future divisor change can't silently reintroduce this.

Not looked at

Whether any other live-storage test signs a URL shorter than 3 s. A quick grep -n "expiresInSeconds: [12]\b" core/tests would show it.

Found by Claude while investigating Zayin/Signage CI run #43 (found-by-claude).

## What happened `migration-smoke` failed on `main` in [run #234](https://git.zayin.net/Zayin/Ekklesia/actions/runs/290), the push of `437720f` (the 0.2.5 release merge). Every other job in that run passed, and the same commit had passed on its PR run. The failure: ``` FAIL core/tests/module-delivery.integration.ts > live direct object delivery > stops working once it expires AssertionError: expected 403 to be 200 at core/tests/module-delivery.integration.ts:195:48 ``` Line 195 is the **first** fetch, the one that should still succeed. The URL had already expired before it was used once. ## Why: the URL is valid for 0–1 s, not 1 s The test signs a URL with `expiresInSeconds: 1` at the provider seam (`core/tests/module-delivery.integration.ts:184-198`), immediately expects `200`, waits 2.5 s, and expects `403`. `createReadUrl` in `core/src/storage/s3-provider.ts` doesn't sign at "now". `readUrlWindow()` (line 202) floors the signing time to a stability window, so repeated refreshes produce the same URL and browsers can cache it: ```ts const windowSeconds = Math.max(1, Math.floor(expiresInSeconds / READ_URL_STABILITY_DIVISOR)); // divisor = 3 const signedAtMs = Math.floor(nowMs / windowMs) * windowMs; expiresAt = signedAtMs + expiresInSeconds * 1000; guaranteedSeconds = expiresInSeconds - windowSeconds; ``` For `expiresInSeconds: 1` the window is 1 s, so the URL expires at the **next whole second**. Its real lifetime is whatever is left of the current second, anywhere from ~1000 ms down to ~0 ms. The provider's own `guaranteedSeconds` is `1 − 1 = 0`. So the test fails whenever signing → fetch → SeaweedFS's check takes longer than the rest of the current second. On an idle machine that's rare. On the shared runner, under load from parallel jobs, it happens. This only affects the test. Production lifetimes are clamped to a much longer minimum by `module-storage.ts` (`resolveExpirySeconds`), where the window is a small fraction of the lifetime. ## How to reproduce The failure depends on timing, not on any particular commit. To show the window directly: sign with `expiresInSeconds: 1` when `Date.now() % 1000 > 900`, then fetch after ~150 ms. The response is `403`. ## Suggested fix (test only) Use a lifetime whose guaranteed part covers the first fetch, and wait for the expiry the provider reports instead of a fixed sleep: ```ts const delivery = await sharedProvider().createReadUrl(bucket, "fleeting.txt", { expiresInSeconds: 3 }); // window = max(1, floor(3/3)) = 1 s, so at least 2 s are guaranteed expect((await fetch(delivery.url)).status).toBe(200); await new Promise((r) => setTimeout(r, Math.max(0, delivery.expiresAt.getTime() - Date.now()) + 1_500)); expect((await fetch(delivery.url)).status).toBe(403); ``` That costs about 2 s of extra test time at most. It still proves the server enforces expiry and doesn't depend on runner speed. Alternatively, assert on `guaranteedSeconds > 0` for the chosen lifetime so a future divisor change can't silently reintroduce this. ## Not looked at Whether any other live-storage test signs a URL shorter than 3 s. A quick `grep -n "expiresInSeconds: [12]\b" core/tests` would show it. --- _Found by Claude while investigating Zayin/Signage CI run #43 (`found-by-claude`)._

ekklesia-bot referenced this issue

2026-10-14 03:58:18 +00:00

docs(claude): file an issue for what you find outside the task #99

ekklesia-bot added the found-by-claude label 2026-10-14 04:05:26 +00:00

ekklesia-bot referenced this issue

2026-10-14 04:13:33 +00:00

test(api): let a shutdown's closed connections leave pg_stat_activity #100

Sign in to join this conversation.