Skip to content

fix(analytics): ask the object-level read grant before serving an inline dataset — one admission verdict on every driver - #16860

Draft
os-trump wants to merge 11 commits into
mainfrom
claude/issue-16645-analytics-native-sql-authz
Draft

fix(analytics): ask the object-level read grant before serving an inline dataset — one admission verdict on every driver#16860
os-trump wants to merge 11 commits into
mainfrom
claude/issue-16645-analytics-native-sql-authz

Conversation

@os-trump

@os-trump os-trump commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Fixes #16645

POST /api/v1/analytics/dataset/query accepts an inline dataset definition (body.dataset) from any authenticated caller. On a SQL driver the compiled statement ran through the driver's raw execute() — documented in sql-driver.ts as a tenant-isolation bypass ("Unlike find/update/delete etc., raw execute() does NOT inject the organization_id predicate") and with no middleware in front of it — so the request reached the database having passed exactly one of the three read layers: the row scope, threaded since ADR-0021 D-C through getReadScope. A caller with no grant of any kind on an object received its row count, and with dimensions its grouped counts by any column. The memory driver refused the identical request, because there the query falls through to the ObjectQL engine and the engine applies all three layers in one place.

What changed

One gate, at the door, ahead of strategy selection. ⛔ Not a layer per strategy: two strategies each enforcing their own copy is the cause of this defect, not its remedy — driver-memory is correct today precisely because it hands the request to the engine. AnalyticsService now asks the object-level read admission once, for the base object and every joined object, before any strategy is chosen. Both strategies — and any strategy added later — inherit one verdict by construction.

