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 writeenabled: 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

No one assigned

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions