From e5c627ea9b6688dd894044f1bf1b08c71439beb9 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 8 Jul 2026 18:49:11 +0000 Subject: [PATCH] Fix built-in routine stock hash normalization Normalize built-in routine defaults through the same projection used by persisted routine status checks so pristine bundles do not report operator_modified after a clean reconcile. Add regression coverage for the status read path after Reflection Coach materialization. Co-Authored-By: Paperclip --- server/src/__tests__/built-in-agents.test.ts | 32 +++++++- server/src/services/built-in-agents.ts | 82 +++++++++++++------- 2 files changed, 85 insertions(+), 29 deletions(-) diff --git a/server/src/__tests__/built-in-agents.test.ts b/server/src/__tests__/built-in-agents.test.ts index a3e97681baa..53690250f99 100644 --- a/server/src/__tests__/built-in-agents.test.ts +++ b/server/src/__tests__/built-in-agents.test.ts @@ -448,9 +448,10 @@ describeEmbeddedPostgres("built-in agents", () => { instructionsEntryFile: "AGENTS.md", }); expect(state.agent?.adapterConfig).not.toMatchObject({ model: "gpt-5.4", apiKey: "do-not-copy" }); - expect(state.resources.map((resource) => [resource.resourceKind, resource.stockStatus]).slice(0, 2)).toEqual([ + expect(state.resources.map((resource) => [resource.resourceKind, resource.stockStatus])).toEqual([ ["instructions", "stock_current"], ["skill", "stock_current"], + ["routine", "stock_current"], ]); expect(state.resources.find((resource) => resource.resourceKind === "routine")).toMatchObject({ resourceId: expect.any(String), @@ -681,6 +682,14 @@ describeEmbeddedPostgres("built-in agents", () => { ["skill", "stock_current"], ["routine", "stock_current"], ]); + const reported = await builtInAgentService(db).get(companyId, "reflection-coach"); + const reportedRoutine = reported.resources.find((resource) => resource.resourceKind === "routine"); + expect(reportedRoutine).toMatchObject({ + stockStatus: "stock_current", + updateAvailable: false, + resetAvailable: false, + }); + expect(reportedRoutine?.currentHash).toBe(reportedRoutine?.stockHash); const [skill] = await db .select() @@ -730,6 +739,7 @@ describeEmbeddedPostgres("built-in agents", () => { const created = await builtIns.ensure(companyId, "reflection-coach"); expect(created.status).toBe("paused"); expect(created.resources.find((resource) => resource.resourceKind === "routine")).toMatchObject({ + stockStatus: "stock_current", scheduleEnabled: false, }); @@ -741,6 +751,7 @@ describeEmbeddedPostgres("built-in agents", () => { ); expect(enabled.status).toBe("needs_setup"); expect(enabled.resources.find((resource) => resource.resourceKind === "routine")).toMatchObject({ + stockStatus: "stock_current", scheduleEnabled: true, }); const [enabledRoutine] = await db.select().from(routines).where(eq(routines.companyId, companyId)); @@ -755,6 +766,7 @@ describeEmbeddedPostgres("built-in agents", () => { { userId: "board-user" }, ); expect(disabled.resources.find((resource) => resource.resourceKind === "routine")).toMatchObject({ + stockStatus: "stock_current", scheduleEnabled: false, }); const [disabledRoutine] = await db.select().from(routines).where(eq(routines.id, enabledRoutine!.id)); @@ -826,6 +838,10 @@ describeEmbeddedPostgres("built-in agents", () => { .update(companySkills) .set({ markdown: "---\nname: reflection-coach\n---\n\n# Custom skill\n" }) .where(eq(companySkills.companyId, companyId)); + await db + .update(routines) + .set({ title: "DRIFTED BY TEST - do not clobber" }) + .where(eq(routines.companyId, companyId)); const drifted = await builtInAgentService(db).ensure(companyId, "reflection-coach"); expect(drifted.resources.find((resource) => resource.resourceKind === "instructions")).toMatchObject({ @@ -836,10 +852,16 @@ describeEmbeddedPostgres("built-in agents", () => { stockStatus: "operator_modified", resetAvailable: true, }); + expect(drifted.resources.find((resource) => resource.resourceKind === "routine")).toMatchObject({ + stockStatus: "operator_modified", + resetAvailable: true, + }); expect((await instructionsSvc.readFile(drifted.agent!, "AGENTS.md")).content).toContain("Do not overwrite me."); + const [preservedRoutine] = await db.select().from(routines).where(eq(routines.companyId, companyId)); + expect(preservedRoutine?.title).toBe("DRIFTED BY TEST - do not clobber"); const reset = await builtInAgentService(db).reset(companyId, "reflection-coach", { - resources: ["instructions"], + resources: ["instructions", "routine"], }); expect(reset.resources.find((resource) => resource.resourceKind === "instructions")).toMatchObject({ stockStatus: "stock_current", @@ -849,6 +871,12 @@ describeEmbeddedPostgres("built-in agents", () => { stockStatus: "operator_modified", resetAvailable: true, }); + expect(reset.resources.find((resource) => resource.resourceKind === "routine")).toMatchObject({ + stockStatus: "stock_current", + resetAvailable: false, + }); expect((await instructionsSvc.readFile(reset.agent!, "AGENTS.md")).content).toContain("You are Reflection Coach"); + const [resetRoutine] = await db.select().from(routines).where(eq(routines.companyId, companyId)); + expect(resetRoutine?.title).toBe("Review recent agent trajectories for coaching proposals"); }); }); diff --git a/server/src/services/built-in-agents.ts b/server/src/services/built-in-agents.ts index ddd53a976cf..89e321b8897 100644 --- a/server/src/services/built-in-agents.ts +++ b/server/src/services/built-in-agents.ts @@ -6,6 +6,7 @@ import { readPaperclipSkillSyncPreference, writePaperclipSkillSyncPreference } f import { and, desc, eq, ne } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { agents, builtInManagedResources, companies, issueThreadInteractions, issues, routines, routineTriggers } from "@paperclipai/db"; +import { syncRoutineVariablesWithTemplate } from "@paperclipai/shared"; import type { Agent, Approval, CompanySkill, Routine, RoutineTrigger, RoutineVariable } from "@paperclipai/shared"; import { conflict, HttpError, notFound, unprocessable } from "../errors.js"; import { logActivity } from "./activity-log.js"; @@ -824,20 +825,63 @@ export function builtInAgentService(db: Db) { }); } - function routineDefaultsHash(routine: BuiltInAgentBundleDefinition["routine"], triggers: BuiltInAgentBundleDefinition["routine"]["triggers"]) { + function normalizeRoutineVariablesForHash(input: { + title: string; + description?: string | null; + variables?: RoutineVariable[] | null; + }) { + return syncRoutineVariablesWithTemplate( + [input.title, input.description ?? ""], + input.variables ?? [], + ).map((variable) => ({ + name: variable.name, + label: variable.label ?? null, + type: variable.type ?? "text", + defaultValue: variable.defaultValue ?? null, + required: variable.required ?? true, + options: variable.options ?? [], + })); + } + + function normalizeRoutineTriggersForHash( + triggers: Array<{ + kind: string; + label?: string | null; + cronExpression?: string | null; + timezone?: string | null; + }>, + ) { + return triggers + .filter((trigger) => trigger.kind === "schedule") + .map((trigger) => ({ + kind: "schedule", + label: trigger.label ?? null, + cronExpression: trigger.cronExpression ?? "", + timezone: trigger.timezone ?? "UTC", + })) + .sort((left, right) => stableJson(left).localeCompare(stableJson(right))); + } + + function routineDefaultsHash( + routine: Pick< + BuiltInAgentBundleDefinition["routine"], + "title" | "description" | "priority" | "concurrencyPolicy" | "catchUpPolicy" | "variables" + >, + triggers: Array<{ + kind: string; + label?: string | null; + cronExpression?: string | null; + timezone?: string | null; + }>, + ) { return stockHash({ title: routine.title, - description: routine.description, + description: routine.description ?? "", priority: routine.priority, concurrencyPolicy: routine.concurrencyPolicy, catchUpPolicy: routine.catchUpPolicy, - variables: routine.variables, - triggers: triggers.map((trigger) => ({ - kind: trigger.kind, - label: trigger.label, - cronExpression: trigger.cronExpression, - timezone: trigger.timezone, - })), + variables: normalizeRoutineVariablesForHash(routine), + triggers: normalizeRoutineTriggersForHash(triggers), }); } @@ -1013,21 +1057,13 @@ export function builtInAgentService(db: Db) { const stock = routineDefaultsHash(bundle.routine, bundle.routine.triggers); const { binding, routine, triggers } = await getRoutineByBinding(agent.companyId, definition); const currentHash = routine ? routineDefaultsHash({ - ...bundle.routine, title: routine.title, description: routine.description ?? "", - status: routine.status as "active" | "paused", priority: routine.priority as "critical" | "high" | "medium" | "low", concurrencyPolicy: routine.concurrencyPolicy as "always_enqueue" | "coalesce_if_active" | "skip_if_active", catchUpPolicy: routine.catchUpPolicy as "enqueue_missed_with_cap" | "skip_missed", variables: routine.variables ?? [], - }, triggers.filter((trigger) => trigger.kind === "schedule").map((trigger) => ({ - kind: "schedule" as const, - label: trigger.label, - enabled: trigger.enabled, - cronExpression: trigger.cronExpression ?? "", - timezone: trigger.timezone ?? "UTC", - }))) : null; + }, triggers) : null; const currentState = stockState({ resourceKind: "routine", resourceKey: bundle.routine.routineKey, @@ -1096,21 +1132,13 @@ export function builtInAgentService(db: Db) { ? stockHash(skillFiles) : null; const routineHash = routine ? routineDefaultsHash({ - ...bundle.routine, title: routine.title, description: routine.description ?? "", - status: routine.status as "active" | "paused", priority: routine.priority as "critical" | "high" | "medium" | "low", concurrencyPolicy: routine.concurrencyPolicy as "always_enqueue" | "coalesce_if_active" | "skip_if_active", catchUpPolicy: routine.catchUpPolicy as "enqueue_missed_with_cap" | "skip_missed", variables: routine.variables ?? [], - }, triggers.filter((trigger) => trigger.kind === "schedule").map((trigger) => ({ - kind: "schedule" as const, - label: trigger.label, - enabled: trigger.enabled, - cronExpression: trigger.cronExpression ?? "", - timezone: trigger.timezone ?? "UTC", - }))) : null; + }, triggers) : null; return [ stockState({ resourceKind: "instructions",