From 7dfeb78c3777fd73116a2144a0eff81102d30e5b Mon Sep 17 00:00:00 2001 From: Alexandre <44178713+alexbelgium@users.noreply.github.com> Date: Tue, 4 Aug 2026 19:38:09 +0200 Subject: [PATCH 1/2] fix(claude_desktop): stop the GPU flags from disabling the GPU; make max_resolution work; drop the dead driver install (#2938) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The GPU acceleration shipped in 2026.08.03 was not inert — it was what turned the GPU off. `--use-gl=angle --use-angle=gl-egl` forces Mesa's EGL X11 platform, which offers no window-capable EGLConfig under this Xvfb. The GPU process logged `gl_surface_egl.cc:262 No suitable EGL configs found`, abandoned GL, and was relaunched with `--use-gl=disabled` while every renderer got `--disable-gpu-compositing`. Chromium already renders on the GPU here with no flags at all: LSIO's Xvfb runs `-vfbdevice /dev/dri/renderD128`, so its GLX is backed by the real render node. The premise that Xvfb offers only an indirect/software path was wrong for this base image. Measured on a separate display, including at the production 15360x8640 screen — with the flags the GPU process loads libEGL_mesa and holds 1 fd on the render node; without them it loads the Mesa gallium megadriver over DRI3, holds 8, and no renderer carries --disable-gpu-compositing. claude-gpu-probe was not wrong about the hardware, it answered the wrong question: it exercised ANGLE's default GLX path, which works, so it passed while the flags it gated disabled the GPU. Removed with the gpu_acceleration option. Also fixes two other changes from the same release that never did anything: - max_resolution wrote MAX_RES into the s6 container_environment, but svc-xorg starts `#!/usr/bin/env bashio`, not with-contenv, and never reads it. Renaming the option to MAX_RES makes the add-on env layer inject it into every service run script, which is how DRINODE already reaches Xvfb. It ships with no default, so nothing changes until it is set: carrying over the old 1920x1080 default would have silently shrunk every existing desktop, since that value had never taken effect. Schema bounds each axis to 100-9999. - The amd64 driver install has never run in any release: guarded by `if [[ ]]` with no SHELL directive, so it runs under dash, which has no `[[` — condition false, RUN still exit 0. It is deleted rather than repaired, because it had nothing to add. Hardware GL already works without it, Mesa already comes from the LSIO base at a newer backports version, and intel-media-va-driver-non-free was installed live on the running add-on and changed nothing: VA-API failed identically to the free driver on both DRM nodes. That failure is below the add-on, in the host i915 stack. Co-authored-by: Claude Opus 5 --- claude_desktop/CHANGELOG.md | 47 +++++ claude_desktop/Dockerfile | 51 ++--- claude_desktop/README.md | 3 +- claude_desktop/config.yaml | 7 +- claude_desktop/rootfs/defaults/autostart | 61 ++---- .../etc/cont-init.d/22-display_tuning.sh | 66 ------ .../etc/cont-init.d/85-openbox_autostart.sh | 20 -- .../rootfs/usr/local/bin/claude-gpu-probe | 196 ------------------ 8 files changed, 92 insertions(+), 359 deletions(-) delete mode 100755 claude_desktop/rootfs/etc/cont-init.d/22-display_tuning.sh delete mode 100755 claude_desktop/rootfs/usr/local/bin/claude-gpu-probe diff --git a/claude_desktop/CHANGELOG.md b/claude_desktop/CHANGELOG.md index dcabe9809a..83975875fa 100644 --- a/claude_desktop/CHANGELOG.md +++ b/claude_desktop/CHANGELOG.md @@ -1,3 +1,50 @@ +## 2026.08.04 (04-08-2026) +- Fix: reverted the GPU acceleration added in 2026.08.03. It did not just fail to help — it was + what disabled the GPU. `--use-gl=angle --use-angle=gl-egl` forces Mesa's EGL X11 platform, + which offers no window-capable EGLConfig under this Xvfb, so the GPU process logged + `gl_surface_egl.cc:262 No suitable EGL configs found`, abandoned GL, and was relaunched with + `--use-gl=disabled` while every renderer got `--disable-gpu-compositing`. + Chromium already renders on the GPU here with no flags at all, because LSIO's Xvfb runs with + `-vfbdevice /dev/dri/renderD128` and its GLX is therefore backed by the real render node — + the premise that Xvfb offers only a software path was wrong for this base image. Measured + three times, including at the production 15360x8640 screen: with the flags the GPU process + loads `libEGL_mesa` and holds 1 fd on the render node; without them it loads `libGLX_mesa`, + holds 8, and no renderer carries `--disable-gpu-compositing`. + Removes `claude-gpu-probe` and the `gpu_acceleration` option. The probe was not wrong about + the hardware, it was answering the wrong question: it exercised ANGLE's default GLX path, + which works, and so it passed while the flags it gated disabled the GPU. +- Fix: `max_resolution` never did anything; the option is renamed to `MAX_RES`. It wrote + `MAX_RES` into the s6 `container_environment`, but the base image's `svc-xorg` starts + `#!/usr/bin/env bashio` rather than `with-contenv` and never reads that directory, so Xvfb + kept starting at 15360x8640. Naming the option `MAX_RES` makes the add-on env layer inject + it directly into every service `run` script, which is how `DRINODE` already reaches Xvfb. + `MAX_RES` ships with no default, so nothing changes for anyone until it is set: the screen + stays at the base image's 15360x8640. The old `max_resolution` default of `1920x1080` is + deliberately not carried over — it had never taken effect, so shipping it now would have + silently shrunk every existing desktop, including on 4K displays. Home Assistant drops + options that are no longer in the schema (it logs a warning), so a saved `max_resolution` + is discarded rather than migrated. The schema bounds each axis to 100-9999, which keeps the + worst case (~400 MB of framebuffer) below the 15360x8640 default it replaces. + Removes `22-display_tuning.sh`. +- Removed the amd64 GPU driver install, which has never run in any release. The 2026.08.03 + build corrected its gate variable but the guard is `if [[ ... ]]` and there is no `SHELL` + directive, so it runs under dash, which has no `[[` — the condition was false and the `RUN` + still exited 0. Verified in the shipped 2026.08.03 image: no `vainfo`, no + `intel-media-va-driver-non-free`, and no matching install in its apt history. + It was deleted rather than repaired, because it had nothing to add. Hardware OpenGL already + works without it: the GPU process loads the Mesa gallium megadriver over DRI3 and holds 8 + fds on `/dev/dri/renderD128`, with no swrast/llvmpipe in the GL path. `libgl1-mesa-dri` and + `mesa-vulkan-drivers` already come from the LSIO base at a newer backports version (Mesa + 25.0.7) than reinstalling them would give. And `intel-media-va-driver-non-free`, the one + genuinely new payload, was installed live on the running add-on and changed nothing: VA-API + failed identically to the free driver (`iHD_drv_video.so init failed`) on both + `/dev/dri/renderD128` and `card0`. That failure is below the add-on, in the host i915 stack. + Repairing the guard would have turned dead code into an untested apt transaction that swaps + a base-image package for no measured benefit. +- A `[[` in the Dockerfile's legacy `/etc/services.d` shim is corrected to `[` for the same + dash reason; that directory does not exist on this s6-overlay v3 base, so it is + behaviour-neutral here. + ## 2026.08.03 (03-08-2026) - Performance: Claude Desktop now uses the GPU instead of rendering on the CPU. Under Xvfb, Chromium probed GLX, found only Xvfb's software path, and fell back to diff --git a/claude_desktop/Dockerfile b/claude_desktop/Dockerfile index 4d1cd34206..7156ca9825 100644 --- a/claude_desktop/Dockerfile +++ b/claude_desktop/Dockerfile @@ -74,7 +74,7 @@ VOLUME [ "/sys/fs/cgroup" ] # hadolint ignore=SC2015,DL4006,SC2013,SC2086 RUN \ usermod --home /data/data abc && \ - if [[ -d /etc/services.d ]] && ls /etc/services.d/*/run 1> /dev/null 2>&1; then sed -i "1a set +e" /etc/services.d/*/run; fi + if [ -d /etc/services.d ] && ls /etc/services.d/*/run 1> /dev/null 2>&1; then sed -i "1a set +e" /etc/services.d/*/run; fi ARG TEMPLATE_BASE_URL="https://raw.githubusercontent.com/alexbelgium/hassio-addons/master/.templates" @@ -142,36 +142,29 @@ RUN install -d -m 0755 /etc/apt/keyrings && \ apt-get clean && \ rm -rf /var/lib/apt/lists/* -# The Intel N150/Twin Lake iGPU uses the host's i915 kernel driver through the mapped -# /dev/dri nodes. Explicitly install the amd64 userspace stack needed for accelerated -# OpenGL rendering, VA-API video encoding, and Vulkan, then fail the build if any driver -# payload is missing. Keep aarch64 unchanged because these Intel packages are amd64-only. +# No bespoke GPU driver install. The LSIO base image already provides the userspace stack that +# hardware rendering actually uses, and it does so at a newer version than adding it here would: +# libgl1-mesa-dri and mesa-vulkan-drivers come from bookworm-backports Mesa 25.0.7, where a +# plain `apt-get install` of the same names is at best a no-op. # -# Gate on BUILD_ARCH, not TARGETARCH. TARGETARCH is a BuildKit-provided platform ARG; the -# repo's builder (.github/workflows/onpush_builder.yaml) passes BUILD_ARCH explicitly and -# that is the contract this repo can rely on. This block previously used TARGETARCH and -# silently never ran: the shipped amd64 image has no `vainfo` and no -# `intel-media-va-driver-non-free`, and its apt history contains no matching install — so -# the "fail the build if a payload is missing" guarantee below had never once executed. +# There used to be an amd64 block here installing intel-media-va-driver-non-free, libgl1-mesa-dri, +# mesa-vulkan-drivers and vainfo. It never ran in any release: it was guarded by +# `if [[ ... ]]`, and with no SHELL directive RUN executes under /bin/sh — dash, which has no +# `[[`. dash printed `[[: not found`, the condition was false, and the RUN still exited 0, so +# every build silently skipped it and still passed. (An earlier fix corrected the gate variable +# from TARGETARCH to BUILD_ARCH but left the `[[`, so it stayed dead.) Confirmed in the shipped +# 2026.08.03 amd64 image: no vainfo, no intel-media-va-driver-non-free, no matching apt history. # -# The Vulkan ICD payload is checked via the driver directory rather than a single -# distro-specific filename: the previous `test -f intel_icd.x86_64.json` named a file Debian -# does not ship (it installs `intel_icd.json`), so restoring the guard without this change -# would have turned dead code straight into a failing build. -RUN if [[ "${BUILD_ARCH}" == "amd64" ]]; then \ - apt-get update && \ - apt-get install -y --no-install-recommends \ - intel-media-va-driver-non-free \ - libgl1-mesa-dri \ - mesa-vulkan-drivers \ - vainfo && \ - test -f /usr/lib/x86_64-linux-gnu/dri/iHD_drv_video.so && \ - test -f /usr/lib/x86_64-linux-gnu/dri/iris_dri.so && \ - ls /usr/share/vulkan/icd.d/intel_icd*.json > /dev/null && \ - command -v vainfo > /dev/null; \ - fi && \ - apt-get clean && \ - rm -rf /var/lib/apt/lists/* +# It was removed rather than repaired, because measurement showed it had nothing to add: +# - Hardware OpenGL already works without it. In the shipped image the Chromium GPU process +# loads the Mesa gallium megadriver over DRI3 and holds 8 fds on /dev/dri/renderD128, with +# no swrast/llvmpipe in the GL path. +# - The only genuinely new payload, intel-media-va-driver-non-free, does not fix anything +# here. Installed live alongside vainfo on the running add-on, VA-API failed identically to +# the free driver — `iHD_drv_video.so init failed` on both /dev/dri/renderD128 and card0, +# with both 23.1.1 builds. The failure is below the add-on, in the host i915/GuC stack. +# Repairing the guard would therefore have turned dead code into an untested apt transaction +# that swaps a base-image package for no measured benefit. # Install the current upstream hadolint and actionlint releases for both supported ARG HADOLINT_VERSION=v2.14.0 diff --git a/claude_desktop/README.md b/claude_desktop/README.md index 5d6665cede..ffcbf587c2 100644 --- a/claude_desktop/README.md +++ b/claude_desktop/README.md @@ -97,8 +97,7 @@ Git synchronization hooks. A repository is indexed only when it is listed in | `KEYBOARD` | | Optional Selkies keyboard layout. | | `PASSWORD` | | Optional password for direct Selkies ports. | | `DRINODE` | | Optional GPU device override for Selkies. | -| `gpu_acceleration` | `auto` | Whether Claude Desktop renders on the GPU. `auto` adds Chromium's ANGLE/EGL flags only when a probe confirms a hardware GL context is available, `on` forces them without probing, `off` keeps software rendering. Use `on` with care: it skips every safety check, so on a host that cannot actually drive those flags the desktop can come up black — set the option back to `auto` or `off` to recover. | -| `max_resolution` | `1920x1080` | Caps the virtual screen. Selkies still resizes dynamically below this; raise it only if you drive the desktop from a larger display. | +| `MAX_RES` | _(unset)_ | Optional cap on the virtual screen, as `WIDTHxHEIGHT` (100-9999 per axis). Unset means the base image default, 15360x8640 — Selkies resizes dynamically below whatever the cap is, so this only sets the ceiling. Named `MAX_RES` because that is the environment variable the base image's Xvfb service reads. Setting it lowers the area Xvfb and the Selkies capture loop track for damage; the framebuffer itself is lazily populated, so this is a CPU saving, not a memory one. | | `DNS_server` | `8.8.8.8` | DNS server used by the standard DNS module. | | `permission_mode` | `auto` | Claude Code permission policy: `strict`, `auto`, or `bypass`. | | `install_headroom` | `true` | Register Headroom MCP and run the supervised local proxy. | diff --git a/claude_desktop/config.yaml b/claude_desktop/config.yaml index 331d08889e..96b9e5f415 100644 --- a/claude_desktop/config.yaml +++ b/claude_desktop/config.yaml @@ -56,7 +56,6 @@ options: github_username: "" enable_tools_health_report: true expose_headroom_dashboard: false - gpu_acceleration: auto headroom_auto_compress: true headroom_wrap_claude_code: true install_caveman: false @@ -66,7 +65,6 @@ options: install_headroom: true install_rtk: true install_tokensave: true - max_resolution: 1920x1080 mcp_servers_desktop: - headroom - tokensave @@ -98,6 +96,7 @@ schema: data_location: str? DRINODE: list(/dev/dri/card0|/dev/dri/card1|/dev/dri/card2|/dev/dri/renderD128|/dev/dri/renderD129|)? KEYBOARD: list(da-dk-qwerty|de-de-qwertz|en-gb-qwerty|en-us-qwerty|es-es-qwerty|fr-ch-qwertz|fr-fr-azerty|it-it-qwerty|ja-jp-qwerty|pt-br-qwerty|sv-se-qwerty|tr-tr-qwerty)? + MAX_RES: match(^[1-9][0-9]{2,3}x[1-9][0-9]{2,3}$)? PASSWORD: str? PGID: int PUID: int @@ -118,7 +117,6 @@ schema: github_username: str? enable_tools_health_report: bool expose_headroom_dashboard: bool - gpu_acceleration: list(auto|on|off)? headroom_auto_compress: bool? headroom_wrap_claude_code: bool install_caveman: bool @@ -128,7 +126,6 @@ schema: install_headroom: bool install_rtk: bool install_tokensave: bool - max_resolution: str? mcp_servers_desktop: - list(headroom|tokensave|homeassistant|codex) mcp_servers_code: @@ -139,5 +136,5 @@ schema: slug: claude_desktop udev: true url: https://github.com/alexbelgium/hassio-addons -version: "2026.08.03" +version: "2026.08.04" video: true diff --git a/claude_desktop/rootfs/defaults/autostart b/claude_desktop/rootfs/defaults/autostart index 85ee09cfdc..aa1fab8596 100644 --- a/claude_desktop/rootfs/defaults/autostart +++ b/claude_desktop/rootfs/defaults/autostart @@ -21,48 +21,28 @@ # force-overrides ANTHROPIC_BASE_URL (headroom #869), so Desktop uses the registered Headroom # MCP tools instead. -# GPU acceleration. +# GPU acceleration: deliberately no flags. Do not add ANGLE/EGL flags here. # -# Under Xvfb, Chromium probes GLX, finds only Xvfb's indirect/software path, and falls back to -# rendering everything on the CPU (`--use-gl=disabled` on the GPU process, and -# `--disable-gpu-compositing` on the renderer). On a small Home Assistant host that is the -# add-on's single largest CPU consumer. +# Chromium already renders on the GPU on this base image, because LSIO's Xvfb is started with +# `-vfbdevice /dev/dri/renderD128` — so GLX here is backed by the real render node, not the +# indirect/software path. Left alone, Chromium picks Mesa's GLX (libGLX_mesa), initialises the +# GPU process, and composites on the GPU. # -# Routing Chromium through ANGLE's OpenGL backend over EGL uses the real render node instead. -# The flags are only added when /usr/local/bin/claude-gpu-probe confirms that Claude Desktop's -# own bundled ANGLE can create a hardware GL context here, because forcing them on a host with -# no render node (or one where Mesa falls back to llvmpipe) trades a working software desktop -# for a black window or a GPU-process crash loop. Any probe failure leaves the command line -# untouched, i.e. exactly the pre-existing software-rendering behaviour. +# Passing `--use-gl=angle --use-angle=gl-egl` actively *broke* that. It forces Mesa's EGL X11 +# platform, which offers no window-capable EGLConfig under this Xvfb, so the GPU process logged +# ui/gl/gl_surface_egl.cc:262 No suitable EGL configs found. +# gave up on GL entirely, and Chromium relaunched it with `--use-gl=disabled` while stamping +# `--disable-gpu-compositing` on every renderer — i.e. the flags caused the CPU rendering they +# were meant to remove. Measured three times, including at the production 15360x8640 screen: +# with the flags the GPU process loads libEGL_mesa and holds 1 fd on the render node; with no +# flags it loads libGLX_mesa and holds 8, and no renderer carries --disable-gpu-compositing. # -# `--use-angle=gles-egl` is deliberately not used: Mesa rejects it with "Intel or NVIDIA -# OpenGL ES drivers are not supported". +# A standalone ANGLE probe is not evidence for any of this: Claude Desktop's bundled ANGLE +# happily creates a hardware context here via its default (GLX) path, which is why the probe +# that used to gate these flags passed while the flags themselves disabled the GPU. # -# /run/claude-desktop-gpu-mode is written each boot by /etc/cont-init.d/85-openbox_autostart.sh -# from the `gpu_acceleration` add-on option (auto|on|off). -GPU_FLAGS="" -GPU_MODE="auto" -if [ -r /run/claude-desktop-gpu-mode ]; then - GPU_MODE="$(cat /run/claude-desktop-gpu-mode)" -fi -case "$GPU_MODE" in - off) - echo "claude-desktop: gpu_acceleration=off; using software rendering" >&2 - ;; - on) - # Escape hatch for hosts where the probe is wrong in either direction. - GPU_FLAGS="--ozone-platform=x11 --use-gl=angle --use-angle=gl-egl" - echo "claude-desktop: gpu_acceleration=on; forcing ANGLE/EGL without probing" >&2 - ;; - *) - # Bounded: this sits in the desktop's startup path, and a driver that wedges during - # EGL init must not leave the user staring at an empty screen. A timeout is treated - # exactly like a failed probe, i.e. software rendering. - if timeout 15 claude-gpu-probe; then - GPU_FLAGS="--ozone-platform=x11 --use-gl=angle --use-angle=gl-egl" - fi - ;; -esac +# `--use-angle=gl` also works, but it only reproduces what Chromium already chooses by itself, +# so it is not passed either. # Shared memory. # @@ -80,7 +60,6 @@ if [ -n "$SHM_KB" ] && [ "$SHM_KB" -ge 262144 ]; then SHM_FLAGS="" fi -# GPU_FLAGS and SHM_FLAGS must stay unquoted so they expand to separate arguments, or to -# nothing at all. +# SHM_FLAGS must stay unquoted so it expands to a separate argument, or to nothing at all. # shellcheck disable=SC2086 -exec claude-desktop --no-sandbox --password-store=basic $SHM_FLAGS $GPU_FLAGS +exec claude-desktop --no-sandbox --password-store=basic $SHM_FLAGS diff --git a/claude_desktop/rootfs/etc/cont-init.d/22-display_tuning.sh b/claude_desktop/rootfs/etc/cont-init.d/22-display_tuning.sh deleted file mode 100755 index 084ac5d028..0000000000 --- a/claude_desktop/rootfs/etc/cont-init.d/22-display_tuning.sh +++ /dev/null @@ -1,66 +0,0 @@ -#!/usr/bin/with-contenv bashio -# shellcheck shell=bash -set -e - -# Cap the virtual screen the desktop is drawn on. -# -# The Selkies base image starts Xvfb at DEFAULT_RES=15360x8640 so that a client on any monitor -# can resize into it. Nothing here needs a 133-megapixel screen: it enlarges the area Xvfb and -# the Selkies capture loop track for damage on every frame, which the add-on pays for -# continuously — measurably so even with no browser connected at all. -# -# MAX_RES is the base image's own knob for this (svc-xorg prefers it over DEFAULT_RES) and it -# only sets the *maximum*; Selkies still resizes dynamically underneath it, so a smaller cap -# costs nothing until a client actually asks for something larger. -# -# This is a CPU and address-space saving, not a memory one: the framebuffer is a lazily -# populated SysV shared segment, so the unused portion of the oversized screen was never -# resident to begin with. -# -# Note SELKIES_MANUAL_WIDTH/HEIGHT is a different knob that *pins* the resolution and disables -# dynamic resizing. It is deliberately not used here. -MAX_RESOLUTION="$(bashio::config 'max_resolution' '1920x1080')" - -if [ -z "$MAX_RESOLUTION" ]; then - bashio::log.info "max_resolution is empty; leaving the base image's default virtual screen size" - exit 0 -fi - -# Matched with bash's own =~ rather than grep: grep anchors per *line*, so a multi-line value -# such as "1920x1080\n640x480" satisfies ^...$ on its first line and would be passed through to -# Xvfb verbatim. Bash anchors the whole string, so an embedded newline is rejected. -if [[ ! "$MAX_RESOLUTION" =~ ^[0-9]{1,5}x[0-9]{1,5}$ ]]; then - bashio::log.warning "max_resolution '${MAX_RESOLUTION}' is not WIDTHxHEIGHT; leaving the base image default" - exit 0 -fi - -# A syntactically valid but nonsensical size (0x0, 99999x99999) would either stop Xvfb from -# starting at all or ask it for a framebuffer larger than the default this option exists to -# shrink. Bound it to something Xvfb and Selkies can actually serve; the upper bound is the -# base image's own default, so this option can only ever reduce the screen. -MAX_WIDTH="${MAX_RESOLUTION%%x*}" -MAX_HEIGHT="${MAX_RESOLUTION##*x}" -if [ "$MAX_WIDTH" -lt 640 ] || [ "$MAX_HEIGHT" -lt 480 ] || - [ "$MAX_WIDTH" -gt 15360 ] || [ "$MAX_HEIGHT" -gt 8640 ]; then - bashio::log.warning "max_resolution '${MAX_RESOLUTION}' is outside the supported range (640x480 to 15360x8640); leaving the base image default" - exit 0 -fi - -# cont-init.d completes before any s6-rc service starts, so svc-xorg picks this up on the same -# boot. Both paths are written because the base image's scripts read the legacy /var/run alias. -written=0 -for envdir in /var/run/s6/container_environment /run/s6/container_environment; do - if [ -d "$envdir" ]; then - printf '%s' "$MAX_RESOLUTION" > "${envdir}/MAX_RES" - written=1 - fi -done - -# Claiming success after writing nothing would send someone hunting for a cap that svc-xorg -# never saw. -if [ "$written" -eq 0 ]; then - bashio::log.warning "No s6 environment directory found; leaving the base image's default virtual screen size" - exit 0 -fi - -bashio::log.info "Virtual screen capped at ${MAX_RESOLUTION} (Selkies still resizes dynamically below this)" diff --git a/claude_desktop/rootfs/etc/cont-init.d/85-openbox_autostart.sh b/claude_desktop/rootfs/etc/cont-init.d/85-openbox_autostart.sh index 70960d5f98..feb6b60dd9 100755 --- a/claude_desktop/rootfs/etc/cont-init.d/85-openbox_autostart.sh +++ b/claude_desktop/rootfs/etc/cont-init.d/85-openbox_autostart.sh @@ -15,26 +15,6 @@ set -e # regardless, so every boot picks up the current /defaults/autostart content; ownership/mode # is left in the normal abc-writable state that init-selkies-config itself uses when # RESTART_APP is unset, and re-locked by that oneshot afterward if RESTART_APP is set. -# The autostart decides whether to hand Chromium the ANGLE/EGL flags, but it runs as abc under -# openbox where bashio is not available. Publish the resolved option to a file it can read. -# /run is tmpfs, so this is rewritten on every boot and never goes stale. -GPU_MODE="$(bashio::config 'gpu_acceleration' 'auto')" -case "$GPU_MODE" in - auto | on | off) ;; - *) - bashio::log.warning "Unknown gpu_acceleration '${GPU_MODE}'; falling back to auto" - GPU_MODE="auto" - ;; -esac -# Best-effort: the autostart falls back to "auto" when the file is absent, so a failure here -# must not abort this script under errexit and leave the openbox autostart unsynced. -if printf '%s\n' "$GPU_MODE" > /run/claude-desktop-gpu-mode 2> /dev/null; then - chmod 0644 /run/claude-desktop-gpu-mode - bashio::log.info "GPU acceleration mode: ${GPU_MODE}" -else - bashio::log.warning "Could not write /run/claude-desktop-gpu-mode; Claude Desktop will probe for GPU support (auto)" -fi - if [ -f /defaults/autostart ]; then mkdir -p "$HOME/.config/openbox" cp -f /defaults/autostart "$HOME/.config/openbox/autostart" diff --git a/claude_desktop/rootfs/usr/local/bin/claude-gpu-probe b/claude_desktop/rootfs/usr/local/bin/claude-gpu-probe deleted file mode 100755 index 95bb4f0b40..0000000000 --- a/claude_desktop/rootfs/usr/local/bin/claude-gpu-probe +++ /dev/null @@ -1,196 +0,0 @@ -#!/usr/bin/env python3 -"""Decide whether Claude Desktop can render on the GPU on this host. - -Claude Desktop is Electron/Chromium. Left to itself under Xvfb it probes GLX, finds -only the indirect/software path Xvfb offers, and gives up: the GPU process ends up -running `--use-gl=disabled` and the renderer `--disable-gpu-compositing`, so every -frame is rastered and composited on the CPU. On a small Home Assistant host that is -the single largest CPU consumer the add-on has. - -The fix is to point Chromium at ANGLE's OpenGL backend over EGL instead of GLX, but -those flags are only safe where they actually work — forcing them on a host with no -render node, or where Mesa falls back to a software rasterizer, trades a working -software desktop for a black window or a GPU-process crash loop. - -So rather than guessing from the presence of /dev/dri, this probe exercises the exact -code path Chromium will use: it loads Claude Desktop's *own bundled ANGLE* libEGL, -initializes the OpenGL backend, creates a real pbuffer context, and reads GL_RENDERER -back. Exit 0 means Chromium's GL stack is known-good here; any other exit means the -caller must leave Chromium alone and keep today's software rendering. - -Notes for future readers: - * ANGLE's OpenGL backend needs a reachable X display, so this must run after Xorg is - up (i.e. from the openbox autostart, not from cont-init.d). - * `gles-egl` is deliberately not attempted: Mesa reports "Intel or NVIDIA OpenGL ES - drivers are not supported" and ANGLE refuses to initialize. - * A renderer string naming SwiftShader/llvmpipe/softpipe is a *failure* here. That is - software rendering wearing a GL hat, and forcing the flags for it would add ANGLE - translation overhead on top of the CPU rasterization we are trying to avoid. -""" - -import ctypes -import os -import sys - -LIBDIR = "/usr/lib/claude-desktop" - -# EGL/ANGLE constants (see ANGLE's eglext.h); hardcoded to avoid a build dependency. -EGL_NONE = 0x3038 -EGL_PLATFORM_ANGLE_ANGLE = 0x3202 -EGL_PLATFORM_ANGLE_TYPE_ANGLE = 0x3203 -EGL_PLATFORM_ANGLE_TYPE_OPENGL_ANGLE = 0x320D -EGL_OPENGL_ES_API = 0x30A0 -EGL_SURFACE_TYPE = 0x3033 -EGL_PBUFFER_BIT = 0x0001 -EGL_RENDERABLE_TYPE = 0x3040 -EGL_OPENGL_ES2_BIT = 0x0004 -EGL_WIDTH = 0x3057 -EGL_HEIGHT = 0x3056 -EGL_CONTEXT_CLIENT_VERSION = 0x3098 -GL_VENDOR = 0x1F00 -GL_RENDERER = 0x1F01 - -SOFTWARE_MARKERS = ("swiftshader", "llvmpipe", "softpipe", "lavapipe", "software rasterizer") - - -class ProbeFailure(Exception): - """Raised when this host cannot give Chromium a hardware GL context.""" - - -def log(message): - """Write a probe diagnostic to stderr, where it lands in the add-on log.""" - sys.stderr.write(f"claude-gpu-probe: {message}\n") - - -def load_angle(): - """Load Claude Desktop's bundled ANGLE and declare the signatures we call.""" - egl_path = os.path.join(LIBDIR, "libEGL.so") - gles_path = os.path.join(LIBDIR, "libGLESv2.so") - if not (os.path.exists(egl_path) and os.path.exists(gles_path)): - raise ProbeFailure(f"bundled ANGLE libraries not found under {LIBDIR}") - - try: - egl = ctypes.CDLL(egl_path, mode=ctypes.RTLD_GLOBAL) - gles = ctypes.CDLL(gles_path, mode=ctypes.RTLD_GLOBAL) - except OSError as err: - raise ProbeFailure(f"could not load bundled ANGLE: {err}") from err - - egl.eglGetProcAddress.restype = ctypes.c_void_p - egl.eglGetError.restype = ctypes.c_int - egl.eglInitialize.argtypes = [ - ctypes.c_void_p, - ctypes.POINTER(ctypes.c_int), - ctypes.POINTER(ctypes.c_int), - ] - egl.eglCreatePbufferSurface.restype = ctypes.c_void_p - egl.eglCreateContext.restype = ctypes.c_void_p - gles.glGetString.restype = ctypes.c_char_p - gles.glGetString.argtypes = [ctypes.c_uint] - return egl, gles - - -def open_angle_display(egl): - """Initialize ANGLE's OpenGL backend and return its EGL display.""" - addr = egl.eglGetProcAddress(b"eglGetPlatformDisplayEXT") - if not addr: - raise ProbeFailure("bundled ANGLE has no eglGetPlatformDisplayEXT") - get_platform_display = ctypes.CFUNCTYPE( - ctypes.c_void_p, ctypes.c_uint, ctypes.c_void_p, ctypes.POINTER(ctypes.c_int) - )(addr) - - attrs = (ctypes.c_int * 3)( - EGL_PLATFORM_ANGLE_TYPE_ANGLE, EGL_PLATFORM_ANGLE_TYPE_OPENGL_ANGLE, EGL_NONE - ) - display = get_platform_display(EGL_PLATFORM_ANGLE_ANGLE, None, attrs) - if not display: - raise ProbeFailure(f"no ANGLE OpenGL display (egl error 0x{egl.eglGetError():x})") - - major, minor = ctypes.c_int(), ctypes.c_int() - if not egl.eglInitialize(ctypes.c_void_p(display), ctypes.byref(major), ctypes.byref(minor)): - raise ProbeFailure( - f"ANGLE OpenGL backend failed to initialize (egl error 0x{egl.eglGetError():x})" - ) - return display - - -def make_current_context(egl, display): - """Bring up a real pbuffer context. - - Initialization alone is not proof of anything: only a current context makes - GL_RENDERER report the driver Chromium would actually be handed. - """ - egl.eglBindAPI(EGL_OPENGL_ES_API) - config = ctypes.c_void_p() - count = ctypes.c_int() - config_attrs = (ctypes.c_int * 5)( - EGL_SURFACE_TYPE, EGL_PBUFFER_BIT, EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT, EGL_NONE - ) - chosen = egl.eglChooseConfig( - ctypes.c_void_p(display), config_attrs, ctypes.byref(config), 1, ctypes.byref(count) - ) - if not chosen or count.value == 0: - raise ProbeFailure(f"no usable EGL config (egl error 0x{egl.eglGetError():x})") - - surface_attrs = (ctypes.c_int * 5)(EGL_WIDTH, 64, EGL_HEIGHT, 64, EGL_NONE) - surface = egl.eglCreatePbufferSurface(ctypes.c_void_p(display), config, surface_attrs) - if not surface: - raise ProbeFailure(f"could not create pbuffer surface (egl error 0x{egl.eglGetError():x})") - - context_attrs = (ctypes.c_int * 3)(EGL_CONTEXT_CLIENT_VERSION, 2, EGL_NONE) - context = egl.eglCreateContext(ctypes.c_void_p(display), config, None, context_attrs) - if not context: - raise ProbeFailure(f"could not create GL context (egl error 0x{egl.eglGetError():x})") - - if not egl.eglMakeCurrent( - ctypes.c_void_p(display), - ctypes.c_void_p(surface), - ctypes.c_void_p(surface), - ctypes.c_void_p(context), - ): - raise ProbeFailure( - f"could not make the GL context current (egl error 0x{egl.eglGetError():x})" - ) - - -def describe_renderer(gles): - """Return (renderer, vendor), rejecting software rasterizers.""" - renderer = (gles.glGetString(GL_RENDERER) or b"").decode(errors="replace") - vendor = (gles.glGetString(GL_VENDOR) or b"").decode(errors="replace") - if not renderer: - raise ProbeFailure("GL context reported no renderer") - - lowered = renderer.lower() - if any(marker in lowered for marker in SOFTWARE_MARKERS): - raise ProbeFailure( - f"software renderer ({renderer}); leaving Chromium on its own software path" - ) - return renderer, vendor - - -def main(): - """Exit 0 only when Chromium's GL stack is known-good on this host.""" - if not os.environ.get("DISPLAY"): - log("no DISPLAY; ANGLE's OpenGL backend needs an X server") - return 1 - - try: - egl, gles = load_angle() - display = open_angle_display(egl) - make_current_context(egl, display) - renderer, vendor = describe_renderer(gles) - except ProbeFailure as err: - log(str(err)) - return 1 - # A probe is advisory: whatever goes wrong in these native calls, the desktop must still - # start. Any unexpected failure is reported and treated as "no GPU". - # pylint: disable=broad-exception-caught - except Exception as err: - log(f"unexpected probe error: {err}") - return 1 - - log(f"hardware GL available: {renderer} | {vendor}") - return 0 - - -if __name__ == "__main__": - sys.exit(main()) From 74456a03b6f5b9e5e4517a93ef2b0adb3e187396 Mon Sep 17 00:00:00 2001 From: Alexandre <44178713+alexbelgium@users.noreply.github.com> Date: Tue, 4 Aug 2026 22:29:20 +0200 Subject: [PATCH 2/2] chore: add hassio-addon-workflow skill for Claude Code (#2939) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * chore: add hassio-addon-workflow skill for Claude Code Checks in the repo-specific skill so future Claude Code sessions get the tiered scope->measure->plan->implement->verify workflow, repo traps, and helper scripts (preflight/measure/env_trace/validate/pr_review) without depending on a local machine's ~/.claude config. * fix(skill): address PR review feedback from Copilot and Codex - SKILL.md: repo-relative script invocation (skill is now checked in); correct the CI-gates list — the PR add-on linter is blocking, only the weekly Super-Linter is non-blocking - preflight.sh: git-aware repo detection (worktrees have a .git file) - pr_review.sh: header now documents resolve's actual --all behavior - measure.sh: CPU% uses getconf CLK_TCK; sample all processes, not the top-24 by RSS - env_trace.sh: validate VAR as a strict env-var name before regex use - validate.sh: shellcheck also covers extensionless run/finish; --vs-master skips visibly when a linter is missing instead of reporting a false clean * fix(skill): address CodeRabbit review feedback - pr_review.sh: resolve exits nonzero unless every thread actually resolved; watch exits nonzero and says so when checks settle with failures instead of reporting bare "settled" - validate.sh: pass the config.yaml path to Python as argv instead of interpolating $ADDON into the source (CWE-94) --- .claude/skills/hassio-addon-workflow/SKILL.md | 364 ++++++++++++++++++ .../hassio-addon-workflow/references/traps.md | 212 ++++++++++ .../scripts/env_trace.sh | 136 +++++++ .../hassio-addon-workflow/scripts/measure.sh | 99 +++++ .../scripts/pr_review.sh | 112 ++++++ .../scripts/preflight.sh | 87 +++++ .../hassio-addon-workflow/scripts/validate.sh | 128 ++++++ 7 files changed, 1138 insertions(+) create mode 100644 .claude/skills/hassio-addon-workflow/SKILL.md create mode 100644 .claude/skills/hassio-addon-workflow/references/traps.md create mode 100755 .claude/skills/hassio-addon-workflow/scripts/env_trace.sh create mode 100755 .claude/skills/hassio-addon-workflow/scripts/measure.sh create mode 100755 .claude/skills/hassio-addon-workflow/scripts/pr_review.sh create mode 100755 .claude/skills/hassio-addon-workflow/scripts/preflight.sh create mode 100755 .claude/skills/hassio-addon-workflow/scripts/validate.sh diff --git a/.claude/skills/hassio-addon-workflow/SKILL.md b/.claude/skills/hassio-addon-workflow/SKILL.md new file mode 100644 index 0000000000..7f1ccf0b88 --- /dev/null +++ b/.claude/skills/hassio-addon-workflow/SKILL.md @@ -0,0 +1,364 @@ +--- +name: hassio-addon-workflow +description: >- + End-to-end workflow for alexbelgium/hassio-addons add-on work — scope the change, diagnose + against the live add-on with real measurements, get an independent Codex (gpt-5.6-sol) review + of the plan, implement, have Codex review the code adversarially, open a PR, resolve the + CodeRabbit / Copilot / Codex-connector review comments, and verify the merged result actually + works. Use this whenever the task touches a Home Assistant add-on in this repo — fixing a bug + or reported issue, tuning RAM, CPU or performance, editing a Dockerfile, config.yaml, + cont-init.d script or s6 service, bumping an add-on version, or opening and iterating a PR + against hassio-addons. Also use it when asked to "check with codex", "verify with chatgpt", or + to resolve bot review comments on an add-on PR. Small tasks (typo fixes, version bumps, + one-file tweaks, simple coding questions) route through a light path that skips measurement + and Codex reviews — invoking this skill is cheap for small asks too. +--- + +# Home Assistant add-on workflow + +Triage first, then one of two paths: + +- **Light path** (small, contained tasks): scope → implement the simplest mechanism → + validate → PR → resolve bot comments → report honestly. +- **Full loop** (diagnosis, performance, defaults, new mechanisms): scope → measure → plan → + Codex attacks the plan → implement → simplify → Codex attacks the code → PR → resolve bot + comments → verify in production → report with calibrated confidence. + +**The standing rule on both paths:** when a simple solution and a more efficient-but-more-complex +one both work, ship the simple one. Complexity is only paid for by a **measurement** showing the +simple version has a concrete, user-visible cost on a real host — never by reasoning about +hypothetical performance. Slightly less efficient and obviously correct beats faster and harder +to review. + +The disciplines below exist because each one, when skipped, produced a specific failure in this +repo — the examples throughout are real, not illustrative. + +| Discipline | The failure it prevents | +|---|---| +| **Triage before you start** | Full ceremony spent on a one-line fix | +| **Scope before you work** | Solving the wrong problem, elegantly | +| **Evidence before reasoning** | Confident claims that measurement contradicts | +| **Reason adversarially** | Shipping something that only works on your host | +| **Simplify to the smallest thing that works** | 500 lines of machinery where an option would do | +| **Verify before declaring done** | "This should work" — and it doesn't | +| **Calibrate and report** | Verified and assumed presented as the same thing | + +**Where things are.** The repo is `alexbelgium/hassio-addons`; each add-on is a top-level +directory (`claude_desktop/`, `birdnet-go/`, …). This skill and its scripts are checked into +the repo at `.claude/skills/hassio-addon-workflow/` — invoke scripts from the repo root, e.g. +`bash "$(git rev-parse --show-toplevel)/.claude/skills/hassio-addon-workflow/scripts/preflight.sh"`. +(In the claude_desktop add-on environment the checkout lives at `/data/claude/hassio-addons`; +a copy of this skill may also exist under `~/.claude/skills/` — the checked-in copy is +canonical.) + +**Three facts to know before you touch anything**, because each is silent when violated: + +- **You cannot test the Docker build** — dockerd does not start here. CI is the only gate. +- **Never `git stash` under `/data/claude`** — `refs/stash` is shared across worktrees and + sessions, so it is not isolated even in your own worktree. +- **Work in a worktree under `/data`, not `/tmp`** (`/tmp` is `noexec`). + +Repo-specific traps live in `references/traps.md`. Read it before implementing — it is the +accumulated cost of previous sessions. The repo's own `CLAUDE.md` covers structure, Dockerfile +conventions, CI and lint rules. + +--- + +## 0. Triage — pick the path + +Classify the task before doing anything else. + +**Light path** — typo/doc fixes, CHANGELOG edits, version bumps, one-file small edits at +mechanism levels 1–3 (the ladder in step 3), simple coding questions. Flow: one-sentence scope → +pick the simplest mechanism → implement → `scripts/validate.sh --vs-master` → PR +(version bump + CHANGELOG still required) → resolve bot comments. Skip measurement, both Codex +reviews, and post-deploy measurement — but still label claims Verified / Checked / Assumed +honestly in the report. + +**Full loop** — performance/RAM/CPU work, diagnosis ("why is X slow/broken"), anything changing +a shipped default, changes spanning several scripts or mechanisms (a version bump's +config.yaml + CHANGELOG + Dockerfile touch is still light), anything at ladder levels 4–6, or +when the user asks for a Codex check. + +**Escalation rule** — if a light task grows mid-flight (touches a default, needs a new script or +service, reveals a deeper problem), stop and upgrade to the full loop rather than continuing +light. + +`references/traps.md` is required reading on **both** paths — traps bite one-liners too. + +## 1. Scope before you work + +Write this down before generating anything. Two sentences is enough, but they must be explicit: + +- **Goal** — the observable outcome, in the user's terms. +- **Non-goals** — what you are deliberately not touching. This is the load-bearing half. +- **Constraints** — what cannot change (defaults for other users, upgrade paths, arch support). +- **Definition of done** — what evidence will demonstrate success. + +A diagnosis task and a fix task have different scopes. "Why is it slow?" is answered by +measurement and a written finding; it does not automatically authorize a rewrite. When the user's +ask genuinely spans several changes, list them and say which you are doing now. + +Ask about defaults when the answer changes who is affected. Changing a shipped default touches +every user of a 120-add-on repo; making it opt-in touches nobody until they choose. That is the +user's call, not yours, and it is cheap to ask before implementing rather than after. + +## 2. Evidence before reasoning + +State no cause you have not observed. On a live add-on (`$BUILD_VERSION` set, `HOME=/data/data`) +measure the running system rather than reasoning from source — reviewers will hold you to the +numbers, and source-derived guesses are where wrong plans come from. + +Pick the tool to the question: + +- **"what is consuming RAM/CPU?"** → `scripts/measure.sh` (PSS and private, not summed RSS; keep + the sample at ≥20 s — a 3 s sample measured 2.3% where 20 s measured 21.6%). +- **"I set an option and nothing happened"** → `scripts/env_trace.sh `, which + walks all four stages of the option plumbing and names the one that dropped the value. +- **"is this flag/driver/package actually present?"** → look at the artifact itself: + `/proc//cmdline`, `command -v`, `/var/log/apt/history.log`. + +`references/traps.md#measurement` explains why summed RSS and reserved-vs-resident both matter. + +Before asserting anything, ask what would show it false, then go look: + +- "This process is duplicated" → is it? `ps -ef --forest`, compare parents and start times. +- "This costs 500 MB" → is it resident? `grep Rss /proc//smaps`. +- "This block never runs" → is its payload in the image? `command -v`, `apt` history. +- "The flag isn't set" → `tr '\0' '\n' < /proc//cmdline`. + +**Verify the revision you are reading.** `scripts/preflight.sh` compares the checkout's +`config.yaml` version against the running `$BUILD_VERSION`. A stale branch reads as completely +normal and has already cost one full analysis pass. + +When you correct yourself mid-analysis, keep the correction visible in your notes and in what you +report. A retracted claim that stays retracted is worth more than one quietly dropped. + +## 3. Plan, then have Codex attack it + +### Choose the mechanism level first (both paths) + +Rank mechanisms and start from the top — choose the level **before writing code** and state it +in the plan. Each step down costs more to write, more to review, and more to keep working across +base-image upgrades: + +1. **A config value** — an option, a schema constraint, an existing env var. +2. **An existing knob** the base image already reads (`MAX_RES`, `DRINODE`, `SELKIES_*`). +3. **A few lines in an existing script**, at the point that already runs. +4. **A new init script.** +5. **A new service, wrapper, or long-running process.** +6. **Custom protocol code, or patching someone else's internals.** + +Levels 4–6 automatically mean full loop, and need a reason that survives being said out loud. +"Upstream has no knob for this, and I checked" is a reason. "It felt cleaner" is not. If two +levels both solve it, the higher (simpler) level wins even when the lower one would be more +efficient — see the standing rule at the top. + +### Codex review of the plan (full loop only — skip on the light path) + +Write the plan around the measurements — each proposed change tied to a number — then get an +independent read **before** writing code. Codex is a genuinely different model reading the files +itself; on this workload it has repeatedly been worth the minutes. + +**Use the CLI, not the MCP tool, for prompts of this size.** `mcp__codex__codex` timed out +twice on ~4 KB prompts (2026-08-03); the CLI with the same content succeeded. This overrides the +global CLAUDE.md note recommending the MCP tool — that guidance still holds for short questions. +Run it backgrounded (`--sandbox read-only` means Codex cannot run anything, so paste every number +into the prompt; `- <` feeds the prompt file on stdin): + +```bash +codex exec --model gpt-5.6-sol --sandbox read-only --skip-git-repo-check \ + -c approval_policy='"never"' - < prompt.md > codex_out.txt 2>&1 +``` + +Write the prompt to a file. Include the files to read, your measurements **with numbers**, the +proposed changes, and explicit instructions to challenge you. Ask direct questions ("is this +really add-on-fixable?", "give the precise flag set") rather than "review this". Codex's sandbox +often cannot run local commands and falls back to reading GitHub, so paste the evidence in rather +than assuming it will find it. + +**Codex agrees with confident premises.** It has confirmed a wrong conclusion stated too +confidently, and separately caught a genuine methodology error in the same review. Treat its +confirmations with the same scepticism as its objections — especially about the build. + +### Attack your own plan too + +Before implementing, spend a moment actively trying to break it: + +- What does this do on a host **unlike this one** — no GPU, small `/dev/shm`, aarch64, a VM? +- What happens on **upgrade** to someone who configured this by hand? +- What is the **blast radius** if the assumption underneath it is wrong? +- What am I **inferring** that I could instead **detect at runtime** or **record explicitly**? + +That last question is the highest-yield one here; see [the recurring failure +mode](#the-failure-mode-this-loop-keeps-producing). + +## 4. Implement + +Read `references/traps.md` first; the bashio, s6-env, arch-guard and versioning traps are all +live and each has shipped a bug. + +Validate with `scripts/validate.sh --vs-master`. + +Write behavioural tests for anything with branches. Extract an embedded Python heredoc and drive +it against fixtures with stubbed env vars; stub `bashio::*` and `df` to exercise shell paths. Test +**the regression a reviewer described**, not just the happy path — a test that only covers the +case you were already thinking about adds little. + +## 5. Simplify — is this the simplest thing that works? + +Do this once you have something working and before you ask anyone to review it. The question is +not "is this good code" but **"what is the smallest change that makes the symptom go away, and +why isn't that enough?"** If you cannot answer the second half, the smaller change is the answer. +And restating the standing rule: obviously correct and slightly less efficient beats faster and +harder to review — efficiency only buys complexity when a measurement shows it matters. + +Checks worth running against your own diff: + +- **Did the diff stay at the ladder level chosen in step 3?** If it crept up a level, either + justify that out loud or redo it at the level you chose. +- **Can this be solved by deleting instead of adding?** A flag that shouldn't be passed, a + process that shouldn't start, a registration that shouldn't be duplicated. Removals cannot + regress on hosts you can't test. +- **Is the fix bigger than the thing it fixes?** That is a smell, not a rule — but it usually + means the problem was framed one level too deep. +- **How does this fail in three years**, when the base image, Electron, or upstream has moved? + Code that reads a documented knob keeps working. Code that reaches into private internals + does not. + +The evidence from this repo is blunt: + +- A rejected PR spent a **388-line TCP proxy plus a 142-line monkeypatch of a private upstream + method** to reclaim 159 MB — placing custom transport code in the path of every API request. + Both independent reviewers said close it rather than iterate on it. +- A ~180-line `ctypes` probe was written to decide whether to enable GPU flags. It worked + perfectly, proved the driver was fine, and the change **still did nothing**, because the + question it answered was not the question that mattered. +- A resolution cap shipped as a **new init script writing an s6 envdir** — the wrong mechanism + entirely. Renaming the option to the env var the service already reads (level 1) would have + worked, and the new script did not. + +In all three cases the simpler option existed and was skipped. Being able to build the +complicated thing is not a reason to. + +## 6. Codex attacks the code (full loop only) + +Same invocation, pointed at `git diff origin/master...HEAD` plus the reasoning behind each hunk. +Ask specifically what breaks: upgrade paths, hosts unlike this one, users who configured things +by hand. Ask it directly whether a simpler mechanism would achieve the same thing — an outside +reader spots one-level-too-deep framing far more easily than the person who just built it. + +## 7. Open the PR + +What CI actually gates on a PR: **`CHANGELOG.md` updated** (hard `exit 1`), the **HA add-on +linter** (`frenck/action-addon-linter` in `onpr_check-pr.yaml`, no `continue-on-error` — a +config.yaml schema error blocks the PR), and the **add-on image build**. The non-blocking lint +is the *weekly Super-Linter*, not the PR checks — don't confuse the two. Nothing checks the +version bump — but bump it +anyway (`X.Y.Z.N`, never `X.Y.Z-N`, see `references/traps.md#versioning`), because Supervisor will +not offer a rebuild without it, and update `README.md` if you added options. Match the existing +CHANGELOG heading format, `## X.Y (DD-MM-YYYY)`. + +Write the body to a file and use `gh pr create --body-file`. State what was measured, what +changed, **what is not verified**, and how to roll back the riskiest hunk on its own. + +## 8. Resolve review comments + +`scripts/pr_review.sh list|reply|resolve|status|watch `. + +For every comment, **reproduce the claim before agreeing or disagreeing.** A CodeRabbit finding +that `grep -E '^…$'` anchors per line — letting a multi-line value pass validation — was real and +provable in one command. A finding that a changelog heading needed a blank line was a false +positive against this repo's `.markdownlint.yaml`. + +Reply with the evidence, then resolve the thread. **Push back when you are right**, on the thread, +so the maintainer can overrule you — a resolved-but-wrong thread is worse than an open one. +Equally, when a reviewer is right, fix the cause rather than papering over the symptom. + +## 9. Verify before declaring done + +Do not write "this should work". Either it was exercised, or say plainly that it wasn't. + +On the light path, verification is `validate.sh` plus CI — anything beyond that is **Assumed**, +and the report must say so plainly. + +Distinguish three states and never let them blur: + +- **Verified** — you ran it and observed the result. +- **Checked but not exercised** — it parses, lints, type-checks. +- **Assumed** — reasoning only. Name the assumption. + +**CI passing and the PR merging prove the build works, not that the change does anything.** Once +the rebuilt add-on is running, re-run the measurement that motivated the work. Both changes in the +session that produced this skill passed CI, merged, and were **inert**: + +- The Xvfb resolution cap wrote its env file correctly and Xvfb still started at the base-image + default — wrong env mechanism for that service. +- The GPU flags reached Chromium's command line exactly as intended, and the GPU process still + reported `--use-gl=disabled`, having overridden them after its own init failed. + +Cheap post-deploy checks: `tr '\0' '\n' < /proc//cmdline` for flags, `/proc//environ` +for env vars, `scripts/env_trace.sh ` for the whole option plumbing, and a repeat +CPU/PSS sample against the pre-change numbers. + +Some fixes cannot be self-verified. A service reads its environment only at start, so an env-var +fix is unproven until the add-on restarts — which needs the user, or `ha-cli` with their +agreement. If you cannot restart, the change is **Assumed**, not Verified, and must be reported +that way. + +## 10. Calibrate and report + +Close against the scope from step 1, not against what you ended up doing. Structure: + +``` +What was asked / what shipped — mapped to the original scope +Evidence — the numbers, before and after +Verified — observed, with how +Not verified — and why (e.g. no dockerd locally; CI is the gate) +Known broken / left out — explicitly, including anything descoped +Risk + rollback — the riskiest hunk and how to revert it alone +``` + +Lead with anything that did not work. A merged PR that achieved nothing is the single most +important sentence in the report, and it must not appear after the summary of what went well. + +Give confidence per claim, not one blanket number, and make it mean something: "measured", "CI +verified", "unverified — reasoning only". If a number came from one host, say so. + +--- + +## The failure mode this loop keeps producing + +Every bug shipped from the source session came from one move: **measuring this host correctly, +then generalising it to all hosts.** + +- `/dev/shm` was 7.7 GB here, so a flag looked useless — but Home Assistant ignores `shm_size`, so + elsewhere it is Docker's 64 MB default and removing the flag reintroduces a crash loop. +- An MCP entry was identified by its URL — but that URL is the documented default, so the rule + would have deleted a user's hand-written configuration. +- A GPU probe created a hardware context — but that proved the driver worked, not that Chromium's + GPU path did. + +The pattern is always *inference standing in for detection*. Before changing a default, ask what +this is like on a host unlike yours. Prefer detecting the condition at runtime over asserting it. +When ownership matters, **record it rather than infer it**. + +## Token efficiency + +`rtk` wraps commands via hook automatically. Compress large structured output you will re-read +with `mcp__headroom__headroom_compress` (skip error/stack output). Use +`mcp__tokensave__tokensave_context` for code exploration. Redirect big output to a file and read +only what you need, and poll CI in a **background** task rather than blocking. + +## Bundled files + +| File | Use | +|---|---| +| `scripts/preflight.sh` | Tools, live-add-on check, revision-vs-running-image check. Exits 2 on mismatch | +| `scripts/measure.sh` | RAM (PSS/private) + CPU snapshot; reserved vs resident. Sample ≥20 s | +| `scripts/env_trace.sh` | Trace one env var through all four plumbing stages — for "my option did nothing" | +| `scripts/validate.sh` | Local linters + CI gates; `--vs-master` shows only findings your diff added | +| `scripts/pr_review.sh` | Fetch / reply to / resolve PR review threads; watch checks | +| `references/traps.md` | Repo-specific traps — read before implementing | + +Each script's header explains its reasoning; read the script when you use it. diff --git a/.claude/skills/hassio-addon-workflow/references/traps.md b/.claude/skills/hassio-addon-workflow/references/traps.md new file mode 100644 index 0000000000..0d8a3ada66 --- /dev/null +++ b/.claude/skills/hassio-addon-workflow/references/traps.md @@ -0,0 +1,212 @@ +# Repo-specific traps + +Things that look correct and are not. Each cost real time or shipped broken. Read this before +implementing; skim the headings, read the ones you're about to touch. + +The repo's own `CLAUDE.md` documents structure, Dockerfile conventions, `updater.json`, CI +workflows and lint rules — that is not repeated here. + +## Contents + +- [Environment and workspace](#environment-and-workspace) +- [Measurement](#measurement) +- [Passing values into base-image services](#passing-values-into-base-image-services) +- [Shell and bashio](#shell-and-bashio) +- [Dockerfile and architecture](#dockerfile-and-architecture) +- [Versioning](#versioning) +- [Chromium / Electron under Xvfb](#chromium--electron-under-xvfb) +- [CI and review bots](#ci-and-review-bots) + +--- + +## Environment and workspace + +**The checkout is probably on the wrong branch.** Checkouts under `/data/claude` are shared and +persistent; another session leaves them wherever it finished. A stale branch looks entirely +normal. Compare the add-on's `config.yaml` `version` against the running `$BUILD_VERSION` before +trusting anything you read. `scripts/preflight.sh` does this. + +**Never run `git stash` under `/data/claude`.** `refs/stash` is shared across every worktree and +concurrent session, so it is *not* isolated even in your own worktree. A bare `stash` / `stash +pop` pair in a clean worktree once restored another session's stash, producing conflict markers +in six untouched files. To compare a file against another revision use +`git show : > /tmp/x`. If a pop does go wrong: a conflicted pop **keeps** the stash +entry, so nothing is lost — confirm `git rev-parse HEAD` matches what you pushed, then +`git reset --hard HEAD`. + +**Work in a worktree under `/data`, not `/tmp`** — `/tmp` is `noexec`, so scripts there won't run. + +```bash +git worktree add --detach /data/claude/.work/ origin/master +``` + +**You cannot test the Docker build.** dockerd does not start in this environment. CI is the only +gate. One observed run took ~3 hours, with 20+ runs queued against 2 executing — that was account +runner contention, not the diff. Check `gh run list` before concluding your PR is stuck. Poll in +a background task, and never claim the build is verified when it hasn't run. + +## Measurement + +**Summed RSS overstates savings.** Shared library pages are counted once per process, so removing +a duplicate frees its *private* memory, not its RSS. Measured example: four MCP shims summed to +882 MB RSS but 643 MB PSS / 564 MB private, and per-process private ranged 54 MB down to 2 MB — +which completely changes which duplicate is worth removing. Quote private when arguing "removing +this saves N MB". + +**A large mapping is often not resident.** SysV/tmpfs segments are lazily populated. Xvfb's +506 MB framebuffer shows `Rss: 0` in `/proc//smaps`. Check before calling anything a leak. + +**`/proc/meminfo` and `free` show host figures** — there is no memory cgroup namespace here. +Never attribute those totals to the add-on. + +**`rtk` filters some command output.** For a complete listing, redirect to a file and read that +(`ps ... > $SP/ps.txt`), or use `rtk proxy `. + +## Passing values into base-image services + +The plumbing has four stages. `scripts/env_trace.sh ` walks all four and tells +you which one drops the value — use it rather than reasoning about this from memory. + +1. `/data/options.json` — the user's saved options. +2. **Injected export block** — `.templates/00-global_var.sh` writes a literal + `export