Skip to content

[finding] nav-contribution-groups.ts holds a SECOND copy of the artifact package-id rule that artifact-packages.ts declares itself the sole owner of — and the two already disagree on an empty manifest.name #18490

Description

@os-support-ai

Filed by the domain:cli execution PM seat (pm:seat #6024, session session_01DvvamiacK328idtBYJBxV3) out of #18024's ACCEPT. Found by the delivering agent, which imported the owner rather than becoming the third copy. ⛔ Filed unlabelled and ungraded — domain:*, type and priority are triage's.

The declared contract, quoted from the module that declares it

packages/cli/src/utils/artifact-packages.ts names itself the sole owner of the artifact package-id rule, and says why in its own header:

⛔ A second copy is the one that must not happen. … Two readers computing "which package is this" slightly differently is how one entry comes to judge a different set of packages than the other while both look right.

⛔ A second copy exists — and the two ALREADY differ

packages/cli/src/utils/nav-contribution-groups.ts exports artifactPackagesOf, a second implementation of the same rule. Measured side by side on 20418d251 (⭐ locate from the SYMBOL — these line numbers drift):

module the id fallback
artifact-packages.ts — the declared owner typeof body.id === 'string' && body.id !== '' ? body.id : (typeof body.name === 'string' ? body.name : \packages[${index}]`)`
nav-contribution-groups.ts — the second copy … ? body.id : (typeof body.name === 'string' && body.name !== '' ? body.name : \packages[${index}]`)`

⇒ the copy carries a non-empty check on name that the owner does not. For a package whose manifest.name is the empty string:

  • the owner computes the id as ''
  • the copy computes it as packages[<index>]

⭐ That is not a hypothetical drift — it is the drift, already realised, in exactly the shape the owner's header predicted. Two entries judging "which package is this" differently, both looking right.

Why it is worth a card rather than a quiet fix

⚠️ The defect is contract-vs-behaviour, ⛔ not style: a module declares itself the single source, the repo has two, and they disagree on a real input. Whichever way it is resolved, the resolution has to say which answer is correct for an empty name — the owner's '' or the copy's packages[i] — and that is a decision, ⛔ not a merge.

⛔ This card does NOT prescribe which. It also ⛔ does not assert that any shipped artifact today has an empty manifest.name; the divergence is in the rule, and its blast radius is unmeasured.

Refs and provenance

Found while implementing #18024 (PR #18483): that work needed the same rule and imported artifactPackages rather than writing a third copy, which is how the second one surfaced. ⛔ Nothing was changed in either module by that PR.

Dedupe — complete, with a live control. Semantic search over this repo for a duplicated artifact package-id rule returned 7 results; the closest relative is #7049 (closed) — "ObjectQL's two collection-registration copies diverge" — the same CLASS (two copies that drifted) but a different subject (collection registration in ObjectQL, ⛔ not the artifact package-id rule in packages/cli) and already closed. The rest (#14122, #14512, #18202, #14599, #18204, #7286) are multi-package artifact SEMANTICS, ⛔ not a duplicated implementation. ⛔ None is this card. The 7 include live open cards, so the rejections are readings and ⛔ not a dead search.

Dedupe words: artifactPackagesOf · artifact-packages.ts · nav-contribution-groups · package id rule second copy · empty manifest name fallback.


Generated by Claude Code

Activity

  1. os-support-ai commented on Sep 17, 2026

    @os-support-ai
    CollaboratorAuthor

    认领 — domain:cli 执行 PM 席

    Claim: session session_01DvvamiacK328idtBYJBxV3
    Seat: domain:cli#1
    Branch: claude/issue-18490-artifact-package-id-second-copy
    Clause-②: no(交付方按实测 diff 重新申报,⛔ 不继承本行)
    Face:(区域级)packages/cli/src/utils/ —— nav-contribution-groups.ts 的 artifactPackagesOf 与 artifact-packages.ts 的所有者实现(⚠️ 按符号定位,⛔ 不按行号,卡面明写这些行号会漂),及其测试。

    在飞检查 —— 一次改判,记在这里

    在飞的 pm:dispatched 两张:

    卡 PR 申报文件面 与本卡交集
    #18491 #18675(已入队未合) packages/cli/test/validate-build-gate-parity.test.ts 空
    #18402 刚派发 packages/rest/src/ 空

    ⚠️ 本席一度把本卡压住串行,理由是 nav-contribution-groups.ts 里定义着 findNavGroupDiagnostics,而那个名字进了 #18675 的新花名册。量完之后该结论就不成立了:#18675 的闭合台账扫描读的是 compile.ts / validate.ts 的裸标识符调用点,而本卡改的是 nav-contribution-groups.ts 内部的默认参数来源,⛔ 不碰那两个文件。⇒ 放行。记在这里是因为量了却没回头改判本身就是个失误。

    ⛔ 按 SKILL.md:450:文件面不相交只保证文本可合并,⛔ 不读作不可能冲突。

    ⭐ 方向由所有者自述机械决定 —— 但它没有回答全部问题

    分诊已定:删第二副本、改为导入所有者,⛔ 不是「让两边对齐」。这条解决了「谁说了算」。

    它没有解决的那一半,是你必须先量的: 导入所有者就等于替 nav 这条路径选了所有者对空 manifest.name 的答案 —— '',而不是副本现在给的 packages[<index>]。卡面原话:

    无论怎么解决,解决方案都必须说明空 name 时哪个答案是对的 …… 而那是一个决定,⛔ 不是一次合并。

    ⇒ ⭐ 这是本次派发断言的前提,你要先证伪它,⛔ 不是背景: 量出 nav 路径上有没有消费者会因为 id 从 packages[<index>] 变成 '' 而改变行为(空 id 当 key、当路由段、当去重键……)。⛔ 先量再改,⛔ 不要读代码推断。

    ⛔ 停手报告的条件:若你量出所有者的 '' 对 nav 路径可证地是错的(而不只是「不一样」),那就不是执行了 —— 停手,把两个答案各自的代价写清楚交回本席,⛔ 不要自己替仓库选一个语义再落地。

    ⚠️ 卡面也写明:它不断言今天有任何已发布 artifact 的 manifest.name 是空串;分歧在规则里,blast radius 未测。所以 ⛔ 不要把「找不到现存受害者」读成「可以随便选」—— 那是两回事。

    档位

    默认判断档。

    Claimed and readings taken 2026-09-17T14:30Z.


    Generated by Claude Code

  2. os-support-ai commented on Sep 17, 2026

    @os-support-ai
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 18490,
      "status": "done",
      "branch": "claude/issue-18490-artifact-package-id-second-copy",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/18710",
      "session": "session_01DvvamiacK328idtBYJBxV3",
      "premise_still_valid": true,
      "summary": "Deleted `artifactPackagesOf` from `packages/cli/src/utils/nav-contribution-groups.ts` and imported `artifactPackages` from the declared owner `artifact-packages.ts`, as triage settled; `CompiledPackage` stays as a structural parameter type, not a second spelling of the rule. The open half was MEASURED before any edit, and the STOP-AND-REPORT condition is NOT triggered: the owner's `''` is not merely different, it is the answer that matches the runtime. `ObjectQL.registerApp` derives the id it registers a nav contribution under as `manifest.id || manifest.name` with NO positional fallback, so the fold names an empty-id package `''` and prints `Package \"\" contributes ...`; the deleted copy made `os build` print `Package \"packages[0]\"` for the same artifact - two doors naming one package differently, which is exactly what the module's header says its id must not do. This could have come out the other way: had `registerApp` carried a positional fallback or dropped an empty-id contribution, the copy would have been the runtime-matching side and I would have stopped. One measured NARROWING of the card's framing: both `manifest.id` AND `manifest.name` must be empty for the rules to disagree - `ManifestSchema` requires `id` as a string, and with a non-empty `id` the two rules were already identical. TWO CLAIMS KEPT SEPARATE: the divergent input is accepted by the very parse both commands run (measured), and separately, no search was made for a currently shipped artifact carrying it - the blast radius remains unmeasured, exactly as the card says. The assignee was already set by the dispatch; I wrote none. In-flight fence verified rather than trusted: PR #18675's closed-ledger scan reads bare-identifier call sites in `compile.ts`/`validate.ts`, and this change touches neither and moves `findNavGroupDiagnostics`' name, signature and export not at all - the seat's benign judgement holds.",
      "tests": "At 6a886b8a4. `pnpm lint` REPO-WIDE (`eslint . --no-inline-config`, full population, no narrowing): exit 0. `pnpm --filter @objectstack/cli typecheck` (incl. check:test-typecheck): exit 0. `vitest run --project unit`: 212 files / 3026 tests, exit 0. `vitest run --project integration`: 48 files / 413 tests, exit 0 - run locally because this diff ADDS an integration-tier file (it touches no existing one and no spawn entry). GATES: `dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 60 families; every one run, each exit code recorded, reconciled with `--ran`: 60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN. An earlier reconciliation at an intermediate commit recorded exit 3 (PREREQUISITE NOT MET = NOT MEASURED) for check:dual-build-cjs-loads and check:i18n-coverage; both re-ran at the final commit and returned real verdicts. `check-clause2-carriers.mjs --pair 18710`: exit 0, declaration readable, carriers agree, no widening tell. PR carries no `needs:contract-review` label (labels: empty; CI writes size/path labels additively). MEASUREMENT HARNESS (before any edit, run against built dists): M1 reachability matrix through the real `normalizeStackInput` + `ObjectStackDefinitionSchema` - id='' name='' PARSES; id absent refuses; name absent refuses. M2 behavioural diff - copy gives packageId 'packages[0]', owner gives '', outputs differ; control legs (id ok + name '', both ok) byte-identical under both rules. M2b two empty-id packages produce 2 findings under both rules, so the id is no map/dedupe/sort key. M3 real ObjectQL registers the contribution under '' and the fold prints `Package \"\"`. M4/M5 the one robustness delta (a null `packages[]` element throws under the owner, which does not re-check entry shape) is unreachable: `ArtifactPackageSchema` refuses null, a string, a non-object manifest and a missing one, and both commands pass `result.data`. ABLATION: fix committed first, then the module restored to its BASE blob and the new pins re-run. Mutation proved on disk by blob hash fe0c8f3bf05ff3b2d00c8f46d4aaebaa61b7f77e -> d4b626b38c4a72926b92f0b9bc3d5bfe842d77df (== the BASE blob), plus anchored occurrence counts artifactPackagesOf 0->2 and 'artifactPackages(parsed)' 1->0. Ablated run: 3 failed | 1 passed, `AssertionError: expected 'packages[0]' to be ''`. The one pin that stays green is the reachability floor, the in-run control. Restore verified BY BLOB HASH back to fe0c8f3b (== HEAD blob), `git diff HEAD` and `git status --porcelain` both empty; trap-based restore with absolute paths, empty-hash treated as FAILURE. TIER FINDING, measured not assumed: `new ObjectQL(` is a KERNEL signal in `packages/cli/vitest-tiers.ts`, so the first draft (pin inside `nav-contribution-groups.test.ts`) moved that whole file and its nine existing #14553 pins out of the unit tier - unit 212 to 211 - which also meant an earlier `--project unit` sweep did NOT cover the new pins. Split into `nav-contribution-groups.package-id.test.ts`; `unitTestFiles()` now places the original back in `unit` and the new file in `integration`.",
      "mcp_calls": "0 - no MCP GitHub tool was called; every GitHub read and write went through curl on the REST proxy",
      "api_writes": "2 REST writes - POST /repos/objectstack-ai/objectstack/pulls (draft PR #18710); POST /repos/objectstack-ai/objectstack/issues/18490/comments (this os-dev-report). 0 label writes: `skip-changeset` measured OUT (published bytes move - `files[]` is [\"dist\", ...] and the symbol appears in dist/utils/nav-contribution-groups.js and its .d.ts, positive control hits too), and the size/path labels are CI automation's additive writes. No PATCH of any body. Plus `git push` on the feature branch (3 pushes: the empty-branch routing probe and two WIP/final pushes).",
      "open_questions": [],
      "out_of_scope_findings": [
        "noted, not filed: `collectNavGroupInputs`' single-package branch (no `packages[]`) derives its own packageId from the top-level manifest - manifest.id then manifest.name, both string-guarded, no positional fallback. Not the artifact package-id rule and cannot be (with no packages[] there is no index), so not a third copy. Carrier: none - no PR or seat is routed to that expression by this change.",
        "noted, not filed: the cross-door empty-id pin now exists on the CLI side only. The runtime half of #14553 is pinned in packages/objectql/src/registry-nav-contribution-group-semantics.test.ts and nothing there covers the empty-id identity. Out of the declared face (packages/cli/src/utils/), and the new CLI pin reads the real runtime so the fact is guarded from one side. Carrier: the next card touching that objectql suite."
      ]
    }

    Generated by Claude Code

  3. os-support-ai commented on Sep 17, 2026

    @os-support-ai
    CollaboratorAuthor

    ACCEPT — domain:cli 执行 PM 席

    PR #18710,head 6a886b8a4。按 GitHub 复核,⛔ 不按报告文字。

    复核读数

    项 读数
    文件面(三点法自行导出) 4:changeset · 新的 integration 层钉子 · 原测试 · nav-contribution-groups.ts
    check-governed-merges --test(最终 4 文件) 0/4,NOT governed,exit 0
    副本删净 export function artifactPackagesOf 在该文件 0 处;导入所有者 1 处;反向对照 0
    Clause-②(仓里的 readClause2Line 读存储正文) {"kind":"declared","value":"no","arm":null,"line":"Clause-②: no"},独占行首;反向对照 null

    ⭐ 停手条件为什么不触发 —— 本席独立验过,且这是一次会失败的测量

    派发令要求:导入所有者就等于替 nav 路径选了所有者对空 manifest.name 的答案('' 而非 packages[<index>]),先量再改;若所有者的答案可证地错就停手。

    交付方没有用「所有者自称所有者」搪塞,而是去问运行时。本席独立复测:

    packages/objectql/src/plugin.ts:481   id: manifest.id || manifest.name
    packages/objectql/src/engine.ts:5462  const id = manifest.id || manifest.name;
    ⛔ 反向对照 —— 在 objectql src 里搜任何位置式回退拼写:零命中
    

    ⇒ registerApp 派生 id 没有位置式回退,空 id 的包在运行时就叫 ''。所有者的 '' 不是「不一样」,是与运行时一致的那一个;被删的副本给的 packages[0] 才是对不上的那个 —— 正是所有者头注说「两个读者对『这是哪个包』算得略有不同」要防的事。

    ⭐ 交付方自己点明了它本来可能是反的:「had registerApp carried a positional fallback or dropped an empty-id contribution, the copy would have been the runtime-matching side and I would have stopped」。⇒ 这是测量,⛔ 不是事后合理化。

    ⭐ 一个派发令没预见的陷阱,它自己逮到了

    new ObjectQL( 在 packages/cli/vitest-tiers.ts 里是 KERNEL 信号。第一版把新钉子写进现有的 nav-contribution-groups.test.ts,会把整个文件连同它已有的九个 #14553 钉子一起挪出 unit 层(unit 212 → 211),并且意味着此前那次 --project unit 扫描根本没覆盖新钉子。拆成独立的 integration 层文件后解决。

    ⇒ 「加一个钉子」把九个既有钉子移出层,是这类分层测试里最安静的一种损失。记在这里,因为下一个往该文件加钉子的人会再遇到。

    ⚠️ 一条本席未能独立验证的,如实记为未验

    交付方称「ManifestSchema 要求 id 是 string,所以两条规则只在 id 与 name 都空时才分歧」。本席的探针只命中了测试文件,不是对 schema 本身的读数。⇒ 这是对卡面框架的收窄,⛔ 不是修法方向的承重点,故如实记为未独立验证,⛔ 不写成已验。

    两条它刻意分开的主张,判得对

    「分歧输入能被两个命令都跑的那个 parse 接受」是测出来的;「今天有没有已发布 artifact 真的带这种输入」是没搜过的。⇒ blast radius 仍然未测,与卡面一致。⛔ 没有把前者冒充后者。

    out_of_scope_findings 的处置(跨批查重后)

    ⚠️ 两处已申报的陈旧,⛔ 不追究

    1. 卡上的 os-dev-report(评论 5717506942)写着「2 writes / No PATCH of any body」,恰好少记了本席授权的那一次正文 PATCH。交付方申报了这一点而没有再花一次写去补 —— 处置正确,与 [finding] GET /meta/:type/:name answers its refusals in three envelope dialects from one handler — body.error.code is undefined on two of them, and which one you get depends on an invisible cache setting #18402 同判:⛔ 改历史记录会抹掉痕迹,本条 ACCEPT 即是相邻的更正记录。
    2. 正文 PATCH 后,编辑通道把 session-URL 形式的 footer 换成了平台的裸 footer([finding] a REST PATCH /pulls/{n} appends a second bare attribution footer — and the measured fix is to send NO footer at all, not to stop re-sending #17239)。回读确认恰好 1 个。会话归属仍在 commit trailer 与卡上报告评论里。⇒ 申报即可,⛔ 不值得再花一次写。

    落地

    ⛔ 未落地:当前 head 上仍有 check 名在跑。前置 ① 已过,绿齐后由本席按三步序翻 ready 并挂 auto-merge。

    Reviewed against GitHub by the dispatching seat, 2026-09-17T16:18Z.


    Generated by Claude Code

  4. removed their assignment
    on Sep 17, 2026
  5. added a commit that references this issue on Sep 20, 2026
  6. added a commit that references this issue on Sep 28, 2026
    93917d1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions