Merge remote-tracking branch 'public-gh/master' into feature/workspace-runtime-support

* public-gh/master:
  Rebind seeded project workspaces to the current worktree
  Copy seeded secrets key into worktree instances
  server: make approval retries idempotent (#499)
  fix: address review feedback — stale error message and * wildcard
  Update server/src/routes/assets.ts
  feat: make attachment content types configurable via env var
  fix: wire parentId query filter into issues list endpoint

server/src/__tests__/approval-routes-idempotency.test.ts

@@ -0,0 +1,110 @@
+import express from "express";
+import request from "supertest";
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { approvalRoutes } from "../routes/approvals.js";
+import { errorHandler } from "../middleware/index.js";
+
+const mockApprovalService = vi.hoisted(() => ({
+  list: vi.fn(),
+  getById: vi.fn(),
+  create: vi.fn(),
+  approve: vi.fn(),
+  reject: vi.fn(),
+  requestRevision: vi.fn(),
+  resubmit: vi.fn(),
+  listComments: vi.fn(),
+  addComment: vi.fn(),
+}));
+
+const mockHeartbeatService = vi.hoisted(() => ({
+  wakeup: vi.fn(),
+}));
+
+const mockIssueApprovalService = vi.hoisted(() => ({
+  listIssuesForApproval: vi.fn(),
+  linkManyForApproval: vi.fn(),
+}));
+
+const mockSecretService = vi.hoisted(() => ({
+  normalizeHireApprovalPayloadForPersistence: vi.fn(),
+}));
+
+const mockLogActivity = vi.hoisted(() => vi.fn());
+
+vi.mock("../services/index.js", () => ({
+  approvalService: () => mockApprovalService,
+  heartbeatService: () => mockHeartbeatService,
+  issueApprovalService: () => mockIssueApprovalService,
+  logActivity: mockLogActivity,
+  secretService: () => mockSecretService,
+}));
+
+function createApp() {
+  const app = express();
+  app.use(express.json());
+  app.use((req, _res, next) => {
+    (req as any).actor = {
+      type: "board",
+      userId: "user-1",
+      companyIds: ["company-1"],
+      source: "session",
+      isInstanceAdmin: false,
+    };
+    next();
+  });
+  app.use("/api", approvalRoutes({} as any));
+  app.use(errorHandler);
+  return app;
+}
+
+describe("approval routes idempotent retries", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockHeartbeatService.wakeup.mockResolvedValue({ id: "wake-1" });
+    mockIssueApprovalService.listIssuesForApproval.mockResolvedValue([{ id: "issue-1" }]);
+    mockLogActivity.mockResolvedValue(undefined);
+  });
+
+  it("does not emit duplicate approval side effects when approve is already resolved", async () => {
+    mockApprovalService.approve.mockResolvedValue({
+      approval: {
+        id: "approval-1",
+        companyId: "company-1",
+        type: "hire_agent",
+        status: "approved",
+        payload: {},
+        requestedByAgentId: "agent-1",
+      },
+      applied: false,
+    });
+
+    const res = await request(createApp())
+      .post("/api/approvals/approval-1/approve")
+      .send({});
+
+    expect(res.status).toBe(200);
+    expect(mockIssueApprovalService.listIssuesForApproval).not.toHaveBeenCalled();
+    expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled();
+    expect(mockLogActivity).not.toHaveBeenCalled();
+  });
+
+  it("does not emit duplicate rejection logs when reject is already resolved", async () => {
+    mockApprovalService.reject.mockResolvedValue({
+      approval: {
+        id: "approval-1",
+        companyId: "company-1",
+        type: "hire_agent",
+        status: "rejected",
+        payload: {},
+      },
+      applied: false,
+    });
+
+    const res = await request(createApp())
+      .post("/api/approvals/approval-1/reject")
+      .send({});
+
+    expect(res.status).toBe(200);
+    expect(mockLogActivity).not.toHaveBeenCalled();
+  });
+});

server/src/__tests__/approvals-service.test.ts

@@ -0,0 +1,110 @@
+import { beforeEach, describe, expect, it, vi } from "vitest";
+import { approvalService } from "../services/approvals.js";
+
+const mockAgentService = vi.hoisted(() => ({
+  activatePendingApproval: vi.fn(),
+  create: vi.fn(),
+  terminate: vi.fn(),
+}));
+
+const mockNotifyHireApproved = vi.hoisted(() => vi.fn());
+
+vi.mock("../services/agents.js", () => ({
+  agentService: vi.fn(() => mockAgentService),
+}));
+
+vi.mock("../services/hire-hook.js", () => ({
+  notifyHireApproved: mockNotifyHireApproved,
+}));
+
+type ApprovalRecord = {
+  id: string;
+  companyId: string;
+  type: string;
+  status: string;
+  payload: Record<string, unknown>;
+  requestedByAgentId: string | null;
+};
+
+function createApproval(status: string): ApprovalRecord {
+  return {
+    id: "approval-1",
+    companyId: "company-1",
+    type: "hire_agent",
+    status,
+    payload: { agentId: "agent-1" },
+    requestedByAgentId: "requester-1",
+  };
+}
+
+function createDbStub(selectResults: ApprovalRecord[][], updateResults: ApprovalRecord[]) {
+  const selectWhere = vi.fn();
+  for (const result of selectResults) {
+    selectWhere.mockResolvedValueOnce(result);
+  }
+
+  const from = vi.fn(() => ({ where: selectWhere }));
+  const select = vi.fn(() => ({ from }));
+
+  const returning = vi.fn().mockResolvedValue(updateResults);
+  const updateWhere = vi.fn(() => ({ returning }));
+  const set = vi.fn(() => ({ where: updateWhere }));
+  const update = vi.fn(() => ({ set }));
+
+  return {
+    db: { select, update },
+    selectWhere,
+    returning,
+  };
+}
+
+describe("approvalService resolution idempotency", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    mockAgentService.activatePendingApproval.mockResolvedValue(undefined);
+    mockAgentService.create.mockResolvedValue({ id: "agent-1" });
+    mockAgentService.terminate.mockResolvedValue(undefined);
+    mockNotifyHireApproved.mockResolvedValue(undefined);
+  });
+
+  it("treats repeated approve retries as no-ops after another worker resolves the approval", async () => {
+    const dbStub = createDbStub(
+      [[createApproval("pending")], [createApproval("approved")]],
+      [],
+    );
+
+    const svc = approvalService(dbStub.db as any);
+    const result = await svc.approve("approval-1", "board", "ship it");
+
+    expect(result.applied).toBe(false);
+    expect(result.approval.status).toBe("approved");
+    expect(mockAgentService.activatePendingApproval).not.toHaveBeenCalled();
+    expect(mockNotifyHireApproved).not.toHaveBeenCalled();
+  });
+
+  it("treats repeated reject retries as no-ops after another worker resolves the approval", async () => {
+    const dbStub = createDbStub(
+      [[createApproval("pending")], [createApproval("rejected")]],
+      [],
+    );
+
+    const svc = approvalService(dbStub.db as any);
+    const result = await svc.reject("approval-1", "board", "not now");
+
+    expect(result.applied).toBe(false);
+    expect(result.approval.status).toBe("rejected");
+    expect(mockAgentService.terminate).not.toHaveBeenCalled();
+  });
+
+  it("still performs side effects when the resolution update is newly applied", async () => {
+    const approved = createApproval("approved");
+    const dbStub = createDbStub([[createApproval("pending")]], [approved]);
+
+    const svc = approvalService(dbStub.db as any);
+    const result = await svc.approve("approval-1", "board", "ship it");
+
+    expect(result.applied).toBe(true);
+    expect(mockAgentService.activatePendingApproval).toHaveBeenCalledWith("agent-1");
+    expect(mockNotifyHireApproved).toHaveBeenCalledTimes(1);
+  });
+});

