add agent and debug macros, improve activity timeouts, migrate claude and codex to cli (#224)

2026-02-04 22:29:45 +00:00
parent adc165d95f
commit 6fbff21fca
20 changed files with 5030 additions and 21553 deletions
@@ -1,12 +1,16 @@
 // changes to effort level configuration should be reflected in wiki/effort.md and docs/effort.mdx
 // changes to tool permissions should be reflected in wiki/granular-tools.md
 // changes to web search configuration should be reflected in wiki/websearch.md
-import { type Options, query, type SDKMessage } from "@anthropic-ai/claude-agent-sdk";
+import { mkdirSync, writeFileSync } from "node:fs";
+import { join } from "node:path";
+import type { SDKMessage } from "@anthropic-ai/claude-agent-sdk";
 import type { Effort } from "../external.ts";
 import { ghPullfrogMcpName } from "../external.ts";
 import packageJson from "../package.json" with { type: "json" };
+import { markActivity } from "../utils/activity.ts";
 import { log } from "../utils/cli.ts";
 import { installFromNpmTarball } from "../utils/install.ts";
+import { spawn } from "../utils/subprocess.ts";
 import { type AgentRunContext, agent } from "./shared.ts";

 // Model selection based on effort level
@@ -38,6 +42,26 @@ function buildDisallowedTools(ctx: AgentRunContext): string[] {
  return disallowed;
 }

+/**
+ * Write MCP config file for Claude CLI.
+ * Returns the path to the config file.
+ */
+function writeMcpConfig(ctx: AgentRunContext): string {
+  const configDir = join(ctx.tmpdir, ".claude");
+  mkdirSync(configDir, { recursive: true });
+  const configPath = join(configDir, "mcp.json");
+
+  const mcpConfig = {
+    mcpServers: {
+      [ghPullfrogMcpName]: { type: "http", url: ctx.mcpServerUrl },
+    },
+  };
+
+  writeFileSync(configPath, JSON.stringify(mcpConfig, null, 2), "utf-8");
+  log.info(`» MCP config written to ${configPath}`);
+  return configPath;
+}
+
 async function installClaude(): Promise<string> {
  const versionRange = packageJson.dependencies["@anthropic-ai/claude-agent-sdk"] || "latest";
  return await installFromNpmTarball({
@@ -64,32 +88,100 @@ export const claude = agent({
      log.info(`» disallowed tools: ${disallowedTools.join(", ")}`);
    }

-    const queryOptions: Options = {
-      permissionMode: "bypassPermissions" as const,
-      disallowedTools,
-      mcpServers: {
-        [ghPullfrogMcpName]: { type: "http", url: ctx.mcpServerUrl },
-      },
-      model,
-      pathToClaudeCodeExecutable: cliPath,
-      env: process.env,
-    };
+    // write MCP config file
+    const mcpConfigPath = writeMcpConfig(ctx);

-    const queryInstance = query({
-      prompt: ctx.instructions.full,
-      options: queryOptions,
+    // build CLI args
+    // claude -p "prompt" --dangerously-skip-permissions --mcp-config ./mcp.json --model opus --output-format stream-json --verbose
+    const args: string[] = [
+      cliPath,
+      "-p",
+      ctx.instructions.full,
+      "--dangerously-skip-permissions",
+      "--mcp-config",
+      mcpConfigPath,
+      "--model",
+      model,
+      "--output-format",
+      "stream-json",
+      "--verbose",
+    ];
+
+    // add disallowed tools if any
+    if (disallowedTools.length > 0) {
+      args.push("--disallowedTools");
+      args.push(...disallowedTools);
+    }
+
+    log.info("» running Claude CLI...");
+
+    let stdoutBuffer = "";
+    let finalOutput = "";
+
+    // Track bash tool IDs to identify when bash tool results come back
+    const bashToolIds = new Set<string>();
+
+    const result = await spawn({
+      cmd: "node",
+      args,
+      cwd: process.cwd(),
+      env: process.env,
+      stdio: ["ignore", "pipe", "pipe"],
+      onStdout: async (chunk) => {
+        finalOutput += chunk;
+
+        // buffer incomplete lines across chunks (NDJSON format)
+        stdoutBuffer += chunk;
+        const lines = stdoutBuffer.split("\n");
+
+        // keep the last element (may be incomplete) in the buffer
+        stdoutBuffer = lines.pop() || "";
+
+        for (const line of lines) {
+          const trimmed = line.trim();
+          if (!trimmed) continue;
+
+          try {
+            const message = JSON.parse(trimmed) as SDKMessage;
+            markActivity(); // reset activity timeout on every event
+            log.debug(JSON.stringify(message, null, 2));
+
+            const handler = messageHandlers[message.type];
+            if (handler) {
+              await handler(message as never, bashToolIds);
+            }
+          } catch {
+            // ignore parse errors - might be non-JSON output
+            log.debug(`[claude] non-JSON stdout line: ${trimmed.substring(0, 200)}`);
+          }
+        }
+      },
+      onStderr: (chunk) => {
+        const trimmed = chunk.trim();
+        if (trimmed) {
+          log.debug(`[claude stderr] ${trimmed}`);
+          log.warning(trimmed);
+          finalOutput += trimmed + "\n";
+        }
+      },
    });

-    // Stream the results
-    for await (const message of queryInstance) {
-      log.debug(JSON.stringify(message, null, 2));
-      const handler = messageHandlers[message.type];
-      await handler(message as never);
+    if (result.exitCode !== 0) {
+      const errorMessage =
+        result.stderr || finalOutput || result.stdout || "Unknown error - no output from Claude CLI";
+      log.error(`Claude CLI exited with code ${result.exitCode}: ${errorMessage}`);
+      return {
+        success: false,
+        error: errorMessage,
+        output: finalOutput || result.stdout || "",
+      };
    }

+    log.info("» Claude CLI completed successfully");
+
    return {
      success: true,
-      output: "",
+      output: finalOutput || result.stdout || "",
    };
  },
 });
@@ -97,18 +189,16 @@ export const claude = agent({
 type SDKMessageType = SDKMessage["type"];

 type SDKMessageHandler<type extends SDKMessageType = SDKMessageType> = (
-  data: Extract<SDKMessage, { type: type }>
+  data: Extract<SDKMessage, { type: type }>,
+  bashToolIds: Set<string>
 ) => void | Promise<void>;

 type SDKMessageHandlers = {
  [type in SDKMessageType]: SDKMessageHandler<type>;
 };

-// Track bash tool IDs to identify when bash tool results come back
-const bashToolIds = new Set<string>();
-
 const messageHandlers: SDKMessageHandlers = {
-  assistant: (data) => {
+  assistant: (data, bashToolIds) => {
    if (data.message?.content) {
      for (const content of data.message.content) {
        if (content.type === "text" && content.text?.trim()) {
@@ -127,24 +217,24 @@ const messageHandlers: SDKMessageHandlers = {
      }
    }
  },
-  user: (data) => {
+  user: (data, bashToolIds) => {
    if (data.message?.content) {
      for (const content of data.message.content) {
        if (content.type === "tool_result") {
          const toolUseId = (content as any).tool_use_id;
          const isBashTool = toolUseId && bashToolIds.has(toolUseId);

+          const outputContent =
+            typeof content.content === "string"
+              ? content.content
+              : Array.isArray(content.content)
+                ? content.content
+                    .map((c: any) => (typeof c === "string" ? c : c.text || JSON.stringify(c)))
+                    .join("\n")
+                : String(content.content);
+
          if (isBashTool) {
            // Log bash output in a collapsed group
-            const outputContent =
-              typeof content.content === "string"
-                ? content.content
-                : Array.isArray(content.content)
-                  ? content.content
-                      .map((c: any) => (typeof c === "string" ? c : c.text || JSON.stringify(c)))
-                      .join("\n")
-                  : String(content.content);
-
            log.startGroup(`bash output`);
            if (content.is_error) {
              log.warning(outputContent);
@@ -155,9 +245,10 @@ const messageHandlers: SDKMessageHandlers = {
            // Clean up the tracked ID
            bashToolIds.delete(toolUseId);
          } else if (content.is_error) {
-            const errorContent =
-              typeof content.content === "string" ? content.content : String(content.content);
-            log.warning(`Tool error: ${errorContent}`);
+            log.warning(`Tool error: ${outputContent}`);
+          } else {
+            // log successful non-bash tool result at debug level
+            log.debug(`tool output: ${outputContent}`);
          }
        }
      }
@@ -3,34 +3,24 @@
 // changes to web search configuration should be reflected in wiki/websearch.md
 import { mkdirSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
-import {
-  Codex,
-  type CodexOptions,
-  type ModelReasoningEffort,
-  type ThreadEvent,
-  type ThreadOptions,
-} from "@openai/codex-sdk";
+import type { ThreadEvent } from "@openai/codex-sdk";
 import type { Effort } from "../external.ts";
 import { ghPullfrogMcpName } from "../external.ts";
+import { markActivity } from "../utils/activity.ts";
 import { log } from "../utils/cli.ts";
 import { installFromNpmTarball } from "../utils/install.ts";
+import { spawn } from "../utils/subprocess.ts";
 import { type AgentRunContext, agent } from "./shared.ts";

-// model configuration based on effort level
-const codexModel: Record<Effort, string> = {
-  mini: "gpt-5.1-codex-mini",
-  // https://developers.openai.com/codex/models/
-  // gpt-5.2-codex is not yet available via api key (even through codex cli)
-  auto: "gpt-5.1-codex",
-  max: "gpt-5.1-codex-max",
-} as const;
-
-// reasoning effort configuration based on effort level
-// uses modelReasoningEffort parameter from ThreadOptions
-const codexReasoningEffort: Record<Effort, ModelReasoningEffort | undefined> = {
-  mini: "low",
-  auto: undefined, // use default
-  max: "high",
+// configuration based on effort level
+// https://developers.openai.com/codex/models/
+// gpt-5.2-codex is not yet available via api key (even through codex cli)
+type ModelReasoningEffort = "minimal" | "low" | "medium" | "high" | "xhigh";
+type CodexEffortConfig = { model: string; reasoningEffort?: ModelReasoningEffort };
+const codexEffortConfig: Record<Effort, CodexEffortConfig> = {
+  mini: { model: "gpt-5.1-codex-mini", reasoningEffort: "low" },
+  auto: { model: "gpt-5.1-codex" },
+  max: { model: "gpt-5.1-codex-max", reasoningEffort: "high" },
 };

 function writeCodexConfig(ctx: AgentRunContext): string {
@@ -81,96 +71,142 @@ export const codex = agent({
  name: "codex",
  install: installCodex,
  run: async (ctx) => {
-    // install CLI at start of run
-    const cliPath = await installCodex();
-
-    // create config directory for codex before setting HOME
-    const configDir = join(ctx.tmpdir, ".config", "codex");
-    mkdirSync(configDir, { recursive: true });
-
-    const codexDir = writeCodexConfig(ctx);
-
-    process.env.HOME = ctx.tmpdir;
-    process.env.CODEX_HOME = codexDir;
-
-    // get model and reasoning effort based on effort level
-    const model = codexModel[ctx.payload.effort];
-    const modelReasoningEffort = codexReasoningEffort[ctx.payload.effort];
-    log.info(`» using model: ${model}`);
-    if (modelReasoningEffort) {
-      log.info(`» using modelReasoningEffort: ${modelReasoningEffort}`);
-    }
-
-    // Configure Codex
+    // validate API key first
    const apiKey = process.env.OPENAI_API_KEY;
    if (!apiKey) {
      throw new Error("OPENAI_API_KEY is required for codex agent");
    }

-    const codexOptions: CodexOptions = {
-      apiKey,
-      codexPathOverride: cliPath,
-    };
+    // install CLI at start of run
+    const cliPath = await installCodex();

-    const codex = new Codex(codexOptions);
+    // write config file (creates ~/.codex/config.toml)
+    const codexDir = writeCodexConfig(ctx);

-    // build thread options based on tool permissions
-    const threadOptions: ThreadOptions = {
-      model,
-      approvalPolicy: "never" as const,
-      // write: "disabled" → read-only sandbox, otherwise full access for git ops
-      sandboxMode: ctx.payload.write === "disabled" ? "read-only" : "danger-full-access",
-      // web: controls network access
-      networkAccessEnabled: ctx.payload.web !== "disabled",
-      // search: controls web search
-      webSearchEnabled: ctx.payload.search !== "disabled",
-      ...(modelReasoningEffort && { modelReasoningEffort }),
-    };
+    // get model and reasoning effort based on effort level
+    const effortConfig = codexEffortConfig[ctx.payload.effort];
+    log.info(`» using model: ${effortConfig.model} (effort: ${ctx.payload.effort})`);
+    if (effortConfig.reasoningEffort) {
+      log.info(`» using modelReasoningEffort: ${effortConfig.reasoningEffort}`);
+    }
+
+    // determine sandbox mode based on write permission
+    // write: "disabled" → read-only sandbox, otherwise full access for git ops
+    const sandboxMode = ctx.payload.write === "disabled" ? "read-only" : "danger-full-access";
+
+    // determine network and search permissions
+    // web: "disabled" → no network access, otherwise enabled
+    const networkAccessEnabled = ctx.payload.web !== "disabled";
+    // search: "disabled" → no web search, otherwise enabled
+    const webSearchEnabled = ctx.payload.search !== "disabled";
+
+    const args: string[] = [
+      cliPath,
+      "exec",
+      ctx.instructions.full,
+      "--dangerously-bypass-approvals-and-sandbox",
+      "--model",
+      effortConfig.model,
+      "--sandbox",
+      sandboxMode,
+      "--json",
+      "--config",
+      `sandbox_workspace_write.network_access=${networkAccessEnabled}`,
+      "--config",
+      `features.web_search_request=${webSearchEnabled}`,
+    ];
+
+    if (effortConfig.reasoningEffort) {
+      args.push("--config", `model_reasoning_effort="${effortConfig.reasoningEffort}"`);
+    }

    log.info(
-      `» Codex options: sandboxMode=${threadOptions.sandboxMode}, networkAccessEnabled=${threadOptions.networkAccessEnabled}, webSearchEnabled=${threadOptions.webSearchEnabled}`
+      `» Codex options: sandboxMode=${sandboxMode}, networkAccess=${networkAccessEnabled}, webSearch=${webSearchEnabled}`
    );
+    log.info("» running Codex CLI...");

-    const thread = codex.startThread(threadOptions);
+    let stdoutBuffer = "";
+    let finalOutput = "";

-    try {
-      const streamedTurn = await thread.runStreamed(ctx.instructions.full);
+    // Track command execution IDs to identify when command results come back
+    const commandExecutionIds = new Set<string>();

-      let finalOutput = "";
-      for await (const event of streamedTurn.events) {
-        const handler = messageHandlers[event.type];
-        log.debug(JSON.stringify(event, null, 2));
-        if (handler) {
-          handler(event as never);
+    const env: NodeJS.ProcessEnv = {
+      ...process.env,
+      HOME: ctx.tmpdir,
+      CODEX_HOME: codexDir,
+      CODEX_API_KEY: apiKey,
+    };
+
+    const result = await spawn({
+      cmd: "node",
+      args,
+      cwd: process.cwd(),
+      env,
+      stdio: ["ignore", "pipe", "pipe"],
+      onStdout: async (chunk) => {
+        finalOutput += chunk;
+
+        // buffer incomplete lines across chunks (NDJSON format)
+        stdoutBuffer += chunk;
+        const lines = stdoutBuffer.split("\n");
+
+        // keep the last element (may be incomplete) in the buffer
+        stdoutBuffer = lines.pop() || "";
+
+        for (const line of lines) {
+          const trimmed = line.trim();
+          if (!trimmed) continue;
+
+          try {
+            const event = JSON.parse(trimmed) as ThreadEvent;
+            markActivity(); // reset activity timeout on every event
+            log.debug(JSON.stringify(event, null, 2));
+
+            const handler = messageHandlers[event.type as keyof typeof messageHandlers];
+            if (handler) {
+              await handler(event as never, commandExecutionIds);
+            }
+          } catch {
+            // ignore parse errors - might be non-JSON output
+            log.debug(`[codex] non-JSON stdout line: ${trimmed.substring(0, 200)}`);
+          }
        }
-
-        if (event.type === "item.completed" && event.item.type === "agent_message") {
-          finalOutput = event.item.text;
+      },
+      onStderr: (chunk) => {
+        const trimmed = chunk.trim();
+        if (trimmed) {
+          log.debug(`[codex stderr] ${trimmed}`);
+          log.warning(trimmed);
+          finalOutput += trimmed + "\n";
        }
-      }
+      },
+    });

-      return {
-        success: true,
-        output: finalOutput,
-      };
-    } catch (error) {
-      const errorMessage = error instanceof Error ? error.message : String(error);
-      log.error(`Codex execution failed: ${errorMessage}`);
+    if (result.exitCode !== 0) {
+      const errorMessage =
+        result.stderr || finalOutput || result.stdout || "Unknown error - no output from Codex CLI";
+      log.error(`Codex CLI exited with code ${result.exitCode}: ${errorMessage}`);
      return {
        success: false,
        error: errorMessage,
-        output: "",
+        output: finalOutput || result.stdout || "",
      };
    }
+
+    log.info("» Codex CLI completed successfully");
+
+    return {
+      success: true,
+      output: finalOutput || result.stdout || "",
+    };
  },
 });

-// Track command execution IDs to identify when command results come back
-const commandExecutionIds = new Set<string>();
-
 type ThreadEventHandler<type extends ThreadEvent["type"]> = (
-  event: Extract<ThreadEvent, { type: type }>
-) => void;
+  event: Extract<ThreadEvent, { type: type }>,
+  commandExecutionIds: Set<string>
+) => void | Promise<void>;

 const messageHandlers: {
  [type in ThreadEvent["type"]]: ThreadEventHandler<type>;
@@ -198,7 +234,7 @@ const messageHandlers: {
  "turn.failed": (event) => {
    log.error(`Turn failed: ${event.error.message}`);
  },
-  "item.started": (event) => {
+  "item.started": (event, commandExecutionIds) => {
    const item = event.item;
    if (item.type === "command_execution") {
      commandExecutionIds.add(item.id);
@@ -227,7 +263,7 @@ const messageHandlers: {
      }
    }
  },
-  "item.completed": (event) => {
+  "item.completed": (event, commandExecutionIds) => {
    const item = event.item;
    if (item.type === "agent_message") {
      log.box(item.text.trim(), { title: "Codex" });
@@ -7,6 +7,7 @@ import { homedir } from "node:os";
 import { join } from "node:path";
 import type { Effort } from "../external.ts";
 import { ghPullfrogMcpName } from "../external.ts";
+import { markActivity } from "../utils/activity.ts";
 import { log } from "../utils/cli.ts";
 import { installFromCurl } from "../utils/install.ts";
 import { type AgentRunContext, agent } from "./shared.ts";
@@ -271,6 +272,7 @@ export const cursor = agent({

          try {
            const event = JSON.parse(text) as CursorEvent;
+            markActivity(); // reset activity timeout on every event
            log.debug(JSON.stringify(event, null, 2));

            // skip empty thinking deltas
@@ -6,6 +6,7 @@ import { homedir } from "node:os";
 import { join } from "node:path";
 import type { Effort } from "../external.ts";
 import { ghPullfrogMcpName } from "../external.ts";
+import { markActivity } from "../utils/activity.ts";
 import { log } from "../utils/cli.ts";
 import { installFromGithub } from "../utils/install.ts";
 import { spawn } from "../utils/subprocess.ts";
@@ -226,6 +227,7 @@ export const gemini = agent({

            try {
              const event = JSON.parse(trimmed) as GeminiEvent;
+              markActivity(); // reset activity timeout on every event
              const handler = messageHandlers[event.type as keyof typeof messageHandlers];
              if (handler) {
                await handler(event as never);
@@ -4,6 +4,7 @@
 import { mkdirSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { ghPullfrogMcpName } from "../external.ts";
+import { getIdleMs, markActivity } from "../utils/activity.ts";
 import { log } from "../utils/cli.ts";
 import { installFromNpmTarball } from "../utils/install.ts";
 import { spawn } from "../utils/subprocess.ts";
@@ -59,7 +60,6 @@ export const opencode = agent({
    log.debug(`» XDG_CONFIG_HOME: ${env.XDG_CONFIG_HOME}`);

    const startTime = Date.now();
-    let lastActivityTime = startTime;
    let eventCount = 0;

    let output = "";
@@ -95,7 +95,7 @@ export const opencode = agent({
            // debug log all events to diagnose ordering and missing MCP/bash tool calls
            log.debug(JSON.stringify(event, null, 2));

-            const timeSinceLastActivity = Date.now() - lastActivityTime;
+            const timeSinceLastActivity = getIdleMs();
            if (timeSinceLastActivity > 10000) {
              const activeToolCalls = toolCallTimings.size;
              const toolCallInfo =
@@ -106,7 +106,7 @@ export const opencode = agent({
                `» no activity for ${(timeSinceLastActivity / 1000).toFixed(1)}s${toolCallInfo} (${eventCount} events processed so far)`
              );
            }
-            lastActivityTime = Date.now();
+            markActivity(); // reset activity timeout on every event
            const handler = messageHandlers[event.type as keyof typeof messageHandlers];
            if (handler) {
              await handler(event as never);
@@ -261,6 +261,8 @@ export interface WriteablePayload {
  cwd?: string | undefined;
  /** pre-created progress comment ID for updating status */
  progressCommentId?: string | undefined;
+  /** whether debug mode is enabled (LOG_LEVEL=debug) */
+  debug?: boolean | undefined;
 }

 // immutable payload type for agent execution
@@ -0,0 +1,16 @@
+// Enforce type-only imports from SDK packages
+// These SDK packages should only be used for type imports (stream output parsing)
+// Runtime SDK usage should be replaced with CLI invocations
+// Note: This rule only catches single-specifier imports; for multi-specifier imports,
+// the noUnusedImports rule will flag unused runtime imports
+
+or {
+  `import { $specifiers } from "@anthropic-ai/claude-agent-sdk"`,
+  `import { $specifiers } from "@openai/codex-sdk"`,
+  `import { $specifiers } from "@opencode-ai/sdk"`
+} as $import where {
+  register_diagnostic(
+    span = $import,
+    message = "SDK packages must use `import type` only. Use CLI invocation instead of runtime SDK usage."
+  )
+}
@@ -64,6 +64,13 @@ export async function main(): Promise<MainResult> {
    // resolve payload after runContextData so permissions can use DB settings
    // precedence: action inputs > json payload > repoSettings > fallbacks
    const payload = resolvePayload(resolvedPromptInput, runContext.repoSettings);
+
+    // enable debug logging if #debug macro was used
+    if (payload.debug) {
+      process.env.LOG_LEVEL = "debug";
+      log.info("» debug mode enabled via #debug macro");
+    }
+
    if (payload.cwd && process.cwd() !== payload.cwd) {
      process.chdir(payload.cwd);
    }
@@ -116,17 +116,19 @@ export function computeModes(): Mode[] {
   - **Consider lifecycle**: Initialization, cleanup, error recovery. Are resources acquired before use? Released after? What happens on cancellation?
   - Do NOT stop at "this looks reasonable." Dig until you either find a problem or have concrete evidence there isn't one.

-4. **DRAFT** - For each issue found, create an inline comment. Use the NEW line number from the diff (second column: \`| OLD | NEW | TYPE | CODE\`).
+4. **DRAFT LINE-BY-LINE COMMENTS** - For each issue found, draft an inline comment on the specific line. Use the NEW line number from the diff (second column: \`| OLD | NEW | TYPE | CODE\`). If no issues found, skip to step 6.

-5. **FILTER** - Remove noise, keep substance:
-   - Remove style-only comments (formatting, naming conventions) unless they cause real confusion
-   - Remove compliments that aren't actionable
-   - Keep: bugs, logic errors, missing error handling, security issues, race conditions, resource leaks, incorrect assumptions
+5. **FILTER LINE-BY-LINE COMMENTS** - Each inline comment must be actionable. Remove anything that doesn't require action:
+   - **Not actionable → no comment**: Do NOT create inline comments for compliments (e.g., "this looks clean", "nice refactor") or general observations. These waste reviewer attention.
+   - **Actionable by agent → keep**: Bugs, logic errors, missing error handling, security issues, race conditions, resource leaks, incorrect assumptions.
+   - **Requires human decision → keep**: If something needs human judgment (architectural choice, product decision, tradeoff evaluation), create a comment clearly stating what decision is needed and why.
+   - Remove style-only comments (formatting, naming conventions) unless they cause real confusion.

-6. **SUBMIT** — Use ${ghPullfrogMcpName}/create_pull_request_review:
-   - \`comments\`: Inline feedback on specific diff lines
-   - \`body\`: 1-3 sentence summary with urgency level and any concerns about code outside the diff
-   - If no issues found, submit with empty comments and a brief approving body
+6. **WRITE SUMMARY** - Draft a 1-3 sentence summary for the review body. Include urgency level and any concerns about code outside the diff.
+
+7. **SUBMIT** — Use ${ghPullfrogMcpName}/create_pull_request_review:
+   - \`body\`: The summary from step 6
+   - \`comments\`: The filtered inline comments from step 5

 ${permalinkTip}
 `,
@@ -53,8 +53,9 @@ function validator(result: AgentResult): ValidationCheck[] {
 }

 export const test: TestRunnerOptions = {
-  name: "nobashcreative tests",
+  name: "nobashcreative",
  fixture,
  validator,
  agentEnv,
+  tags: ["adhoc"],
 };
@@ -25,9 +25,9 @@ function validator(result: AgentResult): ValidationCheck[] {
 }

 export const test: TestRunnerOptions = {
-  name: "timeout tests",
+  name: "timeout",
  fixture,
  validator,
  expectFailure: true,
-  agnostic: true,
+  tags: ["agnostic"],
 };
@@ -0,0 +1,86 @@
+import type { AgentResult, TestRunnerOptions, ValidationCheck } from "../utils.ts";
+import { defineFixture, generateAgentUuids, getStructuredOutput } from "../utils.ts";
+
+/**
+ * MCP merge tests - validate MCP server configuration with repo-level MCP configs present.
+ *
+ * uses GITHUB_REPOSITORY=pullfrog/test-repo-mcp which has robinMCP server.
+ *
+ * two variants:
+ * - mcpmerge-full: validates repo-level MCP servers merge correctly with gh_pullfrog
+ *   (claude, cursor, gemini, opencode - agents that auto-discover repo MCPs)
+ * - mcpmerge-pullfrog-only: validates gh_pullfrog remains available when repo has MCP config
+ *   (codex - doesn't auto-discover repo MCPs)
+ */
+
+// shared env for both tests
+const sharedEnv = {
+  GITHUB_REPOSITORY: "pullfrog/test-repo-mcp",
+};
+
+// --- mcpmerge-full: test repo-level MCP discovery ---
+
+const fullTestUuids = generateAgentUuids(["PULLFROG_MCP_TEST"]);
+
+const fullFixture = defineFixture(
+  {
+    prompt: `Call the get_test_value tool from the robinMCP server, then call set_output with the exact value returned.`,
+    effort: "mini",
+  },
+  { localOnly: true }
+);
+
+function fullValidator(result: AgentResult): ValidationCheck[] {
+  const output = getStructuredOutput(result);
+  const setOutputCalled = output !== null;
+  const expectedUuid = fullTestUuids.getUuid(result.agent, "PULLFROG_MCP_TEST");
+  const correctValue = setOutputCalled && output === expectedUuid;
+
+  return [
+    { name: "set_output", passed: setOutputCalled },
+    { name: "repo_mcp", passed: correctValue },
+  ];
+}
+
+// --- mcpmerge-pullfrog-only: test gh_pullfrog availability only ---
+
+const baseFixture = defineFixture(
+  {
+    prompt: `Call set_output with "PULLFROG_MCP_WORKS".`,
+    effort: "mini",
+  },
+  { localOnly: true }
+);
+
+function baseValidator(result: AgentResult): ValidationCheck[] {
+  const output = getStructuredOutput(result);
+  const setOutputCalled = output !== null;
+  const correctValue = setOutputCalled && output === "PULLFROG_MCP_WORKS";
+
+  return [
+    { name: "set_output", passed: setOutputCalled },
+    { name: "correct_value", passed: correctValue },
+  ];
+}
+
+// --- exports ---
+
+export const tests: Record<string, TestRunnerOptions> = {
+  "mcpmerge-full": {
+    name: "mcpmerge-full",
+    fixture: fullFixture,
+    validator: fullValidator,
+    tags: ["mcpmerge"],
+    agents: ["claude", "cursor", "gemini", "opencode"],
+    env: sharedEnv,
+    agentEnv: fullTestUuids.agentEnv,
+  },
+  "mcpmerge-pullfrog-only": {
+    name: "mcpmerge-pullfrog-only",
+    fixture: baseFixture,
+    validator: baseValidator,
+    tags: ["mcpmerge"],
+    agents: ["codex"],
+    env: sharedEnv,
+  },
+};
@@ -39,7 +39,7 @@ function validator(result: AgentResult): ValidationCheck[] {
 }

 export const test: TestRunnerOptions = {
-  name: "nobash tests",
+  name: "nobash",
  fixture,
  validator,
  agentEnv,
@@ -49,7 +49,7 @@ function validator(result: AgentResult): ValidationCheck[] {
 }

 export const test: TestRunnerOptions = {
-  name: "restricted tests",
+  name: "restricted",
  fixture,
  validator,
  agentEnv,
@@ -26,7 +26,7 @@ function validator(result: AgentResult): ValidationCheck[] {
 }

 export const test: TestRunnerOptions = {
-  name: "smoke tests",
+  name: "smoke",
  fixture,
  validator,
 };
@@ -25,24 +25,20 @@ import {
 *
 * usage: node test/run.ts [filters...]
 *
- * filters can be test names, agent names, or special keywords:
- *   node test/run.ts               # run all tests (excludes adhoc tests)
- *   node test/run.ts smoke         # run smoke test for all agents
- *   node test/run.ts claude        # run all tests for claude (excludes adhoc)
- *   node test/run.ts smoke claude  # run smoke test for claude only
- *   node test/run.ts agnostic      # run all agnostic tests (with claude)
- *   node test/run.ts timeout codex # run timeout test with codex
- *   node test/run.ts adhoc         # run all adhoc tests (exploratory, not CI)
- *   node test/run.ts nobashcreative # run specific adhoc test by name
+ * filters can be test names, tags, or agent names:
+ *   node test/run.ts               # run all tests (excludes adhoc-tagged tests)
+ *   node test/run.ts smoke         # run tests named "smoke" or tagged "smoke"
+ *   node test/run.ts claude        # run all tests for claude only
+ *   node test/run.ts mcpmerge      # run all tests tagged "mcpmerge"
+ *   node test/run.ts agnostic      # run all agnostic-tagged tests (with claude)
+ *   node test/run.ts adhoc         # run all adhoc-tagged tests
+ *   node test/run.ts smoke claude  # run smoke tests for claude only
+ *
+ * special tags:
+ *   - "agnostic": runs with claude only, excluded when filtering by agent
+ *   - "adhoc": excluded from default runs, must be explicitly requested
 *
 * by default, runs in a Docker container for isolation.
- * automatically detects if already inside Docker and skips spawning another container.
- *
- * tests mark themselves as agnostic via the `agnostic: true` option.
- * agnostic tests only need to run once with any agent (default: claude).
- *
- * adhoc tests are in the `adhoc/` directory and are excluded from default runs.
- * they are for exploration and manual testing, not CI.
 */

 const __dirname = dirname(fileURLToPath(import.meta.url));
@@ -91,86 +87,89 @@ type CancelState = {
  signal: NodeJS.Signals | null;
 };

-async function loadTest(testName: string): Promise<TestInfo | null> {
-  // check crossagent directory first
-  const crossagentPath = join(__dirname, "crossagent", `${testName}.ts`);
-  if (existsSync(crossagentPath)) {
-    const module = await import(crossagentPath);
-    return { name: testName, config: module.test };
+type TestModule = {
+  test?: TestRunnerOptions;
+  tests?: Record<string, TestRunnerOptions>;
+};
+
+// load all tests from all directories
+async function loadAllTests(): Promise<TestInfo[]> {
+  const testInfos: TestInfo[] = [];
+  const dirs = ["crossagent", "agnostic", "adhoc"];
+
+  for (const dir of dirs) {
+    const dirPath = join(__dirname, dir);
+    if (!existsSync(dirPath)) continue;
+
+    const files = readdirSync(dirPath).filter((f) => f.endsWith(".ts"));
+    for (const file of files) {
+      const filePath = join(dirPath, file);
+      const module = (await import(filePath)) as TestModule;
+
+      if (module.test) {
+        testInfos.push({ name: module.test.name, config: module.test });
+      } else if (module.tests) {
+        const entries = Object.entries(module.tests);
+        for (const entry of entries) {
+          testInfos.push({ name: entry[0], config: entry[1] });
+        }
+      }
+    }
  }

-  // check agnostic directory
-  const agnosticPath = join(__dirname, "agnostic", `${testName}.ts`);
-  if (existsSync(agnosticPath)) {
-    const module = await import(agnosticPath);
-    return { name: testName, config: module.test };
-  }
-
-  // check adhoc directory
-  const adhocPath = join(__dirname, "adhoc", `${testName}.ts`);
-  if (existsSync(adhocPath)) {
-    const module = await import(adhocPath);
-    return { name: testName, config: module.test };
-  }
-
-  return null;
+  return testInfos;
 }

-function listTestsFromDir(dir: string): string[] {
-  if (!existsSync(dir)) return [];
-  return readdirSync(dir)
-    .filter((f) => f.endsWith(".ts"))
-    .map((f) => f.replace(".ts", ""));
-}
-
-function listAdhocTests(): string[] {
-  const adhocDir = join(__dirname, "adhoc");
-  return listTestsFromDir(adhocDir);
-}
-
-function listCoreTests(): string[] {
-  const crossagentDir = join(__dirname, "crossagent");
-  const agnosticDir = join(__dirname, "agnostic");
-  return [...listTestsFromDir(crossagentDir), ...listTestsFromDir(agnosticDir)];
-}
-
-function listAvailableTests(): string[] {
-  return [...listCoreTests(), ...listAdhocTests()];
+// check if test has a specific tag
+function hasTag(test: TestInfo, tag: string): boolean {
+  return test.config.tags?.includes(tag) ?? false;
 }

 type ParsedArgs = {
-  tests: string[];
-  agents: string[];
-  agnosticOnly: boolean;
-  adhocOnly: boolean;
+  filters: string[]; // test names or tags
+  agentFilters: string[];
 };

-function parseArgs(args: string[]): ParsedArgs {
-  const availableTests = listAvailableTests();
-  const tests: string[] = [];
-  const agentsFound: string[] = [];
-  let agnosticOnly = false;
-  let adhocOnly = false;
+function parseArgs(args: string[], allTests: TestInfo[]): ParsedArgs {
+  const testNames = new Set(allTests.map((t) => t.name));
+  const allTags = new Set(allTests.flatMap((t) => t.config.tags ?? []));
+
+  const filters: string[] = [];
+  const agentFilters: string[] = [];

  for (const arg of args) {
-    if (arg === "agnostic") {
-      agnosticOnly = true;
-    } else if (arg === "adhoc") {
-      adhocOnly = true;
-    } else if (availableTests.includes(arg)) {
-      tests.push(arg);
-    } else if (agents.includes(arg as (typeof agents)[number])) {
-      agentsFound.push(arg);
+    if (agents.includes(arg as (typeof agents)[number])) {
+      agentFilters.push(arg);
+    } else if (testNames.has(arg) || allTags.has(arg)) {
+      filters.push(arg);
    } else {
      console.error(`unknown argument: ${arg}`);
-      console.error(`available tests: ${availableTests.join(", ")}`);
+      console.error(`available tests: ${[...testNames].join(", ")}`);
+      console.error(`available tags: ${[...allTags].join(", ")}`);
      console.error(`available agents: ${agents.join(", ")}`);
-      console.error(`special keywords: agnostic, adhoc`);
      process.exit(1);
    }
  }

-  return { tests, agents: agentsFound, agnosticOnly, adhocOnly };
+  return { filters, agentFilters };
+}
+
+// filter tests based on filters (names or tags)
+function filterTests(allTests: TestInfo[], filters: string[]): TestInfo[] {
+  if (filters.length === 0) {
+    // default: exclude adhoc tests
+    return allTests.filter((t) => !hasTag(t, "adhoc"));
+  }
+
+  // match tests by name or tag
+  return allTests.filter((t) => {
+    for (const filter of filters) {
+      if (t.name === filter || hasTag(t, filter)) {
+        return true;
+      }
+    }
+    return false;
+  });
 }

 type RunContext = {
@@ -202,16 +201,16 @@ function buildCanceledValidation(ctx: CanceledValidationContext): ValidationResu
 }

 async function runTestForAgent(ctx: RunContext): Promise<ValidationResult> {
-  const config = ctx.testInfo.config;
+  const testConfig = ctx.testInfo.config;
  const env: Record<string, string> = {};
-  if (config.env) {
-    const entries = Object.entries(config.env);
+  if (testConfig.env) {
+    const entries = Object.entries(testConfig.env);
    for (const entry of entries) {
      env[entry[0]] = entry[1];
    }
  }
-  if (config.agentEnv) {
-    const agentEnv = config.agentEnv.get(ctx.agent);
+  if (testConfig.agentEnv) {
+    const agentEnv = testConfig.agentEnv.get(ctx.agent);
    if (agentEnv) {
      const entries = Object.entries(agentEnv);
      for (const entry of entries) {
@@ -227,14 +226,14 @@ async function runTestForAgent(ctx: RunContext): Promise<ValidationResult> {
  const result = await runAgentStreaming({
    test: ctx.testInfo.name,
    agent: ctx.agent,
-    fixture: config.fixture,
+    fixture: testConfig.fixture,
    env,
    isCanceled: () => ctx.cancelState.canceled,
  });

-  const validation = validateResult(result, config.validator, {
+  const validation = validateResult(result, testConfig.validator, {
    test: ctx.testInfo.name,
-    expectFailure: config.expectFailure,
+    expectFailure: testConfig.expectFailure,
  });
  ctx.results.set(getRunKey(ctx.testInfo.name, ctx.agent), validation);
  return validation;
@@ -245,8 +244,7 @@ async function main(): Promise<void> {

  // run in Docker unless already inside
  if (!isInsideDocker) {
-    // acquire token for docker if needed (before spawning containers)
-    // this ensures GITHUB_TOKEN is available when setupTestRepo() runs inside docker
+    // acquire token for docker if needed
    if (!process.env.GITHUB_TOKEN && !process.env.GH_TOKEN) {
      if (process.env.GITHUB_APP_ID && process.env.GITHUB_PRIVATE_KEY) {
        console.log("» acquiring github installation token...");
@@ -257,35 +255,12 @@ async function main(): Promise<void> {
    runTestsInDocker(args);
  }

-  const parsed = parseArgs(args);
-  const adhocTests = listAdhocTests();
+  // load all tests
+  const allTests = await loadAllTests();
+  const parsed = parseArgs(args, allTests);

-  // determine which tests to load
-  let testsToLoad: string[];
-  if (parsed.tests.length > 0) {
-    testsToLoad = parsed.tests;
-  } else if (parsed.adhocOnly) {
-    testsToLoad = adhocTests;
-  } else {
-    // by default, exclude adhoc tests
-    testsToLoad = listCoreTests();
-  }
-
-  // load all test configs
-  const testInfos: TestInfo[] = [];
-  for (const testName of testsToLoad) {
-    const info = await loadTest(testName);
-    if (!info) {
-      console.error(`failed to load test: ${testName}`);
-      process.exit(1);
-    }
-    testInfos.push(info);
-  }
-
-  // filter to agnostic-only if requested
-  const filteredTests = parsed.agnosticOnly
-    ? testInfos.filter((t) => t.config.agnostic)
-    : testInfos;
+  // filter tests
+  const filteredTests = filterTests(allTests, parsed.filters);

  if (filteredTests.length === 0) {
    console.error("no tests to run");
@@ -293,41 +268,36 @@ async function main(): Promise<void> {
  }

  // determine which agents to run
-  const agentsToRun = parsed.agents.length > 0 ? parsed.agents : [...agents];
+  const agentsToRun = parsed.agentFilters.length > 0 ? parsed.agentFilters : [...agents];

  // build list of test runs
  type TestRun = { testInfo: TestInfo; agent: string };
  const runs: TestRun[] = [];

-  // determine behavior for agnostic tests:
-  // - if specific tests were requested (e.g., `timeout codex`), run with specified agent
-  // - if agnosticOnly flag is set, run with specified agent (or claude)
-  // - if only agents specified (e.g., `codex`), skip agnostic tests (they run separately)
-  // - if no filters at all, include agnostic tests with claude
-  const specificTestsRequested = parsed.tests.length > 0;
-  const onlyAgentsSpecified =
-    parsed.agents.length > 0 && !specificTestsRequested && !parsed.agnosticOnly;
-  const noFiltersAtAll =
-    parsed.tests.length === 0 && parsed.agents.length === 0 && !parsed.agnosticOnly;
-
  for (const testInfo of filteredTests) {
-    if (testInfo.config.agnostic) {
-      // skip agnostic tests when only agents are specified (e.g., `pnpm runtest codex`)
-      if (onlyAgentsSpecified) continue;
+    const isAgnostic = hasTag(testInfo, "agnostic");

-      // agnostic: run once with appropriate agent
-      // - if specific tests requested or agnosticOnly: use specified agent or first in list
-      // - otherwise (no filters): use default agent (claude)
-      const agent = noFiltersAtAll ? agents[0] : agentsToRun[0];
-      runs.push({ testInfo, agent });
+    if (isAgnostic) {
+      // agnostic tests: skip if only filtering by agent, otherwise run with claude
+      if (parsed.filters.length === 0 && parsed.agentFilters.length > 0) {
+        continue;
+      }
+      runs.push({ testInfo, agent: "claude" });
    } else {
-      // non-agnostic: run for all specified agents
-      for (const agent of agentsToRun) {
+      // determine which agents to run for this test
+      const testAgents = testInfo.config.agents ?? agents;
+      const effectiveAgents = agentsToRun.filter((a) => testAgents.includes(a));
+      for (const agent of effectiveAgents) {
        runs.push({ testInfo, agent });
      }
    }
  }

+  if (runs.length === 0) {
+    console.error("no test runs after filtering");
+    process.exit(1);
+  }
+
  // describe what we're running
  const runTestNames = [...new Set(runs.map((r) => r.testInfo.name))];
  const runAgentNames = [...new Set(runs.map((r) => r.agent))];
@@ -377,9 +347,8 @@ async function main(): Promise<void> {
  setSignalHandler(handleCancel);
  installSignalHandlers();

-  // run tests with limited concurrency to avoid overwhelming agent APIs
-  // group by agent to ensure each agent only runs one test at a time
-  const maxConcurrency = 5; // one per agent
+  // run tests with limited concurrency
+  const maxConcurrency = 5;
  const validations = await runWithConcurrencyLimit(
    runs,
    maxConcurrency,
@@ -412,7 +381,6 @@ async function runWithConcurrencyLimit<T, R>(
        results.push(result);
      },
      (err: unknown) => {
-        // fn must never reject; log and rethrow to surface bugs
        console.error("runWithConcurrencyLimit: fn rejected unexpectedly", err);
        throw err;
      }
@@ -1,6 +1,6 @@
 import { spawn } from "node:child_process";
 import { randomUUID } from "node:crypto";
-import { mkdirSync } from "node:fs";
+import { mkdirSync, writeFileSync } from "node:fs";
 import { dirname, join } from "node:path";
 import { fileURLToPath } from "node:url";
 import { agentsManifest } from "../external.ts";
@@ -169,6 +169,18 @@ export async function runAgentStreaming(options: RunStreamingOptions): Promise<A
    const testHome = `/tmp/home-${mcpPort}-${Date.now()}`;
    mkdirSync(testHome, { recursive: true });

+    // write env vars to files for MCP servers that don't inherit parent env vars
+    // (e.g., Cursor CLI doesn't pass env vars to repo-level MCP servers)
+    // uses testHome path which is unique per test and set as HOME
+    if (options.env) {
+      const envDir = join(testHome, ".pullfrog-env");
+      mkdirSync(envDir, { recursive: true });
+      const entries = Object.entries(options.env);
+      for (const entry of entries) {
+        writeFileSync(join(envDir, entry[0]), entry[1]);
+      }
+    }
+
    const child = spawn("node", ["play.ts", "--raw", JSON.stringify(fixture)], {
      cwd: actionDir,
      env: {
@@ -267,12 +279,16 @@ export interface TestRunnerOptions {
  env?: Record<string, string>;
  // per-agent env vars (for unique markers)
  agentEnv?: Map<string, Record<string, string>>;
+  // specific agents to run this test on (defaults to all agents)
+  agents?: string[];
  // if true, test passes when agent fails AND validation checks pass
  // (used for tests like timeout that expect the agent run to fail)
  expectFailure?: boolean;
-  // if true, test is agent-agnostic and only needs to run once with any agent
-  // agnostic tests run with claude by default, but can be run with specific agent via CLI
-  agnostic?: boolean;
+  // tags for grouping tests (e.g., ["mcpmerge"], ["agnostic"])
+  // special tags:
+  //   - "agnostic": runs with claude only, excluded when filtering by agent
+  //   - "adhoc": excluded from default runs, must be explicitly requested
+  tags?: string[];
 }

 export function printSingleValidation(validation: ValidationResult): void {
@@ -27,6 +27,24 @@ type WriteFunction = {
  (chunk: string | Uint8Array, encoding?: BufferEncoding, cb?: WriteCallback): boolean;
 };

+// module-level activity tracking - allows agents to mark activity on any event
+let _lastActivity = Date.now();
+
+/**
+ * mark activity to reset the no-output timeout.
+ * call this whenever the agent emits any event, even if it isn't logged to stdout.
+ */
+export function markActivity(): void {
+  _lastActivity = Date.now();
+}
+
+/**
+ * get the time since last activity in milliseconds
+ */
+export function getIdleMs(): number {
+  return Date.now() - _lastActivity;
+}
+
 function wrapWrite(original: WriteFunction, onActivity: () => void): WriteFunction {
  const wrapped: WriteFunction = (
    chunk: string | Uint8Array,
@@ -43,21 +61,17 @@ function wrapWrite(original: WriteFunction, onActivity: () => void): WriteFuncti
 }

 function startProcessOutputMonitor(ctx: OutputMonitorContext): OutputMonitor {
-  let lastActivity = Date.now();
  let timedOut = false;

  const originalStdoutWrite: WriteFunction = process.stdout.write.bind(process.stdout);
  const originalStderrWrite: WriteFunction = process.stderr.write.bind(process.stderr);

-  function markActivity(): void {
-    lastActivity = Date.now();
-  }
-
+  // stdout/stderr writes also mark activity
  process.stdout.write = wrapWrite(originalStdoutWrite, markActivity);
  process.stderr.write = wrapWrite(originalStderrWrite, markActivity);

  const intervalId = setInterval(() => {
-    const idleMs = Date.now() - lastActivity;
+    const idleMs = getIdleMs();
    if (timedOut || idleMs <= ctx.timeoutMs) return;
    timedOut = true;
    ctx.onTimeout(idleMs);
@@ -73,6 +87,8 @@ function startProcessOutputMonitor(ctx: OutputMonitorContext): OutputMonitor {
 }

 export function createProcessOutputActivityTimeout(ctx: ActivityTimeoutContext): ActivityTimeout {
+  markActivity(); // reset baseline
+
  let rejectFn: ((error: Error) => void) | null = null;
  const promise = new Promise<never>((_, reject) => {
    rejectFn = reject;
@@ -22,8 +22,9 @@ export const JsonPayload = type({
  "repoInstructions?": "string",
  "event?": "object",
  "effort?": Effort.or("undefined"),
-  "timeout?": type.string.or("undefined"),
-  "progressCommentId?": type.string.or("undefined"),
+  "timeout?": "string | undefined",
+  "progressCommentId?": "string | undefined",
+  "debug?": "boolean | undefined",
 });

 // permission levels that indicate collaborator status (have push access)
@@ -165,6 +166,7 @@ export function resolvePayload(
    effort: inputs.effort ?? jsonPayload?.effort ?? "auto",
    timeout: inputs.timeout ?? jsonPayload?.timeout,
    cwd: resolveCwd(inputs.cwd),
+    debug: jsonPayload?.debug,

    // permissions: inputs > repoSettings > fallbacks
    web: inputs.web ?? repoSettings.web ?? "enabled",