From 6a19179e8d9061cb9ab4e6a63951e4d93fd08f89 Mon Sep 17 00:00:00 2001
From: Brad Harris <bmharris@gmail.com>
Date: Thu, 21 May 2026 20:23:00 -0600
Subject: [PATCH] Add unit tests for template arg-parser, cron intervals, and
 job notifier edge cases

Coverage additions from test-enforcer run: 28 new tests for the previously untested
template arg-parser (parsing, modifier merging, deduplication, substitution), 5 tests
for the untested cron interval validator, and 12 tests for job notifier gaps (duration
formatting, mrkdwn escaping, status routing, defensive config handling).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 apps/server/test/jobs/cron.test.ts           |  37 +++-
 apps/server/test/jobs/job-notifier.test.ts   | 156 ++++++++++++++
 apps/server/test/template-arg-parser.test.ts | 202 +++++++++++++++++++
 3 files changed, 394 insertions(+), 1 deletion(-)
 create mode 100644 apps/server/test/template-arg-parser.test.ts

diff --git a/apps/server/test/jobs/cron.test.ts b/apps/server/test/jobs/cron.test.ts
index bc21b1b0..628c5a8f 100644
--- a/apps/server/test/jobs/cron.test.ts
+++ b/apps/server/test/jobs/cron.test.ts
@@ -1,6 +1,10 @@
 import { describe, expect, it } from "vitest";
 
-import { getNextRun, validateCronExpression } from "../../src/jobs/cron.js";
+import {
+  getNextRun,
+  validateCronExpression,
+  validateCronInterval,
+} from "../../src/jobs/cron.js";
 
 describe("cron utilities", () => {
   describe("getNextRun", () => {
@@ -49,4 +53,35 @@ describe("cron utilities", () => {
       expect(validateCronExpression("60 * * * *")).toBe(false);
     });
   });
+
+  describe("validateCronInterval", () => {
+    it("returns null for schedules at or above 5-minute interval", () => {
+      expect(validateCronInterval("*/5 * * * *")).toBeNull();
+      expect(validateCronInterval("0 * * * *")).toBeNull();
+      expect(validateCronInterval("0 0 * * *")).toBeNull();
+    });
+
+    it("returns error for schedules under 5-minute interval", () => {
+      const result = validateCronInterval("* * * * *");
+      expect(result).toMatch(/runs too frequently/);
+      expect(result).toMatch(/every 60s/);
+    });
+
+    it("returns error for every-2-minute schedule", () => {
+      const result = validateCronInterval("*/2 * * * *");
+      expect(result).toMatch(/runs too frequently/);
+      expect(result).toMatch(/every 120s/);
+    });
+
+    it("returns error for invalid cron expression", () => {
+      expect(validateCronInterval("not valid")).toBe(
+        "Invalid cron expression."
+      );
+    });
+
+    it("returns null for schedules that only fire once", () => {
+      const result = validateCronInterval("0 12 1 1 *");
+      expect(result).toBeNull();
+    });
+  });
 });
diff --git a/apps/server/test/jobs/job-notifier.test.ts b/apps/server/test/jobs/job-notifier.test.ts
index 5cc7f123..0aa516a7 100644
--- a/apps/server/test/jobs/job-notifier.test.ts
+++ b/apps/server/test/jobs/job-notifier.test.ts
@@ -181,4 +181,160 @@ describe("JobNotifier", () => {
     const body = JSON.parse(fetchSpy.mock.calls[0][1].body);
     expect(body.attachments[0].color).toBe("#ef4444");
   });
