duly_catalog_item has no recurring_needs_frequency-equivalent validation

[os-warren]

Found while implementing #61.

duly_duty has recurring_needs_frequency (a script validation refusing form == "recurring" && isBlank(frequency)). duly_catalog_item has never had an equivalent rule — before #61 this was mostly masked by frequency's unconditional default: true option marker always filling in 'monthly' on insert, but the gap is real: an UPDATE that explicitly blanks a recurring catalog item's frequency (e.g. { frequency: null }) has nothing to refuse it, since applyFieldDefaults only runs on INSERT.

This matters more than a bare schema gap because of applyCatalogHandler (src/actions/catalog.handlers.ts): it copies frequency (among the other cadence fields) from the catalog item onto every new duly_duty verbatim. A recurring catalog item quietly left with a blank frequency would replicate that blank onto every person who takes the role — each of whom would then individually trip duly_duty's own recurring_needs_frequency on their first save, rather than the org catching the problem once, at the source, on the catalog item itself.

Not fixed as part of #61 — that issue's adjudicated scope was specifically the standing-duty-carries-a-frequency defect (the converse direction), not this pre-existing, independent "recurring requires a frequency" gap on the catalog object. Also worth deciding at the same time: duly_catalog_item has no effective_*-window equivalent either (N/A — it has no effective_from/effective_to fields at all), so this issue is scoped to recurring_needs_frequency only.

Suggested fix

Add a recurring_needs_frequency validation to duly_catalog_item (src/objects/catalog-item.object.ts), mirroring duly_duty's wording, alongside the standing_no_frequency / non_recurring_no_due_timing / standing_no_grace_days rules #61 adds there.

[os-warren]

Claiming this issue.

• Session: session_01SqkTcrxUFci7nqXdbBSe2p
• Branch: claude/issue-65-catalog-item-cadence-rules

Plan: mirror duly_duty's cadence validation reasoning onto duly_catalog_item, and cover the applyCatalogHandler path (not only direct writes on the object).

---

Generated by Claude Code

[os-warren]

