Harden MCP bridge for Fortress: call timeout, abort signal, stderr level and startup tail, stable-channel workaround, screenshot path guidance

src/agent/fortress-mcp.test.ts

@@ -30,7 +30,7 @@ describe("createFortressMcpBridge", () => {
       expect(seen).toEqual([{
         command: "python",
         args: ["-m", "tilion.mcp"],
-        env: { TILION_PROXY: "http://p:1", TILION_MCP_HEADLESS: "1", TILION_MCP_WRITE_DIR: writeDir },
+        env: { FORTRESS_CHANNEL: "latest", TILION_PROXY: "http://p:1", TILION_MCP_HEADLESS: "1", TILION_MCP_WRITE_DIR: writeDir },
       }]);
       expect((await stat(writeDir)).isDirectory()).toBe(true);
       expect(bridge.tools.map((t) => t.name)).toEqual(["fortress_read_page", "fortress_fetch_protected_page"]);
@@ -41,8 +41,24 @@ describe("createFortressMcpBridge", () => {
 
   it("buildEnv maps headless=false to TILION_MCP_HEADLESS=0", () => {
     expect(_internals.buildEnv({ headless: false, writeDir: "/w", passthroughEnv: {} })).toEqual({
+      FORTRESS_CHANNEL: "latest",
       TILION_MCP_HEADLESS: "0",
       TILION_MCP_WRITE_DIR: "/w",
     });
   });
+
+  it("buildEnv lets an explicit FORTRESS_CHANNEL override the latest default", () => {
+    expect(_internals.buildEnv({ headless: true, writeDir: "/w", passthroughEnv: { FORTRESS_CHANNEL: "stable" } }).FORTRESS_CHANNEL).toBe("stable");
+  });
+
+  it("raises the per-call timeout above tilion-mcp's own 120 s tool cap", async () => {
+    let seen: unknown;
+    const c: McpClientLike = { ...client, callTool: async (_r, _s, o) => { seen = o; return { content: [] }; } };
+    const bridge = await createFortressMcpBridge({
+      command: ["tilion-mcp"], headless: true, writeDir: path.join(os.tmpdir(), "fortress-timeout-test"),
+      passthroughEnv: {}, logger: silentLogger, clientFactory: async () => c,
+    });
+    await bridge.tools[0].execute("c", {}, undefined, undefined, {} as never);
+    expect(seen).toEqual({ signal: undefined, timeout: 180_000 });
+  });
 });

src/agent/fortress-mcp.ts

@@ -8,7 +8,7 @@ export interface FortressMcpOptions {
   headless: boolean;
   // Absolute dir inside the workspace; screenshots, saved pages, downloads and profiles land here.
   writeDir: string;
-  // TILION_PROXY / TILION_REGION / CAPTCHA_API_KEY when set.
+  // TILION_PROXY / TILION_REGION / CAPTCHA_API_KEY / FORTRESS_CHANNEL when set.
   passthroughEnv: Record<string, string>;
   logger: Logger;
   // Test seam, forwarded to createMcpBridge.
@@ -26,6 +26,10 @@ export async function createFortressMcpBridge(opts: FortressMcpOptions): Promise
     args,
     env: buildEnv(opts),
     toolPrefix: "fortress_",
+    // Above tilion-mcp's own TILION_MCP_TOOL_TIMEOUT (120 s) so its structured timeout error wins.
+    callTimeoutMs: 180_000,
+    // tilion-mcp logs every request and CDP attach on stderr; keep that out of the info log.
+    stderrLevel: "debug",
     logger: opts.logger,
     clientFactory: opts.clientFactory,
   });
@@ -33,6 +37,11 @@ export async function createFortressMcpBridge(opts: FortressMcpOptions): Promise
 
 function buildEnv(opts: Pick<FortressMcpOptions, "headless" | "writeDir" | "passthroughEnv">): Record<string, string> {
   return {
+    // tilion-fortress 151.0.7908.0.post5 can't launch its "stable" channel: the v149 bundle
+    // extracts to tillion-fortress/tillion but it looks for tilion-fortress/tilion, so it
+    // re-downloads and fails. "latest" (v151) has the expected layout. FORTRESS_CHANNEL
+    // in the environment still overrides this.
+    FORTRESS_CHANNEL: "latest",
     ...opts.passthroughEnv,
     TILION_MCP_HEADLESS: opts.headless ? "1" : "0",
     TILION_MCP_WRITE_DIR: opts.writeDir,

src/agent/mcp-bridge.test.ts

@@ -1,11 +1,15 @@
 import { describe, expect, it, vi } from "vitest";
 import { createLogger } from "../log.js";
-import { createMcpBridge, type McpClientLike, type SpawnSpec } from "./mcp-bridge.js";
+import { createMcpBridge, type McpCallOptions, type McpClientLike, type SpawnSpec } from "./mcp-bridge.js";
 
 const silentLogger = createLogger("error", () => {});
 
 function fakeClient(overrides: Partial<McpClientLike> = {}) {
-  const state = { closed: 0, calls: [] as Array<{ name: string; arguments: Record<string, unknown> }> };
+  const state = {
+    closed: 0,
+    calls: [] as Array<{ name: string; arguments: Record<string, unknown> }>,
+    options: [] as Array<McpCallOptions | undefined>,
+  };
   const client: McpClientLike = {
     listTools: async () => ({
       tools: [
@@ -13,8 +17,9 @@ function fakeClient(overrides: Partial<McpClientLike> = {}) {
         { name: "snapshot", description: "Capture snapshot", inputSchema: { type: "object", properties: {} } },
       ],
     }),
-    callTool: async (req) => {
+    callTool: async (req, _schema, options) => {
       state.calls.push(req);
+      state.options.push(options);
       return { content: [{ type: "text", text: `called ${req.name}` }] };
     },
     close: async () => { state.closed += 1; },
@@ -42,6 +47,14 @@ describe("createMcpBridge", () => {
     expect(result.content).toEqual([{ type: "text", text: "called navigate" }]);
   });
 
+  it("forwards the abort signal and the call timeout to callTool", async () => {
+    const { client, state } = fakeClient();
+    const bridge = await createMcpBridge({ ...base, callTimeoutMs: 180_000, clientFactory: async () => client });
+    const ac = new AbortController();
+    await bridge.tools[0].execute("c1", {}, ac.signal, undefined, {} as never);
+    expect(state.options).toEqual([{ signal: ac.signal, timeout: 180_000 }]);
+  });
+
   it("passes command, args, and env to the client factory", async () => {
     const { client } = fakeClient();
     const seen: SpawnSpec[] = [];
@@ -69,4 +82,11 @@ describe("createMcpBridge", () => {
     await expect(bridge.dispose()).resolves.toBeUndefined();
     expect(warn).toHaveBeenCalledWith(expect.stringContaining("fortress mcp close failed"), expect.any(Error));
   });
+
+  it("includes the server's stderr tail in the error when it dies during the handshake", async () => {
+    const script = "console.error('Traceback: ModuleNotFoundError: mcp.server.fastmcp'); process.exit(1)";
+    await expect(
+      createMcpBridge({ ...base, command: process.execPath, args: ["-e", script], env: {} }),
+    ).rejects.toThrow(/ModuleNotFoundError: mcp\.server\.fastmcp/);
+  });
 });

src/agent/mcp-bridge.ts

@@ -2,7 +2,7 @@ import { Client } from "@modelcontextprotocol/sdk/client/index.js";
 import { StdioClientTransport } from "@modelcontextprotocol/sdk/client/stdio.js";
 import { defineTool } from "@earendil-works/pi-coding-agent";
 import { Type } from "typebox";
-import type { Logger } from "../log.js";
+import type { Logger, LogLevel } from "../log.js";
 import type { CustomTools } from "./pi-backend.js";
 
 export interface SpawnSpec {
@@ -16,6 +16,10 @@ export interface McpBridgeOptions extends SpawnSpec {
   label: string;
   // Prepended to every Pi tool name; the MCP server still sees the original name.
   toolPrefix?: string;
+  // Per-call timeout; the MCP SDK default is 60 s.
+  callTimeoutMs?: number;
+  // Level for the server's stderr lines (default "warn"). Chatty servers can use "debug".
+  stderrLevel?: LogLevel;
   logger: Logger;
   // Test seam: inject an already-connected MCP client. When omitted, the
   // bridge spawns `command` as a stdio subprocess.
@@ -31,10 +35,19 @@ export interface McpBridge {
 // implement it in a few lines.
 export interface McpClientLike {
   listTools: () => Promise<{ tools: McpToolDescriptor[] }>;
-  callTool: (req: { name: string; arguments: Record<string, unknown> }) => Promise<McpCallResult>;
+  callTool: (
+    req: { name: string; arguments: Record<string, unknown> },
+    resultSchema?: undefined,
+    options?: McpCallOptions,
+  ) => Promise<McpCallResult>;
   close: () => Promise<void>;
 }
 
+export interface McpCallOptions {
+  signal?: AbortSignal;
+  timeout?: number;
+}
+
 export interface McpToolDescriptor {
   name: string;
   description?: string;
@@ -49,10 +62,10 @@ export interface McpCallResult {
 
 export async function createMcpBridge(opts: McpBridgeOptions): Promise<McpBridge> {
   const spec: SpawnSpec = { command: opts.command, args: opts.args, env: opts.env };
-  const factory = opts.clientFactory ?? ((s) => defaultClientFactory(s, opts.label, opts.logger));
+  const factory = opts.clientFactory ?? ((s) => defaultClientFactory(s, opts.label, opts.stderrLevel ?? "warn", opts.logger));
   const client = await factory(spec);
   const { tools: mcpTools } = await client.listTools();
-  const tools: CustomTools = mcpTools.map((t) => wrapTool(t, client, opts.toolPrefix ?? ""));
+  const tools: CustomTools = mcpTools.map((t) => wrapTool(t, client, opts.toolPrefix ?? "", opts.callTimeoutMs));
   return {
     tools,
     dispose: async () => {
@@ -65,14 +78,18 @@ export async function createMcpBridge(opts: McpBridgeOptions): Promise<McpBridge
   };
 }
 
-function wrapTool(t: McpToolDescriptor, client: McpClientLike, prefix: string) {
+function wrapTool(t: McpToolDescriptor, client: McpClientLike, prefix: string, timeout?: number) {
   return defineTool({
     name: prefix + t.name,
     label: t.annotations?.title ?? t.name,
     description: t.description ?? "",
     parameters: Type.Unsafe<Record<string, unknown>>(t.inputSchema ?? { type: "object" }),
-    execute: async (_toolCallId, params) => {
-      const result = await client.callTool({ name: t.name, arguments: params as Record<string, unknown> });
+    execute: async (_toolCallId, params, signal) => {
+      const result = await client.callTool(
+        { name: t.name, arguments: params as Record<string, unknown> },
+        undefined,
+        { signal, timeout },
+      );
       // MCP callTool returns { content, isError? }; Pi tools return { content, details }.
       // MCP's content shape (type: "text" | "image" | "resource"; text/data/uri) is compatible
       // with what Pi passes through to the model.
@@ -81,16 +98,31 @@ function wrapTool(t: McpToolDescriptor, client: McpClientLike, prefix: string) {
   });
 }
 
-async function defaultClientFactory(spec: SpawnSpec, label: string, logger: Logger): Promise<McpClientLike> {
+const STDERR_TAIL_LINES = 20;
+
+async function defaultClientFactory(spec: SpawnSpec, label: string, stderrLevel: LogLevel, logger: Logger): Promise<McpClientLike> {
   const env = spec.env
     ? { ...(Object.fromEntries(Object.entries(process.env).filter(([, v]) => v !== undefined)) as Record<string, string>), ...spec.env }
     : undefined;
   const transport = new StdioClientTransport({ command: spec.command, args: spec.args, env, stderr: "pipe" });
-  const client = new Client({ name: "volcanic-secretary", version: "0.1.0" });
-  await client.connect(transport);
+  // Subscribe before connect: with stderr "pipe" the transport exposes a PassThrough up front,
+  // so a server that crashes during the handshake still gets its output captured.
+  const tail: string[] = [];
   transport.stderr?.on("data", (buf: Buffer | string) => {
-    const line = (typeof buf === "string" ? buf : buf.toString("utf8")).trimEnd();
-    if (line) logger.warn(`${label} mcp: ${line}`);
+    const text = (typeof buf === "string" ? buf : buf.toString("utf8")).trimEnd();
+    if (!text) return;
+    logger[stderrLevel](`${label} mcp: ${text}`);
+    tail.push(...text.split("\n"));
+    tail.splice(0, Math.max(0, tail.length - STDERR_TAIL_LINES));
   });
+  const client = new Client({ name: "volcanic-secretary", version: "0.1.0" });
+  try {
+    await client.connect(transport);
+  } catch (err) {
+    // The stderr pipe can lag the exit event; give it a tick to flush.
+    await new Promise((r) => setTimeout(r, 50));
+    const detail = tail.length ? `\n${tail.join("\n")}` : "";
+    throw new Error(`${label} mcp failed to start: ${(err as Error).message}${detail}`, { cause: err });
+  }
   return client as unknown as McpClientLike;
 }

src/agent/system-prompt.test.ts

@@ -25,6 +25,7 @@ describe("secretarySystemPrompt", () => {
     expect(on).toContain("## Web access (Fortress)");
     expect(on).toContain("fortress_fetch_protected_page");
     expect(on).toContain("fortress/");
+    expect(on).toContain("fortress_screenshot_page");
   });
 });
 

src/agent/system-prompt.ts

@@ -41,7 +41,8 @@ const FORTRESS_SECTION = `
 - For reading or extracting web content, prefer fortress_read_page, fortress_extract_page, and fortress_search_web. They return clean, bounded markdown.
 - When a site blocks you (403, Cloudflare/captcha page, empty JS shell), retry with fortress_fetch_protected_page.
 - Use the browser_* tools for step-by-step interactive browsing when you need to watch the page.
-- Fortress writes screenshots, saved pages, and downloads under fortress/ in the workspace. Send them with \`send\`.`;
+- Always pass a file name as \`path\` to fortress_screenshot_page. Without it a small screenshot comes back as base64 text and floods your context.
+- Fortress writes screenshots, saved pages, downloads, and saved profiles under fortress/ in the workspace, and returns their absolute path. Send them with \`send\`.`;
 
 /** "Current date and time: <formatted> (<timezone>)", for a before_agent_start handler to append each turn. */
 export function currentTimeLine(timezone: string, date: Date = new Date()): string {

src/config.ts

@@ -37,7 +37,7 @@ export interface FortressConfig {
   env: Record<string, string>;
 }
 
-const FORTRESS_PASSTHROUGH = ["TILION_PROXY", "TILION_REGION", "CAPTCHA_API_KEY"] as const;
+const FORTRESS_PASSTHROUGH = ["TILION_PROXY", "TILION_REGION", "CAPTCHA_API_KEY", "FORTRESS_CHANNEL"] as const;
 
 export class ConfigError extends Error {}