From 1cca8b25bb1f5f72deb5dbb625f1a1258b700f58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Mon, 7 Sep 2026 02:14:09 +0200 Subject: [PATCH 1/4] fix: planner prompts pin tests to Minitest under test/ MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production project 40 asked for "all business logic covered by automated tests" and got RSpec: spec/ files in every revision prompt, rspec-rails in the Gemfile, and 62 examples the verification step never ran (rails_test is gated on test/**/*_test.rb). The framework was decided in the plan — the code agent named the conflict with its own "Minitest, not RSpec" rule and followed the task's file paths anyway. Nothing told the planner. Five dry runs of the creation planner with that project's exact intent produced RSpec in 3; one Minitest plan invented SimpleCov. So the rule goes where the choice is made, in the positive-only register both prompts already use: Minitest under test/, the Rails default, with the file layout named and the test stack closed against extra gems. Measured after: 5/5 creation runs and 2/2 modification runs name test/ only, no spec/, no RSpec, no coverage tooling. The tests extend the never-names regex with rspec|spec/|factory_bot|simplecov so the framing cannot slip. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RJNJrvTLzXwvxpgDVXEnKp --- app/prompts/plan_application_creation_system.md | 1 + app/prompts/plan_application_modification_system.md | 1 + .../plan_application_creation/ad_hoc_llm_test.rb | 10 +++++++++- .../plan_application_modification/ad_hoc_llm_test.rb | 10 +++++++++- 4 files changed, 20 insertions(+), 2 deletions(-) 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/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") From 7443727233fd42fe96d52e98c69af7afe0a85026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Mon, 7 Sep 2026 02:19:07 +0200 Subject: [PATCH 2/4] fix: RevisionPrompt translates a spec/ task to Minitest instead of following it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Minitest, not RSpec" was one line in the rules, and it lost to a file path: in production project 40 the agent said out loud that the conventions and the task conflicted, then followed the task and wrote spec/. A rule that only states the preference gives no answer when the plan disagrees with it. Both places the prompt already uses for this class of problem now cover tests. The anti-reflex list names the reflex the way it names Devise and Sidekiq, and the rule says which side wins and what to do with the other: write the equivalent Minitest tests under test/, add no test gem. Verified in dev on project 37 with a hand-made revision whose prompt names spec/models/score_spec.rb: the commit carries test/models/score_test.rb (ActiveSupport::TestCase, both validations plus the zero boundary), no spec/ directory, no rspec in the Gemfile — and W2.4 ran rails test, which is the check project 40 never reached. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RJNJrvTLzXwvxpgDVXEnKp --- lib/roast/revision_prompt.rb | 3 ++- test/lib/revision_prompt_test.rb | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) 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/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 From 9986ea314aa06983e5959eb63c84a07515011357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Mon, 7 Sep 2026 02:39:28 +0200 Subject: [PATCH 3/4] feat: W2.B repairs the bundle before the route smoke baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every W2.B in production project 40 after revision 106 died in ~240 ms with Bundler::GemNotFound and failing_routes: [] — no per-route data, which is the one thing the baseline exists to produce. Each revision runs in a fresh `docker run --rm` that mounts the workspace and nothing else, so the `bundle install` an earlier revision's agent ran went into a container that no longer exists: the lockfile names rspec-rails, this BUNDLE_PATH does not have it, and W2.B is the first step to walk into the hole. It was blind exactly when a gem had been added, and the agent then spent a turn rediscovering the same thing, every revision. AutoRemediate.ensure_bundle runs `bundle check` and, on failure, the recipe W2.AR already applies after a failed W2.4 — the same install, moved before the baseline instead of after the damage. W2.B smokes only once the bundle is whole and says so in the sentinel when it is not; VerifyRevision.tally gives the three callers that build {checks, passed, failed} one place to do it. Verified in dev on a scratch copy of project 43 whose lockfile names an uninstalled gem: bundle check FAIL, ensure_bundle installs, both checks PASS afterwards with the smoke booting Rails (1 891 ms, 5 runs). E2E green at 652 s with both checks in every revision's baseline and no `applied`; the 29-workspace sweep is unchanged at 26/29. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RJNJrvTLzXwvxpgDVXEnKp --- lib/roast/auto_remediate.rb | 16 +++++++ lib/roast/revision_workflow.rb | 32 ++++++++++--- lib/roast/verify_revision.rb | 17 +++---- test/integration/generate_todo_list_test.rb | 4 +- test/lib/auto_remediate_test.rb | 52 +++++++++++++++++++++ test/lib/verify_revision_test.rb | 22 +++++++++ 6 files changed, 127 insertions(+), 16 deletions(-) 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_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/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 From bc250986da168e2d26e3faeca7a5bed4eca0875c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Mon, 7 Sep 2026 03:12:26 +0200 Subject: [PATCH 4/4] docs: record the Minitest planning rule and the W2.B bundle step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit W2.B in the canon now says it checks the bundle and installs what is missing before it smokes, with the reason (the sandbox container is throwaway, so an earlier revision's gems are in the lockfile and nowhere else). The W2.4 as-built note gains the project-40 finding that motivated the planner change: the test framework is decided in the plan, and the code agent follows a task's file paths over its own rules. Follow-ups records the three things this work deliberately did not build: the per-project gem volume (with its two open questions — lifecycle on project deletion, behaviour across image upgrades), the mechanical Minitest guard the measurement did not justify, and the fact that a revision writing no tests leaves no trace in metrics["verify"] at all. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01RJNJrvTLzXwvxpgDVXEnKp --- CLAUDE.md | 4 ++-- docs/02-architecture/01-workflows-and-decisions.md | 13 +++++++++---- docs/09-ideas/05-followups.md | 8 ++++++++ 3 files changed, 19 insertions(+), 6 deletions(-) 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/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"]`.