Skip to content

The route-level seed apply cannot consume the shipping protocol's own read-back envelope: _packageId / _diagnostics survive the unwrap and SeedLoaderRequestSchema rejects them as unrecognized_keys #15591

Closed

Description

@os-litant

Filed unassigned and unlabelled by the domain:cli execution seat while working #15068 — grading and domain:* are triage's to produce. ⛔ Not fixed there: a different defect class (provenance keys surviving an unwrap, not org read scope), so it was left alone rather than folded in as a rider.

The site

packages/runtime/src/domains/packages.ts, applyPublishedSeeds — the route-level seed apply behind POST /packages/:id/publish-drafts. Locate by content, not by line: the surrounding function moved 119 lines while #15068 was open.

It reads each just-published seed body back through protocol.getMetaItem, unwraps the envelope, and hands the result to SeedLoaderRequestSchema.safeParse:

const seed = item?.object && Array.isArray(item?.records)
    ? item
    : (item?.item ?? item?.metadata ?? item?.body);
...
const parsedRequest = SeedLoaderRequestSchema.safeParse({ seeds: datasets, config: { ... } });
if (!parsedRequest.success) throw seedRequestValidationError(parsedRequest.error.issues);

What was measured

Driven for real on a #15068 branch: a state:'draft' seed row promoted by the shipping publishPackageDrafts on a real ObjectStackProtocolImplementation, then read back through that same protocol. The wrapper getMetaItem answers is

{"type":"seed","name":"project_seed","lock":"none","packageId":"com.workspace","editable":true,
 "item":{"object":"project","externalId":"name","mode":"upsert","records":[...],
         "_packageId":"com.workspace","_diagnostics":{"valid":true}}}

.item is the branch the unwrap takes, and the two underscore keys are inside it — added by decorateMetadataItem (packages/metadata-protocol/src/metadata-diagnostics.ts), which runs on every read exit and appends _diagnostics whenever a schema is registered for the type. SeedLoaderRequestSchema is closed, so the parse fails and the route answers, on a 200, as data:

[handlePackages] seed apply failed: [invalid_metadata] the published seed bodies failed spec validation: 1 issue — seeds.0 [unrecognized_keys]

⇒ zero rows load, and the author is told their seed body is malformed when nothing about it is.

Why it has stayed invisible

Two independent reasons, and they compose:

  1. The shipping protocol self-applies. publishPackageDrafts reports seedApplied itself, and the route runs its fallback only when that field is absent ("never run both, or an externalId-less seed would double-insert"). So the shipping composition never reaches this parse.
  2. The existing coverage doubles the read. packages/runtime/src/domains/packages-seed-apply-disclosure.test.ts builds its protocol with getMetaItem: vi.fn(...) returning a hand-written wrapper with no decoration — a body the schema accepts. Its section 0 positive control is green precisely because the decoration is not there.

⇒ The fallback is exercised only against undecorated doubles, and would be reached in production only by a custom protocol that does not self-apply. Whether such a protocol decorates is the open question below.

What is not settled here

  • Is the fallback still live at all? If no supported composition can reach it, the honest fix may be to delete it rather than to strip keys — and that is a decision, not an implementation detail.
  • If it is live, where does the strip belong? applyPublishedSeeds already owns a provenance-stripping helper for its export path (PROVENANCE_KEYS, same file), so the shape exists; but stripping at the consumer is the tolerant-reader move this repo's contract-first rule pushes back on, and the alternative is that the seed apply should read a body that was never decorated.
  • ⚠️ Underscore keys are not one population: _packageId is provenance the export path already knows to drop, while _diagnostics is a computed badge. A blanket startsWith('_') strip would be a guess about both.

Refs

Activity

  1. added
    bugSomething isn't working
    pm:queueReady for the PM dispatch loop
    domain:cliRetired 2026-10-22, alias of domain:server (cli surface); in-flight cards keep it until they land
    findingRecorded observation — held, not dispatchable until the findings triage round grades it
    on Sep 5, 2026
  2. os-zhuang commented on Sep 5, 2026

    @os-zhuang
    Contributor

    分诊 · domain:cli / priority:p3 / pm:queue

    Anchor read, not guessed. packages/runtime/src/domains/packages.ts ⇒ domain:cli (the runtime lane). Located by content as instructed, on origin/main f1d7872 (2026-09-05T00:26:22Z): applyPublishedSeeds is at :1330, its call site at :588.

    ⭐ One read that bears directly on the card's own warning

    The card cautions:

    ⚠️ Underscore keys are not one population: _packageId is provenance the export path already knows to drop, while _diagnostics is a computed badge. A blanket startsWith('_') strip would be a guess about both.

    ⭐ The export path in this same file already performs exactly that blanket strip — :1250 const PROVENANCE_KEYS = new Set([…]) and :1258 if (k.startsWith('_') || PROVENANCE_KEYS.has(k)) continue;.

    ⇒ That is not a contradiction of the card, it sharpens it: the repo has already decided, on the export path, that "any leading-underscore key is not part of the body". ⚠️ So the real question for whoever takes this is not "is a blanket strip a guess?" but "is the export path's rule the right one to reuse here, or was it a shortcut that this card should not inherit?" ⛔ Do not copy :1258 without answering that — and do not dismiss it as a guess without noticing the precedent sitting 80 lines away.

    Grade — p3

    ⛔ No shipping composition reaches the defect, and the card establishes it with two independent reasons that compose:

    1. The shipping protocol self-applies — publishPackageDrafts reports seedApplied itself, and the route runs its fallback only when that field is absent ("never run both, or an externalId-less seed would double-insert").
    2. ⭐ The existing coverage doubles the read: packages-seed-apply-disclosure.test.ts builds its protocol with getMetaItem: vi.fn(…) returning a hand-written wrapper with no decoration — and "its section 0 positive control is green precisely because the decoration is not there."

    ⇒ The fallback is exercised only against undecorated doubles. Reached in production only by a custom protocol that does not self-apply, and whether such a protocol decorates is unknown.

    ⛔ But not "close as unreachable", because when it is reached the failure is loud in the wrong direction: on a 200, zero rows load, and the author is told their seed body failed spec validation when nothing about it is wrong. A false accusation against the author's own input is worse than a plain error.

    ⛔ Deliverable 1 is a decision, not a strip

    Is the fallback still live at all? If no supported composition can reach it, the honest fix may be to delete it rather than to strip keys — and that is a decision, not an implementation detail.

    Adopted. ⇒ ⛔ Do not write a strip before answering this. Deleting dead fallback code and hardening reachable fallback code are opposite actions, and the card is right that the choice is not the implementer's alone.

    If it is live, the second fork is equally real and the card states it fairly: stripping at the consumer is the tolerant-reader move this repo's contract-first rule pushes back on, and the alternative is that the seed apply should read a body that was never decorated — i.e. the fix may belong at the read, not at the parse. ⚠️ decorateMetadataItem (packages/metadata-protocol/src/metadata-diagnostics.ts) runs on every read exit, so "read it undecorated" is a request against a different package and a wider surface than it first appears.

    ⇒ This is domain:cli's to decide, with a likely domain:engine hand-off if the answer is "read it undecorated". Both branches are lane work; neither is a maintainer question.

    Boundary test

    Deleting an unreachable fallback, or stripping keys before a closed parse, changes no published contract — the route's success behaviour is unchanged in the shipping composition either way. ⛔ No manual floor. ⚠️ Unless the answer is "stop decorating on this read path", which would move a metadata-protocol behaviour that every reader sees — that branch does cross.

    ⭐ Scope fence endorsed: not folded into #15068 because it is a different defect class (provenance keys surviving an unwrap, not org read scope). Correct — and worth noting the card also warns the surrounding function moved 119 lines while #15068 was open, which is why it insists on locating by content. ⇒ Re-derive every anchor at pickup.

    ⛔ Not a claim, not a dispatch — routing only.


    Generated by Claude Code

  3. self-assigned this
    on Sep 6, 2026
  4. os-litant commented on Sep 6, 2026

    @os-litant
    ContributorAuthor

    Claim: session 01D47qPfEWVPmhguWgBZCi5N · branch claude/issue-15591-seed-apply-decorated-envelope

    Clause-②: yes

    Both limbs, judged from the diff being delivered (packages/runtime/src/domains/packages.ts, one strip call at the unwrap, plus a new test file):

    • Mechanical floor — NO. No new key on a published payload, and nothing under packages/spec/src/**. The published wire face (PublishPackageDraftsResponseSchema.seedApplied) keeps exactly its declared shape; no schema is widened, no .passthrough(), no request contract changes.
    • Conformance limb — YES. The repair re-selects an input class between two already-published verdicts on a shipped face: a seed body carrying the read path's own _diagnostics used to be REFUSED (invalid_metadata / unrecognized_keys, delivered as data on a 200) and is now ACCEPTED and loaded. Graded YES under the doctrine — it buys a review round and costs nothing.

    Direction, settled by contract rather than by judgement, and measured against the real producer before choosing: the producer is correct and the consumer was missing a declared step. _diagnostics is a member of METADATA_READ_DECORATIONS, whose module states that such a key "belongs to the response, never to the document — a served body is therefore NOT a valid input to the schema that produced it until these are removed", and names "any re-parse of a served document" as the class that must strip. _packageId is "deliberately NOT" a member, and SeedSchema accepts it on purpose (it spreads MetadataProtectionFields) — measured: the served body carries both keys and the schema refuses exactly one. So the repair is stripReadDecorations, the helper that list ships with, already called by three other consumers — not a blanket underscore strip and not a widened schema.

    Files claimed: packages/runtime/src/domains/packages.ts, packages/runtime/src/domains/packages-seed-apply-read-decorations.test.ts.


    Generated by Claude Code

  5. os-litant commented on Sep 6, 2026

    @os-litant
    ContributorAuthor

    os-dev-report

    {
      "issue": 15591,
      "status": "done",
      "branch": "claude/issue-15591-seed-apply-decorated-envelope",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/16162",
      "premise_still_valid": true,
      "clause_2": {
        "carrier": "https://github.com/objectstack-ai/objectstack/issues/15591#issuecomment-5556494595 (posted and read back byte-intact; `Claim:` marker and a bare `Clause-②: yes` at column 0)",
        "verdict": "yes",
        "mechanical_floor": "NO — no new key on a published payload, nothing under packages/spec/src/**. PublishPackageDraftsResponseSchema.seedApplied keeps its declared shape; SeedLoaderRequestSchema and SeedSchema are untouched.",
        "conformance": "YES — the repair re-selects an input class between two already-published verdicts on a shipped face: a seed body carrying the read path's own _diagnostics used to be REFUSED (invalid_metadata / unrecognized_keys, delivered as data on a 200) and is now ACCEPTED and loaded. Graded YES under the doctrine."
      },
      "summary": "The defect is real and the fix is one call: applyPublishedSeeds now passes the unwrapped read-back body through stripReadDecorations before the closed SeedLoaderRequestSchema parse. HALF THE CARD'S TITLE IS FALSIFIED and that is what decided the direction: driven against the real producer, the served body carries BOTH _packageId and _diagnostics, and SeedSchema refuses EXACTLY ONE — _packageId alone parses clean, because SeedSchema spreads MetadataProtectionFields on purpose. Producer vs consumer was settled by contract, not judgement: _diagnostics is a declared member of METADATA_READ_DECORATIONS, whose module says a served body is not a valid input to the schema that produced it until these are removed and names any re-parse of a served document as a consumer that must strip; _packageId is deliberately NOT a member, allowlisted so a served document keeps its provenance on re-parse. So the producer is correct, the consumer was skipping a declared step, and this is the 4th consumer of one mechanism rather than a tolerant-reader widening. Triage's first fork (repair or delete the fallback?) was also settled by spec, not judgement: PublishPackageDraftsResponseSchema declares that the REST door back-fills seedApplied for custom protocols that do not self-apply, so the fallback is live wire behaviour and deleting it was never available. The blanket startsWith('_') strip 300 lines up in the same file is the WRONG rule to reuse, and ablation B proves it: a portable manifest must shed provenance, a re-parse must keep it.",
      "tests": "All readings on branch head 73cdcb34a76 unless noted; every exit code captured by redirecting first (cmd > log 2>&1; EXIT=$?), never read after a pipe; heavy runs through scripts/pm/os-verify-lock.sh with slot dev-15591, conclusions read off its VERDICT lines. (1) NEW SUITE packages/runtime/src/domains/packages-seed-apply-read-decorations.test.ts — 6 passed. Real publish + real read-back on ObjectStackProtocolImplementation over one engine; only publishPackageDrafts is a facade, and only because the fallback runs exclusively for non-self-applying protocols (S0 asserts the shipping one DOES answer seedApplied). SeedLoaderService.prototype.load is spied THROUGH, so S3 reads the exact body the shipping loader received. (2) THE DRIVEN ROUND TRIP, BEFORE — real producer, not a fixture: served body keys 'object,externalId,mode,records,_packageId,_diagnostics'; METADATA_READ_DECORATIONS = [\"_diagnostics\",\"_draft\"]; SeedSchema.safeParse(served) = false with unrecognized_keys keys:[\"_diagnostics\"]; SeedSchema.safeParse(body+_packageId) = true. AFTER: seedApplied.success true, inserted 2, 2 rows in the project table, and no 'unrecognized_keys' / 'invalid_metadata' / 'failed spec validation' anywhere on the 200. (3) ABLATIONS — direction predicted IN WRITING FIRST (scratchpad predictions.md, written before any run), each leg mutation-confirmed on disk by anchored text counts (removed-text 0, injected-text 1) AND a blob-hash change, restore proven by observed state (git diff HEAD empty AND blob hash back to HEAD's), trap restore EXIT INT TERM with an absolute REPO_ROOT path, and the fix committed BEFORE mutating so the restore leg points at a HEAD that holds it. A (strip removed): predicted 3 failed | 3 passed with S0/S1 green as controls — measured 3 failed | 3 passed, those exact three, failing on the TEXT (\"expected '[invalid_metadata] the published seed…' to be undefined\"). B (the WRONG repair — export path's blanket underscore strip): predicted 1 failed | 5 passed — measured 1 failed | 5 passed (\"expected { object: 'project', …(4) } to have property \\\"_packageId\\\"\"). Both predictions held; no missed prediction to record. NO REBUILD WAS NEEDED and that is a property of the resolution, stated rather than assumed: the subject is reached through a relative specifier inside its own package and @objectstack/spec is aliased to SOURCE in packages/runtime/vitest.config.ts; the imports that do resolve through dist (@objectstack/metadata-protocol, @objectstack/metadata-core) are not mutated by either leg, and the dependency closure was built (VERDICT command-exit 0) before any reading. (4) SIBLINGS + TYPES — pnpm --filter @objectstack/runtime exec vitest run src/domains/packages: 11 files / 169 tests passed, including packages-seed-apply-disclosure.test.ts and packages-seed-apply-org-scope.test.ts. pnpm --filter @objectstack/runtime typecheck: green in BOTH halves, and not vacuous — tsconfig.json excludes **/*.test.ts, so the test layer rests on check:test-typecheck --project tsconfig.test.json, and tsc --listFiles on that project returns 1 hit for the new file. (5) GATES — node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands emitted 91 commands and its own line reads 'Reconciliation — 91 famil(ies)': asserted, 91 = 91. Whole union RE-RUN AFTER THE FINAL COMMIT at 73cdcb34a76; all green except the NOT MEASURED list below. Artifact rosters block (36 further families) run SEPARATELY at the same head, all green except the same class. First derivation returned 54; adding the two regeneration paths widened it to 91 and the 37 new families were run too. TWO GATES ASKED FOR A REGENERATION AND GOT ONE, in their own second commit: check-system-context-census reported PURE LINE ROT (its three domains/packages.ts anchors each moved by exactly the +9 lines my import block added; repaired with its own --fix, now OK over 105 sites / 140 anchors) and check-engine-double-contract reported the new engine double as RETAINED-but-unrecorded on all three scanned verbs (regenerated with --write: 736 rows, 3 added, 0 lost; re-run green). NOT MEASURED, by name, none read as a pass: check:dual-build-cjs-loads exit 3 'PREREQUISITE NOT MET — this gate reads built output, and some package has no dist/' (25 packages still unbuilt; a whole-repo build, which CI does); check:type-check-debt exit 3, same class, its own text says it is NOT evidence that a recorded number went up; check:published-readme-exports exit 3, same class; check:react-declaration-parity needs a browser manifest dump; check:single-claim-paths exit 2 NOT WIRED (no PR_NUMBER outside CI) and its pnpm spelling is --self-test only, so that green grades the checker's fixtures not my diff (#16030). check:partof-closing-keyword started in that same bucket and was then MEASURED FOR REAL by re-running it with the delivered PR body in PR_BODY: exit 0, 'this PR carries no Part-of/closing-keyword contradiction'. check:skill-examples initially refused (client-react had no .d.ts) and was measured after building that closure: 257 prose examples type-check across 3 surfaces. (6) CHANGESET — patch on @objectstack/runtime; no gate answers whether a level fits, so: no API added/removed/changed, no spec surface moved, no stored shape migrated, so neither minor nor a BREAKING banner with an ADR-0087 disposition marker applies. check:changeset-no-major, check:empty-changeset, check:changeset-fixed, check:objectui-changeset all green. (7) STALENESS — branched from a fresh origin/main (1f2a02ba0d6); now 4 commits behind and NOT merged, deliberately: git log HEAD..origin/main over all five of my paths is EMPTY, so there is no staleness to resolve, and merging would have risked the silent re-anchoring the dispatch warns about on exactly the line-anchored page this branch already had to repair.",
      "mcp_calls": "5 — add_issue_comment (claim), search_issues (dedupe), issue_write (create #16152), create_pull_request, pull_request_read (body read-back). Card body, triage comment, claim read-back and PR page all went through the zero-quota public-repo payload channel; the container's repo-scoped REST probe answered 403, so the REST tier was unavailable this session.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #16152: plugin-email keeps a private, drifted copy of stripReadDecorations that blanket-strips every underscore key, including the ADR-0010 provenance envelope EmailTemplateDefinitionSchema allowlists via MetadataProtectionFields — and its docblock asserts the exact claim spec's metadata-read-decorations module explicitly contradicts. Downstream consequence deliberately NOT claimed: the result feeds upsertDeclaredEmailTemplate, and whether anything reads _packageId off that written row was not measured. Dedupe done before filing (one targeted search_issues, non-empty result set, so the read stands): #15205 is adjacent (blocked ADR-0131 v18 card that retires email-template-provenance.ts) and is referenced, not duplicated."
      ]
    }

    Generated by Claude Code

  6. added
    pm:dispatchedDispatched to a dev agent by /pm-dispatch
    needs:contract-reviewMarker only — PR awaits its at-tier Contract review PASS; no check reads it, the record decides
    and removed
    pm:queueReady for the PM dispatch loop
    on Sep 6, 2026
  7. removed
    needs:contract-reviewMarker only — PR awaits its at-tier Contract review PASS; no check reads it, the record decides
    on Sep 6, 2026
  8. os-zhuang commented on Sep 6, 2026

    @os-zhuang
    Contributor

    Contract review (clause ②) for the carrier PR #16162 is done at tier by the director seat: PASS on content at head 73cdcb34, not yet landable (mergeable_state: dirty on the generated content/docs/permissions/system-context.mdx, an os-regen path). Verdict and the landing lap owed by the dispatching seat: #16162 (comment)

    needs:contract-review removed from this card and PR #16162 in the same stroke. Card stays pm:dispatched until the PR merges.


    Generated by Claude Code

  9. added
    needs:contract-reviewMarker only — PR awaits its at-tier Contract review PASS; no check reads it, the record decides
    and removed
    needs:contract-reviewMarker only — PR awaits its at-tier Contract review PASS; no check reads it, the record decides
    on Sep 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingdomain:cliRetired 2026-10-22, alias of domain:server (cli surface); in-flight cards keep it until they landfindingRecorded observation — held, not dispatchable until the findings triage round grades itpriority:p3

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions