fix(locking): a parked run keeps its locks, and the lock nodes exist in the editor

[rubenvdlinde]

Three defects in the run-scoped object locking that #3444 merged. All three were found by walking a live instance; none by the suite. The first one means the feature does not work at all.

1. A run lost every lock the moment it parked

The PR's claim was right as far as it went: the dispatch predicate really is FlowRunMapper::update() asking the persisted row isTerminal(), and there is no status whitelist. The row really did say completed — transiently, in the middle of a pass that went on to store suspended.

FlowEngine::fireOnStream() computes enabledAfter by asking FlowStreamWalk::workRemains() for the transitions enabled on the marking it has just advanced. That method answers from the walk's in-memory stream picture, and commitFiring() only re-reads that picture after the commit. So it compared the NEW enabled transitions against the OLD places, they never intersect on a linear flow, and it answered "no work remains" at every ordinary mid-flow firing. FlowRunCommit::applyDerivedStatus() then took its "nothing enabled, nothing parked, nothing terminal" arm and wrote completed; the mapper announced terminality; FlowRunLockReleaseListener released the run's locks. Every firing, of every run — the locks are simply the first thing that noticed.

The park path had the mirror of the same bug: workRemains() is evaluated as an argument to park(), before park() marks the stream parked, so its token still enabled the transition it was waiting on and the run was derived queued with no wake time until finalize() corrected it. Harmless while the pass survives; a run left queued with no resumeAt if it does not.

workRemains() now takes what the caller's commit is about to change: produced (the places the firing takes) and settling (a stream whose place stops counting because it is parking). Over-counting is the safe direction — it yields queued, which the next pass corrects; under-counting is what produced a false terminal.

2. A run-kind lock refused the run that held it

SaveObject asked the guard "which user", never "which run", so a flow that locked a case was turned away by its own lock at its next write. The run identity now reaches the guard through the ambient FlowRunContext, for the same reason attribution does: the write is routinely several calls deep inside code that has never heard of flows. Absent, it reads as a person — the fail-closed answer a run lock already gives.

The D-6 sweep, re-done over the guard's callers rather than lockObject()'s:

site	decides	run identity
SaveObject::findAndValidateExistingObject	refusal	now passed — this was the defect
RevertHandler::revert	refusal	now passed
ObjectsController::update (L2859)	refusal	now passed
ObjectsController post-save auto-unlock ×3	release	deliberately NOT passed, and the code says why: a run's lock must outlive every write the run makes
LockHandler::callerMayUnlock	authorization	already parameterised
LockObjectNode / UnlockObjectNode	take / release	already passed theirs
MagicMapper::lockObject/unlockObject	mapper-level	no guard; no callers in lib/

The same predicate had the opposite hole. A user lock did not refuse a run executing as its holder, so a run passing over a person's locked object did not merely pass the guard: ObjectEntity::lock() took the extend branch, rewrote the payload as its own run lock, and destroyed the person's lock when the run ended. A run and the person it runs as are different holders in both directions.

3. Both new nodes were invisible in the flow editor

core/img/actions/lock.svg and unlock.svg exist in neither NC 33 nor NC 34, IURLGenerator::imagePath() throws for an image the server does not ship, and palette() caught that along with everything else. The catalogue held 25 nodes instead of 27 and neither node could be added to a flow at all.

The icons are now app-owned (img/lock.svg, img/unlock.svg), which cannot go missing under us. And the silent skip is loud: the icon is resolved on its own, an unresolvable one is an error naming the node and the icon, and the node is served with the app icon rather than deleted from the catalogue. A node the author can see and place with the wrong picture beats a node that does not exist. The remaining outer catch — a node whose display name or description throws — now logs at error too, saying the node was dropped and cannot be added to a flow.

Tests

Each proven red first, and each calling the production predicate rather than restating it. The existing coverage could not catch any of this: FlowRunCommitTest mocks FlowRunMapper::update(), so the dispatch never happens, and FlowEngineStreamWalkTest's fake restates workRemains(), so it agreed with the bug.

• RunLockReleaseTerminalityTest drives the real engine, the real FlowStreamWalk, the real FlowRunCommit, a real FlowRunMapper and the real listener, with only the database replaced. A suspended run announces nothing and keeps its locks; each status in FlowRun::TERMINAL releases them, iterated from the constant so a fifth terminal status fails the test rather than going untested. Red before the fix with announced = ['completed','completed'] — once per firing.
• SaveObjectTest: the holding run writes to its own locked object; a different run and a person are both refused; a run is refused by a person's lock even when it runs as that person.
• ObjectEntityRunLockTest: a person's lock survives a run passing over the object, payload byte-identical, and the lock is still a user lock afterwards.
• FlowNodePaletteIconsTest sweeps every registered node (27 of them, not these two) and asserts its icon names an image that exists — against the real server tree when the tests can see one, which they always can in CI, and against a recorded inventory otherwise, so a standalone clone gets a verdict rather than a skip. Its second half asserts the behaviour that stops this recurring: a node whose icon throws stays in the palette and is reported.

Verified locally, by exit code

phpunit (19,201 tests), phpcs, psalm, phpstan, phpmd on the changed files (both rulesets), and hydra-gates on the whole tree with HYDRA_GATE_BASE_REF=origin/development. CI is bottlenecked, so this is local verification, not a CI green.

Verified on a rig, not only in tests

Own compose project, NC 34.0.3 + PostgreSQL 16 on a free port (never :8080), the working tree installed as the app. Fresh install, so all migrations ran and oc_openregister_run_object_locks is present — the skipped-migration trap did not apply. Each leg run twice, once with the fix and once with the pre-fix file swapped back in:

leg	pre-fix	fixed
flow locks a case, then parks	_locked null, log: Released 1 object lock(s) of run 8fc8b735-… (completed) for a run stored suspended	run suspended, lock held by the run, one registry row
that run reaches a terminal outcome	—	lock released, registry row gone
node catalogue	25 nodes, both lock nodes absent	27, both present with resolving icons
holding run writes to its own locked object	—	write succeeds; another run and a person both refused, naming the run
person's lock, run passes over it	lock destroyed (null) when the run ended	run parks, payload byte-identical

The palette leg is the server-side catalogue the editor reads (GET /api/flow/node-catalog), which is where the node was being dropped; the JS bundle was not built on the rig. Rig torn down with docker compose down -v.

🤖 Generated with Claude Code

[github-actions]

Quality Report — ConductionNL/openregister @ ef168cf

Check	PHP	Vue	Security	License	Tests
lint	❌
phpcs	✅
phpmd	❌
psalm	❌
phpstan	✅
phpmetrics	❌
eslint		✅
stylelint		✅
build		❌
check-specs		✅
test-l10n		✅
test-l10n-parity		✅
format		✅
check-schema-l10n		✅
check-l10n-js		✅
composer			✅	✅ 174/174
npm			✅	✅ 543/543
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					❌
Newman					❌
Playwright					❌
Hydra gates					❌

---

Quality workflow — 2026-09-05 15:17 UTC

Download the full PDF report from the workflow artifacts.

[github-actions]

Quality Report — ConductionNL/openregister @ 17d2b33

Check	PHP	Vue	Security	License	Tests
lint	✅
phpcs	✅
phpmd	✅
psalm	✅
phpstan	✅
phpmetrics	✅
eslint		✅
stylelint		✅
build		✅
check-specs		✅
test-l10n		✅
test-l10n-parity		✅
format		✅
check-schema-l10n		✅
check-l10n-js		✅
composer			✅	✅ 174/174
npm			✅	✅ 543/543
app:check-code	⏭️
info.xml	✅
REUSE	❌
PHPUnit					❌
Newman					✅
Playwright					⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development
Hydra gates					✅

---

Quality workflow — 2026-09-05 15:28 UTC

Download the full PDF report from the workflow artifacts.