feat(FN-1524): add POST /routines/:id/trigger endpoint
- Add POST /routines/:id/trigger as canonical endpoint for manual routine triggers - Keep POST /routines/:id/run as backward-compatible alias with identical behavior - Remove duplicate recordRun calls from routine route handlers (persistence handled by RoutineRunner.completeRoutineExecution) - Update webhook auth to return 401 instead of 403 for missing/invalid signature headers - Add comprehensive tests for /trigger endpoint and double-persist fix
This commit is contained in:
@@ -1783,7 +1783,7 @@ export async function deleteRoutine(id: string): Promise<void> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function runRoutine(id: string): Promise<RoutineRunResponse> {
|
export function runRoutine(id: string): Promise<RoutineRunResponse> {
|
||||||
return api<RoutineRunResponse>(`/routines/${id}/run`, {
|
return api<RoutineRunResponse>(`/routines/${id}/trigger`, {
|
||||||
method: "POST",
|
method: "POST",
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -9133,21 +9133,17 @@ describe("Routine routes", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("POST /routines/:id/run", () => {
|
describe("POST /routines/:id/run", () => {
|
||||||
it("runs a routine and records the result", async () => {
|
it("runs a routine via RoutineRunner.triggerManual (double-persist fix)", async () => {
|
||||||
const mockStore = createMockRoutineStore();
|
const mockStore = createMockRoutineStore();
|
||||||
const { app } = buildRoutineApp(mockStore);
|
const { app, routineRunner } = buildRoutineApp(mockStore);
|
||||||
const res = await REQUEST(app, "POST", "/api/routines/routine-001/run");
|
const res = await REQUEST(app, "POST", "/api/routines/routine-001/run");
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
expect(res.body.result).toBeDefined();
|
expect(res.body.result).toBeDefined();
|
||||||
expect(res.body.result.triggerType).toBe("cron");
|
expect(res.body.result.triggerType).toBe("cron");
|
||||||
expect(mockStore.recordRun).toHaveBeenCalledWith(
|
// Verify triggerManual was called (persistence handled by RoutineRunner)
|
||||||
"routine-001",
|
expect(routineRunner.triggerManual).toHaveBeenCalledWith("routine-001");
|
||||||
expect.objectContaining({
|
// Verify recordRun was NOT called (double-persist fix)
|
||||||
success: true,
|
expect(mockStore.recordRun).not.toHaveBeenCalled();
|
||||||
startedAt: expect.any(String),
|
|
||||||
completedAt: expect.any(String),
|
|
||||||
}),
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns 404 for missing routine", async () => {
|
it("returns 404 for missing routine", async () => {
|
||||||
@@ -9166,6 +9162,86 @@ describe("Routine routes", () => {
|
|||||||
const res = await REQUEST(app, "POST", "/api/routines/routine-001/run");
|
const res = await REQUEST(app, "POST", "/api/routines/routine-001/run");
|
||||||
expect(res.status).toBe(503);
|
expect(res.status).toBe(503);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("returns 400 when routine is disabled", async () => {
|
||||||
|
const mockStore = createMockRoutineStore();
|
||||||
|
mockStore.getRoutine.mockResolvedValue({
|
||||||
|
...FAKE_ROUTINE,
|
||||||
|
enabled: false,
|
||||||
|
});
|
||||||
|
const { app } = buildRoutineApp(mockStore);
|
||||||
|
const res = await REQUEST(app, "POST", "/api/routines/routine-001/run");
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error).toContain("disabled");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 503 when routineRunner not available", async () => {
|
||||||
|
const store = createMockStore();
|
||||||
|
const routineStore = createMockRoutineStore();
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
app.use("/api", createApiRoutes(store, { routineStore: routineStore as any }));
|
||||||
|
const res = await REQUEST(app, "POST", "/api/routines/routine-001/run");
|
||||||
|
expect(res.status).toBe(503);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("POST /routines/:id/trigger", () => {
|
||||||
|
it("returns 200 with routine and result on success", async () => {
|
||||||
|
const mockStore = createMockRoutineStore();
|
||||||
|
const { app, routineRunner } = buildRoutineApp(mockStore);
|
||||||
|
const res = await REQUEST(app, "POST", "/api/routines/routine-001/trigger");
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(res.body.routine).toBeDefined();
|
||||||
|
expect(res.body.result).toBeDefined();
|
||||||
|
expect(routineRunner.triggerManual).toHaveBeenCalledWith("routine-001");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 404 for missing routine (ENOENT)", async () => {
|
||||||
|
const mockStore = createMockRoutineStore();
|
||||||
|
mockStore.getRoutine.mockRejectedValue(Object.assign(new Error("not found"), { code: "ENOENT" }));
|
||||||
|
const { app } = buildRoutineApp(mockStore);
|
||||||
|
const res = await REQUEST(app, "POST", "/api/routines/missing/trigger");
|
||||||
|
expect(res.status).toBe(404);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 400 for disabled routine", async () => {
|
||||||
|
const mockStore = createMockRoutineStore();
|
||||||
|
mockStore.getRoutine.mockResolvedValue({
|
||||||
|
...FAKE_ROUTINE,
|
||||||
|
enabled: false,
|
||||||
|
});
|
||||||
|
const { app } = buildRoutineApp(mockStore);
|
||||||
|
const res = await REQUEST(app, "POST", "/api/routines/routine-001/trigger");
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error).toContain("disabled");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 503 when routineStore not available", async () => {
|
||||||
|
const store = createMockStore();
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
app.use("/api", createApiRoutes(store));
|
||||||
|
const res = await REQUEST(app, "POST", "/api/routines/routine-001/trigger");
|
||||||
|
expect(res.status).toBe(503);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns 503 when routineRunner not available", async () => {
|
||||||
|
const store = createMockStore();
|
||||||
|
const routineStore = createMockRoutineStore();
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
app.use("/api", createApiRoutes(store, { routineStore: routineStore as any }));
|
||||||
|
const res = await REQUEST(app, "POST", "/api/routines/routine-001/trigger");
|
||||||
|
expect(res.status).toBe(503);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does NOT call recordRun (double-persist fix)", async () => {
|
||||||
|
const mockStore = createMockRoutineStore();
|
||||||
|
const { app } = buildRoutineApp(mockStore);
|
||||||
|
await REQUEST(app, "POST", "/api/routines/routine-001/trigger");
|
||||||
|
expect(mockStore.recordRun).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("GET /routines/:id/runs", () => {
|
describe("GET /routines/:id/runs", () => {
|
||||||
@@ -9218,12 +9294,14 @@ describe("Routine routes", () => {
|
|||||||
...FAKE_ROUTINE,
|
...FAKE_ROUTINE,
|
||||||
trigger: { type: "webhook" as const, webhookPath: "/trigger/test" },
|
trigger: { type: "webhook" as const, webhookPath: "/trigger/test" },
|
||||||
});
|
});
|
||||||
const { app } = buildRoutineApp(mockStore);
|
const { app, routineRunner } = buildRoutineApp(mockStore);
|
||||||
const res = await REQUEST(app, "POST", "/api/routines/routine-001/webhook", JSON.stringify({}), { "Content-Type": "application/json" });
|
const res = await REQUEST(app, "POST", "/api/routines/routine-001/webhook", JSON.stringify({}), { "Content-Type": "application/json" });
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
expect(res.body.result).toBeDefined();
|
expect(res.body.result).toBeDefined();
|
||||||
expect(res.body.result.triggerType).toBe("webhook");
|
expect(res.body.result.triggerType).toBe("webhook");
|
||||||
expect(mockStore.recordRun).toHaveBeenCalled();
|
expect(routineRunner.triggerWebhook).toHaveBeenCalled();
|
||||||
|
// Verify recordRun was NOT called (double-persist fix)
|
||||||
|
expect(mockStore.recordRun).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("returns 400 when routine is not a webhook type", async () => {
|
it("returns 400 when routine is not a webhook type", async () => {
|
||||||
@@ -9278,8 +9356,37 @@ describe("Routine routes", () => {
|
|||||||
const res = await REQUEST(app, "POST", "/api/routines/routine-001/webhook", JSON.stringify({}), { "Content-Type": "application/json" });
|
const res = await REQUEST(app, "POST", "/api/routines/routine-001/webhook", JSON.stringify({}), { "Content-Type": "application/json" });
|
||||||
expect(res.status).toBe(200);
|
expect(res.status).toBe(200);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("returns 401 when secret is configured but signature header is missing (was 403)", async () => {
|
||||||
|
const mockStore = createMockRoutineStore();
|
||||||
|
mockStore.getRoutine.mockResolvedValue({
|
||||||
|
...FAKE_ROUTINE,
|
||||||
|
trigger: { type: "webhook" as const, webhookPath: "/trigger/test", secret: "test-secret" },
|
||||||
|
});
|
||||||
|
// Set up rawBody via middleware so the route doesn't return 400 for missing rawBody
|
||||||
|
const store = createMockStore();
|
||||||
|
const routineStore = mockStore;
|
||||||
|
const routineRunner = createMockRoutineRunner();
|
||||||
|
const testApp = express();
|
||||||
|
testApp.use(express.json());
|
||||||
|
testApp.use((req, _res, next) => {
|
||||||
|
// Simulate rawBody being set by middleware
|
||||||
|
(req as any).rawBody = Buffer.from("{}");
|
||||||
|
next();
|
||||||
|
});
|
||||||
|
testApp.use("/api", createApiRoutes(store, { routineStore: routineStore as any, routineRunner }));
|
||||||
|
const res = await REQUEST(testApp, "POST", "/api/routines/routine-001/webhook", JSON.stringify({}), { "Content-Type": "application/json" });
|
||||||
|
expect(res.status).toBe(401);
|
||||||
|
expect(res.body.error).toContain("Missing signature header");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Note: The "invalid signature" webhook auth test is skipped because:
|
||||||
|
// - vi.doMock persists across test files in the same worker
|
||||||
|
// - The missing signature header test already verifies 401 behavior
|
||||||
|
// - The Webhook HMAC verification tests verify verifyWebhookSignature works correctly
|
||||||
|
// - Route-level 401 status code change is verified by the missing signature test
|
||||||
|
|
||||||
describe("Webhook HMAC verification", () => {
|
describe("Webhook HMAC verification", () => {
|
||||||
// These tests verify the verifyWebhookSignature function directly
|
// These tests verify the verifyWebhookSignature function directly
|
||||||
// since testing through HTTP requires complex middleware setup
|
// since testing through HTTP requires complex middleware setup
|
||||||
|
|||||||
@@ -8680,7 +8680,7 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
// POST /routines/:id/run — manual trigger (record a manual run)
|
// POST /routines/:id/run — manual trigger (backward-compatible alias for /trigger)
|
||||||
router.post("/routines/:id/run", async (req: Request, res: Response) => {
|
router.post("/routines/:id/run", async (req: Request, res: Response) => {
|
||||||
if (!routineStore) {
|
if (!routineStore) {
|
||||||
throw new ApiError(503, "Routine store not available");
|
throw new ApiError(503, "Routine store not available");
|
||||||
@@ -8697,9 +8697,41 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
|||||||
throw badRequest("Routine is disabled");
|
throw badRequest("Routine is disabled");
|
||||||
}
|
}
|
||||||
|
|
||||||
// Execute via RoutineRunner
|
// Execute via RoutineRunner (persistence handled by RoutineRunner.completeRoutineExecution)
|
||||||
|
const result = await routineRunner.triggerManual(id);
|
||||||
|
const updated = await routineStore.getRoutine(id);
|
||||||
|
res.json({ routine: updated, result });
|
||||||
|
} catch (err: any) {
|
||||||
|
if (err instanceof ApiError) {
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
if (err.code === "ENOENT") {
|
||||||
|
throw notFound("Routine not found");
|
||||||
|
}
|
||||||
|
rethrowAsApiError(err);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
// POST /routines/:id/trigger — canonical manual trigger (uses RoutineRunner)
|
||||||
|
// POST /routines/:id/run is a backward-compatible alias with identical behavior
|
||||||
|
router.post("/routines/:id/trigger", async (req: Request, res: Response) => {
|
||||||
|
if (!routineStore) {
|
||||||
|
throw new ApiError(503, "Routine store not available");
|
||||||
|
}
|
||||||
|
if (!routineRunner) {
|
||||||
|
throw new ApiError(503, "Routine execution not available");
|
||||||
|
}
|
||||||
|
try {
|
||||||
|
const id = Array.isArray(req.params.id) ? req.params.id[0] : req.params.id;
|
||||||
|
const routine = await routineStore.getRoutine(id);
|
||||||
|
|
||||||
|
// Validate routine is enabled
|
||||||
|
if (!routine.enabled) {
|
||||||
|
throw badRequest("Routine is disabled");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Execute via RoutineRunner (persistence handled by RoutineRunner.completeRoutineExecution)
|
||||||
const result = await routineRunner.triggerManual(id);
|
const result = await routineRunner.triggerManual(id);
|
||||||
await routineStore.recordRun(id, result);
|
|
||||||
const updated = await routineStore.getRoutine(id);
|
const updated = await routineStore.getRoutine(id);
|
||||||
res.json({ routine: updated, result });
|
res.json({ routine: updated, result });
|
||||||
} catch (err: any) {
|
} catch (err: any) {
|
||||||
@@ -8759,24 +8791,23 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
|||||||
const rawBody = (req as any).rawBody as Buffer | undefined;
|
const rawBody = (req as any).rawBody as Buffer | undefined;
|
||||||
const signatureHeader = req.headers["x-hub-signature-256"] as string | undefined;
|
const signatureHeader = req.headers["x-hub-signature-256"] as string | undefined;
|
||||||
|
|
||||||
// If webhook secret is configured, verify the signature
|
// If webhook secret is configured, verify the signature (auth failures return 401)
|
||||||
if (routine.trigger.secret) {
|
if (routine.trigger.secret) {
|
||||||
if (!rawBody) {
|
if (!rawBody) {
|
||||||
throw badRequest("Raw body not available for signature verification");
|
throw badRequest("Raw body not available for signature verification");
|
||||||
}
|
}
|
||||||
if (!signatureHeader) {
|
if (!signatureHeader) {
|
||||||
throw new ApiError(403, "Missing signature header");
|
throw new ApiError(401, "Missing signature header");
|
||||||
}
|
}
|
||||||
const verification = verifyWebhookSignature(rawBody, signatureHeader, routine.trigger.secret);
|
const verification = verifyWebhookSignature(rawBody, signatureHeader, routine.trigger.secret);
|
||||||
if (!verification.valid) {
|
if (!verification.valid) {
|
||||||
throw new ApiError(403, verification.error ?? "Invalid signature");
|
throw new ApiError(401, verification.error ?? "Invalid signature");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Execute via RoutineRunner
|
// Execute via RoutineRunner (persistence handled by RoutineRunner.completeRoutineExecution)
|
||||||
const payload = req.body;
|
const payload = req.body;
|
||||||
const result = await routineRunner.triggerWebhook(id, payload, signatureHeader);
|
const result = await routineRunner.triggerWebhook(id, payload, signatureHeader);
|
||||||
await routineStore.recordRun(id, result);
|
|
||||||
const updated = await routineStore.getRoutine(id);
|
const updated = await routineStore.getRoutine(id);
|
||||||
res.json({ routine: updated, result });
|
res.json({ routine: updated, result });
|
||||||
} catch (err: any) {
|
} catch (err: any) {
|
||||||
|
|||||||
Reference in New Issue
Block a user