fix: address greptile routine review
This commit is contained in:
parent
99eb317600
commit
9093cfbe4f
6 changed files with 157 additions and 19 deletions
|
|
@ -1,4 +1,4 @@
|
||||||
import { randomUUID } from "node:crypto";
|
import { createHmac, randomUUID } from "node:crypto";
|
||||||
import fs from "node:fs";
|
import fs from "node:fs";
|
||||||
import net from "node:net";
|
import net from "node:net";
|
||||||
import os from "node:os";
|
import os from "node:os";
|
||||||
|
|
@ -6,9 +6,12 @@ import path from "node:path";
|
||||||
import { eq } from "drizzle-orm";
|
import { eq } from "drizzle-orm";
|
||||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||||
import {
|
import {
|
||||||
|
activityLog,
|
||||||
agents,
|
agents,
|
||||||
applyPendingMigrations,
|
applyPendingMigrations,
|
||||||
companies,
|
companies,
|
||||||
|
companySecrets,
|
||||||
|
companySecretVersions,
|
||||||
createDb,
|
createDb,
|
||||||
ensurePostgresDatabase,
|
ensurePostgresDatabase,
|
||||||
heartbeatRuns,
|
heartbeatRuns,
|
||||||
|
|
@ -16,6 +19,7 @@ import {
|
||||||
projects,
|
projects,
|
||||||
routineRuns,
|
routineRuns,
|
||||||
routines,
|
routines,
|
||||||
|
routineTriggers,
|
||||||
} from "@paperclipai/db";
|
} from "@paperclipai/db";
|
||||||
import { issueService } from "../services/issues.ts";
|
import { issueService } from "../services/issues.ts";
|
||||||
import { routineService } from "../services/routines.ts";
|
import { routineService } from "../services/routines.ts";
|
||||||
|
|
@ -99,8 +103,12 @@ describe("routine service live-execution coalescing", () => {
|
||||||
}, 20_000);
|
}, 20_000);
|
||||||
|
|
||||||
afterEach(async () => {
|
afterEach(async () => {
|
||||||
|
await db.delete(activityLog);
|
||||||
await db.delete(routineRuns);
|
await db.delete(routineRuns);
|
||||||
|
await db.delete(routineTriggers);
|
||||||
await db.delete(routines);
|
await db.delete(routines);
|
||||||
|
await db.delete(companySecretVersions);
|
||||||
|
await db.delete(companySecrets);
|
||||||
await db.delete(heartbeatRuns);
|
await db.delete(heartbeatRuns);
|
||||||
await db.delete(issues);
|
await db.delete(issues);
|
||||||
await db.delete(projects);
|
await db.delete(projects);
|
||||||
|
|
@ -421,4 +429,39 @@ describe("routine service live-execution coalescing", () => {
|
||||||
|
|
||||||
expect(routineIssues).toHaveLength(1);
|
expect(routineIssues).toHaveLength(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("accepts standard second-precision webhook timestamps for HMAC triggers", async () => {
|
||||||
|
const { routine, svc } = await seedFixture();
|
||||||
|
const { trigger, secretMaterial } = await svc.createTrigger(
|
||||||
|
routine.id,
|
||||||
|
{
|
||||||
|
kind: "webhook",
|
||||||
|
signingMode: "hmac_sha256",
|
||||||
|
replayWindowSec: 300,
|
||||||
|
},
|
||||||
|
{},
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(trigger.publicId).toBeTruthy();
|
||||||
|
expect(secretMaterial?.webhookSecret).toBeTruthy();
|
||||||
|
|
||||||
|
const payload = { ok: true };
|
||||||
|
const rawBody = Buffer.from(JSON.stringify(payload));
|
||||||
|
const timestampSeconds = String(Math.floor(Date.now() / 1000));
|
||||||
|
const signature = `sha256=${createHmac("sha256", secretMaterial!.webhookSecret)
|
||||||
|
.update(`${timestampSeconds}.`)
|
||||||
|
.update(rawBody)
|
||||||
|
.digest("hex")}`;
|
||||||
|
|
||||||
|
const run = await svc.firePublicTrigger(trigger.publicId!, {
|
||||||
|
signatureHeader: signature,
|
||||||
|
timestampHeader: timestampSeconds,
|
||||||
|
rawBody,
|
||||||
|
payload,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(run.source).toBe("webhook");
|
||||||
|
expect(run.status).toBe("issue_created");
|
||||||
|
expect(run.linkedIssueId).toBeTruthy();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -132,6 +132,12 @@ function nextResultText(status: string, issueId?: string | null) {
|
||||||
return status;
|
return status;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function normalizeWebhookTimestampMs(rawTimestamp: string) {
|
||||||
|
const parsed = Number(rawTimestamp);
|
||||||
|
if (!Number.isFinite(parsed)) return null;
|
||||||
|
return parsed > 1e12 ? parsed : parsed * 1000;
|
||||||
|
}
|
||||||
|
|
||||||
export function routineService(db: Db, deps: { heartbeat?: IssueAssignmentWakeupDeps } = {}) {
|
export function routineService(db: Db, deps: { heartbeat?: IssueAssignmentWakeupDeps } = {}) {
|
||||||
const issueSvc = issueService(db);
|
const issueSvc = issueService(db);
|
||||||
const secretsSvc = secretService(db);
|
const secretsSvc = secretService(db);
|
||||||
|
|
@ -1064,8 +1070,8 @@ export function routineService(db: Db, deps: { heartbeat?: IssueAssignmentWakeup
|
||||||
const providedSignature = input.signatureHeader?.trim() ?? "";
|
const providedSignature = input.signatureHeader?.trim() ?? "";
|
||||||
const providedTimestamp = input.timestampHeader?.trim() ?? "";
|
const providedTimestamp = input.timestampHeader?.trim() ?? "";
|
||||||
if (!providedSignature || !providedTimestamp) throw unauthorized();
|
if (!providedSignature || !providedTimestamp) throw unauthorized();
|
||||||
const tsMillis = Number(providedTimestamp);
|
const tsMillis = normalizeWebhookTimestampMs(providedTimestamp);
|
||||||
if (!Number.isFinite(tsMillis)) throw unauthorized();
|
if (tsMillis == null) throw unauthorized();
|
||||||
const replayWindowSec = trigger.replayWindowSec ?? 300;
|
const replayWindowSec = trigger.replayWindowSec ?? 300;
|
||||||
if (Math.abs(Date.now() - tsMillis) > replayWindowSec * 1000) {
|
if (Math.abs(Date.now() - tsMillis) > replayWindowSec * 1000) {
|
||||||
throw unauthorized();
|
throw unauthorized();
|
||||||
|
|
|
||||||
71
ui/src/lib/routine-trigger-patch.test.ts
Normal file
71
ui/src/lib/routine-trigger-patch.test.ts
Normal file
|
|
@ -0,0 +1,71 @@
|
||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import type { RoutineTrigger } from "@paperclipai/shared";
|
||||||
|
import { buildRoutineTriggerPatch } from "./routine-trigger-patch";
|
||||||
|
|
||||||
|
function makeScheduleTrigger(overrides: Partial<RoutineTrigger> = {}): RoutineTrigger {
|
||||||
|
return {
|
||||||
|
id: "trigger-1",
|
||||||
|
companyId: "company-1",
|
||||||
|
routineId: "routine-1",
|
||||||
|
kind: "schedule",
|
||||||
|
label: "Daily",
|
||||||
|
enabled: true,
|
||||||
|
cronExpression: "0 10 * * *",
|
||||||
|
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"),
|
||||||
|
...overrides,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("buildRoutineTriggerPatch", () => {
|
||||||
|
it("preserves an existing schedule trigger timezone when saving edits", () => {
|
||||||
|
const patch = buildRoutineTriggerPatch(
|
||||||
|
makeScheduleTrigger({ timezone: "UTC" }),
|
||||||
|
{
|
||||||
|
label: "Daily label edit",
|
||||||
|
cronExpression: "0 10 * * *",
|
||||||
|
signingMode: "bearer",
|
||||||
|
replayWindowSec: "300",
|
||||||
|
},
|
||||||
|
"America/Chicago",
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(patch).toEqual({
|
||||||
|
label: "Daily label edit",
|
||||||
|
cronExpression: "0 10 * * *",
|
||||||
|
timezone: "UTC",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back to the local timezone when a schedule trigger has none", () => {
|
||||||
|
const patch = buildRoutineTriggerPatch(
|
||||||
|
makeScheduleTrigger({ timezone: null }),
|
||||||
|
{
|
||||||
|
label: "",
|
||||||
|
cronExpression: "15 9 * * 1-5",
|
||||||
|
signingMode: "bearer",
|
||||||
|
replayWindowSec: "300",
|
||||||
|
},
|
||||||
|
"America/Chicago",
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(patch).toEqual({
|
||||||
|
label: null,
|
||||||
|
cronExpression: "15 9 * * 1-5",
|
||||||
|
timezone: "America/Chicago",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
30
ui/src/lib/routine-trigger-patch.ts
Normal file
30
ui/src/lib/routine-trigger-patch.ts
Normal file
|
|
@ -0,0 +1,30 @@
|
||||||
|
import type { RoutineTrigger } from "@paperclipai/shared";
|
||||||
|
|
||||||
|
export type RoutineTriggerEditorDraft = {
|
||||||
|
label: string;
|
||||||
|
cronExpression: string;
|
||||||
|
signingMode: string;
|
||||||
|
replayWindowSec: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
export function buildRoutineTriggerPatch(
|
||||||
|
trigger: RoutineTrigger,
|
||||||
|
draft: RoutineTriggerEditorDraft,
|
||||||
|
fallbackTimezone: string,
|
||||||
|
) {
|
||||||
|
const patch: Record<string, unknown> = {
|
||||||
|
label: draft.label.trim() || null,
|
||||||
|
};
|
||||||
|
|
||||||
|
if (trigger.kind === "schedule") {
|
||||||
|
patch.cronExpression = draft.cronExpression.trim();
|
||||||
|
patch.timezone = trigger.timezone ?? fallbackTimezone;
|
||||||
|
}
|
||||||
|
|
||||||
|
if (trigger.kind === "webhook") {
|
||||||
|
patch.signingMode = draft.signingMode;
|
||||||
|
patch.replayWindowSec = Number(draft.replayWindowSec || "300");
|
||||||
|
}
|
||||||
|
|
||||||
|
return patch;
|
||||||
|
}
|
||||||
|
|
@ -24,6 +24,7 @@ import { useCompany } from "../context/CompanyContext";
|
||||||
import { useBreadcrumbs } from "../context/BreadcrumbContext";
|
import { useBreadcrumbs } from "../context/BreadcrumbContext";
|
||||||
import { useToast } from "../context/ToastContext";
|
import { useToast } from "../context/ToastContext";
|
||||||
import { queryKeys } from "../lib/queryKeys";
|
import { queryKeys } from "../lib/queryKeys";
|
||||||
|
import { buildRoutineTriggerPatch } from "../lib/routine-trigger-patch";
|
||||||
import { timeAgo } from "../lib/timeAgo";
|
import { timeAgo } from "../lib/timeAgo";
|
||||||
import { EmptyState } from "../components/EmptyState";
|
import { EmptyState } from "../components/EmptyState";
|
||||||
import { PageSkeleton } from "../components/PageSkeleton";
|
import { PageSkeleton } from "../components/PageSkeleton";
|
||||||
|
|
@ -61,7 +62,7 @@ const concurrencyPolicyDescriptions: Record<string, string> = {
|
||||||
};
|
};
|
||||||
const catchUpPolicyDescriptions: Record<string, string> = {
|
const catchUpPolicyDescriptions: Record<string, string> = {
|
||||||
skip_missed: "Ignore schedule windows that were missed while the routine or scheduler was paused.",
|
skip_missed: "Ignore schedule windows that were missed while the routine or scheduler was paused.",
|
||||||
enqueue_missed_with_cap: "Catch up missed schedule windows with a capped backlog after recovery.",
|
enqueue_missed_with_cap: "Catch up missed schedule windows in capped batches after recovery.",
|
||||||
};
|
};
|
||||||
const signingModeDescriptions: Record<string, string> = {
|
const signingModeDescriptions: Record<string, string> = {
|
||||||
bearer: "Expect a shared bearer token in the Authorization header.",
|
bearer: "Expect a shared bearer token in the Authorization header.",
|
||||||
|
|
@ -212,20 +213,7 @@ function TriggerEditor({
|
||||||
<Button
|
<Button
|
||||||
variant="outline"
|
variant="outline"
|
||||||
size="sm"
|
size="sm"
|
||||||
onClick={() =>
|
onClick={() => onSave(trigger.id, buildRoutineTriggerPatch(trigger, draft, getLocalTimezone()))}
|
||||||
onSave(trigger.id, {
|
|
||||||
label: draft.label.trim() || null,
|
|
||||||
...(trigger.kind === "schedule"
|
|
||||||
? { cronExpression: draft.cronExpression.trim(), timezone: getLocalTimezone() }
|
|
||||||
: {}),
|
|
||||||
...(trigger.kind === "webhook"
|
|
||||||
? {
|
|
||||||
signingMode: draft.signingMode,
|
|
||||||
replayWindowSec: Number(draft.replayWindowSec || "300"),
|
|
||||||
}
|
|
||||||
: {}),
|
|
||||||
})
|
|
||||||
}
|
|
||||||
>
|
>
|
||||||
<Save className="mr-1.5 h-3.5 w-3.5" />
|
<Save className="mr-1.5 h-3.5 w-3.5" />
|
||||||
Save
|
Save
|
||||||
|
|
|
||||||
|
|
@ -43,7 +43,7 @@ const concurrencyPolicyDescriptions: Record<string, string> = {
|
||||||
};
|
};
|
||||||
const catchUpPolicyDescriptions: Record<string, string> = {
|
const catchUpPolicyDescriptions: Record<string, string> = {
|
||||||
skip_missed: "Ignore windows that were missed while the scheduler or routine was paused.",
|
skip_missed: "Ignore windows that were missed while the scheduler or routine was paused.",
|
||||||
enqueue_missed_with_cap: "Catch up missed schedule windows with a capped backlog after recovery.",
|
enqueue_missed_with_cap: "Catch up missed schedule windows in capped batches after recovery.",
|
||||||
};
|
};
|
||||||
|
|
||||||
function autoResizeTextarea(element: HTMLTextAreaElement | null) {
|
function autoResizeTextarea(element: HTMLTextAreaElement | null) {
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue