fix: avoid reader use after seeding side-effect in inline completions fetch (fixes #323531)

[vs-code-engineering]

Summary

InlineCompletionsModel's _fetchInlineCompletionsPromise derived performs a state-mutating side-effect — seedInlineCompletionsWithSuggestWidget() — in the middle of its compute, then continues to read reader-tracked observables. The seed opens a transaction whose synchronous commit re-enters the derived through the recomputeInitiallyAndOnChange keep-alive and invalidates the active reader, so the next this._didUndoInlineEdits.read(reader) throws "The reader object cannot be used outside its compute function!". The crash is intermittent and most common when inlineSuggest.experimental.showOnSuggestConflict is active, which forces the seed gate to fire while the suggest widget is open.

Fixes #323531
Recommended reviewer: @hediet

Culprit Commit

Field	Value
Commit	e61cfda
Author	@hediet
PR	N/A — pushed directly to main (tracks an internal backlog item)
Message	Implements inlineSuggest.experimental.showOnSuggestConflict
Why	This commit added the _shouldShowOnSuggestConflict override (if (this._shouldShowOnSuggestConflict.read(undefined)) { suggestItem = undefined; }) immediately before the seed gate if (suggestWidgetInlineCompletions && !suggestItem). Forcing suggestItem = undefined makes the gate fire even while a suggestion is selected, driving seedInlineCompletionsWithSuggestWidget() — and its reader-invalidating re-entrant transaction — far more often. The underlying seed-in-compute pattern predates this commit, so this is best characterized as the recent-regression driver rather than the sole origin; precise build bisection was not possible because the workflow checkout is shallow (fetch-depth=1), which prevents local git blame/ancestry verification.

Code Flow

sequenceDiagram
    participant Trigger as Suggest widget / config
    participant Fetch as fetchInlineCompletionsPromise derived
    participant Seed as seedInlineCompletionsWithSuggestWidget
    participant KeepAlive as recomputeInitiallyAndOnChange
    participant Derived as derivedImpl recompute

    Trigger->>Fetch: recompute, reader is valid
    Note over Fetch: reads _selectedSuggestItem and _textModelVersionId via reader
    Fetch->>Seed: seedInlineCompletionsWithSuggestWidget at L388
    Note over Seed: Root cause: opens transaction and mutates _state
    Seed->>KeepAlive: transaction commit re-enters the keep-alive observer
    KeepAlive->>Derived: recomputes the same derived re-entrantly
    Note over Derived: finally block sets _isReaderValid = false
    Derived-->>Fetch: returns, reader now invalid
    Fetch->>Fetch: _didUndoInlineEdits.read(reader) at L395
    Note over Fetch: Error thrown: reader used outside its compute function

Loading

Affected Files

File	Role	Evidence
src/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionsModel.ts	crash site + root cause	L395 (crash, from stack): this._didUndoInlineEdits.read(reader) executes after the seed; L388 (root cause): this._source.seedInlineCompletionsWithSuggestWidget() is called inside the derived compute, before two later reader reads (L395, L428)
src/vs/editor/contrib/inlineCompletions/browser/model/inlineCompletionsSource.ts	re-entrancy producer	L444-L462: seedInlineCompletionsWithSuggestWidget() opens transaction(tx => { ... this._state.set(...) ... }), whose synchronous commit re-enters the keep-alive observer that recomputes the fetch derived
src/vs/base/common/observableInternal/observables/derivedImpl.ts	invariant enforcement (framework — not modified)	L335: _ensureReaderValid() throws the error; _recompute()'s finally sets _isReaderValid = false, which the re-entrant recompute leaves behind

Repro Steps

This is a timing-dependent re-entrancy bug; it requires the seed gate to fire during a fetch recompute. To maximize the likelihood:

1. Enable inlineSuggest.experimental.showOnSuggestConflict (this forces suggestItem = undefined, widening the seed gate).
2. With an active inline-completion provider, type to trigger automatic suggestions so the inline completions fetch derived recomputes.
3. Open the built-in suggest (IntelliSense) widget so _source.suggestWidgetInlineCompletions is populated while no inline suggest item is selected.
4. On the next recompute the gate suggestWidgetInlineCompletions && !suggestItem is true, seedInlineCompletionsWithSuggestWidget() runs, its transaction re-enters and invalidates the reader, and the following this._didUndoInlineEdits.read(reader) throws. Without the experimental flag the same crash occurs whenever the suggest widget has completions but no item is selected during a fetch recompute.

How the Fix Works

Chosen approach (inlineCompletionsModel.ts): The two reader-tracked reads that previously ran after the seed — this._didUndoInlineEdits.read(reader) (was L395) and this._inlineEditsEnabled.read(reader) (was L428) — are captured into locals immediately before the seed gate, and the later sites use those locals. The seeding side-effect itself is unchanged, but it is now the last reader-affecting operation in the compute, so no read(reader) can run after the reader has been invalidated by the seed's re-entrant transaction. This fixes the producer of the invalid sequence — the side-effect ordering inside the derived — rather than guarding the crash site. It mirrors the discipline already present in this same compute, where the clear transaction is immediately followed by return precisely so that no reader read follows it. The governing principle is to order all reader reads before any state-mutating side-effect within a derived compute.

Behavioral equivalence is preserved: _inlineEditsEnabled is inlineSuggest.map(v => !!v.edits.enabled) (config-derived) and _didUndoInlineEdits tracks only _textModelVersionId; neither depends on _state, so reading them a few lines earlier returns identical values. Hoisting _didUndoInlineEdits above the dontRefetch early-return adds no new effective subscription, because its sole upstream _textModelVersionId is already read unconditionally earlier in the same compute.

Alternatives considered:

• Relocating the seed out of the derived into a separate autorun — rejected: it changes when seeding runs relative to the fetch and risks subtle double-fetch/ordering changes, which is broader than the verified defect.
• Adding a guard or try/catch at the crash-site read — rejected: that hides the symptom at the consumer instead of fixing the producer-side ordering, and would silence the error telemetry without removing the re-entrancy.
• Moving the seed below the reader reads instead of moving the reads above it — rejected: the seed currently runs before the dontRefetch and didUndo early-returns, so relocating it past them would skip seeding in those branches, a behavior change.

Recommended Owner

@hediet — Henning Dieterichs authored both the inline-completions model / observable reader framework and the inlineSuggest.experimental.showOnSuggestConflict feature (commit e61cfda) whose override widens the seed trigger. He is a core VS Code editor-team maintainer with write access and is actively committing to this exact file (merges through 2026-02), satisfying both the team-membership and liveness gates. Cascade step 1 (culprit-commit author) selects him directly.

Generated by errors-fix · 2.5K AIC · ⌖ 151.1 AIC · ⊞ 69.4K · ◷

[Copilot]

Copilot can't review bot-authored pull requests automatically. A user with Copilot access can request a review manually.

[Copilot]

Copilot can't review bot-authored pull requests automatically. A user with Copilot access can request a review manually.