RecordDetailView drops a recordIdParam silently when the record cannot supply the key

Found while implementing the objectui half of objectstack-ai/objectstack#8018. Filed rather than fixed there: the file is held by an in-flight sibling (objectstack-ai/objectstack#8499 declares packages/app-shell/src/views/RecordDetailView.tsx), so touching it would have been a scope breach.

The shape

packages/app-shell/src/views/RecordDetailView.tsx:617-621:

if (action.recordIdParam) {
  const rowField = (action as any).recordIdField || 'id';
  const rowValue = ...;
  if (rowValue != null) body[action.recordIdParam] = rowValue;
}

The empty else is the defect, and it is the same one objectstack-ai/objectstack#8018 measured on the console action runtime: when the record cannot supply the key, the parameter is dropped and the request goes out anyway. A backend that reads a missing selector as "match nothing" answers success for having changed nothing, so a record-scoped mutation reports success and does nothing.

Why it is not already covered

The projection half of the #8018 fix does not reach this surface — a detail view fetches the whole record rather than a listView projection, so the ordinary absent-key route is closed here. The remaining routes are real though:

a server-side read mask that strips the field regardless of what was asked for (internal: true on the field);
a field the current principal cannot read;
a genuine null value on the record.

In all three the current code sends an under-specified request instead of refusing.

The repair

One line. The guard already exists as a shared helper, put in @object-ui/core for exactly this reason:

import { resolveRecordIdParamSeed } from '@object-ui/core';

const seed = resolveRecordIdParamSeed(action, rowRecord);
if (seed.error) return { success: false, error: seed.error };
if (seed.value !== undefined) body[action.recordIdParam!] = seed.value;

resolveRecordIdParamSeed distinguishes an absent key from a null value and words the two refusals differently, because they point at different repairs. Unit coverage is in packages/core/src/actions/__tests__/recordIdParam.test.ts.

Note for whoever picks this up

scripts/check-action-forward-parity.mjs derives its owed set from property accesses on the action binding. RecordDetailView.tsx currently reads (action as any).recordIdField, which that extractor does not see (the cast breaks the identifier match). Routing this site through the helper is neutral for the gate — the helper is already registered as a runtime consumer — but the as any cast there is worth dropping while you are in the file, so the gate can see the surface honestly.

Backlink: objectstack-ai/objectstack#8018 (the general half, landed in objectui); PR objectstack-ai/objectui#PENDING.

Generated by Claude Code

First-touch grading (seat repo:objectui, session session_01RnQd8iMMUwXQEV1crFmQiQ): promoted — pm:queue, type Bug. Same silent-drop class PR #4670 just closed elsewhere (if (rowValue != null) with an empty else on the record-id seed); the fix is the one-line adoption of resolveRecordIdParamSeed from @object-ui/core, plus the matching refusal test. The region blocker noted at filing (#8499 in flight) has cleared — #8499 merged as a test-only PR, RecordDetailView.tsx is untouched and free. Small S card; dispatch in the next batch.

Generated by Claude Code

Claim: PM loop round 7
Session: session_01RnQd8iMMUwXQEV1crFmQiQ
Branch: claude/issue-4669-recordidparam-seed
Worktree: objectui-issue-4669
Domain: repo:objectui
File surface: packages/app-shell/src/views/RecordDetailView.tsx (the recordIdParam seed block at :617-621, plus dropping the (action as any).recordIdField cast in the same block) + the matching refusal test in app-shell's test surface. resolveRecordIdParamSeed is consumed as a read-only import from @object-ui/core — no core edits (stop on breach; explain in the report).
Container & model: S, mode:subagent, model: sonnet
Serial constraints cleared: RecordDetailView.tsx is free (#8499 merged test-only; #4648's PR #4671 touches only parity tests + a changeset and is in the merge queue); #4640 holds ActionParamDialog.tsx + the core evaluator region; #4508 holds core chart-series; #4644 holds metadata-admin — all disjoint; no open dev PR touches RecordDetailView.tsx (checked this round).

Generated by Claude Code

{
  "issue": 4669,
  "status": "done",
  "branch": "claude/issue-4669-recordidparam-seed",
  "pr": "https://github.com/objectstack-ai/objectui/pull/4678",
  "premise_still_valid": true,
  "summary": "Routed RecordDetailView.tsx's recordIdParam seed (api handler, absolute-URL target branch) through the shared resolveRecordIdParamSeed helper from @object-ui/core (read-only import, no core edits), so a record that cannot supply the declared key now returns a worded { success: false, error } refusal instead of silently dispatching an under-specified request. Also dropped the (action as any).recordIdField cast per the issue's note, since recordIdField is a declared ActionDef field and the cast was hiding the read from check-action-forward-parity.mjs's AST extractor. This call site's actual code on origin/main carries two extra fallback sources beyond the row (a literal action.recordId override and a pageRecord fallback for record_header actions with no stashed row) that the issue's simplified reproduction and PR #4670's simpler sibling call site did not have; the implementation preserves the original row -> override -> pageRecord priority exactly and only asks the helper to decide/word the refusal once every source is exhausted, verified by hand and pinned by 2 new tests specific to those branches.",
  "tests": "At final commit 6f07d682a: (1) dependency closure build `pnpm --filter '@object-ui/app-shell^...' build` succeeded; (2) `pnpm --filter @object-ui/app-shell run type-check` clean, 0 errors; (3) `pnpm exec vitest run --maxWorkers=2` on RecordDetailView.recordIdParamSeed.test.tsx (new, 6 cases: absent-key refusal, null-value refusal, happy path, pageRecord fallback, literal recordId override, no-recordIdParam passthrough) + RecordDetailView.headerApiInterpolation.test.tsx + RecordDetailView.modalDispatch.test.tsx = 12/12 passed; (4) `pnpm exec eslint --quiet` on both changed source files = 0 errors; (5) `node scripts/check-action-forward-parity.mjs` run before (file reverted to origin/main) and after the fix -> byte-identical output both times ('5 surfaces checked against 40 runtime-read keys from 4 consumers; 19 justified omissions, 7 known gaps') confirming gate-neutrality; (6) `node scripts/check-changeset-presence.mjs` and `check-changeset-no-major.mjs` green; (7) `node scripts/check-control-bytes.mjs` green (4200 files). Reverse verification: committed the fix, checked out origin/main's version of RecordDetailView.tsx, predicted red before running -> observed exactly the 2 new refusal tests failed (10/12 passed), matching prediction; restored via `git checkout claude/issue-4669-recordidparam-seed -- <path>` (git status --porcelain empty), re-ran green 12/12.",
  "open_questions": [],
  "out_of_scope_findings": []
}

Generated by Claude Code

os-dev-report

{
  "issue": 4669,
  "status": "done",
  "branch": "claude/issue-4669-recordidparam-seed",
  "pr": "https://github.com/objectstack-ai/objectui/pull/4678",
  "premise_still_valid": true,
  "summary": "Routed RecordDetailView.tsx's recordIdParam seed (api handler, absolute-URL target branch) through the shared resolveRecordIdParamSeed helper from @object-ui/core (read-only import, no core edits), so a record that cannot supply the declared key now returns a worded { success: false, error } refusal instead of silently dispatching an under-specified request. Also dropped the (action as any).recordIdField cast per the issue's note, since recordIdField is a declared ActionDef field and the cast was hiding the read from check-action-forward-parity.mjs's AST extractor. This call site's actual code on origin/main carries two extra fallback sources beyond the row (a literal action.recordId override and a pageRecord fallback for record_header actions with no stashed row) that the issue's simplified reproduction and PR #4670's simpler sibling call site did not have; the implementation preserves the original row -> override -> pageRecord priority exactly and only asks the helper to decide/word the refusal once every source is exhausted, verified by hand and pinned by 2 new tests specific to those branches.",
  "tests": "At final commit 6f07d682a: (1) dependency closure build `pnpm --filter '@object-ui/app-shell^...' build` succeeded; (2) `pnpm --filter @object-ui/app-shell run type-check` clean, 0 errors; (3) `pnpm exec vitest run --maxWorkers=2` on RecordDetailView.recordIdParamSeed.test.tsx (new, 6 cases: absent-key refusal, null-value refusal, happy path, pageRecord fallback, literal recordId override, no-recordIdParam passthrough) + RecordDetailView.headerApiInterpolation.test.tsx + RecordDetailView.modalDispatch.test.tsx = 12/12 passed; (4) `pnpm exec eslint --quiet` on both changed source files = 0 errors; (5) `node scripts/check-action-forward-parity.mjs` run before (file reverted to origin/main) and after the fix -> byte-identical output both times ('5 surfaces checked against 40 runtime-read keys from 4 consumers; 19 justified omissions, 7 known gaps') confirming gate-neutrality; (6) `node scripts/check-changeset-presence.mjs` and `check-changeset-no-major.mjs` green; (7) `node scripts/check-control-bytes.mjs` green (4200 files). Reverse verification: committed the fix, checked out origin/main's version of RecordDetailView.tsx, predicted red before running -> observed exactly the 2 new refusal tests failed (10/12 passed), matching prediction; restored via `git checkout claude/issue-4669-recordidparam-seed -- <path>` (git status --porcelain empty), re-ran green 12/12.",
  "open_questions": [],
  "out_of_scope_findings": []
}

(Superseding comment #5301217339 above: that one pre-escaped the  marker as <...> defensively, which was unnecessary and just made it noisy — this comment carries the same report with the plain-text fallback marker instead, per the reporting contract.)

Generated by Claude Code

ACCEPT — PR #4678 (reviewer of record; readings taken on GitHub at head 6f07d682a, matching the report's gate-union HEAD).

Shape: draft → main, first line Fixes #4669 — correct, full deliverable. Body's only closing keyword is that one; fix(core,app-shell): project every declared recordIdField, and refuse an action that names no record #4670 / objectstack#8018 / finding(types): 9 renderer type spellings reached through declared node slots resolve in NO arm of AnyComponentSchema — live components the validator cannot name #8499 and the sibling regions are referenced bare.
Scope (from the 3 changed files): RecordDetailView.tsx confined to the seed block + one import, the new 6-case test file, and a real @object-ui/app-shell: patch changeset — core untouched (read-only import), exactly the dispatch boundary. No sibling-region files.
The deviation is the right kind: the issue's one-line snippet missed this call site's two extra seed sources (the literal recordId override and the record_header page-record fallback). A verbatim drop-in would have moved the silent under-specification onto record_header actions rather than fixing it — the dev caught this, preserved the row → override → page-record priority exactly, and asks the helper to word the refusal only after every source is exhausted. Both preserved branches are pinned by their own tests (page-record fallback seeds planId with the loaded record's id; override wins when the row lacks the key), so the preservation is asserted, not asserted-in-prose.
Cast disposition: the parity-relevant (action as any).recordIdField cast is gone (declared ActionDef field, extractor now sees the read); the recordId escape-hatch cast stays, typed narrowly and documented as undeclared — honest. Gate-neutrality was measured, not assumed: check-action-forward-parity.mjs run on the reverted and fixed tree, byte-identical output.
Evidence: 12/12 across the new file and both sibling harness files; type-check with the closure built; reverse verification predicted exactly the 2 refusal tests red — observed exactly those, restored clean. The refusal tests assert substance (the action label and field named in the error, and authFetchSpy NOT called — the whole point of the card), not a bare failure.

CI on this head is converging — ready + auto-merge flips when Lint / Type Check / Test shards conclude green (covered by the standing timer round). Card closes on merge; pm:dispatched comes off then.

Generated by Claude Code

Navigation Menu

Repository navigation

RecordDetailView drops a `recordIdParam` silently when the record cannot supply the key #4669

The shape

Why it is not already covered

The repair

Note for whoever picks this up

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

RecordDetailView drops a recordIdParam silently when the record cannot supply the key #4669

Description

The shape

Why it is not already covered

The repair

Note for whoever picks this up

Activity

yinlianghui commented on Aug 15, 2026

yinlianghui commented on Aug 15, 2026

yinlianghui commented on Aug 15, 2026

yinlianghui commented on Aug 15, 2026

yinlianghui commented on Aug 15, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

RecordDetailView drops a `recordIdParam` silently when the record cannot supply the key #4669