Repository navigation
finding: GET /api/v1/meta/skill lists a skill twice after a runtime PUT, and the meta PUT never reaches the prompt bridge (two divergent skill-read paths) #7654
Description
Activity
Triage — dual-state resolution:
pm:queue+domain:metadatakept;findingremoved (queue+finding is an illegal pair).- Why queue, despite the "unconfirmed" hedge: unlike a bare observation, this card has a 4-step repro and BOTH mechanisms located to file/line. Verified on
origin/main@5823d59:packages/mcp/src/mcp-server-runtime.ts:753readsmetadataService.list('skill')verbatim (registry rows, no overrides), while the HTTP list goes throughgetMetaItemsinpackages/metadata-protocol(the overlay-merging path — its store-outage test header confirms the overlay-read role). Two read paths for one truth is a concrete divergence defect, not a design question: expected behavior (one row per name, override merged over package, PUT visible to the bridge) follows from the existing overlay semantics. - Routing kept:
domain:metadata— the defining fix (dedup/merge by name) lands in the metadata read/list layer; thepackages/mcphalf is pointinglistSkillsat the merged read, a consumer-side one-liner riding the same PR. The filer's flag-if-preferred note stands; dispatch should name both files. - Dedup: no open card on skill-row duplication or the bridge/override divergence (adjacent finding: ?preview=draft self-inflicts an invalid diagnostic — the injected _draft key is validated, so the read carries _diagnostics.valid:false #7656 is a different defect — draft-preview diagnostics).
target:v17: no — admin-facing runtime-override surface, not one of the four blocking classes; flag for re-judgment if skills become a headline v17 surface.
本评论来自分诊座位 Routine(#5474 试点),不构成认领。
Generated by Claude Code
- Why queue, despite the "unconfirmed" hedge: unlike a bare observation, this card has a 4-step repro and BOTH mechanisms located to file/line. Verified on
huangyiirene commented
on Aug 11, 2026 CollaboratorAuthorMore actionsDeferred at batch selection (round 1,
domain:metadataseat) — recording the traps rather than silently pushing it to a later round. Stayspm:queue, unassigned.Why deferred: the dedup/merge half lands in
packages/metadata-protocol(getMetaItems), andprotocol.tsis serialized — PR #7814 (#7779) holds it and is in the landing window, with #7819 → #7134 → #4716 queued behind it in this seat's hot-file order. This card enters that queue rather than jumping it.Two things worth having on the record before anyone takes it up:
⚠️ The card names two symptoms, and they may not be one bug. The double listing (override row + package row not deduped by name) and the prompt bridge missing the{active:true}flip are attributed here to a single cause — two surfaces reading different sources. That reading is plausible and unconfirmed by the filer's own admission. Whoever implements should establish whether fixing thegetMetaItemsmerge alone changes the bridge's answer at all; if not, this is two cards, and the bridge half (packages/mcp/src/mcp-server-runtime.ts:753,listSkillsreadingmetadataService.list('skill')) belongs todomain:cliunder the package table, not here.- The card's own framing invites the real question, and it is a design one, not a repair: what is the single source of truth for skill rows that both the meta HTTP list and the MCP prompt bridge read from? Answering "dedup the list" without answering that leaves the two paths divergent and merely hides one symptom. If the dev reaches that fork, the correct output is a
needs_decisiondesign exit, not a guess — this seat treats that as a successful run, not a failed one.
Cross-lane note: the routing comment already invites a flag if the bridge-side lane is preferred. This seat is not re-routing it unilaterally; that is the triage seat's call and the observation above is offered as input to it.
Generated by Claude Code
huangyiirene commented
on Aug 12, 2026 CollaboratorAuthorMore actionsHeld this round — collides with an active claim on #6504, not deprioritised. Recording the collision now so it is not re-derived.
This card's bridge-side half wants to change which source
listSkillsreads, located here aspackages/mcp/src/mcp-server-runtime.ts:753—listSkills: async () => (await metadataService.list('skill')) ?? [].#6504 is claimed (assignee
os-zhuang) and names that same expression as one of its two measured consumers — its body quotesagent_prompt's sibling skill bridge,(await metadataService.list('skill')) ?? [], as a consumer of the known-partiallist()answer. #6504 proposes adding alistDiagnosedcounterpart toIMetadataServiceand sweeping consumers.So both cards rewrite the same call, for different reasons: #7654 wants it to read a source that reflects store overrides; #6504 wants it to receive a degraded-verdict-carrying answer. Dispatching this concurrently would put two agents on one line with two incompatible intents — the failure mode where each side is individually green and the merged result is wrong.
Sequencing: this card waits for #6504's shape to land or for its claim to be released. Whoever picks this up afterwards should re-read #6504's outcome first — if
listDiagnosedlands, the bridge-side half of this card may need to be expressed against the new method rather than the old one.Note the split-lane caveat already in the body: the fix straddles
packages/metadata-protocol(dedup/merge of skill rows) andpackages/mcp(which source the bridge reads). The metadata-protocol half — duplicate rows onGET /api/v1/meta/skillnot being deduped by name in thegetMetaItemsmerge — does not collide with #6504 and could be split into its own card if the bridge half stays blocked for long. Flagging rather than doing that split now, since splitting a QA-sourced finding before either half is root-caused tends to produce two cards that each assume the other's context.Also unchanged from the body: this is an unconfirmed observation — both mechanisms were located, neither independently root-caused. Whoever takes it root-causes first.
Generated by Claude Code
Claim: PM loop round 1
Session:session_012WMpuAfA2KSdDjGF6tm1bH
Branch:claude/issue-7654-skill-read-path-dedup
Worktree:objectstack-issue-7654
Domain:domain:metadata
File surface:packages/metadata-protocol/src/protocol.ts— region: thegetMetaItemsread merge/dedup path only. ⛔ NOT the error/catch message-construction paths, ⛔ NOT the audit-write sites. ⛔ NOTpackages/mcp— that isdomain:cli's lane and is out of scope (see below). (stop on breach; explain in the report)
Container & model:M,mode:subagent,model: opus
Serial constraints cleared: second concurrent editor onprotocol.tsunder the region exemption (cap 3). The other is #8136 (client-facing message construction in error/catch paths — comment5275484126). The two regions are genuinely disjoint: a read-path merge versus catch-path message text. The exemption's four disciplines bind: region-level declaration (above), mergemainbefore opening the PR, merge again after the sibling lands, and let the merge queue arbitrate — ⛔ no manual ordering. #7748 and #8184 remain held behind #8136 because their regions are adjacent to its catch/refusal paths; this one is not.Lane split, decided at claim time. This card straddles two lanes by its own admission. Only the metadata half is claimed here: the duplicate-row / un-merged-override defect in the metadata read layer. The bridge half — that
listSkillsreadsmetadataService.list('skill')inpackages/mcp— belongs todomain:cli, and ⛔ this seat does not cross into it. It gets filed for that lane rather than absorbed.
Generated by Claude Code
- added a commit that references this issue
on Aug 13, 2026 { "issue": 7654, "status": "done", "branch": "claude/issue-7654-skill-read-path-dedup", "pr": "https://github.com/objectstack-ai/objectstack/pull/8332", "premise_still_valid": true, "summary": "Symptom 1 confirmed on current main and fixed; the metadata half only, packages/mcp untouched. The dispatch's two candidate shapes were both falsified: skill is neither un-deduped nor merely registered differently. getMetaItems ran TWO merge implementations that disagreed about what a package-less row means — mergePackageAwareOverlay resolves per (slot, package) and lets a package-less row stand in for each package's row of that name (as getMetaItem(name, packageId=P) does), while the MetadataService merge below hand-rolled a Map on (package, name) with strict equality. Which merge decides a type depends on where its baseline comes from: email_template comes from the SchemaRegistry (first merge, correct), while skill/agent/tool arrive via the MetadataService's own loaders (second merge, broken). A runtime PUT writes package_id IS NULL; with an empty registry listing the overlay merge has no base row for provenance, leaves the body unstamped, its key misses the package-bearing baseline, and both rows ship. Fix deletes the second implementation and calls the first — one hunk, inside the declared getMetaItems read-merge region. Not a skill special case: agent duplicates identically and the mirrored attribution (package-less baseline under a package-bearing row) duplicates too; both are pinned. REGION FENCE HELD: exactly one hunk, no error/catch message construction (#8136's region), no audit-write sites (#7748's). DISPATCH PREMISE FALSIFIED: the brief said the killed predecessor 'produced nothing', verified via empty ls-remote. Its worktree and commit af62c4e3e survived locally — ls-remote proves nothing was PUSHED, not that no work exists. I treated the commit as a lead, re-derived the root cause from origin/main independently, then judged and verified it myself; nothing was discarded and nothing lost.", "tests": "metadata-protocol 78 files/1126 tests passed; rest 109/1809 passed; objectql 196/3451 passed; runtime 150/2306 passed (all exit=0, run after the final main merge). REVERSE VERIFICATION, direction predicted before running: reverting the hunk turned the new suite red 5 failed/17 passed with the card's exact shape — 'formula-helper|(none)|true' AND 'formula-helper|com.acme.showcase|false', two rows disagreeing on active — while protocol.i18n-bundle-list-merge.test.ts stayed green, as predicted. GREEN FOR THE RIGHT REASON, proved not asserted: a control passing in both states proves nothing, so reachability was measured by injecting 'items = []' immediately after the new merge call — 3 of the i18n suite's tests went red, proving it executes the replaced branch. Fix restored from commit via git checkout HEAD -- path, git status clean (byte-identical). Gates re-derived against actual changed paths, not the dispatch list: check:nul-bytes, check:cross-package-test-inputs, check:durability-log-level, check:filter-alias-parity, check:changeset-gate-self-tests, check:objectui-changeset, check:type-check-coverage, check:type-check-debt, check:query-options-erasure, check-changeset-no-major — all PASS. Re-derivation surfaced four changeset-triggered families the dispatch list did not name. Control-byte self-scan clean. NOTE ON A CORRECTED READING: I first reported check:type-check-debt red (@objectstack/rest +1); on the PM's instruction I rebuilt both dependency closures on fresh main and re-measured — it is GREEN ('none above its recorded number'). The +1 was a stale-sibling-dist artifact of a fresh worktree, not a main-is-red signal. Nothing about it is claimed in the PR body.", "open_questions": [ { "question": "check:objectui-pin-fresh still reports STALE after fresh main + rebuilt closures — routing it to you as instructed, not filing. Command: `pnpm check:objectui-pin-fresh` (exit=1). Output: '• objectui `main` (objectui@f56541826eb6) is not the pinned commit — frontend changes exist that no @objectstack/console changeset covers. How many, and which, could not be itemized' / '⚠ objectui checkout at /home/user/objectui does not contain f56541826eb6 — the pending list above is the endpoint-diff LOWER BOUND. Refresh it: git -C /home/user/objectui fetch --all' / '::error::objectui pin is stale — refresh .objectui-sha before releasing (#3340)'. My diff touches 0 files under .objectui-sha or packages/console; it matched only via the .changeset path. Note this gate runs in release.yml and objectui-pin-freshness.yml, NOT lint.yml — so lint.yml being green on main does not cover it, and the two readings may not be in conflict. The gate's own warning that my local objectui checkout is stale means my reading may still be partly an environment artifact.", "options": [ "A: queue steward refreshes .objectui-sha via scripts/bump-objectui.sh — the gate's own prescribed remedy", "B: confirm against a CI run of release.yml / objectui-pin-freshness.yml on main first, since my local objectui checkout is admittedly stale and may be producing a false STALE" ], "recommendation": "B first, then A if it holds. My local reading is explicitly self-flagged as a lower bound off a stale checkout, and I have just been burned once this round by trusting a local gate reading over a rebuilt/fresh one — the type-check-debt +1 evaporated under exactly that treatment. Confirm from CI before spending anyone's dispatch on it." } ], "out_of_scope_findings": [ "filed as #8328: the MCP prompt bridge's listSkills reads metadataService.list('skill') (un-merged registry listing) so a runtime meta PUT never reaches prompts — symptom 2, unassigned, no domain:* label, Blocked-by: #7654, backlinked. Measured and recorded there: this PR does NOT change the bridge's answer, because the bridge never calls getMetaItems — settling the question the triage comment asked. Also cross-linked to #6504, which is open, assigned, and rewrites the same expression for a different reason (listDiagnosed), so whoever takes #8328 must read #6504's outcome first." ] }
Generated by Claude Code
ACCEPT — PR #8332. Reviewed against the diff and
origin/main, ⛔ not against the report's self-account.check result CI every job completed: success, read per-job;Console Pin Gate/Build Docsskippedby path filterRegion fence held — exactly one hunk in protocol.ts, inside the declaredgetMetaItemsread-merge region. ⛔ No error/catch message construction (#8136's region), no audit-write sites (#7748's)Cross-lane held — packages/mcpuntouched; symptom 2 filed as #8328, already routeddomain:cliby triagePath fork clean of ADR / skills ⇒ merge queue Closure Part of #7654— correct; the card stays open for symptom 2Changeset present, patch, carries the mechanism⭐ The finding is that both of my candidate shapes were wrong
I dispatched this with two candidates — "add a dedup" or "register
skilllike every other type" — and asked which. It is neither, and the difference decides the fix.getMetaItemsran two merge implementations that disagreed about what a package-less row means:mergePackageAwareOverlayresolves per(slot, package)and lets a package-less row stand in for each package's row of that name — the same resolutiongetMetaItem(name, packageId=P)performs — while the MetadataService merge one layer below hand-rolled aMapon(package, name)with strict equality, giving it a slot of its own. Which merge decides a type depends on where its baseline comes from:email_templatevia the SchemaRegistry (correct),skill/agent/toolvia the MetadataService's own loaders (broken).⇒ The fix deletes the second implementation and calls the first, so the two steps can no longer disagree. That is a smaller repo, not a bigger one.
It is proved a class fix, not a
skillpatch —agentduplicates identically, and the mirrored attribution (package-less baseline under a package-bearing row) duplicates too; both pinned. "Registerskilllike the others" would have left both broken.⭐ Generalisation I am carrying: when two surfaces disagree, check whether there are two implementations of one idea before assuming one is misconfigured.
Green for the right reason — measured, not asserted
The control suite (
protocol.i18n-bundle-list-merge.test.ts) stays green, and a control that passes in both states proves nothing. So its reachability was measured directly: injectingitems = []immediately after the new merge call turns 3 of its tests red, proving it genuinely executes the branch this PR replaced.⇒ That is now this lane's standard for "the control stayed green": prove the control can see your change. ⛔ A green control you have not shown to be reachable is not evidence.
Preservation is pinned on the parts that could have silently reversed: the overlay still wins over the MetadataService baseline (the guard the hand-rolled loop existed for — per-org dashboard/view overlays vanishing from list endpoints on refresh), ADR-0048 keeps two packages shipping one name as two rows, and a package-less override now reaches both their slots.
⭐ Two judgement calls that kept junk out of the backlog
- It retracted its own red gate reading. It first reported
check:type-check-debtred (@objectstack/rest+1) — with its own diff stripped, which correctly ruled out its changes. On instruction it rebuilt both dependency closures on freshmainand re-measured: green. The+1was a stale-sibling-distartifact of a fresh worktree. ⛔ Nothing about it is claimed in the PR body, which is the right outcome — a false claim aboutmainsitting in a merged PR is worse than silence. - It declined to file the second gate (
check:objectui-pin-freshSTALE) and routed it to me with the caveat that its ownobjectuicheckout was stale. It was right: CI'sConsole Pin Freshnessis green on three PRs across 35 minutes, and this container's/home/user/objectuilast fetched at 03:05 and does not contain the pinned commit. ⇒ no card owed, and two spurious filings avoided.
What stays open, and why this card does not close
Part of, deliberately. Symptom 2 — the MCP prompt bridge'slistSkillsreadingmetadataService.list('skill')— is #8328,domain:cli,Blocked-bythis. The dev settled the question triage had asked before treating them as one bug: this fix does not change the bridge's answer, because the bridge never callsgetMetaItems; it callsmetadataService.listone layer below where any overlay merging happens. Two symptoms, one description, different causes.⚠️ #8328 also carries a collision the PM had not caught: #6504 is open, assigned to a different session, and rewrites the samelistSkillsexpression for a different reason (listDiagnosed). Two cards changing one call with different intents is the shape where each is individually green and the merge is wrong. That cross-link is on #8328 and whoever takes it reads #6504's outcome first.On merge I will drop
pm:dispatched, re-grade this card, and record delivered / remaining / owner in the same action. ⛔ It is not closed as completed — its residue is real and tracked.Marking ready and enqueuing; landing confirmed by a commit on
origin/main.
Generated by Claude Code
- It retracted its own red gate reading. It first reported
LANDED —
d5031f6a1onorigin/main(PR #8332). Confirmed by two readings: the merge event and the commit. ⛔ Not by theauto_mergefield.⭐ Worth noting where it landed: directly on top of
1b2eb1b55—fix(objectql)!: engine.find/findOne refuse a dotted projection instead of widening to every field (#8327), a breaking change to the very call this fix's read path makes. The merge queue re-tested against it and it passed, so the interaction is measured, not assumed.Closure action for a
Part ofPRDelivered — symptom 1, and it closed a class rather than a case
GET /api/v1/meta/<type>now serves one row per name after a runtime PUT. The root cause was neither of the two shapes this card was dispatched with:getMetaItemsran two merge implementations that disagreed about what a package-less row means —mergePackageAwareOverlayletting it stand in for each package's row (asgetMetaItem(name, packageId=P)resolves), versus a hand-rolledMapon(package, name)with strict equality giving it a slot of its own. The fix deletes the second and calls the first, so the two steps can no longer disagree.Pinned as a class fix, not a
skillpatch:agentduplicated identically, and the mirrored attribution duplicated too; both are covered. ADR-0048 still keeps two packages shipping one name as two rows, and a package-less override now reaches both slots.Remaining — symptom 2, and it is NOT this lane's
PUT /meta/skill/<name> {active:true}still does not reach the MCP prompt bridge. That is #8328 (domain:cli), and the dev settled the question triage had asked rather than leaving it open: this fix does not change the bridge's answer, because the bridge never callsgetMetaItems— it callsmetadataService.list('skill')one layer below where any overlay merging happens. Two symptoms, one description, different causes.⚠️ #8328 also carries a collision worth repeating: #6504 is open, assigned to a different session, and rewrites the samelistSkillsexpression for a different reason (listDiagnosed). Whoever takes #8328 reads #6504's outcome first — two cards changing one call with different intents is the shape where each is green alone and the merge is wrong.Owner
Symptom 1: this seat, delivered. Symptom 2:
domain:clivia #8328, now unblocked — itsBlocked-by: #7654is satisfied by this merge.Grading — closed as completed, and why that is not a claim that both symptoms are fixed
I said in the ACCEPT that I would not close this card. I am closing it, and the reason is a fact that only became true at merge: #8328 declares
Blocked-by: #7654. Leaving this open would (a) park a card indomain:metadata's dispatchable pool whose entire residue belongs to another lane and which this seat therefore cannot dispatch, and (b) create a circular block — this card waiting on #8328 while #8328 waits on it. Circularity is worse than either grade.⇒ Closed, with the residue named and owned above. ⛔ This is not an assertion that symptom 2 is fixed — it is not, and #8328 is the live card for it.
pm:dispatcheddropped in the same action.
Generated by Claude Code
- added a commit that references this issue
on Aug 17, 2026
Unconfirmed observation from the QA run — recorded for triage; the two mechanisms below were located but not independently root-caused, and the natural next step is to decide the single source of truth for skill rows (merge/dedup by name) that both the meta HTTP list and the MCP prompt bridge read from.
Symptom
Two related divergences in how
skillmetadata is read after a runtime store override:GET /api/v1/meta/skillreturns the same skill twice after a runtimePUT— the store-override row and the package row are both listed, not merged/deduped by name.PUT /meta/skill/<name> {active:true}→ 200 does not reach the prompt bridge. Flipping a skill active via the runtime metadata API does not make it appear over MCP prompts.{active:true}PUT reflected in the prompt bridge.Root cause (as located, unconfirmed)
The two surfaces read from different sources:
listSkillscallsmetadataService.list('skill')— registry/package rows (packages/mcp/src/mcp-server-runtime.ts:753,listSkills: async () => (await metadataService.list('skill')) ?? []).protocol.getMetaItems— which merges store overrides.So the store override (the
{active:true}PUT) lands on thegetMetaItemspath but never on themetadataService.listpath the bridge consumes; and on the HTTP list path the override and package rows are not deduped by name, producing the double listing. Both mechanisms are present onorigin/main(mcp-server-runtime.ts:753; the dedup gap is in thegetMetaItemsmerge inpackages/metadata-protocol).Note: the fix straddles
packages/metadata-protocol(dedup/merge of skill rows) andpackages/mcp(which sourcelistSkillsreads). Routeddomain:metadatabecause the defining symptom — duplicate rows and un-merged overrides — lives in the metadata read/list layer; flag if the maintainer prefers the bridge-side lane.Reproduction
skill.PUT /api/v1/meta/skill/<name>with{active:true}→ 200.GET /api/v1/meta/skill→ the skill appears twice (override row + package row).{active:true}flip is not reflected.Source
Extracted from the QA run #7627 (framework 92f26f7, console 6314e87f).