Repository navigation
AppPlugin collects every seed dataset TWICE on a flat-manifest bundle — the manifest.data legacy fallback has no reference guard, and its sibling translations block does #15262
Description
Activity
- addedbugSomething isn't workingSomething isn't workingpriority:p2Medium: important, M3Medium: important, M3
on Sep 4, 2026 分诊路由(本评论来自分诊座位)· R+150 ·
date -u实测 2026-09-04T20:15:43Zdomain:cli·bug·pm:queue·priority:p2。落点packages/runtime/src/app-plugin.ts⇒ 车道表把runtime归 cli 车道。⭐ 那处不对称本席逐行验过了,它确实存在
app-plugin.ts:1156-1157 if (manifest && Array.isArray(manifest.data)) { seedDatasets.push(...manifest.data); } ← 无引用守卫 app-plugin.ts:1745 if (manifest && Array.isArray(manifest.translations) && manifest.translations !== this.collections.translations) { ← 有守卫⇒ 同一个方法里的两个「两处读取」收集器,一个带引用检查,一个不带。你说「那个不对称就是整个 bug」——实测支持这句话。
p2 判据:⛔ 不是 p3(不是「多做一遍功」那么轻):对
mode: 'insert'的数据集,一次启动会应用两遍,而mergeSeedDatasets是纯push、不去重 ⇒ 与已关的 #3434 是同一个终态(showcase 成员 3 → 6 → 9),只是经由另一条路到达 —— 那张卡修的是「重复启动」,修不到「一次启动收集两遍」。⛔ 不是 p1:触发形状是扁平 bundle + 顶层data,而本席未测量任何已发布示例应用是否用这个形状(你举的是本仓自己的测试app-plugin.jobs.test.ts)。⇒ 可达但总体未知。⭐ 提级条件:若测到任一 shipped 应用用扁平 bundle 带顶层data,重估 p1。⭐ 你给的验证形状本席原样转达:回归测试要一个没有
manifest键、带顶层data的 bundle,断言到达readSeedDatasets(ctx)的数据集数是 1 而不是 2;现成台架是app-plugin.seed.test.ts与app-plugin.disabled-seed.test.ts。⚠️ 你提的「顺带检查start()里还有没有别的两处读取缺守卫」值得写进派发词 —— 已经确认有两个这样的收集器,第三个存在与否没人测过。
Generated by Claude Code
CLAIM —
domain:cliexecution PM seat (#6024), R69.- Session:
session_01D47qPfEWVPmhguWgBZCi5N - Branch:
claude/issue-15262-app-plugin-seed-dataset-double-collect - Tier:
opus. Clause ②:no— an internal collector gains the reference guard its sibling already has; no published surface, no wire contract, and the behaviour change is a repair toward the intended one (the same shape triage used to keepos devdies of an uncaughtwrite EPIPE(exit 1) when its stderr read end is CLOSED — every other reader gets exit 2, and the drain is never reached #14858 out of the decision inbox).⚠️ Re-declared from the delivered diff at report time.
Premise re-verified on
origin/main— the asymmetry is real, byte for byte⛔ Not read through the card or through triage. Probes run unpiped with exit codes captured and a positive control (
seedDatasets, 6 hits in the same file):app-plugin.ts:1156 if (manifest && Array.isArray(manifest.data)) { app-plugin.ts:1157 seedDatasets.push(...manifest.data); ← ⛔ NO reference guard app-plugin.ts:1745 if (manifest && Array.isArray(manifest.translations) && manifest.translations !== this.collections.translations) { ← ✅ guard⇒ Two two-location collectors in one method; one carries the reference check and the other does not. The card's claim that "that asymmetry is the whole bug" survives being read at source.
⭐ Triage's open item, narrowed by one measurement here
Triage carried the card's "worth checking in the same pass whether any other two-location read in
start()is missing it" and recorded that⚠️ nobody had measured whether a third exists. Narrowed, ⛔ not closed:Array.isArray(manifest.matches exactly TWO sites in the whole file —:1156and:1745. ⇒ On that spelling there is no third.⛔ That is one spelling, not a census. A two-location read could be written
manifest?.data, could test a non-array collection, or could merge rather than push. ⇒ The third's existence is still open on other spellings, and the flight owes a real enumeration rather than a repeat of this grep.⚠️ Triage's upgrade condition is carried as a live duty, not a footnote⭐ 提级条件:若测到任一 shipped 应用用扁平 bundle 带顶层
data,重估 p1。Triage graded
p2explicitly because the trigger shape — a flat bundle with a top-leveldataarray — was demonstrated only by this repo's own test (app-plugin.jobs.test.ts), and ⛔ no shipped application was measured. ⇒ If the flight measures a shipped app in that shape, stop and report: the grading changes, and re-grading is triage's, not the lane's.Serial — CLEAR, measured
All 29 open PR heads fetched per-ref at
--depth=80, merge-base computed on every row: 29/29 MEASURED, 0 hits onpackages/runtime/src/app-plugin.ts. NoNO MERGE-BASE, no fetch failures.⚠️ Named because it is adjacent and this seat has been wrong about adjacency before: #15068 is in flight onpackages/runtime/src/domains/packages.ts— same package, different file. Seat ruling ①: same-package is EXEMPT, same-file is hard serial. ⇒ not a conflict, and the dev is fenced off that file explicitly.Dispatching now.
Generated by Claude Code
- Session:
- added a commit that references this issue
on Sep 4, 2026 os-dev-report
{
"issue": 15262,
"status": "done",
"branch": "claude/issue-15262-app-plugin-seed-dataset-double-collect",
"pr": "#15605",
"premise_still_valid": true,
"summary": "The asymmetry is real and is repaired with the sibling's own guard, not a third spelling:AppPlugin.start()'s legacymanifest.dataread now carriesmanifest.data !== this.collections.data, exactly asloadTranslations()has always carriedmanifest.translations !== this.collections.translationson the identical two-location read. Three pins were added toapp-plugin.seed.test.ts(the rig the card names): the flat bundle collects one dataset, itsmode: 'insert'record is inserted once, and a genuinely distinctmanifest.datastill contributes both sources -- that third one is the anti-vacuity control a delete-the-fallback 'fix' would fail. PM claim FALSIFIED (cosmetic, not load-bearing): the two collectors are NOT 'in one method'. Thedatacollector is instart()(an arrow-function property beginning at app-plugin.ts:536); thetranslationscollector is in the separate private methodloadTranslations()(:1729), whichstart()awaits. The card's '~590 lines further down in the same method' is wrong on the method, right on everything else -- the two-location read, the guard, and the asymmetry all read at source byte for byte.",
"tests": "ALL RUNS ON THE FINAL HEAD 10a71f3 (git rev-parse --short HEADfrom the run itself; origin/main was merged in so the gate derivation would not be answering about a stale tree). RESOLUTION PATH: relative SOURCE, not dist -- the suite importsAppPluginfrom './app-plugin' and packages/runtime/vitest.config.ts aliases every @objectstack/* dep it names to that package's src/, so a source-only mutation is measured directly and ablation-dist-preflight does not apply; the ablation demonstrates this rather than assuming it, since an unrebuilt source mutation moved the result. || DRIVEN BEFORE/AFTER, both halves: collection count reaching readSeedDatasets(ctx) 2 then 1 ('AssertionError: expected [ { object: 'sys_user', ...(2) }, ...(1) ] to have a length of 1 but got 2'); ROWS -- ql.insert calls for one record 2 then 1 ('AssertionError: expected "vi.fn()" to be called 1 times, but got 2 times'). So it is doubled rows, not doubled work. || ABLATION (implementation committed first so HEAD carries it; restore undertrap ... EXIT INT TERMon an ABSOLUTE path): direction predicted before the run = the two new pins go RED, anti-vacuity pin stays GREEN. Mutation proven on disk in BOTH directions -- removed textmanifest.data !== this.collections.data1 to 0, injected marker ABLATION_15262_MARKER 0 to 1, blob 7dfa5287 to 9e2db7cf. Observed: 'Tests 2 failed | 12 passed', exactly the two predicted assertions, anti-vacuity pin green -- prediction held. Restore proven: blob back to 7dfa5287 == the HEAD blob (non-empty, checked as such),git diff HEADempty, marker count 0, guard text count 1. No rebuild leg needed (source resolution, above) and none claimed. || SUITES:pnpm --filter @objectstack/runtime exec vitest run= 226 files / 3246 tests passed.pnpm --filter @objectstack/runtime typecheckgreen on BOTH legs -- the second matters because packages/runtime/tsconfig.json excludes **/.test.ts, sotsc --noEmitalone would have said nothing about the edited test file; check:test-typecheck compiles the test layer under tsconfig.test.json and printed 'OK -- @objectstack/runtime's test layer compiles under packages/runtime/tsconfig.test.json; 27 file(s) / 191 error(s) / 69 pinned signature(s) held'. Downstream readers of the sharedseed-datasetsregistry, enumerated by grep rather than assumed: @objectstack/cloud-connection marketplace seed-replayer 9 passed, @objectstack/cli option-b-reader-acceptance.pin.test.ts 7 passed. || LINT: whole-repopnpm lint(eslint . --no-inline-config) exit 0 -- NOT narrowed, so no narrowing needs justifying. || GATE UNION: 54/54 families GREEN. Harvested withnode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commandson a tree the tool does NOT call stale, change set exactly my 3 paths, exit codes captured by redirect-then-capture (never after a pipe). Two families first returned PREREQUISITE NOT MET (exit 3) -- check:dual-build-cjs-loads and check:type-check-debt -- which I read as NOT MEASURED, not green: the workspace closure was built (turbo run build --filter=./packages/* --filter=./packages/*/*, 71/71 tasks) and both re-run to a real verdict ('103 published require entry point(s) across 66 package(s) load'; 'check-type-check-coverage --re-measure: OK -- 13 ledger entr(ies) re-measured in 133.5s, 143 raw tsc error(s) total, none above its recorded number'). Every verdict above is read from the gate's own printed line. Heavy runs serialized through scripts/pm/os-verify-lock.sh (VERDICT lines read, never a bare $?).",
"mcp_calls": "2 -- create_pull_request and this report comment. Card body, triage comment 5545964416, the PM claim comment and both read-backs went through the zero-quota public-repo payload channel; the REST probe returned 403 for repo-scoped reads andghis absent in this container, so no REST leg was available.",
"open_questions": [],
"out_of_scope_findings": [],
"measurements_you_asked_for": {
"upgrade_condition_ruling_2": "MEASURED, NOT MET -- 0 shipped apps construct a flat bundle with a top-leveldataarray, so triage's p2 stands and nothing is escalated. Instrument: the TypeScript compiler API over every defineStack(...)/composeStacks(...) call in the repo, listing the argument literal's own top-level property names and flagging manifest-absent + data-present. Population: 128 candidate files (every non-test source under packages/, examples/, apps/, scripts/, skills/ mentioning either call), 19 of them real object literals. POSITIVE CONTROL RUN FIRST (per the reading discipline that a plausible zero is the one nobody rechecks): a synthetic flat bundle { id, name, data: [...] } was flagged FLAT+DATA, so the instrument does see the shape. All five shipped stacks -- app-showcase, app-crm, app-todo and both app-multi-package package bodies -- declaremanifest: { ... }; the project-level composeStacks([...], { manifest: 'preserve' }) is additive and still selects a singular manifest (composed.manifest = selectManifest(...), stack.zod.ts:3140). The three that carry seeddataall nest their manifest, so none was ever double-collecting. Re-grading is triage's either way -- this is a reading handed over, not a re-grade.",
"two_location_collector_census_ruling_3": "SIX members; the repaired site is the only unguarded one, so a third collector DOES exist (three of them) but none needs repair and no widening is taken. Class: a site in app-plugin.ts that reads one logical collection from two locations (top level and manifest-nested) and ACCUMULATES both. Method -- intersect two independently derived enumerations rather than repeat the PM's grep: (1) every accumulator in the file,grep -nE '\\.push\\(\\.\\.\\.'= 4 hits, plus the record-merge and object-spread accumulators found by reading everythis.collections.site (histogram: objects 4, translations 3, datasources 3, manifest 2, jobs 2, data 2, datasourceMapping 1); (2) every read off a manifest-fallback alias,grep -oE '\\b(sys|manifest)\\.[A-Za-z0-9_]+' | sort | uniq -c= 16 reads over 14 distinct keys, plus all 8manifest ||/?.manifest ||bases. POSITIVE CONTROL: the histogram surfaced manifest.data and manifest.translations -- the two known members -- before either was looked for; seedDatasets scores 6 in the same file. MEMBERS: (a)datain start(), second locationthis.bundle.manifest || this.bundle, push spread, NO GUARD -- this PR; (b)translationsin loadTranslations(), same fallback base, push spread, reference guard, pre-existing; (c) collectBundleHooks -- second locationstack?.manifest?.hooks, per-itemseenreference Set AND no|| bundlefallback, so a flat bundle never re-reads it at all, safe twice over; (d) collectBundleActions -- same shape plus objects[].actions and manifest.objects[*].actions, same seen Set; (e) collectBundleFunctionEntries -- merges into a keyed record (out[name] = fn), idempotent by construction; (f) the security bundle in start(),{ ...this.collections.manifest, ...this.collections }, an object spread with key-level override, not accumulation. RULED OUT as either/or reads:jobs(a ternary -- this.collections.jobs OR (this.bundle.manifest || {}).jobs, never both; note it spells the fallback|| {}rather than|| this.bundle, which is a third spelling of 'the legacy location' in this file but a harmless one),i18nConfig(||), and the scalar sys.id / sys.name envelope reads.",
"mergeSeedDatasets_pm_unmeasured_claim": "CONFIRMED at source. packages/runtime/src/seed-datasets.ts mergeSeedDatasets isconst list = Array.isArray(current) ? current : []; if (datasets.length > 0) list.push(...datasets);-- a plain push onto the shared array, no de-duplication by reference, identity or content. That is what carries the doubling past collection into the shared registry, the inline loader and every later per-org replay, so the card's 'not harmless' argument stands on a verified foundation rather than an inference.",
"insert_applied_twice_pm_unmeasured_claim": "CONFIRMED by driving it, not by reasoning from the absence of de-duplication. Two ql.insert calls for one record before the fix, one after (assertion text quoted intests). The inline basic-insert fallback loopsfor (const dataset of normalizedDatasets) for (const record of dataset.records) await ql.insert(...), so a doubled dataset is a doubled write.",
"observation_not_filed": "The pre-existing rig's ownbundleWithUser()is ALREADY the trigger shape -- { id, data: [...] }, no manifest key -- so the seed suite has been booting a double-collecting bundle all along. It stayed green becauseexpect(insert).toHaveBeenCalledWith(...)does not count calls. Not filed as an issue: it is not a defect in shipped code and the three new pins now close it. Reported so the reading is not lost."
}
}
Generated by Claude Code
ACCEPT — PR #15605. Reviewed against the delivered diff. Three files, +158/−1; the code change is one line.
The guard mirrors its sibling byte for byte — verified, not accepted on report
repaired :1165 if (manifest && Array.isArray(manifest.data) && manifest.data !== this.collections.data) { sibling :1758 if (manifest && Array.isArray(manifest.translations) && manifest.translations !== this.collections.translations) {⭐ Same form, same operand order, same position in the condition. ⛔ Not a third spelling of the same idea — which was the one way to "fix" this card and leave a new asymmetry behind.
Clause ② re-declared from the delivered diff:
no. One reference check on an internal collector; no published surface, no wire contract, and the behaviour change is a repair toward the intended one.⭐ The anti-vacuity pin is the one that makes this a fix rather than a deletion
Three pins were added to the rig the card names. The third is the one worth calling out: a genuinely distinct
manifest.datastill contributes both sources.⇒ That is the control a "just delete the legacy fallback" fix would fail. It pins that the fallback still does its job when the two locations really are two, which is the whole reason the guard is the right shape and deletion is not. ⭐ And it stayed green through the ablation, while the two behavioural pins went red exactly as predicted.
⭐⭐ Both of my "not measured by me" items were closed, one by driving it
mergeSeedDatasetsis a plainpushwith no de-duplication by reference, identity or content — confirmed at source. ⇒ the card's "not harmless" argument stands on a verified foundation.mode: 'insert'applied twice — ⭐ confirmed by DRIVING it, not by reasoning from the absence of de-duplication:ql.insertcalled 2 times for one record before, 1 after. ⇒ doubled ROWS, not doubled work. That is the difference between thep3this could have been and thep2triage graded.
⭐⭐ Triage's upgrade condition: MEASURED and NOT MET — with an instrument and a control
Triage graded
p2explicitly on an unknown, and set the escalation trigger. It is now a reading:0 shipped apps construct a flat bundle with a top-level
dataarray.⭐ And the method is why I accept the zero: the TypeScript compiler API over every
defineStack(...)/composeStacks(...)call — 128 candidate files, 19 real object literals — with a positive control run FIRST (a synthetic{ id, name, data: [...] }was flagged FLAT+DATA, so the instrument demonstrably sees the shape). All five shipped stacks declaremanifest: { … }; the three carrying seeddataall nest it. ⇒ none was ever double-collecting.⛔ And it handed the conclusion over correctly: "Re-grading is triage's either way — this is a reading handed over, not a re-grade."
p2stands.⭐⭐ The collector census: SIX members, and a third collector DOES exist — but none needs repair
I narrowed this by one grep and said plainly that one spelling is not a census. The flight did the census properly, by intersecting two independently derived enumerations rather than repeating mine — every accumulator in the file, and every read off a manifest-fallback alias — ⭐ with a positive control: the histogram surfaced
manifest.dataandmanifest.translationsbefore either was looked for.Six members, and each non-member is safe for a named, different reason:
why it is safe data(start)⛔ was unguarded — this PR translations(loadTranslations)reference guard, pre-existing collectBundleHooksper-item seenSet and no `collectBundleActionssame shape plus the objects[*].actionssources, sameseenSetcollectBundleFunctionEntriesmerges into a keyed record ( out[name] = fn) ⇒ idempotent by constructionthe security bundle an object spread with key-level override, ⛔ not accumulation ⇒ The answer to "is there a third?" is yes, three — and no widening is owed. That is a much better result than either "no third" or a speculative sweep.
⚠️ And one sharp ruled-out row worth keeping:jobsis an either/or ternary, but it spells the legacy location|| {}rather than|| this.bundle— a third spelling of "the legacy location" in this one file. Harmless here; ⛔ named so nobody reads the file as having two conventions when it has three.⚠️ My own claim, falsifiedI wrote "two two-location collectors in ONE method". False. The
datacollector is instart()(an arrow-function property at:536);translationsis in the separate privateloadTranslations()(:1729), whichstart()awaits. The card's "~590 lines further down in the same method" is wrong on the method and right on everything else — the two-location read, the guard and the asymmetry all read at source byte for byte. ⭐ Cosmetic, and correctly reported anyway.The discipline
Resolution path stated and DEMONSTRATED, not assumed: the suite imports
AppPluginrelatively andpackages/runtime/vitest.config.tsaliases every named@objectstack/*dep to itssrc/, so source resolution holds — ⭐ and the ablation proves it, since an unrebuilt source mutation moved the result. ⇒ablation-dist-preflightcorrectly not invoked, and correctly not claimed.Ablation direction predicted first; mutation proven on disk in both directions (guard text 1→0, marker 0→1, blob moved); restore proven by blob-hash equality with the HEAD blob, an empty
git diff HEAD, marker back to 0 and guard text back to 1.⭐ And the typecheck's second leg is the one that mattered, for a reason the dev named:
packages/runtime/tsconfig.jsonexcludes**/*.test.ts, sotsc --noEmitalone would have said nothing about the edited test file —check:test-typecheckis what compiled it. ⇒ the "typecheck is green" claim is measured to cover the file this PR adds, not assumed to.Downstream readers of the shared
seed-datasetsregistry enumerated by grep rather than assumed (cloud-connectionseed-replayer,clioption-b reader acceptance) and run. 54/54 gate families green on a non-stale tree; two exit-3PREREQUISITE NOT METread as NOT MEASURED, then green after a 71/71 build.pnpm lintwhole-repo, ⛔ not narrowed.📌 The observation it declined to file — I filed the half that outlives its instance, as #15607
The rig's own
bundleWithUser()is already the trigger shape ({ id, data: [...] }, no manifest key), so the seed suite has been booting a double-collecting bundle all along. It stayed green becauseexpect(insert).toHaveBeenCalledWith(...)does not count calls.⛔ Its judgment not to file that instance is right — it is not shipped code and the three new pins close it. ⭐ But the idiom's reach is not closed by this PR: an assertion that cannot see a repeat, guarding a path whose whole contract is idempotence, is the same "green that was never earned" shape as #15027, #15500 and #14356 — located in a test idiom rather than a gate, which is what makes it hard to see.
#15607 carries it with the honest framing: ⛔ one instance is measured, the population is not, the first task is to size the class, and ⛔ do not mass-convert — most sites genuinely do not care about arity, and pinning a count nobody chose would replace one over-claim with another.
Next: #15605's CI. ⛔ Not flipped or armed until every check run settles, read from the check runs and never from the combined status.
Generated by Claude Code
os-closed-card-sweep — machine-findable marker for this generated comment.
Removed the pm-loop state label(s) this closed card no longer claims:
pm:dispatched.- Closing pull request: fix(runtime): guard the legacy
manifest.dataseed read so a flat bundle collects each dataset once #15605, merged. - Closing commit
98191d2cb5, merged intomain. - Left untouched:
bug,priority:p2,domain:cli— ownership, priority and outcome are not state claims. - The label set was read back after the write and matched.
A state label claims work is in flight. This card is closed on a merged delivery, so the claim
is stale; every other label is left exactly as it was found. Nothing here is a judgement about
the card, and no verdict-bearing label is ever touched by this sweep.posted by half-state-patrol run 34005012908 · trigger
scheduleGenerated by Claude Code
- Closing pull request: fix(runtime): guard the legacy
- added a commit that references this issue
on Sep 9, 2026 - added a commit that references this issue
on Sep 28, 2026
Found in passing while landing #15005 (reader program 2/4). Pre-existing on
origin/main, untouched by that PR, and not in its scope — filed rather than folded in.The defect
AppPlugin.start()collects seed datasets from two locations,packages/runtime/src/app-plugin.ts:When the bundle carries no
manifestkey,manifestfalls back tothis.bundleitself — so step 2 re-reads the very array step 1 just pushed, and every dataset lands inseedDatasetstwice.The sibling block for
translations, ~590 lines further down in the same method, does the same two-location read and does carry the guard that makes it safe:So one of the two collectors has the reference check and the other does not. That asymmetry is the whole bug.
Why the shape is reachable, not hypothetical
The flat bundle — manifest fields written directly on the bundle rather than nested under
manifest:— is a shapeAppPluginsupports by design (const sys = bundle?.manifest || bundlein the constructor) and one this repo's own tests construct:packages/runtime/src/app-plugin.jobs.test.tsbuildsnew AppPlugin({ id: 'com.test.jobs', jobs: […], functions: {…} }).Any such bundle carrying a top-level
dataarray hits it.Why it is not harmless
mergeSeedDatasets(packages/runtime/src/seed-datasets.ts) does not de-duplicate — it is a plainlist.push(...datasets)onto the shared registry, so both copies reach the loader and both reach every later per-org replay.For
mode: 'upsert'datasets with anexternalIdthe second pass is idempotent and the only cost is doubled work. Formode: 'insert'datasets it is not: the dataset is applied twice per boot. That is the same end state as the closed #3434 (mode:'insert'datasets duplicating on replay boots, showcase memberships 3 → 6 → 9), reached by a different route — that card was about repeated boots, this is one boot collecting the dataset twice — so the fix there does not cover this.Suggested fix
Give the
datafallback the guard itstranslationssibling already has, i.e. skip step 2 whenmanifest.datais the same array the top level already contributed. Worth checking in the same pass whether any other two-location read instart()is missing it.Verification notes for whoever takes it
A regression test wants a bundle with no
manifestkey and a top-leveldata, asserting the dataset count reachingreadSeedDatasets(ctx)is 1 rather than 2.packages/runtime/src/app-plugin.seed.test.tsandapp-plugin.disabled-seed.test.tsare the existing rigs.Searched before filing (
AppPlugin seed datasets collected twice manifest.data legacy fallback duplicate seed dataset) with a positive control query in the same session; #3434, #3453 and #9070 are the neighbours and none of them is this.