ai-config: concurrent rebuilds in watchResolvedProviderCatalog drop or invert change events

[jonvanausdeln]

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.