From bbd8517d79deabdc0a50b635d6e9b8abcba2312f Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 14 Jun 2026 11:03:52 +0500 Subject: [PATCH 1/2] fix(ADR-0048): rescope os lint naming/namespace-prefix to intra-package duplicates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0048 §3.4 retired the per-item cross-package collision throw, but the naming/namespace-prefix lint rule was never updated to match. It fired on every bare-named UI/automation item (hotcrm: 63 false positives) and claimed the package would "collide on the registry key and fail at install" — both false. The rule now warns only on a genuine intra-package duplicate (type, name) pair within the linted config (the authoring-hygiene case §3.4 leaves to os lint). Unique bare names produce zero warnings. Message no longer claims an install failure. Runtime/registry behavior unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../fix-lint-namespace-prefix-adr0048.md | 26 +++++ packages/cli/src/commands/lint.ts | 100 ++++++++++------- .../cli/test/lint-namespace-prefix.test.ts | 104 ++++++++++++------ 3 files changed, 156 insertions(+), 74 deletions(-) create mode 100644 .changeset/fix-lint-namespace-prefix-adr0048.md diff --git a/.changeset/fix-lint-namespace-prefix-adr0048.md b/.changeset/fix-lint-namespace-prefix-adr0048.md new file mode 100644 index 0000000000..73bf450867 --- /dev/null +++ b/.changeset/fix-lint-namespace-prefix-adr0048.md @@ -0,0 +1,26 @@ +--- +"@objectstack/cli": patch +--- + +fix(ADR-0048): rescope the `os lint` `naming/namespace-prefix` rule to intra-package duplicates + +ADR-0048 §3.4 retired the per-item cross-package collision throw — two +installed packages may legitimately ship the same bare name (e.g. `page/home`), +stored under distinct composite keys and disambiguated by package-scoped +resolution. The `naming/namespace-prefix` lint rule was never updated to match, +so it still: + +- **fired on every bare-named UI/automation item** (apps/pages/dashboards/flows/ + actions/reports/datasets) regardless of whether a duplicate existed — a normal + single-package app got dozens of false positives (hotcrm: 63), and +- **claimed the package would "collide on the registry key and fail at install"**, + which is no longer true. + +The rule now warns **only on a genuine intra-package duplicate `(type, name)` +pair** within the linted config — the narrow authoring-time hygiene case ADR-0048 +§3.4 explicitly leaves to `os lint` ("an author shipping two `page/home` in one +package"). A unique bare name produces zero warnings. The message no longer +claims an install failure; it explains the items shadow each other on the +registry key and that distinct packages may reuse the same name freely (the +namespace prefix is an optional convention). Runtime/registry behavior is +unchanged. diff --git a/packages/cli/src/commands/lint.ts b/packages/cli/src/commands/lint.ts index 5b099b09c2..9af099b3ae 100644 --- a/packages/cli/src/commands/lint.ts +++ b/packages/cli/src/commands/lint.ts @@ -216,48 +216,68 @@ export function lintConfig(config: any): LintIssue[] { } } - // ── Namespace-prefix advisory for bare-named metadata (ADR-0048 §3.3) ── - // Cross-package collision detection (ADR-0048) makes a clash between two - // packages' bare-named items (`page`/`flow`/`action`/…) a loud error at - // registration time. This shifts that left: a non-fatal nudge to prefix - // the name with the package namespace at authoring time, so the clash is - // unlikely to ever happen. Objects are already prefix-*enforced* (error) - // in defineStack; views are object-derived; `doc` has its own build lint - // — so they are excluded here. Warning only — prefix stays a recommended - // convention for legacy types, not a retroactive requirement. + // ── Intra-package duplicate-name advisory (ADR-0048 §3.4) ── + // ADR-0048 §3.4 retired the per-item CROSS-package throw: package ids are + // globally unique, so two installed packages shipping the same bare name + // (e.g. `page/home`) legitimately COEXIST under distinct composite keys and + // each caller resolves to its own via package-scoped resolution. A bare name + // is therefore NOT a collision risk and must not warn on its own. + // + // What the lint still earns its keep on is the narrow authoring-time hygiene + // case the ADR explicitly leaves to `os lint`: "an author shipping two + // `page/home` in one package". Two items of the same (type, name) declared + // within ONE package's config share a single composite registry key and + // shadow each other (last-write-wins). We only see one package's config here, + // so the legitimate signal is a genuine duplicate `(type, name)` pair within + // it — never a unique bare name. + // + // Objects are already prefix-*enforced* (error) in defineStack; views are + // object-derived; `doc` has its own build lint — so they are excluded here. const ns: string | undefined = config.manifest?.namespace; - if (ns) { - // A name is namespace-safe if it IS the namespace (ADR-0019: a package's - // single app is conventionally named after the namespace, e.g. `crm`), - // is prefixed with it (`crm_lead`), or is a platform-reserved `sys_` name. - const isNamespaceSafe = (name: string) => - name === ns || name.startsWith(`${ns}_`) || name.startsWith('sys_'); - - // Bare-named UI/automation types subject to the cross-package collision - // (ADR-0048 §1.1). Data-driven so a new bare-named type is one line. - const PREFIXED_TYPES: Array<{ key: string; label: string }> = [ - { key: 'apps', label: 'App' }, - { key: 'pages', label: 'Page' }, - { key: 'dashboards', label: 'Dashboard' }, - { key: 'flows', label: 'Flow' }, - { key: 'actions', label: 'Action' }, - { key: 'reports', label: 'Report' }, - { key: 'datasets', label: 'Dataset' }, - ]; - - for (const { key, label } of PREFIXED_TYPES) { - const items: any[] = Array.isArray(config[key]) ? config[key] : []; - for (let i = 0; i < items.length; i++) { - const name = items[i]?.name; - if (typeof name !== 'string' || !name || isNamespaceSafe(name)) continue; - issues.push({ - severity: 'warning', - rule: 'naming/namespace-prefix', - message: `${label} "${name}" is not namespace-prefixed. Two packages defining "${name}" collide on the registry key and fail at install (ADR-0048). Rename to "${ns}_${name}".`, - path: `${key}[${i}].name`, - fix: `${ns}_${name}`, - }); + + // Bare-named UI/automation types that share the generic registry namespace. + // Data-driven so a new bare-named type is one line. + const PREFIXED_TYPES: Array<{ key: string; label: string }> = [ + { key: 'apps', label: 'App' }, + { key: 'pages', label: 'Page' }, + { key: 'dashboards', label: 'Dashboard' }, + { key: 'flows', label: 'Flow' }, + { key: 'actions', label: 'Action' }, + { key: 'reports', label: 'Report' }, + { key: 'datasets', label: 'Dataset' }, + ]; + + for (const { key, label } of PREFIXED_TYPES) { + const items: any[] = Array.isArray(config[key]) ? config[key] : []; + // First occurrence of each name → its index, so a later duplicate can point + // back at the original declaration. + const firstSeen = new Map(); + for (let i = 0; i < items.length; i++) { + const name = items[i]?.name; + if (typeof name !== 'string' || !name) continue; + const original = firstSeen.get(name); + if (original === undefined) { + firstSeen.set(name, i); + continue; } + // Genuine intra-package duplicate: two items of the same (type, name) + // declared in this package's config. They collapse onto one registry key + // and shadow each other. Renaming one with the package namespace prefix + // (`crm_home`) is the simplest fix; any distinct name works. + const suggestion = ns && !name.startsWith(`${ns}_`) ? `${ns}_${name}` : undefined; + issues.push({ + severity: 'warning', + rule: 'naming/namespace-prefix', + message: + `${label} "${name}" is declared more than once in this package ` + + `(also at ${key}[${original}].name). Two items of the same type sharing ` + + `a bare name within one package shadow each other on the registry key ` + + `(ADR-0048 §3.4) — rename one${suggestion ? `, e.g. "${suggestion}"` : ''}. ` + + `Distinct packages may reuse the same name freely; the namespace prefix ` + + `is an optional convention, not a collision-avoidance requirement.`, + path: `${key}[${i}].name`, + ...(suggestion ? { fix: suggestion } : {}), + }); } } diff --git a/packages/cli/test/lint-namespace-prefix.test.ts b/packages/cli/test/lint-namespace-prefix.test.ts index 10ccb7256e..2e04e437e1 100644 --- a/packages/cli/test/lint-namespace-prefix.test.ts +++ b/packages/cli/test/lint-namespace-prefix.test.ts @@ -6,63 +6,99 @@ import { lintConfig } from '../src/commands/lint'; const RULE = 'naming/namespace-prefix'; const prefixIssues = (config: any) => lintConfig(config).filter((i) => i.rule === RULE); -describe('lint — namespace-prefix advisory (ADR-0048 §3.3)', () => { - it('warns on a bare-named page without the namespace prefix', () => { +describe('lint — intra-package duplicate-name advisory (ADR-0048 §3.4)', () => { + it('stays silent on unique bare names — they are NOT a collision (ADR-0048 §3.4)', () => { + // The cross-package throw was retired: distinct packages coexist on the + // same bare name via package-scoped resolution. A single package with + // unique bare names must produce zero warnings (was 63 false positives + // for hotcrm under the old over-broad rule). const issues = prefixIssues({ manifest: { namespace: 'crm' }, - pages: [{ name: 'home', label: 'Home' }], + apps: [{ name: 'crm' }], + pages: [{ name: 'home' }, { name: 'settings' }], + dashboards: [{ name: 'overview' }], + flows: [{ name: 'onboard' }], + actions: [{ name: 'send' }], + reports: [{ name: 'pipeline' }], + datasets: [{ name: 'sales' }], + }); + expect(issues).toHaveLength(0); + }); + + it('warns on an actual intra-package duplicate (type, name) pair', () => { + const issues = prefixIssues({ + manifest: { namespace: 'crm' }, + pages: [{ name: 'home', label: 'Home' }, { name: 'home', label: 'Home 2' }], }); expect(issues).toHaveLength(1); expect(issues[0].severity).toBe('warning'); - expect(issues[0].path).toBe('pages[0].name'); - expect(issues[0].fix).toBe('crm_home'); + // The warning points at the duplicate occurrence, not the first. + expect(issues[0].path).toBe('pages[1].name'); + expect(issues[0].message).toContain('declared more than once'); + expect(issues[0].message).toContain('pages[0].name'); expect(issues[0].message).toContain('ADR-0048'); + // Suggests a namespace-prefixed rename when a namespace is available. + expect(issues[0].fix).toBe('crm_home'); }); - it('accepts a namespace-prefixed name', () => { - expect( - prefixIssues({ manifest: { namespace: 'crm' }, flows: [{ name: 'crm_onboarding' }] }), - ).toHaveLength(0); + it('does not claim the package will fail at install', () => { + const issues = prefixIssues({ + manifest: { namespace: 'crm' }, + flows: [{ name: 'onboard' }, { name: 'onboard' }], + }); + expect(issues).toHaveLength(1); + // ADR-0048 §3.4 retired the per-item cross-package throw — the old + // "collide on the registry key and fail at install" claim is false. + expect(issues[0].message).not.toContain('fail at install'); + expect(issues[0].message).not.toContain('Two packages'); }); - it('exempts an app named after the namespace (ADR-0019 single-app convention)', () => { - // `defineApp({ name: 'crm' })` in namespace `crm` must NOT warn. - expect( - prefixIssues({ manifest: { namespace: 'crm' }, apps: [{ name: 'crm' }] }), - ).toHaveLength(0); + it('detects duplicates per type independently across every bare-named type', () => { + const issues = prefixIssues({ + manifest: { namespace: 'crm' }, + apps: [{ name: 'a' }, { name: 'a' }], + pages: [{ name: 'p' }, { name: 'p' }], + dashboards: [{ name: 'd' }, { name: 'd' }], + flows: [{ name: 'f' }, { name: 'f' }], + actions: [{ name: 'ac' }, { name: 'ac' }], + reports: [{ name: 'r' }, { name: 'r' }], + datasets: [{ name: 'ds' }, { name: 'ds' }], + }); + expect(issues).toHaveLength(7); + expect(new Set(issues.map((i) => i.severity))).toEqual(new Set(['warning'])); }); - it('exempts platform-reserved sys_ names', () => { + it('treats the same name under different types as distinct (no false positive)', () => { + // `page/home` and `flow/home` live under different registry collections — + // they do not collide, so a shared name across types must not warn. expect( - prefixIssues({ manifest: { namespace: 'crm' }, pages: [{ name: 'sys_admin' }] }), + prefixIssues({ + manifest: { namespace: 'crm' }, + pages: [{ name: 'home' }], + flows: [{ name: 'home' }], + }), ).toHaveLength(0); }); - it('covers every bare-named UI/automation type', () => { + it('warns on duplicates even when no namespace is declared (omits the fix)', () => { + // Duplicate names shadow each other regardless of namespace; without a + // namespace there is no prefix to suggest, so no `fix` is offered. const issues = prefixIssues({ - manifest: { namespace: 'crm' }, - apps: [{ name: 'other' }], - pages: [{ name: 'home' }], - dashboards: [{ name: 'overview' }], - flows: [{ name: 'onboard' }], - actions: [{ name: 'send' }], - reports: [{ name: 'pipeline' }], - datasets: [{ name: 'sales' }], + pages: [{ name: 'home' }, { name: 'home' }], }); - expect(issues).toHaveLength(7); - expect(new Set(issues.map((i) => i.severity))).toEqual(new Set(['warning'])); - }); - - it('is silent when the package declares no namespace', () => { - // No manifest.namespace → nothing to prefix against; stays quiet. - expect(prefixIssues({ pages: [{ name: 'home' }] })).toHaveLength(0); + expect(issues).toHaveLength(1); + expect(issues[0].fix).toBeUndefined(); + expect(issues[0].message).toContain('declared more than once'); }); it('does not touch objects (already prefix-enforced as an error) or views', () => { const issues = prefixIssues({ manifest: { namespace: 'crm' }, - objects: [{ name: 'lead', fields: { id: { type: 'text' } } }], - views: [{ name: 'lead.all' }], + objects: [ + { name: 'lead', fields: { id: { type: 'text' } } }, + { name: 'lead', fields: { id: { type: 'text' } } }, + ], + views: [{ name: 'lead.all' }, { name: 'lead.all' }], }); expect(issues).toHaveLength(0); }); From 095b1defb0229f38b7c979277807590eb622b250 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Sun, 14 Jun 2026 11:12:15 +0500 Subject: [PATCH 2/2] feat(GA): close P0-5 (single-instance caps) + P1-2 (unbounded growth) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lands the two GA-gating launch-readiness items, scoped to the v1 single-instance topology decision. P0-5 (realtime/feed in-memory, no cluster coordination) — accepted as single-instance/non-HA for v1 with memory backstops: - InMemoryFeedAdapter defaults maxItems to DEFAULT_MAX_FEED_ITEMS (100k) - InMemoryRealtimeAdapter defaults maxSubscriptions to DEFAULT_MAX_SUBSCRIPTIONS (50k) - 0 = explicit unbounded opt-out; both plugins document the non-HA contract - HA (Redis realtime adapter + DB feed adapter) recorded as post-GA roadmap P1-2 (unbounded execution logs / job runs / event log): - service-job: new JobRunRetention sweeper (mirrors NotificationRetention), default-on at 30d, swept every 6h via an unref'd timer wired in JobServicePlugin kernel:ready (DB path); retentionDays:0 disables - service-messaging: retention flipped default-on (0 -> 90d). Behaviour change: notification history now auto-prunes at 90d (set 0 to keep forever) - service-automation: exec-log ring buffer cap made configurable via AutomationServicePluginOptions.maxLogSize (default unchanged at 1000) +11 tests across feed/realtime/job/automation; all touched packages build (incl. DTS) and test green. launch-readiness.md updated (P0-5/P1-2 verified, topology checkbox, HA roadmap). Changesets included. Co-Authored-By: Claude Opus 4.8 (1M context) --- .changeset/ga-p0-5-single-instance-caps.md | 25 ++++ .changeset/ga-p1-2-job-run-retention.md | 19 +++ .../ga-p1-2-messaging-automation-retention.md | 20 +++ docs/launch-readiness.md | 41 ++++++- .../service-automation/src/engine.test.ts | 26 +++- .../services/service-automation/src/engine.ts | 25 +++- .../services/service-automation/src/index.ts | 3 +- .../services/service-automation/src/plugin.ts | 11 +- .../service-feed/src/feed-service-plugin.ts | 6 + .../src/in-memory-feed-adapter.test.ts | 20 ++- .../src/in-memory-feed-adapter.ts | 18 ++- packages/services/service-job/src/index.ts | 9 ++ .../service-job/src/job-run-retention.test.ts | 108 ++++++++++++++++ .../service-job/src/job-run-retention.ts | 115 ++++++++++++++++++ .../service-job/src/job-service-plugin.ts | 56 ++++++++- .../src/messaging-service-plugin.ts | 18 +-- .../service-messaging/src/retention.ts | 9 ++ .../src/in-memory-realtime-adapter.test.ts | 20 ++- .../src/in-memory-realtime-adapter.ts | 17 ++- .../src/realtime-service-plugin.ts | 7 ++ 20 files changed, 550 insertions(+), 23 deletions(-) create mode 100644 .changeset/ga-p0-5-single-instance-caps.md create mode 100644 .changeset/ga-p1-2-job-run-retention.md create mode 100644 .changeset/ga-p1-2-messaging-automation-retention.md create mode 100644 packages/services/service-job/src/job-run-retention.test.ts create mode 100644 packages/services/service-job/src/job-run-retention.ts diff --git a/.changeset/ga-p0-5-single-instance-caps.md b/.changeset/ga-p0-5-single-instance-caps.md new file mode 100644 index 0000000000..02b641e9f8 --- /dev/null +++ b/.changeset/ga-p0-5-single-instance-caps.md @@ -0,0 +1,25 @@ +--- +"@objectstack/service-feed": minor +"@objectstack/service-realtime": minor +--- + +feat(P0-5): default-on memory caps for in-memory feed/realtime (single-instance GA) + +Launch-readiness P0-5 is resolved by formally scoping v1 to **single-instance** +(non-HA) and shipping memory backstops so the process-local adapters can't grow +until OOM: + +- `InMemoryFeedAdapter` now defaults `maxItems` to `DEFAULT_MAX_FEED_ITEMS` + (100k) instead of unbounded. `createFeedItem` throws loudly at the cap + (fail-loud beats a silent OOM kill). +- `InMemoryRealtimeAdapter` now defaults `maxSubscriptions` to + `DEFAULT_MAX_SUBSCRIPTIONS` (50k). +- Passing `0` is an explicit unbounded opt-out (tests / short-lived processes). +- Both plugin JSDocs now state the non-HA contract; HA (a Redis-backed realtime + adapter over the existing `RedisPubSub`, and a DB-backed feed adapter) is a + documented post-GA fast-follow. + +**Behaviour change:** a deployment that previously relied on unbounded in-memory +feed/realtime will now hit the cap and receive an error past the ceiling — set +`maxItems: 0` / `maxSubscriptions: 0` to restore the old behaviour, or raise the +number. diff --git a/.changeset/ga-p1-2-job-run-retention.md b/.changeset/ga-p1-2-job-run-retention.md new file mode 100644 index 0000000000..14a5811cf3 --- /dev/null +++ b/.changeset/ga-p1-2-job-run-retention.md @@ -0,0 +1,19 @@ +--- +"@objectstack/service-job": minor +--- + +feat(P1-2): default-on retention for the sys_job_run execution log + +Every job execution appended a `sys_job_run` row with no cleanup path, so the +table grew unbounded on long-running deployments (launch-readiness P1-2). New +`JobRunRetention` (mirroring `service-messaging`'s `NotificationRetention`) +performs a bulk `delete sys_job_run where created_at < cutoff` under a system +context. `JobServicePlugin` wires it **default-on** at `kernel:ready` (DB-backed +adapter only) — runs once on boot then every 6h via an unref'd timer. + +- `retentionDays` defaults to `DEFAULT_JOB_RUN_RETENTION_DAYS` (30); set `0` to + disable (rows kept forever; operator owns cleanup). +- `retentionSweepMs` defaults to `DEFAULT_JOB_RUN_SWEEP_MS` (6h). + +**Behaviour change:** job-run history older than 30 days is now pruned by +default. Set `retentionDays: 0` to keep the previous keep-forever behaviour. diff --git a/.changeset/ga-p1-2-messaging-automation-retention.md b/.changeset/ga-p1-2-messaging-automation-retention.md new file mode 100644 index 0000000000..17e9f8f0b3 --- /dev/null +++ b/.changeset/ga-p1-2-messaging-automation-retention.md @@ -0,0 +1,20 @@ +--- +"@objectstack/service-messaging": minor +"@objectstack/service-automation": minor +--- + +feat(P1-2): messaging retention default-on; automation log cap configurable + +Closes the remaining two P1-2 unbounded-growth items (launch-readiness): + +- **service-messaging** — notification-pipeline retention is now **default-on**. + `MessagingServicePlugin`'s `retentionDays` defaults to + `DEFAULT_NOTIFICATION_RETENTION_DAYS` (90) instead of `0`; the + already-built/tested sweeper now prunes `sys_notification` (+ delivery / inbox / + receipt) older than 90 days by default. **Behaviour change:** notification + history auto-prunes at 90d — set `retentionDays: 0` to keep it forever. +- **service-automation** — the in-memory execution-log ring buffer (already + bounded; no OOM risk) gets a tunable window via + `AutomationServicePluginOptions.maxLogSize`, defaulting to + `DEFAULT_MAX_EXECUTION_LOG_SIZE` (1000, unchanged). Durable + `sys_automation_run`-style persistence remains a post-GA HA item. diff --git a/docs/launch-readiness.md b/docs/launch-readiness.md index ea5146bbc1..5befa5a050 100644 --- a/docs/launch-readiness.md +++ b/docs/launch-readiness.md @@ -97,7 +97,17 @@ fix or acceptance.** - **Action (cluster launch):** Provide a Redis-backed realtime adapter and a DB-backed feed adapter, **or** formally restrict v1 to single-instance and document it as non-HA. Enforce a `maxItems` cap if shipping in-memory feed. -- **Owner:** _______ · Verify ☐ · Sign-off ☐ · Notes: _______ +- **Decision = single-instance for v1 (non-HA).** Recorded: realtime/feed stay + process-local for GA; HA is a post-GA fast-follow (the Redis primitive already + exists — `service-cluster-redis`'s `RedisPubSub` — so the realtime adapter is a + wrap, and a DB-backed feed adapter is the larger remaining piece). Both plugin + JSDocs now state the non-HA contract explicitly. +- **Backstops shipped:** safety caps are now **default-on** — `InMemoryFeedAdapter` + caps at `DEFAULT_MAX_FEED_ITEMS` (100k) and `InMemoryRealtimeAdapter` at + `DEFAULT_MAX_SUBSCRIPTIONS` (50k); `createFeedItem`/`subscribe` throw loudly at + the cap (fail-loud beats silent OOM). `0` is an explicit unbounded opt-out + (tests / short-lived processes). +4 tests (default-cap + opt-out, both packages). +- **Owner:** _______ · Verify ✅ (confirmed real @ `main`) · Sign-off ☐ · Notes: **Accepted for v1 as single-instance/non-HA**; caps default-on + documented. HA adapters tracked as post-GA roadmap. Awaiting human sign-off. --- @@ -130,7 +140,27 @@ fix or acceptance.** - **Risk:** Long-running pods OOM; history tables grow without bound. - **Action:** Make retention **default-on** for all event/run tables; schedule sweepers at startup; persist automation logs to a table rather than memory. -- **Owner:** _______ · Verify ☐ · Sign-off ☐ · Notes: _______ +- **Verification finding (corrected scope):** only one of the three is truly + unbounded. **automation** exec logs are *already* a bounded 1000-entry ring + buffer (`engine.ts` `recordLog` — `splice`); memory-safe today, never persisted + → no OOM risk. **`sys_job_run`** is the real leak: append-only, zero retention. + **messaging** retention was fully built + tested (`NotificationRetention`) but + shipped opt-in (`retentionDays: 0`). +- **Fixes shipped:** + - **service-job** — new `JobRunRetention` (mirrors `NotificationRetention`): + bulk `delete sys_job_run where created_at < cutoff` under a system context, + **default-on** at `DEFAULT_JOB_RUN_RETENTION_DAYS` (30d), swept every 6h via + an unref'd timer wired in `JobServicePlugin`'s `kernel:ready` (DB path only); + `retentionDays: 0` disables. +5 tests. + - **service-messaging** — flipped `retentionDays` default `0 → 90` + (`DEFAULT_NOTIFICATION_RETENTION_DAYS`); sweeper/timer/shutdown already + existed. **Behaviour change**: notification history now auto-prunes at 90d by + default (set `0` to keep the old keep-forever behaviour). Changeset notes it. + - **service-automation** — exec-log ring buffer cap made configurable via + `AutomationServicePluginOptions.maxLogSize` (default unchanged at 1000, + `DEFAULT_MAX_EXECUTION_LOG_SIZE`); +2 tests. Durable `sys_automation_run`-style + persistence is deferred to the HA fast-follow (roadmap), not a GA blocker. +- **Owner:** _______ · Verify ✅ (confirmed real @ `main`; scope corrected) · Sign-off ☐ · Notes: `sys_job_run` retention is the one true fix; messaging default-flipped; automation already bounded (now tunable). Awaiting human sign-off. ### P1-3 — Graceful shutdown (mostly a false positive; one real drain bug fixed) - **Area:** `core` (`kernel.ts`), `cli` (`serve.ts`), `plugin-hono-server` (`adapter.ts`) @@ -196,6 +226,10 @@ notes, not treated as blockers. - ☐ **Unverified features — confirm stub vs. minimal, then include or exclude:** `knowledge-ragflow`, `connector-openapi` (the sweep could not locate full implementations; the packages exist — verify scope before GA). +- ☐ **HA / multi-node (post-GA fast-follow, from P0-5 decision):** Redis-backed + realtime adapter (wrap `service-cluster-redis`'s `RedisPubSub`), DB-backed feed + adapter, and persisting automation execution logs to a table (currently a + process-local 1000-entry ring buffer — see P1-2). Not needed for single-instance v1. - ☐ **Proposed ADRs (roadmap):** ADR-0021 (analytics semantic layer), ADR-0022/0023/0024 (connectors / OpenAPI→connector / MCP connectors), ADR-0025/0026 (plugin & client-UI distribution), ADR-0027 (metadata authoring @@ -211,7 +245,8 @@ notes, not treated as blockers. - ☐ Pending changesets reviewed (4 at last check) and version bump intentional. - ☐ `pnpm run release` path verified (`build` → `build-console.sh` → `changeset publish`). - ☐ Required env vars documented for go-live (at minimum `OS_AUTH_SECRET`; see P0-1). -- ☐ Deployment topology decided: **single-instance** vs. **HA/cluster** (drives P0-5). +- ☑ Deployment topology decided: **single-instance** for v1 (drives P0-5; HA is a + post-GA fast-follow). Realtime/feed run process-local with default-on memory caps. --- diff --git a/packages/services/service-automation/src/engine.test.ts b/packages/services/service-automation/src/engine.test.ts index ff40c4d9dd..8a3bdc1c03 100644 --- a/packages/services/service-automation/src/engine.test.ts +++ b/packages/services/service-automation/src/engine.test.ts @@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { LiteKernel } from '@objectstack/core'; -import { AutomationEngine } from './engine.js'; +import { AutomationEngine, DEFAULT_MAX_EXECUTION_LOG_SIZE } from './engine.js'; import { AutomationServicePlugin } from './plugin.js'; import { registerScreenNodes } from './builtin/screen-nodes.js'; import { InMemorySuspendedRunStore } from './suspended-run-store.js'; @@ -30,6 +30,30 @@ describe('AutomationEngine', () => { engine = new AutomationEngine(createTestLogger()); }); + describe('Execution-log ring buffer (P1-2)', () => { + it('defaults the cap to DEFAULT_MAX_EXECUTION_LOG_SIZE', () => { + const e = new AutomationEngine(createTestLogger()); + expect(DEFAULT_MAX_EXECUTION_LOG_SIZE).toBeGreaterThan(0); + expect((e as unknown as { maxLogSize: number }).maxLogSize).toBe( + DEFAULT_MAX_EXECUTION_LOG_SIZE, + ); + }); + + it('honours a configured maxLogSize and evicts oldest beyond it', async () => { + const e = new AutomationEngine(createTestLogger(), undefined, { maxLogSize: 3 }); + expect((e as unknown as { maxLogSize: number }).maxLogSize).toBe(3); + + // Drive the private ring buffer directly: push 5, keep newest 3. + const rec = (e as any).recordLog.bind(e); + for (let i = 0; i < 5; i++) { + rec({ id: `run_${i}`, flowName: 'f', status: 'success' }); + } + const buf = (e as unknown as { executionLogs: Array<{ id: string }> }).executionLogs; + expect(buf).toHaveLength(3); + expect(buf.map((l) => l.id)).toEqual(['run_2', 'run_3', 'run_4']); + }); + }); + describe('Node Executor Registration', () => { it('should register a node executor', () => { const executor: NodeExecutor = { diff --git a/packages/services/service-automation/src/engine.ts b/packages/services/service-automation/src/engine.ts index 43a6719fba..590efc542e 100644 --- a/packages/services/service-automation/src/engine.ts +++ b/packages/services/service-automation/src/engine.ts @@ -197,6 +197,26 @@ export interface ConnectorDescriptor { // ─── Core Automation Engine ───────────────────────────────────────── +/** + * Default ceiling on the in-memory execution-log ring buffer. Execution logs are + * process-local and diagnostic only (launch-readiness.md P1-2); the buffer keeps + * the most recent N entries and evicts the oldest, so memory is bounded + * regardless of throughput. Operators tune the window via + * {@link AutomationServicePluginOptions.maxLogSize}. Durable, queryable run + * history is the DB-backed `sys_automation_run` store (a post-GA HA item), not + * this buffer. + */ +export const DEFAULT_MAX_EXECUTION_LOG_SIZE = 1000; + +/** Construction options for {@link AutomationEngine}. */ +export interface AutomationEngineOptions { + /** + * Max in-memory execution-log entries retained (ring buffer; oldest evicted). + * Defaults to {@link DEFAULT_MAX_EXECUTION_LOG_SIZE}. Must be > 0. + */ + maxLogSize?: number; +} + /** * Execution step log entry. Part of a {@link SuspendedRun}'s persisted state, so * it survives serialization to a durable {@link SuspendedRunStore}. @@ -334,7 +354,7 @@ export class AutomationEngine implements IAutomationService { /** Connectors registered by integration plugins, keyed by connector name (ADR-0018 §Addendum). */ private connectors = new Map(); private executionLogs: ExecutionLogEntry[] = []; - private maxLogSize = 1000; + private readonly maxLogSize: number; private logger: Logger; /** * Runs paused at a node, keyed by runId (ADR-0019). In-memory hot cache — @@ -354,9 +374,10 @@ export class AutomationEngine implements IAutomationService { */ private resuming = new Set(); - constructor(logger: Logger, store?: SuspendedRunStore) { + constructor(logger: Logger, store?: SuspendedRunStore, options?: AutomationEngineOptions) { this.logger = logger; this.store = store; + this.maxLogSize = options?.maxLogSize ?? DEFAULT_MAX_EXECUTION_LOG_SIZE; } /** diff --git a/packages/services/service-automation/src/index.ts b/packages/services/service-automation/src/index.ts index 3ede8be07e..7f91229630 100644 --- a/packages/services/service-automation/src/index.ts +++ b/packages/services/service-automation/src/index.ts @@ -1,8 +1,9 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. // Core engine -export { AutomationEngine } from './engine.js'; +export { AutomationEngine, DEFAULT_MAX_EXECUTION_LOG_SIZE } from './engine.js'; export type { + AutomationEngineOptions, NodeExecutor, NodeExecutionResult, FlowTrigger, diff --git a/packages/services/service-automation/src/plugin.ts b/packages/services/service-automation/src/plugin.ts index ed172ce48c..581db36141 100644 --- a/packages/services/service-automation/src/plugin.ts +++ b/packages/services/service-automation/src/plugin.ts @@ -24,6 +24,13 @@ export interface AutomationServicePluginOptions { * survives a process restart and can be resumed after a cold boot. */ suspendedRunStore?: 'auto' | 'memory'; + /** + * Max in-memory execution-log entries retained per process (ring buffer; + * oldest evicted). The buffer is diagnostic-only and already bounded + * (launch-readiness.md P1-2); this just makes the window tunable. Defaults + * to {@link DEFAULT_MAX_EXECUTION_LOG_SIZE} (1000). + */ + maxLogSize?: number; } /** @@ -71,7 +78,9 @@ export class AutomationServicePlugin implements Plugin { } async init(ctx: PluginContext): Promise { - this.engine = new AutomationEngine(ctx.logger); + this.engine = new AutomationEngine(ctx.logger, undefined, { + maxLogSize: this.options.maxLogSize, + }); // Register as global service — other plugins access via ctx.getService('automation') ctx.registerService('automation', this.engine); diff --git a/packages/services/service-feed/src/feed-service-plugin.ts b/packages/services/service-feed/src/feed-service-plugin.ts index 7ff56d7ba2..1ee2e0b0ee 100644 --- a/packages/services/service-feed/src/feed-service-plugin.ts +++ b/packages/services/service-feed/src/feed-service-plugin.ts @@ -21,6 +21,12 @@ export interface FeedServicePluginOptions { * Registers a Feed/Chatter service with the kernel during the init phase. * Currently supports in-memory storage for single-process environments. * + * **v1 deployment contract (launch-readiness.md P0-5): single-instance only.** + * The in-memory adapter is process-local and non-durable — feed data is lost on + * restart and is not shared across nodes. A default `maxItems` safety cap is + * enforced (see {@link InMemoryFeedAdapter}) so the store can't grow until OOM. + * HA (a DB-backed feed adapter) is a post-GA fast-follow. + * * @example * ```ts * import { ObjectKernel } from '@objectstack/core'; diff --git a/packages/services/service-feed/src/in-memory-feed-adapter.test.ts b/packages/services/service-feed/src/in-memory-feed-adapter.test.ts index ccc0710e97..ad9746b1ce 100644 --- a/packages/services/service-feed/src/in-memory-feed-adapter.test.ts +++ b/packages/services/service-feed/src/in-memory-feed-adapter.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { InMemoryFeedAdapter } from './in-memory-feed-adapter'; +import { InMemoryFeedAdapter, DEFAULT_MAX_FEED_ITEMS } from './in-memory-feed-adapter'; import type { IFeedService, CreateFeedItemInput } from '@objectstack/spec/contracts'; /** Helper to create a standard comment input. */ @@ -125,6 +125,24 @@ describe('InMemoryFeedAdapter', () => { .rejects.toThrow(/Maximum feed item limit reached/); }); + it('enforces a default maxItems cap when none is configured (P0-5 backstop)', async () => { + // The cap is default-on so a long-running single-instance feed can't grow + // until OOM. Adapter constructed with no options uses the default cap. + const feed = new InMemoryFeedAdapter(); + expect(DEFAULT_MAX_FEED_ITEMS).toBeGreaterThan(0); + expect(Number.isFinite(DEFAULT_MAX_FEED_ITEMS)).toBe(true); + expect((feed as unknown as { maxItems: number }).maxItems).toBe(DEFAULT_MAX_FEED_ITEMS); + }); + + it('treats maxItems: 0 as an explicit unbounded opt-out', async () => { + const feed = new InMemoryFeedAdapter({ maxItems: 0 }); + expect((feed as unknown as { maxItems: number }).maxItems).toBe(0); + for (let i = 0; i < 5; i++) { + await feed.createFeedItem(commentInput({ body: `c${i}` })); + } + expect(feed.getItemCount()).toBe(5); + }); + // ========================================== // Feed Listing & Filtering // ========================================== diff --git a/packages/services/service-feed/src/in-memory-feed-adapter.ts b/packages/services/service-feed/src/in-memory-feed-adapter.ts index 6f47490e91..f22683b0cc 100644 --- a/packages/services/service-feed/src/in-memory-feed-adapter.ts +++ b/packages/services/service-feed/src/in-memory-feed-adapter.ts @@ -11,11 +11,25 @@ import type { import type { FeedItem, Reaction } from '@objectstack/spec/data'; import type { RecordSubscription } from '@objectstack/spec/data'; +/** + * Default safety cap on stored feed items. This adapter is process-local and + * non-durable (the v1 single-instance contract — see launch-readiness.md P0-5), + * so an unbounded store would grow until the pod OOMs. The cap is a backstop: + * `createFeedItem` throws loudly once it's reached rather than letting memory + * grow without bound. Operators who need a higher ceiling raise `maxItems`; + * `maxItems: 0` opts out entirely (unbounded — only for tests / short-lived + * processes). + */ +export const DEFAULT_MAX_FEED_ITEMS = 100_000; + /** * Configuration options for InMemoryFeedAdapter. */ export interface InMemoryFeedAdapterOptions { - /** Maximum number of feed items to store (0 = unlimited) */ + /** + * Maximum number of feed items to store. Defaults to + * {@link DEFAULT_MAX_FEED_ITEMS}; `0` = unbounded (explicit opt-out). + */ maxItems?: number; } @@ -50,7 +64,7 @@ export class InMemoryFeedAdapter implements IFeedService { private readonly maxItems: number; constructor(options: InMemoryFeedAdapterOptions = {}) { - this.maxItems = options.maxItems ?? 0; + this.maxItems = options.maxItems ?? DEFAULT_MAX_FEED_ITEMS; } async listFeed(options: ListFeedOptions): Promise { diff --git a/packages/services/service-job/src/index.ts b/packages/services/service-job/src/index.ts index 43ea7fcd9b..0d639ea361 100644 --- a/packages/services/service-job/src/index.ts +++ b/packages/services/service-job/src/index.ts @@ -8,3 +8,12 @@ export { CronJobAdapter } from './cron-job-adapter.js'; export type { CronJobAdapterOptions } from './cron-job-adapter.js'; export { DbJobAdapter } from './db-job-adapter.js'; export type { DbJobAdapterOptions, JobEngineLike, JobLoggerLike } from './db-job-adapter.js'; +export { + JobRunRetention, + DEFAULT_JOB_RUN_RETENTION_DAYS, + DEFAULT_JOB_RUN_SWEEP_MS, +} from './job-run-retention.js'; +export type { + JobRunRetentionOptions, + JobRunPruneOutcome, +} from './job-run-retention.js'; diff --git a/packages/services/service-job/src/job-run-retention.test.ts b/packages/services/service-job/src/job-run-retention.test.ts new file mode 100644 index 0000000000..b0e66cf505 --- /dev/null +++ b/packages/services/service-job/src/job-run-retention.test.ts @@ -0,0 +1,108 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { JobRunRetention, DEFAULT_JOB_RUN_RETENTION_DAYS } from './job-run-retention'; + +const DAY = 86_400_000; + +/** Minimal engine supporting the `delete … where created_at < cutoff` shape. */ +function makeFakeEngine() { + const rows: Array<{ id: string; created_at: string }> = []; + let deleteCalls = 0; + return { + rows, + get deleteCalls() { + return deleteCalls; + }, + seed(...createdAts: string[]) { + createdAts.forEach((created_at, i) => rows.push({ id: `run_${i}`, created_at })); + }, + async find() { + return [...rows]; + }, + async insert() { + return {}; + }, + async update() { + return {}; + }, + async delete(_table: string, opts: any) { + deleteCalls++; + const cutoff = opts?.where?.created_at?.$lt as string | undefined; + if (cutoff === undefined) return 0; + let deleted = 0; + for (let i = rows.length - 1; i >= 0; i--) { + if (rows[i].created_at < cutoff) { + rows.splice(i, 1); + deleted++; + } + } + return deleted; + }, + }; +} + +const silentLogger = { info() {}, warn() {} }; + +describe('JobRunRetention', () => { + it('prunes only rows older than the retention window', async () => { + const engine = makeFakeEngine(); + const nowMs = Date.parse('2026-06-14T00:00:00.000Z'); + // 3 old (>30d), 2 recent (<30d) + engine.seed( + new Date(nowMs - 100 * DAY).toISOString(), + new Date(nowMs - 60 * DAY).toISOString(), + new Date(nowMs - 31 * DAY).toISOString(), + new Date(nowMs - 10 * DAY).toISOString(), + new Date(nowMs - 1 * DAY).toISOString(), + ); + + const retention = new JobRunRetention({ + getEngine: () => engine as any, + logger: silentLogger, + now: () => nowMs, + }); + + const outcome = await retention.prune(30); + expect(outcome.deleted).toBe(3); + expect(outcome.error).toBeUndefined(); + expect(engine.rows).toHaveLength(2); + }); + + it('is a no-op when retentionDays is not positive', async () => { + const engine = makeFakeEngine(); + engine.seed(new Date(0).toISOString()); + const retention = new JobRunRetention({ getEngine: () => engine as any, logger: silentLogger }); + + const outcome = await retention.prune(0); + expect(outcome.deleted).toBe(0); + expect(engine.deleteCalls).toBe(0); + expect(engine.rows).toHaveLength(1); + }); + + it('is a no-op when no data engine is available', async () => { + const retention = new JobRunRetention({ getEngine: () => undefined, logger: silentLogger }); + const outcome = await retention.prune(30); + expect(outcome.deleted).toBe(0); + }); + + it('reports an error (without throwing) when the engine delete fails', async () => { + const retention = new JobRunRetention({ + getEngine: () => + ({ + delete() { + throw new Error('db down'); + }, + }) as any, + logger: silentLogger, + }); + const outcome = await retention.prune(30); + expect(outcome.error).toMatch(/db down/); + expect(outcome.deleted).toBeUndefined(); + }); + + it('exposes a sane positive default retention window', () => { + expect(DEFAULT_JOB_RUN_RETENTION_DAYS).toBeGreaterThan(0); + expect(Number.isFinite(DEFAULT_JOB_RUN_RETENTION_DAYS)).toBe(true); + }); +}); diff --git a/packages/services/service-job/src/job-run-retention.ts b/packages/services/service-job/src/job-run-retention.ts new file mode 100644 index 0000000000..977cea5a33 --- /dev/null +++ b/packages/services/service-job/src/job-run-retention.ts @@ -0,0 +1,115 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +import type { JobEngineLike, JobLoggerLike } from './db-job-adapter.js'; + +const RUN_TABLE = 'sys_job_run'; +const SYSTEM_CTX = { isSystem: true, roles: [], permissions: [] } as const; + +/** + * Default retention window for `sys_job_run` rows, in days. Every job execution + * appends a run row (see {@link DbJobAdapter}); without pruning the table grows + * unbounded on a long-running deployment (launch-readiness.md P1-2). 30 days + * keeps recent history for operational triage while bounding growth. Operators + * raise/lower it via `JobServicePlugin` options; `0` disables retention. + */ +export const DEFAULT_JOB_RUN_RETENTION_DAYS = 30; + +/** + * Default interval between retention sweeps. Job-run volume is far lower than the + * notification pipeline's, so a 6-hour cadence is ample — the sweep is a single + * bulk `delete … where created_at < cutoff`. + */ +export const DEFAULT_JOB_RUN_SWEEP_MS = 6 * 3_600_000; + +export interface JobRunRetentionOptions { + /** Resolve the data engine; `undefined` ⇒ prune is a no-op. */ + getEngine(): JobEngineLike | undefined; + logger: JobLoggerLike; + /** Override the swept object (tests). Defaults to `sys_job_run`. */ + object?: string; + /** Timestamp field used for the cutoff (ISO-8601). Defaults to `created_at`. */ + tsField?: string; + /** Clock injection for deterministic tests. Defaults to `Date.now()`. */ + now?(): number; +} + +export interface JobRunPruneOutcome { + object: string; + /** `undefined` when the driver doesn't report a count. */ + deleted?: number; + error?: string; +} + +/** + * Retention sweeper for `sys_job_run` (launch-readiness.md P1-2). + * + * Mirrors the proven `NotificationRetention` shape in `service-messaging`: + * a single bulk delete of rows older than a cutoff, under a system context + * (retention is a cross-tenant operator policy). Isolated from job execution — + * a sweep failure is logged and never throws into the scheduler. + * + * Unlike the messaging sweeper, this one is **default-on** in the plugin: an + * append-only run log with no ceiling is a guaranteed slow leak, so GA ships + * with a sensible window rather than requiring opt-in. + */ +export class JobRunRetention { + private readonly now: () => number; + private readonly object: string; + private readonly tsField: string; + + constructor(private readonly opts: JobRunRetentionOptions) { + this.now = opts.now ?? (() => Date.now()); + this.object = opts.object ?? RUN_TABLE; + this.tsField = opts.tsField ?? 'created_at'; + } + + /** + * Delete `sys_job_run` rows older than `retentionDays`. No-op when no data + * engine is available, the engine can't delete, or `retentionDays` is not a + * positive number. + */ + async prune(retentionDays: number): Promise { + const engine = this.opts.getEngine(); + if (!engine || typeof engine.delete !== 'function') { + this.opts.logger.warn('[job] retention: no deletable data engine; prune skipped'); + return { object: this.object, deleted: 0 }; + } + if (!(retentionDays > 0)) { + this.opts.logger.warn(`[job] retention: invalid retentionDays=${retentionDays}; prune skipped`); + return { object: this.object, deleted: 0 }; + } + + const cutoffIso = new Date(this.now() - retentionDays * 86_400_000).toISOString(); + try { + const res = await engine.delete(this.object, { + where: { [this.tsField]: { $lt: cutoffIso } }, + multi: true, + context: SYSTEM_CTX, + }); + const deleted = countDeleted(res); + if (deleted === undefined || deleted > 0) { + this.opts.logger.info( + `[job] retention: pruned ${deleted ?? '?'} ${this.object} rows older than ${cutoffIso}`, + ); + } + return { object: this.object, deleted }; + } catch (err) { + const msg = (err as Error)?.message ?? String(err); + this.opts.logger.warn(`[job] retention: prune of ${this.object} failed (${msg})`); + return { object: this.object, error: msg }; + } + } +} + +/** Best-effort row-count extraction from a driver's delete result. */ +function countDeleted(res: unknown): number | undefined { + if (typeof res === 'number') return res; + if (Array.isArray(res)) return res.length; + if (res && typeof res === 'object') { + const r = res as Record; + for (const k of ['deletedCount', 'deleted', 'count', 'affected', 'affectedRows']) { + if (typeof r[k] === 'number') return r[k] as number; + } + } + return undefined; +} diff --git a/packages/services/service-job/src/job-service-plugin.ts b/packages/services/service-job/src/job-service-plugin.ts index b454acdd2c..2ef6f8d618 100644 --- a/packages/services/service-job/src/job-service-plugin.ts +++ b/packages/services/service-job/src/job-service-plugin.ts @@ -6,7 +6,12 @@ import { IntervalJobAdapter } from './interval-job-adapter.js'; import type { IntervalJobAdapterOptions } from './interval-job-adapter.js'; import { CronJobAdapter } from './cron-job-adapter.js'; import { DbJobAdapter } from './db-job-adapter.js'; -import type { DbJobAdapterOptions } from './db-job-adapter.js'; +import type { DbJobAdapterOptions, JobEngineLike } from './db-job-adapter.js'; +import { + JobRunRetention, + DEFAULT_JOB_RUN_RETENTION_DAYS, + DEFAULT_JOB_RUN_SWEEP_MS, +} from './job-run-retention.js'; /** * Configuration options for the JobServicePlugin. @@ -26,6 +31,17 @@ export interface JobServicePluginOptions { db?: DbJobAdapterOptions; /** Whether to also wire CronJobAdapter for cron schedules (default: true when available) */ enableCron?: boolean; + /** + * Retention window in days for `sys_job_run` execution-history rows + * (launch-readiness.md P1-2). Every run appends a row, so without pruning the + * table grows unbounded. **Default-on** at {@link DEFAULT_JOB_RUN_RETENTION_DAYS} + * — a periodic sweep deletes rows older than this. Set to `0` to disable + * retention (rows kept forever; operator owns cleanup). Only applies on the + * DB-backed adapter (no `sys_job_run` table exists for interval/cron). + */ + retentionDays?: number; + /** Retention sweep interval in ms (default {@link DEFAULT_JOB_RUN_SWEEP_MS}). Only used when `retentionDays > 0`. */ + retentionSweepMs?: number; } /** @@ -44,9 +60,16 @@ export class JobServicePlugin implements Plugin { private readonly options: JobServicePluginOptions; private dbAdapter?: DbJobAdapter; private intervalAdapter?: IntervalJobAdapter; + private retentionTimer?: ReturnType; constructor(options: JobServicePluginOptions = {}) { - this.options = { adapter: 'auto', enableCron: true, ...options }; + this.options = { + adapter: 'auto', + enableCron: true, + retentionDays: DEFAULT_JOB_RUN_RETENTION_DAYS, + retentionSweepMs: DEFAULT_JOB_RUN_SWEEP_MS, + ...options, + }; } async init(ctx: PluginContext): Promise { @@ -125,10 +148,39 @@ export class JobServicePlugin implements Plugin { } catch (err) { ctx.logger.warn('JobServicePlugin: replaceService failed; staying on IntervalJobAdapter', err as any); } + + // Retention sweep (launch-readiness.md P1-2): bound the append-only + // sys_job_run log. Default-on — an unbounded run history is a guaranteed + // slow leak. Runs once now then on a low-frequency interval; the timer is + // unref'd so it never keeps the process alive. Only wired on the DB path + // (the table exists only there). + const retentionDays = this.options.retentionDays ?? DEFAULT_JOB_RUN_RETENTION_DAYS; + if (retentionDays > 0) { + const retention = new JobRunRetention({ + getEngine: () => engine as JobEngineLike, + logger: ctx.logger, + }); + const sweepMs = this.options.retentionSweepMs ?? DEFAULT_JOB_RUN_SWEEP_MS; + const sweep = () => { + void retention.prune(retentionDays).catch((err) => + ctx.logger.warn(`JobServicePlugin: retention sweep failed: ${(err as Error)?.message ?? err}`), + ); + }; + sweep(); + this.retentionTimer = setInterval(sweep, sweepMs); + this.retentionTimer.unref?.(); + ctx.logger.info( + `JobServicePlugin: sys_job_run retention on (prune > ${retentionDays}d every ${Math.round(sweepMs / 1000)}s)`, + ); + } }); } async destroy(): Promise { + if (this.retentionTimer) { + clearInterval(this.retentionTimer); + this.retentionTimer = undefined; + } await this.dbAdapter?.destroy(); await this.intervalAdapter?.destroy(); } diff --git a/packages/services/service-messaging/src/messaging-service-plugin.ts b/packages/services/service-messaging/src/messaging-service-plugin.ts index a0746025b9..9d178c6bc1 100644 --- a/packages/services/service-messaging/src/messaging-service-plugin.ts +++ b/packages/services/service-messaging/src/messaging-service-plugin.ts @@ -9,7 +9,7 @@ import { SqlNotificationOutbox } from './sql-outbox.js'; import { SqlHttpOutbox } from './sql-http-outbox.js'; import { NotificationDispatcher, type DispatchCluster } from './dispatcher.js'; import { HttpDispatcher } from './http-dispatcher.js'; -import { NotificationRetention } from './retention.js'; +import { NotificationRetention, DEFAULT_NOTIFICATION_RETENTION_DAYS } from './retention.js'; import { createEmailChannel } from './email-channel.js'; import { NotificationTemplateStore } from './template-renderer.js'; import { @@ -46,11 +46,15 @@ export interface MessagingServicePluginOptions { mandatoryTopics?: readonly string[]; /** * Retention window in days for the notification pipeline (ADR-0030 - * hardening). When set (> 0), a periodic sweep prunes events, deliveries, - * inbox materializations and receipts older than this — bounding the - * event-log growth from high-frequency periodic flows. **Opt-in**: unset (or - * ≤ 0) disables retention (default), so no notification data is ever deleted - * without explicit operator policy. + * hardening). When > 0, a periodic sweep prunes events, deliveries, inbox + * materializations and receipts older than this — bounding the event-log + * growth from high-frequency periodic flows. + * + * **Default-on** at {@link DEFAULT_NOTIFICATION_RETENTION_DAYS} as of GA + * (launch-readiness.md P1-2): an unbounded notification log is a slow leak, + * so retention ships enabled rather than opt-in. Set to `0` to disable + * retention entirely (notification history kept forever; operator owns + * cleanup). Raise/lower the number to widen/narrow the window. */ retentionDays?: number; /** Retention sweep interval in ms (default 1 hour). Only used when `retentionDays` is set. */ @@ -98,7 +102,7 @@ export class MessagingServicePlugin implements Plugin { partitionCount: 8, dispatchIntervalMs: 500, mandatoryTopics: [], - retentionDays: 0, + retentionDays: DEFAULT_NOTIFICATION_RETENTION_DAYS, retentionSweepMs: 3_600_000, ...options, }; diff --git a/packages/services/service-messaging/src/retention.ts b/packages/services/service-messaging/src/retention.ts index c63abcde94..8248d25da8 100644 --- a/packages/services/service-messaging/src/retention.ts +++ b/packages/services/service-messaging/src/retention.ts @@ -5,6 +5,15 @@ import { NOTIFICATION_EVENT_OBJECT } from './messaging-service.js'; import { DELIVERY_OBJECT } from './sql-outbox.js'; import { INBOX_OBJECT, RECEIPT_OBJECT } from './inbox-channel.js'; +/** + * Default retention window for the notification pipeline, in days. Default-on as + * of GA (launch-readiness.md P1-2): 90 days keeps a quarter of in-app history + * for the bell / audit while bounding `sys_notification` (+ delivery / inbox / + * receipt) growth. Operators override via `MessagingServicePlugin` options; + * `0` disables pruning entirely. + */ +export const DEFAULT_NOTIFICATION_RETENTION_DAYS = 90; + /** Minimal logger surface (matches the channel/service context). */ interface RetentionLogger { info(msg: string): void; diff --git a/packages/services/service-realtime/src/in-memory-realtime-adapter.test.ts b/packages/services/service-realtime/src/in-memory-realtime-adapter.test.ts index 0881b8193e..3ba78375b8 100644 --- a/packages/services/service-realtime/src/in-memory-realtime-adapter.test.ts +++ b/packages/services/service-realtime/src/in-memory-realtime-adapter.test.ts @@ -1,7 +1,7 @@ // Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { InMemoryRealtimeAdapter } from './in-memory-realtime-adapter'; +import { InMemoryRealtimeAdapter, DEFAULT_MAX_SUBSCRIPTIONS } from './in-memory-realtime-adapter'; import type { IRealtimeService, RealtimeEventPayload } from '@objectstack/spec/contracts'; describe('InMemoryRealtimeAdapter', () => { @@ -198,6 +198,24 @@ describe('InMemoryRealtimeAdapter', () => { ); }); + it('enforces a default maxSubscriptions cap when none is configured (P0-5 backstop)', () => { + const realtime = new InMemoryRealtimeAdapter(); + expect(DEFAULT_MAX_SUBSCRIPTIONS).toBeGreaterThan(0); + expect(Number.isFinite(DEFAULT_MAX_SUBSCRIPTIONS)).toBe(true); + expect((realtime as unknown as { maxSubscriptions: number }).maxSubscriptions).toBe( + DEFAULT_MAX_SUBSCRIPTIONS, + ); + }); + + it('treats maxSubscriptions: 0 as an explicit unbounded opt-out', async () => { + const realtime = new InMemoryRealtimeAdapter({ maxSubscriptions: 0 }); + expect((realtime as unknown as { maxSubscriptions: number }).maxSubscriptions).toBe(0); + for (let i = 0; i < 5; i++) { + await realtime.subscribe(`ch${i}`, () => {}); + } + expect(realtime.getSubscriptionCount()).toBe(5); + }); + it('should not break publish loop on handler error', async () => { const realtime = new InMemoryRealtimeAdapter(); const received: RealtimeEventPayload[] = []; diff --git a/packages/services/service-realtime/src/in-memory-realtime-adapter.ts b/packages/services/service-realtime/src/in-memory-realtime-adapter.ts index 5242a71306..8ad6f00409 100644 --- a/packages/services/service-realtime/src/in-memory-realtime-adapter.ts +++ b/packages/services/service-realtime/src/in-memory-realtime-adapter.ts @@ -17,11 +17,24 @@ interface Subscription { options?: RealtimeSubscriptionOptions; } +/** + * Default safety cap on active subscriptions. This adapter is process-local + * (the v1 single-instance contract — see launch-readiness.md P0-5); an + * unbounded subscription map would grow until the pod OOMs under a subscription + * leak or an abusive client. The cap is a backstop: `subscribe` throws once + * it's reached. Operators raise `maxSubscriptions` for high-fan-out nodes; + * `maxSubscriptions: 0` opts out entirely (unbounded — only for tests). + */ +export const DEFAULT_MAX_SUBSCRIPTIONS = 50_000; + /** * Configuration options for InMemoryRealtimeAdapter. */ export interface InMemoryRealtimeAdapterOptions { - /** Maximum number of subscriptions allowed (0 = unlimited) */ + /** + * Maximum number of subscriptions allowed. Defaults to + * {@link DEFAULT_MAX_SUBSCRIPTIONS}; `0` = unbounded (explicit opt-out). + */ maxSubscriptions?: number; } @@ -59,7 +72,7 @@ export class InMemoryRealtimeAdapter implements IRealtimeService { private readonly maxSubscriptions: number; constructor(options: InMemoryRealtimeAdapterOptions = {}) { - this.maxSubscriptions = options.maxSubscriptions ?? 0; + this.maxSubscriptions = options.maxSubscriptions ?? DEFAULT_MAX_SUBSCRIPTIONS; } async publish(event: RealtimeEventPayload): Promise { diff --git a/packages/services/service-realtime/src/realtime-service-plugin.ts b/packages/services/service-realtime/src/realtime-service-plugin.ts index ecbef1a3c5..c9b00c839e 100644 --- a/packages/services/service-realtime/src/realtime-service-plugin.ts +++ b/packages/services/service-realtime/src/realtime-service-plugin.ts @@ -21,6 +21,13 @@ export interface RealtimeServicePluginOptions { * Registers a realtime pub/sub service with the kernel during the init phase. * Currently supports in-memory pub/sub for single-process environments. * + * **v1 deployment contract (launch-readiness.md P0-5): single-instance only.** + * The in-memory adapter is process-local — events published on node A are not + * delivered to subscribers on node B. A default `maxSubscriptions` safety cap is + * enforced (see {@link InMemoryRealtimeAdapter}) so the subscription map can't + * grow until OOM. HA (a Redis-backed adapter over the existing `RedisPubSub` in + * `service-cluster-redis`) is a post-GA fast-follow. + * * @example * ```ts * import { ObjectKernel } from '@objectstack/core';