Skip to content

feat(cli): os meta delete can pin its reset and can discard only the pending draft - #13346

Merged
os-trump merged 3 commits into
mainfrom
claude/issue-13024-meta-delete-pin-and-draft-flags
Aug 30, 2026
Merged

feat(cli): os meta delete can pin its reset and can discard only the pending draft#13346
os-trump merged 3 commits into
mainfrom
claude/issue-13024-meta-delete-pin-and-draft-flags

Conversation

@os-trump

Copy link
Copy Markdown
Collaborator

Fixes#13024

packages/cli/src/commands/meta/delete.ts was the only in-repo caller of the SDK reset and called it with two arguments, so every CLI reset was the unpinned, full one — unpinned (a concurrent edit silently destroyed instead of 409 metadata_conflict, ADR-0008) and full (the published overlay dropped along with any pending draft). DeleteMetaItemOptions (ifMatch, state) landed on bothdeleteItem declarations in #12181; only the CLI surface was missing.

The flag spelling, and the convention that decides it

--if-match VERSION — a Flags.string, like every other value-bearing flag in this CLI. It carries an opaque token; it is forwarded verbatim (DeleteMetaItemOptions.ifMatch: "echo it verbatim, never parse it").

--draft — a Flags.boolean, not--state active|draft. Measured against the sibling commands rather than chosen by taste:

  • Every value-listing flag in packages/cli/src/commands enumerates three or more members with a meaningful non-boolean default: --format json|table|yaml (14 commands), --visibility private|org|marketplace, --log-level (6), --package-manager npm|pnpm|yarn|bun, --observability minimal|default|full. There is no two-valued options: [...] flag anywhere in the tree.
  • Binary opt-ins that default off are Flags.boolean throughout — 115 of them, including --dry-run, --apply, --strict, --pre-release, --submit, --fresh, --step, --stored.
  • The carrier itself is binary by construction: state?: 'active' | 'draft' where 'active' deliberately sends nothing (the door acts on state=draft alone and the SDK drops the other spelling). --state active would be a CLI value with no effect on the wire.

--state is not merely undocumented — it is refused by the parser, pinned beside a positive control that --draft is accepted.

FROM to TO — what the CLI sends

runpathqueryrequest headers
FROM (any run, before)/api/v1/meta/TYPE/NAME(none)content-type
TO — no flags/api/v1/meta/TYPE/NAME(none)content-type
TO--if-match TOKENunchanged(none)content-type, if-match: TOKEN
TO--draftunchanged?state=draftcontent-type
TO — bothunchanged?state=draftcontent-type, if-match: TOKEN

A flagless run is byte-identical to what this command always sent: metaDeleteOptions returns undefined (never {}), the SDK's query builder returns the empty string and its header builder returns undefined, so the headers key is omitted entirely. Pinned as a measurement, not as prose — the unflagged case asserts search === '' and the exact header-name set.

Driven evidence, not asserted predicates

New file packages/cli/src/commands/meta/delete-reset-carriers.test.ts — 20 cases — boots a real stack (real ObjectQL on a real SQLite database, real sys_metadata / sys_metadata_history / sys_metadata_audit tables created by real DDL, real ObjectStackProtocolImplementation, real RestServer routes) and drives the real oclif command, argv in and printed output out. Two seams are stubbed, both the credential boundary: createApiClient (replaced with a real ObjectStackClient whose fetch bridges into the registered handlers) and resolveExecCtx ("better-auth says this bearer holds manage_metadata"), the same seam every neighbouring /meta door test stubs.

The pin, both directions:

  • unpinned, stale (the BEFORE state): os meta delete view race_probe exits 0, prints Metadata deleted: view/race_probe, and the other author's row is gone (sys_metadata rows 1 to 0). The probe on protocol.deleteMetaItem shows noparentVersion key.
  • pinned, stale: the same reset with --if-match STALE_TOKEN exits 1, prints [metadata_conflict] view/race_probe has been modified since you loaded it. Expected parent sha256:... but current is sha256:..., and the other author's row survives (1 row). The probe shows parentVersion equal to the stale token, carried as the if-matchheader with no ?ifMatch= on the URL. Asserted on the envelope the operator branches on, never as a bare "something failed".
  • pinned, current: --if-match CURRENT_VERSION exits 0 and the row goes — so the pin refuses a stale write, not every write. Without this case "always 409" would pass the one above.
  • --format json carries the same refusal machine-readably: {"success": false, "error": "[metadata_conflict] ..."}, exit 1, row intact.

--draft, against the full reset:

  • A published overlay plus a pending draft is 2 rows (state values active and draft). os meta delete view draft_probe --draft puts ?state=draft on the wire, the door threads state: 'draft' into the protocol call, and afterwards 1 row remains — state: 'active'. The published overlay is still serving.
  • The same item then reset without the flag drops that published overlay too (1 to 0), and the same probe shows stateabsent on that request — the positive control that makes the presence assertion above mean something.
  • Both carriers at once: ?state=draft and if-match together, parentVersion and state both threaded, only the draft discarded.

The CLI's own refusal. An empty--if-match is refused before the client is built. os meta delete view v --if-match "$VERSION" with VERSION unset expands to an empty argument, and the SDK's rule for that layer (an empty token omits the header, because the door reads the header's presence as "pin this") would have run the unpinned reset while the operator's command line reads as pinned — exactly the silent destruction this card exists to prevent. Pinned as a refusal: exit 1, the refusal text in both human and --format json output, zerocreateApiClient calls, zero protocol calls, and the row still there — beside a positive control in which the same two instruments both move on a run that is allowed through. A whitespace-only pin is refused the same way; a non-empty token with surrounding whitespace is forwarded verbatim.

Human-mode wording. A --draft run now prints Pending draft discarded: TYPE/NAME. The old sentence would be a false report of what happened on the run where the operator deliberately chose the narrower verb. json/yaml payload keys are unchanged.

Both SDK twins

Measured, this command reaches exactly one of the two textually identical deleteItem declarations: createApiClient builds a plain ObjectStackClient and passes any environment id as a config value (sent as the X-Environment-Id header), and run() holds client.meta — the unscoped namespace. That is asserted as a measurement, not assumed: the command's reset lands on /api/v1/meta/view/mount_probe, with no /environments/ segment.

So the obligation is discharged by driving the bag this command builds through the other twin, against the same booted door — which is why metaDeleteOptions is an exported function rather than an object literal inside run():

  • the same --if-match STALE bag through client.environment('env-13024').meta.deleteItem answers code: 'METADATA_CONFLICT', httpStatus: 409, the row survives, and the probe shows the same parentVersion;
  • the same --draft bag through the scoped twin discards only the draft and leaves state: 'active' serving;
  • an IN STEP pair compares the two twins against each other rather than restating a literal: identical query (?state=draft), identical if-match header, differing in exactly one way — the path prefix. And unflagged, both send an empty query and the identical header-name set.

Scope fences, held

  • "Default to pinning" is NOT decided here, in either direction. Whether os meta delete should read the current version first and pin automatically is a contract question, not wiring, and is not covered by the inherited ruling. Only the explicit flags are implemented. Nothing in the work made defaulting look necessary — the two behaviours are cleanly separable, and the pin is opt-in on the door and on the SDK too.
  • ?dropStorage stays withheld. No flag, under either spelling, pinned with a positive control that the two flags that are declared are found by the same lookup.

Reverse verification

Direction predicted before each run; the implementation was committed first so each restore had a real reference, and each leg proved the mutation and the restore on disk by blob hash (git hash-object against git rev-parse HEAD:PATH, plus an empty git diff HEAD), never by an exit code. No dist/ leg: the test imports the command relatively, so vitest resolves the source in the checkout and there is nothing built to go stale between the legs.

  • Leg A — remove the options forwarding (deleteItem(type, name, options) back to deleteItem(type, name)). Predicted: the four forwarding pins go red, controls stay green. Measured: 5 failed / 15 passed — the prediction under-counted by one, a NON-empty pin is forwarded verbatim, which is a forwarding pin in the same direction. Red: the stale-pin refusal, its --format json twin, the draft-only discard, both-carriers-at-once, verbatim forwarding. Green throughout: the whole declared-flag surface, the unpinned baseline, the current-version pin, the unscoped-mount measurement, all three empty-pin refusals, and every scoped-twin case — so the pins discriminate rather than all failing together.
  • Leg B — remove the empty-pin refusal (the throw only). Predicted: the three refusal cases go red, everything else green. Measured: 3 failed / 17 passed, exactly those three. a NON-empty pin is forwarded verbatim stayed green, which is what distinguishes "the refusal was ablated" from "the file was broken".

Verification

Gate families derived from the real change set with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (three-dot against the merge base, working tree included) — not recalled. The union below was re-run after the final commit, on aa80a3010; each verdict is the gate's own pass line, with the exit code captured before any pipe.

Green: check:nul-bytes, check:test-source-alias, check:cross-package-test-inputs, check:engine-double-contract, check:where-matcher, check:objectql-double-limit, check:undeclared-dep-imports, check:changeset-gate-self-tests, check:objectui-changeset, check:pm-half-states, check:keyed-text-bounds, check:published-files, check:slot-lookup, check:page-declaration-shape, check:logger-receiver-detach, check:type-source-resolution, check:query-options-erasure, check:type-check-coverage, check:type-check-debt (the --re-measure ratchet), check:i18n, check:i18n-coverage, check:dual-build-cjs-loads, check:shard-attestation, plus check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-ci-filter-parity, check-comment-mask-adoption, check-plugin-teardown-shape.

node scripts/check-test-completeness.mjs with no argument is recorded NOT MEASURED (exit 3, PREREQUISITE NOT MET — it grades a saved turbo run test log, which only CI produces). It is not folded into the green list. check:i18n, check:i18n-coverage and check:dual-build-cjs-loads each refused first with PREREQUISITE NOT MET; all three were cleared by building the closure each named and then really ran.

pnpm lint (eslint . --no-inline-config, the whole repo) is clean on aa80a3010 — run in full rather than narrowed, so no scoping argument is owed.

pnpm --filter @objectstack/cli typecheck exits 0, and tsc --noEmit --listFiles confirms both edited files are inside that program (this package's tsconfig.json includes src with no test exclusion), so the green covers the new test file rather than merely the command.

Declared narrowing on the package suite.@objectstack/cli is this repo's largest suite (measured at ~13 minutes in its own vitest.config.ts), which does not fit this container's foreground ceiling, so vitest was run on a scoped population instead of the whole package: the new file, every test file that reaches the changed module, and the whole src/commands/meta directory. The population was read from the tool rather than guessed — git grep for meta/delete / MetaDelete across packages/cli/src and packages/cli/test returns exactly three files, one of which is the module itself — and the two new exported names (metaDeleteOptions, EMPTY_IF_MATCH_REFUSAL) are reachable from nowhere else in the tree by the same grep. 3 test files, 48 tests, all passing on aa80a3010. CI runs the full farm regardless.


Generated by Claude Code

…rom `os meta delete`
`os meta delete <type> <name>` was the only in-repo caller of the SDK reset and
called it with two arguments, so every CLI reset was the unpinned, full one: a
concurrent edit was silently destroyed instead of answering `409
metadata_conflict` (ADR-0008), and the published overlay went with any pending
draft. `DeleteMetaItemOptions` landed on both `deleteItem` declarations in
#12181; only the CLI surface was missing.
Adds `--if-match <version>` (string, forwarded to `options.ifMatch`) and
`--draft` (boolean, forwarded as `options.state = 'draft'`). An empty
`--if-match` is refused before the client is built rather than silently
downgraded to the unpinned reset. `?dropStorage` stays withheld.
A run with neither flag sends byte-identical bytes to what it always sent.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TvqBFLRzXdSPcbusDoED9k
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 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 — 23 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 b7f645a8a3b91cd26f2cf52a2647c415beb94b28packageMentionDocs.

Which tree this was computed on

This run read content/docs from 016a7e6ab7d02a304f2697ede190f50ce1f9be44 — the merge of head aa80a3010cacd1aa99ccb7153453ca645ba8dac3 into base b7f645a8a3b91cd26f2cf52a2647c415beb94b28, 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 016a7e6ab7d02a304f2697ede190f50ce1f9be44 && git checkout 016a7e6ab7d02a304f2697ede190f50ce1f9be44
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b7f645a8a3b91cd26f2cf52a2647c415beb94b28 aa80a3010cacd1aa99ccb7153453ca645ba8dac3 && git checkout -B drift-repro b7f645a8a3b91cd26f2cf52a2647c415beb94b28 && git merge --no-ff aa80a3010cacd1aa99ccb7153453ca645ba8dac3
node scripts/docs-audit/affected-docs.mjs --json b7f645a8a3b91cd26f2cf52a2647c415beb94b28

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

@os-trumpClaude

Copy link
Copy Markdown
CollaboratorAuthor

Out-of-scope finding from this work, filed rather than folded in:#13347 — the CLI's --format json failure envelopes drop the ADR-0112 error code (48 sites spell error: error.message), so the 409 metadata_conflict this PR makes reachable is machine-readable only by substring-matching an English sentence. Different defect class, spans the whole command family, and it carries a contract question about what the envelope should declare — so it is a separate card, unassigned, and nothing in this PR depends on it.

Linked here rather than added to the PR body on purpose: a body edit normalizes the session-URL attribution footer down to the bare form (measured 2026-08-26), and the body is otherwise exactly as created.


Generated by Claude Code

@os-trumpClaude

Copy link
Copy Markdown
CollaboratorAuthor

✅ PM review — ACCEPT once CI is green; ⛔ not while it is running (#13024)

Undrafting now; arm follows on a complete green read. ⛔ No rework owed. Clause ②: does not attach — 3 files, none under packages/spec/src/**, and additive CLI flags are not an answer on a public REST door.

✅ All three fences held, verified from the diff

  • Fence 1 — "default to pinning" is OUT.metaDeleteOptions reads flags and nothing else; no code path reads the current version first, and the question is not decided in either direction. Unpinned remains the default and remains last-write-wins.
  • Fence 2 — ?dropStorage withheld. ✅ Absent under both spellings, and pinned with a positive control: the same Object.keys(MetaDelete.flags) lookup that finds no dropStorage/drop-storagedoes find if-match and draft. That is how an absence assertion earns its keep.
  • Fence 3 — both twins or neither. ✅ Handled better than the fence asked. The command was measured to reach only the unscoped twin (/api/v1/meta/view/mount_probe, no /environments/ segment — createApiClient builds a plain ObjectStackClient and passes the environment id as the X-Environment-Id header). So rather than assert a shared type, it exported metaDeleteOptionsspecifically so the bag the CLI builds can be driven through the scoped twin against the same booted door — same 409 on a stale token with the row surviving, same draft-only discard — plus two IN STEP cases comparing the twins against each other rather than against a restated literal. ⭐ That is the right discharge for a fence whose literal premise ("the CLI can reach both") turned out false.

⭐ The best thing here is something I did not ask for

An empty or whitespace-only --if-match is refused before the client is even built. The reasoning is exactly right and is worth quoting, because it is a real operational footgun on a destructive verb:

os meta delete view x --if-match "$VERSION" with VERSION unset expands to an empty argument, and inheriting the SDK's omission would run the UNPINNED, last-write-wins reset while the operator reads their own command line as pinned.

And it correctly identifies that the SDK's omit-on-empty rule is right at the SDK layer (an emitted empty header would pin against the empty string and refuse a reset nobody asked to pin) and only becomes a trap at the CLI boundary. Refusal only — a non-empty token is forwarded verbatim, never rewritten, because the version is opaque. Pinned with both limbs measured (createCalls 0, protocol never called, row intact) and a positive control showing both instruments move on an allowed run.

The evidence is driven, not asserted

A pin that checked "the command constructed { ifMatch }" would have verified nothing. This boots a real RestServer, real ObjectStackProtocolImplementation, real sys_metadata* tables on real SQLite, and drives the real oclif command — argv in, envelope out. Stubbing only the credential boundary.

casemeasured
unpinned, staleexit 0, Metadata deleted, row 1 → 0, no parentVersion on the protocol call, empty query and only content-type on the wire (the byte-identity claim)
pinned, staleexit 1, [metadata_conflict] … Expected parent sha256:… but current is sha256:…, row SURVIVES, token carried as the if-matchheader with no ?ifMatch=
pinned, currentexit 0, row goes ⇒ the pin refuses a stale write, not every write
--draft2 rows → 1, ?state=draft on the wire, published overlay left active; then the same item reset without the flag drops it (1 → 0) with state ABSENT on the same probe

⭐ The current-version case is the one that stops "always 409" from passing, and the full-reset follow-up is what makes the --draft claim a discrimination rather than a coincidence.

The flag spelling was measured, not preferred: every Flags.string with options: in packages/cli/src/commands enumerates three or more members with a meaningful non-boolean default, there is no two-valued options flag anywhere in the tree, and 115 binary opt-ins are Flags.boolean — plus the structural argument that state: 'active' deliberately sends nothing, so --state active would be "a boolean wearing a costume."

⭐ And the human-mode wording moved: --draft prints Pending draft discarded, not Metadata deleted, because the full-reset sentence would be a false report of what happened on the run where the operator chose the narrower verb.

Reverse verification predicted 4 red on leg A and measured 5 — ⭐ reported as an under-count in the same direction rather than quietly rounded to "as predicted". Leg B predicted 3, measured exactly 3, with a NON-empty pin is forwarded verbatim staying green as the discriminator. Blob hashes both ways, git diff HEAD empty on restore.

⚠️ One caveat on what this PR actually buys, and it is the dev's own finding

The 409 this card makes reachable is, in --format json, {"success": false, "error": "[metadata_conflict] …"}the code is recoverable only by substring-matching an English sentence. Filed as #13347 (triaged bug · p2 · pm:queue · domain:cli): 48 sites under packages/cli/src/commands spell error: error.message, discarding the err.code and err.httpStatus the SDK deliberately normalises.

⚠️A script cannot yet branch on this conflict cleanly. That is not a defect in this PR — the envelope shape predates it across 48 sites and changing it is a contract question of its own — but it is an honest limit on the value delivered, and it is why the follow-up matters. ⛔ It was correctly filed rather than folded in.

Also recorded: the test run is a declared narrowing (@objectstack/cli is ~13 min and does not fit the container's foreground ceiling), with its three pieces present — population read from git grep (exactly 3 files reach the changed module), count read from vitest (3 files / 48 tests), and invariance stated via the package typecheck compiling every test file. check-test-completeness recorded NOT MEASURED with its own refusal text; check:i18n, check:i18n-coverage and check:dual-build-cjs-loads each refused first and were cleared by building the closure they named, so their green is a real reading.


Generated by Claude Code

@os-trump
os-trump marked this pull request as ready for review August 30, 2026 05:09
@os-trump
os-trump added this pull request to the merge queueAug 30, 2026
Merged via the queue into main with commit 2331b1eAug 30, 2026
34 checks passed
@os-trump
os-trump deleted the claude/issue-13024-meta-delete-pin-and-draft-flags branch August 30, 2026 06:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

os meta delete cannot pin a reset or discard only the pending draft — the CLI stays one layer behind the SDK it calls

2 participants

@os-trump@claude