Skip to content

refactor(webtop): drop nginx, bind selkies directly on port 3000 - #50

Merged
gitricko merged 14 commits into
mainfrom
refactor/webtop-drop-nginx
Sep 7, 2026
Merged

refactor(webtop): drop nginx, bind selkies directly on port 3000#50
gitricko merged 14 commits into
mainfrom
refactor/webtop-drop-nginx

Conversation

@gitricko

@gitricko gitricko commented Sep 7, 2026

Copy link
Copy Markdown
Owner

Summary

Selkies already speaks WebSocket natively — the nginx reverse proxy was just adding indirection. This PR removes nginx as a dependency and binds selkies directly to port 3000.

What changed

  • selkies-native.sh: Remove nginx start/stop/status, remove config template deployment. Change defaults to , . Add Rust toolchain + build deps install (nasm, cmake, libudev-dev, etc.) since pixelflux/pcmflux require them. Simplify install to build all three from git directly.
  • SKILL.md: Remove nginx from prerequisites, commands, procedure, pitfalls, security, files tree. Update architecture diagram, config table, code examples.
  • troubleshooting.md: Remove nginx entries, update port references from 8082 to 3000.
  • Delete .

Architecture

Before: Browser (port 3000) → nginx → selkies (127.0.0.1:8082)
After: Browser (port 3000) → selkies (0.0.0.0:3000)

One fewer moving part. Users forward port 3000 and open in browser.

Verification

  • CI lint check: all 7 checks passed
  • selkies confirmed working on 0.0.0.0:3000 (HTTP 200, WS handshake)

Selkies already speaks WebSocket natively — the nginx reverse proxy
was just adding indirection. Bind selkies to 0.0.0.0:3000 directly,
removing nginx as a dependency entirely.

Changes:
- selkies-native.sh: remove nginx start/stop/status, remove config
  template deployment, change SELKIES_ADDR default to 0.0.0.0,
  SELKIES_PORT default to 3000. Add Rust toolchain + build deps
  install (nasm, cmake, libudev-dev, etc.) since pixelflux/pcmflux
  require them. Simplify install to build all three from git directly.
- SKILL.md: remove nginx from prerequisites, commands, procedure,
  pitfalls, security, files tree. Update architecture diagram,
  config table, code examples to reflect direct selkies on port 3000.
- troubleshooting.md: remove nginx entries, update port references
  from 8082 to 3000.
- Delete templates/nginx.conf.template (no longer needed).
Copilot AI lite review requested due to automatic review settings September 7, 2026 19:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@greptile-apps

greptile-apps Bot commented Sep 7, 2026

Copy link
Copy Markdown

Greptile Summary

The PR removes nginx from the webtop runtime and exposes Selkies directly on port 3000. It also moves installation to pinned source revisions, adds native build dependencies, builds the dashboard client, and updates the supporting documentation.

  • Binds Selkies directly to 0.0.0.0:3000
  • Removes nginx configuration and lifecycle management
  • Adds pinned Rust-backed dependency and web-client builds
  • Adds migration cleanup and disk-space cleanup

Confidence Score: 3/5

The PR does not appear safe to merge until startup cleanup is scoped to the legacy Selkies nginx configuration and removes that stale configuration independently of current port occupancy.

Startup still issues a global nginx shutdown after detecting any listener on the configured port, and it removes the stale Selkies nginx site only when that port is occupied, leaving both previously reported migration failures reachable.

Files Needing Attention: .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh

Important Files Changed

Filename Overview
.devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Reworks installation and startup around direct Selkies serving; the previously reported nginx migration-cleanup defects remain outstanding.
.devcontainer/skills/codespace-webtop/SKILL.md Updates setup, architecture, configuration, security guidance, and verification for the nginx-free deployment.
.devcontainer/skills/codespace-webtop/references/architecture.md Documents the direct browser-to-Selkies architecture and migration behavior.
.devcontainer/skills/codespace-webtop/references/troubleshooting.md Updates troubleshooting guidance for port 3000 and source-based dependency builds.
.devcontainer/skills/codespace-webtop/references/disk-optimization.md Documents cleanup of build-only Rust, Cargo, source, and package-cache artifacts.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    B[Browser] -->|HTTP and WebSocket :3000| S[Selkies]
    S --> P[Pixelflux capture]
    P --> X[Xvfb :20]
    X --> D[XFCE desktop]
Loading

Reviews (15): Last reviewed commit: "fix(webtop): use SELKIES_WEB_ROOT in cmd..." | Re-trigger Greptile

Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
After pixelflux/pcmflux compile their Rust extensions and the web client
is built, remove build-time-only artifacts that the running webtop no
longer needs:
- ~/.rustup (1.5GB Rust toolchain) — only needed during pip install
- ~/.cargo (612MB registry + build cache) — only needed during build
- ~/.selkies/selkies-src (170MB web clone) — only dist/ matters
- ~/.cache/pip (32MB) — download cache

Only the compiled .so files in the venv + web_root dist are needed at
runtime (~250MB), down from ~3GB after install.
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Greptile P1: existing installations with nginx still running on port
3000 would block selkies from binding. Add a guard at the top of
cmd_start() that detects and gracefully stops any legacy nginx process
before starting the new stack.
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Update wiki articles to reflect the new architecture where selkies
binds directly to port 3000 (no nginx):

- codespace-webtop.md: remove nginx from architecture diagram, key
  components table, and WebSocket endpoint section. Update package
  source and security sections.
- codespace-port-visibility.md: remove nginx from integration example.
- SKILL.md: update References section to say 'install from git source'
  instead of 'GitHub Actions selkies-wheel artifact'.
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
- Nginx cleanup now checks if nginx is actually listening on our port
  (3000) before stopping it — unrelated nginx instances on VM/bare-metal
  hosts are left alone.
- Also removes stale /etc/nginx/sites-enabled/selkies config so a later
  nginx restart won't reload the obsolete proxy.
- Rust cleanup now only removes ~/.rustup and ~/.cargo if the install
  script itself installed them (rust_installed_by_us=1). If the user
  had a pre-existing Rust toolchain, only the cargo build cache is
  cleaned (~575MB), preserving the toolchain.
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Greptile P1: mutable git HEADs can change between installs, causing
inconsistent builds. Pin to the current HEAD SHAs for each repo:

- pixelflux: bf07c68
- pcmflux: d2683ef
- selkies: 1d9b67b

Update by running `git ls-remote` on each repo and updating the pins.
SKILL.md now documents this process.
1. Rust ownership detection (P1): check for ~/.cargo and ~/.rustup dirs
   on disk, not just PATH. If an existing toolchain is off-PATH, we
   detect it and reuse rather than reinstall and later delete it.

2. Rust installer verification (P2): download rustup-init binary + sha256
   checksum separately, verify before executing. No more curl | sh.

3. Install failure propagation (P1): pip install failures now abort with
   return 1 instead of continuing silently. Also verify selkies is
   importable after install.

4. Legacy nginx detection (P1): use ss -tln (no -p) to check port
   occupancy without needing root process metadata. If the port is
   occupied, try nginx -s quit first, then fuser -k as fallback.
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh
- architecture.md: reflect new nginx-free stack (selkies on 0.0.0.0:3000),
  add Migration section for legacy nginx cleanup
- Add references/disk-optimization.md: documents ~2.3GB cleanup of build-time
  artifacts (Rust toolchain, cargo registry, git clone, pip cache) and git
  commit pinning strategy for reproducibility
- SKILL.md: reference new disk-optimization.md
1. Web build failure propagation (P1): cmd_build_web now returns
   non-zero on git clone failure, npm build failure, or dashboard
   dist copy failure. cmd_install propagates these and aborts instead
   of writing .installed with an empty web_root.

2. Cleanup kills unrelated port owners (P1): before force-killing
   a process on the target port, verify with sudo ss -tlnp that
   the listener is actually nginx. If it's an unrelated service,
   warn and abort instead of killing it.
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Greptile P1: web client was cloned from unpinned HEAD while Python
deps (pixelflux, pcmflux, selkies) are pinned to specific commits.
Now the web clone also checks out the pinned selkies commit
(1d9b67be6f9c695f187a0509a3c1d3b3e204807b) for full reproducibility.
Centralize all commit pins as env vars at top of selkies-native.sh:
- SELKIES_WEB_COMMIT (selkies web client)
- SELKIES_PIXELFLUX_COMMIT (pixelflux)
- SELKIES_PCMFLUX_COMMIT (pcmflux)

Consumers (pip install, SKILL.md) reference these env vars instead of
hardcoded SHAs. Makes it easy to update pins: just set the env vars or
edit the defaults.
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
1. Web commit override ignored: cmd_build_web now uses global
   SELKIES_WEB_COMMIT env var instead of hardcoded local value.
   Override via env var now works end-to-end.

2. Web build failures report success: cmd_install now checks
   cmd_build_web return value and aborts on failure.
…dary

Documents the supply-chain risk model: checksum + TLS detects
accidental corruption but not a compromised distribution origin.
Applies equally to npm/pip/cargo/apt. Per Karpathy guidelines: be
explicit about assumptions, no silent assumptions in implementation.
Comment thread .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh Outdated
Respects the env var override so web client builds to the correct
destination matching cmd_start's serve path.
@gitricko
gitricko merged commit d46dac6 into main Sep 7, 2026
7 checks passed
@gitricko
gitricko deleted the refactor/webtop-drop-nginx branch September 7, 2026 23:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants