Skip to content

fix(app-shell): a record locked by an approval is shown locked to a reader outside that approval - #26402

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-i81sbn
Oct 27, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/project-thread-i81sbn

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Requested by Steve · project thread
Fixes #25934

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么: 记录页在审批列表里找不到待审单时,再问一次服务器"这条记录锁了吗"(client.approvals.getRecordLock);答"锁了"就按锁定显示:编辑置灰、不给行内编辑、显示"审批锁定"横幅且不显示撤回按钮。
  • 为什么改: 能读记录但不在审批名单上的成员,读审批列表永远是空的,页面就当"没在审批",让他双击编辑,保存时才被服务器 409 拒绝(裁决 A,6422939157)。
  • 风险与回滚: 只改 app-shell 两个文件,不动导出类型和服务端;每次打开没有可见待审单的记录多一个 GET 请求。没有审批服务的部署该接口答 501,行为不变。回滚即 revert 本 PR。
  • 先看: ui/packages/app-shell/src/views/RecordDetailView.tsx(lockedForReader 与撤回按钮的 approvalIsSubmitter)、ui/packages/app-shell/src/hooks/useRecordApprovals.ts(startRead 里的锁读取)、新测试 RecordDetailView.approvalLock-25934.test.tsx。
  • 两分钟验证: pnpm --filter @object-ui/app-shell exec vitest run --root ../.. packages/app-shell/src/views/RecordDetailView.approvalLock-25934.test.tsx,预期 4 个用例全绿。
  • 需要你做的: 无,席位落地。

What changes

The approvals list (GET /approvals/requests?object=…&recordId=…) serves a request only to its participants, so a member who can read a record but is not on its approval read that list as 200 { data: [] } while a lockRecord: true approval held the record. The record page took the answered empty read for "no approval is running" (rule 2 of resolveApprovalLock) and offered inline edit, and the save answered 409 RECORD_LOCKED.

Ruling A (6422939157) added a lock signal every reader can see: GET /approvals/records/:object/:recordId/lock and client.approvals.getRecordLock (#26314, PR #26361), answering { locked } only. This PR is the console half.

  • useRecordApprovals (app-shell): when the list read finds no pending request, the shared read also calls client.approvals.getRecordLock(object, recordId) through the mounted adapter's client (no hand-rolled URL), and the result carries recordLock: { locked } | null. A pending request the viewer can see still decides and the lock is not read. With no adapter mounted, the lock read is skipped. A lock read that never answered (501 without the approvals service) leaves null; a later failure keeps the last answer, as the rows are kept.
  • RecordDetailView (lock region only): { locked: true } from the record lock is fed to resolveApprovalLock as its open approval. That is rule 1's input exactly, since the server answers true only when a pending request from a node whose lockRecord is not false exists. So the page reads locked and pending, Edit is disabled and canEdit is false. No lock answer means no open approval, so rule 2 is unchanged for a composition without the approvals service. For a lock known only from the record lock, approvalIsSubmitter is false: the reader cannot read the request, and its submitter always can, so the band offers no recall lever.
  • No change to ui/packages/react (resolveApprovalLock / ApprovalLockInput need no new input, because the signal is rule 1's input) or to ui/packages/plugin-detail (the DetailView band computes from the host's threaded locked / approvalPending / approvalIsSubmitter, it does not compute its own lock input). No sdui record is written: app-shell is not a registry package, and the per-package records are retired (ci(sdui): retire the per-package SDUI manifest records; CI's regeneration holds freshness (#26137 PR B) #26367).
  • Clause-②: no: the claim graded it yes (widening) provisionally, on the assumption that ApprovalLockInput would gain an input. It does not. The one type that gains a member, the hook's result interface, is not reachable from @object-ui/app-shell's entry: src/index.ts does not re-export useRecordApprovals and the interface is not exported. This was measured on the source entry. The app-shell dist was not built in this container.

Tests

  • New ui/packages/app-shell/src/views/RecordDetailView.approvalLock-25934.test.tsx (same probe as RecordDetailView.approvalLock-12220, record with no approval_status mirror, like showcase):
    • a non-participant (list 200 empty, lock { locked: true }) gets locked, approvalPending, canEdit: false, approvalIsSubmitter: false, and the lock is read with the page's object and record id;
    • lock { locked: false } opens editable;
    • lock read refused 501 (no approvals service) opens editable, the answered empty read is not locked;
    • a participant's pending lock_record: false row decides (pending, editable) and the lock is not read.
  • The objectui#12220 tests are unchanged and green: RecordDetailView.approvalLock-12220.test.tsx, useRecordApprovals.answered-12220.test.tsx (and resolveApprovalLock-12220.test.ts / DetailView.approvalReadAnswered-12220.test.tsx, whose packages this diff does not touch). No test edit was forced.

Acceptance notes

  • One extra GET per record open where the list finds no pending request. It goes through the adapter's authenticated, settle-counted client fetch.
  • The lock read starts with the list read on mount, on refresh(), and again when the mounted adapter changes (lockReader is a useCallback over [adapter], since 40ca935). The console's AdapterProvider mounts the adapter before the record page reads.

Verification

  • Premises (re-run before any edit, origin/main 61c07986). 1: InlineEditContext.tsx:107 rule 2 holds. 2: resolveApprovalLock({ at RecordDetailView.tsx:1664 and DetailView.tsx:1716. 3: the only lock source was the list read, useRecordApprovals.ts:480. 6: lifecycle-hooks.ts beforeUpdate :538, lockedError(recordLockRefusal :633, refusal text :294. 7: lock_record?: boolean at approval-service.ts:336. 9: PR fix(app-shell,react,plugin-detail): fold an override-only admin's approval levers into one menu and draw Recall once #26053 merged (054652bf), so the serial condition is met. Line numbers drifted, and every substance held.
  • Final head 7d11d10e (main merged after fix(app-shell): a member takes back their own comment reaction through the unreact door #26384; 40ca935 drops an unused act import, the Lint UI red, and the hook's render-time ref write): one locked run after a fresh install: turbo run build --filter='@object-ui/app-shell^...' 63/63, turbo lint --filter=@object-ui/app-shell 0 errors, app-shell type-check exit 0, 82 neighbouring files / 554 tests, VERDICT command-exit 0. Earlier, on fca25b0b, one locked run: turbo run build --filter='@object-ui/app-shell^...' (64/64), then pnpm --filter @object-ui/app-shell type-check (src and tsconfig.test.json), then 4 files: the new test, RecordDetailView.approvalLock-12220, useRecordApprovals.answered-12220, RecordDetailView.approvalRecallGate. Test Files 4/4, Tests 25/25, VERDICT command-exit 0.
  • Neighbours (pre-merge head e0f9e560): all 71 RecordDetailView.*.test.tsx, the useRecordApprovals* hook tests, RecordApprovalsPanel* and AppContent.recordSaveToastNamesRecord-26172. Test Files 82/82, Tests 551/551, VERDICT command-exit 0. The full app-shell suite is left to CI Test UI.
  • Ablation. The fix was committed, then useRecordApprovals.ts and RecordDetailView.tsx were restored to the base 61c07986 (on disk: getRecordLock 3→0, lockedForReader 3→0). The new test's non-participant case went red at :235 (expected 'false' to be 'true', locked). Both lock-read cases went red because getRecordLock was never called. The participant control stayed green: Tests 3 failed / 1 passed, VERDICT command-exit 1. Restored with git checkout HEAD --: git diff HEAD empty, blob hashes equal HEAD's. app-shell tests import the source directly, so no dist leg applies.
  • Gates on 7d11d10e: dispatch-gates --commands derived 34 (main moved), all 34 exit 0, 0 NOT-MEASURED. On fca25b0b: derived 35. All 35 exit 0, check:type-check-debt included. --ran: 35 derived, 35 run, 0 NOT-MEASURED, 0 UNRUN. Roster gate under .changeset: check-changeset-fixed.mjs exit 0. All other families: this PR's check-runs.

Generated by Claude Code

…approval

The approvals list serves a request only to its participants, so a
non-participant reader read an answered empty list while a lockRecord
approval ran, and the record page offered inline edit the server then
refused with RECORD_LOCKED. When the list finds no pending request,
useRecordApprovals now reads client.approvals.getRecordLock and the page
feeds a locked answer to resolveApprovalLock as the open approval.

Claude-Session: https://claude.ai/code/session_01ADHkmQYqTcUW7GgPkfWCzw
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · seat domain:ui#1 · 2026-10-27T03:05Z

Head 7d11d10e (read from the PR object). It merges main after #26367 and after #26384, which edits the same file. The FAIL round fix, 40ca935, drops the unused act import (the Lint UI red, object-ui/no-unused-imports). It also removes a render-time ref write in the hook: lockReader is now a useCallback over [adapter], so an adapter change re-reads the lock. The seat corrected the body's acceptance note. Re-verified on this head: build 63/63, app-shell lint 0 errors, type-check, 82 files / 554 tests, 34/34 gates.

  • Shape. Draft on main, Fixes #25934, carrying out ruling 6422939157 (letter A, readers). 4 files, all app-shell plus the changeset:
    • useRecordApprovals.ts: reads client.approvals.getRecordLock (feat(plugin-approvals,rest,client): a record-anchored approval lock read, { locked } for every caller who can read the record #26361) through the adapter's client, only when the request list finds no pending row.
    • RecordDetailView.tsx: feeds { locked: true } to resolveApprovalLock as openApproval, which is rule 1's input as written. A signal-only lock threads approvalIsSubmitter = false, so the band offers that reader no recall.
    • A new non-participant regression test and a root changeset.
    • react, plugin-detail and DetailView.tsx are untouched, because the band reads the host's threaded verdict.
    • Clause-②: no, re-declared from the provisional yes (widening): ApprovalLockInput is unchanged, and the only widened type is the hook's non-exported result. Metadata-protocol: none.
  • Premises. Premises 1–3, 6, 7 and 9 of 6419712936 re-ran and hold; the only drift is line numbers. Rule 2 is unchanged: with no approvals service the route answers 501, so there is no signal and the record is not locked. The four objectui#12220 tests don't flip. ⛔ No retreat to B or D.
  • Checks pinned to the head. All check runs completed: 0 failing, 0 pending. The seat's landing script checked this before posting.
  • Governed surface. check-governed-merges.mjs --pr 26402: not governed.
  • Evidence (dev).
    • 35/35 gates, 0 NOT-MEASURED.
    • Final head: 4 files / 25 tests, type-check green.
    • Before the merge: 82 neighbouring files / 551 tests.
    • Ablation: reverting the source turns the non-participant case red.
  • Serial constraints. RecordDetailView.tsx is shared with fix(app-shell): a member takes back their own comment reaction through the unreact door #26384, which has merged; this head is built on top of it (a clean merge, then type-check and tests). The ⛔ paths are untouched.

Landing: ready + auto-merge through the relay.

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 tooling

Projects

None yet

2 participants