diff --git a/db/schema.rb b/db/schema.rb index a5358ad72..f5e188151 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_07_15_000000) do +ActiveRecord::Schema[8.1].define(version: 2026_07_21_000010) do create_table "active_storage_attachments", force: :cascade do |t| t.bigint "blob_id", null: false t.datetime "created_at", null: false @@ -233,6 +233,7 @@ t.datetime "created_at", null: false t.json "data", default: {}, null: false t.text "description" + t.string "kind" t.integer "origin_id" t.integer "parent_id" t.float "progress", default: 0.0 @@ -243,6 +244,7 @@ t.index ["origin_id"], name: "index_creatives_on_origin_id" t.index ["parent_id"], name: "index_creatives_on_parent_id" t.index ["user_id"], name: "index_creatives_on_user_id" + t.index ["user_id"], name: "index_creatives_on_user_id_profile_unique", unique: true, where: "kind = 'profile'" end create_table "devices", force: :cascade do |t| diff --git a/engines/collavre/app/controllers/concerns/collavre/users_controller/ai_user_management.rb b/engines/collavre/app/controllers/concerns/collavre/users_controller/ai_user_management.rb index 485b74c6c..5117b52d3 100644 --- a/engines/collavre/app/controllers/concerns/collavre/users_controller/ai_user_management.rb +++ b/engines/collavre/app/controllers/concerns/collavre/users_controller/ai_user_management.rb @@ -43,6 +43,7 @@ def create_ai @user.agent_conf = params[:agent_conf] if @user.respond_to?(:agent_conf=) && params[:agent_conf].present? if @user.save + @user.sync_profile_system_prompt! Collavre::Contact.ensure(user: Current.user, contact_user: @user) share_ai_agent_to_creative(@user, params[:creative_id]) redirect_to user_path(Current.user, tab: "contacts"), notice: I18n.t("collavre.users.create_ai.success") @@ -61,6 +62,12 @@ def update_ai ai_params = params.require(:user).permit(:name, :system_prompt, :llm_vendor, :llm_model, :llm_api_key, :gateway_url, :searchable, :routing_expression, :agent_conf, tools: []) if @user.update(ai_params) + # Only mirror the column into the canonical profile prompt when the form + # actually submitted system_prompt. A partial PATCH (e.g. routing_expression + # only) leaves the legacy column at its stored value, which may be stale + # relative to a directly-edited profile; syncing it would clobber the + # canonical data["markdown_source"]. + @user.sync_profile_system_prompt! if ai_params.key?(:system_prompt) redirect_to edit_ai_user_path(@user), notice: I18n.t("collavre.users.update_ai.success") else @available_tools = load_available_tools diff --git a/engines/collavre/app/models/collavre/creative.rb b/engines/collavre/app/models/collavre/creative.rb index 503eec42c..1c5ed4080 100644 --- a/engines/collavre/app/models/collavre/creative.rb +++ b/engines/collavre/app/models/collavre/creative.rb @@ -5,11 +5,23 @@ module Collavre class Creative < ApplicationRecord self.table_name = "creatives" + # Promote the `kind` discriminator (stored in `data`) to a real column so the + # profile-uniqueness index is a plain-column index that dumps identically on + # SQLite and PostgreSQL, instead of a JSON-expression index that serializes + # per-adapter and crashes `db:schema:load` on Postgres (see + # docs/engine_development.md and 20260721000010_add_unique_index_to_profile_creatives). + # `data` stays the source of truth; `kind` is re-derived from it on every save. + include Collavre::IndexedJsonColumns + indexed_json_columns json: :data, columns: { kind: "kind" } + # --------------------------------------------------------------------------- # Reserved metadata key registry — engines register their own namespaces so # `update_metadata` preserves them without core naming vendor-specific keys. + # "kind" is the discriminator (`inbox`/`profile`/`skill`) that scopes like + # `profiles` query on (`data->>'kind'`); it must be preserved or a metadata + # save that omits it makes the row undiscoverable and a duplicate is created. # --------------------------------------------------------------------------- - BUILTIN_RESERVED_METADATA_KEYS = %w[markdown_source content_type editor].freeze + BUILTIN_RESERVED_METADATA_KEYS = %w[markdown_source content_type editor kind].freeze class << self def registered_reserved_metadata_keys @@ -69,6 +81,17 @@ def to_partial_path # --- Inbox --- scope :inboxes, -> { where("data->>'kind' = 'inbox'") } + # --- Profile --- + PROFILE_KIND = "profile" + SKILL_KIND = "skill" + + scope :profiles, -> { where("data->>'kind' = ?", PROFILE_KIND) } + + # A profile creative is an agent's system prompt, never a tool source. + def profile? + data&.dig("kind") == PROFILE_KIND + end + SYSTEM_TOPIC_NAME = "System" MAIN_TOPIC_NAME = "Main" CONTENT_TOPIC_NAME = "Content" @@ -135,6 +158,25 @@ def self.inbox_for(user) ) end + # Find or create the profile creative for a given user. + # Places it as a root creative (no parent) owned by the user. + # + # Guarantees a single profile creative per user. A partial unique index + # (index_creatives_on_user_id_profile_unique) prevents a duplicate at the DB, + # and create_or_find_by! resolves a concurrent insert to the surviving row — + # so there is never a second, separately-editable profile to split-brain on. + # The read-first fast path keeps the common (already-exists) case cheap. + def self.profile_for(user) + existing = profiles.where(user: user).order(:id).first + return existing if existing + + profiles.create_or_find_by!(user: user) do |creative| + creative.description = user.name.to_s + creative.data = { "kind" => PROFILE_KIND } + creative.progress = 0.0 + end + end + attr_accessor :filtered_progress belongs_to :user, class_name: Collavre.configuration.user_class_name, optional: true @@ -352,6 +394,10 @@ def progress_service end def mark_mcp_tools_sync_pending + # Profile creatives hold an agent's system prompt, so a fenced + # `extend ToolMeta` example in the prompt must not register a real tool. + return if profile? + @mcp_tools_sync_pending = true end diff --git a/engines/collavre/app/models/collavre/creative/describable.rb b/engines/collavre/app/models/collavre/creative/describable.rb index 0cdb7cff5..427cb8606 100644 --- a/engines/collavre/app/models/collavre/creative/describable.rb +++ b/engines/collavre/app/models/collavre/creative/describable.rb @@ -10,6 +10,11 @@ module Describable # textarea. Stored in data["editor"]; decoupled from the storage format # (content_type), which is now Markdown for both surfaces. attr_accessor :markdown_editor + # When true, convert_markdown_to_html stores markdown_source verbatim + # instead of rewriting inline data-URI images into Active Storage blobs. + # Used by prompt sync, where markdown_source is the canonical text sent + # to the LLM and must stay byte-for-byte what the user entered. + attr_accessor :skip_data_uri_rewrite attr_reader :markdown_source def markdown_source=(value) @@ -147,7 +152,17 @@ def convert_markdown_to_html # FIRST, then persist the rewritten source. Subsequent edits # around the same image carry the blob path instead of the # data URI, so re-renders no longer create duplicate blobs. - rewritten_source = Collavre::MarkdownConverter.rewrite_data_uri_images(new_source) + # skip_data_uri_rewrite callers (prompt sync) keep the source + # verbatim; the derived HTML still renders the data URI inline. + # Profile creatives are agent prompts: markdown_source is sent + # verbatim to the LLM, so the rewrite must never fire for them + # regardless of caller — a direct Creative-editor edit reaches this + # path without the skip flag, but its source is equally LLM-bound. + rewritten_source = if skip_data_uri_rewrite || data["kind"] == PROFILE_KIND + new_source + else + Collavre::MarkdownConverter.rewrite_data_uri_images(new_source) + end self.data["markdown_source"] = rewritten_source self.description = Collavre::MarkdownConverter.markdown_to_html(rewritten_source) else diff --git a/engines/collavre/app/models/collavre/user.rb b/engines/collavre/app/models/collavre/user.rb index effc446ef..bdc6fdbb7 100644 --- a/engines/collavre/app/models/collavre/user.rb +++ b/engines/collavre/app/models/collavre/user.rb @@ -3,6 +3,7 @@ class User < ApplicationRecord self.table_name = "users" include HasInboxCreative + include HasProfileCreative has_many :user_themes, class_name: "Collavre::UserTheme", dependent: :destroy @@ -207,6 +208,45 @@ def ai_user? llm_vendor.present? end + # Mirror the agent's system prompt into its profile creative as Markdown, + # so the raw prompt lands losslessly in data["markdown_source"] — the + # canonical home for the prompt. Writing the prompt straight into + # `description` would run it through the HTML sanitizer (Describable), which + # strips / tags and entity-escapes < > &, corrupting + # every prompt; the derived `description` is only a rendered display view. + # The system_prompt column is legacy, pending removal. + # No-op for humans and for prompt-less agents (profile keeps its name seed). + def sync_profile_system_prompt! + return unless ai_user? + creative = profile_creative + if system_prompt.present? + return if creative.data&.dig("markdown_source") == system_prompt + # Store the prompt verbatim: markdown_source is the canonical text sent + # to the LLM, so an inline data-URI image must NOT be rewritten into a + # blob path (which would mutate the prompt the agent receives). + creative.skip_data_uri_rewrite = true + creative.update!(content_type_input: "markdown", markdown_source: system_prompt) + elsif creative.data&.dig("markdown_source").present? + # The prompt was cleared. Demote out of markdown mode so the stale + # markdown_source is dropped and render falls back to the (now-blank) + # column — matching the pre-profile behavior where blanking the prompt + # took effect immediately. Also reseed `description` back to the name + # seed so the old rendered prompt doesn't stay visible/searchable on the + # owned profile root. Without this, the old prompt stays authoritative. + creative.update!(content_type_input: "html", description: name.to_s) + end + end + + # The effective system prompt for all read consumers. The canonical prompt + # lives in the profile creative's data["markdown_source"]; the legacy + # `system_prompt` column is the fallback for rows not yet backfilled. Every + # prompt reader (orchestration matching, type classification, typo agent, + # admin view) must route here so that editing the profile creative directly + # takes effect everywhere, not just in AiAgentService. + def effective_system_prompt + profile_creative_if_present&.data&.dig("markdown_source").presence || system_prompt + end + def claude_channel_agent? llm_model == "claude-code" end diff --git a/engines/collavre/app/models/concerns/collavre/has_profile_creative.rb b/engines/collavre/app/models/concerns/collavre/has_profile_creative.rb new file mode 100644 index 000000000..01969cfc2 --- /dev/null +++ b/engines/collavre/app/models/concerns/collavre/has_profile_creative.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Collavre + module HasProfileCreative + extend ActiveSupport::Concern + + included do + after_create_commit :create_profile_creative + end + + # Returns the user's profile creative, creating one if it doesn't exist. + # Use only from write paths (e.g. sync_profile_system_prompt!) — never from + # a hot read path, since creating a profile creative also creates a Topic. + def profile_creative + Collavre::Creative.profile_for(self) + end + + # Read-only lookup: the existing profile creative or nil, never creating one. + # Read paths (effective_system_prompt) must use this so a hot-path read never + # writes, and so rendering never fails when the in-memory user is invalid + # (e.g. re-rendering the AI form after a failed save leaves name blank). + def profile_creative_if_present + return nil unless persisted? + + # A partial unique index enforces one profile per user, so at most one row + # matches; order(:id) is defensive (matches profile_for's selection) for + # any pre-index legacy duplicate, keeping reads and writes on the same row. + Collavre::Creative.profiles.where(user: self).order(:id).first + end + + private + + def create_profile_creative + Collavre::Creative.profile_for(self) + rescue StandardError => e + Rails.logger.error("[HasProfileCreative] Failed to create profile for user #{id}: #{e.message}") + end + end +end diff --git a/engines/collavre/app/models/concerns/collavre/indexed_json_columns.rb b/engines/collavre/app/models/concerns/collavre/indexed_json_columns.rb index bdb38bf39..00c40d09f 100644 --- a/engines/collavre/app/models/concerns/collavre/indexed_json_columns.rb +++ b/engines/collavre/app/models/concerns/collavre/indexed_json_columns.rb @@ -51,6 +51,14 @@ def indexed_json_columns(json:, columns:) def sync_indexed_json_columns json = public_send(indexed_json_source) || {} indexed_json_column_map.each do |column, json_key| + # A promoted column may not exist yet when a record is saved during a + # migration that runs before the add-column migration -- a full replay + # reaches older record-saving migrations (e.g. 20251230113607_refactor_labels + # calls Creative.create!) first. The JSON attribute stays the source of + # truth, so skip the promoted column until it exists rather than aborting + # the save with MissingAttributeError; the next save re-derives it. + next unless has_attribute?(column) + self[column] = json[json_key] end end diff --git a/engines/collavre/app/services/collavre/agent_type_classifier.rb b/engines/collavre/app/services/collavre/agent_type_classifier.rb index aa7c85bea..fd0efae22 100644 --- a/engines/collavre/app/services/collavre/agent_type_classifier.rb +++ b/engines/collavre/app/services/collavre/agent_type_classifier.rb @@ -8,7 +8,7 @@ module AgentTypeClassifier # Returns the agent type string ("developer", "pm", "qa", "researcher", # "marketer", "planner") or "agent" as the default when nothing matches. def classify(user) - prompt = user.system_prompt.to_s.downcase + prompt = user.effective_system_prompt.to_s.downcase case prompt when /developer|개발/ then "developer" when /pm|project.?manager|프로젝트/ then "pm" diff --git a/engines/collavre/app/services/collavre/ai_agent/message_builder.rb b/engines/collavre/app/services/collavre/ai_agent/message_builder.rb index c40a199b5..713961eda 100644 --- a/engines/collavre/app/services/collavre/ai_agent/message_builder.rb +++ b/engines/collavre/app/services/collavre/ai_agent/message_builder.rb @@ -244,7 +244,15 @@ def context_changed_since_last_reply? .exists? agent_changed = @agent.updated_at > last_reply_at - creative_changed || agent_changed + # The canonical system prompt now lives in the agent's profile creative's + # data["markdown_source"]. Editing it directly bumps only that Creative + # row (not the agent User row), and the profile creative sits outside the + # rendered topic tree — so without this a stateful re-send would keep the + # old session prompt and drop the freshly-edited one. + profile_updated_at = @agent.profile_creative_if_present&.updated_at + prompt_changed = profile_updated_at.present? && profile_updated_at > last_reply_at + + creative_changed || agent_changed || prompt_changed end # Collects the IDs of all creatives whose content is included in the diff --git a/engines/collavre/app/services/collavre/ai_agent_service.rb b/engines/collavre/app/services/collavre/ai_agent_service.rb index b69739e2a..5f0c91db3 100644 --- a/engines/collavre/app/services/collavre/ai_agent_service.rb +++ b/engines/collavre/app/services/collavre/ai_agent_service.rb @@ -136,8 +136,13 @@ def prepare_rendering_context end def render_system_prompt(rendering_context) + # The prompt lives losslessly in the profile creative's markdown_source + # (data["markdown_source"]); `description` is the sanitized rendered view + # and would corrupt tags/angle-brackets, so never read it here. Fall back + # to the legacy system_prompt column for rows not yet backfilled. + template = @agent.effective_system_prompt rendered = AiSystemPromptRenderer.new( - template: @agent.system_prompt, + template: template, context: rendering_context ).render diff --git a/engines/collavre/app/services/collavre/mcp_service.rb b/engines/collavre/app/services/collavre/mcp_service.rb index 471fe1d4e..5070b06fe 100644 --- a/engines/collavre/app/services/collavre/mcp_service.rb +++ b/engines/collavre/app/services/collavre/mcp_service.rb @@ -139,6 +139,9 @@ def self.delete_tool(tool_name) def update_from_creative(input_creative) creative = input_creative.effective_origin + # Profile creatives are agent system prompts, not tool sources: a fenced + # `extend ToolMeta` example in a prompt must never register a real tool. + return if creative.profile? return unless creative.description.present? # Parse HTML to find code blocks diff --git a/engines/collavre/app/services/collavre/orchestration/agent_context_builder.rb b/engines/collavre/app/services/collavre/orchestration/agent_context_builder.rb index 36cd8ef7b..938d8ddd4 100644 --- a/engines/collavre/app/services/collavre/orchestration/agent_context_builder.rb +++ b/engines/collavre/app/services/collavre/orchestration/agent_context_builder.rb @@ -178,7 +178,7 @@ def permission_rank(permission) end def extract_agent_description(user) - prompt = user.system_prompt.to_s + prompt = user.effective_system_prompt.to_s # Extract first meaningful line or sentence first_line = prompt.lines.find { |l| l.strip.present? && !l.start_with?("#") } return "AI Agent" unless first_line diff --git a/engines/collavre/app/services/collavre/orchestration/arbiter.rb b/engines/collavre/app/services/collavre/orchestration/arbiter.rb index 39553c49f..d60b0501a 100644 --- a/engines/collavre/app/services/collavre/orchestration/arbiter.rb +++ b/engines/collavre/app/services/collavre/orchestration/arbiter.rb @@ -208,12 +208,15 @@ def calculate_keyword_score(agent, message_text) def extract_expertise_text(agent) texts = [] - # From agent's system_prompt (primary source for AI agents) - texts << agent.system_prompt if agent.respond_to?(:system_prompt) && agent.system_prompt.present? + # From agent's system_prompt (primary source for AI agents). Route + # through effective_system_prompt so a directly-edited profile creative + # is matched on, falling back to the legacy column for other agent types. + texts << agent.effective_system_prompt if agent.respond_to?(:effective_system_prompt) && agent.effective_system_prompt.present? + texts << agent.system_prompt if !agent.respond_to?(:effective_system_prompt) && agent.respond_to?(:system_prompt) && agent.system_prompt.present? # From AI agent profile (for nested ai_agent association) if agent.respond_to?(:ai_agent) && agent.ai_agent.present? - texts << agent.ai_agent.system_prompt if agent.ai_agent.respond_to?(:system_prompt) + texts << agent.ai_agent.effective_system_prompt if agent.ai_agent.respond_to?(:effective_system_prompt) texts << agent.ai_agent.description if agent.ai_agent.respond_to?(:description) end diff --git a/engines/collavre/app/services/collavre/typo_corrector.rb b/engines/collavre/app/services/collavre/typo_corrector.rb index 1c60ab61d..e179ce302 100644 --- a/engines/collavre/app/services/collavre/typo_corrector.rb +++ b/engines/collavre/app/services/collavre/typo_corrector.rb @@ -49,7 +49,7 @@ def build_client AiClient.new( vendor: agent&.llm_vendor.presence || ENV.fetch("COLLAVRE_DEFAULT_LLM_VENDOR", "gemini"), model: agent&.llm_model.presence || ENV.fetch("COLLAVRE_DEFAULT_LLM_MODEL", "gemini-3.1-flash-lite"), - system_prompt: agent&.system_prompt.presence || FALLBACK_SYSTEM_PROMPT, + system_prompt: agent&.effective_system_prompt.presence || FALLBACK_SYSTEM_PROMPT, llm_api_key: agent&.llm_api_key, gateway_url: agent&.gateway_url, # Vendor adapters (e.g. OpenClaw) resolve the gateway/key from the context diff --git a/engines/collavre/app/views/collavre/users/edit_ai.html.erb b/engines/collavre/app/views/collavre/users/edit_ai.html.erb index 13a3ab27a..36325f1e8 100644 --- a/engines/collavre/app/views/collavre/users/edit_ai.html.erb +++ b/engines/collavre/app/views/collavre/users/edit_ai.html.erb @@ -9,7 +9,17 @@
<%= form.label :system_prompt, t('collavre.users.new_ai.system_prompt_label') %> - <%= form.text_area :system_prompt, required: true, class: "stacked-form-control", rows: 5 %> + <%# Pre-fill from the canonical prompt (profile creative's markdown_source), + not the legacy system_prompt column. If the profile creative was edited + directly, the column is stale; posting it back through update_ai's sync + would silently clobber the profile-authored prompt on any unrelated + setting change. Rendering the effective value makes a no-touch save a + no-op on the prompt. + On a validation-error re-render, though, show the *submitted* value + (@user.system_prompt) — the failed save never synced the profile, so + effective_system_prompt would read the old canonical prompt and silently + discard the user's unsaved edit. %> + <%= form.text_area :system_prompt, value: (@user.errors.any? ? @user.system_prompt : @user.effective_system_prompt), required: true, class: "stacked-form-control", rows: 5 %>
<%= render "collavre/users/trigger_field", form: form, routing_expression: @user.routing_expression %> diff --git a/engines/collavre/app/views/collavre/users/new_ai.html.erb b/engines/collavre/app/views/collavre/users/new_ai.html.erb index 93f90b3c5..604acebb0 100644 --- a/engines/collavre/app/views/collavre/users/new_ai.html.erb +++ b/engines/collavre/app/views/collavre/users/new_ai.html.erb @@ -23,7 +23,7 @@
<%= form.label :system_prompt, t('collavre.users.new_ai.system_prompt_label') %> - <%= form.text_area :system_prompt, required: true, class: "stacked-form-control", rows: 5, value: @copy_source&.system_prompt || AiClient::SYSTEM_INSTRUCTIONS %> + <%= form.text_area :system_prompt, required: true, class: "stacked-form-control", rows: 5, value: @copy_source&.effective_system_prompt || AiClient::SYSTEM_INSTRUCTIONS %>
<%= render "collavre/users/trigger_field", form: form, routing_expression: @copy_source&.routing_expression %> diff --git a/engines/collavre/app/views/collavre/users/show.html.erb b/engines/collavre/app/views/collavre/users/show.html.erb index c1cb380ae..65acba888 100644 --- a/engines/collavre/app/views/collavre/users/show.html.erb +++ b/engines/collavre/app/views/collavre/users/show.html.erb @@ -33,7 +33,7 @@
<%= @user.llm_model %>
<%= t('collavre.users.new_ai.system_prompt_label') %>
- 
<%= @user.system_prompt %>
+ 
<%= @user.effective_system_prompt %>
<% else %> diff --git a/engines/collavre/db/migrate/20260720235959_add_kind_column_to_creatives.rb b/engines/collavre/db/migrate/20260720235959_add_kind_column_to_creatives.rb new file mode 100644 index 000000000..c2ab49185 --- /dev/null +++ b/engines/collavre/db/migrate/20260720235959_add_kind_column_to_creatives.rb @@ -0,0 +1,36 @@ +class AddKindColumnToCreatives < ActiveRecord::Migration[8.1] + # Promote the `kind` discriminator (stored in the `data` json column) to a + # real column. The unique index on it is added later by + # 20260721000010_add_unique_index_to_profile_creatives. + # + # WHY this runs BEFORE the profile backfills (20260721000000/000001): + # Collavre::IndexedJsonColumns installs a before_save that re-derives every + # promoted column from `data` on EVERY Creative save (`self["kind"] = ...`). + # The backfills save Creatives through the full model, so on an upgrade from + # main the column must already exist — otherwise each save raises + # ActiveModel::MissingAttributeError, the backfill's rescue swallows it, and + # the migration "succeeds" while creating no profiles at all. + # + # Idempotent: a fresh install loads schema.rb (column already present) and + # never runs migrations, but a machine that applied an earlier revision of + # this PR may already have the column, so guard the add. + def up + unless column_exists?(:creatives, :kind) + add_column :creatives, :kind, :string + end + + # Any Creative model already loaded in this migrate run cached its columns + # before `kind` existed; refresh so the backfills' before_save can write it. + Collavre::Creative.reset_column_information + + if connection.adapter_name == "PostgreSQL" + execute "UPDATE creatives SET kind = data->>'kind'" + else + execute "UPDATE creatives SET kind = json_extract(data, '$.kind')" + end + end + + def down + remove_column :creatives, :kind + end +end diff --git a/engines/collavre/db/migrate/20260721000000_backfill_profile_creatives.rb b/engines/collavre/db/migrate/20260721000000_backfill_profile_creatives.rb new file mode 100644 index 000000000..252ba6abc --- /dev/null +++ b/engines/collavre/db/migrate/20260721000000_backfill_profile_creatives.rb @@ -0,0 +1,15 @@ +class BackfillProfileCreatives < ActiveRecord::Migration[8.1] + disable_ddl_transaction! + + def up + Collavre::User.find_each(batch_size: 200) do |user| + Collavre::Creative.profile_for(user) + rescue => e + Rails.logger.error("[BackfillProfileCreatives] user #{user.id}: #{e.message}") + end + end + + def down + # No-op: profiles are idempotent and safe to keep. + end +end diff --git a/engines/collavre/db/migrate/20260721000001_backfill_profile_descriptions.rb b/engines/collavre/db/migrate/20260721000001_backfill_profile_descriptions.rb new file mode 100644 index 000000000..f9591554a --- /dev/null +++ b/engines/collavre/db/migrate/20260721000001_backfill_profile_descriptions.rb @@ -0,0 +1,15 @@ +class BackfillProfileDescriptions < ActiveRecord::Migration[8.1] + disable_ddl_transaction! + + def up + Collavre::User.find_each(batch_size: 200) do |user| + user.sync_profile_system_prompt! + rescue => e + Rails.logger.error("[BackfillProfileDescriptions] user #{user.id}: #{e.message}") + end + end + + def down + # No-op: descriptions are safe to keep. + end +end diff --git a/engines/collavre/db/migrate/20260721000010_add_unique_index_to_profile_creatives.rb b/engines/collavre/db/migrate/20260721000010_add_unique_index_to_profile_creatives.rb new file mode 100644 index 000000000..ebab58b4c --- /dev/null +++ b/engines/collavre/db/migrate/20260721000010_add_unique_index_to_profile_creatives.rb @@ -0,0 +1,66 @@ +class AddUniqueIndexToProfileCreatives < ActiveRecord::Migration[8.1] + # Enforce one profile creative per user at the DB level so a concurrent + # SELECT-then-INSERT race can never leave a split-brain duplicate — a second + # profile row the Creative tree/UI can edit while execution keeps reading the + # oldest one. Deterministic order(:id) lookups only *converged* on a duplicate; + # this *prevents* it. + # + # The discriminator lives in the `data` json column, but a partial unique + # index must NOT reference a JSON expression (`data->>'kind'`): schema.rb is + # dumped from the SQLite dev DB, where the predicate serializes to + # `json_extract(data, '$.kind')`, which is not a PostgreSQL function, so + # `db:schema:load` crashes on production. (An earlier revision used the JSON + # expression and rationalized it as Postgres-IMMUTABLE; that missed the + # SQLite-dump path this project documents in docs/engine_development.md.) + # Instead promote the discriminator to a real `kind` column kept in sync from + # `data` by Collavre::IndexedJsonColumns, and index that plain column — it + # dumps identically on both adapters. Mirrors the reference promote-and-reindex + # migration 20260702000005_promote_channel_config_index_columns. + # + # The `kind` column itself is added earlier by 20260720235959, BEFORE the + # profile backfills (20260721000000/000001) run, so those model saves don't + # raise on a missing column. By the time this migration runs, every profile + # row (pre-existing and backfilled) already carries kind = 'profile'. + def up + collapse_duplicate_profiles! + + add_index :creatives, :user_id, + unique: true, + where: "kind = 'profile'", + name: "index_creatives_on_user_id_profile_unique" + end + + def down + remove_index :creatives, name: "index_creatives_on_user_id_profile_unique" + end + + private + + # Keep the oldest profile per user (matches profile_for's order(:id) selection) + # and remove any race-created duplicates, which are freshly-created-and-unused + # by definition. Runs after the `kind` column is backfilled, so it groups on + # the plain column. + # + # Destroy through the model, not delete_all: every Creative gets a `Main` topic + # via after_create :create_main_topic, and topics.creative_id is a + # non-cascading foreign key. delete_all would bypass has_many :topics, + # dependent: :destroy — on Postgres the FK rejects the DELETE, and on SQLite + # (FKs often off) it leaves orphaned topic rows. destroy cascades to the Main + # topic (and its own dependents), which for a fresh duplicate are empty. + def collapse_duplicate_profiles! + dup_user_ids = Collavre::Creative + .where(kind: "profile") + .group(:user_id) + .having("COUNT(*) > 1") + .pluck(:user_id) + + dup_user_ids.each do |uid| + duplicates = Collavre::Creative + .where(kind: "profile") + .where(user_id: uid) + .order(:id) + .to_a + duplicates.drop(1).each(&:destroy!) + end + end +end diff --git a/engines/collavre/test/controllers/users_controller_ai_test.rb b/engines/collavre/test/controllers/users_controller_ai_test.rb index 7331e2263..8e54af4cd 100644 --- a/engines/collavre/test/controllers/users_controller_ai_test.rb +++ b/engines/collavre/test/controllers/users_controller_ai_test.rb @@ -118,4 +118,77 @@ class UsersControllerAiTest < ActionDispatch::IntegrationTest assert_equal "Name can't be blank", flash[:alert] assert_select "form[action=?]", update_ai_user_path(@ai_user) end + + # Regression: on a validation-error re-render, the form must preserve the + # user's submitted prompt edit. The failed save never synced the profile, so + # rendering value: @user.effective_system_prompt would read the old canonical + # markdown_source and silently discard the unsaved edit. + test "update_ai preserves the submitted prompt edit on a validation error" do + @ai_user.profile_creative.update!(content_type_input: "markdown", markdown_source: "OLD CANONICAL PROMPT") + + patch update_ai_user_url(@ai_user), params: { + user: { + name: "", # invalid -> validation error, re-render + system_prompt: "MY EDITED PROMPT" + } + } + assert_response :unprocessable_entity + assert_select "textarea#user_system_prompt", text: /MY EDITED PROMPT/ + assert_select "textarea#user_system_prompt", text: /OLD CANONICAL PROMPT/, count: 0 + end + + # Regression: the edit form must pre-fill the prompt from the canonical + # profile creative (data["markdown_source"]), not the legacy system_prompt + # column. If the profile creative was edited directly, the column is stale; + # rendering it would post it back and update_ai's sync would clobber the + # profile-authored prompt on any unrelated setting change. + test "edit_ai pre-fills the prompt from the canonical profile creative, not the stale column" do + @ai_user.update_column(:system_prompt, "STALE COLUMN PROMPT") + @ai_user.profile_creative.update!(content_type_input: "markdown", markdown_source: "CANONICAL EDITED PROMPT") + + get edit_ai_user_url(@ai_user) + assert_response :success + assert_select "textarea#user_system_prompt", text: /CANONICAL EDITED PROMPT/ + assert_select "textarea#user_system_prompt", text: /STALE COLUMN PROMPT/, count: 0 + end + + # Regression: with the form now posting the effective value, an unrelated + # setting change (model) re-posts the canonical prompt and must not erase a + # directly-edited profile prompt. + test "update_ai preserves a directly-edited profile prompt when re-posting the effective value" do + @ai_user.update_column(:system_prompt, "STALE COLUMN PROMPT") + @ai_user.profile_creative.update!(content_type_input: "markdown", markdown_source: "CANONICAL EDITED PROMPT") + + patch update_ai_user_url(@ai_user), params: { + user: { + system_prompt: @ai_user.effective_system_prompt, # what the fixed form posts + llm_model: "gemini-1.5-pro" + } + } + assert_redirected_to edit_ai_user_path(@ai_user) + @ai_user.reload + assert_equal "gemini-1.5-pro", @ai_user.llm_model + assert_equal "CANONICAL EDITED PROMPT", @ai_user.effective_system_prompt + assert_equal "CANONICAL EDITED PROMPT", @ai_user.profile_creative.data["markdown_source"] + end + + # Regression: a partial settings update that omits system_prompt entirely + # (e.g. a routing_expression-only PATCH) must not run the prompt sync at all. + # Otherwise the stale legacy column is copied back over the canonical + # profile-authored prompt, silently erasing a directly-edited prompt. + test "update_ai does not clobber the canonical prompt on a PATCH that omits system_prompt" do + @ai_user.update_column(:system_prompt, "STALE COLUMN PROMPT") + @ai_user.profile_creative.update!(content_type_input: "markdown", markdown_source: "CANONICAL EDITED PROMPT") + + patch update_ai_user_url(@ai_user), params: { + user: { + routing_expression: 'event_name == "comment_created"' + } + } + assert_redirected_to edit_ai_user_path(@ai_user) + @ai_user.reload + assert_equal 'event_name == "comment_created"', @ai_user.routing_expression + assert_equal "CANONICAL EDITED PROMPT", @ai_user.profile_creative.data["markdown_source"] + assert_equal "CANONICAL EDITED PROMPT", @ai_user.effective_system_prompt + end end diff --git a/engines/collavre/test/fixtures/topics.yml b/engines/collavre/test/fixtures/topics.yml new file mode 100644 index 000000000..2ba95205e --- /dev/null +++ b/engines/collavre/test/fixtures/topics.yml @@ -0,0 +1,10 @@ +_fixture: + model_class: Collavre::Topic + +# Intentionally empty. Registering the topics table as fixture-managed makes +# Rails DELETE FROM topics during fixture loading, before the whole-DB +# foreign-key check runs. Non-transactional (system) tests can create a user +# whose profile creative (Collavre::HasProfileCreative) is committed together +# with a main topic; the raw fixture DELETE of creatives bypasses +# dependent: :destroy and would otherwise leave that topic orphaned, tripping +# check_all_foreign_keys_valid! for every subsequent fixture load. diff --git a/engines/collavre/test/models/collavre/creative_profile_test.rb b/engines/collavre/test/models/collavre/creative_profile_test.rb new file mode 100644 index 000000000..70ff5501e --- /dev/null +++ b/engines/collavre/test/models/collavre/creative_profile_test.rb @@ -0,0 +1,161 @@ +# engines/collavre/test/models/collavre/creative_profile_test.rb +require "test_helper" + +module Collavre + class CreativeProfileTest < ActiveSupport::TestCase + setup { @user = Collavre::User.create!(name: "Ann", email: "ann@example.com", password: "password123") } + + test "profile_for creates a profile creative owned by the user" do + profile = Collavre::Creative.profile_for(@user) + assert_equal "profile", profile.data["kind"] + assert_equal @user.id, profile.user_id + end + + test "profile_for is idempotent" do + first = Collavre::Creative.profile_for(@user) + assert_no_difference -> { Collavre::Creative.profiles.where(user: @user).count } do + assert_equal first.id, Collavre::Creative.profile_for(@user).id + end + end + + test "kind discriminator is a reserved metadata key" do + # `kind` scopes profile/inbox/skill discovery; it must survive a metadata + # save that omits it, or the profile becomes undiscoverable and duplicates. + assert_includes Collavre::Creative.reserved_metadata_keys, "kind" + end + + test "preserving reserved keys keeps a profile discoverable after a metadata save" do + profile = Collavre::Creative.profile_for(@user) + # Simulate update_metadata's preservation loop with a payload omitting kind. + incoming = { "theme" => "dark" } + Collavre::Creative.reserved_metadata_keys.each do |key| + if profile.data.key?(key) + incoming[key] = profile.data[key] + else + incoming.delete(key) + end + end + profile.update!(data: incoming) + assert_equal profile.id, Collavre::Creative.profile_for(@user).id + assert_equal "profile", profile.reload.data["kind"] + end + + # A duplicate profile is a split-brain hazard: prompt edits could land on one + # profile (via the generic Creative tree) while agent execution reads another. + # A partial unique index (index_creatives_on_user_id_profile_unique) enforces + # one profile per user at the DB, so the duplicate can never exist. The index + # is over the promoted `kind` column (IndexedJsonColumns keeps it synced from + # `data`), not a `data->>'kind'` JSON expression, so it dumps identically on + # SQLite and PostgreSQL and never crashes `db:schema:load`. + test "the database rejects a second profile creative for the same user" do + Collavre::Creative.profile_for(@user) + assert_raises(ActiveRecord::RecordNotUnique) do + Collavre::Creative.create!( + description: @user.name.to_s, + data: { "kind" => Collavre::Creative::PROFILE_KIND }, + user: @user, + progress: 0.0 + ) + end + end + + # profile_for creates via create_or_find_by!: a create that loses the race + # (unique-index violation) must resolve to the surviving row, not raise and + # not add a second profile. Exercise that path directly against the real + # scope, since profile_for's read-first fast path would otherwise short it. + test "create_or_find_by resolves a lost create race to the surviving profile" do + first = Collavre::Creative.profile_for(@user) + resolved = nil + assert_no_difference -> { Collavre::Creative.profiles.where(user: @user).count } do + assert_nothing_raised do + resolved = Collavre::Creative.profiles.create_or_find_by!(user: @user) do |creative| + creative.description = @user.name.to_s + creative.data = { "kind" => Collavre::Creative::PROFILE_KIND } + creative.progress = 0.0 + end + end + end + assert_equal first.id, resolved.id + end + + # The State A migration collapses any pre-index duplicate profiles onto the + # oldest before adding the unique index. Each duplicate Creative has a `Main` + # topic (after_create :create_main_topic) behind a non-cascading + # topics.creative_id FK, so a raw delete_all would be rejected on Postgres and + # leave orphaned topics on SQLite. The collapse must destroy through the model + # so the Main topic goes with it. + test "collapse_duplicate_profiles removes duplicates and their Main topics" do + conn = Collavre::Creative.connection + index = "index_creatives_on_user_id_profile_unique" + conn.remove_index :creatives, name: index + begin + keeper = Collavre::Creative.profile_for(@user) + duplicate = Collavre::Creative.create!( + description: @user.name.to_s, + data: { "kind" => Collavre::Creative::PROFILE_KIND }, + user: @user, + progress: 0.0 + ) + dup_topic_id = duplicate.topics.find_by(name: Collavre::Creative::MAIN_TOPIC_NAME).id + + require Rails.root.join("engines/collavre/db/migrate/20260721000010_add_unique_index_to_profile_creatives").to_s + AddUniqueIndexToProfileCreatives.new.send(:collapse_duplicate_profiles!) + + assert Collavre::Creative.exists?(keeper.id), "oldest profile must survive" + assert_not Collavre::Creative.exists?(duplicate.id), "duplicate profile must be removed" + assert_not Collavre::Topic.exists?(dup_topic_id), "duplicate's Main topic must not orphan" + ensure + conn.add_index :creatives, :user_id, unique: true, + where: "kind = 'profile'", name: index + end + end + + # Profile and inbox share user_id but differ by kind; both must coexist. + test "profile and inbox creatives coexist for the same user" do + profile = Collavre::Creative.profile_for(@user) + inbox = Collavre::Creative.inbox_for(@user) + assert_not_equal profile.id, inbox.id + assert_equal "profile", profile.data["kind"] + assert_equal "inbox", inbox.data["kind"] + end + + # The `kind` discriminator is promoted from `data` into a real column + # (Collavre::IndexedJsonColumns) so the profile-uniqueness index is a portable + # plain-column index rather than a per-adapter JSON expression. The promoted + # column must stay synced with data["kind"] on every save — the DB index + # enforces uniqueness against it, so a stale column would defeat the guarantee. + test "promotes the kind discriminator from data into the real column on save" do + profile = Collavre::Creative.profile_for(@user) + assert_equal "profile", profile.kind + assert_equal "profile", Collavre::Creative.find(profile.id).kind + + inbox = Collavre::Creative.inbox_for(@user) + assert_equal "inbox", inbox.reload.kind + end + + # IndexedJsonColumns installs a before_save that writes `self["kind"]` from + # `data` on EVERY Creative save. The profile backfill migrations save + # Creatives through the full model, so on an upgrade from main the `kind` + # column MUST be added before those backfills run — otherwise each save + # raises MissingAttributeError, the backfill's rescue swallows it, and the + # migration silently creates no profiles. CI can't catch this (it loads + # schema.rb, never replaying migration order), so lock the invariant here. + test "the kind column is added before any migration that backfills profiles" do + migrate_dir = Collavre::Engine.root.join("db", "migrate") + version = ->(glob) do + path = Dir[migrate_dir.join(glob)].sort.first + assert path, "expected a migration matching #{glob}" + File.basename(path)[/\A\d+/] + end + + add_kind = version.call("*_add_kind_column_to_creatives.rb") + backfill_creatives = version.call("*_backfill_profile_creatives.rb") + backfill_descriptions = version.call("*_backfill_profile_descriptions.rb") + + assert add_kind < backfill_creatives, + "kind column (#{add_kind}) must be added before backfilling profiles (#{backfill_creatives})" + assert add_kind < backfill_descriptions, + "kind column (#{add_kind}) must be added before backfilling descriptions (#{backfill_descriptions})" + end + end +end diff --git a/engines/collavre/test/models/collavre/indexed_json_columns_test.rb b/engines/collavre/test/models/collavre/indexed_json_columns_test.rb index 98f54f413..678c88ea1 100644 --- a/engines/collavre/test/models/collavre/indexed_json_columns_test.rb +++ b/engines/collavre/test/models/collavre/indexed_json_columns_test.rb @@ -65,5 +65,28 @@ class IndexedJsonColumnsTest < ActiveSupport::TestCase assert_nil channel.reload.worktree_id end + + # Regression: a full migration replay reaches older record-saving migrations + # (e.g. 20251230113607_refactor_labels calls Creative.create!) before the + # add-column migration that promotes the indexed key. The before_save must + # not abort the save by writing a column the table does not have yet. + test "skips promoted columns absent from the table instead of raising" do + original_map = Channel.indexed_json_column_map + Channel.indexed_json_column_map = original_map.merge("kind_not_yet_added" => "kind").freeze + begin + channel = nil + assert_nothing_raised do + channel = Channel.create!( + topic: @topic, + type: "Collavre::Channel", + config: { "repo_full_name" => "acme/app", "kind" => "profile" } + ) + end + # Existing promoted columns are still synced; only the missing one is skipped. + assert_equal "acme/app", channel.repo_full_name + ensure + Channel.indexed_json_column_map = original_map + end + end end end diff --git a/engines/collavre/test/models/collavre/user_profile_creative_test.rb b/engines/collavre/test/models/collavre/user_profile_creative_test.rb new file mode 100644 index 000000000..4a07846af --- /dev/null +++ b/engines/collavre/test/models/collavre/user_profile_creative_test.rb @@ -0,0 +1,16 @@ +require "test_helper" + +module Collavre + class UserProfileCreativeTest < ActiveSupport::TestCase + test "human user gets a profile creative on create" do + u = Collavre::User.create!(name: "Human", email: "h@example.com", password: "password123") + assert_equal "profile", u.profile_creative.data["kind"] + end + + test "AI agent also gets a profile creative on create" do + a = Collavre::User.create!(name: "Bot", email: "bot@example.com", password: "password123", llm_vendor: "google") + assert a.ai_user? + assert_equal "profile", a.profile_creative.data["kind"] + end + end +end diff --git a/engines/collavre/test/models/collavre/user_profile_system_prompt_test.rb b/engines/collavre/test/models/collavre/user_profile_system_prompt_test.rb new file mode 100644 index 000000000..cd8a9f350 --- /dev/null +++ b/engines/collavre/test/models/collavre/user_profile_system_prompt_test.rb @@ -0,0 +1,166 @@ +require "test_helper" + +module Collavre + class UserProfileSystemPromptTest < ActiveSupport::TestCase + test "sync mirrors the agent system prompt into the profile markdown_source" do + agent = Collavre::User.create!(name: "NamedBot", email: "syncbot1@ai.local", + password: "password123", llm_vendor: "google", + system_prompt: "REAL PROMPT") + # after_create seeds the profile description with the name — the bug we fix. + assert_equal "NamedBot", agent.profile_creative.description + agent.sync_profile_system_prompt! + assert_equal "REAL PROMPT", agent.profile_creative.reload.data["markdown_source"] + end + + test "sync stores prompts with tags and angle brackets losslessly" do + # Prompts routinely contain , tool tags, and < > &. These MUST + # survive verbatim in markdown_source; the sanitized `description` would + # strip/entity-escape them, which is exactly why the prompt is NOT stored + # in description. + prompt = "Wrap reasoning in .... " \ + "When count < 5 and result > 3 emit x. Use JSON & XML." + agent = Collavre::User.create!(name: "TagBot", email: "tagbot@ai.local", + password: "password123", llm_vendor: "google", + system_prompt: prompt) + agent.sync_profile_system_prompt! + assert_equal prompt, agent.profile_creative.reload.data["markdown_source"] + end + + test "sync stores an inline data-URI image in the prompt losslessly" do + # A prompt may legitimately reference an inline data-URI image outside a + # code span. The Markdown save path rewrites data-URI images into Active + # Storage blob paths (to avoid duplicate blobs on editor re-render), but + # effective_system_prompt sends markdown_source straight to the LLM, so the + # agent must receive the exact prompt the user entered — not a blob path. + prompt = "Reference logo you must watermark: " \ + "![logo](data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==)" + agent = Collavre::User.create!(name: "ImgBot", email: "imgbot@ai.local", + password: "password123", llm_vendor: "google", + system_prompt: prompt) + agent.sync_profile_system_prompt! + stored = agent.profile_creative.reload.data["markdown_source"] + assert_equal prompt, stored + refute_includes stored, "/rails/active_storage/blobs" + refute_includes stored, "/public-assets/blobs" + # The canonical prompt reaches every reader verbatim. + assert_equal prompt, agent.reload.effective_system_prompt + end + + test "editing the profile creative directly keeps a data-URI prompt verbatim" do + # The sync path sets skip_data_uri_rewrite, but a *direct* edit of the + # profile Creative through the normal Markdown save path does not — yet its + # markdown_source is equally LLM-bound. The data-URI rewrite must not fire + # for profile creatives regardless of caller (sync vs direct editor save). + prompt = "Watermark with this: " \ + "![logo](data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==)" + agent = Collavre::User.create!(name: "DirectBot", email: "directbot@ai.local", + password: "password123", llm_vendor: "google") + # Simulate a direct Creative-editor save: Markdown mode, no skip flag set. + agent.profile_creative.update!(content_type_input: "markdown", markdown_source: prompt) + stored = agent.profile_creative.reload.data["markdown_source"] + assert_equal prompt, stored + refute_includes stored, "/rails/active_storage/blobs" + refute_includes stored, "/public-assets/blobs" + assert_equal prompt, agent.reload.effective_system_prompt + end + + test "sync is idempotent and does not re-write an unchanged prompt" do + agent = Collavre::User.create!(name: "IdemBot", email: "idembot@ai.local", + password: "password123", llm_vendor: "google", + system_prompt: "STABLE PROMPT") + agent.sync_profile_system_prompt! + before = agent.profile_creative.reload.updated_at + agent.sync_profile_system_prompt! + assert_equal before, agent.profile_creative.reload.updated_at + end + + test "clearing the system prompt drops the stale markdown_source" do + agent = Collavre::User.create!(name: "ClearBot", email: "clearbot@ai.local", + password: "password123", llm_vendor: "google", + system_prompt: "FIRST PROMPT") + agent.sync_profile_system_prompt! + assert_equal "FIRST PROMPT", agent.profile_creative.reload.data["markdown_source"] + + # Admin blanks the prompt textarea — the old prompt must NOT stay authoritative. + agent.update!(system_prompt: "") + agent.sync_profile_system_prompt! + assert_nil agent.profile_creative.reload.data&.dig("markdown_source") + + # The old rendered prompt must not linger as the visible profile description; + # it is reseeded back to the name so it isn't searchable/shown on the root. + assert_equal "ClearBot", agent.profile_creative.reload.description + + svc = Collavre::AiAgentService.allocate + svc.instance_variable_set(:@agent, agent) + svc.instance_variable_set(:@creative, nil) + refute_includes svc.send(:render_system_prompt, {}), "FIRST PROMPT" + end + + test "effective_system_prompt prefers profile markdown_source over the legacy column" do + agent = Collavre::User.create!(name: "EffBot", email: "effbot@ai.local", + password: "password123", llm_vendor: "google", + system_prompt: "LEGACY COLUMN") + # Before sync the column is the only source. + assert_equal "LEGACY COLUMN", agent.effective_system_prompt + # A direct profile edit (no column write) must take effect for all readers. + agent.profile_creative.update!(content_type_input: "markdown", markdown_source: "EDITED IN PROFILE") + assert_equal "EDITED IN PROFILE", agent.reload.effective_system_prompt + end + + test "sync is a no-op for human users" do + human = Collavre::User.create!(name: "Human", email: "human1@example.com", password: "password123") + human.sync_profile_system_prompt! + assert_nil human.profile_creative.data&.dig("markdown_source") + assert_equal "Human", human.profile_creative.description + end + + test "sync is a no-op when the agent has no system prompt" do + agent = Collavre::User.create!(name: "Blanky", email: "syncbot2@ai.local", + password: "password123", llm_vendor: "google") + agent.sync_profile_system_prompt! + assert_nil agent.profile_creative.data&.dig("markdown_source") + assert_equal "Blanky", agent.profile_creative.description + end + + test "a profile prompt containing a tool-definition example does not register an MCP tool" do + # The profile description is the agent's system prompt, NOT a tool source. + # A prompt may legitimately show a fenced `extend ToolMeta` example (to + # instruct the agent), but Creative's description-change callback must not + # feed a profile creative into McpService, or prompt examples would create + # real, approval-gated tools on backfill / AI-settings sync / direct edit. + prompt = <<~PROMPT + When defining a tool, write: + + ```ruby + class Tools::PromptExampleTool + extend ToolMeta + tool_name "prompt_example_tool" + tool_description "Should never be registered from a prompt" + end + ``` + PROMPT + agent = Collavre::User.create!(name: "ToolBot", email: "toolbot@ai.local", + password: "password123", llm_vendor: "google", + system_prompt: prompt) + # Queue adapter is :inline in test, so UpdateMcpToolsJob would run here. + agent.sync_profile_system_prompt! + assert_nil McpTool.find_by(name: "prompt_example_tool"), + "profile prompt examples must not be registered as MCP tools" + end + + test "McpService#update_from_creative skips profile creatives regardless of caller" do + agent = Collavre::User.create!(name: "ToolBot2", email: "toolbot2@ai.local", + password: "password123", llm_vendor: "google") + agent.profile_creative.update!(content_type_input: "html", description: <<~HTML) + 
class Tools::DirectProfileTool
+          extend ToolMeta
+          tool_name "direct_profile_tool"
+          tool_description "Should never be registered from a profile"
+        end
+ HTML + McpService.new.update_from_creative(agent.profile_creative) + assert_nil McpTool.find_by(name: "direct_profile_tool"), + "a profile creative is never a tool source" + end + end +end diff --git a/engines/collavre/test/services/collavre/ai_agent/message_builder_test.rb b/engines/collavre/test/services/collavre/ai_agent/message_builder_test.rb index af440cae2..0fb3f7264 100644 --- a/engines/collavre/test/services/collavre/ai_agent/message_builder_test.rb +++ b/engines/collavre/test/services/collavre/ai_agent/message_builder_test.rb @@ -303,6 +303,32 @@ class MessageBuilderTest < ActiveSupport::TestCase assert result[:context_changed], "Should detect agent settings change" end + test "context_changed detects a direct profile prompt edit after last reply" do + # Create prior conversation + @creative.comments.create!(content: "Question", user: @user, topic_id: @comment.topic_id) + agent_reply = @creative.comments.create!(content: "Answer", user: @agent, topic_id: @comment.topic_id) + + # Edit the agent's profile creative (canonical prompt home) directly + # AFTER the reply. This bumps only the Creative row, not the agent (User) + # row, and the profile creative sits outside the rendered topic tree. + profile = @agent.profile_creative + profile.skip_data_uri_rewrite = true + profile.update!(content_type_input: "markdown", markdown_source: "You are a helpful specialist.") + assert profile.updated_at > agent_reply.created_at + assert_not @agent.updated_at > agent_reply.created_at, + "a direct profile edit must not touch the agent User row" + + context = { + "comment" => { "id" => @comment.id, "content" => "Follow-up" }, + "creative" => { "id" => @creative.id } + } + + builder = MessageBuilder.new(agent: @agent, context: context, original_comment: @comment) + result = builder.build + + assert result[:context_changed], "Should detect a direct profile prompt edit" + end + # An approval-action comment (approve button / approved label) is a human # decision surface. Blocking it at the dispatch seams is not enough: the # chat-history query would still load it as context on a later dispatch, diff --git a/engines/collavre/test/services/collavre/ai_agent_system_prompt_test.rb b/engines/collavre/test/services/collavre/ai_agent_system_prompt_test.rb new file mode 100644 index 000000000..da69d45f5 --- /dev/null +++ b/engines/collavre/test/services/collavre/ai_agent_system_prompt_test.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require "test_helper" + +module Collavre + class AiAgentSystemPromptTest < ActiveSupport::TestCase + setup do + @agent = Collavre::User.create!(name: "Bot", email: "bot2@example.com", + password: "password123", llm_vendor: "google", + system_prompt: "COLUMN PROMPT") + end + + test "prefers the profile markdown_source over the column" do + @agent.profile_creative.update!(content_type_input: "markdown", markdown_source: "PROFILE PROMPT") + svc = Collavre::AiAgentService.allocate + svc.instance_variable_set(:@agent, @agent) + svc.instance_variable_set(:@creative, nil) + assert_includes svc.send(:render_system_prompt, {}), "PROFILE PROMPT" + end + + test "renders a tag-bearing prompt verbatim, uncorrupted by the sanitizer" do + prompt = "Wrap reasoning in .... count < 5 and result > 3." + @agent.profile_creative.update!(content_type_input: "markdown", markdown_source: prompt) + svc = Collavre::AiAgentService.allocate + svc.instance_variable_set(:@agent, @agent) + svc.instance_variable_set(:@creative, nil) + out = svc.send(:render_system_prompt, {}) + assert_includes out, "..." + assert_includes out, "count < 5 and result > 3" + end + + test "falls back to the column when the profile has no markdown_source" do + # A profile creative not yet backfilled has no data["markdown_source"]; + # render must fall back to the legacy column, not the name-seeded description. + assert_nil @agent.profile_creative.data&.dig("markdown_source") + svc = Collavre::AiAgentService.allocate + svc.instance_variable_set(:@agent, @agent) + svc.instance_variable_set(:@creative, nil) + assert_includes svc.send(:render_system_prompt, {}), "COLUMN PROMPT" + end + + test "a normally created agent renders its prompt after sync, not its name" do + agent = Collavre::User.create!(name: "RenderBot", email: "renderbot@ai.local", + password: "password123", llm_vendor: "google", + system_prompt: "SYSTEM PROMPT TEXT") + agent.sync_profile_system_prompt! + svc = Collavre::AiAgentService.allocate + svc.instance_variable_set(:@agent, agent) + svc.instance_variable_set(:@creative, nil) + out = svc.send(:render_system_prompt, {}) + assert_includes out, "SYSTEM PROMPT TEXT" + refute_includes out, "RenderBot" + end + end +end diff --git a/engines/collavre/test/services/collavre/orchestration/agent_context_builder_test.rb b/engines/collavre/test/services/collavre/orchestration/agent_context_builder_test.rb index 591bcd8ee..750aa85d9 100644 --- a/engines/collavre/test/services/collavre/orchestration/agent_context_builder_test.rb +++ b/engines/collavre/test/services/collavre/orchestration/agent_context_builder_test.rb @@ -89,6 +89,29 @@ class AgentContextBuilderTest < ActiveSupport::TestCase assert_equal "write", collaborator["permission"] end + test "collaborator description follows the directly-edited profile prompt" do + Collavre::CreativeShare.create!( + creative: @creative, + user: @qa_agent, + permission: :write + ) + + # Simulate editing the collaborator's profile Creative directly: the + # canonical prompt now lives in data["markdown_source"] and diverges from + # the stale legacy column. The collaborator description must follow the + # canonical prompt, not the stale column. + profile = @qa_agent.profile_creative + profile.update!(content_type_input: "markdown", markdown_source: "You are a security auditor reviewing access control.") + + builder = AgentContextBuilder.new(agent: @ai_agent, creative: @creative) + result = builder.build + + collaborator = result["collaborators"].first + assert_equal @qa_agent.id, collaborator["id"] + assert_includes collaborator["description"], "security auditor" + refute_includes collaborator["description"], "QA engineer" + end + test "maps admin permission to escalation role" do Collavre::CreativeShare.create!( creative: @creative, diff --git a/engines/collavre/test/services/mcp_service_test.rb b/engines/collavre/test/services/mcp_service_test.rb index 835f4efca..662737ac7 100644 --- a/engines/collavre/test/services/mcp_service_test.rb +++ b/engines/collavre/test/services/mcp_service_test.rb @@ -58,6 +58,46 @@ class Tools::LexicalTool assert_includes tool.source_code, "class Tools::LexicalTool" end + test "update_from_creative skips profile creatives so prompt examples never register tools" do + user = users(:one) + creative = Creative.create!(user: user, data: { "kind" => Creative::PROFILE_KIND }, description: <<~HTML) +

