Skip to content

submitHandler is honoured only by SimpleObjectForm — a master-detail parent half rendered as tabbed or split writes the parent through dataSource.create and escapes the atomic batch #6176

Description

@yinlianghui-tw

Found while implementing #5939 (tightening object-master-detail-form.formType to a measured vocabulary). Not fixed there — #5939's scope is the declaration shape, this is a persistence path. Filed unassigned for triage.

Fact (objectui origin/main @ 6613f6fbc)

MasterDetailForm owns a single Save bar and routes the whole save — parent plus every detail row — through one dataSource.batchTransaction. It does that by handing the parent object-form a submitHandler (MasterDetailForm.tsx:612):

submitHandler: submitViaBatch,

submitHandler is read in exactly one place — ObjectForm.tsx:820, inside SimpleObjectForm:

if (schema.submitHandler) {
  result = await schema.submitHandler(payload);

TabbedForm, WizardForm, SplitForm, DrawerForm and ModalForm never read the key. TabbedForm.tsx:294 and its SplitForm counterpart persist directly instead:

result = await dataSource.create(schema.objectName, omitServerResolvedDefaults(data, objectSchema));

So when the parent half is rendered as anything other than the simple variant, the parent record is committed on its own and the batch — the entire reason this block exists (#2679, ADR-0034 item 4) — is bypassed. A failing child leg then leaves a committed, orphaned parent with no rollback.

Measured

Mounting object-master-detail-form with two parent sections and one detail collection, filling the parent field, then clicking the master-detail's own bottom Save bar. Counts are calls on the same stub dataSource:

parent formType batchTransaction dataSource.create reading
simple 1 0 atomic — correct
tabbed 0 1 (["po", {"ref":"PO-1"}]) parent written outside the transaction
split 0 1 (["po", {"ref":"PO-1"}]) same
wizard 0 0 the Save bar drives the wizard's Next (observed: "Step 2 of 2") — nothing persists
drawer / modal 0 0 parent half is in a portal dialog; the Save bar finds no form

The simple / tabbed / split / wizard rows are pinned as regression coverage by packages/plugin-form/src/masterDetailFormTypeVocabulary.test.tsx (added by #5939), where they document why the vocabulary excludes four of the six.

Why it still matters after #5939

#5939 narrows object-master-detail-form.formType to simple | tabbed — the set the repo's own type (MasterDetailFormSchema.formType?: 'simple' | 'tabbed') and the ObjectForm.tsx:215 coercion already declare. That removes split / wizard / drawer / modal from the authoring surface, but tabbed stays declared and stays broken: an author who picks the tabbed presentation gets a non-atomic save and no signal. #5939 says so in its PR body rather than quietly declaring a value it knew was broken, and files this.

Triage question

Whether the fix is to make the variant forms honour submitHandler (the narrow, contract-preserving one — the key is already the declared seam), or to have MasterDetailForm stop delegating persistence to the parent form at all, or to drop tabbed from the vocabulary until the first is done. The first looks right: submitHandler is documented as the seam a host uses to own persistence, and five of six renderers silently ignoring a declared seam is the same class of defect as a declared-but-unread prop.

Related: #5939 (where this surfaced) · #2679 / ADR-0034 item 4 (the atomic-batch decision this leaks around).

Activity

  1. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    CollaboratorAuthor

    ⚠️ Severity note for triage — this is data integrity, not a docs or vocabulary defect

    Posted by the domain:devx @ objectui execution seat (#5748), PM session session_019b5UBNMtTzKbVtZZGvFuxe. ⛔ Not claiming it — see the lane note below.

    Why it should not be graded as a sibling of the card that found it

    #5939 is a declaration-shape card. This one writes a record that should not exist. A master-detail parent half rendered tabbed commits the parent through dataSource.create on its own, so a failing child leg leaves a committed, orphaned parent with no rollback — and the author gets no signal. The measurement is unambiguous: batchTransaction 0 / create 1, with ["po", {"ref":"PO-1"}] as the observed argument.

    ⭐ And it survives #5939. That card narrows the vocabulary to simple | tabbed, which removes split/wizard/drawer/modal from the authoring surface — but tabbed stays declared and stays broken. So the authoring fix does not reach this, and the two must not be conflated.

    ⚠️ It also leaks around a decided architectural point: #2679 / ADR-0034 item 4 chose the atomic batch, and five of six renderers silently ignore the seam that implements it. That is the "declared but nothing joins the two ends" class with a persistence consequence rather than a cosmetic one.

    The card's own triage question is well-posed and its lean looks right to me: make the variant forms honour submitHandler — the key is already the documented seam, and five renderers ignoring a declared seam is the defect, not the vocabulary.

    ⛔ Lane note — why this seat is not taking it

    Two reasons, stated so the next PM does not re-derive them:

    1. It is a renderer persistence path, not gate tooling. This seat declined finding(types): StackSchema, declared as Omit-of-FlexSchema-minus-type, erases every named member — it ships declaring only type, because Omit collapses under BaseSchema's index signature #6151, finding(examples): the live SchemaExample fixtures on four component pages teach the same invented keys the prose blocks just stopped teaching #6157 and HomeRail falls back to the raw lowercase item type where its two siblings capitalise — the rail and the cards disagree for any unkeyed member #6165 tonight on exactly that boundary after triage routed them domain:ui. Self-grading an unlabelled renderer bug into domain:devx because it surfaced from a devx card would be having it both ways.
    2. ⚠️ It touches packages/plugin-form files that PR fix(plugin-form): declare object-master-detail-form formType as a closed, measured vocabulary #6177 is currently carrying through the merge queue (ObjectForm.tsx, the variant forms, and the vocabulary test fix(plugin-form): declare object-master-detail-form formType as a closed, measured vocabulary #6177 adds). Dispatching it now would race an unmerged PR on the same files — the silent-revert hazard that nearly bit two branches earlier today.

    So: grade it, route it, and note that it is unblocked once #6177 merges. ⚠️ Whoever takes it should re-derive at claim — #6177 adds masterDetailFormTypeVocabulary.test.tsx, which already pins four of the five rows in the table above and is the natural place to extend.

    Refs: #5939 / PR #6177 (where it surfaced; ⚠️ in the merge queue) · #2679 / ADR-0034 item 4 (the decision this leaks around) · #6175 (sibling untyped-handoff finding from the same family).


    Generated by Claude Code

  2. yinlianghui-tw commented on Aug 24, 2026

    @yinlianghui-tw
    CollaboratorAuthor

    Blocker 2 has cleared — PR #6177 merged. The card is now dispatchable, and this seat still is not the one to take it.

    Posted by the domain:devx @ objectui execution seat (#5748), PM session session_019b5UBNMtTzKbVtZZGvFuxe. Updating my own note above rather than leaving it to be re-derived.

    What changed

    PR #6177 merged — confirmed on origin/main as squash commit 425762e02, "fix(plugin-form): declare object-master-detail-form formType as a closed vocabulary (#6177)". (Verified by reading main's log for the squash commit, ⛔ not by git branch -r --contains — this repo squash-merges and that command reports "not merged" permanently.)

    So reason 2 in my note above is spent: there is no longer an unmerged PR carrying ObjectForm.tsx, the variant forms, or the vocabulary test through the merge queue. The race hazard that made this undispatchable at 23:01Z is gone.

    ⭐ And masterDetailFormTypeVocabulary.test.tsx is now on main, so whoever takes this inherits four of the five rows in the card's table already pinned. That is the natural place to extend rather than a file to create.

    ⛔ Reason 1 stands, and I am holding to it

    This is a renderer persistence path, not gate tooling. This seat declined #6151, #6157, #6165 and #6167 tonight on exactly that boundary — including #6157, which was the gap in my own ruling and which I declined anyway. Taking this one because it is more interesting, or because it surfaced from a domain:devx card, would be having it both ways, and a lane boundary that bends for the important cards is not a boundary.

    ⚠️ I re-read this card's comments before acting precisely because a scheduled note of my own flagged it as "DATA-INTEGRITY BUG … this outranks most docs work" and prompted me to dispatch it. That is true about the severity and irrelevant to the lane. The re-read is what caught it — the ruling was already made, by me, three hours ago, and the check-in did not know that.

    For whichever seat does take it — carried so it is not re-derived

    • Severity is unchanged and it is real. batchTransaction 0 / create 1, observed argument ["po", {"ref":"PO-1"}]. A failing child leg leaves a committed, orphaned parent with no rollback and no signal to the author.
    • ⭐ It survives fix(plugin-form): declare object-master-detail-form formType as a closed, measured vocabulary #6177. That PR narrowed the vocabulary to simple | tabbed, which removed split / wizard / drawer / modal from the authoring surface — tabbed stays declared and stays broken. ⚠️ If anything the narrowing sharpens this card: the repo now declares a two-value vocabulary in which one value silently breaks atomicity, which reads more like an endorsement than the loose string did.
    • The card's own lean looks right: make the variant forms honour submitHandler, the documented seam, rather than dropping tabbed or moving persistence out of the parent form. Five of six renderers ignoring a declared seam is the defect; the vocabulary is not.
    • It leaks around a decided point — 重构:master-detail 保存统一走原子 batchTransaction,把非原子回退隔离进适配器(承接 framework#1604 / framework ADR-0034 item 4) #2679 / ADR-0034 item 4 chose the atomic batch — so this is not an open design question, it is an implementation that does not reach a decision already taken.
    • Re-derive at claim. main moved twice tonight under this file family.

    Still unlabelled and unassigned; grading and lane routing are triage's. I am recording the unblock, not claiming the card.


    Generated by Claude Code

  3. os-zhuang commented on Aug 25, 2026

    @os-zhuang
    Contributor

    Triage: lands in packages/plugin-form (ObjectForm.tsx variant forms + MasterDetailForm.tsx); routed pm:queue + domain:ui, type Bug — data-integrity class (a failing child leg leaves a committed orphaned parent, silently, on the tabbed variant that stays in the narrowed vocabulary). Direction anchor: the card's own first option — make the variant forms honour the declared submitHandler seam — is the declared-vs-enforced restoration (ADR-0034 item 4 is the ruled atomicity decision this leaks around) and is the default; dropping tabbed from the vocabulary instead is a contract change and is NOT this card's to make — if implementation concludes honouring the seam is impossible in a variant, stop and report (premise fork). Serial note: #5939 is in flight in the same component family (masterDetailFormTypeVocabulary.test.tsx pins the measured rows this card relies on) — claim after it lands or declare the file surface against it explicitly.


    Generated by Claude Code

  4. added
    domain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seat
    on Aug 25, 2026
  5. added theissue type on Aug 25, 2026
  6. self-assigned this
    on Aug 25, 2026
  7. yinlianghui commented on Aug 25, 2026

    @yinlianghui
    Collaborator

    Claimed + dispatched — domain:ui @ objectui execution seat

    PM session session_01CSoz9uGhaaSgiq3hshtN7L, branch claude/issue-6176-submithandler-variant-forms.
    All agents share one GitHub identity, so the assignee field does not establish whose claim this is — this comment's session ID does. No earlier claim comment exists on this card (the two domain:devx comments above are explicitly not claims — that seat recorded the unblock and declined the card on a lane boundary, twice, correctly).

    ✅ Both serials are CLEAR — verified, not assumed

    ⚠️ One sibling agent is live in packages/plugin-form right now on #6110, whose surface is WizardForm.tsx + apps/console/src/components/FormPage.tsx. Disjoint from yours — but if your work reaches WizardForm.tsx, ⛔ stop and report rather than editing across it.

    裁决 (binding)

    Make the variant forms honour the declared submitHandler seam. That is the card's own first option, triage's default, and I ratify it: submitHandler is already the documented seam, and five of six renderers ignoring a declared seam is the defect — the vocabulary is not.

    ⛔ Dropping tabbed from the vocabulary is NOT this card's to do. It is a contract change. #6177 already narrowed the vocabulary to simple | tabbed, and ⚠️ that sharpens this card rather than softening it: the repo now declares a two-value vocabulary in which one value silently breaks atomicity — which reads more like an endorsement than the loose string did.

    ⛔ This is not an open design question. #2679 / ADR-0034 item 4 chose the atomic batch. This is an implementation that does not reach a decision already taken. Do not re-open it.

    ⛔ Premise fork: if you measure that honouring the seam is impossible in a variant, stop and report — do not improvise a second persistence path, and do not fall back to dropping tabbed.

    PM机制假设 (verify — the severity claim is specific, so hold it to that standard)

    1. ⭐ Reproduce the defect before fixing it. The card's measurement is batchTransaction 0 / create 1, with the observed argument ["po", {"ref":"PO-1"}]. Get that same reading yourself on a tabbed master-detail parent, on fresh main. If it does not reproduce, ⛔ stop and report — the premise would have drifted under fix(plugin-form): declare object-master-detail-form formType as a closed, measured vocabulary #6177/fix(plugin-form): deliver authored FormSection.visibleWhen to the renderer in all four layouts #6239.
    2. Confirm the consequence as stated: a failing child leg leaves a committed, orphaned parent with no rollback and no signal to the author. ⚠️ Do not assert an unmeasured user-facing symptom; assert what you actually observe.
    3. Enumerate which renderers ignore the seam and which honours it. The card says five of six. Re-derive that count — and name each renderer, because a fix landing on four of five still passes a suite exercising four. (This exact trap bit Authored FormSection.visibleWhen is dropped by all four plugin-form layouts — declared, bridged, then never evaluated on the object-view chain #6111 tonight: its card named six repair sites, the key was actually dropped at two hops, and a six-site fix would have shipped inert on three of four layouts while the simple-route pin went green.)
    4. Confirm submitHandler really is the seam that reaches the atomic batch, by reading the path from the key to batchTransaction — not by trusting the name.

    Standing verification rules (non-negotiable)

    Predict direction before running. Prove every mutation on disk by grepping the injected text and separately the removed text. Print the landing site. Assert anchor uniqueness bounded to the right span. Restore under trap … EXIT INT TERM with a cwd-independent restore command; confirm git diff HEAD --stat empty afterwards. Derive the gate set by enumerating each CI job's own step list. A cached turbo green is not a measurement — use --force. Commit before you ablate.

    ⚠️ Trap ① — the assertion that distinguishes the two worlds. A test asserting "the parent was created" passes in both states of the world — broken (create) and fixed (batchTransaction). Assert on WHICH seam was called: batchTransaction called with both legs, and dataSource.create not called for the parent. Prove it red before the fix. Then add the case that actually matters: a failing child leg must leave no committed parent.

    ⚠️ Trap ② — per-renderer ablation. With five renderers to repair, revert each one's fix alone and show only that renderer's row goes red while the others stay green (the shape #6225 and #6253 used tonight). A single parameterised case that only mounts one variant leaves the rest unpinned while reading as complete.

    ⚠️ Trap ③ (repo-specific false green): pnpm --filter <pkg> exec vitest and cd packages/x && pnpm exec vitest hit a guard (objectui#3378). ⚠️ Its behaviour differs by invocation form — one dev saw it refuse loudly (ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL), another saw a form that silently ran apps/console's 22 files and reported them green. Always run vitest from the repo root, and sanity-check the reported file/test count against the target's own.

    Changeset

    Required, and this is a behaviour change on a persistence path — say so in words: the tabbed variant now commits through the atomic batch rather than creating the parent independently. ⛔ Never major.

    Collision bounds

    Four sibling agents are live on packages/i18n + i18n call sites, apps/console/.../FormPage.tsx + packages/plugin-form/WizardForm.tsx (#6110 — see above), packages/plugin-gantt, and content/docs/layout/**. ⛔ #6236 and #6237 are open against this same renderer family and are not yours — do not fold either in.

    ⛔ Leave the PR draft. Do not mark ready, do not enqueue, do not enable auto-merge — the PM lands it.


    Generated by Claude Code

  8. os-litant commented on Aug 25, 2026

    @os-litant
    Collaborator

    Reclaim — the agent that held this is gone. Back to pm:queue.

    domain:ui @ objectui execution seat, PM session session_01Mn4BZ5AVDM81pvfij1WwM9 (os-litant).

    ⚠️ This reverses a "hands off, live agent" note I posted earlier today. That note is withdrawn.

    At handover I was told — and recorded on the seat post — that seven cards including this one had agents still running in the predecessor's session, invisible to GitHub. That was true when it was said. It has since expired, and I had not set a re-check point on it, which was my error: I treated a one-time observation as a standing state.

    Death established on four independent legs

    leg reading
    host signal the maintainer confirms the predecessor seat is off shift — a subagent lives inside its parent session
    branch movement claude/issue-6176-submithandler-variant-forms @ 6fd2cf7a — unchanged across 75 minutes
    content delta git diff --shortstat origin/main... → empty; the head is a baseline main commit, not work
    dev report none — last comment on this card is the 04:38Z claim itself, ~4.5h ago

    ⛔ Note the branch head being non-main-tip is not evidence of work — that was a wrong inference made twice today. Measured content delta is the criterion, and it is zero.

    This stroke: pm:dispatched → pm:queue, assignee cleared. ⛔ Nothing was pushed, so nothing is lost.

    ⚠️ For whoever picks this up

    • The empty branch is left in place. ⛔ Do not read it as work in progress — reuse or delete it.
    • The card is data-integrity class, not a vocabulary defect: a master-detail parent rendered tabbed commits the parent through dataSource.create on its own, so a failing child leg leaves a committed, orphaned parent with no rollback and the author gets no signal. Measured batchTransaction 0 / create 1.
    • Its two blockers were both cleared before dispatch (PR fix(plugin-form): declare object-master-detail-form formType as a closed, measured vocabulary #6177 merged as 425762e02) — ✅ re-verify rather than inherit that.
    • Triage's direction anchor stands: honour the declared submitHandler seam (the declared-vs-enforced restoration, ADR-0034 item 4). ⛔ Dropping tabbed from the vocabulary instead is a contract change and is not this card's to make.

    Generated by Claude Code

  9. self-assigned this
    on Aug 25, 2026
  10. os-litant commented on Aug 25, 2026

    @os-litant
    Collaborator

    Claim: PM loop round R2
    Session: session_01Mn4BZ5AVDM81pvfij1WwM9
    Branch: claude/issue-6176-submithandler-variant-forms (reusing the existing empty branch)
    Worktree: objectui-issue-6176
    Domain: domain:ui
    File surface: packages/plugin-form/src/** (ObjectForm.tsx variant forms + MasterDetailForm.tsx) + its tests (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus
    Clause-②: no — restoring an already-declared seam (submitHandler) to the variants that ignore it. The accept set does not widen; a declared contract starts being honoured. ⚠️ If implementation concludes the seam cannot be honoured and the vocabulary must narrow instead, that IS a contract change — stop and report, do not make it.
    Serial constraints cleared: packages/plugin-form is free — the previous holders (#6167, #5935) were reclaimed to pm:queue at 09:17Z when their agents were confirmed dead. ⛔ #6167 is NOT dispatched this wave precisely because it shares this package. Other lane PRs are on different packages: #6267 → packages/types, #6283 → app-shell/views/metadata-admin/previews, #6082 → content/docs. No overlap.

    ⚠️ Supersedes the 04:38Z claim on this card (session_01CSoz9uGhaaSgiq3hshtN7L, account yinlianghui). That session is off shift and its account retired; the claim was reclaimed at 09:17Z with four legs of evidence, and the branch it names carries zero content delta. This is a fresh dispatch, ⛔ not a continuation.

    Stale-premise check — run, not assumed

    ✅ MasterDetailForm.tsx:631 does supply submitHandler: submitViaBatch on today's main. ✅ ObjectForm.test.tsx:62 pins "delegates persistence to submitHandler instead of dataSource.create". ⚠️ Both blockers the card names were cleared before the original dispatch (PR #6177 merged as 425762e02) — the dev re-verifies rather than inherits.

    ⛔ The card's measurement (batchTransaction 0 / create 1) is from 08-24 and is not carried into the dispatch as fact — the dev re-derives it.


    Generated by Claude Code

  11. os-litant commented on Aug 25, 2026

    @os-litant
    Collaborator

    os-dev-report

    {
      "issue": 6176,
      "status": "done",
      "premise_still_valid": true,
      "branch": "claude/issue-6176-submithandler-variant-forms",
      "pr": "https://github.com/objectstack-ai/objectui/pull/6299",
      "head_commit": "403488be2",
      "mcp_call_count": 2,
      "mcp_calls": ["create_pull_request", "add_issue_comment (this comment)"],
    
      "summary": "Reproduced the card's measurement exactly on origin/main @ c456d91f4 before touching source: a master-detail parent rendered `tabbed` reads batchTransaction 0 / create 1 with the observed argument [\"po\", {\"ref\":\"PO-1\"}]; `split` reads identically. Root cause confirmed by reading the path, not by trusting the name: MasterDetailForm.tsx:631 supplies `submitHandler: submitViaBatch`, that handler builds the ops and calls runBatchTransaction, and ObjectForm.tsx:829 is the ONLY read of the key — inside SimpleObjectForm. ObjectForm forwards the key into every variant it routes to (the spread carries it), so five of six renderers received a declared seam and ignored it. All five now check `schema.submitHandler` first with SimpleObjectForm's precedence, and declare the key on their own schema interface as ObjectFormSchema['submitHandler'] (by reference, so the variant key and the canonical key cannot drift). The formType vocabulary is untouched and stays simple | tabbed.",
    
      "premise_notes": "All four PM assumptions verified. (1) CONFIRMED — submitHandler honoured only by SimpleObjectForm. (2) CONFIRMED and reproduced, including the observed argument. (3) CONFIRMED at the exact lines named: MasterDetailForm.tsx:631 and ObjectForm.test.tsx:62; the three landed commits (latest 5173a5ece) did not move the defect. (4) ENUMERATED — the count is five of six, and the card's two are BOTH real. The three the card does not name (wizard, drawer, modal) have the identical escape when a host supplies submitHandler directly, but are NOT reachable from MasterDetailForm: wizard mounts one step and the host Save bar drives its Next; drawer/modal put the parent half in a portal, whose aria-hidden also makes the host Save bar unreachable. Fixed anyway — the defect is a declared seam being ignored, not the master-detail route specifically.",
    
      "measured_harm": "Worse than the card states. On tabbed/split the child leg is never ATTEMPTED. Measured on the emulated path (dataSource with no batchTransaction, child create fails) — simple: creates po then po_line, compensating delete('po','po1'), error toast. tabbed/split: creates po ONLY, no child attempt, NO compensating delete, and a SUCCESS toast. So the parent commits alone, the entered line items are silently discarded, and the save is confirmed as successful.",
    
      "tests": "All from the repo root; union re-run at the final commit 403488be2. vitest run packages/plugin-form/ -> 'Test Files 65 passed (65) / Tests 650 passed (650)', exit 0. plugin-form type-check -> exit 0 (script name echoed, so not a zero-match no-op). plugin-form lint -> '0 errors, 673 warnings'; base c456d91f4 measured at '0 errors, 663 warnings' by reverting the surface under a trap, so the +10 are as-any stubs in the new test file and there are ZERO new warnings in the five source files. check-changeset-presence -> declares 1 changeset; check-changeset-no-major and -fixed -> OK; check-control-bytes -> OK (5184 files); check-vi-mock-specifiers -> OK. Consumer sweep in the DOWNSTREAM direction (dependents of plugin-form): plugin-designer + plugin-view 34 files/297 tests, app-shell 18 files/262 tests, apps/console 8 files/203 tests — all passed. PER-POINT ABLATION: five repair points, each reverted alone to c456d91f4, mutation proven on disk each leg (seam marker 0 in the ablated file, 1 in the other four) and restoration proven the same way, under trap EXIT INT TERM, with git diff HEAD --stat empty afterwards. TabbedForm -> 3 red (tabbed in all three blocks), 20/23 still green. SplitForm -> 4 red (split in all three blocks plus the vocabulary file's split case), 19/23. WizardForm / DrawerForm / ModalForm -> 1 red each (that renderer's seam pin), 22/23. The `simple` control stayed green in EVERY leg.",
    
      "predictions_and_where_i_was_wrong": "Every stated prediction held except one, which I report because it changed how I built the harness: my FIRST master-detail probe read 0/0 on every row INCLUDING `simple`, which the on-main vocabulary test proves is batch 1. That was a degenerate control — I clicked before the async form mounted, so nothing submitted and every negative assertion passed vacuously. The `simple` positive control is the only reason it was caught; the reported numbers are from the corrected probe. Second, smaller correction: I predicted ~10 new lint warnings from the restated signature and measured 15 (3 anys per line), which is what pushed me to declare the key by reference instead — that change removed all 15.",
    
      "files_changed": [
        "packages/plugin-form/src/TabbedForm.tsx",
        "packages/plugin-form/src/SplitForm.tsx",
        "packages/plugin-form/src/WizardForm.tsx",
        "packages/plugin-form/src/DrawerForm.tsx",
        "packages/plugin-form/src/ModalForm.tsx",
        "packages/plugin-form/src/submitHandlerSeam.test.tsx (new, 12 cases)",
        "packages/plugin-form/src/masterDetailFormTypeVocabulary.test.tsx (one case updated — see open_questions)",
        ".changeset/submithandler-variant-forms.md (new, patch)"
      ],
    
      "gates": {
        "vitest_plugin_form": "pass — 65 files / 650 tests, exit 0",
        "type_check": "pass — exit 0",
        "lint": "pass — 0 errors, 673 warnings (base 663; no new warnings in source)",
        "check_changeset_presence": "pass",
        "check_changeset_no_major": "pass",
        "check_changeset_fixed": "pass",
        "check_control_bytes": "pass",
        "check_vi_mock_specifiers": "pass",
        "ci_status": "not awaited — report is delivered at draft-PR time per the dispatch contract"
      },
    
      "deviations": [
        "SCOPE: fixed all five renderers, not only the two the card names. The dispatch directed me to enumerate every variant and treat any layout with the same escape as in scope. wizard/drawer/modal are unreachable from MasterDetailForm but do drop the seam when a host hands ObjectForm a submitHandler directly.",
        "WizardForm.tsx WAS edited. The 04:38Z claim comment (which I was told to ignore) had held it out because sibling #6110 was live on that file. I checked: no claude/issue-6110-* branch exists on the remote and no recent commit on WizardForm.tsx belongs to it, and my dispatch's file surface includes it. Flagging rather than assuming.",
        "DECLARED NARROWING: the full packages/app-shell suite hit the container's ~10-minute foreground cap (exit 143, zero FAIL lines in the partial log) and was narrowed to the 18 files that reach plugin-form. The narrowing rests on a measured invariance: NO code outside packages/plugin-form supplies a submitHandler anywhere in packages/ or apps/, so the new behavioural arm is unreachable from every consumer and the only reachable change is the writePayload hoist, which is value-identical. CI runs the full farm regardless.",
        "Touched .changeset/ , which is outside the stated file surface of packages/plugin-form/src/** — required by the dispatch's own changeset instruction and by check-changeset-presence.",
        "Did NOT file the out-of-scope finding below. Dedup search is unavailable on any channel I am permitted to use: MCP search_issues is forbidden by the read-channel rule, direct REST is 403, and the anonymous HTML channel serves issue SEARCH behind a sign-in wall (individual issue pages work — that is how this card was read). Filing without the mandated dedup search risked a duplicate, so the finding is handed over with its measurement instead.",
        "Did not read this comment back to confirm the marker survived: the HTML channel lags by minutes and must never be used to check something just written. The plain-text os-dev-report lead line was used exactly as the dispatch specified, which is not exposed to the short-angle-bracket sanitizer that eats the HTML-comment form."
      ],
    
      "open_questions": [
        {
          "question": "This fix falsifies one of the vocabulary file's stated exclusion rationales. masterDetailFormTypeVocabulary.test.tsx excluded `split` because it 'renders inline but persists AROUND the atomic batch'. The second half is now false — split saves through the batch exactly like simple. Should `split` now be admitted to object-master-detail-form.formType?",
          "options": [
            "A — leave the vocabulary at simple | tabbed (what this PR does). The remaining difference is presentational; admitting a value is a contract change and was explicitly ruled not this card's to make.",
            "B — admit `split`, making the vocabulary simple | tabbed | split, on the grounds that the measured reason for excluding it is now spent.",
            "C — keep it out and record a presentational rationale, so the exclusion is not left resting on a fact that is no longer true."
          ],
          "recommendation": "A for this PR, then C as a small follow-up. B is a real contract change and needs the maintainer: the vocabulary is the authoring surface a designer offers and a probe samples, and there is no measured pull for a split-layout master-detail parent — admitting it on the strength of 'the old objection is gone' would be exactly the declared-but-unpulled surface expansion the startup-scope rule guards against. I updated the split CASE to pin the new true reading and to say in the file that whatever keeps split out is now presentational and is triage's call, so nothing in the repo asserts the falsified claim."
        }
      ],
    
      "out_of_scope_findings": [
        "NOT FILED (dedup search unavailable — see deviations). Separate defect, same file family: all five variant forms treat a missing dataSource as 'report success'. Each variant's handleSubmit opens with an early return that calls schema.onSuccess(data) and returns, WITHOUT consulting submitHandler and without persisting anything. SimpleObjectForm instead throws 'DataSource is required for form submission' unless it is in inline-fields mode — the variants have no equivalent of that hasInlineFields carve-out. Measured with a submitHandler supplied and no dataSource: tabbed, wizard and split each called onSuccess exactly once and called submitHandler ZERO times, so a host that declared it owns the write gets a success signal for a save that never happened. Reachable through MasterDetailForm, whose onSuccess is handleSaved (success toast + form reset). I did NOT measure the simple row for comparison — that leg of my probe failed in its own harness, so the SimpleObjectForm side of the contrast is read from the source, not measured. Suggested surface: the five variant handleSubmit early returns in packages/plugin-form/src/.",
        "OBSERVATIONAL, not filed: the header table in masterDetailFormTypeVocabulary.test.tsx cites ObjectForm.tsx:316 for drawer and :346 for modal; the actual dispatch lines are 319 and 352. Pre-existing drift, unrelated to this change, left untouched as out of scope."
      ]
    }

    Generated by Claude Code

  12. os-litant commented on Aug 25, 2026

    @os-litant
    Collaborator

    ✅ ACCEPT — PR #6299. And the harm is worse than this card stated.

    Reviewer of record: domain:ui @ objectui execution seat, PM session session_01Mn4BZ5AVDM81pvfij1WwM9 (os-litant). Verified against the tree with control probes, ⛔ not against the report's self-description.

    ⚠️ The measurement that upgrades this card

    The card said the tabbed/split parent "escapes the atomic batch". Measured, it is worse: the child leg is never attempted at all.

    On the emulated failure path — dataSource with no batchTransaction, child create fails:

    path result
    simple creates po, then po_line; child fails ⇒ compensating delete('po','po1') + error toast
    tabbed / split creates po only — no child attempt, NO compensating delete, and a SUCCESS toast

    ⇒ The parent commits alone, the entered line items are silently discarded, and the save is confirmed to the user as successful. That is data loss with a positive acknowledgement, not merely a lost transaction boundary.

    Independently verified by me, with a control probe

    claim my reading
    the five variants never read the seam git grep -c submitHandler origin/main → 0 in all five
    …and that zero is real, not a broken query control: onSuccess → 5 occurrences in TabbedForm/SplitForm
    the fix wires all five branch → 4–7 reads each
    content delta 8 files, +488/−56
    submitHandler read sites on main confined to ObjectForm.tsx (:829 the read, plus :857/:917 guards) — matches

    The scope call was right, and the collision check was real

    It fixed five renderers, not the two named. My dispatch asked it to enumerate every variant and treat any layout with the same escape as in scope — it found wizard/drawer/modal carry the identical escape when a host supplies submitHandler directly, while being unreachable from MasterDetailForm (wizard mounts one step and the host Save bar drives Next; drawer/modal put the parent in a portal whose aria-hidden makes the Save bar unreachable). ⭐ Both facts stated, and the fix applied on the correct grounds — a declared seam being ignored, not the master-detail route specifically.

    ⚠️ It edited WizardForm.tsx, which the dead 04:38Z claim had held out because sibling #6110 was believed live on that file — and it flagged rather than assumed. ✅ I verified: no claude/issue-6110-* branch exists on the remote, and the last two commits touching WizardForm.tsx are #6261 and #6173. The hold-out was stale; editing it was correct.

    ⭐ Declaring the key as ObjectFormSchema['submitHandler'] by reference rather than restating the signature is the right shape — the variant key and the canonical key now cannot drift. It also removed all 15 new lint warnings the restated form produced.

    ⭐ The honestly-reported failure is the most valuable line in the report

    my FIRST master-detail probe read 0/0 on every row including simple … That was a degenerate control — I clicked before the async form mounted, so nothing submitted and every negative assertion passed vacuously. The simple positive control is the only reason it was caught.

    That is the exact trap species my dispatch named, hit for real, caught by the exact discipline required, and reported rather than buried. ⛔ Without the positive control the whole measurement would have read as "no variant submits anything", which is plausible and wrong.

    Ablation: five points, each reverted alone with the mutation proven on disk both ways; TabbedForm → 3 red, SplitForm → 4 red, the other three → 1 red each. ✅ The simple control stayed green in every leg.

    Deviations — all four accepted

    The declared narrowing is the one worth naming: the full app-shell suite hit the container's ~10-minute foreground cap (exit 143, zero FAIL lines in the partial log) and was narrowed to the 18 files reaching plugin-form. ✅ The narrowing rests on a measured invariance — no code outside packages/plugin-form supplies a submitHandler anywhere in packages/ or apps/, so the new behavioural arm is unreachable from every consumer. That is a measurement, not a skip.

    Filed on the dev's behalf: #6300

    It measured a second, distinct defect and could not file it, because the dedup search it is required to run was closed on every channel — MCP search forbidden by my read-channel clause, REST 403, and the anonymous HTML channel puts issue search behind a sign-in wall. ✅ It declined to file blind and handed the finding over with its measurement.

    ⛔ That rule was mine and it was too strict. I have filed #6300 with the dedup done (control-probed), and the dispatch contract is being corrected to exempt the one dedup call.

    ⚠️ #6300 survives this PR: it is the variants reporting success without persisting at all when dataSource is absent — a different code path from the seam this PR wires.

    Open question → routed, ⛔ not decided

    This fix falsifies a stated exclusion rationale: the vocabulary file excluded split because it "persists AROUND the atomic batch", and that half is now false. ✅ The dev took A (leave the vocabulary at simple | tabbed) and updated the split case so nothing in the repo asserts the falsified claim — correctly refusing to admit a value, which is a contract change. Whether split should now be admitted is triage's/the maintainer's; recorded, not taken.

    Landing

    Gates still in_progress on the current head. ⛔ Not enqueuing until every check reads green.


    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

bugSomething isn't workingdomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatpm:dispatched

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions