mirror of
https://github.com/mksglu/context-mode.git
synced 2026-10-02 04:14:38 +08:00
fix(pi): await MCP bridge bootstrap in before_agent_start (#490)
* fix(pi): await MCP bridge bootstrap in before_agent_start Pi subagents (`pi --mode json -p --no-session`) spawn a fresh process that loads the context-mode extension and immediately fires `before_agent_start` to dispatch the LLM call. The MCP bridge bootstrap (spawn server.bundle.mjs → initialize → tools/list → pi.registerTool × N) was fire-and-forget via `_mcpBridgeReady`, so the LLM call went out with an empty ctx_* tool registry and the routing block (~2.5K tokens) became dead weight — the LLM was told to call `ctx_execute` / `ctx_search` / etc. but Pi had not yet registered them. Awaiting `_mcpBridgeReady` at the top of the `before_agent_start` handler closes the race: by the time the handler resolves, the bridge has settled (success or failure — failures are still logged to stderr but never propagated, matching the original best-effort contract) and the registry contains the ctx_* tools. Reported in mksglu/context-mode#472 (comment 4412197109). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: raise hookTimeout to 30s to absorb Windows better-sqlite3 cleanup flake The default 10s vitest hookTimeout is exhausted on Windows runners by files whose afterAll loops over many better-sqlite3 handles — tests/session/session-pipeline.test.ts is the canonical example. Local runs finish in ~500ms but Windows fork-pool contention plus native addon cleanup can stretch past 10s, surfacing as FAIL tests/session/session-pipeline.test.ts Error: Hook timed out in 10000ms. Match the 30s testTimeout already in this config so the cleanup window matches the work window — same envelope better-sqlite3 needs for tests themselves. No change to local-dev wall time (only fires when a hook exceeds 10s, which is a flake-level event). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
228c34d595
commit
d66f767936
@@ -341,6 +341,19 @@ export default function piExtension(pi: any): void {
|
||||
|
||||
pi.on("before_agent_start", async (event: any) => {
|
||||
try {
|
||||
// Block first agent start until the MCP bridge bootstrap has
|
||||
// settled so the LLM call dispatched right after this handler
|
||||
// sees the ctx_* tools in Pi's registry. Each subagent starts
|
||||
// a fresh `pi --mode json -p --no-session` process whose only
|
||||
// window to register tools is the gap between piExtension(pi)
|
||||
// returning and the first before_agent_start firing — that gap
|
||||
// is too small for the spawn → initialize → tools/list →
|
||||
// pi.registerTool round-trip, so without this await the first
|
||||
// (and often only) prompt of a subagent goes out with an empty
|
||||
// ctx_* registry and the routing block becomes dead weight.
|
||||
// Resolves on bootstrap failure too — the bridge is best-effort.
|
||||
await _mcpBridgeReady;
|
||||
|
||||
if (!_sessionId) return;
|
||||
|
||||
const prompt = String(event?.prompt ?? "");
|
||||
|
||||
@@ -1064,5 +1064,64 @@ describe("Pi MCP bridge (#426)", () => {
|
||||
void sd; // silence unused
|
||||
await wireApi._trigger("session_shutdown");
|
||||
}, 30_000);
|
||||
|
||||
// Race regression — comment 4412197109 on PR #472.
|
||||
//
|
||||
// Each Pi subagent spawns a fresh `pi --mode json -p --no-session`
|
||||
// process that loads context-mode and then immediately fires
|
||||
// `before_agent_start` to dispatch the LLM call. The MCP bridge
|
||||
// bootstrap (spawn server.bundle.mjs → initialize → tools/list →
|
||||
// pi.registerTool × N) is fire-and-forget via `_mcpBridgeReady`, so
|
||||
// without an explicit await the LLM call goes out with an empty
|
||||
// ctx_* tool registry and the routing block (~2.5K tokens) becomes
|
||||
// dead weight — the LLM is told to call ctx_execute / ctx_search /
|
||||
// etc. but Pi has not yet registered them.
|
||||
//
|
||||
// This test pins the contract: by the time `before_agent_start`
|
||||
// resolves, the canonical ctx_* tools MUST have been registered
|
||||
// through pi.registerTool. The pre-trigger assertion confirms the
|
||||
// race window is open (otherwise the test would pass for the wrong
|
||||
// reason — bridge happened to win the race).
|
||||
it("before_agent_start awaits MCP bridge bootstrap so ctx_* are registered before LLM call", async () => {
|
||||
const wireApi = createMockPiApi();
|
||||
await registerPiExtension(wireApi, { projectDir: tempDir });
|
||||
|
||||
// Establish a session so before_agent_start does real work
|
||||
// (the handler early-returns when `!_sessionId`).
|
||||
await wireApi._trigger(
|
||||
"session_start",
|
||||
{},
|
||||
{ session_id: "race-test", project_dir: tempDir },
|
||||
);
|
||||
|
||||
// Sanity: bridge bootstrap is in flight, no tool registered yet.
|
||||
// If this ever fails, the bridge stopped racing and the test
|
||||
// loses meaning — adjust the bootstrap or remove the guard.
|
||||
const preCalls = (wireApi.registerTool as any).mock.calls.length;
|
||||
expect(preCalls).toBe(0);
|
||||
|
||||
// The race: trigger before_agent_start now. Pi will dispatch the
|
||||
// LLM call as soon as this resolves — so the handler MUST block
|
||||
// until the bridge has registered ctx_* tools.
|
||||
await wireApi._trigger("before_agent_start", {
|
||||
sessionID: "race-test",
|
||||
prompt: "anything",
|
||||
systemPrompt: "",
|
||||
});
|
||||
|
||||
const calls = (wireApi.registerTool as any).mock.calls as Array<[any]>;
|
||||
const registeredNames = calls.map(([t]) => t?.name).filter(Boolean);
|
||||
expect(registeredNames).toEqual(
|
||||
expect.arrayContaining([
|
||||
"ctx_execute",
|
||||
"ctx_search",
|
||||
"ctx_index",
|
||||
"ctx_batch_execute",
|
||||
"ctx_fetch_and_index",
|
||||
]),
|
||||
);
|
||||
|
||||
await wireApi._trigger("session_shutdown");
|
||||
}, 30_000);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,6 +6,12 @@ export default defineConfig({
|
||||
test: {
|
||||
include: ["tests/**/*.test.ts"],
|
||||
testTimeout: 30_000,
|
||||
// afterAll cleanup loops over many better-sqlite3 handles on Windows
|
||||
// and can exceed vitest's default 10s hookTimeout under fork contention
|
||||
// (e.g. tests/session/session-pipeline.test.ts cleans every DB it
|
||||
// created). Match testTimeout so the cleanup window matches the work
|
||||
// window — same envelope better-sqlite3 already needs for tests.
|
||||
hookTimeout: 30_000,
|
||||
// Native addons (better-sqlite3) can segfault in worker_threads during
|
||||
// process cleanup. Use forks on all platforms for stable isolation.
|
||||
pool: "forks",
|
||||
|
||||
Reference in New Issue
Block a user