Skip to content

fix(agentHost): await dbClose to resolve flaky session database tests#313810

Open
ruryu wants to merge 3 commits intomicrosoft:mainfrom
ruryu:fix-session-db-flake
Open

fix(agentHost): await dbClose to resolve flaky session database tests#313810
ruryu wants to merge 3 commits intomicrosoft:mainfrom
ruryu:fix-session-db-flake

Conversation

@ruryu
Copy link
Copy Markdown

@ruryu ruryu commented May 1, 2026

Summary

This PR fixes a flaky test issue in the SessionDatabase module (#306057). The root cause was an un-awaited db.close() call, which led to race conditions where a database file might still be locked when the next test case started.

Changes

  • Database Lifecycle: Replaced db.close() with dbClose(db) in sessionDatabase.ts to ensure the asynchronous database closure is properly awaited.
  • Type Mapping: Refactored SQLite row mapping to correctly handle null values from the database, ensuring they are accurately represented in TypeScript.
  • Enable Test: Re-enabled the 'returns empty array when given empty array' test case which was previously skipped due to flakiness.

Verification

  • Verified by running scripts/test.sh --grep "SessionDatabase" on WSL2 with Node v22.
  • 30 tests passing consistently.
  • Stress-tested the suite multiple times to ensure the race condition is resolved.

Fixes #306057

Copilot AI review requested due to automatic review settings May 1, 2026 22:30
@ruryu
Copy link
Copy Markdown
Author

ruryu commented May 1, 2026

@microsoft-github-policy-service agree

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses flakiness in SessionDatabase unit tests by ensuring the underlying SQLite connection is fully closed before subsequent tests run, avoiding file-lock race conditions in the agent host platform layer.

Changes:

  • Await SQLite database closure by switching db.close() to the existing promise wrapper dbClose(db) in SessionDatabase.close().
  • Re-enable the previously skipped regression test for getFileEdits([]) returning an empty array.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/vs/platform/agentHost/node/sessionDatabase.ts Ensures close() truly awaits SQLite connection shutdown by using dbClose(db).
src/vs/platform/agentHost/test/node/sessionDatabase.test.ts Re-enables the previously skipped flaky test case after the close race is addressed.

Comment thread src/vs/platform/agentHost/test/node/sessionDatabase.test.ts Outdated
ruryu and others added 2 commits May 2, 2026 06:35
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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.

Flaky test: sessionDatabase "returns empty array when given empty array"

3 participants