miniSCP/docs/gotchas.md

# Gotchas — landmines this codebase has already stepped on

Every one of these caused a real bug or debugging spiral. Read before
touching the adjacent code.

## Angular (v22, zoneless, signals, OnPush)

- **Never consume push streams through signal+effect.** An effect that reads
  a signal and (directly or transitively) writes signals re-triggers itself —
  this froze the entire app on the first transfer. Push messages
  (transferUpdate etc.) go through plain callback registries in RpcService.
  Signal coalescing would also silently drop intermediate progress snapshots.
- **No signal writes during template render** (NG0600). Anything that loads
  panels from code paths reachable during render defers with
  `queueMicrotask` (see `PanelsService.newWorkspace`).
- **`[value]` on `<select>` is applied before `@for` options render** and
  silently selects nothing. Use per-option `[selected]` bindings (connection
  manager stack picker).
- Template bindings with special characters: `[class.bg-primary/15]` works;
  `[class.shadow-[...]]` (brackets) does NOT parse — use `[style.box-shadow]`.
  Static class attributes may use arbitrary values freely.
- Functions called from templates ARE reactive: signals read inside them
  register on the view's consumer (call-stack-based tracking). That's why
  `iconClass(entry)` reacting to `settings.iconColors()` works.
- Dedicated tools > shell: IDE diagnostics between paired template/class
  edits are often stale mid-sequence — trust `bun run build:ui` (full AOT
  type-check), not the first red squiggle.
- Tailwind v4 canonical class names: `wrap-break-word` not `break-words`,
  `accent-primary` not `accent-[var(--primary)]`, `z-60` not `z-[60]`.
  Every color used in a template must resolve through `@theme inline`.
- **The win7 skin layer outranks Tailwind utilities.** 7.css is imported
  `layer(skin)`, ordered above `utilities`, so any utility you add to a
  `button`/`input`/`select`/`textarea`/`table` is silently dead under that
  skin. If it matters, neutralize in the `@layer skin` block in
  `app/skins/7like/7like.css` — and note `revert-layer` only reaches
  `utilities` from **inside `skin`**; used from `skinfix` it falls back onto
  7.css and is a no-op. This bit once already.
- `@import … layer(name)` works through `@tailwindcss/postcss` (it wraps the
  imported AST), but the **deep path is mandatory**: 7.css ships no `exports`
  map and Tailwind's resolver reads only `style`/`mainFields`, so a bare
  `@import "7.css"` resolves to nothing. Use `7.css/dist/7.scoped.css`.
- Stylesheet `url()` must resolve at BUILD time: assets referenced from
  `styles.css` live under `ui/src/` (esbuild emits them hashed into `media/`
  and rewrites the url), NOT `ui/public/` — a public path fails the build with
  "Could not resolve".
- **Two `class` attributes on one element: the second is silently dropped.**
  No warning, no build error — the layout classes simply never apply. This
  collapsed the tab strip and two window headers at once (`fe028a9`), because
  a scripted edit appended `class="…"` to elements that already had one.
  When removing a conditional binding, check what the branch *contained*: the
  bindings there were genuinely dead, but the layout classes inside them were
  not, and nothing replaced them.
- **A component `template:` is a TS template literal, so no backticks inside
  it** — not even in an HTML comment. A backtick in a comment terminates the
  literal and the errors point at the prose, not at the quoting.

## The interface scale (root font-size lever)

- **electrobun's native page zoom is a silent no-op on Linux — do not reach for
  it.** `webviewSetPageZoom` in the shipped `libNativeWrapper.so` (v1.18.1, and
  the `_cef.so`, and the copy in our own build output) is a compiled
  empty-body stub: the disassembly has no `call` instruction, and
  `webkit_web_view_set_zoom_level` is not even in the library's imports.
  `webviewGetPageZoom` returns a hardcoded `1.0` from `.rodata`, so the failure
  is unobservable through the API — set 2.0, read back "1.0 = success". The
  JSDoc's "(WebKit only)" means macOS WKWebView. This is why the scale is CSS.
- **Everything the UI sizes with must be rem.** The scale works by setting the
  root font-size, so a px value freezes while everything around it grows. The
  three px reservoirs were: the panel-chrome tokens (`--row-h` etc.), 93
  `text-[11px]` arbitraries, and 7.css itself — 370 px values, zero rem, with
  `--w7-font` reaching only 15 rules' typography. 7like.css's `skinfix` carries
  a scale bridge re-expressing 7.css's vars and hardcoded box metrics in rem.
  1px hairlines and focus rects stay px deliberately (crisp at any scale; the
  `tr` border exists to stop selection shifting rows).
- **Restating a 7.css rule in `skinfix` also outranks 7.css's VARIANTS of that
  rule — by layer, regardless of specificity.** The title-bar-controls
  restatement silently shrank the close button from 48px to 29px (7.css widens
  it via `button[aria-label=Close]`), and the tablist padding flattened the
  active tab's 4px raise. Caught only by the harness pixel diff. When bridging
  a rule, grep 7.scoped.css for every variant that overrides the same
  properties and restate those too.
- **px constants in TS that mirror rem-sized CSS are latent scale bugs.** The
  shadlike context menu's `MENU_W = 224` was a hardcoded mirror of `w-56`
  (14rem): box grew, math didn't, flip fired at the wrong edge. Convert through
  `@core/metrics` `remPx()` at use time — it reads the LIVE root font-size,
  and per call, since the scale changes mid-session. The editor's canvas-based
  `charWidth` is cached keyed on the computed font string for the same reason.

## Hover states and the frame edge

- **A child's box-shadow is NOT clipped by its parent's border or radius.** The
  close button's hover glow (7.css: `0 0 7px 3px #e68e75` on a `::before`)
  painted across the window's top border and onto the desktop beyond it — the
  caption buttons sit flush against the top edge (`border-top: 0`), and the
  frame's rounded transparent corners clip nothing. Fixed with
  `clip-path: inset(0 0 -1rem 0)` on `.w7-frame > .title-bar`: glow stops at
  the frame like the real DWM, bottom bleed (authentic) stays open. Note
  clip-path also clips hit-testing — safe here because the drag region is the
  title-bar's own box, entirely inside the clip.
- **Hover states CAN be tested headlessly**: `scripts/hover-probe.ts` copies
  the built stylesheet with `:hover` rewritten to `.simhover`, renders the
  frame markup with the state forced, and pixel-scans artifact regions,
  exiting non-zero on hover-introduced red outside the frame. Firefox
  `--screenshot` has no pointer and `import -window` captures the real
  pointer's state, so the class rewrite is the only programmatic hover on this
  machine. Re-run it after touching title-bar or hover styling.

## The two skin trees (`ui/src/app/skins/`)

- **`.win7` is not a name we may change.** `7.scoped.css` is precompiled with
  every selector prefixed by that literal class, so the skin id in
  `AppSettings` stays `"win7"` even though its folder is `7like/`. The folder
  names are cosmetic; the class and the persisted setting value are not.
- **Reach `core/` through the `@core/*` alias, never a relative path.** Skin
  components sit at varying depths and get moved; the alias is what let 67
  imports survive two relocations untouched. A `../../core/...` path is a
  future merge conflict.
- **`@font-face` `url()` resolves relative to the file that declares it.** When
  the win7 rules moved three directories deeper, `./fonts/...` would silently
  have 404'd — build still green, app just renders a fallback font. The fonts
  therefore live inside the skin that uses them.
- **The `@layer` statement must stay the first statement in `ui/src/styles.css`.**
  It is one of only two rule types allowed above `@import`, and it is what sorts
  `skin` above `utilities`. Every `@import` (Tailwind and both skin sheets) must
  also precede all other rules.
- **Duplicate selectors across skins are safe.** `app-file-panel` exists in both
  trees; standalone components resolve through their importer's `imports:`
  array, never a global registry, and the trees are never mounted together.
- **The build DOES type-check components nothing imports.** ngtsc compiles the
  whole program, so a skin that is currently unreachable still fails
  `bun run build:ui` on a type or template error. Verified deliberately by
  injecting an error into an unreferenced file — worth knowing before assuming
  a green build proves nothing about the inactive skin.

## The win7 skin (7.css)

- **Unlayered declarations beat every `@layer`, regardless of specificity.**
  `html, body { background: … }` in styles.css is unlayered, so no `skinfix`
  rule can override it — the win7 transparent-corner reset has to be unlayered
  too. The frame's rounded corners only read as rounded because nothing paints
  an opaque full-bleed ground behind them; anything that does fills the corners
  and the window becomes a rectangle with a curve drawn on it.
- **7.css restates `border-color` at (0,3,1)** via
  `.win7 button:not(:disabled):not(:hover)` and its `:hover` twin. A two-class
  selector like `.win7 .w7-flat` (0,2,0) loses to it, so paint suppression for
  flat buttons lives in `skinfix` where layer order settles it. The
  `.w7-toolbar` arm only ever won by tying on specificity and landing later in
  the same layer — an accident `w7-flat` did not share, which is why it
  rendered a bordered slab while its neighbours went flat.
- **`overflow-x: auto` also clips vertically.** CSS refuses to pair `visible`
  with a non-visible axis, so the computed `overflow-y` becomes `auto` too.
  7.css raises the active tab with `margin-top: -2px`; inside the horizontally
  scrolling tab strip that put its top border outside the scroll box and
  deleted it silently. The strip reserves `padding-top: 2px` to compensate —
  mirroring the `padding-left: 3px` 7.css already reserves for the same tab's
  `-3px` left margin.
- **7.css hardcodes light-mode literals outside its own var system** —
  `color: #222` on tablist buttons, black/white bevels in the ListView and
  menu. Each one has to be routed through a `--w7-*` token declared in
  `:root.win7` and overridden in **both** dark blocks (the `[data-theme]`
  attribute one *and* the `prefers-color-scheme` one; system dark sets no
  attribute).
- **Its selectors are exact, and a near-miss looks exactly like "7.css doesn't
  support this".** `menu[role=tablist] button` is a DESCENDANT selector, so
  every button inside the strip becomes a tab — the `+` and the right-hand
  tools must live outside it. `.window.glass > .title-bar` is direct-child, so
  an inner wrapper breaks the glass coupling. A wrong guess here already
  produced one hand-written lookalike that was deleted.

## Electrobun

- **Window `close` is not cancellable** (response type `{}`; only the
  app-level `before-quit` supports `event.response = {allow:false}`). All
  unsaved-state guards must either intercept app-controlled close paths
  (⌘W, buttons) or survive the close (editor draft mirroring).
- **One rpc instance per window** — BrowserView binding steals the rpc's
  transport; a shared instance silently sends to the wrong webview.
- **RPC before socket open is silently lost.** Everything gates on the
  socket-open promise (RpcService `ready`).
- The emitter has no `off()`. Window event closures must not capture records
  you want garbage collected.
- `electrobun build` defaults to the **dev** channel; release builds need
  `--env=stable`. The stable app name differs from the dev one
  (`miniSCP` vs `miniSCP-dev`), including the app-data dir implications when
  testing.
- Tray: never call `setImage` after construction on macOS — the template
  treatment is lost and the icon renders black on dark menu bars.
- `ApplicationMenu` is macOS-only in our usage; accelerators on Windows are
  single-character only.
- **`ContextMenu.showContextMenu` is a silent no-op on Linux** (electrobun's own
  `dist/api/bun/proc/linux.md` says so; the app also prints "Application menus are
  not supported on Linux" at boot). Because the UI suppresses the webview's own
  right-click menu, that left EVERY panel context menu dead on Linux — no error,
  nothing happens. The menu is plain HTML now (`ui/src/app/skins/*/context-menu.ts`) on
  every platform; don't reintroduce the native one for "nativeness" on mac/win
  without keeping a Linux path.

## Linux

- **`ERR_DLOPEN_FAILED` on `bun run start`** means a missing system library, not
  broken app code. `libNativeWrapper.so` links `libayatana-appindicator3.so.1`
  *hard* (for the tray), so the app cannot start at all without it — even though
  the tray is a minor feature. Diagnose with
  `ldd build/dev-linux-x64/*/bin/libNativeWrapper.so | grep 'not found'`; the
  `libasar.so => not found` line in that output is a red herring (it ships beside
  the wrapper and resolves at load). Install list per distro is in
  [platform-notes.md](platform-notes.md).
- **Black window on NVIDIA + `Failed to create GBM buffer of size WxH: Invalid
  argument`** (usually with `X11 Error: GLXBadWindow (code 170)`). The app boots,
  logs "miniSCP started", reports no error — and paints nothing. WebKitGTK's
  DMABUF renderer can't allocate buffers on the NVIDIA proprietary driver;
  `WEBKIT_DISABLE_DMABUF_RENDERER=1` is the upstream workaround.
  - **The app cannot apply this to itself.** WebKit reads the variable when it
    forks its web process, and **Bun's `process.env` writes do not propagate to
    child processes** (verified: a spawned child sees it unset). Setting it in
    `src/bun/index.ts` — even in a module imported before `electrobun/bun` —
    does nothing. It must be in the environment before the app process starts.
  - Hence every launch path injects it: `scripts/linux-gpu.ts` (`launchEnv()`)
    for `bun run dev` / `bun run start`, and the `miniSCP.sh` wrapper that
    `release.ts` writes beside `bin/launcher` in the tarball. **A new launch
    path must call `launchEnv()` or it will render black on NVIDIA.**
  - Gated on `/proc/driver/nvidia` so Intel/AMD keep hardware compositing, and
    an explicit `WEBKIT_DISABLE_DMABUF_RENDERER` from the user always wins.
  - A "does it boot?" log check does NOT catch this — grep for the GBM line, or
    look at the window.
- **`bun run start` leaves the old instance running, and
  `import -window miniSCP` grabs the OLDEST match.** Instances pile up across
  rebuilds, so every screenshot silently comes from a stale build and the fix
  you just made looks like it did nothing. Cost three wrong readings in one
  session, including a "the CSS is in the bundle but does not apply" dead end.
  Kill first (`pkill -f dev-linux-x64`), confirm with
  `pgrep -cf 'dev-linux-x64/miniSCP-dev/bin/launcher'`, then launch and capture.
  Note `pkill -f 'electrobun dev'` also matches the shell running it and kills
  your own command (exit 144) — match on the build path instead.
- **X11 capture cannot answer "is this pixel transparent?"** for the frameless
  window. `import -window` grabs the window's own pixels with no compositing,
  so a transparent corner and an opaque black one both come out black;
  `png32:` just fills alpha with 1 and proves nothing. XWayland's root window
  is not the composited desktop, so `import -window root` fails too. Frame
  transparency and the maximized corners need a human looking at the screen —
  say so rather than reporting a screenshot as confirmation.
- Benign startup noise on Linux — non-fatal, the app runs normally:
  `libayatana-appindicator is deprecated. Please use libayatana-appindicator-glib`
  (upstream electrobun's choice, not ours).
- **Window-open flash prevention is three-layer** — remove any one and the
  white/inverted blink returns: (1) windows are created `hidden: true` and
  revealed by `showWhenReady` on the webview's first `dom-ready` (800 ms
  fallback), (2) the bun preload bakes the persisted `data-theme`/tone/accent
  attributes before first paint (`buildPreload`), (3) `ui/src/index.html`
  carries an inline `<style>` with the html background per theme — its oklch
  values MUST stay in sync with the `--background` tokens in styles.css.
  The win7 skin adds a **fourth value to keep in sync**: `buildPreload` also
  bakes the `.win7` class, and index.html carries a matching `html.win7` arm
  for all four theme states. Miss it and every win7 window flashes white.
- **The skin is a LAUNCH-TIME choice. Never apply one to a running window.**
  This shipped broken once: `SettingsService.applySkin` toggled the `.win7`
  class the moment the setting changed, which was harmless when there was one
  component tree and became a broken window as soon as there were two — 7.css
  painted onto shadlike markup that has no `.window.glass`, no `.title-bar`, no
  `menu[role=tablist]` and no `<table>` ListView. Each skin's CSS is written
  against its OWN tree's markup; the class must only ever be written by
  `buildPreload`, for the tree that window actually booted.
  - **A reload does not rescue it.** The preload is a persistent
    `WebKitUserScript` injected at DOCUMENT_START, so its string is frozen at
    webview construction — `webview.loadURL()` re-runs it with the OLD
    `__APP_SKIN`. (`updatePreloadScriptToWebView` exists in the FFI but is
    reachable only from electrobun's internal `<electrobun-webview>` handler.)
  - **`titleBarStyle`/`transparent` are construction-time only** — no setters
    exist, so a live window can never be re-framed. win7 → modern is the
    dangerous direction: the modern tree draws no title bar, so it would land
    in a frameless transparent window with no way to move, minimize or close.
  - Don't "fix" this by recreating the main window either: `exitOnLastWindowClosed`
    is true, the main `rpc` is captured by six push closures, the tray captures
    `win`, listeners cannot be removed (no `off()`) — and it would drop every
    session and tab.
  - Bun pins it: `initSessionSkin` freezes the skin before any window exists,
    and `buildPreload`/`chromeOptions` read that, never the live settings. Skip
    the pin and a window opened after a pending change boots the other skin.
- Dev vs prod URL: `getMainViewUrl()` resolves the ng-serve dev server only
  when `DEV_SERVER_URL` is set AND the updater channel is `dev` AND the
  server answers a HEAD request; secondary windows must reuse the resolved
  URL, never hardcode `views://`.
- Upstream Bun panic (`FFI_Callback_threadsafe_call` → `web_worker.spin`):
  rare segfault under long sessions; upstream electrobun×Bun issue, restart
  recovers, nothing app-side to fix.

## Domain logic

- **sessionId ≠ filesystem identity.** Two panels on the same server are two
  SSH sessions. Every same-file/nested-path guard compares `fsKey`
  (`resolvedIP:port` / `"local"`); comparing sessionId caused a
  truncate-to-zero data-loss bug. When fsKeys differ but paths collide, the
  probe-file check unmasks aliased hosts. Never "simplify" this back.
- `createWriteStream` truncates BEFORE the source is read — that's why
  self-copy is destructive, and why guards must run before any stream opens.
- The synthetic `'..'` row: type `'dir'`, must be excluded from selection,
  targets(), filtering match-count, kind/rights/owner display, and drag
  sources. Any new row-level feature must decide what `'..'` does.
- `Semaphore.wake()` must reserve slots synchronously; waking waiters that
  increment asynchronously loses budget under concurrent release.
- Settings: the backend spread-merges (`{...DEFAULT_SETTINGS, ...data}`) so
  new keys auto-default, but the UI SettingsService reads field-by-field —
  a new setting silently stays at default until wired in all five spots.
- The editor draft mirror sends full content per tick (2 MB cap upstream);
  don't shrink the 400 ms debounce.
- `panel.touched` gates localDir auto-navigation; `connectToken` guards
  against a slow connect resolving into a panel the user already reused.

## i18n

- **Translations live in `src/shared/i18n/*.ts` (TS, not JSON)** — deliberate:
  it sidesteps `resolveJsonModule` differences between the Angular esbuild and
  Bun builds and gives full typing. `en.ts` is the source of truth; other
  locales are partial and fall back to English then the raw key.
- **`i18n.t()` is reactive because it reads the `locale` signal** — that's what
  makes live language switching work with no pipe. If you cache a translated
  string in a plain field it WON'T update on switch; call `t()` in the template
  (or in a signal-reading getter) instead. Same for `i18n.date()/size()`.
- Dictionary values are TS string literals: **French/other apostrophes break
  single-quoted entries** (`'…c'est…'`). Use double quotes for any value
  containing an apostrophe. Plurals are `{ one, other }` maps (not ICU strings);
  zh/ja supply only `other`.
- **Native menus can't use the Angular service** — `menu.ts` goes through backend
  `i18n.ts`, and the app menu must be rebuilt when the locale changes (wired in
  index.ts `settingsChanged`), or it keeps the old language. The context menu no
  longer has this problem: it is Angular now and reads `I18nService` directly.
- `eu`/`zh`/`ja` are AI drafts pending native review (see the i18n README).
- **Silent auto-refresh must snapshot sessionId+cwd and discard a mismatched
  result.** A normal `refresh()` is masked by `busy` (input blocked), but the
  silent variant (poll/watch) runs while the user can navigate — without the
  guard, a poll that started in dir A resolves and stamps A's listing over the
  dir B the user just opened. It also never calls `scrollCursorIntoView`
  (scroll must stay put) and returns early if `busy()` or already refreshing.

## Build / tooling

- Angular CLI rejects Bun's Node-compat version — `scripts/node-version-shim.ts`
  is preloaded via `bunfig.toml` at the root AND in `ui/`. Removing either
  breaks `ng`.
- ssh2's optional native deps must stay `external: ["cpu-features", "*.node"]`
  in `electrobun.config.ts` — bundling them breaks the build; ssh2 falls back
  to pure JS at runtime.
- `bun run --cwd ui build` (flag order matters; `bun --cwd ui run build` does
  not do what you want).
- `version.gen.ts` is gitignored and regenerated by postinstall + every build
  script; a fresh checkout fails `tsc` until `bun install` (or any script)
  has run once. Never commit it — it would bump the commit count it records.
- Icon pipeline is `scripts/gen-icons.ts` (resvg) — regenerate after touching
  `assets/logo.svg` or `assets/tray/tray-glyph.svg`; the .ico and tray PNGs
  are build inputs, not build outputs.