+
+  it("displays 'timed out' as status label for timed_out runs", async () => {
+    await setSetting(
+      pool,
+      "slack_webhook_url",
+      "https://hooks.slack.test/test"
+    );
+    await notifier.onJobRunStateChange(makeRun({ status: "timed_out" }));
+
+    const body = JSON.parse(fetchSpy.mock.calls[0][1].body);
+    const contextText = body.attachments[0].blocks[1].elements[0].text;
+    expect(contextText).toContain("`timed out`");
+  });
+
+  it("escapes special characters in job name and summary", async () => {
+    await setSetting(
+      pool,
+      "slack_webhook_url",
+      "https://hooks.slack.test/test"
+    );
+    await notifier.onJobRunStateChange(
+      makeRun({
+        config: {
+          ...makeRun().config,
+          name: "job <with> & special chars",
+        },
+        report: {
+          status: "completed",
+          summary: "Fixed <script> & stuff",
+          tasks: [],
+        },
+      })
+    );
+
+    const body = JSON.parse(fetchSpy.mock.calls[0][1].body);
+    const text = body.attachments[0].blocks[0].text.text;
+    expect(text).toContain("&amp;");
+    expect(text).toContain("&lt;");
+    expect(text).toContain("&gt;");
+    expect(text).not.toContain("<with>");
+    expect(text).not.toContain("<script>");
+  });
+
+  describe("duration formatting in Slack payload", () => {
+    async function getDurationFromPayload(durationMs: number): Promise<string> {
+      await setSetting(
+        pool,
+        "slack_webhook_url",
+        "https://hooks.slack.test/test"
+      );
+      notifier = new JobNotifier(pool, fakeLogger);
+      fetchSpy.mockClear();
+      await notifier.onJobRunStateChange(makeRun({ durationMs }));
+      const body = JSON.parse(fetchSpy.mock.calls[0][1].body);
+      return body.attachments[0].blocks[1].elements[0].text;
+    }
+
+    it("formats sub-second duration as milliseconds", async () => {
+      const text = await getDurationFromPayload(500);
+      expect(text).toContain("500ms");
+    });
+
+    it("formats seconds-only duration", async () => {
+      const text = await getDurationFromPayload(45_000);
+      expect(text).toContain("45s");
+    });
+
+    it("formats minutes with remaining seconds", async () => {
+      const text = await getDurationFromPayload(125_000);
+      expect(text).toContain("2m 5s");
+    });
+
+    it("formats exact minutes without seconds", async () => {
+      const text = await getDurationFromPayload(120_000);
+      expect(text).toContain("2m");
+      expect(text).not.toContain("2m 0s");
+    });
+
+    it("formats hours with remaining minutes", async () => {
+      const text = await getDurationFromPayload(3_900_000);
+      expect(text).toContain("1h 5m");
+    });
+
+    it("formats exact hours without minutes", async () => {
+      const text = await getDurationFromPayload(3_600_000);
+      expect(text).toContain("1h");
+      expect(text).not.toContain("1h 0m");
+    });
+
+    it("omits duration when durationMs is null", async () => {
+      await setSetting(
+        pool,
+        "slack_webhook_url",
+        "https://hooks.slack.test/test"
+      );
+      notifier = new JobNotifier(pool, fakeLogger);
+      fetchSpy.mockClear();
+      await notifier.onJobRunStateChange(makeRun({ durationMs: null }));
+      const body = JSON.parse(fetchSpy.mock.calls[0][1].body);
+      const contextText = body.attachments[0].blocks[1].elements[0].text;
+      expect(contextText).not.toMatch(/\d+[hms]/);
+    });
+  });
+
+  it("does not send notification when notify config is undefined", async () => {
+    await setSetting(
+      pool,
+      "slack_webhook_url",
+      "https://hooks.slack.test/test"
+    );
+    await notifier.onJobRunStateChange(
+      makeRun({
+        config: {
+          ...makeRun().config,
+          notify: undefined as never,
+        },
+      })
+    );
+    expect(fetchSpy).not.toHaveBeenCalled();
+  });
+
+  it("routes crashed status to on_error channels", async () => {
+    await setSetting(
+      pool,
+      "slack_webhook_url",
+      "https://hooks.slack.test/test"
+    );
+    await notifier.onJobRunStateChange(makeRun({ status: "crashed" }));
+
+    expect(fetchSpy).toHaveBeenCalledOnce();
+    const body = JSON.parse(fetchSpy.mock.calls[0][1].body);
+    expect(body.attachments[0].color).toBe("#ef4444");
+  });
+
+  it("uses pendingQuestion as summary for needs_input when report is absent", async () => {
+    await setSetting(
+      pool,
+      "slack_webhook_url",
+      "https://hooks.slack.test/test"
+    );
+    await notifier.onJobRunStateChange(
+      makeRun({
+        status: "needs_input",
+        report: null,
+        pendingQuestion: "What should I do next?",
+        config: {
+          ...makeRun().config,
+          notify: { onComplete: [], onError: [], onNeedsInput: ["slack"] },
+        },
+      })
+    );
+
+    const body = JSON.parse(fetchSpy.mock.calls[0][1].body);
+    const fallback = body.attachments[0].fallback;
+    expect(fallback).toContain("What should I do next?");
+  });
 });
