Skip to content

test: remove shared-DB and timer flakiness - #338

Open
coleleavitt wants to merge 3 commits into
cortexkit:masterfrom
coleleavitt:test/isolate-flaky-storage
Open

test: remove shared-DB and timer flakiness#338
coleleavitt wants to merge 3 commits into
cortexkit:masterfrom
coleleavitt:test/isolate-flaky-storage

Conversation

@coleleavitt

@coleleavitt coleleavitt commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Summary

  • set plugin test timeout to 30s so SQLite's 5s busy-timeout reports the actual lock stack instead of Bun killing the test first
  • batch the 2,005 embedding-measurement setup inserts in one SQLite transaction
  • return real completion promises from message-index schedulers and await them with a fake boot-quiet clock
  • warm @opentui/core before parallel runtime-specifier imports to remove the TreeSitterClient TDZ race exposed by CI

Addresses #312 (improves lock-failure observability; per-file DB isolation remains open)
Fixes #330
Fixes #333

Verification

  • targeted storage/scheduler scope: 16 pass
  • two concurrent targeted runners: both exit 0
  • 20 repeated scheduler runs: pass
  • 20 repeated TUI runtime-import runs: pass
  • bun run lint
  • bun run typecheck
  • plugin build

Assertions are unchanged; no SQLite timeout was hidden or extended.

Greptile Summary

The PR reduces SQLite and scheduler test flakiness without changing production database timeout behavior.

  • Extends the plugin test timeout to 30 seconds so SQLite lock failures surface before the test runner terminates them.
  • Batches embedding-measurement fixture inserts in one transaction.
  • Exposes scheduler completion promises and uses a fake boot-quiet clock in the overlap test.
  • Serializes the TUI core/testing module initialization path.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/plugin/package.json Raises the package test timeout so SQLite's configured busy timeout can produce a useful failure first.
packages/plugin/src/features/magic-context/message-index-async.ts Returns deduplicated completion promises from reconciliation and clear/reindex scheduling while preserving per-session serialization.
packages/plugin/src/features/magic-context/message-index-async.test.ts Replaces wall-clock waiting with fake timers and awaits both scheduler completion promises.
packages/plugin/src/features/magic-context/storage-embedding-measurements.test.ts Batches the large measurement-corpus fixture setup in one SQLite transaction.
packages/plugin/src/tui/tui-compiled-runtime-imports.test.ts Initializes the TUI core module before parallel runtime imports to avoid a module-initialization TDZ.

Sequence Diagram

sequenceDiagram
    participant Test
    participant BootQuiet as Boot-quiet scheduler
    participant Lock as Session lock
    participant Index as Message index
    Test->>BootQuiet: schedule reconciliation
    Test->>BootQuiet: schedule clear and reindex
    Test->>BootQuiet: advance fake clock
    BootQuiet->>Lock: queue reconciliation
    BootQuiet->>Lock: queue clear and reindex
    Lock->>Index: reconcile existing messages
    Lock->>Index: clear indexed session
    Lock->>Index: rebuild from surviving messages
    Index-->>Test: resolve completion promises
Loading

Reviews (3): Last reviewed commit: "test(tui): stabilize runtime import orde..." | Re-trigger Greptile

Context used:

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 4 files

Re-trigger cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 issues found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/plugin/package.json">

<violation number="1" location="packages/plugin/package.json:42">
P2: The PR's stated first goal is "Enable Bun test-file isolation for plugin tests," but this change removes the `--isolate` flag that was added earlier in this PR, leaving the test command with no isolation flag (only `--timeout 30000`). If Bun does not isolate test files into separate processes by default, the shared-state/shared-DB flakiness the PR exists to fix is not actually removed. Either restore `--isolate` (combined with the timeout) or confirm in the PR description that Bun isolates files by default and that isolation is intentionally dropped.</violation>

<violation number="2" location="packages/plugin/package.json:42">
P3: The new `--timeout 30000` raises the global per-test timeout from Bun's 5000ms default to 30s for every test file, which contradicts the PR's stated verification that "no global timeouts increased" and masks genuinely hanging tests in CI for 30s each. Set only the specific tests that actually need the longer budget instead of raising the global default, or document which tests justify 30s.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

"check:tui-compiled": "bun run build:tui && git diff --exit-code -- src/tui-compiled && test -z \"$(git status --porcelain -- src/tui-compiled)\"",
"typecheck": "tsc -p ../retina-local-fs/tsconfig.build.json && tsc --noEmit && tsc -p tsconfig.scripts.json",
"test": "bun test",
"test": "bun test --timeout 30000",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: The PR's stated first goal is "Enable Bun test-file isolation for plugin tests," but this change removes the --isolate flag that was added earlier in this PR, leaving the test command with no isolation flag (only --timeout 30000). If Bun does not isolate test files into separate processes by default, the shared-state/shared-DB flakiness the PR exists to fix is not actually removed. Either restore --isolate (combined with the timeout) or confirm in the PR description that Bun isolates files by default and that isolation is intentionally dropped.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/plugin/package.json, line 42:

<comment>The PR's stated first goal is "Enable Bun test-file isolation for plugin tests," but this change removes the `--isolate` flag that was added earlier in this PR, leaving the test command with no isolation flag (only `--timeout 30000`). If Bun does not isolate test files into separate processes by default, the shared-state/shared-DB flakiness the PR exists to fix is not actually removed. Either restore `--isolate` (combined with the timeout) or confirm in the PR description that Bun isolates files by default and that isolation is intentionally dropped.</comment>

<file context>
@@ -39,7 +39,7 @@
     "check:tui-compiled": "bun run build:tui && git diff --exit-code -- src/tui-compiled && test -z \"$(git status --porcelain -- src/tui-compiled)\"",
     "typecheck": "tsc -p ../retina-local-fs/tsconfig.build.json && tsc --noEmit && tsc -p tsconfig.scripts.json",
-    "test": "bun test --isolate",
+    "test": "bun test --timeout 30000",
     "lint": "biome check .",
     "lint:fix": "biome check --write .",
</file context>
Suggested change
"test": "bun test --timeout 30000",
"test": "bun test --isolate --timeout 30000",

"check:tui-compiled": "bun run build:tui && git diff --exit-code -- src/tui-compiled && test -z \"$(git status --porcelain -- src/tui-compiled)\"",
"typecheck": "tsc -p ../retina-local-fs/tsconfig.build.json && tsc --noEmit && tsc -p tsconfig.scripts.json",
"test": "bun test",
"test": "bun test --timeout 30000",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P3: The new --timeout 30000 raises the global per-test timeout from Bun's 5000ms default to 30s for every test file, which contradicts the PR's stated verification that "no global timeouts increased" and masks genuinely hanging tests in CI for 30s each. Set only the specific tests that actually need the longer budget instead of raising the global default, or document which tests justify 30s.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/plugin/package.json, line 42:

<comment>The new `--timeout 30000` raises the global per-test timeout from Bun's 5000ms default to 30s for every test file, which contradicts the PR's stated verification that "no global timeouts increased" and masks genuinely hanging tests in CI for 30s each. Set only the specific tests that actually need the longer budget instead of raising the global default, or document which tests justify 30s.</comment>

<file context>
@@ -39,7 +39,7 @@
     "check:tui-compiled": "bun run build:tui && git diff --exit-code -- src/tui-compiled && test -z \"$(git status --porcelain -- src/tui-compiled)\"",
     "typecheck": "tsc -p ../retina-local-fs/tsconfig.build.json && tsc --noEmit && tsc -p tsconfig.scripts.json",
-    "test": "bun test --isolate",
+    "test": "bun test --timeout 30000",
     "lint": "biome check .",
     "lint:fix": "biome check --write .",
</file context>
Suggested change
"test": "bun test --timeout 30000",
"test": "bun test"

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

Labels

None yet

Projects

None yet

1 participant