[api] Answer nested requests on the sync connection in stack order

[Christopher Plieger (cplieger)]

While SyncConn.Call handles a nested request from a client callback, it releases its lock, so another goroutine's call can start inside the client's pending nested request. Responses are matched by method name, so the nested answer and both callback answers then reach the wrong requests.

Impact: a resolver callback that delegates to another resolver, the pattern the API's own test shows, binds most imports to the wrong module once the loader uses more than one thread. Every type and diagnostic the tool reads from that program is then wrong, and no error says so. In the reproduction, 873 of 900 nested answers went to another request.

The sync connection now answers exchanges in the order a synchronous client can handle them. It counts the calls in flight and records whether the innermost one is reading its response. A new call waits while one is reading. A message the client sends from inside a callback clears that flag, so the handler's own calls go through, and the call that received it reads again only once every call made above it has returned. A request is answered at the same point, which is what keeps nested answers from crossing.

TestSyncConnAnswersNestedRequestsInStackOrder drives two overlapping callbacks against a fake synchronous client under testing/synctest. It fails on each of 20 runs without the change. TestSyncConnNotificationHandlerCanCall keeps a call made from a notification handler working, as it does on main. With a tsc built from this branch, the reproduction in the issue gives 900 right answers of 900.

I met this while building deadset-ts, the TypeScript analyzer of deadset, on the TypeScript 7 API, as part of a small set of defects that work hit, which is why there are a few related reports from me.

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 #64631

[Copilot]

Copilot review overview

🔵 Needs a closer look

The bidirectional synchronization and panic-recovery behavior warrant final human review despite no concrete blocking findings.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes #64631 by coordinating synchronous IPC exchanges in stack order so overlapping callbacks receive the correct responses.

Changes:

• Tracks in-flight exchanges and waits before starting calls or answering requests.
• Adds regression tests for nested response ordering and response-write panic handling.

File	Description
tsc/​internal/​ipc/​conn_sync.go	Adds stack-based coordination for calls and responses.
tsc/​internal/​ipc/​conn_sync_test.go	Tests overlapping nested requests and panic propagation.

---

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

[Andrew Branch (andrewbranch)]

I think this is correct and I don't see a simpler way to do it, but Jake Bailey (@jakebailey) has talked me out of using a sync.Cond before, so he may want to take a look.

[Jake Bailey (jakebailey)]

Yeah, I blindly assigned this to you, but now that I look, I suspect there's a better design here

[Andrew Branch (andrewbranch)]

I had tried to think about blocking "unrelated" client callbacks until the current request stack is clear, but it turns out it's kind of hard to determine what's "unrelated" vs. what's truly reentrant here. I think you could do something with context plumbing, but I think that would end up being a lot more complicated.

[Christopher Plieger (cplieger)]

Jake Bailey (@jakebailey) Andrew Branch (@andrewbranch) thanks both, I pushed a smaller version.

• Smaller shape (as Jake suspected): the stack, push/pop and the answered flag are gone. Two fields, a call count and a reading flag, plus one lockTurn carry the same rule.
• Context plumbing (as Andrew suggested): I tried it first. The filesystem callbacks go through vfs.FS, which takes no context, so a file read made while serving a nested request looks unrelated and waits on the call that is waiting for it. Serialising top-level calls hits the same question.
• sync.Cond (following Andrew's note): a channel version came out as a larger hand-written condition variable, so I kept the Cond. Its Wait is no less cancellable than the Lock beside it. Happy to switch if you still prefer channels.
• Notifications: a call from a notification handler no longer blocks. One limit stays exactly as on main: while a notification handler runs, an unrelated call can still start. msgpack has no notifications, so the API server cannot reach it.

[Copilot]

Copilot review overview

🔵 Needs a closer look

Reentrant IPC synchronization needs human review of concurrent interleavings and error paths without runtime validation here.

Review effort: Balanced
Findings: None

[Jake Bailey (jakebailey)]

With the latest push, I think this is fine.