diff --git a/CLAUDE.md b/CLAUDE.md index f046091..ef12dd7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,6 +57,6 @@ Additional sources from the Phase 1 spike: - **LLM model selection**: never hardcode a model id at a call site — `lib/llm/stages.rb` (`LLM::Stages`, note the `LLM` acronym inflection) is the registry of the six LLM stages (chat, plan_creation, plan_modification, template, code, docs), their labels, factory defaults, and the curated `AVAILABLE_MODELS` list (full OpenRouter ids only, never `sonnet`/`haiku` aliases). Profiles store per-user defaults (`default__model`), projects snapshot their own selection (`_model`) at creation; selectors live in the build tab (`model_selections/_pane`), the new-project form, and the account integrations pane. A new stage = registry entry + migration on both tables + threading at the call site. A new **model** = confirm the OpenRouter slug supports `structured_outputs` (plan/template stages) and `tools` (chat) → populate the RubyLLM registry in **every** environment → only then add the id to `AVAILABLE_MODELS`; that order is load-bearing, because RubyLLM owns its registry: it resolves against the `ruby_llm_models` table (its store) and falls back to the gem's bundled `models.json` only when that table is *empty*, so an id absent from both raises `ModelNotFoundError` on the four RubyLLM-backed stages. Check with `bin/verify-model-registry [candidate-id]`, fix with `RubyLLM.models.refresh!` plus a process restart (the registry is memoized per process); the refresh only ever adds or updates rows — per-provider fetch failures are logged and skipped, never allowed to empty the store. Full procedure in `docs/05-runbooks/03-llm-model-registry.md`. Selection applies on the OpenRouter path only — `roast_model_env` keeps claudesubscription runs on the operator's ENV/alias defaults, and an explicit `HIFUMI_DEV_MODEL`/`HIFUMI_DEV_DOCS_MODEL` always wins. - **Agent sandbox (tenant isolation)**: the codegen agent runs `claude` with `skip_permissions!` on user-controlled prompts — treat it as untrusted code execution, so in production it must NOT run in the shared generator container. `ExecuteInstructionJob#execute_revision` wraps the roast invocation via `Roast::Sandbox.wrap` (`lib/roast/sandbox.rb`, plain builder, returns the `docker run` argv) into a throwaway `--rm` container that mounts ONLY this project's workspace (no `workspace_root`, no `/var/run/docker.sock`), runs entirely as the unprivileged `generator` user (`--user`, `--cap-drop=ALL`, zero cap-adds — uniform uid avoids the capless-root/mixed-ownership deadlock of issue #24; the workspace is re-relaxed `a+rwX` before every sandboxed run), and forwards env (incl. `OPENROUTER_API_KEY` and the per-project `HIFUMI_DEV_*` model selection) **by name** so secrets never hit argv. Gated by `sandboxed?` (production OR `FORCE_AGENT_SANDBOX=1`); dev stays direct (single-tenant, Claude-subscription transport, no Docker). Image = the generator's own, from `HIFUMI_AGENT_IMAGE` (set in `deploy.yml`). Not runtime-verifiable on macOS — see the verification checklist + residuals (generator-side socket, egress) in `docs/09-ideas/05-followups.md`. - **Preview infrastructure**: `lib/preview/preview_manager.rb` (plain Ruby, not a Roast workflow) drives Docker. `lib/preview/Dockerfile{,.base}` are owned by this repo — never read from generated apps. `lib/preview/skeleton/` is the canonical fresh-Rails-app baseline copied into every workspace; regenerate with `bin/preview-regen-skeleton` when bumping Rails. Rebuild the base image with `bin/preview-rebuild-base` after Gemfile changes. The `preview-internal` Docker network is created without `--internal` on Docker Desktop (host port mapping wouldn't work otherwise) — Phase 4 reintroduces strict egress isolation on a Linux production host. In remote mode `PreviewManager#run_container` passes `PREVIEW_HOST=.preview.` to the container; the skeleton-overlay's `preview_iframe.rb` initializer appends it to `Rails.application.config.hosts` so Rails 8's dev HostAuthorization doesn't 403 the kamal-proxy request. -- **Modification planner context**: `PlanApplicationModification::AdHocLLM` plans against a snapshot of the real workspace, built by `AppState.build(workspace:)` in `lib/app_state.rb` — gems beyond the default Gemfile, database tables (or the `db/migrate/` listing while `db/schema.rb` is not written yet), `config/routes.rb` verbatim, every `.rb/.erb/.js/.css` under `app/` minus `app/assets/builds/`, and the four `docs/` files. `ModifyApplication#execute` assembles it (inside its `rescue StandardError`, so an unreadable file degrades to a chat-safe error hash) and passes `context: { app_state: }`; the planner renders it after `Intent:`. The docs cap is **per file** (`AppState::DOC_FILE_CAP`, 8 000 chars), never on the total — a body cap evicts whichever file is last, and that is `frontend.md`, the only record of the app's palette. The W2.6 docs-writer prompt in `lib/roast/revision_workflow.rb` hardcodes the same 8 000 (it runs as a Roast subprocess, outside the autoloader) — change both together. The **creation** planner stays blind by ordering, not choice: `CreateApplication` persists the plan before `ExecuteInstructionJob` runs `rails new`, so no workspace exists yet and its system prompt is its only lever. Both planner prompts say only what IS there ("default Rails 8", Tailwind, Hotwire, `has_secure_password` for sign-in) — no gem-absence claims, no Propshaft/Importmap/Solid enumeration, no `--accent`-style token names. Verify any planner change with `bin/inspect-plan-application-modification "" [--blind]` and `bin/inspect-plan-application-creation ""`: both dry-run the planner and persist nothing, and `--blind` is the same-session A/B against planning from the intent alone. -- **W2.4 verification**: `VerifyRevision` (`lib/roast/verify_revision.rb`) runs five checks in order — `bundle check` (lockfile satisfied; the only short-circuit, everything after it would repeat the same error), `db:prepare` (boot + migrations; writes `db/schema.rb`, which the test-env checks need), `zeitwerk:check` (every `app/` file loads — the only check that sees code no route and no test touches), route smoke (every static GET page requested once in the test env, 10 s per request) and `rails test` (when tests exist). All are blocking except route smoke, which is **advisory**: it still fails W2.4 and enters W2.AR/W2.R with the exact exception, but if two fix attempts don't resolve it the revision commits anyway (W2.F0) instead of being reset. **W2.B** runs route smoke alone at the parent commit before the agent, and W2.4 skips the pages it recorded — so only breakage new in this revision reaches the fix agent and a page committed broken is never re-remediated. Route smoke copies `lib/roast/route_smoke.rb` + `route_smoke_check.rb` into the workspace's `tmp/hifumi/` for one `bin/rails test tmp/hifumi/route_smoke_test.rb` and removes the directory afterwards (four April-era workspaces don't gitignore `tmp/`); a fix-prompt hint tells the agent that file is the verifier's. Every verification run (W2.B, W2.4, W2.AR, W2.RV × 2) is printed as a `[HIFUMI:VERIFY]` JSON line — Roast relays cog output through its own logger on **stderr**, decorated (`I, [ts] INFO -- ruby(:verify) ❯ …`), so `VerifyReport.parse_line` finds the prefix anywhere in the line and `ExecuteInstructionJob` scans both streams — and lands in `revision.metrics["verify"]` with per-check pass/fail, tier, duration, capped error text and failing paths. Fix-agent error input is capped head-and-tail at `ERROR_CAP_CHARS` (4 000). Standalone, **dev only** (it executes the workspace's code unsandboxed): `bin/verify-workspace [--check NAME] [--known-failing PATH,PATH] ...`. `boot_check` and `herb_lint` were removed 2026-09-06 (dominated / never ran); the as-built note in `docs/02-architecture/01-workflows-and-decisions.md` records why herb was rejected rather than revived. +- **Modification planner context**: `PlanApplicationModification::AdHocLLM` plans against a snapshot of the real workspace, built by `AppState.build(workspace:)` in `lib/app_state.rb` — gems beyond the default Gemfile, database tables (or the `db/migrate/` listing while `db/schema.rb` is not written yet), `config/routes.rb` verbatim, every `.rb/.erb/.js/.css` under `app/` minus `app/assets/builds/`, and the four `docs/` files. `ModifyApplication#execute` assembles it (inside its `rescue StandardError`, so an unreadable file degrades to a chat-safe error hash) and passes `context: { app_state: }`; the planner renders it after `Intent:`. The docs cap is **per file** (`AppState::DOC_FILE_CAP`, 8 000 chars), never on the total — a body cap evicts whichever file is last, and that is `frontend.md`, the only record of the app's palette. The W2.6 docs-writer prompt in `lib/roast/revision_workflow.rb` hardcodes the same 8 000 (it runs as a Roast subprocess, outside the autoloader) — change both together. The **creation** planner stays blind by ordering, not choice: `CreateApplication` persists the plan before `ExecuteInstructionJob` runs `rails new`, so no workspace exists yet and its system prompt is its only lever. Both planner prompts say only what IS there ("default Rails 8", Tailwind, Hotwire, `has_secure_password` for sign-in) — no gem-absence claims, no Propshaft/Importmap/Solid enumeration, no `--accent`-style token names. In the same positive register, both pin tests to **Minitest under `test/`** with the file layout spelled out and the test stack declared complete ("plan no additional testing gems, frameworks or coverage tools") — the framework is chosen in the plan, and the code agent follows a task's file paths over its own rules, which is how production project 40 got an RSpec suite `rails test` never ran. The prompt tests pin the framing by refusing `rspec|spec/|factory_bot|simplecov` in either prompt. Verify any planner change with `bin/inspect-plan-application-modification "" [--blind]` and `bin/inspect-plan-application-creation ""`: both dry-run the planner and persist nothing, and `--blind` is the same-session A/B against planning from the intent alone. +- **W2.4 verification**: `VerifyRevision` (`lib/roast/verify_revision.rb`) runs five checks in order — `bundle check` (lockfile satisfied; the only short-circuit, everything after it would repeat the same error), `db:prepare` (boot + migrations; writes `db/schema.rb`, which the test-env checks need), `zeitwerk:check` (every `app/` file loads — the only check that sees code no route and no test touches), route smoke (every static GET page requested once in the test env, 10 s per request) and `rails test` (when tests exist). All are blocking except route smoke, which is **advisory**: it still fails W2.4 and enters W2.AR/W2.R with the exact exception, but if two fix attempts don't resolve it the revision commits anyway (W2.F0) instead of being reset. **W2.B** runs at the parent commit before the agent — `AutoRemediate.ensure_bundle` (`bundle check`, then the W2.AR install recipe if it fails) and then route smoke — and W2.4 skips the pages it recorded, so only breakage new in this revision reaches the fix agent and a page committed broken is never re-remediated. The bundle step is not optional: the sandbox container is throwaway, so a gem an earlier revision's agent installed is in the lockfile but not in this container's `BUNDLE_PATH`, and the smoke would die on `Bundler::GemNotFound` with no per-route data — blind exactly when a gem has been added (production project 40). When the install itself fails, W2.B skips the smoke and records that; the baseline's sentinel carries both checks plus `applied`. Route smoke copies `lib/roast/route_smoke.rb` + `route_smoke_check.rb` into the workspace's `tmp/hifumi/` for one `bin/rails test tmp/hifumi/route_smoke_test.rb` and removes the directory afterwards (four April-era workspaces don't gitignore `tmp/`); a fix-prompt hint tells the agent that file is the verifier's. Every verification run (W2.B, W2.4, W2.AR, W2.RV × 2) is printed as a `[HIFUMI:VERIFY]` JSON line — Roast relays cog output through its own logger on **stderr**, decorated (`I, [ts] INFO -- ruby(:verify) ❯ …`), so `VerifyReport.parse_line` finds the prefix anywhere in the line and `ExecuteInstructionJob` scans both streams — and lands in `revision.metrics["verify"]` with per-check pass/fail, tier, duration, capped error text and failing paths. Fix-agent error input is capped head-and-tail at `ERROR_CAP_CHARS` (4 000). Standalone, **dev only** (it executes the workspace's code unsandboxed): `bin/verify-workspace [--check NAME] [--known-failing PATH,PATH] ...`. `boot_check` and `herb_lint` were removed 2026-09-06 (dominated / never ran); the as-built note in `docs/02-architecture/01-workflows-and-decisions.md` records why herb was rejected rather than revived. - **RubyLLM tools must be idempotent within a user turn**. RubyLLM's tool loop will sometimes call the same tool twice in adjacent assistant messages before either result lands, producing the order `assistant(use_X) → assistant(use_Y) → user(result_X) → user(result_Y)` — illegal for Anthropic, and the chat permanently rejects every subsequent message. **There is no tool-side guard any more** — the tool that carried one was deleted in `13e9c1c`. What stands today is a prompt rule (*"Call each tool AT MOST ONCE per user turn"*, `app/prompts/generator_agent/instructions.txt.erb:19`) plus the `RubyLLM::BadRequestError` banner in `ChatRespondJob::FRIENDLY_ERRORS` that turns an already-corrupt chat into *"This conversation can't be continued by the model. Please start a new project."* So a new tool needs its own in-band guard: return an error hash rather than raising, so the second `tool_use` still gets a `tool_result` and history stays valid. Diagnose corrupt chats with `bin/inspect-chat ` (works on prod via `kamal app exec`). diff --git a/app/prompts/plan_application_creation_system.md b/app/prompts/plan_application_creation_system.md index e97d3dd..1df7968 100644 --- a/app/prompts/plan_application_creation_system.md +++ b/app/prompts/plan_application_creation_system.md @@ -4,6 +4,7 @@ Rules for the plan: - 3 to 6 revisions. - Each revision is one atomic, testable change ("add Product model with name/price", not "set up the shop"). - The workspace is a default Rails 8 app with Tailwind and Hotwire, on the default Gemfile. Do NOT include `rails new`. If the app needs sign-in, plan a revision that adds it — `has_secure_password` plus sessions is the Rails-native default. +- Tests are Minitest under `test/`, the Rails default, run with `bin/rails test`: `test/models/_test.rb`, `test/controllers/_controller_test.rb`, `test/integration/_test.rb`, fixtures in `test/fixtures/`. The default test stack is complete — plan no additional testing gems, frameworks or coverage tools. Every revision that adds or changes behaviour names the Minitest file(s) it adds. - Prefer Rails Way: scaffolds, concerns, validations over custom abstractions. - Mount the primary user-facing feature at the root path (`root to: "...#index"`) unless the user explicitly asked for a different landing page. The first revision that introduces that feature must set the root route. - When the plan introduces more than one user-facing feature, include a revision that adds a top or side navigation menu in `app/views/layouts/application.html.erb` linking to each feature — unless the user explicitly asked for a different navigation pattern (e.g. single-page, dashboard-only). diff --git a/app/prompts/plan_application_modification_system.md b/app/prompts/plan_application_modification_system.md index 94444e0..84f9ca5 100644 --- a/app/prompts/plan_application_modification_system.md +++ b/app/prompts/plan_application_modification_system.md @@ -9,6 +9,7 @@ Rules for the plan: - DO NOT re-introduce models, controllers, or views that already exist. Reference existing files by path; describe modifications rather than scaffolds. - DO NOT add a navigation menu unless the user explicitly asks for one. Modify the existing navigation only when relevant. - The app is a default Rails 8 installation — everything Rails ships with is there and needs no setup. On top of that, Tailwind and Hotwire (Turbo + Stimulus) ARE installed: use them, and reach for Turbo Frames / Turbo Streams / Stimulus rather than hand-written fetch or a JS framework. +- Tests are Minitest under `test/`, the Rails default, run with `bin/rails test`: `test/models/_test.rb`, `test/controllers/_controller_test.rb`, `test/integration/_test.rb`, fixtures in `test/fixtures/`. The default test stack is complete — plan no additional testing gems, frameworks or coverage tools. Every revision that adds or changes behaviour names the Minitest file(s) it adds. - Plan only with what the "Gems" section names plus what a default Rails 8 install ships. For sign-in, the Rails-native route is `has_secure_password` plus sessions. - Style with the literal Tailwind classes and hex values the app already uses. `docs/frontend.md` is the palette source; the file list tells you which views exist, not what is in them. Do NOT introduce CSS variables the app does not define. - Do NOT hedge. The application state is given to you. Never write "if it uses X…", "whichever matches your design", "assuming Y exists", or "verify that…". Name the actual file, the actual class, the actual colour. diff --git a/docs/02-architecture/01-workflows-and-decisions.md b/docs/02-architecture/01-workflows-and-decisions.md index 83ee685..b8bbf6e 100644 --- a/docs/02-architecture/01-workflows-and-decisions.md +++ b/docs/02-architecture/01-workflows-and-decisions.md @@ -73,10 +73,13 @@ W2.2 [deterministic] Build prompt: - app manifest (docs/) - revision notes from the previous revision (if any) - plan context (what's done, what remains) -W2.B [deterministic] Route smoke baseline at the parent commit: request every static GET - page once and record which already fail. W2.4 skips those, so only breakage - NEW in this revision is attributed to it — the fix agent is never asked to - repair a page an earlier revision left broken. +W2.B [deterministic] Bundle + route smoke baseline at the parent commit. Runs `bundle check` + first and installs the missing gems if it fails: the sandbox container is + throwaway, so gems an earlier revision added are in the lockfile but not in + the new container. Then requests every static GET page once and records which + already fail. W2.4 skips those, so only breakage NEW in this revision is + attributed to it — the fix agent is never asked to repair a page an earlier + revision left broken. W2.3 [LLM/agent] Execute Claude CLI with the prompt in the workspace cwd → Agent (Claude Code) with a constrained scope: step description + cwd. W2.4 [deterministic] Verification — five checks, in order: @@ -242,6 +245,8 @@ Every place where the LLM makes a decision, explicitly described. **W2.4, as built (2026-09-06)**: the original a–e list carried two checks with no signal of their own. `rails runner "puts :ok"` failed in exactly one row of a measured coverage matrix, and `db:prepare` — which boots the same app, first — failed there too. `herb lint` was guarded on a gem the skeleton never had and returned "not applicable" on every revision of every project since the Phase 1 spike. Herb was **evaluated and rejected**, not silently dropped: it installs and runs in ~0.5 s, but on four real generated apps every `error`-severity finding was a style convention — instance variables in partials, ``, a missing `autocomplete` — 16 and 10 errors on two apps that render fine. As a blocking check it would have wedged both on day one and spent fix-agent turns rewriting partials to satisfy a linter, on a product whose output the user judges by looking at the page. The replacements close measured holes: `zeitwerk:check` catches the orphan-file class (syntax error, constant missing at class body, filename/constant mismatch) that every old check passed, and route smoke catches the raising-page class (project 27's `authenticate_user!` without devise, found only when the user opened the preview) that `rails test` catches only if the agent happened to write a test for that action. Route smoke is **advisory** because one raising page among many is not a reason to discard the revision and the ones queued behind it; it still gets the two remediation attempts with the exact exception. **W2.B** exists because without it a page committed broken under W2.F0 would be re-remediated on every later revision (up to 2 × the fix budget each, forever), and a fix agent told `Timeout::Error` on a page that calls an external API may "fix" it by deleting the feature. "Signal-only" (record, never remediate) was considered and rejected: it delivers nothing user-visible — the user still meets the 500 in the preview with no help. Both additions were run against all 29 existing workspaces before landing; none is wedged. Plan and measurements: `thoughts/shared/plans/2026-09-05/verify-revision-coverage-rework.md`. +**Which framework the suite is written in is decided in the plan, not in the rules (2026-09-06)**: production project 40 was asked for "all business logic covered by automated tests" and got RSpec — `spec/` paths in all six revision prompts, `rspec-rails` in the Gemfile, 62 examples `rails test` never ran, because it is gated on `test/**/*_test.rb` and there were none. The code agent's rules said "Minitest, not RSpec"; the log shows it naming the conflict and following the task's file paths anyway. So the fix goes to the source: both planner prompts pin tests to Minitest under `test/` and close the test stack against extra gems, and the code agent's rule now says what to do when a task disagrees with it — write the equivalent Minitest tests, do not follow the path. Measured on the creation planner with project 40's own intent: 3 of 5 dry runs produced RSpec before, 0 of 5 after. Plan: `thoughts/shared/plans/2026-09-07/minitest-planning-and-baseline-bundle.md`. + --- ## Agent — the only unstructured element diff --git a/docs/09-ideas/05-followups.md b/docs/09-ideas/05-followups.md index 6535f95..7e0c62c 100644 --- a/docs/09-ideas/05-followups.md +++ b/docs/09-ideas/05-followups.md @@ -214,3 +214,11 @@ The check set was reworked (`thoughts/shared/plans/2026-09-05/verify-revision-co - **Tell the code agent about the smoke check** in `RevisionPrompt` ("every static page is requested once during verification; pages must not raise with no session and fixture data"). Likely to reduce smoke failures at the source, but a change to the code-agent prompt changes every generation and deserves its own before/after evaluation. - **`db:prepare` in `init_rails_app`** so `db/schema.rb` exists from the first commit (the modification planner would like that too). The W2.B baseline does not need it. - **Generated tests with hardcoded dates rot.** project_39's suite (green when generated) now fails 26 of 214 tests with `Start date can't be in the past` — fixture dates the calendar has since passed. Not a verification defect, but the kind of thing a `RevisionPrompt` rule (relative dates in fixtures and tests) would prevent; noticed while sweeping all 29 workspaces. + +### Minitest planning + W2.B bundle repair — follow-ups + +Landed with `thoughts/shared/plans/2026-09-07/minitest-planning-and-baseline-bundle.md`: both planner prompts pin tests to Minitest under `test/`, `RevisionPrompt` translates a `spec/` task instead of following it, and W2.B runs `AutoRemediate.ensure_bundle` before the route smoke. Left open: + +- **A per-project gem store that survives the container.** W2.B now reinstalls the missing gems, but it reinstalls them *every revision* — the sandbox is `docker run --rm` and `BUNDLE_PATH=/usr/local/bundle` dies with it, so an app that added `redcarpet` re-downloads it on every later revision for the life of the project. The fix is a named Docker volume mounted at `/usr/local/bundle`: Docker seeds a named volume from the image's content on first use, so the baked skeleton bundle is kept and agent-added gems persist on top of it. Two open questions before building it: (1) **lifecycle** — the volume outlives the project unless something deletes it, so project deletion needs to remove it too; (2) **image upgrades** — a volume seeded from an old image keeps the old gems and shadows the new baked bundle, which argues for keying the volume name by image digest and accepting a re-seed (and one slow revision) per deploy. Neither is hard; both are decisions, not code. +- **A mechanical Minitest guard**, if the prompt rule ever regresses. Phase 5 of the plan above, not built: `VerifyRevision.perform(:rails_test)` fails the revision when `spec/**/*_spec.rb` exists, with a `HINTS[:rails_test]` line telling the fix agent to convert rather than delete. The measured pass rate (0 of 5 creation dry runs producing RSpec, down from 3 of 5) did not justify it — the docs-agent precedent says a prompt rule that measures clean is worth keeping until it measures dirty. +- **A revision that writes no tests is invisible in the metrics.** `rails_test` is skipped when the workspace has none, and a skipped check leaves no record at all — the absence of the entry is the only signal, which makes "how often does a revision ship untested?" unanswerable without re-deriving it from the workspace. Production project 39 shipped five such revisions the same evening project 40 shipped RSpec. An explicit `{check: :rails_test, skipped: true}` entry in the sentinel would make the rate queryable straight off `revision.metrics["verify"]`. diff --git a/lib/roast/auto_remediate.rb b/lib/roast/auto_remediate.rb index 17196ca..71b1874 100644 --- a/lib/roast/auto_remediate.rb +++ b/lib/roast/auto_remediate.rb @@ -47,6 +47,22 @@ def self.run(workspace, errors_text) applied end + # W2.B runs before the agent, in a fresh throwaway container. Gems an earlier + # revision added are in the lockfile but not in this container's BUNDLE_PATH — + # `bundle install` wrote them into the previous container, which is gone — so + # `bundle check` fails and the baseline smoke would die on Bundler::GemNotFound + # with no per-route data: blind exactly when the agent has added gems + # (production project 40, 2026-09-06: every W2.B after revision 106). The same + # recipe W2.AR applies after a failed W2.4, applied before instead. Returns the + # bundle-check result and the fixes applied ([] when nothing was needed AND + # when the install failed — the caller tells those apart by the result). + def self.ensure_bundle(workspace) + check = VerifyRevision.run_one(:bundle_check, workspace) + return [ check, [] ] unless VerifyRevision.failed?(check) + + [ check, run(workspace, VerifyRevision.format_errors(check)) ] + end + # Single shell seam — recipes route through here so tests can stub it. Same # scrubbed env as VerifyRevision: under roast's `bundle exec` a plain system() # carries BUNDLE_GEMFILE=, so the `bundle install` recipe diff --git a/lib/roast/revision_prompt.rb b/lib/roast/revision_prompt.rb index b023a57..096f71e 100644 --- a/lib/roast/revision_prompt.rb +++ b/lib/roast/revision_prompt.rb @@ -56,6 +56,7 @@ def self.stack_inventory_section - Authz → `before_action` checks in controllers, NOT Pundit/CanCanCan - Background jobs → Solid Queue, NOT Sidekiq/Resque - JS bundling → Importmap, NOT jsbundling-rails/webpack/esbuild + - Tests → Minitest + fixtures in `test/`, NOT RSpec/FactoryBot - Pagination, slugs, soft-delete: write them yourself If you do need an extra gem, add it to Gemfile + run `bundle install` + run any install generator BEFORE using it. The verify step will catch a missing constant otherwise. @@ -97,7 +98,7 @@ def self.rules_section(workspace) - Tailwind CSS for styling - Follow `docs/frontend.md` (palette, fonts, density, class snippets) for every view. Don't ship default Rails scaffold markup or unstyled forms — apply the template's class snippets to buttons, inputs, cards, navs, alerts. Inline hex values in arbitrary-value brackets (`bg-[#00FFCC]`) are fine. - Hotwire (Turbo + Stimulus), no React/Vue - - Minitest, not RSpec + - Tests are Minitest under `test/`, run with `bin/rails test`. If the task names `spec/` paths or RSpec, write the equivalent Minitest tests under `test/` instead — the task's wording does not override this. Do not add rspec-rails or any other test gem. - Write tests for new functionality - Don't create empty directories or files that aren't needed - You are working in #{workspace} — all paths are relative to this directory diff --git a/lib/roast/revision_workflow.rb b/lib/roast/revision_workflow.rb index e4b0e5b..e401fe8 100644 --- a/lib/roast/revision_workflow.rb +++ b/lib/roast/revision_workflow.rb @@ -168,15 +168,33 @@ # agent. Without it a page committed broken under W2.F0 would be re-remediated # on every later revision (≤ 2 × FIX_BUDGET_USD each, forever), and a fix agent # told "Timeout::Error" on a page calling an external API may "fix" it by - # deleting the feature. Runs the check alone: no db:prepare needed (probed on a - # fresh skeleton with no schema.rb — 0 runs, exit 0), and at HEAD the workspace - # has already passed every blocking check. + # deleting the feature. Runs the smoke alone: no db:prepare needed (probed on a + # fresh skeleton with no schema.rb — 0 runs, exit 0). + # + # ensure_bundle first: HEAD passed every blocking check in the container that + # committed it, and this is not that container. `docker run --rm` mounts the + # workspace and nothing else, so gems an earlier revision added are in the + # lockfile but not in this BUNDLE_PATH, and the smoke would die on + # Bundler::GemNotFound with no per-route data — blind exactly when a gem has + # been added (production project 40, 2026-09-06). The install also spares the + # agent the turn it used to spend rediscovering the hole. ruby(:smoke_baseline) do - result = VerifyRevision.run_one(:route_smoke, WORKSPACE) - failing = result[:checks].first&.dig(:failing_routes) || [] + bundle, fixes = AutoRemediate.ensure_bundle(WORKSPACE) + puts "[W2.B] #{fixes.join('; ')}" unless fixes.empty? + checks = bundle[:checks] + + failing = [] + if !VerifyRevision.failed?(bundle) || !fixes.empty? + smoke = VerifyRevision.run_one(:route_smoke, WORKSPACE) + checks += smoke[:checks] + failing = smoke[:checks].first&.dig(:failing_routes) || [] + puts "[W2.B] #{failing.empty? ? 'no pages failing at HEAD' : "already failing at HEAD: #{failing.join(', ')}"}" + else + puts "[W2.B] bundle still incomplete after remediation — baseline skipped, W2.4 will see every page" + end + WORKFLOW_STATE[:smoke_baseline] = failing - puts "[W2.B] #{failing.empty? ? 'no pages failing at HEAD' : "already failing at HEAD: #{failing.join(', ')}"}" - puts VerifyRevision.sentinel(result, stage: "W2.B") + puts VerifyRevision.sentinel(VerifyRevision.tally(checks), stage: "W2.B", applied: (fixes.empty? ? nil : fixes)) failing end diff --git a/lib/roast/verify_revision.rb b/lib/roast/verify_revision.rb index 71af9d9..963c08a 100644 --- a/lib/roast/verify_revision.rb +++ b/lib/roast/verify_revision.rb @@ -75,19 +75,20 @@ def self.run(workspace, known_failing_routes: []) end end - { - checks: results, - passed: results.select { |r| r[:passed] }, - failed: results.reject { |r| r[:passed] } - } + tally(results) end # One check, same result shape as run. Used by bin/verify-workspace --check and # by the W2.B baseline. def self.run_one(check, workspace, known_failing_routes: []) - result = perform(check, workspace, known_failing_routes: known_failing_routes) - checks = [ result ].compact - { checks: checks, passed: checks.select { |r| r[:passed] }, failed: checks.reject { |r| r[:passed] } } + tally([ perform(check, workspace, known_failing_routes: known_failing_routes) ].compact) + end + + # The result shape every caller reads: the checks that ran, split by outcome. + # W2.B needs it over a list it assembled itself (bundle_check from + # AutoRemediate.ensure_bundle, then route_smoke) rather than over one run. + def self.tally(checks) + { checks: checks, passed: checks.select { |c| c[:passed] }, failed: checks.reject { |c| c[:passed] } } end def self.failed?(result) diff --git a/test/integration/generate_todo_list_test.rb b/test/integration/generate_todo_list_test.rb index 92f0fae..5e93b8c 100644 --- a/test/integration/generate_todo_list_test.rb +++ b/test/integration/generate_todo_list_test.rb @@ -155,7 +155,9 @@ def assert_verify_metrics_persisted(project) "revision #{revision.position}: metrics carry no verify records — sentinel lost at the subprocess boundary? #{revision.metrics.inspect}" baseline, *runs = verify assert_equal "W2.B", baseline["stage"], "revision #{revision.position}: the first record must be the W2.B baseline" - assert_equal [ "route smoke" ], baseline["checks"].map { |c| c["name"] } + assert_equal [ "bundle check", "route smoke" ], baseline["checks"].map { |c| c["name"] }, + "the baseline ensures the bundle before it smokes" + assert_nil baseline["applied"], "the fixture adds no gem, so no install is needed" assert_not_empty runs, "revision #{revision.position}: no verification run after the baseline" assert_equal "W2.4", runs.first["stage"] runs.each do |run| diff --git a/test/lib/auto_remediate_test.rb b/test/lib/auto_remediate_test.rb index d2df738..f787242 100644 --- a/test/lib/auto_remediate_test.rb +++ b/test/lib/auto_remediate_test.rb @@ -60,6 +60,45 @@ class AutoRemediateTest < ActiveSupport::TestCase end end + # --- ensure_bundle: the W2.B pre-flight ------------------------------------- + # W2.B runs in a fresh container that never saw the `bundle install` an earlier + # revision's agent ran, so the lockfile can name gems this BUNDLE_PATH lacks. + # ensure_bundle applies the recipe below BEFORE the baseline smoke instead of + # after a failed W2.4. + + test "ensure_bundle passes the check through and shells nothing when the bundle is whole" do + with_run_one_stub(passed: true) do + with_shell_stub(->(_ws, _cmd) { flunk("shell must not run when bundle check passes") }) do + check, fixes = AutoRemediate.ensure_bundle("/tmp/ws") + assert_equal [], fixes + refute VerifyRevision.failed?(check) + end + end + end + + test "ensure_bundle runs the bundler recipe when bundle check reports missing gems" do + captured_cmd = nil + output = "The following gems are missing\n * rainbow (3.1.1)\nInstall missing gems with `bundle install`" + with_run_one_stub(passed: false, output: output) do + with_shell_stub(->(_ws, cmd) { captured_cmd = cmd; true }) do + check, fixes = AutoRemediate.ensure_bundle("/tmp/ws") + assert_equal [ "bundler missing gems: ran `bundle install`" ], fixes + assert_includes captured_cmd, "bundle install --jobs 4" + assert VerifyRevision.failed?(check), "the failing check is returned as-is, for the sentinel" + end + end + end + + test "ensure_bundle returns no fixes when the install itself fails, so W2.B can skip the smoke" do + with_run_one_stub(passed: false, output: "Bundler::GemNotFound") do + with_shell_stub(->(_ws, _cmd) { false }) do + check, fixes = AutoRemediate.ensure_bundle("/tmp/ws") + assert_equal [], fixes + assert VerifyRevision.failed?(check) + end + end + end + private def with_shell_stub(stub_proc) @@ -70,4 +109,17 @@ def with_shell_stub(stub_proc) AutoRemediate.singleton_class.alias_method(:shell, :__orig_shell) AutoRemediate.singleton_class.send(:remove_method, :__orig_shell) end + + # ensure_bundle asks VerifyRevision for the bundle_check result; the stub + # supplies one without shelling out to a real workspace. + def with_run_one_stub(passed:, output: "") + original = VerifyRevision.method(:run_one) + VerifyRevision.define_singleton_method(:run_one) do |check, _workspace, **| + result = { check: check, name: "bundle check", passed: passed, output: output, ms: 12 } + VerifyRevision.tally([ result ]) + end + yield + ensure + VerifyRevision.define_singleton_method(:run_one, original) if original + end end diff --git a/test/lib/revision_prompt_test.rb b/test/lib/revision_prompt_test.rb index b938a7a..a33eb69 100644 --- a/test/lib/revision_prompt_test.rb +++ b/test/lib/revision_prompt_test.rb @@ -80,6 +80,11 @@ class RevisionPromptTest < ActiveSupport::TestCase assert_includes out, "NOT jsbundling-rails/webpack/esbuild" end +test "anti-reflex: NOT RSpec/FactoryBot" do + out = build_minimal + assert_includes out, "NOT RSpec/FactoryBot" +end + test "documents the extra-gem escape hatch (Gemfile + bundle install + generator)" do out = build_minimal assert_includes out, "add it to Gemfile" @@ -174,6 +179,23 @@ class RevisionPromptTest < ActiveSupport::TestCase assert_includes out, "Added Todo." end +# ---- rules: the code agent must not follow a task that names spec/ ---- +# Production project 40: the plan named spec/ files, the agent said out loud +# that "Minitest, not RSpec" conflicted with the task, and followed the task. +# The rule now says which one wins and what to do instead. + +test "rules: a task naming spec/ is translated to test/, not followed" do + out = build_minimal + assert_includes out, "Tests are Minitest under `test/`, run with `bin/rails test`" + assert_includes out, "write the equivalent Minitest tests under `test/` instead" + assert_includes out, "the task's wording does not override this" +end + +test "rules: no test gem may be added" do + out = build_minimal + assert_includes out, "Do not add rspec-rails or any other test gem" +end + # ---- ordering: stack inventory precedes manifest precedes rules ---- test "section order: Task → Summary → Stack → (Manifest) → (Snapshot) → (Notes) → Rules" do diff --git a/test/lib/verify_revision_test.rb b/test/lib/verify_revision_test.rb index a8f3ce5..7423e04 100644 --- a/test/lib/verify_revision_test.rb +++ b/test/lib/verify_revision_test.rb @@ -100,6 +100,28 @@ class VerifyRevisionTest < ActiveSupport::TestCase end end + # --- tally ------------------------------------------------------------------- + # run, run_one and the W2.B baseline all return the same shape. W2.B assembles + # its list itself (bundle_check from AutoRemediate.ensure_bundle, then + # route_smoke), so the split lives here rather than inside either runner. + + test "tally splits a mixed list into passed and failed while keeping checks in order" do + checks = [ + { check: :bundle_check, passed: false }, + { check: :route_smoke, passed: true }, + { check: :rails_test, passed: false } + ] + result = VerifyRevision.tally(checks) + + assert_equal checks, result[:checks], "order is the order the checks ran in" + assert_equal [ :route_smoke ], result[:passed].map { |c| c[:check] } + assert_equal [ :bundle_check, :rails_test ], result[:failed].map { |c| c[:check] } + end + + test "tally of an empty list is the empty result run_one returns for a skipped check" do + assert_equal({ checks: [], passed: [], failed: [] }, VerifyRevision.tally([])) + end + # --- the two decisions the workflow rests on --------------------------------- test "blocking_failed? is false when only route_smoke (advisory) failed" do diff --git a/test/services/plan_application_creation/ad_hoc_llm_test.rb b/test/services/plan_application_creation/ad_hoc_llm_test.rb index 37478b9..886f8d8 100644 --- a/test/services/plan_application_creation/ad_hoc_llm_test.rb +++ b/test/services/plan_application_creation/ad_hoc_llm_test.rb @@ -53,7 +53,7 @@ def plan_fixture(name) # as "default Rails 8", never enumerated. test "system prompt never names Devise-class gems, hifumi design tokens, or the default stack's parts" do - refute_match(/devise|pundit|cancancan|sidekiq|--accent|--paper|--ink|propshaft|importmap|solid_/i, + refute_match(/devise|pundit|cancancan|sidekiq|rspec|spec\/|factory_bot|factorybot|simplecov|--accent|--paper|--ink|propshaft|importmap|solid_/i, PlanApplicationCreation::AdHocLLM::SYSTEM_PROMPT) end @@ -64,6 +64,14 @@ def plan_fixture(name) assert_includes prompt, "has_secure_password" end + test "system prompt pins tests to Minitest under test/ and closes the test stack" do + prompt = PlanApplicationCreation::AdHocLLM::SYSTEM_PROMPT + assert_includes prompt, "Minitest under `test/`" + assert_includes prompt, "bin/rails test" + assert_includes prompt, "test/models/_test.rb" + assert_includes prompt, "plan no additional testing gems" + end + test "passes the selected model through to the LLM" do with_llm_response(plan_fixture("valid_plan.json")) do |captured| PlanApplicationCreation::AdHocLLM.call(intent: "todo list", clarifications: {}, context: {}, openrouter_api_key: "sk-or-test", model: "anthropic/claude-opus-4.6") diff --git a/test/services/plan_application_modification/ad_hoc_llm_test.rb b/test/services/plan_application_modification/ad_hoc_llm_test.rb index 2ea14eb..c3b7ac6 100644 --- a/test/services/plan_application_modification/ad_hoc_llm_test.rb +++ b/test/services/plan_application_modification/ad_hoc_llm_test.rb @@ -53,7 +53,7 @@ def plan_fixture(name) # and the default stack is named as "default Rails 8", never enumerated. test "system prompt never names Devise-class gems, hifumi design tokens, or the default stack's parts" do - refute_match(/devise|pundit|cancancan|sidekiq|--accent|--paper|--ink|propshaft|importmap|solid_/i, + refute_match(/devise|pundit|cancancan|sidekiq|rspec|spec\/|factory_bot|factorybot|simplecov|--accent|--paper|--ink|propshaft|importmap|solid_/i, PlanApplicationModification::AdHocLLM::SYSTEM_PROMPT) end @@ -64,6 +64,14 @@ def plan_fixture(name) assert_includes prompt, "has_secure_password" end + test "system prompt pins tests to Minitest under test/ and closes the test stack" do + prompt = PlanApplicationModification::AdHocLLM::SYSTEM_PROMPT + assert_includes prompt, "Minitest under `test/`" + assert_includes prompt, "bin/rails test" + assert_includes prompt, "test/models/_test.rb" + assert_includes prompt, "plan no additional testing gems" + end + test "passes the selected model through to the LLM" do with_llm_response(plan_fixture("valid_plan.json")) do |captured| PlanApplicationModification::AdHocLLM.call(intent: "make banner green", clarifications: {}, context: {}, openrouter_api_key: "sk-or-test", model: "anthropic/claude-opus-4.6")