Skip to content

Commit dd3e80b

Browse files
committed
fix(desktop): cover the shells behind the allowlist, and make the guard fail
Greptile was right on both counts, and the first one is worse than reported. `LogoShell` was allowlisted as "marketing chrome, not reachable in the desktop shell". That claim was simply false: it is the frame for `not-found`, the interfaces shell, the desktop handoff shell, and the public-file access gates — so the password, email, and SSO gates for `/f/[token]` all still drew under the traffic lights. The allowlist existed to make risk visible and instead hid four surfaces behind one unverified sentence. It now carries two entries, both checked: the landing shell (every consumer lives under `app/(landing)/`) and the playground (calls `notFound()` unless `NEXT_PUBLIC_ENABLE_PLAYGROUND` is set). The lane's two halves also travelled separately, so `/oauth-error` and the public-file view reserved the space without the drag strip — clearing the lights but leaving the window with no title bar on those pages. `DesktopTitleBarLane` now ships both together and the audit enforces the pairing. Both new checks were unfailable when first written, and mutation testing is the only reason that surfaced: - the pairing check matched `DesktopTitleBarLane` anywhere in the file, so the import line satisfied it after the JSX was deleted; - the coverage check matched `LogoShell` anywhere, so a shell's own definition file self-certified as covered. Both now match JSX usage (`/<LogoShell\b/`). Reverting either fix, and deleting either half of the lane, now fails. Measured over CDP: /oauth-error and the LogoShell-based 404 each reserve 40px, carry the drag strip, and overflow by zero, with the logo at 56px.
1 parent cf317c8 commit dd3e80b

7 files changed

Lines changed: 66 additions & 14 deletions

File tree

apps/sim/app/(auth)/components/auth-shell.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { ReactNode } from 'react'
22
import Link from 'next/link'
3-
import { DesktopTitleBarController } from '@/app/_shell/desktop-title-bar'
3+
import { DesktopTitleBarLane } from '@/app/_shell/desktop-title-bar'
44
import { LogoMark, SimWordmark } from '@/app/(landing)/components/navbar/components'
55

66
interface AuthShellProps {
@@ -31,8 +31,7 @@ interface AuthShellProps {
3131
export function AuthShell({ children, footer }: AuthShellProps) {
3232
return (
3333
<div className='light desktop-title-bar-page relative flex flex-col bg-[var(--bg)] text-[var(--text-primary)]'>
34-
<DesktopTitleBarController />
35-
<div aria-hidden className='desktop-login-window-drag-region desktop-window-drag-region' />
34+
<DesktopTitleBarLane />
3635
<header>
3736
<nav className='mx-auto flex w-full max-w-[1446px] items-center px-12 py-4 max-sm:px-5 max-lg:px-8'>
3837
<Link href='/' aria-label='Sim home' className='flex h-[30px] items-center'>

apps/sim/app/(interfaces)/resume/[workflowId]/[executionId]/loading.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { Skeleton } from '@sim/emcn'
2+
import { DesktopTitleBarLane } from '@/app/_shell/desktop-title-bar'
23

34
export default function ResumeLoading() {
45
return (
5-
<div className='min-h-screen bg-background'>
6+
<div className='desktop-title-bar-page bg-background'>
7+
<DesktopTitleBarLane />
68
<div className='border-b px-4 py-3'>
79
<div className='mx-auto flex max-w-[1200px] items-center justify-between'>
810
<Skeleton className='h-[24px] w-[80px] rounded-[4px]' />

apps/sim/app/(landing)/components/logo-shell/logo-shell.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { ReactNode } from 'react'
22
import { cn } from '@sim/emcn'
33
import Link from 'next/link'
4+
import { DesktopTitleBarLane } from '@/app/_shell/desktop-title-bar'
45
import { LogoMark, SimWordmark } from '@/app/(landing)/components/navbar/components'
56

67
/**
@@ -26,7 +27,8 @@ interface LogoShellProps {
2627

2728
export function LogoShell({ children, center = false, footer }: LogoShellProps) {
2829
return (
29-
<div className='light relative flex min-h-screen flex-col bg-[var(--bg)] text-[var(--text-primary)]'>
30+
<div className='light desktop-title-bar-page relative flex flex-col bg-[var(--bg)] text-[var(--text-primary)]'>
31+
<DesktopTitleBarLane />
3032
<header>
3133
<nav className='mx-auto flex w-full max-w-[1460px] items-center px-20 py-4 max-sm:px-5 max-lg:px-8'>
3234
<Link href='/' aria-label='Sim home' className='flex h-[30px] items-center'>

apps/sim/app/_shell/desktop-title-bar-surfaces.test.ts

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ const workspaceChrome = read(
2525
)
2626
const sidebar = read('../workspace/[workspaceId]/w/components/sidebar/sidebar.tsx')
2727
const globalStyles = read('../_styles/globals.css')
28+
const desktopTitleBar = read('../_shell/desktop-title-bar.tsx')
2829
const pageHeaderBar = read('../../components/page-header-bar.ts')
2930
const resourceHeader = read(
3031
'../workspace/[workspaceId]/components/resource/components/resource-header/resource-header.tsx'
@@ -40,7 +41,7 @@ describe('desktop title-bar surface audit', () => {
4041
// rest drawing their logo beneath them; per-route gating could not cover
4142
// `/invite/[id]` either, so the shell owns the lane unconditionally.
4243
expect(authShell).toContain('desktop-title-bar-page')
43-
expect(authShell).toContain('<DesktopTitleBarController />')
44+
expect(authShell).toContain('<DesktopTitleBarLane />')
4445
expect(stripComments(authShell)).not.toContain('reserveDesktopTitleBar')
4546
expect(stripComments(authShell)).not.toContain('min-h-screen')
4647
expect(authLayout).toContain('<AuthShell>')
@@ -49,7 +50,7 @@ describe('desktop title-bar surface audit', () => {
4950
it('mounts a real drag surface across login and workspace title-bar lanes', () => {
5051
const dragRegion = globalStyles.match(/\.desktop-window-drag-region\s*\{([^}]*)\}/)?.[1]
5152

52-
expect(authShell).toContain('desktop-login-window-drag-region')
53+
expect(desktopTitleBar).toContain('desktop-login-window-drag-region')
5354
expect(workspaceChrome).toContain('desktop-workspace-window-drag-region')
5455
expect(workspaceChrome).toContain("isCollapsed ? 'h-[var(--desktop-title-bar-height)]' : 'h-2'")
5556
// The sidebar's lane strip composes the same two classes instead of
@@ -165,14 +166,20 @@ describe('desktop title-bar surface audit', () => {
165166
*/
166167
const LANE_EXEMPT: Record<string, string> = {
167168
'app/(landing)/components/landing-shell/landing-shell.tsx':
168-
'Marketing chrome. The desktop shell boots straight to /login or a workspace and has no path to the landing routes.',
169-
'app/(landing)/components/logo-shell/logo-shell.tsx':
170-
'Marketing chrome, same reasoning as landing-shell.',
171-
'app/playground/page.tsx': 'Dev-only scratch route, not shipped in the desktop shell.',
172-
'app/(interfaces)/resume/[workflowId]/[executionId]/loading.tsx':
173-
'Embedded interface surface rendered inside a host page, never as the desktop window root.',
169+
'Marketing chrome. Verified: every consumer lives under app/(landing)/, and the desktop shell boots to /login or a workspace with no path to those routes.',
170+
'app/playground/page.tsx':
171+
'Verified dev-only: the page calls notFound() unless NEXT_PUBLIC_ENABLE_PLAYGROUND is set.',
174172
}
175173

174+
/**
175+
* Shells that reserve the lane for whatever they wrap.
176+
*
177+
* Matched as JSX usage (`<AuthShell`), never as a bare identifier: an import line, or a
178+
* shell's own definition file mentioning its name, would otherwise self-certify as
179+
* covered. Both mistakes were in the first draft of this check and made it unfailable.
180+
*/
181+
const LANE_AWARE_SHELL_USAGE = /<(AuthShell|LogoShell)\b/
182+
176183
describe('desktop traffic-light lane coverage', () => {
177184
it('leaves no full-viewport root outside workspace chrome unaccounted for', () => {
178185
const appDir = new URL('../', import.meta.url)
@@ -184,10 +191,30 @@ describe('desktop traffic-light lane coverage', () => {
184191
const source = stripComments(read(`../${file.slice('app/'.length)}`))
185192
const fillsViewport = /\b(min-h-screen|h-screen)\b/.test(source)
186193
if (!fillsViewport) return false
187-
const laneAware = source.includes('desktop-title-bar-page') || source.includes('AuthShell')
194+
// Composition counts: a surface is covered if it wears a shell that reserves the
195+
// lane. `LogoShell` was previously allowlisted as marketing chrome — a claim that
196+
// was simply false, and it hid not-found, the interfaces shell, the desktop handoff
197+
// shell and the public-file access gates behind one wrong sentence.
198+
const laneAware =
199+
source.includes('desktop-title-bar-page') || LANE_AWARE_SHELL_USAGE.test(source)
188200
return !laneAware && !(file in LANE_EXEMPT)
189201
})
190202

191203
expect(unaccounted).toEqual([])
192204
})
205+
206+
it('never reserves the lane without also making it draggable', () => {
207+
const appDir = new URL('../', import.meta.url)
208+
const orphaned = readdirSync(appDir, { recursive: true, encoding: 'utf8' })
209+
.filter((f) => f.endsWith('.tsx'))
210+
.map((f) => `app/${f}`)
211+
.filter((file) => {
212+
const source = stripComments(read(`../${file.slice('app/'.length)}`))
213+
// The class alone clears the lights but leaves the strip undraggable, so the
214+
// window loses its title bar on that page. Shipped that way twice in this PR.
215+
return source.includes('desktop-title-bar-page') && !/<DesktopTitleBarLane\b/.test(source)
216+
})
217+
218+
expect(orphaned).toEqual([])
219+
})
193220
})

apps/sim/app/_shell/desktop-title-bar.tsx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,24 @@ export function applyDesktopTitleBarMode(
3131
root.setAttribute('data-sim-desktop-title-bar', mode)
3232
}
3333

34+
/**
35+
* Reserves the macOS traffic-light lane for a full-viewport surface outside workspace
36+
* chrome. Pair it with `desktop-title-bar-page` on the surface's own root.
37+
*
38+
* The two halves ship together on purpose. Reserving the space without this leaves the
39+
* lane visually clear but undraggable — the window loses its title bar on that page —
40+
* and that is exactly what happened when `/oauth-error` and the public-file view were
41+
* given the class alone. The surface audit enforces the pairing.
42+
*/
43+
export function DesktopTitleBarLane() {
44+
return (
45+
<>
46+
<DesktopTitleBarController />
47+
<div aria-hidden className='desktop-login-window-drag-region desktop-window-drag-region' />
48+
</>
49+
)
50+
}
51+
3452
/**
3553
* Keeps the inset correct across native fullscreen transitions, where the traffic lights
3654
* disappear and the lane must collapse. Rendered by `AuthShell`; workspace routes retain

apps/sim/app/f/[token]/public-file-view.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { Chip } from '@sim/emcn'
55
import { Download } from '@sim/emcn/icons'
66
import Link from 'next/link'
77
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
8+
import { DesktopTitleBarLane } from '@/app/_shell/desktop-title-bar'
89
import { SimWordmark } from '@/app/(landing)/components/navbar/components'
910
import { buildProvenance } from '@/app/f/[token]/utils'
1011
import { FileViewer } from '@/app/workspace/[workspaceId]/files/components/file-viewer'
@@ -66,6 +67,7 @@ export function PublicFileView({
6667

6768
return (
6869
<div className='light desktop-title-bar-page flex flex-col bg-[var(--bg)]'>
70+
<DesktopTitleBarLane />
6971
<header className='sticky top-0 z-10 flex items-center justify-between gap-4 border-[var(--border)] border-b bg-[var(--bg)] px-4 py-3'>
7072
<div className='flex min-w-0 items-center gap-3'>
7173
{!brand.logoUrl && (

apps/sim/app/oauth-error/page.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type { Metadata } from 'next'
2+
import { DesktopTitleBarLane } from '@/app/_shell/desktop-title-bar'
23

34
export const metadata: Metadata = {
45
title: 'Sign-in couldn’t be completed',
@@ -37,6 +38,7 @@ export default async function OAuthErrorPage({ searchParams }: OAuthErrorPagePro
3738

3839
return (
3940
<main className='desktop-title-bar-page flex items-center justify-center px-6'>
41+
<DesktopTitleBarLane />
4042
<div className='max-w-sm text-center'>
4143
<h1 className='font-semibold text-foreground text-lg'>Couldn’t complete that</h1>
4244
<p className='mt-2 text-muted-foreground text-sm'>{messageForError(code)}</p>

0 commit comments

Comments
 (0)