Skip to content

Commit f19caef

Browse files
xuyushun441-sysos-zhuangclaude
authored
feat(GA): close P0-5 (single-instance caps) + P1-2 (unbounded growth) (#1837)
* fix(ADR-0048): rescope os lint naming/namespace-prefix to intra-package duplicates 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) <noreply@anthropic.com> * feat(GA): close P0-5 (single-instance caps) + P1-2 (unbounded growth) 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) <noreply@anthropic.com> --------- Co-authored-by: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 707aeed commit f19caef

23 files changed

Lines changed: 706 additions & 97 deletions
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
fix(ADR-0048): rescope the `os lint` `naming/namespace-prefix` rule to intra-package duplicates
6+
7+
ADR-0048 §3.4 retired the per-item cross-package collision throw — two
8+
installed packages may legitimately ship the same bare name (e.g. `page/home`),
9+
stored under distinct composite keys and disambiguated by package-scoped
10+
resolution. The `naming/namespace-prefix` lint rule was never updated to match,
11+
so it still:
12+
13+
- **fired on every bare-named UI/automation item** (apps/pages/dashboards/flows/
14+
actions/reports/datasets) regardless of whether a duplicate existed — a normal
15+
single-package app got dozens of false positives (hotcrm: 63), and
16+
- **claimed the package would "collide on the registry key and fail at install"**,
17+
which is no longer true.
18+
19+
The rule now warns **only on a genuine intra-package duplicate `(type, name)`
20+
pair** within the linted config — the narrow authoring-time hygiene case ADR-0048
21+
§3.4 explicitly leaves to `os lint` ("an author shipping two `page/home` in one
22+
package"). A unique bare name produces zero warnings. The message no longer
23+
claims an install failure; it explains the items shadow each other on the
24+
registry key and that distinct packages may reuse the same name freely (the
25+
namespace prefix is an optional convention). Runtime/registry behavior is
26+
unchanged.
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"@objectstack/service-feed": minor
3+
"@objectstack/service-realtime": minor
4+
---
5+
6+
feat(P0-5): default-on memory caps for in-memory feed/realtime (single-instance GA)
7+
8+
Launch-readiness P0-5 is resolved by formally scoping v1 to **single-instance**
9+
(non-HA) and shipping memory backstops so the process-local adapters can't grow
10+
until OOM:
11+
12+
- `InMemoryFeedAdapter` now defaults `maxItems` to `DEFAULT_MAX_FEED_ITEMS`
13+
(100k) instead of unbounded. `createFeedItem` throws loudly at the cap
14+
(fail-loud beats a silent OOM kill).
15+
- `InMemoryRealtimeAdapter` now defaults `maxSubscriptions` to
16+
`DEFAULT_MAX_SUBSCRIPTIONS` (50k).
17+
- Passing `0` is an explicit unbounded opt-out (tests / short-lived processes).
18+
- Both plugin JSDocs now state the non-HA contract; HA (a Redis-backed realtime
19+
adapter over the existing `RedisPubSub`, and a DB-backed feed adapter) is a
20+
documented post-GA fast-follow.
21+
22+
**Behaviour change:** a deployment that previously relied on unbounded in-memory
23+
feed/realtime will now hit the cap and receive an error past the ceiling — set
24+
`maxItems: 0` / `maxSubscriptions: 0` to restore the old behaviour, or raise the
25+
number.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/service-job": minor
3+
---
4+
5+
feat(P1-2): default-on retention for the sys_job_run execution log
6+
7+
Every job execution appended a `sys_job_run` row with no cleanup path, so the
8+
table grew unbounded on long-running deployments (launch-readiness P1-2). New
9+
`JobRunRetention` (mirroring `service-messaging`'s `NotificationRetention`)
10+
performs a bulk `delete sys_job_run where created_at < cutoff` under a system
11+
context. `JobServicePlugin` wires it **default-on** at `kernel:ready` (DB-backed
12+
adapter only) — runs once on boot then every 6h via an unref'd timer.
13+
14+
- `retentionDays` defaults to `DEFAULT_JOB_RUN_RETENTION_DAYS` (30); set `0` to
15+
disable (rows kept forever; operator owns cleanup).
16+
- `retentionSweepMs` defaults to `DEFAULT_JOB_RUN_SWEEP_MS` (6h).
17+
18+
**Behaviour change:** job-run history older than 30 days is now pruned by
19+
default. Set `retentionDays: 0` to keep the previous keep-forever behaviour.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
"@objectstack/service-messaging": minor
3+
"@objectstack/service-automation": minor
4+
---
5+
6+
feat(P1-2): messaging retention default-on; automation log cap configurable
7+
8+
Closes the remaining two P1-2 unbounded-growth items (launch-readiness):
9+
10+
- **service-messaging** — notification-pipeline retention is now **default-on**.
11+
`MessagingServicePlugin`'s `retentionDays` defaults to
12+
`DEFAULT_NOTIFICATION_RETENTION_DAYS` (90) instead of `0`; the
13+
already-built/tested sweeper now prunes `sys_notification` (+ delivery / inbox /
14+
receipt) older than 90 days by default. **Behaviour change:** notification
15+
history auto-prunes at 90d — set `retentionDays: 0` to keep it forever.
16+
- **service-automation** — the in-memory execution-log ring buffer (already
17+
bounded; no OOM risk) gets a tunable window via
18+
`AutomationServicePluginOptions.maxLogSize`, defaulting to
19+
`DEFAULT_MAX_EXECUTION_LOG_SIZE` (1000, unchanged). Durable
20+
`sys_automation_run`-style persistence remains a post-GA HA item.

docs/launch-readiness.md

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,17 @@ fix or acceptance.**
9797
- **Action (cluster launch):** Provide a Redis-backed realtime adapter and a
9898
DB-backed feed adapter, **or** formally restrict v1 to single-instance and
9999
document it as non-HA. Enforce a `maxItems` cap if shipping in-memory feed.
100-
- **Owner:** _______ · Verify ☐ · Sign-off ☐ · Notes: _______
100+
- **Decision = single-instance for v1 (non-HA).** Recorded: realtime/feed stay
101+
process-local for GA; HA is a post-GA fast-follow (the Redis primitive already
102+
exists — `service-cluster-redis`'s `RedisPubSub` — so the realtime adapter is a
103+
wrap, and a DB-backed feed adapter is the larger remaining piece). Both plugin
104+
JSDocs now state the non-HA contract explicitly.
105+
- **Backstops shipped:** safety caps are now **default-on**`InMemoryFeedAdapter`
106+
caps at `DEFAULT_MAX_FEED_ITEMS` (100k) and `InMemoryRealtimeAdapter` at
107+
`DEFAULT_MAX_SUBSCRIPTIONS` (50k); `createFeedItem`/`subscribe` throw loudly at
108+
the cap (fail-loud beats silent OOM). `0` is an explicit unbounded opt-out
109+
(tests / short-lived processes). +4 tests (default-cap + opt-out, both packages).
110+
- **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.
101111

102112
---
103113

@@ -130,7 +140,27 @@ fix or acceptance.**
130140
- **Risk:** Long-running pods OOM; history tables grow without bound.
131141
- **Action:** Make retention **default-on** for all event/run tables; schedule
132142
sweepers at startup; persist automation logs to a table rather than memory.
133-
- **Owner:** _______ · Verify ☐ · Sign-off ☐ · Notes: _______
143+
- **Verification finding (corrected scope):** only one of the three is truly
144+
unbounded. **automation** exec logs are *already* a bounded 1000-entry ring
145+
buffer (`engine.ts` `recordLog``splice`); memory-safe today, never persisted
146+
→ no OOM risk. **`sys_job_run`** is the real leak: append-only, zero retention.
147+
**messaging** retention was fully built + tested (`NotificationRetention`) but
148+
shipped opt-in (`retentionDays: 0`).
149+
- **Fixes shipped:**
150+
- **service-job** — new `JobRunRetention` (mirrors `NotificationRetention`):
151+
bulk `delete sys_job_run where created_at < cutoff` under a system context,
152+
**default-on** at `DEFAULT_JOB_RUN_RETENTION_DAYS` (30d), swept every 6h via
153+
an unref'd timer wired in `JobServicePlugin`'s `kernel:ready` (DB path only);
154+
`retentionDays: 0` disables. +5 tests.
155+
- **service-messaging** — flipped `retentionDays` default `0 → 90`
156+
(`DEFAULT_NOTIFICATION_RETENTION_DAYS`); sweeper/timer/shutdown already
157+
existed. **Behaviour change**: notification history now auto-prunes at 90d by
158+
default (set `0` to keep the old keep-forever behaviour). Changeset notes it.
159+
- **service-automation** — exec-log ring buffer cap made configurable via
160+
`AutomationServicePluginOptions.maxLogSize` (default unchanged at 1000,
161+
`DEFAULT_MAX_EXECUTION_LOG_SIZE`); +2 tests. Durable `sys_automation_run`-style
162+
persistence is deferred to the HA fast-follow (roadmap), not a GA blocker.
163+
- **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.
134164

135165
### P1-3 — Graceful shutdown (mostly a false positive; one real drain bug fixed)
136166
- **Area:** `core` (`kernel.ts`), `cli` (`serve.ts`), `plugin-hono-server` (`adapter.ts`)
@@ -196,6 +226,10 @@ notes, not treated as blockers.
196226
-**Unverified features — confirm stub vs. minimal, then include or exclude:**
197227
`knowledge-ragflow`, `connector-openapi` (the sweep could not locate full
198228
implementations; the packages exist — verify scope before GA).
229+
-**HA / multi-node (post-GA fast-follow, from P0-5 decision):** Redis-backed
230+
realtime adapter (wrap `service-cluster-redis`'s `RedisPubSub`), DB-backed feed
231+
adapter, and persisting automation execution logs to a table (currently a
232+
process-local 1000-entry ring buffer — see P1-2). Not needed for single-instance v1.
199233
-**Proposed ADRs (roadmap):** ADR-0021 (analytics semantic layer),
200234
ADR-0022/0023/0024 (connectors / OpenAPI→connector / MCP connectors),
201235
ADR-0025/0026 (plugin & client-UI distribution), ADR-0027 (metadata authoring
@@ -211,7 +245,8 @@ notes, not treated as blockers.
211245
- ☐ Pending changesets reviewed (4 at last check) and version bump intentional.
212246
-`pnpm run release` path verified (`build``build-console.sh``changeset publish`).
213247
- ☐ Required env vars documented for go-live (at minimum `OS_AUTH_SECRET`; see P0-1).
214-
- ☐ Deployment topology decided: **single-instance** vs. **HA/cluster** (drives P0-5).
248+
- ☑ Deployment topology decided: **single-instance** for v1 (drives P0-5; HA is a
249+
post-GA fast-follow). Realtime/feed run process-local with default-on memory caps.
215250

216251
---
217252

packages/cli/src/commands/lint.ts

Lines changed: 60 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -216,48 +216,68 @@ export function lintConfig(config: any): LintIssue[] {
216216
}
217217
}
218218

219-
// ── Namespace-prefix advisory for bare-named metadata (ADR-0048 §3.3) ──
220-
// Cross-package collision detection (ADR-0048) makes a clash between two
221-
// packages' bare-named items (`page`/`flow`/`action`/…) a loud error at
222-
// registration time. This shifts that left: a non-fatal nudge to prefix
223-
// the name with the package namespace at authoring time, so the clash is
224-
// unlikely to ever happen. Objects are already prefix-*enforced* (error)
225-
// in defineStack; views are object-derived; `doc` has its own build lint
226-
// — so they are excluded here. Warning only — prefix stays a recommended
227-
// convention for legacy types, not a retroactive requirement.
219+
// ── Intra-package duplicate-name advisory (ADR-0048 §3.4) ──
220+
// ADR-0048 §3.4 retired the per-item CROSS-package throw: package ids are
221+
// globally unique, so two installed packages shipping the same bare name
222+
// (e.g. `page/home`) legitimately COEXIST under distinct composite keys and
223+
// each caller resolves to its own via package-scoped resolution. A bare name
224+
// is therefore NOT a collision risk and must not warn on its own.
225+
//
226+
// What the lint still earns its keep on is the narrow authoring-time hygiene
227+
// case the ADR explicitly leaves to `os lint`: "an author shipping two
228+
// `page/home` in one package". Two items of the same (type, name) declared
229+
// within ONE package's config share a single composite registry key and
230+
// shadow each other (last-write-wins). We only see one package's config here,
231+
// so the legitimate signal is a genuine duplicate `(type, name)` pair within
232+
// it — never a unique bare name.
233+
//
234+
// Objects are already prefix-*enforced* (error) in defineStack; views are
235+
// object-derived; `doc` has its own build lint — so they are excluded here.
228236
const ns: string | undefined = config.manifest?.namespace;
229-
if (ns) {
230-
// A name is namespace-safe if it IS the namespace (ADR-0019: a package's
231-
// single app is conventionally named after the namespace, e.g. `crm`),
232-
// is prefixed with it (`crm_lead`), or is a platform-reserved `sys_` name.
233-
const isNamespaceSafe = (name: string) =>
234-
name === ns || name.startsWith(`${ns}_`) || name.startsWith('sys_');
235-
236-
// Bare-named UI/automation types subject to the cross-package collision
237-
// (ADR-0048 §1.1). Data-driven so a new bare-named type is one line.
238-
const PREFIXED_TYPES: Array<{ key: string; label: string }> = [
239-
{ key: 'apps', label: 'App' },
240-
{ key: 'pages', label: 'Page' },
241-
{ key: 'dashboards', label: 'Dashboard' },
242-
{ key: 'flows', label: 'Flow' },
243-
{ key: 'actions', label: 'Action' },
244-
{ key: 'reports', label: 'Report' },
245-
{ key: 'datasets', label: 'Dataset' },
246-
];
247-
248-
for (const { key, label } of PREFIXED_TYPES) {
249-
const items: any[] = Array.isArray(config[key]) ? config[key] : [];
250-
for (let i = 0; i < items.length; i++) {
251-
const name = items[i]?.name;
252-
if (typeof name !== 'string' || !name || isNamespaceSafe(name)) continue;
253-
issues.push({
254-
severity: 'warning',
255-
rule: 'naming/namespace-prefix',
256-
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}".`,
257-
path: `${key}[${i}].name`,
258-
fix: `${ns}_${name}`,
259-
});
237+
238+
// Bare-named UI/automation types that share the generic registry namespace.
239+
// Data-driven so a new bare-named type is one line.
240+
const PREFIXED_TYPES: Array<{ key: string; label: string }> = [
241+
{ key: 'apps', label: 'App' },
242+
{ key: 'pages', label: 'Page' },
243+
{ key: 'dashboards', label: 'Dashboard' },
244+
{ key: 'flows', label: 'Flow' },
245+
{ key: 'actions', label: 'Action' },
246+
{ key: 'reports', label: 'Report' },
247+
{ key: 'datasets', label: 'Dataset' },
248+
];
249+
250+
for (const { key, label } of PREFIXED_TYPES) {
251+
const items: any[] = Array.isArray(config[key]) ? config[key] : [];
252+
// First occurrence of each name → its index, so a later duplicate can point
253+
// back at the original declaration.
254+
const firstSeen = new Map<string, number>();
255+
for (let i = 0; i < items.length; i++) {
256+
const name = items[i]?.name;
257+
if (typeof name !== 'string' || !name) continue;
258+
const original = firstSeen.get(name);
259+
if (original === undefined) {
260+
firstSeen.set(name, i);
261+
continue;
260262
}
263+
// Genuine intra-package duplicate: two items of the same (type, name)
264+
// declared in this package's config. They collapse onto one registry key
265+
// and shadow each other. Renaming one with the package namespace prefix
266+
// (`crm_home`) is the simplest fix; any distinct name works.
267+
const suggestion = ns && !name.startsWith(`${ns}_`) ? `${ns}_${name}` : undefined;
268+
issues.push({
269+
severity: 'warning',
270+
rule: 'naming/namespace-prefix',
271+
message:
272+
`${label} "${name}" is declared more than once in this package ` +
273+
`(also at ${key}[${original}].name). Two items of the same type sharing ` +
274+
`a bare name within one package shadow each other on the registry key ` +
275+
`(ADR-0048 §3.4) — rename one${suggestion ? `, e.g. "${suggestion}"` : ''}. ` +
276+
`Distinct packages may reuse the same name freely; the namespace prefix ` +
277+
`is an optional convention, not a collision-avoidance requirement.`,
278+
path: `${key}[${i}].name`,
279+
...(suggestion ? { fix: suggestion } : {}),
280+
});
261281
}
262282
}
263283

0 commit comments

Comments
 (0)