Skip to content

fix(agent-memory-sync): support lone-value cron step form a/n as a-max/n#62

Merged
LanNguyenSi merged 2 commits into
masterfrom
fix/cron-lone-value-step-form
Jul 5, 2026
Merged

fix(agent-memory-sync): support lone-value cron step form a/n as a-max/n#62
LanNguyenSi merged 2 commits into
masterfrom
fix/cron-lone-value-step-form

Conversation

@LanNguyenSi

Copy link
Copy Markdown
Owner

Problem

In packages/agent-memory-sync/src/memory-sync/scheduler.ts, parsePart's step branch routed a lone range token (5 in 5/2) through parseRange, whose missing end token failed parseInteger(undefined, ...) and threw the misleading cron value 'undefined' is outside the allowed range. Standard Vixie cron reads 5/2 on minutes as 5-59/2.

Fix (AC option a)

The step branch now distinguishes a dashed range token (a-b/n, existing path, unchanged) from a lone-value token, parsed as a single integer and filled from start to the field's max: a/n == a-max/n. 5/2 on minutes yields {5,7,9,...,59}; per-field max honored.

The step<=0 guard (#61) is untouched: parseStep runs before the branch dispatch, so 5/0 still throws before reaching the new path.

Tests

  • Flipped the 5/2 pinning test (throws -> valid) and added membership 5/2 -> {5,7,...,59}, edge 20/10 hours -> {20}, and a 5/0 rejection test (defense-in-depth for the fix(agent-memory-sync): reject cron step<=0 to stop scheduler infinite-loop hang (+ coverage) #61 infinite-loop guard on the new branch).
  • Mutation-verified: reverting the new branch fails the 3 flipped/new tests.
  • */n and a-b/n untouched (existing tests green). typecheck clean; 52/52 tests; coverage gate green (scheduler.ts 100% lines).

Reviewed (accept-with-nits, LOW only).

Refs: c1117b17-ef8f-4b2c-b2b3-4aecf8c5f469

nguyen-si-pp and others added 2 commits July 5, 2026 08:54
…-max/n'

Standard Vixie cron reads 5/2 on minutes as 5-59/2 (start at 5, step by
2 through the rest of the field range). The parser's step-match branch
previously routed the lone range token through parseRange, whose
missing end token failed parseInteger(undefined, ...) with a
misleading "'undefined' ... outside range" error. Add a branch that
treats a step token without a dash as start-max, matching how */n and
a-b/n are already handled.

Refs: c1117b17-ef8f-4b2c-b2b3-4aecf8c5f469
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The new a/n lone-value branch reuses parseStep, same as */n and a-b/n,
so step 0 must still be rejected. Add a targeted test since the
existing */0 test only pins the shared guard, not this specific form.

Refs: c1117b17-ef8f-4b2c-b2b3-4aecf8c5f469
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@LanNguyenSi LanNguyenSi merged commit f48ec1d into master Jul 5, 2026
3 of 4 checks passed
@LanNguyenSi LanNguyenSi added review:tests-pass merge-approval gate prerequisite review:checklist-complete merge-approval gate prerequisite review:comments-resolved merge-approval gate prerequisite review:scope-matches-task merge-approval gate prerequisite review:evidence-logged merge-approval gate prerequisite labels Jul 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review:checklist-complete merge-approval gate prerequisite review:comments-resolved merge-approval gate prerequisite review:evidence-logged merge-approval gate prerequisite review:scope-matches-task merge-approval gate prerequisite review:tests-pass merge-approval gate prerequisite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants