Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .devcontainer/skills/codespace-webtop/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ Validates: OS, sudo, python3, pip3, and all apt packages listed above.
- **(a)** Use a vendored `selkies-0.0.0.dev0-py3-none-any.whl` if present in `wheels/`
- **(b)** Try `cmd_download_wheel` (GitHub Actions `selkies-wheel` artifact) — NOTE this needs auth and usually 401s, so treat as best-effort
- **(c)** **Reliable fallback: build from git source** — `pip wheel git+https://github.com/selkies-project/selkies.git`. This is the only path that works unattended on a fresh Codespace. (PyPI `selkies==1.6.1` is the WRONG legacy GStreamer package; the `releases/latest` wheel URL is dead.)
3. **Build the web client** (`cmd_build_web`): clone selkies repo, copy `addons/selkies-web-core/`, run `npm ci` (fallback `npm install`) then `npm run build`, copy `dist/` → `~/.selkies/web_root`. The web client is **NOT** bundled in the wheel — selkies serves a 404 on `/` unless `--web-root` points at a built client.
3. **Build the web client** (`cmd_build_web`): clone the full selkies repo (both `addons/selkies-web-core` and `addons/selkies-dashboard` must be siblings), `npm install` + `npm run build` **selkies-web-core first** (the dashboard's prebuild imports its `dist/selkies-core.js`), then build **selkies-dashboard**, and copy `addons/selkies-dashboard/dist/` → `~/.selkies/web_root`. The web client is **NOT** bundled in the wheel — serve the dashboard, NOT bare web-core (see Pitfalls: bare core = no sidebar).
4. Deploys nginx config (template with placeholder substitution) to `/etc/nginx/sites-enabled/selkies`
5. Validates `nginx -t`

Expand Down Expand Up @@ -145,7 +145,9 @@ Environment variables (all optional, with defaults):
- **Port conflicts**: Default ports 8082 (selkies) and 3000 (nginx) must be free.
- **CI lint gate (repo `.devcontainer/skills/**`)**: Before committing any change to this skill, run `bash .devcontainer/skills/ci-lint-check/scripts/ci_lint_check.sh`. Markdown URLs MUST be wrapped in angle brackets `<https://...>` or they fail `MD034/no-bare-urls` (this broke the initial PR #3 — two bare URLs in a References block).
- **selkies wheel acquisition is NOT plug-and-play**: The GitHub Actions artifact (`selkies-wheel`) requires auth (401 unauthenticated), the `releases/latest/download/selkies-wheel.zip` URL returns 404, and the PyPI package (`selkies==1.6.1`) is the wrong legacy GStreamer package. **The only reliable unattended path is `pip wheel git+https://github.com/selkies-project/selkies.git`** (built into `cmd_install` as fallback (c)). If you add a vendored wheel to `wheels/` it will be used, but the skill no longer ships one.
- **Web client is NOT bundled in the selkies wheel**: The wheel contains only the Python streaming server (`selkies.selkies_web` namespace is empty). The React/Vite client lives in `addons/selkies-web-core` in the selkies repo. `install` **must** run `cmd_build_web` (clone repo → npm install → npm run build → copy to `~/.selkies/web_root`) and `start` **must** pass `--web-root=~/.selkies/web_root`. Without this, selkies returns HTTP 404 on `/` and nginx has no UI to proxy.
- **Web client MUST be `selkies-dashboard`, not `selkies-web-core` (this is the #1 failure mode).** The selkies repo has TWO web addons: `addons/selkies-web-core` (the embeddable streaming **Core** only, README literally says "for an external dashboard to interact with the client") and `addons/selkies-dashboard` (the standalone UI that embeds the Core). If `install` serves `selkies-web-core`, the webtop WILL launch and the desktop WILL render, but there is **NO sidebar** — the Core mounts with `_isSidebarOpen = !1` (closed) and only opens on a `toggleDashboard` window.postMessage. Symptom when this is wrong: video/audio/clipboard work fine, WS handshake returns `MODE websockets`, HTTP 200 — but the Selkies sidebar chrome (settings/stats/clipboard/files/shortcuts) is absent. Fix: build `selkies-dashboard` (its `prebuild` copies `../selkies-web-core/dist/selkies-core.js`, so both addons must be siblings under one cloned repo), then point `--web-root` at the dashboard's `dist/`. Verify by grepping the served bundle for `sidebar`/`toggle` (dashboard has ~90 `sidebar` + ~100 `toggle`; bare core has almost none and starts closed). See `references/web-client-no-sidebar.md` for the full diagnosis recipe.
- **`autostart enable` must keep `{ ...; }` with a trailing `;`**: the appended rc line is `[[ -x ... ]] && { "$SCRIPT_DIR/selkies-native.sh" start; } &>/dev/null &`. A missing `;` before `}` (i.e. `{ ... start }`) makes bash throw `syntax error: unexpected end of file` for EVERY new login shell — it silently breaks `.bashrc` and anything that sources it. The template and script both use the `;`-terminated form; if you ever regenerate the hook by hand, keep it.
- **`npm ci` fails on a fresh shallow clone**: `git clone --depth 1` may not ship a `package-lock.json` that `npm ci` requires, so `npm ci` errors and the build silently produces nothing (then the dashboard's `prebuild` aborts with "missing selkies-core.js"). Always wrap as `npm ci ... || npm install ...`, and build `selkies-web-core` BEFORE `selkies-dashboard` — dashboard's `prebuild` (`copy-core.js`) exits 1 if `../selkies-web-core/dist/selkies-core.js` is absent.

## Security

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Web client: no-sidebar diagnosis & correct build

## Symptom
Webtop launches: video renders, mouse/keyboard work, clipboard syncs, WS
handshake returns `MODE websockets`, `curl /` returns HTTP 200 — but the
Selkies **sidebar** (video/audio/screen settings, stats, clipboard, file
transfer, keyboard shortcuts) is missing. The page is just the bare desktop.

## Root cause
The selkies repo ships TWO web addons under `addons/`:
- `selkies-web-core` — the **embeddable streaming Core** only. Its README
literally states it is "for an external dashboard to interact with the
client." The built bundle mounts with the sidebar CLOSED
(`_isSidebarOpen = !1`) and only opens it when a separate dashboard posts a
`window.postMessage({type:'toggleDashboard'})`. Served alone it is a desktop
feed with no sidebar chrome.
- `selkies-dashboard` — the **standalone UI** (React app, `react`/`react-dom`).
Its `prebuild` (`copy-core.js`) copies `../selkies-web-core/dist/selkies-core.js`
into `src/`; vite then bundles the Core *plus* the full sidebar UI.

Serving `selkies-web-core/dist` as `--web-root` is the wrong client. The
correct `--web-root` is `selkies-dashboard/dist`.

## Confirm which client is being served
```bash
# download the served bundle
curl -s http://127.0.0.1:3000/selkies-core.js > /tmp/core.js 2>/dev/null \
|| curl -s "http://127.0.0.1:3000/$(curl -s http://127.0.0.1:3000/ | grep -oE 'assets/index-[A-Za-z0-9]+\.js' | head -1)" > /tmp/core.js

# bare Core has almost no sidebar refs and starts closed:
grep -oE "_isSidebarOpen=![01]" /tmp/core.js # bare core: !1 (closed)
grep -oc "sidebar" /tmp/core.js # bare core: ~0-4 ; dashboard: ~90
grep -oc "toggle" /tmp/core.js # bare core: ~0 ; dashboard: ~100

# also: index.html. Dashboard's <div id="root"></div> + jsdb/ + assets/ dir;
# bare core's is a single selkies-core.js module with no assets/ folder.
curl -s http://127.0.0.1:3000/ | head
ls ~/.selkies/web_root/
```

## Correct build + serve (do this, not bare core)
```bash
cd ~/.selkies
rm -rf selkies-src && git clone --depth 1 https://github.com/selkies-project/selkies.git selkies-src

for a in selkies-web-core selkies-dashboard; do
( cd selkies-src/addons/$a && { npm ci --no-audit --no-fund 2>/dev/null || npm install --no-audit --no-fund; } && npm run build )
done
# web-core FIRST (dashboard prebuild imports its dist); both must be siblings.

# swap web_root to the dashboard dist
rm -rf ~/.selkies/web_root && cp -r ~/.selkies/selkies-src/addons/selkies-dashboard/dist ~/.selkies/web_root

# restart (selkies reads --web-root at startup)
bash ~/.hermes/skills/codespace/codespace-webtop/scripts/selkies-native.sh restart
```
Verify: `curl /` now references `assets/index-*.js`; the served JS contains
~90 `sidebar` refs; open the forwarded port — sidebar chrome present.

## Pitfalls recap
- Build `selkies-web-core` before `selkies-dashboard` (prebuild dependency).
- `npm ci` can fail on a fresh `--depth 1` clone (no lockfile) → use `|| npm install`.
- The control script `cmd_build_web` now does this; older versions built bare
`selkies-web-core` (no sidebar) — the #1 historical cause of this bug.
55 changes: 33 additions & 22 deletions .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ cmd_install() {
pixelflux pcmflux \
"$SELKIES_WHEEL"

# Build and install selkies web frontend from addons/selkies-web-core
echo "[web] building selkies-web-core (vite)..."
# Build and install selkies web frontend (selkies-dashboard + embedded core)
echo "[web] building selkies-dashboard web client (vite)..."
cmd_build_web

# Install nginx config template (substitute placeholders)
Expand Down Expand Up @@ -228,37 +228,48 @@ cmd_download_wheel() {
rm -rf "$tmpdir" "$tmpzip"
}

# Build selkies web frontend from addons/selkies-web-core
# Build selkies web frontend.
# CRITICAL: serve selkies-dashboard, NOT selkies-web-core.
# selkies-web-core is only the embeddable streaming Core: it mounts with the
# sidebar CLOSED and only opens on a toggleDashboard postMessage, so serving it
# alone gives a desktop feed with NO sidebar chrome. selkies-dashboard is the
# standalone UI (sidebar + Core embedded); its copy-core.js prebuild pulls
# ../selkies-web-core/dist/selkies-core.js, so both addons must be siblings
# under one cloned repo. Build web-core first, then dashboard, serve dashboard dist.
cmd_build_web() {
echo "[web] building selkies-web-core..."
echo "[web] building selkies web client (dashboard + embedded core)..."

local web_src="$HOME/.selkies/selkies-web-core"
local repo_dir="$HOME/.selkies/selkies-src"
local web_core_dir="$repo_dir/addons/selkies-web-core"
local dashboard_dir="$repo_dir/addons/selkies-dashboard"
local web_dist="$HOME/.selkies/web_root"

# Clone or update the web-core repo
if [[ ! -d "$web_src/.git" ]]; then
echo "[web] cloning selkies-web-core..."
git clone --depth 1 https://github.com/selkies-project/selkies.git /tmp/selkies-full 2>/dev/null
mkdir -p "$web_src"
cp -r /tmp/selkies-full/addons/selkies-web-core/* "$web_src/"
rm -rf /tmp/selkies-full
# Clone full selkies repo once (both addons must be siblings)
if [[ ! -d "$repo_dir/.git" ]]; then
echo "[web] cloning selkies (full repo, both addons needed)..."
rm -rf "$repo_dir"
git clone --depth 1 https://github.com/selkies-project/selkies.git "$repo_dir" 2>&1 | tail -3
else
echo "[web] updating existing clone..."
(cd "$web_src" && git pull --depth 1) 2>/dev/null || true
(cd "$repo_dir" && git pull --depth 1) 2>/dev/null || true
fi

# Install deps and build with Vite
(cd "$web_src" && npm ci --no-audit --no-fund >/dev/null 2>&1) || {
echo "[web] npm ci failed, trying npm install..."
(cd "$web_src" && npm install --no-audit --no-fund 2>&1 | tail -3)
build_addon() {
local dir="$1"
echo "[web] npm build in $dir ..."
( cd "$dir" && { npm ci --no-audit --no-fund 2>/dev/null || npm install --no-audit --no-fund; } && npm run build ) 2>&1 | tail -8
}
(cd "$web_src" && npm run build 2>&1 | tail -5)

# Copy built dist to web_root (where selkies will serve from)
# web-core first (dashboard prebuild imports its dist), then dashboard
build_addon "$web_core_dir"
build_addon "$dashboard_dir"
Comment on lines +264 to +265

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Addon build failures are ignored

When dependency installation or either addon build fails, cmd_build_web continues, can clear web_root, and can return success from the final ls; cmd_install then creates the installed marker and reports completion even though Selkies has an empty, stale, or incompatible frontend.

Prompt To Fix With AI
This is a comment left during a code review.
Path: .devcontainer/skills/codespace-webtop/scripts/selkies-native.sh
Line: 264-265

Comment:
**Addon build failures are ignored**

When dependency installation or either addon build fails, `cmd_build_web` continues, can clear `web_root`, and can return success from the final `ls`; `cmd_install` then creates the installed marker and reports completion even though Selkies has an empty, stale, or incompatible frontend.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.


# Serve the DASHBOARD dist (has the sidebar + embeds the Core)
mkdir -p "$web_dist"
cp -r "$web_src/dist/"* "$web_dist/"
rm -rf "$web_dist"/* 2>/dev/null || true
cp -r "$dashboard_dir/dist/"* "$web_dist/"

echo "[web] built and copied to $web_dist"
echo "[web] built and copied dashboard to $web_dist"
ls -la "$web_dist/"
}

Expand Down Expand Up @@ -475,7 +486,7 @@ cmd_autostart() {
cat >> "$target_rc" <<EOF

$hook_marker
[[ -x "$SCRIPT_DIR/selkies-native.sh" ]] && { $hook_cmd } &>/dev/null &
[[ -x "$SCRIPT_DIR/selkies-native.sh" ]] && { $hook_cmd; } &>/dev/null &
EOF
echo "[autostart] enabled (appended to $target_rc)"
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
# Guards prevent duplicate entries and double-start

# selkies-native autostart
[[ -x "$HOME/.selkies/selkies-native.sh" ]] && { "$HOME/.selkies/selkies-native.sh" start } &>/dev/null &
[[ -x "$HOME/.selkies/selkies-native.sh" ]] && { "$HOME/.selkies/selkies-native.sh" start; } &>/dev/null &
Loading