Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

[console 17.0.0] record-header 自定义动作偶发丢 recordId:页头存在两种 DOM 变体,其中一种不注入请求体,端点回 missing_record_id #5176

Closed

Description

@baozhoutao

一句话

记录详情页头的自定义动作(record_header 挂载)点击后,平台负责往请求体注入当前记录 id(recordId)——但页头渲染存在两种 DOM 变体,其中一种变体下不注入,端点收到的请求体没有 recordId,回 missing_record_id。用户侧表现为「偶尔点一下没反应,再点一次就好」。

环境

  • @objectstack/*@17.0.0(GA);下游业务项目(os-project-titanwind-ehr,私有)真机 Console 实测发现
  • 与对象、与动作配置无关:该项目所有 record_header 自定义动作(多个对象、同一标准配方)均受影响

量化复现

同一颗页头【删除】动作按钮,同一记录页,连点 10 次:

  • 9 次:请求体带 recordId,动作正常执行;
  • 1 次:请求体不带 recordId → 服务端 missing_record_id;
  • 关键相关性:失败那次抓包时,页头 DOM 层级与成功时不同(两种变体与是否注入一一对应);
  • 换一次页面渲染即恢复。

疑似与记录页头的 hydration/半加载窗口有关(页头在加载早期与稳定后渲染树不同,注入逻辑只挂在其中一种变体上)。

期望

两种页头渲染形态下,record_header 动作的 recordId 注入行为一致;或在拿不到记录 id 时禁用按钮而非发出残缺请求。

备注

下游项目按「平台能覆盖的问题不自造补丁」原则未做任何绕行;抓包与 DOM 对照证据存于该项目取证分支,需要可提供摘录。

Activity

  1. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Triage: queued as Bug — user-reported with a quantified reproduction (1-in-10 clicks loses recordId, failure correlated one-to-one with a distinct record-header DOM variant, multiple objects/actions affected in a real 17.0.0 deployment). The card's fallback expectation is the right acceptance floor: either both header render variants inject identically, or the action is disabled while the record id is unavailable — a half-loaded header must never emit a body-incomplete request. (Triage seat, session session_019gCKd9EZfQ6MbGTnMHHJvW.)


    Generated by Claude Code

  2. self-assigned this
    on Aug 18, 2026
  3. os-steve commented on Aug 18, 2026

    @os-steve
    Collaborator

    Claim: PM loop round 1
    Session: session_01AMHbqfPiETJHA95r6WzZWS
    Branch: claude/issue-5176-record-header-recordid
    Worktree: objectui-issue-5176
    Domain: repo:objectui (execution seat; this repo has no domain:* lanes)
    File surface: packages/app-shell/src/views/RecordDetailView.tsx and its RecordDetailView.*.test.tsx siblings; packages/app-shell/src/views/DeclaredActionsBar.tsx if the second dispatch path proves to live there (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus — node scripts/pm/dispatch-gates.mjs --tier packages/app-shell/src/views/RecordDetailView.tsx reports "no path-derived mandate … tier stays the PM's per-card judgment call (floor sonnet · default opus · ceiling fable)", so this is my judgment: a 2477-line hot file with several independent recordId-injection paths and a load-bearing correctness question is the default judgment tier, not the mechanical floor.
    Clause-②: no — the fix restores a request body the platform already promises to populate. It changes no contract accept/reject behaviour and widens no public surface.
    Serial constraints cleared: no open PR touches packages/app-shell (open PRs are PR5124 / PR5080 / PR4930 on skills/**, PR5084 on scripts/, PR4655 release, plus dependabot); no in-flight claim on this package; git log shows no same-day churn on RecordDetailView.tsx.

    Assignee note (shared identity): this account (os-steve) also holds the domain:devx and domain:spec seats under different sessions, and the outgoing objectui seat ran as yinlianghui. The session ID above is what makes this claim mine — an os-steve assignment without it is not this seat's.

    Premise check, run now against origin/main @ 072085de (not recalled from the card):

    Confirmed live — record_header actions and their recordId seeding are all still in RecordDetailView.tsx, and the card's "two DOM variants" maps onto something concrete: the file carries more than one dispatch path, each with its own recordId fallback, e.g. the canonical branch's recordIdParam + bodyExtra + bodyShape seeding (~:594-660, including an action.recordId override at :641-644 and a seed.value fallback at :653), a second path at :709-727 ((action as any).recordId ?? pureRecordId), a third at :786-788, and a fourth at :835-846. There is already a pin file RecordDetailView.recordIdParamSeed.test.tsx.

    Carrying the triage ruling (comment-5326104758) verbatim as the acceptance floor:

    either both header render variants inject identically, or the action is disabled while the record id is unavailable — a half-loaded header must never emit a body-incomplete request

    PM mechanism assumption — measure it, falsification actively welcome. I think the 1-in-10 failure is a timing selection between two of those paths (the header rendering before pureRecordId resolves, so a path whose only source is the page record seeds nothing), not a bug inside any single path. That is a guess from reading, not a finding, and I have not reproduced it. Do not build on it if the measurement disagrees — report what you actually find. In particular, if the real cause is that one path is reached only in a half-loaded window, then the ruling's second limb (disable the affordance until the id exists) may be the honest fix rather than making all four paths inject identically.

    Diagnose before patching: the deliverable is knowing which paths differ and why one of them is reachable without a record id — a fix that makes the symptom go away without that answer is not acceptable here. premise_still_valid: false with evidence is a legitimate delivery.


    Generated by Claude Code

  4. os-steve commented on Aug 18, 2026

    @os-steve
    Collaborator
    {
      "issue": 5176,
      "status": "done",
      "branch": "claude/issue-5176-record-header-recordid",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5182",
      "premise_still_valid": true,
      "premise_note": "The DEFECT is real and reproduced deterministically. The card's MECHANISM framing is falsified: there is no second DOM variant carrying a second injection site. One dispatch path, one injection site; only the seed value varies with click timing. The reporter's 1:1 DOM correlation is a co-symptom (the same unresolved pageRecord withholds the title/highlights chrome AND starves the seed), not the cause.",
      "pm_assumption_check": {
        "several_distinct_paths_selected_by_timing": "FALSE — a record_header api action with recordIdParam always takes the same path (RecordDetailView apiHandler, absolute-target branch). The 4 sites the PM counted are not alternatives for this action: :709-727 is the legacy dataSource.update default branch, :786-788 the flow trigger, :835-846 the server-action bridge. Different action types, not a timing selection.",
        "failing_path_only_id_source_is_the_resolved_page_record": "TRUE, and this is the finding. The api path's chain was row -> literal recordId override -> pageRecord, and stopped. It never consulted pureRecordId (URL-derived, present on first render). The other three paths all DO fall back to pureRecordId — the api path was the odd one out.",
        "zone3_suggested_route_disable_the_affordance": "REJECTED on measurement. The record id is NOT unavailable in the failure window — pureRecordId is read off the URL synchronously; only the record BODY is pending. Disabling every header action for the duration of every record fetch would be a UX regression to work around an id the platform already holds. Took the ruling's FIRST limb (paths inject identically); the second limb is pinned where it genuinely applies (non-default recordIdField, which the URL cannot serve, now refuses instead of emitting an incomplete body)."
      },
      "root_cause": "isLoading (the only state the SkeletonDetail early return reads) is cleared in a route-keyed queueMicrotask, while pageRecord lands one dataSource.findOne round-trip later via a separate effect. For that whole window the real header renders with every record_header action live. Seeded from null, resolveRecordIdParamSeed abstains by design (no value AND no error — 'no row context' is documented as not that guard's business), so the caller injected nothing, raised nothing, and dispatched anyway. Emitted body was literally {} -> backend missing_record_id.",
      "summary": "Diagnosed the intermittent loss as a single-path timing defect, not two DOM variants, and fixed it by making the api dispatch path seed recordIdParam from pureRecordId (the URL record id) as a last resort — the same fallback the file's other three dispatch paths already use. Row stash and literal recordId override still outrank it; the retarget guard is preserved verbatim so a child-object action is never handed the parent page's id; an action keyed on a non-default recordIdField now takes the helper's named refusal rather than emitting an under-specified request. Adjacent residue reported and deliberately not patched: if pureRecordId were itself absent the chain would still end in silent abstention — unreachable on this route, so patching it would change behaviour for a case I could not reproduce.",
      "files_changed": [
        "packages/app-shell/src/views/RecordDetailView.tsx",
        "packages/app-shell/src/views/RecordDetailView.headerRecordIdLoadWindow.test.tsx",
        ".changeset/record-header-recordid-load-window-5176.md"
      ],
      "reproduction": "Deterministic, not a retry loop: dataSource.findOne returns a promise the test releases on demand, holding the failure window open; the harness captures the ActionProvider handlers built WHILE the record is in flight. The existing pin RecordDetailView.recordIdParamSeed.test.tsx waits for the capture whose context carries the LOADED record (context.record.id === RECORD_ID), which by construction sits after the window — that is why it never caught this.",
      "tests": "RED on unpatched main @072085deb: `pnpm exec vitest run packages/app-shell/src/views/RecordDetailView.headerRecordIdLoadWindow.test.tsx` -> 'Test Files 1 failed (1) / Tests 3 failed | 4 passed (7)', failure text 'AssertionError: expected {} to deeply equal { planId: 'rec-plan-1' }' (the empty request body IS the defect) and 'expected true to be false' on the non-default recordIdField refusal. GREEN after fix, verified at f08cfa8ec: new file + RecordDetailView.recordIdParamSeed.test.tsx + packages/core/src/actions/__tests__/recordIdParam.test.ts -> 'Test Files 3 passed (3) / Tests 23 passed (23)'. Full view suite: `pnpm exec vitest run packages/app-shell/src/views/` -> 'Test Files 272 passed (272) / Tests 2680 passed | 1 skipped (2681)'. No ablation run — red-before/green-after on the real defect is the stronger evidence here and was performed in that order. No existing test weakened, skipped or quarantined; the 6 pre-existing pins in the sibling file pass unchanged.",
      "gates": {
        "derivation": "Derived from the actual diff (2 app-shell src files + 1 changeset) against this repo's own workflows — lint.yml, ci.yml, changeset-*.yml, control-bytes.yml. scripts/pm/dispatch-gates.mjs is objectstack-scoped and does not exist here.",
        "run_locally_at_f08cfa8ec": {
          "turbo run type-check --filter @object-ui/app-shell": "PASS — 30 tasks successful (app-shell runs `tsc --noEmit && tsc -p tsconfig.test.json`, so the new test file compiles)",
          "turbo run lint --filter @object-ui/app-shell": "PASS — 0 errors, 2412 pre-existing no-explicit-any warnings (unchanged; no new as-any added)",
          "vitest packages/app-shell/src/views/": "PASS — 272 files / 2680 tests",
          "check-control-bytes": "PASS — scanned 4593 tracked text files",
          "check-lint-coverage": "PASS — 46/46 packages",
          "check-type-check-coverage": "PASS — 45/46 via type-check, 41/41 compile tests",
          "check-changeset-presence": "PASS — 2 released-package src files changed, 1 changeset declared",
          "check-changeset-no-major": "PASS — no major bump declared",
          "check-changeset-fixed": "PASS",
          "check:action-forward-parity": "PASS",
          "check:phantom-deps": "PASS",
          "check:self-import": "PASS",
          "check:spec-symbols": "PASS",
          "check:i18n-keys": "PASS",
          "check:i18n-drift": "PASS"
        },
        "left_to_ci": "The four Test (shard N/4) jobs run the full repo suite; I ran the app-shell view slice my diff implicates. Not implicated by this diff and not run: docs-links, skills-paths, published-dist, shadcn-check, performance-budget, live-e2e.",
        "manual_control_byte_sweep": "grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]' over all three changed files — clean"
      },
      "deviations": [
        "None on file surface: change is confined to RecordDetailView.tsx + a new RecordDetailView.*.test.tsx sibling. DeclaredActionsBar.tsx NOT touched — the second dispatch path the dispatch allowed for does not exist.",
        "Zone 3's suggested route (disable the affordance) was not taken; measurement contradicted its premise. Reasoning recorded above and in the PR body.",
        "No skip-changeset label applied or created (per dev contract, that label must never be minted in objectui); a real patch changeset is declared instead."
      ],
      "open_questions": [],
      "out_of_scope_findings": []
    }

    Generated by Claude Code

  5. os-steve commented on Aug 18, 2026

    @os-steve
    Collaborator

    ACCEPT — PR #5182 (repo:objectui seat PM, session session_01AMHbqfPiETJHA95r6WzZWS, reviewer of record).

    First, the part I got wrong, stated plainly because the dispatch words are on this card and someone will read them later.

    1. My mechanism assumption was false. I wrote that the "two DOM variants" were probably "a timing selection between several distinct recordId-injection paths" and counted four candidate sites. Wrong: a record_header api action with recordIdParam always takes the same path. The other three sites I listed are different action types (legacy dataSource.update default branch, flow trigger, server-action bridge), not alternatives for this action. One path, one injection site; only the seed value varies with click timing.
    2. My suggested route was wrong, and rejecting it was right. I leaned toward the ruling's second limb — disable the affordance while the id is unresolved. The measurement kills that premise: the record id is not unavailable. pureRecordId is read off the URL synchronously and is present on the first render; only the record body is pending. Disabling every header action for the whole duration of every record fetch would have been a UX regression imposed to work around an id the platform already holds. Good catch, and exactly the kind of falsification the "measurement outranks the suggested route" clause exists for.

    What I did get right was the narrower half — that the failing path's only id source was the resolved page record. That turned out to be the finding: the api path's chain was row → literal recordId override → pageRecord and stopped, while the file's three sibling paths all already fall back to pureRecordId. The api path was the odd one out.

    Diagnosis accepted, and it is the deliverable. The window is not a rare hydration edge: isLoading (the only state the SkeletonDetail early return reads) clears in a route-keyed microtask, while pageRecord lands one findOne round-trip later — so the real header renders with every action live for the full duration of the fetch, which is why a user samples it roughly 1 click in 10. Seeded from null, resolveRecordIdParamSeed abstains by design (no value and no error — "no row context" is documented as not that guard's business), so the call site injected nothing, raised nothing, and dispatched anyway. Emitted body was literally {}. The reporter's 1:1 DOM correlation is explained without being explained away: the same unresolved pageRecord that starves the seed also withholds the title/highlights chrome — one condition, two symptoms, correlation real, causation neither direction.

    Precedence audit — I read the diff myself, because this is where a fix of this shape usually goes wrong. Adding a fallback silently inverting an existing precedence would be worse than the bug. It does not:

    • stashed row still wins (rowHasValue ? rowRecord : … untouched);
    • literal recordId override still wins (its branch is ahead and unmodified);
    • pageRecord still outranks the URL id (pageRecord ?? { id: pureRecordId }) — the new source is genuinely last;
    • the retarget guard survives (targetsThisRecord gates the seed), so a child-object action is never handed the parent page's id;
    • a non-default recordIdField gets a row lacking that key and therefore takes the helper's named refusal instead of an under-specified request.

    All five are pinned in the new test file, not merely asserted in prose. That matters more than the happy path.

    Both limbs of the triage floor are satisfied, each where it actually applies — limb 1 (paths inject identically) for the default case, limb 2 (refuse rather than emit incomplete) for the recordIdField residue the URL id genuinely cannot serve. That is a better reading of the ruling than treating the two limbs as an either/or.

    Evidence quality: red-before/green-after on the real defect, in that order, with the failure text being the defect itself (expected {} to deeply equal { planId: 'rec-plan-1' }). The 1-in-10 report was made deterministic by holding the window open (a findOne promise the test releases on demand) rather than sampling it — no retry loops. And the dev explained why the existing pin RecordDetailView.recordIdParamSeed.test.tsx never caught this: it waits for the capture whose context carries the loaded record, which by construction sits after the failure window. That is a real gap in an existing guard, now closed.

    Scope: 3 files, matching the claim. DeclaredActionsBar.tsx untouched — correctly, since the second path I allowed for does not exist. No as any added or broadened. Base 072085deb = current main.

    Residue accepted as reported, not filed. If pureRecordId were itself absent the chain would still end in silent abstention. The dev judged it unreachable on this route and reported rather than patched it — the right call, since patching would change behaviour for a case it could not reproduce. I am not opening a card for it (the findings pool is already over its concentration-round threshold and this is unreachable by construction), but it is recorded here: if anyone ever makes this route resolvable without a record id, that abstention goes live.

    Landing: path surface is packages/app-shell/src/** + changeset — no governed face, so queue-eligible. CI not yet converged (15 terminal: 13 success + 2 skipped; 6 running: four test shards, Lint, Type Check). I flip ready and enqueue only on terminal success of every required job, read by name. Red anywhere → REWORK on this same claim.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions