fix(agent-memory-sync): reject cron step<=0 to stop scheduler infinite-loop hang (+ coverage) (#61)

* fix(agent-memory-sync): reject cron step<=0 to stop scheduler hang

parsePart validated a step token against the field range via
parseInteger(stepToken, min, max). For fields with min=0
(minute/hour/dayOfWeek) step=0 was accepted, and fillRange(result, min,
max, 0) then looped forever (cursor += 0) — a typo'd operator cron
string like '*/0 * * * *' hung the sync daemon unbounded.

Validate the step independently as an integer >= 1 (new parseStep), with
no upper bound: a step larger than the range simply collapses to the
start value in a single terminating fillRange iteration. Adds tests for
the rejected '*/0', the now-valid range-independent '*/90', and pins the
existing throw for the unsupported lone-value 'a/n' step form.

Refs: agent-memory task 124c89e0

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(memory-router): assert default embed timeout is wired to fetch

The embed-provider suite proved an abort throws but never that the
DEFAULT_TIMEOUT_MS (5000) is the value handed to AbortSignal.timeout and
that its signal reaches fetch. Spy over the real AbortSignal.timeout
(call-through) to pin the 5000ms default and the explicit-override
branch of `opts.timeoutMs ?? DEFAULT_TIMEOUT_MS`.

Refs: agent-memory task 124c89e0

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(memory-digest-cli): exercise the real generate action body

The generate suite replaced the action with a spy, so the real body
(scan → extract → generate → format → write/print + catch→exit) had ~0%
coverage. Add integration tests driving the real registerGenerateCommand
action against today-dated temp-dir fixtures: markdown --output write,
--json --output write (also parsing non-default --days/--max), stdout
print when --output is omitted, and the catch/process.exit(1) branch via
an unwritable --output path. Stubs console.log/error and process.exit
with save/restore. Tightens the tautological description assertion to
the exact string.

Refs: agent-memory task 124c89e0

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Lan Nguyen Si <nguyen-si@publicplan.de>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: 3 people

packages/agent-memory-sync/src/memory-sync/scheduler.ts

@@ -65,7 +65,7 @@ function parsePart(value: string, min: number, max: number): Set<number> {
     const stepMatch = /^(\*|\d+(?:-\d+)?)\/(\d+)$/.exec(segment);
     if (stepMatch) {
       const [, rangeToken, stepToken] = stepMatch;
-      const step = parseInteger(stepToken, min, max);
+      const step = parseStep(stepToken);
       if (rangeToken === "*") {
         fillRange(result, min, max, step);
       } else {
@@ -109,6 +109,20 @@ function parseInteger(value: string, min: number, max: number): number {
   return parsed;
 }
 
+// A step must be a positive integer, validated independently of the field's
+// min/max. Fields with min=0 (minute/hour/dayOfWeek) formerly accepted step=0
+// via parseInteger, and fillRange(..., 0) then looped forever (cursor += 0),
+// hanging the sync daemon on a typo'd cron string like '*/0 * * * *'. There is
+// deliberately no upper bound: a step larger than the range simply collapses to
+// the start value, which fillRange handles in a single terminating iteration.
+function parseStep(value: string): number {
+  const parsed = Number(value);
+  if (!Number.isInteger(parsed) || parsed < 1) {
+    throw new CliError(`cron step '${value}' must be a positive integer (>= 1).`, 2);
+  }
+  return parsed;
+}
+
 function fillRange(target: Set<number>, start: number, end: number, step: number): void {
   for (let cursor = start; cursor <= end; cursor += step) {
     target.add(cursor);

packages/agent-memory-sync/tests/unit/scheduler.test.ts

@@ -88,6 +88,30 @@ test("parseCron: invalid — range with start > end throws CliError", () => {
   assert.throws(() => validateCronExpression("5-3 * * * *"), /invalid/);
 });
 
+test("parseCron: invalid — step */0 is rejected (guards the infinite-loop hang)", () => {
+  // Regression guard. Formerly '*/0' passed parseInteger for min=0 fields
+  // (minute/hour/dayOfWeek), then fillRange(result, min, max, 0) looped forever
+  // (cursor += 0), hanging the sync daemon on a typo'd cron string. The step is
+  // now validated as an integer >= 1, independent of the field range. If the
+  // guard is removed this assertion never returns — the call hangs (bounded only
+  // by the CI job timeout), which still fails the run rather than passing.
+  assert.throws(() => validateCronExpression("*/0 * * * *"), /must be a positive integer/);
+});
+
+test("parseCron: step larger than the field range is valid (step is range-independent)", () => {
+  // '*/90' on minutes: the step exceeds max=59, so only minute 0 matches — this
+  // is valid, not an error. Formerly parseInteger(stepToken, 0, 59) rejected any
+  // step > 59; the fix decouples the step from the field's min/max on purpose.
+  assert.doesNotThrow(() => validateCronExpression("*/90 * * * *"));
+});
+
+test("parseCron: single-value step form 'a/n' (e.g. 5/2) throws — lone-value step unsupported", () => {
+  // Standard cron reads 5/2 as 5-max/2, but this parser routes the lone range
+  // token through parseRange, whose missing end token fails parseInteger. Pin the
+  // current behavior: it throws (rather than silently misbehaving or hanging).
+  assert.throws(() => validateCronExpression("5/2 * * * *"), /outside the allowed range/);
+});
+
 test("parseCron: invalid — non-numeric field value throws CliError", () => {
   assert.throws(() => validateCronExpression("* * * * abc"), /outside the allowed range/);
 });

packages/memory-digest-cli/tests/generate.test.ts

@@ -9,6 +9,9 @@
 
 import { test } from "node:test";
 import assert from "node:assert/strict";
+import * as fs from "node:fs/promises";
+import * as os from "node:os";
+import * as path from "node:path";
 import { Command } from "commander";
 import { registerGenerateCommand } from "../src/commands/generate.js";
 
@@ -35,11 +38,11 @@ test("registerGenerateCommand: registers a subcommand named 'generate'", () => {
   assert.equal(generateCmd.name(), "generate");
 });
 
-test("registerGenerateCommand: subcommand has a non-empty description", () => {
+test("registerGenerateCommand: subcommand has the expected description", () => {
   const { generateCmd } = buildProgram();
-  assert.ok(
-    generateCmd.description().length > 0,
-    "description should not be empty"
+  assert.equal(
+    generateCmd.description(),
+    "Generate a memory digest from markdown files"
   );
 });
 
@@ -154,3 +157,167 @@ test("generate: all non-default flags can be passed together", async () => {
   assert.equal(capturedOptions!.recursive, true);
   assert.equal(capturedOptions!.json, true);
 });
+
+// ─── real-action integration tests ──────────────────────────────────────────
+//
+// The tests above all replace the action handler with a spy, so the real
+// action body in generate.ts (scan → extract → generate → format →
+// write/print, plus the catch/process.exit path) is never exercised. The
+// tests below run the REAL action registered by registerGenerateCommand
+// against real temp-dir fixtures on disk.
+//
+// The scanner only matches files literally named `YYYY-MM-DD.md` and filters
+// by the date encoded in the filename (not mtime), so fixtures must use
+// today's date computed at test-run time -- a hardcoded date would drift out
+// of the default 7-day window and start failing later.
+
+function todayFileName(): string {
+  return `${new Date().toISOString().slice(0, 10)}.md`;
+}
+
+// Sentinel thrown by the stubbed process.exit so the real action's async
+// control flow stops exactly where the real process.exit would have
+// terminated the process, without actually killing the test runner.
+class ProcessExitSentinel extends Error {
+  code: number | undefined;
+  constructor(code: number | undefined) {
+    super(`process.exit called with code ${code}`);
+    this.code = code;
+  }
+}
+
+// Runs `fn` with console.log/console.error/process.exit stubbed out, and
+// returns what was captured. console.log calls are recorded (the action's
+// stdout print branch uses it); console.error is silenced (the action logs
+// progress and error messages through it); process.exit is replaced with a
+// spy that records the exit code and throws ProcessExitSentinel so the
+// action's control flow halts the same way a real process.exit would, then
+// swallows that specific sentinel so the test doesn't fail on it.
+async function withStubbedIO(
+  fn: () => Promise<void>
+): Promise<{ logs: unknown[][]; exitCode: number | undefined; exitCalled: boolean }> {
+  const originalLog = console.log;
+  const originalError = console.error;
+  const originalExit = process.exit;
+
+  const logs: unknown[][] = [];
+  let exitCode: number | undefined;
+  let exitCalled = false;
+
+  console.log = (...args: unknown[]) => {
+    logs.push(args);
+  };
+  console.error = () => {};
+  process.exit = ((code?: number) => {
+    exitCalled = true;
+    exitCode = code;
+    throw new ProcessExitSentinel(code);
+  }) as typeof process.exit;
+
+  try {
+    await fn();
+  } catch (error) {
+    if (!(error instanceof ProcessExitSentinel)) {
+      throw error;
+    }
+  } finally {
+    console.log = originalLog;
+    console.error = originalError;
+    process.exit = originalExit;
+  }
+
+  return { logs, exitCode, exitCalled };
+}
+
+test("generate: real action writes a markdown digest to --output", async () => {
+  const { program } = buildProgram();
+  const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "generate-real-md-"));
+  await fs.writeFile(
+    path.join(tmpDir, todayFileName()),
+    "# Today\n\n- Decided to adopt the new digest format\n- Fixed a bug in the extractor\n",
+    "utf-8"
+  );
+  const outFile = path.join(tmpDir, "out.md");
+
+  const { exitCalled } = await withStubbedIO(async () => {
+    await program.parseAsync(["generate", "--dir", tmpDir, "--output", outFile], {
+      from: "user",
+    });
+  });
+
+  assert.equal(exitCalled, false, "process.exit must not be called on success");
+  const content = await fs.readFile(outFile, "utf-8");
+  assert.ok(content.length > 0, "output file should be non-empty");
+});
+
+test("generate: real action writes valid JSON to --output with --json (and parses --days/--max)", async () => {
+  const { program } = buildProgram();
+  const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "generate-real-json-"));
+  await fs.writeFile(
+    path.join(tmpDir, todayFileName()),
+    "# Today\n\n- Learned something new\n- Shipped a fix\n",
+    "utf-8"
+  );
+  const outFile = path.join(tmpDir, "out.json");
+
+  const { exitCalled } = await withStubbedIO(async () => {
+    await program.parseAsync(
+      [
+        "generate",
+        "--dir", tmpDir,
+        "--output", outFile,
+        "--json",
+        "--days", "30",
+        "--max", "5",
+      ],
+      { from: "user" }
+    );
+  });
+
+  assert.equal(exitCalled, false, "process.exit must not be called on success");
+  const content = await fs.readFile(outFile, "utf-8");
+  assert.doesNotThrow(() => JSON.parse(content), "written file must be valid JSON");
+});
+
+test("generate: real action prints the digest to stdout when --output is omitted", async () => {
+  const { program } = buildProgram();
+  const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "generate-real-stdout-"));
+  await fs.writeFile(
+    path.join(tmpDir, todayFileName()),
+    "# Today\n\n- Did a thing worth remembering\n",
+    "utf-8"
+  );
+
+  const { logs, exitCalled } = await withStubbedIO(async () => {
+    await program.parseAsync(["generate", "--dir", tmpDir], { from: "user" });
+  });
+
+  assert.equal(exitCalled, false, "process.exit must not be called on success");
+  assert.equal(logs.length, 1, "console.log should be called exactly once with the digest");
+  assert.ok(
+    typeof logs[0][0] === "string" && logs[0][0].length > 0,
+    "printed digest should be a non-empty string"
+  );
+});
+
+test("generate: real action calls process.exit(1) when --output cannot be written", async () => {
+  const { program } = buildProgram();
+  const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "generate-real-exit-"));
+  await fs.writeFile(
+    path.join(tmpDir, todayFileName()),
+    "# Today\n\n- A note\n",
+    "utf-8"
+  );
+  // Parent directory of the output path does not exist, so fs.writeFile
+  // rejects with ENOENT and the action's catch block must run.
+  const badOutput = path.join(tmpDir, "does-not-exist-subdir", "out.md");
+
+  const { exitCode, exitCalled } = await withStubbedIO(async () => {
+    await program.parseAsync(["generate", "--dir", tmpDir, "--output", badOutput], {
+      from: "user",
+    });
+  });
+
+  assert.equal(exitCalled, true, "process.exit should have been called");
+  assert.equal(exitCode, 1, "process.exit should be called with code 1");
+});

packages/memory-router/tests/unit/embed-provider.test.ts

@@ -240,6 +240,78 @@ test('embedBatch: res.text() failure in non-ok path falls back to "<no body>"',
   }
 });
 
+test('embedBatch: omitted timeoutMs wires the DEFAULT_TIMEOUT_MS (5000) into AbortSignal.timeout', async () => {
+  // Not just "an abort throws" — pin that the 5000ms default is the value passed
+  // to AbortSignal.timeout and that its signal reaches fetch. Spies over the real
+  // AbortSignal.timeout (call-through so fetch's init stays well-formed) and fetch.
+  const origFetch = (globalThis as { fetch?: typeof fetch }).fetch;
+  const origTimeout = AbortSignal.timeout;
+  let capturedTimeoutArg: number | undefined;
+  let capturedSignal: AbortSignal | undefined;
+
+  try {
+    (AbortSignal as { timeout: (ms: number) => AbortSignal }).timeout = (ms: number) => {
+      capturedTimeoutArg = ms;
+      return origTimeout.call(AbortSignal, ms);
+    };
+    (globalThis as { fetch: typeof fetch }).fetch = (async (_url: string, init?: RequestInit) => {
+      capturedSignal = init?.signal ?? undefined;
+      return {
+        ok: true,
+        status: 200,
+        statusText: 'OK',
+        json: async () => ({ data: [] }),
+        text: async () => '',
+      } as unknown as Response;
+    }) as unknown as typeof fetch;
+
+    await embedBatch({ apiKey: 'k', model: 'm', inputs: ['x'] }); // timeoutMs omitted
+
+    assert.equal(capturedTimeoutArg, 5000, 'default embed timeout must be 5000ms');
+    assert.ok(capturedSignal instanceof AbortSignal, 'the timeout signal must be wired to fetch');
+  } finally {
+    AbortSignal.timeout = origTimeout;
+    if (origFetch) {
+      (globalThis as { fetch: typeof fetch }).fetch = origFetch;
+    } else {
+      delete (globalThis as { fetch?: typeof fetch }).fetch;
+    }
+  }
+});
+
+test('embedBatch: explicit timeoutMs overrides the default', async () => {
+  // Pins both sides of the `opts.timeoutMs ?? DEFAULT_TIMEOUT_MS` branch so a
+  // mutation collapsing it to a constant fails.
+  const origFetch = (globalThis as { fetch?: typeof fetch }).fetch;
+  const origTimeout = AbortSignal.timeout;
+  let capturedTimeoutArg: number | undefined;
+
+  try {
+    (AbortSignal as { timeout: (ms: number) => AbortSignal }).timeout = (ms: number) => {
+      capturedTimeoutArg = ms;
+      return origTimeout.call(AbortSignal, ms);
+    };
+    (globalThis as { fetch: typeof fetch }).fetch = (async () => ({
+      ok: true,
+      status: 200,
+      statusText: 'OK',
+      json: async () => ({ data: [] }),
+      text: async () => '',
+    } as unknown as Response)) as unknown as typeof fetch;
+
+    await embedBatch({ apiKey: 'k', model: 'm', inputs: ['x'], timeoutMs: 1234 });
+
+    assert.equal(capturedTimeoutArg, 1234, 'explicit timeoutMs must win over the default');
+  } finally {
+    AbortSignal.timeout = origTimeout;
+    if (origFetch) {
+      (globalThis as { fetch: typeof fetch }).fetch = origFetch;
+    } else {
+      delete (globalThis as { fetch?: typeof fetch }).fetch;
+    }
+  }
+});
+
 // ─── resolveProviderConfig ───────────────────────────────────────────────────
 
 test('resolveProviderConfig: OPENAI_API_KEY missing → returns null', () => {