Skip to content

Fix attachment retention test lifecycle race - #2337

Open
baixiaohang wants to merge 1 commit into
mainfrom
fix/background-retention-lifecycle
Open

Fix attachment retention test lifecycle race#2337
baixiaohang wants to merge 1 commit into
mainfrom
fix/background-retention-lifecycle

Conversation

@baixiaohang

Copy link
Copy Markdown
Collaborator

Summary

  • disable automatic attachment-retention scheduling in generic Server test apps while keeping production and explicit opt-in behavior enabled
  • track active retention sweeps and wait for them during background-task shutdown before the database pool closes
  • add deterministic regressions for the test-helper wiring and the in-flight shutdown boundary

Root cause

Main CI run 31784141632 failed when the per-test TRUNCATE ... CASCADE deadlocked with the startup attachment-retention query. The retention sweep was launched fire-and-forget from backgroundTasks.start(), so a test app could return from ready() while the sweep was still using the same worker database.

The failed me-multi-org test was only the cleanup victim. PR #2334 did not change the test harness, background task, retention service, or failed test.

Validation

Passed locally:

  • related Server suites: 4 files / 49 tests with --maxWorkers=2
  • background-task and bare-test-app final suites: 2 files / 7 tests
  • full Server suite: 293 files / 3441 tests with --maxWorkers=2
  • pnpm --filter @first-tree/server typecheck
  • changed-file Biome checks
  • git diff --check

Mutation checks also proved both regressions are effective:

  • removing the in-flight stop() wait changes the settlement race from sweep-pending to stopped
  • removing the createTestApp() default-disable wiring starts the mocked retention sweep and fails the helper-level test

@baixiaohang
baixiaohang requested a review from yuezengwu as a code owner August 14, 2026 09:13

@yuezengwu yuezengwu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the lifecycle fix. createTestApp() now disables the retention sweep by default while production remains enabled unless explicitly opted out; active retention sweeps are tracked and awaited before the database pool closes. The helper-level and in-flight shutdown regressions cover both failure boundaries. No schema or core data-structure changes. No blocking findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants