Conversation
Audit finding A1 (spec 2026-08-08-cockpit-audit-remediation): light #a3a3a3 measured 2.52:1 on white at 11-12px across ~301 text-soft-foreground usages plus the global ::placeholder; dark #7b7b7b measured 4.08:1 on --card. New values measured live: light #6f6f6f = 5.02:1 on white / 4.69:1 on --muted; dark #848484 = 5.20:1 on the page / 4.79:1 on --card.
Audit finding B3 (WCAG 2.4.1, level A): every page started its tab order in the sidebar's 7+ stops. The link is sr-only until focus-visible, and focuses #main explicitly because the SPA layer swallows same-page fragment navigation (verified with a real keyboard).
Audit finding B5 (WCAG 1.3.1 / 3.3.2, axe label critical): task-list checkboxes rendered with no accessible name. A rehype pass labels each checkbox from its own item text at the hast level, composing onto Streamdown's default rehype plugins so every default renderer is untouched. Also records B4 as withdrawn (harness false positive; Radix Escape-to-close verified by test).
Audit finding X1: the interactive field/control border was #ebebeb — 1.19:1 on white, so the Search box had almost no boundary. Bumped to #949494 (3.03:1 measured live), kept distinct from the decorative --border. Also darkens the switch's unchecked track, itself a non-text-contrast win.
Audit finding A4 (WCAG 1.1.1): the run-persisted image alt was image.name
("screenshot-1.png") — a filename is not a description. Alt now describes the
image; the filename moves to a hover title for sighted users.
Audit finding B1 (WCAG 2.1.1, axe scrollable-region-focusable): Streamdown's overflow wrappers for wide tables and code blocks, and the workflow yaml <pre>, scrolled with a mouse but were unreachable by keyboard. tagScrollableRegions tags overflowing wrappers as focusable named groups after each render (they are Streamdown-internal DOM, so tagging beats overriding renderers); the yaml pre gets the attributes inline. Verified live: axe no longer reports the rule.
Audit finding A2 (Nielsen visibility of system status): a done or cancelled run kept an amber (running-colored) progress bar at 100%, and a plan snapshot frozen with an in-progress entry pulsed and read in the present tense forever. railBarTone settles the step bar from the steps alone (amber only while a step is active; success when the workflow ran to the end; danger on cancel/fail), and PlanDock takes a 'settled' flag (the existing runIsTerminal) that drops the present-tense current line and stops the in-progress row pulsing and tagging. Verified live: a done run now shows a green step bar and a plain 'Plan · 2/4'.
The image primitive test pinned the old behavior (filename as alt). It now asserts the descriptive alt and the filename-as-title from the A4 fix.
…mercato#765) The desktop header action row had grown to six buttons beside four tabs (a 'Mark unread' open-mercato#776 landed while this branch was open). Keep the state's primary actions inline — Finish / Continue / Open in… — and fold Notes, Mark unread, Archive and the destructive pair into one 'More actions' overflow menu. The mobile kebab already holds the full set, so this is desktop-only. Closes the audit's header-density finding (open-mercato#765, round-4 D4).
…erences
Header polish following the audit's density work and the house 'no middot
separators' rule: the run tabs (Session/Changes/Commits/Files) get Lucide
icons; the meta row renders workflow, branch, reference and the diff stat as
adjacent chips spaced by the row gap instead of dot-joined text (the +/-
colours stay inside the diff chip so it no longer floats past the row); a
github.com reference chip carries the GitHub mark; the step rail shows the step
kind as a subtle tag ('AGENT'/'CHECK'); and the header gets roomier top/bottom
padding (pt-5 pb-2).
Replace the scattered header metadata (plan mirror, status pill, loose cost/agent dot-list) with one aligned, hairline-separated stat strip — Plan, Status, Cost, Agent, Mode — each a small-caps label over its value, matching the mockup. The Agent tile shows the runner's mark (a Claude burst for claude, a neutral bot otherwise) and name, and stays a button that opens the runner/account/model breakdown so the per-run account (open-mercato#416, agent-profiles) keeps its only home. Adds ClaudeIcon to components/icons since lucide ships no brand marks. Queue position now reads inside the Status tile ('queued open-mercato#2').
The identity chips (workflow, branch, reference, diff) now share one 24px height so the row reads as a set, not a jumble. The header breathes: more top/ bottom padding (pt-6 pb-4), more room under the title (meta mt-3.5) and above the tabs (mt-5), and wider stat tiles (px-6, gap-2) so labels and dividers are not crammed.
The collapsed head read 'Plan · N/M — {current}'; both the middot and the
em-dash are the separator grammar the house rule forbids. The count now sits by
the row gap and the current item behind a hairline divider (house rule).
The dim lifecycle lines led with a '· ' bullet and failures with a '✗ ' glyph — both the middot the house rule forbids and a glyph standing in for an icon. A dim line is marker enough on its own; a failure now carries a real CircleX icon. Realigns the run-header integration tests to the stat strip (status/plan now read from their tiles) and the overflow (Mark unread opens from 'More actions').
The task view forced everything — prose, tables, code, diffs — into one 820px column, so code and tables cramped while half the screen sat empty. Widen the shared column (--measure 1040, wide 1320) and cap flowing TEXT at a new --prose measure (720/760): paragraphs, lists, headings and note lines stay a readable line length while tables, code blocks, diffs and tool cards fill the column. Measured live: prose 720px, table 940px, code 960px in a 960px column.
…omposer The step rail lived permanently in the sticky header and the plan dock sat permanently open above the composer — two always-on blocks eating vertical room even collapsed. Consolidate both into one thin ThreadContextBar right above the input: a 'Verify N/M' chip and a 'Plan N/M' chip, each collapsed by default and expanding UPWARD into a popover on click (overlay — it never pushes the thread or composer). Progress still reads at a glance from the collapsed counts. The step rail leaves the header (shorter sticky header); WorkflowSteps and the PlanDock wrapper retire, their content reused as StepRail and the new PlanList.
… top-right Per the design pass: the Verify and Plan chips become folder-style TABS glued to the composer's top-right edge — rounded top, open bottom over the composer's own border, the same bg-card surface, overlapping each other, each led by an icon (a check for Verify, a todo list for Plan). Right-aligned; the popover still expands upward on click. Icons replace the status dots/current-item text for a compact tab.
- Fit the Verify/Plan popovers to content (w-max + p-3) and drop the step rail's ml-auto so the panel stops manufacturing empty width. - Give each tab a status-colored top edge (Verify = workflow tone, Plan = accent) with smaller uppercase labels, so the two read apart.
- The Verify/Plan chips now sit below the composer, detached (a gap above), separated (no overlap) and icon-less — a status-colored top edge is all that distinguishes them. - Fold the thread's font sizes down to four steps (10/12/13/15px): drop the half-pixel one-offs (10.5/11.5/13.5) and near-duplicates (11px, 14px) onto the nearest canonical size.
The textarea was text-base md:text-sm — 16px on mobile, 14px on desktop — so placeholder, typed text and the desktop/mobile views all read at different sizes. Pin it to a single 15px (placeholder inherits it) and fold the dictation transcript onto the 13px body step.
… sizes - The Verify/Plan chips sit at the composer's top-right again, but now slide UNDER its top edge (rounded-t, open bottom, -mb overlap) instead of fusing with it as folder tabs — the input's opaque edge covers their lower half. Still separated, no overlap between them. - User-message bubbles were text-[13px] against the assistant's 15px — bump them (and the inline editor) to 15px/1.65 so both sides read the same size.
- Light-mode --background moves off pure white to #f7f7fb so the white cards and composer read as raised surfaces against the page. - Give the Verify/Plan chips a touch more visible height (pt-1 -> pt-2).
shadow-modal is a 48px-blur, zero-spread modal shadow — a wide halo under a small pill, and it read as a detached smudge now that the pill shared the page's new tint. Switch to shadow-md (tight, negative-spread lift) and bg-card so the white pill sits crisply above #f7f7fb.
…n under the composer - PlanIcon dropped its hand-drawn SVGs for the same lucide set the workflow rail uses (CircleCheck/LoaderCircle/Circle/CircleSlash), so a completed plan item and a done step now share one check glyph. - The 'take over interactively: cd … && claude --resume …' line is now a compact 'Take over in terminal' button mounted under the composer; it copies the command on click instead of printing the raw path.
- Plan and Status tiles are gone: Plan already lives in the context tab, and Status only echoed the paused/attention hint by the composer. - Cost/Tokens/Agent/Mode moved to a compact meta row (RunMetaFooter) UNDER the composer; the Agent stat keeps its runner/account/model menu. - The attention hint now sits at the tabs' height, left of the Verify/ Plan chips. Tests repointed to the new homes.
…columns - The runner/model picker pills move OUT of the message field into the meta row under it (right side, beside Cost); the pills are stateful in the follow-up hook, so they drive the same continue request wherever they render. Read-only Agent/Mode still shows when a run can't continue. - The step rail is now a 4-track subgrid (icon · name · kind · position) so the kind tags and 'step N of M' line up in columns across rows.
- Finish/Continue/Open in…/overflow move up to the title row (right, at the h1's height); the tabs row below carries only the tabs. Mobile still folds the actions into the kebab. - The runner picker menu drops the backend-description subtitle — each row is just the agent name (claude/codex/opencode).
…ooter stat The step rail and plan live behind their context chips, the status pill and visible title are gone (crumb + sr-only h1), actions split into the floating CTA and the More menu, the branch+diff ride the one git tab, and the run-meta footer now names the workflow — its one home on the thread. The mobile tab row wraps instead of overflowing 390px.
…ditor Desktop navigation goes through the bar's icon tabs, the aggregate ± and the branch read off the git tab, the toolbar leads with its local lenses, chip links no longer count as mobile tabs, and idle-socket fetches retry once.
…llows paging Real bug: the pill's raw smooth scrollTo fought the anchor preservation (which re-applied the parked offset every frame), so the click read as dead; it now pins through the machinery's own setOffset with the history reset in parallel. The spec measures flat on the tail page, grows the retained window through the boundary button (waiting out its one-frame guard), and parks mid-thread for the pill.
…team skills Fixture serves isolate the operator's user-level skill catalog (a developer machine leaked dozens of personal skills into every fixture); the /new spec accepts the auto-loaded team collection inside Project skills and asserts the cold quick-task default.
…ent, DOM-clicks step controls
… appearance attrs, four agents, and seeds a monitoring baseline
… through React's input
…quiet zero diffstats - The primary CTA no longer floats over the git tabs: the Changes/Commits toolbars reserve a real flex slot in their middle and dock RunPrimaryCta there, so it can never overlap Unified/Split again. Session and Files keep the centered float. A task-changes e2e asserts the no-overlap invariant. - The bar stops resurrecting the boot repo's name on workspace-level views: ProjectBar takes projectName verbatim (the container's null is deliberate), so Skills and Workspace settings read as the workspace's, per the container's own contract comment. - An all-zero diff stat renders in the quiet tone instead of success/danger — a clean tree no longer lights change colors on five surfaces. - The composer's Start | Plan first segment becomes ONE Plan-first switch: the verb-labelled segment read as the submit button next to the send arrow. - Table headers uppercase uniformly (the transform restated on the fold buttons), the tokens column is 'In / out', the fold tooltip drops its middot. - Document titles use slash + parens (house separator rules) instead of em dash + middot. - The mobile top bar titles itself from the crumb, so a task thread says the task, not 'Tasks'; the Close-session menu row gets a leave icon instead of a checkmark; fixtureServeEnv satisfies the cez-home guardian rule again.
Hover-reveal hid the settings gear and the + on a fresh start, where nothing hinted the row had actions at all — the two icons now sit on the row permanently; the active row's bold weight still carries the emphasis.
The headline and the three run-mode notes join the empty states' voice (no em-dash separators, per the house rule), and the suggestion chips get an opaque background so the grid backdrop no longer shows through them.
|
🤖 |
pat-lewczuk
left a comment
pat-lewczuk
left a comment
There was a problem hiding this comment.
🔍 Code Review: Cockpit audit remediation: bar-borne task header, projects-tree sidebar, slim composer
🎯 Summary
This PR carries the cockpit through a full UX pass: WCAG remediation from the phase-1 audit spec, a task header that moves onto the app bar, a projects-tree sidebar replacing the flat quick-list, a slimmer composer, pixel-art empty states, a new brand mark, and a port of the whole e2e suite to the new information architecture. It is 205 commits over roughly 180 files and ~9,000 changed lines, of which 75 are test files — the test discipline is genuinely good, and it shows: the review found no any, no alert(, no console.log, no dangerouslySetInnerHTML, no unvalidated request body, and no weakening of a security or degradation path.
The contract work is done correctly. runIndexEntrySchema gains groupId/variant and skillSchema gains category as optional fields, and the server populates them additively (packages/cezar/src/server/server.ts:5351), so old runs.json files and older clients are unaffected — exactly the convention BACKWARD_COMPATIBILITY.md §3 asks for. The five new static asset routes are additive and /open-mercato.svg keeps answering for tabs that cached the old icon. The accessibility work is real rather than cosmetic: rehypeLabelTaskCheckboxes names GFM checkboxes at the hast level instead of overriding Streamdown's renderers, tagScrollableRegions observes only childList/subtree so its attribute writes cannot re-trigger it, and RunStatusAnnouncer deliberately stays silent on the first observation so opening a finished run does not proclaim "run finished" out of nowhere. Those are careful implementations.
What holds it back is not the new code but what the redesign left behind. The sidebar was rebuilt around ProjectsSection, but the props the old sidebar was driven by (taskQuickList, projectGroups, multiProject) are still declared and threaded through AppShell while AppShellContainer no longer passes any of them. The result is a slot that renders nothing, a divider above nothing, and an "All tasks" link that exists in the code and in the test suite but can never render in the running app. The head is also conflicted against main, so the diff reviewed here is not the diff that would merge.
Reviewed scope: the full PR diff. The contract, server and web src/ changes were read line by line; the e2e port was reviewed at the level of what it asserts, not statement by statement.
Verdict
❌ request changes — the head conflicts with origin/main in seven files (Blocker 1), and the sidebar redesign left three dead wiring paths that produce a visible artifact and an unreachable navigation entry (Majors 2–4). None of this is deep: the conflicts are a rebase, and the dead wiring is a delete-or-connect decision. The validation gate is green and the rest of the change is solid.
🧪 Validation Gate
Run at 48246475 inside an isolated worktree, npm ci from the committed lockfile.
| Command | Status | Notes |
|---|---|---|
npm run typecheck |
✅ PASS | Clean across contract, api-client, server and web. |
npm test |
✅ PASS | 6186/6193 passed. The 7 reported failures are reviewer-environment artifacts, not this PR: six (git-worktree.test.ts, server/git.test.ts, server/git-changes.test.ts, server/health-forge.test.ts, server/automations-api.test.ts, server/projects-api.test.ts) assert "outside a git repository" behaviour and failed only because this sandbox exports a TMPDIR that lives inside a git checkout; re-run with a clean TMPDIR they pass 176/176. The seventh (repo-git.test.tsx → "a clean tree renders the honest empty state") was a waitFor timeout under CPU contention — it passes on repeat runs here, and it passes on origin/main too, so it is flake, not a regression. |
npm run test:unit |
✅ PASS | node:test core-module suite green. |
npm run build |
✅ PASS | Server + web build, and check:pack confirms the tarball still carries the built cockpit. The new public/ assets ride into packages/cezar/web/dist, which files already ships. |
npm run test:package |
✅ PASS | Packaged CLI E2E green. |
Findings
⛔ Blocker
1. The head does not merge into main. mergeable: CONFLICTING, mergeStateStatus: DIRTY. main is four commits ahead (feb85666, ae9b38b2, 0c36809c, 878027f9) and a trial merge conflicts in seven files:
packages/web/src/routes/new-task.tsxpackages/web/src/routes/new-task.test.tsxpackages/web/src/routes/new-task-project.test.tsxpackages/web/e2e/new-task.e2e.tspackages/web/e2e/composer-defaults.e2e.tspackages/web/e2e/plan-mode.e2e.tspackages/web/e2e/agent-browser.ts
The overlap is with #919 ("let a picked skill be taken back off, and open every task without one"), which touched the same composer surface this PR slims down. This is a blocker on its own terms: the composer rework and #919's skill-picker fix have to be reconciled by hand, and until they are, the green validation gate above describes a tree that will not exist after the merge. Please merge origin/main into the branch, resolve the composer conflicts deliberately (rather than taking either side wholesale), and re-run the gate.
⚠️ Major
2. packages/web/src/components/app-shell.tsx:635 — the taskQuickList slot renders nothing, leaving a divider above empty space. AppShellContainer no longer passes taskQuickList (the import of TaskQuickListContainer was removed in this PR), but SidebarContent still renders the slot unconditionally, immediately followed by a separator:
<div data-slot="task-quick-list" className="px-2.5">
<SidebarNavigateContext.Provider value={onNavigate}>
{taskQuickList} {/* always undefined in the running app */}
</SidebarNavigateContext.Provider>
</div>
<hr aria-hidden="true" className="mx-5 mt-2 mb-1 border-border" />In production the sidebar therefore opens with an empty box and a horizontal rule that separates nothing from the Projects section below it — the divider's own comment ("the work above, the places below") describes a layout that no longer exists, because the work moved into ProjectsSection. Fix: delete the taskQuickList slot and the <hr> from SidebarContent, drop the now-unused taskQuickList prop from AppShellProps/NavProps, and keep SidebarNavigateContext.Provider wrapping the projectsMenu nav (which is where the rows that need it now live).
3. packages/web/src/components/app-shell.tsx:602 — "All tasks" is unreachable from the sidebar, and the test suite hides that. AllTasksLink is rendered only inside the projectGroups ? … : … true-branch. AppShellContainer stopped passing projectGroups in this PR (the ProjectGroups mount was replaced by ProjectsSection), so that branch is dead and the link can never render. The workspace-wide /tasks page is consequently reachable only from the ⌘K palette (command-palette.tsx:478) — ProjectsSection's heading links to /settings/global/projects, and ProjectSwitcher lists views rather than a tasks door.
Two things make this a Major rather than a cleanup. First, the code claims otherwise: the comment at app-shell.tsx:626 states that in a multi-project workspace "only the All-tasks door is pinned above the flat column", and multiProject is declared at app-shell.tsx:114, defaulted at :183, packed into nav at :257, re-declared in NavProps at :353 and destructured at :537 — and then never read. The prop that was supposed to pin the door does nothing. Second, app-shell.test.tsx:286 still contains a passing describe('All tasks link (multi-project only)') block, so the suite reports coverage for a path the app cannot reach. Fix: decide the intent. If the door should exist, render AllTasksLink in the flat branch gated on multiProject (which then earns its keep) and retarget the test at the real branch. If it should not, delete AllTasksLink, the multiProject prop chain and that describe block, and leave a comment saying the palette is the sole entry point.
4. packages/web/src/components/project-groups.tsx:67 and packages/web/src/components/task-quick-list.tsx:709 — two components are now unreachable from the app. ProjectGroups and TaskQuickListContainer have no production caller left after this PR; both survive only through tests and through the dead slots in Majors 2 and 3. NavItem.global (nav-items.ts:25) is in the same state — visibleNavItems filters on it and SidebarContent:678 branches on it (const RowLink = item.global ? RouterLink : Link), but no entry in NAV_ITEMS sets it, so the flag is inert and the RouterLink branch is unreachable. CODE_REVIEW.md puts simplicity — "every module is meant to be read in one sitting" — among the review priorities, and three abandoned mechanisms left standing in the shell is exactly the debt that rule exists to prevent. Fix: remove what the redesign superseded, or connect it. If ProjectGroups is being kept deliberately for a follow-up, say so in a comment naming the issue, so the next reader does not have to re-derive that it is unused.
🔹 Minor
5. packages/web/src/routes/task-thread/run-header.tsx:589 — the app-bar portal target is resolved once and never re-checked.
const [target, setTarget] = useState<HTMLElement | null>(null)
useEffect(() => {
setTarget(document.querySelector<HTMLElement>('[data-slot="bar-actions"]'))
}, [])ProjectBar returns null when name, projectSwitcher and crumb are all falsy (app-shell.tsx), and on a cold load all three are async: projectName waits on the registry and on /api/v1/health, crumb waits on the run query. If RunHeader mounts in a commit where the bar is still absent, target stays null for the lifetime of that mount and the task's verbs sit in the header rather than on the bar — the documented fallback, but reached by a race rather than by the intended condition (no bar on mobile / bare test mounts). Fix: re-resolve when the bar can appear — either drop the dependency array so the lookup re-runs each render, or observe the shell with a small MutationObserver, or (cleanest) expose the slot through a React context that AppShell provides, which removes the DOM lookup entirely.
6. BACKWARD_COMPATIBILITY.md §2 was not updated for the new static surface. The PR adds five routes — /cezar-logo.svg, /cezar-logo-dark.svg, /cezar-hero.png, /cezar-hero-git.png, /cezar-hero-archive.png (packages/cezar/src/server/server.ts:1428-1433) — and repoints packages/web/index.html's favicon from /open-mercato.svg to /cezar-logo.svg. Nothing here is breaking: the additions are additive and the old route still answers. But §2 enumerates the static/GUI surface explicitly ("GET /… /assets/:file, /open-mercato.svg"), and CODE_REVIEW.md routes surface changes through that document. Fix: add the five paths to the §2 static list, and note that /open-mercato.svg is retained for cached tabs rather than because the app still points at it.
7. packages/web/src/routes/task-thread/run-actions.ts:136 — primaryRunCta has no exhaustiveness guard. The switch (run.status) covers all seven current RunStatus members and typechecks today, but it has no default, so an eighth status added later would fall off the end and return undefined where the signature promises PrimaryRunCta | null. Callers testing if (!cta) would survive, but the type would be lying. Fix: add default: { const never: never = run.status; return never } (or a satisfies never assertion) so a new status becomes a compile error rather than a silent undefined.
8. packages/web/src/routes/task-git/git-toolbar.tsx:3 — ExternalLinkIcon is now an unused import. It was the icon for the view-pr action, which this PR removed from ACTION_ICONS and from ActionButton. Harmless, but it survives only because noUnusedLocals is not failing the build here. Fix: drop the import.
9. packages/web/src/components/diff-stat.tsx:38 — the quiet-zero rule keys off adds/dels while the sentence still counts files. const zero = stat.adds === 0 && stat.dels === 0 renders the soft tone, but the accessible label built two lines below still reads +0 −0 across N files. A change set of pure renames, mode changes or binary files has files > 0 with zero line counts, so the chip goes quiet while its own text insists something changed. Fix: either fold stat.files === 0 into zero, or leave the tone as-is and have the label say "no line changes across N files" when the counts are zero — the second reads better, since a rename genuinely is a change.
💅 Nit
10. packages/web/src/api/global-events.tsx:363 — the offline banner has no settling delay. setConnectionOffline(true) fires on every EventSource error, and the surrounding comment already acknowledges that an ordinary drop leaves the stream CONNECTING with the browser retrying on its own. A one-second network blip therefore flashes a role="alert" bar that both interrupts screen readers and shifts the layout. A short debounce (say 2s) before flipping to offline — with the open/event handler cancelling it — would keep the honest signal without the flicker.
11. packages/web/src/components/app-shell-container.tsx:210 — the crumb expression is a six-way ternary on one line. It reads titleRun ? … : libraryView ? … : globalSettings ? … : workspaceTasks ? … : activeTo === '/settings' ? … : null. Lifting it into a small function crumbFor(...) above the component, with one if per case, would make the precedence between "library view", "workspace tasks" and "global settings" reviewable at a glance.
12. Review scale. 205 commits, ~180 files and ~9,000 lines in a single PR is past the point where a human reviewer can give every hunk real attention — which is likely how the three dead wiring paths in Majors 2–4 survived to here. Nothing to fix in this PR, but the sidebar IA, the accessibility remediation, the brand/empty-state work and the e2e port were four independently shippable changes, and splitting them would have caught this class of leftover at a tenth of the cost. Worth considering for the next pass of this size.
💥 Breaking Changes
- No exported/public symbol removed or renamed without a deprecation path —
GitActionId's'view-pr'member andGitAction.hrefare removed, butpackages/webis a private workspace package with no published entry point, so this is an internal type change. - No function signature changed in a breaking way —
GitToolbarswapsbranch/statforrunId/cta, again internal topackages/web; every call site is updated in this PR. - No required type field removed or narrowed —
runIndexEntrySchema.groupId/.variantandskillSchema.categoryare all.optional(). - No HTTP route URL removed or renamed — five static routes added,
/open-mercato.svgretained. - No field removed or retyped in an existing response shape — the run-index entry additions are additive, matching the "adding a field is additive" rule in §2.
- No event or message name renamed or removed.
- No CLI command or flag renamed or removed.
- No database table or column renamed or removed.
- No config key renamed and no default changed silently —
DEFAULT_ACCENTflips fromlimetoviolet(packages/web/src/lib/appearance.ts:34), withindex.html's pre-paint script inverted to match. I traced this and it is safe, which is why it is listed here rather than as a finding:writeStoredAppearancealways persists the literal value, so a user who chosevioletreads backviolet(now the no-attribute default) and a user who choselimereads backlime(now the stamped deviation, andindex.css:280has the matching:root[data-accent='lime']block). Only users who never expressed a preference see the change, which is the intent. It is nonetheless a user-visible default flip and deserves a line in the release notes. - Where a contract had to change: old surface kept working —
/open-mercato.svgstill serves, and the pre-#934cez-accentvalues both still resolve correctly.
🧪 Test Coverage
Coverage on this PR is a strength. 75 of the ~180 changed files are tests, and the new behaviour is genuinely exercised rather than smoke-checked: offline-banner.test.tsx covers the connection store's flip and retry path; status-announcer.test.tsx covers the "silent on first observation, speaks on transition" rule that is the whole point of that component; projects-section.test.tsx covers the project tree's open/closed default and its task folding; thread-context-bar.test.tsx covers the two chips; lib/skills.test.ts gains 62 lines covering paletteGroups, including the author-declared-category branch and the vendor-prefix family inference; skills.test.ts on the server side pins the category frontmatter contract (scalar only, trimmed, capped at 40, arrays rejected) with a table-style case per rule. The e2e suite was ported rather than deleted — 35 files against the new IA, with a regression test asserting the docked CTA overlaps no toolbar control, which is precisely the bug the float-to-dock change was made to fix.
Gaps worth closing, all small:
packages/web/src/components/status-mark.tsxhas no direct test. It is exercised transitively throughtasks-overview.test.tsx,task-quick-list.test.tsxandcommand-palette.test.tsx, but theGLYPH/INKmaps are keyed byAttentionKind/AttentionToneand nothing asserts the two records stay total as those unions grow.attention.test.tsalready has the precedent — it asserts the tone set matches the UI's — so add astatus-mark.test.tsxthat iterates everyAttentionKindand asserts a glyph renders with the expecteddata-kind/data-tone.packages/web/src/api/connection-state.tsis only tested through the banner. The module has real edge behaviour worth pinning directly:setConnectionOfflineis a no-op when the value is unchanged (so subscribers are not woken needlessly), andsetConnectionRetry(null)on unmount clears the handler. A dozen-line unit test would cover both.- No test asserts the sidebar's rendered structure without
taskQuickList. This is the gap that let Major 2 through: every sidebar test supplies the slot, so nothing catches the empty div and orphaned<hr>that the real container produces. A singleapp-shell-container.test.tsxcase asserting the sidebar's first child is the Projects section — and that no<hr>precedes it — would have failed on this diff, and would keep failing if the slot is resurrected without content. primaryRunCta(Minor 7) has no case for a status with no session.failedanddone/cancelledboth returnnullwhenlastSessionId(run)is undefined, which is the branch that decides whether the header shows a CTA at all;run-header.test.tsxcovers the populated cases. Add the two session-less cases torun-actions' tests.
pat-lewczuk
added
changes-requested
|
🤖
|
|
Thanks @zielivia — review found actionable items, so I'm handing this PR back to you for the next pass. When the updates are pushed, re-request review and the automation can pick it up from the latest head. Note that |
|
🤖 One blocker (the branch does not merge into |
2 participants
Cockpit audit remediation, carried through a full UX pass of the shell. Started from the phase-1 audit spec (
.ai/specs) and grew into the redesigned cockpit this branch ships end to end: accessibility fixes, the bar-borne task header, the projects-tree sidebar, a slimmer composer, the empty-state artwork, and an e2e suite that walks the new IA.What changed
Accessibility and correctness (audit phase 1). WCAG AA contrast for soft foreground and light inputs, a skip-to-content link, accessible names for GFM checkboxes and agent images, keyboard-reachable scrollable blocks, and a finished run that looks finished.
Task header on the app bar. The thread header collapses into the shell's bar: compact view tabs (Session, the branch with its ± as ONE control, Files), the task's reference chips, an icon-only Open in menu and one overflow. The stateful primary CTA (Stop / Reply / Review changes / Retry / Reopen) floats top-center under the bar on Session and Files, and docks into a real flex slot in the git toolbars' middle, where a centered float used to overlap Unified/Split. The bar itself never reflows as the run moves through its lifecycle.
Sidebar and navigation. Projects first, their tasks inside, ordered by last use; the project row carries its settings and new-task actions permanently. Skills is a workspace library at the sidebar's bottom (the bar names the view, no project over it), Providers moved to Workspace settings → Agent accounts, and hover/selection surfaces settle on gray with violet reserved for signal.
Composer. The row slims from 12 controls to 7: agent and model as one pill, the rare knobs behind a Run options disclosure with a non-default marker, and plan mode as a single Plan-first switch (the old verb-labelled
Start | Plan firstsegment read as the submit button). The headline and run-mode notes speak in the product's register.Empty states. Pixel-art heroes on a shared 232px canvas (tasks, archive, no-repository), a grid ground, copy in Caesar's voice, and no layout jump when switching Active/Archived.
Design-review remediation. A zero diff stat renders in the quiet tone on every surface instead of lighting success/danger for a clean tree; table headers uppercase uniformly; document titles drop the em-dash/middot grammar; the mobile top bar titles itself from the open task.
Tests. The unit suite is green (3288) and the e2e suite (35 files, 208 tests + 6 env-gated skips) was ported to the new IA: hermetic fixture HOME, a one-retry guard for stale keep-alive sockets, and a regression test asserting the docked CTA overlaps no toolbar control.
Two real bugs fixed along the way: the jump-to-latest pill fought scroll anchoring and never moved, and the thread tab row overflowed at 390px.
Validation
npx tsc -b packages/web,npx vitest run packages/web(3288 passed),npm run build(check:pack ok)