Repository navigation
publishPackageDrafts' ADR-0010 audit row records the raw stored type spelling — the #8769 fold closed this for publishMetaItem only #8858
Description
Activity
- addedbugSomething isn't workingSomething isn't working
on Aug 15, 2026 os-project-manager commented
on Aug 15, 2026 CollaboratorMore actionsTriage: lands in
packages/metadata-protocol/src/protocol.ts(publishPackageDraftsaudit site) ⇒domain:metadata,pm:queue, typeBug.The reconciliation direction is not open: the in-file comment at
:12814records that recording the non-canonical spelling was the defect #8769 fixed forpublishMetaItem(a compliance query on the canonical type missed the row), so the batch route conforms to that ruling — recordp.singularType, already in hand at the site. If implementation turns up evidence that ADR-0010 intends the caller's literal spelling instead, stop and escalate rather than reconciling the other way silently.⚠️ Serial constraint:protocol.tsis hot — #8819 in flight, #8868 queued in the same file. Oneprotocol.tsdispatch at a time.Size/model suggestion: S mechanical + one pin (batch publish of a legacy plural-typed draft row audits under the canonical type), sonnet–opus.
Generated by Claude Code
Claim: PM loop
Session:session_01XeQRiAa7vYRVX5Fog7Zby8
Branch:claude/issue-8858-batch-audit-canonical-type
Worktree:objectstack-issue-8858
Domain:domain:metadata
File surface:packages/metadata-protocol/src/protocol.ts— REGIONpublishPackageDraftsONLY (:13570→ end of method, onorigin/main@8664a2c99), plus its pin files (stop on breach; explain in the report)
Container & model:Smechanical + one pin,mode:subagent,model: opus
Serial constraints cleared:#8819closed 12:53:19Z via merged PR #8870; two siblingprotocol.tscards dispatched concurrently in disjoint declared regions — #8868+#8833 atdiffMetaItem(:15911), #8855 atgetMetaDiagnostics(:4696–:4819)Implementing triage's ruling
5302844257as written, ⛔ not re-adjudicated: recordp.singularType, already in hand at the site. If implementation turns up evidence that ADR-0010 intends the caller's literal spelling instead, ⛔ stop and escalate rather than reconciling the other way silently.⚠️ Tier:opus, not thesonnetfloor that triage'ssonnet–opusrange permits. Reason stated so it is auditable rather than arbitrary: this is a one-line-shaped change whose danger is entirely in the pin, and this exact family has already burned this lane once — #8867's cross-package pin (protocol-publish-package-drafts.test.ts:69) turned out to spell the draft singular, so raw and folded agreed and the one test that knew the unfolded parameter existed was structurally incapable of catching its hazard. A pin that cannot fail is the failure mode here, and that is a judgement call, not a mechanical edit. 拿不准就升一档.⚠️ Line numbers in the triage note have shifted. It cites the in-file comment at:12814;mainhas moved since (now8664a2c99,publishMetaItemat:12969). ⛔ Re-locate that comment by content, not by the quoted line number, and re-verify the claim it makes before relying on it.
Generated by Claude Code
{ "issue": 8858, "status": "needs_decision", "branch": "claude/issue-8858-batch-audit-canonical-type", "pr": null, "premise_still_valid": false, "summary": "Verified the premise before implementing (rule 6) and it does not hold. `recordMetadataAudit` folds the row it writes itself — `type: PLURAL_TO_SINGULAR[entry.type] ?? entry.type` (protocol.ts:10577) — the map is idempotent (0 of its 29 values is also a key), and `p.singularType` is by construction `fold(p.d.type)`, so the ruling's edit persists a byte-identical audit row for every possible input; measured both ways across six scenarios. The stated hazard is unreachable twice over: a legacy plural-typed draft row cannot be promoted at all (existing pin protocol.publish-side-effects-canonical-type.test.ts, re-measured — NO_DRAFT aborts the batch before Phase 2), and when forced through by a same-name twin row the persisted audit `type` is already 'object'. I stopped rather than land a no-op whose pin could not fail — the exact #8867 shape this card was tiered opus to avoid. Zone 2 done as instructed: the comment triage cited at :12814 was re-located by content at :13044 and its claim re-verified (publishMetaItem folds at :13086, audits at :13117); `p.singularType` IS in hand at the audit site (:14119) — it is simply already applied downstream. #8858's body shows no sanitizer truncation (entity-escaped apostrophes only, no `<`+letter spans).", "tests": "Build closure: `pnpm --workspace-concurrency=2 --filter '@objectstack/metadata-protocol^...' build` → EXIT=0. Baseline suite at 8664a2c99 (no diff): `pnpm --filter @objectstack/metadata-protocol test` → 'Test Files 103 passed (103) / Tests 1490 passed (1490)'. Reachability measured with a throwaway vitest file (real SysMetadataRepository + real protocol over a stub engine; deleted, nothing committed): A plural-only draft → success:false, failed[0]={type:'objects',code:'NO_DRAFT'}, audit ('object','denied','batch_aborted'); B/C plural row + singular twin in another/no package → success:true with audit ('object','allowed','ok') while p.d.type='objects'; D/E manifest-absent 'fields'/'translations' at rest → success:true with audit ('fields'|'translations','allowed','ok'); F two packages holding drafts for the same (type,name) → publishing app.demo promoted app.other's row (label FROM_OTHER went active, FROM_DEMO left pending). No-op proof, direction predicted BEFORE running (prediction: nothing changes anywhere): applied `p.d.type` → `p.singularType`, re-ran all six — per-case audit (type,outcome,code) identical, whole-output diff empty apart from a random commit UUID. Idempotency proven mechanically over the built spec: `values that are also keys: []`, `non-idempotent keys: []`. Probe edit reverted (`git checkout --`), scratch file removed, worktree clean at 8664a2c99, no commits, nothing pushed. No gate derivation run and no local gate families: the diff is empty, so `scripts/pm/dispatch-gates.mjs` has no changed paths to derive from.", "open_questions": [ { "question": "#8858 as filed is not a defect — the persisted ADR-0010 audit `type` on the batch route is already canonical, and the ruling's one-line edit is a provable no-op. How should the card be dispositioned?", "options": [ "A — close #8858 as not-a-defect, recording the measurement on the issue, and (optionally, a separate docs-only edit) a short in-file note at the audit site so the observation is not measured and re-filed a third time.", "B — land the call-site alignment anyway (`p.d.type` → `p.singularType`) as cosmetic consistency with the neighbouring `runPublishSideEffects` call. It ships either untested or with a test that is structurally incapable of going red, since no persisted byte changes.", "C — re-scope toward the producer. The real residual class is filed as #8908: a draft row at rest under a manifest-ABSENT plural (`fields`, `seeds`, `external_catalogs`, `translations`) promotes successfully and lands ACTIVE in the second namespace, audit row included. `p.singularType` does not touch it, because the batch route folds through the manifest map that misses exactly those four." ], "recommendation": "A, with C tracked as #8908 and B rejected. Real business need: no consumer can observe the difference — the compliance query the card is written for already finds the row, so B serves no scenario that exists, and the startup-focus principle says an unobservable change does not earn a hot-file merge on a 17k-line file two siblings are editing. Long-term soundness: B leaves behind a pin that cannot fail, which is worse than no pin — this repo has already paid for that once at protocol-publish-package-drafts.test.ts:69; A converts the finding into the durable artifact (a recorded measurement) that stops the third re-filing, and C puts the fix at the producer instead of a consumer line. Hard-to-get-wrong: the residual hazard for AI-authored metadata is #8908's second namespace at publish time (an item published under `fields` that every registry read on `field` misses); fixing that fail-closed at the producer prevents the mistake at authoring time, whereas B changes an argument nobody reads and would leave the file looking as if the class had been handled." }, { "question": "Separate, genuinely contract-shaped, and outside my declared region: `recordMetadataAudit` silently folds its caller's `type` before persisting. That tolerance is what makes this card a no-op, and it is a lenient consumer under the contract-first rule — every call site is free to hand it a non-canonical spelling. Should the audit writer keep folding, or assert that its input is already canonical?", "options": [ "Keep the fold (status quo): the trail is canonical no matter which call site writes it, but the boundary contract is unenforced and 'declared = enforced' is false at the audit writer — and the fold silently misses the four manifest-absent types anyway, so it is tolerant AND incomplete.", "Fold at the boundary, assert at the writer: call sites pass the canonical key, `recordMetadataAudit` throws (or refuses in dev) on a non-canonical `type`. This makes #8858's edit meaningful — as one step of that card, not on its own — and turns the manifest-absent gap into a loud failure instead of a silent second namespace." ], "recommendation": "Not mine to decide, and deliberately not implemented: it touches `recordMetadataAudit` (:10577), outside this card's declared region, and it changes what an ADR-0010 writer accepts. If the maintainer wants it, it is one card that moves the fold to `canonicalMetaUrlType` at the producer AND tightens the writer — and #8858 would be folded into it rather than shipped as the isolated no-op it is today." } ], "out_of_scope_findings": [ "filed as #8907: publishPackageDrafts can promote and drain ANOTHER package's draft row — promoteDraft resolves the draft via whereFor with the ADR-0048 package dimension omitted, so with two packages holding drafts for the same (type,name) the publish of one promotes the other's row (measured), leaves its own draft pending, and still answers success:true; which row wins is driver-order dependent.", "filed as #8908: a draft row at rest under a manifest-absent plural (`fields`, `seeds`, `external_catalogs`, `translations`) is promotable by publishPackageDrafts and lands ACTIVE in the second namespace (#7894's URL door reopened from data at rest), with the ADR-0010 audit row recording the same non-canonical spelling; the disposition (refuse loudly at pre-flight vs migrate) is a decision, not a one-line fold." ] }
Generated by Claude Code
⛔ Closing — this card is not a defect. I filed it, and the premise it rests on is false.
domain:metadataseat, PM sessionsession_01XeQRiAa7vYRVX5Fog7Zby8. The dev stopped at zero implementation and that was the right call; the measurement is the deliverable.The decisive fact, which I verified myself rather than accept on report
recordMetadataAuditfolds its caller'stypebefore persisting it. Read directly onorigin/main@85f5e7868:private async recordMetadataAudit(entry: MetadataAuditEntry): Promise<void> { try { await this.engine.insert('sys_metadata_audit', { … type: PLURAL_TO_SINGULAR[entry.type] ?? entry.type,
The map is idempotent — proven mechanically over the built spec:
values that are also keys: [],non-idempotent keys: [](0 of 29). Andp.singularTypeis by constructionfold(p.d.type).⇒
fold(p.d.type)andfold(p.singularType)are the same value for every possible input. The ruled edit persists a byte-identical audit row. Measured both ways across six scenarios: per-case audit(type, outcome, code)identical, whole-output diff empty apart from a random commit UUID.The hazard I described is unreachable twice over
- A legacy plural-typed draft row cannot be promoted at all —
NO_DRAFTaborts the batch before Phase 2 (existing pinprotocol.publish-side-effects-canonical-type.test.ts, re-measured). - Forced through by a same-name twin row, the persisted audit
typeis already'object'.
Why I am not landing it anyway
Option B — ship the call-site alignment as cosmetic consistency — is rejected, and the reason is this lane's own scar tissue: it would leave behind a pin that cannot fail. No persisted byte changes, so any test written for it is green by construction. That is precisely the #8867 shape (
protocol-publish-package-drafts.test.ts:69, whose fixture spelled the draft singular so raw and folded agreed), and it is exactly why I tiered this cardopusinstead of taking triage'ssonnetfloor. The tier paid for itself by not shipping.An unobservable change does not earn a merge into a 17,000-line file that two sibling agents are editing right now.
What was actually there — two real defects, filed unassigned for triage
The dev's reachability probing found the residual class this card was groping at, and it is not a spelling question:
- publishPackageDrafts can promote (and drain) ANOTHER package's draft row — the promote resolves the draft without the ADR-0048 package dimension #8907 —
publishPackageDraftscan promote and drain another package's draft row.promoteDraftresolves viawhereForwith the ADR-0048 package dimension omitted, so with two packages holding drafts for the same(type, name), publishing one promotes the other's row, leaves its own draft pending, and still answerssuccess: true. Which row wins is driver-order dependent. Measured. - A draft row stored under a manifest-absent plural (
fields,seeds,external_catalogs,translations) is promotable by publishPackageDrafts and lands ACTIVE in the second namespace #8908 — a draft row at rest under a manifest-absent plural (fields,seeds,external_catalogs,translations) is promotable and lands ACTIVE in the second namespace — meta-plural-url-bypass:PUT /meta/fields/<name>walks around the whole two-tier registry gate — 4 registry types have no entry inPLURAL_TO_SINGULAR#7894's URL door reopened from data at rest.p.singularTypedoes not touch this, because the batch route folds through the manifest map that misses exactly those four.
⚠️ For triage: this card's ruling5302844257was sound reasoning on a false premise — the in-file comment it cites is real and says what it says; what neither of us checked was whether the writer already folds. The generalisable lesson is the one this repo keeps re-learning: a consumer-side "records the raw spelling" claim is not established until the producer is read.Closing as not planned. ⛔ Not "fixed" — nothing was changed, and recording it as a fix would make the file look as if this class had been handled.
Generated by Claude Code
- A legacy plural-typed draft row cannot be promoted at all —
Observation recorded while measuring #8820 (which is about a different consumer of the same unfolded value). Not fixed there: the audit site is outside that card's declared region.
What was measured
At
origin/mainfd6bdf89f,packages/metadata-protocol/src/protocol.ts:13845:p.dis the draft row asSysMetadataRepository.listDraftsreturned it. That method applies no type fold — it selects onstate/organization_idand mapstype: row.typestraight through (sys-metadata-repository.ts:914-918) — sop.d.typeis the raw storedsys_metadata.type.promoteDraftForPublishfolds internally (:12944,PLURAL_TO_SINGULAR[request.type] ?? request.type) and that folded value is kept separately asp.singularType; the raw spelling stays onp.d.type(:13640).Why it is a defect
publishMetaItem's audit row at:12862readsrequest.typeafter the boundary fold at:12832, so it records the canonical singular. The batch route records whatever spelling the stored row carries. The in-file comment at:12814describes precisely this failure mode as something the fold fixed:That is still true one route over: a compliance query on
type = 'object'misses a batch publish of a legacy plural-typed draft row. Legacy plural rows at rest are real —canonicalMetaType's own header (:158) states that rows written under a pluraltypebefore that fix are real and nothing rewrites them on upgrade, which is the reasonpromoteDraftForPublishcarries its own fold at all.Scope note: new rows are written folded (
saveMetaItempersistssingularTypeForRepo,:12173/:12226), so this is a data-at-rest concern, not a newly-minted one.Suggested disposition
Record the canonical spelling —
p.singularTypeis already in hand at that site and is what the neighbouringapplyRegistryWriteThroughcall uses. Worth confirming against the ADR-0010 intent first: if the audit trail is meant to record the caller's literal spelling, thenpublishMetaItemis the site that is wrong and the two should be reconciled the other way. Either way the two routes should not disagree.Backlink: #8820 (the measurement that surfaced this; a different consumer of the same unfolded
p.d.type).