From c4f11fac7022ea9c1f348953ccc2d9e7facb7b87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Sun, 23 Aug 2026 22:46:17 +0200 Subject: [PATCH 01/13] Bump ruby_llm from 1.15.0 to 1.16.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A waypoint, not a destination: the gem's own upgrade guide asks you to reach 1.16 one minor at a time before the 2.0 line, and landing it separately means any breakage after the 2.0 commit is attributable to 2.0 rather than to a minor bump. The delta is exactly two lines — the version and its CHECKSUMS digest. ruby_llm-schema stays at 0.3.0; the swap to schematist landed upstream after the 1.16.0 release, so it belongs to the next commit. Verified: full suite green (561 runs), rubocop, brakeman, bundler-audit, and bin/verify-model-registry resolving all five offered models. No RubyLLM deprecation warnings were emitted during the test run — grepped the captured log for them specifically, since they would have previewed what 2.0 removes. --- Gemfile.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 26cc050..0664059 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -395,7 +395,7 @@ GEM ruby-vips (2.3.0) ffi (~> 1.12) logger - ruby_llm (1.15.0) + ruby_llm (1.16.0) base64 event_stream_parser (~> 1) faraday (>= 1.10.0) @@ -688,7 +688,7 @@ 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 (1.16.0) sha256=26bd5310cf2ce55f74a60f8aae0b0d0327b586ff4532c84828103c3b2b905a18 ruby_llm-schema (0.3.0) sha256=a591edc5ca1b7f0304f0e2261de61ba4b3bea17be09f5cf7558153adfda3dec6 rubyzip (3.3.0) sha256=a372fc67892a4f8c0bc8ec906b720353d8e48807a64b2e63adf99b1e3583a034 sawyer (0.9.3) sha256=0d0f19298408047037638639fe62f4794483fb04320269169bd41af2bdcf5e41 From 6e229f414250f1dfef5bded487306a9892daacf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Sun, 23 Aug 2026 22:46:35 +0200 Subject: [PATCH 02/13] Drop the redundant ToolCall pill-refresh hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hook existed to force a second broadcast because "RubyLLM attaches tool_calls AFTER the parent message is saved". That was true of ruby_llm 1.14.1, the version in the lockfile when the hook landed. It no longer is: persist_message_completion wraps @message.save! and persist_tool_calls in one transaction (chat_methods.rb:344-375 in the 1.16.0 now installed), so Message#after_update_commit already sees the tool-call rows, and broadcast_replace_later_to re-renders from a fresh reload regardless. Removed under 1.16 rather than inside the 2.0 commit, so that if the pill did need a trigger after all it would surface here — where chat.after_message is still available as a fallback — instead of inside a commit that also renames two tables. It did not: verified in dev on both tools, create_application and modify_application, with the pill rendering at tool-call time while the build was still running. No replacement mechanism and no new test: the behaviour under test is the gem's transaction boundary, not app code. The regression net is the existing pill coverage in messages_helper_test and projects_controller_show_test. Also drops one UPDATE plus one re-broadcast per tool-call row per turn, and with it the Chat to Project touch cascade each one triggered. --- app/models/tool_call.rb | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/app/models/tool_call.rb b/app/models/tool_call.rb index 1db3ed0..1ebd927 100644 --- a/app/models/tool_call.rb +++ b/app/models/tool_call.rb @@ -1,20 +1,3 @@ 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 From cc0e21c44232cbd15e44f9ac0d5fa665bcc1b418 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Sun, 23 Aug 2026 22:47:21 +0200 Subject: [PATCH 03/13] Upgrade to RubyLLM 2.0 (git-pinned) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 c45ebd78 rather than tracking main: main is explicitly in development and moved 25 commits in the 27 hours around this work, 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. Everything here is mutually dependent and cannot be split: the acts_as layer needs the renamed columns, app/models/model.rb raises NoMethodError the moment it loads without acts_as_model, and use_new_acts_as raises on boot with the new gem. Schema. The generated migration renames models to ruby_llm_models and tool_calls to ruby_llm_tool_calls in place, converts messages.model_id from an integer FK to provider + model_id strings, moves messages.tool_call_id onto ruby_llm_tool_calls.result_id, backfills ruby_llm_usages from historical token columns and then drops them. It is irreversible — no down. Locally: 0.68s, 410 models and 56 tool calls carried over unchanged, 185 usage rows created, message_type set on every row, no tool-call-id renumbering (this schema already carried the unique index), integrity_check ok and foreign_key_check empty. system_injected, thinking_text and thinking_signature survive untouched. Code. Structured output now returns the JSON String from .content and the Hash from .parsed, so the two planners and Templates::Picker read .parsed. RubyLLM::Schema became Schematist::Schema. The tool DSL's params became parameters. message.tool_calls is now a Hash keyed by provider tool-call id, so the pill helper takes .values, and the association is ruby_llm_tool_calls, which the show-page eager load and bin/inspect-chat both needed. Chat's hand-written with_context override is gone — the gem provides it on the record. Two containment changes, because v2 removed a defence. v1 parsed structured output defensively and kept the String on a parse error, which degraded into an in-band tool error; v2's Message#parsed raises. Unhandled, that would escape Tool#execute and leave a persisted tool_use with no tool_result — the state that permanently breaks a chat, and one v2's own orphan cleanup does not cover since it only runs for RubyLLM/Faraday/Timeout errors. Both tools now rescue JSON::ParserError alongside InvalidResponse, and Templates::Picker maps it onto its own InvalidPick. Both are covered by new tests. Three deviations from the generator's output, all necessary here: - The migration class is AddRubyLLMV20Columns, not the generated AddRubyLlmV20Columns. This app declares inflect.acronym "LLM", so Rails camelizes the filename to the former and db:migrate aborts on the latter. - bin/verify-model-registry reads RubyLLM.config.model_registry_store after eager_load! rather than RubyLLM::ActiveRecord::Model directly. The store is wired from an on_load :active_record hook, so under this script's standalone require the constant and the store are both absent — and the resolver would silently fall back to the bundled JSON, the exact confusion the script exists to catch. - PlanSchema requires "schematist" explicitly. The gem requires it only from lazily-loaded files, so the constant's existence rode on load order. Nothing in the suite resolved PlanSchema — both planner suites stub invoke_llm above it — so test/schemas/plan_schema_test.rb now does. Verified: 565 runs green, rubocop, brakeman, bundler-audit, bin/verify-model-registry resolving all five offered ids, and a Docker build that loads the git-sourced gem inside the image (the Dockerfile already strips bundler/gems/*/.git, which is the path shape a git source creates). In dev, all four RubyLLM-backed stages exercised end to end: a streamed reply, a build from chat creating a real Instruction with Revisions, the template stage picking cyber, a model switch to Opus 5 answering, and a modification plan. Both migrated and v2-native chats dump clean through bin/inspect-chat. --- Gemfile | 11 +- Gemfile.lock | 34 +- app/controllers/projects_controller.rb | 2 +- app/helpers/messages_helper.rb | 7 +- app/models/chat.rb | 9 - app/models/model.rb | 3 - app/models/tool_call.rb | 3 - app/schemas/plan_schema.rb | 7 +- .../plan_application_creation/ad_hoc_llm.rb | 2 +- .../ad_hoc_llm.rb | 2 +- app/tools/create_application.rb | 7 +- app/tools/modify_application.rb | 7 +- bin/inspect-chat | 48 +-- bin/verify-model-registry | 29 +- config/initializers/ruby_llm.rb | 1 - ...0260822224622_add_ruby_llm_v2_0_columns.rb | 347 ++++++++++++++++++ db/schema.rb | 128 +++++-- lib/templates/picker.rb | 8 +- .../projects_controller_show_test.rb | 2 +- test/helpers/messages_helper_test.rb | 8 +- test/jobs/chat_respond_job_test.rb | 4 +- test/lib/templates/picker_test.rb | 25 +- test/schemas/plan_schema_test.rb | 17 + test/tools/create_application_test.rb | 23 ++ test/tools/modify_application_test.rb | 23 ++ 25 files changed, 624 insertions(+), 133 deletions(-) delete mode 100644 app/models/model.rb delete mode 100644 app/models/tool_call.rb create mode 100644 db/migrate/20260822224622_add_ruby_llm_v2_0_columns.rb create mode 100644 test/schemas/plan_schema_test.rb 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 0664059..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.16.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.16.0) sha256=26bd5310cf2ce55f74a60f8aae0b0d0327b586ff4532c84828103c3b2b905a18 - 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/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/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 1ebd927..0000000 --- a/app/models/tool_call.rb +++ /dev/null @@ -1,3 +0,0 @@ -class ToolCall < ApplicationRecord - acts_as_tool_call -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..05f22ef 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) diff --git a/app/services/plan_application_modification/ad_hoc_llm.rb b/app/services/plan_application_modification/ad_hoc_llm.rb index af0fbe7..adc52ac 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) diff --git a/app/tools/create_application.rb b/app/tools/create_application.rb index 1b2f9bf..2419182 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,7 +65,10 @@ 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." } end diff --git a/app/tools/modify_application.rb b/app/tools/modify_application.rb index e17f844..8c918cf 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,7 +65,10 @@ 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." } end diff --git a/bin/inspect-chat b/bin/inspect-chat index 2110019..6de6e2a 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,24 @@ 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}" + prev = 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..e3fa96d 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. # @@ -33,8 +28,16 @@ candidates = ARGV offered = LLM::Stages::AVAILABLE_MODELS.keys failures = [] -puts "models table rows: #{Model.count}" -puts "resolved registry: #{RubyLLM::Models.instance.all.size}" +# RubyLLM wires config.model_registry_store to the ruby_llm_models table from an +# `on_load :active_record` hook, so in a lazy-loading environment the store is +# still nil until something touches ActiveRecord — 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. +Rails.application.eager_load! +store = RubyLLM.config.model_registry_store + +puts "registry store rows: #{store ? store.count : "(no store configured)"}" +puts "resolved registry: #{RubyLLM::Models.instance.all.size}" def check(id, failures) info, = RubyLLM::Models.resolve(id) @@ -54,7 +57,7 @@ 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!'" + 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/lib/templates/picker.rb b/lib/templates/picker.rb index 48636af..3d3546d 100644 --- a/lib/templates/picker.rb +++ b/lib/templates/picker.rb @@ -36,10 +36,12 @@ 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 - name = content.is_a?(Hash) ? content["template"] : nil - raise InvalidPick, "picker returned #{content.inspect}" unless Templates::NAMES.include?(name) + parsed = chat.with_schema(SCHEMA).ask("Description: #{description}").parsed + name = parsed.is_a?(Hash) ? parsed["template"] : nil + raise InvalidPick, "picker returned #{parsed.inspect}" unless Templates::NAMES.include?(name) name + rescue JSON::ParserError => e + raise InvalidPick, "picker returned malformed JSON: #{e.message}" end def self.apply(workspace:, 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/lib/templates/picker_test.rb b/test/lib/templates/picker_test.rb index b82a2a3..397810c 100644 --- a/test/lib/templates/picker_test.rb +++ b/test/lib/templates/picker_test.rb @@ -25,7 +25,16 @@ class Templates::PickerTest < ActiveSupport::TestCase end end - test "pick passes the selected model to the chat" do +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") assert_equal "anthropic/claude-opus-4.6", captured[:chat_kwargs][:model] @@ -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/schemas/plan_schema_test.rb b/test/schemas/plan_schema_test.rb new file mode 100644 index 0000000..f0f144a --- /dev/null +++ b/test/schemas/plan_schema_test.rb @@ -0,0 +1,17 @@ +require "test_helper" + +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/tools/create_application_test.rb b/test/tools/create_application_test.rb index 6bb0241..d39d044 100644 --- a/test/tools/create_application_test.rb +++ b/test/tools/create_application_test.rb @@ -109,6 +109,29 @@ def stub_create_plan(result_or_proc) ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber end + 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 + 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 + test "on unexpected error from PlanApplicationCreation: propagates and persists nothing" do raising = ->(**) { raise RuntimeError, "upstream boom" } diff --git a/test/tools/modify_application_test.rb b/test/tools/modify_application_test.rb index 9d9f7d1..8d3b1a3 100644 --- a/test/tools/modify_application_test.rb +++ b/test/tools/modify_application_test.rb @@ -125,6 +125,29 @@ def stub_planner(result_or_proc) ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber end + 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 + 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 + test "on unexpected error from PlanApplicationModification: propagates and persists nothing" do raising = ->(**) { raise RuntimeError, "upstream boom" } From 37bb9083bf5dcedffb8f87a703e4d650fb36ee15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Sun, 23 Aug 2026 22:47:44 +0200 Subject: [PATCH 04/13] Update RubyLLM conventions for the v2 registry and tool DSL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four documents described mechanics the upgrade removed, and one of them, CLAUDE.md, is loaded into every session — leaving it stale actively misleads future work. The registry story is rewritten everywhere it appears: RubyLLM owns ruby_llm_models as its store and falls back to the bundled models.json only when that table is empty, so the v1 framing about a partially-filled table shadowing the JSON is gone, along with Model.refresh! in favour of RubyLLM.models.refresh!. The runbook also now records why the refresh is safe to run against production without a real OpenRouter key: per-provider fetch failures are rescued individually and logged "Keeping existing.", and the store's write is find_or_initialize_by + update! with no deletes. Verified locally rather than asserted — foreign_key_check stayed empty across a refresh with 26 live chats.ruby_llm_model_id rows. Two corrections that came out of running the runbook rather than reading it: - The row count changed meaning. v1's refresh wrote only what OpenRouter discovery returned; v2 fetches the published registry for every provider and merges discovery over it, so the store went 410 to 1464 on first refresh. The migration preserves the count exactly and a refresh grows it — worth stating, since the runbook asks you to compare counts before and after. - The test environment's bundled-JSON fallback carries sonnet-5 but not opus-5, and no test resolves either. Confirmed by resolving both against the empty test store. CLAUDE.md's tool-idempotency bullet cited a guard that no longer exists: the tool carrying it was deleted in 13e9c1c. It now describes what actually holds this invariant today — the prompt rule and the BadRequestError banner — and says plainly that there is no tool-side guard, so the next tool knows it needs its own. Pre-existing staleness the upgrade merely exposed. The followups entry proposing a catalog picker "backed by the dormant models table" is restated: that table is now gem-owned, populated and maintained, so what remains is capability filtering and a picker, not a table to wake up. Deliberately untouched: the ~15 SuggestPrompts mentions across the vision, architecture and phase-2 plan documents. Those are point-in-time records of what was designed, and the tool's deletion does not make them wrong as history. CHANGELOG gets an Unreleased entry written for self-hosters, naming the two things that actually affect them: the git pin, and the one-way migration that runs automatically at container boot. --- CHANGELOG.md | 17 +++++ CLAUDE.md | 5 +- docs/02-architecture/03-tech-stack.md | 2 +- docs/05-runbooks/03-llm-model-registry.md | 75 +++++++++++++++-------- docs/09-ideas/05-followups.md | 2 +- lib/llm/stages.rb | 11 ++-- 6 files changed, 76 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c8113fe..1503c1b 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). +## [Unreleased] + +### 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..7e3af2e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -11,6 +11,7 @@ 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 `ruby_llm_models`, `ruby_llm_tool_calls`, `ruby_llm_usages` and `ruby_llm_batches` (renamed in place by an **irreversible** migration), `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. @@ -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:18`) plus the `RubyLLM::BadRequestError` banner in `ChatRespondJob::FRIENDLY_ERRORS` that turns an already-corrupt chat into *"This conversation can't be continued by the model. Please start a new project."* So a new tool needs its own in-band guard: return an error hash rather than raising, so the second `tool_use` still gets a `tool_result` and history stays valid. Diagnose corrupt chats with `bin/inspect-chat ` (works on prod via `kamal app exec`). diff --git a/docs/02-architecture/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..25c2d63 100644 --- a/docs/05-runbooks/03-llm-model-registry.md +++ b/docs/05-runbooks/03-llm-model-registry.md @@ -5,17 +5,15 @@ 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 an id has to be resolvable from one of the two: the store, or the bundle. A +model absent from both raises `ModelNotFoundError`. 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 +24,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 +56,10 @@ 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'" +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. @@ -70,8 +75,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 +135,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 +151,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/09-ideas/05-followups.md b/docs/09-ideas/05-followups.md index 0d603e6..8b292b1 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). 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 From fb49eca0b125beaeda11a4ed8635b4d6c638d45d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Mon, 24 Aug 2026 22:55:49 +0200 Subject: [PATCH 05/13] Review fixes: keep the picker's local named content, fix test indentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two points from the PR review. The picker's local goes back to `content`. Only the accessor changed between v1 and v2 — the data it holds is the same response content, so the way of receiving it is no reason to rename the variable. It was also the only such rename in the branch, which left the picker disagreeing with both sibling planners, where the local is still `content` and feeds `build_result(content)`. Swept the rest of the diff to confirm: every other occurrence of `parsed` is the accessor call itself, a comment naming `Message#parsed`, or the test double's struct member mirroring the real object's API. Those stay. The indentation fix covers two lines, not one. The reported test sat at column 0, but the same bad string insertion had also pushed the test after it out to column 4; both are back at column 2 now. Worth recording why the linter did not catch it: rubocop-rails-omakase ships Layout/IndentationWidth and Layout/IndentationConsistency disabled, so a green `bin/rubocop` says nothing about indentation in this repo. Left alone here rather than fixed in a RubyLLM upgrade. Suite still 565 runs green. --- lib/templates/picker.rb | 6 +++--- test/lib/templates/picker_test.rb | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/templates/picker.rb b/lib/templates/picker.rb index 3d3546d..085f861 100644 --- a/lib/templates/picker.rb +++ b/lib/templates/picker.rb @@ -36,9 +36,9 @@ 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) - parsed = chat.with_schema(SCHEMA).ask("Description: #{description}").parsed - name = parsed.is_a?(Hash) ? parsed["template"] : nil - raise InvalidPick, "picker returned #{parsed.inspect}" unless Templates::NAMES.include?(name) + content = chat.with_schema(SCHEMA).ask("Description: #{description}").parsed + name = content.is_a?(Hash) ? content["template"] : nil + raise InvalidPick, "picker returned #{content.inspect}" unless Templates::NAMES.include?(name) name rescue JSON::ParserError => e raise InvalidPick, "picker returned malformed JSON: #{e.message}" diff --git a/test/lib/templates/picker_test.rb b/test/lib/templates/picker_test.rb index 397810c..ccb5ccc 100644 --- a/test/lib/templates/picker_test.rb +++ b/test/lib/templates/picker_test.rb @@ -25,7 +25,7 @@ class Templates::PickerTest < ActiveSupport::TestCase end end -test "pick raises InvalidPick when the response body is not valid JSON" do + 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") @@ -34,7 +34,7 @@ class Templates::PickerTest < ActiveSupport::TestCase end end - test "pick passes the selected model to the chat" do + 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") assert_equal "anthropic/claude-opus-4.6", captured[:chat_kwargs][:model] From c9656dde3a42532f00894824a93680cdb1cdf29b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Mon, 24 Aug 2026 23:00:23 +0200 Subject: [PATCH 06/13] Add the RubyLLM v2 production rollout runbook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The upgrade migration is irreversible and runs itself: bin/docker-entrypoint runs db:prepare whenever the command is ./bin/rails server, so `kamal deploy` is what triggers it. There is no manual gate, which makes snapshot-before-deploy the single most important thing to write down — after the containers boot, the old schema is gone. Four sections: rehearse locally on a production snapshot, deploy, verify the four RubyLLM-backed stages, recover. Every safety claim in it was verified rather than assumed: - WAL is really in use (PRAGMA journal_mode returns wal, and both sidecars sit beside the database file), so restores stop the process and delete -wal/-shm first. Snapshots need no such care: a .backup of a live WAL database produced exactly one self-contained file, no sidecars. - sqlite3 is present at runtime, not just at build time — installed in the Dockerfile's base stage, which the runtime stage inherits. Confirmed by running sqlite3 --version inside the built image: 3.46.1. - `kamal app start` starts an existing container while `kamal app boot` recreates one (checked against Kamal 2.11.0's own help), which matters in rollback: boot re-sources .kamal/secrets from the caller's shell, and SMTP_PASSWORD and GITHUB_CLIENT_SECRET are read from the environment there. A local export has silently reached production this way before, so the recovery section prefers start and gates boot behind a shell check. Two things the runbook says that a reader would otherwise get wrong: rollback is code *and* data, because neither image can read the other's schema; and the ruby_llm_models row count means different things after a migration (preserved exactly) than after a refresh (grows, 410 to 1464 locally), so a changed count has to be attributed to whichever step just ran. Indexed from CLAUDE.md, which closes the two links to this file that the documentation commit had left dangling. --- CLAUDE.md | 2 +- docs/05-runbooks/04-ruby-llm-v2-rollout.md | 218 +++++++++++++++++++++ 2 files changed, 219 insertions(+), 1 deletion(-) create mode 100644 docs/05-runbooks/04-ruby-llm-v2-rollout.md diff --git a/CLAUDE.md b/CLAUDE.md index 7e3af2e..c771f18 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -26,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//`. 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..d55861e --- /dev/null +++ b/docs/05-runbooks/04-ruby-llm-v2-rollout.md @@ -0,0 +1,218 @@ +# 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 count, migrate, record the after count. +sqlite3 storage/development.sqlite3 "SELECT COUNT(*) FROM models;" # before +bin/rails db:migrate +sqlite3 storage/development.sqlite3 "SELECT COUNT(*) FROM ruby_llm_models;" # after — MUST match + +# 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 step 1.5's two numbers 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.1 Capture the currently-running version. You need this to roll back, and +# it is unavailable once the new one is running. +kamal app version | tail -1 # write it down — call it PREV +``` + +```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 Deploy. db:prepare runs the migration during boot. +kamal deploy + +# 3.3 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. +kamal app start --version="$PREV" # preferred: reuses the existing container +kamal app boot --version="$PREV" # only if that container is gone +``` + +> ⚠️ **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. From aa3bc90353f680ae5d6a23842b8ef25aac887720 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Mon, 24 Aug 2026 23:00:40 +0200 Subject: [PATCH 07/13] Release 1.4.0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Covers the RubyLLM 2.0 upgrade: the git pin, the irreversible migration that renames two tables in place, and the rollout runbook that has to be read before deploying it. Minor rather than patch. By the file's own rule this is a judgement call — the entry adds no functionality a hifumi.dev user can observe, chat and builds behave exactly as before, which reads as patch. Called minor deliberately: a major dependency jump onto an unreleased line, plus a one-way migration that fires automatically on container boot, is not something a self-hoster should find in a patch release. The weight belongs in the version. --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1503c1b..138f570 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ 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). -## [Unreleased] +## [1.4.0] - 2026-08-24 ### Changed From 328c7c906d86610e6a92fc5b751e195391b51301 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Fri, 28 Aug 2026 00:14:45 +0200 Subject: [PATCH 08/13] Review fixes: close the JSON shape hole and the other half of the chmod race MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from review, three of which reach production. Malformed-but-valid JSON could still kill a chat. The upgrade widened both tools' rescues to JSON::ParserError, but .parsed is JSON.parse — valid JSON that is not an object sails through it and dies one line later, where Array(content["revisions"]) raises TypeError on an array and NoMethodError on a boolean. Neither is rescued by #execute, and the gem's own cleanup_orphaned_tool_results only runs for RubyLLM/Faraday/Timeout errors, so the tool_use stays persisted with no tool_result and the chat is finished for good. Guarded in build_result rather than by lengthening the rescue lists: InvalidResponse is vocabulary both tools already handle, and it makes the three structured-output call sites agree with Templates::Picker, which has checked is_a?(Hash) all along. This predates the upgrade; what is new is that the code claimed to have closed it. force: true covered only half the race it was added for. FileUtils wraps just ent.chmod in its rescue — the walk's own Dir.children sits outside it, so a file vanishing mid-walk was survivable but a directory was not, and git gc prunes empty .git/objects/ fanouts. Now retried once against a settled tree. The workspaces this mattered for are every one created before maintenance.auto/gc.auto started being set at init, which is all of production's, so those settings are also backfilled on each pass — idempotent, and two git configs are nothing beside a roast run measured in minutes. bin/inspect-chat misreported the shape it exists to find: messages[i - 1] with i = 0 wraps to the last message, so a chat opening with an orphaned tool result was blamed on the wrong parent instead of reported as having none. The deploy runbook gates on this script, so it needs to be right. Templates::Picker's rescue was method-scoped, covering the LLM call as well as the parse. RubyLLM decodes provider error bodies as JSON too, so an OpenRouter 502 returning an HTML page surfaced as "picker returned malformed JSON" and pointed whoever read the failed revision at the prompt rather than the transport. Scoped to .parsed alone. Each new test was verified by reverting its fix and re-running. That caught one of them being vacuous: the backfill assertion passed either way, because test_helper injects the same two settings via GIT_CONFIG_* into every git subprocess — it now asserts against --local, which ignores the environment. --- app/jobs/execute_instruction_job.rb | 35 +++++++++++--- .../plan_application_creation/ad_hoc_llm.rb | 5 ++ .../ad_hoc_llm.rb | 5 ++ bin/inspect-chat | 5 +- lib/templates/picker.rb | 13 +++-- test/jobs/execute_instruction_job_test.rb | 48 +++++++++++++++++++ .../ad_hoc_llm_test.rb | 14 ++++++ .../ad_hoc_llm_test.rb | 14 ++++++ 8 files changed, 128 insertions(+), 11 deletions(-) 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/services/plan_application_creation/ad_hoc_llm.rb b/app/services/plan_application_creation/ad_hoc_llm.rb index 05f22ef..6e9161a 100644 --- a/app/services/plan_application_creation/ad_hoc_llm.rb +++ b/app/services/plan_application_creation/ad_hoc_llm.rb @@ -28,6 +28,11 @@ 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| { summary: r.fetch("summary"), prompt: r.fetch("prompt") } diff --git a/app/services/plan_application_modification/ad_hoc_llm.rb b/app/services/plan_application_modification/ad_hoc_llm.rb index adc52ac..e44e319 100644 --- a/app/services/plan_application_modification/ad_hoc_llm.rb +++ b/app/services/plan_application_modification/ad_hoc_llm.rb @@ -28,6 +28,11 @@ 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| { summary: r.fetch("summary"), prompt: r.fetch("prompt") } diff --git a/bin/inspect-chat b/bin/inspect-chat index 6de6e2a..c377004 100755 --- a/bin/inspect-chat +++ b/bin/inspect-chat @@ -50,7 +50,10 @@ messages.each_with_index do |m, i| issues << "[#{i}] msg #{m.id}: tool_result with no ruby_llm_tool_calls row (orphan)" next end - prev = messages[i - 1] + # 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 diff --git a/lib/templates/picker.rb b/lib/templates/picker.rb index 085f861..2e6e02c 100644 --- a/lib/templates/picker.rb +++ b/lib/templates/picker.rb @@ -36,12 +36,19 @@ 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}").parsed + 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 - rescue JSON::ParserError => e - raise InvalidPick, "picker returned malformed JSON: #{e.message}" end def self.apply(workspace:, name:) 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/services/plan_application_creation/ad_hoc_llm_test.rb b/test/services/plan_application_creation/ad_hoc_llm_test.rb index 9f77df6..176bc6d 100644 --- a/test/services/plan_application_creation/ad_hoc_llm_test.rb +++ b/test/services/plan_application_creation/ad_hoc_llm_test.rb @@ -74,6 +74,20 @@ 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 + 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..6a4a497 100644 --- a/test/services/plan_application_modification/ad_hoc_llm_test.rb +++ b/test/services/plan_application_modification/ad_hoc_llm_test.rb @@ -74,6 +74,20 @@ 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 + test "raises InvalidResponse when revisions array is empty" do with_llm_response(plan_fixture("empty_revisions.json")) do assert_raises(PlanApplicationModification::AdHocLLM::InvalidResponse) do From 9ce19d60739720714cbad035a12762bdd233085d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Fri, 28 Aug 2026 00:22:04 +0200 Subject: [PATCH 09/13] Review follow-ups: cheaper registry check, fewer Hash builds, accurate canon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small ones from the same review, plus the two that need their own migration recorded rather than fixed. bin/verify-model-registry no longer eager-loads the whole app. The guard itself is still needed — this script boots through require "config/environment" rather than `bin/rails runner`, and that path really does leave config.model_registry_store nil, so without it the resolver would silently fall back to the bundled JSON, which is the confusion the script exists to catch. But referencing ActiveRecord::Base fires the same on_load hook, so a check documented as read-only stops loading app/ and lib/ on every run, production `kamal app exec` included. Message#tool_calls is an unmemoized Hash the gem rebuilds on every call, one RubyLLM::ToolCall per row. Two of the three callers only wanted to know whether any exist, and the gem ships tool_call? for exactly that. On the largest local chat (53 messages) that is ~106 fewer Hash builds per render. The helper keeps .values, since it needs the objects. CLAUDE.md claimed all four RubyLLM tables were "renamed in place". Only two were; ruby_llm_usages and ruby_llm_batches are created fresh, and the usage rows are backfilled from columns the migration then drops. Since this file is loaded into every session and the sentence is about an irreversible migration, a reader planning a rollback would have gone looking for tables that never existed. The runbook already had it right. Recorded in docs/09-ideas/05-followups.md rather than fixed, because the migration has already run and each needs a new one: - index_messages_on_provider_and_model_id indexes two columns v2 never writes (verified: all 45 messages written since the upgrade have provider NULL, against 241 backfilled), on the table that takes a row per streamed message. Dropping the columns as well as the index needs a decision first — those backfilled values are the only record of which model answered a pre-upgrade message. - The tool_calls to messages foreign key is gone, dropped because the column became polymorphic. Harmless while the dependent: :destroy chain is the only deletion path, and worth settling before anything ever deletes messages outside AR callbacks. Kept out of the rollout runbook deliberately: neither changes a deploy step, and the runbook is a procedure, not a data-model changelog. --- CLAUDE.md | 2 +- app/models/message.rb | 2 +- app/views/messages/_message.html.erb | 2 +- bin/verify-model-registry | 12 +++++---- docs/09-ideas/05-followups.md | 39 ++++++++++++++++++++++++++++ 5 files changed, 49 insertions(+), 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c771f18..4871aed 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -11,7 +11,7 @@ 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 `ruby_llm_models`, `ruby_llm_tool_calls`, `ruby_llm_usages` and `ruby_llm_batches` (renamed in place by an **irreversible** migration), `Schematist::Schema` replaced `RubyLLM::Schema`, and structured output reads `.parsed` rather than `.content`. Rollout procedure: `docs/05-runbooks/04-ruby-llm-v2-rollout.md`. +- **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. 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/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/verify-model-registry b/bin/verify-model-registry index e3fa96d..245b1d9 100755 --- a/bin/verify-model-registry +++ b/bin/verify-model-registry @@ -29,11 +29,13 @@ offered = LLM::Stages::AVAILABLE_MODELS.keys failures = [] # RubyLLM wires config.model_registry_store to the ruby_llm_models table from an -# `on_load :active_record` hook, so in a lazy-loading environment the store is -# still nil until something touches ActiveRecord — 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. -Rails.application.eager_load! +# `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 puts "registry store rows: #{store ? store.count : "(no store configured)"}" diff --git a/docs/09-ideas/05-followups.md b/docs/09-ideas/05-followups.md index 8b292b1..d9a63e6 100644 --- a/docs/09-ideas/05-followups.md +++ b/docs/09-ideas/05-followups.md @@ -161,6 +161,45 @@ The codegen agent now runs each revision in a per-instruction throwaway containe --- +## 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. + ## 2026-06-17 ### Move untrusted previews to a separate registrable domain From 44d51f96306069b56e13d587fcd27361dbb0f234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Wed, 2 Sep 2026 21:30:31 +0200 Subject: [PATCH 10/13] Close the remaining routes out of Tool#execute The is_a?(Hash) guard from 328c7c9 covered the top level only. A valid JSON object whose `revisions` hold non-objects cleared it and raised one line later: NoMethodError for String/Integer/nil#fetch, TypeError for Array#fetch, since Array(hash) reaches the map as [[k, v]]. Neither is rescued, and neither is caught by RubyLLM's orphan cleanup, which only fires for RubyLLM/Faraday/Timeout errors. The result is the state CLAUDE.md calls unrecoverable: a persisted tool_use with no tool_result, after which every message in that chat is rejected. Reject non-object elements in both planners, and add a StandardError backstop at both #execute boundaries. The backstop is the point: a rescue list cannot be complete here. `revisions.create!` raises RecordInvalid on a blank summary or prompt, which no shape check in build_result can prevent. This inverts the propagation test. Letting an error escape was what orphaned the tool_use, and ChatRespondJob already rescues StandardError, so the exception never reached an operator anyway - it only killed the chat. It is reported via Rails.error instead of swallowed. --- .../plan_application_creation/ad_hoc_llm.rb | 6 +++++ .../ad_hoc_llm.rb | 6 +++++ app/tools/create_application.rb | 9 +++++++ app/tools/modify_application.rb | 9 +++++++ .../ad_hoc_llm_test.rb | 26 ++++++++++++++++++- .../ad_hoc_llm_test.rb | 26 ++++++++++++++++++- test/tools/create_application_test.rb | 20 ++++++++++---- test/tools/modify_application_test.rb | 20 ++++++++++---- 8 files changed, 110 insertions(+), 12 deletions(-) diff --git a/app/services/plan_application_creation/ad_hoc_llm.rb b/app/services/plan_application_creation/ad_hoc_llm.rb index 6e9161a..1ddd98d 100644 --- a/app/services/plan_application_creation/ad_hoc_llm.rb +++ b/app/services/plan_application_creation/ad_hoc_llm.rb @@ -35,6 +35,12 @@ def self.build_result(content) 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 e44e319..611dcd6 100644 --- a/app/services/plan_application_modification/ad_hoc_llm.rb +++ b/app/services/plan_application_modification/ad_hoc_llm.rb @@ -35,6 +35,12 @@ def self.build_result(content) 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 2419182..9576f01 100644 --- a/app/tools/create_application.rb +++ b/app/tools/create_application.rb @@ -70,6 +70,15 @@ def execute(intent:, clarifications: {}) # 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 8c918cf..8e3e8b4 100644 --- a/app/tools/modify_application.rb +++ b/app/tools/modify_application.rb @@ -70,6 +70,15 @@ def execute(intent:, clarifications: {}) # 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/test/services/plan_application_creation/ad_hoc_llm_test.rb b/test/services/plan_application_creation/ad_hoc_llm_test.rb index 176bc6d..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) @@ -88,6 +90,28 @@ def plan_fixture(name) 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 6a4a497..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) @@ -88,6 +90,28 @@ def plan_fixture(name) 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 d39d044..2c544b4 100644 --- a/test/tools/create_application_test.rb +++ b/test/tools/create_application_test.rb @@ -132,16 +132,26 @@ 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 + # 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 - 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: {}) } + 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 8d3b1a3..9574843 100644 --- a/test/tools/modify_application_test.rb +++ b/test/tools/modify_application_test.rb @@ -148,16 +148,26 @@ def stub_planner(result_or_proc) ActiveSupport::Notifications.unsubscribe(subscriber) if subscriber end - test "on unexpected error from PlanApplicationModification: propagates and persists nothing" do + # 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 - assert_no_difference -> { Instruction.count } do - assert_no_difference -> { Revision.count } do - stub_planner(raising) do - assert_raises(RuntimeError) { @tool.execute(intent: "x", clarifications: {}) } + 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 From f263727df714457488c92f7011d82bcab679e78b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Wed, 2 Sep 2026 21:30:38 +0200 Subject: [PATCH 11/13] Fail the registry check on a store that shadows the bundle The script passed on the exact state it exists to catch. With an empty ruby_llm_models table every id resolves from the gem's bundled registry, so it printed "registry store rows: 0" and then "all ids resolve". But find_or_create_model writes one row per successful resolution, so a fresh environment self-poisons: the first chat leaves a single row, and from the next boot that row is the whole registry, because a non-empty store shadows the bundle completely. That is the 2026-08-12 failure. Warn when the store is empty, fail when it holds fewer rows than the offered ids. Runbook 03 said an id had to be "resolvable from one of the two", which invites the false converse; state that the store wins whenever it has any rows at all. Also note that refresh! now holds the write lock for 1464 rows rather than 410, against the live production database. --- bin/verify-model-registry | 23 ++++++++++++++++++++++- docs/05-runbooks/03-llm-model-registry.md | 14 +++++++++++--- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/bin/verify-model-registry b/bin/verify-model-registry index 245b1d9..8cc322c 100755 --- a/bin/verify-model-registry +++ b/bin/verify-model-registry @@ -27,6 +27,7 @@ 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" @@ -37,10 +38,27 @@ failures = [] # 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 ? store.count : "(no store configured)"}" +puts "registry store rows: #{store ? rows : "(no store configured)"}" 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) puts format(" OK %-32s provider=%-11s ctx=%s", id, info.provider, info.context_window || "?") @@ -59,6 +77,9 @@ end if failures.any? puts "\n#{failures.size} id(s) did not resolve: #{failures.join(', ')}" +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 diff --git a/docs/05-runbooks/03-llm-model-registry.md b/docs/05-runbooks/03-llm-model-registry.md index 25c2d63..931a6bc 100644 --- a/docs/05-runbooks/03-llm-model-registry.md +++ b/docs/05-runbooks/03-llm-model-registry.md @@ -12,8 +12,11 @@ The gem's railtie wires `config.model_registry_store` to `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 an id has to be resolvable from one of the two: the store, or the bundle. A -model absent from both raises `ModelNotFoundError`. +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 @@ -59,6 +62,11 @@ provider fetch is what populates the ids. kamal app exec --reuse "bin/rails runner 'puts RubyLLM.config.model_registry_store.count'" # 2. Populate +# 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 @@ -67,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" ``` From c7a68002a728fd369e6aaaab3de064fd3c30f041 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Wed, 2 Sep 2026 21:30:49 +0200 Subject: [PATCH 12/13] Make the v2 rollout runbook safe to execute as written Five gaps, each one a step someone following the procedure would get wrong: - `kamal app version | tail -1` returns a BLANK line, because kamal prints "App Host: " first and ends with a blank. An empty --version is not .presence, so Kamal falls through to the local git SHA - during a rollback, the v2 image. The documented rollback booted v2 against the restored v1 database and re-ran the one-way migration over the snapshot, and the site came back up either way. Take line 2, and verify code and data after restoring. - `kamal deploy` alone leaves the old v1 container serving while the new one migrates, against tables just renamed: "no such table: tool_calls" on every project page, and with SOLID_QUEUE_IN_PUMA a v1 job writing dropped columns can orphan a tool_use. Stop the app first, after pre-building so the outage is only boot plus migrate. - A failed deploy stops the NEW container and leaves the old one routed, with the migration already committed - the one state neither image can read. Say so, and check rather than assuming nothing happened. - The rehearsal produced no go/no-go: nothing timed the migration against kamal's 30s default, and nothing checked that backfill_usage_entries carried every row before remove_legacy_message_columns dropped the source. - The secrets warning sat on `kamal app boot`, but `kamal deploy` re-sources .kamal/secrets identically, and a prefix check cannot tell a stale key from a live one. Compare hashes against production before deploying. Also split the two mutually exclusive rollback commands, since a block-level copy-paste ran both, and clear jobs enqueued under v2 - the queue is a separate database that the restore does not touch, and discard_on DeserializationError is commented out. --- docs/05-runbooks/04-ruby-llm-v2-rollout.md | 115 +++++++++++++++++++-- 1 file changed, 107 insertions(+), 8 deletions(-) diff --git a/docs/05-runbooks/04-ruby-llm-v2-rollout.md b/docs/05-runbooks/04-ruby-llm-v2-rollout.md index d55861e..d17bc51 100644 --- a/docs/05-runbooks/04-ruby-llm-v2-rollout.md +++ b/docs/05-runbooks/04-ruby-llm-v2-rollout.md @@ -69,10 +69,30 @@ 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 count, migrate, record the after count. +# 1.5 Record the before counts, migrate (TIMED), record the after counts. sqlite3 storage/development.sqlite3 "SELECT COUNT(*) FROM models;" # before -bin/rails db:migrate -sqlite3 storage/development.sqlite3 "SELECT COUNT(*) FROM ruby_llm_models;" # after — MUST match +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;" @@ -99,17 +119,45 @@ token data. `ruby_llm_batches` is created empty. > 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 step 1.5's two numbers differ, or 1.6 reports anything, **stop** — do not +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. -kamal app version | tail -1 # write it down — call it PREV +# +# 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 @@ -130,10 +178,32 @@ they are. kamal app exec --reuse \ "sqlite3 /rails/storage/production.sqlite3 \".backup '/rails/storage/production.sqlite3.pre-v2'\"" -# 3.2 Deploy. db:prepare runs the migration during boot. +# 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.3 Confirm the migration ran cleanly rather than assuming it did. +# 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. @@ -178,8 +248,37 @@ ssh root@77.42.95.154 "docker run --rm -v hifumi_dev_storage:/s alpine sh -c \ # 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 -kamal app boot --version="$PREV" # only if that container is gone +``` + +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 From 3f869679e1b866479d727f73e45de322c097d43f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Strza=C5=82kowski?= Date: Wed, 2 Sep 2026 21:30:57 +0200 Subject: [PATCH 13/13] Cover the tool-call visibility branch and correct the canon visible_in_chat?'s tool_call? disjunct had no test: the one case that puts a tool call on a message gives it prose too, so content.present? short-circuits first. The shape it exists for is the empty-content build-started message, and a regression would silently hide every pill behind message_row_class's "hidden". CLAUDE.md cited instructions.txt.erb:18 for the tool-idempotency rule, which is on 19 - and since the tool-side guard went in 13e9c1c, that bullet is the only description of the invariant left. Its Phase 2 note also quoted a pill string that no longer exists and the same deleted guard. Note in plan_schema_test.rb that CI's eager_load, not the test, is what guards the require "schematist": under parallelize any test touching CreateApplication loads RubyLLM::Tool, which requires schematist for the rest of the process. Also record messages.content_raw as a third migration residual, and file the 2026-08-28 section in date order so it isn't hidden mid-file. --- CLAUDE.md | 4 +-- docs/09-ideas/05-followups.md | 52 ++++++++++++++++++-------------- test/models/message_test.rb | 20 ++++++++++++ test/schemas/plan_schema_test.rb | 8 +++++ 4 files changed, 60 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4871aed..6368b12 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,7 @@ Hosted at **[hifumi.dev](https://hifumi.dev)** · Source: this repo. - **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. @@ -57,4 +57,4 @@ Additional sources from the Phase 1 spike: - **LLM model selection**: never hardcode a model id at a call site — `lib/llm/stages.rb` (`LLM::Stages`, note the `LLM` acronym inflection) is the registry of the six LLM stages (chat, plan_creation, plan_modification, template, code, docs), their labels, factory defaults, and the curated `AVAILABLE_MODELS` list (full OpenRouter ids only, never `sonnet`/`haiku` aliases). Profiles store per-user defaults (`default__model`), projects snapshot their own selection (`_model`) at creation; selectors live in the build tab (`model_selections/_pane`), the new-project form, and the account integrations pane. A new stage = registry entry + migration on both tables + threading at the call site. A new **model** = confirm the OpenRouter slug supports `structured_outputs` (plan/template stages) and `tools` (chat) → populate the RubyLLM registry in **every** environment → only then add the id to `AVAILABLE_MODELS`; that order is load-bearing, because RubyLLM owns its registry: it resolves against the `ruby_llm_models` table (its store) and falls back to the gem's bundled `models.json` only when that table is *empty*, so an id absent from both raises `ModelNotFoundError` on the four RubyLLM-backed stages. Check with `bin/verify-model-registry [candidate-id]`, fix with `RubyLLM.models.refresh!` plus a process restart (the registry is memoized per process); the refresh only ever adds or updates rows — per-provider fetch failures are logged and skipped, never allowed to empty the store. Full procedure in `docs/05-runbooks/03-llm-model-registry.md`. Selection applies on the OpenRouter path only — `roast_model_env` keeps claudesubscription runs on the operator's ENV/alias defaults, and an explicit `HIFUMI_DEV_MODEL`/`HIFUMI_DEV_DOCS_MODEL` always wins. - **Agent sandbox (tenant isolation)**: the codegen agent runs `claude` with `skip_permissions!` on user-controlled prompts — treat it as untrusted code execution, so in production it must NOT run in the shared generator container. `ExecuteInstructionJob#execute_revision` wraps the roast invocation via `Roast::Sandbox.wrap` (`lib/roast/sandbox.rb`, plain builder, returns the `docker run` argv) into a throwaway `--rm` container that mounts ONLY this project's workspace (no `workspace_root`, no `/var/run/docker.sock`), runs entirely as the unprivileged `generator` user (`--user`, `--cap-drop=ALL`, zero cap-adds — uniform uid avoids the capless-root/mixed-ownership deadlock of issue #24; the workspace is re-relaxed `a+rwX` before every sandboxed run), and forwards env (incl. `OPENROUTER_API_KEY` and the per-project `HIFUMI_DEV_*` model selection) **by name** so secrets never hit argv. Gated by `sandboxed?` (production OR `FORCE_AGENT_SANDBOX=1`); dev stays direct (single-tenant, Claude-subscription transport, no Docker). Image = the generator's own, from `HIFUMI_AGENT_IMAGE` (set in `deploy.yml`). Not runtime-verifiable on macOS — see the verification checklist + residuals (generator-side socket, egress, bundle vendoring) in `docs/09-ideas/05-followups.md`. - **Preview infrastructure**: `lib/preview/preview_manager.rb` (plain Ruby, not a Roast workflow) drives Docker. `lib/preview/Dockerfile{,.base}` are owned by this repo — never read from generated apps. `lib/preview/skeleton/` is the canonical fresh-Rails-app baseline copied into every workspace; regenerate with `bin/preview-regen-skeleton` when bumping Rails. Rebuild the base image with `bin/preview-rebuild-base` after Gemfile changes. The `preview-internal` Docker network is created without `--internal` on Docker Desktop (host port mapping wouldn't work otherwise) — Phase 4 reintroduces strict egress isolation on a Linux production host. In remote mode `PreviewManager#run_container` passes `PREVIEW_HOST=.preview.` to the container; the skeleton-overlay's `preview_iframe.rb` initializer appends it to `Rails.application.config.hosts` so Rails 8's dev HostAuthorization doesn't 403 the kamal-proxy request. -- **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:18`) 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`). +- **RubyLLM tools must be idempotent within a user turn**. RubyLLM's tool loop will sometimes call the same tool twice in adjacent assistant messages before either result lands, producing the order `assistant(use_X) → assistant(use_Y) → user(result_X) → user(result_Y)` — illegal for Anthropic, and the chat permanently rejects every subsequent message. **There is no tool-side guard any more** — the tool that carried one was deleted in `13e9c1c`. What stands today is a prompt rule (*"Call each tool AT MOST ONCE per user turn"*, `app/prompts/generator_agent/instructions.txt.erb:19`) plus the `RubyLLM::BadRequestError` banner in `ChatRespondJob::FRIENDLY_ERRORS` that turns an already-corrupt chat into *"This conversation can't be continued by the model. Please start a new project."* So a new tool needs its own in-band guard: return an error hash rather than raising, so the second `tool_use` still gets a `tool_result` and history stays valid. Diagnose corrupt chats with `bin/inspect-chat ` (works on prod via `kamal app exec`). diff --git a/docs/09-ideas/05-followups.md b/docs/09-ideas/05-followups.md index d9a63e6..dd632ac 100644 --- a/docs/09-ideas/05-followups.md +++ b/docs/09-ideas/05-followups.md @@ -161,6 +161,31 @@ The codegen agent now runs each revision in a per-instruction throwaway containe --- +## 2026-06-17 + +### Move untrusted previews to a separate registrable domain + +**Motivation**: previews (untrusted user/LLM code) are served as subdomains of the generator's own apex (`.preview.hifumi.dev`), so they share the registrable domain `hifumi.dev` with the trusted control plane and are therefore **same-site** with it. The cookie/header hardening shipped in PR #32 (`__Host-` session cookie, `force_ssl`/`assume_ssl` Secure cookies, host-only cookies, `Origin-Agent-Cluster`, CSRF) is a **mitigation, not a cure** — the boundary is structurally weak. The cure is serving previews from a **separate registrable domain**, which makes them cross-site to the generator. Full threat model + the single-vs-multiple-domain reasoning: `docs/02-architecture/05-tenant-isolation-and-domains.md`. + +**Why it matters**: a malicious generated app can set a `Domain=hifumi.dev` cookie that shadows the generator's session cookie (login CSRF / session fixation), `SameSite` offers no protection across the shared registrable domain, and `document.domain` can relax SOP to the parent. Each is currently patched, but any future feature that trusts "same-site" re-opens the class. + +**The migration is mostly config, not code** — the preview hostname is already parameterized on `PREVIEW_DOMAIN`: +- `Preview::Config.domain` ← `ENV["PREVIEW_DOMAIN"]` (`config/initializers/preview_config.rb`) +- `public_preview_host` = `.preview.#{domain}` (`lib/preview/preview_manager.rb`) → drives the kamal-proxy `--host`, the container's `PREVIEW_HOST` env, and `project.preview_url` (the studio iframe `src`) +- CSP `frame-src https://*.preview.#{PREVIEW_DOMAIN}` (`config/initializers/content_security_policy.rb`) follows it + +So flipping the preview apex is: register a separate registrable domain (pick a `.dev` to keep auto-HSTS); add wildcard A `*.preview.` → host IP; issue a wildcard cert for it (same flow as `docs/05-runbooks/02-preview-wildcard-tls.md`, DNS-01 on the new domain's provider; repoint `PREVIEW_TLS_*` at it); set `PREVIEW_DOMAIN=` in `config/deploy.yml`. The generator stays on `hifumi.dev`; previews become `.preview.`, now cross-site to the control plane. + +**Decisions / caveats**: +- One separate domain isolates **generator ↔ preview** (the high-value win). Previews still share the new apex, so **preview ↔ preview** cookie-tossing remains — acceptable for throwaway demos; full per-preview isolation needs unique domains (don't, unless previews hold secrets from each other). +- Existing `.preview.hifumi.dev` links break, but previews are ephemeral (reaped ~30 min) — nothing durable to migrate. +- Keep the PR #32 apex hardening as defense-in-depth after the move. +- 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 @@ -200,25 +225,8 @@ neither was in scope for the upgrade itself. periodic orphan sweep or a partial constraint. Worth deciding before writing any code that deletes messages outside AR callbacks. -## 2026-06-17 - -### Move untrusted previews to a separate registrable domain - -**Motivation**: previews (untrusted user/LLM code) are served as subdomains of the generator's own apex (`.preview.hifumi.dev`), so they share the registrable domain `hifumi.dev` with the trusted control plane and are therefore **same-site** with it. The cookie/header hardening shipped in PR #32 (`__Host-` session cookie, `force_ssl`/`assume_ssl` Secure cookies, host-only cookies, `Origin-Agent-Cluster`, CSRF) is a **mitigation, not a cure** — the boundary is structurally weak. The cure is serving previews from a **separate registrable domain**, which makes them cross-site to the generator. Full threat model + the single-vs-multiple-domain reasoning: `docs/02-architecture/05-tenant-isolation-and-domains.md`. - -**Why it matters**: a malicious generated app can set a `Domain=hifumi.dev` cookie that shadows the generator's session cookie (login CSRF / session fixation), `SameSite` offers no protection across the shared registrable domain, and `document.domain` can relax SOP to the parent. Each is currently patched, but any future feature that trusts "same-site" re-opens the class. - -**The migration is mostly config, not code** — the preview hostname is already parameterized on `PREVIEW_DOMAIN`: -- `Preview::Config.domain` ← `ENV["PREVIEW_DOMAIN"]` (`config/initializers/preview_config.rb`) -- `public_preview_host` = `.preview.#{domain}` (`lib/preview/preview_manager.rb`) → drives the kamal-proxy `--host`, the container's `PREVIEW_HOST` env, and `project.preview_url` (the studio iframe `src`) -- CSP `frame-src https://*.preview.#{PREVIEW_DOMAIN}` (`config/initializers/content_security_policy.rb`) follows it - -So flipping the preview apex is: register a separate registrable domain (pick a `.dev` to keep auto-HSTS); add wildcard A `*.preview.` → host IP; issue a wildcard cert for it (same flow as `docs/05-runbooks/02-preview-wildcard-tls.md`, DNS-01 on the new domain's provider; repoint `PREVIEW_TLS_*` at it); set `PREVIEW_DOMAIN=` in `config/deploy.yml`. The generator stays on `hifumi.dev`; previews become `.preview.`, now cross-site to the control plane. - -**Decisions / caveats**: -- One separate domain isolates **generator ↔ preview** (the high-value win). Previews still share the new apex, so **preview ↔ preview** cookie-tossing remains — acceptable for throwaway demos; full per-preview isolation needs unique domains (don't, unless previews hold secrets from each other). -- Existing `.preview.hifumi.dev` links break, but previews are ephemeral (reaped ~30 min) — nothing durable to migrate. -- Keep the PR #32 apex hardening as defense-in-depth after the move. -- 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. +**`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/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 index f0f144a..6171083 100644 --- a/test/schemas/plan_schema_test.rb +++ b/test/schemas/plan_schema_test.rb @@ -1,5 +1,13 @@ 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