diff --git a/CHANGELOG.md b/CHANGELOG.md index c8113fe..138f570 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,23 @@ 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.4.0] - 2026-08-24 + +### Changed + +- The conversation layer moved to RubyLLM 2.0. Nothing changes in how the app + behaves — chat still streams, builds still start from the same two tools — + but self-hosters upgrading past this point should know two things. First, the + gem is pinned to a specific commit of its `main` branch rather than a + released version, because 2.0 has not shipped to RubyGems yet. Second, the + upgrade runs a **one-way** migration: it renames `models` to + `ruby_llm_models` and `tool_calls` to `ruby_llm_tool_calls` in place, moves + per-message token counts into a new `ruby_llm_usages` table, and drops the + columns it replaced. There is no `down`, and the migration runs + automatically when the container boots, so a deploy is what triggers it. + Snapshot the database first; the procedure is in + `docs/05-runbooks/04-ruby-llm-v2-rollout.md`. + ## [1.3.0] - 2026-08-18 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 8568ee0..6368b12 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -11,9 +11,10 @@ Hosted at **[hifumi.dev](https://hifumi.dev)** · Source: this repo. - **Phase 3** (preview isolation via Kamal + Docker): **closed at the local-PoC level**. Button-driven start/stop, hardened Docker container (`--cap-drop=ALL`, `--read-only`, memory/CPU/pids capped, `preview-internal` network), iframe in side-by-side layout, `CleanupIdlePreviewsJob` reaps previews running >30 min, `instruction.requested` auto-stops a running preview before generation. E2E test gated by `E2E_PREVIEW=1 bin/rails test test/integration/preview_lifecycle_test.rb`. - **Phase 4** (production deploy + multi-tenant auth): **closed**. Live at [hifumi.dev](https://hifumi.dev) on Hetzner via Kamal + kamal-proxy. Devise email/password + Sign in with GitHub (OmniAuth). Per-user OpenRouter BYOK (key encrypted at rest via Active Record `encrypts`). Production Dockerfile bundles the `claude` CLI as Roast's transport pointed at OpenRouter. - **Post-launch review (2026-06-11)**: hardening + robustness findings recorded in `docs/04-reviews/01-post-launch-review.md`. Actioned since: the codegen agent now runs in a per-instruction isolated container (`Roast::Sandbox`); the CVE'd gems were bundle-updated (2026-06-12, CI gates on `bundler-audit`). Phase 5 remains unscoped; the review's last section lists the candidate directions discussed. +- **RubyLLM v2 upgrade (2026-08-23)**: `ruby_llm` is now a **git pin** — `crmne/ruby_llm@c45ebd78`, the unreleased 2.0 line, which still reports `VERSION 1.16.0` because the bump happens at release. Pinned rather than tracking `main` because `main` moves daily and `BUNDLE_DEPLOYMENT=1` plus `HIFUMI_AGENT_IMAGE` reusing this image make the resolved revision a reviewed decision; swap to a version constraint once 2.0 ships to RubyGems. RubyLLM now owns four tables, all via one **irreversible** migration: `ruby_llm_models` and `ruby_llm_tool_calls` are the old `models` / `tool_calls` renamed in place, while `ruby_llm_usages` (backfilled from token columns the migration then drops from `messages`) and `ruby_llm_batches` are created fresh. `Schematist::Schema` replaced `RubyLLM::Schema`, and structured output reads `.parsed` rather than `.content`. Rollout procedure: `docs/05-runbooks/04-ruby-llm-v2-rollout.md`. Deferred observations from Phase 2 (revisit later, not blockers): -- refused-tool-call pill UX (Step 6) — the `🌀 Starting generation…` flash when the LLM ignores the state rule and Phase 5's tool guard rescues. +- 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. @@ -25,7 +26,7 @@ All project documentation lives in `docs/`, grouped by topic. Folder and file nu - **`docs/02-architecture/`** — technical canon: workflows and decisions, layer integration, tech stack, design system, and tenant isolation & preview-domain strategy (`05-tenant-isolation-and-domains.md` — why untrusted previews on a subdomain of the apex are a structural hazard, and the separate-registrable-domain antidote). - **`docs/03-plans/`** — active implementation plans per phase (currently Phase 2 + Phase 3 analysis). - **`docs/04-reviews/`** — point-in-time reviews of the running system. `01-post-launch-review.md` (2026-06-11) records the post-Phase-4 robustness/OSS-readiness findings; read it before planning Phase 5. -- **`docs/05-runbooks/`** — step-by-step verification procedures. `01-agent-sandbox-and-model-selection-e2e.md` verifies per-project model selection + agent-sandbox isolation, locally and on prod (`kamal app exec --reuse`); `02-preview-wildcard-tls.md` switches preview hosts from per-host on-demand Let's Encrypt to a pre-issued wildcard cert; `03-llm-model-registry.md` populates and verifies the RubyLLM model registry (`bin/verify-model-registry`) — read it before adding a model to the picker. +- **`docs/05-runbooks/`** — step-by-step verification procedures. `01-agent-sandbox-and-model-selection-e2e.md` verifies per-project model selection + agent-sandbox isolation, locally and on prod (`kamal app exec --reuse`); `02-preview-wildcard-tls.md` switches preview hosts from per-host on-demand Let's Encrypt to a pre-issued wildcard cert; `03-llm-model-registry.md` populates and verifies the RubyLLM model registry (`bin/verify-model-registry`) — read it before adding a model to the picker; `04-ruby-llm-v2-rollout.md` is the RubyLLM v2 production rollout — snapshot, deploy, verify, recover — and the migration it covers is irreversible and fires automatically on container boot, so read it before deploying that upgrade. - **`docs/09-ideas/`** — brainstorm / idea dump (explicitly marked as non-canon). - **`spikes/roast/`** — reference implementation of Phase 1 (proven, don't touch without reason). Future spikes: `spikes//`. @@ -53,7 +54,7 @@ Additional sources from the Phase 1 spike: - **Design system: Hifumi.** All visible chrome (colors, type, components, status tags, marketing pipeline) follows the Hifumi design system applied 2026-05-01. Tokens + component classes live in a single file: `app/assets/tailwind/application.css`. Use the tokens (`--accent`, `--paper-100`, `--ink-800`, `--hi-font-mono`, etc.) — never hardcode hex values. Status indicators are rectangular outlined boxes in mono caps with a stripe + blinking dot for live states, no emoji. Sentence case in every UI string. See `docs/02-architecture/04-design-system.md` for the full token map, component-to-view inventory, and anti-patterns. - **Pin `.ruby-version` by writing the file** (Write tool), not via the version manager CLI (`frum local`, `rbenv local`, etc.). User wants to verify state from the file itself. - **Roast runner**: `bin/roast-claudesubscription` is the dev default (uses Claude Code subscription — wrapper unsets `ANTHROPIC_*` ENV + pins PATH to `.ruby-version` via frum). `bin/roast-openrouter` is the per-token alternative used in production and when `FORCE_OPENROUTER=1` in dev. `bin/roast` (the bundler binstub) calls `bundle exec roast` raw, no env setup — for direct testing only. `ExecuteInstructionJob` picks `-openrouter` in production / when `FORCE_OPENROUTER=1` / whenever sandboxed, else `-claudesubscription`. -- **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 `use_new_acts_as` + `acts_as_model` make RubyLLM resolve against the `models` DB table and fall back to the gem's bundled `models.json` only when that table is *empty*, so a partially-filled table silently shadows the JSON and any model missing from it raises `ModelNotFoundError` on the four RubyLLM-backed stages (dev and prod both sat at one row until 2026-08-12). Check with `bin/verify-model-registry [candidate-id]`, fix with `Model.refresh!` plus a process restart (the registry is memoized per process) — 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. +- **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. -- **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. Tool-side guard pattern: `SuggestPrompts#duplicate_in_turn?` returns an in-band error 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`). +- **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/Gemfile b/Gemfile index d5ad8a6..083879e 100644 --- a/Gemfile +++ b/Gemfile @@ -40,8 +40,15 @@ gem "thruster", require: false # Use Active Storage variants [https://guides.rubyonrails.org/active_storage_overview.html#transforming-images] gem "image_processing", "~> 1.2" -# Conversation layer: LLM chat + tools (generator UI) -gem "ruby_llm" +# Conversation layer: LLM chat + tools (generator UI). +# +# 2.0 is unreleased — it exists only as crmne/ruby_llm@main, which still +# reports VERSION '1.16.0' because the bump happens at release. Pinned to a +# SHA rather than tracking main: main is explicitly "in development", and +# BUNDLE_DEPLOYMENT=1 plus HIFUMI_AGENT_IMAGE reusing this image mean the +# resolved revision has to be a reviewed decision. Swap to a version +# constraint once 2.0 ships to RubyGems. +gem "ruby_llm", github: "crmne/ruby_llm", ref: "c45ebd78c819b83696849a3486619d671dbafab6" # Orchestration of generation workflows (per-revision Roast subprocess) gem "roast-ai", "~> 1.1" diff --git a/Gemfile.lock b/Gemfile.lock index 26cc050..65a70b2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,3 +1,19 @@ +GIT + remote: https://github.com/crmne/ruby_llm.git + revision: c45ebd78c819b83696849a3486619d671dbafab6 + ref: c45ebd78c819b83696849a3486619d671dbafab6 + specs: + ruby_llm (1.16.0) + base64 + event_stream_parser (~> 1) + faraday (>= 1.10.0) + faraday-multipart (>= 1) + faraday-net_http (>= 1) + faraday-retry (>= 1) + marcel (~> 1) + schematist (~> 1.1) + zeitwerk (~> 2) + GEM remote: https://rubygems.org/ specs: @@ -395,21 +411,11 @@ GEM ruby-vips (2.3.0) ffi (~> 1.12) logger - ruby_llm (1.15.0) - base64 - event_stream_parser (~> 1) - faraday (>= 1.10.0) - faraday-multipart (>= 1) - faraday-net_http (>= 1) - faraday-retry (>= 1) - marcel (~> 1) - ruby_llm-schema (~> 0) - zeitwerk (~> 2) - ruby_llm-schema (0.3.0) rubyzip (3.3.0) sawyer (0.9.3) addressable (>= 2.3.5) faraday (>= 0.17.3, < 3) + schematist (1.1.0) securerandom (0.4.1) selenium-webdriver (4.44.0) base64 (~> 0.2) @@ -529,7 +535,7 @@ DEPENDENCIES rails (~> 8.1.3) roast-ai (~> 1.1) rubocop-rails-omakase - ruby_llm + ruby_llm! selenium-webdriver solid_cable solid_cache @@ -688,10 +694,10 @@ CHECKSUMS rubocop-rails-omakase (1.1.0) sha256=2af73ac8ee5852de2919abbd2618af9c15c19b512c4cfc1f9a5d3b6ef009109d ruby-progressbar (1.13.0) sha256=80fc9c47a9b640d6834e0dc7b3c94c9df37f08cb072b7761e4a71e22cff29b33 ruby-vips (2.3.0) sha256=e685ec02c13969912debbd98019e50492e12989282da5f37d05f5471442f5374 - ruby_llm (1.15.0) sha256=ca207465bca1cca007010a79fce500d4012b1fbe188b025040cd37b884fb98af - ruby_llm-schema (0.3.0) sha256=a591edc5ca1b7f0304f0e2261de61ba4b3bea17be09f5cf7558153adfda3dec6 + ruby_llm (1.16.0) rubyzip (3.3.0) sha256=a372fc67892a4f8c0bc8ec906b720353d8e48807a64b2e63adf99b1e3583a034 sawyer (0.9.3) sha256=0d0f19298408047037638639fe62f4794483fb04320269169bd41af2bdcf5e41 + schematist (1.1.0) sha256=905d8f0286830926e77e0d631bee325ee741c4b99640bc99735593b40ba73e7b securerandom (0.4.1) sha256=cc5193d414a4341b6e225f0cb4446aceca8e50d5e1888743fac16987638ea0b1 selenium-webdriver (4.44.0) sha256=6f1df072529af369589c46f0e01132952aabb250cfd683c274d74dc1eb5d8477 snaky_hash (2.0.6) sha256=3663cae48cdef582b517025cf8a39d8789996eaf0b4ed89e2f0624836505654a diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 789c9e0..668807a 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -63,7 +63,7 @@ def active_revisions_for(project) end def build_chat_events(project) - messages = project.chat.messages.includes(:tool_calls).to_a + messages = project.chat.messages.includes(:ruby_llm_tool_calls).to_a status_instructions = project.instructions .where(phase: %w[completed failed]) .to_a diff --git a/app/helpers/messages_helper.rb b/app/helpers/messages_helper.rb index 5dc16b9..abf77c0 100644 --- a/app/helpers/messages_helper.rb +++ b/app/helpers/messages_helper.rb @@ -6,13 +6,16 @@ def message_row_class(message) end def tool_call_pill_text(message) - call = message.tool_calls.first + # v2: message.tool_calls is a Hash keyed by provider tool-call id, whose + # values are RubyLLM::ToolCall. The persisted rows are ruby_llm_tool_calls. + calls = message.tool_calls.values + call = calls.first case call&.name when "create_application", "modify_application" intent = call.arguments["intent"].to_s intent.empty? ? "🌀 Build started" : "🌀 Build started: #{intent}" else - "running: #{message.tool_calls.map(&:name).uniq.join(", ")}" + "running: #{calls.map(&:name).uniq.join(", ")}" end end diff --git a/app/jobs/execute_instruction_job.rb b/app/jobs/execute_instruction_job.rb index 0edfe44..02437ae 100644 --- a/app/jobs/execute_instruction_job.rb +++ b/app/jobs/execute_instruction_job.rb @@ -114,17 +114,38 @@ def init_rails_app(workspace) # already a+rw inside the tenant boundary and generated apps ship empty # credentials. def relax_workspace_permissions(workspace) - # force: chmod_R stats every entry it walked, and anything git drops in - # between (a lock file from background housekeeping, an index refresh) - # raises ENOENT and aborts the whole walk — failing the revision. Workspaces - # created before the maintenance.auto/gc.auto config above still exist, so - # this is not redundant with it. Relaxation is best-effort by nature: a - # genuinely unchmoddable file surfaces at the next operation instead. - FileUtils.chmod_R("a+rwX", workspace, force: true) + disable_git_housekeeping(workspace) + + # force: covers the per-entry chmod, and nothing else — FileUtils wraps only + # `ent.chmod` in its rescue, while the walk's own `Dir.children` sits + # outside it. So a *file* git drops mid-walk is survivable but a *directory* + # is not (gc prunes empty .git/objects/ fanouts), and ENOENT still + # aborts the whole walk and fails the revision. Retry once against a settled + # tree; relaxation is best-effort by nature, so a second failure is not + # worth failing a revision for — a genuinely unchmoddable file surfaces at + # the next operation with a clearer error than a half-finished walk. + attempts = 0 + begin + FileUtils.chmod_R("a+rwX", workspace, force: true) + rescue Errno::ENOENT + retry if (attempts += 1) < 2 + end master_key_path = File.join(workspace, "config/master.key") File.chmod(0o644, master_key_path) if File.exist?(master_key_path) end + # init_rails_app sets these at creation, but every workspace made before that + # landed still generates the lock files that race the walk above — and those + # are all of production's. Idempotent, and two git configs are nothing beside + # a roast run measured in minutes. + def disable_git_housekeeping(workspace) + return unless File.directory?(File.join(workspace, ".git")) + + %w[maintenance.auto gc.auto].zip(%w[false 0]).each do |key, value| + system("git", "-C", workspace, "config", key, value, out: File::NULL, err: File::NULL) + end + end + # Cwd for rails new / git / bundle is outside this repo (Project.workspace_root), # so the frum shim can't resolve Ruby from .ruby-version there. Prepend the # pinned Ruby's bin dir to PATH the same way bin/roast does. diff --git a/app/models/chat.rb b/app/models/chat.rb index 486dee7..e772cf5 100644 --- a/app/models/chat.rb +++ b/app/models/chat.rb @@ -2,13 +2,4 @@ class Chat < ApplicationRecord acts_as_chat belongs_to :project, touch: true - - # `acts_as_chat` (use_new_acts_as = true) delegates with_temperature / - # with_thinking / with_params / with_headers / with_schema to to_llm but - # not with_context — patch it in here, mirroring the same shape so the - # AR record's chat lifecycle stays intact. - def with_context(context) - to_llm.with_context(context) - self - end end diff --git a/app/models/message.rb b/app/models/message.rb index 66a91d2..b147eca 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -13,7 +13,7 @@ class Message < ApplicationRecord def visible_in_chat? return false if system_injected? return true if role == "user" - role == "assistant" && (content.to_s.strip.present? || tool_calls.any?) + role == "assistant" && (content.to_s.strip.present? || tool_call?) end private diff --git a/app/models/model.rb b/app/models/model.rb deleted file mode 100644 index 781a38d..0000000 --- a/app/models/model.rb +++ /dev/null @@ -1,3 +0,0 @@ -class Model < ApplicationRecord - acts_as_model -end diff --git a/app/models/tool_call.rb b/app/models/tool_call.rb deleted file mode 100644 index 1db3ed0..0000000 --- a/app/models/tool_call.rb +++ /dev/null @@ -1,20 +0,0 @@ -class ToolCall < ApplicationRecord - acts_as_tool_call - - # RubyLLM attaches tool_calls AFTER the parent message is saved, so the - # message's own after_update_commit re-broadcasts before tool_calls exist. - # Touch the parent here to trigger another replace once the call is persisted - # — the re-render then sees message.tool_calls.any? and renders the pill. - # - # Skip on :destroy. RubyLLM's cleanup_failed_messages destroys the parent - # message on API failure; the cascading ToolCall destroy would otherwise - # try to touch an already-destroyed record and raise ActiveRecordError, - # masking the original API error. - after_commit :touch_message, on: [ :create, :update ] - - private - - def touch_message - message&.touch - end -end diff --git a/app/schemas/plan_schema.rb b/app/schemas/plan_schema.rb index a1ab119..34cb65e 100644 --- a/app/schemas/plan_schema.rb +++ b/app/schemas/plan_schema.rb @@ -1,4 +1,9 @@ -class PlanSchema < RubyLLM::Schema +# Schematist ships as a RubyLLM v2 runtime dependency, but the gem only +# `require`s it from lazily-loaded files (tool.rb, agent.rb), so the constant +# is not guaranteed to exist when Rails autoloads this class. +require "schematist" + +class PlanSchema < Schematist::Schema string :instruction_description, description: "One-sentence human description of the whole plan." diff --git a/app/services/plan_application_creation/ad_hoc_llm.rb b/app/services/plan_application_creation/ad_hoc_llm.rb index a44aaab..1ddd98d 100644 --- a/app/services/plan_application_creation/ad_hoc_llm.rb +++ b/app/services/plan_application_creation/ad_hoc_llm.rb @@ -14,7 +14,7 @@ def self.invoke_llm(system:, user:, openrouter_api_key:, model:) ctx = RubyLLM.context { |c| c.openrouter_api_key = openrouter_api_key } chat = ctx.chat(model: model) chat.with_instructions(system) - chat.with_schema(PlanSchema).ask(user).content + chat.with_schema(PlanSchema).ask(user).parsed end def self.build_user_prompt(intent, clarifications, _context) @@ -28,8 +28,19 @@ def self.build_user_prompt(intent, clarifications, _context) def self.build_result(content) raise InvalidResponse, "LLM returned no content" if content.nil? + # Message#parsed is JSON.parse: valid JSON that isn't an object gets + # through it, and `Array(array_or_number["revisions"])` then raises + # TypeError — which CreateApplication#execute does not rescue, leaving a + # persisted tool_use with no tool_result and a permanently dead chat. + raise InvalidResponse, "expected a JSON object, got #{content.class}" unless content.is_a?(Hash) revisions = Array(content["revisions"]).map do |r| + # The Hash check above only covers the top level. A valid JSON object + # whose `revisions` hold non-objects raises NoMethodError (String#fetch) + # or TypeError (Array#fetch, via Array(hash) => [[k, v]]) here — neither + # of which #execute rescues, and both of which orphan the tool_use. + raise InvalidResponse, "expected revision objects, got #{r.class}" unless r.is_a?(Hash) + { summary: r.fetch("summary"), prompt: r.fetch("prompt") } end raise InvalidResponse, "empty revisions" if revisions.empty? diff --git a/app/services/plan_application_modification/ad_hoc_llm.rb b/app/services/plan_application_modification/ad_hoc_llm.rb index af0fbe7..611dcd6 100644 --- a/app/services/plan_application_modification/ad_hoc_llm.rb +++ b/app/services/plan_application_modification/ad_hoc_llm.rb @@ -14,7 +14,7 @@ def self.invoke_llm(system:, user:, openrouter_api_key:, model:) ctx = RubyLLM.context { |c| c.openrouter_api_key = openrouter_api_key } chat = ctx.chat(model: model) chat.with_instructions(system) - chat.with_schema(PlanSchema).ask(user).content + chat.with_schema(PlanSchema).ask(user).parsed end def self.build_user_prompt(intent, clarifications, _context) @@ -28,8 +28,19 @@ def self.build_user_prompt(intent, clarifications, _context) def self.build_result(content) raise InvalidResponse, "LLM returned no content" if content.nil? + # Message#parsed is JSON.parse: valid JSON that isn't an object gets + # through it, and `Array(array_or_number["revisions"])` then raises + # TypeError — which ModifyApplication#execute does not rescue, leaving a + # persisted tool_use with no tool_result and a permanently dead chat. + raise InvalidResponse, "expected a JSON object, got #{content.class}" unless content.is_a?(Hash) revisions = Array(content["revisions"]).map do |r| + # The Hash check above only covers the top level. A valid JSON object + # whose `revisions` hold non-objects raises NoMethodError (String#fetch) + # or TypeError (Array#fetch, via Array(hash) => [[k, v]]) here — neither + # of which #execute rescues, and both of which orphan the tool_use. + raise InvalidResponse, "expected revision objects, got #{r.class}" unless r.is_a?(Hash) + { summary: r.fetch("summary"), prompt: r.fetch("prompt") } end raise InvalidResponse, "empty revisions" if revisions.empty? diff --git a/app/tools/create_application.rb b/app/tools/create_application.rb index 1b2f9bf..9576f01 100644 --- a/app/tools/create_application.rb +++ b/app/tools/create_application.rb @@ -4,7 +4,7 @@ def name = "create_application" "Call this only when the project has no application yet (workspace is empty). " \ "The user must have explicitly confirmed they're ready to start before you call this." - params do + parameters do string :intent, description: "Plain-language description of what the user wants, e.g. 'flower shop with inventory and Stripe'." object :clarifications, @@ -65,8 +65,20 @@ def execute(intent:, clarifications: {}) revision_count: result.revisions.size, instruction_description: result.instruction_description } - rescue PlanApplicationCreation::AdHocLLM::InvalidResponse => e + rescue PlanApplicationCreation::AdHocLLM::InvalidResponse, JSON::ParserError => e + # JSON::ParserError: v2's Message#parsed raises on a malformed plan where v1 + # silently kept the String. Nothing may escape #execute — an exception here + # leaves a persisted tool_use with no tool_result and the chat is finished. { error: "Could not generate a plan: #{e.message}. Ask the user to rephrase." } + rescue StandardError => e + # Backstop for the same invariant. A rescue list cannot be complete: the + # revisions loop can raise NoMethodError/TypeError on an off-schema plan, + # and `revisions.create!` can raise RecordInvalid on a blank summary or + # prompt. RubyLLM's own orphan cleanup does not cover any of these — it + # only fires for RubyLLM/Faraday/Timeout errors — so anything reaching + # here would kill the chat permanently. Report it rather than swallow it. + Rails.error.report(e, handled: true, context: { project_id: @project.id }) + { error: "Could not generate a plan. Ask the user to rephrase." } end private diff --git a/app/tools/modify_application.rb b/app/tools/modify_application.rb index e17f844..8e3e8b4 100644 --- a/app/tools/modify_application.rb +++ b/app/tools/modify_application.rb @@ -4,7 +4,7 @@ def name = "modify_application" "Call this when the project already has a generated application and the user wants a change. " \ "The user must have explicitly confirmed they're ready to apply the change before you call this." - params do + parameters do string :intent, description: "Plain-language description of the change the user wants, e.g. 'make the primary color teal'." object :clarifications, @@ -65,8 +65,20 @@ def execute(intent:, clarifications: {}) revision_count: result.revisions.size, instruction_description: result.instruction_description } - rescue PlanApplicationModification::AdHocLLM::InvalidResponse => e + rescue PlanApplicationModification::AdHocLLM::InvalidResponse, JSON::ParserError => e + # JSON::ParserError: v2's Message#parsed raises on a malformed plan where v1 + # silently kept the String. Nothing may escape #execute — an exception here + # leaves a persisted tool_use with no tool_result and the chat is finished. { error: "Could not generate a modification plan: #{e.message}. Ask the user to rephrase." } + rescue StandardError => e + # Backstop for the same invariant. A rescue list cannot be complete: the + # revisions loop can raise NoMethodError/TypeError on an off-schema plan, + # and `revisions.create!` can raise RecordInvalid on a blank summary or + # prompt. RubyLLM's own orphan cleanup does not cover any of these — it + # only fires for RubyLLM/Faraday/Timeout errors — so anything reaching + # here would kill the chat permanently. Report it rather than swallow it. + Rails.error.report(e, handled: true, context: { project_id: @project.id }) + { error: "Could not generate a modification plan. Ask the user to rephrase." } end private diff --git a/app/views/messages/_message.html.erb b/app/views/messages/_message.html.erb index fc63f16..2849679 100644 --- a/app/views/messages/_message.html.erb +++ b/app/views/messages/_message.html.erb @@ -4,7 +4,7 @@
<%= message.role %>
- <% if message.role == "assistant" && message.tool_calls.any? %> + <% if message.role == "assistant" && message.tool_call? %>
<%= tool_call_pill_text(message) %>
<% end %> <% if message.content.to_s.strip.present? %> diff --git a/bin/inspect-chat b/bin/inspect-chat index 2110019..c377004 100755 --- a/bin/inspect-chat +++ b/bin/inspect-chat @@ -12,8 +12,15 @@ require_relative "../config/environment" project_id = ARGV.first || abort("usage: bin/inspect-chat ") project = Project.find(project_id) chat = project.chat -messages = chat.messages.includes(:tool_calls).order(:id).to_a -tcs_by_id = ToolCall.where(id: messages.map(&:tool_call_id).compact).index_by(&:id) + +# RubyLLM v2 owns the rows: ruby_llm_tool_calls, reached through the +# ruby_llm_tool_calls / ruby_llm_parent_tool_call associations. The +# Message#tool_calls reader returns a Hash of value objects instead, and +# the tool_use ↔ tool_result link now lives on the row (result_id), not on +# messages.tool_call_id — which the upgrade migration dropped. +messages = chat.messages + .includes(:ruby_llm_tool_calls, :ruby_llm_parent_tool_call) + .order(:id).to_a puts "Project ##{project.id} (#{project.name}) — chat ##{chat.id} — #{messages.size} messages" puts "=" * 80 @@ -22,19 +29,12 @@ messages.each_with_index do |m, i| preview = m.content.to_s.gsub(/\s+/, " ").strip[0, 80] puts "[#{i}] msg_id=#{m.id} role=#{m.role.ljust(9)} content=#{preview.inspect}" - if m.tool_calls.any? - m.tool_calls.each do |tc| - puts " tool_use: name=#{tc.name} provider_id=#{tc.tool_call_id} (tc_row=#{tc.id})" - end + m.ruby_llm_tool_calls.each do |tc| + puts " tool_use: name=#{tc.name} provider_id=#{tc.tool_call_id} (tc_row=#{tc.id})" end - if m.tool_call_id - tc = tcs_by_id[m.tool_call_id] - if tc - puts " tool_result: tc_row=#{tc.id} provider_id=#{tc.tool_call_id} parent_msg=#{tc.message_id}" - else - puts " tool_result: tc_row=#{m.tool_call_id} ← MISSING tool_calls row (orphan)" - end + if (parent = m.ruby_llm_parent_tool_call) + puts " tool_result: tc_row=#{parent.id} provider_id=#{parent.tool_call_id} parent_msg=#{parent.message_id}" end end @@ -45,24 +45,27 @@ puts "-" * 80 issues = [] messages.each_with_index do |m, i| next unless m.role == "tool" - prev = messages[i - 1] - tc = tcs_by_id[m.tool_call_id] - unless tc - issues << "[#{i}] msg #{m.id}: tool_result references missing tool_calls.id=#{m.tool_call_id}" + parent = m.ruby_llm_parent_tool_call + unless parent + issues << "[#{i}] msg #{m.id}: tool_result with no ruby_llm_tool_calls row (orphan)" next end - parent_msg_id = tc.message_id - if prev.nil? || prev.id != parent_msg_id - issues << "[#{i}] msg #{m.id}: tool_result for tc #{tc.tool_call_id} expects parent msg=#{parent_msg_id}, but previous is msg=#{prev&.id}" + # i.zero? guard: messages[-1] would wrap to the LAST message, so a chat that + # opens with an orphaned tool result — exactly what this script hunts for — + # would be reported against the wrong parent instead of as having none. + prev = i.zero? ? nil : messages[i - 1] + if prev.nil? || prev.id != parent.message_id + issues << "[#{i}] msg #{m.id}: tool_result for tc #{parent.tool_call_id} expects parent msg=#{parent.message_id}, but previous is msg=#{prev&.id}" end end # Assistant tool_calls with no following tool message messages.each_with_index do |m, i| - next unless m.role == "assistant" && m.tool_calls.any? - next_msgs = messages[i + 1..(i + m.tool_calls.size)] || [] - m.tool_calls.each do |tc| - matched = next_msgs.any? { |n| n.role == "tool" && n.tool_call_id == tc.id } + calls = m.ruby_llm_tool_calls + next unless m.role == "assistant" && calls.any? + next_msgs = messages[i + 1..(i + calls.size)] || [] + calls.each do |tc| + matched = next_msgs.any? { |n| n.role == "tool" && n.id == tc.result_id } unless matched issues << "[#{i}] msg #{m.id}: assistant tool_use #{tc.tool_call_id} (#{tc.name}) has no following tool result" end diff --git a/bin/verify-model-registry b/bin/verify-model-registry index b252796..8cc322c 100755 --- a/bin/verify-model-registry +++ b/bin/verify-model-registry @@ -12,16 +12,11 @@ # On a deployed host: # kamal app exec --reuse "bin/verify-model-registry" # -# Why this exists: `use_new_acts_as` + app/models/model.rb (acts_as_model) make -# RubyLLM prefer the `models` DB table over the gem's bundled models.json, and -# fall back to the JSON only when that table is EMPTY. A partially-populated -# table therefore shadows the JSON completely — on 2026-08-12 both dev and -# production held a single row (haiku only), so the Sonnet 4.6 and Opus 4.6 -# selections raised ModelNotFoundError on every RubyLLM-backed stage. -# -# The fix is to populate the table (`Model.refresh!`); this script is how you -# confirm it. Pass candidate ids to check a model BEFORE adding it to -# LLM::Stages::AVAILABLE_MODELS. +# Why this exists: with acts_as installed, RubyLLM v2 uses the ruby_llm_models +# table as its registry store and falls back to the gem's bundled models.json +# only when that table is empty (models.rb:85-97). A model absent from both +# raises ModelNotFoundError on every RubyLLM-backed stage. Pass candidate ids +# to check a model BEFORE adding it to LLM::Stages::AVAILABLE_MODELS. # # See docs/05-runbooks/03-llm-model-registry.md. # @@ -32,9 +27,37 @@ require_relative "../config/environment" candidates = ARGV offered = LLM::Stages::AVAILABLE_MODELS.keys failures = [] +store_underpopulated = false + +# RubyLLM wires config.model_registry_store to the ruby_llm_models table from an +# `on_load :active_record` hook. This script boots via require "config/environment" +# rather than `bin/rails runner`, and that path does not touch ActiveRecord — so +# without the line below the store is nil and RubyLLM::Models would silently +# resolve against the gem's bundled JSON instead of the DB, which is the exact +# confusion this script exists to prevent. Referencing ActiveRecord::Base fires +# the hook on its own; a full eager_load! is not needed. +ActiveRecord::Base.connection_pool +store = RubyLLM.config.model_registry_store +rows = store ? store.count : 0 + +puts "registry store rows: #{store ? rows : "(no store configured)"}" +puts "resolved registry: #{RubyLLM::Models.instance.all.size}" -puts "models table rows: #{Model.count}" -puts "resolved registry: #{RubyLLM::Models.instance.all.size}" +# Three states, only one of which is healthy. A NON-EMPTY store shadows the +# bundled registry completely, and find_or_create_model writes one row per +# successful resolution -- so a fresh environment self-poisons: the first chat +# leaves exactly one row, and from the next boot that row IS the registry. +# Resolving everything from the bundle is therefore not a pass, it is a warning. +if rows.zero? + puts "\nWARNING: the store is empty, so the ids below resolve from the gem's" + puts "bundled registry. The first chat will write ONE row, and that row will" + puts "then shadow the bundle entirely. Run RubyLLM.models.refresh! before use." +elsif rows < offered.size + puts "\nFAIL: the store holds #{rows} row(s) but #{offered.size} ids are offered." + puts "A non-empty store shadows the bundle, so ids missing from it raise" + puts "ModelNotFoundError even when the bundle carries them." + store_underpopulated = true +end def check(id, failures) info, = RubyLLM::Models.resolve(id) @@ -54,7 +77,10 @@ end if failures.any? puts "\n#{failures.size} id(s) did not resolve: #{failures.join(', ')}" - puts "Populate the registry with: bin/rails runner 'Model.refresh!'" +end + +if failures.any? || store_underpopulated + puts "Populate the registry with: bin/rails runner 'RubyLLM.models.refresh!'" puts "Then restart long-lived processes — the registry is memoized per process." exit 1 end diff --git a/config/initializers/ruby_llm.rb b/config/initializers/ruby_llm.rb index c6d04a2..9aa85ad 100644 --- a/config/initializers/ruby_llm.rb +++ b/config/initializers/ruby_llm.rb @@ -13,5 +13,4 @@ config.openrouter_api_key = ENV["OPENROUTER_API_KEY"].presence || "placeholder-overridden-per-user-via-with_context" config.default_model = "anthropic/claude-haiku-4.5" - config.use_new_acts_as = true end diff --git a/db/migrate/20260822224622_add_ruby_llm_v2_0_columns.rb b/db/migrate/20260822224622_add_ruby_llm_v2_0_columns.rb new file mode 100644 index 0000000..4daef25 --- /dev/null +++ b/db/migrate/20260822224622_add_ruby_llm_v2_0_columns.rb @@ -0,0 +1,347 @@ +# Class name deviates from `bin/rails generate ruby_llm:upgrade` output +# (AddRubyLlmV20Columns): this app declares `inflect.acronym "LLM"`, so Rails +# camelizes this filename to AddRubyLLMV20Columns and rejects the generated name. +class AddRubyLLMV20Columns < ActiveRecord::Migration[8.1] + LEGACY_TOKEN_COLUMNS = %w[input_tokens output_tokens cache_read_tokens cache_write_tokens thinking_tokens].freeze + LEGACY_COST_COLUMNS = %w[total_cost cost_details].freeze + + def up + add_chat_and_message_columns + move_models + move_tool_calls + move_batches + create_usage_entries + backfill_usage_entries + remove_legacy_message_columns + end + + def down + raise ActiveRecord::IrreversibleMigration, + "RubyLLM v2 moves application-owned records into RubyLLM-owned tables" + end + + private + + def add_chat_and_message_columns + add_column :chats, :cancelled, :boolean, null: false, default: false unless column_exists?(:chats, :cancelled) + + add_column :messages, :citations, :json unless column_exists?(:messages, :citations) + unless column_exists?(:messages, :server_tool_calls) + add_column :messages, :server_tool_calls, :json + end + add_column :messages, :raw_content, :json unless column_exists?(:messages, :raw_content) + add_column :messages, :raw_reasoning, :json unless column_exists?(:messages, :raw_reasoning) + add_column :messages, :finish_reason, :string unless column_exists?(:messages, :finish_reason) + unless column_exists?(:messages, :cache_until_here) + add_column :messages, :cache_until_here, :boolean, null: false, default: false + end + if column_exists?(:messages, :cached_tokens) && + !column_exists?(:messages, :cache_read_tokens) + rename_column :messages, :cached_tokens, :cache_read_tokens + end + if column_exists?(:messages, :cache_creation_tokens) && + !column_exists?(:messages, :cache_write_tokens) + rename_column :messages, :cache_creation_tokens, :cache_write_tokens + end + end + + def move_models + move_table(:models, :ruby_llm_models) { create_models } + normalize_chat_model_reference(:chats, :model_id) + replace_message_model_reference(:messages, :model_id) + end + + def create_models + create_table :ruby_llm_models do |t| + t.string :model_id, null: false + t.string :name, null: false + t.string :provider, null: false + t.string :family + t.datetime :model_created_at + t.integer :context_window + t.integer :max_output_tokens + t.date :knowledge_cutoff + + t.json :modalities, default: {} + t.json :capabilities, default: [] + t.json :pricing, default: {} + t.json :metadata, default: {} + + t.timestamps + t.index [ :provider, :model_id ], unique: true + t.index :provider + t.index :family + end + end + + def normalize_chat_model_reference(table, legacy_column) + return unless column_exists?(table, legacy_column) + + column = connection.columns(table).find { |candidate| candidate.name == legacy_column.to_s } + model_primary_key = connection.primary_key(:ruby_llm_models) + model_key_column = connection.columns(:ruby_llm_models).find { |candidate| candidate.name == model_primary_key } + unless column.type == model_key_column.type + raise "Expected #{table}.#{legacy_column} to match ruby_llm_models.#{model_primary_key}" + end + + if legacy_column != :ruby_llm_model_id + remove_foreign_key table, column: legacy_column if foreign_key_exists?(table, column: legacy_column) + remove_index table, legacy_column if index_exists?(table, legacy_column) + rename_column table, legacy_column, :ruby_llm_model_id + end + + add_index table, :ruby_llm_model_id unless index_exists?(table, :ruby_llm_model_id) + unless foreign_key_exists?(table, :ruby_llm_models, column: :ruby_llm_model_id) + add_foreign_key table, :ruby_llm_models, column: :ruby_llm_model_id + end + end + + def replace_message_model_reference(table, legacy_column) + return unless column_exists?(table, legacy_column) + + add_column table, :provider, :string unless column_exists?(table, :provider) + column = connection.columns(table).find { |candidate| candidate.name == legacy_column.to_s } + + if column.type == :string + rename_column table, legacy_column, :model_id if legacy_column != :model_id + add_index table, [ :provider, :model_id ] unless index_exists?(table, [ :provider, :model_id ]) + return + end + + add_column table, :ruby_llm_model_id, :string + records = migration_record(table) + migration_record(:ruby_llm_models).find_each do |model| + records.where(legacy_column => model.id).update_all( + ruby_llm_model_id: model.model_id, + provider: model.provider + ) + end + + remove_foreign_key table, column: legacy_column if foreign_key_exists?(table, column: legacy_column) + remove_index table, legacy_column if index_exists?(table, legacy_column) + remove_column table, legacy_column + rename_column table, :ruby_llm_model_id, :model_id + add_index table, [ :provider, :model_id ] unless index_exists?(table, [ :provider, :model_id ]) + end + + def move_tool_calls + move_table(:tool_calls, :ruby_llm_tool_calls) { create_tool_calls } + normalize_tool_calls + end + + def create_tool_calls + create_table :ruby_llm_tool_calls do |t| + t.references :message, polymorphic: true, null: false, type: :bigint, index: false + t.references :result, polymorphic: true, type: :bigint, index: false + t.string :tool_call_id, null: false + t.string :name, null: false + t.text :thought_signature + t.string :approval + + t.json :arguments, default: {} + + t.timestamps + end + end + + def normalize_tool_calls + table = :ruby_llm_tool_calls + legacy_message_column = :message_id + + if legacy_message_column != :message_id && column_exists?(table, legacy_message_column) + remove_foreign_key table, column: legacy_message_column if foreign_key_exists?(table, column: legacy_message_column) + rename_column table, legacy_message_column, :message_id + end + remove_foreign_key table, column: :message_id if foreign_key_exists?(table, column: :message_id) + + add_column table, :message_type, :string unless column_exists?(table, :message_type) + migration_record(table).where(message_type: nil).update_all(message_type: 'Message') + change_column_null table, :message_type, false + + add_column table, :result_type, :string unless column_exists?(table, :result_type) + add_column table, :result_id, :bigint unless column_exists?(table, :result_id) + add_column table, :approval, :string unless column_exists?(table, :approval) + backfill_tool_results + + add_index table, [ :message_type, :message_id ] unless index_exists?(table, [ :message_type, :message_id ]) + add_index table, [ :result_type, :result_id ] unless index_exists?(table, [ :result_type, :result_id ]) + deduplicate_tool_call_ids unless index_exists?(table, :tool_call_id) + add_index table, :tool_call_id, unique: true unless index_exists?(table, :tool_call_id) + add_index table, :name unless index_exists?(table, :name) + end + + # Provider tool-call ids are only unique within one request, so legacy data + # can repeat them across chats. The unique index needs them globally unique; + # renamed rows keep their result links, which join by primary key. + def deduplicate_tool_call_ids + tool_calls = migration_record(:ruby_llm_tool_calls) + duplicated = tool_calls.group(:tool_call_id).having("COUNT(*) > 1").pluck(:tool_call_id) + duplicated.each do |tool_call_id| + tool_calls.where(tool_call_id: tool_call_id).order(:id).offset(1).each do |record| + record.update!(tool_call_id: "#{tool_call_id}-migrated-#{record.id}") + end + end + end + + def backfill_tool_results + result_column = :tool_call_id + return unless column_exists?(:messages, result_column) + + messages = migration_record(:messages) + tool_calls = migration_record(:ruby_llm_tool_calls) + messages.where.not(result_column => nil).pluck(messages.primary_key, result_column).each do |message_id, tool_call_id| + tool_calls.where(tool_calls.primary_key => tool_call_id).update_all( + result_id: message_id, + result_type: 'Message' + ) + end + + remove_foreign_key :messages, column: result_column if foreign_key_exists?(:messages, column: result_column) + remove_index :messages, result_column if index_exists?(:messages, result_column) + remove_column :messages, result_column + end + + def move_batches + move_table(:batches, :ruby_llm_batches) { create_batches } + add_column :ruby_llm_batches, :chat_type, :string unless column_exists?(:ruby_llm_batches, :chat_type) + add_column :ruby_llm_batches, :batch_protocol, :string unless column_exists?(:ruby_llm_batches, :batch_protocol) + add_column :ruby_llm_batches, :request_counts, :json unless column_exists?(:ruby_llm_batches, :request_counts) + migration_record(:ruby_llm_batches).where(chat_type: nil).update_all(chat_type: 'Chat') + add_index :ruby_llm_batches, [ :provider, :provider_batch_id ], unique: true unless index_exists?(:ruby_llm_batches, [ :provider, :provider_batch_id ]) + add_index :ruby_llm_batches, :status unless index_exists?(:ruby_llm_batches, :status) + end + + def create_batches + create_table :ruby_llm_batches do |t| + t.string :provider_batch_id, null: false + t.string :provider, null: false + t.string :status + t.boolean :completed, null: false, default: false + t.string :chat_type + t.string :batch_protocol + + t.json :chat_ids, default: [] + + t.json :request_counts + t.timestamps + end + end + + def create_usage_entries + return if table_exists?(:ruby_llm_usages) + + create_table :ruby_llm_usages do |t| + t.references :chat, polymorphic: true, null: false, type: :bigint, index: false + t.references :message, polymorphic: true, type: :bigint, index: false + t.string :operation, null: false + t.string :provider, null: false + t.string :model, null: false + t.string :status, null: false + t.integer :input_tokens + t.integer :output_tokens + t.integer :cache_read_tokens + t.integer :cache_write_tokens + t.integer :thinking_tokens + t.decimal :input_cost, precision: 16, scale: 10 + t.decimal :output_cost, precision: 16, scale: 10 + t.decimal :cache_read_cost, precision: 16, scale: 10 + t.decimal :cache_write_cost, precision: 16, scale: 10 + t.decimal :thinking_cost, precision: 16, scale: 10 + t.decimal :total_cost, precision: 16, scale: 10 + t.timestamps + t.index [ :chat_type, :chat_id ] + t.index [ :message_type, :message_id ] + t.index :status + t.check_constraint "operation IN ('chat', 'embedding', 'moderation', 'image', 'speech', 'transcription', 'ocr', 'rerank')" + t.check_constraint "status IN ('pending', 'succeeded', 'failed', 'cancelled')" + end + end + + def backfill_usage_entries + present = LEGACY_TOKEN_COLUMNS.select { |column| column_exists?(:messages, column) } + return if present.empty? + + entries = migration_record(:ruby_llm_usages) + condition = present.map { |column| "#{column} IS NOT NULL" }.join(' OR ') + migration_record(:messages).where(condition).find_each do |message| + provider, model = message_model_identity(message) + next unless provider && model + + entries.create!(legacy_usage_attributes(message, provider:, model:)) + end + end + + def message_model_identity(message) + identity = [ message['provider'], message['model_id'] ] + return identity if identity.all? + + model_identities[chat_model_ids[message['chat_id']]] + end + + def model_identities + @model_identities ||= migration_record(:ruby_llm_models).all.to_h do |model| + [ model.id, [ model['provider'], model['model_id'] ] ] + end + end + + def chat_model_ids + @chat_model_ids ||= if column_exists?(:chats, :ruby_llm_model_id) + migration_record(:chats).pluck(:id, :ruby_llm_model_id).to_h + else + {} + end + end + + def legacy_usage_attributes(message, provider:, model:) + details = message['cost_details'] + details = JSON.parse(details) if details.is_a?(String) + details ||= {} + { + chat_type: 'Chat', + chat_id: message['chat_id'], + message_type: 'Message', + message_id: message.id, + operation: 'chat', + provider: provider, + model: model, + status: 'succeeded', + input_tokens: message['input_tokens'], + output_tokens: message['output_tokens'], + cache_read_tokens: message['cache_read_tokens'], + cache_write_tokens: message['cache_write_tokens'], + thinking_tokens: message['thinking_tokens'], + input_cost: details['input'], + output_cost: details['output'], + cache_read_cost: details['cache_read'], + cache_write_cost: details['cache_write'], + thinking_cost: details['thinking'], + total_cost: message['total_cost'] || details['total'], + created_at: message['created_at'], + updated_at: message['updated_at'] + } + end + + def remove_legacy_message_columns + (LEGACY_TOKEN_COLUMNS + LEGACY_COST_COLUMNS).each do |column| + remove_column :messages, column if column_exists?(:messages, column) + end + end + + def move_table(source, target) + if source != target && table_exists?(source) + if table_exists?(target) + raise "Both #{source} and #{target} exist. Merge or remove one before running this migration." + end + rename_table source, target + elsif !table_exists?(target) + yield + end + end + + def migration_record(table) + Class.new(ActiveRecord::Base) do + self.table_name = table.to_s + self.inheritance_column = :_type_disabled + end + end +end diff --git a/db/schema.rb b/db/schema.rb index c758a91..23d9e39 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_06_11_175719) do +ActiveRecord::Schema[8.1].define(version: 2026_08_22_224622) do create_table "active_storage_attachments", force: :cascade do |t| t.bigint "blob_id", null: false t.datetime "created_at", null: false @@ -40,12 +40,13 @@ end create_table "chats", force: :cascade do |t| + t.boolean "cancelled", default: false, null: false t.datetime "created_at", null: false - t.integer "model_id" t.integer "project_id", null: false + t.integer "ruby_llm_model_id" t.datetime "updated_at", null: false - t.index ["model_id"], name: "index_chats_on_model_id" t.index ["project_id"], name: "index_chats_on_project_id" + t.index ["ruby_llm_model_id"], name: "index_chats_on_ruby_llm_model_id" end create_table "contact_messages", force: :cascade do |t| @@ -83,46 +84,26 @@ end create_table "messages", force: :cascade do |t| - t.integer "cache_creation_tokens" - t.integer "cached_tokens" + t.boolean "cache_until_here", default: false, null: false t.integer "chat_id", null: false + t.json "citations" t.text "content" t.json "content_raw" t.datetime "created_at", null: false - t.integer "input_tokens" - t.integer "model_id" - t.integer "output_tokens" + t.string "finish_reason" + t.string "model_id" + t.string "provider" + t.json "raw_content" + t.json "raw_reasoning" t.string "role", null: false + t.json "server_tool_calls" t.boolean "system_injected", default: false, null: false t.text "thinking_signature" t.text "thinking_text" - t.integer "thinking_tokens" - t.integer "tool_call_id" t.datetime "updated_at", null: false t.index ["chat_id"], name: "index_messages_on_chat_id" - t.index ["model_id"], name: "index_messages_on_model_id" + t.index ["provider", "model_id"], name: "index_messages_on_provider_and_model_id" t.index ["role"], name: "index_messages_on_role" - t.index ["tool_call_id"], name: "index_messages_on_tool_call_id" - end - - create_table "models", force: :cascade do |t| - t.json "capabilities", default: [] - t.integer "context_window" - t.datetime "created_at", null: false - t.string "family" - t.date "knowledge_cutoff" - t.integer "max_output_tokens" - t.json "metadata", default: {} - t.json "modalities", default: {} - t.datetime "model_created_at" - t.string "model_id", null: false - t.string "name", null: false - t.json "pricing", default: {} - t.string "provider", null: false - t.datetime "updated_at", null: false - t.index ["family"], name: "index_models_on_family" - t.index ["provider", "model_id"], name: "index_models_on_provider_and_model_id", unique: true - t.index ["provider"], name: "index_models_on_provider" end create_table "profiles", force: :cascade do |t| @@ -184,17 +165,87 @@ t.index ["project_id"], name: "index_revisions_on_project_id" end - create_table "tool_calls", force: :cascade do |t| + create_table "ruby_llm_batches", force: :cascade do |t| + t.string "batch_protocol" + t.json "chat_ids", default: [] + t.string "chat_type" + t.boolean "completed", default: false, null: false + t.datetime "created_at", null: false + t.string "provider", null: false + t.string "provider_batch_id", null: false + t.json "request_counts" + t.string "status" + t.datetime "updated_at", null: false + t.index ["provider", "provider_batch_id"], name: "index_ruby_llm_batches_on_provider_and_provider_batch_id", unique: true + t.index ["status"], name: "index_ruby_llm_batches_on_status" + end + + create_table "ruby_llm_models", force: :cascade do |t| + t.json "capabilities", default: [] + t.integer "context_window" + t.datetime "created_at", null: false + t.string "family" + t.date "knowledge_cutoff" + t.integer "max_output_tokens" + t.json "metadata", default: {} + t.json "modalities", default: {} + t.datetime "model_created_at" + t.string "model_id", null: false + t.string "name", null: false + t.json "pricing", default: {} + t.string "provider", null: false + t.datetime "updated_at", null: false + t.index ["family"], name: "index_ruby_llm_models_on_family" + t.index ["provider", "model_id"], name: "index_ruby_llm_models_on_provider_and_model_id", unique: true + t.index ["provider"], name: "index_ruby_llm_models_on_provider" + end + + create_table "ruby_llm_tool_calls", force: :cascade do |t| + t.string "approval" t.json "arguments", default: {} t.datetime "created_at", null: false t.integer "message_id", null: false + t.string "message_type", null: false t.string "name", null: false + t.bigint "result_id" + t.string "result_type" t.text "thought_signature" t.string "tool_call_id", null: false t.datetime "updated_at", null: false - t.index ["message_id"], name: "index_tool_calls_on_message_id" - t.index ["name"], name: "index_tool_calls_on_name" - t.index ["tool_call_id"], name: "index_tool_calls_on_tool_call_id", unique: true + t.index ["message_id"], name: "index_ruby_llm_tool_calls_on_message_id" + t.index ["message_type", "message_id"], name: "index_ruby_llm_tool_calls_on_message_type_and_message_id" + t.index ["name"], name: "index_ruby_llm_tool_calls_on_name" + t.index ["result_type", "result_id"], name: "index_ruby_llm_tool_calls_on_result_type_and_result_id" + t.index ["tool_call_id"], name: "index_ruby_llm_tool_calls_on_tool_call_id", unique: true + end + + create_table "ruby_llm_usages", force: :cascade do |t| + t.decimal "cache_read_cost", precision: 16, scale: 10 + t.integer "cache_read_tokens" + t.decimal "cache_write_cost", precision: 16, scale: 10 + t.integer "cache_write_tokens" + t.bigint "chat_id", null: false + t.string "chat_type", null: false + t.datetime "created_at", null: false + t.decimal "input_cost", precision: 16, scale: 10 + t.integer "input_tokens" + t.bigint "message_id" + t.string "message_type" + t.string "model", null: false + t.string "operation", null: false + t.decimal "output_cost", precision: 16, scale: 10 + t.integer "output_tokens" + t.string "provider", null: false + t.string "status", null: false + t.decimal "thinking_cost", precision: 16, scale: 10 + t.integer "thinking_tokens" + t.decimal "total_cost", precision: 16, scale: 10 + t.datetime "updated_at", null: false + t.index ["chat_type", "chat_id"], name: "index_ruby_llm_usages_on_chat_type_and_chat_id" + t.index ["message_type", "message_id"], name: "index_ruby_llm_usages_on_message_type_and_message_id" + t.index ["status"], name: "index_ruby_llm_usages_on_status" + t.check_constraint "operation IN ('chat', 'embedding', 'moderation', 'image', 'speech', 'transcription', 'ocr', 'rerank')" + t.check_constraint "status IN ('pending', 'succeeded', 'failed', 'cancelled')" end create_table "users", force: :cascade do |t| @@ -211,18 +262,15 @@ add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" - add_foreign_key "chats", "models" add_foreign_key "chats", "projects", on_delete: :cascade + add_foreign_key "chats", "ruby_llm_models" add_foreign_key "github_connections", "users" add_foreign_key "instructions", "messages", column: "anchor_message_id", on_delete: :cascade add_foreign_key "instructions", "projects", on_delete: :cascade add_foreign_key "messages", "chats" - add_foreign_key "messages", "models" - add_foreign_key "messages", "tool_calls" add_foreign_key "profiles", "users" add_foreign_key "projects", "users" add_foreign_key "revisions", "instructions", on_delete: :cascade add_foreign_key "revisions", "projects", on_delete: :cascade add_foreign_key "revisions", "revisions", column: "parent_id", on_delete: :nullify - add_foreign_key "tool_calls", "messages" end diff --git a/docs/02-architecture/03-tech-stack.md b/docs/02-architecture/03-tech-stack.md index bdc72b4..a91c636 100644 --- a/docs/02-architecture/03-tech-stack.md +++ b/docs/02-architecture/03-tech-stack.md @@ -178,7 +178,7 @@ Separate list — gems used in the generator itself, not in generated apps. | Gem | What for | |-----|----------| -| `ruby_llm` | Conversation layer, chat, tools | +| `ruby_llm` | Conversation layer, chat, tools. Git-pinned to `crmne/ruby_llm@c45ebd78` (the unreleased 2.0 line; see `docs/05-runbooks/04-ruby-llm-v2-rollout.md`) — swap to a version constraint once 2.0 ships to RubyGems. Brings `schematist`, which replaced `ruby_llm-schema` as the structured-output DSL | | `roast-ai` (~> 1.1) | Orchestration of generation workflows. **Requires Ruby >= 3.3.** | | `devise` | Auth for generator users | | `solid_queue` | Background jobs (generation, preview) | diff --git a/docs/05-runbooks/03-llm-model-registry.md b/docs/05-runbooks/03-llm-model-registry.md index 0efa000..931a6bc 100644 --- a/docs/05-runbooks/03-llm-model-registry.md +++ b/docs/05-runbooks/03-llm-model-registry.md @@ -5,17 +5,18 @@ procedure for adding a new model to the picker. ## Why this is needed -`config/initializers/ruby_llm.rb` sets `use_new_acts_as = true`, and -`app/models/model.rb` declares `acts_as_model`. That combination monkey-patches -`RubyLLM::Models.load_models` to read the `models` DB table and fall back to the -gem's bundled `models.json` **only when that table is empty** -(`ruby_llm/active_record/acts_as.rb`). A partially-populated table therefore -shadows the JSON completely. - -The table fills itself one row at a time: `ChatMethods#resolve_model_from_strings` -does `find_or_create_by!` after each *successful* resolution. So the first -success writes one row, and from then on that single row is the entire registry — -every other model, including ones present in the bundled JSON, stops resolving. +RubyLLM owns its model registry and stores it in the `ruby_llm_models` table. +The gem's railtie wires `config.model_registry_store` to +`RubyLLM::ActiveRecord::Model` whenever ActiveRecord loads, and +`Models.load_models` reads that store first, falling back to the gem's bundled +`models.json` **only when the table is empty** (`ruby_llm/models.rb:85-97`, +logging *"Model registry store is empty, falling back to the registry file"*). + +So the store wins whenever it holds any rows at all: a **non-empty store +shadows the bundle completely**, and an id missing from it raises +`ModelNotFoundError` even when the bundle carries that id. The bundle only ever +serves a store that is entirely empty — which is also why a fresh environment +self-poisons, since each successful resolution writes one row back. Affected stages are the four RubyLLM-backed ones (chat, plan_creation, plan_modification, template). `code` and `docs` are unaffected: they pass the id @@ -26,17 +27,24 @@ banner *"The configured model is unavailable. Contact the operator."* (`ChatRespondJob::FRIENDLY_ERRORS`); on the template stage `ExecuteInstructionJob` has no rescue, so the job fails into Solid Queue. -`Model.refresh!` (`RubyLLM.models.refresh!` + `save_to_database`) is the fix. It -fetches live from every configured provider, and persists with -`find_or_initialize_by(model_id:, provider:) + update!` inside a transaction — -**no deletes**, so `chats.model_id` foreign keys survive. Use it rather than -`bin/rails ruby_llm:load_models`, which only reloads the *bundled* JSON and so -lags the live catalogue. +`RubyLLM.models.refresh!` is the fix. It fetches the published registry +(`rubyllm.com/models.json`), merges per-provider discovery over it, and persists +through the store with `find_or_initialize_by(model_id:, provider:) + update!` +inside a transaction — **no deletes**, so `chats.ruby_llm_model_id` foreign keys +survive. Use it rather than `bin/rails ruby_llm:load_models`, which only reloads +the *bundled* JSON and so lags the live catalogue. + +**It is failure-safe.** Per-provider fetches are rescued individually into a +`failed` list (`models.rb:151-164`); models belonging to a failed provider are +carried over unchanged and the failure is logged *"Keeping existing."* +(`:222-230`). A models.dev outage degrades the same way. So a refresh can only +add or update rows, never empty the registry — which is why production needs no +real OpenRouter key for this (see below). ## Local ```bash -bin/rails runner 'Model.refresh!; puts Model.count' +bin/rails runner 'RubyLLM.models.refresh!; puts RubyLLM.config.model_registry_store.count' bin/verify-model-registry ``` @@ -51,10 +59,15 @@ provider fetch is what populates the ids. ```bash # 1. Baseline (read-only) -kamal app exec --reuse "bin/rails runner 'puts Model.count'" +kamal app exec --reuse "bin/rails runner 'puts RubyLLM.config.model_registry_store.count'" # 2. Populate -kamal app exec --reuse "bin/rails runner 'Model.refresh!; puts Model.count'" +# Run this during a quiet window: refresh! wraps every row in ONE transaction, +# and v2 fetches every provider's registry (1464 rows, not v1's 410). That holds +# the write lock on production.sqlite3 while users are chatting, and +# config/database.yml sets timeout: 5000 — a chat write that waits longer than +# 5s raises SQLite3::BusyException. +kamal app exec --reuse "bin/rails runner 'RubyLLM.models.refresh!; puts RubyLLM.config.model_registry_store.count'" # 3. Verify. bin/verify-model-registry only exists in the image once it has been # deployed; until then use the inline equivalent below. @@ -62,7 +75,7 @@ kamal app exec --reuse "bin/verify-model-registry" kamal app exec --reuse "bin/rails runner 'LLM::Stages::AVAILABLE_MODELS.keys.each { |id| begin; RubyLLM::Models.resolve(id); puts %(OK #{id}); rescue => e; puts %(FAIL #{id} #{e.class}); end }'" # 4. Restart — required; see the warning below -V=$(kamal app version | tail -1) +V=$(kamal app version | sed -n '2p') # line 2: kamal prints "App Host:" first, and tail -1 is blank kamal app stop kamal app start --version="$V" ``` @@ -70,8 +83,10 @@ kamal app start --version="$V" No `OPENROUTER_API_KEY` is needed. The container has no global key (BYOK is per-user), so the provider sends the placeholder from `config/initializers/ruby_llm.rb` as its bearer token — and OpenRouter's -`/api/v1/models` returns 200 regardless of auth. Do **not** pass a real key -inline: kamal echoes the full `docker exec` command into its own log output. +`/api/v1/models` returns 200 regardless of auth. Even if it did not, the refresh +degrades gracefully rather than emptying the store (see above). Do **not** pass a +real key inline: kamal echoes the full `docker exec` command into its own log +output. Step 4 is required because `RubyLLM::Models.instance` is memoized per process (`@instance ||= new`); the running Puma keeps the stale registry until replaced. @@ -128,9 +143,11 @@ raises the moment a user selects it. ## Standing caveats -- **Test environment** has an empty `models` table, so it falls back to the - bundled JSON — which in ruby_llm 1.15.0 contains no Claude 5 ids. Stub - `ctx.chat` in new tests rather than resolving a 5-family id for real. +- **Test environment** has an empty `ruby_llm_models` table, so it falls back to + the bundled JSON. At the pinned `c45ebd78` that bundle carries + `anthropic/claude-sonnet-5` but **not** `anthropic/claude-opus-5`, and no test + resolves either. Stub `ctx.chat` in new tests rather than resolving a + 5-family id for real. - **Local dev codegen ignores per-project selection by design.** `bin/roast-claudesubscription` gets the bare aliases `sonnet` (code) and `haiku` (docs), so the model that actually runs is whatever the operator's @@ -142,9 +159,21 @@ raises the moment a user selects it. ## Recorded baseline -**2026-08-12** — both environments were found holding a single `models` row +**2026-08-23** (post-v2, local) — the upgrade migration carried the 410 rows +over to `ruby_llm_models` unchanged, and the first `RubyLLM.models.refresh!` +under v2 took the store to **1464 rows**. The jump is expected and is the one +number that changed meaning: v1's refresh only wrote what OpenRouter discovery +returned, while v2 fetches the *published* registry (`rubyllm.com/models.json`, +every provider) and merges provider discovery over it. Two consequences when +comparing counts: the **migration** preserves the row count exactly, a +**refresh** grows it — so attribute any change to whichever step you just ran. +`PRAGMA foreign_key_check` stayed empty across both, confirming the no-deletes +claim above against real `chats.ruby_llm_model_id` rows. + +**2026-08-12** (pre-v2, when the table was still `models`) — both environments +were found holding a single row (`openrouter anthropic/claude-haiku-4.5`), with `anthropic/claude-sonnet-4.6` and -`anthropic/claude-opus-4.6` raising `ModelNotFoundError`. After `Model.refresh!`: +`anthropic/claude-opus-4.6` raising `ModelNotFoundError`. After a refresh: 410 rows in each, all three offered ids resolving, and `anthropic/claude-opus-5` / `-sonnet-5` / `-fable-5` resolving as candidates (1M context each). Production restarted at version diff --git a/docs/05-runbooks/04-ruby-llm-v2-rollout.md b/docs/05-runbooks/04-ruby-llm-v2-rollout.md new file mode 100644 index 0000000..d17bc51 --- /dev/null +++ b/docs/05-runbooks/04-ruby-llm-v2-rollout.md @@ -0,0 +1,317 @@ +# Runbook 04 — RubyLLM v2 production rollout + +Deploy the RubyLLM 2.0 upgrade to hifumi.dev, and recover if it goes wrong. + +## Why this needs a runbook + +The upgrade migration **renames tables in place and has no `down`** +(`db/migrate/20260822224622_add_ruby_llm_v2_0_columns.rb` — `raise +ActiveRecord::IrreversibleMigration`). And it runs **by itself**: +`bin/docker-entrypoint:4-5` runs `db:prepare` whenever the command is +`./bin/rails server`, so **`kamal deploy` is what triggers it**. There is no +manual gate between deploying and migrating. + +Two consequences drive every step below: + +1. **The snapshot happens before the deploy, not after.** Once containers boot, + the old schema is gone. +2. **Rollback is code *and* data.** The pre-upgrade image cannot read + `ruby_llm_models` / `ruby_llm_tool_calls`, and the upgraded image cannot read + `models` / `tool_calls`. Restoring one without the other leaves production + broken either way. + +## The WAL constraint + +`config/database.yml` sets no `journal_mode`, so Rails 8's SQLite adapter runs +WAL — verified locally: `PRAGMA journal_mode` returns `wal`, and +`storage/production.sqlite3-wal` / `-shm` sit beside the database file. + +- **Snapshots are safe on a live database.** `.backup` uses SQLite's online + backup API and writes **one self-contained file, no sidecars** (verified: + a `.backup` of a live WAL database produced exactly one file). +- **Restores are not.** Copying a file back while SQLite holds it open leaves + the *other* database's WAL in place, and the next open replays it onto the + file you just restored. + +So: every restore in this runbook **stops the process first and deletes the +`-wal` / `-shm` sidecars**. No exceptions, including the local rehearsal. + +`sqlite3` is available inside the container — it is installed in the Dockerfile's +`base` stage (`Dockerfile:22`), which the runtime stage inherits (`Dockerfile:110`). +Verified in the built image: `sqlite3 3.46.1`. + +## 1. Rehearse locally on production data + +The only way to know the migration survives real rows. Do this before the deploy, +not as a formality. + +```bash +# 1.1 Snapshot production's primary database. Only the primary needs it: +# cache/queue/cable are separate databases with their own migrations_paths +# (config/database.yml), and the upgrade migration lives in db/migrate. +kamal app exec --reuse \ + "sqlite3 /rails/storage/production.sqlite3 \".backup '/rails/storage/production.sqlite3.pre-v2'\"" + +# 1.2 Copy it down. Reading straight off the volume needs no running container. +ssh root@77.42.95.154 "docker run --rm -v hifumi_dev_storage:/s -v /tmp:/out alpine \ + cp /s/production.sqlite3.pre-v2 /out/production.sqlite3.pre-v2" +scp root@77.42.95.154:/tmp/production.sqlite3.pre-v2 /tmp/production.sqlite3.pre-v2 + +# 1.3 STOP bin/dev. Everything below swaps the primary database file out from +# under Rails, and a running Puma/Solid Queue holds an open WAL. + +# 1.4 Park your own dev database, then move the snapshot into its place. +# hifumi is multi-database, so DATABASE_URL cannot reliably point one +# command at one file — swapping the primary's file is unambiguous. +# Drop the sidecars after each copy: they belong to the file being +# replaced, not to the one arriving. +sqlite3 storage/development.sqlite3 ".backup 'storage/development.sqlite3.mine'" +cp /tmp/production.sqlite3.pre-v2 storage/development.sqlite3 +rm -f storage/development.sqlite3-wal storage/development.sqlite3-shm + +# 1.5 Record the before counts, migrate (TIMED), record the after counts. +sqlite3 storage/development.sqlite3 "SELECT COUNT(*) FROM models;" # before +sqlite3 storage/development.sqlite3 \ + "SELECT COUNT(*) FROM messages WHERE input_tokens IS NOT NULL + OR output_tokens IS NOT NULL OR cache_read_tokens IS NOT NULL + OR cache_write_tokens IS NOT NULL OR thinking_tokens IS NOT NULL;" # before + +time bin/rails db:migrate +# -> This is the go/no-go number. Kamal's deploy_timeout defaults to 30s and +# config/deploy.yml does not override it, so a migration slower than that +# is reported as a FAILED deploy while still running on production. On +# SQLite every remove_column/rename_column goes through alter_table, which +# copies the whole table twice; `messages` is hit 12 times, so runtime +# scales with roughly 24x its row count, not 1x. Over ~10s here: set +# deploy_timeout in config/deploy.yml before deploying. + +sqlite3 storage/development.sqlite3 "SELECT COUNT(*) FROM ruby_llm_models;" # after — MUST match +sqlite3 storage/development.sqlite3 "SELECT COUNT(*) FROM ruby_llm_usages;" # after — MUST match +# -> The usages count MUST equal the token-column count above. +# backfill_usage_entries SKIPS any message whose provider/model_id are +# null and whose chat has no ruby_llm_model_id, and +# remove_legacy_message_columns then drops the source columns. A shortfall +# is silent, irreversible data loss and nothing else in this runbook +# catches it. + +# 1.6 Structural checks. Both must be clean. +sqlite3 storage/development.sqlite3 "PRAGMA integrity_check; PRAGMA foreign_key_check;" +sqlite3 storage/development.sqlite3 \ + "SELECT COUNT(*) FROM ruby_llm_tool_calls WHERE message_type IS NULL;" # MUST be 0 + +# 1.7 Every offered model still resolves against the rehearsed copy. +bin/verify-model-registry + +# 1.8 Give yourself your dev database back — same sidecar rule. +cp storage/development.sqlite3.mine storage/development.sqlite3 +rm -f storage/development.sqlite3-wal storage/development.sqlite3-shm +``` + +**What to expect.** The migration completes in well under a second on a database +this size (0.68s locally against 410 models / 423 messages / 56 tool calls) and +preserves row counts exactly: `models` → `ruby_llm_models` is a rename, not a +rebuild. `ruby_llm_usages` is created and gets one row per message that carried +token data. `ruby_llm_batches` is created empty. + +> **Do not confuse this count with a refresh.** The *migration* preserves the +> `ruby_llm_models` row count exactly. A `RubyLLM.models.refresh!` **grows** it — +> v1's refresh wrote only OpenRouter discovery, while v2 fetches the published +> registry for every provider (410 → 1464 locally on first refresh). If the count +> changed, attribute it to whichever step you just ran. See runbook 03. + +If either of step 1.5's paired counts differ, or 1.6 reports anything, **stop** — do not +deploy. Production has data shapes the rehearsal just found and the plan did not. + +## 2. Before deploying + +```bash +# 2.0 Check your shell FIRST. Both `kamal deploy` and `kamal app boot` +# re-source .kamal/secrets from the caller's environment (`kamal app start` +# does not — it reuses the container's env). SMTP_PASSWORD and +# GITHUB_CLIENT_SECRET come from your shell, so a local dev export ships +# silently to production. This has happened (2026-05-15). +# +# Prefixes cannot tell a stale key from a live one — every Resend key +# starts `re_` — so compare hashes against what production runs now. +for v in SMTP_PASSWORD GITHUB_CLIENT_SECRET; do + mine=$(printenv "$v" | shasum | cut -c1-8) + live=$(kamal app exec --reuse "sh -c 'printenv $v | shasum | cut -c1-8'" | sed -n '2p') + echo "$v shell=$mine prod=$live" +done +# -> MUST match. A mismatch means your shell would overwrite production's +# value. Fix your environment before going further. + +# 2.1 Capture the currently-running version. You need this to roll back, and +# it is unavailable once the new one is running. +# +# Take line 2, not `tail -1`: kamal prints "App Host: " first and ends +# with a blank line, so `tail -1` returns EMPTY. An empty --version is not +# `.presence`, so Kamal falls through to the LOCAL git SHA — which during a +# rollback is the v2 image, and booting that against a restored v1 database +# re-runs the one-way migration over the snapshot you just restored. +PREV=$(kamal app version | sed -n '2p') +[ -n "$PREV" ] || echo "FAILED to capture the running version — do not deploy" +echo "PREV=$PREV" # write this down as well; $PREV dies with your shell +``` + +> Any pipe applied to **kamal's own stdout** has to skip the `App Host:` line. +> Pipes *inside* a quoted container command (`kamal app exec --reuse "... | tail -3"`) +> run in the container and are unaffected. + +```bash +# 2.2 Check for chats parked mid-tool-round. backfill_tool_results moves +# messages.tool_call_id onto ruby_llm_tool_calls.result_id and then drops +# the column, so a turn sitting between tool_use and tool_result is +# simplest to complete or cancel beforehand. +kamal app exec --primary "bin/inspect-chat " +``` + +Anything reported as `has no following tool result` on an *active* project is +worth finishing or cancelling first. Historical ones are fine — they migrate as +they are. + +## 3. Deploy + +```bash +# 3.1 Snapshot. The deploy itself migrates, so this is the last moment. +# (If the rehearsal snapshot from 1.1 is still fresh, this replaces it.) +kamal app exec --reuse \ + "sqlite3 /rails/storage/production.sqlite3 \".backup '/rails/storage/production.sqlite3.pre-v2'\"" + +# 3.2 Pre-build, so the outage below is boot+migrate rather than +# build+push+boot+migrate. +kamal build push + +# 3.3 STOP the app before anything migrates. `kamal deploy` on its own boots +# the new container, migrates, and only routes traffic once it is healthy — +# leaving the OLD v1 container live against tables the migration has just +# renamed. In that window `GET /projects/:id` raises +# "no such table: tool_calls" on every project page, and because +# SOLID_QUEUE_IN_PUMA is true a v1 ChatRespondJob writes dropped columns; +# if its assistant(tool_use) already persisted, the tool_result never lands +# and that chat is permanently dead. A planned minute of downtime is +# cheaper. Do not skip this step to save it. +kamal app stop + +# 3.4 Deploy. db:prepare runs the migration during boot, with nothing attached. +kamal deploy + +# 3.5 If `kamal deploy` exited NON-ZERO, do NOT assume nothing happened. Kamal +# stops the NEW container on a failed healthcheck, but the migration has +# already committed — leaving the pre-upgrade image on a v2 schema, the one +# state neither image can read. Check before deciding, and go to section 5 +# if it ran. +kamal app exec --reuse "bin/rails db:migrate:status | tail -3" + +# 3.6 Confirm the migration ran cleanly rather than assuming it did. +kamal app logs --lines 100 + +# 3.4 Confirm the schema and the registry on the running container. +kamal app exec --reuse "bin/rails db:migrate:status | tail -3" +kamal app exec --reuse "bin/verify-model-registry" +``` + +`HIFUMI_AGENT_IMAGE` points at the same image tag (`config/deploy.yml:37`), so +sandboxed codegen containers pick up the new bundle with no separate action. + +## 4. Verify + +Exercise the four RubyLLM-backed stages in order — they fail independently, and +a passing chat says nothing about the planners. + +1. **chat** — send a message; the reply streams. +2. **plan_creation** — ask for an app and confirm; the pill renders and an + `Instruction` with `Revision` rows appears. +3. **template** — let the build reach the template step; no `InvalidPick`. +4. **plan_modification** — ask for a change on a built project; a plan comes back. + +Then, on a real project: + +```bash +kamal app exec --primary "bin/inspect-chat " # expect: no structural issues +``` + +## 5. Recover + +There is no `down`. Rollback is **code and data, in that order: stop, restore, +boot.** + +```bash +# 5.1 Stop the app so nothing holds the database open. +kamal app stop + +# 5.2 Restore over the volume, with the sidecars removed. This runs without a +# container, which is the point — see the WAL constraint above. +ssh root@77.42.95.154 "docker run --rm -v hifumi_dev_storage:/s alpine sh -c \ + 'rm -f /s/production.sqlite3-wal /s/production.sqlite3-shm && \ + cp /s/production.sqlite3.pre-v2 /s/production.sqlite3'" + +# 5.3 Bring back the pre-upgrade version (PREV, from step 2.1). db:prepare +# finds the v1 schema and no pending migration, so it is a no-op. +# 5.3 Bring back the pre-upgrade version (PREV, from step 2.1). An EMPTY +# --version resolves to the local git SHA, i.e. the v2 image, which would +# re-run the one-way migration over the database you just restored. +[ -n "$PREV" ] || echo "refusing: PREV is unset — recover it before continuing" +kamal app start --version="$PREV" # preferred: reuses the existing container +``` + +Only if that container is gone — note the secrets warning below, which applies +to `boot` but not to `start`: + +```bash +kamal app boot --version="$PREV" +``` + +```bash +# 5.4 Confirm you are on v1 code AND v1 data. Without this the failure mode in +# 5.3 is silent: the site comes back up either way. +kamal app exec --reuse "bin/rails db:migrate:status | tail -3" +# -> 20260822224622 MUST read "down" +kamal app exec --reuse \ + "sqlite3 /rails/storage/production.sqlite3 'SELECT COUNT(*) FROM models;'" +# -> MUST succeed. "no such table: models" means you re-migrated. + +# 5.5 Discard jobs enqueued under v2. Solid Queue lives in its own database +# (config/database.yml), so the restore above did not touch it, and those +# jobs now point at rows the restored primary does not have. They will not +# discard themselves — `discard_on ActiveJob::DeserializationError` is +# commented out in app/jobs/application_job.rb — they pile into +# solid_queue_failed_executions instead. +kamal app exec --reuse \ + "bin/rails runner 'SolidQueue::Job.where(finished_at: nil).delete_all'" +``` + +> ⚠️ **Do not restore via `kamal app exec --reuse`.** It needs a running +> container — exactly the state that makes the copy unsafe. + +> ⚠️ **Bare `kamal app start` leaves production down.** It looks for an +> unversioned container name, fails to match the real versioned one, and stops +> there. Always pass `--version`. (`kamal app restart` does not exist in Kamal +> 2.11.0.) + +> ⚠️ **`kamal app boot` re-sources `.kamal/secrets` from your shell.** +> `SMTP_PASSWORD` and `GITHUB_CLIENT_SECRET` are read from the caller's +> environment, so a local export can silently push the wrong value to +> production — this has happened before (2026-05-15). Prefer `app start`, and if +> you must use `boot`, check your shell first: +> `echo "${SMTP_PASSWORD:0:4} ${GITHUB_CLIENT_SECRET:0:4}"`. + +### If you only realise later + +The snapshot at `/rails/storage/production.sqlite3.pre-v2` stays on the volume +until deleted, so 5.1–5.3 remain available. What it costs is **everything +written since the deploy** — chats, projects, instructions. Past a few minutes of +real traffic, rolling forward with a fix is usually better than rolling back; +weigh that before running 5.2. + +## Future + +- **Once 2.0 ships to RubyGems**: replace the git pin in `Gemfile` with a version + constraint, `bundle update ruby_llm`, redeploy. No migration involved — the + schema work is done. +- **Re-pinning before then**: the pin is a reviewed decision, not a moving + target. `main` moves daily; re-run the compatibility checks recorded in + `thoughts/shared/plans/2026-08-19/ruby-llm-v2-upgrade.md` ("Working Against + the Pinned SHA") against any candidate SHA before bumping it. +- **`ruby_llm_usages` grows one row per provider attempt.** Nothing reads it + yet. Worth revisiting if it becomes the largest table. diff --git a/docs/09-ideas/05-followups.md b/docs/09-ideas/05-followups.md index 0d603e6..dd632ac 100644 --- a/docs/09-ideas/05-followups.md +++ b/docs/09-ideas/05-followups.md @@ -145,7 +145,7 @@ A full robustness/OSS-readiness review of the production deployment was done; fi Shipped 2026-06-11 (`feature/per-project-model-selection`): per-stage model columns on profiles (user defaults) + projects (per-project snapshot), selectors in the build tab / new-project form / account integrations pane, threaded through all six LLM stages via `LLM::Stages` (`lib/llm/stages.rb`). Deliberately deferred: -- **Curated list is 3 Anthropic models.** `LLM::Stages::AVAILABLE_MODELS` is a hand-maintained hash. The dormant `models` table (`acts_as_model`, never populated) could back a full OpenRouter catalog picker instead — needs capability filtering per stage (`structured_outputs` for plan/template, `tools` for chat) and a refresh job hitting `GET openrouter.ai/api/v1/models`. See `thoughts/shared/research/2026-05-11/per-user-model-config-per-stage.md`. +- **Curated list is 5 Anthropic models.** `LLM::Stages::AVAILABLE_MODELS` is a hand-maintained hash. A full catalog picker could read RubyLLM's own registry store instead — since the v2 upgrade that is `ruby_llm_models`, owned by the gem, already populated (410 rows) and maintained by `RubyLLM.models.refresh!`. So the work is not waking a dormant table: it is capability filtering per stage (`structured_outputs` for plan/template, `tools` for chat) plus a picker over `RubyLLM::ActiveRecord::Model`. See `thoughts/shared/research/2026-05-11/per-user-model-config-per-stage.md`. - **Code/docs stages are Anthropic-only by transport.** They run through the `claude` CLI's Anthropic API surface (`bin/roast-openrouter`); non-Anthropic ids have never been exercised there. The "direct-API Roast provider" Phase 5 candidate would lift this. - **No cost display.** The 2026-05-11 research scoped per-model pricing display next to each selector; AVAILABLE_MODELS would need pricing metadata (or the models table). @@ -183,3 +183,50 @@ So flipping the preview apex is: register a separate registrable domain (pick a - Optional cleanup: the `.preview.` sublabel is hardcoded in `public_preview_host` / `PREVIEW_HOST` / the CSP; if the new domain is preview-dedicated you could drop it (small code change, not required). **Cost**: a domain registration + the wildcard-cert setup you've now done once; the code change is ~one env var. The expense is operational (DNS + cert on a new domain), not engineering. + +--- + +## 2026-08-28 + +### RubyLLM v2 migration leftovers — both need their own migration + +Two residues of the gem-generated upgrade migration +(`db/migrate/20260822224622_add_ruby_llm_v2_0_columns.rb`). Neither is fixable +by editing that migration — it has already run — so each needs a new one, and +neither was in scope for the upgrade itself. + +- **Dead index on the hottest-written table.** The migration converts + `messages.model_id` from an integer FK into `provider` + `model_id` strings + and indexes them as `index_messages_on_provider_and_model_id`. But v2 never + writes those columns: `ChatMethods#message_attributes` assigns only + thinking/citations/server_tool_calls/raw_content/raw_reasoning/finish_reason/ + cache_until_here, and per-message model identity moved to `ruby_llm_usages`. + Measured locally after the upgrade: 241 rows carry `provider` (all from the + one-time backfill) and **all 45 written since have it NULL**. So the index is + pure insert overhead on the table that takes one row per streamed message. + Dropping it — and the columns — is safe only after confirming nothing reads + them; the backfilled values are the sole historical record of which model + answered a pre-upgrade message, so consider whether that is worth keeping + before dropping the columns as well. + Also leftover: `index_ruby_llm_tool_calls_on_message_id` survives the rename + alongside the new `(message_type, message_id)` composite, which fully covers + it — the gem only ever queries with `message_type` present. + +- **The `tool_calls → messages` foreign key is gone with nothing replacing it.** + `normalize_tool_calls` drops it because the column became polymorphic, so + `ruby_llm_tool_calls.message_id` is now a bare integer. Today this is + defence-in-depth only: the `Project → chat → messages → ruby_llm_tool_calls` + `dependent: :destroy` chain still cleans up. It matters if a future cleanup + job or console fix ever uses `delete_all`, because an orphan whose provider + tool-call id later recurs would hit the UNIQUE index on `tool_call_id` and + make `persist_tool_calls` raise `RecordNotUnique` **inside** + `persist_message_completion`'s transaction, rolling back the assistant + message. A polymorphic FK can't be restored directly; the options are a + periodic orphan sweep or a partial constraint. Worth deciding before writing + any code that deletes messages outside AR callbacks. + +**`messages.content_raw` is a third migration residual.** The upgrade adds +v2's `raw_content` and leaves v1's `content_raw` in place. Nothing in the gem, +`app/`, `lib/` or `bin/` reads it, so it silently holds every pre-upgrade raw +payload under a name no v2 code path reaches. Same shape as the two above: +needs its own migration, since the upgrade migration has already run. diff --git a/lib/llm/stages.rb b/lib/llm/stages.rb index 6277838..2bfedbe 100644 --- a/lib/llm/stages.rb +++ b/lib/llm/stages.rb @@ -20,11 +20,12 @@ # `claude` CLI's Anthropic API surface, and the plan/template stages need # structured output — both rule out arbitrary OpenRouter catalog entries. # -# Adding an id here is NOT sufficient. RubyLLM must also be able to resolve it, -# which means the `models` table has to carry it in every environment — a -# partially-filled table shadows the gem's bundled registry entirely. Run -# `bin/verify-model-registry ` before shipping a new entry, and populate -# with `Model.refresh!`. Procedure: docs/05-runbooks/03-llm-model-registry.md. +# Adding an id here is NOT sufficient. RubyLLM must also be able to resolve it: +# it reads the `ruby_llm_models` table as its registry store and falls back to +# the gem's bundled registry only when that table is empty, so an id missing +# from both raises ModelNotFoundError. Run `bin/verify-model-registry ` +# before shipping a new entry, and populate with `RubyLLM.models.refresh!`. +# Procedure: docs/05-runbooks/03-llm-model-registry.md. # # Deliberate exclusions (2026-08-12), so they don't get "fixed" back in: # - `:batch` and `-fast` variants — async Batch API / premium fast mode, both diff --git a/lib/templates/picker.rb b/lib/templates/picker.rb index 48636af..2e6e02c 100644 --- a/lib/templates/picker.rb +++ b/lib/templates/picker.rb @@ -36,7 +36,16 @@ def self.pick(description:, openrouter_api_key:, model:) ctx = RubyLLM.context { |c| c.openrouter_api_key = openrouter_api_key } chat = ctx.chat(model: model) chat.with_instructions(SYSTEM_PROMPT) - content = chat.with_schema(SCHEMA).ask("Description: #{description}").content + response = chat.with_schema(SCHEMA).ask("Description: #{description}") + # Scoped to `.parsed` alone: RubyLLM decodes provider error bodies as JSON + # too, so a method-wide rescue would relabel an OpenRouter 502 that returns + # an HTML page as "picker returned malformed JSON" and send whoever reads + # the failed revision to the prompt instead of the transport. + content = begin + response.parsed + rescue JSON::ParserError => e + raise InvalidPick, "picker returned malformed JSON: #{e.message}" + end name = content.is_a?(Hash) ? content["template"] : nil raise InvalidPick, "picker returned #{content.inspect}" unless Templates::NAMES.include?(name) name diff --git a/test/controllers/projects_controller_show_test.rb b/test/controllers/projects_controller_show_test.rb index fa9a591..8ca7144 100644 --- a/test/controllers/projects_controller_show_test.rb +++ b/test/controllers/projects_controller_show_test.rb @@ -206,7 +206,7 @@ class ProjectsControllerShowTest < ActionDispatch::IntegrationTest # happen. test "a tool-call message that also carries prose renders the pill and a formatted body" do message = @chat.messages.create!(role: :assistant, content: "Starting on the **habit tracker** now.") - message.tool_calls.create!( + message.ruby_llm_tool_calls.create!( tool_call_id: "tc_show", name: "create_application", arguments: { "intent" => "habit tracker" } ) diff --git a/test/helpers/messages_helper_test.rb b/test/helpers/messages_helper_test.rb index e7f123e..718b209 100644 --- a/test/helpers/messages_helper_test.rb +++ b/test/helpers/messages_helper_test.rb @@ -8,7 +8,7 @@ class MessagesHelperTest < ActionView::TestCase end test "tool_call_pill_text renders Build started for modify_application with intent" do - @message.tool_calls.create!( + @message.ruby_llm_tool_calls.create!( tool_call_id: "tc_modify", name: "modify_application", arguments: { "intent" => "make banner green" } @@ -19,7 +19,7 @@ class MessagesHelperTest < ActionView::TestCase end test "tool_call_pill_text renders Build started for create_application with intent" do - @message.tool_calls.create!( + @message.ruby_llm_tool_calls.create!( tool_call_id: "tc_create", name: "create_application", arguments: { "intent" => "build a todo list" } @@ -30,7 +30,7 @@ class MessagesHelperTest < ActionView::TestCase end test "tool_call_pill_text falls back to generic Build started when intent is missing" do - @message.tool_calls.create!( + @message.ruby_llm_tool_calls.create!( tool_call_id: "tc_no_intent", name: "modify_application", arguments: {} @@ -40,7 +40,7 @@ class MessagesHelperTest < ActionView::TestCase end test "tool_call_pill_text falls back to running: for unknown tools" do - @message.tool_calls.create!( + @message.ruby_llm_tool_calls.create!( tool_call_id: "tc_other", name: "some_other_tool", arguments: {} diff --git a/test/jobs/chat_respond_job_test.rb b/test/jobs/chat_respond_job_test.rb index 2bce1fc..57622e5 100644 --- a/test/jobs/chat_respond_job_test.rb +++ b/test/jobs/chat_respond_job_test.rb @@ -203,8 +203,8 @@ class ChatRespondJobTest < ActiveJob::TestCase test "with_context preserves acts_as_chat persistence callbacks (assistant message persisted)" do # The plan §Phase 4 step 6 paranoia: switching agent.complete → - # agent.with_context(ctx).complete must not strip the on_new_message / - # on_end_message callbacks that acts_as_chat installs. + # agent.with_context(ctx).complete must not strip the before_message / + # after_message callbacks that acts_as_chat installs. stub_complete(chunks: [ "callback survived" ]) do perform_enqueued_jobs { ChatRespondJob.perform_now(@user_message.id) } end diff --git a/test/jobs/execute_instruction_job_test.rb b/test/jobs/execute_instruction_job_test.rb index ec108ee..be9130d 100644 --- a/test/jobs/execute_instruction_job_test.rb +++ b/test/jobs/execute_instruction_job_test.rb @@ -239,6 +239,54 @@ class ExecuteInstructionJobTest < ActiveJob::TestCase end end + test "relax_workspace_permissions survives a DIRECTORY that disappears mid-walk" do + # force: does not cover this. FileUtils wraps only ent.chmod in its rescue; + # the walk's own Dir.children is outside it, so a directory git prunes + # between the stat and the listing raises ENOENT out of chmod_R itself. + Dir.mktmpdir("hifumi-dev-vanish-dir-") do |root| + ws = File.join(root, "project_vanish_dir") + FileUtils.mkdir_p(File.join(ws, ".git/objects/ab")) + File.write(File.join(ws, "Gemfile.lock"), "GEM\n") + + raised = false + original = Dir.method(:children) + Dir.define_singleton_method(:children) do |path, **opts| + if !raised && path.to_s.end_with?(".git/objects/ab") + raised = true + raise Errno::ENOENT, path.to_s + end + original.call(path, **opts) + end + + begin + ExecuteInstructionJob.new.send(:relax_workspace_permissions, ws) + ensure + Dir.singleton_class.send(:remove_method, :children) + Dir.define_singleton_method(:children, original) + end + + assert raised, "the test must actually have exercised the vanishing-directory path" + assert_equal 0o666, File.stat(File.join(ws, "Gemfile.lock")).mode & 0o777, + "the retry must complete the walk against the settled tree" + end + end + + test "relax_workspace_permissions disables git housekeeping on pre-existing workspaces" do + Dir.mktmpdir("hifumi-dev-backfill-") do |root| + ws = File.join(root, "project_backfill") + FileUtils.mkdir_p(ws) + Dir.chdir(ws) { system("git init -q") } + + ExecuteInstructionJob.new.send(:relax_workspace_permissions, ws) + + # --local is load-bearing: test_helper injects these same two settings via + # GIT_CONFIG_* into every git subprocess, so a plain --get would report + # them as set whether or not the backfill ran at all. + assert_equal "false", `git -C #{Shellwords.escape(ws)} config --local --get maintenance.auto`.strip + assert_equal "0", `git -C #{Shellwords.escape(ws)} config --local --get gc.auto`.strip + end + end + test "relax_workspace_permissions survives an entry that disappears mid-walk" do # git's background housekeeping drops transient lock files under .git. # chmod_R lists a directory, then chmods each entry it listed — so a file diff --git a/test/lib/templates/picker_test.rb b/test/lib/templates/picker_test.rb index b82a2a3..ccb5ccc 100644 --- a/test/lib/templates/picker_test.rb +++ b/test/lib/templates/picker_test.rb @@ -25,6 +25,15 @@ class Templates::PickerTest < ActiveSupport::TestCase end end + test "pick raises InvalidPick when the response body is not valid JSON" do + stub_pick("cyber", raising: true) do + error = assert_raises(Templates::Picker::InvalidPick) do + Templates::Picker.pick(description: "x", openrouter_api_key: "sk-test", model: "anthropic/claude-haiku-4.5") + end + assert_match(/malformed JSON/, error.message) + end + end + test "pick passes the selected model to the chat" do stub_pick("cyber") do |captured| Templates::Picker.pick(description: "x", openrouter_api_key: "sk-test", model: "anthropic/claude-opus-4.6") @@ -94,15 +103,23 @@ def in_workspace # Replace RubyLLM.context for the duration of the block. The fake context # mirrors the real chain (`chat → with_instructions → with_schema → ask - # → content`) so Templates::Picker.pick exercises its real code path, + # → parsed`) so Templates::Picker.pick exercises its real code path, # only the LLM endpoint is swapped. Minitest 6 dropped Object#stub, so we # use the same singleton-method-swap pattern as verify_revision_test.rb. - def stub_pick(value) + def stub_pick(value, raising: false) fake_content = value.nil? ? nil : { "template" => value, "reasoning" => "stub" } fake_chat = Object.new fake_chat.define_singleton_method(:with_instructions) { |_| self } fake_chat.define_singleton_method(:with_schema) { |_| self } - fake_chat.define_singleton_method(:ask) { |_| Struct.new(:content).new(fake_content) } + fake_chat.define_singleton_method(:ask) do |_| + response = Struct.new(:content, :parsed).new(fake_content&.to_json, fake_content) + # `parsed` is a plain struct member, so it can never raise on its own — + # only an override reaches Picker.pick's JSON::ParserError branch. + if raising + response.define_singleton_method(:parsed) { raise JSON::ParserError, "unexpected token at 'not json'" } + end + response + end captured = {} fake_ctx = Object.new diff --git a/test/models/message_test.rb b/test/models/message_test.rb index 6ea14a7..7afa372 100644 --- a/test/models/message_test.rb +++ b/test/models/message_test.rb @@ -49,6 +49,26 @@ class MessageTest < ActiveSupport::TestCase assert msg.visible_in_chat? end + # The tool_call? disjunct of visible_in_chat?. The one other test that puts a + # tool call on a message gives it prose too, so content.present? short-circuits + # and this branch is never reached there. If it regressed, message_row_class + # would return "hidden" and every build-started pill would vanish silently. + test "an assistant message with no prose is visible_in_chat when it carries a tool call" do + msg = @chat.messages.create!(role: :assistant, content: "") + msg.ruby_llm_tool_calls.create!( + tool_call_id: "tc_visible", name: "create_application", + arguments: { "intent" => "habit tracker" } + ) + + assert msg.visible_in_chat? + end + + test "an assistant message with neither prose nor a tool call is not visible_in_chat" do + msg = @chat.messages.create!(role: :assistant, content: "") + + refute msg.visible_in_chat? + end + test "system_injected messages do not enqueue an append broadcast" do assert_no_enqueued_jobs(only: Turbo::Streams::ActionBroadcastJob) do @chat.messages.create!(role: :user, content: "hi", system_injected: true) diff --git a/test/schemas/plan_schema_test.rb b/test/schemas/plan_schema_test.rb new file mode 100644 index 0000000..6171083 --- /dev/null +++ b/test/schemas/plan_schema_test.rb @@ -0,0 +1,25 @@ +require "test_helper" + +# These assertions pin the schema's SHAPE. They do NOT guard the +# `require "schematist"` in app/schemas/plan_schema.rb: under parallelize + +# random order, any test touching CreateApplication loads RubyLLM::Tool, which +# requires schematist for the rest of the process -- so this file resolves +# PlanSchema either way. What actually guards that require is +# `config.eager_load = ENV["CI"].present?` in config/environments/test.rb: on CI +# the app eager-loads and `class PlanSchema < Schematist::Schema` fails at +# definition time without it. Locally (eager_load off) its removal passes. +class PlanSchemaTest < ActiveSupport::TestCase + # The only test that resolves PlanSchema at all. Both AdHocLLM suites stub + # invoke_llm above the schema, so without this a missing `require + # "schematist"` or a wrong superclass only surfaces at runtime. + test "builds a JSON Schema document with the fields build_result reads" do + doc = PlanSchema.new.to_json_schema + + assert_equal "object", doc["type"] + assert_equal "string", doc.dig("properties", "instruction_description", "type") + + revisions = doc.dig("properties", "revisions") + assert_equal "array", revisions["type"] + assert_equal %w[prompt summary], revisions.dig("items", "properties").keys.sort + 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 9f77df6..102f986 100644 --- a/test/services/plan_application_creation/ad_hoc_llm_test.rb +++ b/test/services/plan_application_creation/ad_hoc_llm_test.rb @@ -2,7 +2,9 @@ class PlanApplicationCreation::AdHocLLMTest < ActiveSupport::TestCase # Stubs invoke_llm so the test drives build_result with fixture content directly, - # mimicking what RubyLLM returns from chat.with_schema(...).ask(...).content. + # mimicking what RubyLLM returns from chat.with_schema(...).ask(...).parsed -- + # v2's .content is the raw String; .parsed is the decoded Hash, and it RAISES + # on malformed JSON where v1 silently kept the String. def with_llm_response(content) captured = {} original = PlanApplicationCreation::AdHocLLM.method(:invoke_llm) @@ -74,6 +76,42 @@ def plan_fixture(name) end end + # Message#parsed is JSON.parse, so a top-level array/number/boolean reaches + # build_result intact. Without the is_a?(Hash) guard `Array(content["revisions"])` + # raises TypeError, which CreateApplication#execute does not rescue — the + # tool_use is then persisted with no tool_result and the chat is dead for good. + test "raises InvalidResponse when the response parses to a non-object" do + [ [ { "summary" => "a", "prompt" => "b" } ], 42, true, "plain string" ].each do |content| + with_llm_response(content) do + assert_raises(PlanApplicationCreation::AdHocLLM::InvalidResponse, "expected #{content.class} to be rejected") do + PlanApplicationCreation::AdHocLLM.call(intent: "x", clarifications: {}, context: {}, openrouter_api_key: "sk-or-test", model: "anthropic/claude-haiku-4.5") + end + end + end + end + + # The is_a?(Hash) guard above is TOP-LEVEL only. Each of these parses to an + # object and clears it, then raises inside the revisions map: NoMethodError + # for String/Integer/nil#fetch, TypeError for Array#fetch (a Hash reaches the + # map as [[k, v]] via Array()). Neither is rescued by #execute, so each one + # would orphan the tool_use and kill the chat permanently. + test "raises InvalidResponse when revisions hold non-objects" do + [ + [ "Add a Cart model" ], + [ 42 ], + [ nil ], + "oops", + { "summary" => "a", "prompt" => "b" } + ].each do |revisions| + content = { "instruction_description" => "d", "revisions" => revisions } + with_llm_response(content) do + assert_raises(PlanApplicationCreation::AdHocLLM::InvalidResponse, "expected #{revisions.inspect} to be rejected") do + PlanApplicationCreation::AdHocLLM.call(intent: "x", clarifications: {}, context: {}, openrouter_api_key: "sk-or-test", model: "anthropic/claude-haiku-4.5") + end + end + end + end + test "raises InvalidResponse when revisions array is empty" do with_llm_response(plan_fixture("empty_revisions.json")) do assert_raises(PlanApplicationCreation::AdHocLLM::InvalidResponse) do 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 55cf28f..5c22015 100644 --- a/test/services/plan_application_modification/ad_hoc_llm_test.rb +++ b/test/services/plan_application_modification/ad_hoc_llm_test.rb @@ -2,7 +2,9 @@ class PlanApplicationModification::AdHocLLMTest < ActiveSupport::TestCase # Stubs invoke_llm so the test drives build_result with fixture content directly, - # mimicking what RubyLLM returns from chat.with_schema(...).ask(...).content. + # mimicking what RubyLLM returns from chat.with_schema(...).ask(...).parsed -- + # v2's .content is the raw String; .parsed is the decoded Hash, and it RAISES + # on malformed JSON where v1 silently kept the String. def with_llm_response(content) captured = {} original = PlanApplicationModification::AdHocLLM.method(:invoke_llm) @@ -74,6 +76,42 @@ def plan_fixture(name) end end + # Message#parsed is JSON.parse, so a top-level array/number/boolean reaches + # build_result intact. Without the is_a?(Hash) guard `Array(content["revisions"])` + # raises TypeError, which ModifyApplication#execute does not rescue — the + # tool_use is then persisted with no tool_result and the chat is dead for good. + test "raises InvalidResponse when the response parses to a non-object" do + [ [ { "summary" => "a", "prompt" => "b" } ], 42, true, "plain string" ].each do |content| + with_llm_response(content) do + assert_raises(PlanApplicationModification::AdHocLLM::InvalidResponse, "expected #{content.class} to be rejected") do + PlanApplicationModification::AdHocLLM.call(intent: "x", clarifications: {}, context: {}, openrouter_api_key: "sk-or-test", model: "anthropic/claude-haiku-4.5") + end + end + end + end + + # The is_a?(Hash) guard above is TOP-LEVEL only. Each of these parses to an + # object and clears it, then raises inside the revisions map: NoMethodError + # for String/Integer/nil#fetch, TypeError for Array#fetch (a Hash reaches the + # map as [[k, v]] via Array()). Neither is rescued by #execute, so each one + # would orphan the tool_use and kill the chat permanently. + test "raises InvalidResponse when revisions hold non-objects" do + [ + [ "Add a Cart model" ], + [ 42 ], + [ nil ], + "oops", + { "summary" => "a", "prompt" => "b" } + ].each do |revisions| + content = { "instruction_description" => "d", "revisions" => revisions } + with_llm_response(content) do + assert_raises(PlanApplicationModification::AdHocLLM::InvalidResponse, "expected #{revisions.inspect} to be rejected") do + PlanApplicationModification::AdHocLLM.call(intent: "x", clarifications: {}, context: {}, openrouter_api_key: "sk-or-test", model: "anthropic/claude-haiku-4.5") + end + end + end + end + test "raises InvalidResponse when revisions array is empty" do with_llm_response(plan_fixture("empty_revisions.json")) do assert_raises(PlanApplicationModification::AdHocLLM::InvalidResponse) do diff --git a/test/tools/create_application_test.rb b/test/tools/create_application_test.rb index 6bb0241..2c544b4 100644 --- a/test/tools/create_application_test.rb +++ b/test/tools/create_application_test.rb @@ -109,16 +109,49 @@ def stub_create_plan(result_or_proc) ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber end - test "on unexpected error from PlanApplicationCreation: propagates and persists nothing" do - raising = ->(**) { raise RuntimeError, "upstream boom" } + test "on malformed plan JSON: returns error hash, persists nothing, no notification" do + # RubyLLM v2's Message#parsed raises instead of degrading to a String, and + # nothing may escape #execute — an exception here would leave a persisted + # tool_use with no tool_result, which permanently breaks the chat. + raising = ->(**) { raise JSON::ParserError, "unexpected token at 'not json'" } + payloads = [] + subscriber = ActiveSupport::Notifications.subscribe("instruction.requested") { |*, p| payloads << p } + result = nil assert_no_difference -> { Instruction.count } do assert_no_difference -> { Revision.count } do stub_create_plan(raising) do - assert_raises(RuntimeError) { @tool.execute(intent: "x", clarifications: {}) } + result = @tool.execute(intent: "x", clarifications: {}) + end + end + end + + assert_match(/Could not generate a plan/, result[:error]) + assert_empty payloads + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber + end + + # Was: assert_raises(RuntimeError). Propagation is precisely what orphans the + # tool_use -- and ChatRespondJob:30 rescues StandardError anyway, so the + # exception never reached an operator; it only killed the chat. #execute now + # reports and returns an error hash so the tool_use still gets a tool_result. + test "on unexpected error from PlanApplicationCreation: reports it, returns an error hash, persists nothing" do + raising = ->(**) { raise RuntimeError, "upstream boom" } + result = nil + + reports = capture_error_reports(RuntimeError) do + assert_no_difference -> { Instruction.count } do + assert_no_difference -> { Revision.count } do + stub_create_plan(raising) do + result = @tool.execute(intent: "x", clarifications: {}) + end end end end + + assert_match(/Could not generate a plan/, result[:error]) + assert_equal [ "upstream boom" ], reports.map { |r| r.error.message } end test "refuses and persists nothing when an implementing instruction already exists" do diff --git a/test/tools/modify_application_test.rb b/test/tools/modify_application_test.rb index 9d9f7d1..9574843 100644 --- a/test/tools/modify_application_test.rb +++ b/test/tools/modify_application_test.rb @@ -125,16 +125,49 @@ def stub_planner(result_or_proc) ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber end - test "on unexpected error from PlanApplicationModification: propagates and persists nothing" do - raising = ->(**) { raise RuntimeError, "upstream boom" } + test "on malformed plan JSON: returns error hash, persists nothing, no notification" do + # RubyLLM v2's Message#parsed raises instead of degrading to a String, and + # nothing may escape #execute — an exception here would leave a persisted + # tool_use with no tool_result, which permanently breaks the chat. + raising = ->(**) { raise JSON::ParserError, "unexpected token at 'not json'" } + payloads = [] + subscriber = ActiveSupport::Notifications.subscribe("instruction.requested") { |*, p| payloads << p } + result = nil assert_no_difference -> { Instruction.count } do assert_no_difference -> { Revision.count } do stub_planner(raising) do - assert_raises(RuntimeError) { @tool.execute(intent: "x", clarifications: {}) } + result = @tool.execute(intent: "x", clarifications: {}) + end + end + end + + assert_match(/Could not generate a modification plan/, result[:error]) + assert_empty payloads + ensure + ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber + end + + # Was: assert_raises(RuntimeError). Propagation is precisely what orphans the + # tool_use -- and ChatRespondJob:30 rescues StandardError anyway, so the + # exception never reached an operator; it only killed the chat. #execute now + # reports and returns an error hash so the tool_use still gets a tool_result. + test "on unexpected error from PlanApplicationModification: reports it, returns an error hash, persists nothing" do + raising = ->(**) { raise RuntimeError, "upstream boom" } + result = nil + + reports = capture_error_reports(RuntimeError) do + assert_no_difference -> { Instruction.count } do + assert_no_difference -> { Revision.count } do + stub_planner(raising) do + result = @tool.execute(intent: "x", clarifications: {}) + end end end end + + assert_match(/Could not generate a modification plan/, result[:error]) + assert_equal [ "upstream boom" ], reports.map { |r| r.error.message } end test "refuses and persists nothing when an implementing instruction already exists" do