Skip to content

feat(spec,approvals): escalation.enabled defaults to true and the SLA sweep enforces it (#12278) - #12737

Merged
os-sales merged 2 commits into
mainfrom
claude/issue-12278-enforce-escalation-enabled
Aug 28, 2026
Merged

feat(spec,approvals): escalation.enabled defaults to true and the SLA sweep enforces it (#12278)#12737
os-sales merged 2 commits into
mainfrom
claude/issue-12278-enforce-escalation-enabled

Conversation

@os-sales

Copy link
Copy Markdown
Collaborator

Fixes#12278

Implements the Option-C package ruled by the maintainer on 2026-08-27 (issue comment 5434864364, which explicitly reverses point 2 of the 2026-08-26 Route-1 ruling): the spec declaration moves to the truth, and the ruled sweep gate lands verbatim on top of it.

What changed

  • specApprovalEscalationSchema.enabled: .default(false).default(true) (stays z.boolean(), no tri-state), with a docstring teaching the ruled semantics: the feature switch is whether an escalation block exists at all; within a block carrying timeoutHours, escalation is on unless explicitly turned off (packages/spec/src/automation/approval.zod.ts:616).
  • default-change declaration (gate-required rider)packages/spec/scripts/lib/default-changes.ts gains the automation/ApprovalEscalation:enabled entry under major 17 (check:authorable-surface refuses an in-place default move until it is declared there — measured, the gate is green with the entry and the flip). packages/spec/src/migrations/entries/semantic/17.approval-escalation-enabled-default-flip.ts registers the semantic migration entry (precedent: 17.import-run-automations-declared-default-corrected, the previous declared-default correction).
  • runtimerunEscalations now skips a pending request whose snapshot carries an explicit escalation.enabled === false (packages/plugins/plugin-approvals/src/approval-service.ts). Strict === false: an absent key escalates (pre-materialization historical rows keep behaviour, and the new default materializes true).
  • transition (i), read-side, keyed on snapshot created_at — snapshots created before ESCALATION_ENABLED_FLIP_CUTOFF_MS (2026-08-28T00:00:00Z) are exempt from the gate: their stored false is the old schema default materialized by the executor parse, and every stored false + timeoutHours row is escalating today (the ruling's measured basis), so exempting them is behaviour-preserving. Zero tenant rows rewritten; the window self-retires as pending requests drain.
  • teaching surfaces, same stroke — docstring (above); authorable-defaults/automation.json now records = true (regenerated); generated reference pages (content/docs/references/automation/approval.mdx) regenerated by repo tooling; spec-changes.json + docs/protocol-upgrade-guide.md regenerated from the registry.
  • changeset — one changeset, @objectstack/spec minor + @objectstack/plugin-approvals minor, with the BREAKING annotation (launch-window convention) and the adr-0087: registered approval-escalation-enabled-default-flip disposition (the marker the check:adr-0087-registration gate requires for a declared-breaking changeset — measured green).

Measured: surfaces the ruling listed that needed NO edit

  • skills/objectstack-automation/SKILL.md — measured for every spelling of a default-false statement (default.*false, false.*default, off by default, disabled by default, case-insensitive): zero hits. Line 610 documents the key set without stating a default and the "set enabled: true" guidance stays valid, so the net-line-budget-0 constraint is met by a zero-line diff. No skills edit in this PR.
  • content/docs/automation/approvals.mdx — same measurement, zero default statements; its "set enabled: true and timeoutHours" guidance stays valid per the ruling. Untouched.
  • ADR sweep: docs/adr/0042-approval-sla-escalation.md never mentions enabled (grep, zero hits) — no ADR governs the flipped default; ADR-0049 is the gap this closes.

Transition choice: (i) read-side over (ii) normalization — reasoning

The ruling offered (i) read-side keyed on snapshot created_at (zero tenant writes, self-retiring) or (ii) idempotent normalization enabled := true where timeoutHours > 0, pick the cheaper after verifying premises. Measured: this repo's migration precedents (metadata-protocol/src/migrations/*, plugin-approvals' own backfill-platform-row-organizations.ts) run per-boot and rely on predicate idempotency — there is no run-once ledger. Normalization's predicate (enabled=false ∧ timeoutHours>0) re-matches post-flip authored-false rows forever, so a per-boot (ii) would eventually rewrite exactly the rows the new pin protects (an explicit false a code-first author writes after the flip), and a safe (ii) needs new run-once machinery plus tenant-data rewrites. (i) needs neither. Chosen: (i).

Honest bound, recorded in the constant's doc comment: the cutoff is a compiled constant, so an install that upgrades long after 2026-08-28 has pending requests created between the cutoff and its upgrade whose materialized false post-dates the cutoff; those honour the declared off. The population is bounded by each request's own timeoutHours window around the upgrade and self-drains. The alternatives (per-install marker row, or normalization) buy that edge back at the price of new machinery or tenant writes — both the more expensive option under the ruling's instruction.

Premise readings (dev-FIRST, measured before any edit)

  • p1 — sweep consumes only open-request snapshots; no other runtime reader: confirmed at d29e42f. runEscalations reads sys_approval_request (status: 'pending') → node_config_json; escalateRequest is called only from the sweep; repo-wide grep for (esc|escalation|escalationConfig|escalationCfg|sla)\.enabled over packages/ + apps/: zero hits outside the declaration. slaDueAt reads only timeoutHours and stays ungated (ruled, point 6).
  • p2 — flow definitions store RAW config: confirmed. FlowSchema node config is z.record(z.string(), z.unknown()) (flow.zod.ts:271) — flow-level parse never descends into ApprovalNodeConfigSchema; the executor (approval-node.ts:133, re-verified) is the only production parse site, and its parse happens at request-open time. So the transition's reach is request snapshots only: a definition that omits enabled opens post-flip requests that materialize true (behaviour preserved), and a definition with authored false opens requests that are skipped (the declared intent, per the ruling).
  • p3 — snapshot created_at availability: confirmed. openNodeRequest stamps created_at from the injected clock (approval-service.ts:2064/2117), the sweep already reads raw.created_at for the due computation, and the column is declared on sys-approval-request.object.ts:296. A row with a missing/unparsable created_at fails the cutoff comparison into the legacy (escalate) branch and is then dropped by the existing due-date check — no new failure mode.
  • Other parse sites of the schema (PM mechanism assumption): production parse sites are the executor and the memoized JSON-Schema generation (getApprovalNodeConfigJsonSchema, republished to the Studio designer with the new default automatically). Spec tests and lint fixtures author enabled: true explicitly and are unaffected. objectui mirrors are out of this PR's surface — one drift noted for the PM below.

Tests & verification (gate union, pin file and ablation at head 64cff1b8; the full suites ran on the byte-identical tree the single commit captured — zero edits between suite runs and commit, git status clean before and after)

  • @objectstack/spec: full suite 435 files / 11542 tests passed + typecheck green in the same lock-held run (VERDICT command-exit 0). New pin: omitted enabled inside a block materializes true; explicit false survives; a config with no escalation block keeps the key absent (no-block vs declared-off stay distinguishable).
  • @objectstack/plugin-approvals: full suite 33 files / 613 tests passed + typecheck green (VERDICT command-exit 0). The 11 pre-existing behaviour tests (omit enabled, expect escalation) stay green and are now honest. Three new pins, each asserting concrete outcomes (status, audit-action sequence, resume calls — not just counters):
    1. explicit enabled: false post-flip snapshot does not escalate (auto_approve shape: stays pending, actions = ['submit'], zero resumes);
    2. pre-flip snapshot with materialized falsekeeps escalating (legacy window: approved, ['submit','escalate','approve'], resume fired);
    3. omitted key still escalates post-flip.
  • Reverse verification (pin red before fix): gate ablation on the committed tree — mutation proven on disk before any verdict (esc.enabled === false token count 1→0, git diff --stat 2 deletions; the test consumes the mutated file via a same-package relative src import, so no build leg applies), run: exactly pin 1 red, 291 passed (predicted direction: escalation fires), restore proven by blob equality (git hash-object = HEAD blob dcb137ef…) + empty git diff HEAD. Script carried trap restore EXIT INT TERM with absolute paths.
  • check:generated: all 14 artifacts up to date (two passes). check:authorable-surface green with the declared default change; check:adr-0087-registration green on the changeset marker; check:changeset-no-major green.
  • Gate union via node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (no paths — change set derived by the script from merge-base d29e42f8, 12 paths, stderr header verified naming this repo at commit 64cff1b8): 49 derived commands + check:nul-bytes, all run at 64cff1b8. 47 passed outright; three were prerequisite refusals, not red measurements, each read from the gate's own printed verdict:
    • check:skill-examples refused on an unbuilt client-react/dist — prerequisite built, gate re-run: ✅ 260 prose examples type-check across 3 surface(s) (VERDICT command-exit 0).
    • check-dev-prereqs.mjs printed The workspace is not built — 1 unmet precondition, not a list of problems (32/67 packages have no local dist; this worktree deliberately builds only affected closures). The gate compares package.json entry declarations against dist presence; this diff touches no package.json, so the reading is about local build state, not this change — CI runs it on a fully built tree. Declared narrowing.
    • check-half-states.mjs exited 3 with its own PREREQUISITE NOT MET — the token in the environment is not a valid GitHub credential text (container proxy placeholder token; the gate's text: "It is not a clean board and it is not a dirty one — it is no reading at all"). Not diff-sensitive; CI carries credentials. Declared narrowing.
  • The default-changes.ts edit is gate-owned surface, so its own suite ran in addition to the derived families: packages/spec/scripts/authorable-defaults.test.ts — 23 tests passed.

Note for the PM (out of this PR's surface)

objectui's flow-node inspector declares the escalation.enabled toggle with defaultValue: 'false' (flow-node-config.ts:732) — post-flip that reads "off" for a block the runtime now treats as on. Same territory as objectui#6499 (no-prune-on-save); flagged in the report for routing rather than edited here.

This PR touches no governed surface (skills/** measured zero-diff), carries the required changeset, and is delivered as a draft for PM review — no ready/enqueue action taken by this seat.

Generated by Claude Code


Generated by Claude Code

…eep enforces it (#12278)
Maintainer ruling 2026-08-27 (Option C): the feature switch is whether an
escalation block exists at all; within a block carrying timeoutHours,
escalation is on unless explicitly turned off.
- spec: ApprovalEscalationSchema.enabled .default(false) -> .default(true),
declared in DEFAULT_CHANGES_BY_MAJOR (17) and registered as the
approval-escalation-enabled-default-flip semantic migration entry;
authorable-defaults, reference docs, spec-changes and upgrade guide
regenerated by repo tooling.
- runtime: runEscalations skips snapshots carrying an explicit
enabled === false; snapshots created before ESCALATION_ENABLED_FLIP_CUTOFF_MS
ride a read-side legacy window (they carry the old default materialized by
the executor parse and are escalating today) - zero tenant writes,
self-retiring as pending requests drain.
- tests: explicit-false-does-not-escalate pin, legacy-window pin,
omitted-key pin; spec pin for the new default materialization.
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvjTCjJQn9zSTXEhUKgT7s
@github-actionsgithub-actionsBot added size/m documentation Improvements or additions to documentation tests tooling labels Aug 27, 2026
@github-actions

github-actionsBot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-approvals, @objectstack/spec, touching 5 documentable anchor(s). ⚠️2 changed file(s) yielded no anchor (packages/spec/authorable-defaults/automation.json, packages/spec/spec-changes.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/automation/flows.mdx(via ApprovalService (symbol))
What this run could not see
  • 2 changed file(s) yielded no anchor (packages/spec/authorable-defaults/automation.json, packages/spec/spec-changes.json) — pages documenting those are invisible to this run
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 126 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 86df0c99e80ad8d280015994cf4b4dbdf0b77342packageMentionDocs.

Which tree this was computed on

This run read content/docs from 0554f4cd70f8f4ef4dd101244f795cd24341a8ea — the merge of head 71ccc576dd90c88117c9a53b0047ed7ea1d8084a into base 86df0c99e80ad8d280015994cf4b4dbdf0b77342, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 0554f4cd70f8f4ef4dd101244f795cd24341a8ea && git checkout 0554f4cd70f8f4ef4dd101244f795cd24341a8ea
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 86df0c99e80ad8d280015994cf4b4dbdf0b77342 71ccc576dd90c88117c9a53b0047ed7ea1d8084a && git checkout -B drift-repro 86df0c99e80ad8d280015994cf4b4dbdf0b77342 && git merge --no-ff 71ccc576dd90c88117c9a53b0047ed7ea1d8084a
node scripts/docs-audit/affected-docs.mjs --json 86df0c99e80ad8d280015994cf4b4dbdf0b77342

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 86df0c99e80ad8d280015994cf4b4dbdf0b77342 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

…ebt ratchet back to 347
The three new escalation pins accessed row properties (.id, indirectly
created_at) on the raw openNodeRequest return type
(ApprovalRequestRow | ApprovalNodeAutoOutcome), adding 8 raw TS2339
errors to the plugin's shrink-only TEST_DEBT layer (347 recorded, 355
measured). Fixed with the file's own sanctioned narrowing pattern
('autoApproved' in req => throw) after each open — no ledger change, no
any. Raw count re-measured at exactly 347; composition returns to the
ledger note to the unit (TS2339 x296).
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JvjTCjJQn9zSTXEhUKgT7s
@os-sales
os-sales marked this pull request as ready for review August 28, 2026 04:32
@os-sales
os-sales enabled auto-merge August 28, 2026 04:32
@os-sales
os-sales added this pull request to the merge queueAug 28, 2026
Merged via the queue into main with commit 277948fAug 28, 2026
35 checks passed
@os-sales
os-sales deleted the claude/issue-12278-enforce-escalation-enabled branch August 28, 2026 05:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

approvals: escalation.enabled has zero readers — the SLA sweep gates only on timeoutHours, so enabled: false still escalates

2 participants

@os-sales@claude