Skip to content

Add ui_eval record: start a local demo app, record it, stop it - #4

Merged
sangyeT merged 5 commits into
mainfrom
claude/ui-eval-record
Sep 30, 2026
Merged

sangyeT merged 5 commits into
mainfrom
claude/ui-eval-record

Conversation

@sangyeT

@sangyeT sangyeT commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Why

The demo lane still needed someone to start the app by hand before ui_eval.py run, and to remember to stop it afterwards. record does the whole pass in one command: start the app, record the run, stop the app.

What changed

ui_eval.py record SCRIPT.json --launch demos/<name>/launch.json

  • Launch file: command is an argument list run without a shell, with $PYTHON replaced by the current interpreter. cwd must be inside the repo, so symlink, absolute and .. escapes are rejected. Also ready_path, ready_timeout_s, stop_grace_s and env. base_url must be on localhost.
  • Before starting: it refuses if anything is already listening on the port (a TCP connect check), so it never records a stale server.
  • Readiness: the app is ready when ready_path answers with a status below 400. Redirects aren't followed and proxies aren't used. It re-checks that the process is still alive before the browser starts.
  • Recording: it runs the existing run() with video. Each run's videos go in their own video/<run_marker>/ folder and are listed with their SHA256.
  • Stopping: it sends SIGTERM to the whole process group, waits stop_grace_s (default 10), then SIGKILLs anything still running.
    • This covers children that outlive the leader and children that ignore SIGTERM.
    • The final wait is bounded.
    • SIGTERM and SIGHUP sent to record itself are deferred while the server is starting or stopping and re-raised once the group is gone, so the server is never left running.
  • Report: adds server: the command as given, cwd relative to the repo, pid, readiness time, exit code, env names without values, and server.log. It also adds reasons:
    • server_exited_before_ready
    • server_not_ready
    • server_exited_during_run
    • browser_failed: …
  • When the app never becomes ready: the report has empty steps and videos, and review reports it as failed.

Other

  • Playwright is imported only when a browser run starts. Launch validation and its tests run without Playwright, and the test helper's skip path, which never worked before, now works.
  • The default per-check control.py verify timeout is raised from 60s to 180s. Browser-backed UI tests had pushed the stack suite to about 52s locally.
  • Docs: sstack-verify/references/ui-eval.md has a "Start and record a local app" section. sstack-demo step 2 and rehearsals use record for local apps. tools/sstack/README.md is updated.

Validation

  • control.py verify changed --base origin/main --jobs 2 passes on bb24ad4: docs, 134 stack tests and lint, with the browser tests running in real Chromium.
  • New tests launch real server processes. They cover:
    • a passing recording
    • a server that exits early or never becomes ready
    • a 503 at the ready path
    • a server that exits during the run
    • a stale server already on the port
    • a child that ignores SIGTERM, with the leader alive and after it exits
    • SIGTERM sent to record
    • SIGTERM arriving while stop() is inside its grace wait
    • env values staying out of the report
    • a reused output folder listing only its own videos
  • Mutation checks: an independent reviewer's 8 surviving mutations are all caught now, as are the stale-check and deferred-signal mutations. Examples: signalling only the leader, dropping SIGKILL escalation, treating 5xx as ready, and removing the signal handlers.
  • After every run, no test or review process was left running.
  • The same independent reviewer did two rounds: the first review, then a re-review of the fixes. The one remaining issue it found, SIGTERM during the stop grace, is fixed and has a test.

Known limits

  • record is for apps on localhost only. Hosted Kizen still uses ui_eval.py run with an approved origin.
  • A local app can still call remote services, so the approved-write rules apply to whatever it talks to. The server log may contain the app's output; inspect it before sharing.
  • A child that ignores SIGTERM costs the full stop_grace_s before it's killed.
  • control.py's own timeout kill only reaches the test runner's process group. If a suite times out, test servers in their own sessions can outlive it. With the new 180s default this isn't expected, but the gap is in control.py, not in record.

🤖 Generated with Claude Code

https://claude.ai/code/session_01HSV4zUy6rF1vgHFSyUmuva


Generated by Claude Code

record reads a launch file, refuses to run if something already answers
at base_url, starts the command without a shell, waits for the ready
path, runs the UI script with video and stops only the process group it
started. Reports add the server's command, pid, readiness time, exit
code and log, and run reports now list recorded videos with hashes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSV4zUy6rF1vgHFSyUmuva
stop() now signals the process group even after the leader exits,
escalates to SIGKILL after a configurable grace for children that ignore
SIGTERM, and never waits unbounded. record turns SIGTERM and SIGHUP into
cleanup, refuses when anything listens on the port, needs a ready status
below 400 without following redirects, reports browser launch failures,
and lists only this run's videos. Reports record cwd relative to the
repository, and review treats readiness failures as failed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSV4zUy6rF1vgHFSyUmuva
A SIGTERM arriving during stop()'s grace wait aborted it before SIGKILL,
leaving children that ignore SIGTERM running, and one arriving between
Popen and cleanup skipped stop() entirely. Signals are now deferred in
both windows and re-raised once the group is stopped. Playwright is
imported only when a browser run starts, the test helper's skip path
works without Playwright, and CI allows 180s per check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSV4zUy6rF1vgHFSyUmuva
Browser-backed UI tests put the stack suite near the old 60 second
limit, so local runs and CI now share a default with headroom instead of
a CI-only override.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSV4zUy6rF1vgHFSyUmuva
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HSV4zUy6rF1vgHFSyUmuva
@sangyeT
sangyeT merged commit d353ede into main Sep 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants