fix(app-shell): declare the spec defaults lockRecord and interrupting already apply - #9339
Draft
os-tesla wants to merge 4 commits into
Draft
fix(app-shell): declare the spec defaults lockRecord and interrupting already apply#9339os-tesla wants to merge 4 commits into
os-tesla wants to merge 4 commits into
Conversation
… already apply `approval.config.lockRecord` and `boundary_event.boundaryConfig.interrupting` declared no `defaultValue` while the installed `@objectstack/spec` (17.4.0) materialises `true` for both keys when a node omits them. Since the boolean config control began seeding its checked state from `defaultValue`, declaring nothing is not a missing claim on these two controls — it is the wrong one. Both boxes drew UNCHECKED: the inspector told the author the record stays editable while an approval is pending (the runtime locks it) and that a boundary event leaves its host activity running (it cancels it). Both values are derived from the installed spec, and the acceptance pin re-derives them from `ApprovalNodeConfigSchema` / `FlowNodeSchema` at assertion time rather than restating the literal, so an upstream flip reddens instead of drifting. Three rows per field: the declaration against the contract, the RENDERED checkbox (which reads the spec directly, so dropping a declaration reddens it on the control the author sees), and the stored-`false` row that an always-checked control fails. The exact-count declaration pin moves with them, count and comment together. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
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
|
…aration pin objectui#9337 (card objectui#9278) landed while this branch was open and added `end.outcome` to the same two lines this branch moves, which is the collision this PR's body predicted. Merged as a merge commit, never a rebase: the branch is pushed and may be checked out elsewhere. `flow-node-config.ts` auto-merged — the two sides add to different node types. The conflict was `FlowNodeInspector.declaredDefault.test.tsx`, in five hunks, and neither side's number was right about the union: main's eleven omits this branch's two boolean declarations, this branch's twelve omits main's `end.outcome`. Resolved by RE-DERIVING off `FLOW_NODE_CONFIG`, not by picking a side. Swept `fieldsForNodeType` over the same 24 node types the pin sweeps: 13 fields declare a defaultValue (8 select, 4 boolean, 1 number) 8 of the 13 are select-kind Both one-sided resolutions were run first and both reddened for the right reason: main's file verbatim misses `approval.lockRecord` and `boundary_event.boundaryConfig.interrupting`; this branch's file verbatim misses `end.outcome`. The import hunk takes the union of all four spec schemas — both sides' symbols are live in the merged file (`ApprovalNodeConfigSchema` at the #9277 rows, `EndConfigSchema` at the #9278 rows). Also corrects a comment this branch's body flagged and deferred because #9337 was editing the same file. That reason has expired. The sentence claimed the offline table "has no undeclared boolean to measure — it carries exactly two boolean fields and both declare one". Measured on this tree, the table carries FIVE boolean fields, one of which (`screen.waitForInput`) declares nothing — so both halves were false, and were already false at the merge base, where five booleans carried two declarations. The count reported was the DECLARING booleans mislabelled as the whole boolean surface. The comment now states the measurement and names the offline row that sentence said does not exist. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
… correctly The merge commit's comment names `screen.waitForInput` as the offline undeclared boolean the old sentence claimed did not exist. Naming it without measuring it lays a trap: the next reader sees an undeclared boolean in a file whose whole subject is missing declarations, and declares one. Measured on the installed `@objectstack/spec` (17.4.0): `waitForInput` is `z.boolean().optional()` with no `.default(...)`. An omitted key materialises nothing, so under this file's own stated convention — a `defaultValue` mirrors what the spec applies to an omitted key — there is nothing for a declaration to mirror, and writing one would invent a default the runtime never applies. That is the opposite direction from `approval.config.lockRecord` and `boundary_event.boundaryConfig.interrupting`, where the spec does materialise `true` and the declaration was therefore missing. Comment only; no assertion moves. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
Collaborator
Author
Round 2 — merged
|
| leg | pin title it carried | reddened because |
|---|---|---|
origin/main's file verbatim |
exactly eleven fields declare a defaultValue |
derived set adds approval.lockRecord and boundary_event.boundaryConfig.interrupting |
| this branch's file verbatim | exactly twelve fields declare a defaultValue |
derived set adds end.outcome |
AssertionError: expected [ 'approval.behavior', ...(12) ] to deeply equal [ 'approval.behavior', ...(10) ]
+ "approval.lockRecord",
+ "boundary_event.boundaryConfig.interrupting",
AssertionError: expected [ 'approval.behavior', ...(12) ] to deeply equal [ 'approval.behavior', ...(11) ]
+ "end.outcome",
The numbers were RE-DERIVED, not computed as 11 + 2
Swept off FLOW_NODE_CONFIG through fieldsForNodeType, over the same 24 node types the pin sweeps (FLOW_NODE_TYPE_OPTIONS plus the five off-picker types), on the merged tree:
declaringCount = 13
byKind = { select: 8, boolean: 4, number: 1 }
selectCount = 8
11 + 2 also gives 13. The two agreed; that is a result, not the method — the number here came off the table. The comment now says so, and says the next lander re-reads rather than inherits.
The two count lines, against each parent:
# vs this branch's pre-merge head 6e1b4c415
- it('exactly twelve fields declare a defaultValue, and these are they', () => {
+ it('exactly thirteen fields declare a defaultValue, and these are they', () => {
- 'the select-kind half of the declaration surface — seven of the twelve',
+ 'the select-kind half of the declaration surface — eight of the thirteen',
+ 'end.outcome',
# vs origin/main b2bb8134d
- it('exactly eleven fields declare a defaultValue, and these are they', () => {
+ it('exactly thirteen fields declare a defaultValue, and these are they', () => {
- 'the select-kind half of the declaration surface — eight of the eleven',
+ 'the select-kind half of the declaration surface — eight of the thirteen',
+ 'boundary_event.boundaryConfig.interrupting',The select-kind numerator held at eight: this branch's two additions are both boolean-kind, so only the denominator moved. The import hunk took the union of all four spec schemas — every one of them is live in the merged file (ApprovalNodeConfigSchema and FlowNodeSchema at the objectui#9277 rows, EndConfigSchema and FlowNodeSchema at the objectui#9278 rows).
The stale comment this body deferred — decided, not deferred again
This body left a false sentence in place because "objectui#9337 is editing this same file right now". That reason expired when objectui#9337 landed, so it was decided this round: the sentence is still false on the merged tree, and it was corrected.
It claimed the offline table "has no undeclared boolean to measure — it carries exactly two boolean fields and both declare one". Measured by the same sweep:
| boolean fields | declaring | undeclared | |
|---|---|---|---|
merge base 0f3d15314 |
5 | 2 | 3 |
| merged tree | 5 | 4 | 1 |
Both halves false, and false at the merge base too — the "two" was the declaring booleans mislabelled as the whole boolean surface. The comment now carries the measurement and names screen.waitForInput as the undeclared one.
A second commit (7cae0d19c) then measured why it is undeclared, because naming it without that is a trap: the installed @objectstack/spec 17.4.0 types waitForInput as z.boolean().optional() with no .default(...), so nothing is materialised for an omitted key and there is no spec answer for a declaration to mirror. It is not another instance of this card's class, and the comment says so in as many words.
Verification on the merged tree
All heavy runs through the shared verify lock; VERDICT lines quoted, never a bare exit code.
| what | verdict |
|---|---|
vitest run packages/app-shell/src/views/metadata-admin/ |
VERDICT command-exit 0 — 254 files, 2689 passed, 1 skipped |
| the count pin, named | ✓ ... exactly thirteen fields declare a defaultValue, and these are they |
| the select-kind pin, named | ✓ ... every select-kind declaring field renders its declared default |
turbo run type-check — app-shell + all three dependents |
VERDICT command-exit 0 — 39 tasks successful |
turbo run lint — same set |
VERDICT command-exit 0 — 5 successful, 0 errors |
| whole file after the second commit | VERDICT command-exit 0 — 32 passed (32), lint warnings unchanged at 2995 |
Gates re-run on the merged tree, all exit 0: check-changeset-fixed, check-changeset-no-major, check-changeset-claims (report-only), check-changeset-presence (7 changesets for 13 published source files), check-control-bytes, check-test-path-roots, check-new-cross-file-line-citations.
check:changeset-claims re-flagged .changeset/6499-inactive-values-retained.md for its "all 33 gated fields" claim. Still true: this merge adds no showWhen group, and the mechanical pin behind that count is inside the 254 green files.
Dependent-set membership read — redone here, not inherited
Read off every manifest naming the package under change:
depends on @object-ui/app-shell -> apps/console (type-check: YES)
examples/byo-backend-console (type-check: YES)
examples/console-starter (type-check: YES)
apps/site (@object-ui/site) -> declares type-check, but does NOT name app-shell
Worth stating precisely, because the conclusion and the reason come apart: @object-ui/site does declare a type-check script. It is out of the set because it is not a dependent at all — its @object-ui/* dependencies are the plugin family plus components, core, fields, layout, react, types and example-schema-catalog. Structurally out of reach, not script-less.
Untouched, deliberately
Still draft, no auto-merge, not enqueued, and needs:contract-review neither added nor removed — all of that belongs to the PM.
Generated by Claude Code
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
|
Collaborator
Author
Contract reviewHead judged: ① Derived judgments
1 & 2 — what the declaration asserts, and why "declares nothing" was not neutral. Since objectui#8451 the 3 — the leg I most want on the record. The probe ran both boolean values of each key against every sibling enum value (all four 4 — how thirteen was reached. objectui#9337 landed mid-flight and took the pin's base from ten to eleven, which turned the merge into a real conflict in five hunks. The seat did not pick a side:
⭐ And it derived that before reading my prediction back. I had predicted 13 and 5 — the stale sentence, and it was worse than the card reported. It claimed the offline table "has no undeclared boolean to measure — it carries exactly two boolean fields and both declare one". Measured by the same sweep: five boolean fields; at the merge base 2 declaring / 3 undeclared, on the merged tree 4 declaring / 1 undeclared. ⇒ both halves false, and already false at the merge base — the "two" was the declaring booleans mislabelled as the whole boolean surface. This card added no boolean field, it added ⭐ The round-1 reason for leaving it ("objectui#9337 is editing this file right now") had expired, and the seat did not reuse it. Better: a second commit measured why ② Semver
③ Boundary flags
Independence⛔ For this lane a clause-② review is default-tier self-review plus the gates, not an independence-qualified ruling — the independence clause and the downgrade fuse bind the VerdictPASS. Both carriers cleared in the same stroke as this comment. Arming waits on the nine required contexts going green on Generated by Claude Code |
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
|
This was referenced Sep 13, 2026
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 #9277
approval.config.lockRecordandboundary_event.boundaryConfig.interruptingnow declare thedefaultValuethe installed spec already applies, so the two checkboxes stop asserting the opposite of what the runtime does.Two table lines, six pin rows, one exact-count pin moved.
The spec probe triage could not run — it reads CONFIRMING
@objectstack/specwas not installed at the triage seat, so both spec-side claims arrived NOT MEASURED. Run here against the installed 17.4.0. The probe prints the resolvedpackage.jsonpath and version before it parses anything, so a missing install would have been an instrumented failure, never a quiet negative answer.ApprovalNodeConfigSchema.safeParse({ approvers: [{ type: 'user', value: 'u1' }] })lockRecord: trueFlowNodeSchema.safeParse(... boundary_event ...).data.boundaryConfiginterrupting: truePositive controls in the same output — this is what makes the two absences a reading rather than a dead probe. The approval parse returns
behavior: 'first_response',onEmptyApprovers: 'admin_rescue'andmaxRevisions: 3alongsidelockRecord, and the form declares a matching'first_response'/'admin_rescue'/'3'for all three; the boundary parse returnseventType: 'error'alongsideinterrupting, and the form declares'error'for it.So the declared value is
'true'on both —String(specDefault), derived from the installed spec, never typed from taste, exactly as thedefaultValuedoc comment requires.The probe was taken past the first accepted value
Prompted by objectui#9336, which the sibling seat found by doing this: I probed both boolean values of each key, and each against every sibling enum value (all four
behaviorvalues, all foureventTypevalues). No sibling constraint on either key —trueandfalseare both accepted everywhere, and an omitted key materialisestrueeverywhere.That negative carries a positive control that fires: the same probe run asserts
EndConfigSchema, and it does surface the objectui#9336 superRefine, quoting both halves of it. A probe that can see that constraint and sees none here is a reading.This also matters to the pin: the stored-
falserows assert a shape the spec genuinely accepts.The harm, and why "declares nothing" was not neutral
Since objectui#8451 the
booleancontrol seeds its checked state fromdefaultValue, so an absent declaration does not render as "no claim" — it renders as unchecked. The screen was already asserting something, and it was false:Lock recordunchecked told the author the record stays editable while the request is pending. The runtime locks it.Interruptingunchecked told the author the boundary event leaves its host activity running. It cancels it.Note on how this survived: the canvas node seeder in
flow-canvas-parts.tsxalready writeslockRecord: trueandinterrupting: trueinto a freshly dropped node, so palette-created nodes stored the key and drew correctly. Only nodes that omit the key — imported flows, JSON-authored nodes, older drafts — rendered the lie. The seeder's values and the spec's agree with the declarations added here.Where the pin count came from
Base ten, plus this card's two = twelve. Re-read three times as
origin/mainadvanced under me (0f3d153, then 80830ab, then a686403) — the pin readtenat all three, because objectui#9278's PR objectui#9337 is still open and unmerged.The select-kind count on the same pin, which the PM flagged separately. Its numerator does not move here: both fields added are
boolean-kind, so the select-kind half stays at seven, while its denominator follows the field count. That line now readsseven of the twelve— base-plus-own arithmetic applied to the half that actually moved.Reddening this pin is expected, not routed around: its own comment says a change that adds a further declaration moves the line. The comment is updated with the count, and I added a standing note that the number is a live base plus the landing card's own additions, naming both cards, so whoever lands second re-reads rather than inherits. PR objectui#9337 added an equivalent instruction to the same comment — the two are compatible in wording and intent.
If objectui#9337 lands first, the base becomes eleven and the correct answer here is thirteen (and the select-kind line becomes
eight of the thirteen). That is a re-read, not an arithmetic I can pre-commit.Ablation leg
Run from the committed state, one declaration removed at a time. Each leg proves the mutation reached the disk before reading any result (declaration count 4 to 3 in the file, plus a blob hash differing from the
HEADblob), and proves the restore by blob-hash equality with theHEADblob and an emptygit diff HEAD— never by an exit code. A trap restores on any early exit.lockRecord's declarationapproval.lockRecord declares ...,approval.lockRecord draws a CHECKED box ..., the exact-count pininterruptingrowsinterrupting's declarationboundaryConfig.interrupting declares ...,boundaryConfig.interrupting draws a CHECKED box ..., the exact-count pinlockRecordrowsThe seeding assertion is the one the acceptance names, and it reddens on the rendered control:
Zero cross-contamination in either direction, so each leg measures its own field rather than knocking the file over.
The seeding rows read the spec rather than the table, which is why dropping a declaration reddens them on the control the author sees instead of only on the declaration row above.
And the pin was written before the code
The "unmodified" arm is the real base tree, not a reconstruction: the six new rows plus the moved count were committed to the working tree first and run against untouched
flow-node-config.ts, giving5 failed | 23 passed, with the two seeding rows failingexpected false to be true— the defect itself, read on screen. The two stored-falserows passed on the base tree, correctly: an undeclared boolean already draws unchecked for a storedfalse.Verification
vitest run packages/app-shell/(6 shards, all under the shared verify lock)turbo run type-checkfor app-shell + all three dependentsturbo run lintfor the same setscripts/__tests__/check-changeset-claims.test.ts(namesflow-node-config)Gate family derived by hand from
package.jsonplus.github/workflows/— this repo has no dispatch-gates script. All green:check:control-bytes,check-changeset-presence,check:changeset-fixed,check:changeset-no-major,check:changeset-claims(report-only, see below),check:new-line-citations,check:designer-field-key-parity,check:i18n-designer-parity,check:i18n-drift,check:i18n-dead-keys,check:comment-mask-corpus,check:test-path-roots,check:vi-mock-specifiers,quick-reference:check,check:unreferenced-sources.Two readings worth stating rather than hiding:
half-state-patrolis not in this diff's gate family. My first invocation spelled itscripts/check-half-states.mjsand exited 1 withMODULE_NOT_FOUND— that is NOT MEASURED, not a red gate. The real script isscripts/pm/check-half-states.mjs, and it sweeps the issue board, not the tree.eslint . --no-inline-configreports 95 errors across 79 files. That is the configuration CI never uses, in the shape AGENTS.md warns about forprettier: the gate is per-packageeslint .throughturbo run lint, which reports 0 errors everywhere. My two changed files are 0 errors, 0 warnings in both invocations.The dependent-set membership read
Done here rather than inherited, because a declared narrowing can be the load-bearing one:
@object-ui/siteis out of reach structurally, not because it is "a docs app": its@object-ui/*dependencies are the plugin family,components,core,fields,layout,react,typesandexample-schema-catalog— app-shell is not among them. All three real dependents were type-checked and linted.Blast radius beyond the package (objectui#9273)
examples/schema-catalog/: out of reach, measured. It contains no flow-node metadata at all — noapproval,boundary_event,lockRecordorinterruptinganywhere in its sources. Nothing there can render these two controls.flow-node-config, of which two live outsidemetadata-admin(providers/ExpressionProvider.tsxandscripts/__tests__/check-changeset-claims.test.ts). That is exactly why the whole package was run in shards instead of narrowing to the inspectors directory.showWhencontroller (25 controllers exist in the table, and neither key is referenced by any of them), so declaring a default here cannot move which fields are on screen. ThecontrollerAdmitsread site is untouched.What
check:changeset-claimsasked, and the answerIt flagged
.changeset/6499-inactive-values-retained.md, whose body namesflow-node-config.tsand claims "all 33 gated fields across the descriptor tables". The paragraph did not go false:showWhenoccurrences in that file are 36 on the base and 36 at this head, this change adds no gate, and neither new declaration belongs to a gated field. No correction needed.Measured, reported, deliberately NOT repaired here
loop.config.iteratorVariable's placeholder is wrong. The form hintscurrentItem;LoopConfigSchema.safeParse({ collection: '{x}' })appliesitem. They disagree, so an author writing acurrentItemreference against the form's own hint gets an unresolved reference.map.config.iteratorVariablehintsitemand the spec also appliesitem— those agree. Both aretextcontrols, which readplaceholderand neverdefaultValue, so this is not the same repair and is out of scope for this round per the dispatch. Measured and handed back for filing.screen.waitForInputis the offline table's one remaining undeclared boolean, and it is legitimately undeclared:ScreenConfigSchema.safeParse({})returns{}, materialising no default. Not a third instance of this class.FlowNodeInspector.declaredDefault.test.tsxjustifies one row with "the offline table has no undeclared boolean to measure — it carries exactly two boolean fields and both declare one". Measured: the table carries five boolean fields (screen.waitForInput,approval.lockRecord,approval.escalation.enabled,approval.escalation.notifySubmitter,boundary_event.boundaryConfig.interrupting), three of them undeclared before this change. The sentence was already false on the base tree, and it is a symptom of precisely the blind spot this card closes. Left untouched: it is another card's rationale, the row it justifies still measures something real, and objectui#9337 is editing this same file right now..changeset/6830-flownode-select-declared-default.mdsays "Seven of the ten declaring fields are select-kind". This change moves the denominator. Left as written: it is a historical measurement in another card's pending release note,check:changeset-claimsexcludes counts from its scope by a measured decision, and any number written there now would be falsified again by objectui#9337. Flagged rather than edited.Coordination
UNDECLARED_REGISTERcontaining exactly these two rows (approval:config / lockRecordandboundary_event:boundaryConfig / interrupting). It has not landed: measured 0 hits forUNDECLARED_REGISTERonorigin/main, withcontrollerAdmitsat 12 hits anddefaultValueat 8 hits in the same files as the positive control that the grep could fire. Per the acceptance, no register row was invented here.FlowConfigField.defaultValue's doc comment was left alone on purpose: objectui#9279 rewrites that exact block.Changeset
patch, matching objectui#8451 and objectui#6620, the two nearest precedents. Justified by measurement: no prop, option or metadata key is added and no authored document changes meaning — what changes is that two rendered controls stop contradicting the runtime.majoris unavailable in this repo's singlefixedgroup, and this is not breaking, so no**BREAKING**carrier is needed.Session reference, as a code span so it survives a body edit:
https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ🤖 Generated with Claude Code
https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
Generated by Claude Code