buildiq breaks object creation instance-wide: AutomationApprovalTriggerListener depends on ApprovalChainMapper, deleted by openregister #3302

[rubenvdlinde]

AutomationApprovalTriggerListener takes OCA\OpenRegister\Db\ApprovalChainMapper as a constructor dependency:

// lib/Listener/AutomationApprovalTriggerListener.php:105
private readonly ApprovalChainMapper $chainMapper,

OpenRegister deleted that class in #3302 (feat(approval): consolidate the approval engine onto ordered task sequences), merged 2026-09-01 22:27. AutomationCompilerService imports it too.

Why this is worse than a broken feature

The listener is registered globally, on ObjectCreatedEvent / ObjectUpdatedEvent with no register or schema filter (lib/AppInfo/Application.php:445-455, and the comment there explains the filter is deliberately absent so it fires for automations authored later).

So on any instance with buildiq enabled, Nextcloud instantiates this listener on every object write in every app, DI cannot resolve the missing class, and the write fails. Measured on the shared dev instance just now, creating an opencatalogi object:

403 {"error":"Could not resolve OCA\\OpenRegister\\Db\\ApprovalChainMapper!
     Class \"OCA\\OpenRegister\\Db\\ApprovalChainMapper\" does not exist"}

That is object creation broken instance-wide, not just buildiq's automations.

Why CI has not caught it

buildiq's last Code Quality run on development was 19:05, three hours before OR #3302 merged at 22:27. The green is real but stale — it was measured against an OpenRegister that still had the class. The next run should go red.

What the replacement looks like

OR #3302 ships a machine-readable contract at tests/fixtures/approval-consolidation/retired-approval-surface.json, which lists ApprovalChainMapper under retiredClasses and says explicitly:

an app calling one of these routes or registering a listener for one of these classes is a BROKEN INTEGRATION finding, not a style finding

Also retired: ApprovalChain, ApprovalStep, ApprovalStepMapper, ApprovalService, ApprovalController, the /api/approval-chains and /api/approval-steps routes, and the four ApprovalStep*Event classes. The replacement is a task template plus an opened sequence (TaskSequenceService), with MigrateApprovalChainsToTasks as the data-side repair.

The companion hitl-retirement-inventory.json maps every retired property to its new home, but it is scoped to openconnector/integriq's approval_request schema. buildiq does not appear in it, so this looks like an untracked casualty of the consolidation rather than a planned migration.

Not fixed here

Migrating AutomationCompilerService and AutomationApprovalTriggerListener onto the task-sequence model is real work against a 141-file, 7k-line change, and it needs the design context from whoever owns flow-approval-consolidation. Filing rather than guessing at the mapping.

Two things worth deciding quickly, given the blast radius: whether buildiq should be disabled on shared instances until it is migrated, and whether the anti-pattern gate that reads retired-approval-surface.json runs fleet-wide yet — it would have named this immediately.

[github-actions]

Thank you for opening this issue! It has been assigned to @rubenvdlinde for review.

What happens next:

1. A maintainer will review this issue and determine the appropriate action
2. If accepted, it may be converted into an OpenSpec change for structured development
3. You will be notified of any status changes

Triage labels:

• triage:accept-feature — accepted as a new feature
• triage:accept-bugfix — accepted as a bug fix
• triage:needs-info — more information needed
• triage:wontfix — will not be addressed
• triage:duplicate — duplicate of another issue

[rubenvdlinde]

PR #653 fixes the crash half: AutomationApprovalTriggerListener now compiles a task template from the schema annotation and opens a task sequence, instead of constructor-injecting ApprovalChainMapper, ApprovalStepMapper and ApprovalService.

The collaborators are resolved lazily from the container behind class_exists(), matching the rule this file already documented for ObjectServiceInterface. That is deliberate: a hard constructor type on a cross-app class is what turned a deletion in OpenRegister into a 403 on every object write in every app. Reached lazily, the next retirement is a logged warning instead.

Still open on this issue, and out of #653 on purpose:

1. AutomationCompilerService writes chain rows via createFromArray / updateFromArray (lines 1519-1531) and resolves them with findBySchemaAndName (584, 1579). Under the new model the chain is a schema declaration, so compiling an automation should write x-openregister-approval-chains onto the schema rather than upsert a row. It also still imports ApprovalController.
2. ApprovalOutcomeListener + Application.php subscribe to ApprovalStepApprovedEvent / ApprovalStepRejectedEvent, both retired. This one degrades silently rather than fataling — registerEventListener() takes a class-name string, so a missing event class simply never fires. The replacements are TaskTerminalEvent, TaskTransitionedEvent and TaskSequenceCompletedEvent.
3. The frontend calls the retired routes directly: src/runtimeRegistry.js and src/components/runtime/MyApprovalsWidget.vue both hit /api/approval-steps/{id}/approve|reject, and retired-approval-surface.json lists those under retiredRoutes.