server/src/__tests__/attachment-types.test.ts

@@ -0,0 +1,97 @@
+import { describe, it, expect } from "vitest";
+import {
+  parseAllowedTypes,
+  matchesContentType,
+  DEFAULT_ALLOWED_TYPES,
+} from "../attachment-types.js";
+
+describe("parseAllowedTypes", () => {
+  it("returns default image types when input is undefined", () => {
+    expect(parseAllowedTypes(undefined)).toEqual([...DEFAULT_ALLOWED_TYPES]);
+  });
+
+  it("returns default image types when input is empty string", () => {
+    expect(parseAllowedTypes("")).toEqual([...DEFAULT_ALLOWED_TYPES]);
+  });
+
+  it("parses comma-separated types", () => {
+    expect(parseAllowedTypes("image/*,application/pdf")).toEqual([
+      "image/*",
+      "application/pdf",
+    ]);
+  });
+
+  it("trims whitespace", () => {
+    expect(parseAllowedTypes(" image/png , application/pdf ")).toEqual([
+      "image/png",
+      "application/pdf",
+    ]);
+  });
+
+  it("lowercases entries", () => {
+    expect(parseAllowedTypes("Application/PDF")).toEqual(["application/pdf"]);
+  });
+
+  it("filters empty segments", () => {
+    expect(parseAllowedTypes("image/png,,application/pdf,")).toEqual([
+      "image/png",
+      "application/pdf",
+    ]);
+  });
+});
+
+describe("matchesContentType", () => {
+  it("matches exact types", () => {
+    const patterns = ["application/pdf", "image/png"];
+    expect(matchesContentType("application/pdf", patterns)).toBe(true);
+    expect(matchesContentType("image/png", patterns)).toBe(true);
+    expect(matchesContentType("text/plain", patterns)).toBe(false);
+  });
+
+  it("matches /* wildcard patterns", () => {
+    const patterns = ["image/*"];
+    expect(matchesContentType("image/png", patterns)).toBe(true);
+    expect(matchesContentType("image/jpeg", patterns)).toBe(true);
+    expect(matchesContentType("image/svg+xml", patterns)).toBe(true);
+    expect(matchesContentType("application/pdf", patterns)).toBe(false);
+  });
+
+  it("matches .* wildcard patterns", () => {
+    const patterns = ["application/vnd.openxmlformats-officedocument.*"];
+    expect(
+      matchesContentType(
+        "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet",
+        patterns,
+      ),
+    ).toBe(true);
+    expect(
+      matchesContentType(
+        "application/vnd.openxmlformats-officedocument.wordprocessingml.document",
+        patterns,
+      ),
+    ).toBe(true);
+    expect(matchesContentType("application/pdf", patterns)).toBe(false);
+  });
+
+  it("is case-insensitive", () => {
+    const patterns = ["application/pdf"];
+    expect(matchesContentType("APPLICATION/PDF", patterns)).toBe(true);
+    expect(matchesContentType("Application/Pdf", patterns)).toBe(true);
+  });
+
+  it("combines exact and wildcard patterns", () => {
+    const patterns = ["image/*", "application/pdf", "text/*"];
+    expect(matchesContentType("image/webp", patterns)).toBe(true);
+    expect(matchesContentType("application/pdf", patterns)).toBe(true);
+    expect(matchesContentType("text/csv", patterns)).toBe(true);
+    expect(matchesContentType("application/zip", patterns)).toBe(false);
+  });
+
+  it("handles plain * as allow-all wildcard", () => {
+    const patterns = ["*"];
+    expect(matchesContentType("image/png", patterns)).toBe(true);
+    expect(matchesContentType("application/pdf", patterns)).toBe(true);
+    expect(matchesContentType("text/plain", patterns)).toBe(true);
+    expect(matchesContentType("application/zip", patterns)).toBe(true);
+  });
+});