From 24ff6df825edc998aa115a7c3d696855f501aaea Mon Sep 17 00:00:00 2001 From: Sean Prashad <13009507+seanprashad@users.noreply.github.com> Date: Thu, 29 Oct 2026 11:02:07 -0400 Subject: [PATCH 01/18] Bind approvals to current content and review cycles --- ..._bind_review_cycles_to_versions.co_plan.rb | 27 ++++ db/schema.rb | 10 +- docs/DEMO_LIBRARY.md | 2 + docs/REVIEWS.md | 33 +++++ .../coplan/api/v1/operations_controller.rb | 5 +- .../coplan/api/v1/plans_controller.rb | 13 +- .../api/v1/review_requests_controller.rb | 11 +- .../coplan/review_requests_controller.rb | 13 +- .../coplan/review_tray_controller.js | 32 +++-- engine/app/models/coplan/plan_version.rb | 16 +++ engine/app/models/coplan/review_request.rb | 22 +++- .../services/coplan/plans/advance_version.rb | 30 +++++ .../services/coplan/plans/commit_session.rb | 5 +- engine/app/services/coplan/plans/create.rb | 2 +- .../services/coplan/plans/replace_content.rb | 5 +- .../services/coplan/plans/toggle_checkbox.rb | 2 +- .../app/services/coplan/reviews/evidence.rb | 27 ++++ .../services/coplan/reviews/record_request.rb | 2 +- engine/app/services/coplan/reviews/request.rb | 4 +- engine/app/services/coplan/reviews/respond.rb | 33 +++-- .../app/services/coplan/reviews/serialize.rb | 14 +- .../app/services/coplan/reviews/withdraw.rb | 5 +- .../agent_instructions/guides/api.text.erb | 6 +- .../guides/comments.text.erb | 4 +- .../agent_instructions/guides/live.text.erb | 1 + .../views/coplan/plan_versions/show.html.erb | 3 + .../views/coplan/reviews/_activity.html.erb | 2 +- .../views/coplan/reviews/_controls.html.erb | 16 ++- engine/app/views/coplan/reviews/_log.html.erb | 1 + .../app/views/coplan/reviews/_roster.html.erb | 6 +- .../app/views/coplan/reviews/_table.html.erb | 4 +- engine/config/routes.rb | 2 + engine/db/demo/content/changed-review.md | 5 + ...29000000_bind_review_cycles_to_versions.rb | 26 ++++ engine/lib/coplan/development_seed.rb | 12 +- spec/helpers/application_helper_spec.rb | 4 +- spec/helpers/plans_helper_spec.rb | 8 +- spec/lib/development_seed_spec.rb | 7 + spec/models/plan_version_spec.rb | 17 +++ spec/requests/api/v1/operations_spec.rb | 2 +- spec/requests/api/v1/review_evidence_spec.rb | 55 ++++++++ spec/requests/api/v1/review_requests_spec.rb | 2 +- .../changed_sections_highlight_spec.rb | 4 +- spec/requests/human_document_editor_spec.rb | 2 +- spec/requests/plan_content_editing_spec.rb | 2 +- spec/requests/plan_versions_spec.rb | 2 +- spec/requests/plans_spec.rb | 10 +- spec/requests/review_requests_spec.rb | 11 +- spec/requests/toggle_checkbox_spec.rb | 4 +- spec/services/broadcaster_spec.rb | 4 +- .../references/extract_from_content_spec.rb | 4 +- .../coplan/review_concurrency_spec.rb | 83 ++++++++++++ spec/services/coplan/reviews_spec.rb | 15 ++- .../services/coplan/versioned_reviews_spec.rb | 121 ++++++++++++++++++ spec/services/link_previews_spec.rb | 4 +- spec/services/plans/section_updates_spec.rb | 2 +- spec/support/version_fixtures.rb | 12 ++ spec/system/changed_sections_ux_spec.rb | 2 +- spec/system/checkbox_spec.rb | 4 +- spec/system/comment_ux_spec.rb | 18 +-- spec/system/folders_workspace_spec.rb | 2 +- spec/system/plan_reviews_spec.rb | 62 ++++++++- 62 files changed, 740 insertions(+), 124 deletions(-) create mode 100644 db/migrate/20261029141536_bind_review_cycles_to_versions.co_plan.rb create mode 100644 docs/REVIEWS.md create mode 100644 engine/app/services/coplan/plans/advance_version.rb create mode 100644 engine/app/services/coplan/reviews/evidence.rb create mode 100644 engine/db/demo/content/changed-review.md create mode 100644 engine/db/migrate/20261029000000_bind_review_cycles_to_versions.rb create mode 100644 spec/requests/api/v1/review_evidence_spec.rb create mode 100644 spec/services/coplan/review_concurrency_spec.rb create mode 100644 spec/services/coplan/versioned_reviews_spec.rb create mode 100644 spec/support/version_fixtures.rb diff --git a/db/migrate/20261029141536_bind_review_cycles_to_versions.co_plan.rb b/db/migrate/20261029141536_bind_review_cycles_to_versions.co_plan.rb new file mode 100644 index 00000000..ceff6de7 --- /dev/null +++ b/db/migrate/20261029141536_bind_review_cycles_to_versions.co_plan.rb @@ -0,0 +1,27 @@ +# This migration comes from co_plan (originally 20261029000000) +class BindReviewCyclesToVersions < ActiveRecord::Migration[8.1] + def up + add_column :coplan_plans, :review_sequence, :bigint, null: false, default: 0 + add_column :coplan_review_requests, :generation, :bigint, null: false, default: 1 + add_column :coplan_review_requests, :current_response_id, :string, limit: 36 + add_column :coplan_review_requests, :reset_reason, :string + add_column :coplan_review_responses, :request_generation, :bigint + add_column :coplan_review_responses, :decision_sequence, :bigint + add_index :coplan_review_requests, :current_response_id + add_foreign_key :coplan_review_requests, :coplan_review_responses, column: :current_response_id, on_delete: :nullify + # Old timestamps cannot prove which request cycle a response completed. + # Keep those receipts as history; require a fresh decision for active tasks. + execute "UPDATE coplan_review_requests SET generation = 2" + execute "UPDATE coplan_review_requests SET status = 'pending', reset_reason = 'legacy_history' WHERE status <> 'withdrawn'" + end + + def down + remove_foreign_key :coplan_review_requests, column: :current_response_id + remove_column :coplan_review_requests, :current_response_id + remove_column :coplan_review_requests, :generation + remove_column :coplan_review_requests, :reset_reason + remove_column :coplan_review_responses, :request_generation + remove_column :coplan_review_responses, :decision_sequence + remove_column :coplan_plans, :review_sequence + end +end diff --git a/db/schema.rb b/db/schema.rb index a954c7a9..65543fc8 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_10_19_185311) do +ActiveRecord::Schema[8.1].define(version: 2026_10_29_141536) 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" @@ -411,6 +411,7 @@ t.integer "current_revision", default: 0, null: false t.json "metadata" t.string "plan_type_id", limit: 36, null: false + t.bigint "review_sequence", default: 0, null: false t.text "search_text", size: :medium t.string "slug", null: false t.string "slug_suffix", limit: 8 @@ -454,14 +455,18 @@ create_table "coplan_review_requests", id: { type: :string, limit: 36 }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "created_at", null: false + t.string "current_response_id", limit: 36 + t.bigint "generation", default: 1, null: false t.string "plan_id", limit: 36, null: false t.datetime "requested_at", null: false t.string "requested_by_user_id", limit: 36, null: false t.string "requested_plan_version_id", limit: 36, null: false t.string "requirement", null: false + t.string "reset_reason" t.string "reviewer_id", limit: 36, null: false t.string "status", null: false t.datetime "updated_at", null: false + t.index ["current_response_id"], name: "index_coplan_review_requests_on_current_response_id" t.index ["plan_id", "reviewer_id"], name: "index_coplan_review_requests_on_plan_id_and_reviewer_id", unique: true t.index ["requested_by_user_id"], name: "fk_rails_c5db6a7f68" t.index ["requested_plan_version_id"], name: "fk_rails_9648b04dbb" @@ -475,8 +480,10 @@ t.string "comment_thread_id", limit: 36 t.datetime "created_at", null: false t.string "decision", null: false + t.bigint "decision_sequence" t.text "note" t.string "plan_version_id", limit: 36, null: false + t.bigint "request_generation" t.string "review_request_id", limit: 36, null: false t.string "user_id", limit: 36, null: false t.index ["api_token_id"], name: "fk_rails_edc220f557" @@ -603,6 +610,7 @@ add_foreign_key "coplan_references", "coplan_plans", column: "target_plan_id" add_foreign_key "coplan_review_requests", "coplan_plan_versions", column: "requested_plan_version_id" add_foreign_key "coplan_review_requests", "coplan_plans", column: "plan_id" + add_foreign_key "coplan_review_requests", "coplan_review_responses", column: "current_response_id", on_delete: :nullify add_foreign_key "coplan_review_requests", "coplan_users", column: "requested_by_user_id" add_foreign_key "coplan_review_requests", "coplan_users", column: "reviewer_id" add_foreign_key "coplan_review_responses", "coplan_api_tokens", column: "api_token_id", on_delete: :nullify diff --git a/docs/DEMO_LIBRARY.md b/docs/DEMO_LIBRARY.md index 914e979a..fe0014ca 100644 --- a/docs/DEMO_LIBRARY.md +++ b/docs/DEMO_LIBRARY.md @@ -37,6 +37,8 @@ install demo users or documents. ## Named scenarios +The **Workspace invitations: review dismissed after edits** scenario keeps v1 approvals as history while v2 awaits fresh review. Sign in as Alex to approve v2, or Hampton to edit it again. See [Version-bound reviews](REVIEWS.md) for the source scope and lifecycle contract. + The index starts with **Diagrams**, **Presentations**, **Tables and rich text**, and **References**. Every link labels the document's actual plan type. The **Every plan type** section links to a real example of each of the twelve diff --git a/docs/REVIEWS.md b/docs/REVIEWS.md new file mode 100644 index 00000000..048b10a8 --- /dev/null +++ b/docs/REVIEWS.md @@ -0,0 +1,33 @@ +# Version-bound reviews + +A native review means satisfaction with a specific Markdown source version. It grants no execution authority or permissions. An agent response remains an agent response attributed to its human principal; it is never a human sign-off. + +## Content and decision identity + +Responses retain `plan_version_id`, that version’s SHA-256, `request_generation`, and a server-assigned `decision_sequence`. UUIDs identify transport retries; they do not order decisions. A request’s `current_response_id` is the authoritative projection. A response is effective only when it belongs to that request, generation and current plan version and matches the task status. `effective_human_decision` is null for pending, withdrawn, dismissed or agent decisions. `latest_approval` is positive history for comparison, not qualification. + +The approval covers **Markdown source bytes only**. Title, tags, attachments, linked and embedded resources, child plans, and rendered output are excluded. URLs and embedded source syntax are approved as literal text, not as the bytes they might display. Historical pages expose exact source and hash and label the current title as metadata. Full rich-page approval requires a separate immutable asset envelope and renderer contract. + +## Edits and cycles + +All current-version writers use `Plans::AdvanceVersion`: Create, ReplaceContent (including SaveHumanDraft/web saves), CommitSession, ToggleCheckbox and the direct operations API. They already hold the plan row lock; CommitSession retains session-before-plan lock order. The version pointer, revision, review sequence, active task resets, generation increments and cleared current pointers commit together. Withdrawn tasks and all receipts remain intact. Resets broadcast after commit without sending a review-request notification for every edit. + +An identical normalized ReplaceContent save creates no version and preserves approval. An empty session creates no version. A nonempty session or operations commit creates a new version even if its operations net to identical bytes, and dismisses decisions. Returning A→B→A always produces a distinct version and requires fresh review. Existing edit leases, human-edit fences and merge guards remain in place. + +An explicit re-request advances the task generation and clears its effective decision. Creating an already existing request is a retry and leaves its state alone. Withdrawal clears the effective decision and preserves receipts. A fresh response supersedes the previous effective response, including a later agent response. The latest sequenced human response on the current version is exposed separately so consumers can inspect human feedback across request cycles; native CoPlan does not impose an external approval or objection policy. + +New responses send the viewed revision, fetched generation and a fresh UUID `idempotency_key`. Stale revisions or generations fail with 409. Repeating a recorded key returns its immutable receipt without touching status, current pointer, sequence, attribution or notifications, even after an edit, withdrawal or re-request. Unkeyed identical legacy retries return history too. First-cycle clients may omit generation; subsequent cycles require it, including first-seen delayed keyed submissions. A fresh intentional identical response needs a new key and the current generation. + +## Reads and compatibility + +`GET /api/v1/plans/:id/review_evidence` takes one plan-row transaction and returns exact current source/hash/version, scope, review sequence, current projection and complete response history. It creates no PlanRead or agent session. Ordinary authentication may still update token usage. The review sequence covers body and native review changes, not excluded metadata, sharing or external authorization. Existing snapshot reads retain their older behavior; consumers requiring coherent evidence use this endpoint. + +`GET /api/v1/plans/:id/versions/:version_id` returns that exact historical Markdown and hash. It never substitutes the current version. Both endpoints use existing Bearer authentication and visibility rules; neither establishes a read-only workload credential or employee identity mapping. + +Migration preserves old receipts with unknown generation/sequence. Existing active tasks become pending with `reset_reason=legacy_history`; withdrawn tasks stay withdrawn. Existing tasks enter generation 2 so first-seen old requests without a cycle cannot complete them. No timestamp or UUID ordering is used to invent legacy effective approvals. Hosts must install the engine migration and update clients to send generation. Downgrading drops the new projection but does not restore prior statuses. + +Persisted version IDs, plan IDs, revisions, Markdown and hashes reject ordinary Rails mutation, including `update_columns`. Writing provenance backfills remain supported. Hashes are checked at creation and before new responses or source evidence reads. These are application guarantees, not database/WORM protection against administrator SQL, bulk writes or deletion. + +## Local tryout + +Seed the shared demo library with `bin/rails coplan:demo:seed`. Sign in as `alex@example.com` and open **Workspace invitations: review dismissed after edits** in Hampton’s Demo scenarios. v1 approvals are history, v2 is pending, and the activity link opens the original source. Approve v2, then sign in as `hampton@example.com` and edit its body or toggle its checkbox. Alex’s task returns to pending. Re-seeding preserves edits and decisions. diff --git a/engine/app/controllers/coplan/api/v1/operations_controller.rb b/engine/app/controllers/coplan/api/v1/operations_controller.rb index 4086251b..7b83fb57 100644 --- a/engine/app/controllers/coplan/api/v1/operations_controller.rb +++ b/engine/app/controllers/coplan/api/v1/operations_controller.rb @@ -230,10 +230,7 @@ def commit_version(current_content, result) reason: params[:reason] ) - @plan.update!( - current_plan_version: version, - current_revision: new_revision - ) + Plans::AdvanceVersion.call(plan: @plan, version: version) @plan.comment_threads.mark_out_of_date_for_new_version!(version) diff --git a/engine/app/controllers/coplan/api/v1/plans_controller.rb b/engine/app/controllers/coplan/api/v1/plans_controller.rb index fb7cc2ab..29f0bb56 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, :locations, :writing_provenance ] - before_action :authorize_plan_access!, only: [ :show, :update, :versions, :comments, :snapshot, :locations, :writing_provenance ] + before_action :set_plan, only: [ :show, :update, :version, :versions, :comments, :snapshot, :review_evidence, :locations, :writing_provenance ] + before_action :authorize_plan_access!, only: [ :show, :update, :version, :versions, :comments, :snapshot, :review_evidence, :locations, :writing_provenance ] def index plans = Plan @@ -270,6 +270,11 @@ def update render json: { error: e.record.errors.full_messages.join(", ") }, status: :unprocessable_content end + def version + version = @plan.plan_versions.find(params[:version_id]) + render json: version_json(version).merge(content_markdown: version.verified_content!, approval_scope: Reviews::Evidence::SCOPE) + end + def versions versions = @plan.plan_versions.order(revision: :desc) render json: versions.map { |v| version_json(v) } @@ -307,6 +312,10 @@ def comments render json: threads.map { |t| thread_json(t) } end + def review_evidence + render json: Reviews::Evidence.call(plan: @plan) + end + def snapshot record_plan_read!(@plan) threads = @plan.comment_threads.includes(:comments, :created_by_user).order(created_at: :desc) diff --git a/engine/app/controllers/coplan/api/v1/review_requests_controller.rb b/engine/app/controllers/coplan/api/v1/review_requests_controller.rb index c9b9d4da..015761d5 100644 --- a/engine/app/controllers/coplan/api/v1/review_requests_controller.rb +++ b/engine/app/controllers/coplan/api/v1/review_requests_controller.rb @@ -37,7 +37,10 @@ def update Reviews::Request.call(plan: @plan, reviewer: @review_request.reviewer, actor: current_user, requirement: params[:requirement].presence || @review_request.requirement, api_token: @api_token) else - @plan.with_lock { @review_request.update!(requirement: params.require(:requirement)) } + @plan.with_lock do + @review_request.update!(requirement: params.require(:requirement)) + @plan.update!(review_sequence: @plan.review_sequence + 1) + end Reviews::Changed.call(request: @review_request, event_type: "review.requested", actor_token_id: api_token_id, payload: { "action" => "requirement_changed", "requirement" => @review_request.requirement }) end @@ -54,10 +57,12 @@ def respond return render json: { error: "Only the requested reviewer can respond" }, status: :forbidden end response = Reviews::Respond.call(request: @review_request, actor: current_user, - revision: params.require(:revision), decision: params.require(:decision), note: params[:note], api_token: @api_token, idempotency_key: params[:idempotency_key]) + revision: params.require(:revision), decision: params.require(:decision), note: params[:note], api_token: @api_token, idempotency_key: params[:idempotency_key], generation: params[:generation]) render json: { id: response.id, decision: response.decision, revision: response.plan_version.revision, actor_type: response.actor_type, agent_name: response.agent_name, - comment_thread_id: response.comment_thread_id }, status: :created + comment_thread_id: response.comment_thread_id, plan_version_id: response.plan_version_id, + content_sha256: response.plan_version.content_sha256, generation: response.request_generation, + decision_sequence: response.decision_sequence, effective: @review_request.reload.effective_response&.id == response.id }, status: :created end private diff --git a/engine/app/controllers/coplan/review_requests_controller.rb b/engine/app/controllers/coplan/review_requests_controller.rb index f62be26d..dfd31bf7 100644 --- a/engine/app/controllers/coplan/review_requests_controller.rb +++ b/engine/app/controllers/coplan/review_requests_controller.rb @@ -31,7 +31,10 @@ def update Reviews::Request.call(plan: @plan, reviewer: @review_request.reviewer, actor: current_user, requirement: attributes[:requirement].presence || @review_request.requirement) else - @plan.with_lock { @review_request.update!(requirement: attributes.fetch(:requirement)) } + @plan.with_lock do + @review_request.update!(requirement: attributes.fetch(:requirement)) + @plan.update!(review_sequence: @plan.review_sequence + 1) + end Reviews::Changed.call(request: @review_request, event_type: "review.requested", payload: { "action" => "requirement_changed", "requirement" => @review_request.requirement }) end @@ -46,11 +49,13 @@ def destroy def respond raise NotAuthorizedError unless @review_request.reviewer_id == current_user.id - attributes = params.expect(review_response: [ :revision, :decision, :note, :idempotency_key ]) + attributes = params.expect(review_response: [ :revision, :decision, :note, :idempotency_key, :generation ]) response = Reviews::Respond.call(request: @review_request, actor: current_user, - revision: attributes[:revision], decision: attributes[:decision], note: attributes[:note], idempotency_key: attributes[:idempotency_key]) + revision: attributes[:revision], decision: attributes[:decision], note: attributes[:note], idempotency_key: attributes[:idempotency_key], generation: attributes[:generation]) render json: { id: response.id, decision: response.decision, revision: response.plan_version.revision, - comment_thread_id: response.comment_thread_id }, status: :created + comment_thread_id: response.comment_thread_id, plan_version_id: response.plan_version_id, + content_sha256: response.plan_version.content_sha256, generation: response.request_generation, + decision_sequence: response.decision_sequence, effective: @review_request.reload.effective_response&.id == response.id }, status: :created end def compare diff --git a/engine/app/javascript/controllers/coplan/review_tray_controller.js b/engine/app/javascript/controllers/coplan/review_tray_controller.js index e818fb8b..14a23378 100644 --- a/engine/app/javascript/controllers/coplan/review_tray_controller.js +++ b/engine/app/javascript/controllers/coplan/review_tray_controller.js @@ -125,13 +125,17 @@ export default class extends Controller { requestChanges(event) { this.responseUrl = event.currentTarget.dataset.url - if (this.discussionTarget.hidden) this.changesRevision = this.viewedRevision() + if (this.discussionTarget.hidden) { + this.changesRevision = this.viewedRevision() + this.changesGeneration = this.element.dataset.reviewGeneration + } this.expand() } confirmApproval(event) { this.responseUrl = event.currentTarget.dataset.url this.approvalRevision = this.viewedRevision() + this.approvalGeneration = this.element.dataset.reviewGeneration this.approvalErrorTarget.textContent = "" this.approveButtonTarget.textContent = `Approve v${this.approvalRevision}` this.approvalTitleTarget.textContent = `Approve v${this.approvalRevision}?` @@ -142,12 +146,14 @@ export default class extends Controller { this.approvalTarget.close() } - responseIntent(url, revision, decision, note) { + responseIntent(url, revision, decision, note, generation) { this.responseIntents ||= {} + // A lost result may be retried after a re-request. Keep its original + // generation with the key until the server acknowledges that intent. const fields = JSON.stringify([url, String(revision), note.trim()]) const previous = this.responseIntents[decision] if (previous?.fields === fields) return previous - return this.responseIntents[decision] = { fields, key: crypto.randomUUID() } + return this.responseIntents[decision] = { fields, generation, key: crypto.randomUUID() } } confirmIntent(decision, intent) { @@ -157,13 +163,15 @@ export default class extends Controller { async approve(event) { event.preventDefault() await this.withBusy(event.target, this.approvalErrorTarget, async () => { - const intent = this.responseIntent(this.responseUrl, this.approvalRevision, "approved", this.approvalNoteTarget.value) - const result = await this.write(this.responseUrl, "POST", { review_response: { revision: this.approvalRevision, decision: "approved", note: this.approvalNoteTarget.value, idempotency_key: intent.key } }) + const intent = this.responseIntent(this.responseUrl, this.approvalRevision, "approved", this.approvalNoteTarget.value, this.approvalGeneration) + const result = await this.write(this.responseUrl, "POST", { review_response: { revision: this.approvalRevision, generation: intent.generation, decision: "approved", note: this.approvalNoteTarget.value, idempotency_key: intent.key } }) this.confirmIntent("approved", intent) const synced = !result.comment_thread_id || await this.refreshThreads(result.comment_thread_id) this.approvalTarget.close() this.approvalNoteTarget.value = "" - if (this.hasNoticeTarget) this.noticeTarget.textContent = `Your approval of v${this.approvalRevision} is recorded.` + (synced ? "" : " Refresh the page to see your comment.") + if (this.hasNoticeTarget) this.noticeTarget.textContent = result.effective === false + ? "Your earlier decision was already recorded. Review the current request again." + : `Your approval of v${this.approvalRevision} is recorded.` + (synced ? "" : " Refresh the page to see your comment.") const tray = this.element.querySelector(".review-tray") if (tray) delete tray.dataset.reconsidering await this.refresh() @@ -177,8 +185,8 @@ export default class extends Controller { if (form.closest('[data-dictation-busy="true"]') || form.querySelector('[data-dictation-busy="true"]')) return const error = form.querySelector('[role="alert"]') await this.withBusy(form, error, async () => { - const intent = this.responseIntent(form.action, this.changesRevision, "changes_requested", input.value) - const result = await this.write(form.action, "POST", { review_response: { revision: this.changesRevision, decision: "changes_requested", note: input.value, idempotency_key: intent.key } }) + const intent = this.responseIntent(form.action, this.changesRevision, "changes_requested", input.value, this.changesGeneration) + const result = await this.write(form.action, "POST", { review_response: { revision: this.changesRevision, generation: intent.generation, decision: "changes_requested", note: input.value, idempotency_key: intent.key } }) this.confirmIntent("changes_requested", intent) // Feedback lives in the ordinary discussion collection. const synced = await this.refreshThreads(result.comment_thread_id) @@ -218,6 +226,7 @@ export default class extends Controller { if (!response.ok) return const incoming = new DOMParser().parseFromString(await response.text(), "text/html") if (sequence !== this.refreshSequence || !this.element.isConnected) return + this.element.dataset.reviewGeneration = incoming.getElementById("plan-review-controls")?.dataset.reviewGeneration || "" const tray = incoming.querySelector(".review-tray") const existingTray = this.element.querySelector(".review-tray") const wasWithdrawn = this.reviewWithdrawn @@ -431,6 +440,13 @@ export default class extends Controller { let result try { result = await response.json() } catch { throw new Error("Couldn't save your response. Your draft is still here. Please try again.") } if (!response.ok) { + // A definite rejection did not record this intent. Refresh the cycle + // so closing and deliberately reopening the dialog can start a new one. + // Network failures and lost results keep their original key/generation. + if (String(url).endsWith("/respond") && response.status >= 400 && response.status < 500) { + this.responseIntents = {} + await this.refresh() + } const error = new Error(result.error || "Couldn't save. Please try again.") if (result.code === "authentication_required") error.signInPath = result.sign_in_path throw error diff --git a/engine/app/models/coplan/plan_version.rb b/engine/app/models/coplan/plan_version.rb index 66d25607..3279d2b1 100644 --- a/engine/app/models/coplan/plan_version.rb +++ b/engine/app/models/coplan/plan_version.rb @@ -1,5 +1,7 @@ module CoPlan class PlanVersion < ApplicationRecord + attr_readonly :id, :plan_id, :revision, :content_markdown, :content_sha256 + attr_accessor :edit_lease_token, :input_sources, :writing_identity before_create :enforce_edit_lease @@ -35,6 +37,16 @@ def self.ransackable_associations(_auth_object = nil) before_validation :compute_sha256, if: -> { content_markdown.present? && content_sha256.blank? } + validate :content_hash_matches, on: :create + + def verified_content! + unless Digest::SHA256.hexdigest(content_markdown.to_s) == content_sha256 + errors.add(:content_sha256, "does not match the stored Markdown bytes") + raise ActiveRecord::RecordInvalid, self + end + content_markdown + end + # Marker so a mixed history feed (PlanVersion + PlanEvent) can render each # item appropriately without introspecting class names. def history_kind @@ -99,6 +111,10 @@ def broadcast_history_update ) end + def content_hash_matches + errors.add(:content_sha256, "must match the Markdown bytes") unless Digest::SHA256.hexdigest(content_markdown.to_s) == content_sha256 + end + def compute_sha256 self.content_sha256 = Digest::SHA256.hexdigest(content_markdown) end diff --git a/engine/app/models/coplan/review_request.rb b/engine/app/models/coplan/review_request.rb index 27b9d963..140d1c12 100644 --- a/engine/app/models/coplan/review_request.rb +++ b/engine/app/models/coplan/review_request.rb @@ -10,7 +10,8 @@ class ReviewRequest < ApplicationRecord # Historical context of when the request was sent, never a review target. # Pending requests follow the plan; only responses bind to a version. belongs_to :requested_plan_version, class_name: "CoPlan::PlanVersion" - has_many :review_responses, -> { order(created_at: :desc, id: :desc) }, dependent: :delete_all + has_many :review_responses, -> { order(Arel.sql("CASE WHEN decision_sequence IS NULL THEN 0 ELSE 1 END DESC"), decision_sequence: :desc, created_at: :desc, id: :desc) }, dependent: :delete_all + belongs_to :current_response, class_name: "CoPlan::ReviewResponse", optional: true has_many :notifications, dependent: :destroy validates :reviewer_id, uniqueness: { scope: :plan_id } @@ -23,13 +24,30 @@ class ReviewRequest < ApplicationRecord scope :pending, -> { where(status: "pending") } def pending? - status == "pending" + effective_status == "pending" end + # Positive history is for comparison only. Never use it as current evidence. def latest_approval review_responses.find { |response| response.decision == "approved" } end + def effective_response + response = current_response + response if response && status == response.decision && + response.review_request_id == id && response.request_generation == generation && + response.plan_version_id == plan.current_plan_version_id + end + + def effective_status + status == "withdrawn" ? "withdrawn" : (effective_response&.decision || "pending") + end + + def effective_human_decision + response = effective_response + response.decision if response&.actor_type == "human" + end + def self.ransackable_attributes(_auth_object = nil) %w[id plan_id reviewer_id requirement status requested_at created_at updated_at] end diff --git a/engine/app/services/coplan/plans/advance_version.rb b/engine/app/services/coplan/plans/advance_version.rb new file mode 100644 index 00000000..fcd6d126 --- /dev/null +++ b/engine/app/services/coplan/plans/advance_version.rb @@ -0,0 +1,30 @@ +module CoPlan + module Plans + # All current-version writers call this inside their plan-row transaction. + # CommitSession still locks its session before the plan. + class AdvanceVersion + def self.call(plan:, version:) + raise ArgumentError, "Version advancement requires the writer’s plan transaction" unless plan.class.connection.transaction_open? + raise ArgumentError, "Version must belong to the plan and advance its revision" unless version.plan_id == plan.id && version.revision > plan.current_revision + + plan.update!(current_plan_version: version, current_revision: version.revision, + review_sequence: plan.review_sequence + 1) + requests = plan.review_requests.active.to_a + requests.each do |request| + request.update!(status: "pending", generation: request.generation + 1, + current_response: nil, reset_reason: "content_changed") + end + # No new review notifications for each edit, and no reset broadcast + # for a transaction that rolls back. + ActiveRecord.after_all_transactions_commit do + requests.each do |request| + Reviews::Changed.call(request: request, event_type: "review.requested", + payload: { "action" => "content_changed", "revision" => version.revision, + "generation" => request.generation }) + end + end + version + end + end + end +end diff --git a/engine/app/services/coplan/plans/commit_session.rb b/engine/app/services/coplan/plans/commit_session.rb index 7aae8325..e694834f 100644 --- a/engine/app/services/coplan/plans/commit_session.rb +++ b/engine/app/services/coplan/plans/commit_session.rb @@ -149,10 +149,7 @@ def call base_revision: @session.base_revision ) - plan.update!( - current_plan_version: version, - current_revision: new_revision - ) + Plans::AdvanceVersion.call(plan: plan, version: version) plan.comment_threads.mark_out_of_date_for_new_version!(version) diff --git a/engine/app/services/coplan/plans/create.rb b/engine/app/services/coplan/plans/create.rb index d72961e8..5ad811ae 100644 --- a/engine/app/services/coplan/plans/create.rb +++ b/engine/app/services/coplan/plans/create.rb @@ -43,7 +43,7 @@ def call agent_name: @agent_name, api_token_id: @api_token_id ) - plan.update!(current_plan_version: version, current_revision: 1) + AdvanceVersion.call(plan: plan, version: version) plan end diff --git a/engine/app/services/coplan/plans/replace_content.rb b/engine/app/services/coplan/plans/replace_content.rb index f509011b..56858384 100644 --- a/engine/app/services/coplan/plans/replace_content.rb +++ b/engine/app/services/coplan/plans/replace_content.rb @@ -136,10 +136,7 @@ def call reason: @reason ) - @plan.update!( - current_plan_version: version, - current_revision: new_revision - ) + Plans::AdvanceVersion.call(plan: @plan, version: version) @plan.comment_threads.mark_out_of_date_for_new_version!(version) diff --git a/engine/app/services/coplan/plans/toggle_checkbox.rb b/engine/app/services/coplan/plans/toggle_checkbox.rb index 1b5b32e8..0c3a5d82 100644 --- a/engine/app/services/coplan/plans/toggle_checkbox.rb +++ b/engine/app/services/coplan/plans/toggle_checkbox.rb @@ -82,7 +82,7 @@ def call base_revision: @base_revision ) - @plan.update!(current_plan_version: version, current_revision: new_revision) + AdvanceVersion.call(plan: @plan, version: version) @plan.comment_threads.mark_out_of_date_for_new_version!(version) Broadcaster.replace_to( diff --git a/engine/app/services/coplan/reviews/evidence.rb b/engine/app/services/coplan/reviews/evidence.rb new file mode 100644 index 00000000..7aa499c6 --- /dev/null +++ b/engine/app/services/coplan/reviews/evidence.rb @@ -0,0 +1,27 @@ +module CoPlan + module Reviews + # Body/review writers serialize on this row too. This read deliberately + # creates no PlanRead, agent session, or notification acknowledgement. + class Evidence + SCOPE = { + kind: "markdown_source", includes: [ "content_markdown" ], + excludes: %w[title tags attachments linked_resources embedded_resources child_plans rendered_output], + meaning: "Approval covers the Markdown source bytes only. Linked or embedded content is not captured or approved. Grants no execution authority or permissions." + }.freeze + + def self.call(plan:) + plan.with_lock do + version = plan.current_plan_version + { + schema_version: 1, plan_id: plan.id, review_sequence: plan.review_sequence, + approval_scope: SCOPE, + version: { id: version.id, revision: version.revision, + content_sha256: version.content_sha256, content_markdown: version.verified_content! }, + review_requests: plan.review_requests.includes(:reviewer, :current_response, + review_responses: :plan_version).order(:id).map { |request| Serialize.call(request) } + } + end + end + end + end +end diff --git a/engine/app/services/coplan/reviews/record_request.rb b/engine/app/services/coplan/reviews/record_request.rb index fbcb90da..7eba7829 100644 --- a/engine/app/services/coplan/reviews/record_request.rb +++ b/engine/app/services/coplan/reviews/record_request.rb @@ -16,7 +16,7 @@ def call api_token: api_token, agent_name: api_token && ApiToken.normalized_agent_name(api_token.agent_name.presence || api_token.name), created_at: request.requested_at, metadata: { "reviewer_name" => request.reviewer.name, "revision" => request.requested_plan_version.revision, - "requirement" => request.requirement }) + "requirement" => request.requirement, "generation" => request.generation, "review_sequence" => request.plan.review_sequence }) end end end diff --git a/engine/app/services/coplan/reviews/request.rb b/engine/app/services/coplan/reviews/request.rb index 5e646a28..8badd4c6 100644 --- a/engine/app/services/coplan/reviews/request.rb +++ b/engine/app/services/coplan/reviews/request.rb @@ -30,8 +30,10 @@ def call RecordRequest.call(request: request) end request.assign_attributes(requested_by_user: @actor, requested_plan_version: @plan.current_plan_version, - requirement: @requirement, status: "pending", requested_at: Time.current) + requirement: @requirement, status: "pending", requested_at: Time.current, + generation: request.persisted? ? request.generation + 1 : 1, current_response: nil, reset_reason: request.persisted? ? "requested_again" : nil) request.save! + @plan.update!(review_sequence: @plan.review_sequence + 1) RecordRequest.call(request: request, api_token: @api_token) # One unread nudge per task. Reading it never completes the task. request.notifications.unread.where(user: @reviewer).update_all(read_at: Time.current) diff --git a/engine/app/services/coplan/reviews/respond.rb b/engine/app/services/coplan/reviews/respond.rb index a004c2ca..f8fade4a 100644 --- a/engine/app/services/coplan/reviews/respond.rb +++ b/engine/app/services/coplan/reviews/respond.rb @@ -10,12 +10,13 @@ def initialize(message, code: "review_version_changed") end end - def self.call(request:, actor:, revision:, decision:, note: nil, api_token: nil, idempotency_key: nil) - new(request:, actor:, revision:, decision:, note:, api_token:, idempotency_key:).call + def self.call(request:, actor:, revision:, decision:, note: nil, api_token: nil, idempotency_key: nil, generation: nil) + new(request:, actor:, revision:, decision:, note:, api_token:, idempotency_key:, generation:).call end - def initialize(request:, actor:, revision:, decision:, note:, api_token:, idempotency_key:) + def initialize(request:, actor:, revision:, decision:, note:, api_token:, idempotency_key:, generation:) @request, @actor, @revision, @decision, @note, @api_token = request, actor, revision, decision, note, api_token + @generation = generation @idempotency_key = idempotency_key.presence end @@ -45,16 +46,26 @@ def call end response.id = @idempotency_key end + # A legacy identical retry is historical even after a re-request. + # It cannot change status, attribution, sequence or the current pointer. + existing = @request.review_responses.find_by(user: @actor, + plan_version_id: plan.plan_versions.where(revision: @revision).select(:id), + decision: @decision, note: response.note) + next existing if existing && !@idempotency_key + raise Conflict.new("This review request was withdrawn.", code: "review_request_withdrawn") if @request.status == "withdrawn" raise Conflict, "The plan has changed. Review the latest version before recording your response." unless @revision.to_s == plan.current_revision.to_s - # Legacy clients cannot distinguish two identical intentions. Keep - # the old response and preserve any newly re-requested pending task. - # Token renewal changes the credential, not the human's intention; - # returning the saved record preserves its original attribution. - existing = @request.review_responses.find_by(user: @actor, plan_version: plan.current_plan_version, - decision: @decision, note: response.note) - next existing if existing && !@idempotency_key + # First-cycle legacy clients remain usable. After any reset they + # must fetch and send the generation; a first-seen delayed request + # must never complete a reopened task at the same body revision. + unless @generation.to_s == @request.generation.to_s || (@generation.nil? && @request.generation == 1) + raise Conflict.new("The review request changed. Reload it before recording a new decision.", code: "review_request_changed") + end + plan.current_plan_version.verified_content! + plan.update!(review_sequence: plan.review_sequence + 1) + response.request_generation = @request.generation + response.decision_sequence = plan.review_sequence if response.note.present? thread = plan.comment_threads.create!(plan_version: plan.current_plan_version, created_by_user: @actor) @@ -64,7 +75,7 @@ def call end response.save! @new_response = true - @request.update!(status: @decision) + @request.update!(status: @decision, current_response: response, reset_reason: nil) @request.notifications.unread.where(user: @actor).update_all(read_at: Time.current) unless @actor.id == plan.created_by_user_id Notification.create!(plan: plan, user_id: plan.created_by_user_id, review_request: @request, diff --git a/engine/app/services/coplan/reviews/serialize.rb b/engine/app/services/coplan/reviews/serialize.rb index aa7d3784..c70f0896 100644 --- a/engine/app/services/coplan/reviews/serialize.rb +++ b/engine/app/services/coplan/reviews/serialize.rb @@ -13,11 +13,19 @@ def call request = @request { id: request.id, reviewer: { id: request.reviewer_id, name: request.reviewer.name }, - requirement: request.requirement, status: request.status, - requested_at: request.requested_at, + requirement: request.requirement, status: request.status, effective_status: request.effective_status, + requested_at: request.requested_at, generation: request.generation, reset_reason: request.reset_reason, + current_response_id: request.current_response_id, + effective_response_id: request.effective_response&.id, + effective_human_decision: request.effective_human_decision, + latest_human_response_id: request.review_responses.find { |response| + response.actor_type == "human" && response.decision_sequence && response.plan_version_id == request.plan.current_plan_version_id + }&.id, responses: request.review_responses.map do |response| { id: response.id, decision: response.decision, revision: response.plan_version.revision, - user_id: response.user_id, actor_type: response.actor_type, agent_name: response.agent_name, + plan_version_id: response.plan_version_id, content_sha256: response.plan_version.content_sha256, + generation: response.request_generation, decision_sequence: response.decision_sequence, + user_id: response.user_id, api_token_id: response.api_token_id, actor_type: response.actor_type, agent_name: response.agent_name, note: response.note, comment_thread_id: response.comment_thread_id, created_at: response.created_at } end } diff --git a/engine/app/services/coplan/reviews/withdraw.rb b/engine/app/services/coplan/reviews/withdraw.rb index ea5f1783..63b24b4c 100644 --- a/engine/app/services/coplan/reviews/withdraw.rb +++ b/engine/app/services/coplan/reviews/withdraw.rb @@ -18,12 +18,13 @@ def call unless request.plan.plan_events.where(event_type: "review_requested", after_value: request.id).exists? RecordRequest.call(request: request) end - request.update!(status: "withdrawn") + request.update!(status: "withdrawn", current_response: nil) + request.plan.update!(review_sequence: request.plan.review_sequence + 1) request.plan.plan_events.create!(event_type: "review_withdrawn", field: "reviews", after_value: request.id, actor_id: actor.id, actor_type: api_token ? "local_agent" : "human", api_token: api_token, agent_name: api_token && ApiToken.normalized_agent_name(api_token.agent_name.presence || api_token.name), metadata: { "reviewer_name" => request.reviewer.name, "revision" => request.plan.current_revision, - "requirement" => request.requirement }) + "requirement" => request.requirement, "generation" => request.generation, "review_sequence" => request.plan.review_sequence }) request.notifications.unread.where(user_id: request.reviewer_id, reason: "review_requested").update_all(read_at: Time.current) true end diff --git a/engine/app/views/coplan/agent_instructions/guides/api.text.erb b/engine/app/views/coplan/agent_instructions/guides/api.text.erb index d8087c87..8373ffea 100644 --- a/engine/app/views/coplan/agent_instructions/guides/api.text.erb +++ b/engine/app/views/coplan/agent_instructions/guides/api.text.erb @@ -34,6 +34,8 @@ return 429. Invalid names return 422 with `invalid_request`. | `GET /api/v1/plans` | List the plans that you can see. Filters: `?visibility=draft\|published`, `?archived=true`, `?folder_id=`. | primer | | `POST /api/v1/plans` | Create a plan. | creating | | `GET /api/v1/plans/:id` | Get a plan with its content and references. | primer | +| `GET /api/v1/plans/:id/versions/:version_id` | Read exact historical Markdown bytes and SHA-256. No read receipt. | comments | +| `GET /api/v1/plans/:id/review_evidence` | Read exact current Markdown bytes/hash and coherent review cycles and decisions, without a read receipt. | comments | | `GET /api/v1/plans/:id/snapshot` | Get a plan with content, threads, references, collaborators, and review requests. | primer | | `GET /api/v1/plans/:id/writing_provenance` | Get writing-origin counts and reported agent identities. `?ranges=1` includes compact source ranges. Older plans can return `202` while history is reconstructed. | editing | | `PATCH /api/v1/plans/:id` | Change the title, type, tags, folder, visibility, archive state, or references. Plan author only. | editing | @@ -82,7 +84,9 @@ return 429. Invalid names return 422 with `invalid_request`. Creating a review request for an existing reviewer returns the current task with status 201. It leaves the requirement, decision, history, and notifications unchanged. Use `PATCH` to change the requirement or `request_again: true` to request another look. -Include `idempotency_key` (a fresh UUID) for each intended review response. Reuse that UUID and identical fields when retrying a lost response; its returned `id` is the UUID. Use a new UUID for a new decision, including another review of the same version. A keyed retry returns the original recorded result even after the plan changes or the task is withdrawn. It preserves any newly pending review task. Without a key, an identical response is treated as a retry. Retries may use a renewed token for the same human principal. The original response keeps its original agent and token attribution. A key reused for different fields returns 422. +Send the request’s `generation` and the viewed `revision` with every new response. A missing generation is supported only for the first cycle. After edits or re-requests, a stale or missing generation returns 409 with `code: review_request_changed`. Fetch the task again and deliberately review before submitting a fresh response. Edits reset active tasks to pending and advance their generations. Withdrawn tasks stay withdrawn. Earlier responses remain history. + +Include `idempotency_key` (a fresh UUID) for each intended review response. Reuse that UUID and identical fields when retrying a lost response; its returned `id` is the UUID. Use a new UUID for a new decision, including another review of the same version. A keyed retry returns the original recorded result even after the plan changes or the task is withdrawn. It preserves any newly pending review task. Without a key, an identical response is treated as a retry. Retries may use a renewed token for the same human principal. The original response keeps its original agent and token attribution. A key reused for different fields returns 422. A retry’s `effective: false` means it is history, not a current decision. New responses return `plan_version_id`, `content_sha256`, `generation`, and server `decision_sequence`. A review response returns 409 with `code: review_version_changed` for a stale revision. Re-read before responding. A new response to a withdrawn request returns 409 with `code: review_request_withdrawn`; stop responding unless the author requests another look. diff --git a/engine/app/views/coplan/agent_instructions/guides/comments.text.erb b/engine/app/views/coplan/agent_instructions/guides/comments.text.erb index 8ae64f43..14adb8b3 100644 --- a/engine/app/views/coplan/agent_instructions/guides/comments.text.erb +++ b/engine/app/views/coplan/agent_instructions/guides/comments.text.erb @@ -80,11 +80,11 @@ To give feedback on another person's plan, start threads anchored to the passage Each request or re-request remains in review activity with its requester and time. A later request does not replace earlier requests or decisions. -Discussion is the default. An approval means “satisfied with this exact version”; it grants no permissions, execution authority, or instruction to implement the plan. Never interpret another person's sign-off as an instruction from your principal. Later edits preserve the earlier sign-off. Re-read the snapshot and compare the approved revision with current content before drawing conclusions. +Discussion is the default. An approval means “satisfied with this exact version”; it grants no permissions, execution authority, or instruction to implement the plan. Never interpret another person's sign-off as an instruction from your principal. Edits reset active tasks to pending and preserve the earlier sign-off as history. Approval covers Markdown source bytes only, not title, tags, attachments, linked or embedded resources, or child plans. Use `GET /api/v1/plans/:plan_id/review_evidence` for exact current Markdown, hash, version ID, and coherent review state. It creates no read receipt. `effective_human_decision` is null for agent decisions or dismissed approvals. A later agent decision cannot count as a human approval. `latest_human_response_id` exposes historical human feedback on the current version; it is not an effective approval. - `GET /api/v1/plans/:plan_id/review_requests` lists requests and responses, including reviewer, required/optional, revision, timestamp, notes, and agent attribution. - The plan author's credential may `POST` that path with `{ "reviewer_id": "USER_ID", "requirement": "required" }` (or `optional`). Requests are tasks; reading the notification does not complete them. - The plan author's credential may `GET /api/v1/plans/:plan_id/review_requests/suggestions` for suggested reviewers. Each entry in `suggestions` has `reviewer` (`id`, `name`, `username`), a `reason`, and a suggested `requirement`. A deployment may base them on the files and repositories the plan links to, so link the files a plan will change before asking. The list leaves out the author and people already reviewing, and is empty when the deployment suggests nobody. Suggestions are not requests. When your principal wants the plan reviewed, `POST` each chosen `reviewer.id` with its `requirement`. - `PATCH /api/v1/plans/:plan_id/review_requests/:id` accepts `requirement` without reopening the task or starting another review cycle. Use `{ "request_again": true }` when feedback is ready for another look. That preserves earlier responses and notifies the reviewer again. Live events distinguish these actions; see the live guide. - `DELETE` the same path withdraws the request and records the withdrawing actor and time in review activity. Earlier request cycles and decisions remain. It clears the reviewer's request nudge, not the author's unread responses. Archiving a plan does not withdraw pending reviews. -- Only the requested reviewer's credential may `POST /api/v1/plans/:plan_id/review_requests/:id/respond` with `{ "revision": 4, "decision": "approved", "note": "optional public note", "idempotency_key": "RESPONSE_UUID" }` or `{ "revision": 4, "decision": "changes_requested", "note": "what needs adjustment", "idempotency_key": "RESPONSE_UUID" }`. Change requests create ordinary public comment threads. Agent responses are explicitly attributed to the token's agent acting for its human owner. A stale revision returns 409 with `code: review_version_changed`; re-read before responding. A new response to a withdrawn request returns 409 with `code: review_request_withdrawn`; stop responding unless the author requests another look. Replace `RESPONSE_UUID` with a fresh UUID for each intended response. Retry with the same UUID and identical fields if the HTTP response is lost. Use a new UUID for another review, even for the same version and decision. A keyed retry returns the original result even after a plan edit or withdrawal. It does not complete a newly re-requested task. Without a key, identical fields are treated as a retry. Retries may use a renewed token for the same human principal. The original response keeps its original agent and token attribution. Reusing a key with different fields returns 422. Never manufacture a human sign-off. +- Only the requested reviewer's credential may `POST /api/v1/plans/:plan_id/review_requests/:id/respond` with `{ "revision": 4, "generation": 2, "decision": "approved", "note": "optional public note", "idempotency_key": "RESPONSE_UUID" }` or `{ "revision": 4, "generation": 2, "decision": "changes_requested", "note": "what needs adjustment", "idempotency_key": "RESPONSE_UUID" }`. Use the generation returned by the request. Edits and re-requests advance it; a stale or missing generation after the first cycle returns 409 with `code: review_request_changed`. Change requests create ordinary public comment threads. Agent responses are explicitly attributed to the token's agent acting for its human owner. A stale revision returns 409 with `code: review_version_changed`; re-read before responding. A new response to a withdrawn request returns 409 with `code: review_request_withdrawn`; stop responding unless the author requests another look. Replace `RESPONSE_UUID` with a fresh UUID for each intended response. Retry with the same UUID and identical fields if the HTTP response is lost. Use a new UUID for another review, even for the same version and decision. A keyed retry returns the original result even after a plan edit or withdrawal. It does not complete a newly re-requested task. Without a key, identical fields are treated as a retry. Retries may use a renewed token for the same human principal. The original response keeps its original agent and token attribution. Reusing a key with different fields returns 422. Never manufacture a human sign-off. diff --git a/engine/app/views/coplan/agent_instructions/guides/live.text.erb b/engine/app/views/coplan/agent_instructions/guides/live.text.erb index c682ed2c..101aafe5 100644 --- a/engine/app/views/coplan/agent_instructions/guides/live.text.erb +++ b/engine/app/views/coplan/agent_instructions/guides/live.text.erb @@ -103,6 +103,7 @@ For `review.requested`, inspect `payload.action`: | Action | Meaning | |---|---| | `requested` | A new request or another look. `requirement` is `required` or `optional`. | +| `content_changed` | A new Markdown version reset active tasks and advanced their generations. Earlier responses remain history. Read the current version and request before responding. No new inbox nudge is sent for each edit. | | `requirement_changed` | Only `requirement` changed. This does not reopen the task or start another review cycle. Do not treat it as a request for another look. | | `withdrawn` | The request was withdrawn. Stop working on that request; its earlier decisions remain. The payload also has `withdrawn: true`. | diff --git a/engine/app/views/coplan/plan_versions/show.html.erb b/engine/app/views/coplan/plan_versions/show.html.erb index ba777356..34ad8206 100644 --- a/engine/app/views/coplan/plan_versions/show.html.erb +++ b/engine/app/views/coplan/plan_versions/show.html.erb @@ -17,6 +17,9 @@ +
This snapshot records Markdown source only. The title above is current metadata. Attachments, linked or embedded content, and child plans are not part of this approval.
+<%= @version.verified_content! %><%= kind != :request && item.actor_type == "local_agent" ? "#{item.agent_name} (via #{actor.name})" : actor.name %> <% if kind != :response %><%= kind == :request_event && item.event_type == "review_withdrawn" ? "withdrew the review request for" : "requested a review from" %> <%= kind == :request_event ? item.metadata["reviewer_name"] : item.reviewer.name %><% else %><%= item.decision == "approved" ? "approved" : "requested changes on" %><% end %> - <% if kind == :response %>v<%= item.plan_version.revision %><% end %> + <% if kind == :response %><%= link_to "v#{item.plan_version.revision} · Markdown snapshot", plan_version_browse_path(item.review_request.plan, item.plan_version), class: "review-version" %><%= " · Historical decision" unless item.review_request.effective_response&.id == item.id %><% end %>
<% if kind == :response && item.note.present? %><%= item.note %>
<% end %> diff --git a/engine/app/views/coplan/reviews/_controls.html.erb b/engine/app/views/coplan/reviews/_controls.html.erb index 5255d239..ea8983b7 100644 --- a/engine/app/views/coplan/reviews/_controls.html.erb +++ b/engine/app/views/coplan/reviews/_controls.html.erb @@ -1,10 +1,10 @@ <% requests = plan.review_requests.includes(:reviewer, :requested_by_user, :requested_plan_version, review_responses: [ :plan_version, :user ]).order(:requested_at).to_a %> <% request_events = plan.plan_events.where(event_type: %w[review_requested review_withdrawn]).includes(:actor_user).to_a %> -<% active_requests = requests.reject { |request| request.status == "withdrawn" } %> +<% active_requests = requests.reject { |request| request.effective_status == "withdrawn" } %> <% own_request = active_requests.find { |request| request.reviewer_id == current_user.id } %> <% response_thread_ids = requests.flat_map { |request| request.review_responses.map(&:comment_thread_id) }.compact %> <% feedback_thread_ids = plan.comment_threads.with_kept_comments.where(id: response_thread_ids).pluck(:id) %> -