fix(app-shell): seed the flow-node boolean control from the declared defaultValue (objectui#6830 arm A) - #8473
Merged
os-justin merged 1 commit intoSep 8, 2026
Conversation
…defaultValue
objectui#6830 arm A ("show, do not write", triage 2026-09-04), the boolean
half — objectui#8451. The select half stays untouched: it is blocked on
objectui#8450.
The control drew `value === true`, which flattened an ABSENT key and a stored
`false` onto one unchecked box although the runtime treats them oppositely on a
key the spec defaults to true. It now shows the value in effect — the stored
boolean, else the declared `defaultValue` in the same 'true'/'false' spelling
`controllerAdmits` already resolves an unset controller against. Nothing is
written; the first author edit still commits an explicit boolean.
`isUnsetFieldValue` is exported from `flow-node-config.ts` so the three sites
that have to answer "has the author stored anything here" (the re-show rule,
the `showWhen` resolver, and now the control) share one definition instead of
three copies of `=== undefined || === null || === ''`.
⛔ No caption naming the default is rendered, deliberately. Of the two boolean
fields the offline table declares a default for, `escalation.enabled` declares
the OPPOSITE of what installed `@objectstack/spec` 17.3.0 applies to the
omitted key (objectui#6620), so a "(default)" label would have shipped that
wrong claim to authors in words. The seed alone leaves that field rendering
byte-identically to before, which is pinned.
PR #8431's pin is turned green against the repaired behaviour rather than
weakened: its three leg-B rows now assert the new states, and the stored-`false`
rows are kept load bearing so an always-checked control — an implementation
strictly worse than the bug — still fails.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-justin
marked this pull request as ready for review
September 8, 2026 01:27
Collaborator
Author
|
PM contract review — accepted, flipped to ready, auto-merge armed. You refused both options I offered and the refusal is right. The wrong declared default: neither of my options survived measurementI offered "gate on objectui#6620" or "derive that row from the spec". You measured that exactly TWO boolean fields in the offline table declare a default — ⇒ a caption asserting "this is the declared default" would have shipped a false statement on HALF the surface it annotates. Your answer — ship the seed and no words — is better than either of mine, and the reason it works is that the seed renders ⭐ The omitted i18n key pair is the answer, not a shortfall. My card budgeted "one i18n key pair" as part of the scope. That key pair would have paid for exactly the caption that puts the wrong declaration into words. Not spending it is the fix. Both of my named options, refuted rather than declined
Beyond scope in the right directionExporting And the load-bearing marker on the stored-
|
os-justin
enabled auto-merge
September 8, 2026 01:27
os-justin
deleted the
claude/issue-8451-flownode-boolean-declared-default
branch
September 8, 2026 01:55
os-justin
pushed a commit
that referenced
this pull request
Sep 8, 2026
Conflict in FlowNodeInspector.declaredDefault.test.tsx's docblock only, and only in its point 1: PR #8473 (objectui#8451) rewrote the BOOLEAN half of that paragraph while this branch rewrote its SELECT half. Resolved by taking the incoming boolean text verbatim and re-applying the select correction on top of it - the select now draws InspectorSelectField's own em-dash placeholder rather than a blank trigger, which is the same fact about `defaultValue` as before. The file's select assertions merged cleanly and are unchanged by this commit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8451 — objectui#6830 arm A ("show, do not write", triage 2026-09-04), the
boolean half. The select half is not addressed here and stays open on objectui#8450.
Base
1706d8be4· installed@objectstack/spec17.3.0.What changed
FlowNodeConfigField'scase 'boolean'drewvalue === true. That expression flattenedan absent key and a stored
falseonto the same unchecked box although the runtimetreats them oppositely on a key the spec defaults to
true— on an approval node omittingescalation.notifySubmitter, the inspector said "Notify submitter: off" for a flow thatnotifies. The control now shows the value in effect:
'true'/'false'spelling — the onecontrollerAdmitsalready compares an unsetshowWhencontroller against;=== true, unchanged. Deliberately not widened to also accept a storedstring
'true': that would be a lenient renderer-side alias for off-spec metadata(AGENTS.md #0.1), and the producer is ours to fix.
isUnsetFieldValueis exported fromflow-node-config.tsso the three sites that must answer"has the author stored anything here" — the re-show rule, the
showWhenresolver, and nowthe control — share one definition instead of three copies of
=== undefined || === null || === ''. Without that, one node could render a gate that says "unset, so use the default"beside a control that says "stored, so ignore it".
Nothing is written. Rendering a seeded control patches the draft exactly zero times
(pinned); a node that never carried the key still ships without it.
The affordance's shape, and why it carries no caption
An absent key reads as distinct from a stored
falsethrough the checkbox state itself:declared-
true+ absent → checked; storedfalse→ unchecked. Those wereouterHTML-identical before and are not now.The card budgeted "one i18n key pair" for a caption naming the default. No caption ships,
and that is the answer to the wrong declared default — see below. Absent-vs-stored-
truedeliberately still render alike: both mean
trueat runtime, so drawing them differentlywould be inventing a distinction the metadata does not have. The distinction that does
exist — and that this card is about — is absent vs stored-
false.The one wrong declared default: I ran objectui#6620's own restart probe, and it fires
Exactly two boolean fields in the offline table declare a default. One of them is the
wrong one, so a caption asserting "this is the declared default" would have shipped a false
statement on half the surface it annotates:
approval.escalation.notifySubmitter'true'true✅approval.escalation.enabled'false'true❌ objectui#6620Neither of the two options the card offered survived measurement:
Deriving that row from the spec cannot be display-only without making the panel
contradict itself (a ticked "SLA escalation" above four hidden dependants). Feeding both
means flipping the table's
defaultValueto'true', which changes visibility forevery approval node omitting the key, reds a pin row that leg B did not predict
(
the offline table hides the same siblings, because it declares the gate false), andflips objectui#6620's tripwire in
flow-node-config.inactiveRetained.test.ts. That isobjectui#6620's fix, done from an unrelated PR.
Gating on objectui#6620 reads as "wait", and the measurement says it is not a wait.
Its triage comment (5442325479) sets a mechanical restart criterion:
It prints
true. On installed 17.3.0,ApprovalEscalationSchema.safeParse({ timeoutHours: 24 }).datais{"enabled":true,"timeoutHours":24,"action":"notify","notifySubmitter":true}. Therestart condition is met; objectui#6620 is live, not latent, and still carries
pm:on-hold. Reported for triage, not acted on here.So this PR does the third thing: it ships the seed and no words. The seed renders
escalation.enabledbyte-identically to before (unchecked — the declaration says'false'),so no new claim about that declaration reaches an author, and a new case pins exactly that:
the gate's control carries its label and nothing else. When objectui#6620 lands, its
one-line flip now repairs the rendered toggle as well as the gating, because the control
reads the same declaration.
ApprovalEscalationSchemafrom
@objectstack/spec, where it isundefinedon 17.3.0 — the probe throwsTypeError: Cannot read properties of undefined (reading 'parse'), which is easy to misreadas "not available yet, the hold is correct". The working spelling is
@objectstack/spec/automation, which is what the repo's ownflow-node-config.spec-reconciliation.test.tsalready uses.The pin: turned green, not weakened
FlowNodeInspector.declaredDefault.test.tsx— 13 cases → 17, all still assertions aboutrendered values. Applying the implementation to the unmodified pin reproduced PR #8431's
ablation leg B exactly, by name (3 failed / 10 passed):
boolean: an unset key draws an UNCHECKED box, though the table declares truerenders identically whether the key is missing or explicitly falsethe boolean control exists, is a checkbox, and commits the author editEach was rewritten against the repaired behaviour rather than deleted or softened, and every
select case is carried over untouched. Net new cases: show-do-not-write (zero patches),
a stored
falsebeats the declaredtrue, a field declaring no default stays unchecked(measured on the online writer — the offline table has no undeclared boolean), and the
objectui#6620 no-worded-claim case. Two existing cases were strengthened: the online
default: trueis now asserted to reach the control, and the commit row now exercises bothdirections (seeded-
true→ writesfalse; stored-false→ writestrue).Would an implementation strictly worse than the bug pass? No — measured
Two ablations, both mutating the read site from the committed tree, both proved to reach
disk by
git hash-objectagainst the HEAD blob plus removed-text/injected-text grep counts inboth directions, both restored by state (
git diff HEADempty and hashes equal).HEAD blob of
FlowNodeConfigField.tsx:085103868528ff4bd971a7d6bd00776797163f4b.const checked = value === true)d45bb482d38da528ca61a87241fd27fbef7e7af8const checked = true)2937d11f607172fffcaf7af26e73c91f848f7d02Leg A red rows:
boolean: an unset key draws a CHECKED box, because the table declares true·boolean: showing the default WRITES nothing — the node still omits the key·a gate whose default is true reveals its siblings for a node that omits the key·renders differently when the key is missing than when it is explicitly false·the boolean control exists, is a checkbox, and commits the author edit.Leg B red rows:
boolean: a stored FALSE beats the declared true and draws an UNCHECKED box·boolean: a field declaring NO default still draws unchecked when unset·boolean: the #6620-wrong declaration ships NO worded claim (objectui#6620)·renders differently when the key is missing than when it is explicitly false·the boolean control exists, is a checkbox, and commits the author edit.Leg B is the answer to the question: a control that renders always checked satisfies
"an absent key shows checked" and is strictly worse than the bug. The stored-
falserowsrefuse it, and they are marked ⛔ load bearing in the file so they are not later softened
into "differs from absent" — which an always-checked control also fails only by accident.
Verification
pnpm exec vitest run …/FlowNodeInspector.declaredDefault.test.tsx— 17 passed, exit 0.pnpm exec vitest run packages/app-shell/src/views/metadata-admin/— 238 files, 2481 passed /1 skipped, exit 0. Declared narrowing, with the measurement that it excludes nothing:
packages/app-shellcarries 650 test files and this run covered 238 of them. The whole-packagerun was attempted and the container killed it at its foreground ceiling (
exit 144), so thenarrowing is measured rather than assumed — a repo-wide grep for importers of the three changed
modules (
flow-node-config,FlowNodeConfigField,isUnsetFieldValue) returns 21 files, allunder
views/metadata-admin/. The single hit outside that directory,providers/ExpressionProvider.tsx:324, is a prose comment naming the path, not an import; thesame grep shape with a lit control (
MetadataClient) does return files outside the directory.CI runs the full farm.
pnpm --workspace-concurrency=2 --filter '@object-ui/app-shell^...' build— exit 0 (closurebuilt before type-checking).
pnpm --filter @object-ui/app-shell type-check— exit 0 (tsc --noEmitandtsc -p tsconfig.test.json). The pin file is in the test program:tsc -p tsconfig.test.json --listFileslists it once, withFlowNodeConfigField.tsxbesideit and a zero-hit negative control.
pnpm --filter @object-ui/app-shell lint— exit 0, 0 errors;eslint --format jsononthe three changed files reports
errors=0 warnings=0for each, against a lit control thatdoes report warnings.
check:control-bytes✅ OK·check:vi-mock-specifiers✅ OK·check:vi-mock-inherit✅ OK·check:designer-field-key-parityOK·check:i18n-keysOK.check-governed-queue-guard --teston all four changed paths —✅ NOT GOVERNED — 4 path(s) checked against 5 governed surface(s); none matched.check-changeset-presence—✅ 3 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s).check-changeset-no-major✅·check-changeset-fixed✅·
check-changeset-overwrite✅.Changeset
@object-ui/app-shell: patch. No prop, option or metadata key is added and no authoreddocument changes meaning; what changes is that one control stops contradicting the runtime
for metadata that already parses.
minorwas weighed and not taken — nothing here is a newcapability an author can reach for. (
majoris forbidden in this repo.)🤖 Generated with Claude Code
https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S
Generated by Claude Code