Skip to content

Commit 9a0108c

Browse files
fix(app-shell): a record locked by an approval is shown locked to a reader outside that approval (#26402)
<!-- claude-projects-attribution: {"github_login":"os-steve"} --> _Requested by **Steve** · [project thread](https://claude.ai/code/project/chan_01FdL1XeUyvVNxLreh3F4Qi9?thread=cmsg_01FdL1XeUyvVNxLreh3F4Qi99vermfGkBLv9Vtsw2EJzVK)_ 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 (#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 #26053 merged (`054652bf`), so the serial condition is met. Line numbers drifted, and every substance held. - **Final head `7d11d10e`** (main merged after #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](https://claude.ai/code/session_01ADHkmQYqTcUW7GgPkfWCzw)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent e0d6317 commit 9a0108c

4 files changed

Lines changed: 369 additions & 7 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@object-ui/app-shell': patch
3+
---
4+
5+
fix(app-shell): a record locked by an approval is shown locked to a reader outside that approval
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
A member who can read a record but is not on its approval (not an approver, not the submitter, no override) was offered inline edit while a `lockRecord: true` approval held the record, and the save was then refused with `409 RECORD_LOCKED`. The approvals list serves a request only to its participants, so the record page read that member's empty list as "no approval is running".
11+
12+
When the list finds no pending request, the record page now also reads the record's own lock, `client.approvals.getRecordLock(object, recordId)`, which the server answers to every caller who can read the record with `{ locked }` and nothing else. A locked answer shows the record locked: Edit is disabled, inline edit is not offered, and the "Locked for approval" band is drawn without a recall button, because a reader who cannot see the request did not submit it. The reader still sees nothing of the request itself (approvers, comments, attachments).
13+
14+
Unchanged: a pending request the viewer can see still decides with its own node's `lockRecord`, and a composition without the approvals service (the lock read answers `501`) still opens an answered empty record editable.

‎ui/packages/app-shell/src/hooks/useRecordApprovals.ts‎

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,18 @@
2828
* approvals service still mounts the list route and answers it
2929
* `200 { data: [] }`: that is an answered read with no request, not an absent
3030
* route, and the result's `answered` says which one happened (objectui#12220).
31+
*
32+
* That list serves a request only to its participants (slate, submitter,
33+
* override actor), so for anyone else an answered empty read cannot tell "no
34+
* approval is running" from "one is running that you may not see". When the
35+
* list finds no pending request, the hook also asks the server whether the
36+
* record is held by a locking approval (`client.approvals.getRecordLock`,
37+
* answered to every caller who can read the record — objectstack#25934 ruling
38+
* A) and hands the answer back as `recordLock`.
3139
*/
3240

3341
import { useCallback, useEffect, useMemo, useRef, useState, useSyncExternalStore } from 'react';
42+
import { useAdapter } from '../providers/AdapterProvider.js';
3443
import { bearerAuthHeaders } from '../utils/authToken.js';
3544
import type { DecisionOutputDef } from '../utils/decisionOutputParams.js';
3645

@@ -278,6 +287,18 @@ interface UseRecordApprovalsResult {
278287
*/
279288
pendingRequest: ApprovalRequestLite | null;
280289
latestRequest: ApprovalRequestLite | null;
290+
/**
291+
* The server's record lock (`GET /approvals/records/:object/:recordId/lock`,
292+
* objectstack#25934), read only when the list found no pending request:
293+
* `{ locked: true }` means a pending approval whose node locks the record is
294+
* running, though this viewer cannot see its request. `null` when there is
295+
* no answer to give: a pending request was found (it decides, with its own
296+
* `lock_record`), no adapter is mounted, or the read has never answered — a
297+
* composition without the approvals service answers it 501, so there the
298+
* record is not locked by this signal. A later failed read keeps the last
299+
* answer, as the list read does.
300+
*/
301+
recordLock: { locked: boolean } | null;
281302
refresh: () => Promise<void>;
282303
}
283304

@@ -415,6 +436,8 @@ interface ApprovalsSnapshot {
415436
answered: boolean;
416437
/** Rows as the server sent them, the pending one enriched. Unsorted. */
417438
rows: ApprovalRequestLite[];
439+
/** The server's record lock, when the rows hold no pending request. */
440+
recordLock: { locked: boolean } | null;
418441
}
419442

420443
const EMPTY_ROWS: ApprovalRequestLite[] = [];
@@ -423,6 +446,7 @@ const INITIAL_SNAPSHOT: ApprovalsSnapshot = Object.freeze({
423446
available: true,
424447
answered: false,
425448
rows: EMPTY_ROWS,
449+
recordLock: null,
426450
}) as ApprovalsSnapshot;
427451

428452
interface ApprovalsEntry {
@@ -467,8 +491,16 @@ function holdEntry(key: string): () => void {
467491
};
468492
}
469493

494+
/** Reads the server's record lock for the record a read is for. */
495+
type RecordLockReader = () => Promise<{ locked: boolean }>;
496+
470497
/** Send the record's read and publish its answer to every consumer of the key. */
471-
function startRead(key: string, objectName: string, recordId: string): Promise<void> {
498+
function startRead(
499+
key: string,
500+
objectName: string,
501+
recordId: string,
502+
readLock: RecordLockReader | null,
503+
): Promise<void> {
472504
const seq = ++readSeq;
473505
latestSeq.set(key, seq);
474506
const entry = entries.get(key);
@@ -484,11 +516,26 @@ function startRead(key: string, objectName: string, recordId: string): Promise<v
484516
// header — so exactly one follow-up read, never one per row.
485517
const pending = rows.find((r) => r.status === 'pending');
486518
const full = pending ? await fetchProgressEnrichment(pending) : null;
519+
// A pending row the viewer can see decides the lock with its own node's
520+
// `lock_record`. Without one, ask the record itself (objectstack#25934):
521+
// the list hides a request from a non-participant, the lock does not.
522+
let recordLock: { locked: boolean } | null = null;
523+
if (!pending && readLock) {
524+
recordLock = entries.get(key)?.snapshot.recordLock ?? null;
525+
try {
526+
const lock = await readLock();
527+
recordLock = { locked: lock?.locked === true };
528+
} catch {
529+
// A read that never answered (501 without the approvals service)
530+
// leaves none; a failure after an answer keeps it, as the rows are.
531+
}
532+
}
487533
if (latestSeq.get(key) !== seq) return;
488534
publish(key, {
489535
rows: full ? rows.map((r) => (r === pending ? full : r)) : rows,
490536
available: true,
491537
answered: true,
538+
recordLock,
492539
});
493540
} catch (err: any) {
494541
if (latestSeq.get(key) !== seq) return;
@@ -543,6 +590,14 @@ export function useRecordApprovals(
543590
// for the render, mirrored into a ref the mount effect and `refresh` read.
544591
const [unavailable, setUnavailable] = useState(false);
545592
const unavailableRef = useRef(false);
593+
// The record-lock read goes through the SDK (`client.approvals.getRecordLock`),
594+
// so it needs the adapter's client; with no adapter mounted it is skipped.
595+
const adapter = useAdapter();
596+
const lockReader = useCallback(
597+
(object: string, id: string): RecordLockReader | null =>
598+
adapter ? () => adapter.getClient().approvals.getRecordLock(object, id) : null,
599+
[adapter],
600+
);
546601

547602
const subscribe = useCallback((l: () => void) => subscribeKey(readKey, l), [readKey]);
548603
const getSnapshot = useCallback(
@@ -565,19 +620,19 @@ export function useRecordApprovals(
565620
// read only when neither exists. The entry may be new while a read is in
566621
// flight (StrictMode's replayed mount): that read publishes into it.
567622
if (entry && !entry.read && !inFlight.has(readKey) && !unavailableRef.current) {
568-
void startRead(readKey, objectName, recordId);
623+
void startRead(readKey, objectName, recordId, lockReader(objectName, recordId));
569624
} else if (entry && inFlight.has(readKey)) {
570625
entry.read = true;
571626
if (!entry.snapshot.loading) publish(readKey, { loading: true });
572627
}
573628
return release;
574-
}, [readKey, objectName, recordId]);
629+
}, [readKey, objectName, recordId, lockReader]);
575630

576631
const refresh = useCallback(async () => {
577632
if (!objectName || !recordId || !readKey) return;
578633
if (unavailableRef.current) return;
579-
await startRead(readKey, objectName, recordId);
580-
}, [objectName, recordId, readKey]);
634+
await startRead(readKey, objectName, recordId, lockReader(objectName, recordId));
635+
}, [objectName, recordId, readKey, lockReader]);
581636

582637
const requests = readKey === null ? EMPTY_ROWS : snapshot.rows;
583638
const loading = snapshot.loading;
@@ -613,6 +668,7 @@ export function useRecordApprovals(
613668
requests: sortedRequests,
614669
pendingRequest,
615670
latestRequest,
671+
recordLock: readKey === null ? null : snapshot.recordLock,
616672
refresh,
617673
};
618674
}

0 commit comments

Comments
 (0)