Skip to content

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

Merged
Connor Peet (connor4312) merged 3 commits into
microsoft:mainfrom
ruryu:fix-session-db-flake
May 4, 2026
Merged

Connor Peet (connor4312) merged 3 commits into
microsoft:mainfrom
ruryu:fix-session-db-flake

Conversation

@ruryu

Copy link
Copy Markdown
Contributor

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
Contributor Author

@microsoft-github-policy-service agree

Copilot AI 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.

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.

@connor4312 Connor Peet (connor4312) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

thanks!

@connor4312
Connor Peet (connor4312) enabled auto-merge (squash) May 4, 2026 15:03
@connor4312
Connor Peet (connor4312) merged commit a832f4a into microsoft:main May 4, 2026
26 checks passed
@vs-code-engineering vs-code-engineering Bot added this to the 1.120.0 milestone May 4, 2026
@vs-code-engineering vs-code-engineering Bot locked and limited conversation to collaborators Jun 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

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"

4 participants