Skip to content

test(service-storage): resolve @objectstack/core from source so a stale dist can't decide a pin (#7668) - #7778

Merged
huangyiirene merged 1 commit into
mainfrom
claude/issue-7668-storage-test-build-order
Aug 11, 2026
Merged

test(service-storage): resolve @objectstack/core from source so a stale dist can't decide a pin (#7668)#7778
huangyiirene merged 1 commit into
mainfrom
claude/issue-7668-storage-test-build-order

Conversation

@huangyiirene

Copy link
Copy Markdown
Collaborator

Fixes#7668

Root cause — declared, not inherited from the issue

The issue's LEAD pointed at "build ordering / CI"; the PM brief guessed turbo.json's task dependencies. I reproduced both paths and neither is the defect.turbo.json already declares:

"test": { "dependsOn": ["^build"], ... }

and pnpm turbo run test --filter=@objectstack/service-storage builds core first and passes 352/352 (24 files) on origin/main @ 7a8476f — measured, not assumed. turbo.json is untouched by this PR.

The actual root cause is one directory down:

packages/services/service-storage has no vitest.config.ts, so @objectstack/core resolves through the workspace link to packages/core/dist/index.js — a build artifact. The verdict of every unit pin in the package is therefore a function of build state, not of the source in the checkout.

That is what #7668 reports. attachment-access-hooks.test.ts is the only executable guard on the #4757 predicate-less unscoped-multi-delete refusal (it cannot be expressed over REST — deleteMany with no ids/where is rejected 400 before the hook is reached), and against a prebuilt tree whose core dist predated the export it errored TypeError: withoutOperationPrivateKeys is not a function — while packages/core/src/security/operation-private-keys.ts:117 was correct the whole time.

The loud error is the mild half. A core dist merely behind rather than missing the symbol lets a pin run green against core's old behaviour — a passing test that is not testing the code in the checkout, with nothing in the output saying so.

Why ordering cannot fix this. Turbo already orders correctly, so turbo run test was never the failing path. The paths that broke are the ones turbo does not mediate: pnpm test inside the package, vitest run <file>, an editor runner, or a QA agent in a tree built at an older commit (which is how #7635 found it). Those are exactly the paths a pin is re-run on while someone is changing core — i.e. when it most needs to be telling the truth. No dependsOn edit reaches them. Taking the artifact out of the resolution path does.

The fix

A new packages/services/service-storage/vitest.config.ts aliases @objectstack/corepackages/core/src/index.ts, matching what service-knowledge, plugin-audit, runtime, metadata, driver-memory, driver-sql, knowledge-memory, knowledge-ragflow, plugin-dev and plugin-hono-server already do.

  • Anchored regex, array form — the object form matches by prefix and would swallow @objectstack/core/logger into core/src/index.ts/logger (ENOTDIR). Same shape and reasoning as service-knowledge's config.
  • Aliasing is graph-wide, so the deps still loaded from dist (spec, observability, platform-objects, objectql) resolve to this same single core instance rather than a second copy; the shared tsup.config.ts externalizes workspace deps, so none of them inline one.
  • Scoped to @objectstack/core deliberately — it is the package that owns the pin's subject symbol and the one named in the failure. Aliasing the other four as well would widen the dual-instance surface for no defect on the table.

No product code and no test assertions changed. Diff is 2 new files (config + changeset).

Reproduction and verification

All run on this branch, worktree at 7a8476f.

1. Baseline repro — the suite cannot load without a built core (clean pnpm install, no dists):

$ cd packages/services/service-storage && npx vitest run src/attachment-access-hooks.test.ts
Error: Failed to resolve entry for package "@objectstack/core".
❯ src/attachment-access-hooks.ts:3:1 import { withoutOperationPrivateKeys } from '@objectstack/core';

2. turbo.json exoneratedpnpm turbo run test --filter=@objectstack/service-storageTest Files 24 passed, Tests 352 passed, before any change.

3. The decisive A/B — the exact #7668 condition simulated by stripping the export from the builtpackages/core/dist/index.js (source untouched), then running the same file twice:

core distvitest.config.tsresult
stale (export stripped)present (this PR)30/30 pass
stale (export stripped)absent (pre-PR state)17 failed / 13 passed

The 17 failures reproduce the issue's count exactly, with its verbatim message:

Caused by: TypeError: withoutOperationPrivateKeys is not a function

So the config is demonstrably what closes the hole, and #4757 now has a pin that a build artifact cannot silence.

4. Regression sweep (core dist restored):

  • npx vitest run in the package (the previously-broken un-mediated path) → 24 files / 352 tests passed
  • pnpm turbo run test --filter=@objectstack/service-storage352 passed
  • npx eslint packages/services/service-storage/vitest.config.ts --no-inline-config → clean
  • pnpm check:published-files → ✓ (the gate classifies vitest.config.ts as test-harness config that must not ship; this package's files whitelist already excludes it)
  • pnpm check:empty-changeset → ✓

turbo.json was not modified, so the hot-file conflict risk the brief flagged does not apply; git merge origin/main on this branch was already up to date at push time.

Out-of-scope findings — reported, not fixed

  • The same hazard shape exists wherever a package with unit tests imports @objectstack/core and ships no vitest config. This PR fixes the one package the issue names; a repo-wide sweep (or a lint gate asserting the invariant) is a separate change and should be its own issue rather than a rider here.
  • service-storage's remaining runtime imports (@objectstack/spec/*, observability, platform-objects/*, objectql, types) still resolve to dist, so the suite continues to require a build for those. That is correct today — turbo orders it — and no observed defect argues for widening the alias set now.

Known-unrelated CI red

check:platform-checklistcoverage.json · qa: UNCLASSIFIED is the pre-existing #7347 failure on base, not from this change.


Generated by Claude Code

…tale dist can't decide a pin (#7668)
`packages/services/service-storage` had no `vitest.config.ts`, so its unit
suite resolved `@objectstack/core` through the workspace link to
`packages/core/dist/index.js` — a build artifact. The verdict of every unit
pin in the package was a function of build state, not of the source in the
checkout.
All 17 cases of `attachment-access-hooks.test.ts` — the only executable guard
on the #4757 predicate-less unscoped-multi-delete refusal, which cannot be
expressed over REST — errored with `TypeError: withoutOperationPrivateKeys is
not a function` against a tree whose prebuilt core predated that export, while
`packages/core/src/security/operation-private-keys.ts` was correct throughout.
The loud error is the mild half: a core dist merely BEHIND rather than missing
the symbol lets a pin run green against core's old behaviour, with nothing in
the output saying so.
This is not a task-ordering bug. `turbo.json` already declares `test`
dependsOn `^build` and `turbo run test --filter=@objectstack/service-storage`
passes 352/352; it needed no change. The paths that broke are the ones turbo
does not mediate — `pnpm test` in the package, `vitest run <file>`, an editor
runner, a QA tree built at an older commit — which is where a pin is re-run
while someone is changing core. Ordering cannot fix that; taking the artifact
out of the resolution path can.
Verified by simulating the exact #7668 condition (core's built `index.js`
stripped of the export): without the config 17/30 cases fail with the issue's
verbatim TypeError; with it, 30/30 pass. Full suite 352/352 green both via
`turbo run test` and via a bare `vitest run` in the package.
Fixes#7668
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UqnHVpBA1ij5Jb87JaMXyM
@vercel

vercelBot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 11, 2026 2:32pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-storage.

3 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/api/plugin-endpoints.mdx(via @objectstack/service-storage)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/service-storage)
  • content/docs/plugins/packages.mdx(via @objectstack/service-storage)

1 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/service-storage)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 31510769498 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/3) — 失败步骤: Run this shard's tests

    �[41m�[1m FAIL �[22m�[49m src/data/api-methods-batch-conformance.test.ts�[2m > �[22mapiMethods conformance — single-record writes imply batch (#3026)�[2m > �[22mgrants bulk wherever it grants create /
    

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 20 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 31512979457 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/3) — 失败步骤: Run this shard's tests

    �[41m�[1m FAIL �[22m�[49m src/data/api-methods-batch-conformance.test.ts�[2m > �[22mapiMethods conformance — single-record writes imply batch (#3026)�[2m > �[22mgrants bulk wherever it grants create /
    

历史信号:

  • ⚠️本 PR 过去 24h 已在队列失败 1 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 36 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 31513732396 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/3) — 失败步骤: Run this shard's tests

    �[41m�[1m FAIL �[22m�[49m src/data/api-methods-batch-conformance.test.ts�[2m > �[22mapiMethods conformance — single-record writes imply batch (#3026)�[2m > �[22mgrants bulk wherever it grants create /
    

历史信号:

  • ⚠️本 PR 过去 24h 已在队列失败 2 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 41 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 31514484197 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/3) — 失败步骤: Run this shard's tests

    �[41m�[1m FAIL �[22m�[49m src/data/api-methods-batch-conformance.test.ts�[2m > �[22mapiMethods conformance — single-record writes imply batch (#3026)�[2m > �[22mgrants bulk wherever it grants create /
    

历史信号:

  • ⚠️本 PR 过去 24h 已在队列失败 3 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 45 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 31515154824 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/3) — 失败步骤: Run this shard's tests

    �[41m�[1m FAIL �[22m�[49m src/data/api-methods-batch-conformance.test.ts�[2m > �[22mapiMethods conformance — single-record writes imply batch (#3026)�[2m > �[22mgrants bulk wherever it grants create /
    

历史信号:

  • ⚠️本 PR 过去 24h 已在队列失败 4 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 49 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Aug 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 31516019973 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Test Core (1/3) — 失败步骤: Run this shard's tests

    �[41m�[1m FAIL �[22m�[49m src/data/api-methods-batch-conformance.test.ts�[2m > �[22mapiMethods conformance — single-record writes imply batch (#3026)�[2m > �[22mgrants bulk wherever it grants create /
    

历史信号:

  • ⚠️本 PR 过去 24h 已在队列失败 5 次(不含本次)。 内容未变而反复失败 ⇒ 高度怀疑 flaky 测试或与同组 PR 的语义冲突,重排不解决。
  • 过去 24h 队列共有 55 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 在其他 PR 的同类评论里搜同名测试;出现过 ⇒ flaky 实锤,开 issue 修/隔离那条测试。修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

@huangyiirene
huangyiirene added this pull request to the merge queueAug 11, 2026
Merged via the queue into main with commit 93be029Aug 11, 2026
26 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-7668-storage-test-build-order branch August 11, 2026 19:49
hotlong pushed a commit that referenced this pull request Aug 12, 2026
action-execution-destructive.test.ts reads the REAL sys_* identity
declarations to prove today's platform objects are excluded before
actionLooksDestructive ever runs on them. That import resolved through
`exports` to platform-objects/dist -- a build artifact -- so all 66 pins
were a verdict about build state rather than about the declarations in
the checkout. `pnpm check:test-source-alias` (#7668/#7778) reported it as
a NEW unaliased artifact import on @objectstack/runtime.
Aliases platform-objects to source in packages/runtime/vitest.config.ts.
resolve.alias becomes the ARRAY form because only that form accepts a
RegExp find; the pre-existing string entries keep the prefix-match
semantics they had as object keys (Vite normalizes an alias object into
exactly this list, in this order), so no other resolution changes.
The new entries are ANCHORED, one rule for every namespace rather than an
enumeration of the ones reached today -- the PR #7778 constraint, same
shape as @objectstack/spec in packages/qa/downstream-contract (PR #8129).
`/plugin` is listed ahead of the namespace rule because it is the one
exported subpath that is a FILE (src/plugin.ts) and not a directory.
The registry entry in scripts/check-test-source-alias.mjs is untouched.
Measured, both directions:
- artifact-resolved (before): 66 passed
- source-resolved (after): 66 passed
- per-test diff of the two verbose runs: IDENTICAL, name for name.
The 14-action pins read `type`/`ai.exposed` off the imported objects
through actionByName(), which throws when an action is missing, so
an identical name+verdict set means source and dist agree on every
declaration these pins touch. No pin changed verdict; none modified.
Reverse verification (the alias is live, not decorative): with
sys_user.ban_user's `type` flipped 'api' -> 'script' in SOURCE only and
no rebuild, the suite reports 1 failed / 65 passed --
`expected 'script' to be 'api'` at :309. dist/identity/index.mjs:81 still
carries `type: "api"`, i.e. the identical tree read green through the
pre-alias config. Injection reverted; no test was weakened.
Part of #7828
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
os-zhuang pushed a commit that referenced this pull request Aug 13, 2026
…fordance sweep (#7934)
`check:test-source-alias` went red on this branch: adding `@objectstack/lint`
as a devDependency made it a NEW unaliased artifact import for this package.
Unaliased, `validateManagedApiMethods` resolved through `exports` to
`lint/dist`, so the sweep was a verdict about build state rather than about
the rule in the checkout — acutely wrong here, since the sweep's whole purpose
is to run the CURRENT rule over the CURRENT objects, and a stale rule narrows
the population silently while the sweep keeps reporting zero findings.
This package had no `vitest.config.*` at all, so the fix is the package's
first one. It carries the alias and nothing else: no `test` block, so suite
discovery stays on the vitest defaults it ran on before (17 files / 351 tests,
unchanged) and this file's only effect is the resolution.
Array form with an anchored `/^@objectstack\/lint$/`, which is load-bearing
rather than stylistic: `@objectstack/lint` exports a second subpath
(`./runtime`), and the object form matches by PREFIX, so a bare key with a
FILE replacement would swallow it and resolve to `…/src/index.ts/runtime`
(ENOTDIR at run time). Same shape as `packages/rest` (#7955) and
`service-storage` (#7778).
The registry in `scripts/check-test-source-alias.mjs` is untouched — it is
shrink-only, and its own message says widening it is not the fix.
Verified: sweep census unchanged at 76 files / 51 in-scope / 9 packages, zero
findings, now measured against lint's source.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012WMpuAfA2KSdDjGF6tm1bH
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/steststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

attach-requires-parent-edit c3: #4757 multi-delete pin can't execute against the prebuilt @objectstack/core (stale-dist / CI)

2 participants

@huangyiirene@claude