Files
miniSCP/docs/gotchas.md
T
jgamboaandClaude 6bb4c3cd7d Merge main into feat/7css-skin + give the HTML context menu an Aero skin
main retired electrobun's native context menu (a documented no-op on Linux) in
favour of an HTML one on every platform. Merging that here needs the menu to
look right under the Win7 skin as well.

Conflicts, all resolved by keeping both sides:
- src/bun/index.ts — createAppRPC keeps the branch's mainWindow getter, drops
  main's removed contextMenu dep.
- ui/src/app/app.ts — imports both TitleBar and ContextMenu.
- ui/src/app/platform.ts — kept the branch's version (its IS_MAC_CHROME accounts
  for the frameless win7 chrome, and MOD_LABEL correctly keys on the real
  platform rather than the chrome), plus exporting IS_MAC and adding IS_LINUX
  for the menu's per-OS shell wording.

Aero menu (branch-only): 7.css has no menu component, so it is hand-authored
across both layers — the row <button>s shed their push-button slab in `skin`
(revert-layer only reaches `utilities` from there), and the square-cornered
slab, hairline border and gradient selection band are stated in `skinfix`.
Built strictly from existing --w7-* vars, never literal hex, so the
hand-written dark Win7 palette recolours it for free. Hover and the keyboard
cursor share one band (the component marks the cursor with `bg-accent`, which
is overridden alongside :hover) as a real Win7 menu does.

Also answers a question the merge raised: frameless windows DO work on Linux.
The GTK wrapper honours titleBarStyle "hidden" — it calls
gtk_window_set_decorated(false) and logs "GTK: Created window without
decorations (custom titlebar)". Verified on GNOME/Wayland and documented in
architecture.md.

Verified: backend `tsc --noEmit` clean, `bun run build:ui` clean (492.52 kB
initial, still under the 500 kB budget), app launches with the skin active,
frameless, no console errors and no GBM failures.

Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-15 14:59:02 +02:00

192 lines
12 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
`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 at the bottom of
styles.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".
## 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.
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.