From 8211fa6ad56b7756c3a43e736243a9540ade86fc Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Thu, 13 Aug 2026 19:38:08 -0500 Subject: [PATCH 1/2] Add agent-first library organization API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Give agents a bulk view of a library (folder map with descriptions, counts, tags, activity), compact plan contents for classification, placement operations (move, move_many, move_by_tag, folder create/describe/delete) with dry_run and per-op savepoints, a run_id grouping every organize request, and an append-only LibraryEvent audit log that records who moved what where — human, local agent, cloud persona, or system. Amp-Thread-ID: https://ampcode.com/threads/T-019ffc42-a65a-707b-b5e3-79b0276a17c5 Co-authored-by: Amp --- app/admin/library_events.rb | 37 ++ ...d_description_to_coplan_folders.co_plan.rb | 9 + ...09_create_coplan_library_events.co_plan.rb | 36 ++ db/schema.rb | 57 ++- .../coplan/agent_instructions_controller.rb | 11 + .../coplan/api/v1/folders_controller.rb | 43 ++ .../coplan/api/v1/libraries_controller.rb | 265 +++++++++++++ .../coplan/api/v1/plans_controller.rb | 44 ++- .../controllers/coplan/folders_controller.rb | 12 + engine/app/models/coplan/folder.rb | 28 +- engine/app/models/coplan/library.rb | 3 + engine/app/models/coplan/library_event.rb | 47 +++ .../services/coplan/libraries/log_event.rb | 63 +++ .../app/services/coplan/libraries/organize.rb | 362 +++++++++++++++++ engine/app/services/coplan/plans/place.rb | 47 ++- .../agent_instructions/organizing.text.erb | 118 ++++++ .../coplan/agent_instructions/show.text.erb | 13 +- engine/config/routes.rb | 20 + ...00000_add_description_to_coplan_folders.rb | 8 + ...0813000001_create_coplan_library_events.rb | 35 ++ spec/requests/api/v1/libraries_spec.rb | 374 ++++++++++++++++++ spec/requests/api/v1/plans_spec.rb | 30 ++ spec/services/plans/place_spec.rb | 49 +++ 23 files changed, 1692 insertions(+), 19 deletions(-) create mode 100644 app/admin/library_events.rb create mode 100644 db/migrate/20260813180708_add_description_to_coplan_folders.co_plan.rb create mode 100644 db/migrate/20260813180709_create_coplan_library_events.co_plan.rb create mode 100644 engine/app/controllers/coplan/api/v1/libraries_controller.rb create mode 100644 engine/app/models/coplan/library_event.rb create mode 100644 engine/app/services/coplan/libraries/log_event.rb create mode 100644 engine/app/services/coplan/libraries/organize.rb create mode 100644 engine/app/views/coplan/agent_instructions/organizing.text.erb create mode 100644 engine/db/migrate/20260813000000_add_description_to_coplan_folders.rb create mode 100644 engine/db/migrate/20260813000001_create_coplan_library_events.rb create mode 100644 spec/requests/api/v1/libraries_spec.rb diff --git a/app/admin/library_events.rb b/app/admin/library_events.rb new file mode 100644 index 00000000..ae5d7100 --- /dev/null +++ b/app/admin/library_events.rb @@ -0,0 +1,37 @@ +ActiveAdmin.register CoPlan::LibraryEvent, as: "LibraryEvent" do + actions :index, :show + + index do + selectable_column + id_column + column :library + column :event_type + column :before_value + column :after_value + column :actor_type + column :actor_user + column :created_at + actions + end + + filter :library + filter :event_type, as: :select, collection: CoPlan::LibraryEvent::EVENT_TYPES + filter :actor_type, as: :select, collection: CoPlan::LibraryEvent::ACTOR_TYPES + filter :created_at + + show do + attributes_table do + row :id + row :library + row :event_type + row :plan_id + row :folder_id + row :before_value + row :after_value + row :actor_type + row :actor_user + row :metadata + row :created_at + end + end +end diff --git a/db/migrate/20260813180708_add_description_to_coplan_folders.co_plan.rb b/db/migrate/20260813180708_add_description_to_coplan_folders.co_plan.rb new file mode 100644 index 00000000..806c6447 --- /dev/null +++ b/db/migrate/20260813180708_add_description_to_coplan_folders.co_plan.rb @@ -0,0 +1,9 @@ +# This migration comes from co_plan (originally 20260813000000) +class AddDescriptionToCoplanFolders < ActiveRecord::Migration[8.1] + def change + # A short human/agent-readable statement of what belongs in the folder + # (e.g. "Active Q3 work — move to Done when shipped"). Surfaced in the + # library overview API so agents can organize by meaning, not just name. + add_column :coplan_folders, :description, :string, limit: 255 + end +end diff --git a/db/migrate/20260813180709_create_coplan_library_events.co_plan.rb b/db/migrate/20260813180709_create_coplan_library_events.co_plan.rb new file mode 100644 index 00000000..43da5db3 --- /dev/null +++ b/db/migrate/20260813180709_create_coplan_library_events.co_plan.rb @@ -0,0 +1,36 @@ +# This migration comes from co_plan (originally 20260813000001) +class CreateCoplanLibraryEvents < ActiveRecord::Migration[8.1] + def change + # Append-only audit log for a library's organization: who filed/moved/ + # removed which plan, and who created/renamed/moved/deleted folders — + # with actor_type distinguishing humans from agents. Mirrors + # coplan_plan_events, but scoped to the library (the shelf), not the + # plan (the document). + # + # plan_id / folder_id are deliberately not foreign keys: audit rows must + # survive the deletion of what they describe. Paths and titles are + # denormalized into before/after/metadata so the log stays readable. + create_table :coplan_library_events, id: { type: :string, limit: 36 } do |t| + t.string :library_id, limit: 36, null: false + t.string :actor_id, limit: 36 + t.string :actor_type, null: false + t.string :event_type, null: false + t.string :plan_id, limit: 36 + t.string :folder_id, limit: 36 + # Groups every event applied by one bulk organize call, so a + # 2,000-move run reads as one entry point in the log, not noise. + t.string :run_id, limit: 36 + t.text :before_value + t.text :after_value + t.json :metadata + t.datetime :created_at, null: false + + t.index [ :library_id, :created_at ] + t.index :plan_id + t.index :event_type + t.index :run_id + end + + add_foreign_key :coplan_library_events, :coplan_libraries, column: :library_id + end +end diff --git a/db/schema.rb b/db/schema.rb index e83c344e..cc9bff5e 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_19_165429) do +ActiveRecord::Schema[8.1].define(version: 2026_08_13_180709) do create_table "active_admin_comments", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.bigint "author_id" t.string "author_type" @@ -53,16 +53,46 @@ t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true end + create_table "coplan_agent_events", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.datetime "acked_at" + t.string "api_token_id", limit: 36, null: false + t.string "comment_id", limit: 36 + t.string "comment_thread_id", limit: 36 + t.datetime "created_at", null: false + t.string "event_type", null: false + t.json "payload" + t.string "plan_id", limit: 36, null: false + t.index ["api_token_id", "acked_at"], name: "index_coplan_agent_events_on_api_token_id_and_acked_at" + t.index ["api_token_id", "id"], name: "index_coplan_agent_events_on_api_token_id_and_id" + t.index ["plan_id"], name: "index_coplan_agent_events_on_plan_id" + end + + create_table "coplan_agent_sessions", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.string "agent_name", null: false + t.string "api_token_id", limit: 36, null: false + t.datetime "created_at", null: false + t.datetime "last_activity_at" + t.string "plan_id", limit: 36, null: false + t.string "state", default: "pending", null: false + t.string "state_detail" + t.datetime "updated_at", null: false + t.index ["api_token_id"], name: "index_coplan_agent_sessions_on_api_token_id" + t.index ["plan_id", "api_token_id"], name: "index_coplan_agent_sessions_on_plan_id_and_api_token_id", unique: true + end + create_table "coplan_api_tokens", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.string "agent_name" t.datetime "created_at", null: false t.timestamp "expires_at" t.timestamp "last_used_at" t.string "name", null: false + t.string "parent_id", limit: 36 t.timestamp "revoked_at" t.string "token_digest", null: false t.string "token_prefix", limit: 8 t.datetime "updated_at", null: false t.string "user_id", limit: 36, null: false + t.index ["parent_id", "revoked_at"], name: "index_coplan_api_tokens_on_parent_id_and_revoked_at" t.index ["token_digest"], name: "index_coplan_api_tokens_on_token_digest", unique: true t.index ["user_id"], name: "index_coplan_api_tokens_on_user_id" end @@ -139,6 +169,7 @@ create_table "coplan_folders", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "created_at", null: false t.string "created_by_user_id", limit: 36 + t.string "description" t.string "library_id", limit: 36, null: false t.string "name", null: false t.string "parent_id", limit: 36 @@ -158,6 +189,24 @@ t.index ["owner_type", "owner_id"], name: "index_coplan_libraries_on_owner_type_and_owner_id", unique: true end + create_table "coplan_library_events", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.string "actor_id", limit: 36 + t.string "actor_type", null: false + t.text "after_value" + t.text "before_value" + t.datetime "created_at", null: false + t.string "event_type", null: false + t.string "folder_id", limit: 36 + t.string "library_id", limit: 36, null: false + t.json "metadata" + t.string "plan_id", limit: 36 + t.string "run_id", limit: 36 + t.index ["event_type"], name: "index_coplan_library_events_on_event_type" + t.index ["library_id", "created_at"], name: "index_coplan_library_events_on_library_id_and_created_at" + t.index ["plan_id"], name: "index_coplan_library_events_on_plan_id" + t.index ["run_id"], name: "index_coplan_library_events_on_run_id" + end + create_table "coplan_notifications", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.string "comment_id", limit: 36 t.string "comment_thread_id", limit: 36, null: false @@ -363,6 +412,11 @@ add_foreign_key "active_storage_attachments", "active_storage_blobs", column: "blob_id" add_foreign_key "active_storage_variant_records", "active_storage_blobs", column: "blob_id" + add_foreign_key "coplan_agent_events", "coplan_api_tokens", column: "api_token_id" + add_foreign_key "coplan_agent_events", "coplan_plans", column: "plan_id" + add_foreign_key "coplan_agent_sessions", "coplan_api_tokens", column: "api_token_id" + add_foreign_key "coplan_agent_sessions", "coplan_plans", column: "plan_id" + add_foreign_key "coplan_api_tokens", "coplan_api_tokens", column: "parent_id" add_foreign_key "coplan_api_tokens", "coplan_users", column: "user_id" add_foreign_key "coplan_comment_threads", "coplan_plan_versions", column: "addressed_in_plan_version_id" add_foreign_key "coplan_comment_threads", "coplan_plan_versions", column: "out_of_date_since_version_id" @@ -377,6 +431,7 @@ add_foreign_key "coplan_folders", "coplan_folders", column: "parent_id" add_foreign_key "coplan_folders", "coplan_libraries", column: "library_id" add_foreign_key "coplan_folders", "coplan_users", column: "created_by_user_id" + add_foreign_key "coplan_library_events", "coplan_libraries", column: "library_id" add_foreign_key "coplan_notifications", "coplan_comment_threads", column: "comment_thread_id" add_foreign_key "coplan_notifications", "coplan_comments", column: "comment_id" add_foreign_key "coplan_notifications", "coplan_plans", column: "plan_id" diff --git a/engine/app/controllers/coplan/agent_instructions_controller.rb b/engine/app/controllers/coplan/agent_instructions_controller.rb index 82d3b4e4..467b743f 100644 --- a/engine/app/controllers/coplan/agent_instructions_controller.rb +++ b/engine/app/controllers/coplan/agent_instructions_controller.rb @@ -49,6 +49,17 @@ def show end end + # Sub-instructions: the library-organizing guide, linked from the main + # doc and from library API responses. Markdown-only — it's fetched by + # agents mid-task, not browsed by humans (the main doc has the pretty + # HTML front door). + def organizing + @auth_instructions = CoPlan.configuration.agent_auth_instructions + @curl = CoPlan.configuration.agent_curl_prefix + @base = "#{request.base_url}#{root_path.chomp("/")}" + render layout: false, content_type: "text/markdown", formats: [:text] + end + private def prefers_html? diff --git a/engine/app/controllers/coplan/api/v1/folders_controller.rb b/engine/app/controllers/coplan/api/v1/folders_controller.rb index e9921eda..eef117ae 100644 --- a/engine/app/controllers/coplan/api/v1/folders_controller.rb +++ b/engine/app/controllers/coplan/api/v1/folders_controller.rb @@ -42,10 +42,15 @@ def create folder = Folder.create!( name: params[:name], + description: params[:description], parent: parent, library: library, created_by_user: current_user ) + Libraries::LogEvent.call( + library: library, actor: current_user, actor_type: api_author_type, + event_type: "folder_created", folder: folder, after: folder.path + ) render json: folder_json(folder), status: :created rescue ActiveRecord::RecordInvalid => e render json: { error: e.record.errors.full_messages.join(", ") }, status: :unprocessable_content @@ -59,6 +64,7 @@ def update attrs = {} attrs[:name] = params[:name] if params.key?(:name) + attrs[:description] = params[:description] if params.key?(:description) if params.key?(:parent_id) if params[:parent_id].present? parent = @folder.library.folders.find_by(id: params[:parent_id]) @@ -69,7 +75,10 @@ def update end end + old_path = @folder.path + old_description = @folder.description @folder.update!(attrs) + log_folder_update(old_path, old_description) render json: folder_json(@folder) rescue ActiveRecord::RecordInvalid => e render json: { error: e.record.errors.full_messages.join(", ") }, status: :unprocessable_content @@ -81,7 +90,13 @@ def destroy return render json: { error: "Not authorized" }, status: :forbidden end + path = @folder.path if @folder.destroy + Libraries::LogEvent.call( + library: @folder.library, actor: current_user, actor_type: api_author_type, + event_type: "folder_deleted", before: path, + metadata: { folder_name: @folder.name } + ) head :no_content else render json: { error: @folder.errors.full_messages.join(", ") }, status: :unprocessable_content @@ -95,6 +110,33 @@ def set_folder render json: { error: "Folder not found" }, status: :not_found unless @folder end + # One update call can rename, move, and re-describe at once — log + # each change as its own audit event so the library log stays + # readable ("renamed A → B", not "something about this folder"). + def log_folder_update(old_path, old_description) + new_path = @folder.path + if @folder.saved_change_to_name? + Libraries::LogEvent.call( + library: @folder.library, actor: current_user, actor_type: api_author_type, + event_type: "folder_renamed", folder: @folder, before: old_path, after: new_path + ) + end + if @folder.saved_change_to_parent_id? + Libraries::LogEvent.call( + library: @folder.library, actor: current_user, actor_type: api_author_type, + event_type: "folder_moved", folder: @folder, before: old_path, after: new_path + ) + end + if @folder.saved_change_to_description? + Libraries::LogEvent.call( + library: @folder.library, actor: current_user, actor_type: api_author_type, + event_type: "folder_described", folder: @folder, + before: old_description, after: @folder.description, + metadata: { path: new_path } + ) + end + end + # `paths` and `counts` let index serialize the whole tree without # per-folder queries. `plans_count` is the folder's own visible # placements (not including subfolders). @@ -102,6 +144,7 @@ def folder_json(folder, paths: nil, counts: nil) { id: folder.id, name: folder.name, + description: folder.description, library_id: folder.library_id, parent_id: folder.parent_id, path: paths ? paths[folder.id] : folder.path, diff --git a/engine/app/controllers/coplan/api/v1/libraries_controller.rb b/engine/app/controllers/coplan/api/v1/libraries_controller.rb new file mode 100644 index 00000000..25f1f6ae --- /dev/null +++ b/engine/app/controllers/coplan/api/v1/libraries_controller.rb @@ -0,0 +1,265 @@ +module CoPlan + module Api + module V1 + # The agent-facing organization API. A library is a shelf (folder tree + # + placements); this controller is how an agent learns a shelf's + # layout in one call (show), bulk-reads what's on it (contents), + # rearranges it (organize), and audits who rearranged it (events). + # + # Reads are open to any authenticated caller (counts and rows stay + # viewer-filtered, matching library browsing on the web); the audit + # log and all writes require write access to the library. + class LibrariesController < BaseController + before_action :set_library + + def index + libraries = Library.includes(:owner).order(:created_at) + render json: libraries.map { |library| + { + id: library.id, + name: library.name, + owner: owner_json(library), + writable: library.writable_by?(current_user), + mine: library.writable_by?(current_user) + } + } + end + + # The map of one library: nested-by-path folder list with + # descriptions and counts, unfiled work, top tags, and recent + # activity. Designed so one GET gives an agent everything it needs + # to understand the layout before organizing. + def show + folders = @library.folders.order(:name).to_a + paths = Folder.paths_by_id(folders) + counts = visible_placement_counts + totals = subtree_totals(folders, counts) + + json = { + id: @library.id, + name: @library.name, + owner: owner_json(@library), + writable: writable?, + folders: folders.sort_by { |f| paths[f.id].downcase }.map { |f| + { + id: f.id, + name: f.name, + description: f.description, + path: paths[f.id], + parent_id: f.parent_id, + plans_count: counts.fetch(f.id, 0), + total_plans_count: totals.fetch(f.id, 0) + } + }, + top_tags: top_tags_json, + organize_instructions_url: CoPlan::Engine.routes.url_helpers.agent_instructions_organizing_path + } + if writable? + json[:unfiled_count] = unfiled_plans.count + json[:recent_activity] = @library.library_events.recent_first.limit(10).map { |e| event_json(e) } + end + render json: json + end + + # Bulk read: one compact row per shelved plan — title, summary, + # tags, dates, location. Filters: folder_id/folder_path (+ + # recursive=true for the whole subtree), tag, unfiled=true (your + # own unshelved plans; writable libraries only), archived=true. + def contents + if params[:unfiled].to_s == "true" + return render json: { error: "unfiled=true requires write access to the library" }, status: :forbidden unless writable? + plans = unfiled_plans.includes(:tags, :created_by_user).order(updated_at: :desc) + return render json: { + library_id: @library.id, + count: plans.size, + items: plans.map { |plan| content_row(plan, placement: nil, paths: {}) } + } + end + + placements = @library.placements + .visible_to(current_user) + .includes(:placed_by_user, plan: [ :tags, :created_by_user ]) + placements = placements.where(plan: params[:archived].to_s == "true" ? Plan.archived : Plan.active) + + if params[:folder_id].present? || params[:folder_path].present? + folder = find_folder_param + return render json: { error: "Folder not found" }, status: :not_found unless folder + folder_ids = [ folder.id ] + folder_ids += folder.descendants.map(&:id) if params[:recursive].to_s == "true" + placements = placements.where(folder_id: folder_ids) + end + + if params[:tag].present? + placements = placements.where(plan_id: Plan.with_tag(params[:tag]).select(:id)) + end + + paths = Folder.paths_by_id(@library.folders.to_a) + rows = placements.to_a.sort_by { |p| [ paths[p.folder_id].to_s.downcase, p.plan.title.to_s.downcase ] } + + limit = params[:limit].present? ? params[:limit].to_i.clamp(1, 1000) : 500 + offset = params[:offset].to_i.clamp(0, rows.size) + + render json: { + library_id: @library.id, + count: rows.size, + items: rows[offset, limit].to_a.map { |p| content_row(p.plan, placement: p, paths: paths) } + } + end + + # The audit log: who filed/moved/removed what, when, and whether a + # human or an agent did it. Owner (and admin) only. + def events + unless writable? || current_user.admin? + return render json: { error: "Not authorized" }, status: :forbidden + end + + events = @library.library_events.recent_first.includes(:actor_user) + events = events.where(created_at: ...Time.iso8601(params[:before])) if params[:before].present? + events = events.where(event_type: params[:event_type]) if params[:event_type].present? + events = events.where(plan_id: params[:plan_id]) if params[:plan_id].present? + events = events.where(run_id: params[:run_id]) if params[:run_id].present? + limit = params[:limit].present? ? params[:limit].to_i.clamp(1, 200) : 50 + + render json: events.limit(limit).map { |e| event_json(e) } + rescue ArgumentError + render json: { error: "before must be an ISO 8601 timestamp" }, status: :unprocessable_content + end + + # Bulk write: an array of operations (see Libraries::Organize for + # the vocabulary), each atomic, with dry_run for proposing a + # reorganization before applying it. + def organize + result = Libraries::Organize.call( + library: @library, + actor: current_user, + operations: params[:operations], + actor_type: api_author_type, + actor_label: @api_token&.name, + dry_run: params[:dry_run].to_s == "true" + ) + unless result.success? + return render json: { error: result.error }, status: :unprocessable_content + end + + render json: { + dry_run: result.dry_run, + applied: !result.dry_run, + run_id: result.run_id, + ok_count: result.results.count { |r| r[:status] == "ok" }, + error_count: result.results.count { |r| r[:status] == "error" }, + results: result.results + } + end + + private + + # Blank id (the bare /api/v1/library routes) means the caller's own. + def set_library + @library = if params[:id].present? + Library.find_by(id: params[:id]) + else + current_user.library + end + render json: { error: "Library not found" }, status: :not_found unless @library + end + + def writable? + @library.writable_by?(current_user) + end + + def owner_json(library) + owner = library.owner + { + type: library.owner_type.demodulize.underscore, + id: library.owner_id, + name: owner.respond_to?(:name) ? owner.name : nil + } + end + + def visible_placement_counts + @library.placements + .visible_to(current_user) + .where(plan: Plan.active) + .group(:folder_id) + .count + end + + # Own count + every descendant's, computed from the in-memory tree. + def subtree_totals(folders, counts) + children = folders.group_by(&:parent_id) + totals = {} + compute = lambda do |folder, seen| + return 0 unless seen.add?(folder.id) + totals[folder.id] ||= counts.fetch(folder.id, 0) + + (children[folder.id] || []).sum { |child| compute.call(child, seen) } + end + folders.each { |f| compute.call(f, Set.new) } + totals + end + + # The library owner's active plans not yet shelved in this library — + # only meaningful when the caller can write (i.e. it's their shelf). + def unfiled_plans + current_user.created_plans + .active + .where.not(id: @library.placements.select(:plan_id)) + end + + def top_tags_json + placed_plan_ids = @library.placements.visible_to(current_user).select(:plan_id) + Tag.joins(:plan_tags) + .where(coplan_plan_tags: { plan_id: placed_plan_ids }) + .group("coplan_tags.id", "coplan_tags.name") + .order(Arel.sql("COUNT(*) DESC"), "coplan_tags.name ASC") + .limit(10) + .count + .map { |(_id, name), count| { name: name, plans_count: count } } + end + + def content_row(plan, placement:, paths:) + { + plan_id: plan.id, + title: plan.title, + summary: plan.summary, + tags: plan.tag_names, + visibility: plan.visibility, + archived: plan.archived?, + author: plan.created_by_user&.name, + folder_id: placement&.folder_id, + folder_path: placement ? paths[placement.folder_id] : nil, + placed_by: placement&.placed_by_user&.name, + placed_at: placement&.updated_at, + created_at: plan.created_at, + updated_at: plan.updated_at + } + end + + def event_json(event) + { + id: event.id, + event_type: event.event_type, + actor_type: event.actor_type, + agent: event.actor_type != "human", + actor: event.actor_user && { id: event.actor_user.id, name: event.actor_user.name }, + actor_label: event.metadata["actor_label"], + run_id: event.run_id, + plan_id: event.plan_id, + plan_title: event.metadata["plan_title"], + folder_id: event.folder_id, + before: event.before_value, + after: event.after_value, + created_at: event.created_at + } + end + + def find_folder_param + if params[:folder_id].present? + @library.folders.find_by(id: params[:folder_id]) + else + Folder.find_by_path(params[:folder_path], library: @library) + end + end + end + end + end +end diff --git a/engine/app/controllers/coplan/api/v1/plans_controller.rb b/engine/app/controllers/coplan/api/v1/plans_controller.rb index 58c6eb43..92b00750 100644 --- a/engine/app/controllers/coplan/api/v1/plans_controller.rb +++ b/engine/app/controllers/coplan/api/v1/plans_controller.rb @@ -2,8 +2,8 @@ module CoPlan module Api module V1 class PlansController < BaseController - before_action :set_plan, only: [ :show, :update, :versions, :comments, :snapshot ] - before_action :authorize_plan_access!, only: [ :show, :update, :versions, :comments, :snapshot ] + before_action :set_plan, only: [ :show, :update, :versions, :comments, :snapshot, :locations ] + before_action :authorize_plan_access!, only: [ :show, :update, :versions, :comments, :snapshot, :locations ] def index plans = Plan @@ -106,7 +106,7 @@ def update if params.key?(:folder_id) || params.key?(:folder_path) folder = resolve_folder_params return if performed? # resolve_folder_params rendered an error - result = Plans::Place.call(plan: @plan, folder: folder, actor: current_user) + result = Plans::Place.call(plan: @plan, folder: folder, actor: current_user, actor_type: api_author_type) unless result.success? render json: { error: result.error }, status: :unprocessable_content raise ActiveRecord::Rollback @@ -202,6 +202,31 @@ def versions render json: versions.map { |v| version_json(v) } end + # Everywhere this plan is shelved — the reverse lookup of "what + # folder is this document actually in?", across every library + # (yours, other people's, and future team libraries). + def locations + placements = @plan.placements.includes(:placed_by_user, library: :owner, folder: { parent: :parent }) + render json: placements.map { |placement| + library = placement.library + { + library_id: library.id, + library_name: library.name, + owner: { + type: library.owner_type.demodulize.underscore, + id: library.owner_id, + name: library.owner.respond_to?(:name) ? library.owner.name : nil + }, + writable: library.writable_by?(current_user), + folder_id: placement.folder_id, + folder_path: placement.folder.path, + folder_description: placement.folder.description, + placed_by: placement.placed_by_user&.name, + placed_at: placement.updated_at + } + } + end + def comments threads = @plan.comment_threads.includes(:comments, :created_by_user).order(created_at: :desc) render json: threads.map { |t| thread_json(t) } @@ -288,11 +313,20 @@ def resolve_folder_params render json: { error: "Unknown folder_id" }, status: :unprocessable_content unless folder folder elsif params[:folder_path].present? - Folder.find_or_create_by_path!( + created = [] + folder = Folder.find_or_create_by_path!( params[:folder_path], library: current_user.library, - created_by_user: current_user + created_by_user: current_user, + created: created ) + created.each do |f| + Libraries::LogEvent.call( + library: current_user.library, actor: current_user, actor_type: api_author_type, + event_type: "folder_created", folder: f, after: f.path + ) + end + folder else nil # blank folder_id / folder_path unfiles the plan end diff --git a/engine/app/controllers/coplan/folders_controller.rb b/engine/app/controllers/coplan/folders_controller.rb index b2613ba4..724f0d99 100644 --- a/engine/app/controllers/coplan/folders_controller.rb +++ b/engine/app/controllers/coplan/folders_controller.rb @@ -19,8 +19,16 @@ def update return render json: { error: "Unknown destination folder" }, status: :unprocessable_content unless parent end + old_path = folder.path folder.parent = parent if folder.save + if folder.saved_change_to_parent_id? + Libraries::LogEvent.call( + library: library, actor: current_user, + event_type: "folder_moved", folder: folder, + before: old_path, after: folder.path + ) + end render json: { parent_id: folder.parent_id, path: folder.path, @@ -57,6 +65,10 @@ def create ) if folder.save + Libraries::LogEvent.call( + library: library, actor: current_user, + event_type: "folder_created", folder: folder, after: folder.path + ) redirect_to plans_path(folder: folder.id), notice: "Folder “#{folder.name}” created." else redirect_back fallback_location: plans_path, diff --git a/engine/app/models/coplan/folder.rb b/engine/app/models/coplan/folder.rb index cd29c4cd..de605c88 100644 --- a/engine/app/models/coplan/folder.rb +++ b/engine/app/models/coplan/folder.rb @@ -31,6 +31,9 @@ class Folder < ApplicationRecord uniqueness: { scope: [ :library_id, :parent_id ], case_sensitive: false }, format: { with: NAME_FORMAT, message: "cannot contain \"/\"" }, length: { maximum: 100 } + # What belongs in this folder, in one line — read by agents (via the + # library overview API) to organize by meaning, not just name. + validates :description, length: { maximum: 255 } validate :parent_cannot_create_cycle validate :parent_must_share_library validate :depth_within_limit @@ -73,7 +76,10 @@ def path # invalid. Returns nil for a blank path. Lookup is case-insensitive # (matching the uniqueness validation); creation preserves the given # casing. - def self.find_or_create_by_path!(path, library:, created_by_user: nil) + # + # Pass an array as `created:` to collect the folders this call had to + # create (root-first) — callers use it to audit implicit creations. + def self.find_or_create_by_path!(path, library:, created_by_user: nil, created: nil) segments = path.to_s.split("/").map(&:strip).reject(&:blank?) return nil if segments.empty? @@ -82,11 +88,27 @@ def self.find_or_create_by_path!(path, library:, created_by_user: nil) transaction do segments.reduce(nil) do |parent, name| library.folders.where(parent_id: parent&.id).where("LOWER(name) = ?", name.downcase).first || - create!(name: name, parent: parent, library: library, created_by_user: created_by_user) + create!(name: name, parent: parent, library: library, created_by_user: created_by_user).tap do |folder| + created << folder if created + end end end end + # Case-insensitive lookup of a "/"-separated path within one library. + # Returns nil when any segment is missing — the read-only sibling of + # find_or_create_by_path!. + def self.find_by_path(path, library:) + segments = path.to_s.split("/").map(&:strip).reject(&:blank?) + return nil if segments.empty? + + segments.reduce(nil) do |parent, name| + folder = library.folders.where(parent_id: parent&.id).where("LOWER(name) = ?", name.downcase).first + return nil unless folder + folder + end + end + # Full "A/B/C" path for every given folder, keyed by id, computed from # the in-memory list (no per-folder queries). Shared by the folders API # and the folder-picker helper. @@ -105,7 +127,7 @@ def self.paths_by_id(folders = order(:name).to_a) end def self.ransackable_attributes(_auth_object = nil) - %w[id name library_id parent_id created_by_user_id created_at updated_at] + %w[id name description library_id parent_id created_by_user_id created_at updated_at] end def self.ransackable_associations(_auth_object = nil) diff --git a/engine/app/models/coplan/library.rb b/engine/app/models/coplan/library.rb index 5a4d9db9..12e36adf 100644 --- a/engine/app/models/coplan/library.rb +++ b/engine/app/models/coplan/library.rb @@ -14,6 +14,9 @@ class Library < ApplicationRecord belongs_to :owner, polymorphic: true has_many :folders, class_name: "CoPlan::Folder", dependent: :destroy has_many :placements, class_name: "CoPlan::PlanPlacement", dependent: :destroy + # Append-only audit rows; delete_all (not destroy) — no callbacks to run, + # and the FK would otherwise block destroying the library. + has_many :library_events, class_name: "CoPlan::LibraryEvent", dependent: :delete_all validates :name, presence: true, length: { maximum: 100 } validates :owner_id, uniqueness: { scope: :owner_type } diff --git a/engine/app/models/coplan/library_event.rb b/engine/app/models/coplan/library_event.rb new file mode 100644 index 00000000..b4e63482 --- /dev/null +++ b/engine/app/models/coplan/library_event.rb @@ -0,0 +1,47 @@ +module CoPlan + # A first-class audit entry for a library's organization — plans filed, + # moved, or removed; folders created, renamed, moved, described, or + # deleted. The library-side counterpart to PlanEvent: PlanEvent answers + # "what happened to this document?", LibraryEvent answers "who rearranged + # this shelf, and was it a human or an agent?". + # + # plan_id / folder_id are soft references (no FK): audit rows outlive the + # things they describe. Paths live in before_value/after_value and titles + # in metadata, so the log stays readable after deletion. + # + # Records are append-only — never updated, never destroyed except through + # the parent library's cascade. + class LibraryEvent < ApplicationRecord + # Mirrors PlanEvent::ACTOR_TYPES so both logs render uniformly. + ACTOR_TYPES = %w[human local_agent cloud_persona system].freeze + + EVENT_TYPES = %w[ + plan_filed + plan_moved + plan_removed + folder_created + folder_renamed + folder_moved + folder_described + folder_deleted + ].freeze + + belongs_to :library, class_name: "CoPlan::Library", inverse_of: :library_events + belongs_to :actor_user, class_name: "CoPlan::User", foreign_key: "actor_id", optional: true + + after_initialize { self.metadata ||= {} } + + validates :actor_type, presence: true, inclusion: { in: ACTOR_TYPES } + validates :event_type, presence: true, inclusion: { in: EVENT_TYPES } + + scope :recent_first, -> { order(created_at: :desc, id: :desc) } + + def self.ransackable_attributes(_auth_object = nil) + %w[id library_id actor_id actor_type event_type plan_id folder_id run_id before_value after_value created_at] + end + + def self.ransackable_associations(_auth_object = nil) + %w[library actor_user] + end + end +end diff --git a/engine/app/services/coplan/libraries/log_event.rb b/engine/app/services/coplan/libraries/log_event.rb new file mode 100644 index 00000000..35dccfbb --- /dev/null +++ b/engine/app/services/coplan/libraries/log_event.rb @@ -0,0 +1,63 @@ +module CoPlan + module Libraries + # Single entry point for recording an organization mutation on a library + # — the library-side sibling of Plans::LogEvent. Every path that files, + # moves, or removes a placement, or mutates a folder, goes through here + # so the audit trail (who, what, where, when, human-or-agent) never + # diverges between the web UI, the API, and bulk organize operations. + # + # `before` / `after` are folder paths (nil for "unfiled"). Pass the plan + # and/or folder involved; titles and names are denormalized into + # metadata so events stay readable after the plan or folder is deleted. + class LogEvent + def self.call(**kwargs) + new(**kwargs).call + end + + def initialize(library:, actor:, event_type:, plan: nil, folder: nil, + before: nil, after: nil, metadata: {}, actor_type: nil, run_id: nil) + @library = library + @actor = actor + @event_type = event_type.to_s + @plan = plan + @folder = folder + @before = before&.to_s + @after = after&.to_s + @metadata = metadata || {} + @actor_type_override = actor_type&.to_s + @run_id = run_id + end + + def call + LibraryEvent.create!( + library: @library, + actor_id: @actor&.id, + actor_type: actor_type, + event_type: @event_type, + plan_id: @plan&.id, + folder_id: @folder&.id, + run_id: @run_id, + before_value: @before, + after_value: @after, + metadata: default_metadata.merge(@metadata) + ) + end + + private + + # Same defaulting as Plans::LogEvent: a user present means human unless + # the caller says otherwise (API bearer tokens pass "local_agent"). + def actor_type + return @actor_type_override if @actor_type_override.present? + @actor.present? ? "human" : "system" + end + + def default_metadata + meta = {} + meta[:plan_title] = @plan.title if @plan + meta[:folder_name] = @folder.name if @folder + meta + end + end + end +end diff --git a/engine/app/services/coplan/libraries/organize.rb b/engine/app/services/coplan/libraries/organize.rb new file mode 100644 index 00000000..91cf8a3c --- /dev/null +++ b/engine/app/services/coplan/libraries/organize.rb @@ -0,0 +1,362 @@ +module CoPlan + module Libraries + # Executes a batch of organization operations against one library — the + # bulk write path behind POST /api/v1/libraries/:id/organize, built for + # agents reorganizing a whole shelf in one round trip. + # + # Operations (each a hash with an "op" key): + # + # { op: "create_folder", path:, description: } + # { op: "rename_folder", folder_path:|folder_id:, name: } + # { op: "describe_folder", folder_path:|folder_id:, description: } + # { op: "move_folder", folder_path:|folder_id:, new_parent_path:|new_parent_id: } (blank → root) + # { op: "delete_folder", folder_path:|folder_id: } (must be empty) + # { op: "move", plan_id:, folder_path:|folder_id:, from_library_id: } (blank dest → unfile) + # { op: "move_many", plan_ids: [...], folder_path:|folder_id:, from_library_id: } + # { op: "move_by_tag", tag:, folder_path:|folder_id:, scope: "library"|"visible" } + # + # Each operation is atomic (a savepoint): a failed op rolls itself back + # and reports an error while the rest of the batch proceeds. With + # `dry_run: true` the whole batch runs inside a transaction that is + # rolled back at the end — the results describe exactly what *would* + # happen, which is what lets an agent propose a reorganization for human + # sign-off before applying it. + # + # Every applied change is audited through Libraries::LogEvent / + # Plans::Place, carrying actor_type so the log distinguishes humans from + # agents. + class Organize + MAX_OPERATIONS = 100 + # Per move_many op — one grouped move should classify a whole batch, + # not smuggle in an unbounded transaction. + MAX_PLANS_PER_MOVE = 500 + + # Raised by op handlers to fail the current op: rolls back that op's + # savepoint and becomes its error result. Never escapes the service. + class OpError < StandardError; end + + Result = Struct.new(:results, :error, :dry_run, :run_id, keyword_init: true) do + def success? = error.nil? + end + + # `actor_label` names the credential behind the actor (e.g. the API + # token's name) so the audit log can distinguish which agent session + # did the work, not just that "a local_agent" did. + def self.call(library:, actor:, operations:, actor_type: nil, actor_label: nil, dry_run: false) + new(library:, actor:, operations:, actor_type:, actor_label:, dry_run:).call + end + + def initialize(library:, actor:, operations:, actor_type: nil, actor_label: nil, dry_run: false) + @library = library + @actor = actor + @operations = operations + @actor_type = actor_type + @actor_label = actor_label + @dry_run = !!dry_run + # One id groups every audit event this call writes — a 2,000-move + # run reads as one entry in the log, filterable via ?run_id=. + @run_id = SecureRandom.uuid + end + + def call + unless @library.writable_by?(@actor) + return Result.new(error: "You can only organize a library you can write to") + end + unless @operations.is_a?(Array) && @operations.any? + return Result.new(error: "operations must be a non-empty array") + end + if @operations.size > MAX_OPERATIONS + return Result.new(error: "Too many operations (max #{MAX_OPERATIONS} per request)") + end + + results = nil + ActiveRecord::Base.transaction do + results = @operations.each_with_index.map { |op, index| perform(normalize(op), index) } + raise ActiveRecord::Rollback if @dry_run + end + Result.new(results: results, dry_run: @dry_run, run_id: @run_id) + end + + private + + def normalize(op) + hash = if op.respond_to?(:to_unsafe_h) + op.to_unsafe_h + elsif op.respond_to?(:to_h) + op.to_h + else + {} # malformed entry → dispatch reports "Unknown op nil" + end + hash.with_indifferent_access + end + + # Each op runs in its own savepoint: raising out of it (OpError, + # RecordInvalid) undoes only that op, and the rescue below turns the + # exception into the op's error result so the batch keeps going. + def perform(op, index) + base = { index: index, op: op[:op] } + outcome = ActiveRecord::Base.transaction(requires_new: true) { dispatch(op) } + base.merge(outcome) + rescue OpError => e + base.merge(status: "error", error: e.message) + rescue ActiveRecord::RecordInvalid => e + base.merge(status: "error", error: e.record.errors.full_messages.join(", ")) + end + + def dispatch(op) + case op[:op] + when "create_folder" then create_folder(op) + when "rename_folder" then rename_folder(op) + when "describe_folder" then describe_folder(op) + when "move_folder" then move_folder(op) + when "delete_folder" then delete_folder(op) + when "move" then move_plan(op) + when "move_many" then move_many(op) + when "move_by_tag" then move_by_tag(op) + else + raise OpError, "Unknown op #{op[:op].inspect}. Valid ops: create_folder, rename_folder, describe_folder, move_folder, delete_folder, move, move_many, move_by_tag" + end + end + + def ok(**details) + { status: "ok" }.merge(details) + end + + # --- folder ops --- + + def create_folder(op) + created = [] + folder = Folder.find_or_create_by_path!( + op[:path], library: @library, created_by_user: @actor, created: created + ) + raise OpError, "path is required" unless folder + + log_created_folders(created) + describe!(folder, op[:description]) if op[:description].present? + ok(folder_id: folder.id, path: folder.path, created_paths: created.map(&:path)) + end + + def rename_folder(op) + folder = resolve_folder!(op) + old_path = folder.path + folder.update!(name: op[:name]) + log!( + event_type: "folder_renamed", folder: folder, + before: old_path, after: folder.path + ) + ok(folder_id: folder.id, path: folder.path) + end + + def describe_folder(op) + folder = resolve_folder!(op) + describe!(folder, op[:description].to_s) + ok(folder_id: folder.id, path: folder.path, description: folder.description) + end + + def move_folder(op) + folder = resolve_folder!(op) + parent = nil + if op[:new_parent_id].present? || op[:new_parent_path].present? + parent = resolve_folder(folder_id: op[:new_parent_id], folder_path: op[:new_parent_path]) + raise OpError, "Unknown destination folder" unless parent + end + old_path = folder.path + folder.update!(parent: parent) + log!( + event_type: "folder_moved", folder: folder, + before: old_path, after: folder.path + ) + ok(folder_id: folder.id, path: folder.path) + end + + def delete_folder(op) + folder = resolve_folder!(op) + path = folder.path + unless folder.destroy + raise OpError, folder.errors.full_messages.join(", ") + end + log!( + event_type: "folder_deleted", before: path, + metadata: { folder_name: folder.name } + ) + ok(path: path) + end + + # --- plan ops --- + + def move_plan(op) + plan = find_visible_plan!(op[:plan_id]) + folder = resolve_destination(op) + place!(plan, folder) + remove_from_source!(plan, op) + ok(plan_id: plan.id, plan_title: plan.title, path: folder&.path) + end + + # The grouped form of `move`: one destination, many plans — so a mass + # classification is ~one op per target folder, not one op per plan. + # Tolerant like move_by_tag: each plan gets its own savepoint, so one + # failure (or a half-completed cross-library transfer) rolls back that + # plan alone and lands in `failed` while the rest proceed. + def move_many(op) + ids = Array(op[:plan_ids]).map(&:to_s).reject(&:blank?).uniq + raise OpError, "plan_ids is required" if ids.empty? + if ids.size > MAX_PLANS_PER_MOVE + raise OpError, "Too many plan_ids (max #{MAX_PLANS_PER_MOVE} per op)" + end + + folder = resolve_destination(op) # nil → unfile + moved = [] + failed = [] + ids.each do |plan_id| + ActiveRecord::Base.transaction(requires_new: true) do + plan = find_visible_plan!(plan_id) + place!(plan, folder) + remove_from_source!(plan, op) + moved << { plan_id: plan.id, plan_title: plan.title } + end + rescue OpError => e + failed << { plan_id: plan_id, error: e.message } + end + ok(path: folder&.path, moved_count: moved.size, moved: moved, failed: failed) + end + + def move_by_tag(op) + raise OpError, "tag is required" if op[:tag].blank? + + folder = resolve_destination(op) + raise OpError, "move_by_tag requires a destination folder" if folder.nil? + + moved = [] + failed = [] + plans_for_tag(op).find_each do |plan| + result = Plans::Place.call( + plan: plan, folder: folder, actor: @actor, library: @library, + actor_type: @actor_type, run_id: @run_id, event_metadata: event_metadata + ) + if result.success? + moved << { plan_id: plan.id, plan_title: plan.title } + else + failed << { plan_id: plan.id, plan_title: plan.title, error: result.error } + end + end + ok(path: folder.path, moved_count: moved.size, moved: moved, failed: failed) + end + + # scope "library" (default): plans already shelved in this library. + # scope "visible": every active plan the actor can list with this tag + # — the "pull everything tagged X onto my shelf" mode. + def plans_for_tag(op) + scoped = Plan.visible_to(@actor).with_tag(op[:tag]) + if op[:scope].to_s == "visible" + scoped.active + else + scoped.joins(:placements).where(coplan_plan_placements: { library_id: @library.id }) + end + end + + # --- helpers --- + + def find_visible_plan!(plan_id) + plan = Plan.find_by(id: plan_id) + unless plan && PlanPolicy.new(@actor, plan).show? + raise OpError, "Plan not found" + end + plan + end + + # Cross-library move: after shelving here, remove the placement from + # the source library. Both sides are gated by Library#writable_by?, + # and a failure raises out of the surrounding savepoint — never half + # a move. + def remove_from_source!(plan, op) + return if op[:from_library_id].blank? || op[:from_library_id] == @library.id + + source = Library.find_by(id: op[:from_library_id]) + raise OpError, "Source library not found" unless source + + removal = Plans::Place.call( + plan: plan, folder: nil, actor: @actor, library: source, + actor_type: @actor_type, run_id: @run_id, event_metadata: event_metadata + ) + unless removal.success? + raise OpError, "Could not remove from source library: #{removal.error}" + end + end + + def place!(plan, folder) + result = Plans::Place.call( + plan: plan, folder: folder, actor: @actor, library: @library, + actor_type: @actor_type, run_id: @run_id, event_metadata: event_metadata + ) + raise OpError, result.error unless result.success? + result + end + + # Every audit event this run writes carries the run id and, when the + # caller named its credential, an actor_label (e.g. the API token + # name) — so the log answers "which agent session?" not just "an + # agent". + def log!(**kwargs) + metadata = kwargs.delete(:metadata) || {} + Libraries::LogEvent.call( + library: @library, actor: @actor, actor_type: @actor_type, + run_id: @run_id, metadata: event_metadata.merge(metadata), **kwargs + ) + end + + def event_metadata + @actor_label.present? ? { actor_label: @actor_label } : {} + end + + def resolve_folder(op) + if op[:folder_id].present? + @library.folders.find_by(id: op[:folder_id]) + elsif op[:folder_path].present? + Folder.find_by_path(op[:folder_path], library: @library) + end + end + + def resolve_folder!(op) + resolve_folder(op) || raise(OpError, folder_missing_message(op)) + end + + # Destination for plan moves: folder_path find-or-creates (audited), + # folder_id must exist, blank means unfile (returns nil). + def resolve_destination(op) + if op[:folder_path].present? + created = [] + folder = Folder.find_or_create_by_path!( + op[:folder_path], library: @library, created_by_user: @actor, created: created + ) + log_created_folders(created) + folder + elsif op[:folder_id].present? + @library.folders.find_by(id: op[:folder_id]) || raise(OpError, "Unknown folder_id #{op[:folder_id].inspect}") + end + end + + def describe!(folder, description) + old = folder.description + return if old.to_s == description.to_s + + folder.update!(description: description.presence) + log!( + event_type: "folder_described", folder: folder, + before: old, after: folder.description, + metadata: { path: folder.path } + ) + end + + def log_created_folders(folders) + folders.each do |folder| + log!(event_type: "folder_created", folder: folder, after: folder.path) + end + end + + def folder_missing_message(op) + ref = op[:folder_path].presence || op[:folder_id].presence + ref ? "Folder #{ref.inspect} not found" : "folder_path or folder_id is required" + end + end + end +end diff --git a/engine/app/services/coplan/plans/place.rb b/engine/app/services/coplan/plans/place.rb index b30dacc1..285a68a4 100644 --- a/engine/app/services/coplan/plans/place.rb +++ b/engine/app/services/coplan/plans/place.rb @@ -13,15 +13,25 @@ class Place def success? = error.nil? end - def self.call(plan:, folder:, actor:, library: nil) - new(plan:, folder:, actor:, library:).call + # `actor_type` distinguishes humans from agents in the audit trail — + # API callers authenticating via bearer token pass "local_agent"; + # the web UI omits it (defaults to "human" via the log services). + # `run_id` / `event_metadata` flow into the library-side audit event + # so bulk organize runs stay grouped and attributable (token label). + def self.call(plan:, folder:, actor:, library: nil, actor_type: nil, + run_id: nil, event_metadata: {}) + new(plan:, folder:, actor:, library:, actor_type:, run_id:, event_metadata:).call end - def initialize(plan:, folder:, actor:, library: nil) + def initialize(plan:, folder:, actor:, library: nil, actor_type: nil, + run_id: nil, event_metadata: {}) @plan = plan @folder = folder @actor = actor @library = library || folder&.library || actor.library + @actor_type = actor_type + @run_id = run_id + @event_metadata = event_metadata || {} end def call @@ -75,21 +85,44 @@ def call private - # The audit trail lives on the plan, but only for the author's own - # library — someone else curating their shelf isn't an event in the - # plan's history. + # Two audit trails, one write path. The plan-side event only fires for + # the author's own library — someone else curating their shelf isn't + # an event in the plan's history. The library-side event always fires: + # every rearrangement of a shelf is part of that library's audit log. def log_move(old_path, new_path) - return unless @plan.created_by_user_id == @actor.id return if old_path == new_path + Libraries::LogEvent.call( + library: @library, + actor: @actor, + actor_type: @actor_type, + event_type: library_event_type(old_path, new_path), + plan: @plan, + folder: @folder, + before: old_path, + after: new_path, + run_id: @run_id, + metadata: @event_metadata + ) + + return unless @plan.created_by_user_id == @actor.id + LogEvent.call( plan: @plan, actor: @actor, + actor_type: @actor_type, event_type: "moved_to_folder", before: old_path, after: new_path ) end + + def library_event_type(old_path, new_path) + if old_path.nil? then "plan_filed" + elsif new_path.nil? then "plan_removed" + else "plan_moved" + end + end end end end diff --git a/engine/app/views/coplan/agent_instructions/organizing.text.erb b/engine/app/views/coplan/agent_instructions/organizing.text.erb new file mode 100644 index 00000000..c6ea9da7 --- /dev/null +++ b/engine/app/views/coplan/agent_instructions/organizing.text.erb @@ -0,0 +1,118 @@ +# CoPlan Library Organization Guide + +Sub-instructions for organizing libraries. Read the main instructions at `<%= @base %>/agent-instructions` first — this guide assumes you already know how to authenticate and shelve a single plan. + +<%= @auth_instructions %> + +## The model in three sentences + +A **library** is a folder tree owned by a user (team libraries use the same model). Filing a plan is a **placement** — the library owner's organization of the plan, never a property of the plan itself, so the same plan can sit in many libraries at once and moving it in one library changes nothing anywhere else. Folders go at most 3 levels deep, carry a one-line `description` of what belongs in them, and every rearrangement is recorded in the library's audit log with whether a human or an agent did it. + +Use **folders** for "where does this live?" and **tags** for "what is it about?" — move-by-tag (below) is the bridge between the two. + +## Step 1 — Learn the layout (one call) + +```bash +<%= @curl %> \ + "<%= @base %>/api/v1/library" | jq . +``` + +Your own library's map: every folder with `id`, `name`, **`description`** (its meaning — trust it over guessing from the name), `path` (`"Team EBT/Q3"`), `plans_count` (directly inside), `total_plans_count` (including subfolders), plus `unfiled_count` (your plans not yet shelved), `top_tags`, and `recent_activity`. + +Any library by id: `GET <%= @base %>/api/v1/libraries/$LIBRARY_ID` (read-only unless it's yours; `GET /api/v1/libraries` lists all libraries with a `writable` flag). + +## Step 2 — Bulk-read the contents + +```bash +<%= @curl %> \ + "<%= @base %>/api/v1/library/contents" | jq . +``` + +One compact row per shelved plan: `plan_id`, `title`, **`summary`** (auto-generated abstract — enough to classify a plan without fetching its content), `tags`, `visibility`, `archived`, `author`, `folder_path`, `placed_by`, and dates. Sorted by folder path, so the whole shelf reads top-to-bottom. + +Filters (combine freely): + +- `?folder_path=Team%20EBT&recursive=true` — one subtree +- `?tag=pricing` — everything shelved here with a tag +- `?unfiled=true` — your plans not yet shelved (file these!) +- `?archived=true` — archived plans (excluded by default) +- `?limit=&offset=` — pagination (default 500 rows) + +Do **not** fetch each plan's full content to classify it — `summary` + `tags` + `title` are there precisely so organizing costs one request, not N. + +## Step 3 — Organize in bulk + +`POST /api/v1/library/organize` (or `/api/v1/libraries/$LIBRARY_ID/organize`) takes up to 100 operations. Each op is atomic — a bad one reports its error and the rest still apply. + +```bash +<%= @curl %> -X POST \ + -H "Content-Type: application/json" \ + -d '{ + "dry_run": true, + "operations": [ + {"op": "create_folder", "path": "Archive/2025", "description": "Shipped or superseded work from 2025"}, + {"op": "describe_folder", "folder_path": "Infra", "description": "Platform and infrastructure plans"}, + {"op": "rename_folder", "folder_path": "Misc", "name": "Inbox"}, + {"op": "move_folder", "folder_path": "Q3", "new_parent_path": "Team EBT"}, + {"op": "move", "plan_id": "'$PLAN_ID'", "folder_path": "Team EBT/Q3"}, + {"op": "move_by_tag", "tag": "pricing", "folder_path": "Pricing"}, + {"op": "delete_folder", "folder_path": "Old Empty Folder"} + ] + }' \ + "<%= @base %>/api/v1/library/organize" | jq . +``` + +The operation vocabulary: + +| op | fields | notes | +|---|---|---| +| `create_folder` | `path`, `description` | creates the whole hierarchy; sets description on the leaf | +| `describe_folder` | `folder_path`\|`folder_id`, `description` | give folders meaning — future agents read these | +| `rename_folder` | `folder_path`\|`folder_id`, `name` | | +| `move_folder` | `folder_path`\|`folder_id`, `new_parent_path`\|`new_parent_id` | blank parent → top level | +| `delete_folder` | `folder_path`\|`folder_id` | only empty folders | +| `move` | `plan_id`, `folder_path`\|`folder_id`, `from_library_id` | blank destination → unfile; `folder_path` creates as needed | +| `move_many` | `plan_ids` (≤500), `folder_path`\|`folder_id`, `from_library_id` | the grouped form of `move` — **prefer this for mass classification**: one op per destination folder, not one per plan. Per-plan tolerant: failures land in `failed`, the rest move | +| `move_by_tag` | `tag`, `folder_path`\|`folder_id`, `scope` | `scope: "library"` (default) moves plans already on this shelf; `scope: "visible"` pulls every visible plan with the tag onto it | + +The response echoes per-op `results` (`status: "ok"` with details, or `status: "error"` with the reason), `ok_count` / `error_count`, and a **`run_id`** grouping every audit event the call wrote — quote it to the user and filter the log with `?run_id=` to review exactly what one run did. + +**Cross-library moves**: `{"op": "move", "plan_id": ..., "folder_path": "Somewhere", "from_library_id": "$OTHER_LIBRARY_ID"}` shelves the plan in the target library and removes it from the source in one atomic op. You need write access to both sides (today: they're yours; team libraries will use membership). + +### Propose first: dry_run + +`"dry_run": true` executes the whole batch, reports exactly what would happen — including which ops would fail and why — then rolls everything back. **This is the intended workflow for "reorganize my whole library":** + +1. Read the overview + contents. +2. Design a structure; build the operations array. +3. POST it with `dry_run: true`; show the user the proposed outline and per-op results. +4. On approval, POST the same body with `dry_run: false`. + +## Where is this plan? (reverse lookup) + +```bash +<%= @curl %> \ + "<%= @base %>/api/v1/plans/$PLAN_ID/locations" | jq . +``` + +Every library the plan is shelved in: `library_name`, `owner`, `folder_path`, `folder_description`, `placed_by`, `writable` (whether *you* can move it there). + +## The audit log + +```bash +<%= @curl %> \ + "<%= @base %>/api/v1/library/events" | jq . +``` + +Who moved what where, when — and whether an agent did it: each event has `event_type` (`plan_filed`, `plan_moved`, `plan_removed`, `folder_created`, `folder_renamed`, `folder_moved`, `folder_described`, `folder_deleted`), `actor` (name), `actor_type` / `agent` (`"local_agent"` means an agent acting on the owner's behalf — that's you), `actor_label` (the API token's name — which agent session), `run_id` (which bulk organize call), `plan_title`, and `before` → `after` paths. Filters: `?event_type=`, `?plan_id=`, `?run_id=`, `?before=` (pagination), `?limit=` (default 50). Owner-only. + +Use it to answer "who moved my plan?", to review what a previous agent session did, or to summarize recent reorganization for the user. + +## Organizing principles + +- **Reuse before invent.** Read the overview first; extend the existing structure rather than imposing a new one. Respect folder descriptions — they are the owner's stated intent. +- **Describe what you create.** Every `create_folder` should carry a `description`. You are writing signage for the next agent (possibly yourself, without this context). +- **Propose, then apply.** For anything beyond a few moves, dry-run and show the user the outline before applying. Never mass-reorganize someone's library unprompted. +- **Shallow beats deep.** 5–9 top-level folders beat one 3-level rabbit hole. The depth cap is 3 for a reason. +- **Unfiled is a to-do list.** `unfiled_count` > 0 means plans nobody shelved; filing them is the highest-value low-risk organizing move. +- **Tags stay orthogonal.** Don't mirror the folder tree in tags or vice versa. If a folder and a tag mean the same thing, `move_by_tag` once and drop the redundancy. diff --git a/engine/app/views/coplan/agent_instructions/show.text.erb b/engine/app/views/coplan/agent_instructions/show.text.erb index 39b3a70b..ba3b977b 100644 --- a/engine/app/views/coplan/agent_instructions/show.text.erb +++ b/engine/app/views/coplan/agent_instructions/show.text.erb @@ -169,9 +169,16 @@ Each folder includes `id`, `name`, `library_id`, `parent_id`, `path` (e.g. `"Tea - `folder_id`/`folder_path` in plan responses are **yours**: where *you* shelved that plan, `null` if you haven't. **Guidelines:** -- Check `GET /api/v1/folders` before creating new folders — reuse the existing structure. -- Folder names read like places (`Team EBT`, `Infra`, `Q3 Launch`), not labels. -- You can offer to organize a user's library: list their plans, propose a folder structure, and shelve plans with `folder_path` once they agree. +- Check `GET <%= @base %>/api/v1/library` (your library's map — folder tree with descriptions and counts) before creating new folders — reuse the existing structure. +- Folder names read like places (`Team EBT`, `Infra`, `Q3 Launch`), not labels. Folders carry a one-line `description` saying what belongs in them — read it before filing, write it when you create folders. + +**Organizing at scale — fetch only when needed.** When you're asked to organize, reorganize, or audit a library (yours or between libraries), there is a dedicated organization API: a one-call library overview, bulk contents with summaries and tags, batch move/rename/describe operations with `dry_run` for proposing changes, move-by-tag, cross-library moves, and a full audit log. Its guide lives at: + +``` +GET <%= @base %>/agent-instructions/organizing +``` + +Don't fetch it for a single filing (`folder_path` above covers that) — fetch it once when a real organizing task starts. ### Visibility & Archiving diff --git a/engine/config/routes.rb b/engine/config/routes.rb index eacff866..6f68865d 100644 --- a/engine/config/routes.rb +++ b/engine/config/routes.rb @@ -51,8 +51,25 @@ namespace :v1 do resources :tags, only: [:index] resources :folders, only: [:index, :create, :update, :destroy] + + # The agent organization API: overview (show), bulk read (contents), + # bulk write (organize), audit log (events). The bare /library routes + # are the caller's own library, no id needed. + resources :libraries, only: [:index, :show] do + member do + get :contents + get :events + post :organize + end + end + get "library", to: "libraries#show", as: :own_library + get "library/contents", to: "libraries#contents", as: :own_library_contents + get "library/events", to: "libraries#events", as: :own_library_events + post "library/organize", to: "libraries#organize", as: :own_library_organize + resources :plans, only: [:index, :show, :create, :update] do get :versions, on: :member + get :locations, on: :member get :comments, on: :member get :snapshot, on: :member resource :content, only: [:update], controller: "content" @@ -93,6 +110,9 @@ get "llms.txt", to: "llms#show", as: :llms_txt get "agent-instructions", to: "agent_instructions#show", as: :agent_instructions + # Sub-instructions: the library-organizing guide, fetched on demand so the + # main instructions stay small (agents only spend context when organizing). + get "agent-instructions/organizing", to: "agent_instructions#organizing", as: :agent_instructions_organizing # Service worker — served from a route (not the asset pipeline) so it has a # stable URL the browser can update in place. Scope is whatever the engine diff --git a/engine/db/migrate/20260813000000_add_description_to_coplan_folders.rb b/engine/db/migrate/20260813000000_add_description_to_coplan_folders.rb new file mode 100644 index 00000000..a9080135 --- /dev/null +++ b/engine/db/migrate/20260813000000_add_description_to_coplan_folders.rb @@ -0,0 +1,8 @@ +class AddDescriptionToCoplanFolders < ActiveRecord::Migration[8.1] + def change + # A short human/agent-readable statement of what belongs in the folder + # (e.g. "Active Q3 work — move to Done when shipped"). Surfaced in the + # library overview API so agents can organize by meaning, not just name. + add_column :coplan_folders, :description, :string, limit: 255 + end +end diff --git a/engine/db/migrate/20260813000001_create_coplan_library_events.rb b/engine/db/migrate/20260813000001_create_coplan_library_events.rb new file mode 100644 index 00000000..76fb4405 --- /dev/null +++ b/engine/db/migrate/20260813000001_create_coplan_library_events.rb @@ -0,0 +1,35 @@ +class CreateCoplanLibraryEvents < ActiveRecord::Migration[8.1] + def change + # Append-only audit log for a library's organization: who filed/moved/ + # removed which plan, and who created/renamed/moved/deleted folders — + # with actor_type distinguishing humans from agents. Mirrors + # coplan_plan_events, but scoped to the library (the shelf), not the + # plan (the document). + # + # plan_id / folder_id are deliberately not foreign keys: audit rows must + # survive the deletion of what they describe. Paths and titles are + # denormalized into before/after/metadata so the log stays readable. + create_table :coplan_library_events, id: { type: :string, limit: 36 } do |t| + t.string :library_id, limit: 36, null: false + t.string :actor_id, limit: 36 + t.string :actor_type, null: false + t.string :event_type, null: false + t.string :plan_id, limit: 36 + t.string :folder_id, limit: 36 + # Groups every event applied by one bulk organize call, so a + # 2,000-move run reads as one entry point in the log, not noise. + t.string :run_id, limit: 36 + t.text :before_value + t.text :after_value + t.json :metadata + t.datetime :created_at, null: false + + t.index [ :library_id, :created_at ] + t.index :plan_id + t.index :event_type + t.index :run_id + end + + add_foreign_key :coplan_library_events, :coplan_libraries, column: :library_id + end +end diff --git a/spec/requests/api/v1/libraries_spec.rb b/spec/requests/api/v1/libraries_spec.rb new file mode 100644 index 00000000..06782636 --- /dev/null +++ b/spec/requests/api/v1/libraries_spec.rb @@ -0,0 +1,374 @@ +require "rails_helper" + +RSpec.describe "Api::V1::Libraries", type: :request do + let(:alice) { create(:coplan_user) } + let(:bob) { create(:coplan_user) } + let(:alice_token) { create(:api_token, user: alice, raw_token: "test-token-alice") } + let(:bob_token) { create(:api_token, user: bob, raw_token: "test-token-bob") } + let(:headers) { { "Authorization" => "Bearer test-token-alice" } } + let(:json_headers) { headers.merge("Content-Type" => "application/json") } + let(:bob_headers) { { "Authorization" => "Bearer test-token-bob" } } + + before do + alice_token + bob_token + end + + describe "GET /api/v1/libraries" do + it "lists libraries with ownership and writability" do + alice.library + bob.library + + get api_v1_libraries_path, headers: headers + expect(response).to have_http_status(:success) + libraries = JSON.parse(response.body) + mine = libraries.find { |l| l["id"] == alice.library.id } + other = libraries.find { |l| l["id"] == bob.library.id } + expect(mine["writable"]).to be(true) + expect(mine["owner"]["name"]).to eq(alice.name) + expect(other["writable"]).to be(false) + end + end + + describe "GET /api/v1/library (overview)" do + it "returns the folder tree with descriptions, paths, and subtree counts" do + root = create(:folder, name: "Team EBT", description: "EBT rollout work", created_by_user: alice) + sub = create(:folder, name: "Q3", parent: root, created_by_user: alice) + plan = create(:plan, :published, created_by_user: alice, tags: [ create(:tag, name: "pricing") ]) + CoPlan::Plans::Place.call(plan: plan, folder: sub, actor: alice) + + get "/api/v1/library", headers: headers + expect(response).to have_http_status(:success) + json = JSON.parse(response.body) + + expect(json["id"]).to eq(alice.library.id) + expect(json["writable"]).to be(true) + + root_json = json["folders"].find { |f| f["id"] == root.id } + sub_json = json["folders"].find { |f| f["id"] == sub.id } + expect(root_json["description"]).to eq("EBT rollout work") + expect(root_json["plans_count"]).to eq(0) + expect(root_json["total_plans_count"]).to eq(1) + expect(sub_json["path"]).to eq("Team EBT/Q3") + expect(sub_json["plans_count"]).to eq(1) + + expect(json["top_tags"]).to include({ "name" => "pricing", "plans_count" => 1 }) + expect(json["organize_instructions_url"]).to include("agent-instructions/organizing") + expect(json).to have_key("unfiled_count") + expect(json).to have_key("recent_activity") + end + + it "counts unfiled plans (own active plans without a placement)" do + create(:plan, :published, created_by_user: alice) + + get "/api/v1/library", headers: headers + expect(JSON.parse(response.body)["unfiled_count"]).to eq(1) + end + + it "hides owner-only fields when browsing someone else's library" do + get api_v1_library_path(bob.library), headers: headers + json = JSON.parse(response.body) + expect(json["writable"]).to be(false) + expect(json).not_to have_key("unfiled_count") + expect(json).not_to have_key("recent_activity") + end + + it "404s for unknown libraries" do + get api_v1_library_path("nope"), headers: headers + expect(response).to have_http_status(:not_found) + end + end + + describe "GET /api/v1/library/contents" do + it "returns one row per placement with summary, tags, and location" do + folder = create(:folder, name: "Infra", created_by_user: alice) + plan = create(:plan, :published, created_by_user: bob, title: "Zonal failover") + plan.update_columns(summary: "A plan about failover.") + plan.tag_names = [ "infra" ] + plan.save! + CoPlan::Plans::Place.call(plan: plan, folder: folder, actor: alice) + + get "/api/v1/library/contents", headers: headers + expect(response).to have_http_status(:success) + json = JSON.parse(response.body) + expect(json["count"]).to eq(1) + row = json["items"].first + expect(row["title"]).to eq("Zonal failover") + expect(row["summary"]).to eq("A plan about failover.") + expect(row["tags"]).to eq([ "infra" ]) + expect(row["folder_path"]).to eq("Infra") + expect(row["author"]).to eq(bob.name) + end + + it "filters by folder subtree with recursive=true" do + root = create(:folder, name: "A", created_by_user: alice) + sub = create(:folder, name: "B", parent: root, created_by_user: alice) + other = create(:folder, name: "C", created_by_user: alice) + in_sub = create(:plan, :published, created_by_user: alice) + in_other = create(:plan, :published, created_by_user: alice) + CoPlan::Plans::Place.call(plan: in_sub, folder: sub, actor: alice) + CoPlan::Plans::Place.call(plan: in_other, folder: other, actor: alice) + + get "/api/v1/library/contents", params: { folder_path: "A", recursive: "true" }, headers: headers + ids = JSON.parse(response.body)["items"].map { |i| i["plan_id"] } + expect(ids).to eq([ in_sub.id ]) + end + + it "filters by tag" do + folder = create(:folder, created_by_user: alice) + tagged = create(:plan, :published, created_by_user: alice) + tagged.tag_names = [ "pricing" ] + tagged.save! + untagged = create(:plan, :published, created_by_user: alice) + CoPlan::Plans::Place.call(plan: tagged, folder: folder, actor: alice) + CoPlan::Plans::Place.call(plan: untagged, folder: folder, actor: alice) + + get "/api/v1/library/contents", params: { tag: "pricing" }, headers: headers + ids = JSON.parse(response.body)["items"].map { |i| i["plan_id"] } + expect(ids).to eq([ tagged.id ]) + end + + it "lists unfiled plans with unfiled=true, own library only" do + create(:plan, :published, created_by_user: alice, title: "Loose plan") + + get "/api/v1/library/contents", params: { unfiled: "true" }, headers: headers + json = JSON.parse(response.body) + expect(json["items"].map { |i| i["title"] }).to eq([ "Loose plan" ]) + expect(json["items"].first["folder_path"]).to be_nil + + get contents_api_v1_library_path(bob.library), params: { unfiled: "true" }, headers: headers + expect(response).to have_http_status(:forbidden) + end + + it "hides other users' drafts" do + folder = create(:folder, created_by_user: alice) + draft = create(:plan, :draft, created_by_user: alice) + CoPlan::Plans::Place.call(plan: draft, folder: folder, actor: alice) + + get contents_api_v1_library_path(alice.library), headers: bob_headers + expect(JSON.parse(response.body)["count"]).to eq(0) + end + end + + describe "POST /api/v1/library/organize" do + it "applies a batch of folder and move operations, auditing each" do + plan = create(:plan, :published, created_by_user: alice, title: "Pricing plan") + + post "/api/v1/library/organize", + params: { + operations: [ + { op: "create_folder", path: "Archive/2025", description: "Old work" }, + { op: "move", plan_id: plan.id, folder_path: "Archive/2025" } + ] + }.to_json, + headers: json_headers + + expect(response).to have_http_status(:success) + json = JSON.parse(response.body) + expect(json["applied"]).to be(true) + expect(json["ok_count"]).to eq(2) + expect(json["error_count"]).to eq(0) + + placement = alice.library.placements.find_by(plan_id: plan.id) + expect(placement.folder.path).to eq("Archive/2025") + expect(placement.folder.parent.name).to eq("Archive") + expect(CoPlan::Folder.find_by(name: "2025").description).to eq("Old work") + + events = alice.library.library_events.order(:created_at) + expect(events.map(&:event_type)).to include("folder_created", "folder_described", "plan_filed") + # Token auth means the agent gets attributed, not a human. + expect(events.map(&:actor_type).uniq).to eq([ "local_agent" ]) + filed = events.find_by(event_type: "plan_filed") + expect(filed.after_value).to eq("Archive/2025") + expect(filed.metadata["plan_title"]).to eq("Pricing plan") + end + + it "reports per-op errors without aborting the batch" do + post "/api/v1/library/organize", + params: { + operations: [ + { op: "delete_folder", folder_path: "Does Not Exist" }, + { op: "create_folder", path: "Real" } + ] + }.to_json, + headers: json_headers + + json = JSON.parse(response.body) + expect(json["error_count"]).to eq(1) + expect(json["ok_count"]).to eq(1) + expect(json["results"][0]["status"]).to eq("error") + expect(alice.library.folders.exists?(name: "Real")).to be(true) + end + + it "rolls everything back with dry_run while reporting what would happen" do + plan = create(:plan, :published, created_by_user: alice) + + post "/api/v1/library/organize", + params: { + dry_run: true, + operations: [ { op: "move", plan_id: plan.id, folder_path: "Proposed/Structure" } ] + }.to_json, + headers: json_headers + + json = JSON.parse(response.body) + expect(json["dry_run"]).to be(true) + expect(json["ok_count"]).to eq(1) + expect(json["results"][0]["path"]).to eq("Proposed/Structure") + + expect(alice.library.folders.count).to eq(0) + expect(alice.library.placements.count).to eq(0) + expect(alice.library.library_events.count).to eq(0) + end + + it "moves every tagged plan on the shelf with move_by_tag" do + inbox = create(:folder, name: "Inbox", created_by_user: alice) + tagged1 = create(:plan, :published, created_by_user: alice) + tagged2 = create(:plan, :published, created_by_user: alice) + untagged = create(:plan, :published, created_by_user: alice) + [ tagged1, tagged2 ].each do |plan| + plan.tag_names = [ "pricing" ] + plan.save! + end + [ tagged1, tagged2, untagged ].each do |plan| + CoPlan::Plans::Place.call(plan: plan, folder: inbox, actor: alice) + end + + post "/api/v1/library/organize", + params: { operations: [ { op: "move_by_tag", tag: "pricing", folder_path: "Pricing" } ] }.to_json, + headers: json_headers + + json = JSON.parse(response.body) + expect(json["results"][0]["moved_count"]).to eq(2) + pricing = CoPlan::Folder.find_by(name: "Pricing", library: alice.library) + expect(pricing.placements.map(&:plan_id)).to match_array([ tagged1.id, tagged2.id ]) + expect(inbox.placements.map(&:plan_id)).to eq([ untagged.id ]) + end + + it "moves a batch of plans in one grouped op with move_many" do + plans = create_list(:plan, 3, :published, created_by_user: alice) + + post "/api/v1/library/organize", + params: { + operations: [ + { op: "move_many", plan_ids: plans.map(&:id) + [ "missing-id" ], folder_path: "Sorted" } + ] + }.to_json, + headers: json_headers + + json = JSON.parse(response.body) + result = json["results"][0] + expect(result["status"]).to eq("ok") + expect(result["moved_count"]).to eq(3) + expect(result["moved"].map { |m| m["plan_id"] }).to match_array(plans.map(&:id)) + expect(result["failed"]).to eq([ { "plan_id" => "missing-id", "error" => "Plan not found" } ]) + sorted = CoPlan::Folder.find_by(name: "Sorted", library: alice.library) + expect(sorted.placements.count).to eq(3) + end + + it "rejects a move_many exceeding the per-op plan cap" do + too_many = (CoPlan::Libraries::Organize::MAX_PLANS_PER_MOVE + 1).times.map { |i| "id-#{i}" } + + post "/api/v1/library/organize", + params: { operations: [ { op: "move_many", plan_ids: too_many, folder_path: "X" } ] }.to_json, + headers: json_headers + + json = JSON.parse(response.body) + expect(json["results"][0]["status"]).to eq("error") + expect(json["results"][0]["error"]).to match(/max #{CoPlan::Libraries::Organize::MAX_PLANS_PER_MOVE}/) + expect(alice.library.folders.find_by(name: "X")).to be_nil + end + + it "groups every audit event under the returned run_id with the token's label" do + plan = create(:plan, :published, created_by_user: alice) + + post "/api/v1/library/organize", + params: { + operations: [ + { op: "create_folder", path: "Grouped" }, + { op: "move", plan_id: plan.id, folder_path: "Grouped" } + ] + }.to_json, + headers: json_headers + + run_id = JSON.parse(response.body)["run_id"] + expect(run_id).to be_present + + events = alice.library.library_events + expect(events.pluck(:run_id).uniq).to eq([ run_id ]) + expect(events.map { |e| e.metadata["actor_label"] }.uniq).to eq([ alice_token.name ]) + + get "/api/v1/library/events", params: { run_id: run_id }, headers: headers + listed = JSON.parse(response.body) + expect(listed.size).to eq(events.count) + expect(listed.map { |e| e["run_id"] }.uniq).to eq([ run_id ]) + expect(listed.first["actor_label"]).to eq(alice_token.name) + end + + it "moves a plan between libraries with from_library_id" do + # Simulate a future team library by making bob's library writable to + # alice — writable_by? is the only gate Organize consults. + allow_any_instance_of(CoPlan::Library).to receive(:writable_by?).and_return(true) + plan = create(:plan, :published, created_by_user: alice) + source_folder = create(:folder, name: "Mine", created_by_user: alice) + CoPlan::Plans::Place.call(plan: plan, folder: source_folder, actor: alice) + + post organize_api_v1_library_path(bob.library), + params: { + operations: [ + { op: "move", plan_id: plan.id, folder_path: "Shared", from_library_id: alice.library.id } + ] + }.to_json, + headers: json_headers + + json = JSON.parse(response.body) + expect(json["ok_count"]).to eq(1) + expect(alice.library.placements.count).to eq(0) + expect(bob.library.placements.first.folder.name).to eq("Shared") + end + + it "refuses to organize a library you cannot write" do + post organize_api_v1_library_path(bob.library), + params: { operations: [ { op: "create_folder", path: "Nope" } ] }.to_json, + headers: json_headers + expect(response).to have_http_status(:unprocessable_content) + end + end + + describe "GET /api/v1/library/events" do + it "returns the audit log, newest first, flagging agent actors" do + folder = create(:folder, name: "Infra", created_by_user: alice) + plan = create(:plan, :published, created_by_user: alice, title: "Audited plan") + CoPlan::Plans::Place.call(plan: plan, folder: folder, actor: alice, actor_type: "local_agent") + CoPlan::Plans::Place.call(plan: plan, folder: nil, actor: alice) + + get "/api/v1/library/events", headers: headers + expect(response).to have_http_status(:success) + events = JSON.parse(response.body) + expect(events.map { |e| e["event_type"] }).to eq([ "plan_removed", "plan_filed" ]) + + filed = events.last + expect(filed["agent"]).to be(true) + expect(filed["actor_type"]).to eq("local_agent") + expect(filed["actor"]["name"]).to eq(alice.name) + expect(filed["plan_title"]).to eq("Audited plan") + expect(filed["after"]).to eq("Infra") + + removed = events.first + expect(removed["agent"]).to be(false) + expect(removed["before"]).to eq("Infra") + expect(removed["after"]).to be_nil + end + + it "filters by event_type and is owner-only" do + folder = create(:folder, created_by_user: alice) + plan = create(:plan, :published, created_by_user: alice) + CoPlan::Plans::Place.call(plan: plan, folder: folder, actor: alice) + + get "/api/v1/library/events", params: { event_type: "plan_filed" }, headers: headers + expect(JSON.parse(response.body).map { |e| e["event_type"] }.uniq).to eq([ "plan_filed" ]) + + get events_api_v1_library_path(alice.library), headers: bob_headers + expect(response).to have_http_status(:forbidden) + end + end +end diff --git a/spec/requests/api/v1/plans_spec.rb b/spec/requests/api/v1/plans_spec.rb index dc4d022c..17104c96 100644 --- a/spec/requests/api/v1/plans_spec.rb +++ b/spec/requests/api/v1/plans_spec.rb @@ -339,4 +339,34 @@ def alice_placement expect(response).to have_http_status(:not_found) end end + + describe "GET /api/v1/plans/:id/locations" do + it "returns every library the plan is shelved in" do + published = create(:plan, :considering, created_by_user: alice) + alice_folder = create(:folder, name: "Mine", description: "Alice's shelf", created_by_user: alice) + carol_folder = create(:folder, name: "Reading List", created_by_user: carol) + CoPlan::Plans::Place.call(plan: published, folder: alice_folder, actor: alice) + CoPlan::Plans::Place.call(plan: published, folder: carol_folder, actor: carol) + + get locations_api_v1_plan_path(published), headers: headers + expect(response).to have_http_status(:success) + locations = JSON.parse(response.body) + expect(locations.size).to eq(2) + + mine = locations.find { |l| l["library_id"] == alice.library.id } + theirs = locations.find { |l| l["library_id"] == carol.library.id } + expect(mine["folder_path"]).to eq("Mine") + expect(mine["folder_description"]).to eq("Alice's shelf") + expect(mine["writable"]).to be(true) + expect(mine["owner"]["name"]).to eq(alice.name) + expect(theirs["folder_path"]).to eq("Reading List") + expect(theirs["writable"]).to be(false) + expect(theirs["placed_by"]).to eq(carol.name) + end + + it "requires auth" do + get locations_api_v1_plan_path(plan) + expect(response).to have_http_status(:unauthorized) + end + end end diff --git a/spec/services/plans/place_spec.rb b/spec/services/plans/place_spec.rb index cc5e6342..9789bc0e 100644 --- a/spec/services/plans/place_spec.rb +++ b/spec/services/plans/place_spec.rb @@ -76,4 +76,53 @@ def place(plan:, folder:, actor:, library: nil) expect(author.library.placements.where(plan: plan).count).to eq(1) expect(result.placement.folder).to eq(second_folder) end + + describe "library audit trail" do + it "logs filed → moved → removed with paths and plan title" do + plan = create(:plan, :considering, created_by_user: author, title: "Audited") + second_folder = create(:folder, name: "Elsewhere", created_by_user: author) + + place(plan: plan, folder: folder, actor: author) + place(plan: plan, folder: second_folder, actor: author) + place(plan: plan, folder: nil, actor: author) + + events = author.library.library_events.order(:created_at) + expect(events.map(&:event_type)).to eq(%w[plan_filed plan_moved plan_removed]) + + moved = events.second + expect(moved.before_value).to eq(folder.path) + expect(moved.after_value).to eq("Elsewhere") + expect(moved.plan_id).to eq(plan.id) + expect(moved.metadata["plan_title"]).to eq("Audited") + expect(moved.actor_id).to eq(author.id) + expect(moved.actor_type).to eq("human") + end + + it "attributes agent moves as local_agent" do + plan = create(:plan, :considering, created_by_user: author) + described_class.call(plan: plan, folder: folder, actor: author, + library: author.library, actor_type: "local_agent") + + event = author.library.library_events.sole + expect(event.actor_type).to eq("local_agent") + expect(author.library.placements.sole.plan_id).to eq(plan.id) + # The plan-side history event carries the same attribution. + expect(plan.plan_events.sole.actor_type).to eq("local_agent") + end + + it "logs shelving someone else's plan in the library trail but not the plan's history" do + plan = create(:plan, :published, created_by_user: other) + place(plan: plan, folder: folder, actor: author) + + expect(author.library.library_events.sole.event_type).to eq("plan_filed") + expect(plan.plan_events).to be_empty + end + + it "logs nothing when a re-file is a no-op" do + plan = create(:plan, :considering, created_by_user: author) + place(plan: plan, folder: folder, actor: author) + expect { place(plan: plan, folder: folder, actor: author) } + .not_to change { author.library.library_events.count } + end + end end From 9bde9be51b284a7899db8f62551e3f28ac5be1a8 Mon Sep 17 00:00:00 2001 From: Hampton Lintorn-Catlin Date: Thu, 13 Aug 2026 19:41:19 -0500 Subject: [PATCH 2/2] Avoid N+1 actor lookups in library overview recent activity Amp-Thread-ID: https://ampcode.com/threads/T-019ffc42-a65a-707b-b5e3-79b0276a17c5 Co-authored-by: Amp --- engine/app/controllers/coplan/api/v1/libraries_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/app/controllers/coplan/api/v1/libraries_controller.rb b/engine/app/controllers/coplan/api/v1/libraries_controller.rb index 25f1f6ae..fd589601 100644 --- a/engine/app/controllers/coplan/api/v1/libraries_controller.rb +++ b/engine/app/controllers/coplan/api/v1/libraries_controller.rb @@ -56,7 +56,7 @@ def show } if writable? json[:unfiled_count] = unfiled_plans.count - json[:recent_activity] = @library.library_events.recent_first.limit(10).map { |e| event_json(e) } + json[:recent_activity] = @library.library_events.recent_first.includes(:actor_user).limit(10).map { |e| event_json(e) } end render json: json end