From f3cc1c4c173b9dbbc2e55235920afbb229e4c062 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 13 Aug 2026 21:24:37 -0700 Subject: [PATCH 1/2] Stop Playwright setup from deleting the package it just installed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting up the browser view failed on every container, and re-running it reproduced the same broken state, because the setup destroyed its own work. `install_packages` ran two `npm install --no-save` commands into /workspace, which has no package.json. With no manifest, npm treats the command line as the whole statement of what the tree should contain and prunes the rest, so installing `playwright` second removed the `@playwright/cli` installed first: "removed 3 packages", leaving an empty node_modules/@playwright/ behind playwright and playwright-core. That empty directory is exactly what the pane then reported as missing. The second install now names both specs; the first one is already present, so it costs nothing and is only there to stop npm pruning it. Two failures were waiting behind that one: Nothing in the tree ever configured the browser, so playwright-cli fell back to channel `chrome` — system Google Chrome — with the Chromium sandbox on. These containers forbid unprivileged user namespaces, so it aborted with "Failed to move to new namespace ... Operation not permitted"; on a base image without Google Chrome the same default failed as "Chromium distribution 'chrome' is not found". entrypoint.sh now seeds ~/.playwright/cli.config.json on every start, which is the only way to reach existing projects: ~/.playwright is inside the home volume, so an image copy would reach new projects only. The launch check passed for a configuration the viewer never uses. It launched bundled chromium with no channel, which resolves to chromium-headless-shell, while the viewer's config pins chrome-for-testing — the full chromium build, a separate download. A container could pass every check and still fail in the pane with 'Browser "chrome-for-testing" is not installed', which is what a stale chromium-1217 against a wanted chromium-1237 did. Chromium is now verified on both channels, the sandbox setting is stated rather than inherited from a default, and a failure names the channel. triple-c-playwright-heal repairs all of it on a container that is already broken, including the missing socat that makes the pane report "127.0.0.1 sent an invalid response" while the container side is perfectly healthy. It verifies by launching a browser rather than trusting the preceding steps — which is how the stale-revision case was found — and lives in /usr/local/bin so a fix to it can still reach an existing project. Co-Authored-By: Claude Opus 5 (1M context) --- app/src-tauri/src/browser_view/install.rs | 100 +++++--- container/Dockerfile | 7 + container/entrypoint.sh | 26 +++ container/triple-c-playwright-heal | 272 ++++++++++++++++++++++ 4 files changed, 379 insertions(+), 26 deletions(-) create mode 100755 container/triple-c-playwright-heal diff --git a/app/src-tauri/src/browser_view/install.rs b/app/src-tauri/src/browser_view/install.rs index c859b82..87aff0e 100644 --- a/app/src-tauri/src/browser_view/install.rs +++ b/app/src-tauri/src/browser_view/install.rs @@ -194,11 +194,24 @@ impl BrowserTarget { } } - /// The `channel` a launch check must pass. `None` means the bundled build. - fn channel(self) -> Option<&'static str> { + /// Every `channel` a launch check must pass, comma-separated, where + /// `default` means "no channel — the bundled build". + /// + /// Chromium is checked twice because the two consumers of this install do + /// not launch the same binary. A script calling `chromium.launch()` with + /// no channel gets `chromium-headless-shell`; the viewer reads + /// `~/.playwright/cli.config.json`, which pins channel + /// `chrome-for-testing`, and that resolves to the *full* `chromium-` + /// build — a separate download under the same `install chromium`. + /// + /// Checking only the first is how a container reaches "verified" and then + /// fails in the pane with `Browser "chrome-for-testing" is not installed`. + /// Observed on a real project, where a stale `chromium-1217` satisfied the + /// headless-shell launch while the viewer wanted `chromium-1237`. + fn channels(self) -> &'static str { match self { - Self::Chromium => None, - Self::Chrome => Some("chrome"), + Self::Chromium => "default,chrome-for-testing", + Self::Chrome => "chrome", } } } @@ -235,7 +248,7 @@ pub async fn install_packages( &format!("Installing @playwright/cli into {}/node_modules…", INSTALL_DIR), ); - let mut step = npm_install(app, project_id, container_id, VIEWER_PACKAGE).await?; + let mut step = npm_install(app, project_id, container_id, &[VIEWER_PACKAGE]).await?; if step.exit_code != 0 { return Err(format!( "npm couldn't install the viewer package in this container (exit {}).\n\nnpm said:\n{}", @@ -246,9 +259,15 @@ pub async fn install_packages( // Second, `playwright` at the version the viewer package pins — see // `VIEWER_PACKAGE`. Installing it as `@latest` is what splits the tree. + // + // The viewer package is named *again* here. It is already installed, so + // this adds no work, but omitting it is what made npm prune it back out — + // see the note on `npm_install`. The pin can only be read after the first + // install has written the manifest, which is why this stays two commands + // rather than one. let spec = pinned_playwright_spec(container_id).await; emit_progress(app, project_id, &format!("Installing {}…", spec)); - let second = npm_install(app, project_id, container_id, &spec).await?; + let second = npm_install(app, project_id, container_id, &[VIEWER_PACKAGE, &spec]).await?; if second.exit_code != 0 { return Err(format!( "npm couldn't install {} in this container (exit {}).\n\nnpm said:\n{}", @@ -290,7 +309,7 @@ pub async fn install_packages( }) } -/// One `npm install` of one spec, into [`INSTALL_DIR`], as `claude`. +/// One `npm install` of one or more specs, into [`INSTALL_DIR`], as `claude`. /// /// `env VAR=… cmd` rather than an exec env: it keeps the one exec path in /// `docker/exec.rs` untouched, and `env` is a real binary so no shell is @@ -298,13 +317,24 @@ pub async fn install_packages( /// has no postinstall (verified — `playwright@1.62.1` declares no `scripts` at /// all), but if a future release brings the browser download back, this step /// must stay small and the download must stay the step the user asked for. +/// +/// **Every package that must survive has to appear in `specs`.** `--no-save` +/// in a directory with no `package.json` — which [`INSTALL_DIR`] is — leaves +/// npm with the command line as its only statement of what the tree should +/// contain, and npm ≥7 reconciles the tree against that on every run by +/// removing whatever it now considers extraneous. Installing `@playwright/cli` +/// and then installing `playwright` in a second command therefore *deletes the +/// first one*: verified in a container, `removed 3 packages`, leaving an empty +/// `node_modules/@playwright/` behind `playwright` and `playwright-core`. That +/// empty directory is why a fresh setup could report success and still leave +/// the pane saying `@playwright/cli` was not installed. async fn npm_install( app: &AppHandle, project_id: &str, container_id: &str, - spec: &str, + specs: &[&str], ) -> Result { - let cmd = vec![ + let mut cmd = vec![ "env".to_string(), "PLAYWRIGHT_SKIP_BROWSER_DOWNLOAD=1".to_string(), "npm".to_string(), @@ -313,8 +343,8 @@ async fn npm_install( "--no-save".to_string(), "--no-fund".to_string(), "--no-audit".to_string(), - spec.to_string(), ]; + cmd.extend(specs.iter().map(|s| s.to_string())); run_step( app, project_id, @@ -704,7 +734,7 @@ async fn verify_launch( ], vec![ format!("TRIPLE_C_PW_DIR={}", dir), - format!("TRIPLE_C_PW_CHANNEL={}", target.channel().unwrap_or("")), + format!("TRIPLE_C_PW_CHANNELS={}", target.channels()), format!("TRIPLE_C_PW_URL={}", REACHABILITY_URL), ], ); @@ -795,27 +825,43 @@ fn parse_launch_output(output: &str) -> LaunchVerdict { /// The launch check. One `argv` element, no newlines, same contract as the /// detection probe. /// -/// Playwright leaves the Chromium sandbox disabled by default, which is what -/// makes this work in a container at all. The timeout exists so a browser that -/// hangs on a missing library still returns a verdict rather than sitting there -/// until the exec is torn down. The navigation is best-effort and never decides -/// `ok` — it exists to tell a TLS-intercepted network apart from a broken -/// install. +/// `chromiumSandbox` is set explicitly rather than left to Playwright's +/// default, so this check states the same thing the seeded +/// `cli.config.json` does instead of agreeing with it by coincidence. The +/// containers forbid unprivileged user namespaces, so a sandboxed Chromium +/// aborts on launch; nothing here should be able to drift back into testing a +/// configuration the viewer will not use. +/// +/// Each channel in `TRIPLE_C_PW_CHANNELS` is launched in turn — see +/// [`BrowserTarget::channels`] for why Chromium needs two — and a failure +/// names the channel that failed, because "is not installed" is meaningless +/// without it. Only the last launch loads a page: the navigation is +/// best-effort, never decides `ok`, and exists to tell a TLS-intercepted +/// network apart from a broken install, so doing it once is enough. +/// +/// The timeout exists so a browser that hangs on a missing library still +/// returns a verdict rather than sitting there until the exec is torn down. const LAUNCH_PROBE: &str = concat!( - r#"const d=process.env.TRIPLE_C_PW_DIR,ch=process.env.TRIPLE_C_PW_CHANNEL||undefined,u=process.env.TRIPLE_C_PW_URL;"#, + r#"const d=process.env.TRIPLE_C_PW_DIR,chs=process.env.TRIPLE_C_PW_CHANNELS||"default",u=process.env.TRIPLE_C_PW_URL;"#, r#"let done=false;const say=(ok,detail,nav)=>{if(done)return;done=true;"#, r#"process.stdout.write("\n__TRIPLE_C_BROWSER_LAUNCH__"+JSON.stringify({ok,detail,nav:nav||null})+"\n");};"#, r#"const one=(e)=>String((e&&e.message)||e).split("\n").slice(0,8).join(" | ");"#, r#"const t=setTimeout(()=>{say(false,"the browser did not finish starting within 90s");process.exit(0);},90000);"#, - r#"(async()=>{let b=null;try{const {chromium}=require(d);b=await chromium.launch(ch?{channel:ch}:{});"#, - r#"let v="";try{v=b.version();}catch(e){}"#, - r#"let nav={ok:true,cert:false,detail:""};"#, + r#"(async()=>{let b=null,cur="";try{const {chromium}=require(d);"#, + r#"const list=chs.split(",").map(s=>s.trim()).filter(Boolean);"#, + r#"let v="",nav={ok:true,cert:false,detail:""};"#, + r#"for(let i=0;i` in /workspace, +# which has no package.json, prunes packages npm considers extraneous, so +# installing @playwright/cli and then installing playwright wipes the +# first one and leaves an empty node_modules/@playwright/. That directory +# reads as "installed" to a naive check, which is why this script tests +# the package *entry point*. +# +# 2. Bundled chromium missing or the wrong revision. Browsers live in the +# home volume and outlive any single @playwright/cli install, so a stale +# chromium- is routinely present while the installed playwright-core +# wants a newer one. Must be installed AS claude: run as root it lands in +# /root/.cache/ms-playwright where the agent cannot see it. +# +# 3. No cli.config.json — the one that breaks an otherwise clean install. +# With no config, playwright-cli resolves to channel `chrome` (system +# Google Chrome) with the sandbox ON. These containers forbid unprivileged +# user namespaces, so Chrome aborts with "Failed to move to new namespace +# ... Operation not permitted". On newer base images Chrome is not present +# at all and it fails with "Chromium distribution 'chrome' is not found". +# Same root cause both ways: the default channel is wrong here. +# +# 4. The storage-state file the config points at is missing. Playwright +# treats an unreadable storageState as a hard error on every launch, not +# as "no saved state", so the file has to exist from the very first run. +# +# 5. xvfb or socat missing (older base images only). Headless Playwright +# needs neither; the `playwright-cli show` dashboard needs xvfb, and the +# browser-view pane needs socat — without it the pane reports +# "127.0.0.1 sent an invalid response" while the container side is fine. +# +# Usage: triple-c-playwright-heal [--seed-config-only] [--force-config] [--quiet] +# --seed-config-only only ensure the config and its storage-state file +# exist. No npm install, no browser download, no apt, no +# verify launch. Cheap and offline — this is the mode +# entrypoint.sh runs on every container start. +# --force-config overwrite an existing config instead of keeping it +# --quiet print only problems and repairs, not healthy no-ops + +set -u + +TARGET_USER=claude +TARGET_HOME=/home/claude +PW_DIR=/workspace +CONFIG_DIR="$TARGET_HOME/.playwright" +CONFIG_FILE="$CONFIG_DIR/cli.config.json" +STATE_FILE="$CONFIG_DIR/storage-state.json" +CLI_ENTRY="$PW_DIR/node_modules/@playwright/cli/playwright-cli.js" + +FORCE_CONFIG=0 +QUIET=0 +SEED_ONLY=0 +for arg in "$@"; do + case "$arg" in + --force-config) FORCE_CONFIG=1 ;; + --quiet) QUIET=1 ;; + --seed-config-only) SEED_ONLY=1 ;; + *) echo "playwright-heal: unknown option: $arg" >&2; exit 2 ;; + esac +done + +changed=0 +failed=0 + +say() { [ "$QUIET" = 1 ] || echo "playwright-heal: $*"; } +warn() { echo "playwright-heal: $*" >&2; } +did() { changed=1; echo "playwright-heal: $*"; } + +# Run as claude whether we were invoked as root (docker exec / entrypoint) or +# as claude (terminal session). Nothing user-visible may end up root-owned. +as_claude() { + if [ "$(id -u)" = 0 ]; then + su "$TARGET_USER" -s /bin/sh -c "$1" + else + sh -c "$1" + fi +} + +# ── 1. @playwright/cli ─────────────────────────────────────────────────────── +if [ "$SEED_ONLY" = 1 ]; then + : +elif [ -f "$CLI_ENTRY" ]; then + say "@playwright/cli present" +else + # An empty leftover @playwright/ can make npm consider the tree settled. + if [ -d "$PW_DIR/node_modules/@playwright" ]; then + say "clearing partial @playwright install" + rm -rf "$PW_DIR/node_modules/@playwright" + fi + say "installing @playwright/cli..." + if as_claude "cd $PW_DIR && npm install --no-save --no-audit --no-fund @playwright/cli" >/tmp/pw-heal-npm.log 2>&1; then + did "installed @playwright/cli" + else + warn "npm install failed; see /tmp/pw-heal-npm.log" + failed=1 + fi +fi + +# ── 2. bundled chromium ────────────────────────────────────────────────────── +# Ask Playwright where *this* version's chromium belongs rather than globbing +# chromium-*, which would call a stale revision "present" and then fail at +# launch with 'Browser "chrome-for-testing" is not installed'. --dry-run prints +# the install location for the installed version and downloads nothing. +if [ "$SEED_ONLY" = 1 ]; then + : +else + chromium_dir="" + if [ -f "$PW_DIR/node_modules/playwright-core/cli.js" ]; then + chromium_dir=$(as_claude "cd $PW_DIR && node node_modules/playwright-core/cli.js install --dry-run chromium 2>/dev/null" \ + | awk '/Install location:/ { print $3; exit }') + fi + + if [ -n "$chromium_dir" ] && [ -d "$chromium_dir" ]; then + say "chromium present ($(basename "$chromium_dir"))" + elif [ -f "$PW_DIR/node_modules/playwright-core/cli.js" ]; then + say "downloading chromium (~300 MB)..." + if as_claude "cd $PW_DIR && node node_modules/playwright-core/cli.js install chromium" >/tmp/pw-heal-browser.log 2>&1; then + did "installed chromium" + else + warn "chromium install failed; see /tmp/pw-heal-browser.log" + failed=1 + fi + else + warn "playwright-core missing, cannot install chromium" + failed=1 + fi +fi + +# ── 3. cli.config.json ─────────────────────────────────────────────────────── +# The *global* config, not a project-level .playwright/, because the project +# one resolves relative to the current working directory and silently stops +# applying the moment you cd elsewhere. +# +# `chrome-for-testing` is the only recognised chromium alias — "chromium" is +# not one and falls back to system Chrome. chromiumSandbox:false is what +# actually appends --no-sandbox. +write_config() { + mkdir -p "$CONFIG_DIR" || return 1 + cat > "$CONFIG_FILE" </dev/null || true +} + +# storageState is a *load* path, and Playwright reads it at context creation. +# A path that does not exist is not treated as "no saved state" — it is a hard +# error, "Error reading storage state from …", on every single launch. So the +# file has to exist before the config that names it can be used at all, and it +# has to be recreated if anything deletes it. An empty state is valid and +# behaves exactly like no state. +# +# The path is read back out of the config rather than assumed, so a +# hand-edited config pointing somewhere else still gets its file created +# instead of being silently broken by ours. +ensure_state_file() { + [ -f "$CONFIG_FILE" ] || return 0 + state_path=$(grep -o '"storageState"[[:space:]]*:[[:space:]]*"[^"]*"' "$CONFIG_FILE" 2>/dev/null \ + | sed 's/.*"\([^"]*\)"[[:space:]]*$/\1/') + [ -n "$state_path" ] || return 0 + [ -f "$state_path" ] && return 0 + mkdir -p "$(dirname "$state_path")" 2>/dev/null + printf '{\n "cookies": [],\n "origins": []\n}\n' > "$state_path" || return 1 + chown "$TARGET_USER:$TARGET_USER" "$state_path" 2>/dev/null || true + did "created empty $state_path (storageState needs it to exist)" +} + +if [ ! -f "$CONFIG_FILE" ]; then + if write_config; then did "wrote $CONFIG_FILE"; else warn "could not write $CONFIG_FILE"; failed=1; fi +elif [ "$FORCE_CONFIG" = 1 ]; then + if write_config; then did "overwrote $CONFIG_FILE (--force-config)"; else warn "could not write $CONFIG_FILE"; failed=1; fi +elif grep -q '"chromiumSandbox"[[:space:]]*:[[:space:]]*false' "$CONFIG_FILE" 2>/dev/null; then + say "config present and disables the sandbox" +else + # Present but hand-edited into a state that will not launch. Do not clobber + # deliberate config silently; say what is wrong and how to replace it. + warn "config at $CONFIG_FILE does not set chromiumSandbox:false — the browser will likely fail to launch. Re-run with --force-config to replace it." +fi + +# Unconditional: the config may name a storageState this run did not write — +# one seeded by an older version of this script, or edited by hand — and a +# missing file there breaks every launch. +ensure_state_file || { warn "could not create the storage-state file"; failed=1; } + +if [ "$SEED_ONLY" = 1 ]; then + [ "$failed" = 1 ] && exit 1 + exit 0 +fi + +# ── 4. xvfb (headed dashboard only) ────────────────────────────────────────── +# Current base images get this from `playwright install-deps` (its `tools` +# group); older ones predate that layer. Headless never needs it, so a missing +# xvfb is a note, not a failure. +if command -v Xvfb >/dev/null 2>&1; then + say "xvfb present" +elif [ "$(id -u)" = 0 ]; then + say "installing xvfb (needed only for the headed dashboard)..." + if (apt-get update -qq && DEBIAN_FRONTEND=noninteractive apt-get install -y -qq xvfb) >/tmp/pw-heal-xvfb.log 2>&1; then + did "installed xvfb" + else + warn "xvfb install failed (headless still works); see /tmp/pw-heal-xvfb.log" + fi +else + say "xvfb missing and not running as root — skipping (headless still works)" +fi + +# ── 4b. socat (the browser-view pane's tunnel) ─────────────────────────────── +# Not Playwright's, but the same class of failure and it presents as a +# Playwright problem: the pane's host-side proxy reaches the dashboard by +# running `socat` *inside* the container over a Docker exec. On a container old +# enough to predate socat in the base image, that exec produces something that +# is not an HTTP response, and the webview reports "127.0.0.1 sent an invalid +# response" — with the container side working perfectly. A project keeps the +# base image it was first built from until it is migrated, so this is the +# normal case on an older project, not an exotic one. +if command -v socat >/dev/null 2>&1; then + say "socat present" +elif [ "$(id -u)" = 0 ]; then + say "installing socat (needed by the browser-view pane)..." + if (apt-get update -qq && DEBIAN_FRONTEND=noninteractive apt-get install -y -qq socat) >/tmp/pw-heal-socat.log 2>&1; then + did "installed socat" + else + warn "socat install failed; the browser-view pane will report an invalid response. See /tmp/pw-heal-socat.log" + failed=1 + fi +else + warn "socat missing and not running as root — the browser-view pane will report an invalid response" +fi + +# ── 5. verify by actually launching ────────────────────────────────────────── +# Every step above can report success while the browser still refuses to +# start — that is precisely how this broke. A dedicated session name keeps +# this clear of whatever the agent already has open. +if [ -f "$CLI_ENTRY" ]; then + verify_out=$(as_claude "cd /tmp && timeout 90 node $CLI_ENTRY -s=heal-verify open 'data:text/html,

ok

' 2>&1") + if printf '%s' "$verify_out" | grep -q 'opened with pid'; then + say "verified: browser launches" + as_claude "cd /tmp && timeout 30 node $CLI_ENTRY -s=heal-verify close" >/dev/null 2>&1 + else + warn "browser still fails to launch:" + printf '%s\n' "$verify_out" | grep -m4 -E 'namespace|Check failed|is not installed|is not found|missing dependencies|Error' >&2 + failed=1 + fi +else + warn "@playwright/cli not installed — nothing to verify" + failed=1 +fi + +[ "$failed" = 1 ] && exit 1 +[ "$changed" = 1 ] && say "done — repairs applied" || say "done — nothing to repair" +exit 0 -- 2.52.0 From 84a67fcd0d4c6ad7c07971309fc99717628d5810 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Thu, 13 Aug 2026 21:24:51 -0700 Subject: [PATCH 2/2] Stop an empty base-image label from silencing the migration notice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A project can be out of date and say nothing about it, in two ways that compound: the lineage lookup treats "unknown" as an answer, and the fallback that exists for unknown lineage disappears when its probe fails. `create_container` always writes triple-c.base-image-id, even when the value is unknown — deliberately, so an inherited image label cannot ride a snapshot forever. That makes Some("") the ordinary reading from a container whose lineage was never established. The lookup filtered for emptiness only on the final result, so that empty string satisfied the container branch and skipped the snapshot entirely: a snapshot that had recorded a real lineage was never consulted, and the project reported "unknown" with the answer one lookup away. Each source is now filtered before it can answer, in pick_recorded_lineage, which is a plain function so the case has a test that fails against the old logic. A genuinely pre-label project stays unknown, and should: its ancestor is not knowable, and inventing one would make it look permanently current. The probe is the intended signal for those — but if the probe failed, get_container_staleness returned early with nothing populated, the banner found no gaps and rendered null, and the probe_error it already knew how to display sat behind a gate that returned before reaching it. Silence there is indistinguishable from "up to date", and it is likeliest for the oldest and largest projects, whose manifests are the ones apt to exceed the inspection limit — one real project measured 6.93 MB against an 8 MB cap. An unknown-lineage container whose probe failed now says the check could not be completed, with the reason, under the tone that means unresolved rather than the one that means something is wrong. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/commands/migration_commands.rs | 71 ++++++++++++++++--- .../home/ContainerMigrationBanner.test.tsx | 40 +++++++++++ .../home/ContainerMigrationBanner.tsx | 29 ++++++-- 3 files changed, 124 insertions(+), 16 deletions(-) diff --git a/app/src-tauri/src/commands/migration_commands.rs b/app/src-tauri/src/commands/migration_commands.rs index 7b86388..f43e348 100644 --- a/app/src-tauri/src/commands/migration_commands.rs +++ b/app/src-tauri/src/commands/migration_commands.rs @@ -73,6 +73,25 @@ use crate::AppState; /// Report how far behind the current base image a project's container is, and /// what migrating it would actually carry across. /// +/// Choose the recorded lineage from the two places it can be written, most +/// authoritative first: the live container's label, then the snapshot image's. +/// +/// **An empty label is absence, not an answer.** `create_container` always +/// writes `triple-c.base-image-id`, even when the value is unknown — that is +/// deliberate, because Docker merges an image's labels into a container's and +/// an inherited value would otherwise ride a snapshot forever. The consequence +/// is that `Some("")` is the *common* reading from a container whose lineage +/// was never established, so treating it as an answer silently skips the +/// snapshot, which may well have recorded a real one. +fn pick_recorded_lineage( + from_container: Option, + from_snapshot: Option, +) -> Option { + from_container + .filter(|v| !v.is_empty()) + .or_else(|| from_snapshot.filter(|v| !v.is_empty())) +} + /// Read-only. Runs two filesystem probes (~3 s each) and is therefore meant to /// be called on demand, not polled. #[tauri::command] @@ -98,20 +117,24 @@ pub async fn get_container_staleness( // Lineage, most authoritative source first: the live container's label, // then the snapshot image's. Both are written by `create_container` and // propagated onto the snapshot by `docker commit`. + // Each source is filtered for emptiness *before* it is allowed to satisfy + // the lookup. `create_container` always writes this label, even when the + // value is unknown — deliberately, so an inherited image label cannot ride + // a snapshot forever — which means the container's copy is very often + // `Some("")`. Filtering only the final result let that empty string count + // as an answer and skip the snapshot entirely, so a snapshot that *did* + // record a lineage was never consulted and the project reported "unknown" + // with the information sitting one lookup away. let container_id = docker::find_existing_container(&project).await.unwrap_or(None); - let recorded = match &container_id { + let from_container = match &container_id { Some(id) => container_label(id, mig::LABEL_BASE_IMAGE_ID).await, None => None, - } - .or_else(|| None); - let recorded = match recorded { - Some(v) => Some(v), - None => mig::image_labels(&snapshot_image) - .await - .get(mig::LABEL_BASE_IMAGE_ID) - .cloned(), - } - .filter(|v| !v.is_empty()); + }; + let from_snapshot = mig::image_labels(&snapshot_image) + .await + .get(mig::LABEL_BASE_IMAGE_ID) + .cloned(); + let recorded = pick_recorded_lineage(from_container, from_snapshot); out.base_image_id = recorded.clone(); out.known = recorded.is_some(); @@ -1671,6 +1694,32 @@ fn summarize( mod tests { use super::*; + #[test] + fn an_empty_lineage_label_is_absence_and_falls_through_to_the_snapshot() { + let some = |s: &str| Some(s.to_string()); + + // The regression: the container always carries the label, so an + // unknown lineage reads as `Some("")`. Letting that satisfy the lookup + // skipped a snapshot that had recorded the real thing. + assert_eq!( + pick_recorded_lineage(some(""), some("sha256:base")), + some("sha256:base") + ); + + // Ordinary precedence still holds: the container wins when it has one. + assert_eq!( + pick_recorded_lineage(some("sha256:container"), some("sha256:snapshot")), + some("sha256:container") + ); + assert_eq!(pick_recorded_lineage(None, some("sha256:snap")), some("sha256:snap")); + + // Genuinely unknown stays unknown — "probe instead", never a lineage + // invented to make the comparison succeed. + assert_eq!(pick_recorded_lineage(None, None), None); + assert_eq!(pick_recorded_lineage(some(""), some("")), None); + assert_eq!(pick_recorded_lineage(some(""), None), None); + } + #[test] fn byte_sizes_read_the_way_a_disk_warning_should() { assert_eq!(human_bytes(512), "512 B"); diff --git a/app/src/components/projects/home/ContainerMigrationBanner.test.tsx b/app/src/components/projects/home/ContainerMigrationBanner.test.tsx index 9b328a5..d140053 100644 --- a/app/src/components/projects/home/ContainerMigrationBanner.test.tsx +++ b/app/src/components/projects/home/ContainerMigrationBanner.test.tsx @@ -127,6 +127,46 @@ describe("ContainerMigrationBanner", () => { expect(container).toBeEmptyDOMElement(); }); + it("speaks up when an unlabelled container could not be probed at all", () => { + // The probe is the only signal a container with no lineage label has. If + // it fails and the banner stays silent, that is indistinguishable from + // "up to date" — the exact reading that let an out-of-date project go + // unnoticed indefinitely. + renderBanner( + migration({ + staleness: { + ...FRESH, + known: false, + stale: false, + probe_error: "output exceeded the inspection limit", + }, + probeSettled: false, + }), + ); + expect( + screen.getByText(/Container base could not be checked/i), + ).toBeInTheDocument(); + expect( + screen.getByText(/output exceeded the inspection limit/i), + ).toBeInTheDocument(); + // And it must not pose as a finding about the container itself. + expect( + screen.queryByText(/Container is missing things/i), + ).not.toBeInTheDocument(); + }); + + it("stays quiet when a labelled container's probe fails but its lineage is current", () => { + // `known` means the version comparison already answered the question, so + // a failed probe is not grounds to raise anything. + const { container } = renderBanner( + migration({ + staleness: { ...FRESH, probe_error: "could not exec in the container" }, + probeSettled: false, + }), + ); + expect(container).toBeEmptyDOMElement(); + }); + it("disables the action and explains why while the container is running", () => { renderBanner(migration({ staleness: STALE }), false); expect( diff --git a/app/src/components/projects/home/ContainerMigrationBanner.tsx b/app/src/components/projects/home/ContainerMigrationBanner.tsx index e8dbae5..c359d35 100644 --- a/app/src/components/projects/home/ContainerMigrationBanner.tsx +++ b/app/src/components/projects/home/ContainerMigrationBanner.tsx @@ -131,7 +131,15 @@ export default function ContainerMigrationBanner({ const probeFoundGaps = !staleness.known && (staleness.missing_features.length > 0 || staleness.missing_paths.length > 0); - if (!staleness.stale && !probeFoundGaps) return null; + + // The probe is the *only* signal a container with no lineage label has, so + // when it fails there is nothing left to be quiet about. Staying silent here + // is indistinguishable from "everything is fine" — and it is the likeliest + // outcome for the oldest, largest projects, whose manifests are the ones apt + // to exceed the inspection limit. Say that the check did not run instead. + const probeUnavailable = !staleness.known && !!staleness.probe_error; + + if (!staleness.stale && !probeFoundGaps && !probeUnavailable) return null; const snapshot = formatSnapshotDate(staleness.snapshot_created_at); const features = joinFeatures(staleness.missing_features); @@ -139,16 +147,25 @@ export default function ContainerMigrationBanner({ return (
@@ -158,7 +175,9 @@ export default function ContainerMigrationBanner({ ? snapshot ? `Running on a saved image from ${snapshot}.` : "Running on a saved image older than the current base." - : "This container predates base-image tracking, so it was probed directly."} + : probeUnavailable + ? "This container predates base-image tracking, so probing it is the only way to tell whether it is behind — and that did not complete." + : "This container predates base-image tracking, so it was probed directly."}

{staleness.missing_features.length > 0 && ( -- 2.52.0