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
48 changes: 48 additions & 0 deletions .changeset/approval-escalation-enabled-default-flip.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/spec": minor
"@objectstack/plugin-approvals": minor
---

feat(spec,plugin-approvals): `escalation.enabled` defaults to `true` and the SLA sweep finally reads it (#12278)

**BREAKING** semantic default flip on a published authorable key, shipped as
`minor` under the repo's launch-window convention for breaking changes.
Maintainer ruling 2026-08-27 (Option C), explicitly reversing the 2026-08-26
"spec stays as declared" ruling with fresh analysis.

`ApprovalEscalationSchema.enabled` declared `default(false)` while the
plugin-approvals escalation sweep never read the key: any escalation block
with a positive `timeoutHours` escalated, and with `action: 'auto_approve'`
that silently approved requests their author had declared off the clock —
the ADR-0049 declared-but-unenforced shape, failing open. Worse, the
approval-node executor parses node config through the schema before
snapshotting it onto the request row, so the old default **materialized**
`enabled: false` into storage for every author who omitted the key, making
"authored off" and "defaulted off" byte-identical at the sweep site.

One change, both halves:

- **spec**: `enabled` now defaults to `true` (stays `z.boolean()`; no
tri-state). The feature-level switch is whether an `escalation` block
exists at all; within a block carrying `timeoutHours`, escalation is on
unless explicitly turned off — which is what the runtime, its eleven
behaviour tests, and every teaching surface have always meant. Declared in
`DEFAULT_CHANGES_BY_MAJOR` (17) and registered as the
`approval-escalation-enabled-default-flip` semantic migration entry.
- **runtime**: `runEscalations` skips a request whose snapshot carries an
explicit `escalation.enabled === false` — the declared switch is enforced.
Request snapshots created **before** the flip cutoff
(`ESCALATION_ENABLED_FLIP_CUTOFF_MS`, 2026-08-28T00:00:00Z) ride a
read-side legacy window and keep escalating exactly as they do today: their
stored `false` is overwhelmingly the old schema default materialized onto an
author who never wrote the key, every such stored row is escalating today,
and the window retires itself as pending requests drain — zero tenant rows
rewritten.

Deployed metadata that omits `enabled` does not change behaviour (it
escalated before, it escalates after). What changes is that writing
`enabled: false` finally binds for newly opened requests — the declared
intent being honoured. To keep an SLA off, write `enabled: false`; to
escalate on timeout, an `escalation` block with `timeoutHours` is enough.

<!-- adr-0087: registered approval-escalation-enabled-default-flip -->
4 changes: 2 additions & 2 deletions content/docs/references/automation/approval.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -37,7 +37,7 @@ const result = ApprovalDecision.parse(data);

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **enabled** | `boolean` | optional (default: `false`) | Enable SLA-based escalation for this node |
| **enabled** | `boolean` | optional (default: `true`) | SLA escalation switch. Defaults to true: an escalation block carrying timeoutHours is live unless this is explicitly false — the feature-level switch is whether the escalation block exists at all |
| **timeoutHours** | `number` | ✅ | Hours before escalation triggers |
| **action** | `Enum<'reassign' \| 'auto_approve' \| 'auto_reject' \| 'notify'>` | optional (default: `"notify"`) | Action on escalation timeout |
| **escalateTo** | `string` | optional | User id or position machine name to escalate to |
Expand DownExpand Up@@ -101,7 +101,7 @@ const result = ApprovalDecision.parse(data);

| Property | Type | Required | Description |
| :--- | :--- | :--- | :--- |
| **enabled** | `boolean` | optional (default: `false`) | Enable SLA-based escalation for this node |
| **enabled** | `boolean` | optional (default: `true`) | SLA escalation switch. Defaults to true: an escalation block carrying timeoutHours is live unless this is explicitly false — the feature-level switch is whether the escalation block exists at all |
| **timeoutHours** | `number` | ✅ | Hours before escalation triggers |
| **action** | `Enum<'reassign' \| 'auto_approve' \| 'auto_reject' \| 'notify'>` | optional (default: `"notify"`) | Action on escalation timeout |
| **escalateTo** | `string` | optional | User id or position machine name to escalate to |
Expand Down
3 changes: 3 additions & 0 deletions docs/protocol-upgrade-guide.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -334,6 +334,9 @@ Finally it removes the 'pdf' member of `view.exportOptions` formats (#8010, main
- **`apimethod-enum-shrink`** — `data.object.enable.apiMethods (the eight legacy non-primitive values)` → the six primitives only — `get` / `list` / `create` / `update` / `delete` / `bulk`: replace each legacy value with the primitives it derives from, de-duplicate, and delete the key entirely if the result names all six
- Why not automatic: The authored `enable.apiMethods` enum is now exactly the six primitives. The eight legacy values — `upsert`, `aggregate`, `history`, `search`, `restore`, `purge`, `import`, `export` — are no longer authorable, because they are DERIVED effective operations resolved by the server's single derivation table, and an enum that lets an author name both a primitive and something derived from it has two spellings for one fact. The FROM → TO is a table rather than a rename: `upsert` → `create` + `update`; `import` → `create` + `update`; `export`, `aggregate` and `search` → `list`; `history` → `get`; and `restore` / `purge` map to NOTHING — they never derived, because `enable.trash` was retired in #2377, so the value is deleted outright. That last row is why this is a semantic entry and not a mechanical conversion, and the reason is a security one: the mapping WIDENS. An allowlist naming `history` was granting read of one record's audit trail; rewritten to `get` it grants ordinary record reads, and an allowlist naming `search` becomes a grant of full `list`. A transform that applied the table silently would broaden real API permissions without anyone reading the diff, so the rewrite is delegated to the author with the widening flagged. The reporter codemod exists for exactly that shape: `node scripts/codemod/apimethods-legacy-to-primitives.mjs` scans, reports the exact replacement per site, and FLAGS the allowlists the mapping would widen so the edit stays reviewable — it reports, it does not rewrite. Stored metadata keeps parsing (permanent tolerance, narrowing only), so nothing breaks at rest; what changes is what an author may newly write. Registered by the #6350 stock reconciliation; #3543 (P2 of #3391) predates the #6148 completeness gate. ADR-0087, #3543 (backfilled #6350).
- Done when: No authored `enable.apiMethods` array names a legacy value; `objectstack validate` passes. Run the reporter codemod first and read its widening flags before applying anything — ⚠️ the migration is only correct if each widened grant was INTENDED. For every object where `history` became `get` or `search` became `list`, confirm the broader operation is one the API should genuinely expose; where it is not, the answer is not a different value in this enum but a permission set that withholds the operation. Where the six primitives are all present, prefer deleting the key: that is equivalent to default-open and it tracks future primitives, whereas a hand-listed six silently stops granting anything added later. `restore` / `purge` are deleted with no replacement — if trash-like behaviour was being relied on, that capability left in #2377 and this entry is not where it returns.
- **`approval-escalation-enabled-default-flip`** — `automation.ApprovalEscalation.enabled — an OMITTED value inside an approval node's escalation block` → nothing, for the common intent (escalate on timeout): an escalation block carrying timeoutHours is live by default. To declare an SLA OFF while keeping its configuration, write enabled: false explicitly — which is now the spelling the escalation sweep actually reads
- Why not automatic: A DECLARED-DEFAULT CORRECTION plus the enforcement that makes the key real (#12278, maintainer ruling 2026-08-27) — the same category as protocol 17's `import-run-automations-declared-default-corrected`: the schema promised `enabled` defaults to `false` (SLA off) while the plugin-approvals sweep never read the key at all — any escalation block with a positive `timeoutHours` escalated, and with `action: 'auto_approve'` that silently approved requests their author had declared off the clock. The flip moves the default to `true` and, in the same change, the sweep starts honouring an explicit `enabled: false`. The feature-level switch is whether an `escalation` block exists at all; within a block carrying `timeoutHours`, escalation is on unless explicitly turned off. Deployed metadata that OMITS `enabled` does not change behaviour: it escalated before (the sweep ignored the key) and escalates after (the parse materializes `true`). Stored request snapshots written before the flip carry a MATERIALIZED `enabled: false` (the approval-node executor parses config through the old schema before snapshotting), so the sweep keeps a read-side legacy window keyed on the snapshot's `created_at`: pre-flip snapshots keep escalating exactly as they do today, and the window retires itself as those pending requests drain. What DOES change is that an explicit `enabled: false` finally binds — a flow that authored it (e.g. the console toggle switched off after a timeout was set) stops escalating on requests opened after the upgrade, which is the declared intent being honoured.
- Done when: A flow whose approval node omits `enabled` inside `escalation` still escalates on timeout (no metadata edit needed). A flow that writes `enabled: false` stops escalating for newly opened requests — verify one such request stays pending past its `timeoutHours` with no `escalate` audit row and no auto-decision. Requests opened BEFORE the upgrade keep their pre-upgrade behaviour (they escalate) regardless of the stored `enabled` bit. Clients that parse metadata through the published JSON Schema now materialize `enabled: true` where they materialized `false`; a client that needs the SLA off must write it explicitly.
- **`audit-log-action-enum-retired`** — `sys_audit_log.action — the values 'export' and 'permission_change' left the select enum declared by plugin-audit (packages/plugins/plugin-audit/src/objects/sys-audit-log.object.ts). The same two values also left the shipped list-view filters on that object: 'permission_change' from the auth_events view and 'export' from the config_changes view` → nothing, for either value — both are removed rather than renamed, because neither named an event this platform records. For permission changes, read the ordinary `create` / `update` rows on the permission objects themselves: a grant or binding write is an ordinary record write and the generic audit writer already ledgers it, so a second semantically-duplicate row was never minted. For `export` there is no replacement and nothing is lost: no export feature ever wrote an audit row. A consumer filtering `sys_audit_log` on either value was reading an empty result set on every deployment, and still is — what changed is that the contract no longer promises otherwise
- Why not automatic: Maintainer ruling 2026-08-12 (#7675), the retirement half of a two-half verdict: the cheap writers get built (#8144 login/logout, #8145 config_change) and the enum values with no feature behind them are retired. 原则记录:空 widget + 永远查不到东西的过滤器是可见产品缺陷;审计面宁窄勿谎. The defect was false compliance on a COMPLIANCE surface, which is the sharpest form of ADR-0049 declared-≠-enforced: an auditor reading the action enum believed the platform captured permission changes and data exports, and the shipped list views and dashboard widgets showed them a filter and a tile for exactly those events. Both were permanently empty. Measured by enumerating every `sys_audit_log` writer in the repo — there are exactly two: plugin-audit`s generic hook writer, whose `actionFor` maps afterInsert/Update/Delete to create/update/delete and nothing else, and plugin-auth`s admin user-import. Neither has ever emitted `export` or `permission_change`. This is an enum-VALUE retirement, so the bookkeeping differs from a key retirement in the two ways `hook-body-crypto-hash-removed`, `dataset-measure-array-string-agg-removed` and `action-global-nav-location-removed` already record: nothing lands in RETIRED_KEYS_BY_MAJOR (no authorable KEY changed) and the four surface ratchets are expected to be byte-identical (no def changed). It differs from all three in being a SEMANTIC entry rather than a D2 conversion, and the reason is that there is no source to rewrite: `sys_audit_log` is a platform-owned, append-only object whose every field is `readonly: true`. Nobody authors an audit row and nobody authors this enum — the values appear only in rows the runtime writes and in queries consumers send. A conversion rewrites authored metadata or a stored `sys_metadata` row; this surface is neither, so the disposition is the one `BatchOptions.validateOnly` and the notification cursor already take in this major. ⚠️ Historical ROWS are deliberately untouched. A deployment that somehow holds a row with either value keeps it, and keeps reading it back: the enum is not enforced on this object at all (`validateRecord` skips `readonly` fields, and every field here is readonly), so nothing rejects stored history and no backfill is required or wanted. Deleting audit history to satisfy a schema narrowing would be the one genuinely destructive reading of this change. ADR-0049 / ADR-0087, #8147.
- Done when: No consumer filters `sys_audit_log` on `action = "export"` or `action = "permission_change"` expecting rows: both were empty everywhere before this change, so a query that returned data has not been identified and a query that returned nothing behaves identically. Concretely, check three places. (1) Saved queries, dashboards and reports over `sys_audit_log`: a filter naming either value should be deleted, not re-pointed — for permission auditing, filter the permission objects` own `create`/`update` rows by `object_name` instead. (2) Any code branching on the action string (a badge map, a label switch, an `if (row.action === ...)`): the arms for these two values are now unreachable and should go, and a `switch` with an exhaustiveness check over the enum type will now fail to compile if they stay — that compile error is the enforced channel for TypeScript consumers. (3) Custom objects or plugins inserting `sys_audit_log` rows with either value: this is the only case that needs a real decision, because the write will NOT be refused (readonly fields are not validated) — it will simply be a row whose action the object no longer declares. Pick a declared value or open an issue for the action you actually need. ⚠️ Do NOT migrate or delete existing rows: audit history is append-only and stays exactly as written.
Expand Down
Loading
Loading