Skip to content

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

Description

@hotlong

Filed by an os-dev agent working an unrelated card in www.objectos.ai (#63, an article about pause/resume and approval SLAs). Recording it rather than fixing it — out of scope for that card. No assignee.

What was measured

On origin/main at 107bb4b:

ApprovalEscalationSchema declares enabled with a false default:

packages/spec/src/automation/approval.zod.ts:616
  enabled: z.boolean().default(false).describe('Enable SLA-based escalation for this node'),

The sweep that actually escalates never reads it. It gates on timeoutHours alone:

packages/plugins/plugin-approvals/src/approval-service.ts:3472-3473
  const esc = cfg?.escalation;
  if (!esc || typeof esc.timeoutHours !== 'number' || esc.timeoutHours <= 0) continue;

escalateRequest reads esc.action, esc.escalateTo and esc.notifySubmitter. It does not read esc.enabled either.

Grep for a reader anywhere in packages/ and apps/:

  • escalation.enabled / esc.enabled / escalation?.enabledzero hits outside the declaration itself.
  • The only other occurrences are three fixture lines in packages/lint/src/validate-approval-approvers.test.ts that write enabled: true.

Why it matters

This is the ADR-0049 declared-but-unenforced shape, and it fails open, which is the worse direction. An author who writes

escalation: { enabled: false, timeoutHours: 24, action: 'auto_approve' }

has stated, in the vocabulary the schema gave them, that this SLA is off. The sweep escalates anyway — and with action: 'auto_approve' that silently approves a request the author believed was not on a clock. The declared default (false) makes it worse: an author who sets timeoutHours and nothing else gets escalation they never turned on, while the schema tells them the feature defaults to off.

Note this is the mirror image of the waitEventConfig.timeoutMs / onTimeout retirement (#4158): those were removed because they claimed a behaviour nothing delivered. This one delivers a behaviour the key claims to gate.

Routes (for triage, not a recommendation to skip the ruling)

  1. Enforce — add if (esc.enabled === false) continue; to runEscalations. Cheapest, but it changes behaviour for any stored flow that set enabled: false alongside a real timeoutHours and has been escalating; that population should be measured before the flip.
  2. Remove — retire enabled and let the presence of timeoutHours be the switch, which is what the runtime already means. One fewer key, and the remaining one cannot disagree with itself.

Either way the ruling is a maintainer's, not this agent's. The liveness question ("who authors enabled, and what did they expect?") wants the usual three-repo read before the route is picked.

Not addressed here

#63 in www.objectos.ai is a content card and does not touch this code. The article deliberately does not document the defect — it shows enabled: true (the correct-as-declared spelling) and makes no claim about what happens when it is false, so it stays accurate under either route above.


Generated by Claude Code

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions