fix(routines): address Greptile review findings

Co-Authored-By: Paperclip <noreply@paperclip.ing>

cli/src/commands/routines.ts

@@ -197,10 +197,14 @@ async function openConfiguredDb(configPath: string): Promise<{
       await ensurePostgresDatabase(adminConnectionString, "paperclip");
       const connectionString = `postgres://paperclip:paperclip@127.0.0.1:${embeddedHandle.port}/paperclip`;
       await applyPendingMigrations(connectionString);
+      const db = createDb(connectionString) as ClosableDb;
       return {
-        db: createDb(connectionString) as ClosableDb,
+        db,
         stop: async () => {
-          await closeDb(createDb(connectionString) as ClosableDb);
+          await closeDb(db);
+          if (embeddedHandle?.startedByThisProcess) {
+            await embeddedHandle.stop().catch(() => undefined);
+          }
         },
       };
     }

packages/shared/src/validators/issue.ts

@@ -1,6 +1,15 @@
 import { z } from "zod";
 import { ISSUE_PRIORITIES, ISSUE_STATUSES } from "../constants.js";
 
+export const ISSUE_EXECUTION_WORKSPACE_PREFERENCES = [
+  "inherit",
+  "shared_workspace",
+  "isolated_workspace",
+  "operator_branch",
+  "reuse_existing",
+  "agent_default",
+] as const;
+
 const executionWorkspaceStrategySchema = z
   .object({
     type: z.enum(["project_primary", "git_worktree", "adapter_managed", "cloud_sandbox"]).optional(),
@@ -14,7 +23,7 @@ const executionWorkspaceStrategySchema = z
 
 export const issueExecutionWorkspaceSettingsSchema = z
   .object({
-    mode: z.enum(["inherit", "shared_workspace", "isolated_workspace", "operator_branch", "reuse_existing", "agent_default"]).optional(),
+    mode: z.enum(ISSUE_EXECUTION_WORKSPACE_PREFERENCES).optional(),
     workspaceStrategy: executionWorkspaceStrategySchema.optional().nullable(),
     workspaceRuntime: z.record(z.unknown()).optional().nullable(),
   })
@@ -43,14 +52,7 @@ export const createIssueSchema = z.object({
   billingCode: z.string().optional().nullable(),
   assigneeAdapterOverrides: issueAssigneeAdapterOverridesSchema.optional().nullable(),
   executionWorkspaceId: z.string().uuid().optional().nullable(),
-  executionWorkspacePreference: z.enum([
+  executionWorkspacePreference: z.enum(ISSUE_EXECUTION_WORKSPACE_PREFERENCES).optional().nullable(),
-    "inherit",
-    "shared_workspace",
-    "isolated_workspace",
-    "operator_branch",
-    "reuse_existing",
-    "agent_default",
-  ]).optional().nullable(),
   executionWorkspaceSettings: issueExecutionWorkspaceSettingsSchema.optional().nullable(),
   labelIds: z.array(z.string().uuid()).optional(),
 });

packages/shared/src/validators/routine.ts

@@ -7,7 +7,10 @@ import {
   ROUTINE_TRIGGER_SIGNING_MODES,
   ROUTINE_VARIABLE_TYPES,
 } from "../constants.js";
-import { issueExecutionWorkspaceSettingsSchema } from "./issue.js";
+import {
+  ISSUE_EXECUTION_WORKSPACE_PREFERENCES,
+  issueExecutionWorkspaceSettingsSchema,
+} from "./issue.js";
 
 const routineVariableValueSchema = z.union([z.string(), z.number().finite(), z.boolean()]);
 
@@ -104,14 +107,7 @@ export const runRoutineSchema = z.object({
   idempotencyKey: z.string().trim().max(255).optional().nullable(),
   source: z.enum(["manual", "api"]).optional().default("manual"),
   executionWorkspaceId: z.string().uuid().optional().nullable(),
-  executionWorkspacePreference: z.enum([
+  executionWorkspacePreference: z.enum(ISSUE_EXECUTION_WORKSPACE_PREFERENCES).optional().nullable(),
-    "inherit",
-    "shared_workspace",
-    "isolated_workspace",
-    "operator_branch",
-    "reuse_existing",
-    "agent_default",
-  ]).optional().nullable(),
   executionWorkspaceSettings: issueExecutionWorkspaceSettingsSchema.optional().nullable(),
 });
 

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

@@ -466,6 +466,54 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => {
     ).rejects.toThrow(/require defaults for required variables/i);
   });
 
+  it("treats malformed stored defaults as missing when validating schedule triggers", async () => {
+    const { companyId, agentId, projectId, svc } = await seedFixture();
+    const variableRoutine = await svc.create(
+      companyId,
+      {
+        projectId,
+        goalId: null,
+        parentIssueId: null,
+        title: "ship check",
+        description: "Review {{approved}}",
+        assigneeAgentId: agentId,
+        priority: "medium",
+        status: "active",
+        concurrencyPolicy: "coalesce_if_active",
+        catchUpPolicy: "skip_missed",
+        variables: [
+          { name: "approved", label: null, type: "boolean", defaultValue: true, required: true, options: [] },
+        ],
+      },
+      {},
+    );
+
+    await db
+      .update(routines)
+      .set({
+        variables: [
+          {
+            name: "approved",
+            label: null,
+            type: "boolean",
+            defaultValue: "definitely",
+            required: true,
+            options: [],
+          },
+        ],
+      })
+      .where(eq(routines.id, variableRoutine.id));
+
+    await expect(
+      svc.createTrigger(variableRoutine.id, {
+        kind: "schedule",
+        label: "daily",
+        cronExpression: "0 10 * * *",
+        timezone: "UTC",
+      }, {}),
+    ).rejects.toThrow(/require defaults for required variables/i);
+  });
+
   it("serializes concurrent dispatches until the first execution issue is linked to a queued run", async () => {
     const { routine, svc } = await seedFixture({
       wakeup: async (wakeupAgentId, wakeupOpts) => {

server/src/services/routines.ts

@@ -213,7 +213,13 @@ function sanitizeRoutineVariableInputs(
 function assertScheduleCompatibleVariables(variables: RoutineVariable[]) {
   const missingDefaults = variables
     .filter((variable) => variable.required)
-    .filter((variable) => isMissingRoutineVariableValue(normalizeRoutineVariableValue(variable, variable.defaultValue)))
+    .filter((variable) => {
+      try {
+        return isMissingRoutineVariableValue(normalizeRoutineVariableValue(variable, variable.defaultValue));
+      } catch {
+        return true;
+      }
+    })
     .map((variable) => variable.name);
   if (missingDefaults.length > 0) {
     throw unprocessable(