Monument Labs·Review desk

PR 418 · Free export storage when a project closes

Patch 418 · 12 Aug 2026

The hidden cost is outside the diff.

PR 418 passes one new option to archive() inside closeProject() so that closing a project frees its export storage. The same option deletes every linked export on the spot, inside the 30 days in which the product still says a closed project can be restored.

Verdict
Request changes
Ship blockers
1
Author
Lena Okafor
Reviewer
Mats Everly
Base
main @ 7c1d0e2
Lines changed
+1 −1 in 1 file
src/projects/close-project.ts+1 −1
@@ -19,7 +19,7 @@ closeProject19 export async function closeProject(id) {20   const project = await projects.get(id)21   assertCanClose(project)22-  await archive(id)22+  await archive(id, { cascade: true })23   audit.log("project.closed", { id })24   return { closed: true }25 }
Fig. 1The change is one argument on line 22. Each numbered mark is a place it reaches outside this file: code, product copy and a test.
  1. 1

    src/storage/archive.ts · line 22

    await exports.purge(projectId)

    Linked exports are hard-deleted at close. No tombstone is written.

  2. 2

    src/ui/CloseProjectDialog.tsx · line 18

    “You can restore a closed project for 30 days.”

    Shown on every close. The code no longer keeps this promise.

  3. 3

    src/projects/restore-project.ts · line 9

    await exports.restore(id)

    Finds no tombstones. The project comes back without its exports.

  4. 4

    test/projects/close-project.test.ts · line 12

    expect(result).toEqual({ closed: true })

    Still passes. It checks the return value and never looks at exports.

Ship blocker 01

Cascade crosses the retention boundary

Before this change, closeProject() called archive(id) with no options. On that path, archive() marks the project closed and soft-deletes its linked exports: each export gets a tombstone with restoreUntil set 30 days out, and the nightly retention job purges whatever is still tombstoned after that.

{ cascade: true } takes the other branch. archive() skips the tombstone and calls exports.purge(), which removes the export rows and their stored files in the same request. Nothing is left for the retention job, and nothing is left for restoreProject() to bring back.

The product still makes the old promise. CloseProjectDialog tells the user they can restore a closed project for 30 days, and retention policy RET-3 says linked exports follow their project for the same window. A user who closes a project by mistake and restores it an hour later gets the project back with no exports and no message.

This is not a rare path. In July, 41 projects were restored inside the window, and 33 of them had linked exports. The goal of the PR is sound: exports on closed projects hold 1.6 TB that STOR-212 wants back. That storage can be freed on day 30 without breaking the promise.

src/storage/archive.ts · lines 12–24Unchanged by this PR
12 export async function archive(projectId, { cascade = false } = {}) {13   await projects.update(projectId, { status: "closed", closedAt: now() })14 15   if (!cascade) {16     // Tombstone: restorable until the retention job runs on day 3017     await exports.softDelete(projectId, { restoreUntil: daysFromNow(30) })18     return19   }20 21   // Rows and stored files, removed now22   await exports.purge(projectId)23   await audit.log("exports.purged", { projectId })24 }

1With cascade: true, close lands on line 22. There is no tombstone, so there is nothing to restore.

Coverage

Why the tests still pass

The only test for closeProject() checks its return value. The return value did not change, so the suite stays green and CI passed in four minutes.

No test closes a project and then restores it. That round trip is the one path where the dialog’s promise matters, and it is the path this change breaks.

In the suite now

Passes on main and on this branch.

test/projects/close-project.test.tslines 9–13
9 test("closes a project", async () => {10   const { id } = await seedProject()11   const result = await closeProject(id)12   expect(result).toEqual({ closed: true })13 })

The round trip that should exist

Proposed addition to the same file.

test/projects/close-project.test.ts+6
15+test("close then restore keeps linked exports", async () => {16+  const { id } = await seedProject({ exports: 3 })17+  await closeProject(id)18+  await restoreProject(id)19+  expect(await exports.list(id)).toHaveLength(3)20+})

On this branch: fails · expected length 3, received 0
On main: passes

Before merge

Requested changes, in order

  1. Keep close non-cascading and free the storage on day 30

    Revert line 22 to archive(id). Add purgeLinkedExports(projectId) to the day-30 retention job, so exports are removed when their tombstone expires. STOR-212 gets its storage back, a month later than this PR would.

    jobs/retention.tssketch
     for (const p of await projects.closedBefore(daysAgo(30))) {   await purgeLinkedExports(p.id) }

  2. Add the close-then-restore test

    Use the test above. It must fail on this branch as it stands and pass after change 01. Keep the existing return-value test; it still earns its place.

    No new fixtures are needed: seedProject() already takes an export count. The test runs in about 0.4 s against the seeded test database.

  3. If deletion at close is the intended behaviour, change the promise in the same PR

    Update the CloseProjectDialog copy and retention policy RET-3 alongside the code, and get sign-off from Support before merge. The code and the promise ship together or not at all.

    src/ui/CloseProjectDialog.tsxonly if chosen

    -You can restore a closed project for 30 days.

    +Closing deletes this project’s exports now. The project itself can be restored for 30 days.

Non-blocking

Rename cascade to an option that says it deletes, for example { purgeExports: true }. A reader should not need to open archive() to learn that the flag is destructive.

Verdict · request changes

Hold until recovery semantics and tests move together.

Mats Everly · Review desk · 12 Aug 2026

Open the reviewer verdictFull note to the author

Lena, thanks for picking up STOR-212. The storage numbers make the case, and the change is small enough to review properly.

I’m requesting changes on one point. { cascade: true } sends archive() down the purge branch, so linked exports are gone the moment a project closes. The close dialog and RET-3 still promise 30 days of recovery, and restoreProject() now returns a project without its exports and without an error. The existing test can’t see any of this because it only checks { closed: true }.

Please move the purge into the day-30 retention job and add the close-then-restore test in this PR. If product wants deletion at close, change the dialog copy and RET-3 here too, so the code and the promise ship together. The cascade rename is optional. I’ll re-review as soon as that test is in.

Mats