Skip to content

finding: ?preview=draft self-inflicts an invalid diagnostic — the injected _draft key is validated, so the read carries _diagnostics.valid:false #7656

Description

@huangyiirene

Unconfirmed observation from the QA run — recorded for triage; the injection site was located but the fix (strip _draft before computing _diagnostics, or whitelist the read-decoration keys in the diagnostics validation) is the natural next step to confirm.

Symptom

Reading a metadata item with ?preview=draft returns a response whose own diagnostics call the item invalid because of a key the read itself injected.

  • Observed: the response carries _diagnostics.valid:false with "Unrecognized key(s) … _draft".
  • Expected: _diagnostics.valid:true for an otherwise-valid draft — the preview marker must not be validated against the item's strict schema.

Root cause (as located, unconfirmed)

The draft-preview read injects _draft:true onto the item and then validates the item with that key still present. In packages/metadata-protocol/src/protocol.ts the single-item draft read stamps (draftItem as any)._draft = true (around protocol.ts:4441, and the list overlay at :4261) and then decorates/validates it; the item schema is strict, so _draft is rejected by name. stripReadDecorations exists to remove exactly these stamped keys before a strict re-parse, but it is not applied on the path that computes _diagnostics for the draft read. Mechanism still present on origin/main.

Related but distinct: #6810 (closed) was the same class of defect for the indexed key at a different injection site — see the module header in packages/metadata-core/src/injected-system-columns.ts. This one is the _draft preview marker.

Reproduction

  1. Create a draft of a valid metadata item (e.g. an object).
  2. GET /api/v1/meta/<type>/<name>?preview=draft.
  3. Inspect _diagnostics in the response → valid:false, "Unrecognized key(s) … _draft".

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).

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. self-assigned this
    on Aug 12, 2026
  3. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    CollaboratorAuthor

    Claim: PM loop round 12
    Session: session_01AmES43BMDg4bPrxTdi5q7t
    Branch: claude/issue-7656-draft-preview-diagnostics
    Worktree: objectstack-issue-7656
    Domain: domain:metadata
    File surface: packages/metadata-protocol/src/protocol.ts — region: the draft-preview read decoration and the _diagnostics computation it feeds (the _draft stamp sites and their validate path) — plus its tests. (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus
    Serial constraints cleared: protocol.ts is hot, with two siblings claimed on it this round in disjoint regions — #8136 (client-facing message construction in error/catch paths) and #8003 (the ARRAY_VALUED_QUERY_SLOTS docblock, ~:1763). #7748 is held behind #8136 (audit-write sites). This card's region (~:4462 / ~:4672) does not intersect any of them. Merge main before the PR and again if a sibling lands; conflicts go to the merge queue, ⛔ not hand-ordered.

    Verified on origin/main immediately before dispatch:

    • (data as any)._draft = true at protocol.ts:4462 and (draftItem as any)._draft = true at protocol.ts:4672 — the card says ~4441 / ~4261, so both have drifted; re-locate by reading, not by line number.
    • stripReadDecorations is live and exported from packages/metadata-protocol/src/index.ts, with existing pins in protocol.read-decorations.test.ts.

    Generated by Claude Code

  4. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    CollaboratorAuthor
    {
      "issue": 7656,
      "status": "done",
      "branch": "claude/issue-7656-draft-preview-diagnostics",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8179",
      "premise_still_valid": true,
      "summary": "Premise confirmed by reproduction before any edit: a draft preview read stamps `_draft:true` and then re-parses the item against the closed schema, so a valid draft came back `_diagnostics.valid:false / Unrecognized key(s) on this object: _draft` — on BOTH exits (single-item `getMetaItem` at protocol.ts:4672 and the list overlay at :4462). Root cause is one layer past the injection sites the card names: `computeMetadataDiagnostics` (packages/metadata-protocol/src/metadata-diagnostics.ts) stripped `_diagnostics` only, via a private one-key helper that predated `_draft` joining the shared METADATA_READ_DECORATIONS list. Fix = that function now calls the shared `stripReadDecorations`, which is the house shape: the module header of spec/kernel/metadata-read-decorations.ts names 'any re-parse of a served document' as a consumer that must strip, and read-time diagnostics are exactly that (third consumer after the #4326 write path and cloud#971's cold-boot bind). Closing it at the shared consumer fixes both injection sites in one edit, covers any future decoration key, and touches none of protocol.ts — deliberate, given #8136/#8003 are claimed on that file this round. PM assumption tested and upheld (strip, not whitelist); the item schema is NOT loosened and a test pins that it still rejects `_draft` by name. #6810's remedy (fix at the injection site) was read and deliberately not copied: `indexed` never belonged on the served body, whereas `_draft` is the badge the UI reads and is a declared read decoration.",
      "tests": "Reproduction first, red on origin/main: 4 new protocol-level cases failed with the card's verbatim message — \"Unrecognized key(s) on this object: `_draft`. Until #4001 closed this shape ...\" — on both the single-item preview read and the list overlay. After the fix: `pnpm --filter @objectstack/metadata-protocol test` => Test Files 75 passed (75), Tests 1100 passed (1100) (re-run after merging origin/main: same). Anti-vacuity: a genuinely broken draft (field `amount` with type `not_a_real_field_type`, seeded directly since saveMetaItem refuses it with 422) still reads `valid:false` with errors naming `amount` and not `_draft` — asserted on BOTH exits, so a fix that stopped computing diagnostics on the draft path would fail. Plus a drift guard (every METADATA_READ_DECORATIONS member must be invisible to the verdict) and an anti-loosening pin (the object schema still rejects `_draft` with unrecognized_keys). REVERSE VERIFICATION: reverting only metadata-diagnostics.ts to origin/main turns 5 of the 6 new cases red; the 6th (anti-loosening pin) is green in both directions BY DESIGN — it pins a constraint the fix must not relax, not the fix itself, so a direction change there would mean the schema had been loosened. Downstream consumer sweep (prefix filter direction, i.e. packages that DEPEND ON metadata-protocol, chosen as those whose suites exercise /meta reads, draft preview or _diagnostics): objectql 191 files/3390 tests, rest 102/1728, runtime 145/2195, metadata 31/603, service-automation 79/940 — all passed. (service-automation and rest first showed 7 collection failures in the fresh worktree: `Failed to resolve entry for package @objectstack/objectql` — unbuilt dist, the AGENTS.md build-closure trap, not the change; green after `pnpm --filter '<pkg>^...' build`.) Type surface unchanged; metadata-protocol has no typecheck script, so type checking came from its DTS build, which succeeded. Gates from scripts/pm/dispatch-gates.mjs on the actual changed files, all green locally: check:error-code-casing, check:cross-package-test-inputs, check:durability-log-level, check:changeset-gate-self-tests, check:objectui-changeset, check-changeset-no-major; plus check:nul-bytes (OK, 7422 files) with a manual control-byte self-scan (no hits) and eslint --no-inline-config on both changed files (clean). CI status at report time: in_progress — reporting at draft-PR time per the dispatch contract; the PM owns CI convergence and the ready-flip. Changeset added (patch, @objectstack/metadata-protocol). origin/main merged into the branch before the PR, cleanly.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Notes for the PM, outside the JSON:


    Generated by Claude Code


    Generated by Claude Code

  5. huangyiirene commented on Aug 12, 2026

    @huangyiirene
    CollaboratorAuthor

    ACCEPT — PR #8179. Verified against the diff on GitHub, not against the report's self-description.

    The scope deviation is approved, and it is the best thing in this delivery. The claim declared protocol.ts — "the draft-preview read decoration and the _diagnostics computation it feeds". The fix landed in metadata-diagnostics.ts instead, and the dev declared and justified that rather than either silently crossing or mechanically obeying a brief that pointed one layer too high. That is exactly the "stop on breach; explain in the report" contract. It is also strictly better: one edit closes both injection sites, covers any future decoration key, and touches none of protocol.ts — which is carrying two other claimed regions this round (#8136, #8003). A protocol.ts-local fix would have patched two sites, contended with two siblings, and left the next decoration to re-break it.

    Root cause is one layer past what the card named, and the card's suggested framing was superseded: computeMetadataDiagnostics stripped _diagnostics only, through a private one-key helper (stripDiagnostics) that predated _draft joining the shared METADATA_READ_DECORATIONS list. So the reader re-parsed its own badge against a closed schema. Calling the shared stripReadDecorations makes read-time diagnostics the third consumer of that list, alongside the #4326 write-path persist and cloud#971's cold-boot bind — whose module header already names "any re-parse of a served document" as a required consumer. This is house shape, not a new invention.

    The PM assumption held and was tested rather than assumed. I offered strip-vs-whitelist and asked for the reasoning; the dev took the strip and pinned the alternative shut — packages/metadata-protocol/src/protocol.read-decorations.test.ts asserts the object schema still rejects _draft with unrecognized_keys. That matters: it keeps the #4326 write-path strip load-bearing instead of cosmetic. ⛔ The schema was not loosened.

    #6810 was read and deliberately not copied, with the distinction stated: indexed never belonged on the served body and was removed at its injection site; _draft is the badge the UI reads and is a declared read decoration, so it belongs on the response and the fix goes where the list is consumed. Correctly reasoned rather than pattern-matched.

    The test evidence carries the parts I asked for:

    • Anti-vacuity on both exits — a genuinely broken draft (a field whose type is not a field type, seeded directly because the save path refuses it with 422) still reads valid:false naming amount, not _draft. Without this, a fix that simply stopped computing diagnostics on the draft path would have passed green. This was the specific trap in the dispatch order and it is closed on both exits.
    • Drift guard — every member of METADATA_READ_DECORATIONS must be invisible to the verdict, so a fourth decoration fails on a unit here rather than as valid:false on somebody's badge later.
    • ⭐ Reverse verification reported honestly. Reverting only metadata-diagnostics.ts turns 5 of 6 new cases red; the sixth is green in both directions by design, and the dev said so and explained why (it pins a constraint the fix must not relax, not the fix itself). A dev claiming 6/6 would have been the answer to distrust.
    • Downstream consumer sweep in the correct direction (packages depending on metadata-protocol): objectql, rest, runtime, metadata, service-automation — all green. The 7 initial collection failures were correctly diagnosed as the unbuilt-dist build-closure trap, not the change.
    • Gates taken from scripts/pm/dispatch-gates.mjs against the actual changed files, per the leads-not-specs rule.

    Changeset present (patch, @objectstack/metadata-protocol). Path face clean — no docs/adr/**, no skills/** — so this takes the normal landing route.

    CI was in_progress at report time, which is the honest reading at draft-PR time; convergence and the ready-flip are mine. Flip is queued for the next patrol: gate-bearing jobs must show their own conclusion: success first.


    Generated by Claude Code

  6. added a commit that references this issue on Aug 17, 2026
    8eb5d8b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions