diff --git a/engine/app/controllers/coplan/agent_instructions_controller.rb b/engine/app/controllers/coplan/agent_instructions_controller.rb index 467b743..7a4efc6 100644 --- a/engine/app/controllers/coplan/agent_instructions_controller.rb +++ b/engine/app/controllers/coplan/agent_instructions_controller.rb @@ -33,6 +33,7 @@ def show # carries the request's SCRIPT_NAME. @base = "#{request.base_url}#{root_path.chomp("/")}" @plan_types = PlanType.order(:name) + @create_example_json = create_example_json if prefers_html? # The page is public, but signed-in visitors should still see their @@ -62,6 +63,23 @@ def organizing private + # The Create Plan curl example, with a real configured plan type so + # agents copy an instance-accurate command. Names are admin-controlled + # free text, so the payload is JSON-serialized (never hand-interpolated) + # and single quotes are escaped for the surrounding shell quoting. + def create_example_json + example_type = @plan_types.reject { |t| t.name.casecmp?(PlanType::GENERAL_NAME) }.first + JSON.generate( + { + title: "My Plan", + content: "# My Plan\n\nContent following the type template.", + plan_type: example_type&.name || "general", + folder_path: "Team EBT/Q3" + }, + space: " " + ).gsub("'", "'\\\\''") + end + def prefers_html? return true if params[:format] == "html" return false if params[:format].present? diff --git a/engine/app/controllers/coplan/api/v1/plan_types_controller.rb b/engine/app/controllers/coplan/api/v1/plan_types_controller.rb new file mode 100644 index 0000000..46f9011 --- /dev/null +++ b/engine/app/controllers/coplan/api/v1/plan_types_controller.rb @@ -0,0 +1,24 @@ +module CoPlan + module Api + module V1 + # Read-only catalog of plan types. Agents fetch this before creating a + # plan to pick the most specific type and read its template — the + # template ships here in full because the whole point is that the + # agent structures its draft against it before writing any content. + class PlanTypesController < BaseController + def index + types = PlanType.order(:name) + render json: types.map { |pt| + { + id: pt.id, + name: pt.name, + description: pt.description, + default_tags: pt.default_tags, + template_content: pt.template_content + } + } + 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 f90074a..a00ebbe 100644 --- a/engine/app/controllers/coplan/api/v1/plans_controller.rb +++ b/engine/app/controllers/coplan/api/v1/plans_controller.rb @@ -64,6 +64,27 @@ def create api_token_id: api_token_id ) + # Filing happens in the same transaction as creation so a bad + # folder param never leaves behind an unfiled plan (or, via + # folder_path, orphaned folders) for a create that failed. + if params.key?(:folder_id) || params.key?(:folder_path) + folder = resolve_folder_params + raise ActiveRecord::Rollback if performed? # resolve rendered an error + if folder + result = Plans::Place.call(plan: plan, folder: folder, actor: current_user, actor_type: api_author_type, agent_name: api_agent_name, api_token_id: api_token_id) + unless result.success? + render json: { error: result.error }, status: :unprocessable_content + raise ActiveRecord::Rollback + end + end + end + + # The plan's type contributes its default_tags; explicit tags in + # the request are added on top. plan.plan_type (not the resolved + # param) so the General fallback's defaults apply too. + tags = plan.plan_type&.default_tags.to_a | Array(params[:tags]).map(&:to_s) + plan.tag_names = tags if tags.any? + if params[:references].is_a?(Array) params[:references].each do |ref_params| next unless ref_params[:url].present? @@ -74,6 +95,7 @@ def create end end end + return if performed? # folder error rendered inside the transaction render json: plan_json(plan).merge( current_content: plan.current_content, diff --git a/engine/app/services/coplan/plans/create.rb b/engine/app/services/coplan/plans/create.rb index 4ffe5f3..2c4435c 100644 --- a/engine/app/services/coplan/plans/create.rb +++ b/engine/app/services/coplan/plans/create.rb @@ -44,14 +44,20 @@ def call plan end - CoPlan::Analytics.track( - "plan_created", - user: @user, - plan_id: plan.id, - plan_type_id: plan.plan_type_id, - visibility: plan.visibility, - content_length: @content.to_s.length - ) + # Deferred: callers may wrap creation in a larger transaction (the + # API's create-and-file does), and a rollback there must not leave + # behind an analytics event for a plan that never existed. Outside + # any transaction this runs immediately. + ActiveRecord.after_all_transactions_commit do + CoPlan::Analytics.track( + "plan_created", + user: @user, + plan_id: plan.id, + plan_type_id: plan.plan_type_id, + visibility: plan.visibility, + content_length: @content.to_s.length + ) + end plan end diff --git a/engine/app/views/coplan/agent_instructions/show.text.erb b/engine/app/views/coplan/agent_instructions/show.text.erb index 793c0c6..b7fa8ce 100644 --- a/engine/app/views/coplan/agent_instructions/show.text.erb +++ b/engine/app/views/coplan/agent_instructions/show.text.erb @@ -70,14 +70,28 @@ Returns: plan metadata, `current_content`, `current_revision`, `comment_threads` ### Create Plan +Creating a plan is a deliberate act of publishing into a shared, organized library — not dumping a file. **Before the POST, do three cheap checks** (skip any step whose answer you already have from this session): + +1. **Pick the folder.** `GET <%= @base %>/api/v1/library` shows your folder tree with each folder's `description`. Choose the folder whose description matches this document; pass it as `folder_path` on create. A plan created without one lands unfiled — someone has to clean up after you. +2. **Pick the type.** `GET <%= @base %>/api/v1/plan_types` (or the [Plan Types](#plan-types) table below) lists every type with its description. Choose the **most specific** type that fits. **General is the fallback of last resort** — reaching for it without reading the list is almost always wrong. +3. **Follow the type's template.** The `plan_types` response includes each type's `template_content` — the document structure readers of that type expect. Structure your content against it: keep its sections (drop one only when it's genuinely inapplicable), fill them with real content rather than placeholder text. + +Then create everything in one call: + ```bash <%= @curl %> -X POST \ -H "Content-Type: application/json" \ - -d '{"title": "My Plan", "content": "# My Plan\n\nContent here.", "plan_type": "general"}' \ + -d '<%= raw @create_example_json %>' \ "<%= @base %>/api/v1/plans" | jq . ``` -Optional fields: `plan_type` (string) — the name of a plan type to use; every plan has a type, so omitting this files the plan under the **General** catch-all (see [Plan Types](#plan-types) below); `visibility` (string) — plans are **shared with the whole org by default**; `"draft"` (shown as "private" in the UI) exists as a rare escape hatch, not a normal step (see [Visibility & Archiving](#visibility--archiving)). +Optional fields: + +- `plan_type` (string) — the name of a plan type (step 2 above). Omitting it files the plan under **General**, which should be a considered choice, not a default. The plan type's `default_tags` are applied to the plan automatically. +- `folder_path` (string) or `folder_id` (string) — where to file the plan in your library (step 1 above). `folder_path` finds or creates the hierarchy (e.g. `"Team EBT/Q3"`). See [Libraries & Folders](#libraries--folders). +- `tags` (array of strings) — added on top of the type's `default_tags`. See [Tags](#tags). +- `visibility` (string) — plans are **shared with the whole org by default**; `"draft"` (shown as "private" in the UI) exists as a rare escape hatch, not a normal step (see [Visibility & Archiving](#visibility--archiving)). +- `references` (array) — see [References](#references). #### Diagrams @@ -184,7 +198,7 @@ Each folder includes `id`, `name`, `library_id`, `parent_id`, `path` (e.g. `"Tea `parent_id` is optional — omit it for a top-level folder. Rename with `PATCH /api/v1/folders/:id` (`{"name": "..."}`); delete with `DELETE /api/v1/folders/:id` (only empty folders, only in your own library). -**Shelve a plan in a folder:** +**File a plan at creation** (preferred — pass `folder_path` on `POST /api/v1/plans`, see [Create Plan](#create-plan)) so plans never sit unfiled. To move or file an existing plan: ```bash <%= @curl %> -X PATCH \ @@ -230,15 +244,27 @@ The API also accepts the legacy `status` field (`brainstorm`/`considering`/`deve ### Plan Types -Plan types categorize plans and provide default tags. Every plan has exactly one type. When creating a plan, pass `plan_type` to pick one — plans created without an explicit type get **General**. +Plan types categorize plans, apply default tags, and carry a **content template** — the document structure readers of that type expect. Every plan has exactly one type. + +```bash +<%= @curl %> \ + "<%= @base %>/api/v1/plan_types" | jq . +``` + +Returns every type with `name`, `description`, `default_tags`, and `template_content`. + +**Guidelines:** +- Pick the **most specific** type that fits before creating a plan. Plans created without an explicit type get **General** — acceptable only when you've reviewed the list and nothing fits. +- **Read the type's `template_content` and structure your document against it** — keep its sections, fill them with substance, drop a section only when it's genuinely inapplicable. The template is the type's contract with its readers. +- The type's `default_tags` are applied automatically on create; add your own on top with `tags`. <% if @plan_types.any? %> **Available plan types:** -| Name | Description | -|------|-------------| +| Name | Description | Template | +|------|-------------|----------| <% @plan_types.each do |pt| %> -| `<%= pt.name %>` | <%= pt.description.present? ? pt.description : "—" %> | +| `<%= pt.name %>` | <%= pt.description.present? ? pt.description : "—" %> | <%= pt.template_content.present? ? "yes — fetch and follow it" : "—" %> | <% end %> <% else %> @@ -569,6 +595,12 @@ For approved changes, the recommended path is: read the snapshot → edit the ma ## Typical Workflow +### Creating a plan + +1. **Pick the folder**: `GET /api/v1/library` — match the new document to a folder's description +2. **Pick the type and read its template**: `GET /api/v1/plan_types` — most specific type wins; structure your draft against its `template_content` +3. **Create filed and typed**: `POST /api/v1/plans` with `{title, content, plan_type, folder_path}` + ### Recommended (full content replacement) 1. **Read** the plan: `GET /api/v1/plans/:id/snapshot` diff --git a/engine/config/routes.rb b/engine/config/routes.rb index 67c51a8..b8232ad 100644 --- a/engine/config/routes.rb +++ b/engine/config/routes.rb @@ -53,6 +53,9 @@ namespace :api do namespace :v1 do resources :tags, only: [:index] + # Plan-type catalog (with templates) — agents read this before + # creating a plan; see the Create Plan section of /agent-instructions. + resources :plan_types, only: [:index] resources :folders, only: [:index, :create, :update, :destroy] # The agent organization API: overview (show), bulk read (contents), diff --git a/spec/requests/agent_instructions_spec.rb b/spec/requests/agent_instructions_spec.rb index 564d4d9..87cc6fe 100644 --- a/spec/requests/agent_instructions_spec.rb +++ b/spec/requests/agent_instructions_spec.rb @@ -42,6 +42,48 @@ expect(response.body).to include('"plan_type"') end + it "walks agents through folder, type, and template before creating" do + get agent_instructions_path + + expect(response.body).to include("**Pick the folder.**") + expect(response.body).to include("**Pick the type.**") + expect(response.body).to include("template_content") + expect(response.body).to include("/api/v1/plan_types") + expect(response.body).to include('"folder_path"') + expect(response.body).to include("fallback of last resort") + end + + it "uses a real configured type (not General) in the create example" do + create(:plan_type, name: "General", description: "Catch-all") + create(:plan_type, name: "Design Doc", description: "For design documents") + + get agent_instructions_path + + expect(response.body).to include('"plan_type": "Design Doc"') + end + + # Type names are admin-controlled free text; the example must survive a + # name that would break JSON quoting or the surrounding shell quoting. + it "keeps the create example valid for hostile plan type names" do + create(:plan_type, name: %q(Bob's "Special" Doc)) + + get agent_instructions_path + + # JSON-escaped double quotes, shell-escaped single quote. + expect(response.body).to include('\"Special\"') + expect(response.body).to include(%q(Bob'\''s)) + end + + it "marks which plan types carry a template" do + create(:plan_type, name: "RFC", template_content: "# RFC") + create(:plan_type, name: "Bare", template_content: nil) + + get agent_instructions_path + + expect(response.body).to match(/`RFC`.*yes — fetch and follow it/) + expect(response.body).to match(/`Bare`.*\| —/) + end + it "distinguishes citations, internal section links, and structured references" do get agent_instructions_path diff --git a/spec/requests/api/v1/plan_types_spec.rb b/spec/requests/api/v1/plan_types_spec.rb new file mode 100644 index 0000000..e9a5f59 --- /dev/null +++ b/spec/requests/api/v1/plan_types_spec.rb @@ -0,0 +1,32 @@ +require "rails_helper" + +RSpec.describe "Api::V1::PlanTypes", type: :request do + let(:alice) { create(:coplan_user, :admin) } + let(:alice_token) { create(:api_token, user: alice, raw_token: "test-token-alice") } + let(:headers) { { "Authorization" => "Bearer test-token-alice" } } + + before do + alice_token # ensure token exists + end + + it "requires auth" do + get api_v1_plan_types_path + expect(response).to have_http_status(:unauthorized) + end + + it "returns every plan type with its template and default tags, sorted by name" do + create(:plan_type, name: "RFC", description: "Request for comments", default_tags: ["rfc"], template_content: "# RFC\n\n## Problem\n\n## Proposal") + create(:plan_type, name: "Design Doc", description: "For design documents") + + get api_v1_plan_types_path, headers: headers + + expect(response).to have_http_status(:success) + types = JSON.parse(response.body) + expect(types.map { |t| t["name"] }).to eq(["Design Doc", "RFC"]) + + rfc = types.last + expect(rfc["description"]).to eq("Request for comments") + expect(rfc["default_tags"]).to eq(["rfc"]) + expect(rfc["template_content"]).to include("## Proposal") + end +end diff --git a/spec/requests/api/v1/plans_spec.rb b/spec/requests/api/v1/plans_spec.rb index 60a3f38..ae5b548 100644 --- a/spec/requests/api/v1/plans_spec.rb +++ b/spec/requests/api/v1/plans_spec.rb @@ -118,6 +118,82 @@ expect(response).to have_http_status(:unprocessable_content) end + describe "filing on create" do + it "files the plan via folder_path, creating the hierarchy in the caller's library" do + post api_v1_plans_path, params: { title: "Filed Plan", content: "# Filed", folder_path: "Team EBT/Q3" }, headers: headers, as: :json + expect(response).to have_http_status(:created) + body = JSON.parse(response.body) + expect(body["folder_path"]).to eq("Team EBT/Q3") + + placement = alice.library.placements.find_by(plan_id: body.fetch("id")) + expect(placement.folder.path).to eq("Team EBT/Q3") + expect(alice.library.folders.count).to eq(2) + end + + it "files the plan via folder_id" do + folder = create(:folder, name: "Infra", created_by_user: alice) + post api_v1_plans_path, params: { title: "Filed Plan", content: "# Filed", folder_id: folder.id }, headers: headers, as: :json + expect(response).to have_http_status(:created) + expect(JSON.parse(response.body)["folder_id"]).to eq(folder.id) + end + + it "records the filing in the library audit log with agent attribution" do + post api_v1_plans_path, params: { title: "Filed Plan", content: "# Filed", folder_path: "Infra", agent_name: "Claude" }, headers: headers, as: :json + expect(response).to have_http_status(:created) + + event = alice.library.library_events.find_by(event_type: "plan_filed") + expect(event).to be_present + expect(event.actor_type).to eq("local_agent") + expect(event.agent_name).to eq("Claude") + end + + it "rolls back the whole create when the folder_id is unknown" do + expect { + post api_v1_plans_path, params: { title: "Doomed Plan", content: "# Doomed", folder_id: "nope" }, headers: headers, as: :json + }.not_to change(CoPlan::Plan, :count) + expect(response).to have_http_status(:unprocessable_content) + expect(JSON.parse(response.body)["error"]).to include("Unknown folder_id") + end + + it "does not emit a plan_created analytics event for a rolled-back create" do + events = capture_analytics_events do + post api_v1_plans_path, params: { title: "Doomed Plan", content: "# Doomed", folder_id: "nope" }, headers: headers, as: :json + end + expect(response).to have_http_status(:unprocessable_content) + expect(events.select { |name, _| name == "plan_created" }).to be_empty + end + + it "emits plan_created exactly once for a successful filed create" do + events = capture_analytics_events do + post api_v1_plans_path, params: { title: "Filed Plan", content: "# Filed", folder_path: "Infra" }, headers: headers, as: :json + end + expect(response).to have_http_status(:created) + expect(events.select { |name, _| name == "plan_created" }.length).to eq(1) + end + end + + describe "tags on create" do + it "applies the plan type's default_tags" do + create(:plan_type, name: "design-doc", default_tags: ["design", "architecture"]) + post api_v1_plans_path, params: { title: "Tagged Plan", content: "# Tagged", plan_type: "design-doc" }, headers: headers, as: :json + expect(response).to have_http_status(:created) + expect(JSON.parse(response.body)["tags"]).to match_array(["design", "architecture"]) + end + + it "merges explicit tags with the type's default_tags" do + create(:plan_type, name: "design-doc", default_tags: ["design"]) + post api_v1_plans_path, params: { title: "Tagged Plan", content: "# Tagged", plan_type: "design-doc", tags: ["pricing", "design"] }, headers: headers, as: :json + expect(response).to have_http_status(:created) + expect(JSON.parse(response.body)["tags"]).to match_array(["design", "pricing"]) + end + + it "accepts explicit tags without a plan_type" do + post api_v1_plans_path, params: { title: "Tagged Plan", content: "# Tagged", tags: ["pricing"] }, headers: headers, as: :json + expect(response).to have_http_status(:created) + expect(JSON.parse(response.body)["tags"]).to eq(["pricing"]) + end + end + describe "PATCH /api/v1/plans/:id" do it "updates plan title" do patch api_v1_plan_path(plan), params: { title: "New Title" }, headers: headers, as: :json