diff --git a/.github/workflows/pr_validation.yml b/.github/workflows/pr_validation.yml index 1c880c520..2347aaf44 100644 --- a/.github/workflows/pr_validation.yml +++ b/.github/workflows/pr_validation.yml @@ -62,19 +62,6 @@ jobs: - name: "dotnet build" run: dotnet build -c Release - # Pre-warm the PowerShell host probe so cold-start (Defender scan, first-run - # init) doesn't push the in-test 5s probe timeout on loaded Windows runners. - # Warm both pwsh.exe and the powershell.exe fallback — the resolver probes - # the fallback when pwsh is missing or its probe fails. Use -Command "exit 0" - # (no interpolated variables) so the outer pwsh shell can't mangle the args. - - name: "Warm PowerShell host probe" - if: runner.os == 'Windows' - shell: pwsh - run: | - pwsh -NoLogo -NoProfile -NonInteractive -Command "exit 0" - & "$env:SystemRoot\System32\WindowsPowerShell\v1.0\powershell.exe" -NoLogo -NoProfile -NonInteractive -Command "exit 0" - exit 0 - # .NET Framework tests can't run reliably on Linux, so we only do .NET 8 - name: "dotnet test" @@ -208,7 +195,7 @@ jobs: - name: "Install shell integration test dependencies" if: runner.os == 'Linux' - run: sudo apt-get update && sudo apt-get install --yes fish zsh + run: bash scripts/smoke/install-shell-test-dependencies.sh - name: "install.sh smoke test" if: runner.os != 'Windows' diff --git a/.github/workflows/smoke.yml b/.github/workflows/smoke.yml index 618934ddd..4aa0188f5 100644 --- a/.github/workflows/smoke.yml +++ b/.github/workflows/smoke.yml @@ -199,9 +199,6 @@ jobs: name: netclaw-native-binaries-linux-x64-${{ github.run_id }}-${{ github.run_attempt }} path: ./publish - - name: Install ImageMagick - run: sudo apt-get install -y --no-install-recommends imagemagick - - name: Mark binaries executable run: | chmod +x publish/cli/netclaw publish/daemon/netclawd publish/mcp-server/Netclaw.SmokeMcpServer diff --git a/scripts/smoke/count-png-differences.py b/scripts/smoke/count-png-differences.py new file mode 100644 index 000000000..3c4aef29f --- /dev/null +++ b/scripts/smoke/count-png-differences.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +"""Count pixels that differ between two PNG files.""" + +from pathlib import Path +import struct +import subprocess +import sys + + +def decode_rgba(path: Path) -> tuple[tuple[int, int], bytes]: + header = path.read_bytes()[:24] + if len(header) != 24 or header[:8] != b"\x89PNG\r\n\x1a\n": + raise ValueError(f"{path} is not a PNG file.") + + dimensions = struct.unpack(">II", header[16:24]) + result = subprocess.run( + [ + "ffmpeg", + "-v", + "error", + "-i", + str(path), + "-f", + "rawvideo", + "-pix_fmt", + "rgba", + "-frames:v", + "1", + "-", + ], + check=True, + stdout=subprocess.PIPE, + ) + return dimensions, result.stdout + + +def main() -> int: + if len(sys.argv) != 3: + print("Usage: count-png-differences.py ", file=sys.stderr) + return 2 + + try: + baseline_dimensions, baseline = decode_rgba(Path(sys.argv[1])) + actual_dimensions, actual = decode_rgba(Path(sys.argv[2])) + except (OSError, subprocess.CalledProcessError, ValueError) as error: + print(error, file=sys.stderr) + return 1 + + if baseline_dimensions != actual_dimensions or len(baseline) != len(actual): + print("PNG dimensions do not match.", file=sys.stderr) + return 1 + + differences = sum( + baseline[index : index + 4] != actual[index : index + 4] + for index in range(0, len(baseline), 4) + ) + print(differences) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/scripts/smoke/install-shell-test-dependencies.sh b/scripts/smoke/install-shell-test-dependencies.sh new file mode 100755 index 000000000..264d07720 --- /dev/null +++ b/scripts/smoke/install-shell-test-dependencies.sh @@ -0,0 +1,80 @@ +#!/usr/bin/env bash +# Install the real fish and zsh processes that the installer smoke test uses. +# The fixed files avoid apt index and mirror resolution during the CI gate. + +set -euo pipefail + +FISH_VERSION="${FISH_VERSION:-4.8.1}" +FISH_SHA256="${FISH_SHA256:-39cab35242ab77bfdbce73b473000c3b045aaf2fe0951b042199bb7fdba3df78}" +ZSH_VERSION="${ZSH_VERSION:-5.9-6ubuntu2}" +ZSH_SHA256="${ZSH_SHA256:-bd5cc8dd3a01a6db38c0a815d75202c356a9c7f378674ba7bed9bc86dcba8af0}" +SHELL_TEST_BIN_DIR="${SHELL_TEST_BIN_DIR:-/usr/local/bin}" + +if [[ "$(uname -s)/$(uname -m)" != "Linux/x86_64" ]]; then + echo "ERROR: The pinned shell test files support Linux x86_64 only." >&2 + exit 1 +fi + +for dependency in curl dpkg-deb sha256sum tar; do + if ! command -v "$dependency" >/dev/null 2>&1; then + echo "ERROR: '${dependency}' is required to install the shell test files." >&2 + exit 1 + fi +done + +install_file() { + local source="$1" + local destination="$2" + + if [[ -w "$SHELL_TEST_BIN_DIR" ]]; then + install -m 0755 "$source" "$destination" + else + sudo install -m 0755 "$source" "$destination" + fi +} + +if [[ ! -d "$SHELL_TEST_BIN_DIR" ]]; then + if [[ -w "$(dirname "$SHELL_TEST_BIN_DIR")" ]]; then + install -d "$SHELL_TEST_BIN_DIR" + else + sudo install -d "$SHELL_TEST_BIN_DIR" + fi +fi + +temporary_dir="$(mktemp -d)" +trap 'rm -rf "$temporary_dir"' EXIT + +if command -v fish >/dev/null 2>&1; then + fish_path="$(command -v fish)" +else + fish_archive="$temporary_dir/fish.tar.xz" + fish_url="https://github.com/fish-shell/fish-shell/releases/download/${FISH_VERSION}/fish-${FISH_VERSION}-linux-x86_64.tar.xz" + + echo "Downloading fish ${FISH_VERSION} from its fixed upstream file." + curl -fsSL "$fish_url" -o "$fish_archive" + echo "${FISH_SHA256} ${fish_archive}" | sha256sum -c - + tar -xJf "$fish_archive" -C "$temporary_dir" + + fish_path="$SHELL_TEST_BIN_DIR/fish" + install_file "$temporary_dir/fish" "$fish_path" +fi + +if command -v zsh >/dev/null 2>&1; then + zsh_path="$(command -v zsh)" +else + zsh_package="$temporary_dir/zsh.deb" + zsh_root="$temporary_dir/zsh-root" + zsh_url="https://archive.ubuntu.com/ubuntu/pool/main/z/zsh/zsh_${ZSH_VERSION}_amd64.deb" + + echo "Downloading zsh ${ZSH_VERSION} from its fixed Ubuntu archive file." + curl -fsSL "$zsh_url" -o "$zsh_package" + echo "${ZSH_SHA256} ${zsh_package}" | sha256sum -c - + mkdir -p "$zsh_root" + dpkg-deb -x "$zsh_package" "$zsh_root" + + zsh_path="$SHELL_TEST_BIN_DIR/zsh" + install_file "$zsh_root/bin/zsh" "$zsh_path" +fi + +"$fish_path" --version +"$zsh_path" --version diff --git a/scripts/smoke/install-smoke.sh b/scripts/smoke/install-smoke.sh index 4cabb0dd5..740c76786 100755 --- a/scripts/smoke/install-smoke.sh +++ b/scripts/smoke/install-smoke.sh @@ -521,17 +521,18 @@ fi # Zsh: resolve a non-exported ZDOTDIR from .zshenv, then execute the selected # startup file under zsh so a Bash-compatible false positive cannot pass. if command -v zsh >/dev/null 2>&1; then + ZSH_EXECUTABLE="$(command -v zsh)" ZSH_HOME="$WORK/shell-zsh" ZDOT_DIR="$ZSH_HOME/custom-zdotdir" ZSH_INSTALL="$ZSH_HOME/netclaw install's/bin" mkdir -p "$ZDOT_DIR" printf "ZDOTDIR='%s'\n" "$ZDOT_DIR" > "$ZSH_HOME/.zshenv" printf '# existing zsh config\n' > "$ZDOT_DIR/.zshrc" - if (unset ZDOTDIR; run_unix_installer "$(command -v zsh)" "$ZSH_HOME" "$ZSH_INSTALL" >/dev/null) \ - && (unset ZDOTDIR; run_unix_installer "$(command -v zsh)" "$ZSH_HOME" "$ZSH_INSTALL" >/dev/null); then + if (unset ZDOTDIR; run_unix_installer "$ZSH_EXECUTABLE" "$ZSH_HOME" "$ZSH_INSTALL" >/dev/null) \ + && (unset ZDOTDIR; run_unix_installer "$ZSH_EXECUTABLE" "$ZSH_HOME" "$ZSH_INSTALL" >/dev/null); then ZSH_INSTALL_PHYSICAL=$(cd "$ZSH_INSTALL" && pwd -P) zsh_path=$(PATH="/usr/bin:/bin" ZDOTDIR="$ZDOT_DIR" \ - zsh -f -c 'source "$ZDOTDIR/.zshrc"; print -rn -- "$PATH"') + "$ZSH_EXECUTABLE" -f -c 'source "$ZDOTDIR/.zshrc"; print -rn -- "$PATH"') assert_path_once "zsh" "$zsh_path" "$ZSH_INSTALL_PHYSICAL" if [ ! -e "$ZSH_HOME/.zshrc" ]; then pass "zsh: non-exported ZDOTDIR is authoritative" @@ -547,15 +548,16 @@ fi # Fish owns a native conf.d file. Execute that file with fish, not Bash. if command -v fish >/dev/null 2>&1; then + FISH_EXECUTABLE="$(command -v fish)" FISH_HOME="$WORK/shell-fish" FISH_INSTALL="$FISH_HOME/netclaw install's/bin" FISH_RC="$FISH_HOME/.config/fish/conf.d/netclaw.fish" if XDG_CONFIG_HOME="$FISH_HOME/.config" \ - run_unix_installer "$(command -v fish)" "$FISH_HOME" "$FISH_INSTALL" >/dev/null \ + run_unix_installer "$FISH_EXECUTABLE" "$FISH_HOME" "$FISH_INSTALL" >/dev/null \ && XDG_CONFIG_HOME="$FISH_HOME/.config" \ - run_unix_installer "$(command -v fish)" "$FISH_HOME" "$FISH_INSTALL" >/dev/null; then + run_unix_installer "$FISH_EXECUTABLE" "$FISH_HOME" "$FISH_INSTALL" >/dev/null; then FISH_INSTALL_PHYSICAL=$(cd "$FISH_INSTALL" && pwd -P) - fish_path=$(PATH="/usr/bin:/bin" fish --no-config -c \ + fish_path=$(PATH="/usr/bin:/bin" "$FISH_EXECUTABLE" --no-config -c \ "source '$FISH_RC'; string join : -- \$PATH") assert_path_once "fish" "$fish_path" "$FISH_INSTALL_PHYSICAL" else diff --git a/scripts/smoke/install-vhs.sh b/scripts/smoke/install-vhs.sh index 82ea4b7ec..526ff5c0e 100755 --- a/scripts/smoke/install-vhs.sh +++ b/scripts/smoke/install-vhs.sh @@ -2,9 +2,9 @@ # Ensure VHS (charmbracelet/vhs) is installed for the interactive tape harness. # # Installation strategy: -# - If `vhs` is already on PATH, do nothing. -# - On Linux x86_64: install vhs from the upstream release with SHA256 verification, -# and ensure ttyd / ffmpeg are present (apt-get if available). +# - If the pinned vhs and its runtime tools exist, do nothing. +# - On Linux x86_64: install vhs and ttyd from pinned upstream releases. +# Install the imageio-ffmpeg static binary. Verify all files with SHA256. # - On macOS: install vhs / ttyd / ffmpeg via Homebrew. # - Other platforms: print install hints and fail. # @@ -21,15 +21,22 @@ VHS_VERSION="${VHS_VERSION:-0.11.0}" # comment above. The pin matters for screenshot regression: a VHS bump can # change the bundled font/renderer and silently drift every baseline PNG. VHS_LINUX_X64_SHA256="${VHS_LINUX_X64_SHA256:-99cb634587eaae0473c1ea377db80c3a048c27f99fe0a7febb1a1e8cb7ee5009}" +TTYD_VERSION="${TTYD_VERSION:-1.7.7}" +# SHA256 of ttyd.x86_64 from the upstream SHA256SUMS file. +TTYD_LINUX_X64_SHA256="${TTYD_LINUX_X64_SHA256:-8a217c968aba172e0dbf3f34447218dc015bc4d5e59bf51db2f2cd12b7be4f55}" +# SHA256 of the imageio-ffmpeg 0.6.0 manylinux2014 x86_64 wheel from PyPI. +FFMPEG_WHEEL_SHA256="${FFMPEG_WHEEL_SHA256:-c7e46fcec401dd990405049d2e2f475e2b397779df2519b544b8aab515195282}" # 0.11.0 is the minimum that supports `Wait+Screen /pattern/`. # Earlier versions (e.g. 0.8.0) parse `Wait` as an unknown command. have() { command -v "$1" >/dev/null 2>&1; } if have vhs; then - installed="$(vhs --version 2>/dev/null | awk '{print $NF}' | sed 's/^v//')" - echo "vhs ${installed:-unknown} already installed at $(command -v vhs)" - exit 0 + installed="$(vhs --version 2>/dev/null | sed -n 's/.*version v\([^ ]*\).*/\1/p')" + if [[ "$installed" == "$VHS_VERSION" ]] && have ttyd && have ffmpeg && have python3; then + echo "vhs ${installed} and its runtime tools are already installed." + exit 0 + fi fi uname_s="$(uname -s)" @@ -76,30 +83,47 @@ EOF esac # Linux x86_64 path. +for dep in curl python3; do + if ! have "$dep"; then + echo "ERROR: '$dep' is required to install and run vhs." >&2 + exit 1 + fi +done -if have apt-get; then - echo "Installing vhs runtime deps (ttyd, ffmpeg) via apt-get..." - # Force non-interactive mode so apt never tries to open whiptail dialogs - # (e.g., the kernel-upgrade prompt) when running on a CI runner or under - # an SSH/agent session. - export DEBIAN_FRONTEND=noninteractive +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT + +if ! have ffmpeg; then + ffmpeg_wheel="$tmp/imageio-ffmpeg.whl" + ffmpeg_url="https://files.pythonhosted.org/packages/a0/2d/43c8522a2038e9d0e7dbdf3a61195ecc31ca576fb1527a528c877e87d973/imageio_ffmpeg-0.6.0-py3-none-manylinux2014_x86_64.whl" + echo "Downloading the pinned imageio-ffmpeg binary..." + curl -fsSL "$ffmpeg_url" -o "$ffmpeg_wheel" + echo "${FFMPEG_WHEEL_SHA256} ${ffmpeg_wheel}" | sha256sum -c - + python3 -m zipfile -e "$ffmpeg_wheel" "$tmp/imageio-ffmpeg" + + ffmpeg_binary="$tmp/imageio-ffmpeg/imageio_ffmpeg/binaries/ffmpeg-linux-x86_64-v7.0.2" + ffmpeg_dest="${FFMPEG_INSTALL_PATH:-/usr/local/bin/ffmpeg}" if [[ "${EUID:-$(id -u)}" -eq 0 ]]; then - apt-get update - apt-get install -y --no-install-recommends ttyd ffmpeg ca-certificates curl + install -m 0755 "$ffmpeg_binary" "$ffmpeg_dest" else - sudo -E apt-get update - sudo -E apt-get install -y --no-install-recommends ttyd ffmpeg ca-certificates curl + sudo install -m 0755 "$ffmpeg_binary" "$ffmpeg_dest" fi -else - for dep in ttyd ffmpeg curl; do - if ! have "$dep"; then - echo "WARNING: '$dep' is not on PATH and apt-get is unavailable. vhs will likely fail." >&2 - fi - done fi -tmp="$(mktemp -d)" -trap 'rm -rf "$tmp"' EXIT +if ! have ttyd; then + ttyd_binary="$tmp/ttyd" + ttyd_url="https://github.com/tsl0922/ttyd/releases/download/${TTYD_VERSION}/ttyd.x86_64" + echo "Downloading ttyd ${TTYD_VERSION} from ${ttyd_url}..." + curl -fsSL "$ttyd_url" -o "$ttyd_binary" + echo "${TTYD_LINUX_X64_SHA256} ${ttyd_binary}" | sha256sum -c - + + ttyd_dest="${TTYD_INSTALL_PATH:-/usr/local/bin/ttyd}" + if [[ "${EUID:-$(id -u)}" -eq 0 ]]; then + install -m 0755 "$ttyd_binary" "$ttyd_dest" + else + sudo install -m 0755 "$ttyd_binary" "$ttyd_dest" + fi +fi archive="$tmp/vhs.tar.gz" url="https://github.com/charmbracelet/vhs/releases/download/v${VHS_VERSION}/vhs_${VHS_VERSION}_Linux_x86_64.tar.gz" diff --git a/scripts/smoke/lib/common.sh b/scripts/smoke/lib/common.sh index 3f0a0dbb6..8694e98a5 100755 --- a/scripts/smoke/lib/common.sh +++ b/scripts/smoke/lib/common.sh @@ -9,13 +9,11 @@ # # Environment knobs: # START_TIMEOUT_SECONDS daemon start/health timeout (default: 180) -# STOP_TIMEOUT_SECONDS daemon stop timeout (default: 90) # STEP_TIMEOUT_SECONDS per-command timeout (default: 120) # DAEMON_BASE_URL health endpoint base (default loopback:56199) # DAEMON_PORT daemon listen port (default: port from DAEMON_BASE_URL or 56199) START_TIMEOUT_SECONDS="${START_TIMEOUT_SECONDS:-180}" -STOP_TIMEOUT_SECONDS="${STOP_TIMEOUT_SECONDS:-90}" STEP_TIMEOUT_SECONDS="${STEP_TIMEOUT_SECONDS:-120}" DAEMON_BASE_URL="${DAEMON_BASE_URL:-http://127.0.0.1:56199}" DAEMON_PORT="${DAEMON_PORT:-${DAEMON_BASE_URL##*:}}" @@ -217,13 +215,18 @@ wait_for_health() { return 1 } -# stop_daemon — best-effort daemon stop. Never fails the caller. +# stop_daemon — stop smoke-owned daemon processes. Never fail the caller. stop_daemon() { - : "${NETCLAW_SMOKE_CLI:?NETCLAW_SMOKE_CLI must be set}" - run_timed "$STOP_TIMEOUT_SECONDS" "$NETCLAW_SMOKE_CLI" daemon stop >/dev/null 2>&1 || true - # `daemon stop` only signals the PID in this NETCLAW_HOME's PID file; make - # sure the listening socket is actually released before the next daemon - # tries to bind it. + local holders + holders="$(lsof -ti "tcp:${DAEMON_PORT}" -sTCP:LISTEN 2>/dev/null || true)" + local pid + for pid in $holders; do + if pid_is_smoke_daemon "$pid"; then + log "stopping smoke daemon (pid=${pid})." + kill "$pid" 2>/dev/null || true + fi + done + ensure_daemon_port_free || true } diff --git a/scripts/smoke/run-native-tape.sh b/scripts/smoke/run-native-tape.sh index a26ac25b1..e7a457631 100755 --- a/scripts/smoke/run-native-tape.sh +++ b/scripts/smoke/run-native-tape.sh @@ -69,11 +69,13 @@ body="${TAPE_BODY_DIR:-${TAPES_DIR}}/${TAPE_NAME}.tape" assertion="${ASSERT_DIR}/${TAPE_NAME}.sh" requires_assertion=false -case "$TAPE_NAME" in - init-wizard|provider-add|provider-rename|config-*) - requires_assertion=true - ;; -esac +if [[ "${TAPE_BODY_DIR:-${TAPES_DIR}}" == "${TAPES_DIR}" ]]; then + case "$TAPE_NAME" in + init-wizard|provider-add|provider-rename|config-*) + requires_assertion=true + ;; + esac +fi if [[ ! -f "$preamble" ]]; then echo "ERROR: preamble not found at $preamble" >&2 @@ -95,10 +97,8 @@ combined="${tmp_dir}/${TAPE_NAME}.tape" cleanup() { # A tape (e.g. init-wizard) may leave a daemon running. Stop it and free - # the shared port 5199 so it cannot squat into the next tape/scenario — - # `daemon stop` is keyed to this tape's NETCLAW_HOME PID file. - NETCLAW_HOME="$NETCLAW_HOME" "$NETCLAW_SMOKE_CLI" daemon stop >/dev/null 2>&1 || true - ensure_daemon_port_free || true + # the shared port so it cannot affect the next tape or scenario. + stop_daemon if [[ "${KEEP_TEMP:-0}" == "1" ]]; then echo "KEEP_TEMP=1 — combined tape retained at: $combined" else diff --git a/scripts/smoke/run-smoke.sh b/scripts/smoke/run-smoke.sh index 2043f5416..e05bd0987 100755 --- a/scripts/smoke/run-smoke.sh +++ b/scripts/smoke/run-smoke.sh @@ -14,7 +14,7 @@ # # The `screenshots` profile provisions Ollama + the binary exactly like # `light`, then runs the capture tapes under tests/smoke/tapes/screenshots/ -# and compares each emitted PNG byte-for-byte against the approved baseline +# and compares each final lossless PNG frame against the approved baseline # in tests/smoke/screenshots/.approved.png. Missing baselines and # mismatches fail the run; the actual/diff PNGs are collected for review. # @@ -54,7 +54,7 @@ SMOKE_LOG_DIR="${SMOKE_LOG_DIR:-${ROOT_DIR}/smoke-logs}" # Cheapest harness checks first so a harness-level break fails fast # before paying for the wizard + probe tapes. -LIGHT_TAPES=(help init-wizard init-existing init-redo-identity provider-add provider-rename config-search config-exposure config-posture config-features config-audience config-channels config-mention-thread config-surfaces config-ops-surfaces config-workspaces-picker config-skill-picker config-back-nav tui-cleanup mcp-permissions approvals model-manager sessions-tui) +LIGHT_TAPES=(help init-wizard init-existing init-redo-identity provider-add provider-rename config-search config-exposure config-posture config-features config-audience config-channels config-mention-thread config-surfaces config-ops-surfaces config-workspaces-picker config-skill-picker config-back-nav tui-cleanup mcp-permissions mcp-permissions-save approvals model-manager sessions-tui) FULL_TAPES=("${LIGHT_TAPES[@]}") LIGHT_SCENARIOS=( @@ -70,11 +70,20 @@ LIGHT_SCENARIOS=( ) FULL_SCENARIOS=("${LIGHT_SCENARIOS[@]}") -# Screenshot capture tapes (under tests/smoke/tapes/screenshots/). Each tape -# may emit several `Screenshot "/tmp/shot-.png"` directives. SHOT_FRAMES -# is the full set of frame names the harness compares against baselines — it -# MUST stay in sync with the Screenshot paths in those tapes. -SHOT_TAPES=(help wizard-screens provider-manager mcp-permissions config-search) +# Screenshot capture tapes are under tests/smoke/tapes/screenshots/. The shared +# preamble records lossless PNG frames. SHOT_FRAMES is the full set of frame +# names that the harness compares against baselines. +SHOT_TAPES=( + help + wizard-provider-picker + wizard-security-posture + provider-manager-empty + mcp-permissions-server-list + mcp-permissions-tool-grid + config-search-selection + config-search-brave-entry + config-search-saved +) SHOT_FRAMES=( help wizard-provider-picker @@ -87,40 +96,11 @@ SHOT_FRAMES=( config-search-saved ) -# Which frames each capture tape emits. Used by the blank-frame retry -# (run_shot_tape_with_retry): after a tape runs, only its own captures are -# inspected for the transient blank described below. Keep in sync with the -# `Screenshot` directives in tests/smoke/tapes/screenshots/.tape. -# -# A function with a case is used instead of `declare -A` because macOS ships -# bash 3.2, which has no associative arrays — `declare -A` there parses the -# `[help]=...` entries as indexed-array assignments and aborts under set -u -# (`help: unbound variable`), breaking the non-screenshot smoke modes too. -shot_tape_frames() { - case "$1" in - help) echo "help" ;; - wizard-screens) echo "wizard-provider-picker wizard-security-posture" ;; - provider-manager) echo "provider-manager-empty" ;; - mcp-permissions) echo "mcp-permissions-server-list mcp-permissions-tool-grid" ;; - config-search) echo "config-search-selection config-search-brave-entry config-search-saved" ;; - *) echo "" ;; - esac -} - -# Max attempts per tape when a transient blank frame is detected. A TUI screen -# can render momentarily blank because Termina emits a full-screen clear -# () + repaint as one write on a startup resize event, and VHS can -# sample the PNG between the clear and the repaint half of that same write. -# The write is atomic from the app's side (real users never see it); only VHS's -# mid-write PTY sampling does. Re-running the tape re-captures a settled frame. -SHOT_BLANK_RETRIES="${SHOT_BLANK_RETRIES:-5}" - -# Pixel tolerance (ImageMagick AE) for a frame to count as matching its -# baseline. Shared by compare_shot_frame (the pass/fail gate) and the retry -# trigger (a capture above this differs enough to re-run). ~2 character cells — -# clears a single shell-cursor cell (~493 px) while still failing on real -# content changes (thousands of px). See compare_shot_frame for the rationale. +# Pixel tolerance for a frame to match its baseline. +# Two character cells cover a shell cursor artifact. +# A real content change differs by thousands of pixels. SHOT_AE_TOLERANCE="${SHOT_AE_TOLERANCE:-1000}" +SHOT_CAPTURE_ATTEMPTS=3 usage() { sed -n '2,30p' "$0" | sed 's/^# \{0,1\}//' @@ -360,6 +340,7 @@ run_one_scenario() { # Artifact dir for screenshot review PNGs (actual / diff / candidate). SHOT_ARTIFACT_DIR="${SMOKE_LOG_DIR}/screenshots" +SHOT_COMPARATOR="${SMOKE_SCRIPTS}/count-png-differences.py" # run_shot_tape — run one capture tape through run-native-tape.sh, # pointed at the screenshot preamble + tapes/screenshots/ body dir. The @@ -372,8 +353,10 @@ run_shot_tape() { echo "════════════════════════════════════════════════════════" local home="${RUN_ROOT}/home/shot-${tape}" local user_home="${RUN_ROOT}/home/user-shot-${tape}" + local frame_dir="/tmp/shot-frames-${tape}" rm -rf "$home" rm -rf "$user_home" + rm -rf "$frame_dir" mkdir -p "$user_home" if ! HOME="$user_home" \ NETCLAW_HOME="$home" \ @@ -391,109 +374,75 @@ run_shot_tape() { fi } -# frame_is_blank — true if the capture is a near-uniform frame, i.e. the -# transient Termina full-refresh blank (see SHOT_BLANK_RETRIES). Baseline- -# independent: it counts unique colors with ImageMagick `identify %k`. A blank -# frame is the solid theme background (~1 color); any populated TUI screen has -# hundreds. The threshold (16) sits far below the sparsest real frame and far -# above a blank, so it never misclassifies a real screen as blank. -frame_is_blank() { - local png="$1" - command -v identify >/dev/null 2>&1 || return 1 # can't tell → treat as not blank - [[ -f "$png" ]] || return 1 # missing capture is handled elsewhere - local colors - colors=$(identify -format '%k' "$png" 2>/dev/null || echo "") - [[ "$colors" =~ ^[0-9]+$ ]] || return 1 - (( colors < 16 )) -} +# copy_final_shot_frame — copy the final lossless recorder +# frame after VHS exits. VHS Screenshot can capture a stale browser frame even +# after Wait+Screen observes the settled terminal state. +copy_final_shot_frame() { + local tape="$1" + local frame="$2" + local frame_dir="/tmp/shot-frames-${tape}" + local final_frame + + final_frame=$(find "$frame_dir" -maxdepth 1 -type f -name '*.png' -print 2>/dev/null \ + | sort | tail -n 1) + if [[ -z "$final_frame" ]]; then + echo " WARN: ${frame} did not produce any lossless recorder frames." >&2 + return 1 + fi -# frame_needs_retry — true if the capture looks like a transient Termina -# full-refresh artifact that re-running the tape can clear. Two shapes: -# * fully blank (frame_is_blank) — VHS sampled the [2J-cleared frame. -# * partial/garbled — VHS sampled mid-repaint, so only the top rows landed -# (e.g. the MCP tool grid captured before its lower rows painted). Such a -# frame has plenty of colors (so frame_is_blank misses it) but differs from -# baseline by far more than the cursor tolerance. -# A genuine regression also trips the second branch, but it reproduces every -# attempt and so still fails at compare time — only the latency differs. -frame_needs_retry() { - local frame="$1" - local capture="/tmp/shot-${frame}.png" - local baseline="${SHOT_BASELINE_DIR}/${frame}.approved.png" - [[ -f "$capture" ]] || return 1 - frame_is_blank "$capture" && return 0 - command -v compare >/dev/null 2>&1 || return 1 - [[ -f "$baseline" ]] || return 1 - local ae ae_int - ae=$(compare -metric AE "$baseline" "$capture" /dev/null 2>&1 || true) - ae_int="${ae%%.*}"; ae_int="${ae_int// /}" - [[ "$ae_int" =~ ^[0-9]+$ ]] || return 1 - (( ae_int > SHOT_AE_TOLERANCE )) + cp "$final_frame" "/tmp/shot-${frame}.png" } -# run_shot_tape_with_retry — run a capture tape, then inspect the frames -# it emits. If any is a transient blank or missing (SHOT_BLANK_RETRIES), re-run -# the whole tape so the next attempt captures a settled frame. Bounded so a -# genuinely broken tape still fails at compare time instead of looping forever. -# -# Two transient shapes are retried: -# * blank / partial frame — VHS sampled between a Termina [2J clear and -# repaint (frame_needs_retry). -# * missing capture — the tape timed out (e.g. a Wait+Screen anchor fired -# too early) before reaching the Screenshot command. frame_needs_retry -# returns false for missing files, so we handle this case explicitly. -# -# In both cases the tape-level failure added by run_shot_tape to failed[] is -# rolled back before the retry so that a clean retry does not count as a run -# failure. If all SHOT_BLANK_RETRIES attempts produce a bad frame, the last -# tape-level failure is left in place for compare_shot_frame to report on. -run_shot_tape_with_retry() { +# capture_stable_shot — require two matching captures before +# baseline comparison. This quorum does not use the baseline. A stable visual +# change reaches compare_shot_frame and fails there. +capture_stable_shot() { local tape="$1" - local frames - frames="$(shot_tape_frames "$tape")" - local attempt=1 - while :; do - local failed_before=${#failed[@]} + local frame="$2" + local candidates="" + local attempt + # A tape run inside the loop can add "shot-tape:${tape}" to failed[] even + # when a later attempt still reaches a clean quorum. A clean retry is not + # a failure, so the entry is rolled back once quorum is reached. + local failed_before=${#failed[@]} + + for (( attempt = 1; attempt <= SHOT_CAPTURE_ATTEMPTS; attempt++ )); do + rm -f "/tmp/shot-${frame}.png" run_shot_tape "$tape" - [[ -z "$frames" ]] && return # no frame map → accept the single run - - local bad="" f - for f in $frames; do - # A missing capture means the tape timed out before reaching Screenshot. - # Treat it the same as a blank/partial frame: retry if budget remains. - if [[ ! -f "/tmp/shot-${f}.png" ]]; then - bad="${f} (missing — tape timed out)" - break - fi - if frame_needs_retry "$f"; then - bad="$f" - break + + local capture="/tmp/shot-${frame}.png" + if ! copy_final_shot_frame "$tape" "$frame"; then + echo " WARN: ${frame} attempt ${attempt} did not produce a capture." >&2 + continue + fi + + local candidate="/tmp/shot-${frame}.candidate-${attempt}.png" + cp "$capture" "$candidate" + + local previous + for previous in $candidates; do + local ae + if ae=$(python3 "$SHOT_COMPARATOR" "$previous" "$candidate") \ + && [[ "$ae" -le "$SHOT_AE_TOLERANCE" ]]; then + cp "$candidate" "$capture" + echo " STABLE: ${frame} reached a two-capture quorum (AE=${ae})." + if (( ${#failed[@]} > failed_before )); then + failed=("${failed[@]:0:$failed_before}") + fi + return fi done - if [[ -z "$bad" ]]; then - # All captures present and settled. Remove any tape-level failure that - # run_shot_tape added for this attempt — a clean retry is not a failure. - if (( ${#failed[@]} > failed_before )); then - failed=("${failed[@]:0:$failed_before}") - fi - return - fi + candidates="${candidates} ${candidate}" + done - if (( attempt >= SHOT_BLANK_RETRIES )); then - echo " WARN: ${tape} produced a transient frame (${bad}) on all ${attempt} attempts;" >&2 - echo " leaving it for compare_shot_frame to fail on." >&2 - return - fi - echo " RETRY: ${tape} attempt ${attempt} produced a transient frame (${bad}) —" >&2 - echo " re-running tape (blank or partial Termina full-refresh capture)." >&2 - # Roll back the tape-level failure before the next attempt. - if (( ${#failed[@]} > failed_before )); then - failed=("${failed[@]:0:$failed_before}") - fi - attempt=$((attempt + 1)) - for f in $frames; do rm -f "/tmp/shot-${f}.png"; done # clear stale captures + mkdir -p "$SHOT_ARTIFACT_DIR" + local candidate + for candidate in $candidates; do + cp "$candidate" "$SHOT_ARTIFACT_DIR/$(basename "$candidate")" done + echo " FAIL: ${frame} did not reach a two-capture quorum." >&2 + failed+=("shot-unstable:${frame}") } # compare_shot_frame — compare /tmp/shot-.png against the @@ -521,47 +470,37 @@ compare_shot_frame() { return fi - # Use ImageMagick pixel comparison rather than cmp -s (byte-for-byte). + # Compare decoded RGBA pixels rather than compressed PNG bytes. # Two sources of false failures are tolerated: # 1. VHS PNG zlib encoder jitter — same pixels, different byte streams # across process invocations (AE = 0, always passes). # 2. Terminal cursor block — Set CursorBlink false freezes the cursor but # not its on/off state; the shell-prompt cursor cell can appear or not # between runs. The block is one character cell (measured AE≈493 at this - # geometry). AE_CURSOR_TOLERANCE is set to ~2 cells so a single cursor + # geometry). SHOT_AE_TOLERANCE covers about two cells, so a single cursor # cell passes with margin, while real regressions still fail — a changed # word/line differs by thousands of px, a blank screen by ~68,000. - # Fall back to cmp -s only if ImageMagick is absent. The tolerance - # (SHOT_AE_TOLERANCE) is shared with the retry trigger (frame_needs_retry). - if command -v compare >/dev/null 2>&1; then - local ae - ae=$(compare -metric AE "$baseline" "$capture" /dev/null 2>&1 || true) - local ae_int="${ae%%.*}" - ae_int="${ae_int// /}" - if [[ "${ae_int:-0}" -le "$SHOT_AE_TOLERANCE" ]]; then - echo " PASS: ${frame} — pixel-close to baseline (AE=${ae_int:-0})." - return - fi - else - if cmp -s "$baseline" "$capture"; then - echo " PASS: ${frame} — pixel-identical to baseline." - return - fi + # SHOT_AE_TOLERANCE applies to all frames. + local ae + if ! ae=$(python3 "$SHOT_COMPARATOR" "$baseline" "$capture"); then + echo " FAIL: ${frame} — the PNG comparator failed." >&2 + failed+=("shot:${frame}") + return + fi + if [[ "$ae" -le "$SHOT_AE_TOLERANCE" ]]; then + echo " PASS: ${frame} — pixel-close to baseline (AE=${ae})." + return fi - # Mismatch — keep the actual, and a visual diff if ImageMagick is around. + # Keep the actual frame and an FFmpeg difference image. cp "$capture" "${SHOT_ARTIFACT_DIR}/${frame}.actual.png" echo " FAIL: ${frame} — differs from baseline." >&2 echo " actual saved to ${SHOT_ARTIFACT_DIR}/${frame}.actual.png" >&2 - if command -v compare >/dev/null 2>&1; then - # `compare` exits non-zero on any difference; that is expected here. - compare "$baseline" "$capture" "${SHOT_ARTIFACT_DIR}/${frame}.diff.png" \ - >/dev/null 2>&1 || true - if [[ -f "${SHOT_ARTIFACT_DIR}/${frame}.diff.png" ]]; then - echo " diff saved to ${SHOT_ARTIFACT_DIR}/${frame}.diff.png" >&2 - fi - else - echo " (ImageMagick 'compare' not found — no diff PNG generated)" >&2 + ffmpeg -y -v error -i "$baseline" -i "$capture" \ + -filter_complex 'blend=all_mode=difference' -frames:v 1 \ + "${SHOT_ARTIFACT_DIR}/${frame}.diff.png" || true + if [[ -f "${SHOT_ARTIFACT_DIR}/${frame}.diff.png" ]]; then + echo " diff saved to ${SHOT_ARTIFACT_DIR}/${frame}.diff.png" >&2 fi failed+=("shot:${frame}") } @@ -569,8 +508,8 @@ compare_shot_frame() { if [[ "$shots_mode" -eq 1 ]]; then # Fresh /tmp so a stale capture from an earlier run cannot be compared. rm -f /tmp/shot-*.png - for tape in "${SHOT_TAPES[@]}"; do - run_shot_tape_with_retry "$tape" + for index in "${!SHOT_TAPES[@]}"; do + capture_stable_shot "${SHOT_TAPES[$index]}" "${SHOT_FRAMES[$index]}" done echo diff --git a/src/Netclaw.Actors.Tests/Jobs/BackgroundJobDefinitionStoreTests.cs b/src/Netclaw.Actors.Tests/Jobs/BackgroundJobDefinitionStoreTests.cs index 8f52ef3b2..e1e2f1fad 100644 --- a/src/Netclaw.Actors.Tests/Jobs/BackgroundJobDefinitionStoreTests.cs +++ b/src/Netclaw.Actors.Tests/Jobs/BackgroundJobDefinitionStoreTests.cs @@ -6,6 +6,7 @@ using System.Text.Json; using System.Text.Json.Serialization; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; using Netclaw.Actors.Jobs; using Netclaw.Configuration; using Xunit; @@ -153,7 +154,17 @@ public void DeleteJobArtifacts_missing_job_returns_false_without_throwing() [Fact] public void DeleteJobArtifacts_keeps_definition_when_output_cleanup_fails_then_retries() { - var store = new BackgroundJobDefinitionStore(_paths); + var rejectCleanup = true; + var store = new BackgroundJobDefinitionStore( + _paths, + NullLogger.Instance, + (path, recursive) => + { + if (rejectCleanup) + throw new IOException("simulated output cleanup failure"); + + Directory.Delete(path, recursive); + }); var jobId = new BackgroundJobId("cleanup-retry-001"); store.Save(new BackgroundJobDefinition { @@ -168,18 +179,17 @@ public void DeleteJobArtifacts_keeps_definition_when_output_cleanup_fails_then_r OriginChannelType = Netclaw.Actors.Channels.ChannelType.Slack }); - var outputLogPath = store.GetOutputLogPathOnly(jobId); + var outputLogPath = store.GetOutputLogPath(jobId); var outputDirectory = Path.GetDirectoryName(outputLogPath)!; - File.WriteAllText(outputDirectory, "path collision"); + File.WriteAllText(outputLogPath, "build output"); var error = Assert.Throws(() => store.DeleteJobArtifacts(jobId)); - Assert.Contains("is not a directory", error.Message); + Assert.Contains("simulated output cleanup failure", error.Message); Assert.NotNull(store.Get(jobId)); - Assert.True(File.Exists(outputDirectory)); + Assert.True(File.Exists(outputLogPath)); - File.Delete(outputDirectory); - File.WriteAllText(store.GetOutputLogPath(jobId), "build output"); + rejectCleanup = false; Assert.True(store.DeleteJobArtifacts(jobId)); Assert.Null(store.Get(jobId)); diff --git a/src/Netclaw.Actors.Tests/Jobs/BackgroundJobManagerActorTests.cs b/src/Netclaw.Actors.Tests/Jobs/BackgroundJobManagerActorTests.cs index 96a16fcdc..70abc154f 100644 --- a/src/Netclaw.Actors.Tests/Jobs/BackgroundJobManagerActorTests.cs +++ b/src/Netclaw.Actors.Tests/Jobs/BackgroundJobManagerActorTests.cs @@ -6,6 +6,7 @@ using Akka.Actor; using Akka.Hosting; using Akka.Hosting.TestKit; +using Microsoft.Extensions.Logging.Abstractions; using Netclaw.Actors.Channels; using Netclaw.Actors.Hosting; using Netclaw.Actors.Jobs; @@ -23,6 +24,7 @@ public class BackgroundJobManagerActorTests : TestKit { private readonly DisposableTempDir _dir = new(); private BackgroundJobDefinitionStore _store = null!; + private string? _rejectedOutputDirectory; public BackgroundJobManagerActorTests(ITestOutputHelper output) : base(output: output) { } @@ -30,7 +32,10 @@ protected override void ConfigureAkka(AkkaConfigurationBuilder builder, IService { var paths = new NetclawPaths(_dir.Path); paths.EnsureDirectoriesExist(); - _store = new BackgroundJobDefinitionStore(paths); + _store = new BackgroundJobDefinitionStore( + paths, + NullLogger.Instance, + DeleteOutputDirectory); builder.StartActors((system, registry, _) => { @@ -52,6 +57,24 @@ protected override async Task AfterAllAsync() private IActorRef GetManager() => ActorRegistry.For(Sys).Get(); + private async Task GetReadyManagerAsync() + { + var manager = GetManager(); + await manager.Ask( + GetBackgroundJobManagerHealth.Instance, + TimeSpan.FromSeconds(30), + TestContext.Current.CancellationToken); + return manager; + } + + private void DeleteOutputDirectory(string path, bool recursive) + { + if (string.Equals(path, Volatile.Read(ref _rejectedOutputDirectory), StringComparison.Ordinal)) + throw new IOException("simulated output cleanup failure"); + + Directory.Delete(path, recursive); + } + private async Task RunTerminalSweepAsync(IActorRef manager) { // Both messages use the same sender, so the health response is a strict @@ -406,7 +429,7 @@ public void Emit(OperationalAlert alert) [Fact] public async Task TerminalSweep_DeletesJobPastRetentionWindow() { - var manager = GetManager(); + var manager = await GetReadyManagerAsync(); var pastWindowMs = TimeProvider.System.GetUtcNow() .Subtract(BackgroundJobManagerActor.TerminalJobRetentionWindow) .Subtract(TimeSpan.FromMinutes(1)) @@ -422,7 +445,7 @@ public async Task TerminalSweep_DeletesJobPastRetentionWindow() [Fact] public async Task TerminalSweep_KeepsJobWithinRetentionWindow() { - var manager = GetManager(); + var manager = await GetReadyManagerAsync(); var recentMs = TimeProvider.System.GetUtcNow().ToUnixTimeMilliseconds(); _store.Save(MakeTerminalDefinition("sweep-recent", BackgroundJobStatus.Completed, recentMs)); @@ -435,7 +458,7 @@ public async Task TerminalSweep_KeepsJobWithinRetentionWindow() [Fact] public async Task TerminalSweep_DoesNotTouchNonTerminalJobs() { - var manager = GetManager(); + var manager = await GetReadyManagerAsync(); var pastWindowMs = TimeProvider.System.GetUtcNow() .Subtract(BackgroundJobManagerActor.TerminalJobRetentionWindow) .Subtract(TimeSpan.FromMinutes(1)) @@ -455,7 +478,7 @@ public async Task TerminalSweep_DoesNotTouchNonTerminalJobs() [Fact] public async Task TerminalSweep_DeletesOutputLogWithDefinition() { - var manager = GetManager(); + var manager = await GetReadyManagerAsync(); var pastWindowMs = TimeProvider.System.GetUtcNow() .Subtract(BackgroundJobManagerActor.TerminalJobRetentionWindow) .Subtract(TimeSpan.FromMinutes(1)) @@ -477,7 +500,7 @@ public async Task TerminalSweep_DeletesOutputLogWithDefinition() [Fact] public async Task TerminalSweep_CleanupFailureDoesNotRestartManagerAndLaterSweepRetries() { - var manager = GetManager(); + var manager = await GetReadyManagerAsync(); var pastWindowMs = TimeProvider.System.GetUtcNow() .Subtract(BackgroundJobManagerActor.TerminalJobRetentionWindow) .Subtract(TimeSpan.FromMinutes(1)) @@ -487,10 +510,11 @@ public async Task TerminalSweep_CleanupFailureDoesNotRestartManagerAndLaterSweep _store.Save(blocked); _store.Save(removable); - var blockedOutputPath = _store.GetOutputLogPathOnly(blocked.Id); + var blockedOutputPath = _store.GetOutputLogPath(blocked.Id); var blockedOutputDirectory = Path.GetDirectoryName(blockedOutputPath)!; - File.WriteAllText(blockedOutputDirectory, "path collision"); + File.WriteAllText(blockedOutputPath, "failed output"); File.WriteAllText(_store.GetOutputLogPath(removable.Id), "completed output"); + Volatile.Write(ref _rejectedOutputDirectory, blockedOutputDirectory); var active = await manager.Ask( MakeStartCommand("sleep 60"), @@ -505,15 +529,14 @@ public async Task TerminalSweep_CleanupFailureDoesNotRestartManagerAndLaterSweep Assert.NotNull(_store.Get(blocked.Id)); Assert.Null(_store.Get(removable.Id)); - File.Delete(blockedOutputDirectory); + Volatile.Write(ref _rejectedOutputDirectory, null); await RunTerminalSweepAsync(manager); Assert.Null(_store.Get(blocked.Id)); } finally { - if (File.Exists(blockedOutputDirectory)) - File.Delete(blockedOutputDirectory); + Volatile.Write(ref _rejectedOutputDirectory, null); await manager.Ask( new CancelBackgroundJob( @@ -529,7 +552,7 @@ await manager.Ask( [Fact] public async Task TerminalSweep_KeepsTerminalJobWithMissingCompletionTime() { - var manager = GetManager(); + var manager = await GetReadyManagerAsync(); var pastWindowMs = TimeProvider.System.GetUtcNow() .Subtract(BackgroundJobManagerActor.TerminalJobRetentionWindow) .Subtract(TimeSpan.FromMinutes(1)) diff --git a/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs b/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs index a16cdab6d..edccd4de3 100644 --- a/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/LlmSessionIntegrationTests.cs @@ -349,12 +349,7 @@ public async Task Delivery_failed_for_latest_turn_retries_once_with_structured_n var sessionManager = ActorRegistry.Get(); var subscriber = CreateTestProbe("delivery-retry-sub"); - await sessionManager.Ask(new JoinSession(subscriber) - { - SessionId = sessionId, - Filter = OutputFilter.TextOnly - }, TimeSpan.FromSeconds(3), cancellationToken: TestContext.Current.CancellationToken); - await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await JoinSessionAsync(sessionManager, subscriber, sessionId); await sessionManager.Ask(new SendUserMessage { @@ -391,12 +386,7 @@ public async Task Stale_delivery_failed_is_ignored_after_new_user_turn_starts() var sessionManager = ActorRegistry.Get(); var subscriber = CreateTestProbe("stale-delivery-sub"); - await sessionManager.Ask(new JoinSession(subscriber) - { - SessionId = sessionId, - Filter = OutputFilter.TextOnly - }, TimeSpan.FromSeconds(3), cancellationToken: TestContext.Current.CancellationToken); - await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await JoinSessionAsync(sessionManager, subscriber, sessionId); await sessionManager.Ask(new SendUserMessage { @@ -435,12 +425,7 @@ public async Task Delivery_failed_while_processing_newer_turn_is_ignored() var sessionManager = ActorRegistry.Get(); var subscriber = CreateTestProbe("processing-delivery-sub"); - await sessionManager.Ask(new JoinSession(subscriber) - { - SessionId = sessionId, - Filter = OutputFilter.TextOnly - }, TimeSpan.FromSeconds(3), cancellationToken: TestContext.Current.CancellationToken); - await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await JoinSessionAsync(sessionManager, subscriber, sessionId); await sessionManager.Ask(new SendUserMessage { @@ -484,12 +469,7 @@ public async Task Delivery_retry_budget_stops_after_two_failed_corrections() var sessionManager = ActorRegistry.Get(); var subscriber = CreateTestProbe("delivery-budget-sub"); - await sessionManager.Ask(new JoinSession(subscriber) - { - SessionId = sessionId, - Filter = OutputFilter.TextOnly - }, TimeSpan.FromSeconds(3), cancellationToken: TestContext.Current.CancellationToken); - await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await JoinSessionAsync(sessionManager, subscriber, sessionId); await sessionManager.Ask(new SendUserMessage { @@ -534,12 +514,7 @@ public async Task Transport_failure_injects_nudge_without_triggering_retry() var sessionManager = ActorRegistry.Get(); var subscriber = CreateTestProbe("transport-failure-sub"); - await sessionManager.Ask(new JoinSession(subscriber) - { - SessionId = sessionId, - Filter = OutputFilter.TextOnly - }, TimeSpan.FromSeconds(3), cancellationToken: TestContext.Current.CancellationToken); - await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await JoinSessionAsync(sessionManager, subscriber, sessionId); await sessionManager.Ask(new SendUserMessage { @@ -586,12 +561,7 @@ public async Task Unknown_delivery_failure_injects_nudge_without_triggering_retr var sessionManager = ActorRegistry.Get(); var subscriber = CreateTestProbe("unknown-failure-sub"); - await sessionManager.Ask(new JoinSession(subscriber) - { - SessionId = sessionId, - Filter = OutputFilter.TextOnly - }, TimeSpan.FromSeconds(3), cancellationToken: TestContext.Current.CancellationToken); - await subscriber.ExpectMsgAsync(cancellationToken: TestContext.Current.CancellationToken); + await JoinSessionAsync(sessionManager, subscriber, sessionId); await sessionManager.Ask(new SendUserMessage { diff --git a/src/Netclaw.Actors.Tests/Sessions/LlmSessionTestBase.cs b/src/Netclaw.Actors.Tests/Sessions/LlmSessionTestBase.cs index 22b0f95b0..1d0a8445d 100644 --- a/src/Netclaw.Actors.Tests/Sessions/LlmSessionTestBase.cs +++ b/src/Netclaw.Actors.Tests/Sessions/LlmSessionTestBase.cs @@ -3,6 +3,7 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- +using Akka.Actor; using Akka.Hosting; using Akka.Hosting.TestKit; using Akka.Persistence.Hosting; @@ -11,10 +12,12 @@ using Netclaw.Actors.Channels; using Netclaw.Actors.Hosting; using Netclaw.Actors.Jobs; +using Netclaw.Actors.Protocol; using Netclaw.Actors.Reminders; using Netclaw.Actors.Tests.Hosting; using Netclaw.Configuration; using Netclaw.Security; +using static Netclaw.Actors.Sessions.SessionProtocol; namespace Netclaw.Actors.Tests.Sessions; @@ -48,6 +51,27 @@ protected LlmSessionTestBase(ITestOutputHelper output) : base(output: output) { protected void AdvanceScheduler(TimeSpan offset) => ((Akka.TestKit.TestScheduler)Sys.Scheduler).Advance(offset); + /// + /// Joins a cold session through its durable subscriber acknowledgement. + /// The timeout is a fault ceiling for recovery, not an orchestration delay. + /// + protected static async Task JoinSessionAsync( + IActorRef sessionManager, + Akka.TestKit.TestProbe subscriber, + SessionId sessionId, + OutputFilter filter = OutputFilter.TextOnly) + { + sessionManager.Tell(new JoinSession(subscriber) + { + SessionId = sessionId, + Filter = filter + }); + + return await subscriber.ExpectMsgAsync( + TimeSpan.FromSeconds(30), + cancellationToken: TestContext.Current.CancellationToken); + } + protected sealed override void ConfigureAkka(AkkaConfigurationBuilder builder, IServiceProvider provider) { if (UseTestScheduler) diff --git a/src/Netclaw.Actors.Tests/Sessions/SubAgentSpawnIntegrationTests.cs b/src/Netclaw.Actors.Tests/Sessions/SubAgentSpawnIntegrationTests.cs index bdc6e1212..b33e16871 100644 --- a/src/Netclaw.Actors.Tests/Sessions/SubAgentSpawnIntegrationTests.cs +++ b/src/Netclaw.Actors.Tests/Sessions/SubAgentSpawnIntegrationTests.cs @@ -26,13 +26,14 @@ namespace Netclaw.Actors.Tests.Sessions; public class SubAgentSpawnIntegrationTests : LlmSessionTestBase { + private const string ApprovalProbeToolName = "approval_probe"; private const string MainIdentityMarker = "You are a test assistant with subagent support."; private const string OperatingRulesMarker = "[embedded agents] Sub-agents inherit operating rules."; private const string AgentsLayerMarker = "[agents] This marker should never appear in routed subagent calls."; private readonly RecordingRoleChatClientProvider _clientProvider = new(); private RecordingContextTool? _recordingFileReadTool; - private RecordingContextTool? _recordingShellTool; + private RecordingContextTool? _recordingApprovalTool; private static FunctionCallContent CreateToolCall( string callId, @@ -149,7 +150,7 @@ You specialize in daemon health checks. { ToolOverrides = new Dictionary(StringComparer.Ordinal) { - ["shell_execute"] = ToolApprovalMode.Approval + [ApprovalProbeToolName] = ToolApprovalMode.Approval } }; var toolAccessPolicy = new ToolAccessPolicy( @@ -176,10 +177,10 @@ You specialize in daemon health checks. }); subAgentRegistry.Register(new SubAgentProfile { - Name = "sheller", - Description = "Run approved shell commands", - SystemPrompt = "You run shell commands when approved.", - ToolNames = ["shell_execute"], + Name = "approval-tester", + Description = "Test an approval request", + SystemPrompt = "You request approval for the test tool.", + ToolNames = [ApprovalProbeToolName], ModelRole = ModelRole.Compaction, Visibility = SubAgentVisibility.UserFacing, EmitStructuredFindings = false @@ -199,8 +200,8 @@ You specialize in daemon health checks. registry.Register(new SpawnAgentTool(subAgentRegistry, spawner, subAgentPaths)); _recordingFileReadTool = new RecordingContextTool("file_read", "stub file content", "file"); registry.Register(_recordingFileReadTool); - _recordingShellTool = new RecordingContextTool("shell_execute", "shell ok", "shell"); - registry.Register(_recordingShellTool); + _recordingApprovalTool = new RecordingContextTool(ApprovalProbeToolName, "approval ok"); + registry.Register(_recordingApprovalTool); services.AddSingleton(registry); services.AddSingleton(subAgentRegistry); @@ -312,18 +313,17 @@ public async Task Spawn_agent_subagent_approval_uses_parent_authority_and_resume "spawn_agent", new Dictionary { - ["agent"] = "sheller", - ["task"] = "Push the current branch" + ["agent"] = "approval-tester", + ["task"] = "Run the approval probe" }) ]; _clientProvider.Compaction.ToolCallsOnFirstCall = [ CreateToolCall( childCallId, - "shell_execute", + ApprovalProbeToolName, new Dictionary { - ["Command"] = "git push origin main", // Per-call timeout hint on the sub-agent path: the sub-agent // loop must extract this via the shared executor seam and apply // it to the tool context (it previously skipped extraction and @@ -347,7 +347,7 @@ await sessionManager.Ask(new JoinSession(subscriber) await sessionManager.Ask(new SendUserMessage { SessionId = sessionId, - Content = "Use a subagent to push the branch", + Content = "Use a subagent to run the approval probe", Source = source }, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); @@ -363,7 +363,7 @@ await sessionManager.Ask(new SendUserMessage Assert.Contains("subagent-approval", request.CallId.Value, StringComparison.Ordinal); Assert.DoesNotContain(childCallId, request.CallId.Value, StringComparison.Ordinal); AssertApprovalButtonValuesRoundTrip(request); - Assert.Equal("shell_execute", request.ToolName.Value); + Assert.Equal(ApprovalProbeToolName, request.ToolName.Value); Assert.Equal(source.SenderId, request.RequesterSenderId); Assert.Equal(source.Principal, request.RequesterPrincipal); Assert.Contains(request.Options, o => o.Key.Value == ApprovalOptionKeys.ApproveOnce); @@ -387,13 +387,13 @@ await sessionManager.Ask(new SendUserMessage await subscriber.ExpectMsgAsync(TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); await subscriber.ExpectMsgAsync(TimeSpan.FromSeconds(3), cancellationToken: TestContext.Current.CancellationToken); - Assert.NotNull(_recordingShellTool); - Assert.True(_recordingShellTool!.WasCalled); - Assert.Equal(TrustAudience.Personal, _recordingShellTool.LastContext?.Audience); + Assert.NotNull(_recordingApprovalTool); + Assert.True(_recordingApprovalTool!.WasCalled); + Assert.Equal(TrustAudience.Personal, _recordingApprovalTool.LastContext?.Audience); // The sub-agent extracted the meta timeout hint and applied it to the // tool context (regression guard for the previously-dropped hint). - Assert.Equal(TimeSpan.FromSeconds(1800), _recordingShellTool.LastContext?.ExecutionTimeout.Value); + Assert.Equal(TimeSpan.FromSeconds(1800), _recordingApprovalTool.LastContext?.ExecutionTimeout.Value); } [Fact] @@ -402,23 +402,20 @@ public async Task Spawn_agent_subagent_approval_expires_after_parent_session_rec _clientProvider.Main.ToolCallsOnFirstCall = [ CreateToolCall( - "call-spawn-shell-expire", + "call-spawn-approval-expire", "spawn_agent", new Dictionary { - ["agent"] = "sheller", - ["task"] = "Push the current branch" + ["agent"] = "approval-tester", + ["task"] = "Run the approval probe" }) ]; _clientProvider.Compaction.ToolCallsOnFirstCall = [ CreateToolCall( - "call-subagent-shell-expire", - "shell_execute", - new Dictionary - { - ["Command"] = "git push origin main" - }) + "call-subagent-approval-expire", + ApprovalProbeToolName, + new Dictionary()) ]; var sessionId = new SessionId("console/subagent-approval-expired"); @@ -436,7 +433,7 @@ await sessionManager.Ask(new JoinSession(subscriber) await sessionManager.Ask(new SendUserMessage { SessionId = sessionId, - Content = "Use a subagent to push the branch", + Content = "Use a subagent to run the approval probe", Source = source }, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); @@ -444,9 +441,9 @@ await sessionManager.Ask(new SendUserMessage await subscriber.ExpectMsgAsync(TimeSpan.FromSeconds(3), cancellationToken: TestContext.Current.CancellationToken); var request = await subscriber.ExpectMsgAsync(TimeSpan.FromSeconds(3), cancellationToken: TestContext.Current.CancellationToken); Assert.Contains("subagent-approval", request.CallId.Value, StringComparison.Ordinal); - Assert.DoesNotContain("call-subagent-shell-expire", request.CallId.Value, StringComparison.Ordinal); + Assert.DoesNotContain("call-subagent-approval-expire", request.CallId.Value, StringComparison.Ordinal); AssertApprovalButtonValuesRoundTrip(request); - Assert.False(_recordingShellTool!.WasCalled); + Assert.False(_recordingApprovalTool!.WasCalled); await ColdRespawnAsync(sessionId); @@ -470,7 +467,7 @@ await sessionManager.Ask(new JoinSession(subscriberB) Assert.Equal(ApprovalNackReasons.PromptExpired, nack.Reason); var notice = await subscriberB.ExpectMsgAsync(TimeSpan.FromSeconds(5), cancellationToken: TestContext.Current.CancellationToken); Assert.Contains("expired", notice.Text, StringComparison.OrdinalIgnoreCase); - Assert.False(_recordingShellTool.WasCalled); + Assert.False(_recordingApprovalTool.WasCalled); await sessionManager.Ask(new SendUserMessage { @@ -486,7 +483,7 @@ await sessionManager.Ask(new SendUserMessage Assert.Contains(resumedCall, message => message.Role == Microsoft.Extensions.AI.ChatRole.Tool && message.Contents.OfType().Any(result => - result.CallId == "call-spawn-shell-expire" + result.CallId == "call-spawn-approval-expire" && result.Result?.ToString()?.Contains("session restarted", StringComparison.OrdinalIgnoreCase) == true)); } diff --git a/src/Netclaw.Actors.Tests/TestShellEnvironment.cs b/src/Netclaw.Actors.Tests/TestShellEnvironment.cs index 371ab7588..21b4d18a1 100644 --- a/src/Netclaw.Actors.Tests/TestShellEnvironment.cs +++ b/src/Netclaw.Actors.Tests/TestShellEnvironment.cs @@ -4,35 +4,16 @@ // // ----------------------------------------------------------------------- using Netclaw.Security; -using Netclaw.Daemon; using ShellSyntaxTree; namespace Netclaw.Actors.Tests; internal static class TestShellEnvironment { - private static readonly object Gate = new(); - private static ShellExecutionEnvironment? _current; - - // Cache success only. The CLR caches a failed static initializer for the - // process lifetime, so a transient PowerShell host probe timeout would - // otherwise convert one slow spawn into hundreds of cached - // TypeInitializationException failures. By re-resolving on each touch after - // a failure, a slow-but-healthy host self-heals on the next consumer - // instead of poisoning the whole test process. - public static ShellExecutionEnvironment Current - { - get - { - var current = _current; - if (current is not null) - return current; - lock (Gate) - { - return _current ??= ResolveEnvironment(); - } - } - } + // The production resolver probes real processes and validates host versions. + // Its focused tests cover that behavior. Actor tests use the CI contract + // directly, so host load cannot poison a test class during static setup. + public static ShellExecutionEnvironment Current { get; } = CreateEnvironment(); public static string PrintWorkingDirectoryCommand => Current.Grammar == ShellGrammar.PowerShell @@ -88,11 +69,18 @@ public static ShellExecutionEnvironment CreateWindowsPowerShell51() PwshDialect.WindowsPowerShell51); } - private static ShellExecutionEnvironment ResolveEnvironment() - => ShellExecutionEnvironmentResolver - .CreateDefault(TimeProvider.System) - .ResolveAsync(ShellExecutionEnvironmentResolver.DetectCurrentPlatform()) - .GetAwaiter() - .GetResult() - .Environment; + private static ShellExecutionEnvironment CreateEnvironment() + { + if (OperatingSystem.IsWindows()) + { + return ShellExecutionEnvironment.CreatePowerShell( + @"C:\Program Files\PowerShell\7\pwsh.exe", + PwshDialect.PowerShell7); + } + + var platform = OperatingSystem.IsMacOS() + ? ShellPlatform.MacOS + : ShellPlatform.Linux; + return ShellExecutionEnvironment.CreateBash(platform); + } } diff --git a/src/Netclaw.Actors/Jobs/BackgroundJobDefinitionStore.cs b/src/Netclaw.Actors/Jobs/BackgroundJobDefinitionStore.cs index b775849c2..b16e47493 100644 --- a/src/Netclaw.Actors/Jobs/BackgroundJobDefinitionStore.cs +++ b/src/Netclaw.Actors/Jobs/BackgroundJobDefinitionStore.cs @@ -29,11 +29,21 @@ public sealed class BackgroundJobDefinitionStore private readonly Dictionary _rejectedLegacyDefinitions = new(StringComparer.Ordinal); private readonly ILogger _logger; + private readonly Action _deleteDirectory; public BackgroundJobDefinitionStore(NetclawPaths paths, ILogger? logger = null) + : this(paths, logger ?? NullLogger.Instance, Directory.Delete) + { + } + + internal BackgroundJobDefinitionStore( + NetclawPaths paths, + ILogger logger, + Action deleteDirectory) { _directory = paths.JobsDirectory; - _logger = logger ?? NullLogger.Instance; + _logger = logger; + _deleteDirectory = deleteDirectory; Directory.CreateDirectory(_directory); } @@ -228,7 +238,7 @@ public bool DeleteJobArtifacts(BackgroundJobId id) if (Directory.Exists(fullDir)) { - Directory.Delete(fullDir, recursive: true); + _deleteDirectory(fullDir, true); removed = true; } } diff --git a/src/Netclaw.Cli.Tests/Doctor/McpServersDoctorCheckTests.cs b/src/Netclaw.Cli.Tests/Doctor/McpServersDoctorCheckTests.cs index a6d62dc60..365c73d16 100644 --- a/src/Netclaw.Cli.Tests/Doctor/McpServersDoctorCheckTests.cs +++ b/src/Netclaw.Cli.Tests/Doctor/McpServersDoctorCheckTests.cs @@ -3,12 +3,11 @@ // Copyright (C) 2026 - 2026 Petabridge, LLC // // ----------------------------------------------------------------------- -using System.Net; -using System.Net.Sockets; using System.Text.Json; using Microsoft.Extensions.Configuration; using Netclaw.Cli.Daemon; using Netclaw.Cli.Doctor; +using Netclaw.Cli.Mcp; using Netclaw.Configuration; using Netclaw.Tests.Utilities; using Xunit; @@ -67,7 +66,13 @@ public async Task ValidStdioServer_UnreachableEndpoint_ReportsError() } }); - var check = new McpServersDoctorCheck(_paths, CreateDaemonApi(_ => throw new HttpRequestException("daemon offline"))); + var check = new McpServersDoctorCheck( + _paths, + CreateDaemonApi(_ => throw new HttpRequestException("daemon offline")), + (_, _, _) => Task.FromResult(new McpProbeResult( + McpProbeStatus.Unreachable, + 0, + "connection failed"))); var result = await check.RunAsync(TestContext.Current.CancellationToken); // Single enabled server that can't connect → Error @@ -234,8 +239,6 @@ public async Task DaemonReportedAwaitingAuth_ReturnsWarning() [Fact] public async Task OfflineOAuthProbe_DoesNotClaimAuthFailure() { - using var server = new UnauthorizedHttpServer(); - WriteConfig(new { configVersion = 1, @@ -244,14 +247,20 @@ public async Task OfflineOAuthProbe_DoesNotClaimAuthFailure() notion = new { Transport = "http", - Url = server.Url, + Url = "https://mcp.example.com", Enabled = true, OAuthClientId = "client-id" } } }); - var check = new McpServersDoctorCheck(_paths, CreateDaemonApi(_ => throw new HttpRequestException("daemon offline"))); + var check = new McpServersDoctorCheck( + _paths, + CreateDaemonApi(_ => throw new HttpRequestException("daemon offline")), + (_, _, _) => Task.FromResult(new McpProbeResult( + McpProbeStatus.AwaitingAuth, + 0, + null))); var result = await check.RunAsync(TestContext.Current.CancellationToken); Assert.Equal(DoctorSeverity.Warning, result.Severity); @@ -274,64 +283,4 @@ private static DaemonApi CreateDaemonApi(Func // // ----------------------------------------------------------------------- +using System.Net; using System.Net.Http; +using System.Text; using System.Text.Json; using Microsoft.Extensions.Configuration; using Netclaw.Cli.Daemon; @@ -11,6 +13,7 @@ using Netclaw.Configuration; using Netclaw.Tests.Utilities; using Netclaw.Tools; +using R3; using Xunit; namespace Netclaw.Cli.Tests.Mcp; @@ -73,6 +76,72 @@ public async Task LoadServers_NonObjectDaemonBody_SurfacesStatusInsteadOfThrowin Assert.Contains("Could not read MCP server statuses", vm.StatusMessage.Value); } + [Fact] + public async Task SelectServer_EntersLoadingStateAndIgnoresReentrantSelection() + { + var requestStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseResponse = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var configuration = new ConfigurationBuilder().Build(); + var httpClientFactory = new GatedToolsHttpClientFactory(requestStarted, releaseResponse.Task); + var daemonApi = new DaemonApi( + httpClientFactory, + configuration, + _paths); + using var vm = new McpToolPermissionsViewModel(_paths, daemonApi); + var toolGridReached = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var subscription = vm.CurrentState.Subscribe(state => + { + if (state == ToolPermissionsState.ToolGrid) + toolGridReached.TrySetResult(); + }); + + vm.CurrentState.Value = ToolPermissionsState.ServerList; + vm.SelectServer(new McpServerName("notion")); + + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + await requestStarted.Task.WaitAsync(cts.Token); + Assert.Equal(ToolPermissionsState.Loading, vm.CurrentState.Value); + Assert.Equal("Loading tools for notion...", vm.StatusMessage.Value); + + vm.SelectServer(new McpServerName("notion")); + Assert.Equal(1, httpClientFactory.RequestCount); + + releaseResponse.TrySetResult(); + await toolGridReached.Task.WaitAsync(cts.Token); + + Assert.Equal(ToolPermissionsState.ToolGrid, vm.CurrentState.Value); + Assert.Equal(["record-tasks"], vm.DiscoveredTools); + + vm.SelectServer(new McpServerName("notion")); + Assert.Equal(1, httpClientFactory.RequestCount); + Assert.Equal(ToolPermissionsState.ToolGrid, vm.CurrentState.Value); + } + + [Fact] + public async Task SelectServer_ToolRequestThrows_ReturnsToServerListWithError() + { + // A failed tool request must not strand the user on the Loading screen: the state + // must fall back to the server list so Esc / GoBack has somewhere to go. + var configuration = new ConfigurationBuilder().Build(); + var daemonApi = new DaemonApi(new ThrowingToolsHttpClientFactory(), configuration, _paths); + using var vm = new McpToolPermissionsViewModel(_paths, daemonApi); + var serverListReached = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + using var subscription = vm.CurrentState.Subscribe(state => + { + if (state == ToolPermissionsState.ServerList) + serverListReached.TrySetResult(); + }); + + vm.CurrentState.Value = ToolPermissionsState.ServerList; + vm.SelectServer(new McpServerName("notion")); + + using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + await serverListReached.Task.WaitAsync(cts.Token); + + Assert.Equal(ToolPermissionsState.ServerList, vm.CurrentState.Value); + Assert.Contains("Error loading tools", vm.StatusMessage.Value); + } + public static TheoryData ServerDefaultCycles => new() { { false, [ToolApprovalMode.Approval, ToolApprovalMode.Deny, ToolApprovalMode.Auto] }, @@ -570,4 +639,57 @@ protected override Task SendAsync( => Task.FromResult(new HttpResponseMessage { Content = new StringContent(body) }); } } + + // Fails every request so GetMcpToolNamesAsync throws, exercising the catch path in + // LoadToolsForServerAsync. + private sealed class ThrowingToolsHttpClientFactory : IHttpClientFactory + { + public HttpClient CreateClient(string name) => new(new ThrowingHandler()); + + private sealed class ThrowingHandler : HttpMessageHandler + { + protected override Task SendAsync( + HttpRequestMessage request, CancellationToken cancellationToken) + => throw new HttpRequestException("connection refused"); + } + } + + private sealed class GatedToolsHttpClientFactory : IHttpClientFactory + { + private readonly TaskCompletionSource _requestStarted; + private readonly Task _releaseResponse; + private int _requestCount; + + public GatedToolsHttpClientFactory(TaskCompletionSource requestStarted, Task releaseResponse) + { + _requestStarted = requestStarted; + _releaseResponse = releaseResponse; + } + + public int RequestCount => Volatile.Read(ref _requestCount); + + public HttpClient CreateClient(string name) => new(new GatedToolsHttpHandler( + _requestStarted, + _releaseResponse, + () => Interlocked.Increment(ref _requestCount))); + + private sealed class GatedToolsHttpHandler( + TaskCompletionSource requestStarted, + Task releaseResponse, + Action onRequest) : HttpMessageHandler + { + protected override async Task SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken) + { + onRequest(); + requestStarted.TrySetResult(); + await releaseResponse.WaitAsync(cancellationToken); + return new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("[\"record-tasks\"]", Encoding.UTF8, "application/json") + }; + } + } + } } diff --git a/src/Netclaw.Cli/Doctor/McpServersDoctorCheck.cs b/src/Netclaw.Cli/Doctor/McpServersDoctorCheck.cs index bf22e1bad..e72b36273 100644 --- a/src/Netclaw.Cli/Doctor/McpServersDoctorCheck.cs +++ b/src/Netclaw.Cli/Doctor/McpServersDoctorCheck.cs @@ -17,17 +17,36 @@ namespace Netclaw.Cli.Doctor; /// Second pass: prefer daemon-reported runtime truth; fall back to explicit /// offline connectivity checks when daemon status is unavailable. /// -public sealed class McpServersDoctorCheck(NetclawPaths paths, DaemonApi daemonApi) : IDoctorCheck +public sealed class McpServersDoctorCheck : IDoctorCheck { + private readonly NetclawPaths _paths; + private readonly DaemonApi _daemonApi; + private readonly Func> _probeServer; + + public McpServersDoctorCheck(NetclawPaths paths, DaemonApi daemonApi) + : this(paths, daemonApi, McpCommand.ProbeServerAsync) + { + } + + internal McpServersDoctorCheck( + NetclawPaths paths, + DaemonApi daemonApi, + Func> probeServer) + { + _paths = paths; + _daemonApi = daemonApi; + _probeServer = probeServer; + } + public async Task RunAsync(CancellationToken cancellationToken = default) { - if (!File.Exists(paths.NetclawConfigPath)) + if (!File.Exists(_paths.NetclawConfigPath)) return DoctorCheckResult.Pass("mcp-servers", "No config file (skipped)"); Dictionary servers; try { - var text = File.ReadAllText(paths.NetclawConfigPath); + var text = File.ReadAllText(_paths.NetclawConfigPath); using var doc = JsonDocument.Parse(text); if (!doc.RootElement.TryGetProperty("McpServers", out var mcpSection)) @@ -82,15 +101,15 @@ public async Task RunAsync(CancellationToken cancellationToke try { - daemonStatuses = await daemonApi.GetMcpServerStatusesAsync(cancellationToken); + daemonStatuses = await _daemonApi.GetMcpServerStatusesAsync(cancellationToken); } catch (HttpRequestException) { - daemonError = $"could not reach daemon at {daemonApi.Endpoint}"; + daemonError = $"could not reach daemon at {_daemonApi.Endpoint}"; } catch (OperationCanceledException) { - daemonError = $"daemon timed out at {daemonApi.Endpoint}"; + daemonError = $"daemon timed out at {_daemonApi.Endpoint}"; } catch (Exception ex) { @@ -108,7 +127,7 @@ private async Task EvaluateOfflineProbesAsync( string daemonError, CancellationToken cancellationToken) { - var fullServers = McpCommand.LoadMcpServers(paths); + var fullServers = McpCommand.LoadMcpServers(_paths); var statusMessages = new List { $"daemon status unavailable: {daemonError}" @@ -165,7 +184,7 @@ private async Task EvaluateOfflineProbesAsync( } var probeEntry = fullServers.TryGetValue(name, out var full) ? full : entry; - var probe = await McpCommand.ProbeServerAsync(new Netclaw.Tools.McpServerName(name), probeEntry, cancellationToken); + var probe = await _probeServer(new Netclaw.Tools.McpServerName(name), probeEntry, cancellationToken); switch (probe.Status) { diff --git a/src/Netclaw.Cli/Mcp/McpToolPermissionsViewModel.cs b/src/Netclaw.Cli/Mcp/McpToolPermissionsViewModel.cs index b8f4081ad..b886baf43 100644 --- a/src/Netclaw.Cli/Mcp/McpToolPermissionsViewModel.cs +++ b/src/Netclaw.Cli/Mcp/McpToolPermissionsViewModel.cs @@ -94,12 +94,11 @@ internal async Task LoadServersAsync() } catch (Exception ex) { - StatusMessage.Value = $"Could not reach daemon: {ex.Message}"; - NotifyStateChanged(); + await SetStatusAsync($"Could not reach daemon: {ex.Message}"); return; } - Servers.Clear(); + var servers = new List<(string Name, string Status, int ToolCount)>(); // A 200 response whose body is not the expected object shape (or a server entry missing // its "state") would otherwise throw out of this fire-and-forget task. Surface it as a @@ -112,41 +111,53 @@ internal async Task LoadServersAsync() ? stateEl.GetString() ?? "unknown" : "unknown"; var toolCount = prop.Value.TryGetProperty("toolCount", out var tc) ? tc.GetInt32() : 0; - Servers.Add((prop.Name, state, toolCount)); + servers.Add((prop.Name, state, toolCount)); } } catch (Exception ex) { - StatusMessage.Value = $"Could not read MCP server statuses: {ex.Message}"; - NotifyStateChanged(); + await SetStatusAsync($"Could not read MCP server statuses: {ex.Message}"); return; } + ToolAudienceProfiles profiles; try { - Profiles = LoadToolConfig().AudienceProfiles; + profiles = LoadToolConfig().AudienceProfiles; } catch (Exception ex) { - StatusMessage.Value = $"Could not load MCP permissions config: {ex.Message}"; - NotifyStateChanged(); + await SetStatusAsync($"Could not load MCP permissions config: {ex.Message}"); return; } - if (Servers.Count == 0) - StatusMessage.Value = "No MCP servers connected. Start the daemon and configure servers first."; - else + await InvokeAsync(() => { - StatusMessage.Value = ""; - CurrentState.Value = ToolPermissionsState.ServerList; - } + Servers.Clear(); + Servers.AddRange(servers); + Profiles = profiles; - NotifyStateChanged(); + if (Servers.Count == 0) + StatusMessage.Value = "No MCP servers connected. Start the daemon and configure servers first."; + else + { + StatusMessage.Value = ""; + CurrentState.Value = ToolPermissionsState.ServerList; + } + + NotifyStateChanged(); + }); } public void SelectServer(McpServerName serverName) { + if (CurrentState.Value != ToolPermissionsState.ServerList) + return; + SelectedServer = serverName.Value; + StatusMessage.Value = $"Loading tools for {serverName.Value}..."; + CurrentState.Value = ToolPermissionsState.Loading; + NotifyStateChanged(); _ = LoadToolsForServerAsync(serverName); } @@ -176,29 +187,41 @@ internal void SetSelectedAudienceForTests(TrustAudience audience) private async Task LoadToolsForServerAsync(McpServerName serverName) { - StatusMessage.Value = $"Loading tools for {serverName.Value}..."; - NotifyStateChanged(); - try { var tools = await _daemonApi.GetMcpToolNamesAsync(serverName.Value, CancellationToken.None); - DiscoveredTools.Clear(); - DiscoveredTools.AddRange(tools); + await InvokeAsync(() => + { + DiscoveredTools.Clear(); + DiscoveredTools.AddRange(tools); - // Initialize pending grants from current config if not already edited - if (!_pendingGrants.ContainsKey(serverName.Value)) - InitializePendingGrantsFromConfig(serverName); + // Initialize pending grants from current config if not already edited + if (!_pendingGrants.ContainsKey(serverName.Value)) + InitializePendingGrantsFromConfig(serverName); - StatusMessage.Value = ""; - CurrentState.Value = ToolPermissionsState.ToolGrid; + StatusMessage.Value = ""; + CurrentState.Value = ToolPermissionsState.ToolGrid; + NotifyStateChanged(); + }); } catch (Exception ex) { - StatusMessage.Value = $"Error loading tools: {ex.Message}"; + // The state must return to the server list on error. A failed request must not + // strand the user on the Loading screen with no visible exit. + await InvokeAsync(() => + { + StatusMessage.Value = $"Error loading tools: {ex.Message}"; + CurrentState.Value = ToolPermissionsState.ServerList; + NotifyStateChanged(); + }); } + } + private Task SetStatusAsync(string status) => InvokeAsync(() => + { + StatusMessage.Value = status; NotifyStateChanged(); - } + }); private void InitializePendingGrantsFromConfig(McpServerName serverName) { diff --git a/tests/smoke/screenshots/config-search-brave-entry.approved.png b/tests/smoke/screenshots/config-search-brave-entry.approved.png index 0b8f97fd1..9d25231ac 100644 Binary files a/tests/smoke/screenshots/config-search-brave-entry.approved.png and b/tests/smoke/screenshots/config-search-brave-entry.approved.png differ diff --git a/tests/smoke/screenshots/config-search-saved.approved.png b/tests/smoke/screenshots/config-search-saved.approved.png index d3d568c63..4f1fd1080 100644 Binary files a/tests/smoke/screenshots/config-search-saved.approved.png and b/tests/smoke/screenshots/config-search-saved.approved.png differ diff --git a/tests/smoke/screenshots/config-search-selection.approved.png b/tests/smoke/screenshots/config-search-selection.approved.png index e9b6454b8..1570fab66 100644 Binary files a/tests/smoke/screenshots/config-search-selection.approved.png and b/tests/smoke/screenshots/config-search-selection.approved.png differ diff --git a/tests/smoke/screenshots/help.approved.png b/tests/smoke/screenshots/help.approved.png index b9b636fe3..be2e1bada 100644 Binary files a/tests/smoke/screenshots/help.approved.png and b/tests/smoke/screenshots/help.approved.png differ diff --git a/tests/smoke/screenshots/mcp-permissions-server-list.approved.png b/tests/smoke/screenshots/mcp-permissions-server-list.approved.png index 94100e399..877a5f7d6 100644 Binary files a/tests/smoke/screenshots/mcp-permissions-server-list.approved.png and b/tests/smoke/screenshots/mcp-permissions-server-list.approved.png differ diff --git a/tests/smoke/screenshots/mcp-permissions-tool-grid.approved.png b/tests/smoke/screenshots/mcp-permissions-tool-grid.approved.png index 9812ae260..09fa55fc8 100644 Binary files a/tests/smoke/screenshots/mcp-permissions-tool-grid.approved.png and b/tests/smoke/screenshots/mcp-permissions-tool-grid.approved.png differ diff --git a/tests/smoke/screenshots/provider-manager-empty.approved.png b/tests/smoke/screenshots/provider-manager-empty.approved.png index 7a564e14b..9ce7d47d3 100644 Binary files a/tests/smoke/screenshots/provider-manager-empty.approved.png and b/tests/smoke/screenshots/provider-manager-empty.approved.png differ diff --git a/tests/smoke/screenshots/wizard-provider-picker.approved.png b/tests/smoke/screenshots/wizard-provider-picker.approved.png index b0f69a6fa..ed6bf0158 100644 Binary files a/tests/smoke/screenshots/wizard-provider-picker.approved.png and b/tests/smoke/screenshots/wizard-provider-picker.approved.png differ diff --git a/tests/smoke/screenshots/wizard-security-posture.approved.png b/tests/smoke/screenshots/wizard-security-posture.approved.png index 69e78a32b..f933a6ec0 100644 Binary files a/tests/smoke/screenshots/wizard-security-posture.approved.png and b/tests/smoke/screenshots/wizard-security-posture.approved.png differ diff --git a/tests/smoke/tapes/README.md b/tests/smoke/tapes/README.md index fb6b29c4a..22e2b1a59 100644 --- a/tests/smoke/tapes/README.md +++ b/tests/smoke/tapes/README.md @@ -53,12 +53,9 @@ that breaks them. `NETCLAW_HOME` and clears it before the tape runs. Tape bodies do not `rm -rf`, do not depend on prior tape runs. -3. **Flow tapes do not produce screenshots.** A flow tape (anything in - this directory other than `tapes/screenshots/`) MAY emit a debug GIF - (`Output /tmp/tape-.gif`) so failures have a visual artifact — - but no `Screenshot` directives in the body. `Screenshot` directives - belong exclusively to the screenshot-regression tapes under - `tapes/screenshots/` (see below). +3. **Flow tapes do not produce screenshot baselines.** A flow tape MAY emit + a debug GIF (`Output /tmp/tape-.gif`). Only tapes under + `tapes/screenshots/` use the lossless frame output from the shared preamble. 4. **Terminal sizing is in pixels, not columns.** VHS interprets `Set Width` / `Set Height` as pixel dimensions (minimum 120x120). @@ -112,9 +109,9 @@ substitution time rather than typed as quoted literals. The screenshot-regression tapes live under `tapes/screenshots/` and ARE the screenshot-capture mechanism for the native harness. Each one drives -the TUI to a settled state and emits a `Screenshot "/tmp/shot-.png"` -directive. The `screenshots` profile of `run-smoke.sh` compares each -captured PNG **byte-for-byte** against the committed baseline in +the TUI to a settled state. The shared preamble records lossless PNG frames. +The harness selects the final frame after VHS exits. The `screenshots` profile +of `run-smoke.sh` compares each captured PNG against the committed baseline in `tests/smoke/screenshots/.approved.png`. ```bash @@ -127,27 +124,21 @@ How it differs from the flow tapes: screenshot preamble adds determinism pins — `Set CursorBlink false` and an explicit `Set Theme "Catppuccin Mocha"` — so a captured PNG is byte-stable given the pinned VHS version + theme + geometry + font size. -- They DO emit `Screenshot` directives — that is their whole purpose. +- The shared preamble emits a lossless PNG frame sequence for each tape. - They have **no post-tape assertion**. The PNG comparison is the test. - The harness points `run-native-tape.sh` at them via the `TAPE_PREAMBLE` and `TAPE_BODY_DIR` env vars. -Every `Screenshot` is preceded by an anchored `Wait+Screen` (we are on the -right screen) and **bracketed by `Sleep 1s`** — one before so the TUI has -finished painting, one after so the next keystroke cannot leak into the -capture. The after-sleep is not optional and must sit **between the -`Screenshot` and the next screen-changing key** (`Ctrl+Q`, `Enter`, `exit`), -never after it: VHS writes the PNG asynchronously on a later render tick, so a -teardown key issued immediately after `Screenshot` can win the race and capture -the restored shell transcript instead of the TUI frame (this broke -`mcp-permissions` frame 2 — the tool grid rendered, the `Wait+Screen` anchors -matched, but `Ctrl+Q` tore the alt screen down before the deferred capture -fired). This is the one place a literal `Sleep` is correct: the no-`Sleep` -rule above is about flow-tape step *synchronization*, whereas a screenshot -needs the frame to be visually *settled*, and a settled static screen -captured at +1s is still deterministic. Never screenshot a screen with a -version string, timestamp, spinner, or token counter — pick a different -settled frame instead. +Keep the recorder at 60 frames per second. End the tape with `Sleep 250ms` after +the final state anchor. This recorder barrier produces 15 final-state frames. +Do not send a terminal key after the final anchor. Use visible state anchors +that prove the complete frame exists. Do not use a fixed delay as an application +render signal. Never capture a screen with a version string, timestamp, spinner, +or token counter. Select a different stable frame. + +The harness requires two matching captures before it checks the baseline. It +can make three capture attempts. This quorum does not use the baseline. Thus, a +stable visual change still reaches the baseline check and fails. ### Baseline workflow @@ -167,8 +158,7 @@ baselines land. ### Adding a screenshot frame -1. Add (or extend) a tape under `tapes/screenshots/` with a - `Screenshot "/tmp/shot-.png"` after an anchored `Wait+Screen`. +1. Add a tape under `tapes/screenshots/` with a final anchored `Wait+Screen`. 2. Add `` to `SHOT_FRAMES` (and the tape short name to `SHOT_TAPES`) in `scripts/smoke/run-smoke.sh`. 3. Run `./scripts/smoke/run-smoke.sh screenshots`; review the uploaded diff --git a/tests/smoke/tapes/mcp-permissions-save.tape b/tests/smoke/tapes/mcp-permissions-save.tape new file mode 100644 index 000000000..e64c1e49e --- /dev/null +++ b/tests/smoke/tapes/mcp-permissions-save.tape @@ -0,0 +1,40 @@ +# Verify the native MCP permission save path. + +Output "/tmp/tape-mcp-permissions-save.gif" + +Hide +Type "netclaw provider add smoke-llm ollama --endpoint http://127.0.0.1:11434" +Enter +Wait+Screen@10s /TAPE\$/ +Type "netclaw model set main smoke-llm qwen2:0.5b" +Enter +Wait+Screen@10s /TAPE\$/ +Type "netclaw mcp add --transport stdio --grant-all smoke-math -- __NETCLAW_SMOKE_MCP_SERVER__" +Enter +Wait+Screen@10s /TAPE\$/ +Type "netclaw daemon start" +Enter +Wait+Screen@20s /TAPE\$/ +Type "until netclaw mcp list 2>/dev/null | grep -q 'connected (4 tools)'; do sleep 2; done" +Enter +Wait+Screen@60s /TAPE\$/ +Show + +Type "netclaw mcp permissions; status=$?; echo MCP_PERMISSIONS_SAVE_EXIT=$status" +Enter +Wait+Screen@15s /MCP Permissions/ +Wait+Screen@10s /Connected, 4 tools/ +Enter +Wait+Screen@30s /record-tasks/ +Wait+Screen@10s /Server default:/ +Down +Space +Wait+Screen@5s /unsaved/ +Enter +Wait+Screen@5s /Enter\/Y.*Save/ +Enter +Wait+Screen@10s /Connected, 4 tools/ +Ctrl+Q +Wait+Screen@5s /MCP_PERMISSIONS_SAVE_EXIT=0/ +Type "exit" +Enter diff --git a/tests/smoke/tapes/mcp-permissions.tape b/tests/smoke/tapes/mcp-permissions.tape index 224d552b6..78332cda4 100644 --- a/tests/smoke/tapes/mcp-permissions.tape +++ b/tests/smoke/tapes/mcp-permissions.tape @@ -9,20 +9,15 @@ Output "/tmp/tape-mcp-permissions.gif" # ─── Launch ────────────────────────────────────────────────────────── -Type "netclaw mcp permissions" +Type "netclaw mcp permissions; status=$?; echo MCP_PERMISSIONS_EXIT=$status" Enter # BuildHeader() always renders regardless of daemon state. Wait+Screen@10s /MCP Permissions/ -Sleep 300ms +Wait+Screen@5s /Could not reach daemon:/ # ─── Exit TUI ──────────────────────────────────────────────────────── Ctrl+Q -Sleep 1s -Wait+Screen@10s /TAPE\$/ - -Type "echo MCP_PERMISSIONS_EXIT=$?" -Enter Wait+Screen@5s /MCP_PERMISSIONS_EXIT=0/ Type "exit" diff --git a/tests/smoke/tapes/screenshot-preamble.tape b/tests/smoke/tapes/screenshot-preamble.tape index 9a7fb2f65..e2b7796b9 100644 --- a/tests/smoke/tapes/screenshot-preamble.tape +++ b/tests/smoke/tapes/screenshot-preamble.tape @@ -39,8 +39,13 @@ Set TypingSpeed 30ms # Determinism pins for stable screenshot pixels. Set CursorBlink false +Set Framerate 60 Set Theme "Catppuccin Mocha" +# Record every frame as a lossless PNG. The harness selects the final frame +# after VHS exits, because the Screenshot command can read a stale browser frame. +Output "/tmp/shot-frames-__TAPE_NAME__/" + Hide # The host vhs-spawned bash may carry an unhelpful PS1; we set our own diff --git a/tests/smoke/tapes/screenshots/config-search-brave-entry.tape b/tests/smoke/tapes/screenshots/config-search-brave-entry.tape new file mode 100644 index 000000000..05d242a2d --- /dev/null +++ b/tests/smoke/tapes/screenshots/config-search-brave-entry.tape @@ -0,0 +1,22 @@ +# Capture the Brave search entry view. +# No terminal action follows the screenshot. + +Output "/tmp/tape-shot-config-search-brave-entry.gif" + +Type "mkdir -p $NETCLAW_HOME/config" +Enter +Type "backend=duckduckgo; jq -n --arg backend $backend '{configVersion:1,Search:{Backend:$backend}}' > $NETCLAW_HOME/config/netclaw.json" +Enter +Type "netclaw config" +Enter +Wait+Screen@10s /Settings Areas/ +Down 5 +Enter +Wait+Screen@10s /Choose the backend Netclaw uses for web search/ +Wait+Screen@5s /DuckDuckGo works without setup/ +Down +Enter +Wait+Screen@10s /Brave Search requires an API key/ +Wait+Screen@5s /Enter Brave Search API key/ +Wait+Screen@5s /Stored in secrets.json./ +Sleep 250ms diff --git a/tests/smoke/tapes/screenshots/config-search-saved.tape b/tests/smoke/tapes/screenshots/config-search-saved.tape new file mode 100644 index 000000000..b7c535cb7 --- /dev/null +++ b/tests/smoke/tapes/screenshots/config-search-saved.tape @@ -0,0 +1,33 @@ +# Capture the saved search configuration view. +# No terminal action follows the screenshot. + +Output "/tmp/tape-shot-config-search-saved.gif" + +Type "mkdir -p $NETCLAW_HOME/config" +Enter +Type "backend=duckduckgo; jq -n --arg backend $backend '{configVersion:1,Search:{Backend:$backend}}' > $NETCLAW_HOME/config/netclaw.json" +Enter +Type "netclaw config" +Enter +Wait+Screen@10s /Settings Areas/ +Down 5 +Enter +Wait+Screen@10s /Choose the backend Netclaw uses for web search/ +Wait+Screen@5s /DuckDuckGo works without setup/ +Down +Enter +Wait+Screen@10s /Brave Search requires an API key/ +Escape +Wait+Screen@5s /Choose the backend Netclaw uses for web search/ +Down +Enter +Wait+Screen@10s /Enter the base URL of your SearXNG instance/ +Type "https://search.test.local" +Enter +Wait+Screen@10s /Search Validation Warning/ +Down 2 +Enter +Wait+Screen@10s /validated and saved/ +Wait+Screen@10s /Saved Search settings./ +Wait+Screen@5s /\[Esc\] Review backends/ +Sleep 250ms diff --git a/tests/smoke/tapes/screenshots/config-search-selection.tape b/tests/smoke/tapes/screenshots/config-search-selection.tape new file mode 100644 index 000000000..1185149fa --- /dev/null +++ b/tests/smoke/tapes/screenshots/config-search-selection.tape @@ -0,0 +1,18 @@ +# Capture the initial search backend selection. +# No terminal action follows the screenshot. + +Output "/tmp/tape-shot-config-search-selection.gif" + +Type "mkdir -p $NETCLAW_HOME/config" +Enter +Type "backend=duckduckgo; jq -n --arg backend $backend '{configVersion:1,Search:{Backend:$backend}}' > $NETCLAW_HOME/config/netclaw.json" +Enter +Type "netclaw config" +Enter +Wait+Screen@10s /Settings Areas/ +Down 5 +Enter +Wait+Screen@10s /Choose the backend Netclaw uses for web search/ +Wait+Screen@5s /DuckDuckGo works without setup, but may hit bot detection./ +Wait+Screen@5s /\[Esc\] Back/ +Sleep 250ms diff --git a/tests/smoke/tapes/screenshots/config-search.tape b/tests/smoke/tapes/screenshots/config-search.tape deleted file mode 100644 index 0866a9c63..000000000 --- a/tests/smoke/tapes/screenshots/config-search.tape +++ /dev/null @@ -1,77 +0,0 @@ -# config-search.tape (screenshot) — capture the Search workflow screens. -# -# Frames captured: -# shot-config-search-selection -# shot-config-search-brave-entry -# shot-config-search-saved - -Output "/tmp/tape-shot-config-search.gif" - -# ─── Seed minimal config so `netclaw config` can launch ──────────────────── -Type "mkdir -p $NETCLAW_HOME/config" -Enter -Type "backend=duckduckgo; jq -n --arg backend $backend '{configVersion:1,Search:{Backend:$backend}}' > $NETCLAW_HOME/config/netclaw.json" -Enter - -# ─── Launch ──────────────────────────────────────────────────────────────── -Type "netclaw config" -Enter - -Wait+Screen@10s /Settings Areas/ -Down 5 -Enter - -# ─── Frame 1: Provider selection ─────────────────────────────────────────── -# SelectBackendForEditing is synchronous. Two anchors are required: the title -# confirms the screen loaded, and the DuckDuckGo description confirms the -# cursor is on row 1 (DuckDuckGo). Without the second anchor, a Down key -# leaked from the "Down 5" Settings navigation can move the cursor to Brave -# before the screenshot fires, capturing the wrong row. -Wait+Screen@10s /Choose the backend Netclaw uses for web search/ -Wait+Screen@5s /DuckDuckGo works without setup/ -Sleep 1s -Screenshot "/tmp/shot-config-search-selection.png" -Sleep 1s - -# ─── Frame 2: Brave entry state ──────────────────────────────────────────── -# Wait+Screen confirms the view state; the settle guard lets Termina finish repainting. -Down -Enter -Wait+Screen@10s /Brave Search requires an API key/ -Sleep 1s -Screenshot "/tmp/shot-config-search-brave-entry.png" -Sleep 1s - -# ─── Frame 3: Saved state ────────────────────────────────────────────────── -# SaveWithoutProbeOverride cascades multiple ReactiveProperty notifications -# and a ReloadPersistedDraft disk read before the frame settles. A single -# Wait+Screen on the success text is not enough — a second anchor on the -# key-binding bar text confirms CurrentScreen==Saved and ActiveDialog==None; the -# settle guard then lets the resulting full refresh finish before capture. -# "[Enter] Settings Areas" is emitted by BuildKeyBindings only in that state -# (SearchConfigEditorPage.cs line 212). -Escape -Down -Enter -Wait+Screen@10s /Enter the base URL of your SearXNG instance/ -Type "https://search.test.local" -Enter -Wait+Screen@10s /Search Validation Warning/ -Down 2 -Enter -Wait+Screen@10s /validated and saved/ -Wait+Screen@5s /\[Enter\] Settings Areas/ -Sleep 1s -Screenshot "/tmp/shot-config-search-saved.png" - -# Sleep 1s BEFORE Ctrl+Q: VHS's Screenshot is captured asynchronously on a -# later render tick, so a screen-destroying key here can race the deferred -# capture and land the restored shell transcript in the PNG instead of this -# frame. Guards this final capture against the timing jitter that broke -# mcp-permissions frame 2 (see that tape's Exit-TUI comment for mechanism). -Sleep 1s -Ctrl+Q -Wait+Screen@10s /TAPE\$/ - -Type "exit" -Enter diff --git a/tests/smoke/tapes/screenshots/help.tape b/tests/smoke/tapes/screenshots/help.tape index 5f0dab4ca..5a13aafb7 100644 --- a/tests/smoke/tapes/screenshots/help.tape +++ b/tests/smoke/tapes/screenshots/help.tape @@ -1,7 +1,8 @@ # help.tape (screenshot) — capture the `netclaw --help` usage screen. +# No terminal action follows the screenshot. # # Capture-only tape: it has NO post-tape assertion. The screenshots mode -# in run-smoke.sh compares the emitted PNG byte-for-byte against the +# in run-smoke.sh compares the final lossless PNG frame against the # committed baseline at tests/smoke/screenshots/help.approved.png. # # The usage screen is fully static (no version string, timestamp, spinner, @@ -11,15 +12,9 @@ Output "/tmp/tape-shot-help.gif" -Type "netclaw --help" -Enter -# Anchor on the usage header, then sleep to let the full terminal output -# finish rendering. `netclaw --help` writes multiple lines and VHS may not -# have flushed all of them into the terminal buffer by the time Wait+Screen -# matches the first visible line. -Wait+Screen@10s /Usage:/ -Sleep 1s -Screenshot "/tmp/shot-help.png" - -Type "exit" +Type "netclaw --help; read -r" Enter +# The read command keeps the shell from printing a prompt after the help text. +# The final help line proves that the process wrote the complete static frame. +Wait+Screen@10s /Docs & guides:/ +Sleep 250ms diff --git a/tests/smoke/tapes/screenshots/mcp-permissions-server-list.tape b/tests/smoke/tapes/screenshots/mcp-permissions-server-list.tape new file mode 100644 index 000000000..fa3942854 --- /dev/null +++ b/tests/smoke/tapes/screenshots/mcp-permissions-server-list.tape @@ -0,0 +1,28 @@ +# Capture the MCP server list with the test server connected. +# No terminal action follows the screenshot. + +Output "/tmp/tape-shot-mcp-permissions-server-list.gif" + +Hide +Type "netclaw provider add smoke-llm ollama --endpoint http://127.0.0.1:11434" +Enter +Wait+Screen@10s /TAPE\$/ +Type "netclaw model set main smoke-llm qwen2:0.5b" +Enter +Wait+Screen@10s /TAPE\$/ +Type "netclaw mcp add --transport stdio --grant-all smoke-math -- __NETCLAW_SMOKE_MCP_SERVER__" +Enter +Wait+Screen@10s /TAPE\$/ +Type "netclaw daemon start" +Enter +Wait+Screen@20s /TAPE\$/ +Type "until netclaw mcp list 2>/dev/null | grep -q 'connected (4 tools)'; do sleep 2; done" +Enter +Wait+Screen@60s /TAPE\$/ +Show + +Type "netclaw mcp permissions" +Enter +Wait+Screen@15s /MCP Permissions/ +Wait+Screen@10s /Connected, 4 tools/ +Sleep 250ms diff --git a/tests/smoke/tapes/screenshots/mcp-permissions-tool-grid.tape b/tests/smoke/tapes/screenshots/mcp-permissions-tool-grid.tape new file mode 100644 index 000000000..b436a9eec --- /dev/null +++ b/tests/smoke/tapes/screenshots/mcp-permissions-tool-grid.tape @@ -0,0 +1,32 @@ +# Capture the MCP tool grid with the test server connected. +# No terminal action follows the screenshot. + +Output "/tmp/tape-shot-mcp-permissions-tool-grid.gif" + +Hide +Type "netclaw provider add smoke-llm ollama --endpoint http://127.0.0.1:11434" +Enter +Wait+Screen@10s /TAPE\$/ +Type "netclaw model set main smoke-llm qwen2:0.5b" +Enter +Wait+Screen@10s /TAPE\$/ +Type "netclaw mcp add --transport stdio --grant-all smoke-math -- __NETCLAW_SMOKE_MCP_SERVER__" +Enter +Wait+Screen@10s /TAPE\$/ +Type "netclaw daemon start" +Enter +Wait+Screen@20s /TAPE\$/ +Type "until netclaw mcp list 2>/dev/null | grep -q 'connected (4 tools)'; do sleep 2; done" +Enter +Wait+Screen@60s /TAPE\$/ +Show + +Type "netclaw mcp permissions" +Enter +Wait+Screen@15s /MCP Permissions/ +Wait+Screen@10s /Connected, 4 tools/ +Enter +Wait+Screen@30s /record-tasks/ +Wait+Screen@10s /Server default:/ +Wait+Screen@5s /Audience:/ +Sleep 250ms diff --git a/tests/smoke/tapes/screenshots/mcp-permissions.tape b/tests/smoke/tapes/screenshots/mcp-permissions.tape deleted file mode 100644 index 9928bbb31..000000000 --- a/tests/smoke/tapes/screenshots/mcp-permissions.tape +++ /dev/null @@ -1,161 +0,0 @@ -# mcp-permissions.tape (screenshot) — capture `netclaw mcp permissions` with -# the smoke MCP server (smoke-math) connected and its tools indexed. -# -# Capture-only tape: NO post-tape assertion. The screenshots mode in -# run-smoke.sh compares each emitted PNG byte-for-byte against the committed -# baseline at tests/smoke/screenshots/.approved.png. -# -# Frames captured: -# mcp-permissions-server-list — ServerList state: smoke-math appears as -# "Connected, 4 tools" below the header. -# mcp-permissions-tool-grid — ToolGrid state: header rows (Audience, -# Server enabled, Server default) and all -# four tool rows (add, echo, record-tasks, process-info) -# simultaneously visible. This is the direct -# regression check for issue #1424 — the -# scroll container must not overwrite the header. -# -# Why these frames matter: the Loading/no-daemon frame only confirmed the -# header layout compiles. These frames confirm the layout holds under real -# data: a connected server, its tool rows rendered at the correct vertical -# offset, and all metadata rows above them untouched. -# -# Prepended preamble: tapes/screenshot-preamble.tape (determinism-pinned). -# __NETCLAW_SMOKE_MCP_SERVER__ is substituted by run-native-tape.sh. - -Output "/tmp/tape-shot-mcp-permissions.gif" - -# ─── Setup: seed provider + register the smoke MCP server ──────────── -# The smoke Ollama instance is already running at 127.0.0.1:11434 in -# screenshots mode. We seed a minimal provider so the daemon starts cleanly. -Hide -Type "netclaw provider add smoke-llm ollama --endpoint http://127.0.0.1:11434" -Enter -Wait+Screen@10s /TAPE\$/ - -Type "netclaw model set main smoke-llm qwen2:0.5b" -Enter -Wait+Screen@10s /TAPE\$/ - -# Register the deterministic smoke MCP server (add, echo, record-tasks, process-info). -# --grant-all means every tool is auto-approved for all audiences. -Type "netclaw mcp add --transport stdio --grant-all smoke-math -- __NETCLAW_SMOKE_MCP_SERVER__" -Enter -Wait+Screen@10s /TAPE\$/ - -# Start the daemon (detaches immediately; MCP handshake follows async). -Type "netclaw daemon start" -Enter -Wait+Screen@20s /TAPE\$/ - -# Poll until the daemon reports smoke-math as fully connected with 4 tools. -# Replaces the former fixed Sleep 5s: we wait for the actual CLI signal -# rather than guessing. `netclaw mcp list` queries the daemon's cached -# server state; "connected (4 tools)" means the stdio handshake and tool -# indexing are complete. The Wait+Screen@60s is a safety net for a hard hang. -Type "until netclaw mcp list 2>/dev/null | grep -q 'connected (4 tools)'; do sleep 2; done" -Enter -Wait+Screen@60s /TAPE\$/ -Show - -# ─── Launch the TUI ────────────────────────────────────────────────── -Type "netclaw mcp permissions" -Enter - -# ─── Frame 1: ServerList ───────────────────────────────────────────── -# smoke-math should appear as "Connected, 4 tools". Do NOT anchor on -# "smoke-math" or "4 tools" alone — both already sit in the shell -# scrollback before the TUI ever paints: -# - "smoke-math" appears in the setup output above ("Added MCP server -# 'smoke-math' (stdio)" / "...adjust approvals for 'smoke-math'."). -# - "4 tools" appears in this tape's own typed readiness-loop command, -# still visible on screen: `... grep -q 'connected (4 tools)' ...`. -# Immediately after Enter, before the TUI switches to the alternate -# screen buffer, Wait+Screen can match that leftover transcript text and -# return instantly, capturing the raw shell instead of the rendered TUI -# (README.md rule 5: anchor on the *next view*, not text that predates -# it). Anchor on TUI-only chrome instead: "MCP Permissions" is the page -# title from McpToolPermissionsPage.BuildHeader (proves the alt screen -# painted), and "Connected, 4 tools" is the exact rendered server-row -# text from McpToolPermissionsPage.BuildServerList — "{Name} ({Status}, -# {ToolCount} tools)" with capital-C Status and a comma, which the -# transcript's lowercase, comma-less "connected (4 tools)" never -# produces. Neither anchor occurs anywhere in the shell transcript. -Wait+Screen@15s /MCP Permissions/ -Wait+Screen@5s /Connected, 4 tools/ -# Sleep 3s: let the server-list frame fully settle before capturing. The -# first match of the anchors above can be a transient render; the -# daemon may push a state update (re-index, status refresh) immediately -# after the initial display. The sleep absorbs that window. -Sleep 3s -Screenshot "/tmp/shot-mcp-permissions-server-list.png" - -# ─── Navigate into the ToolGrid ────────────────────────────────────── -# Re-anchor immediately before Enter: confirm smoke-math is STILL Connected -# after the settle sleep. If the daemon pushed a state update during the -# sleep (periodic re-index / reconnect), these waits will hold until the -# server list is stable again. Without this double-anchor the Enter can -# land on an empty server list (daemon cleared it mid-transition), causing -# the TUI to navigate to a blank or non-existent tool grid. -Wait+Screen@20s /smoke-math/ -Wait+Screen@10s /4 tools/ -# Sleep 5s: extended settle guard (was 2s). Daemon MCP state updates can -# arrive at any point; 5s provides substantially more headroom under CI -# load where the re-index cycle can be slow. -Sleep 5s -Enter - -# ─── Frame 2: ToolGrid ─────────────────────────────────────────────── -# All header rows (Server, Audience, Server enabled, Server default) plus -# all tool rows (add, echo, record-tasks, process-info) must be visible simultaneously. -# If the #1424 regression reappears, tool rows will overwrite the header. -# Timeout is 30s (was 15s) to give the tool grid more headroom to load -# under CI load. /Server default:/ (with colon) matches the rendered -# "Server default: [Auto]" line, unique to the tool-grid view. -Wait+Screen@30s /record-tasks/ -Wait+Screen@10s /Server default:/ -# Sleep 500ms: settle guard against a Spectre Console full-refresh repaint -# racing with VHS Screenshot. The Wait+Screen anchors fire on first match, -# but the TUI can immediately redraw (daemon event, animation tick) before -# Screenshot captures. The sleep absorbs that repaint window. See #1471. -Sleep 500ms -Screenshot "/tmp/shot-mcp-permissions-tool-grid.png" - -# ─── Exercise staged save with the default Enter action ───────────── -# Change the audience, verify the dirty state appears, open confirmation, -# and accept the visible default with Enter. This is the native regression -# path for the confirmation flow that previously accepted only Y/N/Esc. -Down -Space -Wait+Screen@5s /unsaved/ -Enter -Wait+Screen@5s /Enter\/Y.*Save/ -Enter -Wait+Screen@10s /Connected, 4 tools/ - - -# ─── Exit TUI ──────────────────────────────────────────────────────── -# Sleep 2s BEFORE Ctrl+Q — settle guard AFTER the Screenshot. VHS's -# `Screenshot` does not write the PNG synchronously at this line: it hands the -# request to the render loop, which flushes the *current* framebuffer on a -# later tick. Ctrl+Q tears the TUI off the alternate screen buffer and restores -# the shell transcript on the main buffer. With no gap here, that teardown can -# win the race and VHS captures the restored transcript instead of the tool -# grid — the exact failure in run 28682629818 (frame 1 passed on the alt -# screen, frame 2 captured the post-Ctrl+Q main buffer). This is the ONLY -# Screenshot in the tape immediately followed by a screen-destroying key, which -# is why only this frame regressed. The sleep holds the tool grid on screen -# until the deferred capture lands. Proven with a minimal `less` alt-screen -# repro: Screenshot→immediate-q captures the main buffer; Screenshot→Sleep→q -# captures the alt screen. Do NOT remove without re-checking that race. -Sleep 2s -Ctrl+Q -Wait+Screen@10s /TAPE\$/ - -# Stop the daemon so the tape-level cleanup does not race the next tape. -Type "netclaw daemon stop" -Enter -Wait+Screen@10s /TAPE\$/ - -Type "exit" -Enter diff --git a/tests/smoke/tapes/screenshots/provider-manager-empty.tape b/tests/smoke/tapes/screenshots/provider-manager-empty.tape new file mode 100644 index 000000000..be74eb30d --- /dev/null +++ b/tests/smoke/tapes/screenshots/provider-manager-empty.tape @@ -0,0 +1,13 @@ +# Capture the empty provider manager. +# No terminal action follows the screenshot. + +Output "/tmp/tape-shot-provider-manager-empty.gif" + +Type "netclaw provider" +Enter + +# The first list has no state transition before the capture. +Wait+Screen@10s /Ollama/ +Wait+Screen@5s /Venice/ +Wait+Screen@5s /\[Delete\] Remove/ +Sleep 250ms diff --git a/tests/smoke/tapes/screenshots/provider-manager.tape b/tests/smoke/tapes/screenshots/provider-manager.tape deleted file mode 100644 index fb8535827..000000000 --- a/tests/smoke/tapes/screenshots/provider-manager.tape +++ /dev/null @@ -1,55 +0,0 @@ -# provider-manager.tape (screenshot) — drive the `netclaw provider` TUI and -# capture the empty provider list as a PNG. -# -# Capture-only tape: NO post-tape assertion. The screenshots mode in -# run-smoke.sh compares the emitted PNG byte-for-byte against the committed -# baseline at tests/smoke/screenshots/.approved.png. -# -# Frames captured: -# shot-provider-manager-empty — the provider list with no providers -# configured (every type as "(not -# configured)" + "+ Add new provider...") -# -# The "Name your provider" add step is deliberately NOT screenshotted: it -# is a text-input step, and Termina blinks the input caret on its own -# timer — the app-drawn caret cell is not byte-stable across runs. -# -# Navigation + anchors mirror tapes/provider-add.tape; see that file and -# src/Netclaw.Cli/Tui/ProviderManagerPage.cs for the synchronization -# rationale. Wait+Screen anchors on the list content, then Sleep 1s lets -# the full TUI frame flush before the screenshot is captured. -# -# Prepended preamble: tapes/screenshot-preamble.tape (determinism-pinned). - -Output "/tmp/tape-shot-provider-manager.gif" - -# ─── Launch ────────────────────────────────────────────────────────── -Type "netclaw provider" -Enter - -# ─── Frame 1: Empty provider list ──────────────────────────────────── -# With no providers configured the list shows every known type as a -# "(not configured)" row plus the "+ Add new provider..." sentinel. -# Anchor on "Ollama" (a guaranteed type row) so we know the list rendered. -# Sleep lets VHS flush the full frame; the SECOND Wait+Screen is an anti-blank -# re-assert immediately before capture — if a Termina startup full-refresh -# blanked the screen during the Sleep, this blocks until the list repaints. -# (Defence in depth: run-smoke.sh also re-runs the tape if a blank slips through.) -Wait+Screen@10s /Ollama/ -Sleep 1s -Wait+Screen@5s /Ollama/ -Screenshot "/tmp/shot-provider-manager-empty.png" - -# Frame captured; quit the TUI back to the shell. -# Sleep 1s BEFORE Ctrl+Q: VHS's Screenshot is captured asynchronously on a -# later render tick, so a screen-destroying key here can race the deferred -# capture and land the restored shell transcript in the PNG instead of this -# frame. This frame has not regressed on that race, but the guard makes it -# immune to the timing jitter that broke mcp-permissions frame 2 (see that -# tape's Exit-TUI comment for the full mechanism + repro). -Sleep 1s -Ctrl+Q -Wait+Screen@10s /TAPE\$/ - -Type "exit" -Enter diff --git a/tests/smoke/tapes/screenshots/wizard-provider-picker.tape b/tests/smoke/tapes/screenshots/wizard-provider-picker.tape new file mode 100644 index 000000000..979e66b02 --- /dev/null +++ b/tests/smoke/tapes/screenshots/wizard-provider-picker.tape @@ -0,0 +1,10 @@ +# Capture the provider picker from `netclaw init`. +# No terminal action follows the screenshot. + +Output "/tmp/tape-shot-wizard-provider-picker.gif" + +Type "netclaw init" +Enter +Wait+Screen@10s /Choose your LLM provider:/ +Wait+Screen@5s /8\. Venice\.ai/ +Sleep 250ms diff --git a/tests/smoke/tapes/screenshots/wizard-screens.tape b/tests/smoke/tapes/screenshots/wizard-screens.tape deleted file mode 100644 index c35de7e83..000000000 --- a/tests/smoke/tapes/screenshots/wizard-screens.tape +++ /dev/null @@ -1,101 +0,0 @@ -# wizard-screens.tape (screenshot) — walk `netclaw init` and capture two -# settled wizard states as PNGs. -# -# Capture-only tape: NO post-tape assertion. The screenshots mode in -# run-smoke.sh compares each emitted PNG byte-for-byte against the committed -# baseline at tests/smoke/screenshots/.approved.png. -# -# Frames captured: -# shot-wizard-provider-picker — "Choose your LLM provider:" list -# shot-wizard-security-posture — "Who will interact with this Netclaw -# instance" posture list -# -# The identity ("Agent name:") step is deliberately NOT screenshotted: it -# is a text-input step, and Termina blinks the input caret on its own -# timer. VHS's `Set CursorBlink false` only governs VHS's own terminal -# cursor, so the app-drawn caret cell is not byte-stable across runs. -# -# Navigation + anchors mirror tapes/init-wizard.tape; see that file and -# src/Netclaw.Cli/Tui/Wizard/Steps/*StepView.cs for the synchronization -# rationale. Wizard step transitions are synchronous — Wait+Screen is the -# only gate needed. The Sleep 1s after Ctrl+Q below is intentional: it is -# the alt-screen restore guard (CSI ?1049l), not a screenshot timing guard. -# -# Prepended preamble: tapes/screenshot-preamble.tape (determinism-pinned). - -Output "/tmp/tape-shot-wizard-screens.gif" - -# ─── Launch ────────────────────────────────────────────────────────── -Type "netclaw init" -Enter - -# ─── Frame 1: Provider picker ──────────────────────────────────────── -Wait+Screen@10s /Choose your LLM provider:/ -# The heading can appear before the full provider list is painted on slower -# CI runners. Anchor on the last row so the screenshot captures the settled list. -Wait+Screen@5s /8\. Venice\.ai/ -Screenshot "/tmp/shot-wizard-provider-picker.png" - -# Provider list ordering is alphabetical by TypeKey: -# anthropic, deepseek, github-copilot, ollama, openai, openai-compatible, openrouter, veniceai -# Anthropic is the default; three Downs land on Ollama. Two Downs land on -# github-copilot, whose OAuth flow would stop this capture-only tape. -Down 3 -Enter - -# Endpoint input — native Ollama is at http://localhost:11434, which is -# also the prompt's default value. -Wait+Screen@10s /endpoint:/ -# VHS has no End/Home — push the cursor right past the existing text -# (Right N is a no-op at end), then Backspace to clear all 22 chars. -Right 32 -Backspace 32 -Type "http://localhost:11434" -Enter - -# Connection probe + model discovery. The "Connected! Found N models" -# success frame auto-advances; wait directly on the model list header. -Wait+Screen@45s /Select a model/ -# Embedding-only models are filtered from the chat picker, so qwen2 is the -# default highlighted model. Accept it directly. -Enter - -# ─── Identity (navigated through, not screenshotted) ───────────────── -# Identity immediately follows Provider in the current bootstrap flow, so its four -# substeps must be walked to reach the posture screen. These frames are deliberately -# NOT screenshotted — the text-input caret blink makes them non-byte-stable (see the -# header). Navigation + anchors mirror tapes/init-wizard.tape. -Wait+Screen@10s /Agent name:/ -Enter - -Wait+Screen@10s /Communication style:/ -Enter - -Wait+Screen@10s /Your name:/ -Type "SmokeTester" -Enter - -Wait+Screen@10s /Your timezone:/ -Enter - -# ─── Frame 2: Security posture ─────────────────────────────────────── -Wait+Screen@10s /Who will interact with this Netclaw instance/ -Screenshot "/tmp/shot-wizard-security-posture.png" - -# Both frames captured; abandon the wizard. Ctrl+Q is the TUI quit -# shortcut (Ctrl+C requires a double-press under raw input mode). -# Sleep 1s BEFORE Ctrl+Q: VHS's Screenshot is captured asynchronously on a -# later render tick, so quitting immediately can race the deferred capture -# and land the restored shell transcript in the PNG instead of this frame. -# Guards this final capture against the timing jitter that broke -# mcp-permissions frame 2 (see that tape's Exit-TUI comment for mechanism). -Sleep 1s -Ctrl+Q - -# VHS's screen scraper needs a beat to pick up the restored main buffer -# after the TUI tears down the alternate screen (CSI ?1049l). -Sleep 1s -Wait+Screen@10s /TAPE\$/ - -Type "exit" -Enter diff --git a/tests/smoke/tapes/screenshots/wizard-security-posture.tape b/tests/smoke/tapes/screenshots/wizard-security-posture.tape new file mode 100644 index 000000000..e13667d10 --- /dev/null +++ b/tests/smoke/tapes/screenshots/wizard-security-posture.tape @@ -0,0 +1,34 @@ +# Capture the security posture from `netclaw init`. +# No terminal action follows the screenshot. + +Output "/tmp/tape-shot-wizard-security-posture.gif" + +Type "netclaw init" +Enter +Wait+Screen@10s /Choose your LLM provider:/ +Wait+Screen@5s /8\. Venice\.ai/ + +# Ollama is the fourth provider. +Down 3 +Enter +Wait+Screen@10s /endpoint:/ +Right 32 +Backspace 32 +Type "http://localhost:11434" +Enter + +Wait+Screen@45s /Select a model/ +Enter +Wait+Screen@10s /Agent name:/ +Enter +Wait+Screen@10s /Communication style:/ +Enter +Wait+Screen@10s /Your name:/ +Type "SmokeTester" +Enter +Wait+Screen@10s /Your timezone:/ +Enter + +Wait+Screen@10s /Who will interact with this Netclaw instance/ +Wait+Screen@5s /Personal/ +Sleep 250ms