ai-config: concurrent rebuilds in watchResolvedProviderCatalog drop or invert change events
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 5
- Forks
- 2
- Avg merge
- 13h 33m
- Merged PRs (30d)
- 35
Description
How this surfaced
Diagnosing a flaky extension-host test in Positron: providerCatalog > disabling a provider surfaces it in disabledIds (extensions/authentication/src/test/providerCatalog.test.ts) had been timing out on the Electron ext-host lane on most main merge runs since 2026-07-31, with Error: Timed out waiting for onDidChangeProviderCatalog. Chasing why the event never arrived led here.
Two caveats so this isn't over-read:
- It's unproven that this race is what was failing in CI. It predicts a binary outcome (event or no event), while the observed CI timings show a continuous latency spread — 705ms, 2427ms, 4836/5365/5500/5564ms, then timeout — which is better explained by extension-host contention alone. Both may be true.
- The Positron-side flake is being mitigated separately in posit-dev/positron#15294 by removing that test's dependence on the watcher. That mitigation does not fix the bug below, which stands on its own merits.
The bug
rebuild() in packages/ai-config/src/node/watch-catalog.ts is async and unserialized. It awaits every source's read() and only then assigns previousCatalog:
const settled = await Promise.all(sourceProviders.map((p) => p.read())); // L73
...
const change = diffCatalogs(previousCatalog, newCatalog); // L85
previousCatalog = newCatalog; // L86
if (change) { handler(change); }
The initial snapshot fires as a bare void rebuild() (L109), with no coordination against the watch callbacks registered immediately after. If a file edit arrives while that initial read is still in flight, the debounced rebuild (300ms, L105) can complete first and the two rebuilds land out of order.
Two failure modes, depending on what the delayed initial read observes:
| Initial read resolves with | Result |
|---|---|
| pre-edit content | change event fires carrying stale content — reports the provider as enabled when disk says disabled |
| post-edit content | no event at all — both rebuilds produce identical catalogs, diffCatalogs returns undefined (L280), the edit is silently swallowed |
User-visible impact: a providers.json edit landing during startup is either dropped entirely — with nothing re-syncing until the next edit — or applied backwards.
The stale arm also poisons downstream caches. Positron's applyCatalog() consumes change.catalog directly, so an inverted event writes wrong state into the extension's cache.
Repro
Both arms fail deterministically on current main. Controlling readFileConfig via vi.mock forces the interleaving without racing real I/O:
const { readFileConfigMock } = vi.hoisted(() => ({ readFileConfigMock: vi.fn() }));
vi.mock("../node/load-config.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../node/load-config.js")>();
return { ...actual, readFileConfig: readFileConfigMock }; // readEnvFragment stays real
});
it("still fires when the delayed initial read observes the post-edit file", async () => {
const configPath = path.join(tempDir, "providers.json");
await writeConfig(configPath, { providers: { anthropic: { enabled: true } } });
const initialRead = deferred<ProvidersConfig>();
let reads = 0;
readFileConfigMock.mockImplementation(async () => {
reads += 1;
if (reads === 1) { return initialRead.promise; } // park the initial snapshot
return JSON.parse(await fs.readFile(configPath, "utf-8"));
});
const changes: ProviderCatalogChange[] = [];
const watcher = watchResolvedProviderCatalog((c) => changes.push(c), {
baseline: STANDALONE_BASELINE, configPath, logger: mockLogger,
});
await writeConfig(configPath, { providers: { anthropic: { enabled: false } } });
await new Promise((r) => setTimeout(r, 700)); // let the debounced rebuild finish
initialRead.resolve({ providers: { anthropic: { enabled: false } } });
await new Promise((r) => setTimeout(r, 300));
watcher.dispose();
expect(changes.length).toBeGreaterThanOrEqual(1); // actual: 0
});
AssertionError: the edit must not be swallowed: expected 0 to be greater than or equal to 1
The sibling arm — resolving the initial read with the pre-edit content — fails with the catalog reporting enabled: true when disk says false.
Suggested fix
Serialize rebuilds: chain each on the previous in-flight promise, or tag them with a sequence number and discard stale results. Deferring watch registration until the initial snapshot settles is a smaller change but only covers the startup arm — the inverted-event arm can also occur between two edits spaced further apart than the debounce.
Note the existing tests in watch-catalog.test.ts use the same "sleep 500ms for the initial load, then write" shape and are latently exposed to the same race.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in packages/ai-config/src/node/watch-catalog.ts, especially rebuild() and the initial watcher setup, then review the existing watch-catalog.test.ts tests. Reproduce both delayed-initial-read cases from the issue and add regression coverage. Done means concurrent rebuilds cannot drop or invert catalog change events, including edits during startup.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100