fix(reference-carrier): four C2 readers answer absence instead of propagating an unreadable reference - #19197
Conversation
…ating an unreadable `reference` `FieldSchema.reference` is declared an optional STRING. Four readers gated it with a truthiness test, which a non-string object passes, and then propagated the value onward — the approvals one through `String()`, which manufactures the literal target name `[object Object]`. Each site now reads the carrier through the one arbiter, `referenceCarrierOf`, and catches its refusal AT THE SITE so the reader yields absence and reports, rather than aborting: these are best-effort display and diagnostic readers, not the cascade seams that let the throw propagate on a write path. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
…adable control Each site gets a case that fails before the narrowing — the approvals one pins the literal `[object Object]` directly, since that string is the whole evidence — and a control that keeps a readable string target flowing, so "narrowed" and "this path is closed" stay distinguishable. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
📓 Docs Drift CheckThis PR changes 3 package(s): 30 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 7 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 33 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c6f94b833c19cab6849ffb83101b72955bf7a13b && git checkout c6f94b833c19cab6849ffb83101b72955bf7a13b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin d402e328552622e3b421d567e793d4b77c324c44 1130dc81b77ac0d0c32d2bbc9b43475c3fe34ac9 && git checkout -B drift-repro d402e328552622e3b421d567e793d4b77c324c44 && git merge --no-ff 1130dc81b77ac0d0c32d2bbc9b43475c3fe34ac9
node scripts/docs-audit/affected-docs.mjs --json d402e328552622e3b421d567e793d4b77c324c44
|
Fixes #19081
Clause-②: no
Four readers of
FieldSchema.referencegated the carrier with a truthiness test, which a non-string object passes, and then propagated the value onward. The declared contract is an optional string, so the answer a reader owes for a carrier it cannot read is absence. The approvals site did worse than lose the information —out.push({ key, reference: String(f.reference) })manufactures the literal target name[object Object], and hands it on as an object name toengine.find(), where the failure disappears into the caller's owncatch.These four are the class PR #18503 labelled C2 and deliberately left unchanged. This PR acts on that class; it is not a claim that #18503 or #19080 was wrong to leave it, and it establishes no reachability from an authored document — the engine's write path refuses this shape, so a row carrying it had to be written straight through the driver. Cheap insurance, not an incident.
The design question the card leaves open, and what decided it
Whether these sites should route through the arbiter
referenceCarrierOfor narrow locally. Measured first, because the arbiter does not return the same narrowing a localtypeoftest would: it throws aTypeError. That is extra knowledge, and at these four sites it is hostile knowledge:ApprovalService.resolveLookupFieldstry { … } catch { return []; }, so one unreadable field would drop every lookup field of the objectTypeErrorinside dataset compilation, replacing the compiler's own "cannot resolve this relationship" refusal with a crashos doctor(both sites)doctorexists to reportSo the repair is both: the carrier is read through the one arbiter — no fourth hand-copy of the narrowing, and
''/null/undefinedkeep their absence semantics — and the refusal is caught at the site, which yields absence plus one report. That is the deliberate line between these readers and the@objectstack/objectqlcascade seams #19080 landed, which let the same refusal propagate: those assert something positive about the schema on a write path, where the silence cost an orphanedmaster_detailrow and a reported success.Per site
@objectstack/plugin-approvals— the unreadable field is left out of the inbox display enrichment and logged through the service's existinglogger.warn. It is dropped rather than pushed with the target absent because the sole consumer destructures{ key, reference }and usesreferenceas the object-name argument toengine.find; an entry carrying none has nothing for that consumer to do, and keeping it would widen the declared return type for no reader. The effect there is the one this best-effort resolver already produces for every other unresolvable case — the entry stays unresolved.@objectstack/service-analytics— the ADR-0021 relationship resolver answersundefined, which its existing fallback turns into the dataset compiler's refusal, plus onectx.logger.warnnaming the field.@objectstack/cli—os doctor's circular-dependency and unused-object checks report the unreadable carrier as a finding rather than skipping it. Both publish a positive verdict — "No circular references detected", "defined but not referenced" — that an edge nobody could read cannot support, and both already return diagnostic strings thatdoctorprints as warnings, so being loud here needed no new channel. The same file'scollectViewObjectRefsalready narrowed its own carrier.Tests, and the ablation behind them
Each of the four sites has a case that fails before the change and a readable-target control beside it, so "narrowed" and "this path is now closed" stay distinguishable. The
[object Object]string is pinned directly, since it is the card's whole evidence.Ablation: each narrowing was reverted to its truthiness gate through
scripts/ablation-replace.mjs(anchor must hit; on-disk counts and blob hashes are the tool's own verdict), the pin re-run, then restored. Every site is source-imported by its suite, so nodistround trip is involved. Restores verified by the tool:blob == HEADandgit diff HEADempty.expected [ 'crm_account', …(2) ] to not include '[object Object]'expected { name: 'shop_invoice', fields: {} } to be undefinedVerification
All commands run at
1130dc81b.pnpm --filter @objectstack/plugin-approvals --filter @objectstack/service-analytics test— exit 0; 48 files / 775 tests and 113 files / 2411 tests.pnpm --filter @objectstack/cli exec vitest run --project unit— exit 0; 218 files / 3069 tests. Theintegrationtier is declared to CI: the diff touches no integration-tier test file, nobin/entry and no driver or kernel boot path.pnpm --filter … typecheckfor the three packages — exit 0, test layer included.scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack: 64 families, each run with its exit code landed to a file, then reconciled with--ran: 60 run, 4 NOT MEASURED, 0 UNRUN. The four arecheck:dual-build-cjs-loads,check:i18n,check:i18n-coverageandcheck:i18n-walk-parity, each exiting 3 = PREREQUISITE NOT MET in this container because they read whole-tree built output. Not a pass — declared to CI.eslint . --no-inline-config— exit 0 over the 6900 files eslint's own config reads, 0 errors / 0 warnings. Type-aware linting is not enabled ineslint.config.mjs, so no untouched file's verdict can move with this diff; this is the full population rather than a narrowing.Acceptance notes
packages/cli/src/commands/doctor.ts'sdetectCircularDependenciesgained anexportso its carrier reading is assertable; it has no other caller, anddoctor.tsis not a declared entry in this package'sexportsmap, so nothing is added to the published API surface. That is the only change in the file beyond the two gates the card names.referenceCarrierOfhit acrosspackages/objectql,plugin-approvals,service-analyticsandpackages/cli, and it is a test — no longer holds for that four-package corpus:packages/objectql/src/engine.tsnow carries four production hits, landed by Refuse an unreadablereferencecarrier at the ten residual readers (ruling E item 2 residue) #19080 after the card's ref. Inside this PR's three packages the zero stood (onlypackages/cli/test/data-model-rules.test.ts), with the token resolving in 20 files repo-wide as the live control.collectViewObjectRefs(same file) narrows with a baretypeoftest, so it admits''as a target name and reports nothing when a carrier is unreadable. Out of scope here — it is not one of the four propagating sites, and it already answers absence rather than propagating.🤖 Generated with Claude Code
https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Generated by Claude Code