Repository navigation
[finding] sharded-artifacts 另两个读取器同样把 JSON 数组 as string[] 直接下发,条目类型无人校验(历史基线 / api-surface,#6751 的修法覆盖不到) #7076
Description
范围外发现,记录于 #6751 的实现过程中(PR #7075)。观察类:今天没有用户路径命中,门禁红绿不受影响,单纯记录同一文件里同一类的两处剩余缺口。
#6751 修的是 aggregateCategoryShards:它把分片的数组字段 as string[] 一转就喂给 categoryOfDefKey,非字符串条目于是死在 helper 里。修法是在遍历处加一次 typeof 校验(报文件名 + 条目下标 + 锚点)。
同一文件 packages/spec/scripts/lib/sharded-artifacts.ts 里还有两个读取器做了同样的转换、但走不到那次校验,所以 #6751 的落点覆盖不到它们:
1. readShardedKeysAtRev —— 历史基线读取
两条分支(sharded / legacy)都是:
const list = doc[field];
if (!Array.isArray(list)) return { error: `… has no "${field}" array` };
entries.push(...(list as string[]));Array.isArray 只回答「是数组」,不回答「里面是什么」。非字符串条目会原样流进基线集合。它不调用 categoryOfDefKey,所以 #6751 加的那格分支永远碰不到它。
后果不是绕过:删除门禁拿基线与当前集合做差,一个数字条目会被算成「消失的 key」并索要 tombstone —— 响亮但指错人的失败,而不是静默通过。
2. aggregateApiSurfaceShards —— 第四个分片产物
if (!Array.isArray(shard.doc.exports)) {
throw new Error(`… has no "exports" array (#5837).`);
}
surface[shard.doc.entry] = shard.doc.exports;同样只校验容器不校验条目。api-surface/ 是第四个分片产物,#6751 明确不覆盖它 —— 因为它不经 categoryOfDefKey 路由,所以那单的实测表格里也没有它。
为什么归为观察类
- 不是绕过:两处的失败模式都是「照样红,只是消息指错人」,不是静默放行。
- 触发前提与 [finding] 分片读取器 categoryOfDefKey 对「非字符串条目」抛裸 TypeError:被外层 catch 兜住,但消息不报文件名/不报条目/无 issue 锚点(三个分片产物共用) #6751 同族:得有人手工把生成物(或已合并的历史提交)里的条目编成生成器从不写出的类型。今天没有用户路径命中。
- 真实代价只有诊断质量一条,与 [finding] 分片读取器 categoryOfDefKey 对「非字符串条目」抛裸 TypeError:被外层 catch 兜住,但消息不报文件名/不报条目/无 issue 锚点(三个分片产物共用) #6751 同级 —— 只是这两处连「裸报错」都不会有,而是把坏值带下去,让下游门禁去报一个不是病因的病因。
建议修法(窄)
与 #6751 已落地的形状同族:在两处的 Array.isArray 之后各加一次逐条 typeof 校验,复用同样的消息骨架(文件名 + 字段名 + 条目下标 + #5837 锚点)。aggregateCategoryShards 里现成的 jsonTypeLabel 可直接复用。历史基线那处要注意它的返回形状是 { error } 而非 throw,消息得走同一骨架但换承载方式。
关联:#6751(同类,已修,PR #7075)、#5837(分片重做)。
Generated by Claude Code
Activity
os-zhuang commentedon Aug 10, 2026
Finding-triage ruling: Promote —
findingtopm:queue.Mechanical sibling of merged #6751: sharded-artifacts.ts:499/:523 still push JSON arrays as string[] with container-only validation, and aggregateApiSurfaceShards (:429-433) assigns exports unchecked; the jsonTypeLabel helper from #6751 is ready to reuse. Wrong-finger gate diagnostics are expensive agent-hours when corrupted artifacts do appear.
Authorization: maintainer directive (this session) —「对于issue中的findings 执行一次集中分诊，并更新issue 的状态。」Centralized pass under that directive; finding grading is normally the triage seat's single channel. Round: objectstack#4949 finding-triage, 2026-08-10.
Session: 01JaVVMrSxt7Tgi1uwEuDtH7
Generated by Claude Code
os-help commentedon Aug 10, 2026
Held to a later round by the
domain:spec-toolingseat — deliberately deferred, not overlooked, and the reason is recorded here rather than kept in my session.Session:
session_01KJATVrh6V2ysutYUJigh3B. This card is not claimed — it stayspm:queueand unassigned, and any sweep may still see it. What follows is the trap a claimer needs.Why it is not in round 1. #7122 (dist-freshness on the api-surface generators) was claimed this round, and its two landing sites both import this card's file:
packages/spec/scripts/build-api-surface.ts:51-56imports from./lib/sharded-artifactspackages/spec/scripts/build-export-origins.ts:89importswriteShardsfrom itOne of this card's three sites is
aggregateApiSurfaceShards(:429-433) — the api-surface aggregation itself, i.e. the same call graph #7122's dev is changing. Two agents in one generator family is exactly the merge race that same-file serialization exists to prevent, so this waits for #7122 to land rather than riding beside it.Trap for whoever claims it next — do not start from the anchors as written. Triage's promotion (00:10Z) cites
:499,:523and:429-433against the then-current main; #7122 will be editingbuild-api-surface.ts/build-export-origins.tsand may add a shared helper underpackages/spec/scripts/lib/. Re-derive all three anchors on the merged tree before implementing, and check whether #7122's freshness refusal has changed what reaches these readers at all. The card's own premise — that #6751's fix (4e6ca32, PR #7075) does not cover these two readers — was re-verified onorigin/main@3e8e669and stands as of this comment.Nothing here downgrades the card: its grade, labels and dispatchability are unchanged, and it goes out as soon as #7122 lands.
Generated by Claude Code
os-help commentedon Aug 10, 2026
Claim: PM loop round 2
Session:
session_01KJATVrh6V2ysutYUJigh3BBranch:
claude/issue-7076-shard-reader-entry-typesWorktree:
objectstack-issue-7076Domain:
domain:spec-toolingFile surface:
packages/spec/scripts/lib/sharded-artifacts.ts— regions:readShardedKeysAtRev(both sharded and legacy branches) andaggregateApiSurfaceShards. ⛔build-api-surface.tsandbuild-export-origins.tsare OFF this card. Stop on breach; explain in the report.Container & model: M,
mode:subagent,model: opus— the two sites have different error-carrying shapes ({ error }return vsthrow), so reusing one message skeleton across them is a judgement, not a copy-paste.Serial constraints cleared: file last touched by
4e6ca32(#6751 via PR #7075) — the sibling fix this card extends. Released early from the hold recorded above at 01:33Z, under the maintainer's directive to dispatch the whole queue.The hold is lifted, but the reason for it is not — so the replacement discipline is binding. Sibling #7122 is in flight on
build-api-surface.ts/build-export-origins.ts, both of which import this file. They are edit-disjoint by construction (that dispatch forbids #7122's dev from editingsharded-artifacts.ts), so this runs in parallel under region-level declaration rather than serialization. Four rules apply, and they replace the queue-ordering the hold was giving me — they are not a relaxation:origin/mainonce before opening the PR;Premise re-verified live on
origin/main(not relayed from triage):aggregateApiSurfaceShardsis at:415,readShardedKeysAtRevat:473, and the reusablejsonTypeLabelhelper from #6751 at:348. Triage's anchors (:429-433,:499,:523) are inner lines of those functions and will move — work from the function names, not the line numbers, especially given rule 3 above.Generated by Claude Code
os-help commentedon Aug 10, 2026
ACCEPT — PR #7191, reviewed against GitHub rather than against the report.
Verified independently: 2 files (
packages/spec/scripts/lib/sharded-artifacts.ts+80,packages/spec/scripts/sharded-artifacts.test.ts+93), nodocs/adr/**, nocontent/docs/releases/, andbuild-api-surface.ts/build-export-origins.tsuntouched — the parallel-run rules held in both directions, since sibling #7122's PR #7183 equally left this file alone. The four-rule discipline was applied as written: regions respected,origin/mainmerged once at3507e516fbefore opening, and the diff still only the two files.The card's own consequence description was corrected, and the correction strengthens it. The body records that a non-string entry in
readShardedKeysAtRevgets counted as a vanished key and demands a tombstone. Measured, that is exact for theschemaspath only. Thekeyspath never gets that far — the authorable-surface deletion gate maps base entries throughentry.replace(RETIRED_MARK, '')and dies onreplace is not a function, i.e. precisely the bare-JS-error shape #6751 removed one function up. A third consumer,compareAnchorKeys, blames the committed anchor for failing to mirror a baseline it mirrors correctly.Three consumers, three different wrong fingers, none able to name the file — that is a better argument for putting the check at the reader than the one the card made for itself, and it is recorded at the site and in the PR body rather than only in the report.
The reverse verification is the part worth singling out. Removing the fix did not produce "throws a worse error" — all three cases failed by returning the corrupt value:
That middle line is the card's premise made visible:
12345sitting in the baseline set. And #6751's own cases stayed green throughout, so this adds a verdict without moving an existing one — which is exactly what observation-class scope means and is the specific thing I asked not to be violated.The one thing I declined to assert was the one that mattered, and the dev measured it rather than assuming my silence meant "probably fine": one message skeleton does not fit both sites. The carrier had to differ (throw vs
{ error }), and so did the remedy sentence — the legacy branch reads an already-merged commit, so "regenerate rather than reconcile by hand" is advice nobody there can act on, and it was deliberately omitted rather than pasted.aggregateCategoryShardswas correctly not refactored onto the new helper (region rule 1), with pin tests asserting the shared substring across all three sites so divergence goes red.Landing:
skip-changesetapplied by the dev and read back (no bot labels existed yet, so the union write wiped nothing) — mirroring sibling4e6ca32(#6751 / PR #7075), which also shipped without one. Undrafting and queueing wait for the gate-bearing job conclusions.Generated by Claude Code
fix(spec): the two remaining shard readers name the file, entry and a…