On the NVIDIA proprietary driver the app booted, logged "miniSCP started", reported no error — and painted a completely black window, with stderr showing "Failed to create GBM buffer of size WxH: Invalid argument" and an X11 GLXBadWindow. WebKitGTK's DMABUF renderer cannot allocate buffers there; WEBKIT_DISABLE_DMABUF_RENDERER=1 is the upstream workaround. Pre-existing on Linux, unrelated to the context-menu change — it just had never been looked at because nothing on Linux had been run before. 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), so setting it in src/bun/index.ts is inert even from a module imported before `electrobun/bun`. It has to be in the environment before the app process starts, which means every launch path must inject it: - scripts/linux-gpu.ts — shared `launchEnv()` helper, gated on /proc/driver/nvidia so Intel/AMD keep hardware compositing, and never overriding an explicit value from the user's environment. - scripts/dev.ts — uses it for `bun run dev`. - scripts/start.ts (new) — `bun run start` previously called `bunx electrobun dev` straight from package.json, where no env could be set; it now goes through a script like dev does. - scripts/release.ts — writes a `miniSCP.sh` wrapper beside bin/launcher in the Linux tarball, since a packaged app goes through none of the above. Documented in gotchas.md (including that a "does it boot?" log check does NOT catch this — the log looks healthy) and platform-notes.md, with the smoke test updated to require that the UI actually renders. Verified: backend `tsc --noEmit` clean, `bun run build:ui` clean, and `bun run start` now boots with zero GBM/GLX errors. Co-Authored-By: Claude <noreply@anthropic.com>
170 lines
10 KiB
Markdown
170 lines
10 KiB
Markdown
# 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`.
|
||
|
||
## 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/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.
|
||
- 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.
|
||
- 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.
|