diff --git a/packages/shell/src/main/apps/app-icon-resolver.test.ts b/packages/shell/src/main/apps/app-icon-resolver.test.ts new file mode 100644 index 00000000..5777f4ec --- /dev/null +++ b/packages/shell/src/main/apps/app-icon-resolver.test.ts @@ -0,0 +1,101 @@ +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { + AppIconMiss, + appIconMissStatus, + isAppIconHit, + resolveAppIconInBundle, +} from "./app-icon-resolver"; + +const BUNDLE = join("/vaults", "v1", "apps", "studio.northbound.client-pulse", "1.0.0"); + +/** Bundle IO stub: a manifest object plus the files that exist on disk. */ +const io = (manifest: unknown, files: readonly string[] = []) => ({ + readManifest: async () => JSON.stringify(manifest), + fileExists: async (path: string) => files.includes(path), +}); + +describe("resolveAppIconInBundle", () => { + it("resolves a declared icon inside the bundle", async () => { + const resolved = await resolveAppIconInBundle( + BUNDLE, + io({ id: "x", icon: "icon.svg" }, [join(BUNDLE, "icon.svg")]), + ); + expect(isAppIconHit(resolved)).toBe(true); + expect(resolved).toEqual({ path: join(BUNDLE, "icon.svg") }); + }); + + it("resolves an icon in a subdirectory", async () => { + const resolved = await resolveAppIconInBundle( + BUNDLE, + io({ icon: "assets/icon.png" }, [join(BUNDLE, "assets", "icon.png")]), + ); + expect(resolved).toEqual({ path: join(BUNDLE, "assets", "icon.png") }); + }); + + // POLISH-LAY-8 — the sideloaded Client Pulse / Hello case: no `icon` key at + // all. Not an error; the caller draws initials. + it("reports NoIcon for a manifest with no icon key", async () => { + expect(await resolveAppIconInBundle(BUNDLE, io({ id: "x" }))).toEqual({ + miss: AppIconMiss.NoIcon, + }); + }); + + it("reports NoIcon for an empty or non-string icon", async () => { + expect(await resolveAppIconInBundle(BUNDLE, io({ icon: "" }))).toEqual({ + miss: AppIconMiss.NoIcon, + }); + expect(await resolveAppIconInBundle(BUNDLE, io({ icon: 7 }))).toEqual({ + miss: AppIconMiss.NoIcon, + }); + }); + + it("reports NoIcon for a declared icon that isn't on disk", async () => { + // Never hand the caller a path it will fetch as a missing `file://` + // (ERR_UNEXPECTED in the renderer) — the app simply has no artwork. + expect(await resolveAppIconInBundle(BUNDLE, io({ icon: "icon.svg" }, []))).toEqual({ + miss: AppIconMiss.NoIcon, + }); + }); + + it("refuses a path that escapes the bundle", async () => { + expect( + await resolveAppIconInBundle(BUNDLE, io({ icon: "../../secrets.png" }, ["/vaults/secrets.png"])), + ).toEqual({ miss: AppIconMiss.NoIcon }); + }); + + it("contains an absolute-looking icon path inside the bundle", async () => { + // `join` treats a leading slash as relative, so `/etc/passwd` can only + // ever address `/etc/passwd` — and that doesn't exist. + expect( + await resolveAppIconInBundle(BUNDLE, io({ icon: "/etc/passwd" }, ["/etc/passwd"])), + ).toEqual({ miss: AppIconMiss.NoIcon }); + }); + + it("reports Unresolved when the app has no active bundle", async () => { + expect(await resolveAppIconInBundle(null)).toEqual({ miss: AppIconMiss.Unresolved }); + }); + + it("reports Unresolved when the manifest is missing or unparseable", async () => { + expect( + await resolveAppIconInBundle(BUNDLE, { + readManifest: async () => { + throw new Error("ENOENT"); + }, + }), + ).toEqual({ miss: AppIconMiss.Unresolved }); + expect(await resolveAppIconInBundle(BUNDLE, { readManifest: async () => "{ not json" })).toEqual({ + miss: AppIconMiss.Unresolved, + }); + }); +}); + +describe("appIconMissStatus", () => { + it("answers 204 for a declared-no-icon app so no console 404 is logged", () => { + expect(appIconMissStatus(AppIconMiss.NoIcon)).toBe(204); + }); + + it("keeps 404 meaningful for an id that resolves to nothing installed", () => { + expect(appIconMissStatus(AppIconMiss.Unresolved)).toBe(404); + }); +}); diff --git a/packages/shell/src/main/apps/app-icon-resolver.ts b/packages/shell/src/main/apps/app-icon-resolver.ts new file mode 100644 index 00000000..c913131e --- /dev/null +++ b/packages/shell/src/main/apps/app-icon-resolver.ts @@ -0,0 +1,88 @@ +/** + * `brainstorm://app-icon/` resolution — which file (if any) backs an + * app's icon, and WHY there isn't one. + * + * The distinction is the point. An installed app that declares no `icon` is a + * normal, expected state: every icon surface already draws a deterministic + * initials tile for it. Answering that with a 404 turned "this app has no + * artwork" into a console error on every surface that renders the app, every + * boot — four per run for two icon-less sideloaded apps, and the only console + * errors in the VID-build-apps capture (POLISH-LAY-8). It answers 204 instead. + * A 404 stays reserved for an id that resolves to nothing installed, which IS + * worth surfacing. + * + * Path containment (the icon must stay inside the app's own bundle) is part of + * the resolution, not the caller's job. + */ + +import { readFile, stat } from "node:fs/promises"; +import { join, normalize, sep } from "node:path"; + +export enum AppIconMiss { + /** No session, no active record for this id, or an unreadable manifest — a + * real miss the caller should see. */ + Unresolved = "unresolved", + /** Installed, but the manifest declares no in-bundle icon. Not an error. */ + NoIcon = "no-icon", +} + +export type AppIconResolution = { path: string } | { miss: AppIconMiss }; + +export function isAppIconHit(resolution: AppIconResolution): resolution is { path: string } { + return "path" in resolution; +} + +/** HTTP status for a miss — 204 (no content) for a declared-no-icon app so the + * caller's initials fallback runs quietly; 404 for an unresolvable id. */ +export function appIconMissStatus(miss: AppIconMiss): number { + return miss === AppIconMiss.NoIcon ? 204 : 404; +} + +export type AppIconResolverIo = { + /** Read `/manifest.json`. */ + readManifest: (dir: string) => Promise; + /** Does the resolved icon file exist on disk? */ + fileExists: (path: string) => Promise; +}; + +const DEFAULT_IO: AppIconResolverIo = { + readManifest: (dir) => readFile(join(dir, "manifest.json"), "utf8"), + fileExists: (path) => + stat(path).then( + () => true, + () => false, + ), +}; + +/** Resolve the icon file inside an installed app's bundle. `bundleDir` is + * `null` when no active record exists for the id. `io` is injectable for + * tests. + * + * A declared icon that isn't actually on disk resolves to `NoIcon`, not to a + * path: letting the caller `net.fetch` a missing `file://` surfaces as an + * `ERR_UNEXPECTED` in the renderer rather than a clean status (the same trap + * the emoji handler already guards). Either way the app has no usable + * artwork, which is exactly what `NoIcon` means. */ +export async function resolveAppIconInBundle( + bundleDir: string | null, + io: Partial = {}, +): Promise { + if (!bundleDir) return { miss: AppIconMiss.Unresolved }; + const { readManifest, fileExists } = { ...DEFAULT_IO, ...io }; + let manifest: { icon?: unknown }; + try { + manifest = JSON.parse(await readManifest(bundleDir)) as { icon?: unknown }; + } catch { + return { miss: AppIconMiss.Unresolved }; + } + if (typeof manifest.icon !== "string" || manifest.icon.length === 0) { + return { miss: AppIconMiss.NoIcon }; + } + if (manifest.icon.includes("..")) return { miss: AppIconMiss.NoIcon }; + const target = normalize(join(bundleDir, manifest.icon)); + if (!target.startsWith(bundleDir + sep) && target !== bundleDir) { + return { miss: AppIconMiss.NoIcon }; + } + if (!(await fileExists(target))) return { miss: AppIconMiss.NoIcon }; + return { path: target }; +} diff --git a/packages/shell/src/main/dashboard/dashboard-store.test.ts b/packages/shell/src/main/dashboard/dashboard-store.test.ts index 4b55a273..09390a25 100644 --- a/packages/shell/src/main/dashboard/dashboard-store.test.ts +++ b/packages/shell/src/main/dashboard/dashboard-store.test.ts @@ -12,6 +12,11 @@ import { import { OsHandoffConsent } from "@brainstorm-os/sdk-types"; import { ThemeName } from "@brainstorm-os/tokens"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { + ICON_FOOTPRINT_H, + ICON_FOOTPRINT_W, + footprintsOverlap, +} from "../../shared/dashboard-icon-grid"; import { placeDashboardIcon } from "../dev/seed-demo-apps"; import { YDocStore } from "../storage/ydoc-store"; import { DASHBOARD_DOC_ID, DashboardStore, applyLegacyMigration } from "./dashboard-store"; @@ -665,6 +670,87 @@ describe("DashboardStore — shell prefs (locale / regional / chrome / notificat await store.close(); }); + // POLISH-LAY-6 — the install path, end to end over the real store. Main + // used to place onto a fictional 12-column grid of 1×1 cells while the + // renderer stores 8px units and paints an ICON_FOOTPRINT_W × _H box per + // icon, so a newly installed app landed ON TOP of Notes. + it("places an installed app's icon clear of every existing icon's footprint", async () => { + const store = await DashboardStore.open(yStore); + // The seeded fleet as captured on the real dashboard. + const seeded = [ + { id: "io.brainstorm.notes", x: 0, y: 0 }, + { id: "io.brainstorm.files", x: 11, y: 0 }, + { id: "io.brainstorm.database", x: 22, y: 0 }, + { id: "io.brainstorm.books", x: 0, y: 14 }, + ]; + for (const app of seeded) { + store.upsertIcon(`icon_${app.id}_demo`, { + x: app.x, + y: app.y, + kind: "app", + target: app.id, + label: app.id, + }); + } + + expect(placeDashboardIcon(store, "studio.northbound.client-pulse", "Client Pulse")).toBe(true); + const placed = store.snapshot().icons["icon_studio.northbound.client-pulse_demo"]; + expect(placed).toBeDefined(); + if (!placed) throw new Error("unreachable"); + for (const app of seeded) { + expect(footprintsOverlap({ col: placed.x, row: placed.y }, { col: app.x, row: app.y })).toBe( + false, + ); + } + + // A second install stacks on neither the fleet nor the first newcomer. + expect(placeDashboardIcon(store, "io.brainstorm.hello", "Hello")).toBe(true); + const second = store.snapshot().icons["icon_io.brainstorm.hello_demo"]; + if (!second) throw new Error("unreachable"); + for (const other of [...seeded.map((a) => ({ x: a.x, y: a.y })), placed]) { + expect(footprintsOverlap({ col: second.x, row: second.y }, { col: other.x, row: other.y })).toBe( + false, + ); + } + await store.close(); + }); + + it("wraps an install onto the next footprint row once the first row is full", async () => { + const store = await DashboardStore.open(yStore); + // Fill the first footprint row wall to wall for a 1440px-wide surface + // (≈ 16 slots at ICON_FOOTPRINT_W); the placer keeps stepping across + // (the grid is unbounded) but never overlaps. + for (let i = 0; i < 16; i++) { + store.upsertIcon(`icon_seed${i}_demo`, { + x: i * ICON_FOOTPRINT_W, + y: 0, + kind: "app", + target: `seed${i}`, + label: `Seed ${i}`, + }); + } + // One icon parked on the second footprint row's origin: the newcomer + // must clear that too. + store.upsertIcon("icon_seedRow2_demo", { + x: 0, + y: ICON_FOOTPRINT_H, + kind: "app", + target: "seedRow2", + label: "Row 2", + }); + + expect(placeDashboardIcon(store, "io.brainstorm.newcomer", "Newcomer")).toBe(true); + const placed = store.snapshot().icons["icon_io.brainstorm.newcomer_demo"]; + if (!placed) throw new Error("unreachable"); + for (const icon of Object.values(store.snapshot().icons)) { + if (icon.target === "io.brainstorm.newcomer") continue; + expect(footprintsOverlap({ col: placed.x, row: placed.y }, { col: icon.x, row: icon.y })).toBe( + false, + ); + } + await store.close(); + }); + it("isWithinDnd handles same-day and wrap-past-midnight windows", () => { const at = (h: number, m: number) => new Date(2026, 0, 1, h, m); const wrap = { enabled: true, start: "22:00", end: "08:00" }; diff --git a/packages/shell/src/main/dashboard/grid-placement.test.ts b/packages/shell/src/main/dashboard/grid-placement.test.ts index 3c69856f..bc4a6ed3 100644 --- a/packages/shell/src/main/dashboard/grid-placement.test.ts +++ b/packages/shell/src/main/dashboard/grid-placement.test.ts @@ -1,32 +1,87 @@ import { describe, expect, it } from "vitest"; -import { GRID_COLS, firstFreeCell, occupiedCells } from "./grid-placement"; +import { + ICON_FOOTPRINT_H, + ICON_FOOTPRINT_W, + footprintsOverlap, +} from "../../shared/dashboard-icon-grid"; +import { firstFreeCell, occupiedCells } from "./grid-placement"; describe("occupiedCells", () => { - it("collects only integer-cell records (legacy pixel records occupy nothing)", () => { - const taken = occupiedCells({ - a: { x: 0, y: 0 }, - b: { x: 3, y: 1 }, - pixel: { x: 12.5, y: 40.2 }, // legacy float — ignored - }); - expect([...taken].sort()).toEqual(["0:0", "3:1"]); + it("keeps every record — a dropped record is one the placer would land on", () => { + expect( + occupiedCells({ + a: { x: 0, y: 0 }, + b: { x: 11, y: 0 }, + fractional: { x: 12.5, y: 40.2 }, + }), + ).toEqual([ + { col: 0, row: 0 }, + { col: 11, row: 0 }, + { col: 12, row: 40 }, + ]); + }); + + it("clamps negatives to the origin", () => { + expect(occupiedCells({ a: { x: -4, y: -1 } })).toEqual([{ col: 0, row: 0 }]); + }); + + it("skips non-finite records", () => { + expect(occupiedCells({ a: { x: Number.NaN, y: 0 } })).toEqual([]); }); it("is empty for no icons", () => { - expect(occupiedCells({}).size).toBe(0); + expect(occupiedCells({}).length).toBe(0); }); }); describe("firstFreeCell", () => { it("returns 0:0 on an empty grid", () => { - expect(firstFreeCell(new Set())).toEqual({ col: 0, row: 0 }); + expect(firstFreeCell([])).toEqual({ col: 0, row: 0 }); + }); + + it("steps a whole icon footprint, not one grid unit (POLISH-LAY-6)", () => { + // The regression: with Notes at 0,0 the OLD 12-column 1×1-cell scan + // answered {col:1,row:0} — inside Notes' 11×14-unit box — so the newly + // installed app's tile and label printed on top of it. + const cell = firstFreeCell([{ col: 0, row: 0 }]); + expect(cell).toEqual({ col: ICON_FOOTPRINT_W, row: 0 }); + expect(footprintsOverlap(cell, { col: 0, row: 0 })).toBe(false); + }); + + it("never overlaps ANY icon of a populated seeded fleet", () => { + // The real seeded layout the dry-run captured: one icon every + // ICON_FOOTPRINT_W across, wrapping every ICON_FOOTPRINT_H down. + const fleet = [ + { col: 0, row: 0 }, + { col: ICON_FOOTPRINT_W, row: 0 }, + { col: 2 * ICON_FOOTPRINT_W, row: 0 }, + { col: 3 * ICON_FOOTPRINT_W, row: 0 }, + { col: 0, row: ICON_FOOTPRINT_H }, + ]; + const cell = firstFreeCell(fleet); + for (const icon of fleet) expect(footprintsOverlap(cell, icon)).toBe(false); + }); + + it("wraps to the next footprint row when the first row is full", () => { + const row0 = Array.from({ length: 6 }, (_, c) => ({ col: c * ICON_FOOTPRINT_W, row: 0 })); + // Six columns is not the whole (unbounded) row, so the placer keeps + // going across before it wraps. + expect(firstFreeCell(row0)).toEqual({ col: 6 * ICON_FOOTPRINT_W, row: 0 }); }); - it("scans row-major, skipping taken cells", () => { - expect(firstFreeCell(new Set(["0:0", "1:0"]))).toEqual({ col: 2, row: 0 }); + it("places clear of an icon the user dragged off the slot lattice", () => { + // Free drags snap to the 8px grid, not to install slots — a dragged + // icon at 3,3 still blocks the 0,0 slot it overlaps. + const dragged = { col: 3, row: 3 }; + const cell = firstFreeCell([dragged]); + expect(footprintsOverlap(cell, dragged)).toBe(false); }); - it("wraps to the next row when the first is full", () => { - const fullRow0 = new Set(Array.from({ length: GRID_COLS }, (_, c) => `${c}:0`)); - expect(firstFreeCell(fullRow0)).toEqual({ col: 0, row: 1 }); + it("fills a hole left by an uninstalled app before extending the grid", () => { + const fleet = [ + { col: 0, row: 0 }, + { col: 2 * ICON_FOOTPRINT_W, row: 0 }, + ]; + expect(firstFreeCell(fleet)).toEqual({ col: ICON_FOOTPRINT_W, row: 0 }); }); }); diff --git a/packages/shell/src/main/dashboard/grid-placement.ts b/packages/shell/src/main/dashboard/grid-placement.ts index b3160dec..51ec511f 100644 --- a/packages/shell/src/main/dashboard/grid-placement.ts +++ b/packages/shell/src/main/dashboard/grid-placement.ts @@ -1,33 +1,37 @@ /** * Dashboard grid placement — the single source of "where does a new - * pinned tile land". Extracted so the entity-pin path (dashboard-service) - * and the shell-surface-pin path (Bin overlay) compute the first free - * cell identically (DRY — previously inlined only in dashboard-service). + * pinned tile land" on the MAIN side. Every install path (marketplace, + * sideload, vault-source install, dev seeder) and every pin path + * (dashboard-service entity pin, Bin overlay) computes its cell here. * - * Pure: same icon map → same cell. `GRID_COLS` mirrors the renderer's - * fixed-width grid (`dashboard/grid.ts`). + * The geometry itself is NOT defined here: it lives in + * `shared/dashboard-icon-grid`, which the renderer's `dashboard/grid.ts` + * also builds on. This module is only the main-side adapter from a stored + * icon map to that placer. + * + * That indirection is the POLISH-LAY-6 fix. Main used to scan a fictional + * 12-column grid of 1×1 cells while the renderer stores 8px grid units and + * paints an 11×14-cell footprint per icon — so with Notes at `0,0` the cell + * `1,0` looked free and a freshly installed app's tile landed on top of it. + * + * Pure: same icon map → same cell. */ -export const GRID_COLS = 12; +import { + type IconGridCell, + firstFreeIconCell, + occupiedIconCells, +} from "../../shared/dashboard-icon-grid"; + +export type { IconGridCell }; -/** Integer-cell coordinates already occupied by an icon. Legacy pixel - * records (non-integer x/y, rare, renderer-migrated) occupy no cell — - * the renderer's collision layer handles any residual display overlap. */ -export function occupiedCells(icons: Record): Set { - const taken = new Set(); - for (const icon of Object.values(icons)) { - if (Number.isInteger(icon.x) && Number.isInteger(icon.y)) { - taken.add(`${icon.x}:${icon.y}`); - } - } - return taken; +/** Footprint origins already occupied by an icon, in 8px grid units. */ +export function occupiedCells(icons: Record): IconGridCell[] { + return occupiedIconCells(icons); } -/** First free `{col, row}` scanning row-major over the fixed-width grid. */ -export function firstFreeCell(taken: ReadonlySet): { col: number; row: number } { - for (let row = 0; ; row++) { - for (let col = 0; col < GRID_COLS; col++) { - if (!taken.has(`${col}:${row}`)) return { col, row }; - } - } +/** First install slot (row-major, footprint-stepped) whose icon box clears + * every existing icon's box. */ +export function firstFreeCell(occupied: readonly IconGridCell[]): IconGridCell { + return firstFreeIconCell(occupied); } diff --git a/packages/shell/src/main/dev/seed-demo-apps.ts b/packages/shell/src/main/dev/seed-demo-apps.ts index d169647a..55efbabb 100644 --- a/packages/shell/src/main/dev/seed-demo-apps.ts +++ b/packages/shell/src/main/dev/seed-demo-apps.ts @@ -35,14 +35,13 @@ import { FIRST_PARTY_APPS, type FirstPartyApp, firstPartyAppById } from "../apps import { DEV_INSTALL_PROVENANCE, type InstallProvenance } from "../apps/install-provenance"; import { AppInstaller } from "../apps/installer"; import type { DashboardStore } from "../dashboard/dashboard-store"; +import { firstFreeCell, occupiedCells } from "../dashboard/grid-placement"; import { pruneOrphanAppIcons } from "../dashboard/prune-orphan-app-icons"; import { getActiveShortcutRegistry } from "../shortcuts/active-registry"; import { AppsRepository } from "../storage/registry-repo/apps-repo"; import { getActiveVaultSession } from "../vault/session"; import { isBundleBuildStale } from "./bundle-staleness"; -const GRID_COLS = 12; - // Re-exported so existing import sites (`./seed-demo-apps`) keep working; // the canonical catalog now lives in `../apps/first-party`. export { FIRST_PARTY_APPS, type FirstPartyApp } from "../apps/first-party"; @@ -310,14 +309,15 @@ export async function reinstallFirstPartyApp( return outcome.ok ? { ok: true } : { ok: false, reason: outcome.reason }; } -/** Place a dashboard icon at the next free grid cell. Returns false when +/** Place a dashboard icon at the next free install slot. Returns false when * an icon already targets this app (no-op for re-seeds). * - * Wire format is cell-index `{x: col, y: row}` (small integers) — matches - * the renderer's grid in `packages/shell/src/renderer/dashboard/grid.ts`. - * Storing pixel coordinates here would put `col=0` (`x=16`) below the - * legacy-detection threshold and strand it as `col=16` on the rendered - * grid (clamped off-screen). */ + * Wire format is 8px grid units `{x: col, y: row}` — the same units the + * renderer paints with. The free slot comes from the SHARED placer + * (`shared/dashboard-icon-grid` via `dashboard/grid-placement`), which knows + * an icon's box spans `ICON_FOOTPRINT_W × ICON_FOOTPRINT_H` units: a + * local "next free 1×1 cell" scan dropped a new app straight on top of + * Notes (POLISH-LAY-6). */ export function placeDashboardIcon( dashboard: DashboardStore, appId: string, @@ -325,30 +325,18 @@ export function placeDashboardIcon( ): boolean { // The user removed this app's icon on purpose — don't resurrect it. if (dashboard.isAppIconDismissed(appId)) return false; - const existingIcons = Object.entries(dashboard.snapshot().icons); - if (existingIcons.some(([, icon]) => icon.target === appId)) return false; - - const occupied = new Set(existingIcons.map(([, icon]) => keyFor(icon.x, icon.y))); - let cursor = 0; - while (true) { - const col = cursor % GRID_COLS; - const row = Math.floor(cursor / GRID_COLS); - if (!occupied.has(keyFor(col, row))) { - dashboard.upsertIcon(`icon_${appId}_demo`, { - x: col, - y: row, - kind: "app", - target: appId, - label, - }); - return true; - } - cursor += 1; - } -} - -function keyFor(x: number, y: number): string { - return `${x}:${y}`; + const existingIcons = dashboard.snapshot().icons; + if (Object.values(existingIcons).some((icon) => icon.target === appId)) return false; + + const cell = firstFreeCell(occupiedCells(existingIcons)); + dashboard.upsertIcon(`icon_${appId}_demo`, { + x: cell.col, + y: cell.row, + kind: "app", + target: appId, + label, + }); + return true; } async function pathExists(path: string): Promise { diff --git a/packages/shell/src/main/index.ts b/packages/shell/src/main/index.ts index 5819706f..33b4cabe 100644 --- a/packages/shell/src/main/index.ts +++ b/packages/shell/src/main/index.ts @@ -55,6 +55,13 @@ import { createGeminiProvider } from "./ai/gemini-provider"; import { type OllamaHttp, createOllamaProvider } from "./ai/ollama-provider"; import { createOpenAiProvider } from "./ai/openai-provider"; import { ProviderRegistry } from "./ai/provider-registry"; +import { + AppIconMiss, + type AppIconResolution, + appIconMissStatus, + isAppIconHit, + resolveAppIconInBundle, +} from "./apps/app-icon-resolver"; import { maskAppWindowsForLock } from "./apps/app-window-lock"; import { setAppsChangedTarget } from "./apps/apps-changed"; import { wireExternalLinkRouting } from "./apps/external-link-routing"; @@ -556,8 +563,11 @@ function registerBrainstormProtocol(): void { } // brainstorm://app-icon/ — manifest-declared icon asset. // The handler reads the installed bundle's manifest, validates the - // icon path stays inside the bundle, and streams it. Apps without a - // declared icon get 404 — the renderer falls back to initials. + // icon path stays inside the bundle, and streams it. An app that + // declares NO icon answers 204 (no content, not an error) so the + // caller's initials fallback runs without a console 404 per surface + // per boot (POLISH-LAY-8); an id that resolves to no installed app + // still 404s, because that IS worth surfacing. if (url.host === "app-icon") { const session = getActiveVaultSession(); if (!session) return new Response(null, { status: 404 }); @@ -565,9 +575,11 @@ function registerBrainstormProtocol(): void { if (!appId || appId.includes("..") || appId.includes("/")) { return new Response(null, { status: 400 }); } - const target = await resolveAppIconPath(appId); - if (!target) return new Response(null, { status: 404 }); - const response = await net.fetch(pathToFileURL(target).toString()); + const resolved = await resolveAppIconPath(appId); + if (!isAppIconHit(resolved)) { + return new Response(null, { status: appIconMissStatus(resolved.miss) }); + } + const response = await net.fetch(pathToFileURL(resolved.path).toString()); const headers = new Headers(response.headers); // A `?v=` request is immutable: the renderer cache-busts on // app update, so the bytes for a given version never change. Caching @@ -1175,27 +1187,15 @@ async function drainAssetUploads(): Promise { } } -async function resolveAppIconPath(appId: string): Promise { +/** Locate the active bundle for an app id, then resolve its declared icon + * inside it. The resolution rules (and why a no-icon app is NOT a 404) live + * in `apps/app-icon-resolver`. */ +async function resolveAppIconPath(appId: string): Promise { const session = getActiveVaultSession(); - if (!session) return null; + if (!session) return { miss: AppIconMiss.Unresolved }; const registry = await session.dataStores.open("registry"); const repo = new AppsRepository(registry); - const record = repo.getActive(appId); - if (!record) return null; - let manifest: { icon?: unknown }; - try { - const raw = await readFile(join(record.bundleDir, "manifest.json"), "utf8"); - manifest = JSON.parse(raw) as { icon?: unknown }; - } catch { - return null; - } - if (typeof manifest.icon !== "string" || manifest.icon.length === 0) return null; - if (manifest.icon.includes("..")) return null; - const target = normalize(join(record.bundleDir, manifest.icon)); - if (!target.startsWith(record.bundleDir + sep) && target !== record.bundleDir) { - return null; - } - return target; + return resolveAppIconInBundle(repo.getActive(appId)?.bundleDir ?? null); } function resolveIconPath(): string { diff --git a/packages/shell/src/preload/index.ts b/packages/shell/src/preload/index.ts index 4c0021d2..bc3f313b 100644 --- a/packages/shell/src/preload/index.ts +++ b/packages/shell/src/preload/index.ts @@ -44,6 +44,7 @@ import { } from "@brainstorm-os/sdk-types"; import type { ThemeName } from "@brainstorm-os/tokens"; import { contextBridge, ipcRenderer } from "electron"; +import { appIconUrl } from "../shared/app-icon-url"; export type { AppearanceMode, AppearancePair, AppearanceSlot, AppearanceState }; export type { ReleaseInfo, UpdateCheckResult, UpdatePrefs }; @@ -1097,10 +1098,7 @@ const apps = { * app-derived caches (widget titles + entries, icon cache) on this edge * instead of racing the installer once at mount (F-380). */ onChanged: (listener: () => void): (() => void) => subscribe("apps:changed", listener), - iconUrl: (appId: string, version?: string): string => { - const base = `brainstorm://app-icon/${encodeURIComponent(appId)}`; - return version ? `${base}?v=${encodeURIComponent(version)}` : base; - }, + iconUrl: (appId: string, version?: string): string => appIconUrl(appId, version), launch: (appId: string): Promise => ipcRenderer.invoke("apps:launch", appId), uninstall: (appId: string): Promise => ipcRenderer.invoke("apps:uninstall", appId), diff --git a/packages/shell/src/renderer/bin/bin.tsx b/packages/shell/src/renderer/bin/bin.tsx index 1d0a59c9..e81f4e0c 100644 --- a/packages/shell/src/renderer/bin/bin.tsx +++ b/packages/shell/src/renderer/bin/bin.tsx @@ -14,6 +14,8 @@ import { Orientation, useCompositeKeyboard, useFocusTrap } from "@brainstorm-os/ import { useVirtualizer } from "@tanstack/react-virtual"; import { motion } from "framer-motion"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import { occupiedIconCells } from "../../shared/dashboard-icon-grid"; +import { firstFreeCell } from "../dashboard/grid"; import { ShellSurfaceId, shellSurfacePinIconId } from "../dashboard/shell-surfaces"; import { t } from "../i18n/t"; import { Button, ButtonVariant } from "../ui/button"; @@ -86,12 +88,17 @@ export function Bin({ onClose }: BinProps) { setPinned(false); return; } - // {0,0} is a deterministic seed; the dashboard's collision layer - // repositions onto the first free cell on display (the same - // guarantee 7.13 entity pins rely on). + // Place on the first free install slot, exactly as the app-pin and + // install paths do. {0,0} used to be written as a "deterministic + // seed the collision layer repositions" — but icon layout is FREE + // placement with no collision resolution, so that seed dropped the + // Bin tile on top of whatever sits at the origin (POLISH-LAY-6's + // family). + const snap = await window.brainstorm.dashboard.snapshot(); + const cell = firstFreeCell(occupiedIconCells(snap?.icons ?? {})); await window.brainstorm.dashboard.upsertIcon(pinId, { - x: 0, - y: 0, + x: cell.col, + y: cell.row, kind: "shell-surface", target: ShellSurfaceId.Bin, label: t("shell.bin.title"), diff --git a/packages/shell/src/renderer/dashboard.tsx b/packages/shell/src/renderer/dashboard.tsx index 33f4d35c..f0152acf 100644 --- a/packages/shell/src/renderer/dashboard.tsx +++ b/packages/shell/src/renderer/dashboard.tsx @@ -26,6 +26,7 @@ import { import { AnimatePresence, motion } from "framer-motion"; import { Suspense, lazy, memo, useCallback, useEffect, useMemo, useRef, useState } from "react"; import type { DashboardIcon, InstalledApp, VaultEntry, VaultSessionMeta } from "../preload"; +import { occupiedIconCells } from "../shared/dashboard-icon-grid"; import { onSystemPreferenceChange, systemPrefersDark } from "./dashboard/appearance-watcher"; import "./dashboard.css"; import { AppGrid } from "./dashboard/app-grid"; @@ -520,11 +521,7 @@ export function Dashboard() { const onPinApp = useCallback( (app: InstalledApp) => { const id = `icon_${app.id}_${Date.now()}`; - const occupied = Object.values(snapshot?.icons ?? {}).map(({ x, y }) => ({ - col: Math.max(0, Math.floor(x)), - row: Math.max(0, Math.floor(y)), - })); - const cell = firstFreeCell(occupied); + const cell = firstFreeCell(occupiedIconCells(snapshot?.icons ?? {})); void window.brainstorm.dashboard.upsertIcon(id, { x: cell.col, y: cell.row, diff --git a/packages/shell/src/renderer/dashboard/app-icon-cache.test.ts b/packages/shell/src/renderer/dashboard/app-icon-cache.test.ts index 7927ffc0..b78ff8b7 100644 --- a/packages/shell/src/renderer/dashboard/app-icon-cache.test.ts +++ b/packages/shell/src/renderer/dashboard/app-icon-cache.test.ts @@ -1,7 +1,7 @@ /** * @vitest-environment jsdom */ -import { beforeAll, describe, expect, it } from "vitest"; +import { describe, expect, it } from "vitest"; import { APP_ICON_CACHE_KEY, appHasIcon, @@ -11,19 +11,6 @@ import { setAppIcons, } from "./app-icon-cache"; -beforeAll(() => { - // The module reads `window.brainstorm.apps.iconUrl` when resolving a src. - Object.defineProperty(window, "brainstorm", { - configurable: true, - value: { - apps: { - iconUrl: (appId: string, version?: string): string => - version ? `brainstorm://app-icon/${appId}?v=${version}` : `brainstorm://app-icon/${appId}`, - }, - }, - }); -}); - describe("app-icon cache", () => { // Module state starts cold (nothing persisted at import) — this case can // only be observed before the first `setAppIcons`, so it runs first. @@ -62,6 +49,21 @@ describe("app-icon cache", () => { expect(changed).toBe(false); }); + // POLISH-LAY-8 — the widget header wants a per-install-epoch retry (F-380) + // AND the icon-less suppression. It used to hand-build its own URL to get + // the first, and so lost the second. + it("appends the caller's apps-changed epoch to a versioned URL", () => { + setAppIcons([{ id: "com.example.widget", hasIcon: true, version: "1.0.0" }]); + expect(resolveAppIconSrc("com.example.widget", 3)).toBe( + "brainstorm://app-icon/com.example.widget?v=1.0.0&e=3", + ); + }); + + it("suppresses an epoch request for a known icon-less app too", () => { + setAppIcons([{ id: "com.example.noart", hasIcon: false, version: "1.0.0" }]); + expect(resolveAppIconSrc("com.example.noart", 3)).toBeNull(); + }); + it("reports a change when a version bumps", () => { setAppIcons([{ id: "com.example.bump", hasIcon: true, version: "1.0.0" }]); const changed = setAppIcons([{ id: "com.example.bump", hasIcon: true, version: "2.0.0" }]); diff --git a/packages/shell/src/renderer/dashboard/app-icon-cache.ts b/packages/shell/src/renderer/dashboard/app-icon-cache.ts index 992063fd..4c5fa8c1 100644 --- a/packages/shell/src/renderer/dashboard/app-icon-cache.ts +++ b/packages/shell/src/renderer/dashboard/app-icon-cache.ts @@ -10,6 +10,8 @@ * (the protocol handler serves versioned requests `immutable`). */ +import { appIconUrl } from "../../shared/app-icon-url"; + export const APP_ICON_CACHE_KEY = "brainstorm.dashboard.app-icons.v1"; // `null` until the first authoritative `apps.listInstalled()` of a fresh @@ -71,14 +73,25 @@ function sameMap(a: Map, b: Map): boolean { } /** - * Resolve the `src` for an app squircle. Returns a versioned (cacheable) URL - * for a known-iconed app, `null` for a known-iconless one (so no wasted 404), + * Resolve the `src` for an app squircle — the ONE place a renderer surface + * turns an app id into an icon URL. Returns a versioned (cacheable) URL for a + * known-iconed app, `null` for a known-iconless one (so no wasted request), * and an optimistic unversioned URL while the cache is still cold — the * AppIcon's gradient+initials paint behind it, so an optimistic miss degrades * to the placeholder rather than a broken tile. + * + * Hand-built `brainstorm://app-icon/` URLs are what made the running + * strip, the window switcher and the widget headers each fire their own 404 + * for an app that ships no artwork (POLISH-LAY-8) — they all come through + * here now. + * + * `epoch` (optional) appends the caller's apps-changed counter so a load that + * raced an (re)install retries on the next edge instead of pinning the + * initials fallback for the session (F-380 / POLISH-LAY-3). */ -export function resolveAppIconSrc(appId: string): string | null { - if (appHasIcon(appId)) return window.brainstorm.apps.iconUrl(appId, appIconVersion(appId)); - if (appIconsKnown()) return null; - return window.brainstorm.apps.iconUrl(appId); +export function resolveAppIconSrc(appId: string, epoch?: number): string | null { + if (appIconsKnown() && !appHasIcon(appId)) return null; + const base = appIconUrl(appId, appIconVersion(appId)); + if (epoch === undefined) return base; + return `${base}${base.includes("?") ? "&" : "?"}e=${epoch}`; } diff --git a/packages/shell/src/renderer/dashboard/grid.ts b/packages/shell/src/renderer/dashboard/grid.ts index 528ae4ed..3f36cd47 100644 --- a/packages/shell/src/renderer/dashboard/grid.ts +++ b/packages/shell/src/renderer/dashboard/grid.ts @@ -17,7 +17,27 @@ * `layoutIcons`. */ -export const GRID_UNIT = 8; +import { + GRID_UNIT, + ICON_BUTTON_H, + ICON_BUTTON_W, + ICON_FOOTPRINT_H, + ICON_FOOTPRINT_W, + firstFreeIconCell, +} from "../../shared/dashboard-icon-grid"; + +/** The placement geometry lives in `shared/dashboard-icon-grid` so the main + * process places a newly installed tile with the SAME units and the SAME + * occupancy test the renderer paints with (POLISH-LAY-6). Re-exported here so + * every existing renderer call site keeps importing from `./grid`. */ +export { + GRID_UNIT, + ICON_BUTTON_W, + ICON_BUTTON_H, + ICON_FOOTPRINT_W, + ICON_FOOTPRINT_H, +} from "../../shared/dashboard-icon-grid"; + export const GRID_OUTER_MARGIN = 16; /** A layout whose every icon sits within this many cells of the origin is a @@ -41,16 +61,11 @@ export function getCellSize(_viewport?: GridPoint): GridSize { * `cell` argument is ignored, kept for call-site compatibility. */ export type IconSize = { w: number; h: number; tile: number }; -/** Fixed icon-button box (tile + two label lines) and inner tile. Keep in - * lockstep with `--grid-icon-w/-h` + the tile size in `icons-layer.css`. */ -export const ICON_BUTTON_W = 80; -export const ICON_BUTTON_H = 104; +/** Inner tile of the icon button. The button box itself (`ICON_BUTTON_W/H`) and + * its footprint come from the shared placement module re-exported above; keep + * all three in lockstep with `--grid-icon-w/-h` + the tile size in + * `icons-layer.css`. */ export const ICON_TILE = 56; -/** Footprint of one icon, in `GRID_UNIT` cells (button box + a 1-cell gutter) — - * the slot spacing the install placer (`firstFreeCell`) steps by, and the box it - * checks for overlap. Free drags ignore this; only new installs avoid piling. */ -export const ICON_FOOTPRINT_W = Math.ceil(ICON_BUTTON_W / GRID_UNIT) + 1; -export const ICON_FOOTPRINT_H = Math.ceil(ICON_BUTTON_H / GRID_UNIT) + 1; /** * Vertical slot reserved *below the tile, above the label* by every icon @@ -109,29 +124,10 @@ export function clampCell(cell: GridCell): GridCell { return { col: Math.max(0, Math.floor(cell.col)), row: Math.max(0, Math.floor(cell.row)) }; } -/** Do two icon footprints (top-left cells `a`, `b`, each `ICON_FOOTPRINT_W` × - * `ICON_FOOTPRINT_H`) overlap? Used only to place a NEW icon clear of existing - * ones — user drags are free and never overlap-checked. */ -function footprintsOverlap(a: GridCell, b: GridCell): boolean { - return ( - a.col < b.col + ICON_FOOTPRINT_W && - b.col < a.col + ICON_FOOTPRINT_W && - a.row < b.row + ICON_FOOTPRINT_H && - b.row < a.row + ICON_FOOTPRINT_H - ); -} - -/** First free install slot: scan footprint-stepped slots (row-major) for one - * whose footprint clears every occupied icon. New installs land in a tidy grid; - * the user can then freely drag them anywhere on the 8px grid. */ +/** First free install slot — the shared placer, under the name the renderer's + * existing call sites already use. */ export function firstFreeCell(occupied: readonly GridCell[]): GridCell { - for (let r = 0; r < 1024; r++) { - for (let c = 0; c < 1024; c++) { - const slot = { col: c * ICON_FOOTPRINT_W, row: r * ICON_FOOTPRINT_H }; - if (!occupied.some((o) => footprintsOverlap(slot, o))) return slot; - } - } - return { col: 0, row: 1024 * ICON_FOOTPRINT_H }; + return firstFreeIconCell(occupied); } // --- Widget grid (Stage 7.3 / 7.3b) --- diff --git a/packages/shell/src/renderer/dashboard/widgets-layer.tsx b/packages/shell/src/renderer/dashboard/widgets-layer.tsx index faa916ed..e47e7852 100644 --- a/packages/shell/src/renderer/dashboard/widgets-layer.tsx +++ b/packages/shell/src/renderer/dashboard/widgets-layer.tsx @@ -28,6 +28,7 @@ import { launchApp } from "../analytics/track-app-launch"; import { t } from "../i18n/t"; import { Icon, IconName } from "../ui/icon"; import { AppIcon } from "./app-icon"; +import { resolveAppIconSrc } from "./app-icon-cache"; import { GRID_OUTER_MARGIN, type GridCell, @@ -849,14 +850,11 @@ const WidgetCard = memo(function WidgetCardImpl(props: WidgetCardProps) { request racing the installer's uninstall→install window 404s, AppIcon falls back to initials, and a constant src pins that fallback for the whole session (the same F-380 edge the - iframe entry and titles already re-resolve for). */} - + iframe entry and titles already re-resolve for). The shared + resolver answers `null` for an app that ships no icon, so an + icon-less app's widget draws initials with no request at all + (POLISH-LAY-8). */} + {title}