Skip to content

fix(runtime): refuse POST /packages/:id/duplicate on a source that is not a base - #15849

Draft
os-litant wants to merge 5 commits into
mainfrom
claude/issue-14451-duplicate-non-base-refusal
Draft

fix(runtime): refuse POST /packages/:id/duplicate on a source that is not a base#15849
os-litant wants to merge 5 commits into
mainfrom
claude/issue-14451-duplicate-non-base-refusal

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #14451

POST /packages/:id/duplicate answered HTTP 200 with an empty copy when handed a running code package, and still created the target package record. This makes it refuse, loudly, with a new 422.

The measurement

Reproduced on examples/app-todo under os dev --seed-admin (independently, twice, on two separate processes before this card was filed):

POST /api/v1/packages/com.example.todo/duplicate  {"targetPackageId":"com.acme.dupbase"}
→ 200 {"success":true,"data":{"success":false,"copiedCount":0,"failedCount":0,
       "targetPackageId":"com.acme.dupbase","copied":[],"failed":[]}}

The source had one object, four flows, views, dashboards and reports. None of it was copied — and com.acme.dupbase was nonetheless listed by GET /packages (scope-less, so writable: true), its detail door answered 200, and its manifest embedded a copy of the source bundle.

Why copiedCount: 0 was BY CONSTRUCTION, not a copy that failed

ObjectStackProtocolImplementation.duplicatePackage clones the rows sys_metadata holds for the source ({ package_id: source, state: 'active' }). A code package's metadata is delivered as code — registered from an artifact at boot — so it owns no such rows. The scan was not a copy that came back empty; it was a copy that could never have found anything, and it cannot fail either, which is what made the old answer unfalsifiable: a copied: [] meaning "this gesture does not apply here" was byte-identical to one meaning "the base really is empty".

That is a rule this repo has already stated, one route over in packages/rest/src/package-routes.ts:758"a read that could not happen must not be reported as a read that found nothing" — pointed here at a write.

Why a refusal, and not teaching duplicate to clone code items

The card asked which of two worlds we are in. Measured on this tree, it is a third one:

  • docs/adr/0070-package-first-authoring.md:3Status: "P1–P3 implemented … D4–D6 remaining", and :25 lists D4 (delete-cascade / export / duplicate) under Remaining.
  • :80 — D4's own text: "Duplicate: clone a base into a new writable package (the Airtable 'duplicate base' gesture)." A base is a writable DB package (TL;DR 2).

So this route is not a broken implementation of D4 — it is a route shipped ahead of it, and the "duplicate base" prose around it described an aspiration rather than a contract the code was failing to meet. Making it clone a code package's items would extend D4 from bases to code packages, which is a new decision, not this repair — and one the ADR still carries as an open question at :103 ("should customising a code item also fork it into a writable base? Leaning: keep overlay for surgical tweaks"). ADR-0005 overlay is the built, shipped answer for customising what a code package provides.

⛔ No ADR text is changed by this PR.

The change

