diff --git a/apps/desktop/src/main/__tests__/app-region-hygiene-contract.test.ts b/apps/desktop/src/main/__tests__/app-region-hygiene-contract.test.ts index 5b307a2781..2091df108d 100644 --- a/apps/desktop/src/main/__tests__/app-region-hygiene-contract.test.ts +++ b/apps/desktop/src/main/__tests__/app-region-hygiene-contract.test.ts @@ -187,7 +187,14 @@ function findRulesWithDeclaration( function extractStaticUiButtonClassNames(src: string): string[] { const classes = new Set(); - const uiButtonBlocks = src.match(//g) ?? []; + // A topbar UiButton appears either as a direct element + // (…) or, after the Tooltip migration, as the + // render target of a TooltipTrigger + // (} className="…">…). + const uiButtonBlocks = [ + ...(src.match(//g) ?? []), + ...(src.match(//g) ?? []), + ]; for (const block of uiButtonBlocks) { const match = block.match(/\bclassName="([^"]+)"/); assert.ok( diff --git a/apps/desktop/src/main/__tests__/border-width-converge-contract.test.ts b/apps/desktop/src/main/__tests__/border-width-converge-contract.test.ts index 844c75b695..c3b4872783 100644 --- a/apps/desktop/src/main/__tests__/border-width-converge-contract.test.ts +++ b/apps/desktop/src/main/__tests__/border-width-converge-contract.test.ts @@ -83,9 +83,9 @@ const BARE_PX_RE = /(? input[type="checkbox"]:checked::after', - '.maka-permission-raw > summary::before', + '.maka-permission-raw [data-slot="collapsible-trigger"]::before', ]); const BORDER_STYLE_KEYWORDS = new Set([ @@ -211,7 +211,7 @@ describe('border-width whitelist negative cases', () => { it('findCssOffenders allows triangle caret geometry on allowlisted selectors, flags multi-value and single bare-px elsewhere', () => { // Known caret selectors: multi-value geometry is allowed. - assert.deepEqual(findCssOffenders('.maka-turn-thinking summary::before {\n border-width: 4px 0 4px 5px;\n}', 't'), [], 'allowlisted caret: 4px 0 4px 5px must pass'); + assert.deepEqual(findCssOffenders('.maka-turn-thinking [data-slot="collapsible-trigger"]::before {\n border-width: 4px 0 4px 5px;\n}', 't'), [], 'allowlisted caret: 4px 0 4px 5px must pass'); assert.deepEqual(findCssOffenders('.maka-bubble-assistant li.task-list-item > input[type="checkbox"]:checked::after {\n border-width: 0 2px 2px 0;\n}', 't'), [], 'allowlisted caret: 0 2px 2px 0 must pass'); // Non-allowlisted selector: a multi-value bare px is a stroke drift, not // triangle geometry — a heuristic that spared ALL multi-value would miss it. diff --git a/apps/desktop/src/main/__tests__/chat-marker-cascade-contract.test.ts b/apps/desktop/src/main/__tests__/chat-marker-cascade-contract.test.ts index f059b3b9c1..84d9f8d4bb 100644 --- a/apps/desktop/src/main/__tests__/chat-marker-cascade-contract.test.ts +++ b/apps/desktop/src/main/__tests__/chat-marker-cascade-contract.test.ts @@ -51,7 +51,7 @@ describe('chat Marker shell migration contract (#332 PR2)', () => { // `.maka-turn-thinking` is explicitly deferred (pseudo-element chevron + // @starting-style fade don't reduce to leaf utilities); it stays authored. '.maka-turn-thinking', - '.maka-turn-thinking summary', + '.maka-turn-thinking [data-slot="collapsible-trigger"]', ]) { assert.ok(css.includes(selector), `out-of-scope turn rule "${selector}" must be preserved`); } diff --git a/apps/desktop/src/main/__tests__/chat-tool-card-cascade-contract.test.ts b/apps/desktop/src/main/__tests__/chat-tool-card-cascade-contract.test.ts index 31e0380a4a..0d8d0b2b5c 100644 --- a/apps/desktop/src/main/__tests__/chat-tool-card-cascade-contract.test.ts +++ b/apps/desktop/src/main/__tests__/chat-tool-card-cascade-contract.test.ts @@ -22,8 +22,10 @@ import { REPO_ROOT, TOKENS_FILE, readAllRendererCss, stripCssComments } from './ * 2. the running status dot's `@keyframes maka-tool-pulse` ring frames — an * animation can't be a leaf-literal and `getComputedStyle` reads a phase- * dependent value, so the breath is pinned here + by the `chat.tsx` literal; - * 3. native `` marker reset — re-keyed off the retired `.maka-tool` - * class onto the governed `[data-slot="tool"]` hook. + * 3. the `[data-slot="tool"]` base residue (opacity/transform/border-color + * transition) — the native `` marker reset that used to live here is + * gone after the disclosure → Collapsible migration (the trigger is a button + * with no native marker). */ describe('chat tool-card migration contract (#332 PR3b)', () => { it('retires the bespoke tool-card shell selectors (without touching error/preview/maka-code)', async () => { @@ -43,7 +45,7 @@ describe('chat tool-card migration contract (#332 PR3b)', () => { '.maka-tool-body', '.maka-tool-intent', '.maka-tool-count', - // the `
` card base + its status / open / summary selectors + // the retired native-disclosure card base + status/open selectors '.maka-tool {', '.maka-tool >', '.maka-tool[open]', @@ -90,7 +92,7 @@ describe('chat tool-card migration contract (#332 PR3b)', () => { } }); - it('keeps only the marker reset residue, re-keyed onto [data-slot="tool"]', async () => { + it('keeps the tool-card base residue on [data-slot="tool"]', async () => { const tokens = stripCssComments(await readFile(TOKENS_FILE, 'utf8')); assert.ok( !tokens.includes('.maka-tool {') && !tokens.includes('@starting-style {\n .maka-tool'), @@ -101,8 +103,6 @@ describe('chat tool-card migration contract (#332 PR3b)', () => { '[data-slot="tool"] {', 'transform: translateY(0)', 'transition: border-color var(--duration-base) var(--ease-out-strong);', - '[data-slot="tool"] > summary::-webkit-details-marker { display: none; }', - "[data-slot=\"tool\"] > summary::marker { content: ''; }", ]) { assert.ok( tokens.includes(residue), @@ -176,12 +176,35 @@ describe('chat tool-card migration contract (#332 PR3b)', () => { ); } // The open/collapsed divider — the one card surface that differs by state - // (the collapsed default has no bottom border). The computed-diff proves both - // states, but that harness is manual (no CI), so pin the `[open]>summary` - // literal here as the automated guard. + // (the collapsed default has no bottom border). Base UI puts + // `[data-panel-open]` directly on the Collapsible Trigger, so keep the border + // on the styled trigger/header part without adding a root group or crossing + // elements to read root state. assert.ok( - block.includes('[&[open]>summary]:[border-bottom:1px_solid_var(--border)]'), - 'item must keep the `[open]>summary` divider literal (collapsed default has none)', + !rawSrc.includes('[open]>summary'), + 'tool card source must not keep the old native details `[open]>summary` selector, even in comments', + ); + assert.ok( + !rawSrc.includes('group-data-[open]/tool'), + 'tool card source must not use a root group to read open state when Base UI Trigger exposes [data-panel-open]', + ); + const itemStart = block.indexOf('item:'); + const headerStart = block.indexOf('header:', itemStart); + const dotStart = block.indexOf('dot:', headerStart); + assert.ok(itemStart !== -1 && headerStart !== -1 && dotStart !== -1, 'toolVariants item/header/dot parts must stay parseable'); + const itemBlock = block.slice(itemStart, headerStart); + const headerBlock = block.slice(headerStart, dotStart); + assert.ok( + !itemBlock.includes('group/tool'), + 'tool card root must not add a named group for open state when Base UI Trigger exposes [data-panel-open]', + ); + assert.ok( + !itemBlock.includes('border-bottom'), + 'tool card root must not own the open-state divider; put the border on the trigger/header part', + ); + assert.ok( + headerBlock.includes('data-[panel-open]:[border-bottom:1px_solid_var(--border)]'), + 'tool card header must add the divider from Base UI Trigger [data-panel-open]', ); // Anti-drift: pin the distinctive literals and ban the semantic-scale // forms they would be swapped for. Radius uses the `--radius-surface` diff --git a/apps/desktop/src/main/__tests__/disclosure-collapsible-contract.test.ts b/apps/desktop/src/main/__tests__/disclosure-collapsible-contract.test.ts new file mode 100644 index 0000000000..e3342b1151 --- /dev/null +++ b/apps/desktop/src/main/__tests__/disclosure-collapsible-contract.test.ts @@ -0,0 +1,83 @@ +/** + * PR-DISCLOSURE-COLLAPSIBLE-0 (issue #520 PR5 item 17, 2026-07-05): + * the four disclosure sites (turn-thinking, reasoning-panel, permission-raw, + * tool-activity) migrate off native `
`/`` onto Base UI + * Collapsible. The code comments at the sites already said "future Base UI + * Accordion path"; all four are independent single sections (not grouped), + * so Collapsible (not Accordion) is the right primitive. + * + * Why migrate: native `
` gives free keyboard a11y but no CSS hook for + * the open/closed animation state, no controlled-open API for the reasoning + * panel's "default open, first click sticks" behavior (which today reads + * `e.currentTarget.open` from the toggle event), and no `data-slot` for the + * style-hook convention. Base UI Collapsible gives `data-[open]` state, a + * controlled `open` prop, and the `data-slot` hook. + * + * This contract locks the migration: the three files that held the four + * `
` sites must not carry `
`/`` (not even in + * comments — a stale "wrapped in a
" comment is a regression + * signal), and must import Collapsible. The Collapsible primitive itself + * must wrap Base UI Collapsible with the data-slot convention (item 23). + */ + +import { strict as assert } from 'node:assert'; +import { readFile } from 'node:fs/promises'; +import { resolve } from 'node:path'; +import { describe, it } from 'node:test'; +import { REPO_ROOT } from './css-test-helpers.js'; + +const MIGRATED_FILES = [ + 'packages/ui/src/chat-view.tsx', + 'packages/ui/src/permission-dialog.tsx', + 'packages/ui/src/tool-activity.tsx', +]; + +const COLLAPSIBLE_PRIMITIVE = 'packages/ui/src/primitives/collapsible.tsx'; + +/** A Collapsible import: from the @maka/ui barrel, the primitives path, or + * @base-ui/react/collapsible directly. */ +const COLLAPSIBLE_IMPORT_RE = /import\s+\{[^}]*\bCollapsible\b[^}]*\}\s+from\s+['"][^'"]*(?:@maka\/ui|primitives\/collapsible|@base-ui\/react\/collapsible)[^'"]*['"]/; + +describe('PR-DISCLOSURE-COLLAPSIBLE-0 contract', () => { + it('the disclosure sites use Base UI Collapsible (no native
/)', async () => { + for (const rel of MIGRATED_FILES) { + const src = await readFile(resolve(REPO_ROOT, rel), 'utf8'); + assert.ok(!/ (migrate to Base UI Collapsible; also drop stale
mentions in comments)`); + assert.ok(!/ (use Collapsible.Trigger)`); + assert.match(src, COLLAPSIBLE_IMPORT_RE, `${rel}: must import Collapsible from @maka/ui / primitives/collapsible / @base-ui/react/collapsible`); + } + }); + + it('primitives/collapsible.tsx wraps Base UI Collapsible with data-slot on Root / Trigger / Panel', async () => { + const src = await readFile(resolve(REPO_ROOT, COLLAPSIBLE_PRIMITIVE), 'utf8'); + assert.match(src, /@base-ui\/react\/collapsible/, 'must import from @base-ui/react/collapsible'); + for (const slot of ['collapsible', 'collapsible-trigger', 'collapsible-panel']) { + assert.match(src, new RegExp(`data-slot="${slot}"`), `must expose data-slot="${slot}" (style-hook convention, item 23)`); + } + }); + + it('tool-activity Collapsible is controlled (open follows item.status), not defaultOpen', async () => { + // A `defaultOpen` card decides open only on first render, so a card that + // defaults open while pending/running would NOT auto-collapse when it + // settles to completed/interrupted — the pre-Collapsible `
` re-evaluated open every render. The + // controlled form (open + onOpenChange, re-synced via useEffect on + // [item.status]) restores that: status change collapses/expands the card, + // the user can still toggle in between. + const src = await readFile(resolve(REPO_ROOT, 'packages/ui/src/tool-activity.tsx'), 'utf8'); + assert.ok(!/defaultOpen=/.test(src), 'tool-activity must not use defaultOpen (a running card that defaults open would not auto-collapse when it settles); use controlled open that follows item.status'); + assert.match(src, /\bonOpenChange\b/, 'tool-activity Collapsible must be controlled via onOpenChange'); + assert.match(src, /useEffect\([^]*\[item\.status\]/, 'tool-activity must re-sync open when item.status changes (useEffect on [item.status])'); + }); +}); + +describe('disclosure-collapsible negative cases', () => { + it('flags a native
and a missing Collapsible import', () => { + const withDetails = 'import { Collapsible } from "@maka/ui";\nexport function X() { return
hb
; }'; + assert.ok(/ must be detected'); + const noImport = 'export function X() { return null; }'; + assert.ok(!COLLAPSIBLE_IMPORT_RE.test(noImport), 'no Collapsible import must not match'); + const withImport = 'import { Collapsible } from "@maka/ui";\nexport function X() { return ; }'; + assert.ok(COLLAPSIBLE_IMPORT_RE.test(withImport), 'a Collapsible import must match'); + }); +}); \ No newline at end of file diff --git a/apps/desktop/src/main/__tests__/number-field-converge-contract.test.ts b/apps/desktop/src/main/__tests__/number-field-converge-contract.test.ts new file mode 100644 index 0000000000..f98619728e --- /dev/null +++ b/apps/desktop/src/main/__tests__/number-field-converge-contract.test.ts @@ -0,0 +1,61 @@ +/** + * PR-NUMBER-FIELD-CONVERGE-0 (issue #520 PR5 item 21, 2026-07-05): + * the two gateway/proxy port inputs migrate off the native `Input` + + * `Number(event.currentTarget.value)` hand-conversion onto Base UI + * NumberField, which binds `value: number | null` directly and parses + * numeric input itself (no manual string→number, no `|| default` fallback + * gymnastics). + * + * Sites: + * - general-settings-page proxy port (default 0 on empty). + * - open-gateway-settings-page gateway port (default 3939 on empty). + * + * The contract: the two files must not carry the `Number(event.currentTarget + * .value)` hand-conversion and must import NumberField; the primitive must + * wrap Base UI NumberField with the data-slot convention (item 23). + */ + +import { strict as assert } from 'node:assert'; +import { readFile } from 'node:fs/promises'; +import { resolve } from 'node:path'; +import { describe, it } from 'node:test'; +import { REPO_ROOT } from './css-test-helpers.js'; + +const MIGRATED_FILES = [ + 'apps/desktop/src/renderer/settings/general-settings-page.tsx', + 'apps/desktop/src/renderer/settings/open-gateway-settings-page.tsx', +]; + +const NUMBER_FIELD_PRIMITIVE = 'packages/ui/src/primitives/number-field.tsx'; + +/** The hand-conversion the migration removes. */ +const HAND_CONVERT_RE = /Number\(event\.currentTarget\.value\)/; + +/** A NumberField import from the barrel / primitives / @base-ui. */ +const NUMBER_FIELD_IMPORT_RE = /import\s+\{[^}]*\bNumberField\b[^}]*\}\s+from\s+['"][^'"]*(?:@maka\/ui|primitives\/number-field|@base-ui\/react\/number-field)[^'"]*['"]/; + +describe('PR-NUMBER-FIELD-CONVERGE-0 contract', () => { + it('the port-input files use Base UI NumberField (no Number(event.currentTarget.value) hand-conversion)', async () => { + for (const rel of MIGRATED_FILES) { + const src = await readFile(resolve(REPO_ROOT, rel), 'utf8'); + assert.ok(!HAND_CONVERT_RE.test(src), `${rel}: must not hand-convert Number(event.currentTarget.value) — use Base UI NumberField (value: number | null, onValueChange)`); + assert.match(src, NUMBER_FIELD_IMPORT_RE, `${rel}: must import NumberField from @maka/ui / primitives/number-field / @base-ui/react/number-field`); + } + }); + + it('primitives/number-field.tsx wraps Base UI NumberField with data-slot on Root / Input', async () => { + const src = await readFile(resolve(REPO_ROOT, NUMBER_FIELD_PRIMITIVE), 'utf8'); + assert.match(src, /@base-ui\/react\/number-field/, 'must import from @base-ui/react/number-field'); + for (const slot of ['number-field', 'number-field-input']) { + assert.match(src, new RegExp(`data-slot="${slot}"`), `must expose data-slot="${slot}" (style-hook convention, item 23)`); + } + }); +}); + +describe('number-field negative cases', () => { + it('HAND_CONVERT_RE matches the hand-conversion, not other Number() uses', () => { + assert.ok(HAND_CONVERT_RE.test('Number(event.currentTarget.value)'), 'the hand-conversion must match'); + assert.ok(!HAND_CONVERT_RE.test('Number(123)'), 'a plain Number(123) must not match'); + assert.ok(!HAND_CONVERT_RE.test('const n = Number("x")'), 'a different Number() call must not match'); + }); +}); \ No newline at end of file diff --git a/apps/desktop/src/main/__tests__/search-modal-lifecycle-contract.test.ts b/apps/desktop/src/main/__tests__/search-modal-lifecycle-contract.test.ts index 00368103e3..11dcc7bbfc 100644 --- a/apps/desktop/src/main/__tests__/search-modal-lifecycle-contract.test.ts +++ b/apps/desktop/src/main/__tests__/search-modal-lifecycle-contract.test.ts @@ -116,7 +116,7 @@ describe('SearchModal lifecycle contract (PR-SIDEBAR-IA-0 Phase 3 P0 fixup)', () it('returns focus to the sidebar Search trigger when the modal closes', async () => { const main = await readRendererShellCombinedSource(); - const shellSearchButton = main.match(/className="maka-shell-topbar-button"[\s\S]*?data-maka-search-trigger="true"[\s\S]*?<\/UiButton>/)?.[0] ?? ''; + const shellSearchButton = main.match(/className="maka-shell-topbar-button"[\s\S]*?data-maka-search-trigger="true"[\s\S]*?<\/TooltipTrigger>/)?.[0] ?? ''; const closeSearchModal = main.match(/function closeSearchModal\(options\?: \{ restoreFocus\?: boolean \}\) \{[\s\S]*?\n \}/)?.[0] ?? ''; assert.match( diff --git a/apps/desktop/src/main/__tests__/settings-network-gateway-contract.test.ts b/apps/desktop/src/main/__tests__/settings-network-gateway-contract.test.ts index 9a04c46c6e..9db9661aed 100644 --- a/apps/desktop/src/main/__tests__/settings-network-gateway-contract.test.ts +++ b/apps/desktop/src/main/__tests__/settings-network-gateway-contract.test.ts @@ -58,7 +58,7 @@ describe('Settings network and gateway persistence contract', () => { ); assert.match( networkBlock, - /value=\{String\(proxyDraft\.port \|\| ''\)\}[\s\S]*onChange=\{\(event\) => void updateProxy\(\{ port: Number\(event\.currentTarget\.value\) \|\| 0 \}\)\}/, + /value=\{proxyDraft\.port \|\| null\}[\s\S]*onValueChange=\{\(v\) => void updateProxy\(\{ port: v \?\? 0 \}\)\}/, 'Network proxy port input must render from the local draft while persisting in the background', ); assert.match( @@ -231,7 +231,7 @@ describe('Settings network and gateway persistence contract', () => { ); assert.match( gatewayBlock, - /value=\{String\(gatewayDraft\.port\)\}[\s\S]*onChange=\{\(event\) => void updateGateway\(\{ port: Number\(event\.currentTarget\.value\) \|\| 3939 \}\)\}/, + /value=\{gatewayDraft\.port\}[\s\S]*onValueChange=\{\(v\) => void updateGateway\(\{ port: v \?\? 3939 \}\)\}/, 'Open Gateway port input must render from the local draft while persisting in the background', ); assert.doesNotMatch( diff --git a/apps/desktop/src/main/__tests__/style-hook-convention-contract.test.ts b/apps/desktop/src/main/__tests__/style-hook-convention-contract.test.ts new file mode 100644 index 0000000000..b976bbc6a1 --- /dev/null +++ b/apps/desktop/src/main/__tests__/style-hook-convention-contract.test.ts @@ -0,0 +1,107 @@ +/** + * PR-STYLE-HOOK-CONVENTION-0 (issue #520 PR5 item 23, 2026-07-05): + * every Base UI wrapper in `packages/ui/src/ui.tsx` exposes a `data-slot` + * attribute so CSS can target `[data-slot="..."]` (a stable hook that + * survives className drift), matching the `./primitives/` wrappers that + * already do this (accordion / alert / badge / …). New wrappers + * (Collapsible / Tooltip / NumberField / …) follow the same rule. + * + * Boolean state hooks adopt Base UI's native attribute-presence form + * (`[data-active]` / `[data-open]` / `[data-checked]` / `[data-selected]` / + * `[data-pressed]` / `[data-highlighted]` / `[data-disabled]`), NOT the + * attribute-value form (`[data-active="true"]`). Maka's renderer CSS has + * zero state-attribute selectors today, so adopting Base UI's form breaks + * nothing and avoids an override layer. The per-component hook map lives in + * the doc comment at the top of ui.tsx. + * + * This contract locks the data-slot rule: a wrapper that forwards props to a + * `Base*` Base UI component must carry `data-slot`. It does NOT lock the + * state-attribute decision (that is a "don't override" rule, enforced by the + * absence of `[data-active="true"]`-style overrides, which the existing + * CSS-scan contracts already cover indirectly). + */ + +import { strict as assert } from 'node:assert'; +import { readFile } from 'node:fs/promises'; +import { resolve } from 'node:path'; +import { describe, it } from 'node:test'; +import { REPO_ROOT } from './css-test-helpers.js'; + +const UI_FILE = resolve(REPO_ROOT, 'packages/ui/src/ui.tsx'); + +/** A top-level wrapper declaration: `export const X = forwardRef...` / + * `const X = forwardRef...` / `export function X(...)` at column 0. */ +const WRAPPER_DECL_RE = /^(?:export (?:const|function)|const) [A-Z][A-Za-z0-9]*\b/gm; + +/** A Base UI component JSX tag: ` { + const matches = [...source.matchAll(WRAPPER_DECL_RE)]; + const blocks: Array<{ name: string; body: string }> = []; + for (let i = 0; i < matches.length; i++) { + const start = matches[i].index; + const end = i + 1 < matches.length ? matches[i + 1].index : source.length; + const body = source.slice(start, end); + const name = matches[i][0].replace(/^(?:export )?(?:const|function) /, ''); + blocks.push({ name, body }); + } + return blocks; +} + +describe('PR-STYLE-HOOK-CONVENTION-0 contract', () => { + it('every ui.tsx wrapper that forwards to a Base* component carries data-slot', async () => { + const source = await readFile(UI_FILE, 'utf8'); + const blocks = wrapperBlocks(source); + const offenders: string[] = []; + for (const { name, body } of blocks) { + // Only wrappers that render a `` Base UI component are in scope. + // Hand-written native elements (the legacy Input / Textarea / + //