Skip to content

fix(spec): undoable names the written set, not patch alone - #19283

Merged
os-litant merged 3 commits into
mainfrom
claude/issue-19148-undoable-capture-set
Sep 20, 2026
Merged

os-litant merged 3 commits into
mainfrom
claude/issue-19148-undoable-capture-set

Conversation

@os-litant

Copy link
Copy Markdown
Collaborator

Fixes #19148

Clause-②: no

ActionSchema.undoable's published sentence said its patch names exactly the fields whose prior values are captured. An operation: 'update' action also writes whatever its params collect, so on any params-carrying action that sentence describes a strict subset of what the action writes — an Undo built to it restores part of the change and reports the action as undone.

Triage settled the direction at comment 5747751639 and this PR executes it verbatim:

Scope: correct the undoable description so it names what an undo must capture for each operation, and ⛔ do not narrow objectui#7551.

All readings below were taken against origin/main = 1739f71879f, the base this branch is cut from, between 2026-09-20T08:45Z and 2026-09-20T09:30Z. The base had not moved from the one the dispatch order names.

What changed

packages/spec/src/ui/action.zod.ts — the .describe() on undoable:

FROM

operation: 'update' is the declared form of that action — its patch names exactly the fields whose prior values are captured.

TO

operation: 'update' is the one declared operation and the declared form of that action: what the undo captures is the prior value of EVERY field the action writes — the merged write bag, patch UNDER the collected params, not patch alone. An action with no operation declares no write set, so nothing anchors the capture there.

The // comment above the key carried the same claim ("its patch names exactly the fields written") and is corrected with it, now naming the executor symbol that settles the set.

Prose only. No schema change, no refine, no key added or removed; the same author input parses identically before and after. That is the whole basis for Clause-②: no, and it is what the enqueue gate will read off the diff.

Premise reading 1 — does a shipped runtime restore from the DECLARED set?

No. Every reader measured captures the WRITTEN set. This is the stop condition the order named, and it does not fire.

Server, packages/runtime/src/action-execution.ts, contract point 5:

const data = declarativeUpdateWrite(action, params);   // { ...patch, ...params }
...
if (action?.undoable === true) {
    const undoData = {};   // typed Record of string to unknown in the source
    for (const key of Object.keys(data)) undoData[key] = prior[key] ?? null;

declarativeUpdateWrite in the same file returns { ...base, ...params } with base the static patch — so Object.keys(data) IS the union. The DeclarativeUpdateUndo.undoData docblock next to it already reads "The prior value of EXACTLY the fields written".

Console, ../objectui at dda8f3815df, both readers key off the bag they actually send:

  • packages/app-shell/src/hooks/useConsoleActionRuntime.tsx:487for (const k of Object.keys(fields)) undoData[k] = rowRecord[k] ?? null;, fields being the params bag with bodyExtra merged in, i.e. the same bag handed to dataSource.update.
  • packages/app-shell/src/views/RecordDetailView.tsx:837for (const k of Object.keys(params)), params again being the bag handed to dataSource.update.

⇒ correcting the prose closes the divergence rather than moving it. No behaviour change is proposed or needed.

Premise reading 2 — who else reads undoable?

Instrument git grep -n -w 'undoable', exit code captured before any pipe. Firing control: ActionSchema in the same file, 21 hits, exit 0. Dark control: undoableZZZNOSUCH, 0 hits, exit 1. 228 hits over 43 files in this repo; 128 hits over 27 files in ../objectui.

Readers of ui/Action:undoable whose behaviour depends on the flag:

Reader Capture set Verdict
packages/runtime/src/action-execution.ts:1991 merged { ...patch, ...params } written set
objectui useConsoleActionRuntime.tsx:487 the bag sent to dataSource.update written set
objectui RecordDetailView.tsx:837 the bag sent to dataSource.update written set
objectui action-button.tsx:223 forwards the flag only no capture set
objectui ActionDefaultInspector.tsx the authoring checkbox no capture set

Readers that restate the sentence rather than act on it — all corrected or regenerated here: the .describe() itself, the three generated reference tables it renders into (ui/action, data/object, kernel/metadata-plugin), and the hand-written protocol page.

One reader consults the liveness ledger and not the sentence: packages/lint/src/lint-liveness-properties.test.ts:109 asserts the lint stays silent on action.undoable. Unaffected — the ledger row is untouched.

The three other undoable sites — both homonyms, and how that was decided

The order named two files it had not classified. Both are homonyms, on three mechanical legs each rather than on how the word reads.

packages/spec/src/api/export.zod.ts:478 and :523 (ImportJobProgress / ImportJobResults / ImportJobSummary) — homonym:

  1. Different surface id. packages/spec/authorable-surface.base.json lists api/ImportJobProgress:undoable and siblings; the UI flag is ui/Action:undoable. Different schema, different namespace.
  2. Opposite direction. It is a required z.boolean() the SERVER computes and the client reads — packages/rest/src/rest-server.ts:846, undoable: importJobUndoable(row). ui/Action:undoable is an optional flag the AUTHOR writes and the runtime reads.
  3. Different referent. "Whether this job can still be logically rolled back (undo log captured, terminal state, not yet reverted)" — a job-level boolean about an import's undo log. It names no field set at all, so there is no capture set for this card's sentence to be wrong about.

packages/spec/src/system/migration.zod.ts:301 — homonym, and not a schema member at all: the single hit is the English adjective inside a prose docblock ("Every one of those is undoable: a rejected write is retried, a tombstone is lifted on re-attach"). git grep -n -w 'undoable' on that file returns exactly one line and it is a comment.

Two further hits the same grep turned up are the same import-job homonym reaching its consumers (packages/client/src/index.ts:6989, packages/rest/src/rest-server.ts:846 and :875) and one is the adjective again (scripts/pm/check-clause2-carriers.mjs:8263, inside the word "un-undoable").

"For each operation" — the census that makes it a one-row rule

operation is z.enum(['update']). It carries exactly one member; 'delete' and 'custom' are refused at parse time with their own reason. So the per-operation capture rule triage asked for is a one-row rule, and the sentence states it as one rather than inventing a table with a single row.

The sentence also covers the case the enum does not: an action that declares no operation at all. Nothing refuses undoable: true there — the refine chain in action.zod.ts says nothing about the pair — and the server builds no undo for it, because the undo envelope is built only inside executeDeclarativeUpdateAction. That case was previously undescribed; it is now named as un-anchored rather than left to be inferred.

Verification

Gates derived from this tree, not from a list: node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack, exit codes captured before any pipe into a TSV, reconciled with --ran.

Run reconciliation — 100 derived, 99 run, 1 NOT-MEASURED, 0 UNRUN.
✓ dispatch-gates --ran: 100 derived famil(ies) accounted for — 99 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3).
  • pnpm --filter @objectstack/spec run check:docs — exit 0 (the gate that owns the three regenerated tables).
  • pnpm --filter @objectstack/spec run check:generated — exit 0, all 16 artifacts current.
  • pnpm --filter @objectstack/spec test — exit 0, 500 files / 14643 tests.
  • pnpm --filter @objectstack/spec run typecheck — exit 0 (tsc, scripts, test layer).
  • pnpm lint — the repo-wide eslint . --no-inline-config, exit 0 at 76eb07aeb63. Run whole rather than narrowed, so no narrowing needs proving.
  • pnpm check:nul-bytes — exit 0; plus a hand sweep of the edited files for non-NUL control bytes, no hits.

NOT MEASURED (1): pnpm check:dual-build-cjs-loads, recorded exit 3 — PREREQUISITE NOT MET, it reads built output for 83 packages and a repo-wide pnpm build does not fit the container's foreground limit. Exit 3 is that gate's own "nothing was measured" code, neither a pass nor a failure. It reads dist/ loadability and this diff changes no export, entry or build config. CI runs it.

No new test is owed: a prose claim is not a behaviour, and the text is already pinned mechanically — check:docs holds the three generated tables byte-equal to what the .describe() produces, so a future edit to the sentence cannot land without moving them.

One file outside the claim's declared surface

The claim declares packages/spec/src/ui/. content/docs/protocol/objectui/actions.mdx:54 is hand-written and carried the identical false claim — "undoable has its anchor here: the patch names exactly the fields whose prior values are captured" — and is corrected in the same edit under the bounded same-defect exemption, with all four conditions measured:

  1. Same defect class — the same claim about the same key, word for word.
  2. Mechanical — the corrected wording transposes directly.
  3. Held by no open PR — 26 open PRs, every file list read at 2026-09-20T08:52Z. The same scan clears packages/spec/src/ui/action.zod.ts and resolves the order's residual: spec(ui): BulkActionParamSchema is strict and declares dependsOn #19090 lands in packages/spec/src/ui/bulk-action.zod.ts, not action.zod.ts, so this PR is not second on it.
  4. No new verification surface — the derived gate list is byte-identical with and without that file, 95 commands either way.

The reviewing seat may want to amend the claim's file surface to match.

Acceptance notes

Out of scope for this PR, filed nowhere, each with the PR or reader that will reach it:

  • packages/runtime/src/action-execution.ts:1992 — the comment on contract point 5 reads "EXACTLY the fields written — the patch names them". The code beside it keys off the merged bag and is correct; the trailing clause is the same conflation this card corrects, one file over. Not a defect (no behaviour depends on it) and outside this claim's surface. Successor: the next PR touching contract point 5.
  • packages/runtime/src/action-declarative-update.test.ts:509 — the point 5 block pins the patch-only capture, the absent-field null and the no-undoable-no-undo zero, but no case declares undoable: true and params together, so nothing would fail if the executor ever narrowed to patch-only. The behaviour is correct today; the pin that would hold it is absent. Successor: the same PR as above, or a runtime-lane card if the reviewing seat would rather route it.
  • packages/spec/src/ui/action.zod.ts:1109 — the EXECUTOR CONTRACT docblock still says "both keys are planned in the liveness ledger until they land". packages/spec/liveness/action.json flipped operation and patch to live on 2026-09-08. A different defect class from this card's, so deliberately not swept in. Successor: the next PR touching that docblock.

Generated by Claude Code

`ActionSchema.undoable`'s `.describe()` said the `patch` names exactly the
fields whose prior values are captured. An `operation: 'update'` action also
writes what its `params` collect, so on any params-carrying action that
sentence describes a strict subset of what the action writes — an Undo built
to it restores part of the change and reports the action as undone.

The executor already captures the union: `executeDeclarativeUpdateAction`
keys `undoData` off `Object.keys(data)`, `data` being the merged write bag
`{ ...patch, ...params }` from `declarativeUpdateWrite`. The doc block ~200
lines above in the same file already said "exactly the fields written". The
sentence was the outlier; this moves it.

`operation` carries exactly one member, `'update'`, so the per-operation
capture rule is a one-row rule and is stated as one.

Claude-Session: https://claude.ai/code/session_01LvwGppdonww4zGLWZo5rho
Co-authored-by: Claude <noreply@anthropic.com>
…ol page's copy of the same claim

`gen:schema && gen:docs` after the `undoable` describe() correction. The text
renders into three generated tables — `ui/action`, `data/object` and
`kernel/metadata-plugin` — all regenerated here.

`content/docs/protocol/objectui/actions.mdx` is hand-written and carried the
identical false claim ("the patch names exactly the fields whose prior values
are captured"). Corrected in place under the bounded same-defect exemption:
same defect class, mechanical transposition of the corrected sentence, the
file is held by no open PR (26 scanned), and the derived gate-family set is
byte-identical with and without the edit.

Changeset: patch on `@objectstack/spec` — the description ships in the
package, so a consumer reading the reference page sees a different promise.

Claude-Session: https://claude.ai/code/session_01LvwGppdonww4zGLWZo5rho
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/s documentation Improvements or additions to documentation protocol:ui tooling labels Sep 20, 2026
@github-actions

github-actions Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

1 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
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 136 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 15f92842ffca0be5d24e4778698c711310a984eepackageMentionDocs.

Which tree this was computed on

This run read content/docs from e15a436206f1df7d5c39cb642efcfd3a8f1add71 — the merge of head 889d9c7f22613e73d5a58d5eeacc22c9d907f4a7 into base 15f92842ffca0be5d24e4778698c711310a984ee, 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 e15a436206f1df7d5c39cb642efcfd3a8f1add71 && git checkout e15a436206f1df7d5c39cb642efcfd3a8f1add71
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 15f92842ffca0be5d24e4778698c711310a984ee 889d9c7f22613e73d5a58d5eeacc22c9d907f4a7 && git checkout -B drift-repro 15f92842ffca0be5d24e4778698c711310a984ee && git merge --no-ff 889d9c7f22613e73d5a58d5eeacc22c9d907f4a7

node scripts/docs-audit/affected-docs.mjs --json 15f92842ffca0be5d24e4778698c711310a984ee

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

Copy link
Copy Markdown
Collaborator Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 889d9c7f22613e73d5a58d5eeacc22c9d907f4a7

Isolated at-tier review of PR #19283 against card #19148 and its binding grading (comment 5747751639). Every reading below was re-taken in this act at 2026-09-20T10:18Z: this repo at refs/review/19283 = the head above (merge-base with main = e6a03e6), objectui at the sha the PR cites (dda8f3815df), at the pinned .objectui-sha (53ded82bf7a) and at objectui origin/main (0c2eb5eee). Nothing in the PR body or the dev reports was adopted unmeasured.

① Derived judgments

Scope matches the ruling. The grading settled "the sentence moves; do not narrow objectui#7551". The diff (6 files, +31/-8 off merge-base e6a03e6) touches no objectui path and leaves .objectui-sha at 53ded82bf7a; the only source change is the // comment and the .describe() string on ActionSchema.undoable (packages/spec/src/ui/action.zod.ts:1307-1320). No key, enum member, refine, export, registry row or baseline moves; refuseDeclarativeUpdateContradictions is byte-identical.

Every clause of the new sentence holds on the tree. (a) operation is z.enum(['update']) with one member (action.zod.ts:1118), so one capture rule is the right count. (b) The server captures the written set: packages/runtime/src/action-execution.ts:1956 builds data = declarativeUpdateWrite(action, params) = { ...patch, ...params } (:1824-1829), and :1991-1997 keys undoData off Object.keys(data); that file is unchanged since the dev's base 1739f71. (c) Nothing refuses undoable: true on an action without operation — the refine chain never reads undoable (the key's only two hits in the file are the docblock at :1116 and the key itself) — and the undo envelope is built only inside executeDeclarativeUpdateAction, so "un-anchored" is the accurate word. (d) The precedence "patch UNDER params" is what both executors do.

The generated tables are the describe text. The 412-character string extracted from the zod file compares byte-equal to the undoable cell in all three regenerated pages; CI TypeScript Type Check (the job that owns check:docs / check:generated) is success on this head.

Stop condition re-taken; the PR's objectui reading is stale. The premise the ruling hangs on (no shipped runtime restores from the patch-only set) HOLDS, but not on the evidence the PR gives. The PR read ../objectui at dda8f3815df (dated 2026-09-16). objectui#9972 — the union-capture implementation this card exists because of — merged as feac43909 at 2026-09-18T23:41Z and is an ancestor of neither dda8f3815df nor the pinned 53ded82bf7a. Its executor lives in packages/core/src/actions/ActionRunner.ts, a file the PR's reader census does not name. Re-taken at objectui origin/main 0c2eb5eee: executeUpdateOperation builds params = { ...patch, ...collected }, takes writtenFields = Object.keys(params) minus the two transport keys, and captures undoData over exactly those (:1498-1537; captureUpdateUndoData at :841-858, whose own docblock says "never patch alone"). The two app-shell readers the PR does cite (useConsoleActionRuntime.tsx:487, RecordDetailView.tsx:837) are unchanged between dda8f3815df and objectui main and capture the bag they hand to dataSource.update. So: written set everywhere; the census in the PR body is incomplete, not wrong.

Old-sentence survivors at head, for the seat. git grep -F 'names exactly the fields whose prior values are captured' at the head finds the changeset's FROM quote and packages/spec/CHANGELOG.md:3918 — release-owned, which a code PR may not edit; the Documentation Guardrails route is a dedicated docs-only PR, the PR names none, so its "all corrected or regenerated here" is overstated by that one line. packages/runtime/src/action-execution.ts:1993 still reads "the patch names them" (acceptance-noted by the dev; a runtime-lane comment with no behaviour on it). objectui carries no copy of the old sentence. No other restating page carries the patch-only claim (content/docs/ui/actions.mdx:366, skills/objectstack-ui/rules/actions.md:22, packages/runtime/CHANGELOG.md:244 and the liveness ledger's undoable entry each read the written set or say nothing about the set).

Homonyms confirmed. export.zod.ts:478 and :523 are api/ImportJob*:undoable, a server-computed required boolean naming no field set; migration.zod.ts:301 is the adjective inside a docblock.

Declared deviation — the four conditions hold (review-checklist: same defect class, mechanical, no other claim, same gate family). (1) content/docs/protocol/objectui/actions.mdx:54 carried the identical phrase at the base, grep-confirmed. (2) The change is one sentence in one line. (3) Re-taken now over 31 open PRs and their file lists: no other PR holds actions.mdx or action.zod.ts; #19090 lands in bulk-action.zod.ts; only #19147 holds the generated data/object.mdx, which both PRs regenerate. (4) Re-taken in a worktree at this head: dispatch-gates --commands over the six files and over the five without actions.mdx are byte-identical, 100 lines each. Residual for the seat: the same checklist wants the claim's file surface amended in the same round, and claim 5748746720 still reads packages/spec/src/ui/ with no amendment comment on the card at my read.

Report-side inaccuracies, none load-bearing. The dev report cites action-execution.ts:1990 and :1996; the lines are :1956 and :1997 (the PR body's :1991 is right). The objectui census reports 128 hits over 27 files; at dda8f3815df I count 128 hits over 29 files.

Not verified here. The raw log of the failing Check Changeset job — its log host is policy-denied from this session — so the failure was reproduced locally instead (see ②). objectui behaviour is read from source at three shas, not exercised in a browser.

② Semver level

Measured: a prose-only change to a published package. @objectstack/spec is private: false at 17.4.0, sits in the changeset fixed group, and ships src/**/*.zod.ts, so the describe text is a published sentence and a changeset is owed. The accepted-metadata set is unchanged — the same author input parses identically, and check-widening-tells reports no tell on the judged file — which is the lane charter's text-surface case and AGENTS.md's bug-fix case: patch. The changeset grades @objectstack/spec: patch and carries the FROM and TO text. Correct.

The Check Changeset red on this head is not a level misgrade. Reproduced at the head with the PR's own event payload: check-changeset-no-major.mjs folds the needs:contract-review carrier into a clause-② yes (declarationFromPullRequest: carrier present means value yes) and then refuses a patch on a moved package; the same payload with that one label removed exits 0 and prints "declares clause-② no". The label was hung on the PR at 2026-09-20T09:35:33Z, the head was pushed at 2026-09-20T09:36:34Z, so the head's run at 2026-09-20T09:41:22Z read it. The script's own header names this the intended pre-review order ("the carrier is stripped at review pass"). So: after this PASS strips both carriers, the unlabeled re-fire must be read success on this head before ready or auto-merge — a reading for the landing seat, not an assumption. The reds on the superseded head 76eb07a are cancellations plus the same carrier red; zero action.

③ Boundary flags

Clause-②: no is right by the diff, independently of the declaration: no schema key, closed-set member, refine, export, registration, ledger, baseline or gate file moves; the api-surface* artefacts are untouched; no security or permission code is in the file list; no governed surface is touched (docs/adr/**, .claude/**, skills/**, AGENTS.md, CLAUDE.md all absent from the file list), so nothing here waits on the maintainer's word for that reason; .objectui-sha does not move, so the Console Pin Gate is unaffected.

Two flags to route, neither a boundary change:

  1. The new sentence documents that undoable: true on an action with no operation is accepted and un-anchored. The dev records this as "resolved by this diff, no trap left". The prose is accurate, but accuracy is not enforcement: contract-first (Prime Directive Add comprehensive test suite for Zod schema validation #12) would refuse that pair at parse time, which narrows the accept set and is a separate contract card by 基本裁决原则 — not something this prose closes. The seat should decide whether to file it.
  2. The PR's objectui reader census was taken on a checkout that predates the consumer the card names (①). The corrected facts are recorded above; the PR body should not be read as a complete census of undoable readers.

Implemented-by: claude/issue-19148-undoable-capture-set
Reviewed-by: session_01LvwGppdonww4zGLWZo5rho

VERDICT: PASS


Generated by Claude Code

@os-litant
os-litant marked this pull request as ready for review September 20, 2026 10:24
@os-litant
os-litant enabled auto-merge September 20, 2026 10:24
@os-litant
os-litant added this pull request to the merge queue Sep 20, 2026
Merged via the queue into main with commit 44a2332 Sep 20, 2026
39 of 40 checks passed
@os-litant
os-litant deleted the claude/issue-19148-undoable-capture-set branch September 20, 2026 11:03
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 protocol:ui size/s tooling

Projects

None yet

2 participants