Skip to content

fix(app-shell): the record page reads the lock service's 501 silently and asks no more - #26841

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

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

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Irene · project thread

Fixes #26632

Clause-②: no
Metadata-protocol: none

维护者速读

  • 改了什么:记录页读审批锁时不再走 SDK 客户端,改用 hook 自己的 fetch;501 静默并记住,本次页面加载内不再读锁;其它失败仍按原格式记日志。
  • 为什么改:没有审批服务的部署(新脚手架)每打开一条记录就在浏览器控制台打一条红色 HTTP request failed ... -> 501,而 501 正是设计中的「没有锁服务」。
  • 风险与回滚:只动 app-shell 一个 hook;锁读请求改走与审批列表读相同的传输,不再带 SDK 客户端的头。回滚即 revert 本 PR。
  • 先看:ui/packages/app-shell/src/hooks/useRecordApprovals.ts 的 readRecordLock 与 startRead 里的读锁分支;RecordDetailView.approvalLock-25934.test.tsx 新增两条用例。
  • 两分钟验证:cd ui && pnpm exec vitest run packages/app-shell/src/views/RecordDetailView.approvalLock-25934.test.tsx,预期 6/6 通过。
  • 需要你做的:无,席位落地。

What changes

useRecordApprovals asks GET /approvals/records/OBJECT/ID/lock when the approvals list finds no pending request (#25934). It used to read it through client.approvals.getRecordLock. The SDK client logs every non-2xx with logger.error('HTTP request failed', …) before its caller sees the answer, and the console's client logger (createQuietHttpLogger in @object-ui/data-objectstack) prints that with console.error. So on a composition without the approvals service, whose designed answer to that read is 501 NOT_IMPLEMENTED, every record open printed a red line, although the hook already treated the 501 as "not locked".

Measured producer: packages/client/src/index.ts fetch() (the unconditional logger.error on !res.ok) → ui/packages/data-objectstack/src/index.ts createQuietHttpLogger().error → console.error. The PM hypothesis held. The fix lands in app-shell, the expected file surface, so @objectstack/client is not changed.

  • The lock is now read through the hook's own fetchJson, the same transport the approvals list read beside it already uses. That transport logs nothing. The { success, data } envelope is unwrapped as the SDK did.
  • A 501 is read silently and latched in a module-scoped flag (recordLockAbsent): no record asks again until the page reloads. __resetRecordApprovalsStore clears it.
  • Any other failure still logs, through the same createQuietHttpLogger the SDK client would have used: HTTP request failed: GET URL -> STATUS [CODE], a 404 still demoted to debug. The next open asks again.
  • The read still happens only where an adapter is mounted, as before. A first draft dropped that gate, and RecordDetailView.recordOpenWaves-13010 caught the lock read appearing as a second wave on its adapter-less page. The gate is kept.

Why

The triage ruling on the card (6441358448): the record page treats a 501 from the lock read as "no lock service", silent, no retry; any other failure still logs.

Tests

  • RecordDetailView.approvalLock-25934.test.tsx now stubs the lock route on the global fetch rather than the adapter's SDK method, and pins that the SDK method is never called.
  • New: the 501 logs no console.error line naming the lock path, and a second open of the record reads the list again but not the lock.
  • New: a 500 logs one identified line (GET … -> 500), and the next open asks again.
  • Ablation: with the 501 branch mutated to never match (scripts/ablation-replace.mjs, restore verified by blob hash), the new 501 test went red on the HTTP request failed: GET /api/v1/approvals/records/account/acc_northwind/lock -> 501 [NOT_IMPLEMENTED] line, 1 failed / 5 passed.

Acceptance notes

  • The browser's own network line for the first 501 still appears once per page load. JS cannot suppress it, and the console cannot know the service is absent without asking, because the approvals list door answers 200 { data: [] } when the service is missing while the lock door answers 501. Removing that last line needs the two server doors to agree, which is domain:server work (packages/rest/src/approvals-routes.ts) and outside this card. Noted, not filed.
  • The lock read no longer carries the SDK client's X-Environment-Id and Accept-Language headers. The approvals list read beside it never did, and the console's adapter sets no environment id.

Verification

All on head a0185cf, in a shared container (seconds are shared-box seconds).

  • cd ui && pnpm exec vitest run packages/app-shell/src/hooks/useRecordApprovals packages/app-shell/src/hooks/__tests__/useRecordApprovals packages/app-shell/src/views/RecordDetailView.recordOpenWaves-13010.test.tsx packages/app-shell/src/views/RecordDetailView.approvalLock packages/app-shell/src/views/RecordApprovalsPanel: Test Files 15 passed (15), Tests 107 passed (107).
  • pnpm --filter @object-ui/app-shell run type-check: exit 0.
  • node scripts/pm/dispatch-gates.mjs --commands: 34 derived commands, each run locally with exit 0, pnpm check:type-check-debt included (re-run under the verify lock after its first run could not get the lock). node scripts/check-sdui-manifest.mjs exit 0.
  • Narrowed, declared: the full app-shell package suite was not run to completion locally. It was stopped after 40 minutes on the shared box, after it had already surfaced the waves-13010 regression fixed above. CI's Test UI runs it.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic.
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes. (No formatter ran. Lint is left to CI.)
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: its generated artifacts are regenerated and committed. (Not touched.)
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English.
  • The Before Submitting list in CONTRIBUTING.md is done.

Generated by Claude Code

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tooling labels Oct 27, 2026
@objectstack-fleet

objectstack-fleet Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor Author

CI red on head a0185cfb: not this PR's. (2026-10-27T18:36Z; Test UI 3/16 added 18:40Z)

All three failing checks fail in files this PR does not touch (its diff is useRecordApprovals.ts, RecordDetailView.approvalLock-25934.test.tsx and one changeset).

Plan: once #26849 lands on main, the branch takes one merge of main (it carries all three fixes), CI reruns, and the seat lands this PR on green. No code change on this branch.


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

维护者速读 · 席位定稿

  • 改了什么:记录页读审批锁改走 hook 自己的 fetch,不再经 SDK 客户端;501 静默并在本次页面加载内记住、不再读锁;其它失败仍按原格式记日志。
  • 为什么改:没有审批服务的部署每打开一条记录,控制台就打一条红色 HTTP request failed ... -> 501,而 501 正是设计中的「没有锁服务」(console: every record page on a deployment without the approvals service logs two console errors for the designed 501 of GET /approvals/records/OBJECT/ID/lock (18.2.0 scaffold) #26632)。
  • 风险与回滚:只动 app-shell 一个 hook;锁读请求不再带 SDK 客户端的 X-Environment-Id / Accept-Language 头(旁边的审批列表读本来就不带)。回滚即 revert 本 PR。
  • 先看:ui/packages/app-shell/src/hooks/useRecordApprovals.ts 的 readRecordLock 与 startRead 的读锁分支。
  • 两分钟验证:cd ui && pnpm exec vitest run packages/app-shell/src/views/RecordDetailView.approvalLock-25934.test.tsx,预期 6/6 通过。
  • 需要你做的:无,席位落地。
  • 席位意见:ACCEPT。行为与裁定 6441358448 一致(501 静默、不重试,其它失败照常记录);有适配器才读锁的门保留,消融证明新用例能抓回红线。

席位核对

domain:ui seat 2 · session_01J8VVAkHqAv2eFKVSt9LqkC · reviewed at 2026-10-27T23:12Z on head d88214ad88de7e05852e6b24357c61394dc22272 · verdict ACCEPT

  1. Shape: draft → main; first closing line Fixes #26632, the only closing keyword in the body. Three files, +167 / −51: useRecordApprovals.ts, RecordDetailView.approvalLock-25934.test.tsx, one changeset (app-shell, console patch). Inside the claimed surface ui/packages/app-shell/**.
  2. Premise measured: the producer chain (packages/client fetch() logger.error → createQuietHttpLogger().error → console.error) was traced by the dev; the PM hypothesis held. The fix stays in app-shell; @objectstack/client is untouched.
  3. Gates (pinned to head): 52 check-runs, 43 success, 9 skipped, 0 failed; Lint & Repo Gates success, TypeScript Type Check success, every Test UI shard success. mergeable_state: clean. check-governed-merges --pr 26841: NOT governed, 218 changed lines.
  4. CI history: the first run on a0185cfb was red on Test UI 3/16, 14/16 and 15/16, none in this diff (comment 6450507589). Main fixed them in test(app-shell): flowCanvasChip-11665 awaits the lazy studio zh catalogue before reading the tooltip #26800 and fix(ui): the action key inventory lists disabledReason #26859. The branch took one merge of main (relay pr_update_branch, 22:52Z) and went green with no code change.
  5. Accepted deviations: none.
  6. Out of scope: the browser's own network line for the first 501 remains, once per page load. Removing it needs the approvals list and lock doors on the server to agree (domain:server). Noted in the PR, not filed.
  7. Landing: ordinary queue landing: pr_ready + automerge_enable through the relay in this window.

Pointer on the card: ACCEPT → this comment.


Generated by Claude Code

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