diff --git a/apps/server/test/template-arg-parser.test.ts b/apps/server/test/template-arg-parser.test.ts
new file mode 100644
index 00000000..3b3c910f
--- /dev/null
+++ b/apps/server/test/template-arg-parser.test.ts
@@ -0,0 +1,202 @@
+import { describe, expect, it } from "vitest";
+
+import {
+  parseTemplateArgs,
+  substituteArgs,
+  TEMPLATE_ARG_PATTERN,
+} from "../src/templates/arg-parser.js";
+
+describe("TEMPLATE_ARG_PATTERN", () => {
+  it("matches {{D:...}} placeholders", () => {
+    const matches = [
+      ..."Hello {{D:name}} world {{D:age}}".matchAll(
+        new RegExp(TEMPLATE_ARG_PATTERN.source, "g")
+      ),
+    ];
+    expect(matches).toHaveLength(2);
+    expect(matches[0][1]).toBe("name");
+    expect(matches[1][1]).toBe("age");
+  });
+
+  it("does not match plain {{...}} without D: prefix", () => {
+    const matches = [
+      ..."Hello {{name}}".matchAll(
+        new RegExp(TEMPLATE_ARG_PATTERN.source, "g")
+      ),
+    ];
+    expect(matches).toHaveLength(0);
+  });
+});
+
+describe("parseTemplateArgs", () => {
+  it("returns empty array for prompt with no placeholders", () => {
+    expect(parseTemplateArgs("Hello world")).toEqual([]);
+  });
+
+  it("parses a single arg with no modifiers", () => {
+    const result = parseTemplateArgs("Fix {{D:repo_url}} please");
+    expect(result).toEqual([
+      {
+        name: "repo_url",
+        key: "repo_url",
+        placeholder: "{{D:repo_url}}",
+        required: false,
+        multiline: false,
+      },
+    ]);
+  });
+
+  it("parses the required modifier", () => {
+    const result = parseTemplateArgs("Deploy {{D:target|required}}");
+    expect(result).toHaveLength(1);
+    expect(result[0].required).toBe(true);
+    expect(result[0].multiline).toBe(false);
+  });
+
+  it("parses the multiline modifier", () => {
+    const result = parseTemplateArgs("Context: {{D:description|multiline}}");
+    expect(result).toHaveLength(1);
+    expect(result[0].multiline).toBe(true);
+  });
+
+  it("parses the textarea modifier as multiline", () => {
+    const result = parseTemplateArgs("Context: {{D:description|textarea}}");
+    expect(result).toHaveLength(1);
+    expect(result[0].multiline).toBe(true);
+  });
+
+  it("parses multiple modifiers together", () => {
+    const result = parseTemplateArgs("Input: {{D:context|required|multiline}}");
+    expect(result).toHaveLength(1);
+    expect(result[0].required).toBe(true);
+    expect(result[0].multiline).toBe(true);
+  });
+
+  it("is case-insensitive for modifiers", () => {
+    const result = parseTemplateArgs("{{D:foo|Required|MULTILINE}}");
+    expect(result[0].required).toBe(true);
+    expect(result[0].multiline).toBe(true);
+  });
+
+  it("parses multiple distinct args", () => {
+    const result = parseTemplateArgs(
+      "Deploy {{D:service}} to {{D:environment|required}}"
+    );
+    expect(result).toHaveLength(2);
+    expect(result[0].name).toBe("service");
+    expect(result[1].name).toBe("environment");
+    expect(result[1].required).toBe(true);
+  });
+
+  it("deduplicates args by key (case-insensitive)", () => {
+    const result = parseTemplateArgs("First: {{D:Name}}, second: {{D:name}}");
+    expect(result).toHaveLength(1);
+    expect(result[0].name).toBe("Name");
+    expect(result[0].key).toBe("name");
+  });
+
+  it("merges modifiers across duplicates — required wins if either has it", () => {
+    const result = parseTemplateArgs("{{D:input}} then {{D:input|required}}");
+    expect(result).toHaveLength(1);
+    expect(result[0].required).toBe(true);
+  });
+
+  it("merges modifiers across duplicates — multiline wins if either has it", () => {
+    const result = parseTemplateArgs("{{D:notes|multiline}} then {{D:notes}}");
+    expect(result).toHaveLength(1);
+    expect(result[0].multiline).toBe(true);
+  });
+
+  it("keeps the first placeholder for duplicates", () => {
+    const result = parseTemplateArgs("{{D:x|required}} and {{D:x|multiline}}");
+    expect(result).toHaveLength(1);
+    expect(result[0].placeholder).toBe("{{D:x|required}}");
+    expect(result[0].required).toBe(true);
+    expect(result[0].multiline).toBe(true);
+  });
+
+  it("skips arg specs that are empty after trimming", () => {
+    const result = parseTemplateArgs("{{D: }} normal {{D:name}}");
+    expect(result).toHaveLength(1);
+    expect(result[0].name).toBe("name");
+  });
+
+  it("does not match {{D:}} (no content between delimiters)", () => {
+    const result = parseTemplateArgs("{{D:}} normal {{D:name}}");
+    expect(result).toHaveLength(1);
+    expect(result[0].name).toBe("name");
+  });
+
+  it("trims whitespace in arg names", () => {
+    const result = parseTemplateArgs("{{D: spaced }}");
+    expect(result).toHaveLength(1);
+    expect(result[0].name).toBe("spaced");
+    expect(result[0].key).toBe("spaced");
+  });
+});
+
+describe("substituteArgs", () => {
+  it("replaces placeholders with provided values", () => {
+    const result = substituteArgs("Deploy {{D:service}} to {{D:env}}", {
+      service: "api",
+      env: "prod",
+    });
+    expect(result).toBe("Deploy api to prod");
+  });
+
+  it("replaces by key (lowercase match)", () => {
+    const result = substituteArgs("Hello {{D:Name}}", { name: "world" });
+    expect(result).toBe("Hello world");
+  });
+
+  it("replaces by original name when key doesn't match", () => {
+    const result = substituteArgs("Hello {{D:Name}}", { Name: "world" });
+    expect(result).toBe("Hello world");
+  });
+
+  it("replaces missing optional args with empty string", () => {
+    const result = substituteArgs("Hello {{D:name}} {{D:suffix}}", {
+      name: "world",
+    });
+    expect(result).toBe("Hello world ");
+  });
+
+  it("throws when a required arg is missing", () => {
+    expect(() => substituteArgs("Deploy {{D:target|required}}", {})).toThrow(
+      "Missing required template arguments: target"
+    );
+  });
+
+  it("throws listing all missing required args", () => {
+    expect(() =>
+      substituteArgs("{{D:a|required}} {{D:b|required}} {{D:c}}", {})
+    ).toThrow("Missing required template arguments: a, b");
+  });
+
+  it("does not throw when required arg is provided", () => {
+    const result = substituteArgs("Deploy {{D:target|required}}", {
+      target: "prod",
+    });
+    expect(result).toBe("Deploy prod");
+  });
+
+  it("handles multiple occurrences of the same placeholder", () => {
+    const result = substituteArgs("{{D:x}} and {{D:x}} again", { x: "val" });
+    expect(result).toBe("val and val again");
+  });
+
+  it("handles prompt with no placeholders", () => {
+    const result = substituteArgs("No args here", {});
+    expect(result).toBe("No args here");
+  });
+
+  it("replaces whitespace-only arg specs with empty string", () => {
+    const result = substituteArgs("Before {{D: }} after", {});
+    expect(result).toBe("Before  after");
+  });
+
+  it("leaves {{D:}} untouched (regex requires content)", () => {
+    const result = substituteArgs("Before {{D:}} after", {});
+    expect(result).toBe("Before {{D:}} after");
+  });
+});