Example tool your prompt may mention:

+ 
+        class Tools::PromptExampleTool
+          extend ToolMeta
+          tool_name "prompt_example_tool"
+          tool_description "Prompt Example"
+        end
+
+ HTML + + McpService.stub :register_tool_from_source, nil do + McpService.new.update_from_creative(creative) + end + + assert_nil McpTool.find_by(name: "prompt_example_tool"), + "A profile creative's prompt example must not register a real MCP tool" + end + + test "saving a profile creative description does not enqueue MCP tool extraction" do + user = users(:one) + creative = Creative.create!(user: user, data: { "kind" => Creative::PROFILE_KIND }, description: "

initial

") + + enqueued = false + Collavre::UpdateMcpToolsJob.stub :perform_later, ->(*) { enqueued = true } do + creative.update!(description: <<~HTML) + 
+          class Tools::PromptExampleTool
+            extend ToolMeta
+            tool_name "prompt_example_tool"
+          end
+
+ HTML + end + + assert_not enqueued, "Saving a profile creative description must not enqueue MCP tool extraction" + end + test "update_from_creative handles br tags in code blocks" do user = users(:one) creative = Creative.create!(user: user, description: <<~HTML) diff --git a/engines/collavre/test/services/typo_corrector_test.rb b/engines/collavre/test/services/typo_corrector_test.rb index adbbd642d..a3b8794d0 100644 --- a/engines/collavre/test/services/typo_corrector_test.rb +++ b/engines/collavre/test/services/typo_corrector_test.rb @@ -164,7 +164,7 @@ def correct(text, response) # OpenClaw (and any context-driven adapter) reads the gateway/key from the # context user, not the llm_api_key/gateway_url kwargs. Without this, the # adapter is built with user: nil and silently returns no edits. - agent = Struct.new(:llm_vendor, :llm_model, :system_prompt, :llm_api_key, :gateway_url).new + agent = Struct.new(:llm_vendor, :llm_model, :effective_system_prompt, :llm_api_key, :gateway_url).new Collavre.user_class.stub :find_by, agent do client = TypoCorrector.new.send(:build_client) diff --git a/engines/collavre_completion_api/app/controllers/collavre_completion_api/api/v1/chat/completions_controller.rb b/engines/collavre_completion_api/app/controllers/collavre_completion_api/api/v1/chat/completions_controller.rb index ae30685b9..1d069c29c 100644 --- a/engines/collavre_completion_api/app/controllers/collavre_completion_api/api/v1/chat/completions_controller.rb +++ b/engines/collavre_completion_api/app/controllers/collavre_completion_api/api/v1/chat/completions_controller.rb @@ -58,7 +58,11 @@ def resolve_agent def build_system_prompt(messages, agent) parts = [] - parts << agent.system_prompt if agent.system_prompt.present? + # Read through effective_system_prompt so a directly-edited profile + # Creative (canonical data["markdown_source"]) drives API completions + # too, not the stale legacy system_prompt column. + agent_prompt = agent.effective_system_prompt + parts << agent_prompt if agent_prompt.present? system_messages = messages.select { |m| m[:role] == "system" || m["role"] == "system" } system_messages.each do |msg| diff --git a/engines/collavre_completion_api/test/controllers/api/v1/completions_controller_test.rb b/engines/collavre_completion_api/test/controllers/api/v1/completions_controller_test.rb index 86576df14..6d231160f 100644 --- a/engines/collavre_completion_api/test/controllers/api/v1/completions_controller_test.rb +++ b/engines/collavre_completion_api/test/controllers/api/v1/completions_controller_test.rb @@ -91,6 +91,22 @@ class CompletionsControllerTest < ActionDispatch::IntegrationTest assert_response :bad_request end + test "build_system_prompt uses the canonical profile prompt, not the stale column" do + # A directly-edited profile Creative makes data["markdown_source"] canonical + # while the legacy system_prompt column goes stale. + @ai_bot.update!(system_prompt: "CANONICAL PROFILE PROMPT") + @ai_bot.sync_profile_system_prompt! + @ai_bot.update_column(:system_prompt, "STALE COLUMN PROMPT") + + controller = ::CollavreCompletionApi::Api::V1::Chat::CompletionsController.new + def controller.collavre_creative = nil + + prompt = controller.send(:build_system_prompt, [], @ai_bot.reload) + + assert_includes prompt, "CANONICAL PROFILE PROMPT" + refute_includes prompt, "STALE COLUMN PROMPT" + end + private def auth_headers