fix: close remaining routine merge blockers
This commit is contained in:
@@ -1,4 +1,4 @@
|
|||||||
import { describe, expect, it } from "vitest";
|
import { describe, expect, it, vi } from "vitest";
|
||||||
import express from "express";
|
import express from "express";
|
||||||
import request from "supertest";
|
import request from "supertest";
|
||||||
import { boardMutationGuard } from "../middleware/board-mutation-guard.js";
|
import { boardMutationGuard } from "../middleware/board-mutation-guard.js";
|
||||||
@@ -61,8 +61,21 @@ describe("boardMutationGuard", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("does not block authenticated agent mutations", async () => {
|
it("does not block authenticated agent mutations", async () => {
|
||||||
const app = createApp("agent");
|
const middleware = boardMutationGuard();
|
||||||
const res = await request(app).post("/mutate").send({ ok: true });
|
const req = {
|
||||||
expect(res.status).toBe(204);
|
method: "POST",
|
||||||
|
actor: { type: "agent", agentId: "agent-1" },
|
||||||
|
header: () => undefined,
|
||||||
|
} as any;
|
||||||
|
const res = {
|
||||||
|
status: vi.fn().mockReturnThis(),
|
||||||
|
json: vi.fn(),
|
||||||
|
} as any;
|
||||||
|
const next = vi.fn();
|
||||||
|
|
||||||
|
middleware(req, res, next);
|
||||||
|
|
||||||
|
expect(next).toHaveBeenCalledOnce();
|
||||||
|
expect(res.status).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,8 +1,6 @@
|
|||||||
import express from "express";
|
import express from "express";
|
||||||
import request from "supertest";
|
import request from "supertest";
|
||||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
import { companyRoutes } from "../routes/companies.js";
|
|
||||||
import { errorHandler } from "../middleware/index.js";
|
|
||||||
|
|
||||||
const mockCompanyService = vi.hoisted(() => ({
|
const mockCompanyService = vi.hoisted(() => ({
|
||||||
list: vi.fn(),
|
list: vi.fn(),
|
||||||
@@ -44,7 +42,9 @@ vi.mock("../services/index.js", () => ({
|
|||||||
logActivity: mockLogActivity,
|
logActivity: mockLogActivity,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
function createApp(actor: Record<string, unknown>) {
|
async function createApp(actor: Record<string, unknown>) {
|
||||||
|
const { companyRoutes } = await import("../routes/companies.js");
|
||||||
|
const { errorHandler } = await import("../middleware/index.js");
|
||||||
const app = express();
|
const app = express();
|
||||||
app.use(express.json());
|
app.use(express.json());
|
||||||
app.use((req, _res, next) => {
|
app.use((req, _res, next) => {
|
||||||
@@ -58,6 +58,7 @@ function createApp(actor: Record<string, unknown>) {
|
|||||||
|
|
||||||
describe("company portability routes", () => {
|
describe("company portability routes", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
|
vi.resetModules();
|
||||||
mockAgentService.getById.mockReset();
|
mockAgentService.getById.mockReset();
|
||||||
mockCompanyPortabilityService.exportBundle.mockReset();
|
mockCompanyPortabilityService.exportBundle.mockReset();
|
||||||
mockCompanyPortabilityService.previewExport.mockReset();
|
mockCompanyPortabilityService.previewExport.mockReset();
|
||||||
@@ -72,7 +73,7 @@ describe("company portability routes", () => {
|
|||||||
companyId: "11111111-1111-4111-8111-111111111111",
|
companyId: "11111111-1111-4111-8111-111111111111",
|
||||||
role: "engineer",
|
role: "engineer",
|
||||||
});
|
});
|
||||||
const app = createApp({
|
const app = await createApp({
|
||||||
type: "agent",
|
type: "agent",
|
||||||
agentId: "agent-1",
|
agentId: "agent-1",
|
||||||
companyId: "11111111-1111-4111-8111-111111111111",
|
companyId: "11111111-1111-4111-8111-111111111111",
|
||||||
@@ -104,7 +105,7 @@ describe("company portability routes", () => {
|
|||||||
warnings: [],
|
warnings: [],
|
||||||
paperclipExtensionPath: ".paperclip.yaml",
|
paperclipExtensionPath: ".paperclip.yaml",
|
||||||
});
|
});
|
||||||
const app = createApp({
|
const app = await createApp({
|
||||||
type: "agent",
|
type: "agent",
|
||||||
agentId: "agent-1",
|
agentId: "agent-1",
|
||||||
companyId: "11111111-1111-4111-8111-111111111111",
|
companyId: "11111111-1111-4111-8111-111111111111",
|
||||||
@@ -128,7 +129,7 @@ describe("company portability routes", () => {
|
|||||||
companyId: "11111111-1111-4111-8111-111111111111",
|
companyId: "11111111-1111-4111-8111-111111111111",
|
||||||
role: "ceo",
|
role: "ceo",
|
||||||
});
|
});
|
||||||
const app = createApp({
|
const app = await createApp({
|
||||||
type: "agent",
|
type: "agent",
|
||||||
agentId: "agent-1",
|
agentId: "agent-1",
|
||||||
companyId: "11111111-1111-4111-8111-111111111111",
|
companyId: "11111111-1111-4111-8111-111111111111",
|
||||||
@@ -151,7 +152,7 @@ describe("company portability routes", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("keeps global import preview routes board-only", async () => {
|
it("keeps global import preview routes board-only", async () => {
|
||||||
const app = createApp({
|
const app = await createApp({
|
||||||
type: "agent",
|
type: "agent",
|
||||||
agentId: "agent-1",
|
agentId: "agent-1",
|
||||||
companyId: "11111111-1111-4111-8111-111111111111",
|
companyId: "11111111-1111-4111-8111-111111111111",
|
||||||
|
|||||||
@@ -32,6 +32,34 @@ const routine = {
|
|||||||
createdAt: new Date("2026-03-20T00:00:00.000Z"),
|
createdAt: new Date("2026-03-20T00:00:00.000Z"),
|
||||||
updatedAt: new Date("2026-03-20T00:00:00.000Z"),
|
updatedAt: new Date("2026-03-20T00:00:00.000Z"),
|
||||||
};
|
};
|
||||||
|
const pausedRoutine = {
|
||||||
|
...routine,
|
||||||
|
status: "paused",
|
||||||
|
};
|
||||||
|
const trigger = {
|
||||||
|
id: "66666666-6666-4666-8666-666666666666",
|
||||||
|
companyId,
|
||||||
|
routineId,
|
||||||
|
kind: "schedule",
|
||||||
|
label: "weekday",
|
||||||
|
enabled: false,
|
||||||
|
cronExpression: "0 10 * * 1-5",
|
||||||
|
timezone: "UTC",
|
||||||
|
nextRunAt: null,
|
||||||
|
lastFiredAt: null,
|
||||||
|
publicId: null,
|
||||||
|
secretId: null,
|
||||||
|
signingMode: null,
|
||||||
|
replayWindowSec: null,
|
||||||
|
lastRotatedAt: null,
|
||||||
|
lastResult: null,
|
||||||
|
createdByAgentId: null,
|
||||||
|
createdByUserId: null,
|
||||||
|
updatedByAgentId: null,
|
||||||
|
updatedByUserId: null,
|
||||||
|
createdAt: new Date("2026-03-20T00:00:00.000Z"),
|
||||||
|
updatedAt: new Date("2026-03-20T00:00:00.000Z"),
|
||||||
|
};
|
||||||
|
|
||||||
const mockRoutineService = vi.hoisted(() => ({
|
const mockRoutineService = vi.hoisted(() => ({
|
||||||
list: vi.fn(),
|
list: vi.fn(),
|
||||||
@@ -78,7 +106,13 @@ describe("routine routes", () => {
|
|||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
mockRoutineService.create.mockResolvedValue(routine);
|
mockRoutineService.create.mockResolvedValue(routine);
|
||||||
mockRoutineService.get.mockResolvedValue(routine);
|
mockRoutineService.get.mockResolvedValue(routine);
|
||||||
|
mockRoutineService.getTrigger.mockResolvedValue(trigger);
|
||||||
mockRoutineService.update.mockResolvedValue({ ...routine, assigneeAgentId: otherAgentId });
|
mockRoutineService.update.mockResolvedValue({ ...routine, assigneeAgentId: otherAgentId });
|
||||||
|
mockRoutineService.runRoutine.mockResolvedValue({
|
||||||
|
id: "run-1",
|
||||||
|
source: "manual",
|
||||||
|
status: "issue_created",
|
||||||
|
});
|
||||||
mockAccessService.canUser.mockResolvedValue(false);
|
mockAccessService.canUser.mockResolvedValue(false);
|
||||||
mockLogActivity.mockResolvedValue(undefined);
|
mockLogActivity.mockResolvedValue(undefined);
|
||||||
});
|
});
|
||||||
@@ -125,6 +159,87 @@ describe("routine routes", () => {
|
|||||||
expect(mockRoutineService.update).not.toHaveBeenCalled();
|
expect(mockRoutineService.update).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("requires tasks:assign permission to reactivate a routine", async () => {
|
||||||
|
mockRoutineService.get.mockResolvedValue(pausedRoutine);
|
||||||
|
const app = createApp({
|
||||||
|
type: "board",
|
||||||
|
userId: "board-user",
|
||||||
|
source: "session",
|
||||||
|
isInstanceAdmin: false,
|
||||||
|
companyIds: [companyId],
|
||||||
|
});
|
||||||
|
|
||||||
|
const res = await request(app)
|
||||||
|
.patch(`/api/routines/${routineId}`)
|
||||||
|
.send({
|
||||||
|
status: "active",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.status).toBe(403);
|
||||||
|
expect(res.body.error).toContain("tasks:assign");
|
||||||
|
expect(mockRoutineService.update).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("requires tasks:assign permission to create a trigger", async () => {
|
||||||
|
const app = createApp({
|
||||||
|
type: "board",
|
||||||
|
userId: "board-user",
|
||||||
|
source: "session",
|
||||||
|
isInstanceAdmin: false,
|
||||||
|
companyIds: [companyId],
|
||||||
|
});
|
||||||
|
|
||||||
|
const res = await request(app)
|
||||||
|
.post(`/api/routines/${routineId}/triggers`)
|
||||||
|
.send({
|
||||||
|
kind: "schedule",
|
||||||
|
cronExpression: "0 10 * * *",
|
||||||
|
timezone: "UTC",
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.status).toBe(403);
|
||||||
|
expect(res.body.error).toContain("tasks:assign");
|
||||||
|
expect(mockRoutineService.createTrigger).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("requires tasks:assign permission to update a trigger", async () => {
|
||||||
|
const app = createApp({
|
||||||
|
type: "board",
|
||||||
|
userId: "board-user",
|
||||||
|
source: "session",
|
||||||
|
isInstanceAdmin: false,
|
||||||
|
companyIds: [companyId],
|
||||||
|
});
|
||||||
|
|
||||||
|
const res = await request(app)
|
||||||
|
.patch(`/api/routine-triggers/${trigger.id}`)
|
||||||
|
.send({
|
||||||
|
enabled: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.status).toBe(403);
|
||||||
|
expect(res.body.error).toContain("tasks:assign");
|
||||||
|
expect(mockRoutineService.updateTrigger).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("requires tasks:assign permission to manually run a routine", async () => {
|
||||||
|
const app = createApp({
|
||||||
|
type: "board",
|
||||||
|
userId: "board-user",
|
||||||
|
source: "session",
|
||||||
|
isInstanceAdmin: false,
|
||||||
|
companyIds: [companyId],
|
||||||
|
});
|
||||||
|
|
||||||
|
const res = await request(app)
|
||||||
|
.post(`/api/routines/${routineId}/run`)
|
||||||
|
.send({});
|
||||||
|
|
||||||
|
expect(res.status).toBe(403);
|
||||||
|
expect(res.body.error).toContain("tasks:assign");
|
||||||
|
expect(mockRoutineService.runRoutine).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
it("allows routine creation when the board user has tasks:assign", async () => {
|
it("allows routine creation when the board user has tasks:assign", async () => {
|
||||||
mockAccessService.canUser.mockResolvedValue(true);
|
mockAccessService.canUser.mockResolvedValue(true);
|
||||||
const app = createApp({
|
const app = createApp({
|
||||||
|
|||||||
@@ -430,6 +430,27 @@ describe("routine service live-execution coalescing", () => {
|
|||||||
expect(routineIssues).toHaveLength(1);
|
expect(routineIssues).toHaveLength(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("fails the run and cleans up the execution issue when wakeup queueing fails", async () => {
|
||||||
|
const { routine, svc } = await seedFixture({
|
||||||
|
wakeup: async () => {
|
||||||
|
throw new Error("queue unavailable");
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const run = await svc.runRoutine(routine.id, { source: "manual" });
|
||||||
|
|
||||||
|
expect(run.status).toBe("failed");
|
||||||
|
expect(run.failureReason).toContain("queue unavailable");
|
||||||
|
expect(run.linkedIssueId).toBeNull();
|
||||||
|
|
||||||
|
const routineIssues = await db
|
||||||
|
.select({ id: issues.id })
|
||||||
|
.from(issues)
|
||||||
|
.where(eq(issues.originId, routine.id));
|
||||||
|
|
||||||
|
expect(routineIssues).toHaveLength(0);
|
||||||
|
});
|
||||||
|
|
||||||
it("accepts standard second-precision webhook timestamps for HMAC triggers", async () => {
|
it("accepts standard second-precision webhook timestamps for HMAC triggers", async () => {
|
||||||
const { routine, svc } = await seedFixture();
|
const { routine, svc } = await seedFixture();
|
||||||
const { trigger, secretMaterial } = await svc.createTrigger(
|
const { trigger, secretMaterial } = await svc.createTrigger(
|
||||||
|
|||||||
@@ -101,6 +101,13 @@ export function routineRoutes(db: Db) {
|
|||||||
if (assigneeWillChange) {
|
if (assigneeWillChange) {
|
||||||
await assertBoardCanAssignTasks(req, routine.companyId);
|
await assertBoardCanAssignTasks(req, routine.companyId);
|
||||||
}
|
}
|
||||||
|
const statusWillActivate =
|
||||||
|
req.body.status !== undefined &&
|
||||||
|
req.body.status === "active" &&
|
||||||
|
routine.status !== "active";
|
||||||
|
if (statusWillActivate) {
|
||||||
|
await assertBoardCanAssignTasks(req, routine.companyId);
|
||||||
|
}
|
||||||
if (req.actor.type === "agent" && req.body.assigneeAgentId && req.body.assigneeAgentId !== req.actor.agentId) {
|
if (req.actor.type === "agent" && req.body.assigneeAgentId && req.body.assigneeAgentId !== req.actor.agentId) {
|
||||||
throw forbidden("Agents can only assign routines to themselves");
|
throw forbidden("Agents can only assign routines to themselves");
|
||||||
}
|
}
|
||||||
@@ -141,6 +148,7 @@ export function routineRoutes(db: Db) {
|
|||||||
res.status(404).json({ error: "Routine not found" });
|
res.status(404).json({ error: "Routine not found" });
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
await assertBoardCanAssignTasks(req, routine.companyId);
|
||||||
const created = await svc.createTrigger(routine.id, req.body, {
|
const created = await svc.createTrigger(routine.id, req.body, {
|
||||||
agentId: req.actor.type === "agent" ? req.actor.agentId : null,
|
agentId: req.actor.type === "agent" ? req.actor.agentId : null,
|
||||||
userId: req.actor.type === "board" ? req.actor.userId ?? "board" : null,
|
userId: req.actor.type === "board" ? req.actor.userId ?? "board" : null,
|
||||||
@@ -171,6 +179,7 @@ export function routineRoutes(db: Db) {
|
|||||||
res.status(404).json({ error: "Routine not found" });
|
res.status(404).json({ error: "Routine not found" });
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
await assertBoardCanAssignTasks(req, routine.companyId);
|
||||||
const updated = await svc.updateTrigger(trigger.id, req.body, {
|
const updated = await svc.updateTrigger(trigger.id, req.body, {
|
||||||
agentId: req.actor.type === "agent" ? req.actor.agentId : null,
|
agentId: req.actor.type === "agent" ? req.actor.agentId : null,
|
||||||
userId: req.actor.type === "board" ? req.actor.userId ?? "board" : null,
|
userId: req.actor.type === "board" ? req.actor.userId ?? "board" : null,
|
||||||
@@ -257,6 +266,7 @@ export function routineRoutes(db: Db) {
|
|||||||
res.status(404).json({ error: "Routine not found" });
|
res.status(404).json({ error: "Routine not found" });
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
await assertBoardCanAssignTasks(req, routine.companyId);
|
||||||
const run = await svc.runRoutine(routine.id, req.body);
|
const run = await svc.runRoutine(routine.id, req.body);
|
||||||
const actor = getActorInfo(req);
|
const actor = getActorInfo(req);
|
||||||
await logActivity(db, {
|
await logActivity(db, {
|
||||||
|
|||||||
@@ -26,6 +26,7 @@ export function queueIssueAssignmentWakeup(input: {
|
|||||||
contextSource: string;
|
contextSource: string;
|
||||||
requestedByActorType?: "user" | "agent" | "system";
|
requestedByActorType?: "user" | "agent" | "system";
|
||||||
requestedByActorId?: string | null;
|
requestedByActorId?: string | null;
|
||||||
|
rethrowOnError?: boolean;
|
||||||
}) {
|
}) {
|
||||||
if (!input.issue.assigneeAgentId || input.issue.status === "backlog") return;
|
if (!input.issue.assigneeAgentId || input.issue.status === "backlog") return;
|
||||||
|
|
||||||
@@ -39,5 +40,9 @@ export function queueIssueAssignmentWakeup(input: {
|
|||||||
requestedByActorId: input.requestedByActorId ?? null,
|
requestedByActorId: input.requestedByActorId ?? null,
|
||||||
contextSnapshot: { issueId: input.issue.id, source: input.contextSource },
|
contextSnapshot: { issueId: input.issue.id, source: input.contextSource },
|
||||||
})
|
})
|
||||||
.catch((err) => logger.warn({ err, issueId: input.issue.id }, "failed to wake assignee on issue assignment"));
|
.catch((err) => {
|
||||||
|
logger.warn({ err, issueId: input.issue.id }, "failed to wake assignee on issue assignment");
|
||||||
|
if (input.rethrowOnError) throw err;
|
||||||
|
return null;
|
||||||
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -561,6 +561,7 @@ export function routineService(db: Db, deps: { heartbeat?: IssueAssignmentWakeup
|
|||||||
? nextCronTickInTimeZone(input.trigger.cronExpression, input.trigger.timezone, triggeredAt)
|
? nextCronTickInTimeZone(input.trigger.cronExpression, input.trigger.timezone, triggeredAt)
|
||||||
: undefined;
|
: undefined;
|
||||||
|
|
||||||
|
let createdIssue: Awaited<ReturnType<typeof issueSvc.create>> | null = null;
|
||||||
try {
|
try {
|
||||||
const activeIssue = await findLiveExecutionIssue(input.routine, txDb);
|
const activeIssue = await findLiveExecutionIssue(input.routine, txDb);
|
||||||
if (activeIssue && input.routine.concurrencyPolicy !== "always_enqueue") {
|
if (activeIssue && input.routine.concurrencyPolicy !== "always_enqueue") {
|
||||||
@@ -582,7 +583,6 @@ export function routineService(db: Db, deps: { heartbeat?: IssueAssignmentWakeup
|
|||||||
return updated ?? createdRun;
|
return updated ?? createdRun;
|
||||||
}
|
}
|
||||||
|
|
||||||
let createdIssue;
|
|
||||||
try {
|
try {
|
||||||
createdIssue = await issueSvc.create(input.routine.companyId, {
|
createdIssue = await issueSvc.create(input.routine.companyId, {
|
||||||
projectId: input.routine.projectId,
|
projectId: input.routine.projectId,
|
||||||
@@ -637,6 +637,7 @@ export function routineService(db: Db, deps: { heartbeat?: IssueAssignmentWakeup
|
|||||||
mutation: "create",
|
mutation: "create",
|
||||||
contextSource: "routine.dispatch",
|
contextSource: "routine.dispatch",
|
||||||
requestedByActorType: input.source === "schedule" ? "system" : undefined,
|
requestedByActorType: input.source === "schedule" ? "system" : undefined,
|
||||||
|
rethrowOnError: true,
|
||||||
});
|
});
|
||||||
const updated = await finalizeRun(createdRun.id, {
|
const updated = await finalizeRun(createdRun.id, {
|
||||||
status: "issue_created",
|
status: "issue_created",
|
||||||
@@ -652,6 +653,9 @@ export function routineService(db: Db, deps: { heartbeat?: IssueAssignmentWakeup
|
|||||||
}, txDb);
|
}, txDb);
|
||||||
return updated ?? createdRun;
|
return updated ?? createdRun;
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
if (createdIssue) {
|
||||||
|
await txDb.delete(issues).where(eq(issues.id, createdIssue.id));
|
||||||
|
}
|
||||||
const failureReason = error instanceof Error ? error.message : String(error);
|
const failureReason = error instanceof Error ? error.message : String(error);
|
||||||
const failed = await finalizeRun(createdRun.id, {
|
const failed = await finalizeRun(createdRun.id, {
|
||||||
status: "failed",
|
status: "failed",
|
||||||
|
|||||||
Reference in New Issue
Block a user