Files
miniSCP/docs/gotchas.md
T
jgamboaandClaude a83264c77d Rename the darker tone to AMOLED; file icons are always colored
- Tone: the "darker" option is labelled "AMOLED" in all 7 locales — it is
  the true-black AMOLED palette. The persisted value stays "darker", so no
  settings migration.
- File icons: the Plain/Multi option is gone and the per-category glyphs +
  colors are always on. Removed the iconColors setting (type, default,
  signal, setter, options row, strings) and the plain branch in
  file-icons.ts. A stale "iconColors" key in settings.json is carried
  harmlessly and dropped on the next save.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-09-23 16:30:55 +02:00

282 lines
18 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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 a
template calling `i18n.t()` re-renders on a language switch — `t()` reads
the `locale` signal.
- 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`.
- 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.
- **Reach `core/` through the `@core/*` alias, never a relative path.**
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.
- **The build DOES type-check components nothing imports.** ngtsc compiles the
whole program, so an unreferenced component still fails `bun run build:ui` on
a type or template error (verified by injecting an error into an
unreferenced file).
- **`overflow-x: auto` also clips vertically.** CSS refuses to pair `visible`
with a non-visible axis, so the computed `overflow-y` becomes `auto` too:
anything poking out of a horizontally scrolling strip (a raised tab, an
underline at `-bottom-px`) is silently clipped. Reserve padding instead.
## 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
px reservoirs were the panel-chrome tokens (`--row-h` etc.) and 93
`text-[11px]` arbitraries. 1px hairlines and focus rects stay px deliberately
(crisp at any scale).
- **px constants in TS that mirror rem-sized CSS are latent scale bugs.** The
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.
## Electrobun
- **The prebuilt CLI binary ships with a broken ad-hoc signature**, so macOS
SIGKILLs it at exec and `bun run start` prints nothing and exits **0** —
`bin/electrobun.cjs` spawns the real binary and forwards
`process.exit(code || 0)`, and a signal death has `code === null`. The binary
is a Bun single-file executable whose payload is appended *after* the linker
signed it, so `codesign -v` reports "code or signature have been modified";
the arm64 and x64 CLI tarballs are both affected upstream (confirmed by
hashing a fresh download against the shipped copy — it is not local
corruption, and re-installing does not help). `scripts/fix-electrobun-cli.ts`
re-seals it with `codesign --force --sign -`. It runs from `postinstall`
**and** ahead of every electrobun invocation, because the wrapper downloads
the binary **lazily on first use**, not at install time — postinstall alone
would no-op on a fresh checkout. It also fixes `.cache/electrobun`, which the
wrapper restores `bin/` from. Never read a silent `exit 0` from the
electrobun CLI as "nothing happened": run `codesign -v` on it first.
- **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/shadlike/context-menu.ts`) on
every platform; don't reintroduce the native one for "nativeness" on mac/win
without keeping a Linux path.
- **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 and the interface scale 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 branch once replaced (3) with a transparent rule and
the modern skin silently lost it — the other two layers do not cover it.)
- 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.
- **Preloads and frames are construction-time only.** The preload is a
persistent `WebKitUserScript` at DOCUMENT_START, frozen at webview
construction — `webview.loadURL()` re-runs the OLD one
(`updatePreloadScriptToWebView` is only reachable from electrobun's internal
`<electrobun-webview>` handler). `titleBarStyle`/`transparent` have no
setters. Anything `buildPreload` bakes is a first-paint snapshot the UI must
correct live over RPC. Recreating the main window is not an escape hatch:
`exitOnLastWindowClosed`, push closures and the tray capture it, and there
is no `off()`.
- **Drag regions ignore `preventDefault`.** The preload starts a window move
from a bubble-phase `mousedown` on `document` and never reads
`defaultPrevented`; a `pointerdown` handler doesn't stop it either (the
compat `mousedown` still fires). `isAppRegionDrag` checks
`.electrobun-webkit-app-region-no-drag` first — that class on the element is
the only reliable suppressor (`dist/api/bun/preload/dragRegions.ts`).
- **No resize, maximize-event or double-click-maximize primitives (1.18.1).**
`titleBarStyle: "hidden"` drops the whole frame (Cocoa `Titled` — the only
way to remove the traffic lights; `gtk_window_set_decorated(false)` on
Linux) and with it the WM resize borders; nothing restores them (no
`startWindowResize`, no `gtk_window_begin_resize_drag`, no
`setResizable`/min-size options — only `setFrame`/`getFrame`). No maximize
event exists (derive it from `resize` + `isMaximized()`), and drag regions
don't double-click-maximize. A custom-chrome window has to build all three.
## 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. That is only
half the trap: `pkill -f dev-linux-x64` does the same thing, because the
pattern appears in the killing command's OWN argv. Break the literal with a
character class — `pkill -f 'dev-linux-x[6]4'` — the way `ps | grep [x]yz`
has always worked.
- **Launching the app steals focus — ask first.** `bun run start` opens a real
window that takes focus, which interrupts whatever the user is doing; there
is no Xvfb/xvfb-run on this machine and Wayland offers no
launch-without-focus. Batch every live check into a single launch.
- 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).
## 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.
- **Every window's RPC must serve `saveSettings`.** The scale shortcuts
shipped in editor and manager windows while only the main RPC served it:
the request went unanswered, the change applied locally, and main's next
broadcast snapped it back. All three factories now share
`saveSettingsHandler`, and `settingsSaved()` in index.ts rebroadcasts to
every window but the sender — an echo would drag a held Cmd+ backwards.
- 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.