Skip to content

feat(approval): add the cross-app command seam for approval routes - #1023

Closed
rubenvdlinde wants to merge 1 commit into
feat/governance-body-eventsfrom
feat/approval-route-events
Closed

feat(approval): add the cross-app command seam for approval routes#1023
rubenvdlinde wants to merge 1 commit into
feat/governance-body-eventsfrom
feat/approval-route-events

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Stacked on #1019. This PR targets feat/governance-body-events, not development, because it needs the RegisterObjectStore rename from that branch. Merge #1019 first; GitHub will retarget this one automatically.

Why

approval-routes shipped ApprovalRoute, ApprovalAction and ApprovalRouteService, and its own proposal named the consumer it was built for: dossiq's parafering, "implemented today in dossiq, where it works", which "does not belong there".

That migration still cannot start. The engine is reachable only through ApprovalRouteController, and REST is the wrong door for an in-process app-to-app command — ADR-041, enforced by gate-27 — besides refusing a request with no signed-in user, which is exactly the state a migration runs in. Same wall #1019 hit for governance bodies.

Measured while scoping this: dossiq's parafeerroute is marked DEPRECATED (migrate-parafering-to-or-approval-workflow), and the archive records that change as "archived prematurely; implementation not present on development". It is live in 15 PHP files, 7 frontend files, 4 routes and 4 test files, across roughly 1,810 lines of engine in six services. The banner describes an intention; a reader would take it for a state.

What

  • ApprovalRouteRequestedEvent — hold this route, and optionally start one subject travelling it in the same command (a migration that moves a template almost always wants the in-flight subjects too)
  • ApprovalActionRequestedEvent — record one actor's action
  • ApprovalRouteConcludedEvent — emitted when an action decides the final stage
  • ApprovalRouteCommandService, two listeners, and sourceApp + externalReference on ApprovalRoute as the idempotency key

🔴 It is not a second engine

That is the whole design constraint. Which stage is active, what a return does to the stages after it, which actors may act, whether a mandatory stage may be skipped: all of it stays in ApprovalRouteService and none of it is restated in the seam.

testSeamAndServiceCannotDiverge advances two identical subjects — one through the seam, one through the service — and asserts their stage rows are identical. A seam that grew its own rules fails rather than drifting, which is what would otherwise show up months later as the REST path and the event path disagreeing about the same sign-off.

Three smaller decisions worth reviewing

The refusal carries the engine's reason. "You are not the named actor" and "there is nothing to act on" need different handling by the producer, and a bare false collapses them.

No producer-supplied step reaches the engine. The engine decides which stage is active; accepting a step number would let an action be filed against a stage nobody is waiting on.

The event registrations moved to a new CrossAppEventRegistrar. What was one event/listener pair is now four, and each pair costs DomainServiceRegistrar two imports: it went from clean on development, to a coupling of 14 after #1019, to 18 with this change. Splitting fixes the measurement and reads better — the whole inbound cross-app surface is now one map in one file.

Verification

  • 1262 unit tests pass (22 skipped, pre-existing)
  • PHPCS / PHPMD / PHPStan / Psalm clean on every changed file
  • All 75 applicable hydra gates green, and all 75 actually ran. Two initially reported FAIL because ajv was not resolvable in my worktree; that is a wiring failure, not a finding, and the gates correctly refused to run fail-open rather than report a weaker check as a pass.
  • Mutation-checked: making the route lookup always miss, never reporting completion, and dropping the empty-steps guard each turn the suite red
  • The register-fragment merge was run through a real deep-merge rather than assumed: ApprovalRoute gains both properties, required stays ["name","steps"], and GovernanceBody keeps its own pair

Impact

Unblocks the dossiq parafering migration. Nothing in this repo behaves differently until a consumer dispatches one of the new events.

🤖 Generated with Claude Code

`approval-routes` shipped the engine and named the consumer it was built for:
dossiq's parafering, "implemented today in dossiq, where it works", which "does
not belong there". That migration still cannot start, for the same reason
governance bodies could not move until the previous commit fixed it — the
engine is reachable only over REST, and REST is the wrong door for an
in-process app-to-app command (ADR-041, gate-27), besides refusing a request
with no session, which is the state a migration runs in.

So this adds the door:

- ApprovalRouteRequestedEvent — hold this route, and optionally start one
  subject travelling it in the same command
- ApprovalActionRequestedEvent — record one actor's action
- ApprovalRouteConcludedEvent — emitted when an action decides the final stage
- ApprovalRouteCommandService, two listeners, and `sourceApp` +
  `externalReference` on ApprovalRoute as the idempotency key

🔴 IT IS NOT A SECOND ENGINE, and that is the design constraint. Which stage is
active, what a return does, which actors may act, whether a mandatory stage may
be skipped: all of it stays in ApprovalRouteService and none of it is restated.
testSeamAndServiceCannotDiverge advances two identical subjects — one through
the seam, one through the service — and asserts their stage rows are identical,
so a seam that grew its own rules fails rather than drifting.

Two smaller decisions worth reading:

The listener carries the engine's REFUSAL REASON back, not a bare false. "You
are not the named actor" and "there is nothing to act on" need different
handling by the producer and a boolean collapses them.

No producer-supplied `step` reaches the engine. The engine decides which stage
is active; accepting a step number would let an action be filed against a stage
nobody is waiting on.

`ApprovalRouteService::stagesFor()` is promoted to public so the seam can answer
"did that finish the route" by asking the engine rather than running a second
query over decision-stage rows.

The event registrations move out of DomainServiceRegistrar into a new
CrossAppEventRegistrar. What was one event/listener pair is now four, and each
pair cost that class two imports: it went from clean on development to a
coupling of 14 after the governance seam and 18 after this one. Splitting fixes
the measurement and reads better — the whole inbound cross-app surface is now
one map in one file.

Verified: 1262 unit tests pass (22 skipped, pre-existing), PHPCS/PHPMD/PHPStan/
Psalm clean on every changed file, the fragment merge run through a real
deep-merge rather than assumed, and ALL 75 applicable hydra gates green with
all 75 actually running. Mutation-checked: making the route lookup always miss,
never reporting completion, and dropping the empty-steps guard each turn the
suite red.

Stacked on #1019 — it needs the RegisterObjectStore rename.
Unblocks dossiq's parafering migration.
@rubenvdlinde

Copy link
Copy Markdown
Contributor Author

Superseded by #1028. Same content, rebased onto #1019 after the CrossAppEventRegistrar extraction moved there — it belongs in the branch where the coupling threshold is first crossed, so a stacked PR is not what makes its parent green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants