fix(approvals): the record-lock refusal names the record, not its primary key (#18153)

[huangyiirene]

Fixes #18153

Clause-②: no

The per-row RECORD_LOCKED refusal is the one end-user-facing message of the four
lockedError sites in lifecycle-hooks.ts — the console copies it into a toast verbatim
— and it spelled the record record 'ID' of 'API_NAME', putting an opaque primary key and
a machine identifier into user-facing prose on a path that reaches screenshots, screen
recordings and support tickets.

It now reads:

RECORD_LOCKED: Opportunity 'Acme renewal' is locked while an approval is in progress, and cannot be edited until that approval is complete

degrading to This Opportunity is locked … when the object declares no resolvable title,
and to This record is locked … when the registry is unreachable — never back to the id.
The record id and the object API name are not deleted: they move to the console
(logger.info, alongside the pending request's id), which is where a support path reads
them and where a screen recording does not.

The measurement the card asked for (P2)

Triage graded this as "a read this hook does not do today", and the dispatch order asked
for that to be measured rather than assumed. Measured: no read was added at all. Both
halves are already in hand at the refusal.

half	where it comes from	cost
object label, ADR-0079 title pointer	engine.getSchema(object)	in-memory registry, no I/O
the record	ctx.previous, the pre-image the engine already read	zero

ctx.previous was measured against a real ObjectQL + a real sqlite driver on all four
update shapes — by-id, updateManyData, predicate multi, unscoped multi. Every one
dispatches this hook per row with previous bound and input.id set, so the title is
free on each. previous is used only when its id really is the record the pending request
names, so a dispatch that ever carried a different row cannot title the wrong record.

Two things deliberately not done, both stated in the code:

• no system-context read of the record on the deny path. It would title a row the
  caller may not be allowed to READ — and gating rows in exactly that state is what this
  lock exists for (the sys_comment has no record-level authorization: any org member reads and writes comments on records they cannot see #4630 rule the file already carries).
• no mining of payload_json. That column's whole discipline is that it is served
  REDACTED per reader; lifting a field out of it into an error message routes around that.

Known degradation, declared: the title is read as a stored column, so an object whose
nameField points at a formula falls to the This Opportunity is locked … form.
Evaluating a formula title needs evaluateFormulaField, which lives in @objectstack/objectql
— a devDependency here — and promoting that to a runtime dependency is a bigger call than
this card carries. The degradation is safe in the direction that matters: it never reaches
for the id.

Controls, both mandatory ones

• The three operations-facing refusals are untouched and now pinned byte for byte —
  the two PENDING_LOCK_LIMIT cap messages and the unanswerable-intersection message.
  They are raised about a write SHAPE, not about a record, and naming the object's API
  name there is the useful thing to say to whoever has to rescope that write. Pinned so a
  later "harmonise the lock's messages" sweep goes red instead of folding them into the
  end-user shape.
• RECORD_LOCKED and its 409 are pinned in both directions — the code and status are
  asserted to BE those values, status is asserted still absent (the A batch row's httpStatus reads only .status, so two genuine 4xx populations ship a row with no status at all #8570 single-spelling
  pin), and the CODE: message envelope is asserted to still be the envelope.

Ablation — the new pin can fail

Run from the committed state, mutation proved on disk (both directions: deleted-text count
0, injected-text count 1, blob hash moved), restore proved by blob-hash equality plus an
empty git diff HEAD.

leg	result
producer reverted to the old sentence	exit 1 — 3 failed / 8 passed
restored	exit 0 — 11 passed

The three that reddened are exactly the user-facing assertions
(the card's second row, verbatim, now carrying 409,
names the record by its label and leaks neither the id nor the API name,
degrades to the object label — never back to the id — when no title resolves).
The RECORD_LOCKED/409 control and the three operator-refusal controls stayed green
under the mutation, which is what makes the ablation non-vacuous: it reddened the copy and
left the contract alone.

Verification

Measured at c0c490413 (this branch's head, after merging origin/main).

• pnpm --filter @objectstack/plugin-approvals test — 47 files / 771 tests passed
• pnpm --filter @objectstack/plugin-approvals typecheck — exit 0 (check:test-typecheck: OK)
• pnpm --filter '@objectstack/plugin-approvals^...' build — exit 0
• pnpm --filter @objectstack/spec build && check:generated — all 15 generated artifacts up to date
• pnpm lint (repo-wide, eslint . --no-inline-config) — exit 0 in 64s; run whole, so
  no narrowing argument is owed
• Gate roster: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands
  with no paths (merge-base derivation) — 62 families derived, 62 run, 0 NOT-MEASURED,
  0 UNRUN, reconciled with --ran carrying every exit code. Four initially exited 3
  (PREREQUISITE NOT MET, not failures): check:dual-build-cjs-loads, check:i18n and
  check:type-check-debt needed a built repo, and
  check-plugin-teardown-shape --self-test needed a commit this shallow clone lacked. All
  four re-ran green after turbo run build over the packages and a targeted
  git fetch origin SHA.

Two pre-existing pins re-pointed

approval-service.test.ts carried two assertions on the old sentence. One WAS the defect's
own assertion (record 'opp1' of 'opportunity' is locked) and now pins the label-named
sentence plus the console demotion. The other used the id only as a discriminator
between two competing approvals — one locking, one opted out — so that discriminator moves
to the console line, which still carries it. Without that move the test would have passed
on a refusal raised by the wrong request, which is the exact confusion it exists to rule out.

Acceptance notes

Out of scope, noted, not filed:

• packages/metadata-protocol/src/protocol.batch-row-driver-text.test.ts and
  protocol.batch-row-http-status.test.ts construct their own synthetic
  RECORD_LOCKED error and import nothing from plugin-approvals — verified, so they do
  not pin this producer and they stay green. Their docblocks describe that fixture as
  MEASURED off plugin-approvals' lockedError, and the message half of that description
  is now stale; the code / statusCode / envelope half, which is what those files
  actually assert, is unchanged. Comment-accuracy drift in another package, outside this
  card's declared file surface. Successor: whoever next edits the batch-row error table,
  which those two files and protocol.ts (its copies near the RECORD_LOCKED row) share.

Premises

• P1 (line number). Both readings were right about different things, re-taken at the
  branch point: the lockedError( call opens at :392, its template string is at
  :393. Line numbers in this card were not trusted for anything else.
• P4 (blast radius). Verified as described above — the metadata-protocol pins are
  synthetic doubles, so the blast radius stayed inside the declared file surface.
• P5 (history). Left UNREAD, as instructed; nothing here depends on it.
• P6 (hotcrm). Not reachable and not claimed. The producer is what was verified.
• Zero packages/spec changes. The one spec symbol used, resolveDisplayField
  (ADR-0079's single arbiter of "which field is the title"), is imported from the already
  published @objectstack/spec/data — a runtime dependency of this package — and nothing
  in packages/spec is edited.

---

Generated by Claude Code

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-approvals, touching 5 documentable anchor(s).

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

• content/docs/concepts/architecture.mdx (via getSchema (symbol, a method of interface MinimalEngine))
• content/docs/permissions/system-context.mdx (via bindApprovalLockHook (symbol, a top-level function))

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.

Coarse fallback — 6 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 a7e9a6600be9680090609c80e41374b637c0a6aa → packageMentionDocs.

Which tree this was computed on

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

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

⚠️ 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 a7e9a6600be9680090609c80e41374b637c0a6aa → pass the list as
args.docs, on the commit named under Which tree this was computed on.