[Timur Vural (timurzvural)]

Fixes #65071.

Bug

dirty.SyncMap.LoadOrStore returned (nil, false) when the key had a delete tombstone. Callers such as acquireConfigForFile ignore the bool and immediately call entry.ChangeIf(...), which nil-dereferences after closeProjects + openFiles (same or separate updates).

Same root cause as #65054 and #64911.

Fix

When LoadOrStore finds a tombstoned entry, resurrect it with the provided value (delete = false, dirty = true) and return (entry, false) instead of nil.

Tests

• dirty.TestLoadOrStoreAfterDeleteTombstone — base and dirty-only tombstones
• api.TestCloseProjectsThenOpenFilesDoesNotPanic — API: closeProjects then openFiles crashes tsgo (nil ChangeIf in acquireConfigForFile) #65071 repro via standalone snapshot API

AI assistance disclosure

This PR was authored with assistance from Cursor Agent. I have read and understand the change and am prepared to discuss and revise it in review.

[Copilot]

🟢 Approval recommended

No active review issues block the change restoring tombstoned entries to prevent crashes when projects close and files reopen.

No open issues 🎉

What changed in this PR

Fixes #65071 by making dirty.SyncMap.LoadOrStore restore deleted entries instead of returning nil, preventing crashes when snapshot updates close projects and reopen files.

Changes:

• Restore tombstoned entries with the supplied value and mark them dirty.
• Add unit tests for base-map and dirty-only tombstones.
• Add API regression coverage for closing projects and reopening files in combined and separate updates.

File	Description
tsc/​internal/​project/​dirty/​syncmap.go	Restore tombstoned entries in both LoadOrStore paths.
tsc/​internal/​project/​dirty/​syncmap_tombstone_test.go	Test restoration, return values, and subsequent lookups.
tsc/​internal/​api/​session_apistate_test.go	Test close-project and reopen-file snapshot sequences.

_{Review effort: Balanced}

[AHMED YOUNIS (aetherxeg-source)]

One lifecycle edge case may be worth testing before merging the tombstone-resurrection change.

I reviewed the three-file patch in #65072, particularly both resurrection branches in dirty.SyncMap.LoadOrStore().

The change resolves the immediate nil-entry problem, but it also reuses the same *SyncMapEntry object when a deleted key is recreated.

That introduces a potential stale-reference concern: a caller holding an entry from the previous lifecycle may still be able to mutate or delete its replacement.

A small regression probe using the existing testValue fixture would be:

m := NewSyncMap(map[string]*testValue{})

old, _ := m.LoadOrStore("k", &testValue{data: "old"})
old.Delete()

replacement, _ := m.LoadOrStore("k", &testValue{data: "new"})

// The proposed implementation reuses the original pointer.
if old != replacement {
    t.Fatal("unexpected entry identity")
}

// Simulate a late operation through the stale reference.
old.Delete()

_, exists := m.Load("k")
// Under the current patch, exists is expected to be false.

This is a source-derived test proposal, not an independently executed result.

Why it matters: if an entry reference can survive a project-close or snapshot-update boundary, a delayed operation targeting the old lifecycle may affect the newly acquired configuration entry.

The existing tests cover successful resurrection and the original crash, but do not check whether stale entry references remain capable of changing the replacement.

Could we clarify the intended contract?

• If entry handles are guaranteed not to survive deletion, that lifetime invariant should be explicit and enforced by the relevant callers.
• If they can survive, resurrection may need a new entry identity or generation-aware protection, while preserving the existing synchronization and dirty-map semantics.

The important distinction is:

Reusing a key does not necessarily authorize reusing the previous entry's mutable identity.

A targeted stale-reference regression would help establish whether the current fix is safe across snapshot lifecycle transitions.