Skip to content

Commit 07e954b

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-5189-publish-gate-backstop
2 parents ade78fe + d25f20b commit 07e954b

25 files changed

Lines changed: 3017 additions & 35 deletions
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
"@objectstack/plugin-email": minor
3+
---
4+
5+
fix(plugin-email): `sys_email` rows stranded at `queued` are swept at boot, and a failed drain says so at `error` (#5161)
6+
7+
`status: 'queued'` had exactly one consumer: the `afterInsert` outbox drain that
8+
fires during the insert itself (plus, since #5160, the `email.send.async` job
9+
`send()` publishes). Nothing ever looked at such a row again. A process that
10+
died between the insert and the delivery — or a drain whose delivery threw —
11+
left the row at `queued` **forever**: a state named after a queue that had no
12+
reader, while the caller had already been told the message was accepted.
13+
14+
**A once-per-boot sweep is now that reader.** At `kernel:ready`, after the
15+
registries are settled and the `email.send.async` subscriber is attached,
16+
`sweepStrandedOutbox` picks up `sys_email` rows still at `queued` and advances
17+
them:
18+
19+
- **durable queue delivery on** → the row is published as an `{ rowId }` job to
20+
`email.send.async` through the same producer, options and
21+
`sys_email:<id>` idempotency key `send()` uses, so a row that still has a
22+
pending job collapses onto it instead of putting a second worker on it;
23+
- **inline delivery** → the row is delivered and finalized in place (`sent` /
24+
`failed`), which is what the drain hook would have done had the process lived.
25+
26+
Only rows **older than five minutes** are eligible. A row inserted seconds ago
27+
is not stranded, it is someone's in-flight work — this process's `send()`, its
28+
deferred drain hook, or the same on another instance — and sweeping it would
29+
send that message twice. (Age, not "created before this boot": one instance's
30+
boot time says nothing about a sibling's row inserted a second ago.) Rows this
31+
process is delivering right now, and rows that already carry a `message_id`, are
32+
skipped. The batch is bounded at 500 rows per boot, oldest first, and says so
33+
when it truncates. One `info` line reports the counts; boot does **not** wait on
34+
the sweep, and a sweep that cannot run reports at `error` rather than relying on
35+
`kernel:ready` error propagation.
36+
37+
**Drain-hook failures are now `error`, not `warn`.** A drain that throws means
38+
the mail was not sent while the insert reported success and the row still reads
39+
`queued` — the durability class the degradation-log-level rule pins at `error`.
40+
Both lines now name the consequence (this message was NOT sent, the row stays at
41+
`queued`) and the fix (the boot sweep picks it up on the next restart; turn on
42+
durable queue delivery to have failures retried and dead-lettered instead).
43+
`deliverPersistedRow` joins `DURABILITY_CRITICAL_CALLEES`, so a future `catch`
44+
that quietly downgrades it fails `pnpm check:durability-log-level`.
45+
46+
New exports: `sweepStrandedOutbox`, `OUTBOX_OBJECT`, `OUTBOX_SWEEP_MIN_AGE_MS`,
47+
`OUTBOX_SWEEP_LIMIT`, `EmailService.enqueuePersistedRow`, and
48+
`EmailServicePlugin.outboxSweepSettled` (the sweep's promise, for callers that
49+
need determinism). The normal `send()` → deliver path is byte-for-byte
50+
unchanged.
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
---
2+
"@objectstack/platform-objects": minor
3+
"@objectstack/service-queue": minor
4+
---
5+
6+
fix(service-queue): `sys_job_queue` no longer grows forever — `completed` rows expire on a declared 7-day retention (#5179)
7+
8+
`DbQueueAdapter` marked a delivered message `status: 'completed'` and then
9+
**nothing ever touched that row again**. `purge()` had zero production callers
10+
(tests only), `purgeFailed()` is a manual dead-letter API, and the object
11+
declared no lifecycle policy at all — so every queue delivery left a permanent
12+
row, which since #5160 means one permanent row per queued email.
13+
14+
`sys_job_queue` now declares an ADR-0057 policy and the platform
15+
`LifecycleService` enforces it on its existing hourly sweep:
16+
17+
```ts
18+
lifecycle: {
19+
class: 'transient',
20+
retention: { maxAge: '7d', onlyWhen: { status: 'completed' } },
21+
}
22+
```
23+
24+
**Only `completed` rows are swept.** `pending` / `running` are live work, and
25+
`failed` / `dlq` are the dead-letter queue — they exist to wait for a human, so
26+
they are never deleted automatically at any age. `listFailed()` / `replay()` /
27+
`purgeFailed()` remain the only way a dead letter leaves the table. This is
28+
also why the policy is `retention` (age + row filter) rather than a `ttl` on
29+
`completed_at`: TTL has no row filter, and `dlq` rows stamp `completed_at` too.
30+
31+
**No new configuration, and no new sweeper.** ADR-0057 §3.3 puts one reaper in
32+
the platform rather than one per plugin — the same call the sibling
33+
`sys_job_run` (30d) already makes. Any kernel with a data engine already runs
34+
it, its per-sweep `[lifecycle] sweep: … ~N rows reaped` line now accounts for
35+
this table too, and the window is overridable per environment through the
36+
`lifecycle` settings namespace without touching code.
37+
38+
**The dedup window is now an enforced invariant, not a coincidence.** Publish
39+
dedups against a terminal row by comparing its `created_at` to
40+
`idempotencyWindowMs` (default 24h), and the reaper cuts off on that same
41+
`created_at` axis — so retention (7d) ≥ dedup window is what keeps "duplicate
42+
publishes inside the window are suppressed" true. `DbQueueAdapter` reads the
43+
declared window (new export `completedRetentionWindowMs()`) and **throws at
44+
construction** if `idempotencyWindowMs` is configured longer than it, instead of
45+
silently degrading into duplicate deliveries days later. If you raise
46+
`idempotencyWindowMs` past 7 days, raise the object's declared retention (or the
47+
`lifecycle` settings override) to match — the error message names both numbers.
48+
49+
`class: 'transient'` is deliberate: `telemetry`/`event`/`audit` classes
50+
relocate their table to the dedicated `telemetry` datasource wherever one is
51+
registered (ADR-0057 §3.6), and moving a live work queue's storage would be a
52+
migration, not a cleanup.
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
---
2+
"@objectstack/plugin-sharing": patch
3+
---
4+
5+
fix(sharing): deleting a record now revokes every `sys_record_share` row on it, whatever the source (#5103)
6+
7+
A share row says "principal P has level L on (object O, record R)". Delete R and
8+
the row describes nothing — yet until now it stayed in the table forever.
9+
10+
#4779 (PR #5102) bound an `afterDelete` for this, but inside the sharing-RULE
11+
package, where two conditions fenced it in: it revokes only `source: 'rule'`
12+
rows, and it binds only on objects that appear in `sys_sharing_rule`. So an
13+
object that uses nothing but MANUAL shares — a `sharingModel: 'private'` object
14+
with no rule ever configured — had no delete hook at all, and **manual share +
15+
record delete = a permanent orphan**.
16+
17+
Today the harm is bounded, and only because record ids are never reused: the
18+
`record_id IN (…)` predicate `buildReadFilter` emits matches nothing. Nothing
19+
enforces that assumption. A custom primary key, an import that preserves ids, or
20+
any future id recycling turns every one of those rows into a real privilege
21+
escalation — a new record landing on a recycled id inherits the dead record's
22+
recipients outright. Secondarily, `sys_record_share` grew without bound and
23+
Setup's Record Shares list showed rows pointing at nothing.
24+
25+
**What changed**
26+
27+
- **A record-delete cascade on every sharing-capable object.** `plugin-sharing`
28+
binds one `beforeDelete`/`afterDelete` pair with no object filter and judges
29+
the object's sharing posture from its `sharingModel` metadata *per delete*.
30+
Nothing is enumerated at boot, so nothing goes stale: an object that gains
31+
`sharingModel` at runtime is covered on its very next delete, with no rebind.
32+
Bounded deletes (a scalar id, an `$in` list, or a predicate matching at most
33+
1000 rows) are revoked synchronously and set-based; an unbounded one queues an
34+
object-scoped orphan sweep instead. System-context deletes cascade too.
35+
- **A boot-time orphan sweep keyed on record existence.** On
36+
`kernel:bootstrapped`, share rows whose RECORD no longer exists are revoked —
37+
historical orphans, rows a failed hook missed, and the one posture the cascade
38+
deliberately skips (an unmarked system object). This is a different question
39+
from the existing `sweepOrphanedRuleGrants`, which asks whether the RULE row
40+
still exists and therefore can never see a manual share. Bounded per boot:
41+
keyset pages, one batched existence probe per object per page, and a scan cap
42+
that reports when it stopped early. An object whose existence probe FAILS has
43+
its rows left in place — "could not ask" is never read as "the record is gone".
44+
45+
**What did not change**
46+
47+
Rule *recompute* still never touches a manual share. That boundary (#5102) is
48+
the point: while the record exists, a manual grant is a human decision no rule
49+
evaluation may overrule. Only the record's DELETION revokes it, and only because
50+
there is no longer anything to have access to.
51+
52+
New exports for hosts that compose the plugin by hand:
53+
`bindRecordShareCascade` / `unbindRecordShareCascade`,
54+
`objectCanCarryRecordShares`, `SharingService.revokeSharesForDeletedRecords`,
55+
`SharingService.sweepOrphanedRecordShares`, and `effectiveSharingModel`. Nothing
56+
was removed or renamed; the standard `SharingServicePlugin` composition needs no
57+
changes.

packages/platform-objects/src/audit/sys-job-queue.object.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ import { ObjectSchema, Field } from '@objectstack/spec/data';
2222
* Writers: `DbQueueAdapter` (publish/lease/complete/fail).
2323
* Readers: Studio DLQ view, ops dashboards, the adapter's worker loop.
2424
*
25+
* Retention: `completed` rows are swept by the platform LifecycleService —
26+
* see the `lifecycle` block below (#5179).
27+
*
2528
* @namespace sys
2629
*/
2730
export const SysJobQueue = ObjectSchema.create({
@@ -31,6 +34,56 @@ export const SysJobQueue = ObjectSchema.create({
3134
icon: 'inbox',
3235
isSystem: true,
3336
managedBy: 'engine-owned',
37+
38+
/**
39+
* [ADR-0057 §3.1/§3.3, #5179] The queue table only ever GREW: the adapter
40+
* marks a delivered message `completed` and nothing ever touched the row
41+
* again (`purge()` had zero production callers, `purgeFailed()` is a manual
42+
* dead-letter API). Since #5160 that is one permanent row per email.
43+
*
44+
* Bounded declaratively rather than by a sweeper inside `DbQueueAdapter`:
45+
* ADR-0057 §3.3 puts ONE reaper in the platform (`LifecycleService`), not N
46+
* per-plugin ones — the same call the sibling `sys_job_run` already makes.
47+
* That the writer is the adapter itself (never user data) is what makes an
48+
* unattended delete safe here; the declaration is where an operator can see
49+
* the window, and `lifecycle` settings can override it per environment
50+
* without a code change.
51+
*
52+
* `onlyWhen: { status: 'completed' }` is the whole safety story:
53+
* - `pending` / `running` are LIVE work — reaping them would drop
54+
* undelivered messages;
55+
* - `dlq` / `failed` are the dead-letter surface and exist precisely to
56+
* wait for a human (`listFailed` / `replay` / `purgeFailed`), so they
57+
* are never swept automatically, at any age.
58+
* This is also why the policy is `retention` (age by `created_at` + row
59+
* filter) and not `ttl` on `completed_at`: TTL has no row filter, and `dlq`
60+
* rows stamp `completed_at` too — a TTL would eat the dead-letter queue.
61+
*
62+
* Window = 7d, and it MUST stay ≥ the adapter's idempotency window
63+
* (`DbQueueAdapterOptions.idempotencyWindowMs`, default 24h): publish
64+
* dedups against terminal rows by comparing `created_at` to that window
65+
* (`db-queue-adapter.ts`), and the Reaper cuts off on the very same
66+
* `created_at` axis — so a retention ≥ the dedup window means a row the
67+
* dedup check still needs can never have been reaped, with no clock skew
68+
* between the two rules. 7d gives a week of delivery history for debugging
69+
* and 7× headroom over the default dedup window. `DbQueueAdapter` reads
70+
* this declaration and refuses to start when the two are configured the
71+
* wrong way round, so the invariant cannot drift apart silently.
72+
*
73+
* `class: 'transient'` ("workflow / ephemeral state" — ADR-0057 §3.1), not
74+
* `telemetry`: this is live work state, not a log, and per §3.6 a
75+
* `telemetry`/`event`/`audit` class RELOCATES the table to the dedicated
76+
* `telemetry` datasource wherever one is registered. Moving a live queue's
77+
* store is a migration, not a cleanup — `transient` deliberately stays on
78+
* the primary.
79+
*/
80+
lifecycle: {
81+
class: 'transient',
82+
retention: {
83+
maxAge: '7d',
84+
onlyWhen: { status: 'completed' },
85+
},
86+
},
3487
description: 'Durable job/message queue including dead letters',
3588
displayNameField: 'queue',
3689
nameField: 'queue', // [ADR-0079] canonical primary-title pointer (mirrors deprecated displayNameField)

0 commit comments

Comments
 (0)