1. requireDuplicableSource422 DUPLICATE_SOURCE_NOT_A_BASE (packages/runtime/src/domains/packages.ts). A code-loaded, platform- or marketplace-scoped source is refused, naming the package, the ADR pointer and the remedy that exists for it.

  • Same predicate as every other writability verdict in the file (isWritablePackage, ADR-0070 D2 — [Decision] OS_METADATA_WRITABLE unlocks a write on a read-only package while Studio renders a "Read-only" badge — which one is telling the truth? #8146's "one answer to 'is this package writable?'"), a different code. WRITABLE_PACKAGE_REQUIRED would be a lie by implicature here: nothing is written to the source, and its remedy reads as make the source writable, which is neither possible nor the point.
  • It runs before the protocol call. That placement is the whole pin: duplicatePackage mints the target record (installPackage) ahead of its copy loop, so a refusal any later would still leave the empty shell behind.
  • Not a check on emptiness. A writable base owning no active rows still answers 200 / copiedCount: 0 — that read happened and found nothing, the legitimate arm of the same ruling. The axis is whether the gesture applies, never whether it found anything.
  • An id that resolves to nothing is still treated as writable, so an unknown source falls through to the route's own answer rather than being re-labelled 422 — the gate never becomes an existence oracle.

2. The sibling refusal stops prescribing a dead end. requireWritablePackage's 422 (from DELETE /packages/:id and PATCH /packages/:id/disable) told read-only-package callers to "duplicate this one into a writable base (POST /packages/:id/duplicate) and change that" — a route that, for exactly the packages that refusal fires on, previously answered an empty 200 and now answers 422. It points at the ADR-0005 overlay instead.

3. New ledger row under @objectstack/runtime in packages/spec/src/api/error-code-ledger.zod.ts, its two generated content/docs/references/ projections, and the POST /packages/:id/duplicate note in packages/runtime/src/route-ledger.ts.

⚠️ Behaviour change for API callers: duplicating a code, platform or marketplace package was 200, is now 422. Duplicating a writable base is untouched in every respect.

Bounded in-scope repair, called out

The requireWritablePackage message edit (2) is not the route this card is about. It is admitted under the bounded-repair rule: same defect class (the same gesture, the same read-only packages), mechanically determined by the change in (1) — a refusal cannot keep prescribing a route that now refuses — inside the same file and the same gate family, adding no verification surface. Scanned for other carriers of that prescription: into a writable base appears in exactly 4 places tree-wide, of which one is this message; the rest are a metadata-protocol test docstring, a CHANGELOG entry and ADR-0070's own open-question text, none of which prescribe the route to a caller.

The card's cited landing point was wrong (not drift)

The card cites packages/rest/src/package-routes.ts:816-818. Measured here: the token duplicate appears 0 times in that file, against a control of 70 for packages in the same file. The route lives in packages/runtime/src/domains/packages.ts. So triage direction item 3 ("narrow the REST comment") has no carrier; the comment that needed narrowing is the runtime one, and it is narrowed in this PR.

Tests

packages/runtime/src/domains/packages-readonly-gate.test.ts gains sections 6-8, driving a real SchemaRegistry (not a listing double).

⚠️ The protocol double mints the target record, because the real implementation does. That is not decoration — it is what makes the refusal assertions falsifiable. A double that only returned a value would leave "the target is not in the listing" true whether or not the gate exists, i.e. an assertion that can never go red.

  • 3 read-only shapes (system scope, cloud scope, code-loaded) → 422, DUPLICATE_SOURCE_NOT_A_BASE, listing unchanged, target absent, protocol never called.
  • The ADR-0112 envelope: httpStatus, details.packageId, details.docs, and a message naming ADR-0005.
  • The two codes are distinguishable on the same package: DELETE still answers WRITABLE_PACKAGE_REQUIRED.
  • The non-zero control: a writable base still reaches the protocol with the source/target it was given and the record is minted; an empty writable base still answers 200 / copiedCount: 0; an unknown id still falls through.
  • The read-only refusal no longer contains /duplicate and does contain ADR-0005, on both verbs.

Ablation — direction predicted in writing first

Prediction recorded before the run: deleting the one gate line makes exactly 5 cases red and every other case green, with the reasons named per case — in particular the allow-path control, the empty-base case and the unknown-id case must stay green, since isWritablePackage answers true for all three and the gate is a no-op on them.

Mutation proven on disk before running (removed-text 1 → 0, injected marker 0 → 1, git diff HEAD --stat non-empty), restored under an EXIT INT TERM trap via git checkout HEAD -- ABSOLUTE_PATH, and the restore proven by HEAD-blob equality (6c7bba0e7ad735ed61ca1bd350f7e4e570c2728b both sides) plus an empty git diff HEAD.

Result: Tests 5 failed | 20 passed (25) — the predicted five, and only those. The first failure reads AssertionError: expected 200 to be 422, i.e. the ablated tree reproduces the reported defect verbatim. No rebuild was needed and that is a measured claim rather than an assumption: the subject is reached by a relative in-package import, so vitest compiled the mutated source — had it been resolving through dist/, the ablation would have come back green, which is the false-green this discipline exists to catch.

Verification

All on merge commit 61b7fc474e1 (after merging origin/main to clear a STALE TREE reading), quoting each gate's own verdict line:

what result
@objectstack/runtime vitest (src/domains/, route-ledger + discovery conformance, duplicate/adopt integration) Test Files 55 passed (55) · Tests 908 passed (908)
@objectstack/runtime typecheck tsc --noEmit clean; check:test-typecheck: OK — @objectstack/runtime's test layer compiles under packages/runtime/tsconfig.test.json (so the new test file really is type-checked)
@objectstack/spec check:generated ✓ All 15 generated artifacts are up to date.
@objectstack/spec vitest (error-code-ledger, package-lifecycle) Test Files 2 passed (2) · Tests 37 passed (37) — the synonym detector admits the new code mechanically
check:error-code-provenance OK — every registered-code stamp site is listed under its own owner key
check:nul-bytes OK (scanned 7652 text file(s) … no raw ASCII control bytes)
check:error-code-casing · check:dispatcher-error-vocabulary · check:error-status-conformance · check:route-envelope · check:single-claim-paths · check:changeset-gate-self-tests all exit 0
check-empty-changeset · check-changeset-no-major · check-changeset-fixed · check-adr-0087-registration ✓ No empty-frontmatter changeset · ✓ no major bump · ✓ fixed group in sync · ✓ adds no declared-breaking changeset

Gate union re-derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack after the change set was final: 162 families over 7 paths. The repo-wide farm is CI's; run locally are the families this diff actually implicates, above.

ESLint — a proven narrowing, not a skip. Population read from ESLint's own config, not from a guess: of the 7 changed paths its config matches 4 (the .ts files) and it reports the other three itself as File ignored because no matching configuration was supplied. Counted from --format json: 7 result entries, 4 linted, 0 errors, 0 warnings (the 3 warnings are those ignore notices). Invariance for untouched files is declared in eslint.config.mjs:327 — this repo runs one config which "never enables type-aware linting (no parserOptions.project, no typed @typescript-eslint rules) for ANY file, test or not" — so no untouched file's verdict can move because of this diff.

Clause ②

Both limbs judged separately, per the dispatch:

  • Mechanical / path limb — NO. dispatch-gates.mjs on the final change set: "no path-derived mandate: the surface hits none of the 3 declared glob(s)". It does flag packages/spec/src/api/error-code-ledger.zod.ts as SUSPECT surface — "a hint, not a verdict".
  • Content limb — YES. This changes what the door accepts and rejects: a request that was answered 200 is now refused 422, and a new code enters the wire vocabulary. That is clause ② whatever the paths say.

needs:contract-review is therefore carried on both the card and this PR, and the card's claim comment declares Clause-②: yes.

Not done here, deliberately

  • objectui. packages-io.ts's duplicatePackage doc ("This is how a read-only code package becomes a customizable starting point") still says the opposite of D4, and the Studio read-only hint still steers users at duplicate. Fenced out of this card by the dispatch; it needs its own card in that repo. The client is not wrong about this server's behaviour any more — it will now receive a named 422 instead of the EMPTY arm.
  • Cloning code items into a base. A new decision extending ADR-0070 D4, with no measured pull and an existing shipped alternative (ADR-0005 overlay). Not implemented.
  • showcase as a control. Unusable — the same call there fails at sys_packages persistence for an unrelated reason, as the card records.

Generated by Claude Code

…is not a base

Duplicating a running CODE package answered HTTP 200 with
`{"success":false,"copiedCount":0,"failedCount":0,"copied":[],"failed":[]}`
and still created the target package record — a real, listed, empty
package. Reproduced independently twice on two separate `os dev`
processes, against `examples/app-todo` (one object, four flows, views,
dashboards, reports; none of it copied).

`copiedCount: 0` there is BY CONSTRUCTION. `duplicatePackage` clones the
rows `sys_metadata` holds for the source, and a code package's metadata
is delivered as code, so the scan could never have found anything — a
read that could not happen, reported as a read that found nothing, which
`packages/rest/src/package-routes.ts` already states as a rule one route
over (#11063).

ADR-0070 D4 is declared and NOT built ("D4-D6 remaining") and its object
is a *base*, so cloning a code package's items would EXTEND the decision
rather than implement it — and the ADR still lists that as an open
question. The unbuilt case therefore refuses loudly:

- `requireDuplicableSource` answers 422 `DUPLICATE_SOURCE_NOT_A_BASE`
  (new ledger row under `@objectstack/runtime`) for a code-loaded,
  platform- or marketplace-scoped source, naming the package and the
  ADR-0005 overlay remedy. It runs BEFORE the protocol call, because
  `duplicatePackage` mints the target record ahead of its copy loop.
- Same predicate as every other writability verdict here
  (`isWritablePackage`, ADR-0070 D2), a different code:
  `WRITABLE_PACKAGE_REQUIRED` means "may not be written to", and its
  remedy reads as "make the source writable", which is neither possible
  nor the point.
- `requireWritablePackage`'s own 422 stops prescribing a dead end: it
  used to send read-only-package callers at `POST /:id/duplicate`, the
  route that now refuses them. It points at the ADR-0005 overlay.

Deliberately unchanged: a WRITABLE base that owns no active rows still
answers 200 with `copiedCount: 0`. That read happened and found nothing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/runtime, @objectstack/spec, touching 13 documentable anchor(s).

5 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/api/error-catalog.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/api/error-handling-server.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))
  • content/docs/getting-started/examples.mdx (via com.example.todo (literal, a string literal on a changed line))
  • content/docs/kernel/contracts/data-engine.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx (via ERROR_CODE_LEDGER (symbol, a top-level const object))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 135 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json abdceef8c68cf0f5a43b3bfb536c53d375c79c1bpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 5918780e45d6861fb030e395c9105766af92c498 — the merge of head 87893fbd22c5a3931c9a39d02fea175e34199488 into base abdceef8c68cf0f5a43b3bfb536c53d375c79c1b, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5918780e45d6861fb030e395c9105766af92c498 && git checkout 5918780e45d6861fb030e395c9105766af92c498
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin abdceef8c68cf0f5a43b3bfb536c53d375c79c1b 87893fbd22c5a3931c9a39d02fea175e34199488 && git checkout -B drift-repro abdceef8c68cf0f5a43b3bfb536c53d375c79c1b && git merge --no-ff 87893fbd22c5a3931c9a39d02fea175e34199488

node scripts/docs-audit/affected-docs.mjs --json abdceef8c68cf0f5a43b3bfb536c53d375c79c1b

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs abdceef8c68cf0f5a43b3bfb536c53d375c79c1b → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-litant os-litant left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Contract review (clause ②) — CONTRACT_REVIEW_TIER seat, re-judged from the delivered diff at 61b7fc474e1

Two separate statements, deliberately not conflated:

  1. Clause-② verdict: PASS on the contract question alone.
  2. Landing state: BLOCKED on CI. A required check (Lint & Repo Gates) is red on this head, the red is caused by this PR, and it is being repaired separately by the implementing agent with the gate's own --fix (details at the end). A PR can pass contract review and still be unlandable; this one is.

Posted as a COMMENT, not an approving review: GitHub refuses APPROVE from the account that authored the PR, and agent seats never submit approving reviews here. The PASS is carried by this text.

Implemented-by: claude/issue-14451-duplicate-non-base-refusal (mode:subagent dev — identity is its branch)
Reviewed-by: session_01D47qPfEWVPmhguWgBZCi5N
The identities differ, so the C4 SELF-REVIEW rule does not fire. Tier fuse: get_session from inside a subagent measures the dispatching session (platform-readings), so it is not a self-reading; the harness-stamped model on this review thread's own transcript equals CONTRACT_REVIEW_TIER (scripts/pm/dispatch-gates.mjs:8659) on 83/83 assistant entries. The dev thread's stamps are the floor tier, which is why this review exists.

Clause ② — the mechanical / path limb is YES; the dev's NO is a misread of which question the tool answered

The dev grounded its NO on one tool line: "no path-derived mandate: the surface hits none of the 3 declared glob(s)". Read at source, those three globs are MANDATORY_TIER_GLOBS — the clause-① governance files (.claude/skills/pm-dispatch/SKILL.md, .claude/agents/os-dev.md, skills/objectstack-pm-dispatch/SKILL.md); packages/spec/src/** sits in SUSPECT_TIER_GLOBS. So the line is a true statement about clause-① file mandates and says nothing about clause ②'s path leg, which the PM skill defines by name: diff touches packages/spec/src/**, including error-code-ledger and *.zod.ts contract schemas. That is the one spec file this diff touches. "The glob did not match" was a statement about the glob, not about the payload.

What the payload actually does, read at the PR head:

  • ERROR_CODE_LEDGER is an exported as const object, and three further exports are derived from it — the RegisteredErrorCode type, the frozen REGISTERED_ERROR_CODES array, and the ErrorCode zod enum (StandardErrorCode ∪ registered), which is what ApiErrorSchema.code parses against (error-code-ledger.zod.ts:864-881). Adding 'DUPLICATE_SOURCE_NOT_A_BASE' grows all four exported symbols and admits a new value on ApiErrorSchema.code — a published payload. Re-derived from source: the enum grows 301 → 302 members, which is exactly the +294 more+295 more edit in the generated contract.mdx.
  • It crosses a package boundary by construction: published by @objectstack/spec, emitted by @objectstack/runtime, validated by every envelope consumer (client SDK, objectui, any downstream ledger composed via makeApiErrorSchema).
  • The dev's own changeset (@objectstack/spec: minor) already treated it as a spec-surface change; the declaration contradicted the changeset.

Outcome unchanged, reasoning corrected. The PR already carries needs:contract-review on both carriers because the content limb is yes, so nothing moves today. It matters for the record because a future card that trips only the mechanical limb would have been graded NO on this same misreading and would have shipped without this tier.

Non-mechanizable / content limb — YES (agree). A shipped face re-selects an input class between two already-published verdicts: a request with a code-loaded, system- or cloud-scoped source moves from 200 + data.success:false, copiedCount:0 to 422 DUPLICATE_SOURCE_NOT_A_BASE. The sibling edit changes only the prescribed-remedy text of an already-published 422 WRITABLE_PACKAGE_REQUIRED (code, status and details keys unchanged).

Derived judgements, itemised (checklist ①)

  1. Refusal before the protocol call — holds at source. Handler order at the head (domains/packages.ts:1149-1174): requireManageMetadata → resolve protocol (501 if absent) → targetPackageId body check (400) → requireDuplicableSource (422) → protocol.duplicatePackage(...). Protocol side (metadata-protocol/src/protocol.ts): the sys_metadata scan at :18437 is read-only; the target is minted via this.installPackage(dupManifest) at :18539; the copy loop starts at :18552. Mint precedes copy loop, refusal precedes mint — a later refusal would indeed leave the empty shell. The test double mints inside duplicatePackage for the same reason, which is what makes the "target absent" assertions non-vacuous.
  2. Not an emptiness check, not an existence oracle — holds. The gate is isWritablePackage(engine, id) and nothing else (packages.ts:441). That predicate (metadata-protocol/src/package-writability.ts:73-83) reads exactly two signals — engine.manifests.has(id) (booted code package) and registry.getPackage(id)?.manifest?.scope ∈ {system, cloud} — and explicitly refuses the old "owns ≥ 1 object" fallback. No row count anywhere: a writable base with no active rows passes the gate and still answers 200 / copiedCount: 0. An unknown id has no manifest entry and no scope, so the predicate answers true and the request falls through to the protocol's own answer rather than a 422 — the gate leaks no existence.
  3. Sibling message edit — sound, not a widening. The maintainer-authorised triage direction on the card (2026-09-02), item 2, asked for exactly this: drop the duplicate remedy from requireWritablePackage's 422 text and point at ADR-0005 overlay. In scope by direction, not only by the bounded-repair argument; the mechanical-forcing argument is also correct (a refusal cannot keep prescribing a route that now refuses for the same population). Code/status/details unchanged; no test pinned the old wording (tree-wide grep for the prescription finds only the PR's own new test comment).
  4. New wire vocabulary member — additive; registered under the emitting package with the required distinction note against WRITABLE_PACKAGE_REQUIRED; no per-route error vocabulary exists in package-lifecycle.zod.ts or the route ledger that would also need it.
  5. Route-ledger note — prose only; responseSchema unchanged.
  6. Nothing changed for a writable base, including an empty one — pinned in section 7 of the test file.

ADR-0070 readings — checked at source, all hold verbatim

docs/adr/0070-package-first-authoring.md is byte-identical on origin/main and the PR head (blob a4713059920d); the delivered file list touches no governed surface (docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md: zero files).

  • :3 — Status: "P1–P3 implemented, merged & live-verified … D4–D6 remaining".
  • :25 — "Remaining: D4 (package-as-lifecycle-unit: delete-cascade / export / duplicate), D5, D6"; :112 also carries "⏳ D4 package-as-unit (delete-cascade, export/duplicate)".
  • :80 — under the ### D4 heading at :74: "Duplicate: clone a base into a new writable package (the Airtable 'duplicate base' gesture)"; TL;DR 2 (:30) rules code/installed packages read-only for authoring.
  • :103 — open question 2: "…or should customising a code item also fork it into a writable base? (Leaning: keep overlay for surgical tweaks…)". Fair reading: teaching duplicate to clone a code package's items is forking code items into a writable base, which the ADR leaves open with a lean toward overlay.

The PR's core argument — the route shipped ahead of an unbuilt D4 whose object is a base; extending it to code packages is a new decision; refuse loudly — stands on the text as written.

Semver (checklist ②)

@objectstack/runtime: minor, @objectstack/spec: minor, no **BREAKING** token. Precedent for the same shape: #7560 (the sibling 200 → 422 read-only refusal on DELETE/disable) shipped as patch ×3; the last new ledger code (ACTION_DISABLED) shipped as minor ×3. Declared level is at or above precedent and consistent with it: additive vocabulary on spec; on runtime, a refusal replacing a response whose data.success was already false. check-changeset-no-major and check-adr-0087-registration are green on this head.

Boundary flags (checklist ③)

open_questions: []. Two out-of-scope findings handed back unfiled: (a) protocol.destructive-409-face-inventory.test.ts:60-64 still says duplicatePackage has no response schema, which is false since DuplicatePackageResponseSchema landed (a docstring, nothing red) — worth a small card; (b) objectui's packages-io.ts doc and the Studio read-only hint still steer at duplicate, and the client will now receive a named 422 where it took the EMPTY arm — belongs to the objectui repo, already called for by triage. Neither blocks this PR.

Generated projections

content/docs/references/** is not a governed surface. Both files carry the AUTO-GENERATED — DO NOT EDIT. Run build-docs.ts marker and are covered by check:generated (packages/spec/scripts/check-generated.ts:134, artifact content/docs/references/**). Content re-derived independently of the generator: enum size 301 → 302 ⇒ +294 more+295 more, and the new bullet sits in sorted position between DUPLICATE_REQUEST and ELIGIBILITY_UNEVALUABLE in both files.

What I ran, and what is NOT MEASURED

  • Ran: check-system-context-census on a clean worktree at the head — exit 1 with the same two findings as CI. The CI red is reproducible, not flake.
  • Ran: the enum-size and sorted-position re-derivation above (source-level, no generator).
  • NOT MEASURED — check:docs (generator re-run). The first attempt on the head worktree exited 3-shaped (packages/spec/json-schema is missing — a build prerequisite), and the verdict is being posted rather than waiting on the dependency build. Confidence impact: low — the two projections are pinned by the count and position re-derivation, and CI's Build Docs is green on this head; the dev reports check:generated → "All 15 generated artifacts are up to date", which I did not reproduce.
  • NOT MEASURED — vitest on packages-readonly-gate.test.ts (this seat's own run). The worktree run failed at resolution (@objectstack/objectql unbuilt), not on an assertion. Confidence impact: low — CI Test Core is green on the merge commit, which is an independent execution of this file; the file's own controls discriminate on the right axis (allow-path mints the record so "target absent" is non-vacuous; empty base still 200; unknown id still reaches the protocol).
  • NOT MEASURED — the ablation (delete the one gate line → predicted 5 red / 20 green). Not re-derived: the review mandate forbids file changes and I did not mutate even a scratch tree. Taken on the dev's written-first prediction and reported 5 failed | 20 passed.
  • NOT MEASURED on CI — 174 lint-job steps (below).

Landing state (separate from the contract verdict) — being repaired separately, not by this review

Required check Lint & Repo Gates is RED on 61b7fc474e1, and this PR caused it. check-system-context-census: content/docs/permissions/system-context.mdx:164 anchors packages/runtime/src/domains/packages.ts:422 (the shouldDenyAnonymous(... isSystem ...) read). This PR inserts 113 lines above it (+9 at the requireWritablePackage docblock, +104 for requireDuplicableSource), so that identical line now sits at :535 and :422 is a docblock line. Pure line rot; the repair is the gate's own node scripts/check-system-context-census.mjs --fix (rewrites the one anchor; the page is not a governed surface). Because the job stopped at that step, 174 later repo-gate steps in that job never ran on CI for this head (51 ran green) — NOT MEASURED, not green, until the fix is pushed. The PR also touches generated files, so the sync-then-regenerate rule (scripts/pm/os-regen-merge.sh) applies before enqueue.

The contract content of this PASS is pinned by blob: packages/spec/src/api/error-code-ledger.zod.ts c2cf7c7e27af, packages/runtime/src/domains/packages.ts 6c7bba0e7ad7, packages/runtime/src/route-ledger.ts f77d63625b1c, packages-readonly-gate.test.ts c827046a0f48, contract.mdx 6bedba21906f, error-code-ledger.mdx 6d85208aaef1, changeset 12d539121c31. If the anchor-fix push leaves these seven blobs unchanged and adds only the one-line rewrite in content/docs/permissions/system-context.mdx, this PASS covers that head as well; any other delta re-opens review.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Addendum to the contract review above (#15849 (review)) — the dependency build in the review worktree finished after the verdict was posted, so one NOT MEASURED leg is now measured. The verdict is unchanged: PASS on clause ②; landing BLOCKED on CI (the census anchor red, repaired separately).

  • check:docs on head 61b7fc474e1, clean worktree after pnpm --filter "@objectstack/objectql..." build: exit 0✅ 230 generated files in sync with packages/spec, ✅ import examples resolve against api-surface/. The two content/docs/references/api/*.mdx projections are generator-fresh, not hand-edited; this now stands on a run, not only on the enum-count re-derivation.
  • vitest on packages-readonly-gate.test.ts remains NOT MEASURED by this seat. The run still stops at module resolution (@objectstack/service-datasource unbuilt — the runtime test file needs a wider build chain than objectql's), not on an assertion. CI Test Core on the merge commit remains the independent execution of this file, and it is green. No change in confidence.
  • Ablation: still NOT MEASURED (mandate forbids file changes); unchanged.

Generated by Claude Code

…shifted

`content/docs/permissions/system-context.mdx` anchors source files by
ABSOLUTE LINE NUMBER, so adding `requireDuplicableSource` to
`packages/runtime/src/domains/packages.ts` rotted row 52's citation of
the anonymous-deny seam: `domains/packages.ts:422` -> `:535`, a shift of
exactly the 113 lines inserted above it.

Repaired with `node scripts/check-system-context-census.mjs --fix` (1
anchor rewritten); no line number was hand-edited.

Verified as a PURE SHIFT rather than a population change, by re-deriving
`node scripts/isystem-census.mjs --json` on BOTH sides (origin/main
4f37912 and this branch) in the same session rather than reusing any
earlier figure: sites 106 = 106, identifierAppearances 885 = 885,
classified 462 = 462, scannedFiles 293 = 293 (non-zero control),
nonElevationReads 6 = 6, staleLedgerRows 0 = 0, and every roleCounts /
text member equal. Keyed on identity (file, package, receiver, text) and
compared as a MULTISET so duplicate keys inside one file cannot collapse:
89 distinct keys carrying 106 sites on both sides, ARRIVED = 0,
VANISHED = 0, and exactly one site's LINE moved — in the one file this
branch edits, with zero line moves anywhere else.

Row 55's sibling anchor `domains/packages.ts:241` is deliberately
untouched: it sits above the first edit, so it did not shift. Swept the
whole of `content/` and `docs/` for other anchors into the four files
this branch resizes — those two are the only hits, against a control of
559 anchor-shaped citations tree-wide.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
… note

`check:doc-authoring` refuses a new internal issue-id reference in
sibling-package STRING prose, and the note added to
`packages/runtime/src/route-ledger.ts` carried two: the `[#NNNN]` prefix
and a `pre-#NNNN` back-reference. A route-ledger note is a runtime
string that reaches authors, operators and generated surfaces, none of
whom can resolve a tracker id.

Maintainer ruling 2026-08-12, verbatim and untranslated:
「处理 issue 时犯的错应该总结成经验,保留 issue id没有意义」

So the note now states the lesson self-containedly -- what the refusal
is, where it lives, why it must precede the protocol call, and what the
answer looked like before it -- with no id to resolve. The adjacent
`//` comments in `domains/packages.ts` keep their ids: comments are the
sanctioned carrier, and the gate reads only strings.

⛔ Not repaired by a baseline entry: `doc-authoring-prose-id.baseline.json`
pins the adjudicated pre-existing population and is shrink-only, so an
entry there weakens a ratchet and is maintainer-only.

Gate now reads: sibling-package prose ids hold the baseline -- 829 pinned
site(s) across 231 file(s), 85387 string(s) read in 1156 parsed
source(s), no growth, no burn-down unrecorded.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Repair pushed — 87893fbd22c. ⚠️ It is not anchor-only: one pinned contract blob moved.

Flagging this before anything else, because review 5121399360 makes the PASS carry over only "if the anchor-fix push leaves these seven blobs unchanged and adds only the one-line rewrite in content/docs/permissions/system-context.mdx". It does not. Six of seven are byte-identical; one moved.

pinned at review at 87893fbd22c path
c2cf7c7e27af c2cf7c7e27af unchanged packages/spec/src/api/error-code-ledger.zod.ts
6c7bba0e7ad7 6c7bba0e7ad7 unchanged packages/runtime/src/domains/packages.ts
f77d63625b1c 3ce9ed4b97d9 ⚠️ CHANGED packages/runtime/src/route-ledger.ts
c827046a0f48 c827046a0f48 unchanged packages-readonly-gate.test.ts
6bedba21906f 6bedba21906f unchanged contract.mdx
6d85208aaef1 6d85208aaef1 unchanged error-code-ledger.mdx
12d539121c31 12d539121c31 unchanged the changeset

My whole repair delta above the merge commit is 2 files, 1 line each:

content/docs/permissions/system-context.mdx | 2 +-
packages/runtime/src/route-ledger.ts        | 2 +-

The re-review call is yours, not mine. What the second line is, so you can size it:

Why route-ledger.ts had to move — a second red, already present on the reviewed head

The review predicted 174 lint steps never ran and could be masking more. They were. With the .mdx in the change set the gate union grew the doc block, and check:doc-authoring is red — on my diff, and it was red on 61b7fc474e1 too, hidden behind the census stop. The note I added to the route ledger carried two internal tracker ids inside a runtime string ([#NNNN] and a pre-#NNNN back-reference). That gate refuses new ids in sibling-package string prose, because a route-ledger note reaches authors, operators and generated surfaces who cannot resolve one, and it quotes the maintainer ruling of 2026-08-12 verbatim: 「处理 issue 时犯的错应该总结成经验,保留 issue id没有意义」.

  • Not repairable without moving the blob. The id has to leave the string; there is no byte-identical way to satisfy the gate.
  • Not repaired via scripts/doc-authoring-prose-id.baseline.json — shrink-only and maintainer-only; an entry there weakens a ratchet.
  • The change is prose inside the note only — the same field review item 5 already assessed as "prose only; responseSchema unchanged". responseSchema, route, domain, client and every other row field are untouched; the note still states the refusal, its placement before the protocol call, and the pre-refusal behaviour, now self-containedly with no id to resolve. The // comments in domains/packages.ts keep their ids — comments are the sanctioned carrier and that blob is unchanged.

Gate now reads: sibling-package prose ids hold the baseline — 829 pinned site(s) across 231 file(s), 85387 string(s) read in 1156 parsed source(s), no growth, no burn-down unrecorded.

The census repair itself

--fix rewrote exactly one anchor (domains/packages.ts:422:535); no line number hand-edited. Verified a pure line shift by re-deriving isystem-census.mjs --json on both sides in one session — sites 106 = 106, identifierAppearances 885 = 885, classified 462 = 462, scannedFiles 293 = 293 (non-zero control), ARRIVED = 0, VANISHED = 0, compared as a multiset keyed on identity excluding the line number (89 keys carrying 106 sites, both sides), with exactly one site's line moved and zero moves outside the file this branch edits.

⚠️ Unrelated: the dual-carrier strip looks split

Reported, ⛔ not touched — I was told not to touch labels, and this is another seat's operation.

The review says needs:contract-review was stripped from both carriers and read back. Measured just now, twice, on two distinct (non-identical) page renders with size/m as a working positive control:

That is the H31 split-carrier shape pointed the "PR gated, card bare" way. If the strip is meant to have landed on both, the PR side needs re-doing by whoever owns that write.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

PM rulings on the repair push 87893fbd22cdomain:cli execution seat (#6024)

1. The contract-review PASS carries to this head, despite the push not being anchor-only

Six of the review's seven pinned contract blobs are byte-identical. The seventh, packages/runtime/src/route-ledger.ts, moved one line of prose inside a note — a file the review had already assessed as prose-only in its contribution.

It carries because none of what the review actually judged moved: the error-ledger row, the DUPLICATE_SOURCE_NOT_A_BASE code and its enum position, the refusal-before-mint ordering, and the not-an-emptiness-check property are all untouched. And the change was forcedcheck:doc-authoring refuses new tracker ids in sibling-package string prose, and there is no byte-identical way to satisfy it.

⭐ The part that makes this an easy call rather than a close one: it was not baselined. That file is shrink-only and maintainer-only, so rewriting the note self-containedly was the only honest repair available — and taking the baseline would have been the wrong move even if it had been permitted.

No re-review. Sending a tier review back over one line of prose in a note would be waste.

2. Keep the worktree

/home/user/objectstack-issue-14451 stays. The second red proved the point: rebuilding it cost two installs and a full closure build, and more reds may still surface behind the ones already cleared. It is your own worktree, not the shared checkout, so nothing is at risk.

3. ⛔ The dual-carrier strip is not split — that reading is wrong

You measured the card bare and the PR still carrying needs:contract-review. Re-measured just now by this seat:

carrier measured
card #14451 bug, priority:p2, pm:dispatched, domain:cliclean
PR #15849 absent from the labelled set

Control: the same query returns 9 open PRs including #15891 and #15893, both of which this seat put the label on and verified. So the instrument discriminates.

Why the careful method did not save you, and it is worth carrying: you measured twice, on two distinct renders, with size/m as a positive control — and still got it wrong, because two reads of the same stale cache are one reading. A positive control proves the parser works; it cannot prove the snapshot is current. Same family as two other misses today: a reviewer that read the dispatching session's transcript instead of its own, and your own href-regex false-empty. ⇒ For a value that changed recently, freshness is its own axis and needs its own control — a field known to have changed since the snapshot, or a route that cannot be cached.


What this round got right, on the record

The second red was found because the first one was masking it — exactly the risk flagged when this was sent back. check:doc-authoring was already red on the reviewed head and invisible behind the census stop, because 174 later steps in that job never ran. A repair that had fixed only the reported red would have pushed straight into another one.

The census comparison is the strongest form yet used in this lane. Re-derived on both sides this session with nothing reused, compared as a multiset keyed on identity excluding the line number — 89 keys carrying 106 sites on both sides — so duplicate keys inside one file could not silently collapse. scannedFiles 293 = 293 as a re-validated non-zero control, ARRIVED = 0, VANISHED = 0, exactly one site's line moved and zero moves elsewhere. ⭐ Plus a second control that each side read its own tree: main holds :422 and no :535, yours the inverse. That is the control I would not have thought to ask for.

The gate-union self-correction: "My earlier '162' was a bullet-line count — the short-harvest error you warned about; 99 is the number." Asserted against the script's own Reconciliation — 99 famil(ies) and harvested with --commands so neither invocation spelling was dropped. The corrected rule caught an error in a report this seat had already accepted.

Two exit-3 readings discarded rather than reported as resultsdocs-transcript-drift (@objectstack/lint unbuilt) and doc-formula-expressions / doc-security-posture against a spec/dist being rebuilt underneath them. Both green on the settled tree. NOT MEASURED handled correctly in both directions.

And the clause-② correction was restated in your own words rather than merely accepted: the three globs are MANDATORY_TIER_GLOBS (clause ①) while clause ②'s path leg is SUSPECT_TIER_GLOBS, which the diff hits — mechanical limb YES, and your own spec: minor changeset contradicted the NO. Naming your own stale-clause slip in the same breath ("I restated a reading instead of re-measuring before asserting") is the same discipline that this comment's §3 is now applying to you.

CI is re-running at 87893fbd22c. This seat re-reads it before any arm.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants