Skip to content

[api] Reparse an extended config cached under a different resolution stack - #64825

Open
Christopher Plieger (cplieger) wants to merge 1 commit into
microsoft:mainfrom
cplieger:fix-api-owner-cache-panic
Open

Christopher Plieger (cplieger) wants to merge 1 commit into
microsoft:mainfrom
cplieger:fix-api-owner-cache-panic

Conversation

@cplieger

Copy link
Copy Markdown
Contributor

When a config in a circular extends chain is parsed, getExtendedConfig parses the config that closes the cycle without the extended config cache, because it is already on the resolution stack. The cached entry of the config that extends it still lists it in ExtendedSourceFiles, though nothing acquired a cache entry for it. A second project that extends into the cycle reuses that cached entry, because its validity hash covers only file contents, and inherits the uncached path. Snapshot.Clone then calls AddOwner on that path and panics with OwnerCache.AddOwner: entry not found.

Impact: a repository with a circular extends chain and another config that extends into it makes createSnapshot fail with a Go panic whenever the cyclic project was opened first, where tsc -p reports TS18000 for each project, so an analyzer on the API fails on that repository depending on the order it opens its projects in.

The extended config cache's hash now also writes the name of each extended file that is on the current resolution stack, compared by PathKey as the circularity check does. An entry reached with a different set of its files on the stack is parsed again under that stack, and that parse acquires every file it lists. A config outside a cycle never has one of its extended files on its resolution stack, so its hash is the same as before and cache sharing is unchanged. TS18000 is reported as before.

The CLI's own extended config cache, used by tsc -p and tsc -b, has the same order dependence for configs inside an extends cycle, but it cannot panic and matches TypeScript 5.9's behavior, so this change leaves it alone.

TestOpenProjectsExtendingIntoCircularExtendsReportCircularity opens cycle, whose tsconfig.json and a.json extend each other, and then outer, which extends cycle/a.json, in one session. It expects TS18000 as the only config file parsing diagnostic of outer, and on main the second handleCreateSnapshot call panics. With a tsc built from this branch, the reproduction in the issue reports TS18000 in both orders.

TS18000 still prints its {0} placeholder unfilled, which has a separate cause and is left out of this change.

This follows #64754 and #64755. I met it while building deadset-ts, the TypeScript analyzer of deadset, on the TypeScript 7 API.

An AI coding agent wrote this patch. I have read, built and tested it and will handle the review.

  • There is an associated issue in the Backlog milestone (required)
  • Code is up-to-date with the main branch
  • You've successfully run npx hereby test
  • You've successfully run npx hereby lint
  • You've successfully run npx hereby check:format
  • There are new or updated tests validating the change

Fixes #64824

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Stack-sensitive invalidation can cause conflicting recursive cache locks during concurrent snapshot creation.

1 open finding
What changed in this PR

Addresses #64824 by making extended-config cache validation sensitive to the resolution stack, following the circular-extends fix in #64755.

Changes:

  • Invalidates cached configs when relevant resolution-stack membership changes.
  • Adds a regression test for opening a project that extends into a cached cycle.
File Description
tsc/​internal/​project/​extendedconfigcache.go Adds resolution-stack information to cache validation.
tsc/​internal/​api/​session_apistate_test.go Tests sequential project opening into a circular extends chain.

🧠 Review effort: Balanced


💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +52 to +53
if slices.Contains(args.ResolutionStack, args.ConfigFS.CaseSensitivity().PathKey(fileName.AsPath())) {
_, _ = hasher.WriteString(fileName.AsString())

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

[api] createSnapshot panics opening a project that extends into a circular extends chain

2 participants