{
 "issue": 65,
 "status": "done",
 "branch": "claude/issue-65-catalog-item-cadence-rules",
 "pr": "https://github.com/objectstack-ai/duly/pull/81",
 "premise_still_valid": true,
 "summary": "Added `recurring_needs_frequency` to `duly_catalog_item` (src/objects/catalog-item.object.ts), mirroring duly_duty's rule and message verbatim — metadata only, one validations[] entry, no handler. Premise verified against origin/main before writing: the three #61 rules were there and this one was not, and an UPDATE blanking a recurring item's frequency was measurably unrefused. Two details of the issue's narrative came out differently under measurement and are documented in the code and PR rather than papered over: (1) INSERT could never reach the gap, because applyFieldDefaults reads an explicit `frequency: null` as absent and re-stamps the CEL default, so UPDATE is the only write that produces the state; (2) the downstream half of the issue's story does not happen — applyCatalogHandler's duty insert hits the SAME default-masking, so a blank fans out as N duties silently stamped 'monthly' rather than N duties each tripping duly_duty's own rule. That makes the catalog item the last place the blank can be caught, which strengthens the card rather than weakening it. No changeset: this repo has no changeset mechanism (no .changeset/ in git log --all, no @changesets/* dep, no script, no CI step, no mention in AGENTS.md), so creating one would mint a mechanism nothing reads — I reached this independently and the coordinator confirmed it mid-task, so nothing was created and nothing needed deleting.",
 "tests": "Gate union run AFTER the final commit, at `1f0aa43` (git rev-parse --short HEAD), all four through the shared verify lock: `pnpm validate && pnpm typecheck && pnpm test && pnpm build`. Gate-printed verdict lines: `✓ Validation passed (413ms)` · `Test Files 21 passed (21)` · `Tests 545 passed (545)` · `✓ Build complete (614ms)`, with `os-verify-lock: VERDICT command-exit 0 · held the lock 22s · waited 0s`. Exit status read from the wrapper's verdict line, never from a `$?` after a pipe. New suite test/catalog-apply-cadence.test.ts: 4 passed, driving duly_catalog_apply through the app's own data.executeAction against a booted in-memory kernel. ABLATION (reverse verification): the rule was deleted from the committed tree by a script carrying `trap '<git checkout -- …>' EXIT INT TERM`. No rebuild leg applies — these suites import src/ directly and their artifactPath is deliberately unresolvable, so there is no dist/ for a mutation to fail to reach. Mutation confirmed ON DISK before any reading, anchored on the text being removed: grep -c \"name: 'recurring_needs_frequency'\" went 1 -> 0 and the file went 8127 -> 6311 bytes; the script exits 90 rather than measuring if either check fails. Result on the ablated tree: `Tests 2 failed | 30 passed (32)`, the two red being exactly the rule-dependent ones (`refuses an update that blanks frequency on a still-recurring catalog item`, `is declared on both objects under one name with one message`), while `pnpm validate` stayed GREEN — there is no author-time gate for this gap, which is why the tests carry the whole claim. The apply-path suite stayed green under ablation by design: it characterises the path the rule protects, not the rule. Restore confirmed by the trap (rule_count=1, `git status --short` empty). TWO EARLIER ABLATION ATTEMPTS WERE DECLARED VOID rather than quietly re-run to a green: the first missed its anchor (the rule is preceded by a comment block) and the on-disk guard caught it with nothing measured; the second substituted a placeholder rule that broke stack load, so both suites reported `Tests no tests` — a load failure, not an assertion result. Only the third, a clean deletion, produced a reading. Measurements behind the code comments, all on a booted kernel with @objectstack/runtime 17.2.0: insert with explicit `frequency: null` on a recurring item comes back `'monthly'`; update to `{ frequency: null }` was unrefused before the rule and now throws `{ code: 'VALIDATION_FAILED', fields: [{ field: '_record', code: 'rule_violation' }] }` with duly_duty's exact message — the in-process throw carries no `status` (that is added at the HTTP boundary), so `code` plus the exact message is the whole envelope available on this path and is what the tests pin.",
 "open_questions": [
 {
 "question": "#79 (filed below) makes `duly_catalog_apply` a no-op through the real dispatcher — it finds no catalog items and reports a successful run of zero. #5 calls that action 'the onboarding path, and the single biggest adoption risk in the product'. It is filed unassigned with no labels; how should it be triaged relative to the rest of the M1 queue?",
 "options": [
 "A — treat it as top of the M1 dispatch queue: the headline onboarding action is dead in production today, every existing assertion about it passes against a query shape production never produces, and the fix is small (pass the filter flat in four engine.find calls, plus the FakeEngine in test/catalog-instantiate.test.ts).",
 "B — queue it normally behind the current batch: nothing in production is calling the action yet (it has no UI home per #27, and reachability was still being decided as recently as #71), so the user-visible cost today is zero.",
 "C — split it: fix the app side now, and raise the contract half upstream separately (spec's ActionEngineFacade.find declares `query: Record<string, unknown>` and never says whether it is a filter or an envelope — the runtime's implementation is the only thing that decides, and this app read it the other way)."
 ],
 "recommendation": "A, with C's upstream half filed alongside rather than instead. The cost of leaving it is not just the dead action: the suite that covers it is green against the wrong convention, so every future change to those handlers is being verified against a shape that cannot occur — and the next application to read that spec type will make the same choice this one did. The fix is mechanical and the tripwire in test/catalog-apply-cadence.test.ts already marks the spot."
 }
 ],
 "out_of_scope_findings": [
 "filed as #79: duly_catalog_apply finds no catalog items through the real action dispatcher — the handler passes an ObjectQL envelope `{ where: … }` to ctx.engine.find while the runtime's buildActionEngineFacade wraps its argument in a `where` of its own, so the read becomes `{ where: { where: … } }`, returns empty with no error, and the action reports `{ created: 0 }`. Measured both ways (runtime facade -> created 0; flat facade -> created 1) and pinned as a tripwire in the last describe of test/catalog-apply-cadence.test.ts, written to be deleted when #79 lands. Same mechanism also silences syncCatalogHandler's duty read and resolveBusinessUnit's position read. Unassigned, no labels, left for PM triage."
 ]
}

---

Generated by Claude Code

[os-warren]

os-dev-report

Supersedes the previous comment on this issue: GitHub's body sanitizer ate its leading <!-- os-dev-report --> marker (making it invisible to the PM scan) and two angle-bracketed fragments inside the JSON — trap '<git checkout …>' EXIT INT TERM came out as trap '', which misreports the ablation, and a TypeScript generic came out bare. This copy is the same report with those phrases written without angle brackets. Ignore the earlier one.

{
 "issue": 65,
 "status": "done",
 "branch": "claude/issue-65-catalog-item-cadence-rules",
 "pr": "https://github.com/objectstack-ai/duly/pull/81",
 "premise_still_valid": true,
 "summary": "Added `recurring_needs_frequency` to `duly_catalog_item` (src/objects/catalog-item.object.ts), mirroring duly_duty's rule and message verbatim — metadata only, one validations[] entry, no handler. Premise verified against origin/main before writing: the three #61 rules were there and this one was not, and an UPDATE blanking a recurring item's frequency was measurably unrefused. Two details of the issue's narrative came out differently under measurement and are documented in the code and PR rather than papered over: (1) INSERT could never reach the gap, because applyFieldDefaults reads an explicit `frequency: null` as absent and re-stamps the CEL default, so UPDATE is the only write that produces the state; (2) the downstream half of the issue's story does not happen — applyCatalogHandler's duty insert hits the SAME default-masking, so a blank fans out as N duties silently stamped 'monthly' rather than N duties each tripping duly_duty's own rule. That makes the catalog item the last place the blank can be caught, which strengthens the card rather than weakening it. No changeset: this repo has no changeset mechanism (no .changeset/ in git log --all, no @changesets/* dep, no script, no CI step, no mention in AGENTS.md), so creating one would mint a mechanism nothing reads — I reached this independently and the coordinator confirmed it mid-task, so nothing was created and nothing needed deleting.",
 "tests": "Gate union run AFTER the final commit, at `1f0aa43` (git rev-parse --short HEAD), all four through the shared verify lock: `pnpm validate && pnpm typecheck && pnpm test && pnpm build`. Gate-printed verdict lines: `✓ Validation passed (413ms)` · `Test Files 21 passed (21)` · `Tests 545 passed (545)` · `✓ Build complete (614ms)`, with `os-verify-lock: VERDICT command-exit 0 · held the lock 22s · waited 0s`. Exit status read from the wrapper's verdict line, never from a `$?` after a pipe. New suite test/catalog-apply-cadence.test.ts: 4 passed, driving duly_catalog_apply through the app's own data.executeAction against a booted in-memory kernel. ABLATION (reverse verification): the rule was deleted from the committed tree by a script whose EXIT/INT/TERM trap restores the file with `git checkout --`. No rebuild leg applies — these suites import src/ directly and their artifactPath is deliberately unresolvable, so there is no dist/ for a mutation to fail to reach. Mutation confirmed ON DISK before any reading, anchored on the text being removed: grep -c for the rule name went 1 -> 0 and the file went 8127 -> 6311 bytes; the script exits 90 rather than measuring if either check fails. Result on the ablated tree: `Tests 2 failed | 30 passed (32)`, the two red being exactly the rule-dependent ones (`refuses an update that blanks frequency on a still-recurring catalog item`, `is declared on both objects under one name with one message`), while `pnpm validate` stayed GREEN — there is no author-time gate for this gap, which is why the tests carry the whole claim. The apply-path suite stayed green under ablation by design: it characterises the path the rule protects, not the rule. Restore confirmed by the trap (rule_count=1, `git status --short` empty). TWO EARLIER ABLATION ATTEMPTS WERE DECLARED VOID rather than quietly re-run to a green: the first missed its anchor (the rule is preceded by a comment block) and the on-disk guard caught it with nothing measured; the second substituted a placeholder rule that broke stack load, so both suites reported `Tests no tests` — a load failure, not an assertion result. Only the third, a clean deletion, produced a reading. Measurements behind the code comments, all on a booted kernel with @objectstack/runtime 17.2.0: insert with explicit `frequency: null` on a recurring item comes back `'monthly'`; update to `{ frequency: null }` was unrefused before the rule and now throws `{ code: 'VALIDATION_FAILED', fields: [{ field: '_record', code: 'rule_violation' }] }` with duly_duty's exact message — the in-process throw carries no `status` (that is added at the HTTP boundary), so `code` plus the exact message is the whole envelope available on this path and is what the tests pin.",
 "open_questions": [
 {
 "question": "#79 (filed below) makes `duly_catalog_apply` a no-op through the real dispatcher — it finds no catalog items and reports a successful run of zero. #5 calls that action 'the onboarding path, and the single biggest adoption risk in the product'. It is filed unassigned with no labels; how should it be triaged relative to the rest of the M1 queue?",
 "options": [
 "A — treat it as top of the M1 dispatch queue: the headline onboarding action is dead in production today, every existing assertion about it passes against a query shape production never produces, and the fix is small (pass the filter flat in four engine.find calls, plus the FakeEngine in test/catalog-instantiate.test.ts).",
 "B — queue it normally behind the current batch: nothing in production is calling the action yet (it has no UI home per #27, and reachability was still being decided as recently as #71), so the user-visible cost today is zero.",
 "C — split it: fix the app side now, and raise the contract half upstream separately (spec's ActionEngineFacade.find types its query parameter as a plain record of string to unknown and never says whether it is a filter or an envelope — the runtime's implementation is the only thing that decides, and this app read it the other way)."
 ],
 "recommendation": "A, with C's upstream half filed alongside rather than instead. The cost of leaving it is not just the dead action: the suite that covers it is green against the wrong convention, so every future change to those handlers is being verified against a shape that cannot occur — and the next application to read that spec type will make the same choice this one did. The fix is mechanical and the tripwire in test/catalog-apply-cadence.test.ts already marks the spot."
 }
 ],
 "out_of_scope_findings": [
 "filed as #79: duly_catalog_apply finds no catalog items through the real action dispatcher — the handler passes an ObjectQL envelope `{ where: … }` to ctx.engine.find while the runtime's buildActionEngineFacade wraps its argument in a `where` of its own, so the read becomes `{ where: { where: … } }`, returns empty with no error, and the action reports `{ created: 0 }`. Measured both ways (runtime facade -> created 0; flat facade -> created 1) and pinned as a tripwire in the last describe of test/catalog-apply-cadence.test.ts, written to be deleted when #79 lands. Same mechanism also silences syncCatalogHandler's duty read and resolveBusinessUnit's position read. Unassigned, no labels, left for PM triage."
 ]
}

---

Generated by Claude Code