diff --git a/db/schema.rb b/db/schema.rb index 08628ba7e..ea0361d6c 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_24_000010) do +ActiveRecord::Schema[8.1].define(version: 2026_07_24_000000) do create_table "active_storage_attachments", force: :cascade do |t| t.bigint "blob_id", null: false t.datetime "created_at", null: false @@ -234,7 +234,6 @@ 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 @@ -245,7 +244,6 @@ 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 5117b52d3..485b74c6c 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,7 +43,6 @@ 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") @@ -62,12 +61,6 @@ 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 c11b953a4..88aa9bf7d 100644 --- a/engines/collavre/app/models/collavre/creative.rb +++ b/engines/collavre/app/models/collavre/creative.rb @@ -5,21 +5,12 @@ 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. + # "kind" must be reserved: it is the discriminator `inbox?`/`Creative.inboxes` + # match on, so a metadata save that omits it makes the row undiscoverable and + # `inbox_for` creates a duplicate inbox for that user. # --------------------------------------------------------------------------- BUILTIN_RESERVED_METADATA_KEYS = %w[markdown_source content_type editor kind].freeze @@ -81,17 +72,6 @@ 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" @@ -158,63 +138,6 @@ 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 - - created = false - creative = profiles.create_or_find_by!(user: user) do |c| - c.description = user.name.to_s - c.data = { "kind" => PROFILE_KIND } - c.progress = 0.0 - created = true - end - grant_creator_admin_share(creative, user) if created - creative - end - - # A managed AI agent's profile creative is owned by the agent itself, so its - # human creator has no Creative-level path to it (permissions are owner or - # CreativeShare only). Give the creator `admin` on creation: admin — not - # write — because the profile subtree is where the agent's skills live, and - # approving an agent's use of a feedback-level skill is an owner/admin act. - # - # Only for AI agents: `created_by_id` is also set on humans invited by - # another user, and a person's own profile must never be exposed to whoever - # created their account. - # - # Granted once, at profile creation only — never re-asserted on later - # profile_for calls, so a creator who deliberately removes the share (or - # lowers it) keeps that decision. - def self.grant_creator_admin_share(creative, user) - return unless user.respond_to?(:ai_user?) && user.ai_user? - - creator_id = user.try(:created_by_id) - return if creator_id.blank? || creator_id == user.id - - share = Collavre::CreativeShare.new( - creative: creative, - user_id: creator_id, - shared_by: user, - permission: :admin - ) - # The creator is the actor here, so the "X shared a creative with you" - # inbox message would be addressed from them to themselves. - share.skip_recipient_notification = true - share.save! - rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid => e - Rails.logger.warn("[Creative#profile_for] creator share skipped for user #{user.id}: #{e.message}") - end - private_class_method :grant_creator_admin_share - attr_accessor :filtered_progress belongs_to :user, class_name: Collavre.configuration.user_class_name, optional: true @@ -432,10 +355,6 @@ 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 427cb8606..0cdb7cff5 100644 --- a/engines/collavre/app/models/collavre/creative/describable.rb +++ b/engines/collavre/app/models/collavre/creative/describable.rb @@ -10,11 +10,6 @@ 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) @@ -152,17 +147,7 @@ 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. - # 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 + rewritten_source = Collavre::MarkdownConverter.rewrite_data_uri_images(new_source) self.data["markdown_source"] = rewritten_source self.description = Collavre::MarkdownConverter.markdown_to_html(rewritten_source) else diff --git a/engines/collavre/app/models/collavre/creative_share.rb b/engines/collavre/app/models/collavre/creative_share.rb index 6a48f44da..662064a8f 100644 --- a/engines/collavre/app/models/collavre/creative_share.rb +++ b/engines/collavre/app/models/collavre/creative_share.rb @@ -41,12 +41,7 @@ class CreativeShare < ApplicationRecord "permission" => :repropagate }.freeze - # Set on shares the system creates on the recipient's own behalf (e.g. the - # admin share an agent profile grants its creator), where the notification - # would read as the recipient sharing something with themselves. - attr_accessor :skip_recipient_notification - - after_create_commit :notify_recipient, unless: -> { no_access? || skip_recipient_notification } + after_create_commit :notify_recipient, unless: :no_access? after_save :touch_creative_subtree after_destroy :touch_creative_subtree diff --git a/engines/collavre/app/models/collavre/user.rb b/engines/collavre/app/models/collavre/user.rb index bdc6fdbb7..effc446ef 100644 --- a/engines/collavre/app/models/collavre/user.rb +++ b/engines/collavre/app/models/collavre/user.rb @@ -3,7 +3,6 @@ class User < ApplicationRecord self.table_name = "users" include HasInboxCreative - include HasProfileCreative has_many :user_themes, class_name: "Collavre::UserTheme", dependent: :destroy @@ -208,45 +207,6 @@ 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 deleted file mode 100644 index 01969cfc2..000000000 --- a/engines/collavre/app/models/concerns/collavre/has_profile_creative.rb +++ /dev/null @@ -1,39 +0,0 @@ -# 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 00c40d09f..bdb38bf39 100644 --- a/engines/collavre/app/models/concerns/collavre/indexed_json_columns.rb +++ b/engines/collavre/app/models/concerns/collavre/indexed_json_columns.rb @@ -51,14 +51,6 @@ 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 fd0efae22..aa7c85bea 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.effective_system_prompt.to_s.downcase + prompt = user.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 713961eda..c40a199b5 100644 --- a/engines/collavre/app/services/collavre/ai_agent/message_builder.rb +++ b/engines/collavre/app/services/collavre/ai_agent/message_builder.rb @@ -244,15 +244,7 @@ def context_changed_since_last_reply? .exists? agent_changed = @agent.updated_at > last_reply_at - # 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 + creative_changed || agent_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 5f0c91db3..b69739e2a 100644 --- a/engines/collavre/app/services/collavre/ai_agent_service.rb +++ b/engines/collavre/app/services/collavre/ai_agent_service.rb @@ -136,13 +136,8 @@ 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: template, + template: @agent.system_prompt, 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 5070b06fe..471fe1d4e 100644 --- a/engines/collavre/app/services/collavre/mcp_service.rb +++ b/engines/collavre/app/services/collavre/mcp_service.rb @@ -139,9 +139,6 @@ 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 938d8ddd4..36cd8ef7b 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.effective_system_prompt.to_s + prompt = user.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 d60b0501a..39553c49f 100644 --- a/engines/collavre/app/services/collavre/orchestration/arbiter.rb +++ b/engines/collavre/app/services/collavre/orchestration/arbiter.rb @@ -208,15 +208,12 @@ def calculate_keyword_score(agent, message_text) def extract_expertise_text(agent) texts = [] - # 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 agent's system_prompt (primary source for AI agents) + texts << agent.system_prompt if 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.effective_system_prompt if agent.ai_agent.respond_to?(:effective_system_prompt) + texts << agent.ai_agent.system_prompt if agent.ai_agent.respond_to?(: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 e179ce302..1c60ab61d 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&.effective_system_prompt.presence || FALLBACK_SYSTEM_PROMPT, + system_prompt: agent&.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 36325f1e8..13a3ab27a 100644 --- a/engines/collavre/app/views/collavre/users/edit_ai.html.erb +++ b/engines/collavre/app/views/collavre/users/edit_ai.html.erb @@ -9,17 +9,7 @@
<%= form.label :system_prompt, t('collavre.users.new_ai.system_prompt_label') %> - <%# 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 %> + <%= form.text_area :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 604acebb0..93f90b3c5 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&.effective_system_prompt || AiClient::SYSTEM_INSTRUCTIONS %> + <%= form.text_area :system_prompt, required: true, class: "stacked-form-control", rows: 5, value: @copy_source&.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 65acba888..c1cb380ae 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.effective_system_prompt %>
+ 
<%= @user.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 deleted file mode 100644 index c2ab49185..000000000 --- a/engines/collavre/db/migrate/20260720235959_add_kind_column_to_creatives.rb +++ /dev/null @@ -1,36 +0,0 @@ -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 deleted file mode 100644 index 252ba6abc..000000000 --- a/engines/collavre/db/migrate/20260721000000_backfill_profile_creatives.rb +++ /dev/null @@ -1,15 +0,0 @@ -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 deleted file mode 100644 index f9591554a..000000000 --- a/engines/collavre/db/migrate/20260721000001_backfill_profile_descriptions.rb +++ /dev/null @@ -1,15 +0,0 @@ -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 deleted file mode 100644 index ebab58b4c..000000000 --- a/engines/collavre/db/migrate/20260721000010_add_unique_index_to_profile_creatives.rb +++ /dev/null @@ -1,66 +0,0 @@ -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/db/migrate/20260724000010_backfill_agent_profile_creator_shares.rb b/engines/collavre/db/migrate/20260724000010_backfill_agent_profile_creator_shares.rb deleted file mode 100644 index 1c9393e9f..000000000 --- a/engines/collavre/db/migrate/20260724000010_backfill_agent_profile_creator_shares.rb +++ /dev/null @@ -1,36 +0,0 @@ -class BackfillAgentProfileCreatorShares < ActiveRecord::Migration[8.1] - disable_ddl_transaction! - - # Agent profile creatives created before the creator-admin grant landed have - # no share, leaving their human creator without any Creative-level path to the - # profile. Grant the same admin share here. - # - # Only AI agents: created_by_id is also set on humans invited by another user, - # and a person's own profile must not be exposed to whoever created them. - def up - Collavre::User - .where.not(created_by_id: nil) - .where.not(llm_vendor: [ nil, "" ]) - .find_each(batch_size: 200) do |agent| - profile = Collavre::Creative.where(user_id: agent.id, kind: "profile").order(:id).first - next unless profile - next if Collavre::CreativeShare.exists?(creative_id: profile.id, user_id: agent.created_by_id) - - share = Collavre::CreativeShare.new( - creative_id: profile.id, - user_id: agent.created_by_id, - shared_by_id: agent.id, - permission: :admin - ) - share.skip_recipient_notification = true - share.save! - rescue => e - Rails.logger.error("[BackfillAgentProfileCreatorShares] agent #{agent.id}: #{e.message}") - end - end - - def down - # No-op: a share cannot be distinguished from one the creator has since - # adjusted deliberately, so removing them could revoke intended access. - end -end diff --git a/engines/collavre/test/controllers/creatives_controller_update_test.rb b/engines/collavre/test/controllers/creatives_controller_update_test.rb index 31fa8dc7e..189d4efbb 100644 --- a/engines/collavre/test/controllers/creatives_controller_update_test.rb +++ b/engines/collavre/test/controllers/creatives_controller_update_test.rb @@ -104,6 +104,25 @@ class CreativesControllerUpdateTest < ActionDispatch::IntegrationTest assert_equal "baz", creative.data["foo"] end + test "update_metadata preserves the inbox kind so the inbox stays discoverable" do + inbox = Creative.inbox_for(@user) + assert inbox.inbox? + + # A stale popup payload (or an API client) that omits "kind" must not make the + # row stop matching Creative.inboxes — inbox_for would then create a duplicate. + patch update_metadata_creative_url(inbox), params: { + data: { foo: "bar" }.to_json + } + + assert_response :success + inbox.reload + assert inbox.inbox?, "inbox creative lost data[\"kind\"] on a metadata save" + assert_equal "bar", inbox.data["foo"] + assert_no_difference -> { Creative.where(user: @user).inboxes.count } do + assert_equal inbox.id, Creative.inbox_for(@user).id + end + end + test "update_metadata rejects non-hash JSON arrays without clobbering markdown fields" do creative = Creative.create!( description: "

html

", diff --git a/engines/collavre/test/controllers/users_controller_ai_test.rb b/engines/collavre/test/controllers/users_controller_ai_test.rb index 3db77de10..7331e2263 100644 --- a/engines/collavre/test/controllers/users_controller_ai_test.rb +++ b/engines/collavre/test/controllers/users_controller_ai_test.rb @@ -118,116 +118,4 @@ 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 - - # The agent owns its own profile creative, so without this share the creator - # has no Creative-level path to it — and cannot administer the skills that - # will live in its subtree. - test "create_ai gives the creator admin on the new agent's profile creative" do - post create_ai_users_url, params: { - ai_id: "sharedbot", - name: "Shared Bot", - system_prompt: "You are helpful.", - llm_vendor: "google", - llm_model: "gemini-1.5-pro" - } - - agent = Collavre::User.find_by(email: "sharedbot@ai.local") - assert agent, "expected the agent to be created" - - profile = agent.profile_creative - assert_equal agent.id, profile.user_id - assert profile.has_permission?(@admin, :admin), - "the creator should hold admin on the agent's profile creative" - end - - # The point of admin-on-the-profile is reach into the subtree, where the - # agent's skills will be created or linked. - test "creator reaches creatives placed under the agent profile" do - post create_ai_users_url, params: { - ai_id: "skillbot", - name: "Skill Bot", - llm_vendor: "google", - llm_model: "gemini-1.5-pro" - } - agent = Collavre::User.find_by(email: "skillbot@ai.local") - profile = agent.profile_creative - - skill = Collavre::Creative.create!(description: "A skill", user: agent, parent: profile, progress: 0.0) - - assert skill.has_permission?(@admin, :admin), - "admin on the profile should reach a creative placed in its subtree" - end end diff --git a/engines/collavre/test/fixtures/topics.yml b/engines/collavre/test/fixtures/topics.yml deleted file mode 100644 index 2ba95205e..000000000 --- a/engines/collavre/test/fixtures/topics.yml +++ /dev/null @@ -1,10 +0,0 @@ -_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 deleted file mode 100644 index 70ff5501e..000000000 --- a/engines/collavre/test/models/collavre/creative_profile_test.rb +++ /dev/null @@ -1,161 +0,0 @@ -# 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 678c88ea1..98f54f413 100644 --- a/engines/collavre/test/models/collavre/indexed_json_columns_test.rb +++ b/engines/collavre/test/models/collavre/indexed_json_columns_test.rb @@ -65,28 +65,5 @@ 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 deleted file mode 100644 index 1124cedee..000000000 --- a/engines/collavre/test/models/collavre/user_profile_creative_test.rb +++ /dev/null @@ -1,77 +0,0 @@ -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 - - test "AI agent profile creative is shared with its creator as admin" do - creator = Collavre::User.create!(name: "Maker", email: "maker@example.com", password: "password123") - agent = Collavre::User.create!(name: "Bot", email: "bot2@example.com", password: "password123", - llm_vendor: "google", created_by_id: creator.id) - - profile = agent.profile_creative - share = Collavre::CreativeShare.find_by(creative: profile, user: creator) - - assert share, "expected the creator to be shared on the agent profile creative" - assert_equal "admin", share.permission - assert profile.has_permission?(creator, :admin) - # The agent still owns its own profile; the creator reaches it via the share. - assert_equal agent.id, profile.user_id - end - - test "human user profile creative is not shared with whoever created them" do - inviter = Collavre::User.create!(name: "Inviter", email: "inviter@example.com", password: "password123") - human = Collavre::User.create!(name: "Invitee", email: "invitee@example.com", password: "password123", - created_by_id: inviter.id) - - profile = human.profile_creative - - assert_nil Collavre::CreativeShare.find_by(creative: profile, user: inviter), - "a human's personal profile must not be exposed to whoever created their account" - end - - test "agent without a creator gets no profile share" do - agent = Collavre::User.create!(name: "Orphan bot", email: "bot3@example.com", password: "password123", - llm_vendor: "google") - - assert_empty Collavre::CreativeShare.where(creative: agent.profile_creative) - end - - test "removing the creator share is not undone by a later profile_for" do - creator = Collavre::User.create!(name: "Maker", email: "maker2@example.com", password: "password123") - agent = Collavre::User.create!(name: "Bot", email: "bot4@example.com", password: "password123", - llm_vendor: "google", created_by_id: creator.id) - profile = agent.profile_creative - Collavre::CreativeShare.find_by(creative: profile, user: creator).destroy! - - Collavre::Creative.profile_for(agent) - - assert_nil Collavre::CreativeShare.find_by(creative: profile, user: creator), - "a deliberately removed share must not be resurrected" - end - - test "creator share does not post a self-referential inbox notification" do - creator = Collavre::User.create!(name: "Maker", email: "maker3@example.com", password: "password123") - Collavre::Current.user = creator - inbox = Collavre::Creative.inbox_for(creator) - before = Collavre::Comment.where(creative: inbox).count - - Collavre::User.create!(name: "Bot", email: "bot5@example.com", password: "password123", - llm_vendor: "google", created_by_id: creator.id) - - assert_equal before, Collavre::Comment.where(creative: inbox).count, - "the creator should not be told they shared something with themselves" - ensure - Collavre::Current.user = nil - 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 deleted file mode 100644 index cd8a9f350..000000000 --- a/engines/collavre/test/models/collavre/user_profile_system_prompt_test.rb +++ /dev/null @@ -1,166 +0,0 @@ -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 0fb3f7264..af440cae2 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,32 +303,6 @@ 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 deleted file mode 100644 index da69d45f5..000000000 --- a/engines/collavre/test/services/collavre/ai_agent_system_prompt_test.rb +++ /dev/null @@ -1,55 +0,0 @@ -# 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 750aa85d9..591bcd8ee 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,29 +89,6 @@ 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 662737ac7..835f4efca 100644 --- a/engines/collavre/test/services/mcp_service_test.rb +++ b/engines/collavre/test/services/mcp_service_test.rb @@ -58,46 +58,6 @@ 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 a3b8794d0..adbbd642d 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, :effective_system_prompt, :llm_api_key, :gateway_url).new + agent = Struct.new(:llm_vendor, :llm_model, :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 1d069c29c..ae30685b9 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,11 +58,7 @@ def resolve_agent def build_system_prompt(messages, agent) parts = [] - # 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? + parts << agent.system_prompt if agent.system_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 6d231160f..86576df14 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,22 +91,6 @@ 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