Skip to content

feat(devx,datasource,automation): gate undeclared workspace imports in published src (#10062) - #12942

Merged
os-elon merged 9 commits into
mainfrom
claude/issue-10062-undeclared-dep-imports
Aug 28, 2026
Merged

feat(devx,datasource,automation): gate undeclared workspace imports in published src (#10062)#12942
os-elon merged 9 commits into
mainfrom
claude/issue-10062-undeclared-dep-imports

Conversation

@os-elon

@os-elon os-elon commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Fixes #10062

A package's published src/** was free to import any workspace package, declared or not, and nothing in the tree checked it. check:type-source-resolution and check:test-source-alias answer which copy of a dependency's types resolves, never whether the dependency is declared, and eslint.config.mjs carries no import/no-extraneous-dependencies rule.

What the sweep actually measures now

Re-measured on aef1b7e6, with the instrument's own defects fixed first. The card was filed at 1 member and released at 3; the honest count is 4 packages / 15 occurrences, and the release comment's list came from a scan that could not see dynamic imports.

Two fabrications the naive scan produces, both fixed before any conclusion was drawn:

  • comments. rest-server.ts, storage-driver.ts and turso-driver-factory.ts each carry prose that NAMES the package it is explaining. An unmasked scan reds on the authors who documented the situation.
  • string literals. packages/cli/src/commands/create.ts builds a scaffolded README inside a template literal, and that README contains a fenced code block reading import ... from '@objectstack/plugin-PLACEHOLDER'. The comment mask leaves it standing — it is a string, not prose. A match now counts only when its import / from / require keyword sits at a byte the scanner flags as code.

The gate

scripts/check-undeclared-dep-imports.mjs, wired into Lint & Repo Gates and as pnpm check:undeclared-dep-imports.

  • Import-free beyond Node builtins and the shared comment mask, so it runs with an empty node_modules — a reviewer can run it in place rather than read it.
  • Declared means dependencies + peerDependencies + optionalDependencies. A peer is a declaration to the consumer; reading peers as undeclared would red on @objectstack/cli, which did the right thing.
  • Refuses rather than passes quietly. A sweep that finds nothing because it swept nothing reports exactly what a clean tree reports, so three floors exit 2: packages discovered, non-test src files read, and — the one that cannot be seen any other way — @objectstack/* specifiers extracted. A dead matcher produces an empty finding set, and the empty set is what success looks like.
  • --self-test, 41 cases, running first in CI: every import form firing, and the four shapes that must stay silent (declared / peer / optional / self, tests and fixtures, comments, tsconfig-excluded payload). The production run over a fixed tree is green by construction and cannot tell a working matcher from a dead one.
  • packages/create-objectstack/src/templates is judged mechanically, not skipped by name: it is payload copied verbatim into a scaffolded project, and the package's own tsconfig.json excludes it. The evidence lives in the tree, so it cannot rot into a hardcoded list.

Per-member remediation, decided one at a time

Declaring makes a coupling real and installable; routing it away removes it. The two are not interchangeable, so nothing here is applied uniformly.

Fixed — the coupling was real and unguarded. @objectstack/service-datasource now declares @objectstack/driver-sql and @objectstack/driver-memory as dependencies. Both are loaded through an unguarded await import(...) on the postgres, mysql, sqlite and memory arms, so a consumer reaching one of those paths met ERR_MODULE_NOT_FOUND instead of a diagnosis. The type-only SqliteAbsentFileMode import rides along.

Fixed — the coupling was avoidable. isCodeArtifactBody and its private helper isTenantAuthored moved from @objectstack/objectql's registry into @objectstack/metadata-core, by the same criterion as the write-verb dispatch predicates and the audit governance table already sitting there: a second layer needs the answer, and the reverse import would either close a cycle or make the consumer depend on the whole data engine for one predicate. objectql re-exports isCodeArtifactBody from its original path, so its public API is unchanged, and service-automation imports it from a package it already declared.

Recorded, with mechanical evidence — the tree already carries the decision not to declare, and its reason. Seven ledger rows, shrink-only, each red the moment its evidence stops holding. optional-runtime-probe rows demand the occurrence stay a dynamic import(); a type-only row demands every occurrence stay import type. That second kind is this card's original mitigation, turned from prose into a check — it was prose that let flow-precedence.ts become a value import with nothing going red anywhere.

The measurement that closes the card, re-taken rather than quoted

Ablation on flow-precedence.ts, mutation confirmed on disk by blob hash before each build and the restore proved by an empty git diff HEAD plus an --absent dist preflight.

objectql import (defect) metadata-core import (fixed)
isCodeArtifactBody in dist/index.js 10 2
runtime module refs to @objectstack/objectql 0 0
isTenantAuthored — objectql's PRIVATE helper — inlined 8 0
runtime module refs to @objectstack/metadata-core 10 3
check:undeclared-dep-imports exit 1 exit 0

The release comment's claim holds and is sharper than recorded: all four textual @objectstack/objectql occurrences in the defective bundle are inlined JSDoc carried in from objectql's own engine.ts and registry.ts, and there is no runtime import at all. The count is 10 rather than 9 — the build has moved — and the inlining reaches objectql's SchemaRegistry internals, not just the one predicate. After the fix the bundle carries a real import { isCodeArtifactBody } from "@objectstack/metadata-core" and no copy of anyone else's code.

The type-only row's evidence was measured too, and the first phrasing of it was wrong and corrected: packages/rest/dist/index.d.ts and index.d.cts carry zero module references to @objectstack/metadata-protocol — the single textual occurrence is inside a TSDoc comment — and every from specifier in rest's published types names a package rest declares.

One thing measured that is worth a reviewer's eye

Declaring @objectstack/driver-turso as an optional peer of service-datasource was tried and reverted. It makes pnpm link the package in the workspace, which took out a live assertion: default-datasource-driver-factory.test.ts reaches the missing-package arm with no stub because the package does not resolve from here, and it fails with a notice saying exactly that. The three guarded driver arms are therefore left undeclared and ledgered. Declaring them as optional peers is still a defensible hygiene improvement; it needs that test rewritten to stub the import, which is its own change.

Verification

Union run on 2f51f9cc0 (this head, after merging main).

  • check:undeclared-dep-imports — self-test 41 cases PASS; production 78 workspace packages, 2058 non-test src files, 1805 @objectstack/* specifiers, 7 ledger rows, all evidence intact
  • tests, all passing: metadata-core 234, service-automation 1091, service-datasource 585, objectql 4253, rest 2543
  • typecheck green for all five packages
  • pnpm lint — the full repo-wide scan, no narrowing: 5362 files, 0 errors, 0 warnings
  • 50 derived gate families + both convention-triggered obligations run green, including check:self-test-wired, check:step-collectors, check:watch-hint-literal, bare-root-worklist --self-test, check:pm-dispatch-gates, check:dispatcher-error-vocabulary, check:published-files, check:override-consistency, check:type-check-coverage and check:type-check-debt --re-measure (green locally with the full closure built; the check:type-check-debt --re-measure OOMs only on CI — the gate's harness does not pin the heap ceiling, so a local run is not CI-shaped #12856 OOM did not reproduce here)
  • two gates could NOT be measured in this container and are neither green nor red: scripts/pm/check-half-states.mjs (exit 3, PREREQUISITE NOT MET — no GitHub credential) and, before the clone was deepened, check-engine-split-ratio (shallow history; green after git fetch --shallow-since)

The gate declares ROOT_DIR_WATCH_HINTS, so a change under packages/** names it in a dispatch brief instead of leaving it in the invisible-population set.

Follow-up left on the table

scripts/cross-package-test-inputs.mjs was not touched — it is held by #12834 this round. The gate needs no declaration there: it is not a test, and its --self-test reads only temp fixture trees it creates itself. #12834 stays open.


Generated by Claude Code

@github-actions

github-actions Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 4 package(s): @objectstack/metadata-core, @objectstack/objectql, @objectstack/service-automation, @objectstack/service-datasource, touching 3 documentable anchor(s). ⚠️ 3 changed file(s) yielded no anchor (packages/metadata-core/src/index.ts, packages/services/service-automation/src/flow-precedence.ts, packages/services/service-datasource/package.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

12 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/error-catalog.mdx (via sys_metadata (literal))
  • content/docs/api/index.mdx (via sys_metadata (literal))
  • content/docs/concepts/metadata-lifecycle.mdx (via sys_metadata (literal))
  • content/docs/data-modeling/drivers.mdx (via sys_metadata (literal))
  • content/docs/data-modeling/objects.mdx (via sys_metadata (literal))
  • content/docs/deployment/cli.mdx (via sys_metadata (literal))
  • content/docs/deployment/environment-variables.mdx (via sys_metadata (literal))
  • content/docs/deployment/validating-metadata.mdx (via sys_metadata (literal))
  • content/docs/kernel/services-checklist.mdx (via sys_metadata (literal))
  • content/docs/permissions/authorization.mdx (via sys_metadata (literal))
  • content/docs/permissions/permission-sets.mdx (via sys_metadata (literal))
  • content/docs/plugins/packages.mdx (via sys_metadata (literal))

3 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via sys_metadata (literal))
  • content/docs/releases/v16.mdx (via sys_metadata (literal))
  • content/docs/releases/v17.mdx (via sys_metadata (literal))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 3 changed file(s) yielded no anchor (packages/metadata-core/src/index.ts, packages/services/service-automation/src/flow-precedence.ts, packages/services/service-datasource/package.json) — pages documenting those are invisible to this run
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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; 102 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 — 18 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 f4e741bd194f72c880a99d7af2e6a4faae09d63apackageMentionDocs.

Which tree this was computed on

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

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

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

os-elon commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

Repair pushed: 2f51f9cc0 to c078354fc (main merged again at b9dd923b9). The PR body's verification section describes 2f51f9cc0; this comment carries the delta rather than an edit, so the body keeps its original footer.

What was red

Lint & Repo Gates failed on pnpm check:driver-memory-census with two problems that are one edit seen from two sides. That gate keys its ledger on which manifest field declares @objectstack/driver-memory, and this PR moved that declaration on service-datasource from devDependencies to dependencies, so the same move read as an unledgered arrival (LEDGERED) and a stale entry (LIVE).

The declaration itself is correct and stays: the memory arm is an unguarded await import(...), so it is a runtime dependency. The gate was asking for the bookkeeping, not for a withdrawal.

The fix

One row in scripts/driver-memory-census.ledger.jsonmanifests is keyed by {file, field} and carried exactly one row for that manifest, so this is a field change, not a delete plus an add. axis stays product-consumer.

The substantive half is the why, which had become false. It read "all five drivers are devDependencies here by design ... declaring them as runtime dependencies would install five databases' worth of driver". This PR falsifies that for two of the five. The replacement names the two consumers the declaration serves (both already ledgered above it: the memory arm of createDefaultDatasourceDriverFactory, and rung 3 of the sqlite step-down), states why the field moved, and records that the retired argument still holds for the three guarded arms — driver-mongodb, driver-sqlite-wasm, driver-turso — which answer an absent package with a named install command and remain declared nowhere. The #5499 freeze is untouched.

Post-fix output

$ node scripts/check-driver-memory-census.mjs --self-test   -> exit 0
$ node scripts/check-driver-memory-census.mjs               -> exit 0

@objectstack/driver-memory census: 12 module binding(s) in 12 file(s), 5 manifest
declaration(s) — 2 ruled test consumer(s) (#6664 census: 2 ruled consumers).

  ruled    packages/runtime/src/sandbox/undeclared-field-write-driver-split.integration.test.ts  (#5704 Q2 = B, maintainer 2026-08-06)
  ruled    packages/runtime/src/autonumber-seed-cross-side-parity.integration.test.ts  (#6664 A, maintainer 2026-08-08 — inherits #5704 Q2 = B)

check-driver-memory-census: OK — every declaration is ledgered, every ledger entry is
live, and every ruled file states "#6664 census: 2 ruled consumers". Nothing here
invests in the driver (#5499 freeze).

The ruled count did not move: censusMarker is derived from ruledConsumers.length, this edit touches manifests only, and RULED_CEILING stays 2. No census sentence in either ruled file needed rewriting.

Ran past where CI stopped

Lint & Repo Gates dies at its first non-zero step and the census sits at command 124 of the job's 130, so six steps had never been reached. All 130 gate commands in the lint job were extracted from the workflow and run: all exit 0, including the six after the census (check:merge-driver, check:spec-parsed-alias, check:tenant-chokepoint, check:pnpm-filter-targets, check:agent-test-spelling, check:turbo-task-graph). The 12 gate commands of the following typecheck-source-gates job are green too, check:driver-conformance among them. pnpm lint is green repo-wide on this head: 5365 files, 0 errors, 0 warnings.

No sibling census is hiding behind this one. check-driver-memory-census.mjs and driver-memory-census.ledger.json are the repo's only gate-plus-ledger pair of this kind, so the newly declared @objectstack/driver-sql has no census to answer to; the gates that do read manifests — check:override-consistency, check:published-files, check:changeset-fixed, check:driver-conformance — are all green.

Worth knowing: this census gate is one of the two families dispatch-gates reports as unreachable by construction (dead: '@objectstack/driver-memory' — never was a repo path), which is why the derived family never named it and why it was found by CI rather than before the push.


Generated by Claude Code

os-elon commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator Author

ACCEPT — armed (draft:false, then auto-merge). PM seat domain:devx @ objectstack (#6023), session session_01CPrUz21stTFhJRUirdc4yw, round R25.

Entry qualification: every check, not the required subset. 33 check runs at c078354fc — 32 success, 1 skipped (Console Pin Gate), 0 failure, 0 pending. ⚠️ The count moved 32 → 33 mid-flight as the Test Core rollup appeared, which is why this was re-read whole rather than diffed against the earlier sweep.

The repair

One row in scripts/driver-memory-census.ledger.json. The manifests list is keyed {file, field} and already carried exactly one row for that manifest, so this was a field change, not a delete-plus-add — my first assumption, and the dev confirmed it by reading the shape rather than inferring it from the gate printing two problems.

The substantive half was the why, which this PR's own change had made false. It read "all five drivers are devDependencies here by design … declaring them as runtime dependencies would install five databases' worth of driver." The replacement names the two consumers the declaration actually serves, states why the field moved (neither load sits in a try/catch), and — the part that matters — records that the retired argument still holds for the three guarded arms, which remain declared nowhere. A ledger row that deletes the reasoning it supersedes is how the next person re-derives it wrongly.

check-driver-memory-census: OK — every declaration is ledgered, every ledger entry is live,
and every ruled file states "#6664 census: 2 ruled consumers".

Both my Zone 2 assumptions were falsified, and the way they were falsified is the point

"A second census is hiding behind this one" — false, and proven rather than assumed. Lint & Repo Gates dies at its first non-zero step, and the census is command 124 of 130, so six steps had never been reached on any prior run. The dev extracted all 130 from the workflow and ran them: all exit 0, including the six that had never executed. The next job's 12 gate commands are green too. ⭐ That is the correct answer to "what is hiding behind an abort-at-first-failure job" — enumerate and run the tail, ⛔ don't reason about it. It also disposes of the driver-sql worry: no sibling census exists, and this gate-plus-ledger pair is the repo's only one of its kind.

"The #6664 census: 2 prose must move" — false, with a mechanism. censusMarker derives from ruledConsumers.length; this edit touches manifests only, so the ruled set never grew and no sentence in either ruled file needed rewriting. RULED_CEILING stays 2.

The lesson this card bought, recorded on the seat

I derived this card's gate families from the diff and still did not anticipate a gate keyed on which field declares a dependency. ⇒ When a remedy changes a manifest, enumerate the ledgers keyed on that manifest, not just the gates triggered by its path.

⚠️ And the deeper reason it was missed is now a live finding. dispatch-gates reports this family as unreachable by constructiondead: '@objectstack/driver-memory' — never was a repo path, a package-name population no path derivation can express. The tool knows and says so, and that declaration reaches nobody. Recorded as the third instance on #12956, which the #12687 dev has since sharpened into the general claim: the class is gates that cannot be path-filtered. ⚠️ That third instance is reported second-hand and is flagged on #12956 as not independently re-measured by this seat.

Also green on this head: repo-wide pnpm lint (5365 files, 0 errors, 0 warnings) and the re-derived 11-path gate family. Two items correctly reported as neither colour rather than as passes: check-test-completeness.mjs needs a CI turbo log as an argument and is not runnable standalone, and check-half-states.mjs exits 3 (PREREQUISITE NOT MET) for want of a GitHub credential.

⚠️ Carried, now 30 for 30: the arming echo reports method: MERGE while SQUASH was requested. Cosmetic.


Generated by Claude Code

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

Labels

ci/cd dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/xl tooling

Projects

None yet

2 participants