Skip to content

W2.4 verification: route smoke + zeitwerk, advisory tier, persisted per-check results - #47

Merged
pstrzalk merged 7 commits into
mainfrom
verify-revision-coverage-rework
Sep 6, 2026
Merged

W2.4 verification: route smoke + zeitwerk, advisory tier, persisted per-check results#47
pstrzalk merged 7 commits into
mainfrom
verify-revision-coverage-rework

Conversation

@pstrzalk

@pstrzalk pstrzalk commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Summary

VerifyRevision's five checks proved two things about a generated app: the lockfile is satisfied and the app boots. Nothing ever requested a page, herb_lint never ran (herb is not in the skeleton Gemfile), and boot_check failed only where db:prepare had already failed. This replaces the two dead checks with two that close measured coverage holes, caps the fix agent's error input, persists every verification run on the revision, and reconciles the docs.

Plan and measurements: thoughts/shared/plans/2026-09-05/verify-revision-coverage-rework.md (local thoughts/, not in the repo).

Five commits, ordered so the behaviour change lands last with its safety net already in place, each independently revertable:

commit
1 777aeb7 feat: add route smoke check and bin/verify-workspace — unwired, no runtime change
2 21259c8 fix: cap per-check error output fed to the fix agent
3 cc988f1 feat: persist per-check verification results on the revision
4 a35ecf7 feat: verify load-time and runtime failures, drop dominated checks — the behaviour change
5 c8b0fd6 docs: record the reworked W2.4 verification

What changes for a revision

  • CHECKS = %i[bundle_check db_prepare zeitwerk_check route_smoke rails_test]. zeitwerk:check eager-loads every app/ file (the only check that sees code no route and no test touches). Route smoke is a Minitest integration test copied into the workspace's tmp/hifumi/ for one bin/rails test run and removed again: every static GET page must not raise or 5xx, 10 s per request, Rails-mounted controllers skipped.
  • Route smoke is advisory. It still fails W2.4 and gets the two remediation attempts with the exact exception; if those don't resolve it, the revision commits with the failure recorded (W2.F0) instead of being reset. One raising page among many is not a reason to discard a revision and the ones queued behind it.
  • W2.B baseline: route smoke runs once at the parent commit before the agent, and W2.4 skips the pages it recorded. Only breakage new in this revision reaches the fix agent; a page committed broken is never re-remediated on later revisions.
  • revision.metrics["verify"] holds every verification run in order (W2.B, W2.4, W2.AR, W2.RV × 2) with per-check name, pass/fail, tier, duration, capped error text and failing paths. Transport is a [HIFUMI:VERIFY] JSON line on the roast subprocess's output, parsed by VerifyReport in ExecuteInstructionJob. Closes W2.F1/W2.F3 of the canon, which promised this and never had it.
  • Fix-agent error input is capped at 4 000 chars head-and-tail (ERROR_CAP_CHARS), keeping Minitest's counts line — the last unbounded prompt input in the workflow.
  • Standalone runner, dev only: bin/verify-workspace [--check NAME] [--known-failing PATH,PATH] <workspace>....

Verification

  • Unit suite 687 runs green, rubocop clean, at every phase.
  • All 29 existing workspaces on the new set: zeitwerk check 29/29, route smoke 29/29 at HEAD, no tmp/hifumi leftovers (four April-era workspaces do not gitignore tmp/). The only rails test failures are three pre-existing ones (two dirty working trees, one date-rotted fixture set) — identical before and after.
  • Injections on a scratch copy: authenticate_user! → FAIL route smoke (advisory) with the NoMethodError line; orphan-model syntax error → FAIL zeitwerk check alone; sleep 30 → Timeout::Error at 11 s.
  • Gated E2E (E2E_GENERATE=1) green twice: 513 s after commit 3 (every revision records a four-check W2.4), 644 s after commit 4 (every revision records W2.B then a five-check W2.4). The test now asserts the metrics round-trip itself, since its rows roll back.
  • Two deterministic dev runs on project 37: an untested /about page raising authenticate_user! → W2.B clean, W2.4 fails only route smoke (failing_routes: ["/about"]), W2.RV build(deps): bump actions/upload-artifact from 4 to 7 #1 green, committed (59 s). The same page hand-committed broken at HEAD → W2.B failing_routes: ["/about"], W2.4 all green with the page skipped, no remediation (47 s). Plus a deliberately failing test → metrics["verify"] names rails test with the Minitest failure text, then W2.RV green.
  • Preview E2E (E2E_PREVIEW=1) still green.

Where the plan was wrong, and what was done instead

  • Roast relays cog output on stderr, decorated. Probed on roast-ai 1.1.0: everything a cog prints (STDOUT.puts included) arrives as I, [ts] INFO -- ruby(:verify) ❯ <line> on stderr, 25k-char lines intact. So VerifyReport.parse_line locates the prefix anywhere in the line and the job scans both streams; the plan assumed stdout at column 0.
  • cap_error scrubs invalid UTF-8: JSON.generate raises on it, and a logging failure must not abort the verify cog.
  • bin/verify-workspace grew --known-failing and a per-check duration + Minitest counts tail, instead of the plan's throwaway Ruby driver, so the baseline view is reproducible from the repo.
  • The plan's "29/29 on the old check set" was never true: three workspaces fail rails test for reasons unrelated to verification (recorded in the plan). Both new checks are clean fleet-wide.

Not done here

  • The FORCE_AGENT_SANDBOX=1 run: no local generator image, and the followups doc records the sandbox path cannot run on the macOS dev box. After deploy, one production row closes it:

    kamal app exec --reuse "bin/rails runner 'r = Revision.order(:id).last; puts r.status; puts JSON.pretty_generate(r.metrics[\"verify\"])'"

    Expected: a W2.B record first, then W2.4 with five checks and route smoke marked advisory: true.

Follow-ups recorded (docs/09-ideas/05-followups.md, 2026-09-07)

Never discarding a committable state (build only if the "fix made it worse" query returns rows); a net-empty revision still runs W2.6 against the parent commit (pre-existing, observed here); telling the code agent about the smoke check in RevisionPrompt; db:prepare in init_rails_app; hardcoded dates rotting generated test suites.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VkPCN5GsAR8p3gDDVPYZEd

@visuality-pstrzalkowski @claude
feat: add route smoke check and bin/verify-workspace
777aeb7 
VerifyRevision's five checks prove the lockfile is satisfied and the app
boots; nothing in the set ever requests a page. `before_action
:authenticate_user!` without devise (project 27, 2026-05-14) passed every
check and surfaced only when the user opened the preview, because
`rails test` catches it only if the agent happened to write a test for
that action, and is skipped entirely when it wrote none.

This adds the check without wiring it: CHECKS is untouched, so no runtime
behaviour changes, but the check is exercisable against every existing
workspace.

- lib/roast/route_smoke_check.rb is a Minitest integration test copied
  into the workspace as tmp/hifumi/route_smoke_test.rb for one
  `bin/rails test` run and removed again (four April-era workspaces do
  not gitignore tmp/, and W2.5's `git add -A` would commit it). One test
  per static GET route: must not raise, must not 5xx, 10 s per request.
  Rails-mounted controllers (rails/, active_storage/, action_mailbox/,
  turbo/) are skipped — /rails/conductor/action_mailbox/inbound_emails
  500s on every generated app. Exceptions are flunked with one line:
  tmp/ is not one of Rails' app dirs, so the backtrace cleaner falls back
  to the full ~70-frame trace otherwise (12k chars per failure, measured).
- Routes listed in tmp/hifumi/known_failing are skipped and every failing
  path lands in tmp/hifumi/failing — the plumbing for a pre-agent
  baseline so only breakage new in a revision reaches the fix agent.
- VerifyRevision gains an ADVISORY tier (route_smoke triggers remediation
  but never blocks the commit), blocking_failed?, run_one, a per-check
  HINTS line for the fix prompt (Minitest ends with a re-run hint for a
  file the verifier deletes), the check symbol and duration on every
  result, and Shellwords.escape on the workspace in run_cmd.
- bin/verify-workspace runs the checks against a workspace outside a
  roast subprocess; `--check NAME` for one, `--known-failing PATH,PATH`
  for the baseline view. Dev only: it executes the workspace's code
  unsandboxed.

Route smoke passes on all 29 existing workspaces at HEAD (project_27's
working tree carries an untracked BCrypt fixture that fails every test at
load); injected `authenticate_user!`, a known-failing skip and a `sleep
30` timeout behave as designed on a scratch copy of project_43.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VkPCN5GsAR8p3gDDVPYZEd
@visuality-pstrzalkowski @claude
fix: cap per-check error output fed to the fix agent
21259c8 
format_errors was the last unbounded prompt input in the revision
workflow: the full stdout+stderr of every failing check went verbatim
into agent(:fix), up to three times per revision. Measured on project_43,
`rails test` with every action broken is 10 425 chars; 28k chars of
stacktrace across three fix prompts is the shape of incident this closes.
StatCap (60/50 lines), the 16 000-char diff slice in update_docs and
AppState::DOC_FILE_CAP already bound theirs.

Head-and-tail at 4 000 / 1 000 chars rather than head-only because
Minitest puts its run/failure/error counts on the last line — the most
useful single signal once the head is cut, and the same reason StatCap
keeps the git-stat summary line. The marker between says how much was
dropped; the agent can re-run the suite itself. Lands before the per-check
results are persisted, so the stored error text is capped from its first
row.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VkPCN5GsAR8p3gDDVPYZEd
@visuality-pstrzalkowski @claude
feat: persist per-check verification results on the revision
cc988f1 
Nothing but exit codes crossed the roast subprocess boundary: which check
failed, and why, existed only in container logs — contrary to W2.F1 ("save
last verification errors") and W2.F3 in the architecture canon. Landing
this before the check set changes means the new checks' real failure rate
is measured from their first production run.

- VerifyRevision.sentinel emits one [HIFUMI:VERIFY] line of JSON after
  every verification run — W2.4, W2.AR (with the recipes applied) and
  W2.RV (with the attempt number, from the repeat index every cog input
  block receives) — carrying per-check name, pass/fail, advisory flag,
  duration, capped error text and, for route smoke, the failing paths.
- VerifyReport.parse_line (lib/roast, since both sides need the prefix)
  validates the record's shape rather than the prefix alone: agent output
  is arbitrary and the parser runs inside the job's log-streaming threads,
  so anything malformed is nil, never a raise.
- ExecuteInstructionJob scans BOTH subprocess streams. Probed on roast-ai
  1.1.0 with a ruby-only workflow: Roast captures whatever a cog prints
  (STDOUT.puts included) and relays it through its own logger on stderr as
  "I, [ts]  INFO -- ruby(:verify) ❯ <line>", 25k-char lines intact, and
  the bin/roast-* wrappers exec without redirection. So the prefix is
  located anywhere in the line, and run_roast_subprocess returns a fourth
  element — existing three-element stubs destructure to nil. Records land
  in revision.metrics["verify"] in order, and the key is absent when the
  subprocess printed none.
- cap_error scrubs invalid UTF-8: JSON.generate raises on it, and a
  logging failure must not abort the verify cog.

E2E (E2E_GENERATE=1, 2026-09-06, 513 s): all three revisions carry a W2.4
record with the four live checks. The test asserts the round-trip itself
now — it is transactional, so its rows are gone afterwards.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VkPCN5GsAR8p3gDDVPYZEd
@visuality-pstrzalkowski @claude
feat: verify load-time and runtime failures, drop dominated checks
a35ecf7 
CHECKS = %i[bundle_check db_prepare zeitwerk_check route_smoke rails_test]

boot_check out: `rails runner "puts :ok"` failed in exactly one row of the
measured coverage matrix, and db:prepare — which boots the same app,
first — failed there too. herb_lint out: guarded on a gem the skeleton
never had, it returned nil on every revision of every project since the
Phase 1 spike. Reviving it was evaluated and rejected: on four real
generated apps every error-severity finding was a style convention
(instance variables in partials, `<input />`, autocomplete) — 16 and 10
errors on two apps that render fine.

zeitwerk_check in, blocking: eager-loads every app/ file, the only check
that sees code no route and no test touches — a syntax error in an orphan
model, a constant missing at class-body level, a filename/constant
mismatch — all of which passed the old set.

route_smoke in, advisory: requests every static GET page in the test env.
Closes the project-27 class of bug (`before_action :authenticate_user!`
without devise), which passed every old check and surfaced only when the
user opened the preview. Advisory because one raising page among many is
not a reason to throw away a revision and the ones queued behind it: it
still enters W2.AR/W2.R with the exact exception, and if two fix attempts
do not resolve it the revision commits with the failure recorded in the
verify sentinel (W2.F0) instead of resetting.

W2.B — a route smoke baseline at the parent commit, before the agent
runs — is what makes advisory safe: W2.4 skips pages already failing at
HEAD, so only breakage new in this revision reaches the fix agent, and a
page committed broken is never re-remediated on every later revision.

Verified: the unit suite; all 29 existing workspaces on the new set —
zeitwerk 29/29, route smoke 29/29 at HEAD, the same three pre-existing
rails-test failures as before the change and nothing new; the gated E2E
green in 644 s with every revision recording W2.B then a five-check W2.4;
and two deterministic dev runs on project 37: an untested page raising
`authenticate_user!` gave W2.B clean → W2.4 route smoke FAIL (advisory,
failing_routes ["/about"]) with every blocking check green → W2.RV #1
green → committed; the same page hand-committed broken at HEAD gave W2.B
failing_routes ["/about"] → W2.4 all green with the page skipped → no
remediation.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VkPCN5GsAR8p3gDDVPYZEd
@visuality-pstrzalkowski @claude
docs: record the reworked W2.4 verification
c8b0fd6 
The canon described the pre-2026-05-01 shape: an a–e check list with herb
lint and a boot check, no W2.AR, a failure path that promised to save
verification errors nothing ever saved, and a chat agent said to hold the
errors in context. Bring it to what the code does now.

- 01-workflows-and-decisions.md: W2.B (route smoke baseline at the parent
  commit), the five checks with tier and the one-line reason each exists,
  W2.AR (present since 2026-05-01, never documented), W2.F0 (advisory-only
  failure commits), W2.F1/F3 and D6 stated as built — check-level detail
  lives in revision.metrics["verify"], the chat sees status only — and a
  "W2.4, as built" note recording why boot_check and herb_lint went and
  why herb was evaluated and rejected rather than revived.
- 03-tech-stack.md: the Herb + ReActionView section and the two "why not"
  rows that pointed at it are gone; the intent left the canon with the
  decision recorded where decisions live.
- 01-vision-and-principles.md, 02-user-journey.md: the three places that
  repeated the old check chain as current behaviour.
- 05-followups.md: the "grep for missing-gem signals" entry is deleted —
  route smoke catches that class where it breaks, generically — and a dated
  section holds what this work left open, including the net-empty-revision
  W2.6 defect observed while verifying.
- 01-git-integration.md: the hook idea names rubocop, which generated apps
  ship, instead of HERB.
- CLAUDE.md: a W2.4 verification convention bullet — the five checks,
  blocking vs advisory, W2.B, the tmp/hifumi mechanics, the stderr-relayed
  sentinel, metrics["verify"], the error cap, bin/verify-workspace.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VkPCN5GsAR8p3gDDVPYZEd
@visuality-pstrzalkowski @claude
fix: spawn verification checks as argv, no shell
3c78c30 
Brakeman flagged run_cmd's backticks (`cd #{Shellwords.escape(workspace)}
&& #{cmd} 2>&1`) as possible command injection. There was no path for one:
cmd is one of five literals from perform, workspace is the job's own
project.workspace_path, escaped. The warning appeared because the ignore
entry for the old text stopped matching when the line changed.

Instead of re-ignoring, remove the shell. Each check is an argv array
spawned with Open3.capture2e(*argv, chdir: workspace): nothing to escape,
no shell to inject into, arguments reach the program byte-for-byte, and
stderr merges into stdout as `2>&1` did. A program that cannot start
(missing bin/rails, missing workspace) is a failed check carrying the
Errno as its output, which is what the shell's "command not found" used
to be. The two verify_revision.rb entries in config/brakeman.ignore are
gone — one for the line that no longer interpolates, one for
gem_available?, deleted with herb_lint. AutoRemediate.shell keeps its
shell and its entry; its cmd comes from the RECIPES table alone.

Tests pin the property directly: `$HOME; rm -rf / && \`id\` | cat` arrives
at printf as a literal string.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VkPCN5GsAR8p3gDDVPYZEd
@visuality-pstrzalkowski @claude
chore: bump rubyzip to 3.6.0 (CVE-2026-85396)
6732c70 
CI's bin/bundler-audit went red on the ruby-advisory-db update of
2026-09-05: rubyzip 3.3.0, path traversal, fixed in >= 3.4.0. rubyzip is a
transitive test-group dependency via selenium-webdriver (>= 1.2.2, < 4.0),
so `bundle update --conservative rubyzip` moves that one line and nothing
else. Unrelated to the branch it lands on.

The skeleton's own lockfile (lib/preview/skeleton/Gemfile.lock) pins
rubyzip 3.2.2 through the same selenium dependency; CI does not audit it,
and bumping it changes the bundle baked into the generator image and every
new workspace, so it is left for its own change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VkPCN5GsAR8p3gDDVPYZEd
@pstrzalk
pstrzalk merged commit 5b12ed5 into main Sep 6, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@pstrzalk @visuality-pstrzalkowski