From fe6b78fa59657449be2e888402e3a0197b5c0621 Mon Sep 17 00:00:00 2001 From: soryu Date: Thu, 30 Apr 2026 17:08:30 +0100 Subject: feat(directives): per-PR revision snapshots + sidebar history (#112) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 3 of the doc-mode revamp. Builds the foundation for treating contracts as living specifications by freezing their content into a revision every time a PR is raised. ## directive_revisions table (new migration) (id, directive_id, content, pr_url, pr_branch, pr_state, version, frozen_at) with UNIQUE(directive_id, version) and a partial index on pr_state='open' so the next reconciler iteration can poll only what's still in flight. pr_state is constrained to 'open' | 'merged' | 'closed' to mirror GitHub's PR lifecycle. For Stage 3 we only freeze on PR creation; pr_state poll is deferred to a follow-up. ## Repository helpers - create_directive_revision: idempotent on (directive_id, pr_url) so a re-run of the orchestrator's completion task can't double-snapshot. Auto-assigns version = MAX(existing) + 1 per directive. - list_directive_revisions_for_owner: scoped through the directive join so users can only read their own contract history. - update_directive_revision_pr_state: stub for the upcoming poller. - get_latest_merged_revision: returns the most recent merged revision — this is what Stage 4 will diff against on amendments. ## Snapshot trigger update_directive handler now reads the BEFORE pr_url before the update. If pr_url transitions None → Some, it snapshots the directive's current goal as a revision tied to the new pr_url. Failures log and continue — the directive update itself is unaffected. ## API + OpenAPI GET /api/v1/directives/{id}/revisions returns DirectiveRevisionListResponse (revisions newest-first). Schemas registered in OpenAPI. ## Frontend: revisions/ subfolder + read-only viewer Each contract folder now has a third subfolder ("revisions/") that lazily fetches and lists past revisions when the parent directive folder is open. Empty contracts skip the subfolder entirely so brand-new ones aren't cluttered. Each row shows v.md plus a small pill ('open'/'merged'/ 'closed'). Selecting a revision encodes itself into the existing ?task= param as "revision:", so EditorShell can route between the live task stream (realTaskId), the read-only RevisionViewer (revisionId), or the editor itself (neither). The viewer renders the frozen markdown verbatim with a deep-link to the PR — these are immutable historical records, not edit surfaces. Co-authored-by: Claude Opus 4.7 (1M context) --- makima/frontend/src/lib/api.ts | 34 ++++ makima/frontend/src/routes/document-directives.tsx | 216 ++++++++++++++++++++- .../20260430000000_create_directive_revisions.sql | 35 ++++ makima/src/db/models.rs | 16 ++ makima/src/db/repository.rs | 118 +++++++++++ makima/src/server/handlers/directives.rs | 100 +++++++++- makima/src/server/mod.rs | 1 + makima/src/server/openapi.rs | 5 +- 8 files changed, 514 insertions(+), 11 deletions(-) create mode 100644 makima/migrations/20260430000000_create_directive_revisions.sql diff --git a/makima/frontend/src/lib/api.ts b/makima/frontend/src/lib/api.ts index 8896f2c..e3dbc30 100644 --- a/makima/frontend/src/lib/api.ts +++ b/makima/frontend/src/lib/api.ts @@ -3451,6 +3451,40 @@ export interface PickUpOrdersResponse { taskId: string | null; } +/** + * Per-PR snapshot of a directive's goal — frozen at PR creation, lifecycle + * tracked alongside the PR itself. + */ +export interface DirectiveRevision { + id: string; + directiveId: string; + /** Inline-markdown content of the directive goal at the moment the PR was raised. */ + content: string; + prUrl: string; + prBranch: string | null; + /** "open" | "merged" | "closed" — tracks the PR lifecycle. */ + prState: string; + version: number; + frozenAt: string; +} + +export interface DirectiveRevisionListResponse { + revisions: DirectiveRevision[]; + total: number; +} + +export async function listDirectiveRevisions( + directiveId: string, +): Promise { + const res = await authFetch( + `${API_BASE}/api/v1/directives/${directiveId}/revisions`, + ); + if (!res.ok) { + throw new Error(`Failed to list revisions: ${res.statusText}`); + } + return res.json(); +} + export async function createDirectivePR(id: string): Promise { const res = await authFetch(`${API_BASE}/api/v1/directives/${id}/create-pr`, { method: "POST" }); if (!res.ok) throw new Error(`Failed to create PR: ${res.statusText}`); diff --git a/makima/frontend/src/routes/document-directives.tsx b/makima/frontend/src/routes/document-directives.tsx index ada8a3d..87102a2 100644 --- a/makima/frontend/src/routes/document-directives.tsx +++ b/makima/frontend/src/routes/document-directives.tsx @@ -16,11 +16,13 @@ import { failDirectiveStep, skipDirectiveStep, stopTask, + listDirectiveRevisions, } from "../lib/api"; import type { DirectiveStatus, DirectiveSummary, DirectiveWithSteps, + DirectiveRevision, } from "../lib/api"; // Status dot color, matching the existing tabular UI's badge palette so the @@ -365,6 +367,27 @@ function DirectiveFolder({ const orchestratorRunning = !!directive.orchestratorTaskId; // Tasks subfolder open state — independent of the directive folder. const [tasksOpen, setTasksOpen] = useState(true); + // Revisions subfolder — collapsed by default since most contracts have + // no merged history yet. + const [revisionsOpen, setRevisionsOpen] = useState(false); + const [revisions, setRevisions] = useState([]); + // Fetch revisions only when the parent folder is open. Re-fetch whenever + // the directive's pr_url changes so a freshly-raised PR appears. + useEffect(() => { + if (!open) return; + let cancelled = false; + listDirectiveRevisions(directive.id) + .then((res) => { + if (!cancelled) setRevisions(res.revisions); + }) + .catch((err) => { + // eslint-disable-next-line no-console + console.warn("[makima] failed to load revisions", err); + }); + return () => { + cancelled = true; + }; + }, [open, directive.id, directive.prUrl]); return (
@@ -477,12 +500,176 @@ function DirectiveFolder({ )} + + {/* revisions/ subfolder — per-PR frozen snapshots of this contract. + Only rendered when there's at least one revision; otherwise the + folder body would be a confusing empty placeholder. */} + {revisions.length > 0 && ( +
  • + + + {revisionsOpen && ( +
      + {revisions.map((r) => { + const isSelected = + selection?.directiveId === directive.id && + selection?.taskId === `revision:${r.id}`; + return ( +
    • + +
      • + ); + })} +
    + )} +
    • + )} )}
    ); } +/** + * Read-only viewer for a frozen contract revision. We render the markdown as + * plain pre-formatted text — these are immutable historical records, not + * places to edit. A header strip shows the PR state and a deep link. + */ +function RevisionViewer({ + directiveId, + revisionId, +}: { + directiveId: string; + revisionId: string; +}) { + const [revision, setRevision] = useState(null); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + + useEffect(() => { + let cancelled = false; + setLoading(true); + setError(null); + listDirectiveRevisions(directiveId) + .then((res) => { + if (cancelled) return; + const found = res.revisions.find((r) => r.id === revisionId) ?? null; + if (!found) setError("Revision not found"); + setRevision(found); + }) + .catch((err) => { + if (cancelled) return; + setError(err instanceof Error ? err.message : String(err)); + }) + .finally(() => { + if (!cancelled) setLoading(false); + }); + return () => { + cancelled = true; + }; + }, [directiveId, revisionId]); + + if (loading) { + return ( +
    +

    Loading revision…

    +
    + ); + } + if (error || !revision) { + return ( +
    +

    + {error ?? "Revision not found"} +

    +
    + ); + } + + return ( +
    +
    +
    +
    +

    + v{revision.version} +

    + +
    +

    + Frozen {new Date(revision.frozenAt).toLocaleString()} +

    +

    + + {revision.prUrl} + +

    + + {/* Render the frozen markdown as plain pre-formatted text. We + deliberately do not parse it into rich nodes — the goal is to + show the historical record exactly as it was at PR time. */} + 
    +            {revision.content}
+
    +
    +
    +
    + ); +} + +/** Tiny pill showing the PR state of a revision (open / merged / closed). */ +function RevisionStateBadge({ prState }: { prState: string }) { + const tone = + prState === "merged" + ? "text-emerald-300 border-emerald-700/60" + : prState === "closed" + ? "text-[#7788aa] border-[#2a3a5a]" + : "text-amber-300 border-amber-600/40"; + return ( + + {prState} + + ); +} + /** * Right-side status indicator. Composes the colored status dot with optional * "live" pulse (orchestrator running) and "glow" attention ring (pending user @@ -760,14 +947,25 @@ function EditorShell({ ); } + // The "task" param can encode either a real task id, or a revision via the + // `revision:` prefix. Split that out so the right pane can switch + // between the live task stream and the read-only revision viewer. + const revisionId = + selectedTaskId && selectedTaskId.startsWith("revision:") + ? selectedTaskId.slice("revision:".length) + : null; + const realTaskId = revisionId ? null : selectedTaskId; + // Resolve the label for the breadcrumb when a task is selected. - const taskLabel = selectedTaskId - ? selectedTaskId === directive.orchestratorTaskId + const taskLabel = realTaskId + ? realTaskId === directive.orchestratorTaskId ? "orchestrator" - : selectedTaskId === directive.completionTaskId + : realTaskId === directive.completionTaskId ? "completion" - : directive.steps.find((s) => s.taskId === selectedTaskId)?.name ?? - selectedTaskId.slice(0, 8) + : directive.steps.find((s) => s.taskId === realTaskId)?.name ?? + realTaskId.slice(0, 8) + : revisionId + ? "revision" : null; return ( @@ -800,10 +998,12 @@ function EditorShell({ - {selectedTaskId ? ( + {revisionId ? ( + + ) : realTaskId ? ( ) : ( , } +/// Per-PR snapshot of a directive's goal — the immutable record of what the +/// contract said at the moment a PR was raised. Frozen at PR-creation time; +/// `pr_state` mirrors the PR's GitHub lifecycle ('open' | 'merged' | 'closed'). +#[derive(Debug, Clone, FromRow, Serialize, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct DirectiveRevision { + pub id: Uuid, + pub directive_id: Uuid, + pub content: String, + pub pr_url: String, + pub pr_branch: Option, + pub pr_state: String, + pub version: i32, + pub frozen_at: DateTime, +} + /// A step in a directive's DAG. #[derive(Debug, Clone, FromRow, Serialize, Deserialize, ToSchema)] #[serde(rename_all = "camelCase")] diff --git a/makima/src/db/repository.rs b/makima/src/db/repository.rs index cec9a82..1021c35 100644 --- a/makima/src/db/repository.rs +++ b/makima/src/db/repository.rs @@ -5691,6 +5691,124 @@ pub async fn update_directive_goal_keep_orchestrator( .await } +// ============================================================================= +// Directive Revisions — per-PR snapshots of the contract content. +// ============================================================================= + +/// Snapshot the directive's current goal as a revision attached to the given +/// PR URL. The version is auto-assigned as MAX(existing) + 1 per directive. +/// Idempotent on (directive_id, pr_url): if a revision already exists for +/// this directive+pr_url combo, returns the existing row instead of creating +/// a duplicate. +pub async fn create_directive_revision( + pool: &PgPool, + directive_id: Uuid, + content: &str, + pr_url: &str, + pr_branch: Option<&str>, +) -> Result { + // Idempotency: don't double-snapshot if the orchestrator's completion task + // re-runs and re-sets the same pr_url. + if let Some(existing) = sqlx::query_as::<_, crate::db::models::DirectiveRevision>( + r#" + SELECT * FROM directive_revisions + WHERE directive_id = $1 AND pr_url = $2 + ORDER BY frozen_at DESC LIMIT 1 + "#, + ) + .bind(directive_id) + .bind(pr_url) + .fetch_optional(pool) + .await? + { + return Ok(existing); + } + + sqlx::query_as::<_, crate::db::models::DirectiveRevision>( + r#" + INSERT INTO directive_revisions + (directive_id, content, pr_url, pr_branch, pr_state, version, frozen_at) + SELECT + $1, + $2, + $3, + $4, + 'open', + COALESCE(MAX(version), 0) + 1, + NOW() + FROM directive_revisions + WHERE directive_id = $1 + RETURNING * + "#, + ) + .bind(directive_id) + .bind(content) + .bind(pr_url) + .bind(pr_branch) + .fetch_one(pool) + .await +} + +/// List all revisions for a directive, newest first. Scoped by owner via the +/// directive join so callers don't accidentally surface other users' history. +pub async fn list_directive_revisions_for_owner( + pool: &PgPool, + owner_id: Uuid, + directive_id: Uuid, +) -> Result, sqlx::Error> { + sqlx::query_as::<_, crate::db::models::DirectiveRevision>( + r#" + SELECT r.* + FROM directive_revisions r + JOIN directives d ON d.id = r.directive_id + WHERE r.directive_id = $1 AND d.owner_id = $2 + ORDER BY r.frozen_at DESC + "#, + ) + .bind(directive_id) + .bind(owner_id) + .fetch_all(pool) + .await +} + +/// Update the pr_state on a revision (called by the reconciler when it +/// detects a PR transitioned to merged/closed). New state must be one of +/// 'open' | 'merged' | 'closed' to satisfy the table's CHECK constraint. +pub async fn update_directive_revision_pr_state( + pool: &PgPool, + revision_id: Uuid, + new_state: &str, +) -> Result<(), sqlx::Error> { + sqlx::query( + r#"UPDATE directive_revisions SET pr_state = $2 WHERE id = $1"#, + ) + .bind(revision_id) + .bind(new_state) + .execute(pool) + .await?; + Ok(()) +} + +/// Find the most recent merged revision for a directive — used when planning +/// an amendment to know what the previous "frozen" content was so the diff +/// can be passed to the orchestrator. +pub async fn get_latest_merged_revision( + pool: &PgPool, + directive_id: Uuid, +) -> Result, sqlx::Error> { + sqlx::query_as::<_, crate::db::models::DirectiveRevision>( + r#" + SELECT * FROM directive_revisions + WHERE directive_id = $1 AND pr_state = 'merged' + ORDER BY frozen_at DESC + LIMIT 1 + "#, + ) + .bind(directive_id) + .fetch_optional(pool) + .await +} + /// Save a goal to the directive goal history. pub async fn save_directive_goal_history( pool: &PgPool, diff --git a/makima/src/server/handlers/directives.rs b/makima/src/server/handlers/directives.rs index 44bf4ac..91f5892 100644 --- a/makima/src/server/handlers/directives.rs +++ b/makima/src/server/handlers/directives.rs @@ -11,12 +11,14 @@ use uuid::Uuid; use crate::db::models::{ CleanupResponse, CreateDirectiveRequest, CreateTaskRequest, CreateDirectiveStepRequest, Directive, DirectiveListResponse, - DirectiveStep, DirectiveWithSteps, PickUpOrdersResponse, + DirectiveRevision, DirectiveStep, DirectiveWithSteps, PickUpOrdersResponse, UpdateDirectiveRequest, UpdateDirectiveStepRequest, UpdateGoalRequest, CreateDirectiveOrderGroupRequest, DirectiveOrderGroup, DirectiveOrderGroupListResponse, UpdateDirectiveOrderGroupRequest, OrderListResponse, }; +use serde::Serialize; +use utoipa::ToSchema; use crate::db::repository; use crate::orchestration::directive::{ build_cleanup_prompt, build_order_pickup_prompt, classify_goal_change, @@ -185,8 +187,50 @@ pub async fn update_directive( .into_response(); }; + // Capture the BEFORE state so we can detect a pr_url transition (null + // → some-value), which is when the orchestrator's completion task has + // raised a PR for this directive. That transition is the trigger for + // freezing a directive_revisions snapshot. + let before_pr_url = match repository::get_directive_for_owner(pool, auth.owner_id, id).await { + Ok(Some(d)) => d.pr_url.clone(), + _ => None, + }; + match repository::update_directive_for_owner(pool, auth.owner_id, id, req).await { - Ok(Some(directive)) => Json(directive).into_response(), + Ok(Some(directive)) => { + // Detect "PR was just raised" — pr_url went from None to Some. + // Snapshot the current goal as a revision tied to this PR. + // Best-effort: a snapshot failure should not fail the update, + // because the directive's pr_url has already been written. + if before_pr_url.is_none() { + if let Some(ref new_pr_url) = directive.pr_url { + if let Err(e) = repository::create_directive_revision( + pool, + directive.id, + &directive.goal, + new_pr_url, + directive.pr_branch.as_deref(), + ) + .await + { + tracing::warn!( + directive_id = %directive.id, + pr_url = %new_pr_url, + error = %e, + "Failed to snapshot directive revision on PR creation — \ + continuing; revision history will be incomplete" + ); + } else { + tracing::info!( + directive_id = %directive.id, + pr_url = %new_pr_url, + "Snapshotted directive revision on PR creation" + ); + } + } + } + Json(directive).into_response() + } Ok(None) => ( StatusCode::NOT_FOUND, Json(ApiError::new("NOT_FOUND", "Directive not found")), @@ -2038,3 +2082,55 @@ pub async fn pick_up_dog_orders( }) .into_response() } + +// ============================================================================= +// Directive Revisions (per-PR snapshots) +// ============================================================================= + +#[derive(Debug, Serialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct DirectiveRevisionListResponse { + pub revisions: Vec, + pub total: i64, +} + +/// List all per-PR revisions for a directive, newest first. +#[utoipa::path( + get, + path = "/api/v1/directives/{id}/revisions", + params(("id" = Uuid, Path, description = "Directive ID")), + responses( + (status = 200, description = "Revision history", body = DirectiveRevisionListResponse), + (status = 503, description = "Database not configured", body = ApiError), + ), + security(("bearer_auth" = []), ("api_key" = [])), + tag = "Directives" +)] +pub async fn list_directive_revisions( + State(state): State, + Authenticated(auth): Authenticated, + Path(id): Path, +) -> impl IntoResponse { + let Some(ref pool) = state.db_pool else { + return ( + StatusCode::SERVICE_UNAVAILABLE, + Json(ApiError::new("DB_UNAVAILABLE", "Database not configured")), + ) + .into_response(); + }; + + match repository::list_directive_revisions_for_owner(pool, auth.owner_id, id).await { + Ok(revisions) => { + let total = revisions.len() as i64; + Json(DirectiveRevisionListResponse { revisions, total }).into_response() + } + Err(e) => { + tracing::error!("Failed to list directive revisions: {}", e); + ( + StatusCode::INTERNAL_SERVER_ERROR, + Json(ApiError::new("LIST_FAILED", &e.to_string())), + ) + .into_response() + } + } +} diff --git a/makima/src/server/mod.rs b/makima/src/server/mod.rs index 1c1ed6e..31052bf 100644 --- a/makima/src/server/mod.rs +++ b/makima/src/server/mod.rs @@ -253,6 +253,7 @@ pub fn make_router(state: SharedState) -> Router { .route("/directives/{id}/steps/{step_id}/fail", post(directives::fail_step)) .route("/directives/{id}/steps/{step_id}/skip", post(directives::skip_step)) .route("/directives/{id}/goal", put(directives::update_goal)) + .route("/directives/{id}/revisions", get(directives::list_directive_revisions)) .route("/directives/{id}/cleanup", post(directives::cleanup_directive)) .route("/directives/{id}/create-pr", post(directives::create_pr)) .route("/directives/{id}/pick-up-orders", post(directives::pick_up_orders)) diff --git a/makima/src/server/openapi.rs b/makima/src/server/openapi.rs index 37cd113..e3ff757 100644 --- a/makima/src/server/openapi.rs +++ b/makima/src/server/openapi.rs @@ -13,7 +13,7 @@ use crate::db::models::{ CreateManagedRepositoryRequest, CreateOrderRequest, CreateTaskRequest, Daemon, DaemonDirectoriesResponse, DaemonDirectory, DaemonListResponse, Directive, DirectiveListResponse, - DirectiveStep, DirectiveSummary, DirectiveWithSteps, + DirectiveRevision, DirectiveStep, DirectiveSummary, DirectiveWithSteps, File, FileListResponse, FileSummary, LinkDirectiveRequest, MergeCommitRequest, MergeCompleteCheckResponse, MergeResolveRequest, MergeResultResponse, @@ -130,6 +130,7 @@ use crate::server::messages::{ApiError, AudioEncoding, StartMessage, StopMessage directives::fail_step, directives::skip_step, directives::update_goal, + directives::list_directive_revisions, directives::cleanup_directive, directives::create_pr, // Order endpoints @@ -233,6 +234,8 @@ use crate::server::messages::{ApiError, AudioEncoding, StartMessage, StopMessage DirectiveWithSteps, DirectiveSummary, DirectiveListResponse, + DirectiveRevision, + crate::server::handlers::directives::DirectiveRevisionListResponse, CreateDirectiveRequest, UpdateDirectiveRequest, UpdateGoalRequest, -- cgit v1.2.3