feat(glass-office): Breakers & Scheduler as the fourth Basement room - #112
Conversation
The stable breakers room lands the focus route on operations-facing Breakers and Scheduler sections while Queue and System Status stay one tap away. The first FocusPage component parity suite (33 tests) locks every queue category/action, duplicate-click locks, honest failures, recursive detail redaction, Strategy/Runbook links, channel posture, six-item pagination, and empty states; it proved the stale-page shrink/regrow bug RED before the state-correct clamp fix landed. Breakers presents core and plugin facts distinctly from the existing controller seam, separates loaded-empty from not-yet-loaded, and never invents severities or a manual reset. Scheduler shows running/stopped, lock owner/detail, job counts, and stop reasons only from jobsStatus (raw lock paths stay reserved for the File-lock room) and reuses the existing Run and Pause/Resume mutations with synchronous duplicate locks and per-row failure copy. Focus deep links carry a queue intent nonce so Review approval still lands on the Queue. The hidden breakers Office shortcut joins the registry with a ninth-shortcut byte-immutability proof over all eight earlier pinned shortcuts. The p5-breakers e2e drives real gateway ops-state and a forced run failure through a new mock seam, proves lamp identity, pin/door/reload/disabled-restore truth, the visible BS mark, and inner geometry at desktop and 390px with console-error and requestfailed capture; the 390px sub-tab overflow it caught is fixed by wrapping the Focus tab strip. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds Breakers and Scheduler views to Mission Control, wires configurable mock gateway operations state, supports scheduler controls and Office pinning, and adds unit/E2E coverage for routing, pagination, responsive layouts, door restoration, and intentional failures. ChangesBreakers and Scheduler operations
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant FocusPage
participant MockGateway
participant Office
Operator->>FocusPage: Open Breakers or Scheduler
FocusPage->>MockGateway: Request operations status
MockGateway-->>FocusPage: Return breaker and scheduler state
FocusPage-->>Operator: Render facts and controls
Operator->>Office: Pin or reopen breakers room
Office->>FocusPage: Navigate to breakers room
FocusPage-->>Operator: Show Breakers
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
apps/mission-control/src/features/focus/FocusPage.test.tsx (2)
107-110: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDouble cast hides
availabilityshape drift.
as unknown asopts this field out of type checking, so a future change toRunbookSummaryItemResponse["availability"]won't surface here. Construct it to the real type (or extend the type) instead.As per coding guidelines, "Frontend Mission Control: run
npm run typecheckfor TypeScript type validation" — the cast defeats that validation for this fixture.🤖 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/focus/FocusPage.test.tsx` around lines 107 - 110, Update the availability fixture in the test data to conform directly to RunbookSummaryItemResponse["availability"], removing the `as unknown as` double cast. Include all required fields with correctly typed values so `npm run typecheck` can detect future availability shape changes.Source: Coding guidelines
779-791: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThis case never exercises the precedence it claims.
activeRoomIdstays"breakers"across both renders, so FocusPage's room-landing branch (activeRoomId !== lastRoomId) never fires in the second render — only the nonce branch runs. To actually pin the ordering, change the room and the nonce in the same render.♻️ Suggested change
- await render( - baseProps({ activeRoomId: "breakers", queueIntentNonce: 0 }), - ); - expect(subTab("Breakers")?.getAttribute("aria-selected")).toBe("true"); + await render(baseProps({ activeRoomId: null, queueIntentNonce: 0 })); + expect(subTab("Queue")?.getAttribute("aria-selected")).toBe("true"); // A deep link that targets queue content (e.g. Review approval from - // Runbook) must land on the Queue even though the room stays breakers. + // Runbook) must outrank the breakers room landing in the same render. await render( baseProps({ activeRoomId: "breakers", queueIntentNonce: 1 }), );🤖 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/focus/FocusPage.test.tsx` around lines 779 - 791, Update the test case around the queue deep-link precedence to change activeRoomId from "breakers" to a different room while also changing queueIntentNonce in the second render. Keep the assertions verifying that Queue is selected and the queue content is shown, so the test exercises ordering when both the room-landing and nonce branches run.apps/mission-control/e2e/mockGateway.mjs (2)
2871-2884: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value
jobsnow derivesprimary_actionhonestly, butnext_upstill hardcodes"pause".
/api/v1/jobs/:id/updatecan flipjobs[0].enabledtofalse, after which thenext_upentry advertisespausefor a paused job while thejobsentry correctly saysresume. Deriving it the same way keeps the fixture self-consistent.♻️ Suggested alignment (outside the changed range, lines 2855-2869)
- primary_action: "pause", + primary_action: jobs[0].enabled ? "pause" : "resume",🤖 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/mockGateway.mjs` around lines 2871 - 2884, Update the next_up job mapping near the jobs response to derive primary_action from each job’s enabled state, matching the existing jobs mapping: use the pause action when enabled and resume when disabled, rather than hardcoding pause. Keep the jobs mapping unchanged.
5331-5388: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueArrays are stored by reference without element shape validation.
circuit_breakers,plugin_breakers, andtop_stop_reasonsare assigned straight from the request body, so a malformed element surfaces as a confusing UI render rather than a 400 from the seam. Acceptable for an E2E-only fixture (the consumers already coerce withString(...)/truthiness), but a shallow shape filter would make failures point at the spec instead of the 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/mockGateway.mjs` around lines 5331 - 5388, Add shallow element-shape validation in the POST /api/v1/e2e/ops-state handler before assigning circuit_breakers, plugin_breakers, and top_stop_reasons to e2eOpsState. Filter malformed entries while preserving valid ones, and return a 400 response when the payload contains invalid array elements so fixture errors are reported at the request seam.apps/mission-control/src/styles.css (1)
11163-11167: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake
.mc-sched-statsexplicitly flex Adddisplay: flexto both.mc-sched-statsrules so the row/column behavior doesn’t depend on.mc-stat-liststaying flex.🤖 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/styles.css` around lines 11163 - 11167, Add display: flex to both .mc-sched-stats rules, preserving their existing flex-direction, wrapping, and gap declarations so the component explicitly controls its flex layout.apps/mission-control/e2e/p5-breakers-slice.spec.ts (4)
741-747: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAllowed-error accounting is asymmetric.
unexpectedErrorsmust be empty, but the allowed bucket is only bounded above (<= 4), so a regression where a forced-failure run silently stops happening (0 entries) still passes. The comment states the exact expectation — two responses plus two console lines — so assert it exactly, or at minimum add a lower bound.♻️ Tighten the bound
- expect(browserErrors.filter(isAllowed).length).toBeLessThanOrEqual(4); + const allowedCount = browserErrors.filter(isAllowed).length; + expect(allowedCount).toBeGreaterThanOrEqual(2); + expect(allowedCount).toBeLessThanOrEqual(4);🤖 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-breakers-slice.spec.ts` around lines 741 - 747, Update the allowed-error assertion in the browser error validation to require exactly four entries, matching the expected two responses and two console lines, while preserving the existing unexpectedErrors check and isAllowed filtering.
60-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider segmenting this ~690-line test with
test.step.A single test covering identity, breaker facts, scheduler facts, pagination, job controls, pin refusal/success, office door, reload, and 390px geometry makes failures hard to localize and forces the 180s timeout. Wrapping each commented phase in
test.step("...", async () => { ... })keeps the shared browser state while giving named failure attribution in the report.🤖 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-breakers-slice.spec.ts` around lines 60 - 64, Segment the long test in test(...) into named test.step blocks for each commented phase, including identity, breaker/scheduler facts, pagination, job controls, pin flows, office door, reload, and 390px geometry. Keep the shared page and request state across steps, preserving the existing order and assertions while making failures attributable to their phase.
578-583: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
containsXis duplicated inside bothpage.evaluatebodies.The geometry predicate is redefined verbatim at Lines 579-583 and 653-657. Since
evaluateaccepts arguments, you can hoist a single module-level containment helper and pass the selector/config in, or extract one sharedcollectGeometryevaluate wrapper — keeping both blocks in sync manually is the failure mode here.Also applies to: 652-661
🤖 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-breakers-slice.spec.ts` around lines 578 - 583, Extract the duplicated containsX predicate from both page.evaluate bodies into one shared module-level helper, then reuse it through a shared collectGeometry wrapper or by passing it into each evaluate call. Update both geometry collection paths to use the same implementation and preserve the existing containment behavior.
209-212: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueScreenshot paths are CWD-relative.
"../../runtime/qa/p5-breakers-slice/..."resolves against the Playwright process working directory, so these writes silently land elsewhere (or fail) if the suite is invoked from the repo root instead ofapps/mission-control. Prefer anchoring ontest.info().outputPath(...)or a path derived fromimport.meta.url/ a config-defined output dir.#!/bin/bash # Check how other specs and the Playwright config define screenshot/output paths. fd -e ts -e mjs . apps/mission-control/e2e --exec rg -n 'runtime/qa|outputPath|screenshot\(' {} \; fd -g 'playwright.config.*' --exec rg -n 'outputDir|testDir|use' {} \;🤖 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-breakers-slice.spec.ts` around lines 209 - 212, Update the screenshot path in the test containing the breakers-live-desktop screenshot to use Playwright’s test.info().outputPath(...) or the project’s configured output directory instead of a CWD-relative path. Preserve the existing filename and full-page screenshot behavior while ensuring output is stable regardless of the process working directory.apps/mission-control/e2e/core.spec.ts (1)
420-422: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInaccurate copy-pasted comment above the explicit Queue tab click. None of these three flows enter the Breakers & Scheduler room; the real reason for the added click is that the Focus surface no longer guarantees Queue as the initial sub-tab. The clicks themselves are correct.
apps/mission-control/e2e/core.spec.ts#L420-L422: reword the comment in the Strategy flow to state that Queue must be selected explicitly after entering Focus.apps/mission-control/e2e/p3-workflows.spec.ts#L176-L178: apply the same reworded comment in the approvals workflow.apps/mission-control/e2e/runbook.spec.ts#L99-L101: apply the same reworded comment in the runbook entry-points test.🤖 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/core.spec.ts` around lines 420 - 422, Replace the inaccurate comment above the explicit Queue tab click with wording that explains Queue must be selected explicitly after entering Focus. Apply the same comment update at apps/mission-control/e2e/core.spec.ts lines 420-422, apps/mission-control/e2e/p3-workflows.spec.ts lines 176-178, and apps/mission-control/e2e/runbook.spec.ts lines 99-101; leave the click behavior unchanged.
🤖 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 `@docs/plans/2026-07-23-glass-office-claude-resume.md`:
- Around line 293-295: Choose one canonical Breakers room mark, then update
every related implementation checklist entry and E2E browser assertion to use it
consistently, including the references near the visible room mark proof and the
later “BF · Breakers & Scheduler” requirement.
---
Nitpick comments:
In `@apps/mission-control/e2e/core.spec.ts`:
- Around line 420-422: Replace the inaccurate comment above the explicit Queue
tab click with wording that explains Queue must be selected explicitly after
entering Focus. Apply the same comment update at
apps/mission-control/e2e/core.spec.ts lines 420-422,
apps/mission-control/e2e/p3-workflows.spec.ts lines 176-178, and
apps/mission-control/e2e/runbook.spec.ts lines 99-101; leave the click behavior
unchanged.
In `@apps/mission-control/e2e/mockGateway.mjs`:
- Around line 2871-2884: Update the next_up job mapping near the jobs response
to derive primary_action from each job’s enabled state, matching the existing
jobs mapping: use the pause action when enabled and resume when disabled, rather
than hardcoding pause. Keep the jobs mapping unchanged.
- Around line 5331-5388: Add shallow element-shape validation in the POST
/api/v1/e2e/ops-state handler before assigning circuit_breakers,
plugin_breakers, and top_stop_reasons to e2eOpsState. Filter malformed entries
while preserving valid ones, and return a 400 response when the payload contains
invalid array elements so fixture errors are reported at the request seam.
In `@apps/mission-control/e2e/p5-breakers-slice.spec.ts`:
- Around line 741-747: Update the allowed-error assertion in the browser error
validation to require exactly four entries, matching the expected two responses
and two console lines, while preserving the existing unexpectedErrors check and
isAllowed filtering.
- Around line 60-64: Segment the long test in test(...) into named test.step
blocks for each commented phase, including identity, breaker/scheduler facts,
pagination, job controls, pin flows, office door, reload, and 390px geometry.
Keep the shared page and request state across steps, preserving the existing
order and assertions while making failures attributable to their phase.
- Around line 578-583: Extract the duplicated containsX predicate from both
page.evaluate bodies into one shared module-level helper, then reuse it through
a shared collectGeometry wrapper or by passing it into each evaluate call.
Update both geometry collection paths to use the same implementation and
preserve the existing containment behavior.
- Around line 209-212: Update the screenshot path in the test containing the
breakers-live-desktop screenshot to use Playwright’s test.info().outputPath(...)
or the project’s configured output directory instead of a CWD-relative path.
Preserve the existing filename and full-page screenshot behavior while ensuring
output is stable regardless of the process working directory.
In `@apps/mission-control/src/features/focus/FocusPage.test.tsx`:
- Around line 107-110: Update the availability fixture in the test data to
conform directly to RunbookSummaryItemResponse["availability"], removing the `as
unknown as` double cast. Include all required fields with correctly typed values
so `npm run typecheck` can detect future availability shape changes.
- Around line 779-791: Update the test case around the queue deep-link
precedence to change activeRoomId from "breakers" to a different room while also
changing queueIntentNonce in the second render. Keep the assertions verifying
that Queue is selected and the queue content is shown, so the test exercises
ordering when both the room-landing and nonce branches run.
In `@apps/mission-control/src/styles.css`:
- Around line 11163-11167: Add display: flex to both .mc-sched-stats rules,
preserving their existing flex-direction, wrapping, and gap declarations so the
component explicitly controls its flex layout.
🪄 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: d44177a2-ec6f-4d53-b01a-1217abc1583c
📒 Files selected for processing (12)
apps/mission-control/e2e/core.spec.tsapps/mission-control/e2e/mockGateway.mjsapps/mission-control/e2e/p3-workflows.spec.tsapps/mission-control/e2e/p5-breakers-slice.spec.tsapps/mission-control/e2e/runbook.spec.tsapps/mission-control/src/app/AppContent.tsxapps/mission-control/src/features/execassOffice/officeBlocks.tsapps/mission-control/src/features/execassOffice/pinToOffice.test.tsapps/mission-control/src/features/focus/FocusPage.test.tsxapps/mission-control/src/features/focus/FocusPage.tsxapps/mission-control/src/styles.cssdocs/plans/2026-07-23-glass-office-claude-resume.md
1 participant
Summary
Fourth P5 Basement slice: the stable
breakersroom (dedicatedfocusroute) lands on operations-facing Breakers and Scheduler sections on the existing Focus surface, with Queue and System Status one tap away. No second operations controller, no copied Cockpit widget state — everything consumes the existinguseMissionControlControllerseam (jobsStatus, mergedopenBreakers,openPluginBreakers,calendarJobs,runCalendarJobNow,toggleCalendarJob).FocusPagecomponent parity suite (33 tests) locks Queue/System Status before any presentation change: approval approve/deny, failed-job retry, channel reconnect, same-tick duplicate-click locks, honest action failures with retry-clears-error, detail expansion with recursive secret redaction across the whole rendered surface, Strategy task link precedence (task_id → approval → job), Runbook links, channel cards, six-item pagination boundaries, and all empty states.jobsStatusexists. No invented severities, no manual reset button — copy states recovery is automatic with no manual reset.jobsStatusexists. Raw lock paths are deliberately withheld for the later File-lock machinery room. Run and Pause/Resume reuse the existing mutations with synchronous in-flight locks and per-row honest failure copy.queueIntentNoncethat outranks the room landing, so approval deep links still land on the Queue while elevator/door entry lands on Breakers. Unrelated rerenders never reset the operator''s section choice (Connectors landing idiom).breakersOffice shortcut (id/roomIdbothbreakers) plusPinRoomToOffice roomId="breakers"on the ready surface, using only shared primitives. Registry tests prove the ninth shortcut refuses byte-for-byte on a canvas genuinely filled by all eight earlier shortcuts, and joins a freed canvas idempotently.e2e/p5-breakers-slice.spec.ts, @core): a new mock-gatewayPOST /api/v1/e2e/ops-stateseam drives real breaker/plugin/stop-reason payloads, nine real jobs, and a forced 500 on exactly one job run for the honest-error proof (narrow, source-justified capture exception). Proves singleBF · Breakers & Schedulerlamp, landing, loaded-empty truth, live breaker facts, scheduler facts without lock paths, Run/Pause/Resume, page-2 boundary, full-canvas byte immutability, config-only pinning (approvals/jobs/breakers verified unchanged via API), mounted-Office sync, exact door destination, reload persistence, disabled/restored Basement with executed restored door, visible nonzeroBSmark, and inner geometry for tabs, breaker rows, scheduler facts, job controls, the action error, and pagination at desktop and 390px, with console-error/requestfailed/4xx capture..mc-focus-page).Existing e2e anchors that clicked nav-focus and expected the Queue immediately now click the Queue tab first (core, p3-workflows, runbook); the approval/reconnect/retry/runbook behaviors themselves are unchanged.
Validation
npm run typecheckPASSnpm run lintPASSnpm run test:unit -- --runPASS: 71 files / 508 tests (471 → 508: +33 FocusPage suite, +4 pin/registry proofs)npm run buildPASS (pre-existing chunk-size warning only)e2e/p5-breakers-slice.spec.tsPASS (desktop + 390px, console/requestfailed capture)p3-workflows.spec.ts(2/2),runbook.spec.ts(2/2),p4-calendar-slice.spec.ts,p5-events-slice.spec.tspython scripts/validate_execass_contract.pyPASS;contracts/untouchedgit diff --checkPASSruntime/qa/p5-breakers-slice/(local evidence), each visually inspected: breakers-live-desktop, scheduler-desktop-facts, scheduler-desktop-error, breakers-full-canvas-refusal, breakers-pinned-desktop, office-shortcut-block, breakers-disabled-door, breakers-restored-door, breakers-390, scheduler-390-pagination-errorNotes for review
…JSX text renders as raw backslash text in CalendarPage (Running/Working busy spans), BoardsPage loading, StrategyPage filtering label, and CommandPalette placeholder; MailPage/ChatroomsPage/MemoryPage occurrences are inside JS strings and are fine. Also pre-existing: at narrow width the nav badge overlaps the second letter of a badged room mark (any badged room, not specific to this slice).toggleCalendarJobfailure copy surfaces through the controller''s existing global notice (it does not rethrow);runCalendarJobNowrethrows and gets per-row alerts.🤖 Generated with Claude Code
Summary by CodeRabbit