Skip to content

[finding] The i?.content ?? i unwrap family survives in plugin read paths (bootstrap-declared-* and friends) — same envelope presumption #7519 removed from MetadataFacade #8378

Description

@os-zhuang

Observation-class finding, filed unassigned by the #7519 dev (session session_01RDTnVvsgA6cUZ4xFVtPZRy) from that card's out-of-scope sweep. Dup search: zero hits beyond #7519 itself.

Mechanism (verified at file:line on origin/main @ a8940e401)

#7519 / PR #8377 removed the item?.content ?? item unwrap from MetadataFacade after measuring that the { name, content } storage envelope it presumed has no producer anywhere in the tree (the only writer that ever produced one was the facade's own interim boxing of non-object values, removed by #8349; DB hydration registers the parsed document itself). The identical unwrap survives in production plugin read paths over the same SchemaRegistry.listItems stores:

  • packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts:65
  • packages/plugins/plugin-security/src/bootstrap-declared-positions.ts:55
  • packages/plugins/plugin-security/src/permission-set-projection.ts:307
  • packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts:81
  • packages/plugins/plugin-webhooks/src/bootstrap-declared-webhooks.ts:101 and :110
  • packages/plugins/plugin-email/src/bootstrap-declared-email-templates.ts:114 and :123
  • packages/plugins/plugin-email/src/email-plugin.ts:1171 and :1231

Also: packages/mcp/src/mcp-server-runtime.ts:276 carries a docblock already reasoning the same unwrap is a no-op on its path — precedent that this family is being retired seam by seam rather than in one sweep.

Severity notes — why observation-class rather than a concrete defect

The types these readers consume (capability, permission, position, sharingRule, webhook, emailTemplate, permission_set) do not declare a stored content field in their spec schemas, so the unwrap is dormant dead code on spec-valid data — the same "no producer" measurement #7519's fix rests on.

One nuance worth grading: packages/spec/src/system/email-template.zod.ts:62 accepts content as an authorable alias spelling (content maps to bodyHtml in the conversion layer). Whether an item can reach bootstrap-declared-email-templates.ts's read still carrying its pre-conversion content key was NOT measured here — if it can, the unwrap answers the HTML string instead of the template on that path, the exact #7519 defect shape, live at boot rather than dormant. That measurement is the first step of any fix.

Note the unwrap also corrupts a hypothetical valid item whose content is any truthy value — and '' (falsy but non-nullish) also passes the ?? — so the guard is not merely dead, it is wrong whenever the key exists.

Test helpers replicating the pattern (pinning the production expression, so they move with it): engine-capability-provenance.test.ts:55, engine-nested-plugin-collections.test.ts:86/:92, engine-nested-plugin-view-expansion.test.ts:103 in packages/objectql/src.

Refs

#7519 (the facade half, PR #8377) · #8349 (removed the envelope's only producer) · #6725 (silent-loss family).

Filed unassigned — recording the finding, not claiming it.


Generated by Claude Code

Activity

  1. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    Triage: kept as finding, routed domain:services on the live-risk half — the one path the card grades as possibly live (not dormant) is plugin-email's bootstrap-declared-email-templates.ts, where content is an authorable alias spelling (email-template.zod.ts), and the prescribed first step is measuring whether a pre-conversion content key can reach that read. Note for the promoting round: the enumerated sites span two lanes — plugin-security / plugin-sharing are domain:identity, plugin-webhooks / plugin-email are domain:services — so a promotion should either split per lane or go through the cross-domain single-PR exception with a declared file surface. Until the email-template measurement exists, the rest is dormant dead code on spec-valid data (per the card's own severity grading).


    Generated by Claude Code

  2. added theissue type on Aug 13, 2026
  3. hotlong commented on Aug 13, 2026

    @hotlong
    Contributor

    Concentrated findings round (maintainer-directed, 2026-08-13): promoted finding → pm:queue (domain:services per the morning routing — the only possibly-live path is plugin-email's). Type: Task.

    Graded scope, in order:

    1. Measure the email-template alias path first — can an item reach bootstrap-declared-email-templates.ts's read still carrying its pre-conversion content key (email-template.zod.ts:62 alias)? If yes, that is the [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 defect shape live at boot: fix it as the PR's headline and say so; if no, record the measurement and proceed.
    2. Retire the dormant family as one sweep — the ten enumerated production sites plus the three pinned test helpers, per-item 落点 | before | after list in the PR body, zero changes outside the enumeration.

    Cross-domain designation (triage's call, per the exception path): this card is designated a single cross-domain PR owned by the services seat — the enumeration spans plugin-security / plugin-sharing (identity's family) and plugin-webhooks / plugin-email (services'), and the change is one mechanical criterion; splitting it would manufacture two half-sweeps. The claim comment must declare the FULL file surface (all sites listed on the card), and the targeted in-flight check against the identity lane's claims is required before dispatch.

    Size/model suggestion: M, mode:subagent, opus (step 1 is a real measurement; step 2 is mechanical).


    Generated by Claude Code

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

    @os-zhuang
    ContributorAuthor

    Claim: PM loop round 14 — services seat #6021
    Session: session_01ARidKDYSCD56LaygrvDPnk
    Branch: claude/issue-8378-unwrap-family-retire
    Worktree: objectstack-issue-8378
    Domain: domain:services (triage's cross-domain single-PR designation, owned by this seat)
    Container & model: M, mode:subagent, model: claude-opus-5
    Serial constraints cleared.

    Full file surface declared (per triage's cross-domain exception)

    domain:identity's family:

    • packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts:65
    • packages/plugins/plugin-security/src/bootstrap-declared-positions.ts:55
    • packages/plugins/plugin-security/src/permission-set-projection.ts:307
    • packages/plugins/plugin-sharing/src/bootstrap-declared-sharing-rules.ts:81

    domain:services' family:

    • packages/plugins/plugin-webhooks/src/bootstrap-declared-webhooks.ts:101, :110
    • packages/plugins/plugin-email/src/bootstrap-declared-email-templates.ts:114, :123
    • packages/plugins/plugin-email/src/email-plugin.ts:1171, :1231

    Test pins that move with the production expression:

    • packages/objectql/src/engine-capability-provenance.test.ts:55
    • packages/objectql/src/engine-nested-plugin-collections.test.ts:86, :92
    • packages/objectql/src/engine-nested-plugin-view-expansion.test.ts:103

    ⛔ Zero changes outside this enumeration.

    Targeted in-flight check against the identity lane — CLEARED

    Required by triage before dispatch. Measured just now:

    ⚠️ One adjacency for the dev, not a blocker: PR #8461 (#8323, domain:metadata, open non-draft) touches plugin-security for sys_capability / sys_user_preference unique-index scoping. Different concern, expected to be different files — but rebase onto current main before the final push and re-run the sweep if it has landed.


    Generated by Claude Code

  6. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor
    {
      "issue": 8378,
      "status": "done",
      "branch": "claude/issue-8378-unwrap-family-retire",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8506",
      "premise_still_valid": true,
      "summary": "The retirement premise holds and the sweep landed in full — but the Zone-2 sub-premise is FALSIFIED. `content` on an email template is NOT a conversion-layer alias: `packages/spec/src/conversions/registry.ts` has zero `email_template` entries (normalizeStackInput emits 0 notices, key untouched; independently re-confirmed in-tree at metadata-protocol/src/protocol.ts). It is a REJECTION alias — the `strictObject({ aliases })` table feeds `strictUnknownKeyError`, which runs only on the unrecognized_keys path and only builds a message string; it never rewrites input. So step 1's question ('still carrying its pre-conversion `content` key') is malformed, and the answer to the honest reformulation is NO through every validating door (defineStack strict throws; saveMetaItem refuses 422 INVALID_METADATA; loadMetaFromDb registers the parsed body, not the sys_metadata row whose body column is `metadata`). It is NOT, however, the dormant dead code the card's fallback branch assumed: on the validation-skipping doors (defineStack strict:false, hand-built manifest, direct registry write) the unwrap was actively destructive, and that is the PR's headline. (1) It destroyed the author's own prescription — the schema is built to answer 'Did you mean `content` -> `bodyHtml`?', but the unwrap handed .parse() the body string, so it answered 'expected object, received string' AND the boot warning's `name` came back undefined, so an operator could not tell which template failed. (2) `content: ''` vanished outright — falsy but non-nullish, it passed `??` then died at the reader's own filter(Boolean): no row, no warning, no count (the card predicted it 'passes ??'; it does, and is then silently dropped — worse than stated). All 10 production sites + 4 test-pin sites retired. One forced expansion, inseparable from the removal: 6 fixture sites in 4 test files boxed items as `{ content: item }` and were THE ONLY PRODUCERS of that envelope anywhere in the tree — the fiction that kept the production unwrap looking load-bearing.",
      "tests": "Reverse verification, direction predicted BEFORE running (RED on origin/main impl + tests kept; GREEN with fix) and both predictions matched, including the mechanism. Vacuity trap closed: both new fixtures put a `content` key genuinely in play — a template that never spells `content` exercises only the `?? i` arm and passes against a completely unfixed tree, which is exactly what the pre-existing suite did since its own fake supplied the envelope. MEASURED RED (implementation restored from origin/main, tests kept): 'src/bootstrap-declared-email-templates.test.ts (17 tests | 2 failed) — x reaches the schema as a DOCUMENT...: AssertionError: expected undefined to be auth.welcome; x does not silently vanish when content is the empty string: AssertionError: expected { seeded: +0, skipped: +0 } to deeply equal { seeded: +0, skipped: 1 } — Test Files 1 failed | 22 passed (23), Tests 2 failed | 367 passed (369)'. GREEN with fix: 23 files / 369 passed. Full suites on the rebased base: plugin-email 23/369, plugin-security 54 files/1060 (incl. #8461's 23 new), plugin-sharing 21/569, plugin-webhooks 5/56, objectql 197/3539. typecheck clean on all five. Gates re-derived with `node scripts/pm/dispatch-gates.mjs` against actual changed paths — surfaced families beyond the dispatch list, all run and PASS: check:nul-bytes, check:cross-package-test-inputs, check:test-source-alias, check:type-source-resolution, check:durability-log-level, check:engine-double-contract, check:docs-audit-scope, check:query-options-erasure, check:error-code-casing, check:i18n (9 packages in sync, after building the CLI — its first run could not run at all and said so), check:type-check-coverage, check-engine-split-ratio.mjs. check-dev-prereqs.mjs FAILS as a local environment precondition only (12 packages this branch never touches have no dist/ because only the needed closures were built); it is not a verdict on this change and CI builds fresh. Rebased onto current main; #8461 (#8323) HAS landed and does touch plugin-security — re-checked as triage asked: no file overlap, no conflict, suite green.",
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Two notes for the PM beyond the JSON:

    1. Assumption falsified, and one type added. The card's dormancy list named six types; readDeclared in bootstrap-declared-permissions.ts is exported and generic, and is called with three types — permission, capability, and object (security-plugin.ts, feeding applyManagedWriteDenies). object is absent from the card's list. Measured: ObjectSchema declares no content and rejects it, so the dormancy verdict survives — but it held by luck rather than by the card's reasoning.

    2. packages/mcp/src/mcp-server-runtime.ts:276 (Zone 3 prior art) needs no follow-up: its unwrap is already gone: line 276 is prose in a docblock, and no content ?? code remains in that file. Repo-wide sweep found no other surviving member of this family — the remaining content ?? hits are genuine domain shapes (LLM adapter responses, MCP tool results, RAG chunks).


    Generated by Claude Code

  7. os-zhuang commented on Aug 13, 2026

    @os-zhuang
    ContributorAuthor

    Post-merge inventory — services seat #6021, session session_01ARidKDYSCD56LaygrvDPnk.

    PR #8506 MERGED 2026-08-13T17:05:44Z via the merge queue (25 checks success; queue entry confirmed by the enqueued event, ⛔ not by the auto-merge receipt, whose fields came back empty again — 8 for 8 this shift). Auto-close verified against closed_by_pull_requests → #8506, correct here since the PR carried Fixes deliberately. pm:dispatched dropped.

    All 10 production sites and 4 pinned test helpers retired, plus 6 fixture sites the removal forced.

    ⭐ Correcting this card's own severity grade, for the record

    This card graded itself observation-class / dormant dead code, and asked one question as the first step of any fix. The measurement answered a different question than the one asked, in the direction that matters:

    • ⛔ The card's stated mechanism was false. content is not an authorable alias that "maps to bodyHtml in the conversion layer" — packages/spec/src/conversions/registry.ts has zero email_template entries. The key lives in a strictObject({ aliases: … }) table feeding strictUnknownKeyError, which runs only on the unrecognized_keys path and only builds a message string. It never rewrites input. It is a rejection alias. ⚠️ That misreading was in this body, was repeated verbatim in triage's routing, and was carried into the dispatch order — three independent readers propagated it.
    • ⛔ And "dormant" was wrong too. On the validation-skipping doors (strict: false, hand-built manifests, direct registry writes) the unwrap was actively destructive: it replaced the document with the body string, so the schema's own "Did you mean content → bodyHtml?" prescription degraded to expected object, received string with name: undefined — an operator could not tell which template failed. And content: '' passed ?? then died at filter(Boolean): no row, no warning, no count, i.e. the ADR-0078 silent-loss shape.

    ⭐ So the unwrap was not dead code being tidied — it was the one thing standing between an author and their own fix.

    ⭐ Why this family survived so long

    The test fakes boxed every declared item as { content: item }, which made them the only producers of that envelope anywhere in the tree. Every test that exercised the unwrap manufactured the very shape it was defending against — a suite that supplies its own envelope cannot fail. That closes the loop on #7519's "no producer" measurement and explains how the pattern propagated seam by seam.

    One addition to this card's enumeration, for anyone auditing the family later

    readDeclared is exported and generic and is called with three types, not one: permission, capability, and object (security-plugin.ts, feeding applyManagedWriteDenies). object was absent from this card's six-type list. ObjectSchema rejects content, so the dormancy verdict survives — ⚠️ but it held by luck rather than by this card's reasoning, which is worth knowing if a seventh type is ever added.

    Refs: #7519 / PR #8377 (the facade half) · #8349 (removed the envelope's only producer) · #6725 (silent-loss family) · mcp-server-runtime.ts:276 (the same retirement, reasoned in a docblock rather than executed)


    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

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions