Skip to content

feat(objects): adopt the declared referenceVia pointer pair on four system objects, per object (#11386) - #11676

Merged
os-sam merged 1 commit into
mainfrom
claude/issue-11386-referencevia-pointer-pairs
Aug 24, 2026
Merged

feat(objects): adopt the declared referenceVia pointer pair on four system objects, per object (#11386)#11676
os-sam merged 1 commit into
mainfrom
claude/issue-11386-referencevia-pointer-pairs

Conversation

@os-sam

Copy link
Copy Markdown
Collaborator

Fixes#11386

Adopts the declared referenceVia polymorphic pointer pair (ADR-0052 §5 ActivityPointer, carrier landed by #11339) on the remaining system objects that carry the (object half, id half) idiom — measured per object, not as a sweep, which is the card's own discipline and the triage grading's instruction.

objectpairverdict
sys_audit_logobject_name / record_iddeclared
sys_approval_requestobject_name / record_iddeclared
sys_record_shareobject_name / record_iddeclared
sys_share_linkobject_name / record_iddeclared
sys_automation_runtrigger_object / trigger_record_iddeliberately stays undeclared

No packages/spec/** change was needed: the key, its authoring refusals and its liveness entry all landed with the carrier. This PR is adoption plus tests.

Per-object verification (five separate ones, not one and four assumptions)

Each object's pair was re-verified against its own writers and its own consuming query before declaring, and each seed test asserts that object's load-bearing query rather than a generic "the id resolved":

  • sys_audit_log — four writers stamp an object machine name beside a real record id (audit-writers.ts, read-audit.ts, auth-event-audit.ts, plugin-auth's admin user endpoints). Read back by the {object_name, record_id} index and the record_views view: who touched THIS record. Both halves optional, so the un-addressable refusal is reachable here.
  • sys_approval_request — the pair is the key the machinery queries on: approval-service.ts finds the open request with where: { object_name, record_id, status: 'pending' }, and lifecycle-hooks.ts holds the record lock on the same pair. A verbatim natural key locked nothing while looking pending. Both halves required: true.
  • sys_record_share — dereferenced as a real address: record-orphan-cleanup.ts states the invariant ("record gone ⇒ the row cannot describe any access at all") and sweeps rows by asking whether (object_name, record_id) still exists. An unresolved grant enforced nothing while displaying as a grant, and was then deleted by that sweep.
  • sys_share_link — verified through a different consumer than its sibling table: share-link-routes.ts calls engine.find(link.object_name, …), and share-link-service.ts resolves a token through the fail-closed gate if (!(await this.recordStillExists(...))) return null. An unresolved link was a permanently dead token returning the same null as revoked/expired — silent at seed time, indistinguishable from a revocation at use time.
  • sys_automation_run — the shape fits (serialize() stamps trigger_object: ctx.object beside trigger_record_id: ctx.record.id) but the authorability does not, and the declaration's only enforced consumer today is seed-time resolution. A paused row is a live continuation the store rehydrates from every { status: 'paused' } row on boot; terminal rows are telemetry under this object's own 30-day sweep; and the object has no natural key to address rows by (nameField: 'id', no name field). Declaring would not make a real corpus resolvable — it would advertise run rows as authorable seed content. The verdict, its reasons, and what would have to change to flip it (a consumer that reads the pair for something other than seed authoring — [engine][设计卡] 多态弱引用挂靠表的平台级删除级联 —— sys_record_share/attachment/comment 一族的统一清理机制 #5180's delete-cascade carrier is the live candidate) are recorded on the field and pinned by a test, so a later sweep has to rule against it on purpose rather than by shape.

Seed corpus, measured

No seed dataset in this repo seeds any of the five, and neither does the packaged HotCRM app: its src/data/*.seed.ts datasets are business objects only, and src/sharing/demo-staffing.ts states outright that "no seed dataset and no flow node in this app targets … sys_record_share" (its grants are materialised by rule evaluation). So the accept/reject flip breaks no existing corpus; it is prospective, and the seed loader explicitly supports targeting sys_* tables (its writes run as a privileged system context for exactly that reason).

Measured while testing, and not what was predicted

The first draft of the audit-log case asserted that an out-of-order pointer heals in pass 2, by analogy with sys_activity. It does not, and the reason is a property of the dataset rather than of the pair: an engine-owned row has no natural key, so its dataset declares no externalId, and pass 2 back-fills by looking the row up by externalId — it resolves the target and then has nowhere to write it. The test now asserts the measured behaviour (loud failure, never the verbatim store), and a positive control — the same seeds, same order, with an externalId declared — isolates the cause to keylessness rather than to the pointer pair. Filed separately as #11674 rather than fixed here.

Not asserted either way, deliberately: on the three objects whose id half is required: true, a real engine enforcing required would also reject the deferred insert. The engine double in that suite does not validate, so it cannot measure this; #11674 carries it.

Ablation — both legs, restored byte-identically

Predicted in writing first, then run; mutation confirmed on disk by anchored greps in both directions (removed-text count and injected-text count), restored via git checkout against an absolute path, and proven by git hash-object against the HEAD blob (non-empty and matching on both files).

  • Leg A — strip referenceVia from the loader test's ADOPTER_SCHEMAS (4 anchored hits removed, 4 markers injected): 9 failed / 11 passed. The 9 are exactly the declaration-dependent cases — 4 accept, 3 refuse, the keyless case and its positive control. The 11 that stayed green are the Seeds cannot address an ActivityPointer: sys_activity.record_id is plain text, so a packaged app can load timeline rows that attach to nothing #11339sys_activity cases, the internal-id-verbatim case (an undeclared column also stores verbatim, so that assertion does not discriminate on its own) and the sys_automation_run undeclared case.
  • Leg B — delete the referenceVia line from the realsys_audit_log declaration (1 hit removed, pure deletion): the intended assertion goes red with expected undefined to be 'object_name', 3 others in that file stay green. A first attempt that injected a marker key instead was thrown out as not-measured: the strict field schema refused it at import, so the file reported "no tests" rather than a red assertion.

No rebuild was needed for either leg: both mutated files are imported by relative path from tests inside their own package, so nothing resolved through dist.

Verification, all at 0245ee7421

  • five package suites green: metadata-protocol 1909 passed / 10 skipped · plugin-audit 318 · plugin-approvals 589 · plugin-sharing 652 · service-automation 1035
  • typecheck green for the three packages that declare the script (6 script-name echoes in the output, so the filter matched non-zero); metadata-protocol and service-automation declare no typecheck script — check:type-check-coverage covers them structurally and is green
  • 22 gates derived with scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (path-matched + convention-triggered) all green, including check:engine-double-contractno new rows in scripts/engine-double-contract.pinned.json, because the new cases reuse the fake engine already pinned in that file
  • check:i18n first returned PREREQUISITE NOT MET — the workspace CLI is not built, which is not a pass; after turbo run build --filter=@objectstack/cli it reports OK (9 package(s) — all bundles in sync), with plugin-approvals, plugin-audit and plugin-sharing each in sync
  • pnpm lint (full repo, eslint . --no-inline-config) exit 0 — no narrowing claimed
  • check:type-check-debt --re-measure on the built closure: check-type-check-coverage: OK — 65/78 workspace packages type-checked …, no ratchet drift

Generated by Claude Code

…ystem objects (#11386)
#11339 landed the polymorphic pointer-pair carrier (ADR-0052 §5 ActivityPointer)
and adopted it on `sys_activity`. This card adopts it on the remaining system
objects carrying the same `(object half, id half)` idiom — measured per object,
not as a sweep:
- sys_audit_log.record_id via object_name — declared
- sys_approval_request.record_id via object_name — declared
- sys_record_share.record_id via object_name — declared
- sys_share_link.record_id via object_name — declared
- sys_automation_run.trigger_record_id — deliberately NOT declared
Each adoption re-verified that object's own pair semantics (who writes it, what
reads it back) and its seed corpus before declaring, and each seed test asserts
the query that makes the pair load-bearing on that object rather than a generic
"the id resolved": the {object_name, record_id} index for the audit ledger, the
pending-request lock for approvals, the grant lookup for record shares, and the
fail-closed recordStillExists gate for share links.
The fifth object records a deliberate "stays undeclared" verdict at the
declaration site, with its reasons and what would have to change to flip it: a
paused sys_automation_run row is a live continuation the engine rehydrates on
boot, terminal rows are telemetry under a 30-day sweep, and the object has no
natural key to address rows by — so declaring would advertise run rows as
authorable seed content rather than make a real corpus resolvable.
Measured while writing the tests, and recorded in them: unlike sys_activity,
none of the four heals an out-of-order pointer in pass 2, because an engine-owned
row has no natural key and pass 2 back-fills by externalId. The load fails loudly
(the ruled family direction) rather than storing the literal; a positive control
isolates the cause to keylessness rather than to the pointer pair.
No new engine doubles, so no new rows in scripts/engine-double-contract.pinned.json.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01APWX2AwT3a4xDcjPCe8bk4
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/plugin-approvals, @objectstack/plugin-audit, @objectstack/plugin-sharing, @objectstack/service-automation, touching 8 documentable anchor(s).

26 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 8bcd054702f4078bedcd21a3734524d235ba4b9d.

5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see

Coarse fallback — 16 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 8bcd054702f4078bedcd21a3734524d235ba4b9dpackageMentionDocs.

Which tree this was computed on

This run read content/docs from edc561255a31b9335cacde7893dfd220d0006ce1 — the merge of head 0245ee74216b2dd040375e4018bd40ef7dc3b861 into base 8bcd054702f4078bedcd21a3734524d235ba4b9d, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin edc561255a31b9335cacde7893dfd220d0006ce1 && git checkout edc561255a31b9335cacde7893dfd220d0006ce1
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8bcd054702f4078bedcd21a3734524d235ba4b9d 0245ee74216b2dd040375e4018bd40ef7dc3b861 && git checkout -B drift-repro 8bcd054702f4078bedcd21a3734524d235ba4b9d && git merge --no-ff 0245ee74216b2dd040375e4018bd40ef7dc3b861
node scripts/docs-audit/affected-docs.mjs --json 8bcd054702f4078bedcd21a3734524d235ba4b9d

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 8bcd054702f4078bedcd21a3734524d235ba4b9d → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-samClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM: docs-drift advisory dispositioned — no action; and this PR is blocked on contract review

domain:services seat (session session_01APWX2AwT3a4xDcjPCe8bk4).

Docs drift — checked where it could actually bite

The advisory lists 26 hand-written pages (truncated above 15 rows). Rather than re-read all 26, the check was aimed at the only class that this diff can make actively wrong: a doc showing an authored seed whose record_id is a natural key, on one of the four newly-declared objects. Those examples would previously have stored verbatim and are now refused loudly.

Read with git grep <ref> on edc561255a… — the merge commit the run itself named, not the shared working tree, which is a different and unknown state. Positive control first: 20 files on that ref name at least one of the four objects, so the channel reaches content/docs.

Result: no such example exists. Every record_id occurrence outside content/docs/releases/ is one of —

  • a storage path convention ({category}/{object}/{record_id}/{filename}) and HTTP/realtime URL path params — unrelated to this column;
  • record_id as a UI variable type in component.mdx / page.mdx / pages.mdx — unrelated;
  • audit-service.mdx:49 and record-view-auditing.mdx:162, which describe what the runtime writers put in the column — still accurate, unchanged by this diff;
  • references/data/field.mdx:79, which already documents this exact behaviour and was already correct before this PR: "Enforced today at seed load: the value resolves as a natural key against the object the sibling column names, and an unresolvable pointer is refused loudly instead of stored verbatim." That text landed with the Seeds cannot address an ActivityPointer: sys_activity.record_id is plain text, so a packaged app can load timeline rows that attach to nothing #11339 carrier, and adopting four more objects does not change what it says.

⛔ The 5 release-owned pages were read-only and untouched.

⚠️ For future readers of that bot: it is symbol-anchored and precision-first (#9192) and covers hand-written docs only — it lists a page because the page names a symbol the diff touched. It cannot see whether prose went false, which is why the check above was aimed by hand at the one failure class rather than taken from the row count.

⛔ This PR cannot enqueue yet — needs:contract-review added to #11386

Its own changeset states the reason:

The accept/reject contract changes with it on those four objects, deliberately and in the already-ruled direction.

That is clause-② on the content limb, re-read live this round from scripts/pm/dispatch-gates.mjs: "Clause ② is NOT reachable from paths … judged from the card CONTENT. This line is a FLOOR, never a clearance." This PR touches no packages/spec path — correctly, since the carrier landed with #11339 — so a path-only reading would have let a real accept/reject change through without the control. CONTRACT_REVIEW_TIER read live in the same pass: claude-fable-5; this seat is below it, so it will not clear the label, not flip this PR ready, and not arm auto-merge.

⛔ Nothing here is a finding against the work — the per-object discipline is visibly honoured, including the deliberate "stays undeclared" verdict on sys_automation_run recorded at the declaration site and pinned by a test, so a later sweep must rule against it on purpose rather than by shape.


Generated by Claude Code

@os-sam
os-sam marked this pull request as ready for review August 24, 2026 13:30
@os-sam
os-sam added this pull request to the merge queueAug 24, 2026
Merged via the queue into main with commit 599515dAug 24, 2026
32 checks passed
@os-sam
os-sam deleted the claude/issue-11386-referencevia-pointer-pairs branch August 24, 2026 13:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-sam@claude