Skip to content

Commit 4cb3b15

Browse files
fix(tooling): let the clocked-window rule see a load routed through a same-package helper (#19201)
Part of #17658 ## What was silent The clocked-window rule classifies loads by scanning comment-masked source for a **literal** specifier sitting inside an `import(...)` / `require(...)` form. A wrapper offers no such site: ```ts // the test file — the specifier is literal, but it is an ARGUMENT const load = await loadOptionalPackage('@objectstack/cloud-connection'); // one frame down, in the same package's optional-package.ts const module = await import(specifier); // a VARIABLE ``` So the file earned no `clocked` entry **and** no `moduleScope` entry — both of the conditions the diagnostic needs, unmet at once — and the gate said nothing over a probe measured at 5005ms against a 5000ms `testTimeout`. ⭐ The live instance is **not** red today: `packages/cli/src/utils/optional-package.test.ts:61` carries the module-top `import '@objectstack/cloud-connection';` that #17180 landed, and lines 55-59 are a hand-written comment describing this exact blind spot. Until this PR the file was protected by a **comment**, not by a detector. ⛔ No file under `packages/cli` is touched here — that is the measured object. ## Both candidate predicates, priced over the whole repo Each was implemented against the real scan and run over every workspace package with tests. Baseline (today's reader) reports **0** clocked findings repo-wide. | candidate | newly reported files | true positives | false positives | |:--|--:|--:|--:| | **1** — a test file names a registered specifier as a string literal anywhere, with no module-scope load of it | **418** (502 findings) | **0** | **502** | | **2** — follow ONE hop into a same-package helper and judge the load there | **0** | 0 (the one call site it reaches is already compliant) | **0** | Candidate 1's 502 findings, classified by the reading that decides each — not one of them is a load: - **383 — a type-only import clause.** `import type { RemoteTable, SchemaValidationResult } from '@objectstack/spec/contracts'` (`packages/cli/src/commands/datasource/envelope-unwrap.test.ts:76`), `import type { IObjectQLEngine } from '@objectstack/spec/contracts'` (`packages/cli/src/commands/migrate/meta.stored-flow-resolution.integration.test.ts:26`). Erased before anything resolves — this gate's own header states a type-only clause is accepted as **neither** a finding nor a compliance token. - **78 — a `vi.mock` registration.** `vi.mock('@objectstack/platform-objects/plugin', ...)` (`packages/cli/src/commands/secret/orphans.guards.test.ts:48`), `vi.mock('@objectstack/runtime', ...)` (`packages/cloud-connection/src/marketplace-install-local-corrupt-ledger.test.ts:42`). The specifier names a module to REPLACE; no cold transform of the real dependency is paid at that site at all. - **25 — a plain string.** `JSON.stringify({ name: '@objectstack/spec', version: '1.0.0' })` writing a manifest into a tempdir (`packages/cli/src/commands/doctor-unexamined-spec-tree.test.ts:341`); `require_.resolve('@objectstack/plugin-security')` (`packages/cli/src/utils/unmanaged-tables.integration.test.ts:61`) — a resolve, not a load. - **16 — a specifier quoted inside generated source.** `"import { defineStack } from '@objectstack/spec';"` as fixture file CONTENT (`packages/cli/test/i18n-extract-check-dry-run.test.ts:89`), and `expect(survivor.stdout).toContain("import * as Data from '@objectstack/spec/data'")` (`packages/cli/test/generate-agent-retired.e2e.test.ts:150`). ⇒ candidate 1 is not a reach fix, it is a different rule with a 100% false-positive rate on this tree. **Candidate 2 landed.** Candidate 2 reaches **exactly one** call site in the repo — `packages/cli/src/utils/optional-package.test.ts:139`, `loadOptionalPackage('@objectstack/cloud-connection')`, via `./optional-package.js` — and reports nothing, because line 61 pays the load at module top. That is the correct verdict: the file is compliant. Its binding detector finds a same-package loader in 13 test files (`loadOptionalPackage`, `bootStack`, `createHostImporter`, `mergeRuntimeModule`, `probeOrganizations`); none of the others hands it a registered specifier as a literal, so none is reported. ## Red first, then green **The real instance, reproduced.** Line 61 blanked **in memory only** (nothing written to `packages/cli`; `git status packages/cli` stayed clean), full gate over the real tree: | reader | clocked findings | |:--|--:| | before this change | **0** — the card's leg B, on today's tree | | after this change | **1**, verbatim below | ``` ✗ packages/cli/src/utils/optional-package.test.ts:139: `loadOptionalPackage('@objectstack/cloud-connection')` is paid inside a function body — a CLOCKED window. The literal specifier is the ARGUMENT: `loadOptionalPackage` (via `./optional-package.js`) hands it to a dynamic `import()` of a VARIABLE one frame down, so the specifier is literal HERE and nowhere else. ... import '@objectstack/cloud-connection'; ``` That remedy is the one-line change #17180 landed, and it is what the card's leg C printed when the load was spelled as a literal. **The self-test fixture, ablated.** From the committed tree, `helperRoutedLoadSites` was disabled with an early `return []`. The mutation was proved on disk (marker occurrences 0 → 1, `git hash-object` 680a0a3 → different), and restored with `git checkout HEAD -- ...` verified by hash equality **and** an empty `git diff HEAD`. ``` === --self-test with the one-hop reader disabled === EXIT=1 - a load routed one hop through a same-package helper went unseen — the literal scanner is still the only reader (#17658) - the routed finding did not name the hop it followed — an author cannot tell it from a false positive - the routed finding printed no module-top import to add — the remedy is the same one, not a new one - the one-hop reader did not report exactly the one routed load this fixture pays in a clocked window === --self-test with the reader present === EXIT=0 ``` 4 of the 7 new cases drive red under the ablation; the other 3 are the negative controls (the non-loader callee, the bare-specifier callee, the paid control), which are vacuous without the reader by construction. **The control fixture.** `packages/clocked-helper-routed-paid` is the same routed call in a file that already pays the load at module top — the remedy this gate prints, and the state the real instance is in today. It must report nothing, and does. ## Scope held - ⛔ **No population change.** The reported specifier is still one the package was already measured to resolve through `dist/`, read at the same granularity. `@objectstack/cloud-connection` is already in `KNOWN_UNALIASED_TEST_IMPORTS['@objectstack/cli']`; the gate already meant to cover it and simply could not SEE the call. The ledger is byte-identical. - ⛔ **No cold-load verb list.** `bootStack` shows up in the binding scan and is deliberately not reported: the rule still requires a dynamic load, not a boot. That widening has an open question on the thread. - ⛔ **No census.** Not run, not filed. - `SELF_TEST_BATTERY_FLOOR` stays **9**. The clocked-window battery declares **12 to 19**. ## Gates | command | verdict | |:--|:--| | `node scripts/check-test-source-alias.mjs` | exit 0 | | `node scripts/check-test-source-alias.mjs --self-test` | exit 0 | | `pnpm check:test-source-alias` | exit 0 | | `pnpm check:cross-package-test-inputs` | exit 0 — 255 self-test cases, 2036 test files judged | | `node scripts/check-self-test-wired.mjs` + `--self-test` | exit 0, exit 0 | | `node scripts/check-scripts-symbol-anchors.mjs` + `--self-test` | exit 0, exit 0 | | `node scripts/check-declaration-mirrors.mjs` + `--self-test` | exit 0, exit 0 | | `pnpm check:nul-bytes` | exit 0 — 8994 files, no raw control bytes | | all 29 families `dispatch-gates --commands` derives | 29 run, 29 exit 0, 0 NOT-MEASURED (reconciled with `--ran`) | | `pnpm lint` (repo-wide, `eslint . --no-inline-config`) | exit 0 at `a66ebc98` | Type check: the diff is one `.mjs` gate script that no `tsconfig` program compiles and no TypeScript file imports (`git grep` finds only prose references), and it carries no `.d.mts` mirror — `check-declaration-mirrors` audits that in both directions and is green. So there is no type-check lane for this diff, which is why the `dispatch-gates` typecheck-shaped family names are not silence here. Gate timing: 5.1s to 6.4s on the full scan; the projections a file needs are now computed once and handed to both readers instead of per rule. ## Acceptance notes Changeset: **none**, and `skip-changeset` is applied. Nothing already published moves — no published package's `files[]` reaches a repo-root `scripts/` path (checked across every non-private workspace manifest), and the root manifest is private with no `files[]`. Out of scope, noted and not filed: - **A residual reach gap, by design.** A specifier whose ONLY mention anywhere in a package is such a routed call never enters the package's measured set, so it is still not reported. Closing that means feeding routed specifiers into the import walk, which can add a dep to a package's ledger entry — a population change, which the header says is a different card. Stated verbatim in the header so the next reader inherits it rather than rediscovering it as a bug. Carrier: whoever takes the population question already open on #17658. --- _Generated by [Claude Code](https://claude.ai/code/session_017ef78bLdybu3AffehKkhfk)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f49be36 commit 4cb3b15

1 file changed

Lines changed: 326 additions & 11 deletions

File tree

0 commit comments

Comments
 (0)