[finding] MetadataFacade.get/list unwrap item?.content ?? item, but content is a real authorable field — a doc registered through the facade reads back as its Markdown string, not the document

Observation-class finding, filed by the domain:engine-core PM seat (session session_01MhAQCKYvakd4fx78uZXUB2) on behalf of the #7378 dev's out-of-scope report (held for PM dup-triage first — dup search zero hits, mechanism re-verified at origin/main @ 76d74ec before filing). Unassigned, deliberately not queued — triage grades it.

Mechanism (verified, file:line at origin/main)

packages/objectql/src/metadata-facade.ts unwraps stored items on every read:

:149 — return item?.content ?? item; (get)
:164 — return items.map((item: any) => item?.content ?? item); (list)
:204 — item?.name ?? item?.content?.name ?? '' (listNames)

The unwrap assumes content is the facade's own storage envelope. But content is also a REAL authorable field on live metadata types:

packages/spec/src/system/doc.zod.ts:92 — content: z.string().describe('Raw Markdown content (CommonMark + GFM)')
packages/spec/src/ai/knowledge-document.zod.ts:37/:66 — content: z.string()

So register('doc', name, docDocument) followed by get('doc', name) returns the raw Markdown STRING instead of the doc document — silently: no error, and the returned value is truthy and string-typed, so downstream ?.name-style reads yield undefined rather than throwing.

Scope notes

Predates PR fix(objectql,spec): rule the name argument as IMetadataService's effective key (#7378) #7511 (the MetadataFacade answers three register → get round-trip cases differently from every other shipped IMetadataService #7378 ruling work): that PR did not touch the unwrap; its cell-3 { name, content } boxing is produced only for values that are NOT objects, so it cannot collide with an authored object carrying content.
Blast radius today is dormant: no in-tree caller constructs a MetadataFacade (the MetadataFacade answers three register → get round-trip cases differently from every other shipped IMetadataService #7378 body's consumer measurement). It becomes live the moment one is wired.
Same family as the MetadataFacade answers three register → get round-trip cases differently from every other shipped IMetadataService #7378 rows: a shipped implementation resolving a contract ambiguity silently. If the fix ends up wanting an envelope shape change, it should ride whatever ruling settles MetadataFacade answers three register → get round-trip cases differently from every other shipped IMetadataService #7378's remaining cell rather than invent a second envelope.

Refs

#7378 (dev report out_of_scope_findings, 2026-08-11 04:19Z) · PR #7511 · #6725 (silent-loss family).

Filed unassigned — recording the finding, not claiming it.

Triage: finding + domain:engine-core.

Landing anchor (re-verified): unwrap lines confirmed on origin/main @ 1530870 — packages/objectql/src/metadata-facade.ts:149 (return item?.content ?? item;) and :164 (list map) exactly as filed.
Why finding, not queue: real mechanism, but dormant today — the body's own consumer measurement (from MetadataFacade answers three register → get round-trip cases differently from every other shipped IMetadataService #7378) shows no in-tree caller constructs a MetadataFacade. Nothing a user hits until one is wired; the findings-triage round grades promotion, and the body's note to ride the MetadataFacade answers three register → get round-trip cases differently from every other shipped IMetadataService #7378 envelope ruling (rather than invent a second envelope) is a promotion-time constraint.
Dedup: filer's zero-hit dup search accepted; independently checked the open list — the closest kin are MetadataFacade answers three register → get round-trip cases differently from every other shipped IMetadataService #7378 (round-trip cells, pm:dispatched, distinct rows) and MetadataFacade.unregisterPackage removes only object contributors — every non-object item the package shipped stays registered #7221 (unregisterPackage, different mechanism). No convergence needed.
target:v17: no — dormant surface, not release-blocking.

本评论来自分诊座位 Routine（#5474 试点），不构成认领。

Generated by Claude Code

Findings triage round, 2026-08-11 (PM session, maintainer-directed: 「跑一轮集中定级」).

Graded: promote. content is a real authorable field (doc.zod.ts) and the facade's envelope-unwrap heuristic collides with it — a doc registered through the facade reads back as its Markdown string. Concrete, measured at file:line, and adjacent to the #7378 ruling (the facade aligns with the other implementations), so the two should serialize on metadata-facade.ts: hot-file note for the engine-core seat — #7378 first, this second, regions declared in claim comments.

finding → pm:queue (domain:engine-core).

Generated by Claude Code

Claim: PM loop round 3
Session: session_01RDTnVvsgA6cUZ4xFVtPZRy
Branch: claude/issue-7519-facade-content-unwrap
Worktree: objectstack-issue-7519
Domain: domain:engine-core
File surface: packages/objectql/src/metadata-facade.ts + its test files (stop on breach; explain in the report)
Container & model: M, mode:subagent, model: claude-fable-5
Serial constraints cleared: metadata-facade.ts — no open PR touches it (lane in flight 0, enqueued 0 at claim time). ⚠️ Same-day churn, ~30 minutes old: #8349 (#7378's implementation half) landed on this exact file. The dev branches from current origin/main and reads the merged text.

The stated dependency is satisfied — verified, not assumed

This card was held because its fix depended on #7378's cell 3: "if non-object data throws, the { name, content } boxing it must unwrap disappears." It does. #8349 removed both the boxing and the silent reconcile, and the facade's new docblock names this card's seam directly.

⚠️ A grep alone said otherwise, and would have misled me: searching origin/main for the boxing returns 1 hit — which on reading is the docblock describing the removal in past tense. Same shape as a false alarm this seat hit earlier today on via populate. A grep cannot distinguish a claim from a description of a claim; the count is not the finding.

Premise re-checked at claim time

The unwrap is still present, at addresses that have moved: :202, :217, :259 (the card records :149, :164, :204 from 08-11). ⛔ Locate by symbol — this file has churned three times since the card was filed.

Model rationale

claude-fable-5. This determines what MetadataFacade.get / list / listNames return for every metadata type carrying a real content field (doc, knowledge_document), on the very register → get roundtrip contract the maintainer ruled two days ago. ⛔ The clause has no downward discretion and the card's small diff is not the criterion; when the surface is the one that was just ruled, the tier follows the surface.

⛔ Not escalated, and the threshold is worth stating

The fix picks an envelope mechanism, which looks like a design call. It is not one: the invariant is already ruled. #7378 row 1 settled that register(t, n, d) → get(t, n) either round-trips or refuses loudly — a shipped implementation silently returning something other than what was registered is the defect that ruling names. This is a restore-the-invariant fix (its own decision), so it goes to the queue rather than the decision box. ⛔ If the dev finds the invariant genuinely cannot be restored without a contract choice, that is a reported fork, not a silent pick.

Generated by Claude Code

{
  "issue": 7519,
  "status": "done",
  "branch": "claude/issue-7519-facade-content-unwrap",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/8377",
  "premise_still_valid": true,
  "summary": "Removed the content unwrap from all three MetadataFacade reads (get :202, list :217, listNames :259 — located by symbol, addresses exactly as the claim comment re-verified). Mechanism chosen by measurement, not design: the { name, content } envelope the unwrap presumed has NO producer — the only writer that ever produced one was the facade's own #7511 cell-3 boxing of non-object values, removed by #8349 in favour of the shared guard's loud refusal; DB hydration (loadMetaFromDb) registers the parsed document from the sys_metadata.metadata column, never a wrapper; every other in-tree registerItem caller stores the document as-is. So the reads return the stored document verbatim, restoring the ruled round-trip, and NO replacement envelope key exists — the collision surface is gone, not moved to the next authorable key. Both directions pinned: doc-with-content and knowledge_document round-trip whole (new facade tests + a driver-local conformance pin holding all four shipped IMetadataService implementations to one answer, deliberately NOT the shared table under packages/spec/src, which is untouched — scope held, #8350 not widened); content-less documents still round-trip (surviving path pinned); the listNames content?.name limb was dead in both directions and is removed with the measurement in the docblock. PM mechanism assumptions all verified: addresses had moved exactly as stated; I read the merged #8349 text rather than grepping (the docblock describing the removed boxing is indeed the only grep hit); dependency satisfied; blast radius re-measured at HEAD — new MetadataFacade appears only in test files and one migration doc, so the defect stays dormant until a host wires one. Issue body read whole (ends 'Filed unassigned — recording the finding, not claiming it.') plus all 3 comments including the PM claim. Changeset added (patch, @objectstack/objectql), so no skip-changeset label applies.",
  "tests": "Full @objectstack/objectql suite under the shared verify lock: 196 files, 3506 tests, all passed (110s). pnpm --filter @objectstack/objectql typecheck clean. Reverse verification, direction predicted BEFORE running: unwrap restored from origin/main => predicted red on the get-document, list, knowledge_document and conformance (MetadataFacade subject only) pins, green on listNames/exists/getEntry/content-less pins since those members already answered name-carrying documents correctly; observed exactly that — 'Tests 4 failed | 98 passed (102)', the 4 being precisely the predicted set; fix then restored from the committed branch. Gates: check:durability-log-level, check:engine-double-contract ('OK — 191 pinned, 133 in the DEBT ledger, 2 exempt'), check-engine-split-ratio (informational, printed), check:query-options-erasure ('ratchet holds: 67 unswept non-test site(s) ... none new'; test-surface ceiling unchanged at 240), check:type-check-coverage ('OK — 64/77'), check:nul-bytes ('OK, scanned 7586 text file(s)') — all green. dispatch-gates.mjs re-derived on the actual changed paths names exactly the families already run, nothing additional. CI on the PR: in_progress at report time (per the 2026-08-10 report-at-draft ruling).",
  "open_questions": [],
  "out_of_scope_findings": ["filed as #8378: the same i?.content ?? i unwrap family survives in production plugin read paths (plugin-security x3, plugin-sharing, plugin-webhooks x2, plugin-email x3) over the same registry stores — dormant on spec-valid data by the same no-producer measurement, with one graded nuance: email-template accepts content as an authorable ALIAS spelling, and whether a pre-conversion item can reach that read was not measured; observation-class, finding label, no pm:queue"]
}

Generated by Claude Code

PM review — ACCEPT on the merits. PR #8377; landing held on CI convergence.

domain:engine-core seat (#6019), session session_01RDTnVvsgA6cUZ4xFVtPZRy. Reviewed against GitHub and the tree, ⛔ not the report's narrative.

Path face (read from the tree): exactly 4 files — the changeset, metadata-facade.ts, its test, and the objectql conformance driver. 0 hits under docs/adr/**, .claude/skills/**, skills/**, or packages/spec/src/**. ⭐ The fence held where it mattered: the new all-four-implementations pin went into the driver-local block, ⛔ not the shared table under packages/spec/src/ — that table is the spec seat's half of #7378 and is tracked on #8350, which this card did not widen.

⭐ The dev answered a better question than I asked

I told it to pick an envelope mechanism and to check which authorable key it might collide with next. It did not pick one — it measured that the envelope has no producer at all:

the only writer that ever produced one was the facade's own fix(objectql,spec): rule the name argument as IMetadataService's effective key (#7378) #7511 cell-3 boxing, which fix(core,metadata,objectql): enforce the #7378 three-cell IMetadataService register ruling in every shipped implementation #8349 replaced with the shared guard's loud refusal;
DB hydration (loadMetaFromDb) registers the parsed document from sys_metadata.metadata, never a wrapper;
every other in-tree registerItem caller stores the document as-is.

With no envelope on any write path, an unwrap on the read path can only corrupt. So the fix is removal, not a rename — and there is no next authorable key to collide with, because the collision surface is gone rather than moved. That is strictly better than the safer-key answer my dispatch invited, and it is the reason this needed no envelope ruling.

It also removed the dead listNames content?.name limb with the measurement of why it was dead in both directions, rather than quietly.

Verification credited

Reverse verification with the direction and the specific set predicted before running: unwrap restored ⇒ predicted red on get-document / list / knowledge_document / conformance-facade-subject, green on listNames / exists / getEntry / content-less. Measured 4 failed | 98 passed, exactly that set, restored from the committed branch rather than working-tree memory. Full suite 196 files / 3506 tests green.

Blast radius re-measured at HEAD as I asked, not inherited from the card: new MetadataFacade appears only in tests and one migration doc, so the defect stays dormant until a host wires one — and the class is exported from the package root, so downstream hosts pick the fix up via the changeset.

Docs advisory — checked, and clean

15 pages, all package-level via @objectstack/objectql. The one page mentioning MetadataFacade (migration-from-objectql.mdx) documents construction and async-ness, not the unwrap; its example registers an object with no content field, so the fix makes the round-trip it teaches more reliably true. An honest negative — the method that found real defects on three cards today found nothing here, and that is a result rather than a reason to keep digging.

The out-of-scope finding is the more important half

#8378: the same ?.content ?? i unwrap family survives in production plugin read paths — plugin-security ×3, plugin-sharing, plugin-webhooks ×2, plugin-email ×3 — over the same registry stores. ⚠️ Unlike the facade, those are not dormant. And the dev graded the nuance instead of smoothing it: email-template accepts content as an authorable alias spelling, and whether a pre-conversion item can reach that read was not measured — stated as unmeasured, not assumed safe. Filed unassigned with a finding label, ⛔ not self-promoted into this lane's queue; triage grades it, and its routing is likely domain:identity / domain:services, ⛔ not here.

Landing follows once every gate job carries its own conclusion.

Generated by Claude Code

LANDED — PR #8377 merged 2026-08-13 11:48Z. Closed by the merge; `pm:dispatched` cleared.

domain:engine-core seat (#6019), session session_01RDTnVvsgA6cUZ4xFVtPZRy.

What landed

All three reads return the stored document verbatim. The unwrap presumed content marked the facade's own storage envelope; content is a real authorable field (doc.zod.ts declares it as the doc's required raw-Markdown body, knowledge-document.zod.ts on both shapes), so register('doc', n, d) → get('doc', n) answered the Markdown string — truthy, string-typed, silent.

Removal rather than a renamed envelope, measured not assumed: the envelope has no producer. The facade's own interim boxing (#7511 cell 3) was replaced by the shared guard's loud refusal in #8349; DB hydration registers the parsed document itself; every other in-tree registerItem caller stores the document as-is. ⭐ Because no replacement key is introduced, the collision surface is gone, not moved — there is no next authorable key to collide with.

Both directions pinned, and the listNames content.name fallback limb was measured dead in both directions before removal. Reverse verification predicted the direction before running: 4 red / 98 green, exactly as predicted, with the fix restored from the committed branch rather than working-tree memory.

⚠️ The new conformance pin went in the driver-local "beyond the table" block, deliberately not the shared table — that lives under packages/spec/src/ and is the spec seat's half of #7378, tracked on #8350. Fence respected.

On the merge-queue ejection — recorded because the lane will hit this again

This PR was dequeued at 10:55:27Z with CI_FAILURE and it was not this PR's failure. Measured at the time: of the 12 jobs in #8377's own queue run, five were success and seven were still in_progress — not one had failed. It was first in a batch with #8383 stacked on it, and #8383 was re-formed onto the base 2 seconds after #8377 was dropped.

Re-queued standalone at 11:28Z on the same commit, zero changes → merged 20 minutes later. ⭐ A merge-queue red is a batch verdict, not a PR verdict — the discriminator is mechanism ("can this diff physically reach that code path?"), and this diff touches only a class with no production caller. Same shape as #8216. ⛔ Re-queueing before reading the run's own jobs would have been guessing; reading them first is what made the re-queue evidence-based.

Blast radius, restated

new MetadataFacade appears only in test files and one migration doc — no production caller constructs one. The defect was dormant and becomes live the moment a host wires the facade; the class is exported from the package root, so downstream hosts pick up the fix via the patch changeset on @objectstack/objectql.

Generated by Claude Code

fix(objectql): MetadataFacade reads return the stored document — cont…

fix(plugins): retire the `i?.content ?? i` unwrap family from plugin …

Navigation Menu

Repository navigation

[finding] MetadataFacade.get/list unwrap `item?.content ?? item`, but `content` is a real authorable field — a doc registered through the facade reads back as its Markdown string, not the document #7519

Mechanism (verified, file:line at origin/main)

Scope notes

Refs

The stated dependency is satisfied — verified, not assumed

Premise re-checked at claim time

Model rationale

⛔ Not escalated, and the threshold is worth stating

PM review — ACCEPT on the merits. PR #8377; landing held on CI convergence.

⭐ The dev answered a better question than I asked

Verification credited

Docs advisory — checked, and clean

The out-of-scope finding is the more important half

LANDED — PR #8377 merged 2026-08-13 11:48Z. Closed by the merge; `pm:dispatched` cleared.

What landed

On the merge-queue ejection — recorded because the lane will hit this again

Blast radius, restated

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[finding] MetadataFacade.get/list unwrap item?.content ?? item, but content is a real authorable field — a doc registered through the facade reads back as its Markdown string, not the document #7519

Description

Mechanism (verified, file:line at origin/main)

Scope notes

Refs

Activity

os-zhuang commented on Aug 11, 2026

os-zhuang commented on Aug 11, 2026

os-zhuang commented on Aug 13, 2026

The stated dependency is satisfied — verified, not assumed

Premise re-checked at claim time

Model rationale

⛔ Not escalated, and the threshold is worth stating

os-zhuang commented on Aug 13, 2026

os-zhuang commented on Aug 13, 2026

PM review — ACCEPT on the merits. PR #8377; landing held on CI convergence.

⭐ The dev answered a better question than I asked

Verification credited

Docs advisory — checked, and clean

The out-of-scope finding is the more important half

os-zhuang commented on Aug 13, 2026

LANDED — PR #8377 merged 2026-08-13 11:48Z. Closed by the merge; pm:dispatched cleared.

What landed

On the merge-queue ejection — recorded because the lane will hit this again

Blast radius, restated

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

[finding] MetadataFacade.get/list unwrap `item?.content ?? item`, but `content` is a real authorable field — a doc registered through the facade reads back as its Markdown string, not the document #7519

LANDED — PR #8377 merged 2026-08-13 11:48Z. Closed by the merge; `pm:dispatched` cleared.