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 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.
- **`titleBarStyle` is construction-time only.** Switching skin cannot
  re-frame a live window; `chromeOptions` is read once per `new
  BrowserWindow`. The options row tells the user to reopen. Don't "fix" this
  by recreating the main window — that would drop every session and tab.
- 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.