Skip to content

fix(plugin-security): the batched seed existence read's own failure reaches the author with no logger injected - #19281

Merged
huangyiirene merged 2 commits into
mainfrom
claude/issue-18570-seed-name-lookup-sink
Sep 20, 2026
Merged

huangyiirene merged 2 commits into
mainfrom
claude/issue-18570-seed-name-lookup-sink

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes #18570

Clause-②: no

What this changes — one site, the delivery only

packages/plugins/plugin-security/src/seed-name-lookup.ts reported the batched existence read's own failure through a doubly-optional logger?.warn?.(…). That call evaluates to NOTHING when the caller injected no sink: the read failed, the oracle silently degraded to the per-item read, and no human was told.

It now reports through reportThroughSink — the single derivation #18091 landed for exactly this rule, so this sixth instance of the shape (#17516 · #18023 · #18091's five sites) does not become a sixth hand-written copy.

⛔ Exactly one site. The triage seat bounded the caliber in as many words and it is not widened here:

修法是那一个站点,⛔ 不是顺手把全仓 logger?.warn?.( 都改 —— 其余站点各自的 caliber 归各自的卡。

No other logger?.warn?.( spelling is touched, in this package or anywhere else. permission-set-projection.ts, bootstrap-declared-permissions.ts and PermissionSeedOutcome are untouched — #18571 owns that surface.

Delivery only. The wording, the structured meta (object, names, rowBudget, organization) and the two named causes — unreadable and truncated — are axis-specific and stay at the call site, which is the split seed-refusal-sink.ts documents. A read that ANSWERED is as silent as it always was, on every channel.

Why the call site asks typeof rather than handing the parameter straight over: SeedLookupLogger.warn is OPTIONAL where CollisionReportSink.warn is not, so the host is handed over only once it really carries a callable warn — the same question reportThroughSink itself asks, moved one frame up so the declared types stay honest instead of being asserted past. ⛔ seed-refusal-sink.ts is not edited.

The evidence bar — a DRIVEN, differential reading, because that is how the card was graded

New suite seed-name-lookup-degradation-audible.test.ts, five cases. Case 1 is the card's own shape: an unreadable-database pass with no logger argument at all, run beside a lit control in the same run — reportCapabilityRowsUnreadable, an already-repaired #18091 site that reaches the console with no sink. A zero from the site under repair is a reading only while that control fires.

The other four are what keeps case 1 from being satisfiable by an unconditional console.warn:

case what it pins
2 the truncated cause travels the same way, rowBudget and all
3 an injected sink still gets the line with its receiver (a class-based host), and the console stays out of it
4 a read that ANSWERED is silent on every channel with no sink — the discriminating control #18023 established
5 a host whose warn is not callable reaches the author without throwing

Reverse verification — the ablation, run from the committed state

The fix was committed first, then seed-name-lookup.ts alone was restored to the merge base on disk (proved landed: reportThroughSink( count 1 → 0, blob 4d7c35fe → 83629f98), the suite re-run, and the file restored to HEAD (proved: git diff HEAD empty, on-disk blob back to 4d7c35fe).

Ablated run — 3 of 5 red, and the failure lines are the measurement:

  • case 1 failed at the site-under-repair assertion while the lit-control assertion above it passed — i.e. the pre-fix run printed the control line ALONE. That is the card's reading reproduced: ONE author-visible line, this site silent.
  • case 2 failed the same way (expected [] to have a length of 1).
  • case 5 failed with TypeError: logger?.warn is not a function thrown out of buildExistingByName — so the old spelling did not merely mute a sink-less host, it aborted the seeding pass for a host that declared warn and shipped something else. ?.() guards null/undefined, never a non-callable.
  • cases 3 and 4 stayed green pre-fix, exactly as predicted: the mute was never on the injected arm, and a healthy pass was always quiet.

One file outside the declared surface, named here with its evidence

The dispatch's declared file surface was seed-name-lookup.ts + its test. One more file had to move, and it is this card's own harness rather than a second repair:

bootstrap-declared-capabilities.test.ts → [#18091] UNREADABLE ROWS: prints with NO LOGGER INJECTED asserted expect(cap.seen).toHaveLength(1). That ONE is the measurement #18570 was filed on. With both sites now audible the same pass prints two, so the pin reads toHaveLength(2) and selects each line by CONTENT (the read fails before the pass summarises, so the #18570 line arrives first). Every content assertion the case already made is preserved, now against its own line. ⛔ No source file of that seeder is touched, and no other assertion in it moves.

Measured before editing it: across 24 open PRs and 325 changed-file rows, seed-name-lookup.ts is held by 0 and bootstrap-declared-capabilities.test.ts by 0, with a same-package firing control — PR #18319 does hold packages/plugins/plugin-security/src/suggested-audience-bindings.test.ts — so both zeros are readings, not a dead scan.

What was measured

Exit codes captured before any pipe, in every case.

  • pnpm --filter @objectstack/plugin-security test — exit 0, Test Files 115 passed (115) · Tests 2233 passed (2233). (One pre-existing case went red on the first run — the control above — and that red is the finding, repaired as described.)
  • pnpm --filter @objectstack/plugin-security typecheck — exit 0. Three legs, read off the package's own script rather than assumed: tsc --noEmit && tsc --noEmit -p tsconfig.scripts.json && pnpm check:test-typecheck. The third reports 0 file(s) / 0 error(s) / 0 pinned signature(s); this package carries no test-typecheck-debt.json.
  • pnpm --filter '@objectstack/plugin-security^...' build — exit 0 (dependency closure), and later the whole ./packages/* + ./packages/*/* closure, 72 successful, 72 total.
  • Gate set derived from the real change set, not guessed: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands ⇒ 60 commands. All 60 run, all exit 0. Reconciled with --ran: 60 derived famil(ies) accounted for — 60 run, 0 NOT-MEASURED (a DERIVED zero — all 60 recorded an exit code and none of them is 3), 0 UNRUN.
  • Three of the 60 answered exit 3 = PREREQUISITE NOT MET = NOT MEASURED on first run — check:dual-build-cjs-loads, check:i18n, check:type-check-debt — each naming a built-output prerequisite. The closure was built and all three re-run to exit 0. ⛔ None was reported as a pass on its 3.
  • pnpm lint (eslint . --no-inline-config, the repo-wide run) — exit 0. Run whole; no narrowing claimed.
  • Exported surface unchanged: src/index.ts re-exports nothing from seed-name-lookup.ts, and the built dist/index.d.ts carries SeedLookupLogger 0 times against a positive control of SecurityPlugin 22 times. ⇒ Clause-②: no still holds.

Two verdict lines worth carrying out of logs that die with the worktree:

  • check:engine-double-contract — UNRECOGNISED [engine-double-contract]: 9 construct(s) in packages, examples declare a scanned verb (delete, update, findOne) alongside engine siblings, and this gate could not read the implementation -- so they are in NEITHER the pinned population nor the ledger. 213 further construct(s) are SCOPED OUT by a stated criterion and are not counted here. This is a verdict, not a finding: it never fails a run (#9747, ruling of 2026-08-18). None of the nine is a file in this diff.
  • check-changeset-no-major --base origin/main, verbatim:
ℹ️ LEVEL AXIS: NOT APPLICABLE — this run has no `pull_request` to read a declaration from, so the clause-② axis has no input by construction rather than by omission. It is a PR-scoped reading: the RC cut (`cut-rc.yml`) and a local run reach here, and neither is a PR that could have declared.

⇒ that axis is measured on this PR by CI, off the Clause-②: no line above.

Changeset

patch on @objectstack/plugin-security. Decided by reading, not habit: the package declares no private and ships files: ["dist", "README.md", "CHANGELOG.md"], and seed-name-lookup.ts compiles into that dist — so this publishes.

Acceptance notes

  • noted, not filed — logger?.warn?.(…) does not protect a host whose warn is present but not callable; it throws. That is a property of the spelling, so every remaining site in the class carries it, and each of those sites belongs to its own card (triage's bound above). Recorded here rather than filed because the class already has an owner: reportThroughSink is the repair, and the remaining sites are already enumerated on the sibling cards. 承接者: the seat that dispatches the next site in this class.
  • noted, not filed — SeedLookupLogger still declares info?, which nothing in seed-name-lookup.ts reads. Dead declaration, not a defect, and out of scope for a Clause-② no card. 承接者: 无.

Generated by Claude Code

…lure through reportThroughSink

`seed-name-lookup.ts` reported the batched existence read's failure and its
truncation through a doubly-optional `logger?.warn?.(...)`: a caller that
injected no sink got no output at all — the read failed, the oracle switched to
the slow per-item path, and no human was told.

This is the sixth instance of that shape and it takes the ONE derivation
already landed for it rather than a sixth hand-written copy. Delivery only: the
wording, the structured meta and the two named causes are axis-specific and stay
at the call site, and a read that answered is as silent as it always was.

The sink is handed over only once it carries a callable `warn` — the same
`typeof` question `reportThroughSink` asks, moved one frame up because
`SeedLookupLogger.warn` is optional where `CollisionReportSink.warn` is not.

Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Co-authored-by: Claude <noreply@anthropic.com>
…geset

The `[#18091]` UNREADABLE ROWS control in `bootstrap-declared-capabilities.test.ts`
is the harness this repair was measured in: with no logger injected it captured
exactly ONE author-visible line while the batched existence read that failed
first stayed silent. Both sites now deliver through the shared sink, so the
control reads TWO — selected by content, because the read fails before the pass
summarises.

Claude-Session: https://claude.ai/code/session_01AhQASwqJr2Z7XfGWUdvnbF
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 15 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 e6a03e649154252740b8e3534804e7abc453fb5c → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json e6a03e649154252740b8e3534804e7abc453fb5c

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

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

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants