Skip to content

feat(glass): rehome Setup as the eighth Basement room - #116

Merged
EmergentKnowledgeGroup merged 3 commits into
mainfrom
codex/glass-office-p5-setup
Jul 26, 2026
Merged

feat(glass): rehome Setup as the eighth Basement room#116
EmergentKnowledgeGroup merged 3 commits into
mainfrom
codex/glass-office-p5-setup

Conversation

@EmergentKnowledgeGroup

@EmergentKnowledgeGroup EmergentKnowledgeGroup commented Jul 26, 2026

Copy link
Copy Markdown
Owner

Summary

The stable setup room stops landing on the Connectors quick-setup tab and gets the product's promised gateway/token/feature-toggle/onboarding surface — rendered from the exact shared authorities Settings already owns. No duplicate connection controller, runtime config store, feature-control store, or onboarding controller was created.

1. Connection controller locked first (useRuntimeConnectionController)

  • One synchronous action lock guards save / reconnect / clear-token (React busy state is not the mutation lock). A locked duplicate saveConnectionFromInputs rejects without a notice so the onboarding wizard can never mistake a refused duplicate for a successful save; locked duplicate reconnect/clear no-op.
  • Clear-token truth: a failed secure clear keeps tokenConfigured, keeps the live link, and reports Forget token failed — success state is claimed only after the authoritative clear resolves.
  • Authoritative saves now adopt the trimmed URL into the one shared gateway draft. This fixes a real pre-existing hazard: after wizard onboarding the Settings/Setup gateway field was blank, and a blind "Save and connect" persisted an empty connection (reproduced live during e2e development).
  • New mounted hostile suite (25 scenarios): bootstrap identity, desktop bootstrap/startup-error/gateway-terminated, trimmed URL, token reuse, secure-upsert failure with retained typed token, deferred-clear truth, aggregated baseline failure, reconnect truth, same-tick duplicate locks per action, cross-action lock, and lock release after failure.

2. One shared Setup surface (src/features/setup/)

  • SetupControls.tsx: ConnectionControls (gateway URL + history, password-only token entry that never echoes a configured token, save/reconnect/wizard/tour/forget actions, one confirmation with the concrete disconnect consequence) and FeatureControls (kill-switch one-patch coupling preserved). setupPresentation.ts computes identical status labels for both surfaces.
  • AppShell Settings was rewired onto these components with zero behavior change (live-feed focus target, save-closes-modal, URL history, confirm copy all preserved).
  • SetupRoomPage renders the same authority at room density; App builds one setupSurface bundle threaded through AppContent to ConnectorsPage.

3. Room landing + Connectors parity

  • ConnectorsPage early-returns SetupRoomPage for resolved room setup after every hook, so Connectors' internal tab truth survives unrelated rerenders and exact room changes still reland (connectors -> Registry). Connector quick setup, integration status, and access requests remain exactly where they were, under the Connectors room's own Setup tab.
  • Component seam tests updated/extended: distinct surface landing, quick-setup parity, reland matrix, and per-room pin exclusivity (Pin Setup to Office only on the Setup surface).

4. Hidden setup shortcut + thirteenth-shortcut law

  • setup registered as a hidden s room-shortcut block. The refusal fixture genuinely pins all twelve earlier room shortcuts through locks (24/24 cells, precondition asserted as 12 visible s placements), refuses byte-for-byte, admits Setup after freeing one small shortcut, and repeats honestly.

5. Browser proof (e2e/p5-setup-slice.spec.ts, @core)

Desktop and 390px through the stateful mock: exactly one BF · Setup lamp; distinct surface with no connector tab bar; live truth chips; wizard-saved URL present in the shared draft; injected health-500 save failure with exact request/response/console accounting (1/1/1) and no token leakage into localStorage/DOM/text or the failure toast; reconnect and save success; one-confirmation Forget token (cancel changes nothing, sessionStorage really cleared, live link drops to Waiting) plus the existing auto-reoffered wizard dismissed; re-configure requiring a token as warned; Memory-page toggle drives a real lamp availability change and syncs live with Settings in both directions; exactly one onboarding wizard instance; guided-tour entry; config-only pinning proven against recorded operational mutations and unchanged gateway settings/token bytes; full-canvas refusal byte-identical; door open/hide/show/reload/disable/restore to the exact stable room; readable non-overlapped S mark and contained panels/fields/dialogs at 390px; zero unaccounted console/page/request errors.

12 screenshot artifacts under runtime/qa/p5-setup-slice/ (room desktop, quick-setup parity, connection-failed, token-cleared, features, full-canvas refusal, pinned, office shortcut, disabled door, restored door, room 390, door 390) — all 12 enumerated and visually inspected.

Validation

npm run typecheck                  PASS
npm run lint                       PASS (0 problems)
npm run test:unit -- --run         697/697 PASS (77 files; +38 new since handoff's 659)
npm run build                      PASS
npx playwright e2e/p5-setup-slice  PASS (26.9s)
npm run test:e2e:core              44/44 PASS (7.3m local)
generate_execass_contract --check  EXIT 0
scripts/validate_execass_contract  PASS
git diff --check                   PASS

Core-suite honesty note: two earlier full-suite runs each had one different single-spec failure (p4-calendar-slice, then p5-directory-slice) inside the shared onboarding flow; both passed in isolation immediately after and both passed in the final clean 44/44 run. The first flaky run had typecheck/lint/vitest competing for CPU on this machine. Neither spec's surface is touched by this diff.

Scope: frontend-only (16 files, +2485/−319; the bulk is the new mounted controller suite, component tests, and the e2e proof). No Rust, contract, or migration changes. App.tsx/AppContent.tsx were byte-exact-repaired to minimal +45/+4 line insertions to avoid line-ending churn.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added a dedicated Basement Setup room with gateway connection status, token controls, feature switches, setup wizard, and guided tour access.
    • Added support for pinning the Setup room shortcut to Office, including capacity handling and direct navigation.
    • Added responsive Setup layouts for desktop and narrow screens.
  • Bug Fixes
    • Improved connection action handling to prevent duplicate saves, reconnects, or token-clearing operations.
    • Clarified navigation between the Setup and Connectors rooms, preserving connector tabs correctly.
  • Tests
    • Added comprehensive coverage for Setup workflows, persistence, error handling, responsive behavior, and Office shortcuts.

EmergentKnowledgeGroup and others added 2 commits July 25, 2026 23:41
The stable `setup` room lands on the product's distinct gateway/token/
feature-toggle/onboarding surface instead of the Connectors quick-setup
tab, rendered from the exact authorities Settings already owns.

- useRuntimeConnectionController gains one synchronous action lock across
  save/reconnect/clear (a locked duplicate saveConnectionFromInputs
  rejects so the wizard can never mistake a refused duplicate for
  success), honest clear-token failure that keeps the configured truth,
  and authoritative-save adoption into the one shared gateway draft
  (fixes the post-wizard blank-URL field whose blind re-save persisted an
  empty connection). New mounted hostile suite: 25 scenarios.
- SetupControls.tsx hosts the one shared ConnectionControls (confirm-once
  Forget token with its concrete consequence, password-only token entry)
  and FeatureControls (kill-switch one-patch coupling) presentation;
  AppShell Settings consumes it with zero behavior change and
  setupPresentation.ts keeps status labels identical on both surfaces.
- ConnectorsPage early-returns SetupRoomPage for the resolved setup room
  over the setupSurface bundle threaded App -> AppContent; connector
  quick setup stays reachable under the Connectors room and internal tab
  choices still survive unrelated rerenders with exact room relands.
- Hidden `setup` office block registered; the thirteenth-shortcut proof
  pins all twelve earlier room shortcuts through locks (24/24 cells),
  refuses byte-for-byte, admits after freeing one small shortcut, and
  repeats honestly.
- p5-setup e2e proves room identity, token no-leak failure/success paths,
  one-confirmation Forget token, real feature availability change with
  live Settings sync both directions, a single onboarding wizard
  instance, config-only pinning, door/reload/disable/restore truth, and
  390px S-mark/geometry with exact injected-failure accounting.

Validation: typecheck PASS; lint PASS; unit 697/697 PASS; build PASS;
core e2e 44/44 PASS; ExecAss generated contract --check and independent
validator PASS; git diff --check PASS.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 33 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2ecf502a-7ed5-427b-8ff2-98f55c7b43fe

📥 Commits

Reviewing files that changed from the base of the PR and between 3cb87c9 and b5e644f.

📒 Files selected for processing (17)
  • apps/mission-control/e2e/p5-connectors-slice.spec.ts
  • apps/mission-control/e2e/p5-directory-slice.spec.ts
  • apps/mission-control/e2e/p5-setup-slice.spec.ts
  • apps/mission-control/src/App.tsx
  • apps/mission-control/src/app/AppShell.tsx
  • apps/mission-control/src/app/useKeyboardShortcuts.ts
  • apps/mission-control/src/app/useResolvedElevator.test.tsx
  • apps/mission-control/src/app/useResolvedElevator.ts
  • apps/mission-control/src/app/useRuntimeConnectionController.test.tsx
  • apps/mission-control/src/app/useRuntimeConnectionController.ts
  • apps/mission-control/src/features/connectors/ConnectorsPage.test.tsx
  • apps/mission-control/src/features/connectors/ConnectorsPage.tsx
  • apps/mission-control/src/features/setup/SetupControls.test.tsx
  • apps/mission-control/src/features/setup/SetupControls.tsx
  • apps/mission-control/src/features/setup/SetupRoomPage.tsx
  • apps/mission-control/src/features/setup/setupPresentation.test.ts
  • apps/mission-control/src/features/setup/setupPresentation.ts
📝 Walkthrough

Walkthrough

Adds a dedicated Basement Setup room with shared connection and feature controls, serialized connection actions, Setup-to-Office pinning, Connectors/Setup routing separation, responsive styling, unit tests, and comprehensive desktop/mobile E2E coverage.

Changes

Basement Setup room

Layer / File(s) Summary
Shared setup authority and connection safety
apps/mission-control/src/app/useRuntimeConnectionController.*, apps/mission-control/src/features/setup/*
Shared setup contracts and controls now manage connection drafts, token privacy, feature toggles, status presentation, and serialized save/reconnect/clear actions with focused tests.
Setup surface routing and Settings integration
apps/mission-control/src/App.tsx, apps/mission-control/src/app/*, apps/mission-control/src/features/connectors/*, apps/mission-control/src/features/setup/SetupRoomPage.tsx, apps/mission-control/src/styles.css, docs/plans/*
The shared setup surface is passed into ConnectorsPage, which renders SetupRoomPage for the setup room; Settings reuses the shared controls and the Connectors tests cover room/tab and pin identity behavior.
Setup Office shortcut and pinning
apps/mission-control/src/features/execassOffice/*
A hidden setup room shortcut is registered, with tests for capacity refusal, successful pinning, visibility, and idempotent repeats.
Desktop and mobile Setup room proof
apps/mission-control/e2e/*
E2E coverage validates routing, connection and token behavior, feature synchronization, onboarding, Office pinning, door navigation, reload persistence, failure instrumentation, and 390px layout containment.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant App
  participant ConnectorsPage
  participant SetupRoomPage
  participant SetupControls
  participant Office
  App->>ConnectorsPage: pass shared setup surface
  ConnectorsPage->>SetupRoomPage: render setup room
  SetupRoomPage->>SetupControls: render connection and feature controls
  SetupRoomPage->>Office: pin setup shortcut
  Office-->>SetupRoomPage: navigate through setup shortcut
Loading

Possibly related PRs

Suggested reviewers: professahx

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.12% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: rehoming Setup as the eighth Basement room.
Description check ✅ Passed The description is detailed and covers summary, validation, and visual evidence, though it omits the template's checklist sections.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/glass-office-p5-setup

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
apps/mission-control/src/features/connectors/ConnectorsPage.tsx (1)

489-526: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Setup room is blocked by Connectors-controller early returns.

if (activeRoomId === "setup") return <SetupRoomPage .../> (lines 613-619) runs after the four early returns for controller.enabled/availability (lines 489-526). Since the first matching return wins, any time Connectors is disabled, its availability is "unsupported"/"error", or it's still "loading" (very common on cold start before the connectors registry has fetched), a user on the Setup room sees a Connectors-specific error/disabled/loading panel instead of SetupRoomPage — even though Setup is meant to be an independent, always-usable surface (per this file's and SetupControls.tsx's own design comments).

🐛 Proposed fix — check the Setup room before the Connectors-availability gates
+  // The stable Setup room owns a distinct product surface over the shared
+  // Setup authority and must render regardless of the Connectors
+  // controller's enabled/availability state.
+  if (activeRoomId === "setup") {
+    return <SetupRoomPage surface={setupSurface} />;
+  }
+
   if (!controller.enabled || controller.availability === "disabled") {
     return (
       <ConnectorsStatePanel
         title="Connectors are disabled"
         detail="Enable Connectors in Config > Reliability + Rollout to expose connector intake, review, and auth controls."
       />
     );
   }
@@
   if (controller.availability === "loading" && controller.installedConnectors.length === 0) {
     return (
       <ConnectorsStatePanel
         title="Loading Connectors"
         detail="Resolving catalog intake, installed registry state, and paused connector interactions."
       />
     );
   }
@@
-  // The stable Setup room owns a distinct product surface over the shared
-  // Setup authority. Rendering it here — after every hook — keeps this
-  // component mounted across room switches so Connectors' internal tab
-  // truth survives unrelated rerenders and exact room changes reland.
-  if (activeRoomId === "setup") {
-    return <SetupRoomPage surface={setupSurface} />;
-  }
-
   return (

Also applies to: 613-619

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mission-control/src/features/connectors/ConnectorsPage.tsx` around lines
489 - 526, Move the activeRoomId === "setup" branch in the Connectors page ahead
of the controller.enabled and availability early-return gates, returning
SetupRoomPage immediately for the setup room. Preserve the existing
ConnectorsStatePanel handling for all non-setup rooms.
apps/mission-control/src/App.tsx (1)

423-442: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Decouple Setup from the Connectors toggle In apps/mission-control/src/App.tsx:439-442 and the connectors redirect below, Setup is still hidden because it shares route: "connectors" in apps/mission-control/src/glass/floors.ts:111. With connectorsHubEnabled off, the floor filter drops Setup and the fallback sends it back to Boards, making the Setup room unreachable.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mission-control/src/App.tsx` around lines 423 - 442, The available-tab
and redirect logic currently couples Setup visibility to the connectors toggle;
update the relevant tab configuration and connectors redirect in App.tsx, along
with the matching floor definition in floors.ts, so Setup uses its own route or
remains available independently when connectorsHubEnabled is false. Preserve
connectors gating while ensuring Setup does not fall back to Boards.
🧹 Nitpick comments (7)
apps/mission-control/e2e/p5-connectors-slice.spec.ts (1)

108-108: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Scope the Catalog click to the tab bar like the Setup click above.

Lines 99-102 had to scope "Setup" to .mc-connectors-tab-bar to avoid colliding with the new BF · Setup room button. Applying the same scoping to Catalog keeps the tab interactions uniform and immune to a future name collision.

♻️ Suggested change
-  await page.getByRole("button", { name: /^Catalog/ }).click();
+  await page
+    .locator(".mc-connectors-tab-bar")
+    .getByRole("button", { name: /^Catalog/ })
+    .click();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mission-control/e2e/p5-connectors-slice.spec.ts` at line 108, Update the
Catalog button lookup in the connector test to scope it to the
.mc-connectors-tab-bar container, matching the existing scoped Setup interaction
and preserving the intended tab click.
apps/mission-control/e2e/p5-setup-slice.spec.ts (6)

319-324: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Assert the parsed field rather than the exact serialized string.

Exact equality against JSON.stringify({ gateway_url: ... }) breaks whenever the settings object gains a key or changes property order, even though the behavior under test (trim + trailing-slash normalization) is unchanged.

♻️ Suggested change
-  expect(
-    await page.evaluate(
-      (key) => localStorage.getItem(key),
-      GATEWAY_SETTINGS_KEY,
-    ),
-  ).toBe(JSON.stringify({ gateway_url: `${GATEWAY_URL}/` }));
+  expect(
+    JSON.parse(
+      (await page.evaluate(
+        (key) => localStorage.getItem(key),
+        GATEWAY_SETTINGS_KEY,
+      )) ?? "{}",
+    ),
+  ).toMatchObject({ gateway_url: `${GATEWAY_URL}/` });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mission-control/e2e/p5-setup-slice.spec.ts` around lines 319 - 324,
Update the localStorage assertion in the gateway settings test to parse the
value returned by localStorage.getItem using JSON.parse, then assert the
gateway_url field equals the normalized `${GATEWAY_URL}/` value. Keep the test
focused on trailing-slash normalization without comparing the entire serialized
object.

719-724: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

toBeGreaterThan(12) couples the containment check to the current element count.

Any future panel/field/button added or removed in the Setup room flips this without a real regression. Asserting a nonzero sample (toBeGreaterThan(0)) plus the existing .not.toContain(false) keeps the containment guarantee without the brittle magic number.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mission-control/e2e/p5-setup-slice.spec.ts` around lines 719 - 724,
Update the containment assertion in the roomGeometry check to require only a
nonzero contained sample by replacing the fixed count threshold with a
greater-than-zero check. Keep the existing roomGeometry.ok guard and
contained.not.toContain(false) assertion unchanged.

220-222: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reuse the existing helpers/locators instead of re-deriving them.

Line 220 re-derives setup-room-page when the setupRoom(page) helper is right there, and urlInputEarly (Line 250) / urlInput (Line 311) are byte-identical locators under two names. Collapsing these keeps the spec's single source of truth for each element.

Also applies to: 250-251, 311-312

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mission-control/e2e/p5-setup-slice.spec.ts` around lines 220 - 222,
Reuse the existing setupRoom(page) helper or its established locator instead of
calling getByTestId("setup-room-page") directly for the Quick Setup assertion.
Consolidate the byte-identical urlInputEarly and urlInput locators into one
shared locator, then update both usages to reference that single source of
truth.

62-68: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Await the Settings modal before probing checkbox visibility.

isVisible() is an immediate, non-retrying check. If the modal hasn't painted yet, it returns false and the helper clicks the section header — which collapses the section when it was already expanded, then check() times out. Same pattern repeats at Lines 400-405.

♻️ Suggested hardening
   await page.locator('[data-tour-id="nav-config"]').click();
+  await expect(settingsModal(page)).toBeVisible();
   const checkbox = settingsModal(page).getByRole("checkbox", {
     name: "Connectors page",
   });
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mission-control/e2e/p5-setup-slice.spec.ts` around lines 62 - 68, Wait
for the Settings modal to be fully visible before checking the checkbox in the
setup flow around settingsModal(page), replacing the immediate
checkbox.isVisible() probe with a retrying visibility wait that preserves the
expanded-section state. Apply the same change to the repeated section around the
later configuration flow.

450-486: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Line 486 is tautological — it re-counts the array the test just built.

visibleEarlierShortcuts is computed from config.layout, which the same page.evaluate assigned two statements earlier from a hard-coded 12-element list. It can only ever be 12, so it doesn't "prove the capacity precondition" the comment at Line 434-437 claims. Reading the capacity back from the rendered Office canvas (e.g. counting visible office-block-* elements) would make the precondition real.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mission-control/e2e/p5-setup-slice.spec.ts` around lines 450 - 486, The
capacity assertion in the fullCanvas setup is tautological because
visibleEarlierShortcuts is counted from the hard-coded config.layout assignment.
Replace that returned count and its assertion with a measurement of the rendered
Office canvas, such as counting visible office-block-* elements after the
configuration-change event, so the test verifies the UI actually renders all 12
shortcuts.

112-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff

Bracket the ~670-line flow with test.step for diagnosability.

Twelve distinct behaviors (identity, connection failure, token clear, feature sync, wizard/tour, pin, door, reload, 390px) share one sequential test body. The shared state makes splitting into independent tests impractical, but test.step("...") wrappers give named phases in the trace/report so a failure points at the phase rather than a raw line number — no state changes required.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/mission-control/e2e/p5-setup-slice.spec.ts` around lines 112 - 115, Wrap
the sequential flow in the setup-room identity test with named test.step blocks
for each distinct behavior phase, including identity, connection failure, token
clearing, feature-switch sync, wizard/tour, pin-to-office, door hold, reload,
and 390px viewport coverage. Keep the existing order, shared state, assertions,
and test timeout unchanged; only add descriptive step boundaries for trace and
report diagnosability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@apps/mission-control/e2e/p5-setup-slice.spec.ts`:
- Around line 265-297: Keep expectInjectedHealthFailure enabled after
page.unroute until the injected health failure’s asynchronous page.on("console")
and page.on("response") handlers have each recorded one event. Wait for both
injected counts to reach 1, then clear the gate before the final error-budget
assertions.

In `@apps/mission-control/src/features/setup/SetupControls.tsx`:
- Around line 39-48: Wrap the localStorage.setItem call in pushGatewayUrlHistory
with try/catch, matching the protection used by getGatewayUrlHistory and
closeGuidedTour. Swallow storage failures so handleSaveAndConnect can continue
to invoke props.onSaveConnection() when gateway URL history cannot be persisted.

In `@apps/mission-control/src/features/setup/SetupRoomPage.tsx`:
- Around line 22-35: Normalize the gateway health state emitted by the runtime
controller from checking/up/down to the values expected by
connectionStatusPresentation before calling it in SetupRoomPage. Update the
relevant surface-state construction or adapter, preserving the presenter’s
existing labels so the page does not render raw values such as “Gateway: up”.

---

Outside diff comments:
In `@apps/mission-control/src/App.tsx`:
- Around line 423-442: The available-tab and redirect logic currently couples
Setup visibility to the connectors toggle; update the relevant tab configuration
and connectors redirect in App.tsx, along with the matching floor definition in
floors.ts, so Setup uses its own route or remains available independently when
connectorsHubEnabled is false. Preserve connectors gating while ensuring Setup
does not fall back to Boards.

In `@apps/mission-control/src/features/connectors/ConnectorsPage.tsx`:
- Around line 489-526: Move the activeRoomId === "setup" branch in the
Connectors page ahead of the controller.enabled and availability early-return
gates, returning SetupRoomPage immediately for the setup room. Preserve the
existing ConnectorsStatePanel handling for all non-setup rooms.

---

Nitpick comments:
In `@apps/mission-control/e2e/p5-connectors-slice.spec.ts`:
- Line 108: Update the Catalog button lookup in the connector test to scope it
to the .mc-connectors-tab-bar container, matching the existing scoped Setup
interaction and preserving the intended tab click.

In `@apps/mission-control/e2e/p5-setup-slice.spec.ts`:
- Around line 319-324: Update the localStorage assertion in the gateway settings
test to parse the value returned by localStorage.getItem using JSON.parse, then
assert the gateway_url field equals the normalized `${GATEWAY_URL}/` value. Keep
the test focused on trailing-slash normalization without comparing the entire
serialized object.
- Around line 719-724: Update the containment assertion in the roomGeometry
check to require only a nonzero contained sample by replacing the fixed count
threshold with a greater-than-zero check. Keep the existing roomGeometry.ok
guard and contained.not.toContain(false) assertion unchanged.
- Around line 220-222: Reuse the existing setupRoom(page) helper or its
established locator instead of calling getByTestId("setup-room-page") directly
for the Quick Setup assertion. Consolidate the byte-identical urlInputEarly and
urlInput locators into one shared locator, then update both usages to reference
that single source of truth.
- Around line 62-68: Wait for the Settings modal to be fully visible before
checking the checkbox in the setup flow around settingsModal(page), replacing
the immediate checkbox.isVisible() probe with a retrying visibility wait that
preserves the expanded-section state. Apply the same change to the repeated
section around the later configuration flow.
- Around line 450-486: The capacity assertion in the fullCanvas setup is
tautological because visibleEarlierShortcuts is counted from the hard-coded
config.layout assignment. Replace that returned count and its assertion with a
measurement of the rendered Office canvas, such as counting visible
office-block-* elements after the configuration-change event, so the test
verifies the UI actually renders all 12 shortcuts.
- Around line 112-115: Wrap the sequential flow in the setup-room identity test
with named test.step blocks for each distinct behavior phase, including
identity, connection failure, token clearing, feature-switch sync, wizard/tour,
pin-to-office, door hold, reload, and 390px viewport coverage. Keep the existing
order, shared state, assertions, and test timeout unchanged; only add
descriptive step boundaries for trace and report diagnosability.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: a98cbc39-d5b3-4621-a554-43f17f783956

📥 Commits

Reviewing files that changed from the base of the PR and between 3b4d594 and 3cb87c9.

📒 Files selected for processing (17)
  • apps/mission-control/e2e/p5-connectors-slice.spec.ts
  • apps/mission-control/e2e/p5-setup-slice.spec.ts
  • apps/mission-control/src/App.tsx
  • apps/mission-control/src/app/AppContent.tsx
  • apps/mission-control/src/app/AppShell.tsx
  • apps/mission-control/src/app/useRuntimeConnectionController.test.tsx
  • apps/mission-control/src/app/useRuntimeConnectionController.ts
  • apps/mission-control/src/features/connectors/ConnectorsPage.test.tsx
  • apps/mission-control/src/features/connectors/ConnectorsPage.tsx
  • apps/mission-control/src/features/execassOffice/officeBlocks.ts
  • apps/mission-control/src/features/execassOffice/pinToOffice.test.ts
  • apps/mission-control/src/features/setup/SetupControls.test.tsx
  • apps/mission-control/src/features/setup/SetupControls.tsx
  • apps/mission-control/src/features/setup/SetupRoomPage.tsx
  • apps/mission-control/src/features/setup/setupPresentation.ts
  • apps/mission-control/src/styles.css
  • docs/plans/2026-07-23-glass-office-claude-resume.md

Comment thread apps/mission-control/e2e/p5-setup-slice.spec.ts
Comment thread apps/mission-control/src/features/setup/SetupControls.tsx
Comment thread apps/mission-control/src/features/setup/SetupRoomPage.tsx
@EmergentKnowledgeGroup

Copy link
Copy Markdown
Owner Author

Hostile QA correction pushed in b5e644f.

Resolved all five major findings:

  • Setup stays independently available when Connectors/global optional pages are off, and disabling Connectors moves its active room to Setup instead of Boards.
  • Setup renders before every Connectors disabled/unsupported/error/loading gate.
  • URL-history storage failure can no longer abort the real save.
  • Gateway runtime states are normalized centrally for Settings and Setup.
  • The injected 500 gate now waits for exact request/response/console settlement.

Additional source-proven fixes:

  • generation guards prevent late mount-time token/bootstrap promises from overwriting newer owner edits/actions;
  • failed baseline validation preserves the typed token until a complete successful save;
  • token truth-check settlement remains deterministic after save/clear;
  • the pre-existing Directory browser anchor now locates named receipts instead of whichever toast renders first.

Review nits addressed where they strengthen proof: scoped Catalog lookup, parsed gateway setting, deterministic Settings disclosure waits, shared locators, rendered 12-block/24-cell Office precondition, and named critical mobile geometry counts. I intentionally kept the sequential stateful scenario as one test without a large test.step indentation-only rewrite; trace line/phase comments remain specific, and changing 700+ lines for reporting-only structure would add churn without behavioral proof.

Local proof: typecheck/lint/build PASS; unit 712/712; both ExecAss contract checks PASS with contracts untouched; Setup+Connectors focused 2/2; full core 43/44 with only the now-corrected weak Directory toast selector, followed by Directory 1/1 PASS; 14 Setup artifacts regenerated and inspected at desktop/390px.

@EmergentKnowledgeGroup
EmergentKnowledgeGroup merged commit a49de31 into main Jul 26, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant