Skip to content

fix(approvals): admin override to recover an approval routed to an unstaffed position (#3424)#3451

Merged
os-zhuang merged 3 commits into
mainfrom
claude/approval-empty-position-lock-kj8gov
Jul 24, 2026
Merged

fix(approvals): admin override to recover an approval routed to an unstaffed position (#3424)#3451
os-zhuang merged 3 commits into
mainfrom
claude/approval-empty-position-lock-kj8gov

Conversation

@os-zhuang

Copy link
Copy Markdown
Contributor

Summary

Fixes #3424 — an approval node routed to a position (or team/department) with no holders created a permanently undecidable request and locked the target record forever, with no in-product recovery.

Root cause. When an approver spec (e.g. { type: 'position', value: 'sales_manager' }) resolves to zero holders, expandApprovers falls back to the unresolvable literal position:sales_manager in pending_approvers — no concrete user is in the slate. Every normal decision then fails the "is a pending approver" / "is the submitter" checks with FORBIDDEN, and (with lockRecord) the beforeUpdate lock hook keeps the record RECORD_LOCKED for as long as the request is pending. The only exit was editing the DB by hand. Trivial to hit in fresh/demo orgs (positions seeded, holders not) and whenever a role is vacated in production.

The literal fallback is intentional (15.x stored slots stay queryable — pinned by existing tests), so this does not change request creation. Instead it adds the missing admin escape hatch.

What changed

Privileged override (packages/plugins/plugin-approvals/src/approval-service.ts)

  • New isOverrideActor(context, requestOrg) — true for a platform admin (admin_full_access / platform_admin / posture: PLATFORM_ADMIN) or a tenant admin (organization_admin / org_owner / org_admin / posture: TENANT_ADMIN), reading the same resolved-authz signals the engine's superuser bypass already trusts. A platform admin crosses tenants; a tenant admin is scoped to their own org; system contexts always pass.
  • decideNode, reassign, and recall honour an override actor even when they hold no slot / aren't the submitter — so an admin can approve, reject, reassign to a real approver, or recall. The override finalizes the request, which releases the record lock (keyed on a pending request). The decision is audited under the admin's own id.
  • An admin approval is authoritative — it finalizes the node even under unanimous/quorum/per_group, rather than counting as one vote in the (empty) slate.
  • openNodeRequest now warns loudly when a node resolves to no concrete approver, so the misconfiguration surfaces in logs instead of silently locking the record.

Surfacing (sys-approval-request.object.ts, spec/contracts/approval-service.ts)

  • viewer gains a server-computed can_override (true for a privileged admin on a pending request). The approve / reject / reassign declared actions OR it into their visible gate, so the console shows the recovery path with no hand-wired button. Existing approver/submitter gating is unchanged.

The console side (viewer.can_override type + inbox hint) is in the companion objectui PR.

Why not the other options in the issue

  • Empty-approver detection at creation would break the intentional literal-fallback behavior (pinned by position approver: falls back to a position: literal when nobody holds it and siblings), and can't be caught at authoring (a staffed position can be vacated later). A runtime warning is added instead.
  • The admin override is the general recovery mechanism: it also covers the production "role vacated after the request opened" case, and releasing the lock via finalize makes the orphaned-lock scenario moot.

Testing

pnpm turbo run test --filter=@objectstack/plugin-approvals156 passed. New coverage in approval-service.test.ts (admin approve/reject/reassign/recall on a stuck unstaffed-position request, org-scoping, unanimous finalization, viewer.can_override) and sys-approval-request.object.test.ts (the override gate on the declared actions). Full tsup DTS typecheck of the plugin passes.

A patch changeset is included.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VmQPXXbgomoqrXtoxr3CS2


Generated by Claude Code

…staffed position (#3424)

An `approval` node routed to a position/team with no holders resolved to only
the unresolvable `position:<name>` literal in `pending_approvers`, so no
concrete user was in the slate. Every normal `decide` / `reassign` / `recall`
then returned FORBIDDEN and, with `lockRecord`, the target record stayed
RECORD_LOCKED forever — a data-availability dead-end with no in-product recovery.
Trivial to hit in fresh/demo orgs (positions seeded, holders not) and whenever a
role is vacated.

Let a platform or tenant admin act on any pending request to release it: approve,
reject, reassign it to a real approver, or recall it. The override finalizes the
request (which releases the record lock, keyed on a pending request); a tenant
admin's authority is org-scoped, a platform admin's is not, and the action is
audited under the admin's own id. An admin approval is authoritative — it
finalizes the node even under unanimous/quorum/per_group rather than counting as
one vote among the (empty) slate.

- Add server-computed `sys_approval_request.viewer.can_override`; the
  approve/reject/reassign declared actions OR it into their `visible` gate so the
  console surfaces the recovery path with no hand-wired button.
- Warn loudly when a node resolves to no concrete approver, so the
  misconfiguration is visible instead of silently locking the record. The
  literal-fallback behavior (15.x slot back-compat) is otherwise unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmQPXXbgomoqrXtoxr3CS2
@vercel

vercel Bot commented Jul 24, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
spec Ready Ready Preview, Comment Jul 24, 2026 4:07pm

Request Review

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling size/m labels Jul 24, 2026
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/lint, @objectstack/plugin-approvals, @objectstack/spec.

104 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/ai/agents.mdx (via @objectstack/spec)
  • content/docs/ai/skills-reference.mdx (via @objectstack/spec)
  • content/docs/ai/skills.mdx (via @objectstack/spec)
  • content/docs/api/client-sdk.mdx (via @objectstack/spec)
  • content/docs/api/environment-routing.mdx (via @objectstack/spec)
  • content/docs/api/error-catalog.mdx (via @objectstack/spec)
  • content/docs/api/error-handling-client.mdx (via @objectstack/spec)
  • content/docs/api/error-handling-server.mdx (via @objectstack/spec)
  • content/docs/api/index.mdx (via @objectstack/spec)
  • content/docs/automation/approvals.mdx (via @objectstack/plugin-approvals, packages/spec)
  • content/docs/automation/flows.mdx (via @objectstack/spec)
  • content/docs/automation/hook-bodies.mdx (via packages/spec)
  • content/docs/automation/hooks.mdx (via @objectstack/spec)
  • content/docs/automation/index.mdx (via @objectstack/spec)
  • content/docs/automation/webhooks.mdx (via @objectstack/spec)
  • content/docs/automation/workflows.mdx (via @objectstack/spec)
  • content/docs/concepts/architecture.mdx (via @objectstack/spec)
  • content/docs/concepts/design-principles.mdx (via packages/spec)
  • content/docs/concepts/index.mdx (via @objectstack/spec)
  • content/docs/concepts/metadata-driven.mdx (via @objectstack/spec)
  • content/docs/concepts/metadata-lifecycle.mdx (via packages/spec)
  • content/docs/concepts/north-star.mdx (via packages/spec)
  • content/docs/data-modeling/analytics.mdx (via @objectstack/spec)
  • content/docs/data-modeling/drivers.mdx (via @objectstack/spec)
  • content/docs/data-modeling/external-datasources.mdx (via @objectstack/spec)
  • content/docs/data-modeling/field-types.mdx (via @objectstack/spec)
  • content/docs/data-modeling/fields.mdx (via @objectstack/spec)
  • content/docs/data-modeling/formulas.mdx (via @objectstack/spec)
  • content/docs/data-modeling/index.mdx (via @objectstack/spec)
  • content/docs/data-modeling/objects.mdx (via @objectstack/spec)
  • content/docs/data-modeling/queries.mdx (via @objectstack/spec)
  • content/docs/data-modeling/schema-design.mdx (via @objectstack/spec)
  • content/docs/data-modeling/seed-data.mdx (via @objectstack/spec)
  • content/docs/data-modeling/validation-rules.mdx (via @objectstack/spec)
  • content/docs/data-modeling/validation.mdx (via @objectstack/spec)
  • content/docs/deployment/cli.mdx (via @objectstack/spec)
  • content/docs/deployment/troubleshooting.mdx (via @objectstack/spec)
  • content/docs/deployment/validating-metadata.mdx (via @objectstack/spec)
  • content/docs/getting-started/build-with-claude-code.mdx (via @objectstack/spec)
  • content/docs/getting-started/common-patterns.mdx (via @objectstack/spec)
  • content/docs/getting-started/examples.mdx (via @objectstack/spec)
  • content/docs/getting-started/quick-reference.mdx (via @objectstack/spec)
  • content/docs/getting-started/quick-start.mdx (via @objectstack/spec)
  • content/docs/getting-started/your-first-project.mdx (via @objectstack/spec)
  • content/docs/kernel/cluster.mdx (via @objectstack/spec)
  • content/docs/kernel/contracts/auth-service.mdx (via packages/spec)
  • content/docs/kernel/contracts/cache-service.mdx (via packages/spec)
  • content/docs/kernel/contracts/data-engine.mdx (via @objectstack/spec)
  • content/docs/kernel/contracts/index.mdx (via @objectstack/spec)
  • content/docs/kernel/contracts/metadata-service.mdx (via packages/spec)
  • content/docs/kernel/contracts/storage-service.mdx (via packages/spec)
  • content/docs/kernel/index.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/email-service.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/index.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/queue-service.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/sharing-service.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/sms-service.mdx (via packages/spec)
  • content/docs/kernel/runtime-services/storage-service.mdx (via packages/spec)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/spec)
  • content/docs/permissions/authorization.mdx (via @objectstack/lint, @objectstack/spec)
  • content/docs/permissions/permission-sets.mdx (via @objectstack/spec)
  • content/docs/permissions/permissions-matrix.mdx (via @objectstack/spec)
  • content/docs/permissions/positions.mdx (via @objectstack/spec)
  • content/docs/permissions/rls.mdx (via @objectstack/spec)
  • content/docs/permissions/sharing-rules.mdx (via @objectstack/spec)
  • content/docs/plugins/adding-a-metadata-type.mdx (via @objectstack/spec)
  • content/docs/plugins/development.mdx (via @objectstack/spec)
  • content/docs/plugins/index.mdx (via @objectstack/spec)
  • content/docs/plugins/packages.mdx (via @objectstack/plugin-approvals, @objectstack/spec)
  • content/docs/protocol/backward-compatibility.mdx (via @objectstack/spec)
  • content/docs/protocol/diagram.mdx (via packages/spec)
  • content/docs/protocol/kernel/config-resolution.mdx (via @objectstack/spec)
  • content/docs/protocol/kernel/i18n-standard.mdx (via @objectstack/spec)
  • content/docs/protocol/kernel/index.mdx (via @objectstack/spec)
  • content/docs/protocol/kernel/lifecycle.mdx (via @objectstack/spec)
  • content/docs/protocol/kernel/plugin-spec.mdx (via @objectstack/spec)
  • content/docs/protocol/kernel/runtime-capabilities.mdx (via @objectstack/spec)
  • content/docs/protocol/knowledge.mdx (via @objectstack/spec)
  • content/docs/protocol/objectql/index.mdx (via @objectstack/spec)
  • content/docs/protocol/objectql/query-syntax.mdx (via @objectstack/spec)
  • content/docs/protocol/objectql/schema.mdx (via @objectstack/spec)
  • content/docs/protocol/objectql/security.mdx (via packages/spec)
  • content/docs/protocol/objectql/state-machine.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/actions.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/concept.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/index.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/layout-dsl.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/record-alert.mdx (via @objectstack/spec)
  • content/docs/protocol/objectui/widget-contract.mdx (via @objectstack/spec)
  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-approvals, @objectstack/spec)
  • content/docs/releases/index.mdx (via @objectstack/spec)
  • content/docs/releases/v12.mdx (via @objectstack/spec)
  • content/docs/releases/v13.mdx (via @objectstack/spec)
  • content/docs/releases/v16.mdx (via @objectstack/spec)
  • content/docs/releases/v9.mdx (via @objectstack/plugin-approvals, @objectstack/spec)
  • content/docs/ui/actions.mdx (via @objectstack/spec)
  • content/docs/ui/create-vs-edit-form.mdx (via @objectstack/spec)
  • content/docs/ui/dashboards.mdx (via @objectstack/spec)
  • content/docs/ui/forms.mdx (via @objectstack/spec)
  • content/docs/ui/index.mdx (via @objectstack/spec)
  • content/docs/ui/public-data-collection.mdx (via @objectstack/spec)
  • content/docs/ui/setup-app.mdx (via @objectstack/spec)
  • content/docs/ui/translations.mdx (via @objectstack/spec)
  • content/docs/ui/views.mdx (via @objectstack/spec)

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

…3424)

Note the platform/tenant admin recovery path for a request routed to an
unstaffed position (approve / reject / reassign / recall), and add
`can_override` to the per-viewer block description.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmQPXXbgomoqrXtoxr3CS2
@os-zhuang
os-zhuang marked this pull request as ready for review July 24, 2026 15:08
Two authoring-time / demo-time complements to the admin-override recovery:

- lint: new advisory rule `approval-approvers-may-resolve-empty` (info) fires
  when EVERY approver on an approval node routes to a group that can be empty
  (position/team/department) — the exact shape that dead-ends when nobody holds
  the group. It prescribes a guaranteed-staffed fallback
  (`{ type: 'org_membership_level', value: 'owner' }`). Advisory, since staffing
  is runtime data a linter can't see; individual/owner-tier fallbacks suppress
  it. Non-gating (info → suggestion).
- showcase: the seed staffed `manager`/`finance`/`legal` but not `exec`, the
  second tier of `showcase_budget_approval` (manager → exec) — a user driving a
  budget approval to step 2 hit the exact #3424 dead-end. Staff `exec` too so
  every position the showcase flows route to is actionable.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmQPXXbgomoqrXtoxr3CS2
@os-zhuang
os-zhuang merged commit be1c52c into main Jul 24, 2026
16 of 17 checks passed
@os-zhuang
os-zhuang deleted the claude/approval-empty-position-lock-kj8gov branch July 24, 2026 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Approval routed to an empty position permanently locks the record (no admin override, no recovery)

2 participants