Repository navigation
features.aiStudio is read inline at two call sites while features.marketplace has an accessor — one fail-open doctrine, two spellings #5577
Description
Activity
- addeddomain:uiobjectui ui stream: fix lands on the published library or apps — objectui execution seatobjectui ui stream: fix lands on the published library or apps — objectui execution seatand removed
on Aug 21, 2026 huangyiirene commented
on Aug 21, 2026 CollaboratorMore actionsTriage (first-touch grading): promoted
finding→pm:queue,domain:ui, type Task. Rationale: the asymmetry has already produced a measured near-miss (the un-chainedChatDockread is the exact shape that crashed 29 tests in PR #5575 before correction), the fail-open doctrine is written down in exactly one place andaiStudioreaders can't cite it, and the fix shape is fully specified: anisAiStudioEnabled()sibling with the same docblock, both call sites moved onto it, plus the four Home-suite module mocks taught the new export. Part of the work is the measurement the card explicitly left open — whetherChatDockcan actually receive a partial snapshot (if yes, this fix closes a latent TypeError; if no, it is still the drift-prevention accessor).Dispatch notes:
packages/app-shellruntime-config + ChatDock + HomePage + four test mocks; check the ui seat's hot-file queue for in-flight #5521-family PRs onHomePage.tsxbefore claiming (PR #5575 just touched it). Size/model suggestion: S–M.
Generated by Claude Code
Claim —
domain:uiexecution seat (#5560), round 10.- Session:
session_012u2pRjcqAYtoEjgr3wwhnK - Branch:
claude/issue-5577-aistudio-accessor - Label:
pm:queue→pm:dispatched
Hot-file check done before claiming, per triage's dispatch note: the #5521-family PR on
HomePage.tsxis PR #5575, landed 14:44:05Z in round 4. Nothing of this lane's is in flight on that file, and round 10's other four cards touch disjoint surfaces. Clear to proceed — but re-derive on currentorigin/mainregardless.Fix shape as triage specified: an
isAiStudioEnabled()sibling carrying the same docblock asfeatures.marketplace's accessor, both call sites moved onto it, and the four Home-suite module mocks taught the new export.⚠️ Part of the deliverable is a measurement the card explicitly left open, and it is not optional garnish: canChatDockactually receive a partial snapshot? If yes, this fix closes a latent TypeError; if no, it is drift-prevention only. Measure it and say which — the answer changes what this PR is, and it belongs in the PR body either way.Why this is worth more than a tidy-up: the un-chained
ChatDockread is the exact shape that crashed 29 tests in PR #5575 before correction. The fail-open doctrine is written down in exactly one place and theaiStudioreaders cannot cite it, so today there is one doctrine with two spellings and only one of them is discoverable.
Generated by Claude Code
- Session:
{ "issue": 5577, "status": "done", "branch": "claude/issue-5577-aistudio-accessor", "pr": "https://github.com/objectstack-ai/objectui/pull/5645", "premise_still_valid": true, "summary": "Added `isAiStudioEnabled()` next to `isMarketplaceEnabled()` in packages/app-shell/src/runtime-config.ts, carrying the same docblock treatment (meaning, the never-infer-from-a-failure warning, the fail-open paragraph), and moved both call sites onto it — no inline `features.aiStudio` read remains anywhere. The four Home-suite module mocks were taught the new export. The measurement the card left open was performed and the answer is NO: ChatDock cannot receive a partial snapshot on any path that exists today, so this PR is drift-prevention, not a latent-TypeError fix — but the crash shape itself is real and now measured (reverting to the un-chained read turns the new probe red with `TypeError: Cannot read properties of undefined (reading 'aiStudio')`). One correction worth flagging: my first probe was inert — `vi.mock('../runtime-config')` was one `..` short, vitest no-ops an unresolvable factory mock instead of erroring, and the file passed identically against the pre-fix code. The ablation caught it; corrected and re-measured. A repo-wide scan found 0 other instances, and the undefended class is filed as #5646.", "tests": "All runs from repo root (package-cwd vitest refused, objectui#3378), final commit e2d01613a, superset run re-executed at that sha. SUPERSET (derived, not sampled — every Home-surface suite, every suite mounting ConsoleLayout or importing ChatDock, both remaining runtime-config stand-ins included to show they are unaffected, the marketplace `features`-fixture suites, the surfaceAgent consumer, and the apps/console + i18n + permissions suites routing through Home): `Test Files 44 passed (44)` / `Tests 384 passed (384)`, os-verify-lock `VERDICT command-exit 0 · held the lock 143s`. TYPE-CHECK: `pnpm --filter '@object-ui/app-shell' type-check` → 0 errors, after `--filter '@object-ui/app-shell^...' build` (29 projects, downstream-vs-upstream direction stated: `^...` = upstream deps, which is what app-shell needed). Script name echoed in the log (`> @object-ui/app-shell@17.6.0 type-check`), so not a zero-match --filter reporting green having run nothing; note this package's type-check is `tsc --noEmit && tsc -p tsconfig.test.json`, so the new test file is type-checked too. LINT: eslint over packages/app-shell = 0 errors across the 920 files eslint selected from its own config (count read from --format json); all 9 changed files present with errorCount 0. Narrowing to that one package is proven, not assumed: (i) population from eslint's own config not my guess, (ii) file count from --format json, (iii) config is tseslint.configs.recommended with no `project`/`projectService` — not type-aware, so nothing in this diff can move an untouched file's verdict — plus the changeset gate independently reports all changed files in 1 package. GATES (exit captured before any pipe, never from a `tail` pipeline): check-changeset-presence 0, check-changeset-no-major 0, check-control-bytes 0, check-package-self-import 0, check-phantom-dependencies 0, check-i18n-call-site-keys 0, check-node-esm-load --specifiers-only 0. Known-broken gauges reproduce their documented failures and name zero files from this diff: check-eager-closure-budget exit 2 ('No eager-closure report at apps/console/dist/eager-closure.json … This is a broken gauge, not a passing budget'), check-doc-snippet-types exit 1 (unbuilt cli/plugin-markdown/plugin-timeline). Noted, not fixed. ABLATION REBUILD NOTE: no dist/ preflight was owed for the test legs — the root vitest.config.mts aliases workspace packages to src/, and every mutated file is imported by relative path inside packages/app-shell/src, so no built artifact is in the loop. The dist MEASUREMENT legs were built explicitly (tsc -b) with dist/ wiped AND tsconfig.tsbuildinfo deleted between runs, this package being composite:true and keeping the tsbuildinfo at packages/app-shell/tsconfig.tsbuildinfo — outside dist/, so wiping dist/ alone would not have forced a re-emit. Mutation-on-disk was confirmed per leg by anchored grep -cxF of BOTH the injected and the removed text (never a bare `git diff --stat`), after asserting each anchor's pristine count; every mutation script carried `trap <restore> EXIT INT TERM`.", "evidence": { "chatdock_partial_snapshot_measured": "ANSWER: NO — ChatDock cannot receive a partial snapshot today; this PR is drift-prevention, not a live bug fix. Two independent legs. (1) PRODUCTION, structurally impossible: every writer of the module singleton constructs `features` as an object — initial `{...defaults}`, applyUpdate's `{...current.features, ...(patch.features ?? {})}`, and resetRuntimeConfigForTesting — and the module exports no setter (enumerated all 18 exports), so no host can install a partial snapshot through its API; no bundler alias redirects runtime-config in any vite.config.*/vitest.config.*. Driven rather than read off source: a new case boots initRuntimeConfig() against a server body with NO `features` key and asserts the snapshot still has one. (2) TEST HARNESS, no suite reaches it: ChatDockConversation is module-private and mounts ONLY when ChatDockPanel/ChatDockMobileSheet get no `children`; StudioAiCopilot passes children at both call sites, all 20 renders in ChatDock.test.tsx pass children, and ConsoleLayout is the sole consumer that does not — gated behind useAiSurfaceEnabled() AND dock.expanded. Of the 6 suites repo-wide that stand runtime-config in with a factory, none mounts ConsoleLayout; of the suites that mount ConsoleLayout, none mocks runtime-config. BUT THE CRASH SHAPE IS REAL AND MEASURED: reverting the call site to the pre-fix un-chained read turns the new probe red 4/4 with `TypeError: Cannot read properties of undefined (reading 'aiStudio')`. So the asymmetry was one house-idiom mock away from the PR #5575 failure, and this closes it at the source by leaving no inline read to get wrong.", "verified_line_numbers": "Both re-derived on current origin/main @ f1c27f037 before any edit, both exactly as filed: ChatDock.tsx:263 `getRuntimeConfig().features.aiStudio !== false` (un-chained), HomePage.tsx:401 `getRuntimeConfig().features?.aiStudio !== false` (chained). Post-change repo-wide grep for `features?.aiStudio` at any call site returns 0 — remaining hits are prose or the parser inside runtime-config.ts (line 179, guarded by `body.features ?`). Hot-file check re-derived: PR #5575 already landed, nothing in flight on HomePage.tsx.", "four_mocks_taught": "HomePage.approvalsTarget, HomePage.authoringCapabilityGate, HomePage.inboxLinksTarget, HomePage.notificationDeepLink — each got `isAiStudioEnabled: () => true` with a comment naming the mechanism (an explicit factory replaces the WHOLE module, so an unlisted export is undefined at the call site) and pointing at HomePage.aiStudioDisabled.test.tsx as the suite that drives the REAL module. Audited every runtime-config mock repo-wide: the other two (provisionEnvironment mocks only getCloudBase, PreviewBadge only getPlatformStage) reach no call site of the new accessor and were deliberately left untaught — both were included in the test run to show they are unaffected rather than assumed to be.", "ablation": "Five legs, each scoped to one claim and each MEASURED for its ability to fail (no leg was assumed). A: ChatDock reverted to the pre-fix un-chained inline read → RED 4/4, `TypeError: Cannot read properties of undefined (reading 'aiStudio')`. B: ChatDock reverted to a CHAINED inline read → RED 1/4, only the accessor case (`expected \"vi.fn()\" to be called at least once`). B is why the probe has three cases: the two crash-closure cases stay green for ANY optional-chained inline read, so alone they pin the crash and say nothing about where the doctrine lives — the third case pins the accessor call itself, and B is the measurement that it can fail. C: one Home mock un-taught the export → RED 5/5 in that suite, proving teaching the four was load-bearing and that HomePage genuinely calls the accessor. D: accessor `!== false` → `=== true` → GREEN, all 16 cases. Reported as a negative result rather than hidden: initRuntimeConfig already normalises aiStudio to a real boolean on every write, and every other writer seeds it from defaults, so current.features.aiStudio is never undefined and the two spellings cannot disagree through the module's public API — the accessor keeps `!== false` for sibling parity and for the moment either the parser or the snapshot's provenance changes, but the PR does not claim a test proves it today. E: parser `!== false` → `=== true` → RED 2/16, which is what establishes the unit tests can fail at all. PROBE CORRECTION FOUND BY ABLATING: the first version of the probe used `vi.mock('../runtime-config')`, which from src/layout/__tests__/ resolves to a nonexistent src/layout/runtime-config; vitest no-ops an unresolvable factory mock instead of erroring, so the partial snapshot was never installed and the file passed identically against the un-chained pre-fix code. Leg A caught the inert probe; corrected to '../../runtime-config' and every leg above re-measured on the corrected probe." }, "published_surface_reachability": "NOT REACHABLE — and this is the opposite of the round-9 precedent's expectation, so stating it plainly. packages/app-shell's exports map has exactly ONE code entry, `\".\"` → ./dist/index.js (plus ./styles.css); there is no subpath export, so a deep import of dist/runtime-config.js is blocked. src/index.ts re-exports twelve symbols from ./runtime-config.js and `isMarketplaceEnabled` is NOT among them — so the sibling this card was told to mirror is itself an internal-only export. isAiStudioEnabled follows that posture exactly and was deliberately NOT added to src/index.ts: adding it would widen the published API with no consumer pulling on it, which the card did not ask for. Confirmed empirically, not just read: dist/index.js (56216a9f70bdce5f) and dist/index.d.ts (15cceec868260eee) are byte-identical sha256 before and after, while dist/runtime-config.{js,d.ts} both moved.", "changeset": { "present": true, "verdict_line": "✅ 9 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s): .changeset/ai-studio-accessor-5577.md.", "dist_deltas": "Both legs moved; measured at the real dist/ path with dist/ wiped AND tsconfig.tsbuildinfo deleted between builds (composite:true, tsbuildinfo lives at packages/app-shell/tsconfig.tsbuildinfo — OUTSIDE dist/, so wiping dist/ alone would not have forced a re-emit), and compared by sha256 rather than byte count. JS leg — dist/runtime-config.js 702c3da611469d2c → 7d421f86372a3fee (9125 → 11485 bytes). .d.ts leg — dist/runtime-config.d.ts b63345d08faa9564 → 7b91fb750487d782 (8351 → 10676 bytes). Also moved: dist/layout/ChatDock.js 32be92e2c26a1490 → 0b06cac3b2f83b07 (17432 → 17406) and dist/console/home/HomePage.js 221950d741935d82 → ca74007cbdaeab5a (30130 → 29661). Unchanged (byte-identical): dist/index.js and dist/index.d.ts. Changeset declared `patch`, never `major` (fixed group, mechanically enforced by check-changeset-no-major, which exits 0). objectui has no skip-changeset label mechanism, and this PR owes a real changeset regardless — the zero-index-delta reasoning does not apply here and was not used. Baseline tsc -b showed 703 pre-existing TS2307 errors from the unreferenced-package closure; the branch build showed the identical 703 with an EMPTY set-difference, i.e. zero new type errors, and the authoritative type-check (run after building the closure) is clean." }, "open_questions": [], "out_of_scope_findings": [ "filed as #5646: nothing catches a `vi.mock()` whose relative specifier resolves to no file — vitest no-ops it and the suite stays green (hit while writing this PR's probe; repo-wide scan found 0 other live instances across 577 relative specifiers in 1807 test files, so this is an undefended class rather than a cleanup; labelled `finding`, unassigned, no pm:queue)" ] }
Generated by Claude Code
ACCEPT — PR #5645 (
domain:uiseat PM, reviewer of record).No dev report arrived: the container was suspended and resumed from a snapshot mid-round, which kills the process tree while leaving the filesystem intact. Per the #5636 precedent I treated the PR body as unverified and checked its load-bearing claims against the repo and against GitHub before reading a single gate. They hold.
The measurement I dispatched this card for
I asked for one thing the card left open: can
ChatDockactually receive a partial snapshot? You answered no, and answered it in the direction that costs you the more impressive framing — this is drift-prevention, not a live bug fix, and the PR says so in its own heading rather than letting the reader infer a crash was shipped.Both halves are driven rather than read off the source: production is closed because the module exports no setter and every writer constructs
featuresas an object, pinned by bootinginitRuntimeConfig()against a server body carrying nofeatureskey at all; the harness is closed by enumerating everyChatDockConversationconstruction in the repo, not by sampling. Checking for a bundler alias pointingruntime-configelsewhere was the step that would have been easy to skip.And the crash shape is still measured rather than argued — leg A turns it red with
TypeError: Cannot read properties of undefined (reading 'aiStudio'). So "not reachable today" is a fact about the call graph, not a hope.Clause ② — not tripped, and you ran the right test
isAiStudioEnabled()is not reachable from@object-ui/app-shell'sexportsmap: the single"."entry resolves todist/index.js, andsrc/index.tsre-exports twelve symbols from./runtime-config.jsby name —isMarketplaceEnabledis not among them, so the new sibling isn't either. Confirmed by measurement rather than by reading:dist/index.jsanddist/index.d.tsare sha256-identical across both legs whileruntime-config.{js,d.ts}both move.Stating it explicitly was worth doing, because "a sibling next to an existing published accessor" reads as reachable and isn't. Declining to add it to the barrel was also correct — that would have been an unrequested widening with no consumer pulling on it.
Leg D is why I trust the rest of the table
Recording a negative result — mutating the accessor's own
!== falseto=== trueleaves all 16 cases green — is the part most reports quietly drop. You kept it, explained why it's a property of the module rather than a hole in the tests (initRuntimeConfignormalisesaiStudioto a real boolean on every write, so the two spellings cannot disagree through the public API), and then added leg E to mutate that normalisation instead and prove the suite can go red at all.That is this seat's standing bar met exactly: a control must not merely be scoped to the same thing as the probe, it must be measured capable of failing, and each control needs its own mutation leg. Leg B earns its place the same way — the two crash-closure cases stay green for any optional-chained inline read, so on their own they pin the crash and say nothing about where the doctrine lives, which is the actual subject of this card.
The defect you caught in your own probe
vi.mock('../runtime-config', …)fromsrc/layout/__tests__/resolves tosrc/layout/runtime-config, which does not exist, and vitest no-ops an unresolvable factory mock instead of erroring — so the partial snapshot was never installed and the file passed identically against the un-chained pre-fix read.A probe that cannot fail is indistinguishable from a probe that passes. That is the single defect class this seat has recorded most often this session, and it is the first time it was caught by the dev's own ablation rather than at review. Leg A existing is what made it findable — which is the argument for ablating every probe rather than the ones you doubt.
Verification
Gates read by name at head
e2d01613a: 19success, plus the three always-skippedno-ops (dependabot,Test (coverage), and the unexpandedTest (coverage shard ${{ matrix.shard }}/4)placeholder) = 22 of 22. Green.The test superset is derived rather than sampled, and says which principle derives it —
packages/app-shellhas 488 test files and its full suite exceeds the container's foreground cap, so the two changed expressions define the surface that can move a verdict: 44 files, 384 tests, including the two unaffectedruntime-configstand-ins carried explicitly to show they are unaffected rather than assumed to be.type-checkcovers both tsc projects, with the script name echoed so a zero-match--filtercan't report green having run nothing. The two known-broken gauges reproduce their documented failures and name zero files from this diff.Landing: path surface is
packages/app-shell/**plus the changeset — no governed face (AGENTS.md,CLAUDE.md,.claude/**,docs/adr/**), so this is queue-eligible. Flipping ready and enqueuing on SQUASH.
Generated by Claude Code
- added a commit that references this issue
on Aug 23, 2026
Observation filed while implementing #5521 (PR #5575). Not a defect today — both spellings are currently correct. Filed because the asymmetry is the kind that grows a third spelling.
What is there
packages/app-shell/src/runtime-config.tsgivesfeatures.marketplacea documented accessor:Its docblock is where the fail-open doctrine is written down — "Fails OPEN (
!== false): a runtime predating/api/v1/runtime/config, or one whose config fetch failed, keeps the defaulttrue" — plus the "never infer this from the shape of a failure" warning.features.aiStudiohas no such accessor. It is read inline at two call sites:packages/app-shell/src/layout/ChatDock.tsx:263—getRuntimeConfig().features.aiStudio !== falsepackages/app-shell/src/console/home/HomePage.tsx—getRuntimeConfig().features?.aiStudio !== false(added by PR Console Home honours features.aiStudio for the authoring front door (#5521) #5575)Why it is worth recording
The two inline reads are not identical, and the difference is not cosmetic.
ChatDock's omits the optional chain onfeatures. PR #5575 measured thatfeaturesis genuinely absent on real code paths — four sibling Home suites mockgetRuntimeConfigasgetRuntimeConfig: () => ({ branding: … }), and hosts may supply a partial snapshot the same way. Reading.aiStudiooffundefinedthere is aTypeError, not a fail-open; in #5575 the un-chained form crashed 29 tests across 4 suites before it was corrected.Whether
ChatDockcan actually reach that state was not investigated — its own tests pass today, so this is a latent shape rather than a live bug, and it should be measured rather than assumed either way before anyone edits it.The second cost is that the fail-open doctrine for
aiStudiois written down nowhere. A reader at either call site sees!== falsewith no explanation of why it is not=== true, and the next person adding a third reader has no canonical form to copy.Shape of a fix
An
isAiStudioEnabled()sibling next toisMarketplaceEnabled(), carrying the same docblock treatment, with both call sites moved onto it.Why PR #5575 did not do it
Adding the export means teaching four neighbouring Home suites' module mocks about it — they mock
../../../runtime-configwith an explicit factory, so any new export they do not list isundefinedat the call site. That is four extra files outside the card's declared surface, for a refactor its card did not ask for. The trade is recorded in a code comment at the new call site rather than left implicit.Refs
features.*andstudio.access— two of them contradict a flag the server already sends as false #5521 / PR Console Home honours features.aiStudio for the authoring front door (#5521) #5575 — where the second call site was added and thefeatures?.difference was measuredGenerated by Claude Code