Skip to content

test(plugin-security): three read-fault readings for the seams #15598 fenced out — measurement only, no behaviour change - #15998

Draft
zhuangjianguo wants to merge 3 commits into
mainfrom
claude/issue-15840-plugin-security-read-seam-invention
Draft

test(plugin-security): three read-fault readings for the seams #15598 fenced out — measurement only, no behaviour change#15998
zhuangjianguo wants to merge 3 commits into
mainfrom
claude/issue-15840-plugin-security-read-seam-invention

Conversation

@zhuangjianguo

Copy link
Copy Markdown
Collaborator

Part of #15840

Measurement only. No source file in this repository changes — the diff is one new
test file, 403 lines. The card asked which of the three surviving seams has a declared
consumer contract to apply and which does not, and the honest answer is not one answer.

What was measured

A real ObjectQL over a real SqlDriver (better-sqlite3 :memory:), booted exactly as
engine-find-bare-array.pin.test.ts boots it. The only thing standing in is the FAULT —
one verb, for one object, made to throw or to answer an envelope. Every other call, and
every read whose answer is the subject, reaches the real engine untouched. The seam is a
Proxy over the real instance rather than a hand-built stand-in: a literal would be a
second, hand-maintained idea of the engine's call shape, which is the wrong instrument
when the engine's own answer is the thing being read.

Nine cases, and the three seams do not behave alike.

Reading 1 — normalize-managed-by.ts:53 tryFind

The only one of the three where all four conditions of AGENTS.md's read-seam invention
rule hold: a storage read in a try, a catch that logs at no level at all, a path
out returning an invented empty, and no discrimination on the error's type.

Measured: a catalog holding a row that genuinely needs healing, behind a read that cannot
answer, is byte-identical on both channels to a catalog that is already canonical —
the same { positions: 0, permissionSets: 0 } and, on a recording logger that captures
info/warn/debug/error, the same empty line list. The injection is self-proving:
the fault fired exactly 4 times (once per legacy value the pass scans for) and the row it
should have healed is still stamped system.

Consumer reading: security-plugin.ts wraps the call in try / catch and reports
[security] managed_by vocab normalization failed (non-fatal). That is the only handling
any consumer declares, and nothing reaches it — the fault is swallowed one frame
below, so the seam never throws. The returned counts are discarded by the only production
consumer; the pin asserts the resolves so this stays true by measurement.

Reading 2 — auto-org-admin-grant.ts:151 tryFind

Reports, but at debug, and the value it invents flows into a verdict.

Measured on the revoke leg, which is where the invention costs something: with a
standing grant, no qualifying membership, and the sys_user_permission_set read faulted,
reconcileOrgAdminGrant answers { action: 'noop' } — the identical value a pair that
never held a grant produces — while the grant the platform just decided to take away is
still in force. The two runs are separated only by a debug line; no warn, no error.

Measured on the installation-wide sweep: with the sys_member read faulted,
backfillOrgAdminGrants answers { scanned: 0, granted: 0, revoked: 0, skipped: 0 } and
closes with info [security] org-admin grant backfill complete — the same summary and
the same log line as an installation that genuinely has no members. The member that WAS
there goes unreconciled.

Consumer reading: security-plugin.ts discards both values and declares handling only for
a throw; the seam never throws. backfillOrgAdminGrants is the one consumer that branches
on the value, and it branches on action only. reason — the channel a read fault would
have to travel in — is read by nobody: zero hits across packages for a non-test
consumer, with res.action firing as the positive control on the same command and scope.

Reading 3 — claim-seed-ownership.ts:177 idsFrom — the card's premise is partly falsified here

The card's table records this site as "non-array answers [], with no report at all".
That is true of idsFrom read in isolation and not true of the pass it sits in.

idsFrom has no try and no catch. A read fault at readPage is never seen by it: the
throw leaves readPage and is caught by claimSeedOwnership's own per-predicate handler,
which reports at warn, names the object, and states the consequence —
claimSeedOwnership failed for probe_deal; those rows stay unowned and the next run will claim them. Measured, carrying the injected error's own message. That is a declared
disposition, in-file, and it is already the right one.

What is left at this site is the non-array arm alone, which #15598 measured as unreached
on the shipped engine. Driven anyway (the page read made to answer an envelope), it also
reports at warn — but attributes a cause that did not happen: the write refused as over-sized but the predicate matched no rows to page, said while the envelope was in fact
carrying the row.

What this PR deliberately does NOT do

⛔ It changes no seam's disposition. Two of the three open questions the card names —
whether an unreadable page should stop claimSeedOwnership's pass, and whether an
unreadable membership read should make reconcileOrgAdminGrant skip or refuse — are
undecidable from this tree: no consumer branches on the value, and none declares a
contract for the read not answering. Unlike #15598's seventh block, there is no
PermissionEvaluator.resolvePermissionSets here to measure and apply. Choosing one would
be inventing the contract, which is the failure mode this card was filed to avoid.

⛔ No gate's population changes, and no gate is widened.

⛔ No packages/spec/api-surface member, exported type or error code is touched:
git diff origin/main...HEAD -- packages/spec/api-surface/ is empty. Clause-② re-derived
from the diff: no.

The cost of the obvious repair, measured rather than argued

AGENTS.md's read-seam rule names "report the failure once" as a fix. Ablation on reading
1's site, run as a red-then-green pair: the silent catch replaced with a
logger?.warn?. and the logger threaded to it. Mutation confirmed on disk before the run
(git hash-object moved fa327193 to 1b3c244f, the silent-catch grep went 1 to 0, the
injected marker went 0 to 1); the suite went red on exactly one assertion, the
toEqual([]) on the log channel, which now saw 4 lines. Restored with
git checkout HEAD -- against an absolute path and proved byte-identical (git diff HEAD
empty, hash back to fa327193).

So "report once" is really "report four times per boot" at this site — one per legacy
value scanned, not one per pass. That is a real input to the level question AGENTS.md
raises about training people to skim, and it is the kind of thing a uniform sweep across
the three sites would not have noticed.

The pin is therefore discriminating rather than merely green: it bites precisely on the
candidate repair, and only there.

Verification

Run at 4eb354f16, after the final commit.

  • pnpm --filter @objectstack/plugin-security exec vitest run --maxWorkers=2100 test
    files, 1872 tests, all passed
    .
  • pnpm --filter @objectstack/plugin-security run typecheck — exit 0;
    check:test-typecheck reports OK ... 0 file(s) / 0 error(s). The new file is proven
    in the program with tsc --listFiles (1 hit), not assumed.
  • Gate family derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack
    (no hand-built path list): 53 families. 50 green, exit codes captured before any
    pipe. Three report PREREQUISITE NOT MET (exit 3, explicitly "not a pass and not a
    finding"): check:dual-build-cjs-loads, check:published-readme-exports,
    check:type-check-debt — all three read whole-repo dist/, and this worktree built
    only the plugin-security dependency closure. CI builds the repo and runs them.
    check:engine-double-contract, check:test-source-alias,
    check:cross-package-test-inputs, check:published-files, check:nul-bytes and
    check:type-check-coverage are among the green ones.

No changeset: nothing is published from any package by this diff (check:published-files
green), so the PR carries skip-changeset.


Generated by Claude Code

…ay when the read does not answer

Nine readings, one per arm, driven against a real ObjectQL over a real
SqlDriver. Measurement only — no source in this package changes.

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

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions github-actions Bot added the size/m label Sep 5, 2026
@zhuangjianguo zhuangjianguo added skip-changeset PR has no user-facing published change; bypasses the changeset gate and removed size/m labels Sep 5, 2026 — with Claude
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

Nothing in this diff resolved to a documentable surface (no symbol, route or SDK anchor derived from 0 changed package(s)), so this run has no opinion about the docs.

What this run could not see
  • 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 — 0 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 2da2901efa2708d8895e436867838cbeeda191cbpackageMentionDocs.

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Standing down on the red Lint & Repo Gates: not this PR's — the fourth PR today hit by the gate defect filed as #15990.

Lint & Repo Gatescheck:merge-driverscripts/check-regen-pending.mjs --self-test, job 101352667507, on the current head 4eb354f16. Byte-for-byte the same case set and the same ✗ self-test failed -- 1 failure(s) (cases and floor) summary already seen on #15980, #15996 and #15978.

Why it cannot be this PR's: this diff adds one new test file and changes no source in the repository — 403 lines, packages/plugins/plugin-security/src/read-fault-distinguishability.test.ts. It touches nothing in the regen-marker machinery.

Mechanism (measured on #15980, filed as #15990): the self-test writes a stub package.json into a temp dir under tmpdir() carrying only name and scripts, then runs the stub through ownerRunCommand = pnpm <script>. That directory declares no packageManager and has no parent manifest, so the verdict depends on an ambient pnpm resolving there. When it cannot, every stub collapses to "gate exited non-zero", read as stale — so exactly the stubs expecting a non-stale verdict go red. A 2×2 established origin/main fails identically under the same condition: the verdict is a function of the runner, not the tree.

Today's distribution, which is the sharpest evidence it is environmental — same defect, four PRs, and the re-runs disagree:

PR first run re-run
#15980 red reproduced byte-identically
#15996 red cleared
#15978 red cleared
#15998 (this) red spending it now

Same commit, same workflow, two runs, two verdicts. That is per-job launcher resolution, not any property of a diff.

Severity bound: false-RED only, never false-green — a failing launcher makes every artifact look stale, and stale is the refusing verdict.

Fix status: #16009 is open against #15990 and takes the launcher dependency out of the fixture. Nothing to port into this PR before it lands.

⛔ No test skipped, disabled or quarantined; ⛔ no empty commit; ⛔ no registry widened.


Generated by Claude Code

@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Correction to this seat's own comment above: the "re-run cleared it" rows were WRONG, and the mechanism I gave was wrong too.

1. No re-run cleared anything. I reported #15996's and #15978's re-runs as having cleared the failure. They had not. Measured just now, their Lint & Repo Gates check-runs read status=in_progress, conclusion=null — the re-run had replaced the failed check-run with an in-progress one. My filter counted only runs that were completed-and-failed, so an in-progress run fell out of the "failing" bucket and I read that absence as a pass. My own output was showing running=1 beside it and I read past it. A count of failures is not a reading about success while anything is still running.

2. The mechanism I described — "an ambient pnpm fails to resolve" — is falsified. Measured by the round on #15990: it resolves and runs. The real chain:

  • .github/actions/setup-pnpm materialises only the pinned pnpm 10.31.0 and writes no lastKnownGood.json.
  • The fixture's tmpdir pins nothing, so Corepack ignores the repo pin and resolves pnpm's latest from the registry (measured: it downloads 12.3.4).
  • pnpm 12's Rust CLI rejects -s with exit 2 — that, not a missing launcher, is what makes every stub look like "gate exited non-zero" ⇒ stale.
  • Corepack then writes lastKnownGood.json, and it is sticky: the warm re-run costs ~116 ms with no network, matching the failing job's cadence, and the job log shows a Corepack cache hit with no download line.
  • actions/cache@v6 saves COREPACK_HOME back on a miss, so a job restores a store holding either a -s-accepting pnpm (green) or a pnpm 12 (red).

That last point is what my "flaky per job" story could not explain and this one does: flaky ACROSS jobs, byte-identical WITHIN one.

3. origin/main is red on its own push build, which settles the question properly and without re-run archaeology: run 33981169123 (Lint & Type Check, event=push, head_branch=main, head_sha=f7db8f4fd) concluded failure at 17:51Z with the byte-identical signature. main and the four PRs I called green were sampled before that crossover, not exonerated by their trees.

The standing-down conclusion is unchanged and is now better supported: this failure is not this PR's, it can only false-RED, and #16009 takes the launcher dependency out of the fixture. What changes is that my evidence for it was partly wrong, and the corrected evidence is stronger.


Generated by Claude Code

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

Labels

size/m skip-changeset PR has no user-facing published change; bypasses the changeset gate tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants