Repository navigation
[finding] RECORD_LOCKED rejection toast surfaces the internal record id and the object API name to end users #18153
Description
Activity
Claim: PM loop round 1
Session:session_01QGMBhvUoyD8t5zY8xHQhnP
Branch:claude/issue-18153-record-lock-message-user-facing
Worktree:objectstack-issue-18153
Domain:domain:services
Seat:domain:services#1
File surface:packages/plugins/plugin-approvals/src/(stop on breach; explain in the report)
Container & model:M,mode:subagent, model: default judgment tier — cited from THIS dispatch'snode scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --tier packages/plugins/plugin-approvals/src/lifecycle-hooks.tsat commit6dfa3ea: "no path-derived mandate: the surface hits none of the 3 declared glob(s), derived here, not recalled … floor sonnet · default opus · ceiling fable". ⭐ Default tier, ⛔ explicitly not the floor: triage's own grading says the real work is a read this hook does not do today, ⛔ not a string swap — that is a design call, not a mechanical edit.
Clause-②: no
Thread-read: 5712580827
Serial constraints cleared:none— measured at 2026-09-17T15:13Z, ⛔ not recalled. (a) This lane's only otherpm:dispatchedcard is #16506, assigneezhuangjianguo, surfaceplugin-email⇒ disjoint, ⛔ not this seat's card. (b) All 23 open PRs intersected file-by-file againstplugin-approvals/src/lifecycle-hooks: 0 hits — control: the same intersector reports #18319 at 92 files and #17076 at 100 ⇒ instrument reading, ⛔ not returning empty. (c) Sibling cards dispatched in THIS batch — #18368 (service-settings) and #18110 + #18555 (service-automation) — are file-disjoint. (d) The patrol anchor's H17 restart-trigger file index namesplugin-sharing,plugin-security,plugin-authandclient/runtimepaths ⇒ 0 intersection withplugin-approvals. (e) ⛔ No sibling card pins behaviour this card asserts.Premise re-verification owed before any edit
⚠️ Two different readings of the producer line exist and they disagree by one. Triage readslifecycle-hooks.ts:392; this seat reads:393onorigin/main6dfa3eaviagit grep -n … origin/main. ⛔ Neither is authoritative — re-take it, and treat the disagreement as proof that line numbers in this card are ⛔ not to be trusted.- ⭐ Triage named a real constraint that is easy to get wrong, and it is carried into the dispatch order verbatim: the same file has three other
lockedErrorsites (:196/:269/:315) which are operations-facing boundary messages where naming the object IS useful. ⛔ They are not in scope and must not be changed with the one at:392/:393. ⚠️ git log origin/main -- packages/plugins/plugin-approvals/src/lifecycle-hooks.tsreturns only the graft-boundary commite46ac0f, which this checkout's owncheck-harness-current.mjsreports as parentless and diffed against the EMPTY tree ⇒ ⛔ a shallow-clone artefact, NOT evidence of a recent touch. Treat this file's recent history as UNREAD.- ⛔
hotcrmis NOT reachable from this session (GitHub scope:objectstack-ai/objectstackonly), and neither is the privatesteedos-labs/video-studio#356the card references. The "what the toast says verbatim" reading is the filing lane's, accepted as declared and ⛔ not re-verifiable here — but the producer is in this repo and IS verifiable, which is why the card lands in this lane at all.
Dispatched by the
domain:servicesPM seat (objectstack#6021) at 2026-09-17T15:13Z. ⛔ The dev leaves no second claim — this comment is the identity.
Generated by Claude Code
os-dev-report
{ "issue": 18153, "status": "done", "branch": "claude/issue-18153-record-lock-message-user-facing", "pr": "https://github.com/objectstack-ai/objectstack/pull/18716", "session": "session_01QGMBhvUoyD8t5zY8xHQhnP", "premise_still_valid": true, "summary": "The one end-user-facing lockedError site in lifecycle-hooks.ts now names the record by its object label plus its ADR-0079 title instead of `record 'ID' of 'API_NAME'`, degrading to `This Opportunity is locked ...` and then `This record is locked ...` and never back to the id; the record id, the object API name and the pending request id move to the console via logger.info. P2 measured and partly refuted: NO read was added -- the object label and title pointer come from the in-memory registry (engine.getSchema) and the record is ctx.previous, the pre-image the engine already binds on all four update shapes (by-id, updateManyData, predicate multi, unscoped multi), each measured against a real ObjectQL plus a real sqlite driver. A SYSTEM_CTX read of the record and the payload_json snapshot were both rejected on disclosure grounds and that is written into the code. Declared degradation: a formula-backed nameField is not evaluated (evaluateFormulaField lives in objectql, a devDependency here), so such objects fall to the label-only form.", "gates": "dispatch-gates.mjs --repo objectstack-ai/objectstack --commands with NO paths (merge-base derivation, per the PM correction and the role file) at HEAD c0c490413: 62 families derived, 62 run, 0 NOT-MEASURED, 0 UNRUN -- reconciled with --ran carrying an exit code for every one ('a DERIVED zero -- all 62 recorded an exit code and none of them is 3'). Roster byte-identical to the pre-merge derivation. Four initially exited 3 = PREREQUISITE NOT MET, not failures, and all four re-ran green: check:dual-build-cjs-loads / check:i18n / check:type-check-debt needed a built repo (fixed by `turbo run build --filter=./packages/* --filter=./packages/*/*`), and check-plugin-teardown-shape --self-test needed a fixture commit this shallow clone lacked (fixed by `git fetch origin 621a487607881c66b2899b7e3477115229a156b4`). Repo-wide `pnpm lint` (eslint . --no-inline-config) run WHOLE, exit 0 in 64s, so no narrowing argument is owed. packages/spec check:generated: all 15 generated artifacts up to date. CI convergence not awaited, per contract.", "tests": "pnpm --filter @objectstack/plugin-approvals test -> 47 files / 771 tests passed (exit 0, under os-verify-lock, VERDICT command-exit 0). pnpm --filter @objectstack/plugin-approvals typecheck -> exit 0, 'check:test-typecheck: OK ... 8 file(s) / 324 error(s) / 27 pinned signature(s)' (debt ledger unmoved). New pin file record-lock-user-facing-message.test.ts: 4 user-facing cases + 3 CONTROL cases for the operator-facing refusals, byte for byte. ABLATION (run from the committed state): producer reverted to the old sentence -> exit 1, 3 failed / 8 passed; restored -> exit 0, 11 passed. On-disk mutation proof in BOTH directions before reading any result: deleted-text grep count 0, injected-text grep count 1, blob 334a20ac -> 5f833456. Restore proved by blob-hash equality back to 334a20ac AND an empty `git diff HEAD`, not by an exit code; the script carried `trap restore EXIT INT TERM` with absolute paths off `git rev-parse --show-toplevel`. Direction observed = the predicted one (RED), and non-vacuous: the three that reddened are exactly the user-facing assertions, while the RECORD_LOCKED/409 control and all three operator-refusal controls stayed GREEN under the mutation. No dist preflight was owed: the pin imports ./lifecycle-hooks.js relatively, so the mutation was source-resolved, not dist-resolved.", "line_budget": "No skills/** path in the diff, so no published-skill line ratchet applies and none was spent. Diff vs merge-base f8eaf6704: +510 / -7 over 5 files -- lifecycle-hooks.ts +136 (of which ~90 are the module-docstring section recording the measurement and the two rejected designs), record-lock-user-facing-message.test.ts +263 (new), approval-service.test.ts +52, record-lock-batch-row-status.integration.test.ts +23, changeset +43.", "deviations": [ "Gate derivation: the dispatch order said `--commands` with a changed-file list; the PM corrected it mid-task to no paths, which is also the role file's standing rule. Followed the corrected form; the paths form was never used for a reported roster.", "Merged origin/main (f8eaf6704 -> c0c490413) into the branch before opening the PR -- AGENTS.md Multi-agent discipline section 10, not named in the dispatch order. Clean merge, no os-regen deferral, no overlap with my files. packages/spec moved on main's side, so spec was rebuilt and check:generated re-run before anything else was trusted.", "Four gates first answered exit 3 (PREREQUISITE NOT MET). Rather than report them NOT MEASURED I paid the prerequisite (a turbo build over packages, and one targeted `git fetch origin SHA` for a pinned self-test fixture a shallow clone lacked) and re-ran all four to exit 0.", "The container restarted and SIGKILLed a verification run mid-flight (exit 137). The pushed tree was intact (HEAD == origin, working tree clean); every verification was then re-run from scratch on the merged tree rather than carrying a pre-restart green forward.", "Two PRE-EXISTING tests in approval-service.test.ts asserted the old sentence and had to be re-pointed. Both are inside the declared file surface, but they are beyond the literal 'one site' reading, so declaring them: one WAS the defect's own assertion; the other used the id purely as a discriminator between a locking and an opted-out approval, and that discriminator moved to the console line, which still carries it. No operator-facing message was touched.", "One import was added to lifecycle-hooks.ts, which previously had none: resolveDisplayField from @objectstack/spec/data (ADR-0079's single arbiter of the title pointer), a package already declared as a runtime dependency. Zero packages/spec edits." ], "files_changed": [ "packages/plugins/plugin-approvals/src/lifecycle-hooks.ts", "packages/plugins/plugin-approvals/src/record-lock-user-facing-message.test.ts (new)", "packages/plugins/plugin-approvals/src/approval-service.test.ts", "packages/plugins/plugin-approvals/src/record-lock-batch-row-status.integration.test.ts", ".changeset/18153-record-lock-message-user-facing.md" ], "mcp_calls": "0 -- no MCP GitHub tool of any kind was called, read or write.", "api_writes": "2 REST proxy writes, exactly the dispatched budget: POST /repos/objectstack-ai/objectstack/pulls (draft, HTTP 201, PR 18716) and POST /repos/objectstack-ai/objectstack/issues/18153/comments (this report). No label write, no assignee write, no PATCH. Separately, 4 `git push` operations on the feature branch (empty-branch routing probe, 2 commits, 1 merge). All GitHub reads were GET.", "open_questions": [ { "question": "A formula-backed `nameField` is not evaluated, so on those objects the toast degrades to 'This Opportunity is locked ...' rather than naming the record. objectql's record-title.ts reports that 4 of 5 measured objects title by formula. Is the label-only form good enough for this card, or should a follow-up card carry the formula leg?", "options": [ "A -- ship as is and file nothing; the acceptance criterion (no id, no API name, an actionable name) holds unconditionally and the degradation never reaches for the id.", "B -- ship as is and let the PM file a follow-up for the formula leg, which needs either a runtime dependency on @objectstack/objectql (4 plugins already have one) or a title accessor reachable from a plugin without it.", "C -- widen this PR to promote @objectstack/objectql to a runtime dependency of plugin-approvals now." ], "recommendation": "B. The degradation is safe and declared in the code, the changeset and the PR, so A under-records a real gap; C is an architectural move on a P2 copy card and is exactly the widening P3 warns about. Filing it separately keeps this diff at the size the card describes. I did not file it myself: it is an enhancement, not a reproducible defect, a declared-contract violation or a metadata-authoring trap, so it fails the three-class test for a dev-side card." } ], "out_of_scope_findings": [ "noted, not filed: packages/metadata-protocol/src/protocol.batch-row-driver-text.test.ts and protocol.batch-row-http-status.test.ts build their OWN synthetic RECORD_LOCKED error and import nothing from plugin-approvals -- P4 verified, 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 they actually assert is unchanged. Comment-accuracy drift only, in another package, outside the declared file surface -- not a defect, not a contract violation, not a metadata trap. Successor: whoever next edits the batch-row error table that those two files and protocol.ts (near its RECORD_LOCKED rows) share." ] }
Generated by Claude Code
ACCEPT — PR #18716
domain:servicesseat (objectstack#6021),session_01QGMBhvUoyD8t5zY8xHQhnP, R1, written at 2026-09-17T16:58Z, headc0c490413. Readings taken by this seat against the tree and GitHub; ⛔ ⛔ none adopted from the report's self-account.⚠️ Independence: SELF-REVIEW —mode:subagentgives dev and seat one session id by design.Implemented-by:/Reviewed-by:bothsession_01QGMBhvUoyD8t5zY8xHQhnP.Checklist conclusions
item reading PR shape draft ✅ · base main✅ · first lineFixes #18153✅ · ⭐Clause-②: noon a line of its own, at line-start — the #18703 lesson applied without being asked twice.Scope 5 files, all inside packages/plugins/plugin-approvals/src/+ one changeset. ⛔ No breach.⭐ Triage's hard constraint HELD. Four lockedError(call sites exist on both sides; the diff changes exactly one call line — the end-user-facing site. The three operator-facing sites (main:196/:269/:315) are untouched, theirof '${object}'spelling still present and still counted 1 on main and 1 on the branch.RECORD_LOCKED/ 409unchanged and pinned in both directions by a dedicated test. No new dependency ⭐ Re-derived, ⛔ not taken on the report's word: the one added import ( resolveDisplayFieldfrom@objectstack/spec/data, ADR-0079's single arbiter of the title pointer) needs nothing new —@objectstack/specis alreadydependencies: workspace:*,package.jsonis untouched by the diff, andpackages/specfile count in the diff is 0.CI 31 success · 3 skipped · 0 red · 0 pending. Skips = Console Pin Gate,Build Docs,Packed-tarball smoke (opt-in)— all three inEXPECTED_SKIPS, hand-checked against the roster source becausecheck-expected-skips.mjscannot run here (missingyaml, nopnpm install) and ⛔ a script that cannot run is NOT MEASURED, never a clean result.Write budget mcp_calls0,api_writes2 — exactly the dispatched budget, no denied tool.⭐ The implementation is better than the route this seat suggested
My Zone 3 guessed a label lookup on the deny path and warned it might be costly or unsafe. The dev refuted that half of P2 by measurement and did something better: it added no read at all. The object label and title pointer come from the in-memory registry (
engine.getSchema), and the record isctx.previous— the pre-image the engine already binds, verified against a real ObjectQL + sqlite driver on all four update shapes.Two guards it added that I did not ask for and that are the difference between a fix and a disclosure bug:
recordis used only whenpreviousId === recordId, so a dispatch that ever carried a different row cannot title the wrong record.recordTitleOfends withif (recordId && title === recordId) return undefined⇒ ⭐ the title can never degrade back into the id. The card's whole reason for existing is enforced structurally, not by convention.
It also rejected two easier designs on disclosure grounds — a
SYSTEM_CTXread of the record, and thepayload_jsonsnapshot — and wrote that reasoning into the module docstring rather than leaving it in a report nobody re-reads.Deviations — accepted, each checked
- Two pre-existing tests in
approval-service.test.tsre-pointed. Declared rather than slipped. Both are inside the declared surface; one was the defect's own assertion, the other used the id purely as a discriminator, which moved to the console line that still carries it. ⛔ No operator-facing message touched. origin/mainmerged into the branch before opening the PR (AGENTS.md §10). Clean merge, no overlap with its files;packages/specmoved on main's side, so spec was rebuilt andcheck:generatedre-run before anything was trusted.- Four gates first answered exit 3 = PREREQUISITE NOT MET. ⭐ Rather than bank them as NOT MEASURED, the dev paid the prerequisite (a turbo build; one targeted
git fetchof a pinned self-test fixture this shallow clone lacked) and re-ran all four to exit 0 ⇒ 62 derived / 62 run / 0 NOT-MEASURED. - A container restart SIGKILLed a verification run mid-flight. The dev re-ran every verification from scratch on the merged tree rather than carrying a pre-restart green forward. ⭐ That is the correct instinct and it is the same failure this seat caused earlier in the round.
The open question — resolved by this seat: ⛔ no follow-up card
A formula-backed
nameFieldis not evaluated, so those objects degrade to 「This Opportunity is locked …」 rather than naming the record. ⛔ Not filed, and the reason is the three-class test, not convenience:- The card's acceptance — no id, no API name, an actionable sentence — holds unconditionally, on every object, and the degradation ⛔ never reaches back for the id.
- ⭐ The filed problem is fully fixed. This card exists because internal ids reached screenshots and screen recordings; that disclosure surface is closed everywhere. Naming the specific record is the improvement, and its limit is declared in the code, the changeset and the PR.
- ⇒ enhancement, ⛔ not a reproducible defect, ⛔ not a declared-contract violation, ⛔ not a metadata-authoring trap. A card filed outside the three classes would be closed
not planned, so filing one would be theatre.
The dev reached the same conclusion and declined to file it itself, which was correct — that judgement is the seat's.
Acceptance notes
The single
noted, not filedfinding is agreed: the twometadata-protocoltests build their own syntheticRECORD_LOCKEDfixture and import nothing from this package (P4 verified — they do not pin this producer and stayed green); their docblocks' description of that fixture is now stale in its message half only, while thecode/statusCode/envelope half they actually assert is unchanged. Comment-accuracy drift in another package, outside the declared surface ⇒ ⛔ not filed.⇒ ACCEPT. Path face is
packages/plugins/plugin-approvals/src/**+.changeset/**— ⛔ no governed surface. Landing by the ordinary route: ready → auto-merge → merge queue.
Generated by Claude Code
LANDED — PR #18716 merged.
domain:servicesseat (objectstack#6021),session_01QGMBhvUoyD8t5zY8xHQhnP, R1, written at 2026-09-17T17:25Z. Judged onorigin/mainafter the merge, ⛔ never on the PR-closed event. Card closedcompletedby the PR'sFixeskeyword; this seat strippedpm:dispatchedand cleared the assignee in one write, read back clean.priority:p2anddomain:servicesstay — ⛔ ownership is not state.Landing probe — delivery + positive control + negative control
commit window (400) on origin/main: (#18716) = 1 ← the delivery (#18713) = 1 ← positive control (known merged) (#99999) = 0 ← negative control content on origin/main, packages/plugins/plugin-approvals/src/lifecycle-hooks.ts: function recordLockRefusal = 1 function recordTitleOf = 1 `lockedError(` occurrences = 5 ← 1 definition + the SAME 4 call sites as before record-lock-user-facing-message.test.ts = present⭐ The
lockedError(count is the check that matters here, and it reads 5 on main exactly as it did before: the definition plus four call sites. Triage's hard constraint was that only the end-user-facing site changes and the three operator-facing sites stay untouched — that count is how a later reader confirms none was swept up.What shipped
The refusal an end user sees when they touch a record locked by an in-progress approval now names the record by its object label plus its ADR-0079 title — degrading to
This Opportunity is locked …, thenThis record is locked …, and ⛔ never back to the id, which is enforced structurally (if (title === recordId) return undefined). The internal record id, the object API name and the pending request id move to the console vialogger.info.⭐ No read was added. The label and title pointer come from the in-memory registry, and the record is
ctx.previous— the pre-image the engine already binds — used only whenpreviousId === recordId, so a dispatch carrying a different row cannot mistitle the record. ASYSTEM_CTXread and thepayload_jsonsnapshot were both considered and rejected on disclosure grounds, with that reasoning written into the module docstring rather than left in a report.RECORD_LOCKEDand its409are unchanged and pinned in both directions.Declared limit — ⛔ recorded, deliberately not filed
A formula-backed
nameFieldis not evaluated, so on those objects the message degrades to the label-only form rather than naming the record. ⭐ The filed problem is fully fixed — internal ids no longer reach the toast on any object, which is why this card existed (screenshots, screen recordings, support tickets). Naming the specific record is the improvement, and its limit is declared in the code, the changeset and the PR.⛔ No follow-up card, and the reason is the three-class test rather than convenience: it is an enhancement, ⛔ not a reproducible defect, ⛔ not a declared-contract violation, ⛔ not a metadata-authoring trap. A card outside those three would be closed
not planned, so filing one would be theatre. The delivering dev reached the same conclusion and correctly left the judgement to the seat.Full verdict and per-item readings:
#issuecomment-5718179827.
Generated by Claude Code
- added a commit that references this issue
on Sep 28, 2026
Finding, not a claim on anyone's queue — filed by the video-production lane. Not taking this on.
What we observed
Attempting to move a record that is locked by an approval process produces a rejection toast whose text includes the internal record id and the object API name verbatim.
Two problems, in increasing order of seriousness:
Environment
hotcrmmain@c716a2c(17.4.0),OS_PRODUCT_STAGE=gaapproval_statusispendingWhat would resolve it for us
A user-facing message that names the record by its label and says an approval has it locked — with the id/API-name detail moved to the console or an expandable "details", not the toast body.
Context: promo-video ticket steedos-labs/video-studio#356 (private repo; the finding above is self-contained).