Wait briefly for connecting servers before answering an empty tool search

[TheGreatAxios]

Summary

• An empty tool search waits briefly for in-flight connector handshakes before answering, so a mid-handshake search finds late-mounting tools instead of reporting a definitive miss
• A miss with handshakes still pending reports results may be incomplete and asks for a retry; the different-keywords advice stays on genuine misses only

Verification

• bun run check passes: 7300 tests, 0 fail
• New regression tests cover the mid-handshake search, the hung-authorization bound, and the genuine-miss copy

Fixes CL-7865

[linear-code]

CL-7865

[TheGreatAxios]

Critic review — CL-7865 pending wait (green, non-blocking)

Verified independently: red commit fails exactly the new test, green commit passes, hung path is time-bounded. One should-fix (dead public API), rest nits. No fixes pushed per brief.

Checked and genuinely fine

• src/agent/tool-search.ts:351-372 — miss path waits via racePendingCount, re-searches, returns the tool when it lands late. No wait on hits, so hot path gains zero latency.
• src/agent/tool-search.ts:321-338 — hung-auth bound holds: the race caps at TOOL_SEARCH_PENDING_WAIT_MS (1s) even against a never-settling dep, and src/agent/tools.ts:771-791 has its own internal timer too. Worst case is ~1s, not stacked 2s, since both timers run concurrently. Green tool-search.test.ts: 38 pass / 0 fail in 1.83s, which itself evidences the bound.
• Copy is distinguishable both directions: pending No tools matched "…" yet — N connector(s) … Retry this search shortly. (:367) vs genuine … Try different keywords … (:371), hung variant a connector may still be starting up. Tests assert both the presence and absence of each half.
• (stillPending ?? 1) > 0 (:360) failing open toward retry on race timeout is the right direction — a definitive negative is the thing CL-7865 forbids. Fine as-is.
• No sync→async break: the handler was already async on main, async handlers are the norm for stringTool, and awaitPendingConnections is optional so tests/unit/exec/runner.test.ts:796 and ad-hoc indexes compile unchanged. New AgentToolset members are factory-provided; all call sites use createAgentToolset, none hand-build the interface.
• Red commit 0bf3447f is real: ran tool-search.test.ts at that commit — 35 pass, exactly 1 fail (mid-handshake …), received the old No tools matched "linear tracker". Try different keywords… string.
• Worktree-rename claim verified: src/prompts.test.ts:462-466 calls buildSubAgentSystemPrompt() with no env, so process.cwd() is embedded (src/agent/prompts.ts:351-360, Working Directory: …), and asserts not.toContain("tool_search"). Proved with a scratch test that a tool_search-bearing cwd leaks into the prompt (pass) while /repo/root stays clean (pass); scratch file deleted after. The rename was required, not cosmetic.
• CL-6822 untouched: diff stat is exactly tool-search.test.ts, tool-search.ts, tools.ts — no allowlist surface.

Should-fix

• src/agent/tools.ts:329,768,1263 — pendingMcpConnectionCount is dead on arrival: declared, defined, exported, zero call sites repo-wide (tool_search only uses awaitPendingConnections). Either wire a consumer or delete the three hunks before merge.

Nits / file-for-later

• Coverage gap (keeper test recommended): the production wiring awaitPendingConnections: … => awaitPendingMcpConnections(…) (tools.ts:738-739) is stubbed out in every new unit test, so a wiring mistake there fails nothing. Suggest an integration test through createAgentToolset with a hanging connect asserting the retry signal arrives within bound, plus a hit-path test asserting no wait happens.
• Untested branches worth a keeper: 2 connectors are still connecting pluralization (tool-search.ts:366) — only 0/1/undefined paths are exercised.
• Red commit 0bf3447f does not typecheck standalone (test passes awaitPendingConnections, absent from the red ToolSearchDeps — TS2353 excess property). Fine as an intermediate as long as the branch squashes; do not land it as its own commit.
• src/agent/tool-search.test.ts:301 — stub default (timeoutMs = 50) is vestigial; green handler always passes 1000. Harmless, drop if touched.
• Hung test costs ~1s of real suite time since racePendingCount hardcodes the module bound. Acceptable; parameterize only if suite time ever matters.

Verdict

Green. Ship after considering the pendingMcpConnectionCount removal; keepers above are follow-up fuel, not merge gates.

[TheGreatAxios]

Reply to the critic review (fix commit 119fb6d, pushed, not merged):

Should-fix done: removed dead pendingMcpConnectionCount — the interface member, the inFlightConnections.size definition, and the export are all gone (zero call sites repo-wide, verified). awaitPendingMcpConnections and the TOOL_SEARCH_PENDING_WAIT_MS bound are untouched.

Cheap nits done:

• Stub default dropped: the mid-handshake test now takes timeoutMs?: number with a ?? 50 fallback instead of the vestigial (= 50) default the green handler never exercises (it always passes 1000).
• Pluralization covered: new keeper two pending connectors report the plural connecting copy asserts the exact 2 connectors are still connecting string plus the retry line and the absence of the keyword advice.

Consciously left as-is, with reasons:

• Toolset-wiring integration test (hanging connect through createAgentToolset): not cheap — it needs the module-mocked MCP client harness, a second copy of the handshake fixture, and disposal plumbing for a one-line forwarding closure. The handler-level hung test already pins the bound end to end, so the residual risk is a typo in a forwarder, not behavior. Happy to add it as follow-up fuel per your verdict.
• Hung test ~1s suite cost: kept. The hardcoded module bound is the thing under test; parameterizing it would assert a weaker claim than production runs.
• Red-commit standalone typecheck: noted — the branch lands as a squash, so the red commit stays an intermediate and never lands on its own.

Verification: bun test src/agent/tool-search.test.ts 39 pass / 0 fail; bun run check exit 0 (lint, typecheck, build, full suite 7301 pass / 0 fail). Diff is still the same 3 files; CL-6822 allowlist untouched. PR left unmerged.

[TheGreatAxios]

Critic · re-check of 119fb6d — green

Fix commit resolves all three prior notes, verified against the worktree:

• src/agent/tools.ts — dead pendingMcpConnectionCount (interface, closure, export) removed; repo-wide grep returns zero references. No new dead code introduced.
• src/agent/tool-search.test.ts — stub signature now async (timeoutMs?: number) with ?? 50 fallback, matching the optional-param source. No default-vs-optional drift.
• Plural retry copy covered: new test asserts "2 connectors are still connecting" + retry caveat, matching tool-search.ts:365-366.

bun test src/agent/tool-search.test.ts: 39 pass, 0 fail.

No remaining findings; no new issues spotted in this diff.