one-os-dev-wayland-html5-compositor-greenfield-analysis/11-pitfalls.md

# 11 — Pitfalls, Anti-patterns, and What to Avoid

This is the prominent pitfalls chapter. Greenfield's known weaknesses,
anti-patterns, and design pitfalls are catalogued here so the user can
avoid making the same mistakes in their own Wayland-to-HTML compositor
(Odoo 20 + JS/Python). Each pitfall comes with: **what Greenfield does**
(with file/directory reference), **why it's a problem** (technical
reason), and **what the user should do instead** (concrete recommendation
for their JS/Python + Odoo 20 stack).

## Headline numbers

- **299** `TODO|FIXME|HACK|XXX|@deprecated|workaround` hits across non-vendored source (after excluding `examples/sdk/*` upstreams and `xtsb/protocol/xproto.xml`).
- **13 packages** all pinned to `"version": "1.0.0-rc1"` — no `CHANGELOG.md`, no `SECURITY.md`, no `CONTRIBUTING.md`, no issue/PR templates.
- **1 commit** visible on `master` (`6c578f4`, "Fix typo in code snippet (#161)", 2025-10-15) — implying the public repo is a release snapshot, not the live dev branch.
- **3 native Node `.node` addons**, all Linux-x64 only.
- **0** uses of `OffscreenCanvas` or `transferControlToOffscreen` anywhere in the compositor.
- **2** Web Workers used — and only for the WASM H.264 *fallback* decoder path. The primary WebCodecs path uses none.

## Catalogued pitfalls (A–U)

### Critical (will bite you on day 1)

#### A. Native Node addons

**What Greenfield does**: Three Node-API addons:
- `wayland-server-addon.node` (1231 lines C, wraps patched libwayland-server)
- `proxy-encoding-addon.node` (290 lines C, wraps GStreamer)
- `proxy-poll-addon.node` (127 lines C, wraps libuv `uv_poll_t`)

All Linux-x64 only, built with CMake + Ninja. ABI-locked to Node major
versions.

**Why it's a problem**: 
- Linux-x64 only — no ARM64 support
- ABI breaks on Node major upgrades
- Hard to ship — must rebuild for each target platform
- Hard to debug — C N-API debugging requires core dumps
- GStreamer integration is via C, not Python
- No Python story

**What you should do instead**: Use `pywayland` (cffi bindings to
libwayland-server), `python-xkbcommon`, and GStreamer's Python bindings
(`gi.repository.Gst`). One Python process can replace the entire
`compositor-proxy-cli`. See `10-porting-to-odoo-js-python.md` for details.

#### B. rc1 with no policy

**What Greenfield does**: All 13 packages at `1.0.0-rc1`. No CHANGELOG,
no SECURITY.md, no CONTRIBUTING.md.

**Why it's a problem**: Breaking changes can land silently. The
maintainer's roadmap is unclear.

**What you should do instead**: Pin to a specific commit hash if you fork
it. Track upstream commits manually. Plan for the eventuality that
1.0.0 final may break APIs.

#### C. Encoding rigidity

**What Greenfield does**: Three H.264 GStreamer pipelines (x264/nvh264/
vaapih264) with hand-tuned magic constants:
- `bitrate=12000 vbv-buf-capacity=160 bframes=0 rc-lookahead=0`
- VA-API is "experimental"
- AMD (`vah264enc`) absent
- PNG fallback at hardcoded ≤256×256 threshold
- Browser-side hardware decode is explicitly disabled in `Session.ts:93`
  with a FIXME "hardware decoding frame offset is wrong"

**Why it's a problem**: 
- No adaptive bitrate — video plays at 12 Mbps regardless of network
  conditions
- No AMD support — limits hardware encoder options
- Browser decode is software-only — wastes GPU

**What you should do instead**: 
- Build an adaptive multi-codec encoding router (PNG for ≤64×64 or static,
  VP8/H.264 for mid-size, H.264/AV1 for video)
- Drive the router by the `EncoderFeedback` measurement Greenfield already
  produces but doesn't consume
- Use `pykcp` or similar for congestion-aware bitrate adaptation
- Enable browser hardware decode (`prefer-hardware`) — fix the "frame
  offset is wrong" bug

#### D. Wire protocol has no schema version field

**What Greenfield does**: No version prefix on any binary message:
- `EncodedFrame` (FRAME channel)
- `RemoteOutOfBandChannel` (PROTOCOL channel OOB)
- The FD-channel in `RemoteAppLauncher.ts:612`
- Undocumented magic close codes (`4200`, `4201`, `4403`, `4001`, `1005`)

All break-lockstep — if you change the wire format, both sides must
upgrade simultaneously or messages will be misinterpreted.

**Why it's a problem**: 
- Cannot evolve the protocol without breaking all clients
- Cannot detect version mismatches
- Hard to debug — close codes are magic numbers

**What you should do instead**: 
- Prefix every binary message with `u16 protocolVersion`
- Document every close code in a comment
- Use JSON `{"v":1, "type":"..."}` for the control plane
- Build a feature-flag system so old clients can negotiate capabilities

#### E. Main-thread decode + render

**What Greenfield does**: 
- `VideoDecoder` runs on main thread
- WebGL context bound to `HTMLCanvasElement`
- No `OffscreenCanvas`
- No `Worker` in render path
- 0 uses of `OffscreenCanvas` or `transferControlToOffscreen` in the
  compositor
- Only 2 Web Workers used (FFmpeg Wasm fallback), shared across surfaces

**Why it's a problem**: 
- Jank on 1080p surfaces — main thread blocks on decode
- No way to recover if main thread stalls (input also blocked)
- Can't use `requestAnimationFrame` for compositing if main thread is busy

**What you should do instead**: 
- Render in a Web Worker with `OffscreenCanvas` from day one
- Ship `EncodedVideoChunk` / `ArrayBuffer` transfers (zero-copy) to the
  worker
- Use `VideoFrame` directly as a WebGL texture source — no `copyTo`
- Use multiple workers (one per output) for parallel rendering

### High severity

#### F. xkbcommon Wasm is multi-MB

**What Greenfield does**: `libs/compositor-wasm/xkbcommon/build.sh`
clones libxkbcommon 1.5.0, embeds the entire xkeyboard-config database
via `--embed-file`. Result: multi-MB WASM module loaded on every page.

**Why it's a problem**: 
- Slow initial load
- Layout list parsed via fragile string matching
- Hard to update keyboard layouts without rebuilding WASM

**What you should do instead**: 
- Use the browser's `navigator.keyboard.getLayoutMap()` API for basic
  layout detection
- For full xkbcommon support, lazy-load the WASM only when needed
- Consider server-side keymap compilation (Python `python-xkbcommon`)
  and ship only the result string to the browser

#### G. Clipboard edge cases

**What Greenfield does**: 
- Hardcoded MIME list `['text/plain', 'text/html', 'image/png']`
- No primary selection (`wp_primary_selection_v1` not implemented)
- Change detection by blob size (false negatives on same-size different
  content)
- Focus check at `DataOffer.ts:73` is a FIXME
- Browser-side clipboard polling (no `clipboardchange` event)

**Why it's a problem**: 
- Middle-click paste doesn't work (Linux users expect it)
- Large clipboard content (e.g. copied image) is poorly handled
- MIME negotiation races

**What you should do instead**: 
- Implement `zwp_primary_selection_device_manager_v1` from day one
- Use exact MIME matching, not substring
- For large clipboard content, use chunked transfer
- Watch for the `clipboardchange` event in modern browsers (currently
  behind flags in Chrome)

#### H. XWayland/xtsb complexity

**What Greenfield does**: 
- A full XCB reimplementation in TypeScript with its own 5800-line
  `xproto.xml`
- The XWM is a hand-port of wlroots' `xwm.c` with FIXMEs pointing at
  specific wlroots commits
- `XWindowManager.ts:746` literally says "this makes xterm close somehow"
- Title bars painted pixel-by-pixel in `XWindowFrame.ts`

**Why it's a problem**: 
- 5800 lines of X11 protocol XML to maintain
- Hand-ported XWM logic is fragile and out of sync with upstream wlroots
- Magic workarounds in code

**What you should do instead**: 
- Use `python-xlib` on the Python proxy side instead of porting xtsb
- Consider running XWayland with a minimal window manager (e.g. mutter
  in kiosk mode) instead of implementing XWM yourself
- For XWayland apps only (no full X11 desktop needed), the XWM can be
  much simpler than wlroots'

#### I. Adaptive bitrate feedback loop is wired but not consumed

**What Greenfield does**: 
- `EncoderFeedback.ts` measures client decode duration at 1 Hz
- 4-byte feedback messages sent from browser to proxy
- Proxy receives them but does NOT use them to adapt bitrate
- `Channel.ts:181` has a TODO confirming the gap

**Why it's a problem**: 
- Video plays at fixed 12 Mbps regardless of network conditions
- No way to recover from congestion
- Wasted bandwidth on slow networks

**What you should do instead**: 
- Wire `EncoderFeedback` to a bitrate adaptation controller
- Use `GstElement.set_property("bitrate", ...)` to adjust x264enc/nvh264enc
- Implement `requestKeyUnit` plumbing (Greenfield has it end-to-end but
  the browser never calls it)

#### J. Single-seat, single-session model

**What Greenfield does**: 
- One seat per browser session
- One Node process per browser session (per `compositorSessionId`)
- No multi-seat support (e.g. multiple keyboards/mice for collaborative
  editing)
- No multi-region isolation

**Why it's a problem**: 
- Can't support multi-user scenarios (e.g. conference room with multiple
  keyboards)
- No way to isolate apps between tenants

**What you should do instead**: 
- For multi-tenant isolation: separate Python proxy processes per tenant,
  containerized (Docker + bubblewrap)
- For multi-seat: implement `wl_seat` global per seat, route input
  events by seat

#### K. Custom web-bitmapbuf protocol — apps must be Greenfield-aware

**What Greenfield does**: 
- `protocol/web-bitmapbuf-unstable-v1.xml` is Greenfield's custom
  extension
- Lets browser JS apps hand a GPU-backed `ImageBitmap` directly to the
  compositor
- Standard Wayland apps (even browser-resident ones) fall to slow H.264
  path

**Why it's a problem**: 
- Apps must be rebuilt against `@gfld/client-protocol` to use the fast
  path
- Standard Wayland apps can't take advantage of zero-copy GPU textures
- Browser-resident WASM apps are slow unless rebuilt

**What you should do instead**: 
- Document clearly that the fast path requires Greenfield-aware apps
- For native Linux apps (your main use case), this isn't an issue — they
  go through the H.264 path regardless
- For browser-resident WASM apps, encourage users to build against
  `@gfld/client-protocol`

#### L. Connection loss handling

**What Greenfield does**: 
- 10-minute zombie timeout on disconnect (`NativeAppContext.ts:152-160`)
- No state resumption on reconnect
- `eagerBufferContents` cache in `BufferStream.ts:31` is unbounded (TODO
  at `:87` confirms the leak)
- App kill timers: SIGHUP@10min, SIGKILL@+10s
- Close code 1001 → immediate SIGHUP; 4001 → immediate SIGTERM

**Why it's a problem**: 
- User loses all running apps on browser refresh (unless they reconnect
  within 10 min)
- Memory leak from unbounded cache
- No way to survive proxy restart

**What you should do instead**: 
- Bound the `eagerBufferContents` cache (e.g. max 100 frames)
- Persist app state on browser side (localStorage) so users can resume
- For proxy restart, checkpoint app state to disk (json file)
- Implement graceful reconnection with state sync (the KCP plumbing
  exists)

#### M. No sandboxing

**What Greenfield does**: 
- `spawn(executable, args, { env })` with no bubblewrap/seccomp/namespaces
- Basic Auth is a single replayable credential compared with `!==`
- `compositorSessionId` is client-generated

**Why it's a problem**: 
- Native apps have full access to the host filesystem
- One compromised app can read all other apps' data
- Basic Auth credentials can be replayed
- Client-generated session IDs can be spoofed

**What you should do instead**: 
- Sandbox every spawned app with bubblewrap + seccomp + per-session Linux
  user namespace
- Replace Basic Auth with short-lived signed JWT bound to Odoo's session
  model
- Server-generated session IDs (use `secrets.token_urlsafe(32)`)

#### N. Heavy toolchain

**What Greenfield does**: 
- Yarn 4 + CMake + Ninja + Meson + pkg-config
- Two different Emscripten versions: 3.1.46 for FFmpeg, 3.1.60 for SDK
- SDK builds 30+ C libraries from source
- gtk4 WASM is currently broken
- Native build needs `autoconf libtool automake meson ninja-build bison
  cmake build-essential gperf`

**Why it's a problem**: 
- Complex build environment setup
- Long build times
- Emscripten version skew between libs and SDK
- gtk4 WASM broken means no GTK4 browser apps

**What you should do instead**: 
- Stick with a single Emscripten version
- Pre-build WASM modules and ship them as artifacts (don't rebuild every
  time)
- For Python, use `pip install` for all dependencies (simpler than CMake
  + Ninja)
- Skip the WASM app SDK entirely if you only care about native Linux apps

### Medium / low (but worth fixing)

#### O. WebCodecs usage

**What Greenfield does**: 
- WebCodecs IS used (correcting the brief in some agents' notes)
- BUT HW-decode is disabled
- `VideoEncoder` is unused

**Why it's a problem**: 
- Wastes GPU on software decode
- Could encode in browser (for upload to other peers) but doesn't

**What you should do instead**: 
- Enable HW-decode by fixing the "frame offset is wrong" FIXME
- Consider WebCodecs `VideoEncoder` for any peer-to-peer video paths

#### P. WebCodecs hardware decode disabled

**What Greenfield does**: 
- `Session.ts:93` has `// FIXME hardware decoding frame offset is wrong`
- `hardwareAcceleration: 'prefer-software'` is hardcoded

**Why it's a problem**: 
- Software decode at 1080p60 is ~30% CPU on a modern laptop
- Battery life impact on mobile
- Wastes GPU

**What you should do instead**: 
- Investigate and fix the frame offset bug
- Enable `prefer-hardware` for capable browsers
- Provide a runtime toggle

#### Q. Unbounded `eagerBufferContents` cache

**What Greenfield does**: 
- `BufferStream.ts:31` has unbounded cache
- TODO at `:87` confirms the leak

**Why it's a problem**: 
- Memory leak on long-running sessions
- Can OOM the browser tab

**What you should do instead**: 
- Bound the cache to a maximum number of frames (e.g. 100)
- Use LRU eviction
- Periodic cleanup of orphaned entries

#### R. `H264NALDecoder.worker.ts` is minimal

**What Greenfield does**: 
- 22 lines of code
- `@ts-ignore`'d
- No error boundary

**Why it's a problem**: 
- Worker crashes silently
- No retry on transient errors
- No metrics on decode failures

**What you should do instead**: 
- Add error boundary with retry logic
- Log decode failures to the proxy (for diagnostics)
- Pool workers for resilience

#### S. Magic undocumented WebSocket close codes

**What Greenfield does**: 
- Close codes `4200`, `4201`, `4403`, `4001`, `1005` are used without
  documentation
- Only codes 4000-4999 (application-level) tear down KCP

**Why it's a problem**: 
- Hard to debug connection issues
- Close codes are magic numbers

**What you should do instead**: 
- Document every close code in code comments
- Use named constants (e.g. `CLOSE_APP_TERMINATED = 4001`)
- Build a close-code → human-readable mapping

#### T. Generated protocol files committed

**What Greenfield does**: 
- `packages/compositor-proxy/src/protocol/*.js` is generated but
  committed to the repo
- Can drift from XML source

**Why it's a problem**: 
- Easy to forget to regenerate
- PRs may include stale generated code

**What you should do instead**: 
- Don't commit generated files
- Use `prebuild` script to generate them
- Add a CI check that regenerated files match committed ones

#### U. Confirmed copy-paste bug in `webcodec-buffer-decoder.ts`

**What Greenfield does**: 
- `webcodec-buffer-decoder.ts:142-144, 156-157` has:
  `this.opaqueDecoder = undefined` twice
- Should be `alphaDecoder` the second time
- Leaks the alpha decoder after errors

**Why it's a problem**: 
- Memory leak
- Wasm decoder context not cleaned up properly
- Can cause decoder pool exhaustion

**What you should do instead**: 
- Audit all `cleanup` paths in decoder code
- Add unit tests for error scenarios

## Design wins to copy

Not all of Greenfield's choices are bad. Here are 12 design wins worth
copying verbatim:

1. **Generator-driven protocol stubs** (`libs/compositor-generator/`,
   `client-generator/`, `compositor-proxy-generator/`) — single XML source
   for both sides. Eliminates wire-format drift.
2. **Dual-buffer commit semantics** (`Surface.ts`: `state` vs
   `pendingState`, atomic merge with pixman damage regions). Classic
   Wayland correctness.
3. **UserShellApi decoupling** — shell sees only `{events, actions}`,
   never Wayland resources. This is THE integration seam you'll use.
4. **Asynchronous browser compositor** — per-client channels, never blocks
   on a slow client. Essential for multi-app scenarios.
5. **KCP-over-WebSocket** for UDP-like latency over TCP (`kcp.ts`, 1170
   lines). Better than plain TCP for real-time video.
6. **WebCodecs primary + WASM fallback + capability probe** (`Session.ts`).
   Modern browsers get the fast path, older ones still work.
7. **Zero-copy dmabuf → encoder** via standard `linux-dmabuf-unstable-v1`
   (`westfield-dmabuf.c`). Pixels stay on GPU.
8. **Per-surface encoder feedback channel** (`EncoderFeedback.ts`)
   measuring client decode duration — even though adaptation isn't wired
   up yet, the telemetry is right.
9. **`ReconnectingWebSocket`** with exponential backoff + message queue.
   Survives transient network drops.
10. **Minimal shell** — 562 lines of TypeScript total in `compositor-shell`;
    kiosk variant is 40 lines. Demonstrates that the shell can be tiny.
11. **Honest docs** — `docs/pages/design/` openly admits WebSocket/TCP is
    wrong for real-time and waits on WebTransport. Useful as a reference
    for what trade-offs were considered.
12. **AGPL-3.0** — deliberate copyleft, but be aware before integrating.
    If you want to keep your modifications proprietary, fork the MIT
    libs and reimplement the AGPL parts.

## Top-5 recommendations for the user's Odoo 20 + JS/Python compositor

1. **Skip native Node addons entirely.** Use `pywayland` (cffi),
   `python-xkbcommon`, and GStreamer's `gi.repository.Gst` Python
   bindings. One Python process can replace the entire
   `compositor-proxy-cli`. See `10-porting-to-odoo-js-python.md` for
   details.

2. **Sandbox every spawned app** with bubblewrap + seccomp + per-session
   Linux user namespace; replace Basic Auth with short-lived signed JWT
   bound to Odoo's session model.

3. **Render in a Web Worker with `OffscreenCanvas`** from day one; ship
   `EncodedVideoChunk` / `ArrayBuffer` transfers (zero-copy) to the
   worker; use `VideoFrame` directly as a WebGL texture source — no
   `copyTo`.

4. **Build an adaptive multi-codec encoding router** (PNG for ≤64×64 or
   static, VP8/H.264 for mid-size, H.264/AV1 for video), driven by the
   `EncoderFeedback` measurement Greenfield already produces but doesn't
   consume.

5. **Version your wire protocol from commit #1** — prefix every binary
   message with `u16 protocolVersion`; document every close code; use
   JSON `{"v":1, "type":"..."}` for the control plane.

## License implications of AGPL-3.0

The four `packages/*` (compositor, compositor-proxy, compositor-proxy-cli,
compositor-shell) are **AGPL-3.0-or-later**. Implications:

- If you host Greenfield's `compositor-proxy` as a network service and
  modify it, you MUST release your modifications under AGPL-3.0+.
- The browser-side `@gfld/compositor` is also AGPL. If you modify it
  (e.g. to integrate with Odoo), your modifications must be AGPL.
- The lower-level `libs/*` (common, client-protocol, compositor-protocol,
  the generators, compositor-wasm, compositor-ffmpeg-h264, xtsb) are MIT
  or Apache-2.0 — these are safe to use without copyleft obligations.
- **Strategy to avoid AGPL**: use the MIT libs as-is, reimplement the
  AGPL parts from scratch (browser compositor + native proxy). The
  Python port described in `10-porting-to-odoo-js-python.md` is a clean
  way to do this — if you write your Python proxy from scratch, you can
  license it however you want.

## What Greenfield's codebase reveals about its engineering process

The codebase reveals several engineering process insights:

1. **One-person show** — most commits are by Erik De Rijcke. The codebase
   has the strengths (consistent vision, tight integration) and weaknesses
   (single point of failure, no code review diversity) of a one-person
   project.
2. **Long-running project** — code references back to 2017-era commits
   (WebRTC was a previous transport; the `kcp.ts` MTU comment says
   "webrtc datachannel MTU"). The architecture has evolved through several
   major pivots.
3. **rc1 is meaningful** — the maintainer has been working toward 1.0 for
   years and rc1 represents a deliberate freeze. Don't expect 1.0 final
   any time soon, but also don't expect major architecture changes.
4. **Honest about limitations** — docs/pages/design/ explicitly admits
   what's wrong and what's planned. Use this to inform your own roadmap.