Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions .changeset/fix-lint-namespace-prefix-adr0048.md
Original file line number Diff line number Diff line change
@@ -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.
25 changes: 25 additions & 0 deletions .changeset/ga-p0-5-single-instance-caps.md
Original file line number Diff line number Diff line change
@@ -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.
19 changes: 19 additions & 0 deletions .changeset/ga-p1-2-job-run-retention.md
Original file line number Diff line number Diff line change
@@ -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.
20 changes: 20 additions & 0 deletions .changeset/ga-p1-2-messaging-automation-retention.md
Original file line number Diff line number Diff line change
@@ -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.
41 changes: 38 additions & 3 deletions docs/launch-readiness.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

---

Expand Down Expand Up @@ -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`)
Expand Down Expand Up @@ -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
Expand All @@ -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.

---

Expand Down
100 changes: 60 additions & 40 deletions packages/cli/src/commands/lint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, number>();
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 } : {}),
});
}
}

Expand Down
Loading