file what it does
packages/spec/src/contracts/security-service.ts ISecurityService.canReadObject(object, context) — the object-level half of a read, declared beside getReadFilter's row-level half. Optional; absence falls back to explain (not optional), never to "admit".
packages/plugins/plugin-security/src/security-plugin.ts Implements it as the middleware's own read gate, arm for arm and in its order: isSystem bypass, no-permission-sets skip, #3545 unresolved-posture deny, ADR-0066 D3 capability AND-gate, allowRead CRUD grant, ADR-0090 D10 delegator intersection. Same primitives the middleware calls. Exposed on the registered security service.
packages/services/service-analytics/src/read-admission.ts The gate and its PERMISSION_DENIED / 403 refusal. A wired provider that throws or answers false denies. The refusal names only the object the caller themselves named. Its log sink declares warn required beside an optional error, so a refusal always has somewhere to land (#9754).
packages/services/service-analytics/src/analytics-service.ts admitObjectRead config hook; the gate called from callCtx (which query(), generateSql() and — via DatasetExecutor — every dataset door share) and from the draft-preview branch, the one dataset path that never reaches callCtx. queryObjects/cubeObjects is now one derivation feeding both the admission set and the read-scope set, so the set that is admitted and the set that is row-scoped are provably the same set.
packages/services/service-analytics/src/plugin.ts Auto-bridges admitObjectRead to the security service, exactly as getReadScope is bridged. Prefers canReadObject; falls back to explain({object, operation:'read'}).allowed for a security service that predates the method; warns loudly at init when no security service is registered. Tells three resolutions apart — an absent service admits, a wired-but-unusable one (a throwing getService, or a service carrying neither method) denies and reports at error.
packages/verify/src/harness.ts bootStack(app, { databaseDriver: 'sqlite-wasm' | 'memory' }). A two-driver equivalence property cannot be measured on one driver — which is how the strategies were allowed to disagree in the first place.

Direction: this narrows the accept set. Requests already refused by /data are now refused by analytics too; nothing refused becomes admitted. No exemption, no carve-out, no compatible fallback.

What "fails closed" does and does not claim. It is a claim about a wired provider. A deployment that registers no security service keeps its pre-PR analytics behaviour, deliberately: that deployment has no object-level gate on GET /data/OBJECT either — that gate is the absent middleware — so the two doors still agree, and agreement is the property the card asks for rather than refusal for its own sake. What is not left open any more is a security service that is wired and cannot be used; see the patch round below.

Patch round — contract review CHANGES REQUIRED (claude-fable-5-1, isolated seat)

Review: #16860 (comment), on head a3fa4e20c. The substance was judged sound (F4 through F8, F10); four findings were owed and all four are answered here. Patched head: 6d6fbf51e.

origin/main was merged in twice — the branch is published, so ⛔ no rebase, no amend, no force-push at any point. The second merge is the collision the cross-PR pile-up patrol predicted on this PR: #16755 landed and reached content/docs/permissions/system-context.mdx, the one file both diffs touch. It is written up under Verification, because the way it went wrong is worth a reviewer's minute.

F1 — check:optional-error-sink was RED, and the union claim was not true of that head

The gate. AdmissionLogger declared error? and warn?. An optional error with no guaranteed alternative is a contract that permits silence (#9754), and the gate's own fix line is to make warn required. Done: warn(message: string): void is now non-optional, error stays optional (hosts legitimately inject reduced sinks, which is why the gate deliberately never asks for error to be required), and the fail-closed branch reaches for warn when error is absent instead of dropping the report. The gate's census line on the patched head:

SINK CENSUS [optional-error-sink-contract] (#9754): 47 sink type(s) declaring `error` in packages/** —
14 declare it REQUIRED (nothing to guarantee), 32 declare it optional beside a REQUIRED `warn`,
1 permit silence (1 optional-fallback, 0 no-fallback).

AdmissionLogger is no longer among the sinks that permit silence, and the single remaining one is a pre-existing ledger entry this PR does not touch. Ablated: reverting the one token warn to warn? on disk (anchor count 1 → 0, injected 0 → 1, blob bf385d5a differing from the HEAD blob) turns the gate RED and it names the line — packages/services/service-analytics/src/read-admission.ts:129 · interface AdmissionLogger { error? warn? } — then restores to blob equality with an empty git diff HEAD.

The attestation error, which is the more serious half. The previous revision of this body claimed "121 derived families … 0 NOT-MEASURED, every family green". That was not a reading of that head. It is restated in Verification below from an actual run on 6d6fbf51e, exit code by exit code, and ⛔ nothing is rounded up: a family that answers a prerequisite refusal is NOT MEASURED and is never reported as a pass.

F2 — the changeset carrier

The body declared an accept-set narrowing on a published route while the changeset carried neither a **BREAKING** banner nor an ADR-0087 disposition, so check:adr-0087-registration had nothing to check. Fixed on the shape sibling #16805 uses — a ! in the summary line plus the disposition marker. ⛔ No level changed: minor on spec / plugin-security / service-analytics / verify was already right, and during the launch window the bump level is not the carrier. check-changeset-no-major.mjs's own header says so:

The mandatory information carriers for breaking-ness in the meantime are the BREAKING banner the author writes in the changeset body and the ADR-0087 migration-ledger disposition (check-adr-0087-registration.mjs refuses a declared breaking change that states neither). They are not documentation niceties — during the window they are the only signal there is.

The gate's verdict on the patched head:

✓ check-adr-0087-registration: 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition.
    .changeset/analytics-inline-dataset-object-read-admission.md  [BREAKING+bang]  not-required (no-migration-prescription)

[BREAKING+bang] is the gate reading both halves. The disposition argues the category on its own terms: nothing authorable is renamed, retired or re-typed, no stored shape moves, and the remedy for a refused caller is a grant held in permission-set data rather than in an authored file — so objectstack migrate meta has nothing to rewrite. The changeset body carries no migration prescription, which is the one thing that category is mechanically refused for.

F3 — the two silent-admit corners now DENY

assertReadAdmitted no-ops with no hook wired, and the bridge returned true in three states that were spelled identically (return undefined, then if (!svc) return true three lines later). Two of those three were wrong:

bridge state before now
no security service resolves admit admit — unchanged and deliberate; /data has no object gate on that deployment either, so the two doors still agree
ctx.getService('security') throws admit, silently DENY, reported at error
the service carries neither canReadObject nor explain admit, silently DENY, reported at error

Both new denials are wired-but-broken providers, and /data's middleware does not fall open in either state — admitting there would reopen exactly the divergence this PR closes, and would do it invisibly. explain is non-optional on ISecurityService, so a conforming provider never reaches the second row at all.

New cases in packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts (6 cases). Each denial asserts the ADR-0112 envelope (code: 'PERMISSION_DENIED', status: 403), that the database was never reached (a gate that refuses after running the statement has refused nothing), and that the refusal was reported — a security refusal nobody can see is indistinguishable from a gate that never ran. The absent-service case is kept as the negative control: tightening it would refuse every analytics query on every deployment shipping no plugin-security, which is a different and wrong answer from the one /data gives there.

Ablation for both new denial paths is in Verification below.

F9 — the loosened warn assertion, tightened

raw-sql-object-routing.test.ts, case "leaves default-datasource objects exactly where they were", had gone from a blanket not.toHaveBeenCalled() to not.toMatch(/is unavailable/) — which admits every other new warning into a case whose whole job is to prove this object's routing did not regress. It now asserts the fixture emits exactly one warning and that it is the deliberate admission-bridge init report, named; a second warning of any wording fails the case.

F3's third corner, F11 and F12 — recorded, nothing owed

  • F3, the absent-provider default. Stated above and in the changeset rather than changed: it is the state in which the two doors agree, and it is what keeps a deployment shipping no plugin-security working exactly as before.
  • F11 — single-writer ordering. fix(plugin-security)!: evaluate the insert-side RLS check on the row that will be stored, after beforeInsert #16805 also edits security-plugin.ts; the single-writer check is green on this head and the collision is a landing-order fact with a definite end. ⛔ Not resolved here, and no other region of that file is touched by this round.
  • F12 — the REST exposure gate. The equivalence this PR asserts is against the permission verdict. /data also refuses objects the REST exposure gate (apiEnabled / apiMethods) withholds, and an inline dataset over such an object is not covered by canReadObject. Outside the card's stated hole and outside this PR; recorded here so it is not mistaken for covered.

验收备注

Triage's rubric, verbatim, each with the evidence that answers it.

1. Equivalence assertion, ⛔ not per-layer assertions — on sqlite AND memory

packages/qa/dogfood/test/analytics-inline-dataset-admission.dogfood.test.ts boots the same fixture twice, once per driver, and compares the analytics verdict to /data's verdict rather than to a hard-coded expectation. 10/10 green on both drivers, re-run on the final head.

persona object driver GET /data/OBJECT inline POST /analytics/dataset/query
member, no grant admission_walled sqlite-wasm 403 403
member, no grant admission_walled memory 403 403
member, no grant, grouped (GROUP BY region) admission_walled both 403 403
member, grant + owner RLS admission_open sqlite-wasm 200 · 2 200 · 2
member, grant + owner RLS admission_open memory 200 · 2 200 · 2
administrator admission_open both 200 · 3 200 · 3
administrator admission_walled both 200 · 4 200 · 4

The same equivalence is asserted at unit level across both strategy paths (read-admission-gate.test.ts) and against the real middleware in plugin-security (can-read-object-admission.test.ts — its first block drives the registered middleware with a find and asserts canReadObject returns exactly what the middleware decided, for 7 cases).

2. The zero-declaration leg

The fixture app declares 0 datasets and 0 dashboards — stated in its header as the load-bearing property, not an omission. One case asserts a datasetName lookup 404s on that deployment, which is what proves the refusals are not "the dataset was not found"; the inline body.dataset naming an ungranted object is refused on both drivers.

3. The walled-posture leg — turned from inference into a measurement

Measured as the two halves of one chain, each against its real producer:

  • producer — under a walled posture security.getReadFilter composes the Layer-0 wall: getReadFilter('crm_task', MEMBER_CTX) yields { organization_id: 'org-1' } (plugin-security/src/tenant-layer0-verdict-on-operation.test.ts, pre-existing, unchanged by this PR).
  • consumer — that exact predicate must survive into the statement the raw-SQL path compiles for an inline dataset. read-admission-gate.test.ts, "the tenant wall reaches the inline dataset on the raw-SQL path": the compiled SQL carries organization_id bound to the caller's organization; a joined object is walled too (at least 2 occurrences and 2 bindings — a single occurrence would mean a cross-org row can reach the GROUP BY through the join); and the wall holds for a caller the admission gate admits, so the two layers are shown to be independent rather than collapsed into one another.

⚠️ What this does not claim: an end-to-end two-organization boot. @objectstack/verify documents that cross-tenant isolation has exactly one honest proof in this repo — multiTenant: true with the enterprise @objectstack/organizations package, which is not in the open core, and whose stand-in 'posture-only' "performs no tenant isolation whatsoever". Asserting cross-org isolation on that stand-in would assert nothing and pass. The chain above is the strongest honest measurement available here, and the reviewer's F6 records the same reading: this is unit-level, and whether it satisfies condition 3 or rides with the condition-5 card is the maintainer's call.

4. Negative controls

  • The administrator's numbers are identical on both routes, on both drivers, before and after (3 and 4 in the fixture; the two objects carry deliberately different counts so a number arriving from the wrong table cannot pass). An implementation where the native strategy simply refuses would fail these.
  • The member's RLS-scoped count on admission_open (owner policy on created_by) is 2 on both routes and both drivers — security.getReadFilter is still threaded and still doing the narrowing.
  • read-admission-gate.test.ts carries the same controls at unit level: "ADMITS a granted caller and serves the number unchanged", "leaves behaviour unchanged when NO admission provider is wired", and "the admitted path stays identical across both strategies".
  • The patch round adds one more, for the same reason: "ADMITS when NO security service is registered at all" keeps the two new deny corners from being tightened into a blanket refusal.

5. The platform-admin disagreement — ⛔ NOT decided in this PR; escalated to the card

This PR does not make sqlite analytics 12/30/40/14 agree with /data's 0/0/0/0, and states plainly why the choice is not a dev judgement. Measured on origin/main:

⇒ Reported back to #16645 as a needs_decision rather than settled here, and the PM seat concurred. ⛔ Nothing in this PR silently selects the wider side: the object-level layer this PR adds only refuses; the tenant behaviour of every persona in the acceptance table is byte-identical to origin/main. The patch round changes nothing about it.

6. The immediate-mitigation question — answered: no

Should the native strategy fall back to the engine path whenever it cannot confirm the object-level grant, as an interim?

No, and it is not needed, because the shape shipped here is strictly better on both axes.

  • Correctness. A fallback is per-strategy logic — a second copy of the decision inside NativeSQLStrategy — which is the arrangement that produced this defect. The gate here sits ahead of strategy selection, so there is nothing for a strategy to "fail to confirm": the verdict is already made when the chain is entered, and it is made once for every present and future strategy.
  • Cost. A fallback deletes the SQL analytics path for every object it cannot confirm, which on a real deployment is most of them, and turns one compiled statement into engine aggregation. The gate costs one canReadObject per object per query, resolved from a permission-set resolution plugin-security already memoises per execution context, and it runs before any statement is compiled.
  • Interim exposure. There is none to bridge: this PR closes the object-level layer in the same change. The tenant-wall question that remains (condition 5) is not one a fallback would fix either — it is a disagreement between two walls, and routing analytics through the engine would silently adopt one of them, which is precisely the ruling being escalated.

7. Evidence files

Not rebuilt — docs/evidence/issue-8/22-…-memory.md and 23-…-sqlite.md on claude/issue-8-dashboards of objectstack-ai/ats are cited as given. The fixture above reproduces the shape of that matrix inside this repo so it stays measured on every CI run.

Noted, not filed — one adjacent corner deliberately left alone

The row-scope bridge (getReadScope) has the same three-way shape and still collapses it: a throwing getService('security') leaves trySecurity() returning undefined, so the query runs with no row scope rather than refusing. It is pre-existing, the reviewer examined this bridge and did not name it, and closing it would change whether queries run unscoped or refuse — which would move the tenant behaviour this PR deliberately keeps byte-identical to origin/main for condition 5. ⛔ Out of scope for this round, recorded rather than silently fixed or silently ignored.

Clause ② — derived, not inherited

Clause-②: yes — declared by the claiming seat on #16645 (comment 5584629769), which is where that declaration belongs; this PR does not fill it in on anyone's behalf.

  • node scripts/pm/check-widening-tells.mjs --declaration no --diff PR.DIFF answered ✓ 14 changed file(s) read, no widening tell on any declared surface. So there is no widening tell, which matches the direction: this narrows.
  • node scripts/pm/dispatch-gates.mjs --tier --repo objectstack-ai/objectstack answers no path-derived mandate, with packages/spec/src/contracts/security-service.ts flagged Clause ② SUSPECT surface.
  • The declaration is nevertheless yes, on the mechanical floor rather than on a tell: 新导出符号或已发布载荷上的新键恒 yes. This diff adds a new member to a published contract (ISecurityService.canReadObject) and new keys to three published payloads (AnalyticsServiceConfig.admitObjectRead, AnalyticsServicePluginOptions.admitObjectRead, BootOptions.databaseDriver). Absence of a widening tell is not a clearance, and the gate's own output says so.
  • Governed-surface predicate: node scripts/pm/check-governed-merges.mjs --test over the final path list answered 0 of 14 path(s) hit the register (5 surfaces, repo-agnostic) … NOT governed. The contract reviewer re-derived the same verdict independently on all 14 paths.

⚠️ needs:contract-review was dropped from both carriers when the review concluded, and ⛔ this round does not re-hang it — re-hanging is the dispatching seat's act once this patched head exists, and the review then runs again on the moved head.

Verification

All readings below are from head 6d6fbf51e, whose merge base with origin/main is 44c849c7ddispatch-gates prints no STALE TREE warning on it and names that base itself (15 path(s) vs merge base 44c849c7d of 'origin/main' and HEAD). Every command ran in the foreground; heavy runs went through scripts/pm/os-verify-lock.sh on a shared box. Every verdict is quoted from the gate's own line and every exit code was captured with a redirect before any pipe — ⛔ never from a bare $? behind | tail.

The #16755 collision, and the thing the merge driver could not catch

The cross-PR pile-up patrol predicted (comment 5586321991) that whichever of this PR and #16755 landed second would meet a hand-resolvable prose conflict in content/docs/permissions/system-context.mdx. #16755 landed; this is that conflict, and it went wrong in a way worth naming.

The file is MIXED — a generated half plus hand-written prose — so the merge=os-regen driver deliberately declined to defer it and text-merged instead. Two hunks conflicted and were resolved by hand: main's enriched automation row was adopted whole, including its fourth anchor #refuseUngrantedRunLifecycleWrite, carried onto this branch's row number (this branch inserted a row upstream in the same table, so every row below it is offset by one).

⚠️ The part with no conflict marker anywhere near it. Both sides had independently bumped the same seven declared counts from 106 to 107 — main for its new elevation read, this branch for canReadObject. Identical edits text-merge clean, so every one of them silently survived as 107 while the merged tree holds 108. Two correct edits, one wrong sum, no conflict. check:system-context-census is what catches it, and it says outright that there is no mechanical repair; the seven were corrected by hand, after the merge was committed and ⛔ never while the tree was in MERGE state (regenerating there rolls the anchor back to the old fork point, with every gate still green):

check-system-context-census: OK — 108 elevation read sites in 20 packages across 45 files,
living in 91 symbol(s); the page cites 105 symbol(s) against 105 required, over 129 anchors
and 8 file-level citation(s); 22 declared non-read; 9 file(s) hold more than one read in one
symbol (the declared precision loss); 18 row reference(s) resolve to their keyed row.

git merge-tree --write-tree origin/main HEAD now exits 0 — the head is clean against current main.

Suites, types and lint

what result
@objectstack/service-analytics suite Test Files 98 passed (98) · Tests 2190 passed (2190)
@objectstack/service-analytics typecheck (tsc --noEmit) VERDICT command-exit 0
the two-driver dogfood acceptance suite Test Files 1 passed (1) · Tests 10 passed (10)
pnpm lint (eslint . --no-inline-config), whole repo 6387 files linted, 0 errors, 0 warnings, VERDICT command-exit 0

⚠️ Scope of this round's suite run, stated rather than implied. The patch round edits three files, all in @objectstack/service-analytics. The public byte surface of the other three packages is unchanged by it, so their suites are not re-owed; the counts for spec (465 files / 12955 tests), plugin-security (103 / 1917) and verify (12 / 67) in the earlier revision of this body were measured on a3fa4e20c and are ⛔ not restated as readings of this head. CI runs them on the moved head, which is where that reading belongs.

The derived gate union — restated, and this time it is a reading of this head

The previous revision claimed "121 derived families … 0 NOT-MEASURED, every family green." That was not true of a3fa4e20ccheck:optional-error-sink was red on it. That claim is the more serious of the two defects the review found, because it is what a reviewer trusts instead of re-running. The honest reading on 6d6fbf51e:

dispatch-gates: gate list derived from the tree of 'objectstack-ai/objectstack' at commit 6d6fbf51e
  --repo 'objectstack-ai/objectstack' checked against this checkout's 'origin' remote — it holds.

Run reconciliation — 121 derived, 121 run, 0 NOT-MEASURED, 0 UNRUN.
✓ dispatch-gates --ran: 121 derived famil(ies) accounted for — 121 run, 0 NOT-MEASURED.

115 families answered exit 0 on the first pass. Six did not, and none of the six is reported as a pass on the strength of that first answer — every one was a prerequisite refusal against a packages/spec/dist that no longer matched src after the merge, which the gates detect and refuse on rather than measuring ("a verdict now would be computed against a build that no longer matches src … a FALSE GREEN on exactly the change it exists to catch", #7122):

family first answer resolved to
@objectstack/spec check:api-surface prerequisite refusal rebuilt, re-run → exit 0
@objectstack/spec check:browser-reachable-entries prerequisite refusal rebuilt, re-run → exit 0
@objectstack/spec check:dual-source-exports prerequisite refusal rebuilt, re-run → exit 0
@objectstack/spec check:entry-nameability prerequisite refusal rebuilt, re-run → exit 0
@objectstack/spec check:exported-any prerequisite refusal rebuilt, re-run → exit 0
@objectstack/spec check:skill-examples prerequisite refusal rebuilt, re-run → exit 0258 prose examples type-check across 3 surface(s)

A prerequisite refusal is NOT MEASURED, never a pass. The gates say so themselves — "Nothing was measured: this gate exited before running a single check … It is NOT a finding" — so the workspace closure was rebuilt (pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*', exactly as lint.yml does) and each was re-run to a real verdict. An earlier pass on 08825847f hit the same class on seven other families (check:dual-build-cjs-loads, check:i18n, check:type-check-debt, check:docs-transcript-drift and the two @objectstack/lint doc gates, all wanting an unbuilt dist/); each was likewise built and re-run to exit 0 — e.g. check:type-check-debt--re-measure: OK — 5 ledger entr(ies) re-measured, 55 raw tsc error(s) total, none above its recorded number.

What the 121 does NOT cover — the part the earlier claim left out

dispatch-gates's own reconciliation says the 121 is "NOT a complete account of what CI runs on the PR", and this card reaches families outside it that are honestly NOT MEASURED here:

  • 5 families this card's paths reach take a value from the workflow and have no local invocation at all — check-shard-attestation.mjs (three jobs, each pinning a shard index and a runner temp path) and check-test-completeness.mjs (two jobs, each naming a runner temp log). The tool refuses to invent an invocation, and so does this body: they are measured on CI, not here.
  • 42 artifact-roster families, 10 declared-wide-population families, the pending-changeset families and the always-runs step tail each sit outside the 121 with their own counts.

⚠️ One limb of check:changeset-no-major is NOT MEASURED locally, and is not claimed green. Its launch-window half passes (✓ This diff introduces no 'major' bump), but its level axis prints LEVEL AXIS: NOT MEASURED — no clause-② declaration was readable for this PR … no 'pull_request' payload was available to read a declaration from. That limb reads the GitHub event payload, which exists only in CI. Recorded as unmeasured rather than folded into a pass.

check:route-envelope was run explicitly, because it lands in dispatch-gates' Silent bucket even when it applies (#16828): exit 0 (17 dispatcher domains · 13 plugin route modules · 4 express-style modules audited). This change adds no response-writing module — the only res.json(...) additions in the whole diff are two reads of a fetch Response inside the dogfood fixture — so PLUGIN_ROUTE_MODULES is untouched.

⛔ No test was skipped, disabled or quarantined in this round or the previous one.

Ablation — three enforcement points, each proven able to fail

Two legs are new to this round (the F3 corners) and one re-proves the F1 gate reads the type it now constrains. Resolution path matters and is stated per leg: the analytics bridge cases import ../plugin.js — a relative, in-package specifier that resolves to src/, so dist/ is not on the path and no rebuild applies; the earlier dogfood legs (A/B/C in the previous revision, unchanged and still valid) resolve through built exports and were rebuilt with scripts/ablation-dist-preflight.mjs proving the marker reached dist/.

leg mutation on-disk proof result
F3-A — the throwing-getService corner stops denying its unusable return becomes { kind: 'absent' } anchor 1 → 0, marker 0 → 1; blob 84ebcfe9 differs from HEAD b5d20d49 REDpromise resolved "{ rows: [ { cnt: 24 } ], …(1) }" instead of rejecting
F3-B — the neither-method corner stops denying same, on the second corner anchor 1 → 0, marker 0 → 1; blob e8048555 differs from HEAD b5d20d49 RED — same assertion
F1 — the sink type stops forbidding silence one token: warn becomes warn? anchor 1 → 0, 0 → 1; blob f0d201cf differs from HEAD bf385d5a RED — the gate names read-admission.ts:129 · interface AdmissionLogger { error? warn? }

Every leg restored with git checkout HEAD -- ABSOLUTE_PATH under a trap … EXIT INT TERM, and restoration is proven by two readings, never by an exit code: the worktree blob equals the HEAD blob (b5d20d49 / bf385d5a) and git diff HEAD is empty. The script refuses outright on an empty hash or an unmoved anchor, so a no-op "ablation" cannot be reported as one. No ablation artefact remains in the tree.

⚠️ Read what the F3 legs' RED actually says: with either corner reverted, the query succeeds and returns {cnt: 24} — the ablation reproduces the reported exposure itself, which is what makes those two cases able to fail rather than merely present.


🤖 Generated with Claude Code

https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37

@github-actions github-actions Bot added size/xl documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026
@os-trump os-trump added needs:contract-review and removed documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026 — with Claude
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/plugin-security, @objectstack/service-analytics, @objectstack/spec, @objectstack/verify, touching 28 documentable anchor(s).

24 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 44c849c7d66977ed28814f15853d3e9f984275b7.

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

What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: objectName (symbol, 34 pages)
  • 3 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 216 client-bound route-ledger rows — the other 156 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 156: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 135 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 44c849c7d66977ed28814f15853d3e9f984275b7packageMentionDocs.

Which tree this was computed on

This run read content/docs from b85825d20bab8d37c3594844b6d89c467145244a — the merge of head 6d6fbf51eac22f3e16269f03369ae3f9a2a4657a into base 44c849c7d66977ed28814f15853d3e9f984275b7, 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 b85825d20bab8d37c3594844b6d89c467145244a && git checkout b85825d20bab8d37c3594844b6d89c467145244a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 44c849c7d66977ed28814f15853d3e9f984275b7 6d6fbf51eac22f3e16269f03369ae3f9a2a4657a && git checkout -B drift-repro 44c849c7d66977ed28814f15853d3e9f984275b7 && git merge --no-ff 6d6fbf51eac22f3e16269f03369ae3f9a2a4657a

node scripts/docs-audit/affected-docs.mjs --json 44c849c7d66977ed28814f15853d3e9f984275b7

⚠️ 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 44c849c7d66977ed28814f15853d3e9f984275b7 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Contributor

Contract review (claude-fable-5-1, isolated seat) — PR #16860 @ a3fa4e20c

Verdict: CHANGES REQUIRED — the design is sound and honours the card's rubric; two carrier defects stop it: CI is red on this head, and the changeset does not carry the breaking/ADR-0087 marker the repo's sibling narrowing (#16805) carries.

Governed-surface check: none. All 14 paths read; nothing under docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md, or content/docs/releases/**. Two files sit outside the PR table's six but are explained in Verification (content/docs/permissions/system-context.mdx, scripts/check-system-context-census.mjs — census-gate cascade).

Findings

F1 — blocking — CI is red on a3fa4e20c: Lint & Repo Gates fails at check:optional-error-sink. Job 102043044402, conclusion failure: packages/services/service-analytics/src/read-admission.ts:108interface AdmissionLogger { error?; warn? } "permits silence" (#9754). The gate's own fix line: make warn required, keep error?. The PR body's "121 derived families … 0 NOT-MEASURED, every family green" is therefore not a reading of this head — this family either was not in the derived union or was not run on the final diff. Every other check run is success or skipped; nothing in progress.

F2 — blocking (carrier) — the changeset declares a narrowing with no **BREAKING** marker and no ADR-0087 disposition. .changeset/analytics-inline-dataset-object-read-admission.md is 4 × minor (correct level for Clause-② yes; all four packages are published at 17.3.0) and says in bold "This change NARROWS what the analytics doors accept", but carries neither **BREAKING** nor a disposition marker, so check:adr-0087-registration sees nothing to check. The same author's sibling PR #16805 — an accept/refuse change on the same middleware — is fix(plugin-security)!: with not-required (no-migration-prescription). scripts/check-changeset-no-major.mjs header names "an accept-set narrowing" as the class the launch-window convention ships as minor with the breaking declaration as carrier. Either add the banner + disposition (not-required (no-migration-prescription) fits: no authorable key or stored shape moves) or state in the PR why a 200→403 on a published route is not a declared narrowing. ⚠️ I could not locate the written accept-set rule in AGENTS.md by grep; the basis here is the gate header plus #16805's practice — maintainer to confirm.

F3 — record — default posture is OPEN when no provider is wired; "fails closed" is true only of a wired provider. Measured: analytics-service.ts assertReadAdmittedif (!provider) return;; plugin.ts bridge → if (!svc) return true;, and trySecurityAdmission returns undefined (⇒ admit) both when ctx.getService('security') throws and when the service exposes neither canReadObject nor explain. Only a wired provider that throws or answers false denies (read-admission.ts assertObjectsReadable, both branches). The PR states this and it is consistent with the rubric's equivalence property (without plugin-security, /data has no object gate either), and the init warn fires when no security service is present at init. Recording so the ruled posture is explicit: a deployment without plugin-security keeps the pre-PR analytics behaviour. The two silent-admit corners (throwing getService; a Partial<ISecurityService> with neither method) are cheap to turn into deny-with-log and I'd recommend it, but they are not the card's hole.

F4 — record — canReadObject vs the middleware, arm for arm: verdict-equivalent for reads; two non-defect deltas. Middleware (security-plugin.ts main): isSystempublicFormGrantdelegatedAdminGate.assert → anon skip → sets (throw ⇒ deny) → delegator resolve → secMeta/unresolved deny → caps AND-gate (caller + delegator) → checkObjectPermission(op) → delegator CRUD. canReadObject (new, :4700-4760): same primitives; delegatedAdminGate is GOVERNED_OPERATIONS (writes) only, so its omission is correct; the publicFormGrant arm is absent (narrower — a public-form context is denied a read the middleware might admit; insert-oriented, immaterial); the "unresolved posture" deny is evaluated before the dangling-delegator deny, the middleware does the reverse — both deny, so the boolean is identical; only the first-firing reason differs. findallowRead mapping via OPERATION_PERMISSION_MAP holds. Claim stands.

F5 — record — every dataset door passes the gate; verified, not trusted. POST /analytics/dataset/query (rest-server.ts:10719, inline + datasetName + previewDrafts) → queryDatasetDatasetExecutor(this)query()callCtx gate (analytics-service.ts ~:900, ahead of the early return and strategy selection); the draft-preview seedRows branch is gated separately (:1215) and its resolver-failure fallback goes through query(). POST /analytics/query and /analytics/sql are dispatcher routes (runtime/dispatcher-plugin.ts:1152/1170) into query/generateSql, which share callCtx. No strategy falls back to query.cube as an object name (grep of strategies/ and dataset-executor.ts: 0 hits); an unregistered cube yields an empty admission set and fails CUBE_NOT_FOUND. queryObjects/cubeObjects is one derivation feeding both admission and read scope — as claimed.

F6 — record — Zone-1 rubric, item by item. (1) ONE gate ahead of strategy selection ✓ (F5). Equivalence asserted against /data's verdict, not a literal, on both drivers ✓ — analytics-inline-dataset-admission.dogfood.test.ts describe.each(['sqlite-wasm','memory']), bootStack(..., { databaseDriver }), expect(analytics.verdict).toBe(rest.verdict). (2) zero-declaration leg ✓ — fixture declares 0 datasets/0 dashboards and a datasetName 404 case proves refusals are not "not found". (3) walled-posture leg: honest but unit-level — the wall is measured as a compiled-SQL predicate (read-admission-gate.test.ts:263-309, organization_id bound to org_a, ≥2 occurrences for a join), not a two-organization boot; the PR says why ('posture-only' performs no isolation). Triage asked for the cross-org half to become a reading; this is the strongest reading available in open core, and the PR does not overclaim. Maintainer should decide whether that satisfies condition 3 or whether it rides with the condition-5 card. (4) negative controls ✓ — admin 3/4 identical on both routes and both drivers; member RLS-scoped 2 on both. (5) platform-admin wall disagreement escalated, not decided ✓, with a stated reason each direction is closed to a dev seat; tenant behaviour byte-identical to main. (6) fallback-to-engine answered "no" with argument ✓.

F7 — record — spec contract member. ISecurityService.canReadObject? sits beside canExport, docblock is consistent with getReadFilter's (object-half vs row-half, fail-closed, optional-with-explain-fallback); explain is non-optional (security-service.ts:506), so the fallback chain is canReadObject → explain(...).allowed === true, never "admit" — correct. Generated outputs: api-surface/*.json rows are name (kind) per export and signature hashes cover only defineX factories; no baseline in packages/spec enumerates canExport (grep: only CHANGELOG.md and liveness/permission.json evidence text), so no regen is owed for a new interface member. Type Check · source gates and Build Core are green on this head.

F8 — record — BootOptions.databaseDriver (verify/src/harness.ts:304-322): optional new key on a published payload ⇒ Clause-② widening ⇒ minor ✓; default 'sqlite-wasm' unchanged; memory driver gets config: {} rather than a filename. Fixes #16645 present; the "Part-of must not close" and "same issue" guards are green.

F9 — observation — raw-sql-object-routing.test.ts:257. not.toHaveBeenCalled()not.toMatch(/is unavailable/) is a scoped loosening: it matches the sibling case two lines up and the cause (a deliberate init warn from an unwired security service) is real, but any other new warning in that path now passes. Stronger and equally cheap: assert every warn call matches the admission-bridge message. Not blocking.

F10 — observation — test hygiene. No .skip/.only/.todo in the four new files or the edited one. Spot-checked ablation leg A: read-admission-gate.test.ts:69-77 asserts rejects.toMatchObject({ code:'PERMISSION_DENIED', status:403 }) and seen.sql/aggregate empty with admitObjectRead: () => false on both strategies — removing the gate turns it red, as claimed.

F11 — observation — single-writer ordering. #16805 (open, draft, same author) also edits packages/plugins/plugin-security/src/security-plugin.ts; the single-writer check passed on this head, so this is an ordering note from the PM's own comment on #16645, not a red.

F12 — observation — not measured here. The equivalence is against the permission verdict. /data also refuses objects the REST exposure gate (apiEnabled/apiMethods) withholds; an inline dataset over such an object is not covered by canReadObject. Outside the card's stated hole; noting so it is not mistaken for covered.

Maintainer-only merge: yes. Security-boundary change on a p0 with a new member on a published contract, an open breaking/ADR-0087 carrier question (F2), and condition 5 sitting in the decision box — the maintainer's eyes are required even though no governed path is touched.


Generated by Claude Code

os-musk commented Sep 8, 2026

Copy link
Copy Markdown
Collaborator

Pile-up patrol: this PR and #16755 collide on one file

Measured just now by the domain:engine seat's cross-PR patrol — git merge-tree --write-tree over all 20 open PR branches, each against main and pairwise (190 pairs). ⛔ Neither PR is this seat's; this is a heads-up, ⛔ not a review request and ⛔ not an ask to change anything.

vs main 0 conflicts, both branches
the one pairwise conflict, out of 190 #16860 a3fa4e20c9 × #16755 34feecba75
overlapping files between the two diffs exactly onecontent/docs/permissions/system-context.mdx

Synthetic controls in the same run: a same-path/different-content pair exits 1 (positive fired), a disjoint-path pair exits 0 (negative clean). ⇒ the reading is the instrument working, ⛔ not a false alarm.

The conflict is the repo's merge driver doing its job, not a defect. Its own message comes back in the merge output:

…side whole would delete the other side's prose with no conflict and no red gate. Text-merged instead, and it CONFLICTS. Resolve the prose by hand; the anchor…

⇒ whichever of the two lands second will meet a hand-resolvable prose conflict in that one file. Nothing is broken today and nothing is owed before either merges — it is simply cheaper to know now than to discover it at the merge queue.

Both branches are otherwise clean against main, so ⛔ no rebase or main-merge is needed for any other reason.


Generated by Claude Code

…alytics-native-sql-authz

Bring the patch round onto a fresh tree so the gate union is derived from
current main rather than a 25-commit-old snapshot.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 8, 2026
… the two silent bridge corners

Contract review CHANGES REQUIRED on PR #16860, findings F1, F3 and F9.

F1 — `AdmissionLogger` declared `error?` and `warn?`, which is a contract
that permits silence (#9754). `warn` is now REQUIRED, so every value of the
type carries a destination for a refusal report, and the fail-closed branch
reaches for it when `error` is absent instead of dropping the report.

F3 — the analytics -> `security` admission bridge collapsed three resolutions
into one. A `getService('security')` that THROWS, and a registered service
carrying neither `canReadObject` nor `explain`, both returned `undefined` and
were then read as "no security service" — admitting the query silently. Those
are wired-but-broken providers and `/data`'s middleware does not fall open in
either state, so both now DENY and report at `error`. An ABSENT service still
admits: that deployment has no object-level gate on `/data` either, so the two
doors still agree, which is the property being defended.

F9 — `raw-sql-object-routing.test.ts` now asserts that every warning in the
fixture IS the deliberate admission-bridge init report, rather than only
excluding the routing phrase.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…alytics-native-sql-authz

#16755 landed on main and reached the one file both diffs touch,
content/docs/permissions/system-context.mdx, exactly as the cross-PR pile-up
patrol predicted on this PR. The merge driver declined to defer it (MIXED — a
generated half plus hand-written prose) and text-merged it into a conflict.

Resolved by hand in the only direction that keeps both sides' prose: main's
enriched automation row is adopted whole, including its fourth anchor
`#refuseUngrantedRunLifecycleWrite` and the operator run-lifecycle sentence,
carried onto this branch's row number (this branch inserted a row upstream in
the same table, so every row below it is offset by one). The census counts are
re-derived from the merged tree in the follow-up commit, per the driver's own
instruction and the regen ordering rule — never regenerated while in MERGE
state.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…tree

The merge of #16755 is the case the census gate exists to catch and the merge
driver cannot: both sides had independently bumped the SAME declared counts
from 106 to 107, so every one of them text-merged cleanly to 107 while the
merged tree now holds 108 elevation reads — main's new
`#refuseUngrantedRunLifecycleWrite` plus this branch's `canReadObject`. Two
correct edits, one wrong sum, and no conflict marker anywhere near it.

Re-derived from the merged tree, after the merge was committed and never
during MERGE state. Seven declared counts move by one; the gate names each and
states there is no mechanical repair, so each was corrected by hand:

  check-system-context-census: OK — 108 elevation read sites in 20 packages
  across 45 files, living in 91 symbol(s); the page cites 105 symbol(s)
  against 105 required, over 129 anchors and 8 file-level citation(s)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
@os-trump os-trump removed the documentation Improvements or additions to documentation label Sep 8, 2026 — with Claude

Copy link
Copy Markdown
Contributor

Contract review (claude-fable-5-1, isolated seat) — PR #16860 @ 6d6fbf51e

Verdict: PASS — every finding the prior review (5584749018 @ a3fa4e20c) owed is closed on this head, the patch round itself is clean, and CI is green on 6d6fbf51e. Re-measured, not trusted: every gate line below is from my own run on an extracted copy of the head tree (or a pure git show/git diff read against explicit refs), and every CI reading is from get_check_runs on this head.

Governed paths touched: NO. All 15 paths of the merge-base diff (44c849c7d..6d6fbf51e) read; nothing under docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md, or content/docs/releases/**. Governed Surface Queue Guard is success on this head.

Clause-②: yes — stands as declared by the claiming seat (#16645, 5584629769): new member on a published contract (ISecurityService.canReadObject), new keys on three published payloads (AnalyticsServiceConfig.admitObjectRead, AnalyticsServicePluginOptions.admitObjectRead, BootOptions.databaseDriver), and an accept-set narrowing on a published route.

Prior findings

# status evidence on 6d6fbf51e
F1 (blocking) closed read-admission.ts:128-131: AdmissionLogger { error?(…): void; warn(message): void }warn required, error optional. Fail-closed branch: if (logger?.error) logger.error(report, cause); else logger?.warn(…). Ran check-optional-error-sink-contract.mjs on the extracted head: --self-test exit 0 (19 cases); check exit 0, census 47 sink type(s) … 32 declare it optional beside a REQUIRED warn, 1 permit silence — the 1 is a pre-existing baselined sink in a file this PR does not touch. Ablated on the scratch copy (warnwarn?): exit 1, census 2 permit silence, names read-admission.ts:129; restored byte-equal. CI Lint & Repo Gates job 102130234646 success.
F2 (blocking carrier) closed Changeset summary line fix(service-analytics)!: …, **BREAKING** banner, <!-- adr-0087: not-required (no-migration-prescription) … -->; levels unchanged (minor ×4). check-adr-0087-registration.mjs --base origin/main --head refs/review/16860 (script blob identical on both refs): exit 0, 1 declared-breaking changeset(s) … [BREAKING+bang] not-required (no-migration-prescription). check-changeset-no-major.mjs same refs: exit 0, no major bump; level axis NOT APPLICABLE locally (needs the pull_request payload) — Check Changeset jobs success on this head cover it.
F3 (record, two corners) closed plugin.ts:559-609: SecurityAdmissionResolution = usable / absent / unusable. Throwing getService('security')unusablectx.logger.error(…) + return falsePERMISSION_DENIED/403 via assertObjectsReadable. Service with neither canReadObject nor explain → same. Absent → admit, kept as negative control (deliberate; /data has no object gate there either). Logger.error is required on the kernel contract (spec/src/contracts/logger.ts), so the unguarded ctx.logger.error is sound. New admission-bridge-resolution.test.ts (6 cases): both deny cases assert {code:'PERMISSION_DENIED', status:403}, reads empty (DB never reached), and the error sink matched on the resolution message; absent-admit, canReadObject true/false and explain true/false cover the working spellings. Can they fail: yes — under either reverted corner the bridge returns true, the fake engine serves {cnt: 24}, and rejects fails (the body's ablation reading is consistent with the code). ⚠️ Not run locally (needs a workspace install); Test Core 6/6 shards success on this head, and vitest's default include collects src/__tests__/*.test.ts.
F3 (absent-provider default) accepted-as-recorded Stated in read-admission.ts docblock, plugin.ts comment, changeset and body.
F4 accepted security-plugin.ts byte-unchanged since a3fa4e20c; arm-for-arm reading stands.
F5 accepted analytics-service.ts byte-unchanged since a3fa4e20c; assertReadAdmitted still at callCtx (:901, ahead of strategy selection) and the draft-preview branch (:1221).
F6 accepted Condition 5 remains escalated on the card; no maintainer ruling as of the card's last comment (5588412885, 16:24Z). Condition 3 stays the honest unit-level reading.
F7 accepted security-service.ts byte-unchanged since a3fa4e20c.
F8 accepted verify/src/harness.ts byte-unchanged since a3fa4e20c.
F9 (observation) closed raw-sql-object-routing.test.ts:266-273: warnings toHaveLength(1), warnings[0] matches the init message No admitObjectRead configured and no "security" service registered at init (verbatim in plugin.ts:947) and does not match the routing degradation phrase. The fixture emits exactly one warn: the read-scope bridge reports the same absent-at-init state at info (plugin.ts:923-930).
F10 accepted No .skip/.only/.todo in the new test file.
F11 accepted No other open PR may claim the same single-writer path job 102144417762 success at 16:24Z; #16805 landing-order note stands.
F12 accepted-as-recorded REST exposure gate (apiEnabled/apiMethods) stated out of scope in the body.

The patch round itself

  • Scope. PR-own commits since a3fa4e20c are 08825847f and 6d6fbf51e (plus two origin/main merges, 65264f718 and c291fc7ce). Their combined touch set is exactly: read-admission.ts, plugin.ts, the new admission-bridge-resolution.test.ts, raw-sql-object-routing.test.ts, the changeset, and system-context.mdx. Every other path in a3fa4e20c..head is main's (the 28 changesets, packages/qa/dogfood/test/authz-*), none in the merge-base diff. No unexplained file.
  • The feat(runtime): give the two operator run-lifecycle verbs a door (cancel, restore-suspension) #16755 collision. git diff-tree --cc c291fc7ce names one hand-resolved file: content/docs/permissions/system-context.mdx. 6d6fbf51e moves exactly seven declared counts (107→108 at lines 12, 135, 281, 342; 113→114; 103→104; 90→91). Head-vs-origin/main on that file is the PR's own row-6 insertion and the renumber cascade, nothing else. Ran check-system-context-census.mjs on the extracted head (scratch-only git init so its git ls-files sweep works): --self-test all cases passed; check exit 0, OK — 108 elevation read sites in 20 packages across 45 files, living in 91 symbol(s); the page cites 105 symbol(s) against 105 required, over 129 anchors …, 18 row reference(s) resolve to their keyed row, 2 declared unheld. The "109 sites" prose at lines 361-362 is the page's dated, explicitly unenforced decomposition, identical on main — not a count this PR owes.
  • Condition 5 / condition 10. No source line in the diff touches organization_id, tenantId, Layer 0, buildDriverOptions or isTenancyDisabled (the only hit is the docblock quoting sql-driver.ts); strategies/, drivers, core and engine are untouched; security-plugin.ts is additive only. Tenant behaviour is byte-identical to main and the PR picks no side.
  • Trailers. All four new commits (two merges included) carry Claude-Session: and the harness-written Co-Authored-By: trailer — the exemption AGENTS.md names.

CI on 6d6fbf51e

50 check runs: every one success or skipped; none failure, none in progress. The Test Core shard artifact-upload FinalizeArtifact … 403 signature (#16928) is not present on this head — all six shards, the rollup, Dogfood Regression Gate ×3, Dogfood Verify CLI, Temporal Conformance, Build Core, Build Docs, all four Type Check jobs, Spec property liveness, Governed Surface Queue Guard, both single-writer/same-issue guards and Part-of are success. mergeable_state: clean (the body's "unknown" is stale). Head is 9 commits behind origin/main, 11 ahead (8 non-merge + 3 merges); merge-tree clean per the body, consistent with clean. Draft; labels size/xl, needs:contract-review.

New findings

  1. record — init-report asymmetry between the two bridges. For the same state (no security service registered at init, bridge resolves per query) the read-scope bridge reports at info (plugin.ts:923-930) and the admission bridge at warn (:945-952); the per-query absent → admit resolution is not reported at call time at all, only at init. Consistent with the F9 assertion and deliberate as written; recording so nobody reads the two levels as two different states. Expectation: none for this PR; align the levels if either bridge is next touched.
  2. record — the row-scope bridge's collapsed three-way. The body names it ("Noted, not filed"): a throwing getService('security') leaves trySecurity() undefined, so the query runs unscoped rather than refusing. Correctly left out of this PR (it would move tenant behaviour under condition 5), but "noted, not filed" is the wrong end state for an open fail-open corner on a security bridge. Expectation: a card, filed alongside the condition-5 decision ([finding] The InMemory driver silently ignores the engine's tenant scope for objects that OMIT a tenancy block — its guard only refuses an explicit enabled: true, so memory-driver runs show cross-organization rows a SQL driver refuses #16589 / ats#39 family), not a rider here.
  3. record — quote fidelity. The body's census quote drops the gate's trailing 2 declared unheld clause; the value is unchanged from main and the self-test pins it. Cosmetic.

Maintainer-only merge: yes

The two gates the prior review hung its "yes" on are now closed (CI green, breaking carrier present), so this is a ruling rather than a fix — but the reasons for the maintainer's eyes are unchanged and are not the kind a seat clears: a p0 security-boundary narrowing on a published route; a new member on a published contract (ISecurityService.canReadObject); a declared **BREAKING** shipped as minor under the launch-window lockstep convention, which the maintainer should confirm applies here; condition 5 (which tenant wall is the platform's) sitting undecided in the decision box, with this PR deliberately neutral on it; and the #16805 single-writer ordering on security-plugin.ts. Per the director's handoff (5584760938): on this PASS the card goes pm:awaiting-maintainer, the PR stays draft.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

4 participants