Skip to content

Reject relation targets with an unresolvable Source#1043

Draft
malberts wants to merge 2 commits into
source-registry-and-local-sourcefrom
restrict-relation-targets
Draft

Reject relation targets with an unresolvable Source#1043
malberts wants to merge 2 commits into
source-registry-and-local-sourcefrom
restrict-relation-targets

Conversation

@malberts

@malberts malberts commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

For #993 (work order T4). Builds on the T2 Source
contract PR and targets its branch, so the diff shows only this track.

Implements the v1 guard from ADR 23 "Relations across Sources": a Relation target whose source is
not registered gets a blocking relation-target-source-unresolvable violation at write and
validation time; already-persisted data is never rejected on read; cross-source relations open up
later.

Design calls recorded here:

  • The guard is a validation concern, produced by SubjectValidator with the SourceRegistry
    injected from the composition root, so one seam covers the write path and both validate
    endpoints. Placing it in RelationType instead would have pulled the registry into the guard-free
    read/deserialize paths and tripped the hook-ordering constraint ADR 027 documents.

  • Statement-scoped, not schema-scoped: the check runs for every relation-typed statement in the
    payload, including out-of-schema and type-mismatched ones. Schema membership says nothing about
    whether a target's Source resolves, and schema-scoping would let a hand-crafted out-of-schema
    relation statement smuggle an unresolvable target into the slot even with enforcement on
    (probe-confirmed during review, then closed test-first).

  • The guard rides the standard enforcement gate: $wgNeoWikiEnforceValidation (default off)
    decides whether the blocking violation rejects the write, exactly like every other blocking
    constraint. A wiki that disables enforcement is knowingly disabling the whole write-validation
    regime, so this guard gets no special-case bypass of that choice; the docs state the qualifier
    explicitly. Flipping this one violation to unconditional rejection is a small change if review
    prefers ADR 23's stricter reading.

    A post-implementation audit probed the consequences of this gate against the running stack, and
    they are narrower than they might look. Enforcement diffs violations against the persisted subject
    (ViolationDiff::newViolations), so a subject that already carries a foreign target does not
    become un-editable when enforcement is on: a label-only re-save still succeeds, reporting the
    pre-existing violation without blocking. The gate therefore trades against exactly one thing: in
    default (enforcement-off) config, a write with an unresolvable-source target persists the qualified
    target verbatim and projects into Neo4j as a label-less, wiki_id-less stub node plus a real typed
    edge, and that stub outlives removal of the statement (the edge is deleted on re-projection, the
    orphan node is not garbage-collected). So the decision is purely: stop unresolvable targets at
    write time by default, or let them accumulate as orphanable graph stubs until an operator enables
    enforcement.

  • Bare and explicitly-local-qualified targets pass (the id parser canonicalizes before the
    check), registered Sources pass, and StatementDeserializer stays untouched so persisted
    foreign-qualified targets keep round-tripping.

  • The frontend does not hard-reject foreign targets: the client cannot know the server's
    registry (extensions register Sources), so the server stays the authority and the editor surfaces
    the violation through the existing validation rendering with a localized message
    (neowiki-field-relation-target-source-unresolvable, registered in en.json, qqq.json, and the
    ResourceLoader messages array).

Docs: the new code is documented in docs/api/validation-codes.md (including that it fires for
out-of-schema statements, as an exception to their usual silent skip) and the relation-target row
in docs/api/subject-format.md carries the enforcement qualifier.

AI-authored — Claude Code, Fable 5 (max); standing directive from @malberts to complete the #993 tracks without blocking on inter-track human review, design calls delegated; diff not yet human-reviewed; TDD with watched failures on all guard paths, two adversarial AI review lenses with live REST/eval probes (which caught and closed the out-of-schema bypass), make ci and make ts-ci green locally, branch CI pending at posting time.

Production notes

Design and review adjudication by Fable 5 (max); implementation and the fix round by
Opus 4.8 (max) subagents; two Opus 4.8 (max) review lenses (runtime correctness with probes,
tests/conventions/docs) stood in for the usual pre-PR human review at @malberts' direction. The
correctness lens probe-confirmed that the initial schema-scoped guard placement was bypassable via
out-of-schema relation statements under enforcement; the shipped guard is statement-scoped for that
reason.

malberts and others added 2 commits July 15, 2026 11:22
Implement the v1 guard of ADR 23 ("Relations across Sources"): a
Relation may only target a Subject whose Source resolves through the
registry. SubjectValidator now consults the SourceRegistry for each
relation target and emits a blocking `relation-target-source-unresolvable`
violation when the target's id names a source key that is non-null and
not registered.

Because the check lives on SubjectValidator, it fires on both the
validate endpoints (dry-run) and the create/replace write paths that run
ProposedSubjectValidator, so a hand-crafted API write with an
unresolvable-source target is rejected under enforcement rather than
persisted. The guard runs against every statement in the payload rather
than only schema-declared ones: schema membership says nothing about
whether a target's Source resolves, and schema-scoping would let an
out-of-schema or type-mismatched relation statement smuggle an
unresolvable target past enforcement. Bare targets, and ids explicitly
qualified with the local Source (canonicalized to bare by
SubjectIdParser), carry no source key and pass untouched; registered
Sources pass too.

The read path is deliberately left untouched: StatementDeserializer
never revalidates, so an already-persisted foreign-qualified target
still deserializes and round-trips, and cross-source relations can open
up later.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Register the `neowiki-field-relation-target-source-unresolvable` message
(en.json, qqq.json, and the ext.neowiki ResourceLoader messages array)
so the backend's new violation renders as a localized, source-key-aware
field error through the existing validation-response rendering.

The client does not hard-reject foreign targets: the server owns the
Source registry (extensions can register Sources), so the client only
mirrors the server's verdict. RelationInput already routes a relation
property's server violation to its field via the `neowiki-field-<code>`
convention; a spec pins that the new code surfaces with its source-key
argument.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
alistair3149 added a commit that referenced this pull request Jul 20, 2026
* Validate relation statement targets server-side

Fixes #1078

`RelationType::validate()` only emitted `required` and `RelationProperty::getTargetSchema()` had no
consumers, so relation statements went unchecked on write. An API write accepted a relation that
points at a Subject that does not exist, at a Subject whose Schema is not the property's
`targetSchema`, or that holds multiple targets on a `multiple: false` property. The subject editor's
picker hides this, but API-driven import does not go through the picker.

This adds three validations, using the severity model of ADR 26 as actually implemented (blocking vs.
`Violation::NON_BLOCKING_CODES`):

- `single-value-only` (`error`) for a `multiple: false` relation holding more than one target, emitted
  by `RelationType::validate()` exactly as `SelectType` does — no lookups needed.
- `relation-target-schema-mismatch` (`error`, new) when the target Subject exists but its own Schema
  is not the property's `targetSchema`.
- `relation-target-not-found` (`warning`, new, non-blocking) when the target id does not resolve.
  Deliberately non-blocking: pointing at a not-yet-created Subject is wiki-native red-link behaviour,
  and an import may mint the target later.

The last two live in `SubjectValidator` (the placement PR #1043 uses), since they need a lookup and
the writer's-schema `RelationProperty`, which `PropertyType::validate()` cannot reach. Targets are
resolved through the canonical revision-slot-backed `SubjectLookup` the read path uses
(`getSubjectRepository()`), never the secondary Neo4j projection; the target's Schema comes from its
own stored Subject. A missing target short-circuits, so it never also reports a schema mismatch.

Enforcement is unchanged: `relation-target-not-found` joins `Violation::NON_BLOCKING_CODES` (the
warning tier as implemented), and the newly-introduced-blocking-violation gate is untouched.

`RelationInput` already renders server violations generically via `neowiki-field-<code>` messages, so
the two new codes get `i18n/en.json`, `i18n/qqq.json`, and `extension.json` entries and no new UI
code. `docs/api/validation-codes.md` documents both codes and notes `RelationType` as a
`single-value-only` emitter.

ADR 26 (Draft) is intentionally not edited: its per-Constraint severity model is not fully
implemented, and PR #1043 set the precedent of not treating it as a living code catalog. Folding
these two fixed-severity system codes (mismatch = fixed `error`, not-found = fixed `warning`) into
ADR 26 is a follow-up for whoever finalizes it.

A textual conflict with open PR #1043 (which also touches `SubjectValidator`, the same three test
files, `validation-codes.md`, and the i18n / extension.json message list) is expected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Set valuePartIndex on relation-target violations

The two relation-target violations SubjectValidator emits (relation-target-schema-mismatch and
relation-target-not-found) left valuePartIndex unset. ViolationDiff keys violation identity on
(propertyName, code, valuePartIndex), which the Replace write path uses to block only
newly-introduced violations under $wgNeoWikiEnforceValidation. With no index, two schema-mismatch
violations on the same multi-value relation property share one key, so adding a second
schema-mismatched target when one already exists is not reported as new and slips the enforcement
gate.

Carry the target's index within the RelationValue as valuePartIndex, exactly as SelectType and
UrlType do for their parts, and update docs/api/validation-codes.md (guideline 4) to match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Cast relation index to int for the valuePartIndex argument

The foreach key over RelationValue's Relation[] is an array-key to Psalm, which flagged a
MixedArgumentTypeCoercion passing it to the int valuePartIndex parameter. Cast at the call site to
keep the changed file Psalm-clean; PHPStan already inferred int and does not flag the cast as
redundant.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* Correct the relation-target resolution docs and pin part indices

The docblock and validation-codes.md said relation targets are resolved
"never [through] the secondary graph projection". Only half of that holds:
the Schema compared is the target's own writer's-schema read from its
revision slot, but reaching that slot resolves the target id through the
subject -> page index, which lives only in the graph projection. The claim
also contradicted NeoWikiExtension::getPageIdentifiersLookup() and
operations/maintenance.md, which both say the reverse index is Neo4j-only.

Say what actually happens instead, and note the consequence: a target
present in revision slots but missing from the graph reports as not found
until a rebuild, which is the case #1022 leaves behind after an import.
Being non-blocking, that misreport never costs a write.

Also list the two relation checks under "Known limitations", which still
claimed SubjectValidator performs exactly two Subject-level checks.

Add three tests. The existing valuePartIndex test uses two equally invalid
targets, so array position and violation ordinal agree and a counter-based
implementation passes it; the new one puts a valid target first so the two
disagree, and covers the not-found branch that asserted no index at all.
Both fail against a count( $violations ) mutation. The third pins that a
relation property whose JSON omits "multiple" defaults to single-valued,
which now makes a second target a blocking violation.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: alistair3149 <alistair31494322@gmail.com>
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