Skip to content

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

@huangyiirene

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 skill metadata is read after a runtime store override:

  1. GET /api/v1/meta/skill returns the same skill twice after a runtime PUT — the store-override row and the package row are both listed, not merged/deduped by name.
  2. 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.
  • Expected: one row per skill name (override merged over package), and a runtime {active:true} PUT reflected in the prompt bridge.

Root cause (as located, unconfirmed)

The two surfaces read from different sources:

  • The MCP prompt bridge's listSkills calls metadataService.list('skill') — registry/package rows (packages/mcp/src/mcp-server-runtime.ts:753, listSkills: async () => (await metadataService.list('skill')) ?? []).
  • The HTTP meta list goes through protocol.getMetaItems — which merges store overrides.

So the store override (the {active:true} PUT) lands on the getMetaItems path but never on the metadataService.list path 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 on origin/main (mcp-server-runtime.ts:753; the dedup gap is in the getMetaItems merge in packages/metadata-protocol).

Note: the fix straddles packages/metadata-protocol (dedup/merge of skill rows) and packages/mcp (which source listSkills reads). Routed domain:metadata because 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

  1. Boot a showcase with a packaged skill.
  2. PUT /api/v1/meta/skill/<name> with {active:true} → 200.
  3. GET /api/v1/meta/skill → the skill appears twice (override row + package row).
  4. Query the skill over MCP prompts → the {active:true} flip is not reflected.

Source

Extracted from the QA run #7627 (framework 92f26f7, console 6314e87f).

Activity

  1. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    Contributor

    Triage — dual-state resolution: pm:queue + domain:metadata kept; finding removed (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:753 reads metadataService.list('skill') verbatim (registry rows, no overrides), while the HTTP list goes through getMetaItems in packages/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; the packages/mcp half is pointing listSkills at 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

  2. huangyiirene commented on Aug 11, 2026

    @huangyiirene
    CollaboratorAuthor

    Deferred at batch selection (round 1, domain:metadata seat) — recording the traps rather than silently pushing it to a later round. Stays pm:queue, unassigned.

    Why deferred: the dedup/merge half lands in packages/metadata-protocol (getMetaItems), and protocol.ts is 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:

    1. ⚠️ 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 the getMetaItems merge 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, listSkills reading metadataService.list('skill')) belongs to domain:cli under the package table, not here.
    2. 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_decision design 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

  3. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    CollaboratorAuthor

    Held 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 listSkills reads, located here as packages/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 quotes agent_prompt's sibling skill bridge, (await metadataService.list('skill')) ?? [], as a consumer of the known-partial list() answer. #6504 proposes adding a listDiagnosed counterpart to IMetadataService and 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 listDiagnosed lands, 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) and packages/mcp (which source the bridge reads). The metadata-protocol half — duplicate rows on GET /api/v1/meta/skill not being deduped by name in the getMetaItems merge — 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

  4. self-assigned this
    on Aug 13, 2026
  5. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    Contributor

    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: the getMetaItems read merge/dedup path only. ⛔ NOT the error/catch message-construction paths, ⛔ NOT the audit-write sites. ⛔ NOT packages/mcp — that is domain: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 on protocol.ts under the region exemption (cap 3). The other is #8136 (client-facing message construction in error/catch paths — comment 5275484126). 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), merge main before 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 listSkills reads metadataService.list('skill') in packages/mcp — belongs to domain:cli, and ⛔ this seat does not cross into it. It gets filed for that lane rather than absorbed.


    Generated by Claude Code

  6. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    Contributor
    {
      "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

  7. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    Contributor

    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 Docs skipped by path filter
    Region fence held — exactly one hunk in protocol.ts, inside the declared getMetaItems read-merge region. ⛔ No error/catch message construction (#8136's region), no audit-write sites (#7748's)
    Cross-lane held — packages/mcp untouched; symptom 2 filed as #8328, already routed domain:cli by triage
    Path fork clean of ADR / skills ⇒ merge queue
    Closure Part of #7654 — correct; the card stays open for symptom 2
    Changeset 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 skill like every other type" — and asked which. It is neither, and the difference decides the fix.

    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 — the same resolution getMetaItem(name, packageId=P) performs — while the MetadataService merge one layer below hand-rolled a Map on (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_template via the SchemaRegistry (correct), skill/agent/tool via 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 skill patch — agent duplicates identically, and the mirrored attribution (package-less baseline under a package-bearing row) duplicates too; both pinned. "Register skill like 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: injecting items = [] 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-debt red (@objectstack/rest +1) — with its own diff stripped, which correctly ruled out its changes. On instruction it rebuilt both dependency closures on fresh main and re-measured: green. The +1 was a stale-sibling-dist artifact of a fresh worktree. ⛔ Nothing about it is claimed in the PR body, which is the right outcome — a false claim about main sitting in a merged PR is worse than silence.
    • It declined to file the second gate (check:objectui-pin-fresh STALE) and routed it to me with the caveat that its own objectui checkout was stale. It was right: CI's Console Pin Freshness is green on three PRs across 35 minutes, and this container's /home/user/objectui last 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's listSkills reading metadataService.list('skill') — is #8328, domain:cli, Blocked-by this. 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 calls getMetaItems; it calls metadataService.list one 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 same listSkills expression 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

  8. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    Contributor

    LANDED — d5031f6a1 on origin/main (PR #8332). Confirmed by two readings: the merge event and the commit. ⛔ Not by the auto_merge field.

    ⭐ 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 of PR

    Delivered — 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: getMetaItems ran two merge implementations that disagreed about what a package-less row means — mergePackageAwareOverlay letting it stand in for each package's row (as getMetaItem(name, packageId=P) resolves), versus a hand-rolled Map on (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 skill patch: agent duplicated 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 calls getMetaItems — it calls metadataService.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 same listSkills expression 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:cli via #8328, now unblocked — its Blocked-by: #7654 is 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 in domain: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:dispatched dropped in the same action.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions