safe_output_handler_manager.test.cjs

// @ts-check

import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import fs from "fs";
import os from "os";
import path from "path";
import { loadConfig, loadHandlers, processMessages, buildCommentMemoryMessagesFromFiles } from "./safe_output_handler_manager.cjs";

describe("Safe Output Handler Manager", () => {
  beforeEach(() => {
    // Mock global core
    global.core = {
      info: vi.fn(),
      debug: vi.fn(),
      warning: vi.fn(),
      error: vi.fn(),
      setOutput: vi.fn(),
      setFailed: vi.fn(),
    };
  });

  afterEach(() => {
    // Clean up environment variables
    delete process.env.GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG;
    delete process.env.GH_AW_TRACKER_LABEL;
    delete process.env.GH_AW_SAFE_OUTPUT_JOBS;
    delete process.env.GH_AW_SAFE_OUTPUT_SCRIPTS;
    fs.rmSync("/tmp/gh-aw/comment-memory", { recursive: true, force: true });
  });

  describe("loadConfig", () => {
    it("should load config from environment variable and normalize keys", () => {
      const config = {
        "create-issue": { max: 5 },
        "add-comment": { max: 1 },
      };

      process.env.GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG = JSON.stringify(config);

      const result = loadConfig();

      expect(result).toHaveProperty("create_issue");
      expect(result).toHaveProperty("add_comment");
      expect(result.create_issue).toEqual({ max: 5 });
      expect(result.add_comment).toEqual({ max: 1 });
    });

    it("should throw error if environment variable is not set", () => {
      expect(() => loadConfig()).toThrow("GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG environment variable is required but not set");
    });

    it("should throw error if environment variable contains invalid JSON", () => {
      process.env.GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG = "not json";
      expect(() => loadConfig()).toThrow("Failed to parse GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG");
    });
  });

  describe("loadHandlers", () => {
    // These tests are skipped because they require actual handler modules to exist
    // In a real environment, handlers are loaded dynamically via require()
    it.skip("should load handlers for enabled safe output types", async () => {
      const config = {
        create_issue: { max: 1 },
        add_comment: { max: 1 },
      };

      const handlers = await loadHandlers(config);

      expect(handlers.size).toBeGreaterThan(0);
      expect(handlers.has("create_issue")).toBe(true);
      expect(handlers.has("add_comment")).toBe(true);
    });

    it.skip("should not load handlers when config entry is missing", async () => {
      const config = {
        create_issue: { max: 1 },
        // add_comment is not in config
      };

      const handlers = await loadHandlers(config);

      expect(handlers.has("create_issue")).toBe(true);
      expect(handlers.has("add_comment")).toBe(false);
    });

    it.skip("should handle missing handlers gracefully", async () => {
      const config = {
        nonexistent_handler: { max: 1 },
      };

      const handlers = await loadHandlers(config);

      expect(handlers.size).toBe(0);
    });

    it("should throw error when handler main() does not return a function", async () => {
      // This test verifies that if a handler's main() function doesn't return
      // a message handler function, the loadHandlers function will throw an error
      // rather than just logging a warning.
      //
      // Expected behavior:
      // 1. Handler is loaded successfully
      // 2. main() is called with config
      // 3. If main() returns non-function, an error is thrown
      // 4. The error should fail the step
      //
      // This is important because:
      // - Old handlers execute directly and return undefined
      // - New handlers follow factory pattern and return a function
      // - Silent failures from misconfigured handlers are hard to debug
      //
      // The implementation checks: typeof messageHandler !== "function"
      // and throws: "Handler X main() did not return a function"

      // Note: Actual integration testing requires real handler modules
      // This test documents the expected behavior for validation
      expect(true).toBe(true);
    });
  });

  describe("loadHandlers - path traversal sanitization", () => {
    // These tests exercise the path traversal sanitization added to protect against
    // malicious scriptFilename values in GH_AW_SAFE_OUTPUT_SCRIPTS.
    // The sanitization runs BEFORE require(), so no real file needs to exist.

    it("should reject scriptFilename with a leading ../", async () => {
      process.env.GH_AW_SAFE_OUTPUT_SCRIPTS = JSON.stringify({
        evil: "../evil.cjs",
      });

      const handlers = await loadHandlers({}, {});

      expect(handlers.has("evil")).toBe(false);
      expect(core.error).toHaveBeenCalledWith(expect.stringContaining('path traversal detected in "../evil.cjs"'));
    });

    it("should reject scriptFilename with an embedded directory separator", async () => {
      process.env.GH_AW_SAFE_OUTPUT_SCRIPTS = JSON.stringify({
        evil: "subdir/evil.cjs",
      });

      const handlers = await loadHandlers({}, {});

      expect(handlers.has("evil")).toBe(false);
      expect(core.error).toHaveBeenCalledWith(expect.stringContaining('path traversal detected in "subdir/evil.cjs"'));
    });

    it("should reject scriptFilename with an absolute Unix path", async () => {
      process.env.GH_AW_SAFE_OUTPUT_SCRIPTS = JSON.stringify({
        evil: "/etc/passwd",
      });

      const handlers = await loadHandlers({}, {});

      expect(handlers.has("evil")).toBe(false);
      expect(core.error).toHaveBeenCalledWith(expect.stringContaining('path traversal detected in "/etc/passwd"'));
    });

    it("should reject scriptFilename with multiple levels of path traversal", async () => {
      process.env.GH_AW_SAFE_OUTPUT_SCRIPTS = JSON.stringify({
        evil: "../../etc/shadow",
      });

      const handlers = await loadHandlers({}, {});

      expect(handlers.has("evil")).toBe(false);
      expect(core.error).toHaveBeenCalledWith(expect.stringContaining('path traversal detected in "../../etc/shadow"'));
    });

    it("should reject scriptFilename that is just '..'", async () => {
      process.env.GH_AW_SAFE_OUTPUT_SCRIPTS = JSON.stringify({
        evil: "..",
      });

      const handlers = await loadHandlers({}, {});

      expect(handlers.has("evil")).toBe(false);
      expect(core.error).toHaveBeenCalledWith(expect.stringContaining("evil"));
    });

    it("should skip rejected entry but continue loading other valid scripts", async () => {
      process.env.GH_AW_SAFE_OUTPUT_SCRIPTS = JSON.stringify({
        evil: "../evil.cjs",
        // This valid name will attempt require() which will fail (file doesn't exist),
        // and that failure is caught as a non-fatal warning — so the handler is absent
        // but no core.error is called for path traversal.
        safe_script: "safe_output_script_safe_script.cjs",
      });

      const handlers = await loadHandlers({}, {});

      // The evil entry must be rejected with core.error (path traversal)
      expect(core.error).toHaveBeenCalledWith(expect.stringContaining('path traversal detected in "../evil.cjs"'));
      // The safe entry should NOT trigger a path traversal error
      expect(core.error).not.toHaveBeenCalledWith(expect.stringContaining('path traversal detected in "safe_output_script_safe_script.cjs"'));
      // Neither handler is loaded (safe one fails because the file doesn't exist in test env)
      expect(handlers.has("evil")).toBe(false);
    });

    it("should accept a plain filename with no path components", async () => {
      // A well-formed filename — require() will fail because the file doesn't exist in
      // the test environment, but NO core.error for path traversal should be logged.
      process.env.GH_AW_SAFE_OUTPUT_SCRIPTS = JSON.stringify({
        my_script: "safe_output_script_my_script.cjs",
      });

      const handlers = await loadHandlers({}, {});

      // No path traversal error should have been emitted
      expect(core.error).not.toHaveBeenCalledWith(expect.stringContaining("path traversal detected"));
      expect(core.error).not.toHaveBeenCalledWith(expect.stringContaining("Script path outside expected directory"));
      // Handler is absent only because the file doesn't exist in the test env (warning, not error)
      expect(handlers.has("my_script")).toBe(false);
    });
  });

  describe("processMessages", () => {
    it("should process messages in order of appearance", async () => {
      const messages = [
        { type: "add_comment", body: "Comment" },
        { type: "create_issue", title: "Issue" },
      ];

      const mockHandler = vi.fn().mockResolvedValue({ success: true });

      const handlers = new Map([
        ["create_issue", mockHandler],
        ["add_comment", mockHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.results).toHaveLength(2);

      // Verify handlers were called
      expect(mockHandler).toHaveBeenCalledTimes(2);

      // Verify messages were processed in order of appearance (add_comment first, then create_issue)
      expect(result.results[0].type).toBe("add_comment");
      expect(result.results[0].messageIndex).toBe(0);
      expect(result.results[1].type).toBe("create_issue");
      expect(result.results[1].messageIndex).toBe(1);
    });

    it("should pass the shared temporary ID map to handlers", async () => {
      const messages = [
        { type: "create_issue", title: "Issue", body: "Body", temporary_id: "aw_abc123" },
        { type: "update_project", project: "https://github.com/orgs/test/projects/1", content_type: "issue", content_number: "aw_abc123" },
      ];

      const mockCreateHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 101,
        temporaryId: "aw_abc123",
      });

      const mockUpdateProjectHandler = vi.fn().mockImplementation((message, resolvedTemporaryIds, temporaryIdMap) => {
        expect(temporaryIdMap).toBeInstanceOf(Map);
        expect(temporaryIdMap.get("aw_abc123")).toEqual({ repo: "owner/repo", number: 101 });
        expect(resolvedTemporaryIds["aw_abc123"]).toEqual({ repo: "owner/repo", number: 101 });
        return Promise.resolve({ success: true });
      });

      const handlers = new Map([
        ["create_issue", mockCreateHandler],
        ["update_project", mockUpdateProjectHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(mockCreateHandler).toHaveBeenCalledTimes(1);
      expect(mockUpdateProjectHandler).toHaveBeenCalledTimes(1);
    });

    it("should skip messages without type", async () => {
      const messages = [{ type: "create_issue", title: "Issue" }, { title: "No type" }, { type: "add_comment", body: "Comment" }];

      const mockHandler = vi.fn().mockResolvedValue({ success: true });

      const handlers = new Map([
        ["create_issue", mockHandler],
        ["add_comment", mockHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.results).toHaveLength(2);
      expect(core.warning).toHaveBeenCalledWith("Skipping message 2 without type");
    });

    it("should warn and record result when no handler is available for message type", async () => {
      const messages = [
        { type: "create_issue", title: "Issue" },
        { type: "unknown_type", data: "test" },
      ];

      const mockHandler = vi.fn().mockResolvedValue({ success: true });

      // Only create_issue handler is available, unknown_type has no handler
      const handlers = new Map([["create_issue", mockHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.results).toHaveLength(2);

      // First message should succeed
      expect(result.results[0].success).toBe(true);
      expect(result.results[0].type).toBe("create_issue");

      // Second message should be recorded as failed with no handler error
      expect(result.results[1].success).toBe(false);
      expect(result.results[1].type).toBe("unknown_type");
      expect(result.results[1].error).toContain("No handler loaded");

      // Should have logged a warning
      expect(core.warning).toHaveBeenCalledWith(expect.stringContaining("No handler loaded for message type 'unknown_type'"));
    });

    it("should skip custom safe output job types gracefully without error", async () => {
      // Set up custom safe output jobs (e.g., send_slack_message handled by a dedicated job step)
      process.env.GH_AW_SAFE_OUTPUT_JOBS = JSON.stringify({
        send_slack_message: "message_url",
      });

      const messages = [
        { type: "create_issue", title: "Issue" },
        { type: "send_slack_message", channel: "#alerts", text: "Hello" },
      ];

      const mockHandler = vi.fn().mockResolvedValue({ success: true });

      // Only create_issue handler is available; send_slack_message is a custom job
      const handlers = new Map([["create_issue", mockHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.results).toHaveLength(2);

      // First message should succeed
      expect(result.results[0].success).toBe(true);
      expect(result.results[0].type).toBe("create_issue");

      // Custom job message should be skipped gracefully (not an error)
      expect(result.results[1].success).toBe(false);
      expect(result.results[1].type).toBe("send_slack_message");
      expect(result.results[1].skipped).toBe(true);
      expect(result.results[1].reason).toBe("Handled by custom safe output job");
      expect(result.results[1].error).toBeUndefined();

      // Should NOT have logged a "No handler loaded" warning
      expect(core.warning).not.toHaveBeenCalledWith(expect.stringContaining("No handler loaded for message type 'send_slack_message'"));
    });

    it("should skip multiple custom safe output job types gracefully", async () => {
      process.env.GH_AW_SAFE_OUTPUT_JOBS = JSON.stringify({
        send_slack_message: "message_url",
        notion_add_comment: "comment_url",
      });

      const messages = [
        { type: "send_slack_message", channel: "#alerts", text: "Hello" },
        { type: "notion_add_comment", page_id: "abc123", text: "Note" },
        { type: "create_issue", title: "Issue" },
      ];

      const mockHandler = vi.fn().mockResolvedValue({ success: true });
      const handlers = new Map([["create_issue", mockHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.results).toHaveLength(3);

      // Custom job types should be skipped gracefully
      expect(result.results[0].skipped).toBe(true);
      expect(result.results[0].reason).toBe("Handled by custom safe output job");
      expect(result.results[1].skipped).toBe(true);
      expect(result.results[1].reason).toBe("Handled by custom safe output job");

      // create_issue should succeed
      expect(result.results[2].success).toBe(true);
    });

    it("should still warn for unknown types not in custom job types", async () => {
      process.env.GH_AW_SAFE_OUTPUT_JOBS = JSON.stringify({
        send_slack_message: "message_url",
      });

      const messages = [{ type: "completely_unknown_type", data: "test" }];

      const handlers = new Map();

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.results[0].error).toContain("No handler loaded");
      expect(result.results[0].skipped).toBeUndefined();

      // Should have logged a warning for truly unknown types
      expect(core.warning).toHaveBeenCalledWith(expect.stringContaining("No handler loaded for message type 'completely_unknown_type'"));
    });

    it("should handle handler errors gracefully", async () => {
      const messages = [{ type: "create_issue", title: "Issue" }];

      const errorHandler = vi.fn().mockRejectedValue(new Error("Handler failed"));

      const handlers = new Map([["create_issue", errorHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.results).toHaveLength(1);
      expect(result.results[0].success).toBe(false);
      expect(result.results[0].error).toBe("Handler failed");
    });

    it("should treat handler returning success: false as a failure", async () => {
      const messages = [{ type: "create_project_status_update", project: "https://github.com/orgs/test/projects/1", body: "Test" }];

      const failureHandler = vi.fn().mockResolvedValue({
        success: false,
        error: "GraphQL query failed",
      });

      const handlers = new Map([["create_project_status_update", failureHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.results).toHaveLength(1);
      expect(result.results[0].success).toBe(false);
      expect(result.results[0].error).toBe("GraphQL query failed");
      expect(core.error).toHaveBeenCalledWith(expect.stringContaining("failed: GraphQL query failed"));
    });

    it("should track outputs with unresolved temporary IDs", async () => {
      const messages = [
        {
          type: "create_issue",
          body: "See #aw_abc123 for context",
          title: "Test Issue",
        },
      ];

      const mockCreateIssueHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 100,
      });

      const handlers = new Map([["create_issue", mockCreateIssueHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.outputsWithUnresolvedIds).toBeDefined();
      // Should track the output because it has unresolved temp ID
      expect(result.outputsWithUnresolvedIds.length).toBe(1);
      expect(result.outputsWithUnresolvedIds[0].type).toBe("create_issue");
      expect(result.outputsWithUnresolvedIds[0].result.number).toBe(100);
    });

    it("tracks comment_memory outputs using managed body for temporary ID resolution", async () => {
      const messages = [
        {
          type: "comment_memory",
          body: "raw body without ids",
          memory_id: "default",
        },
      ];

      const mockCommentMemoryHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 55,
        commentId: 12345,
        managedBody: '<gh-aw-comment-memory id="default">\nSee #aw_abc123 for details\n</gh-aw-comment-memory>',
      });

      const handlers = new Map([["comment_memory", mockCommentMemoryHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.outputsWithUnresolvedIds.length).toBe(1);
      expect(result.outputsWithUnresolvedIds[0].type).toBe("comment_memory");
      expect(result.outputsWithUnresolvedIds[0].result.commentId).toBe(12345);
    });

    it("should track outputs needing synthetic updates when temporary ID is resolved", async () => {
      const messages = [
        {
          type: "create_issue",
          body: "See #aw_abc123 for context",
          title: "First Issue",
        },
        {
          type: "create_issue",
          temporary_id: "aw_abc123",
          body: "Second issue body",
          title: "Second Issue",
        },
      ];

      const mockCreateIssueHandler = vi
        .fn()
        .mockResolvedValueOnce({
          repo: "owner/repo",
          number: 100,
        })
        .mockResolvedValueOnce({
          repo: "owner/repo",
          number: 101,
          temporaryId: "aw_abc123",
        });

      const handlers = new Map([["create_issue", mockCreateIssueHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.outputsWithUnresolvedIds).toBeDefined();
      // Should track output with unresolved temp ID
      expect(result.outputsWithUnresolvedIds.length).toBe(1);
      expect(result.outputsWithUnresolvedIds[0].result.number).toBe(100);
      // Temp ID should be registered
      expect(result.temporaryIdMap["aw_abc123"]).toBeDefined();
      expect(result.temporaryIdMap["aw_abc123"].number).toBe(101);
    });

    it("should not track output if temporary IDs remain unresolved", async () => {
      const messages = [
        {
          type: "create_issue",
          body: "See #aw_abc123 and #aw_unresolved99 for context",
          title: "Test Issue",
        },
      ];

      const mockCreateIssueHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 100,
      });

      const handlers = new Map([["create_issue", mockCreateIssueHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.outputsWithUnresolvedIds).toBeDefined();
      // Should track because there are unresolved IDs
      expect(result.outputsWithUnresolvedIds.length).toBe(1);
    });

    it("should handle multiple outputs needing synthetic updates", async () => {
      const messages = [
        {
          type: "create_issue",
          body: "Related to #aw_aabbcc11",
          title: "First Issue",
        },
        {
          type: "create_discussion",
          body: "See #aw_aabbcc11 for details",
          title: "Discussion",
        },
        {
          type: "create_issue",
          temporary_id: "aw_aabbcc11",
          body: "The referenced issue",
          title: "Referenced Issue",
        },
      ];

      const mockCreateIssueHandler = vi
        .fn()
        .mockResolvedValueOnce({
          repo: "owner/repo",
          number: 100,
        })
        .mockResolvedValueOnce({
          repo: "owner/repo",
          number: 102,
          temporaryId: "aw_aabbcc11",
        });

      const mockCreateDiscussionHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 101,
      });

      const handlers = new Map([
        ["create_issue", mockCreateIssueHandler],
        ["create_discussion", mockCreateDiscussionHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.outputsWithUnresolvedIds).toBeDefined();
      // Should track 2 outputs (issue and discussion) with unresolved temp IDs
      expect(result.outputsWithUnresolvedIds.length).toBe(2);
      // Temp ID should be registered
      expect(result.temporaryIdMap["aw_aabbcc11"]).toBeDefined();
    });

    it("should populate artifactUrlMap when upload_artifact succeeds", async () => {
      const messages = [
        {
          type: "upload_artifact",
          temporary_id: "aw_chart1",
          path: "chart.png",
        },
      ];

      const mockUploadHandler = vi.fn().mockResolvedValue({
        tmpId: "aw_chart1",
        artifactUrl: "https://github.com/owner/repo/actions/runs/1/artifacts/42",
      });

      const handlers = new Map([["upload_artifact", mockUploadHandler]]);
      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.artifactUrlMap).toBeDefined();
      expect(result.artifactUrlMap.get("aw_chart1")).toBe("https://github.com/owner/repo/actions/runs/1/artifacts/42");
    });

    it("should track issue with artifact reference as needing synthetic update when artifact is uploaded after", async () => {
      const messages = [
        {
          type: "create_issue",
          title: "Issue with chart",
          body: "See chart: ![chart](#aw_chart1)",
        },
        {
          type: "upload_artifact",
          temporary_id: "aw_chart1",
          path: "chart.png",
        },
      ];

      const mockCreateIssueHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 100,
      });

      const mockUploadHandler = vi.fn().mockResolvedValue({
        tmpId: "aw_chart1",
        artifactUrl: "https://github.com/owner/repo/actions/runs/1/artifacts/42",
      });

      const handlers = new Map([
        ["create_issue", mockCreateIssueHandler],
        ["upload_artifact", mockUploadHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.artifactUrlMap.get("aw_chart1")).toBe("https://github.com/owner/repo/actions/runs/1/artifacts/42");
      // The issue was created before the artifact was uploaded, so it should be tracked
      expect(result.outputsWithUnresolvedIds.length).toBe(1);
      expect(result.outputsWithUnresolvedIds[0].result.number).toBe(100);
    });

    it("should replace artifact URL references in issue body when artifact is uploaded before issue", async () => {
      const messages = [
        {
          type: "upload_artifact",
          temporary_id: "aw_chart1",
          path: "chart.png",
        },
        {
          type: "create_issue",
          title: "Issue with chart",
          body: "See chart: ![chart](#aw_chart1)",
        },
      ];

      const mockUploadHandler = vi.fn().mockResolvedValue({
        tmpId: "aw_chart1",
        artifactUrl: "https://github.com/owner/repo/actions/runs/1/artifacts/42",
      });

      const mockCreateIssueHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 100,
      });

      const handlers = new Map([
        ["upload_artifact", mockUploadHandler],
        ["create_issue", mockCreateIssueHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      // When artifact is already uploaded, the body passed to create_issue should have URL replaced
      const calledMessage = mockCreateIssueHandler.mock.calls[0][0];
      expect(calledMessage.body).toContain("https://github.com/owner/repo/actions/runs/1/artifacts/42");
      expect(calledMessage.body).not.toContain("#aw_chart1");
      // The issue does not need synthetic update since body was pre-resolved
      expect(result.outputsWithUnresolvedIds.length).toBe(0);
    });

    describe("@filepath expansion in message body", () => {
      let tempWorkspace;

      beforeEach(() => {
        tempWorkspace = fs.mkdtempSync(path.join(os.tmpdir(), "handler-manager-ws-"));
        process.env.GITHUB_WORKSPACE = tempWorkspace;
      });

      afterEach(() => {
        delete process.env.GITHUB_WORKSPACE;
        fs.rmSync(tempWorkspace, { recursive: true, force: true });
      });

      it("should expand @filepath reference in add_comment body to file contents", async () => {
        const bodyFile = path.join(tempWorkspace, "comment-body.md");
        fs.writeFileSync(bodyFile, "# Hello\n\nThis is the comment body.");

        const messages = [{ type: "add_comment", body: `@${bodyFile}` }];

        const mockHandler = vi.fn().mockResolvedValue({ success: true });
        const handlers = new Map([["add_comment", mockHandler]]);

        await processMessages(handlers, messages);

        const calledMessage = mockHandler.mock.calls[0][0];
        expect(calledMessage.body).toBe("# Hello\n\nThis is the comment body.");
        expect(calledMessage.body).not.toContain("@/");
      });

      it("should expand @filepath reference in create_issue body to file contents", async () => {
        const bodyFile = path.join(tempWorkspace, "issue-body.md");
        fs.writeFileSync(bodyFile, "Issue body content.");

        const messages = [{ type: "create_issue", title: "My Issue", body: `@${bodyFile}` }];

        const mockHandler = vi.fn().mockResolvedValue({ repo: "owner/repo", number: 42 });
        const handlers = new Map([["create_issue", mockHandler]]);

        await processMessages(handlers, messages);

        const calledMessage = mockHandler.mock.calls[0][0];
        expect(calledMessage.body).toBe("Issue body content.");
      });

      it("should allow @filepath reference from /tmp/gh-aw when GITHUB_WORKSPACE is set", async () => {
        // Create a temp file simulating /tmp/gh-aw by writing to tempWorkspace/subdir instead
        // (actual /tmp/gh-aw may not be writable in all CI environments; we test via workspace)
        const bodyFile = path.join(tempWorkspace, "agent-body.md");
        fs.writeFileSync(bodyFile, "Agent comment body.");

        const messages = [{ type: "add_comment", body: `@${bodyFile}` }];

        const mockHandler = vi.fn().mockResolvedValue({ success: true });
        const handlers = new Map([["add_comment", mockHandler]]);

        await processMessages(handlers, messages);

        const calledMessage = mockHandler.mock.calls[0][0];
        expect(calledMessage.body).toBe("Agent comment body.");
      });

      it("should leave body unchanged when no @filepath reference is present", async () => {
        const messages = [{ type: "add_comment", body: "Plain comment body without file refs." }];

        const mockHandler = vi.fn().mockResolvedValue({ success: true });
        const handlers = new Map([["add_comment", mockHandler]]);

        await processMessages(handlers, messages);

        const calledMessage = mockHandler.mock.calls[0][0];
        expect(calledMessage.body).toBe("Plain comment body without file refs.");
      });

      it("should leave disallowed @filepath references unchanged", async () => {
        const messages = [{ type: "add_comment", body: "See @/etc/passwd for details." }];

        const mockHandler = vi.fn().mockResolvedValue({ success: true });
        const handlers = new Map([["add_comment", mockHandler]]);

        await processMessages(handlers, messages);

        const calledMessage = mockHandler.mock.calls[0][0];
        expect(calledMessage.body).toBe("See @/etc/passwd for details.");
      });
    });

    it("should silently skip message types handled by standalone steps", async () => {
      const messages = [
        { type: "create_issue", title: "Issue" },
        { type: "upload_asset", path: "file.txt" },
      ];

      const mockHandler = vi.fn().mockResolvedValue({ success: true });

      // Only create_issue handler is available
      // upload_asset is handled by a standalone step
      const handlers = new Map([["create_issue", mockHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.results).toHaveLength(2);

      // First message should succeed
      expect(result.results[0].success).toBe(true);
      expect(result.results[0].type).toBe("create_issue");

      // Second message should be skipped (standalone step)
      expect(result.results[1].success).toBe(false);
      expect(result.results[1].type).toBe("upload_asset");
      expect(result.results[1].skipped).toBe(true);
      expect(result.results[1].reason).toBe("Handled by standalone step");

      // Should NOT have logged warnings for standalone step types
      expect(core.warning).not.toHaveBeenCalledWith(expect.stringContaining("No handler loaded for message type 'upload_asset'"));

      // Should have logged debug messages
      expect(core.debug).toHaveBeenCalledWith(expect.stringContaining("upload_asset"));
    });

    it("should track skipped message types for logging", async () => {
      const messages = [
        { type: "create_issue", title: "Issue" },
        { type: "upload_asset", path: "file.txt" },
        { type: "unknown_type", data: "test" },
        { type: "another_unknown", data: "test2" },
      ];

      const mockHandler = vi.fn().mockResolvedValue({ success: true });

      // Only create_issue handler is available
      const handlers = new Map([["create_issue", mockHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);

      // Collect skipped standalone types
      const skippedStandaloneResults = result.results.filter(r => r.skipped && r.reason === "Handled by standalone step");
      const standaloneTypes = [...new Set(skippedStandaloneResults.map(r => r.type))];
      expect(standaloneTypes).toEqual(expect.arrayContaining(["upload_asset"]));

      // Collect skipped no-handler types
      const skippedNoHandlerResults = result.results.filter(r => !r.success && !r.skipped && r.error?.includes("No handler loaded"));
      const noHandlerTypes = [...new Set(skippedNoHandlerResults.map(r => r.type))];
      expect(noHandlerTypes).toEqual(expect.arrayContaining(["unknown_type", "another_unknown"]));
    });

    it("should register temporary IDs from deferred messages on retry", async () => {
      const messages = [
        {
          type: "link_sub_issue",
          parent_issue_number: "aw_parent12",
          sub_issue_number: 42,
        },
        {
          type: "create_issue",
          temporary_id: "aw_parent12",
          title: "Parent Issue",
          body: "Parent body",
        },
      ];

      // First call: link_sub_issue is deferred (parent not resolved yet)
      // Second call: create_issue succeeds and registers temp ID
      // Third call: link_sub_issue retry succeeds
      const mockLinkHandler = vi
        .fn()
        .mockResolvedValueOnce({
          deferred: true,
          error: "Unresolved temporary IDs: parent: aw_parent12",
        })
        .mockResolvedValueOnce({
          parent_issue_number: 100,
          sub_issue_number: 42,
          success: true,
        });

      const mockCreateIssueHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 100,
        temporaryId: "aw_parent12",
      });

      const handlers = new Map([
        ["link_sub_issue", mockLinkHandler],
        ["create_issue", mockCreateIssueHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);

      // Temp ID should be registered after create_issue
      expect(result.temporaryIdMap["aw_parent12"]).toBeDefined();
      expect(result.temporaryIdMap["aw_parent12"].number).toBe(100);

      // link_sub_issue should succeed after retry
      const linkResult = result.results.find(r => r.type === "link_sub_issue");
      expect(linkResult.success).toBe(true);
      expect(linkResult.deferred).toBe(false);
    });

    it("should track outputs created during deferred retry with unresolved temp IDs", async () => {
      const messages = [
        {
          type: "create_issue",
          temporary_id: "aw_aabbcc11",
          title: "Issue 1",
          body: "References #aw_ddeeff22",
        },
        {
          type: "link_sub_issue",
          parent_issue_number: "aw_aabbcc11",
          sub_issue_number: "aw_ddeeff22",
        },
        {
          type: "create_issue",
          temporary_id: "aw_ddeeff22",
          title: "Issue 2",
          body: "Issue 2 body",
        },
      ];

      // create_issue for issue1: succeeds with unresolved temp ID in body
      // link_sub_issue: deferred (parent and sub not resolved)
      // create_issue for issue2: succeeds
      // link_sub_issue retry: succeeds
      const mockCreateHandler = vi
        .fn()
        .mockResolvedValueOnce({
          repo: "owner/repo",
          number: 100,
          temporaryId: "aw_aabbcc11",
        })
        .mockResolvedValueOnce({
          repo: "owner/repo",
          number: 101,
          temporaryId: "aw_ddeeff22",
        });

      const mockLinkHandler = vi
        .fn()
        .mockResolvedValueOnce({
          deferred: true,
          error: "Unresolved temporary IDs",
        })
        .mockResolvedValueOnce({
          parent_issue_number: 100,
          sub_issue_number: 101,
          success: true,
        });

      const handlers = new Map([
        ["create_issue", mockCreateHandler],
        ["link_sub_issue", mockLinkHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);

      // Both issues should have temp IDs registered
      expect(result.temporaryIdMap["aw_aabbcc11"]).toBeDefined();
      expect(result.temporaryIdMap["aw_ddeeff22"]).toBeDefined();

      // Issue 1 should be tracked for synthetic update (had unresolved temp ID in body at creation time)
      // Note: By the time all messages are processed, the temp ID is resolved, but Issue 1 was
      // tracked when it was created because at that moment aw_ddeeff22 was not yet in the map
      const trackedIssue1 = result.outputsWithUnresolvedIds.find(o => o.result.number === 100);
      expect(trackedIssue1).toBeDefined();
    });

    it("should handle complex parent/sub-issue creation order", async () => {
      const messages = [
        {
          type: "create_issue",
          temporary_id: "aw_abc11def",
          title: "Parent",
          body: "See #aw_111aaa22 and #aw_333ccc44",
        },
        {
          type: "create_issue",
          temporary_id: "aw_111aaa22",
          title: "Sub 1",
          body: "Sub 1 body",
        },
        {
          type: "create_issue",
          temporary_id: "aw_333ccc44",
          title: "Sub 2",
          body: "Sub 2 body",
        },
      ];

      let issueCounter = 100;
      const mockCreateHandler = vi.fn().mockImplementation(message => {
        const tempId = message.temporary_id;
        const issueNumber = issueCounter++;
        return Promise.resolve({
          repo: "owner/repo",
          number: issueNumber,
          temporaryId: tempId,
        });
      });

      const handlers = new Map([["create_issue", mockCreateHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);

      // All temp IDs should be registered
      expect(result.temporaryIdMap["aw_abc11def"]).toBeDefined();
      expect(result.temporaryIdMap["aw_111aaa22"]).toBeDefined();
      expect(result.temporaryIdMap["aw_333ccc44"]).toBeDefined();

      // Parent issue should be tracked (had unresolved temp IDs at creation time)
      // When the parent was created, aw_111aaa22 and aw_333ccc44 were not yet in the map
      const parentTracked = result.outputsWithUnresolvedIds.find(
        o => o.result.number === 100 // Parent was issue #100
      );
      expect(parentTracked).toBeDefined();
      expect(parentTracked.type).toBe("create_issue");
    });

    it("should collect missing_tool and missing_data messages and include in result", async () => {
      const messages = [
        {
          type: "missing_tool",
          tool: "docker",
          reason: "Need containerization",
          alternatives: "Use VM",
        },
        {
          type: "create_issue",
          title: "Test Issue",
          body: "Issue body",
        },
        {
          type: "missing_data",
          data_type: "api_key",
          reason: "API credentials missing",
          context: "GitHub API access",
        },
        {
          type: "missing_tool",
          tool: "kubectl",
          reason: "Kubernetes management",
        },
      ];

      const mockCreateIssueHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 100,
      });

      const handlers = new Map([
        ["create_issue", mockCreateIssueHandler],
        ["missing_tool", vi.fn().mockResolvedValue({ success: true })],
        ["missing_data", vi.fn().mockResolvedValue({ success: true })],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.missings).toBeDefined();
      expect(result.missings.missingTools).toHaveLength(2);
      expect(result.missings.missingData).toHaveLength(1);

      // Check missing tools
      expect(result.missings.missingTools[0].tool).toBe("docker");
      expect(result.missings.missingTools[0].reason).toBe("Need containerization");
      expect(result.missings.missingTools[0].alternatives).toBe("Use VM");

      expect(result.missings.missingTools[1].tool).toBe("kubectl");
      expect(result.missings.missingTools[1].reason).toBe("Kubernetes management");
      expect(result.missings.missingTools[1].alternatives).toBeNull();

      // Check missing data
      expect(result.missings.missingData[0].data_type).toBe("api_key");
      expect(result.missings.missingData[0].reason).toBe("API credentials missing");
      expect(result.missings.missingData[0].context).toBe("GitHub API access");
    });

    it("should collect noop messages alongside missing_tool and missing_data", async () => {
      const messages = [
        {
          type: "noop",
          message: "No issues found in this review",
        },
        {
          type: "create_issue",
          title: "Test Issue",
          body: "Issue body",
        },
        {
          type: "missing_tool",
          tool: "docker",
          reason: "Need containerization",
        },
        {
          type: "noop",
          message: "Analysis complete",
        },
      ];

      const mockCreateIssueHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 100,
      });

      const handlers = new Map([
        ["create_issue", mockCreateIssueHandler],
        ["missing_tool", vi.fn().mockResolvedValue({ success: true })],
        ["noop", vi.fn().mockResolvedValue({ success: true })],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.missings).toBeDefined();
      expect(result.missings.missingTools).toHaveLength(1);
      expect(result.missings.missingData).toHaveLength(0);
      expect(result.missings.noopMessages).toHaveLength(2);

      // Check missing tools
      expect(result.missings.missingTools[0].tool).toBe("docker");
      expect(result.missings.missingTools[0].reason).toBe("Need containerization");

      // Check noop messages
      expect(result.missings.noopMessages[0].message).toBe("No issues found in this review");
      expect(result.missings.noopMessages[1].message).toBe("Analysis complete");
    });

    it("should collect report_incomplete signals alongside other message types", async () => {
      const messages = [
        {
          type: "report_incomplete",
          reason: "MCP server crashed during execution",
          details: "Connection refused on port 3000",
        },
        {
          type: "create_issue",
          title: "Test Issue",
          body: "Issue body",
        },
        {
          type: "report_incomplete",
          reason: "Missing authentication token",
        },
        {
          type: "missing_tool",
          tool: "docker",
          reason: "Need containerization",
        },
      ];

      const mockCreateIssueHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 100,
      });

      const handlers = new Map([
        ["create_issue", mockCreateIssueHandler],
        ["report_incomplete", vi.fn().mockResolvedValue({ success: true })],
        ["missing_tool", vi.fn().mockResolvedValue({ success: true })],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.missings).toBeDefined();
      expect(result.missings.reportIncomplete).toHaveLength(2);
      expect(result.missings.missingTools).toHaveLength(1);

      expect(result.missings.reportIncomplete[0].reason).toBe("MCP server crashed during execution");
      expect(result.missings.reportIncomplete[0].details).toBe("Connection refused on port 3000");
      expect(result.missings.reportIncomplete[1].reason).toBe("Missing authentication token");
      expect(result.missings.reportIncomplete[1].details).toBeNull();
    });

    it("should return empty arrays when no missing messages present", async () => {
      const messages = [
        {
          type: "create_issue",
          title: "Test Issue",
          body: "Issue body",
        },
      ];

      const mockCreateIssueHandler = vi.fn().mockResolvedValue({
        repo: "owner/repo",
        number: 100,
      });

      const handlers = new Map([["create_issue", mockCreateIssueHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.missings).toBeDefined();
      expect(result.missings.missingTools).toHaveLength(0);
      expect(result.missings.missingData).toHaveLength(0);
      expect(result.missings.noopMessages).toHaveLength(0);
      expect(result.missings.reportIncomplete).toHaveLength(0);
    });
  });

  describe("code-push fail-fast behaviour", () => {
    it("should cancel subsequent non-add_comment messages when push_to_pull_request_branch fails", async () => {
      const messages = [{ type: "push_to_pull_request_branch" }, { type: "add_comment", body: "Success!" }, { type: "create_issue", title: "Issue" }];

      const codePushHandler = vi.fn().mockResolvedValue({ success: false, error: "Branch not found" });
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);
      const issueHandler = vi.fn();

      const handlers = new Map([
        ["push_to_pull_request_branch", codePushHandler],
        ["add_comment", commentHandler],
        ["create_issue", issueHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      // Code-push failure recorded
      expect(result.codePushFailures).toHaveLength(1);
      expect(result.codePushFailures[0].type).toBe("push_to_pull_request_branch");
      expect(result.codePushFailures[0].error).toBe("Branch not found");
      // First result: code-push failed
      expect(result.results[0].success).toBe(false);
      expect(result.results[0].error).toBe("Branch not found");
      // add_comment is NOT cancelled — it should be called with a failure note prepended
      expect(result.results[1].cancelled).toBeUndefined();
      expect(commentHandler).toHaveBeenCalledTimes(1);
      const calledMessage = commentHandler.mock.calls[0][0];
      expect(calledMessage.body).toContain("push_to_pull_request_branch");
      expect(calledMessage.body).toContain("Branch not found");
      // create_issue IS still cancelled (non-add_comment non-code-push type)
      expect(result.results[2].success).toBe(false);
      expect(result.results[2].cancelled).toBe(true);
      // create_issue handler was NOT called
      expect(issueHandler).not.toHaveBeenCalled();
    });

    it("should allow add_comment through when create_pull_request fails via exception", async () => {
      const messages = [{ type: "create_pull_request" }, { type: "add_comment", body: "PR created!" }];

      const codePushHandler = vi.fn().mockRejectedValue(new Error("API error"));
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);

      const handlers = new Map([
        ["create_pull_request", codePushHandler],
        ["add_comment", commentHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.codePushFailures).toHaveLength(1);
      expect(result.codePushFailures[0].type).toBe("create_pull_request");
      expect(result.codePushFailures[0].error).toBe("API error");
      // add_comment is NOT cancelled — handler is called with failure note
      expect(result.results[1].cancelled).toBeUndefined();
      expect(commentHandler).toHaveBeenCalledTimes(1);
      const calledMessage = commentHandler.mock.calls[0][0];
      expect(calledMessage.body).toContain("create_pull_request");
      expect(calledMessage.body).toContain("API error");
      expect(calledMessage.body).toContain("PR created!");
    });

    it("should NOT cancel subsequent code-push messages after a code-push failure", async () => {
      const messages = [{ type: "push_to_pull_request_branch" }, { type: "create_pull_request" }];

      const pushHandler = vi.fn().mockResolvedValue({ success: false, error: "Push failed" });
      const createPRHandler = vi.fn().mockResolvedValue({ success: true, url: "https://github.com/pr/1" });

      const handlers = new Map([
        ["push_to_pull_request_branch", pushHandler],
        ["create_pull_request", createPRHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.codePushFailures).toHaveLength(1);
      // create_pull_request is also a code-push type, so it should NOT be cancelled
      expect(result.results[1].cancelled).toBeUndefined();
      expect(createPRHandler).toHaveBeenCalled();
    });

    it("should not cancel messages when no code-push failure occurs", async () => {
      const messages = [{ type: "push_to_pull_request_branch" }, { type: "add_comment", body: "Success!" }];

      const codePushHandler = vi.fn().mockResolvedValue({ success: true, branch: "my-branch" });
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);

      const handlers = new Map([
        ["push_to_pull_request_branch", codePushHandler],
        ["add_comment", commentHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.codePushFailures).toHaveLength(0);
      expect(result.results[0].success).toBe(true);
      expect(result.results[1].cancelled).toBeUndefined();
      expect(commentHandler).toHaveBeenCalled();
    });

    it("should return empty codePushFailures array when no code-push types present", async () => {
      const messages = [{ type: "create_issue", title: "Issue" }];
      const mockHandler = vi.fn().mockResolvedValue({ repo: "owner/repo", number: 1 });
      const handlers = new Map([["create_issue", mockHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.codePushFailures).toBeDefined();
      expect(result.codePushFailures).toHaveLength(0);
    });

    it("should NOT cancel subsequent messages when push_to_pull_request_branch returns skipped (if_no_changes: warn)", async () => {
      const messages = [{ type: "push_to_pull_request_branch" }, { type: "create_issue", title: "Issue" }, { type: "add_comment", body: "Done!" }];

      // Simulates push handler returning { success: false, skipped: true } for if_no_changes: warn/ignore
      const codePushHandler = vi.fn().mockResolvedValue({ success: false, error: "No patch file found - cannot push without changes", skipped: true });
      const issueHandler = vi.fn().mockResolvedValue({ repo: "owner/repo", number: 42 });
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);

      const handlers = new Map([
        ["push_to_pull_request_branch", codePushHandler],
        ["create_issue", issueHandler],
        ["add_comment", commentHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      // Skipped code-push should NOT be recorded as a codePushFailure
      expect(result.codePushFailures).toHaveLength(0);
      // First result: skipped (not failed)
      expect(result.results[0].success).toBe(false);
      expect(result.results[0].skipped).toBe(true);
      expect(result.results[0].error).toContain("No patch file found");
      // Subsequent results: NOT cancelled — handlers were called
      expect(result.results[1].cancelled).toBeUndefined();
      expect(result.results[2].cancelled).toBeUndefined();
      expect(issueHandler).toHaveBeenCalled();
      expect(commentHandler).toHaveBeenCalled();
    });

    it("should NOT cancel subsequent messages when create_pull_request returns skipped", async () => {
      const messages = [{ type: "create_pull_request" }, { type: "add_comment", body: "Done!" }];

      const codePushHandler = vi.fn().mockResolvedValue({ success: false, error: "No patch file found - cannot push without changes", skipped: true });
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);

      const handlers = new Map([
        ["create_pull_request", codePushHandler],
        ["add_comment", commentHandler],
      ]);

      const result = await processMessages(handlers, messages);

      expect(result.success).toBe(true);
      expect(result.codePushFailures).toHaveLength(0);
      expect(result.results[0].skipped).toBe(true);
      expect(result.results[1].cancelled).toBeUndefined();
      expect(commentHandler).toHaveBeenCalled();
    });

    it("should prepend fallback note to add_comment body when create_pull_request falls back to issue", async () => {
      const messages = [
        {
          type: "create_pull_request",
          title: "My Fix PR",
          body: "This fixes the issue.",
        },
        {
          type: "add_comment",
          body: "A fix PR has been created. Please review and merge.",
        },
      ];

      const prHandler = vi.fn().mockResolvedValue({
        success: true,
        fallback_used: true,
        issue_number: 42,
        issue_url: "https://github.com/owner/repo/issues/42",
        repo: "owner/repo",
      });
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);

      const handlers = new Map([
        ["create_pull_request", prHandler],
        ["add_comment", commentHandler],
      ]);

      await processMessages(handlers, messages);

      // The add_comment handler must have been called with the modified body
      expect(commentHandler).toHaveBeenCalledTimes(1);
      const calledMessage = commentHandler.mock.calls[0][0];
      expect(calledMessage.body).toContain("A fix PR has been created. Please review and merge.");
      expect(calledMessage.body).toContain("pull request was not created");
      expect(calledMessage.body).toContain("#42");
      expect(calledMessage.body).toContain("https://github.com/owner/repo/issues/42");
      // Note should be prepended before the original body
      const noteIndex = calledMessage.body.indexOf("pull request was not created");
      const bodyIndex = calledMessage.body.indexOf("A fix PR has been created");
      expect(noteIndex).toBeLessThan(bodyIndex);
    });

    it("should prepend fallback note to add_comment body when push_to_pull_request_branch falls back to issue", async () => {
      const messages = [
        { type: "push_to_pull_request_branch", branch: "fix-branch" },
        { type: "add_comment", body: "Changes pushed." },
      ];

      const pushHandler = vi.fn().mockResolvedValue({
        success: true,
        fallback_used: true,
        issue_number: 7,
        issue_url: "https://github.com/owner/repo/issues/7",
      });
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);

      const handlers = new Map([
        ["push_to_pull_request_branch", pushHandler],
        ["add_comment", commentHandler],
      ]);

      await processMessages(handlers, messages);

      const calledMessage = commentHandler.mock.calls[0][0];
      expect(calledMessage.body).toContain("pull request was not created");
      expect(calledMessage.body).toContain("#7");
    });

    it("should prepend fallback note to add_comment body when push_to_pull_request_branch falls back to pull request", async () => {
      const messages = [
        { type: "push_to_pull_request_branch", branch: "fix-branch" },
        { type: "add_comment", body: "Changes pushed." },
      ];

      const pushHandler = vi.fn().mockResolvedValue({
        success: true,
        fallback_used: true,
        fallback_type: "pull_request",
        pull_request_number: 71,
        pull_request_url: "https://github.com/owner/repo/pull/71",
      });
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);

      const handlers = new Map([
        ["push_to_pull_request_branch", pushHandler],
        ["add_comment", commentHandler],
      ]);

      await processMessages(handlers, messages);

      const calledMessage = commentHandler.mock.calls[0][0];
      expect(calledMessage.body).toContain("Direct push to the original pull request branch was not possible");
      expect(calledMessage.body).toContain("#71");
      expect(calledMessage.body).toContain("https://github.com/owner/repo/pull/71");
    });

    it("should NOT prepend fallback note when create_pull_request succeeds normally", async () => {
      const messages = [
        { type: "create_pull_request", title: "My Fix PR" },
        { type: "add_comment", body: "A fix PR has been created." },
      ];

      const prHandler = vi.fn().mockResolvedValue({
        success: true,
        pull_request_number: 5,
        pull_request_url: "https://github.com/owner/repo/pull/5",
        repo: "owner/repo",
      });
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);

      const handlers = new Map([
        ["create_pull_request", prHandler],
        ["add_comment", commentHandler],
      ]);

      await processMessages(handlers, messages);

      const calledMessage = commentHandler.mock.calls[0][0];
      expect(calledMessage.body).toBe("A fix PR has been created.");
      expect(calledMessage.body).not.toContain("pull request was not created");
    });

    it("should prepend failure note to add_comment body when create_pull_request fails (e.g. patch application error)", async () => {
      const messages = [
        { type: "create_pull_request", title: "My Fix PR" },
        { type: "add_comment", body: "The agent has completed its work." },
      ];

      const prHandler = vi.fn().mockResolvedValue({ success: false, error: "Failed to apply patch" });
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);

      const handlers = new Map([
        ["create_pull_request", prHandler],
        ["add_comment", commentHandler],
      ]);

      await processMessages(handlers, messages);

      // add_comment handler must have been called (not cancelled)
      expect(commentHandler).toHaveBeenCalledTimes(1);
      const calledMessage = commentHandler.mock.calls[0][0];
      // Failure note should be prepended
      expect(calledMessage.body).toContain("create_pull_request");
      expect(calledMessage.body).toContain("Failed to apply patch");
      expect(calledMessage.body).toContain("The agent has completed its work.");
      // Note should appear before the original body
      const noteIndex = calledMessage.body.indexOf("create_pull_request");
      const bodyIndex = calledMessage.body.indexOf("The agent has completed its work.");
      expect(noteIndex).toBeLessThan(bodyIndex);
    });

    it("should prepend failure note to add_comment body when push_to_pull_request_branch fails", async () => {
      const messages = [
        { type: "push_to_pull_request_branch", branch: "fix-branch" },
        { type: "add_comment", body: "Changes have been pushed." },
      ];

      const pushHandler = vi.fn().mockResolvedValue({ success: false, error: "Branch not found" });
      const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]);

      const handlers = new Map([
        ["push_to_pull_request_branch", pushHandler],
        ["add_comment", commentHandler],
      ]);

      await processMessages(handlers, messages);

      expect(commentHandler).toHaveBeenCalledTimes(1);
      const calledMessage = commentHandler.mock.calls[0][0];
      expect(calledMessage.body).toContain("push_to_pull_request_branch");
      expect(calledMessage.body).toContain("Branch not found");
      expect(calledMessage.body).toContain("Changes have been pushed.");
    });
  });

  describe("output emission via emitSafeOutputActionOutputs", () => {
    it("processMessages result includes create_issue result with number and url for emission", async () => {
      const messages = [{ type: "create_issue", title: "My Issue" }];
      const mockHandler = vi.fn().mockResolvedValue({ number: 42, url: "https://github.com/owner/repo/issues/42" });
      const handlers = new Map([["create_issue", mockHandler]]);

      const result = await processMessages(handlers, messages);

      const issueResult = result.results.find(r => r.type === "create_issue" && r.success);
      expect(issueResult).toBeDefined();
      expect(issueResult.result.number).toBe(42);
      expect(issueResult.result.url).toBe("https://github.com/owner/repo/issues/42");
    });

    it("processMessages result with failed create_issue does not include success result for emission", async () => {
      const messages = [{ type: "create_issue", title: "Failing Issue" }];
      const mockHandler = vi.fn().mockRejectedValue(new Error("API error"));
      const handlers = new Map([["create_issue", mockHandler]]);

      const result = await processMessages(handlers, messages);

      const successfulIssueResult = result.results.find(r => r.type === "create_issue" && r.success);
      expect(successfulIssueResult).toBeUndefined();
    });

    it("core.setOutput is called with created_issue_number when create_issue succeeds", async () => {
      const messages = [{ type: "create_issue", title: "My Issue" }];
      const mockHandler = vi.fn().mockResolvedValue({ number: 7, url: "https://github.com/owner/repo/issues/7" });
      const handlers = new Map([["create_issue", mockHandler]]);

      const result = await processMessages(handlers, messages);

      // Simulate what main() does: call emitSafeOutputActionOutputs with the result
      const { emitSafeOutputActionOutputs } = await import("./safe_outputs_action_outputs.cjs");
      emitSafeOutputActionOutputs(result);

      expect(global.core.setOutput).toHaveBeenCalledWith("created_issue_number", "7");
      expect(global.core.setOutput).toHaveBeenCalledWith("created_issue_url", "https://github.com/owner/repo/issues/7");
    });
  });

  describe("call_workflow handler registration", () => {
    it("processes call_workflow messages without no-handler warnings when handler is registered", async () => {
      const messages = [{ type: "call_workflow", workflow_name: "worker-a" }];
      const mockHandler = vi.fn().mockResolvedValue({
        success: true,
        workflow_name: "worker-a",
        payload: "{}",
      });
      const handlers = new Map([["call_workflow", mockHandler]]);

      const result = await processMessages(handlers, messages);

      expect(result.results).toHaveLength(1);
      expect(result.results[0].type).toBe("call_workflow");
      // Handler was invoked, so no "no handler loaded" error
      expect(result.results[0].error).toBeUndefined();
      expect(mockHandler).toHaveBeenCalledTimes(1);
    });

    it("records no-handler error for call_workflow when handler map is missing entry", async () => {
      const messages = [{ type: "call_workflow", workflow_name: "worker-a" }];
      // Empty handler map - simulates the bug where call_workflow was not in HANDLER_MAP
      const handlers = new Map();

      const result = await processMessages(handlers, messages);

      expect(result.results).toHaveLength(1);
      expect(result.results[0].success).toBe(false);
      expect(result.results[0].error).toContain("No handler loaded for type 'call_workflow'");
    });
  });

  describe("buildCommentMemoryMessagesFromFiles", () => {
    it("loads comment-memory messages from markdown files when configured", () => {
      fs.mkdirSync("/tmp/gh-aw/comment-memory", { recursive: true });
      fs.writeFileSync("/tmp/gh-aw/comment-memory/default.md", "saved memory");

      const messages = buildCommentMemoryMessagesFromFiles([], { comment_memory: { max: "1" } });

      expect(messages).toEqual([
        {
          type: "comment_memory",
          memory_id: "default",
          body: "saved memory",
        },
      ]);
    });

    it("skips file-based comment memory when a message already exists for the same memory_id", () => {
      fs.mkdirSync("/tmp/gh-aw/comment-memory", { recursive: true });
      fs.writeFileSync("/tmp/gh-aw/comment-memory/default.md", "saved memory");

      const messages = buildCommentMemoryMessagesFromFiles([{ type: "comment_memory", memory_id: "default", body: "from output" }], { comment_memory: { max: "1" } });

      expect(messages).toEqual([]);
    });

    it("treats comment_memory messages without memory_id as default memory when checking precedence", () => {
      fs.mkdirSync("/tmp/gh-aw/comment-memory", { recursive: true });
      fs.writeFileSync("/tmp/gh-aw/comment-memory/default.md", "saved memory");

      const messages = buildCommentMemoryMessagesFromFiles([{ type: "comment_memory", body: "from output" }], { comment_memory: { max: "1", memory_id: "default" } });

      expect(messages).toEqual([]);
    });
  });
});