Skip to content

Sequence setMetadata writes per key to fix flaky session config test#311989

Merged
roblourens merged 1 commit intomainfrom
roblou/agents/fix-flaky-ci-session-config-test
Apr 22, 2026
Merged

Sequence setMetadata writes per key to fix flaky session config test#311989
roblourens merged 1 commit intomainfrom
roblou/agents/fix-flaky-ci-session-config-test

Conversation

@roblourens
Copy link
Copy Markdown
Member

Symptom

The integration test Protocol WebSocket - Session Config persistence across restarts > persisted config values are restored on subscribe after server restart has been intermittently failing in CI:

+ expected - actual
 {
-  "branch": "main"
+  "branch": "release"
   "isolation": "worktree"
 }

The test creates a session, dispatches SessionConfigChanged to switch the branch from main to release, restarts the server, and then asserts that release is what comes back. Sometimes main came back instead.

Root cause

@vscode/sqlite3 runs in parallelized mode by default — db.run() calls on the same connection are dispatched to libuv's thread pool and can complete out of submission order.

In this scenario, two setMetadata('configValues', …) writes happen back-to-back on the same connection:

  1. AgentService._persistConfigValues writes the initial config when the session is created.
  2. AgentSideEffects._persistSessionFlag writes the updated config after SessionConfigChanged.

Both writes are correctly awaited via the existing whenIdle() flush, but the second db.run can finish before the first one inside SQLite's worker pool, so the older value ends up as the final on-disk row. Captured timing during a reproduction confirmed this:

  • Write 2 (release) completed at dt=7ms
  • Write 1 (main) completed at dt=13ms → main was the last writer to disk

This is not just a test artefact: any rapid sequence of SessionConfigChanged actions on the same key (e.g. fast UI toggles) could lose the latest value in production.

Fix

Route setMetadata through a SequencerByKey<string> keyed by metadata key, so writes for the same key run in submission order while writes for different keys still run concurrently. This is the same pattern already used for storeFileEdit (keyed by file path) for the same reason.

The fix is intentionally narrow:

  • Other write paths (createTurn, deleteTurn, truncateFromTurn, etc.) are not "last-writer-wins on a key" — they're inserts/deletes/transactions where out-of-order completion doesn't have the same hidden corruption mode.
  • Per-key sequencing preserves concurrency for different keys.

Validation

Stress-tested locally by running 12 parallel test processes in batches:

  • Before fix: ~1–2 failures per 100 runs.
  • After fix: 180 / 180 runs pass.

(Written by Copilot)

@vscode/sqlite3 runs in parallelized mode and dispatches db.run() calls
to libuv's thread pool, so two setMetadata calls for the same key
submitted close together can complete in the opposite order they were
 the later (and intended-final) value silently loses.submitted

Surfaced by the 'Session Config persistence across restarts' integration
test, which dispatches SessionConfigChanged shortly after createSession,
then asserts the new branch is restored after a server restart. Repros
in roughly 1-2 / 100 local runs under load; was a recurring CI flake.

Fix: route setMetadata through a SequencerByKey<string> so writes for
the same key run in submission order while writes for different keys
still run concurrently. Same pattern already used for storeFileEdit.

(Written by Copilot)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 22, 2026 20:02
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 an intermittent CI failure (and a real production risk) where rapid consecutive setMetadata calls could be persisted out-of-order due to @vscode/sqlite3 running db.run() operations in parallelized mode.

Changes:

  • Add a per-metadata-key SequencerByKey<string> to serialize setMetadata writes for the same key.
  • Route setMetadata through the new sequencer while preserving concurrency across different metadata keys.
Show a summary per file
File Description
src/vs/platform/agentHost/node/sessionDatabase.ts Adds a per-key sequencer for session metadata writes and applies it in setMetadata to guarantee submission-order persistence for “last-writer-wins” keys.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@roblourens roblourens merged commit c08fa67 into main Apr 22, 2026
30 checks passed
@roblourens roblourens deleted the roblou/agents/fix-flaky-ci-session-config-test branch April 22, 2026 20:29
@vs-code-engineering vs-code-engineering Bot added this to the 1.118.0 milestone Apr 22, 2026
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.

3 participants