Worth noting for whoever picks these up: OpenRegister ships tests/fixtures/approval-consolidation/retired-approval-surface.json as a machine-readable contract, and says a gate reads it. If that gate runs fleet-wide it would have named all of this on the PR that removed the classes, rather than leaving it to be found by a 403 on an unrelated app's e2e.

[rubenvdlinde]

#653 is merged. The instance-wide breakage is gone: object creation works again in every app, and the Newman suite's 165 Could not resolve ApprovalChainMapper errors are zero.

All three surfaces are migrated — the trigger listener, the compiler (chains are a schema declaration now, not a row), and the My approvals widget (/api/flow-tasks + complete).

What is still open under this issue, both narrowed to something actionable rather than left as "migrate the rest":

1. approvalState() returns none for a declared chain. It used to report the newest ApprovalStep status in the dry-run panel. The task sequence that replaces it is queried per ANCHOR OBJECT, and OpenRegister exposes no "newest sequence across every anchor for this template" query — that is the whole gap. Its test is inverted, not deleted (testApprovalStateDegradesToNoneForADeclaredChain), with the reason in the body, so restoring it is a small readable change once such a query exists.

2. ApprovalOutcomeListener's onReject follow-up cannot fire. Not a buildiq bug: TaskSequenceService::advance() dispatches TaskSequenceCompletedEvent on the approve path, and close() — which handles STATUS_REJECTED and STATUS_TERMINATED — dispatches nothing at all. Raised as No event is dispatched when a task sequence is rejected or terminated, only when it completes openregister#3331. Until that lands there is no signal to subscribe to. The existing subscription to the retired ApprovalStep*Event classes is a silent no-op (registerEventListener takes a class-name string), so it degrades rather than breaks, but it is dead wiring and should come out when the replacement event exists.

One process note worth more than either. buildiq's CI was green three hours after OpenRegister deleted the classes, and it was green partly because tests/stubs/openregister-stubs.php still declares ApprovalChain, ApprovalChainMapper, ApprovalStepMapper and ApprovalService. The suite was passing against stubs of classes that no longer exist, which is why it never noticed the app could not construct its own listener. Those stubs are still there for the reject-half wiring above; when that lands they should go, and it is worth asking whether a stub set that outlives its upstream can fail loudly instead.

OpenRegister ships tests/fixtures/approval-consolidation/retired-approval-surface.json as a machine-readable contract naming every retired class and route, and says a gate reads it. If that gate ran fleet-wide it would have named all of this on the PR that removed the classes, instead of it surfacing as a 403 in an unrelated app's e2e.

[rubenvdlinde]

#655 is merged: onReject fires again, wired to TaskTerminalEvent filtered on a rejecting outcome.

One item left on this issue, and it is genuinely blocked rather than deferred:

approvalState() still reports none for a declared chain. It used to read the newest ApprovalStep's status for the dry-run panel. The task sequence that replaces it is queried per ANCHOR OBJECT (findForAnchor / findNewestForAnchor), and OpenRegister exposes no "newest sequence across every anchor for this template" query — which is exactly what this aggregate needs. Reconstructing it app-side would mean an unindexed scan or a mirror of task state, and removing that mirror is the point of the consolidation.

Its test is inverted rather than deleted (testApprovalStateDegradesToNoneForADeclaredChain) with the trade written in the body, so restoring it is a small readable change once such a query exists.

Worth raising separately. Three times on this branch a change passed locally and failed in CI on a second, hand-maintained description of OpenRegister's surface: tests/stubs/openregister-stubs.php and psalm.xml's cross-app allowlist. Between them they declared classes OpenRegister had deleted, omitted an interface one of their own stubs implemented, and lacked a method a caller needs. Nothing checks either against the installed app.

That is the same mechanism that let #651 happen at all: buildiq's CI was green for three hours after the classes were deleted, partly because the stubs still described them. A gate diffing the stub and allowlist surface against the real OpenRegister would have named all of it at once, instead of it surfacing one CI round at a time — and would have caught the original breakage before an unrelated app's e2e did.