Skip to content

fix(scope): a pending schema ref belongs to its own chain, not to the next caller - #2804

Merged
rubenvdlinde merged 2 commits into
developmentfrom
fix/pending-schema-ref-belongs-to-its-chain
Aug 22, 2026
Merged

fix(scope): a pending schema ref belongs to its own chain, not to the next caller#2804
rubenvdlinde merged 2 commits into
developmentfrom
fix/pending-schema-ref-belongs-to-its-chain

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Fixes the deterministic red e2e on integriq development, and closes the third pass at one mechanism. Written up in full as #2801.

Single-use bounded the damage; it did not remove it

#2792 made the pending schema ref single-use. Its own comment names the failure mode exactly:

"a ref left behind by an operation that set a schema and never set a register would be re-resolved against the NEXT caller's register and refuse it."

Consuming it once means the leak claims one victim instead of many. There is still a victim, and it is whoever calls setRegister() next.

Measured, not inferred

On development today that victim was a flow's object-write, which names its own register and schema and had never heard of the leaked slug:

Schema slug "synchronization_contract" is not carried by register
"fns-mt4du8ca-62fj-reg" (id 15), which carries 2 schema(s)

fns-…-reg is a register the e2e creates for itself. The step failed and took the pipeline with it — synced-id, commit, sweep and end never ran.

Bisected to #2774 by job timestamps: the same spec passed 07:48–08:07 UTC and failed 08:46–09:05 UTC, and #2774 is the only behavioural change in that window. (Not #2790 — that merged at 10:57, two hours after the first failure.)

The rule

A pending ref belongs to the fluent chain that created it. An operation that supplies its own scope through its arguments is by definition a new chain, so it starts by discarding whatever was left behind.

find() already did this (#2790). discardPendingSchemaRef() makes it the rule at every entry point that consumes context: findSilent, findAll, count, saveObject, saveObjects, deleteObject, deleteObjects.

What is deliberately NOT changed

  • Genuine chains. setSchema($s)->setRegister($r) still scopes $s inside $r — no operation runs between the two setters, so nothing is discarded.
  • The register boundary. A caller naming a register still cannot be served a schema from outside it; the scoping that stopped TimeEntry resolving into the wrong app is untouched. What changes is only whose ref the boundary judges.

Verification

  • the test reproduces the CI message exactly, and fails with the guard disabled — verified in both directions rather than assumed
  • tests/Unit/Service/: 10675 tests / 7 errors against a development baseline of 10674 / 7 — one test added, no new failures
  • phpcs, phpmd (with baseline), psalm: clean

Worth one person's attention

This is the third pass at the same mechanism — #2790, #2792, and this. Each was correct about what it saw; none of the three had the whole picture. #2801 also records two further symptoms from the same scoping work that are not fixed here, including GET /api/objects/{register}/… answering Register not found by slug and id while GET /api/registers/65 resolves the same register fine.

… next caller

SINGLE-USE BOUNDED THE DAMAGE; IT DID NOT REMOVE IT. #2792 made the pending
schema ref single-use, and its comment names the exact failure mode: "a ref
left behind by an operation that set a schema and never set a register would be
re-resolved against the NEXT caller's register and refuse it." Consuming it once
means the leak claims ONE victim instead of many. There is still a victim, and
it is whoever calls setRegister() next.

MEASURED, NOT INFERRED. On development today that victim was a flow's
`object-write`, which names its OWN register and schema and had never heard of
the leaked slug:

  Schema slug "synchronization_contract" is not carried by register
  "fns-mt4du8ca-62fj-reg" (id 15), which carries 2 schema(s)

`fns-…-reg` is a register the e2e creates for itself. The step failed, and with
it the whole decomposed pipeline — `synced-id`, `commit`, `sweep` and `end`
never ran. Bisected to #2774 by job timestamps: the same spec passed 07:48-08:07
UTC and failed 08:46-09:05 UTC, and #2774 is the only behavioural change in that
window.

THE RULE. A pending ref belongs to the fluent chain that created it. An
operation that supplies its own scope through its ARGUMENTS is by definition a
new chain, so it starts by discarding whatever was left behind. `find()` already
did this (#2790); `discardPendingSchemaRef()` makes it the rule at every entry
point that consumes context — findSilent, findAll, count, saveObject,
saveObjects, deleteObject, deleteObjects.

Genuine chains are untouched: `setSchema($s)->setRegister($r)` still scopes $s
inside $r, because no operation runs between the two setters. The register
boundary itself is unchanged — a caller naming a register still cannot be served
a schema from outside it. What changes is only WHOSE ref the boundary judges.

The test reproduces the CI message exactly and FAILS with the guard disabled —
verified both directions. tests/Unit/Service sits at the development baseline:
10675 tests / 7 pre-existing errors against 10674 / 7, one test added.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ a919f60

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

Quality workflow — 2026-08-22 16:05 UTC

Download the full PDF report from the workflow artifacts.

#2803 landed mid-flight — a FOURTH pass at this one mechanism, found from the
other end (a docudesk template create failing with openbuild's `application`
slug). It guards `setContextFromParameters()`, the path a caller naming BOTH
register and schema takes. This branch guards the seven public entry points.
The two are complementary, not competing: #2803 covers the "names both" case
wherever it is reached from, this covers every operation that supplies its own
scope at all.

Both test sets are kept. They are different regressions from different apps
against the same root cause, and either would have caught only its own.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ e0b01f3

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

Quality workflow — 2026-08-22 16:33 UTC

Download the full PDF report from the workflow artifacts.

@rubenvdlinde
rubenvdlinde merged commit 6c01a89 into development Aug 22, 2026
48 of 76 checks passed
@rubenvdlinde
rubenvdlinde deleted the fix/pending-schema-ref-belongs-to-its-chain branch August 22, 2026 16:35
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.

1 participant