pneumo

M2: Pushes, files, retention sweeps and the reset CLI (ENG-251) #15

Merged

demi.dev 1w ago

targeting main from stoopkidddd/eng-251-m2-pushes-files-retention-sweeps-and-the-reset-cli

Merged opened by and assigned to

demi.dev 1w ago targeting main from stoopkidddd/eng-251-m2-pushes-files-retention-sweeps-and-the-reset-cli

Linear: https://linear.app/engineereddev/issue/ENG-251

What's in this PR:

index-store/: boot-loads pushes/ (concurrency 32) into a (createdAt, id)-sorted index with an id map; before/limit pagination with nextBefore; a 30 s re-list of the first 100 keys merges unknown records and emits push.
files/: POST /api/files (UUIDv7 fileId; a presigned PUT in direct mode or /api/files/:id/content in proxy mode; 413 over MAX_FILE_BYTES), GET /api/files/:id (presigned GET for 10 min, or the proxy URL), and proxy content routes that stream with the exact declared size.
pushes/: POST /api/pushes with the §8 validations and idempotency (200 same body / 409 different), GET list and one, and DELETE cascading to the file object. Emits push and push_deleted on the event bus.
sweeps/: hourly orphan-file sweep (> 24 h) and daily retention (RETENTION_DAYS > 0), on the injected clock.
cli.ts → dist/cli.js: info and reset --yes (refuses without --yes).
storage/s3.ts: aborts an S3 PUT when its input stream errors (a hung PUT seen on MinIO), with a regression test.

Verify (lead, on 445041d, based on 4291842):

pnpm install --frozen-lockfile → exit 0
mise run check:server → exit 0 (Tests 197 passed (197))
mise run e2e:server → exit 0 (4 files, 14 tests on MinIO: direct-mode presigned upload + push, proxy upload/download byte-equal)
pnpm --filter @pneumo/server build → exit 0 (dist/index.js, dist/cli.js)
node apps/server/dist/cli.js reset (no --yes) → "Refusing to reset without --yes.", exit 1
No pneumo-e2e-* containers left.

Deviations:

An unknown but valid pagination cursor returns an empty page (the spec doesn't say).
storage/s3.ts (ENG-249's file) gains stream-error abort handling, found during MinIO testing.
The integration build runs once in the e2e global setup, so concurrent clean: true builds don't race.

🤖 Generated with Claude Code

Commits 2

M2: Release push mutex during proxy uploads (ENG-251)

+112 -26

M2: Add pushes, file transfers, retention and reset CLI (ENG-251)

+2833 -16

Comments

v0 1w ago

demi.dev 1w ago

pneumo-reviewer: round 1, REQUEST_CHANGES #

The PR covers the issue: all 8 §8 rows with every POST /api/pushes validation rule, the index, sweeps, the CLI and the MinIO e2e. check:server, e2e:server and the build pass. One FIX blocks merge.

Findings #

FIX (often 4 × bad 6 = 24), files/routes.ts:123-158: the proxy PUT /api/files/:id/content holds the global push mutex for the whole streamed upload. While any proxy upload is in flight, every POST /api/pushes, POST /api/files, DELETE, the 30 s reconcile and both sweeps wait. A stalled mobile upload blocks all push creation for up to Node's 300 s request timeout. Reproduced: with an open upload body, POST /api/pushes stayed pending for 2 s and returned only after the upload closed. Fix: hold the mutex only for the checks and state changes, mark the pending file uploading (a second PUT → 409; a push referencing it → 400), stream outside the mutex, and add a route test for "a stalled PUT doesn't block another push". → omp round 2.
FILE (4), sweeps/index.ts:104-113: the first retention run is 24 h after boot, so a server restarted more often than daily never applies RETENTION_DAYS.
FILE (3), sweeps/index.ts:78-88: retention holds the push mutex across every deleteMany; the first run on a large history blocks push creation.
FILE (4), cli.ts:110-112: every CLI failure, including ConfigError, prints only "CLI command failed."
FILE (2), index-store/index.ts:141: an unknown but valid before cursor returns an empty page indistinguishable from the end of history (stated deviation; spec silent).

FILE items are recorded on ENG-275 for triage; code left alone.

Spec checks #

§8 rows and every POST /api/pushes rule OK. Presigned PUT signs the size (3600 s), GET 600 s. §11 descriptors, exact-size proxy streaming and the orphan rules OK (the lock is the availability bug above). §12.2 index (concurrency 32, sort, 30 s reconcile, emit once), §12.4 redaction, §12.5 CLI, §5.3/§5.4 OK. packages/ unchanged.

DoD checks #

Unit tests with MemoryStorage + fake clock, route tests for all 8 routes including auth failures, MinIO e2e (direct presigned PUT + push, proxy upload/download byte-equal, short/long bodies, reload, cascade, sweeps, built CLI), and redaction assertions: all present. The README edit and the minimal storage/s3.ts abort change are accepted.

Tests run by the reviewer: mise run check:server exit 0 (197 tests); mise run e2e:server exit 0 (14 tests); build OK; CLI reset without --yes → exit 1.

v1 1w ago

demi.dev 1w ago

pneumo-reviewer: round 2, APPROVE #

Round 2 (b2f125a) resolves the FIX finding. The proxy PUT holds the push mutex only for its checks and for setting pending.uploading. Streaming, the trailing-byte drain and failure cleanup run outside the lock, and finally clears the flag under the mutex, so a failed upload can be retried. A duplicate PUT during upload → 409; attaching an uploading file → 400. The reviewer's earlier probe now shows a push succeeding in 3 ms while an upload is stalled (round 1: still pending after 2000 ms).

Findings #

Round 1 FIX (score 24), files/routes.ts: resolved. The new route test ("a stalled upload allows independent pushes and uploads…") would have deadlocked under the old lock and is deterministic.
Round 1 FILE findings 2–5: unchanged, recorded on ENG-275. No new findings.

Spec / DoD #

§8 and §11 still OK (the extra 400 for an uploading file falls under the "HEAD shows exactly the declared size" rule); §12.2/§12.5 unchanged; packages/ untouched. Tests, MinIO e2e and redaction assertions present. The round touched only the five files in scope.

Tests run by the reviewer on b2f125a: mise run check:server exit 0 (198 tests); mise run e2e:server exit 0 (14 tests); build OK; CLI reset without --yes → exit 1. Pipeline for b2f125a: 3/3 green.

Versions

pneumo-reviewer: round 1, REQUEST_CHANGES #

Findings #

Spec checks #

DoD checks #

pneumo-reviewer: round 2, APPROVE #

Findings #

Spec / DoD #