Uh oh!
There was an error while loading. Please reload this page.
app-shell: give the pre-publish security block a way to the object it names - #5599
Conversation
… names Studio's pending-changes sheet reports the drafts the publish door would refuse — `object/crmext_visit` — and could not take the author there. The `?surface=<type>:<name>` deep-link captures the URL once, at MOUNT, and the sheet opens over an already-mounted pillar, so writing the param changed the URL and moved nothing. The mount-time capture stays exactly as it was: the mirror half rewrites the param on every in-pillar selection, so a capture that followed the URL would re-trigger its restore on each one. The missing piece is a live target delivered beside the URL. `surfaceDeepLinkChannel` adds it — producers ask by surface identity, the host routes cross-pillar requests back through the URL (that pillar is unmounted, so its capture is the right mechanism) and vetoes what the author declines over unsaved edits, and the mounted pillar applies the rest AT MOST ONCE, by id, so a standing request can never drag them back off a later selection. Off-Studio the producer hook is null and the item name stays the prose it has always been: the sheet is shared with the Home / draft-preview bar, where the designer is not a reachable destination and a dead link is worse than the sentence saying where to go. Reachability is answered by the tree, never by a route string. Fixes#5476 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK
✅ 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-sales
commented
Aug 21, 2026
PM review — ACCEPT (card #5476)Gates. 22 named check runs read individually for The three questions, ruledQ1 — the two They are in the same package, Q2 — ship the landing on the object; do not add a tab axis to the shared surface identity. (A) A tab axis would widen a shared identity to serve one destination, and every later surface would then have to answer what its tab is. Landing on the object keeps the change local to the thing that actually needs it. Q3 — footer form: session URL in PR bodies, bare form in comments. (A) Now a standing rule for this lane, recorded on the seat post rather than re-decided per card. PM fence error, disclosedI fenced this card to This is the second fence error of the same shape in this round (the other is on #5597) and the third this session. The pattern, stated so it stops recurring: I keep fencing the thing being changed rather than the thing being delivered. Recorded on the seat post. Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#5476
Route A as ruled — a live target channel beside the mount-time capture. The
measurement asked for before landing is at the bottom, along with one scope
deviation I could not avoid and am flagging rather than burying.
The defect
The pending-changes sheet names the drafts the publish door would refuse —
object/crmext_visit, with the rule's fix-it hint and "Fix it on the objectunder Settings → Record sharing". Naming it shipped in #5418; reaching it did
not.
useSurfaceDeepLinkreads?surface=TYPE:NAMEexactly once, at mount,and the sheet opens over an already-mounted
DataPillar— so writing the paramchanged the URL and moved nothing.
What route A actually is
The mount-time
useRefis not a bug to route around: the MIRROR half rewritesthe param on every in-pillar selection, so a capture that followed the URL would
re-trigger its restore on each one. Both existing halves are URL-shaped, which
is exactly why a producer already inside the pillar cannot use either. The
missing half is a live target carried beside the URL.
surfaceDeepLinkChannel.ts(new, instudio-design/) is that half:useSurfaceNavigator()and ask by surface identity(
{type, name}) — never by a Studio route the sheet would have to know how tobuild.
StudioDesignSurface) routes a request for ANOTHER pillar'ssurface back through the URL, because that pillar is unmounted and its
mount-time capture is the mechanism built for precisely that; it vetoes what
the author declines over unsaved edits, and closes the sheet.
DataPillar) apply a request AT MOST ONCE, by amonotonic id.
That one-shot rule is the whole regression guard. A standing request re-resolved
on the next rail reload would drag the author back off whatever they had since
selected — the exact behaviour the mount-time ref exists to prevent.
Off-Studio degradation
The sheet's other home is the Home / draft-preview bar, where the Studio object
editor is not a reachable destination at all.
useSurfaceNavigator()returnsnullwherever no host published the channel, so reachability is answeredstructurally, by the tree — never by sniffing a route string, which would
also have crashed the existing suites that render this panel with no router at
all. Off-Studio the item name stays the exact prose #5418 shipped.
Both directions are assertions in
DraftChangesPanel.securityLink.test.tsx:the in-Studio case clicks the control and expects the surface identity; the
off-Studio case asserts no button and no link — not a disabled one, not a dead
anchor — and that the item name and the "Settings → Record sharing" sentence are
still there.
The measurement you asked for: does A change what the other three pillars observe?
No.
useSurfaceDeepLinkis untouched apart from its doc comment — samesignature, same return, same behaviour. The Interfaces / Automations / Access
call sites are byte-identical, and none of them subscribes to the channel.
git diffon the hook is comment-only; the three otheruseSurfaceDeepLink(...)call sites do not appear in the diff at all.
Reverse-verification
Two ablations, each mutated with an anchored
grep -cin both directions plusgit diff --stat, restored through atrap ... EXIT INT TERM, tree verifiedbyte-identical afterwards (
git status --porcelainempty). Both suites reach themutated module by relative same-package path, so no
dist/is in the resolutionpath and there is nothing to rebuild — the import specifiers are printed in the
ablation log.
Ablation 1 — delete the live half's delivery (
setRequested(...), 1 -> 0occurrences, marker 0 -> 1):
The mount-time CONTROL stayed green throughout —
useSurfaceDeepLink.test.ts(the pre-existing pins) and the two new capture pins: the capture ignores every
URL change after mount, and a live request never moves it. Capability added,
not swapped.
Ablation 2 — delete the one-shot guard (
if (requestedSurface.id === ...) return;, 1 -> 0): exactly one test red — "never re-applies it" — while "opensthe object the request names" stayed green.
Ablation 2 is why this PR has a stronger test than it started with. On its first
run it came back all green: my one-shot pin was asserting an absence in a
window where the rail reload had not yet happened, so nothing had had a chance to
violate it. The pin now waits on
client.listactually being called again beforeasserting nothing moved. A phantom assertion that the ablation caught, not a
tuning tweak.
Gates (exit codes captured before any pipe; each gate's own verdict quoted)
All at
a506dff0d, the final commit.pnpm --filter '@object-ui/app-shell^...' buildVERDICT command-exit 0 · held the lock 140spnpm --filter @object-ui/app-shell type-checkEXIT=0—tsc --noEmit && tsc -p tsconfig.test.jsonpnpm --filter @object-ui/app-shell lintEXIT=0—2510 problems (0 errors, 2510 warnings)vitest run(narrowed, see below)Test Files 48 passed (48) · Tests 298 passed (298)check-control-bytesOK (scanned 4650 tracked text file(s); skipped 85 binary)check-changeset-presence7 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)check-changeset-no-majorNo changeset declares a major bump.check-changeset-fixedAll workspace packages are in the changeset fixed group.check-eager-closure-budget3785.3 KB gzipped across 52 of 508 chunks (budget: 3867.2 KB, headroom: 81.9 KB)check-lint-coverage46/46 packages linted, 0 with outstanding errorscheck-type-check-coverage45/46 via type-check·41/41 packages compile their testscheck-i18n-call-site-keyscheck-eager-closure-budgetfirst exited 2 — its documented "no trustworthymeasurement" code, because no console build had written the report. This diff
puts a module into the console's EAGER graph, so a broken gauge was not good
enough:
apps/consolewas built and the gate re-run for the number above. Thenew module imports
reactand nothing else —useSurfaceDeepLinkwasdeliberately NOT imported into the sheet, since it reaches
nav-selectionandthrough it the App-nav inspector.
Declared narrowing of the vitest run. Not the whole package: the run covers
views/studio-design/,preview/, and the threemetadata-adminsuites thatname anything this diff touches. Containment is measured, not assumed — grepping
the whole package for every identifier changed here (
surfaceDeepLinkChannel,useSurfaceDeepLink,DraftChangesPanel,StudioDesignSurface,DataPillar)returns 26 suites, and all 26 are inside that run. Across the rest of
packages/, zero files outsideapp-shellreference any of them. CI runs thefull farm regardless.
Scope deviation — please rule
The dispatch fenced this to
packages/app-shell/src/views/studio-design/**plusa changeset, on the stated grounds that "the only other consumer of the hook is
StudioDesignSurface.tsx, in the same directory". That is true of the hook,but the sheet that has to render the link is
packages/app-shell/src/preview/DraftChangesPanel.tsx— a sibling directory —and the binding requirement ("the link must degrade to prose off-Studio, and
that degradation is a test assertion") has no subject without it.
So two files sit outside the fence, and I did not take them silently:
Same package, no change to the package's public exports (
src/index.tsuntouched), no overlap with #5544's
src/chrome/**, and the import directionpreview/ -> views/is the one already established there(
views/metadata-admin/previews/object-fields-io.js). The alternative wasshipping an unconsumed channel and a degradation requirement with nothing to
degrade. Happy to split the sheet half into its own PR if you would rather rule
the other way.
Deliberate non-goals
— not on its Settings tab. Adding a tab axis would widen the identity four
pillars share; the sheet's sentence still says where to go once the object is
open.
PILLAR_FOR_SURFACE_TYPElists only the types a pillar actually resolves.An unlisted type is delivered in place rather than guessed at: navigating to
the wrong pillar costs the author their position and buys nothing.
Generated by Claude Code