diff --git a/CHANGELOG.md b/CHANGELOG.md index 138f570..013b725 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,46 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); versions follow [semantic versioning](https://semver.org/) (minor for new functionality, patch for fixes and internal changes). +## [1.5.0] - 2026-09-03 + +### Added + +- Change requests are now planned against the application as it actually is. + Before asking the model for a plan, hifumi.dev reads the project's workspace + — the gems installed beyond the Rails defaults, the database tables and + their columns, the routes file, the list of source files under `app/`, and + the project's own `docs/` — and hands that to the planner alongside the + request. Plans name the real files, columns, routes and colours instead of + guessing: "let people delete an entry" no longer invents an authorization + step for an app that has no users, and a styling tweak names the template's + actual hex value rather than a CSS variable the app does not have. The first + build of a project is unaffected; there is no application to read at that + point. The snapshot travels with every change request's planning call — a + few kilobytes for a fresh app, up to ~35 KB for the largest existing one — so + plans cost a little more on a bring-your-own-key account. +- Three maintainer scripts for plans, none of which persists anything. + `bin/inspect-plan-application-modification ""` dry-runs + the change planner and prints the workspace snapshot and the plan it + produces; `--blind` shows what the planner would have done without the + snapshot. `bin/inspect-plan-application-creation` dry-runs the first-build + planner and works again — the RubyLLM 2.0 upgrade had broken it. + `bin/inspect-plans ` reads back every plan already stored for a + project without calling a model. + +### Changed + +- Both planner prompts stopped asserting things that were never true of a + generated app: that Devise is installed (it is not — sign-in is planned with + Rails' own `has_secure_password` and sessions) and that hifumi.dev's own + design tokens such as `--accent` exist in the generated app. Plans that used + to hedge ("if it uses a CSS class…", "assuming a User model exists") are told + not to, now that they can see the answer. +- The documentation agent that runs after every build step is now asked to + keep each of the four `docs/` files under 8 000 characters, condensing stale + sections instead of appending to them. The planner reads each file up to + that length and cuts it there, per file — an oversized file loses its tail, + never a sibling file. + ## [1.4.0] - 2026-08-24 ### Changed diff --git a/CLAUDE.md b/CLAUDE.md index 727e43e..3db86f2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,7 +16,7 @@ Hosted at **[hifumi.dev](https://hifumi.dev)** · Source: this repo. Deferred observations from Phase 2 (revisit later, not blockers): - refused-tool-call pill UX (Step 6) — the `🌀 Build started` pill flashing when the LLM ignores the state rule. Nothing rescues it any more: the tool-side guard was deleted in `13e9c1c`, leaving the prompt rule and the `BadRequestError` banner (see the RubyLLM idempotency bullet below). - deferred-request handling after `✅ Generation finished.` — see `docs/09-ideas/02-deferred-request-handling.md`. -- Step 7 wall-time margin (Step 7) — real run consumed ~900s vs the spike's 496s; the integration test's `WALL_TIME_BUDGET = 900` sits right at the edge. Bump the budget or investigate W2-phase slowdown (looks heavy on the docs-update agent) before relying on this in CI. +- Step 7 wall-time margin (Step 7) — real runs: 496s (spike), ~900s (May, before the per-revision hardening), 550s (2026-09-03, the run that re-validated the repaired E2E test). `WALL_TIME_BUDGET` is 1200 so a slow day is not a red; the budget assertion runs before the generated app's own suite. W2-phase (docs-update agent) is still the heaviest step if the number climbs again. ## Documentation structure @@ -57,4 +57,5 @@ 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, bundle vendoring) 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. - **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/README.md b/README.md index 65baff4..52928e3 100644 --- a/README.md +++ b/README.md @@ -181,6 +181,18 @@ kamal app exec --primary "bin/rails runner bin/inspect-chat 15" # against pr Dumps a project's chat messages and `tool_calls` rows in order, then runs a structural pairing analysis (every `tool_result` must follow its matching assistant `tool_use`). Use when the chat fails with `RubyLLM::BadRequestError` ("unexpected `tool_use_id`...") — the dump tells you whether a tool was called twice in one turn or a tool_result is orphaned. +### Inspect stored and dry-run plans — `bin/inspect-plans`, `bin/inspect-plan-application-*` + +```sh +bin/inspect-plans 42 # every plan stored for project 42 (DB only, no model call) +bin/inspect-plan-application-modification 42 "let people delete an entry" # dry-run the change planner with the workspace snapshot +bin/inspect-plan-application-modification 42 "..." --blind # same intent without the snapshot — the A/B +bin/inspect-plan-application-creation "a flower shop with inventory" # dry-run the first-build planner +kamal app exec --primary "bin/inspect-plans 42" # any of them against prod +``` + +`bin/inspect-plans` reads the database only. The two `inspect-plan-application-*` scripts call the planner LLM with the project owner's (or your profile's) OpenRouter key and persist nothing — run them before shipping any planner-prompt or `AppState` change; the "Modification planner context" convention in `CLAUDE.md` says what to look for. + ### Run the test suite ```sh diff --git a/app/jobs/execute_instruction_job.rb b/app/jobs/execute_instruction_job.rb index 02437ae..26815fb 100644 --- a/app/jobs/execute_instruction_job.rb +++ b/app/jobs/execute_instruction_job.rb @@ -187,9 +187,9 @@ def init_docs_baseline(workspace) docs_dir = File.join(workspace, "docs") FileUtils.mkdir_p(docs_dir) { - "architecture.md" => "# Architecture\n\n(empty — will be filled in by the first revision)\n", - "conventions.md" => "# Conventions\n\n(empty — will be filled in by the first revision)\n", - "domain.md" => "# Domain\n\n(empty — will be filled in by the first revision)\n", + "architecture.md" => "# Architecture\n\n#{AppState::PLACEHOLDER}\n", + "conventions.md" => "# Conventions\n\n#{AppState::PLACEHOLDER}\n", + "domain.md" => "# Domain\n\n#{AppState::PLACEHOLDER}\n", "revision_notes.md" => "# Revision notes\n\n" }.each { |name, content| File.write(File.join(docs_dir, name), content) } diff --git a/app/prompts/plan_application_creation_system.md b/app/prompts/plan_application_creation_system.md index f8c1b20..e97d3dd 100644 --- a/app/prompts/plan_application_creation_system.md +++ b/app/prompts/plan_application_creation_system.md @@ -3,7 +3,7 @@ You are a Rails application planner. Given a user's plain-language intent, emit 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"). -- Assume the workspace is an already-initialized Rails 8 app with Tailwind + Hotwire + Devise gems available. Do NOT include `rails new` or gem installation steps. +- 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. - 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 d1ee757..94444e0 100644 --- a/app/prompts/plan_application_modification_system.md +++ b/app/prompts/plan_application_modification_system.md @@ -1,4 +1,4 @@ -You are a Rails application planner. The application already exists in the workspace — Rails 8 is installed, gems are bundled, and previous revisions have shaped the schema, routes, views, and Tailwind theme. +You are a Rails application planner. The application already exists. The user turn carries a "Current application state" section describing it: installed gems, database tables, routes, the list of files under `app/`, and the project's own `docs/`. Read it before planning and ground every file path, table, route and colour in what it actually says. Your job: given a user's plain-language change request, emit a short plan of one or more atomic revisions matching the required JSON schema. @@ -8,7 +8,10 @@ Rules for the plan: - DO NOT change the root route unless the user explicitly asks for it. - 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. -- Assume Tailwind, Hotwire, Devise, and the previously picked template's design tokens are already wired. Reference existing CSS variables (e.g. `--accent`, `--paper-100`) when applicable rather than introducing new ones. +- 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. +- 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. - Never reference "Claude", "Anthropic", or any LLM provider unless the user explicitly asks for that integration. - Each revision's `prompt` is the full instruction passed to the implementer agent — concrete, file-level, verifiable. Mention specific files (e.g. "in `app/views/layouts/application.html.erb`, change …"). - Each revision's `summary` is a git-commit-style one-liner. diff --git a/app/services/plan_application_modification/ad_hoc_llm.rb b/app/services/plan_application_modification/ad_hoc_llm.rb index 611dcd6..9d016ac 100644 --- a/app/services/plan_application_modification/ad_hoc_llm.rb +++ b/app/services/plan_application_modification/ad_hoc_llm.rb @@ -4,6 +4,13 @@ module AdHocLLM class InvalidResponse < StandardError; end + # Appended when there is no snapshot to give (a --blind probe, or a workspace + # gone between the tool being bound and the call). The system prompt tells + # the planner the state is given and forbids hedging; this is the one place + # that claim is false, so say so where the model reads it. + NO_STATE_NOTE = "No application state snapshot is available for this request. " \ + "Say what you assume about existing files, tables and colours instead of asserting it." + def self.call(intent:, clarifications:, context:, openrouter_api_key:, model:) user_prompt = build_user_prompt(intent, clarifications, context) content = invoke_llm(system: SYSTEM_PROMPT, user: user_prompt, openrouter_api_key: openrouter_api_key, model: model) @@ -17,12 +24,24 @@ def self.invoke_llm(system:, user:, openrouter_api_key:, model:) chat.with_schema(PlanSchema).ask(user).parsed end - def self.build_user_prompt(intent, clarifications, _context) + # The first deliberate divergence from the PlanApplicationCreation twin, + # which cannot have workspace context: the workspace does not exist when + # it runs (CreateApplication persists the plan before ExecuteInstructionJob + # runs `rails new`). + def self.build_user_prompt(intent, clarifications, context) lines = [ "Intent: #{intent}" ] if clarifications.present? lines << "Clarifications:" clarifications.each { |k, v| lines << " - #{k}: #{v}" } end + # Last, matching RevisionPrompt.build, which leads with "## Task" and only + # then appends the stack inventory, the docs manifest and the workspace + # snapshot: request first, reference material after. Keeps the ask from + # being buried behind up to ~35 KB of listing (project_39, the largest). (There is no schema text in + # this turn to sit next to — with_schema ships as OpenRouter's + # `response_format` payload field.) + app_state = context.is_a?(Hash) ? context[:app_state] : nil + lines << "\n#{app_state.presence || NO_STATE_NOTE}" lines.join("\n") end diff --git a/app/tools/modify_application.rb b/app/tools/modify_application.rb index 8e3e8b4..bfb185f 100644 --- a/app/tools/modify_application.rb +++ b/app/tools/modify_application.rb @@ -28,7 +28,11 @@ def execute(intent:, clarifications: {}) result = PlanApplicationModification.call( intent: intent, clarifications: clarifications, - context: { project_id: @project.id }, + # Built here rather than in the planner so AdHocLLM stays a pure function + # of its arguments, and so the file reads sit inside the rescue below — + # ENOENT/EACCES on a workspace file degrades to a chat-safe error hash + # instead of orphaning the tool_use. + context: { app_state: AppState.build(workspace: @project.workspace_path) }, openrouter_api_key: @project.user.profile.openrouter_api_key, model: @project.plan_modification_model ) diff --git a/bin/inspect-plan-application-creation b/bin/inspect-plan-application-creation index c9eaf14..177f076 100755 --- a/bin/inspect-plan-application-creation +++ b/bin/inspect-plan-application-creation @@ -1,50 +1,87 @@ #!/usr/bin/env ruby -# Invoke PlanApplicationCreation against one or more intents and dump the -# resulting revisions, without persisting anything or running the W2 implementer. +# frozen_string_literal: true + +# +# Maintainer/debug helper — not part of normal app flow. +# +# Dry-run the creation planner (PlanApplicationCreation) against one or more +# intents and dump the resulting revisions. Persists nothing and runs no +# implementer: this is the planner call `create_application` makes, minus the +# Instruction/Revision rows and the `instruction.requested` event. +# +# Use it to verify changes to app/prompts/plan_application_creation_system.md +# or to PlanApplicationCreation::AdHocLLM without paying the full ~900s W2 +# generation cost (planner alone is a few cents per intent). # -# Use this to verify changes to app/prompts/plan_application_creation_system.md -# or to PlanApplicationCreation::AdHocLLM by inspecting what the planner emits, -# without paying the full ~900s W2 generation cost (planner alone is haiku, a few cents). +# The creation planner has no workspace to look at — CreateApplication persists +# the plan before ExecuteInstructionJob runs `rails new` — so unlike the +# modification probe there is no snapshot to print and no --blind mode. # -# Usage (locally): -# bin/rails runner bin/inspect-plan-application-creation "your intent here" ["another intent" ...] +# Usage (locally): bin/inspect-plan-application-creation ["" ...] [--model ] +# Usage (on Kamal): kamal app exec --primary "bin/inspect-plan-application-creation ''" +# +# --model OpenRouter model id; default is the plan_creation stage's +# factory default (LLM::Stages) +# --help print usage and exit without an LLM call # # Examples: -# bin/rails runner bin/inspect-plan-application-creation "a todo list app" -# bin/rails runner bin/inspect-plan-application-creation \ +# bin/inspect-plan-application-creation "a todo list app" +# bin/inspect-plan-application-creation \ # "a todo list app" \ # "a personal finance tracker with expenses and budgets and a dashboard" +# bin/inspect-plan-application-creation "a blog with authors who sign in" --model anthropic/claude-sonnet-4.6 # # Defaults: when no intents are given, runs a small built-in suite covering # single-feature, multi-feature, and "user-overrides-default-routing" cases. # Reads the OpenRouter API key from the first Profile that has one set -# (typically your own dev user). Production: not intended; this is a dev tool. - -require "ruby_llm/schema" - -intents = - if ARGV.any? - ARGV - else - [ - "a todo list app", - "a personal finance tracker with expenses, budgets, and a dashboard", - "a private admin dashboard mounted at /admin for managing users" - ] - end +# (typically your own dev user). +# +# Related: bin/inspect-plan-application-modification "" — the same for the modification planner +# bin/inspect-plans — plans already stored for a project +# bin/inspect-chat — the conversation that led there + +require_relative "../config/environment" +require "optparse" + +USAGE = "usage: bin/inspect-plan-application-creation [\"\" ...] [--model ]" + +options = { model: LLM::Stages.find(:plan_creation).default_model } +parser = OptionParser.new do |o| + o.banner = USAGE + o.on("--model ID", "OpenRouter model id (default: #{options[:model]}, the plan_creation stage default)") { |v| options[:model] = v } +end +intents = parser.parse(ARGV) + +if intents.empty? + intents = [ + "a todo list app", + "a personal finance tracker with expenses, budgets, and a dashboard", + "a private admin dashboard mounted at /admin for managing users" + ] +end profile = Profile.where.not(openrouter_api_key: nil).first abort "no Profile with an openrouter_api_key found in this DB" if profile.nil? key = profile.openrouter_api_key +RULE = "=" * 80 + +puts RULE +puts "model: #{options[:model]}" +puts RULE + intents.each do |intent| - puts "=" * 80 + puts + puts RULE puts "INTENT: #{intent}" - puts "=" * 80 + puts RULE - result = PlanApplicationCreation.call(intent: intent, openrouter_api_key: key) + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + result = PlanApplicationCreation.call(intent: intent, openrouter_api_key: key, model: options[:model]) + elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started puts "instruction_description: #{result.instruction_description}" + puts "revisions: #{result.revisions.size} · planner time: #{elapsed.round(1)}s" puts result.revisions.each_with_index do |r, i| puts "--- Revision #{i + 1}: #{r[:summary]} ---" diff --git a/bin/inspect-plan-application-modification b/bin/inspect-plan-application-modification new file mode 100755 index 0000000..5896004 --- /dev/null +++ b/bin/inspect-plan-application-modification @@ -0,0 +1,112 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# +# Maintainer/debug helper — not part of normal app flow. +# +# Dry-run the modification planner (PlanApplicationModification) against a real +# project's workspace and dump the resulting revisions. Persists nothing and +# runs no implementer: this is the planner call `modify_application` makes, +# minus the Instruction/Revision rows and the `instruction.requested` event. +# +# Use it to verify changes to app/prompts/plan_application_modification_system.md, +# to PlanApplicationModification::AdHocLLM, or to AppState (the workspace +# snapshot the planner is fed). The snapshot is printed first, with its size, +# so a prompt change can be inspected next to the plan it produced. +# +# Usage (locally): bin/inspect-plan-application-modification "" ["" ...] [--model ] [--blind] +# Usage (on Kamal): kamal app exec --primary "bin/inspect-plan-application-modification ''" +# +# --model OpenRouter model id; default is the project's plan_modification_model +# --blind omit the workspace snapshot, i.e. plan from the intent alone — +# a same-session A/B isolating what the snapshot contributes under +# the current prompt (not the pre-AppState prompt, which changed too) +# +# Examples: +# bin/inspect-plan-application-modification 42 "let people delete a standup entry" +# bin/inspect-plan-application-modification 42 "let people delete a standup entry" --blind +# bin/inspect-plan-application-modification 39 "add a note field to time entries" --model anthropic/claude-sonnet-4.6 +# +# Reads the OpenRouter key from the project owner's Profile, falling back to the +# first Profile that has one (typically your own dev user). Planner alone is a +# few cents per intent; the W2 implementer is never run. +# +# Related: bin/inspect-plan-application-creation — the same for the creation planner +# bin/inspect-plans — plans already stored for a project +# bin/inspect-chat — the conversation that led there + +require_relative "../config/environment" +require "optparse" + +USAGE = "usage: bin/inspect-plan-application-modification \"\" [\"\" ...] [--model ] [--blind]" + +options = { model: nil, blind: false } +parser = OptionParser.new do |o| + o.banner = USAGE + o.on("--model ID", "OpenRouter model id (default: the project's plan_modification_model)") { |v| options[:model] = v } + o.on("--blind", "omit the workspace snapshot (plan from the intent alone)") { options[:blind] = true } +end +args = parser.parse(ARGV) + +project_id = args.shift || abort(parser.help) +intents = args +abort(parser.help) if intents.empty? + +project = begin + Project.find(project_id) +rescue ActiveRecord::RecordNotFound + abort("no project with id #{project_id.inspect}") +end + +model = options[:model] || project.plan_modification_model +key = project.user.profile.openrouter_api_key.presence || + Profile.where.not(openrouter_api_key: nil).first&.openrouter_api_key +abort("no Profile with an openrouter_api_key found in this DB") if key.nil? + +app_state = options[:blind] ? nil : AppState.build(workspace: project.workspace_path) + +RULE = "=" * 80 + +puts RULE +puts "Project ##{project.id} — #{project.name}" +puts "workspace: #{project.workspace_path}" +puts "model: #{model}#{options[:model] ? ' (--model)' : ' (project.plan_modification_model)'}" +puts "mode: #{options[:blind] ? 'BLIND — no workspace snapshot' : 'with AppState snapshot'}" +puts RULE + +if options[:blind] + puts "\n(blind: the planner sees only the intent)" +elsif app_state.nil? + puts "\n(workspace not initialized — AppState.build returned nil, the planner sees only the intent)" +else + puts "\nAPP STATE PAYLOAD (#{app_state.bytesize} bytes, #{app_state.length} chars, ~#{app_state.length / 4} tokens)" + puts "-" * 80 + puts app_state + puts "-" * 80 +end + +intents.each do |intent| + puts + puts RULE + puts "INTENT: #{intent}" + puts RULE + + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + result = PlanApplicationModification.call( + intent: intent, + clarifications: {}, + context: { app_state: app_state }, + openrouter_api_key: key, + model: model + ) + elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started + + puts "instruction_description: #{result.instruction_description}" + puts "revisions: #{result.revisions.size} · planner time: #{elapsed.round(1)}s" + puts + result.revisions.each_with_index do |r, i| + puts "--- Revision #{i + 1}: #{r[:summary]} ---" + puts r[:prompt] + puts + end +end diff --git a/bin/inspect-plans b/bin/inspect-plans new file mode 100755 index 0000000..8e7b84c --- /dev/null +++ b/bin/inspect-plans @@ -0,0 +1,103 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# +# Maintainer/debug helper — not part of normal app flow. +# +# Dump every implementation plan stored for a project: the initial one +# (written by the `create_application` tool) and each later modification plan +# (written by `modify_application`). A "plan" is not a document — it is one +# Instruction row plus its ordered Revision rows, so this script is the only +# way to read a finished plan back: the build tab renders revisions only while +# the instruction is still running. +# +# Instructions carry no column recording which tool created them, so the +# oldest instruction is labelled INITIAL and the rest MODIFICATION. That +# matches how the tools are bound (CreateApplication is offered only while +# the workspace is empty — see GeneratorAgent#tools). +# +# Usage (locally): bin/inspect-plans [--brief] +# Usage (on Kamal): kamal app exec --primary "bin/inspect-plans " +# +# --brief omit each revision's implementer prompt (summaries only) +# +# Related: bin/inspect-chat — the conversation that led here +# bin/watch-instruction [id] — live status of a running build +# bin/inspect-plan-application-creation — dry-run the creation planner, persist nothing +# bin/inspect-plan-application-modification — dry-run the modification planner against a project's workspace + +require_relative "../config/environment" + +args = ARGV.dup +brief = args.delete("--brief") +project_id = args.shift || abort("usage: bin/inspect-plans [--brief]") +abort("usage: bin/inspect-plans [--brief]") if args.any? + +project = begin + Project.find(project_id) +rescue ActiveRecord::RecordNotFound + abort("no project with id #{project_id.inspect}") +end + +RULE = "=" * 80 + +def stamp(time) + time ? time.utc.strftime("%Y-%m-%d %H:%M UTC") : "—" +end + +def duration(revision) + return "—" unless revision.started_at + + seconds = ((revision.finished_at || Time.current) - revision.started_at).to_i + format("%dm%02ds", seconds / 60, seconds % 60) +end + +def indent(text) + text.to_s.lines.map { |line| " #{line}" }.join.chomp +end + +puts RULE +puts "Project ##{project.id} — #{project.name}" +puts "owner: #{project.user.email} · workspace: #{project.workspace_path}" +puts "plan models (current selection, not necessarily what ran below):" +puts " creation: #{project.plan_creation_model} · modification: #{project.plan_modification_model}" +puts RULE + +instructions = project.instructions.includes(:revisions).sort_by { |i| [ i.created_at, i.id ] } + +if instructions.empty? + puts "\nNo plans yet — nothing has been generated for this project." + exit +end + +instructions.each_with_index do |instruction, index| + revisions = instruction.revisions.sort_by(&:position) + completed = revisions.count(&:completed?) + label = index.zero? ? "INITIAL PLAN (create_application)" : "MODIFICATION #{index} (modify_application)" + + puts + puts "#{label} · instruction ##{instruction.id}" + puts "-" * 80 + puts "phase: #{instruction.phase}" + puts "created: #{stamp(instruction.created_at)}" + puts "revisions: #{revisions.size} (#{completed} completed)" + puts "intent: #{instruction.user_intent.presence || '(none recorded)'}" + puts "plan: #{instruction.description}" + + revisions.each do |revision| + # In full mode each revision is a block of prose — separate them. In brief + # mode the summaries read better as a contiguous list. + puts unless brief + puts format(" R-%03d [%-10s] %-7s %7s %s", + revision.position, + revision.status, + revision.git_sha.to_s[0, 7].presence || "—", + duration(revision), + revision.summary) + puts indent(revision.prompt) unless brief + end +end + +puts +puts RULE +puts "chat that produced these plans: bin/inspect-chat #{project.id}" diff --git a/config/brakeman.ignore b/config/brakeman.ignore index 9c4f220..d88ef34 100644 --- a/config/brakeman.ignore +++ b/config/brakeman.ignore @@ -32,19 +32,8 @@ "line": 67, "link": "https://brakemanscanner.org/docs/warning_types/command_injection/", "note": "name is the hardcoded gem name from perform (\"herb\"); workspace is the generator-built path. No user input reaches the interpolation." - }, - { - "warning_type": "Command Injection", - "warning_code": 14, - "fingerprint": "fd8aa5dfff7e36ecbd77835204203e6caefea562c8e753733396a7bf945c456e", - "check_name": "Execute", - "message": "Possible command injection", - "file": "app/jobs/export_to_github_job.rb", - "line": 78, - "link": "https://brakemanscanner.org/docs/warning_types/command_injection/", - "note": "Array-form Open3.capture3 — no shell is involved, so interpolated values are plain argv entries and cannot inject commands. The token-in-argv trade-off is documented at the call site." } ], - "updated": "2026-06-12", - "brakeman_version": "8.0.4" + "updated": "2026-09-03", + "brakeman_version": "8.0.6" } diff --git a/docs/01-vision/02-user-journey.md b/docs/01-vision/02-user-journey.md index bd9fdb3..436103f 100644 --- a/docs/01-vision/02-user-journey.md +++ b/docs/01-vision/02-user-journey.md @@ -193,7 +193,7 @@ Instructions executed by defined workflows (see `../02-architecture/01-workflows | Step | Type | What happens | |------|------|--------------| -| W4.1 | deterministic | Load app manifest | +| W4.1 | deterministic | Load app state (Gemfile, schema, routes, `app/` files, `docs/`) | | W4.2 | LLM (decision D3) | Research — manifest is enough / look for new solutions / read code | | W4.3 | LLM | Generate plan | | W4.4 | loop → W2 | Execute revisions | diff --git a/docs/02-architecture/01-workflows-and-decisions.md b/docs/02-architecture/01-workflows-and-decisions.md index de65900..821b726 100644 --- a/docs/02-architecture/01-workflows-and-decisions.md +++ b/docs/02-architecture/01-workflows-and-decisions.md @@ -158,7 +158,7 @@ W3 is **fully deterministic**. No LLM decisions. Triggered by: tool call `StartGeneration(intent, clarifications)` in the context of an existing project. Analogous to W1: revisions are already in the DB (generated by `CreatePlan`), W4 iterates over them. ``` -W4.1 [deterministic] Load app manifest (docs/) ← delegated to CreatePlan in Phase 2 +W4.1 [deterministic] Load app state (Gemfile, schema, routes, app/, docs/) ← built 2026-09-03: AppState.build, assembled by ModifyApplication W4.2 [LLM] Research (if needed) ← delegated to CreatePlan in Phase 2 → Decision D3: are manifest + user request enough for a plan? [a] simple (add field, change color) → skip, proceed to W4.3 @@ -170,7 +170,9 @@ W4.5 [deterministic] Mark Instruction as completed W4.6 [deterministic] Trigger W3 (restart preview) ``` -Same principle as W1: W4.1-W4.3 live inside the `CreatePlan` service. Decisions D1/D3 live in `CreatePlan::AdHocLLM`'s prompt engineering, not in the chat LLM. +Same principle as W1 for W4.2-W4.3: they live inside the `PlanApplicationModification` service (the W4 counterpart of `PlanApplicationCreation`, which implements W1's `CreatePlan`), and D3 lives in its prompt engineering, not in the chat LLM. + +W4.1 is different — it is deterministic code, built 2026-09-03. The `ModifyApplication` tool calls `AppState.build` (`lib/app_state.rb`), which reads the workspace fresh: gems beyond the default Gemfile, database tables (or the migrations on disk when `db/schema.rb` has not been written yet), `config/routes.rb` verbatim, every source file under `app/`, and the four `docs/` files each capped at 8 000 characters. The text travels as `context[:app_state]` and `PlanApplicationModification::AdHocLLM` renders it after the intent in the user turn, under a preamble that ranks the lists above the `docs/` prose on any conflict. The creation planner (W1) gets no such snapshot, by ordering rather than choice: `CreateApplication` persists the plan before `ExecuteInstructionJob` runs `rails new`, so at W1 planning time there is no workspace to describe. ### W5: Undo @@ -213,6 +215,8 @@ Every place where the LLM makes a decision, explicitly described. | D5 | Chat | — | What to suggest to the user? | Agent generates suggested prompts | | D6 | Chat | — | How to react to failure? | Agent decides: change approach / ask the user (has verification errors in context) | +**D3, as built (2026-09-03)**: only [a] is implemented — the W4.1 snapshot is the planner's whole research, prefed in a single turn. [c] was measured on `project_42` with a file-reading tool and the same model: matching plan quality at 3–8× the latency (14–29 s against 3.6–4.3 s, 11–15 tool calls per plan), and it would give a model-steered file-read tool to a planner running inside the generator container, outside `Roast::Sandbox`. [a] reads agent-written files there too, but as a fixed read of five known paths — realpath-contained to the workspace, encoding-scrubbed, bodies fenced, `app/` as paths only — with no tool for the model to point anywhere else. Deferred rather than rejected — worth revisiting only if plans start failing for want of code the snapshot does not carry. [b] is not built. + --- ## Agent — the only unstructured element diff --git a/docs/04-reviews/01-post-launch-review.md b/docs/04-reviews/01-post-launch-review.md index 5b4650a..673eb3c 100644 --- a/docs/04-reviews/01-post-launch-review.md +++ b/docs/04-reviews/01-post-launch-review.md @@ -49,7 +49,7 @@ Good: MIT LICENSE, contributor-facing README (mission, pipeline diagram, CLI ent Missing for a credible "learn how to build a Rails generator" repo: - `CONTRIBUTING.md` (only an inline README section exists), `CODE_OF_CONDUCT.md`, GitHub issue/PR templates. -- The gated E2E generator test is broken (matches a fixture instead of the created instruction — see `docs/09-ideas/05-followups.md` 2026-05-14). It is the only end-to-end safety net; any "follow along" reader who runs it gets a false signal. +- The gated E2E generator test is broken (matches a fixture instead of the created instruction). It is the only end-to-end safety net; any "follow along" reader who runs it gets a false signal. *Has since been actioned (2026-09-03): the test scopes to the signed-in user's project, signs in as Phase 4 requires, and ran green in 550s against a 1200s budget.* - Minor sanitization: local `/Users/pawel/...` paths in `spikes/roast/tmp/` logs. ## Candidate directions discussed (decision deferred) @@ -57,7 +57,7 @@ Missing for a credible "learn how to build a Rails generator" repo: Recorded so the next planning session doesn't start from zero. Four tracks, in tension: 1. **Harden for real multi-tenant users** — agent workspace isolation (done — `Roast::Sandbox`) + prompt-intake moderation as a proper Phase 5. -2. **Repo as teaching artifact** — architecture walkthrough docs, CONTRIBUTING, annotated reading paths, good first issues, fix the E2E test. +2. **Repo as teaching artifact** — architecture walkthrough docs, CONTRIBUTING, annotated reading paths, good first issues. (The E2E test fix listed here originally was done 2026-09-03.) 3. **External content** — articles / talks built on this codebase (Rails World angle: new builders Rails could gain). 4. **In-product education** — the `01-git-integration.md` ideas (diff view, annotated commits, "explain this change"). diff --git a/docs/09-ideas/04-prompt-injection-security.md b/docs/09-ideas/04-prompt-injection-security.md index 08658aa..b2ff613 100644 --- a/docs/09-ideas/04-prompt-injection-security.md +++ b/docs/09-ideas/04-prompt-injection-security.md @@ -6,6 +6,18 @@ is fed into `agent(:generate_code)` along with the workspace snapshot, the `docs/` files, and a static rules block. The agent has Edit / Read / Write / Bash tools available inside a generator container. +Since 2026-09-03 there is a second, indirect channel. The modification +planner (`PlanApplicationModification::AdHocLLM`) reads the workspace +through `AppState.build` — `config/routes.rb` and the four `docs/*.md` +verbatim, all writable by the codegen agent on a prior revision — and its +output *becomes* the next `revision_prompt`. So text planted in a doc by +one revision reaches the code agent's prompt on the next. Mitigations in +place: the planner has no tools and emits a fixed JSON schema; the `app/` +section lists paths only, never contents; every verbatim body is fenced; +reads are realpath-contained to the workspace and encoding-scrubbed +(`AppState.read_workspace_file`). Not in place: any moderation of what +the docs agent writes. + Two distinct concerns surfaced 2026-05-01 while reviewing `tmp/blog_application_run_kamal.log` and considering whether to add `--bare` to `agent(:generate_code)` (the codegen agent currently runs diff --git a/docs/09-ideas/05-followups.md b/docs/09-ideas/05-followups.md index dd632ac..3b7c0d3 100644 --- a/docs/09-ideas/05-followups.md +++ b/docs/09-ideas/05-followups.md @@ -8,22 +8,6 @@ Date a section header when adding entries so future-you can see the chronology. ## 2026-05-14 -### Fix the broken E2E generator test - -**File**: `test/integration/generate_todo_list_test.rb` - -**Symptom (seen during the `update_docs` prompt-cap plan's manual verification)**: `E2E_GENERATE=1 bin/rails test test/integration/generate_todo_list_test.rb` finishes in 0.11s instead of the expected ~900s, fails with `Expected # to be completed?`. The `user_intent` on the failing record is `"build a flower shop with inventory"` — that exact string lives at `test/fixtures/instructions.yml:6`, so `Instruction.order(:id).last` is matching the loaded fixture rather than an instruction newly-created by the test's POST. - -**Why it matters**: this is the only end-to-end safety net for the W1+W2 pipeline. Right now any change to `RevisionPrompt`, `StatCap`, `ExecuteInstructionJob`, etc. has to be validated by hand because the test gives a false signal. The W2.1 prompt hardening shipped 2026-05-14 (`739c844`) is unverified at the agent-behavior level for the same reason. - -**Likely shape of the fix** (need to trace before committing): -- The test stub redefines `Chat#complete` to call `CreateApplication.execute(...)`. Either that path isn't reached (so no instruction is created), or it IS reached and creates an instruction, but `.order(:id).last` is still picking up the fixture because the fixture's autoincrement ID happens to outrank the new row's ID. -- First-pass fix candidates: scope by `description` (e.g. `project.instructions.where(user_intent: PROMPT).last`), or assert on `instruction.reload.completed?` only after waiting for a phase transition, or — most robustly — load fewer fixtures in this test class (`fixtures :none` or similar) since this is a full-pipeline test that shouldn't be sharing data with controller-level tests. - -**Cost gate**: a real run is ~$5-$10 of Claude tokens + ~8 minutes wall-time. Validate the fix on a fast-failing variant first (e.g. let the test create the project, assert on `project.instructions.count == 1` before any LLM call) before paying for the full pipeline. - ---- - ### Auto-surface preview errors back into the chat **Motivation**: project 27 (Event RSVP, hifumi.dev) crashed at preview-time with `NoMethodError: undefined method 'authenticate_user!' for an instance of EventsController` because the agent reached for Devise without it being in the Gemfile. The W2.1 prompt hardening (`739c844`) reduces but doesn't eliminate this class of bug. The current recovery flow is "user notices the error in the preview iframe, copy/pastes it into chat, asks the agent to fix it" — but the preview iframe is cross-origin (:3000 → :3027), so the studio JS can't read the error page directly. diff --git a/lib/app_state.rb b/lib/app_state.rb new file mode 100644 index 0000000..00287f4 --- /dev/null +++ b/lib/app_state.rb @@ -0,0 +1,223 @@ +# frozen_string_literal: true + +# Describes a generated application to the modification planner +# (PlanApplicationModification::AdHocLLM), which otherwise plans against nothing +# but the user's sentence and invents file paths, columns and CSS variables. +# +# Deliberately NOT RevisionPrompt: that builds the *implementer's* prompt, globs +# only app/controllers + app/models (missing e.g. app/concerns/), and carries no +# schema or Gemfile diff. The two consumers want different things; the 4-line +# docs glob is duplicated on purpose. +# +# Sections, in order: Gems → Database tables → Routes → Files under app/ → docs/. +# The first four are read fresh and authoritative; docs/ is prose the W2.6 agent +# rewrites after each revision and can lag (measured: it omits real models in 3 +# of 23 populated workspaces, and never invents absent ones). +module AppState + DOC_FILES = %w[architecture.md conventions.md domain.md frontend.md].freeze + + # ExecuteInstructionJob#init_docs_baseline scaffolds three of the four docs as + # a title line plus this line (it interpolates the constant), so File.exist? + # cannot distinguish "the docs agent ran" from "the docs dir was created". + # frontend.md is written by Templates::Picker and is never a placeholder. + PLACEHOLDER = "(empty — will be filled in by the first revision)" + + # Caps each docs file on its own, in characters (see docs_section). There is + # deliberately no cap on the docs total: a body cap over the joined files would + # evict whichever file comes last, and on the two largest workspaces that file + # is frontend.md — the only record of the app's palette. Sized for what new + # projects produce: templates ship frontend.md at ~2.9 KB, the docs agent has + # grown it to 6.7 KB at most, and every file's p90 is under 6 KB. Trims 2 files + # in 1 of 28 current workspaces (project_39). Character slice, as + # revision_workflow.rb's diff cap. + # + # The W2.6 docs-writer prompt in lib/roast/revision_workflow.rb tells the docs + # agent this budget as a literal — that file runs as a Roast subprocess outside + # the Rails autoloader. Update both together. + DOC_FILE_CAP = 8_000 + + SECTIONS = %i[gems schema routes files docs].freeze + + PREAMBLE = <<~TEXT.chomp + ## Current application state + + Read from the workspace just now. The gems, tables, routes and file list are + authoritative. The prose under `docs/` is a summary rewritten after each + revision and may lag behind them — prefer the lists above it on any conflict. + TEXT + + def self.build(workspace:) + # Same predicate as Project#workspace_initialized? — without an app there is + # nothing to describe, and every section's "absent" wording would otherwise + # assert things about an app that does not exist. + return nil unless File.exist?(File.join(workspace, "Gemfile")) + + sections = SECTIONS.filter_map { |s| public_send(:"#{s}_section", workspace).presence } + return nil if sections.empty? + + ([ PREAMBLE ] + sections).join("\n\n") + end + + def self.skeleton_gems + @skeleton_gems ||= File.read(Rails.root.join("lib/preview/skeleton/Gemfile")) + .scan(/^\s*gem\s+["']([^"']+)["']/).flatten.to_set.freeze + end + + # Major.minor from the skeleton Gemfile's `gem "rails", "~> 8.1.3"` line — + # the same file the workspace Gemfile is copied from — so a Rails bump via + # bin/preview-regen-skeleton cannot leave this payload asserting a stale + # version to a planner told not to hedge. + def self.rails_version + @rails_version ||= File.read(Rails.root.join("lib/preview/skeleton/Gemfile"))[/^gem "rails",\s*"[^\d]*(\d+\.\d+)/, 1] || "8" + end + + # Every workspace file is read through here. The workspace is written by the + # sandboxed codegen agent, so a plain File.read does two wrong things: + # + # - It follows a symlink out of the workspace. `docs/domain.md -> + # ../../project_41/docs/domain.md` resolves against the generator's mount, + # where every tenant's workspace is, and the other project's file lands in + # the planner prompt and in the plan shown to the user. realpath containment + # also catches a symlinked directory, which an lstat on the leaf would not. + # - It returns invalid UTF-8 as-is. `rstrip` and `scan` raise on it; the + # tool's rescue then reports a planner failure the user is told to fix by + # rephrasing, and every later request on the project fails the same way. + # `scrub` turns the stray bytes into U+FFFD instead. + # + # Returns nil when the file is missing, not a regular file, or resolves + # outside the workspace — callers treat all three as "not there". + def self.read_workspace_file(workspace, relative) + path = File.join(workspace, relative) + return nil unless File.file?(path) + return nil unless File.realpath(path).start_with?("#{File.realpath(workspace)}/") + + File.read(path, encoding: "UTF-8").scrub + end + + # The single highest-value section: it is what lets the planner see `bcrypt` + # (has_secure_password) on project_30 instead of assuming Devise. + # + # States only what IS there. No "there is no X" sentences: a Gemfile scan + # cannot back a claim about availability (Devise pulls in bcrypt, so "no + # bcrypt" would be false on every Devise app), and a negative in the payload + # is a rule the prompt then has to fight later. The planner infers absence + # from the list itself. "Default Rails 8" + Tailwind + Hotwire is the whole + # stack framing; Propshaft/Importmap/Solid are never enumerated — they are + # what a default install already is, and naming them invites the planner to + # treat them as optional extras worth a revision. + def self.gems_section(workspace) + gemfile = read_workspace_file(workspace, "Gemfile") + return nil unless gemfile + + extra = gemfile.scan(/^\s*gem\s+["']([^"']+)["']/).flatten + .reject { |g| skeleton_gems.include?(g) } + body = "Standard Rails #{rails_version} application with Tailwind and Hotwire, " + body += + if extra.empty? + "on the default Gemfile." + else + "with these gems added on top of the default Gemfile: #{extra.join(', ')}." + end + "### Gems\n\n#{body}" + end + + # Condensed to table + column names: indexes, foreign keys and the 12-line + # header are noise for planning. 8431 -> 1652 bytes on project_30. + def self.schema_section(workspace) + schema = read_workspace_file(workspace, "db/schema.rb") + return migrations_section(workspace) unless schema + + tables = [] + schema.each_line do |line| + case line + when /^\s*create_table "([^"]+)"/ then tables << +"- #{$1}: " + when /^\s*t\.(\w+) "([^"]+)"/ then tables.last << "#{$2} (#{$1}), " unless tables.empty? + end + end + return nil if tables.empty? + + "### Database tables\n\n#{tables.map { |t| t.chomp(', ') }.join("\n")}" + end + + # db/schema.rb only appears after VerifyRevision runs `bin/rails db:prepare`, + # so it lags the migrations. project_3 has two migrations and two models and + # no schema.rb — asserting "this app has no tables" there would be a confident + # falsehood in the section the preamble calls authoritative, contradicting the + # app/models/ entries listed a few lines below it. Name the migrations instead + # and let the planner draw its own conclusion. + def self.migrations_section(workspace) + migrations = Dir.glob("#{workspace}/db/migrate/*.rb").sort.map { |f| File.basename(f) } + return "### Database\n\nNo migrations exist yet — this app has no tables." if migrations.empty? + + "### Database\n\nNo `db/schema.rb` has been written yet (it appears after the first " \ + "verified revision), so the tables are described only by the migrations on disk:\n\n" \ + "#{migrations.map { |m| "- db/migrate/#{m}" }.join("\n")}" + end + + def self.routes_section(workspace) + routes = read_workspace_file(workspace, "config/routes.rb") + return nil unless routes + + "### Routes (config/routes.rb)\n\n```ruby\n#{routes.rstrip}\n```" + end + + # Globs all of app/, not just controllers+models: project_30 put a concern at + # app/concerns/role_authorizable.rb, which a narrower glob misses. + # + # `.css` is in the list because instruction #53 — the stored defect this + # module exists to fix — invented `app/assets/stylesheets/application.tailwind.css`. + # The two real stylesheets (app/assets/stylesheets/application.css and + # app/assets/tailwind/application.css) match no .rb/.erb/.js glob, so without + # `.css` the planner still cannot see the files it hallucinated over. + # app/assets/builds/ is excluded: tailwindcss-rails compiles into it, and + # listing generated output invites the implementer to edit a file that the + # next build overwrites. Costs +2 paths / +75 bytes, uniform across all 28. + def self.files_section(workspace) + builds = "#{workspace}/app/assets/builds/" + files = Dir.glob("#{workspace}/app/**/*.{rb,erb,js,css}").sort + .reject { |f| f.start_with?(builds) } + .map { |f| f.delete_prefix("#{workspace}/") } + return nil if files.empty? + + "### Files under app/\n\n#{files.join("\n")}" + end + + # Each body is fenced, like routes: the docs open with `# Title` and use `##` + # subheadings, which would otherwise outrank this payload's `###` sections in + # the markdown hierarchy — project_28's conventions.md carries a `## Gems` + # table enumerating propshaft/importmap/solid, exactly what gems_section + # refuses to name, and unfenced it sits a heading level above that section + # while the preamble asks the planner to prefer the lists. Four backticks so + # a ``` code block inside a doc cannot close the fence early. + def self.docs_section(workspace) + body = DOC_FILES.filter_map do |name| + content = read_workspace_file(workspace, File.join("docs", name)) + next unless content + next if placeholder?(content) + + "### docs/#{name}\n\n````markdown\n#{cap_doc(content.rstrip, name)}\n````" + end.join("\n\n") + body.presence + end + + # The untouched baseline is a title line and the placeholder line, nothing + # else. A doc the W2.6 agent has populated but appended to (its rules allow + # append-only edits) may still carry the placeholder line; that doc is real + # and must stay, so this is a whole-body test, not a substring search. + def self.placeholder?(content) + lines = content.lines.map(&:strip).reject(&:empty?) + lines.length <= 2 && lines.last == PLACEHOLDER + end + + # Characters, not bytes: byteslice can cut mid-codepoint, and the docs are + # full of em dashes. The result is an invalid-encoding String that JSON + # serialization rejects outright ("source sequence is illegal/malformed + # utf-8"), which the tool's rescue would then report as a generic planner + # failure. String#[] cannot split a character, and it is the same idiom as + # revision_workflow.rb's diff cap — so the marker below is also literally true. + def self.cap_doc(content, name) + return content if content.length <= DOC_FILE_CAP + + "#{content[0, DOC_FILE_CAP]}\n[... docs/#{name} truncated at #{DOC_FILE_CAP} chars ...]" + end +end diff --git a/lib/roast/revision_workflow.rb b/lib/roast/revision_workflow.rb index d4d8700..e1b8926 100644 --- a/lib/roast/revision_workflow.rb +++ b/lib/roast/revision_workflow.rb @@ -248,6 +248,10 @@ # get a structural summary via stat, full bodies for small ones. diff_body = "#{diff_body[0, 16_000]}\n[... diff truncated at 16k chars ...]" if diff_body.length > 16_000 + # The 8000-character budget in the rules below is AppState::DOC_FILE_CAP + # (lib/app_state.rb): the modification planner is fed each docs file whole + # and cut off past that length. Hardcoded here because this file runs as a + # Roast subprocess outside the Rails autoloader. Update both together. <<~PROMPT Revision "#{kwarg(:revision_summary)}" was just committed. Update the docs in docs/ to reflect it. @@ -277,6 +281,7 @@ - Use Edit (small, targeted edits) or append-only operations. Do not rewrite whole files. - If a doc has nothing to update for this revision, skip it — don't write filler. - Be terse. Each section in revision_notes is 1-3 sentences max. + - Keep each of `architecture.md`, `conventions.md`, `domain.md` and `frontend.md` under 8000 characters (~1200 words). They are fed whole to the change planner and cut off past that. When a file is near or over the limit, condensing a stale section with Edit IS the right move and takes precedence over "do not rewrite whole files" — but never rewrite a file top to bottom, and never touch `frontend.md` for length alone. PROMPT end diff --git a/test/integration/generate_todo_list_test.rb b/test/integration/generate_todo_list_test.rb index c8b0b43..5bad9cb 100644 --- a/test/integration/generate_todo_list_test.rb +++ b/test/integration/generate_todo_list_test.rb @@ -6,54 +6,83 @@ # the real `bin/roast` subprocess runs three revisions, and the generated app's # own test suite is green. # -# Stubbed: `Chat#complete` (chat-LLM) and `PlanApplicationCreation.implementation` (plan-LLM) -# so we don't burn tokens on those layers — a real LLM would call CreateApplication -# with whatever intent the user typed; we short-circuit to that decision. +# Stubbed, so no tokens are spent on the LLM layers a real run would consult: +# - `Chat#complete` (chat-LLM) — a real LLM would call CreateApplication with +# whatever intent the user typed; we short-circuit to that decision. +# - `PlanApplicationCreation.implementation` (plan-LLM) — the deterministic +# three-revision todo_list fixture. +# - `Templates::Picker.pick` (template-LLM, the one RubyLLM call on the W2 side) +# — pinned to one template; `apply` stays real so frontend.md and the font +# land in the workspace exactly as in production. # # Real: ExecuteInstructionJob, including the `bin/roast` subprocess that calls -# Claude CLI for each revision. Wall time ≈ 8 minutes; bounded at 900s. +# the Claude CLI for each revision. Wall time ≈ 9 minutes (550s measured +# 2026-09-03, ~900s seen in May); bounded at 1200s. # # Gated by E2E_GENERATE=1 so the default `bin/rails test` stays fast. class GenerateTodoListTest < ActionDispatch::IntegrationTest include ActiveJob::TestHelper PROMPT = "Simple todo list, Tailwind".freeze - WALL_TIME_BUDGET = 900 + WALL_TIME_BUDGET = 1200 + TEMPLATE = "office" setup do skip "set E2E_GENERATE=1 to run (real bin/roast subprocess, ~8 min, burns Claude tokens)" unless ENV["E2E_GENERATE"] == "1" + # ProjectsController has required a signed-in user since Phase 4. The fake + # OpenRouter key on this user is never sent anywhere: every RubyLLM-backed + # stage on this path is stubbed below, and the W2 implementer runs on the + # Claude-subscription transport outside production. + @user = create_user + sign_in @user + require Rails.root.join("test/fixtures/plans/todo_list.rb").to_s @original_create_plan = PlanApplicationCreation.implementation PlanApplicationCreation.implementation = fake_plan_returning(PlanFixtures.todo_list) + stub_template_pick! stub_chat_complete! end teardown do restore_chat_complete! + restore_template_pick! PlanApplicationCreation.implementation = @original_create_plan if @original_create_plan end test "Simple todo list, Tailwind: 3 revisions complete and workspace tests green" do started = Time.current - perform_enqueued_jobs do + # Only the two jobs the pipeline consists of. StopPreviewJob (fired by + # instruction.requested) would drive Docker for a preview that was never + # started, and the Turbo broadcast jobs have no subscriber here. + perform_enqueued_jobs(only: [ ChatRespondJob, ExecuteInstructionJob ]) do post projects_path, params: { project: { description: PROMPT } } end elapsed = Time.current - started - project = Project.order(:id).last - instruction = project.instructions.order(:id).last + # Scoped to the signed-in user: fixtures load projects with large hashed + # ids, so Project.order(:id).last returns a fixture, never the row this + # request created. + project = @user.projects.sole + instruction = project.instructions.sole assert_predicate instruction.reload, :completed?, "instruction phase: #{instruction.phase}" assert_equal 3, project.revisions.count assert project.revisions.all?(&:completed?), "expected all revisions completed, got #{project.revisions.order(:position).map(&:status)}" + # Before the generated app's own suite runs: a blown budget is the answer + # already, no need to spend another minute finding out. + assert_operator elapsed, :<, WALL_TIME_BUDGET, + "generation took #{elapsed.round}s, exceeds #{WALL_TIME_BUDGET}s budget" + workspace = project.workspace_path + # Presence, not the H1: the W2.6 docs agent may edit frontend.md on a styling + # revision, and this asserts that `apply` wrote the pinned template, not + # that the heading survived three revisions untouched. + assert_includes File.read(File.join(workspace, "docs/frontend.md")), TEMPLATE assert_workspace_git_log_at_least(workspace, 4) assert_workspace_tests_pass(workspace) - assert_operator elapsed, :<, WALL_TIME_BUDGET, - "generation took #{elapsed.round}s, exceeds #{WALL_TIME_BUDGET}s budget" end private @@ -66,12 +95,22 @@ def fake_plan_returning(result) # Reaches GeneratorAgent#complete via Forwardable (RubyLLM::Agent delegates # `complete` to the chat record), so redefining Chat#complete is sufficient. + # + # Two turns reach it. The user's first message becomes the CreateApplication + # call a real LLM would make from that intent. The second is the auto-recap + # nudge that the instruction.completed subscriber injects — the real prompt + # forbids tool calls on that turn, so it is a text-only assistant message here + # too. Without that branch the stub would start a second, identical build. def stub_chat_complete! Chat.class_eval do alias_method :_original_complete_for_e2e, :complete unless method_defined?(:_original_complete_for_e2e) define_method(:complete) do |**_kwargs, &_block| - latest_user = messages.where(role: :user).order(:id).last - CreateApplication.new(project: project).execute(intent: latest_user.content.to_s, clarifications: {}) + if project.instructions.none? + latest_user = messages.where(role: :user, system_injected: false).order(:id).last + CreateApplication.new(project: project).execute(intent: latest_user.content.to_s, clarifications: {}) + else + messages.create!(role: :assistant, content: "The todo list is built. What would you like to change next?") + end end end end @@ -85,6 +124,15 @@ def restore_chat_complete! end end + def stub_template_pick! + @original_pick = Templates::Picker.method(:pick) + Templates::Picker.define_singleton_method(:pick) { |**| TEMPLATE } + end + + def restore_template_pick! + Templates::Picker.define_singleton_method(:pick, @original_pick) if @original_pick + end + def assert_workspace_git_log_at_least(workspace, expected) log = `cd #{Shellwords.escape(workspace)} && git log --oneline 2>/dev/null`.lines assert_operator log.size, :>=, expected, @@ -92,7 +140,7 @@ def assert_workspace_git_log_at_least(workspace, expected) end def assert_workspace_tests_pass(workspace) - ruby_version = File.read(Rails.root.join(".ruby-version")).strip + ruby_version = File.read(Rails.root.join(".ruby-version")).strip.delete_prefix("ruby-") frum_bin = File.join(Dir.home, ".frum", "versions", ruby_version, "bin") env = File.directory?(frum_bin) ? { "PATH" => "#{frum_bin}:#{ENV.fetch('PATH', '')}" } : {} diff --git a/test/lib/app_state_test.rb b/test/lib/app_state_test.rb new file mode 100644 index 0000000..cbc61f1 --- /dev/null +++ b/test/lib/app_state_test.rb @@ -0,0 +1,461 @@ +require "test_helper" +require "tmpdir" +require "fileutils" + +class AppStateTest < ActiveSupport::TestCase + SKELETON_GEMFILE = Rails.root.join("lib/preview/skeleton/Gemfile") + + setup do + @workspace = Dir.mktmpdir("app-state-test-") + end + + teardown do + FileUtils.remove_entry(@workspace) if File.exist?(@workspace) + end + + # ---- build: gate on Gemfile ---- + + test "build returns nil when the workspace has no Gemfile" do + write("config/routes.rb", "Rails.application.routes.draw do\nend\n") + assert_nil AppState.build(workspace: @workspace) + end + + test "build returns nil when the workspace directory does not exist" do + assert_nil AppState.build(workspace: File.join(@workspace, "missing")) + end + + # ---- gems ---- + + test "gems: baseline-only Gemfile is described as the default Gemfile" do + copy_skeleton_gemfile + out = AppState.gems_section(@workspace) + + assert_includes out, "### Gems" + assert_includes out, "Standard Rails #{AppState.rails_version} application with Tailwind and Hotwire, on the default Gemfile." + end + + test "gems: the Rails version comes from the skeleton Gemfile, so a Rails bump cannot leave it stale" do + assert_match(/\A\d+\.\d+\z/, AppState.rails_version) + assert_includes File.read(SKELETON_GEMFILE), "gem \"rails\", \"~> #{AppState.rails_version}." + end + + test "gems: extras are listed after the default Gemfile, skeleton gems are not" do + copy_skeleton_gemfile(extra: "gem \"devise\"\ngem 'roo', \"~> 2.10\"\n") + out = AppState.gems_section(@workspace) + + assert_includes out, "with these gems added on top of the default Gemfile: devise, roo." + %w[rails propshaft sqlite3 puma importmap-rails turbo-rails stimulus-rails tailwindcss-rails + solid_cache solid_queue solid_cable bootsnap thruster image_processing].each do |skeleton_gem| + refute_includes out, skeleton_gem, "skeleton gem #{skeleton_gem} must not be listed as an extra" + end + end + + test "gems: the commented-out bcrypt line in the skeleton is not a skeleton gem, so uncommenting it surfaces it" do + copy_skeleton_gemfile(extra: "gem \"bcrypt\", \"~> 3.1.7\"\n") + assert_includes AppState.gems_section(@workspace), "default Gemfile: bcrypt." + end + + test "gems: never asserts an absence" do + copy_skeleton_gemfile + baseline = AppState.gems_section(@workspace) + copy_skeleton_gemfile(extra: "gem \"devise\"\n") + with_extra = AppState.gems_section(@workspace) + + [ baseline, with_extra ].each do |out| + refute_match(/\b(no|not|without|absent|missing)\b/i, out, "gems section must state only what is there") + end + end + + test "gems: section omitted when Gemfile is missing" do + assert_nil AppState.gems_section(@workspace) + end + + # ---- schema ---- + + test "schema: present -> one line per table with column (type) pairs, no indexes or foreign keys" do + write("db/schema.rb", <<~RUBY) + # This file is auto-generated from the current state of the database. + ActiveRecord::Schema[8.1].define(version: 2026_04_29_224254) do + create_table "tasks", force: :cascade do |t| + t.string "title", null: false + t.integer "user_id", null: false + t.datetime "created_at", null: false + t.index ["user_id"], name: "index_tasks_on_user_id" + end + + create_table "users", force: :cascade do |t| + t.string "email" + end + + add_foreign_key "tasks", "users" + end + RUBY + + out = AppState.schema_section(@workspace) + + assert_includes out, "### Database tables" + assert_includes out, "- tasks: title (string), user_id (integer), created_at (datetime)\n" + assert_includes out, "- users: email (string)" + refute_includes out, "index_tasks_on_user_id" + refute_includes out, "add_foreign_key" + refute_includes out, "ActiveRecord::Schema" + end + + test "schema: a column line before any create_table is ignored rather than raising" do + write("db/schema.rb", "ActiveRecord::Schema[8.1].define(version: 1) do\n t.string \"stray\"\n create_table \"tasks\" do |t|\n t.string \"title\"\n end\nend\n") + assert_includes AppState.schema_section(@workspace), "- tasks: title (string)" + end + + test "schema: present but without tables -> section omitted" do + write("db/schema.rb", "ActiveRecord::Schema[8.1].define(version: 0) do\nend\n") + assert_nil AppState.schema_section(@workspace) + end + + test "schema absent, db/migrate empty -> says no migrations exist yet" do + FileUtils.mkdir_p(File.join(@workspace, "db/migrate")) + out = AppState.schema_section(@workspace) + + assert_includes out, "### Database" + assert_includes out, "No migrations exist yet" + end + + test "schema absent, db/migrate populated -> lists migrations and does not claim the app has no tables" do + write("db/migrate/20260429224200_create_users.rb", "class CreateUsers < ActiveRecord::Migration[8.1]; end\n") + write("db/migrate/20260429224254_create_tasks.rb", "class CreateTasks < ActiveRecord::Migration[8.1]; end\n") + + out = AppState.schema_section(@workspace) + + assert_includes out, "- db/migrate/20260429224200_create_users.rb\n- db/migrate/20260429224254_create_tasks.rb" + assert_includes out, "No `db/schema.rb` has been written yet" + refute_includes out, "no tables" + refute_includes out, "No migrations exist yet" + end + + # ---- routes ---- + + test "routes: absent -> section omitted" do + assert_nil AppState.routes_section(@workspace) + end + + test "routes: present -> fenced and verbatim" do + routes = "Rails.application.routes.draw do\n resources :standups, only: [:index, :create]\n root \"standups#index\"\nend\n" + write("config/routes.rb", routes) + + out = AppState.routes_section(@workspace) + + assert_equal "### Routes (config/routes.rb)\n\n```ruby\n#{routes.rstrip}\n```", out + end + + # ---- files ---- + + test "files: empty app/ -> section omitted" do + FileUtils.mkdir_p(File.join(@workspace, "app/models")) + assert_nil AppState.files_section(@workspace) + end + + test "files: lists app/concerns/ alongside controllers, models, views and JS" do + write("app/concerns/role_authorizable.rb", "module RoleAuthorizable; end\n") + write("app/controllers/standups_controller.rb", "class StandupsController; end\n") + write("app/models/standup.rb", "class Standup; end\n") + write("app/views/standups/index.html.erb", "

Standups

\n") + write("app/javascript/controllers/hello_controller.js", "export default class {}\n") + + out = AppState.files_section(@workspace) + + assert_includes out, "### Files under app/" + assert_includes out, "app/concerns/role_authorizable.rb" + assert_includes out, "app/controllers/standups_controller.rb" + assert_includes out, "app/models/standup.rb" + assert_includes out, "app/views/standups/index.html.erb" + assert_includes out, "app/javascript/controllers/hello_controller.js" + refute_includes out, @workspace, "paths must be workspace-relative" + end + + test "files: lists the two real stylesheets and excludes app/assets/builds/" do + write("app/assets/stylesheets/application.css", "/* Rails */\n") + write("app/assets/tailwind/application.css", "@import \"tailwindcss\";\n") + write("app/assets/builds/tailwind.css", "/* compiled */\n") + + out = AppState.files_section(@workspace) + + assert_includes out, "app/assets/stylesheets/application.css" + assert_includes out, "app/assets/tailwind/application.css" + refute_includes out, "app/assets/builds/tailwind.css" + end + + test "files: listing is sorted by full path, not glob order, so the payload is deterministic" do + write("app/foo/a.css", "") + write("app/foo/y.rb", "") + write("app/foo-bar/x.rb", "") + + assert_equal "### Files under app/\n\napp/foo-bar/x.rb\napp/foo/a.css\napp/foo/y.rb", AppState.files_section(@workspace) + end + + test "files: ignores non-source files under app/" do + write("app/assets/images/logo.png", "PNG") + write("app/models/standup.rb", "class Standup; end\n") + + out = AppState.files_section(@workspace) + + assert_includes out, "app/models/standup.rb" + refute_includes out, "logo.png" + end + + # ---- docs ---- + + test "docs: all placeholders -> section omitted" do + write_placeholder_docs + assert_nil AppState.docs_section(@workspace) + end + + test "docs: absent docs/ -> section omitted" do + assert_nil AppState.docs_section(@workspace) + end + + test "docs: a real architecture.md next to a placeholder domain.md -> only the former appears" do + write_placeholder_docs + write("docs/architecture.md", "# Architecture\n\nStandup model, StandupsController.\n") + + out = AppState.docs_section(@workspace) + + assert_includes out, "### docs/architecture.md\n\n````markdown\n# Architecture\n\nStandup model, StandupsController." + refute_includes out, "### docs/domain.md" + refute_includes out, "### docs/conventions.md" + refute_includes out, AppState::PLACEHOLDER + end + + test "docs: a populated doc that still carries the baseline placeholder line is kept" do + write("docs/domain.md", "# Domain\n\n#{AppState::PLACEHOLDER}\n\n## Standup\n\nOne row per daily standup.\n") + + out = AppState.docs_section(@workspace) + + assert_includes out, "### docs/domain.md" + assert_includes out, "One row per daily standup." + end + + test "docs: frontend.md alone appears" do + write("docs/frontend.md", "# Frontend\n\nOffice template. Primary #0052CC.\n") + + out = AppState.docs_section(@workspace) + + assert_includes out, "### docs/frontend.md\n\n````markdown\n# Frontend\n\nOffice template. Primary #0052CC." + end + + test "docs: revision_notes.md is never read" do + write("docs/revision_notes.md", "# Revision notes\n\nrationale rationale\n") + write("docs/frontend.md", "# Frontend\n") + + out = AppState.docs_section(@workspace) + + refute_includes out, "revision_notes" + refute_includes out, "rationale" + end + + # ---- docs cap ---- + + test "docs cap: a file over the cap is truncated with a marker naming that file" do + write("docs/architecture.md", "a" * (AppState::DOC_FILE_CAP + 1)) + + out = AppState.docs_section(@workspace) + + assert_includes out, "[... docs/architecture.md truncated at #{AppState::DOC_FILE_CAP} chars ...]" + assert_includes out, "a" * AppState::DOC_FILE_CAP + refute_includes out, "a" * (AppState::DOC_FILE_CAP + 1) + end + + test "docs cap: a file exactly at the cap is untouched" do + write("docs/architecture.md", "a" * AppState::DOC_FILE_CAP) + + out = AppState.docs_section(@workspace) + + refute_includes out, "truncated" + assert_includes out, "a" * AppState::DOC_FILE_CAP + end + + test "docs cap is per file: an oversized architecture.md leaves frontend.md intact" do + frontend = "# Frontend\n\n" + ("palette #0052CC, #DE350B\n" * 40) + write("docs/architecture.md", "a" * (AppState::DOC_FILE_CAP + 500)) + write("docs/frontend.md", frontend) + + out = AppState.docs_section(@workspace) + + assert_includes out, "[... docs/architecture.md truncated at" + refute_includes out, "[... docs/frontend.md truncated at" + assert_includes out, "### docs/frontend.md\n\n````markdown\n#{frontend.rstrip}" + end + + test "docs cap is per file: two oversized files each carry their own marker" do + write("docs/architecture.md", "a" * (AppState::DOC_FILE_CAP + 1)) + write("docs/domain.md", "d" * (AppState::DOC_FILE_CAP + 1)) + + out = AppState.docs_section(@workspace) + + assert_includes out, "[... docs/architecture.md truncated at" + assert_includes out, "[... docs/domain.md truncated at" + end + + test "docs: bodies are fenced with four backticks, so a code block inside a doc cannot close the fence" do + write("docs/conventions.md", "# Conventions\n\n## Gems\n\n```ruby\ngem \"roo\"\n```\n") + + out = AppState.docs_section(@workspace) + + assert_equal "### docs/conventions.md\n\n````markdown\n# Conventions\n\n## Gems\n\n```ruby\ngem \"roo\"\n```\n````", out + end + + test "docs cap counts characters, so a multi-byte boundary stays valid UTF-8" do + # 7999 ASCII chars, then an em dash straddling the 8000th character. A byte + # slice would cut inside the 3-byte em dash; a character slice cannot. + write("docs/architecture.md", ("a" * (AppState::DOC_FILE_CAP - 1)) + "—" + ("b" * 100)) + + out = AppState.docs_section(@workspace) + + assert_predicate out, :valid_encoding? + assert_includes out, "a—\n[... docs/architecture.md truncated at" + refute_includes out, "b" + end + + # ---- reads: the workspace is agent-written ---- + + test "reads: invalid UTF-8 in any workspace file is scrubbed, never raised" do + # One stray byte in each file the module reads. Before read_workspace_file, + # scan (Gemfile), the per-line regex (schema) and rstrip (routes, docs) each + # raised on it and the tool reported a planner failure for every request after. + write_bytes("Gemfile", (File.read(SKELETON_GEMFILE) + "gem \"roo\" # caf\xE9\n").b) + write_bytes("db/schema.rb", "ActiveRecord::Schema[8.1].define(version: 1) do\n create_table \"standups\" do |t|\n t.string \"name\" # caf\xE9\n end\nend\n".b) + write_bytes("config/routes.rb", "Rails.application.routes.draw do\n resources :standups # caf\xE9\nend\n".b) + write_bytes("docs/architecture.md", "# Architecture\n\nStandup model. caf\xE9\n".b) + + out = nil + assert_nothing_raised { out = AppState.build(workspace: @workspace) } + + assert_predicate out, :valid_encoding? + assert_includes out, "default Gemfile: roo." + assert_includes out, "- standups: name (string)" + assert_includes out, "resources :standups # caf�" + assert_includes out, "### docs/architecture.md\n\n````markdown\n# Architecture\n\nStandup model. caf�" + end + + test "reads: a symlink that resolves outside the workspace is treated as absent" do + outside = Dir.mktmpdir("app-state-outside-") + File.write(File.join(outside, "domain.md"), "# Domain\n\nOTHER TENANT SECRET\n") + File.write(File.join(outside, "routes.rb"), "Rails.application.routes.draw do\n # OTHER TENANT SECRET\nend\n") + copy_skeleton_gemfile + FileUtils.mkdir_p(File.join(@workspace, "docs")) + FileUtils.mkdir_p(File.join(@workspace, "config")) + FileUtils.ln_s(File.join(outside, "domain.md"), File.join(@workspace, "docs/domain.md")) + FileUtils.ln_s(File.join(outside, "routes.rb"), File.join(@workspace, "config/routes.rb")) + + out = AppState.build(workspace: @workspace) + + refute_includes out, "OTHER TENANT SECRET" + refute_includes out, "### docs/domain.md" + refute_includes out, "### Routes" + ensure + FileUtils.remove_entry(outside) if outside && File.exist?(outside) + end + + test "reads: a symlink that stays inside the workspace is read" do + write("notes/architecture.md", "# Architecture\n\nStandup model, linked in.\n") + FileUtils.mkdir_p(File.join(@workspace, "docs")) + FileUtils.ln_s("../notes/architecture.md", File.join(@workspace, "docs/architecture.md")) + + assert_includes AppState.docs_section(@workspace), "### docs/architecture.md\n\n````markdown\n# Architecture\n\nStandup model, linked in." + end + + test "the W2.6 docs-writer prompt quotes DOC_FILE_CAP — the two are maintained together" do + assert_includes File.read(Rails.root.join("lib/roast/revision_workflow.rb")), "under #{AppState::DOC_FILE_CAP} characters" + end + + test "build returns nil when no section has anything to say" do + outside = Dir.mktmpdir("app-state-outside-") + File.write(File.join(outside, "Gemfile"), "gem \"rails\"\n") + FileUtils.ln_s(File.join(outside, "Gemfile"), File.join(@workspace, "Gemfile")) + write("db/schema.rb", "ActiveRecord::Schema[8.1].define(version: 0) do\nend\n") + + assert_nil AppState.build(workspace: @workspace) + ensure + FileUtils.remove_entry(outside) if outside && File.exist?(outside) + end + + # ---- build: assembly ---- + + test "build joins the preamble and every present section in order" do + write_full_workspace + + out = AppState.build(workspace: @workspace) + + positions = { + preamble: out.index("## Current application state"), + gems: out.index("### Gems"), + schema: out.index("### Database tables"), + routes: out.index("### Routes (config/routes.rb)"), + files: out.index("### Files under app/"), + docs: out.index("### docs/architecture.md") + } + + assert(positions.values.all?, "every section should be present in fixture, got #{positions.inspect}") + assert_operator positions[:preamble], :<, positions[:gems] + assert_operator positions[:gems], :<, positions[:schema] + assert_operator positions[:schema], :<, positions[:routes] + assert_operator positions[:routes], :<, positions[:files] + assert_operator positions[:files], :<, positions[:docs] + end + + test "build's preamble marks the lists authoritative and docs/ as a lagging summary" do + write_full_workspace + + out = AppState.build(workspace: @workspace) + + assert_match(/The gems, tables, routes and file list are\s+authoritative\./, out) + assert_includes out, "prefer the lists above it on any conflict." + end + + test "build omits absent sections without leaving gaps" do + copy_skeleton_gemfile + + out = AppState.build(workspace: @workspace) + + assert_includes out, "### Gems" + assert_includes out, "### Database\n\nNo migrations exist yet" + refute_includes out, "### Routes" + refute_includes out, "### Files under app/" + refute_includes out, "### docs/" + refute_match(/\n{3,}/, out, "sections are joined by exactly one blank line") + end + + private + + def write(relative_path, content) + path = File.join(@workspace, relative_path) + FileUtils.mkdir_p(File.dirname(path)) + File.write(path, content) + end + + # File.write transcodes to the external encoding; binwrite keeps a stray byte a stray byte. + def write_bytes(relative_path, bytes) + path = File.join(@workspace, relative_path) + FileUtils.mkdir_p(File.dirname(path)) + File.binwrite(path, bytes) + end + + def copy_skeleton_gemfile(extra: "") + write("Gemfile", File.read(SKELETON_GEMFILE) + extra) + end + + def write_placeholder_docs + %w[architecture conventions domain].each do |name| + write("docs/#{name}.md", "# #{name.capitalize}\n\n#{AppState::PLACEHOLDER}\n") + end + write("docs/revision_notes.md", "# Revision notes\n\n") + end + + def write_full_workspace + copy_skeleton_gemfile + write("db/schema.rb", "ActiveRecord::Schema[8.1].define(version: 1) do\n create_table \"standups\" do |t|\n t.string \"name\"\n end\nend\n") + write("config/routes.rb", "Rails.application.routes.draw do\n resources :standups, only: [:index, :create]\nend\n") + write("app/models/standup.rb", "class Standup < ApplicationRecord; end\n") + write_placeholder_docs + write("docs/architecture.md", "# Architecture\n\nStandup model.\n") + write("docs/frontend.md", "# Frontend\n\nOffice template.\n") + end +end 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 102f986..37478b9 100644 --- a/test/services/plan_application_creation/ad_hoc_llm_test.rb +++ b/test/services/plan_application_creation/ad_hoc_llm_test.rb @@ -46,6 +46,24 @@ def plan_fixture(name) end end + # ---- the system prompt states only what a fresh app will have ---- + # This planner runs before `rails new`, so its prompt is its only lever. It + # used to assert Devise was installed; the workspace never has it. Sign-in is + # planned as has_secure_password + sessions, 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, + PlanApplicationCreation::AdHocLLM::SYSTEM_PROMPT) + end + + test "system prompt frames the stack positively: default Rails 8, default Gemfile, has_secure_password" do + prompt = PlanApplicationCreation::AdHocLLM::SYSTEM_PROMPT + assert_includes prompt, "default Rails 8 app" + assert_includes prompt, "default Gemfile" + assert_includes prompt, "has_secure_password" + 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 5c22015..2ea14eb 100644 --- a/test/services/plan_application_modification/ad_hoc_llm_test.rb +++ b/test/services/plan_application_modification/ad_hoc_llm_test.rb @@ -46,6 +46,24 @@ def plan_fixture(name) end end + # ---- the system prompt states only what a generated app has ---- + # The stored defect the workspace snapshot fixes: this prompt used to assert + # that Devise and hifumi.dev's own `--accent` tokens exist in the generated + # app. Neither does. Absences are for the planner to infer from the snapshot, + # 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, + PlanApplicationModification::AdHocLLM::SYSTEM_PROMPT) + end + + test "system prompt frames the stack positively: default Rails 8, the Gems section, has_secure_password" do + prompt = PlanApplicationModification::AdHocLLM::SYSTEM_PROMPT + assert_includes prompt, "default Rails 8 installation" + assert_includes prompt, '"Gems" section' + assert_includes prompt, "has_secure_password" + 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") @@ -68,6 +86,65 @@ def plan_fixture(name) end end + # ---- context[:app_state] — the workspace snapshot ModifyApplication builds ---- + + APP_STATE = "## Current application state\n\n### Gems\n\nStandard Rails 8.1 application with Tailwind and Hotwire, on the default Gemfile.".freeze + + test "renders context[:app_state] after the intent, separated by a blank line" do + with_llm_response(plan_fixture("valid_plan.json")) do |captured| + PlanApplicationModification::AdHocLLM.call( + intent: "make banner green", clarifications: {}, context: { app_state: APP_STATE }, + openrouter_api_key: "sk-or-test", model: "anthropic/claude-haiku-4.5" + ) + assert_equal "Intent: make banner green\n\n#{APP_STATE}", captured[:user] + end + end + + # The system prompt says the state is given and forbids hedging, so every path + # without a snapshot must say so in the user turn instead of staying silent. + NO_STATE = PlanApplicationModification::AdHocLLM::NO_STATE_NOTE + + test "an empty context appends the no-snapshot note where the state would go" 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-haiku-4.5") + assert_equal "Intent: make banner green\n\n#{NO_STATE}", captured[:user] + end + end + + test "a nil app_state (workspace not initialized, or --blind) appends the no-snapshot note" do + with_llm_response(plan_fixture("valid_plan.json")) do |captured| + PlanApplicationModification::AdHocLLM.call(intent: "make banner green", clarifications: {}, context: { app_state: nil }, openrouter_api_key: "sk-or-test", model: "anthropic/claude-haiku-4.5") + assert_equal "Intent: make banner green\n\n#{NO_STATE}", captured[:user] + end + end + + test "a non-Hash context is treated as no snapshot rather than raising" do + with_llm_response(plan_fixture("valid_plan.json")) do |captured| + PlanApplicationModification::AdHocLLM.call(intent: "make banner green", clarifications: {}, context: nil, openrouter_api_key: "sk-or-test", model: "anthropic/claude-haiku-4.5") + assert_equal "Intent: make banner green\n\n#{NO_STATE}", captured[:user] + end + end + + test "the no-snapshot note never appears when a snapshot is given" do + with_llm_response(plan_fixture("valid_plan.json")) do |captured| + PlanApplicationModification::AdHocLLM.call(intent: "make banner green", clarifications: {}, context: { app_state: APP_STATE }, openrouter_api_key: "sk-or-test", model: "anthropic/claude-haiku-4.5") + refute_includes captured[:user], NO_STATE + end + end + + test "clarifications precede app_state so the request is never buried behind the listing" do + with_llm_response(plan_fixture("valid_plan.json")) do |captured| + PlanApplicationModification::AdHocLLM.call( + intent: "make banner green", + clarifications: { "shade?" => "forest" }, + context: { app_state: APP_STATE }, + openrouter_api_key: "sk-or-test", + model: "anthropic/claude-haiku-4.5" + ) + assert_equal "Intent: make banner green\nClarifications:\n - shade?: forest\n\n#{APP_STATE}", captured[:user] + end + end + test "raises InvalidResponse when LLM returns no content" do with_llm_response(nil) do assert_raises(PlanApplicationModification::AdHocLLM::InvalidResponse) do diff --git a/test/tools/modify_application_test.rb b/test/tools/modify_application_test.rb index 9574843..8cc5a84 100644 --- a/test/tools/modify_application_test.rb +++ b/test/tools/modify_application_test.rb @@ -18,6 +18,10 @@ class ModifyApplicationTest < ActiveSupport::TestCase ) end + teardown do + FileUtils.rm_rf(@project.workspace_path) if File.exist?(@project.workspace_path) + end + def stub_planner(result_or_proc) original = PlanApplicationModification.method(:call) PlanApplicationModification.define_singleton_method(:call) do |**kwargs| @@ -28,6 +32,14 @@ def stub_planner(result_or_proc) PlanApplicationModification.define_singleton_method(:call, original) if original end + def stub_app_state_build(proc) + original = AppState.method(:build) + AppState.define_singleton_method(:build) { |**kwargs| proc.call(**kwargs) } + yield + ensure + AppState.define_singleton_method(:build, original) if original + end + test "persists an Instruction with user_intent, description, implementing phase, and user anchor_message" do stub_planner(@plan) do @tool.execute(intent: "make the primary color teal", clarifications: {}) @@ -170,6 +182,72 @@ def stub_planner(result_or_proc) assert_equal [ "upstream boom" ], reports.map { |r| r.error.message } end + # ---- context[:app_state]: the workspace snapshot fed to the planner ---- + + test "passes the workspace snapshot to the planner as context[:app_state]" do + FileUtils.mkdir_p(@project.workspace_path) + File.write(File.join(@project.workspace_path, "Gemfile"), File.read(Rails.root.join("lib/preview/skeleton/Gemfile"))) + FileUtils.mkdir_p(File.join(@project.workspace_path, "app/models")) + File.write(File.join(@project.workspace_path, "app/models/story.rb"), "class Story < ApplicationRecord; end\n") + + captured = nil + capturing = ->(**kwargs) { captured = kwargs; @plan } + + stub_planner(capturing) do + @tool.execute(intent: "make the primary color teal", clarifications: {}) + end + + assert_kind_of String, captured[:context][:app_state] + assert_includes captured[:context][:app_state], "## Current application state" + assert_includes captured[:context][:app_state], "app/models/story.rb" + assert_equal "make the primary color teal", captured[:intent] + end + + test "with no workspace on disk, context[:app_state] is nil and the plan still persists" do + refute @project.workspace_initialized? + + captured = nil + capturing = ->(**kwargs) { captured = kwargs; @plan } + + assert_difference -> { Instruction.count }, 1 do + assert_difference -> { Revision.count }, 1 do + stub_planner(capturing) do + @tool.execute(intent: "make the primary color teal", clarifications: {}) + end + end + end + + assert_nil captured[:context][:app_state] + end + + # The file reads happen inside #execute so that the rescue StandardError + # backstop covers them: an unreadable workspace file must degrade to an error + # hash (the tool_use still gets its tool_result) rather than escape and kill + # the chat. This is the path that made the backstop necessary. + test "an unreadable workspace file reaches the backstop: error hash, report, nothing persisted" do + # Stubbed rather than chmod 000: root ignores file modes, so the chmod form + # skipped in a root container. What this pins is that AppState.build is + # called inside #execute's rescue, not how the read fails. + planner_called = false + result = nil + reports = capture_error_reports(Errno::EACCES) do + assert_no_difference -> { Instruction.count } do + assert_no_difference -> { Revision.count } do + stub_app_state_build(->(**) { raise Errno::EACCES, "Gemfile" }) do + stub_planner(->(**) { planner_called = true; @plan }) do + result = @tool.execute(intent: "x", clarifications: {}) + end + end + end + end + end + + refute planner_called, "the planner must not be called when the snapshot cannot be read" + assert_match(/Could not generate a modification plan/, result[:error]) + assert_equal 1, reports.size + assert_equal @project.id, reports.first.context[:project_id] + end + test "refuses and persists nothing when an implementing instruction already exists" do @project.instructions.create!( user_intent: "earlier", description: "earlier",