Skip to content

fix(server): one in-flight temporary-folder probe per path - #100

Merged
chrisohearn merged 2 commits into
integration/mcp-events-20261109from
r4/temp-probe-dedupe
Nov 13, 2026
Merged

chrisohearn merged 2 commits into
integration/mcp-events-20261109from
r4/temp-probe-dedupe

Conversation

@chrisohearn

Copy link
Copy Markdown
Owner

Follow-up to #86 (delta review finding): the temporary-folder probe times out after 500 ms, but the underlying access/realpath/statfs calls keep running on a libuv thread. On a dead network mount (NFS, SSHFS) they never settle, so every project create or launch into that path would pin another thread.

  • Change. dedupeProbe wraps the filesystem probe: one probe per path at a time (a retry joins the unanswered probe), and a settled answer is reused for 30 s. A rejected probe is not cached. The cache holds at most 256 paths. Message-getter callers still use the name-only check and never probe.
  • Test. temporaryFolder.test.ts: a never-settling probe is joined by a second call for the same path (one probe call); another path probes on its own; a settled answer is reused until the TTL passes.
  • Checks (focused, under the heavy lock). server typecheck; temporaryFolder.test.ts (15 tests). No migration.
  • Acceptance. Read-only: none needed beyond CI. Behavior is unchanged except fewer probes.

🤖 Generated with Claude Code

…ort result cache

A probe on a dead network mount never settles and pins a libuv thread. A retry now joins the unanswered probe instead of starting another, and a settled answer is reused for 30 s.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-13T09:23:57.262271Z d08e392 New commits
🔒 Security Review ✅ Completed 2026-11-13T09:24:45.970253Z d08e392 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 388d3f91a3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +86 to +87
// Map order is insertion order, so the first key is the oldest.
if (entries.size > MAX_CACHED_PATHS) entries.delete(entries.keys().next().value!);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep in-flight probes out of the bounded result cache

When more than 256 distinct paths are probed while an older probe is still unresolved, this eviction removes that in-flight entry. A later request for the evicted path then starts another filesystem call instead of joining the existing one, so repeated traffic across a sufficiently large project set can again queue unbounded calls against a dead mount—the resource exhaustion this change is meant to prevent. Evict only settled cached results, or track unresolved probes separately until they settle.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in the next commit. Unanswered probes now live in their own set, apart from the bounded cache of settled answers, so eviction cannot forget them. A retry while a probe is unanswered gets no answer at once and goes by the name, so nothing adds a reaction to the pending promise.

Comment on lines +78 to +81
known !== undefined &&
(known.settledAt === null || now() - known.settledAt < resultTtlMs)
) {
return known.answer;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid accumulating waiters on a permanently pending promise

When the probe for one dead path never settles and that project is checked repeatedly, returning the same pending native promise makes every isTemporaryFolder call attach another .catch/await reaction to it. The Effect timeout abandons the caller but cannot detach those native promise reactions, and this cache deliberately retains the pending promise forever, so a single frequently retried path can grow memory without bound even though it now consumes only one libuv thread. Later calls should fall back without subscribing again, or share a single timeout wrapper that is allowed to settle.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in the next commit. Unanswered probes now live in their own set, apart from the bounded cache of settled answers, so eviction cannot forget them. A retry while a probe is unanswered gets no answer at once and goes by the name, so nothing adds a reaction to the pending promise.

…result cache

Eviction could forget an unanswered probe, and every retry added a reaction to its pending promise. A retry during an unanswered probe now gets no answer at once, which sends it to the name check.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@chrisohearn
chrisohearn merged commit a59003e into integration/mcp-events-20261109 Nov 13, 2026
18 checks passed
chrisohearn pushed a commit that referenced this pull request Nov 13, 2026
…ol-reliability

t3_thread_configure reads its configuration only when none of
modelSelection, runtimeMode, applyAt and acknowledgeOnly is given.
Budget re-measured on the tip.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants