diff --git a/packages/cli/migrations/0014_task_objective_backfill.sql b/packages/cli/migrations/0014_task_objective_backfill.sql index 9615e61..5127c0e 100644 --- a/packages/cli/migrations/0014_task_objective_backfill.sql +++ b/packages/cli/migrations/0014_task_objective_backfill.sql @@ -1,8 +1,8 @@ -- Backfill task-level kind='task_objective' artifact rows for every task -- that existed before the shared code-worker prompt composer landed. -- --- The composer's loadOriginalTaskObjective() requires a kind='task_objective' --- artifact with attempt_id IS NULL, written once at enqueue time. Without +-- The composer's loadOriginalTaskObjective() requires at least one +-- kind='task_objective' artifact with attempt_id IS NULL. Without -- this backfill, any pre-existing active task would throw on its next CI / -- crash / stale / wall-clock / malformed retry, on review/conflict respawn, -- or on orchestrator submit-brief. diff --git a/packages/cli/src/admin/api.ts b/packages/cli/src/admin/api.ts index 5e9ac23..2a0cbd1 100644 --- a/packages/cli/src/admin/api.ts +++ b/packages/cli/src/admin/api.ts @@ -877,7 +877,7 @@ function missionControlTaskRows(db: DB): MissionControlTaskRow[] { WHERE ar.task_id = t.task_id AND ar.kind = 'task_objective' AND ar.attempt_id IS NULL - ORDER BY ar.artifact_id ASC + ORDER BY ar.artifact_id DESC LIMIT 1 ) AS objective_file_path, ( @@ -886,7 +886,7 @@ function missionControlTaskRows(db: DB): MissionControlTaskRow[] { WHERE ar.task_id = t.task_id AND ar.kind = 'ticket_snapshot' AND ar.attempt_id IS NULL - ORDER BY ar.artifact_id ASC + ORDER BY ar.artifact_id DESC LIMIT 1 ) AS ticket_snapshot_file_path, CASE diff --git a/packages/cli/src/build/embedded.generated.ts b/packages/cli/src/build/embedded.generated.ts index a2017b0..2dd2a83 100644 --- a/packages/cli/src/build/embedded.generated.ts +++ b/packages/cli/src/build/embedded.generated.ts @@ -3,7 +3,7 @@ import type { Migration } from "../db/migrate.ts"; -export const QUAY_VERSION = "dev+3afbf26+dirty"; +export const QUAY_VERSION = "dev+2623a9f+dirty"; export const EMBEDDED_MIGRATIONS: readonly Migration[] = [ { name: "0001_init.sql", sql: "-- Slice 0 schema: persistence contract for Quay (per quay-spec.md §9).\n-- Foreign keys must be enabled at the connection level (PRAGMA foreign_keys = ON).\n\nCREATE TABLE repos (\n repo_id TEXT PRIMARY KEY,\n repo_url TEXT NOT NULL,\n base_branch TEXT NOT NULL,\n package_manager TEXT NOT NULL,\n install_cmd TEXT NOT NULL,\n test_cmd TEXT,\n ci_workflow_name TEXT,\n contribution_guide_path TEXT,\n archived_at TEXT,\n created_at TEXT NOT NULL\n);\n\nCREATE TABLE preambles (\n preamble_id INTEGER PRIMARY KEY AUTOINCREMENT,\n body TEXT NOT NULL,\n created_at TEXT NOT NULL\n);\n\nCREATE TABLE retry_templates (\n template_id INTEGER PRIMARY KEY AUTOINCREMENT,\n kind TEXT NOT NULL,\n body TEXT NOT NULL,\n created_at TEXT NOT NULL\n);\n\nCREATE TABLE tasks (\n task_id TEXT PRIMARY KEY,\n repo_id TEXT NOT NULL REFERENCES repos(repo_id),\n external_ref TEXT,\n state TEXT NOT NULL,\n branch_name TEXT NOT NULL,\n tmux_id TEXT NOT NULL,\n worktree_path TEXT NOT NULL,\n pr_number INTEGER,\n pr_url TEXT,\n head_sha TEXT,\n base_sha TEXT,\n attempts_consumed INTEGER NOT NULL DEFAULT 0,\n retry_budget INTEGER NOT NULL,\n budget_exhausted INTEGER NOT NULL DEFAULT 0 CHECK (budget_exhausted IN (0, 1)),\n tick_error TEXT,\n slack_thread_ref TEXT,\n claimed_at TEXT,\n claim_id TEXT,\n claim_expirations_consecutive INTEGER NOT NULL DEFAULT 0,\n last_review_id_acted_on TEXT,\n last_conflict_observation TEXT,\n non_budget_respawns_consumed INTEGER NOT NULL DEFAULT 0,\n next_escalation_seq INTEGER NOT NULL DEFAULT 1,\n cancel_requested_at TEXT,\n cancel_close_pr INTEGER NOT NULL DEFAULT 0 CHECK (cancel_close_pr IN (0, 1)),\n cancel_keep_worktree INTEGER NOT NULL DEFAULT 0 CHECK (cancel_keep_worktree IN (0, 1)),\n spawn_failures_consecutive INTEGER NOT NULL DEFAULT 0,\n created_at TEXT NOT NULL,\n updated_at TEXT NOT NULL\n);\n\nCREATE INDEX tasks_state_idx ON tasks(state);\n\nCREATE TABLE attempts (\n attempt_id INTEGER PRIMARY KEY AUTOINCREMENT,\n task_id TEXT NOT NULL REFERENCES tasks(task_id),\n attempt_number INTEGER NOT NULL,\n preamble_id INTEGER NOT NULL REFERENCES preambles(preamble_id),\n template_id INTEGER REFERENCES retry_templates(template_id),\n reason TEXT NOT NULL,\n consumed_budget INTEGER NOT NULL CHECK (consumed_budget IN (0, 1)),\n tmux_session TEXT,\n spawned_at TEXT,\n remote_sha_at_spawn TEXT,\n remote_sha_at_exit TEXT,\n pr_existed_at_spawn INTEGER NOT NULL DEFAULT 0 CHECK (pr_existed_at_spawn IN (0, 1)),\n ended_at TEXT,\n exit_kind TEXT,\n kill_intent TEXT,\n UNIQUE (task_id, attempt_number)\n);\n\nCREATE UNIQUE INDEX one_pending_attempt_per_task\n ON attempts(task_id)\n WHERE spawned_at IS NULL;\n\nCREATE TABLE artifacts (\n artifact_id INTEGER PRIMARY KEY AUTOINCREMENT,\n task_id TEXT NOT NULL REFERENCES tasks(task_id),\n attempt_id INTEGER REFERENCES attempts(attempt_id),\n kind TEXT NOT NULL,\n file_path TEXT NOT NULL,\n content_hash TEXT,\n escalation_seq INTEGER,\n escalation_nonce TEXT,\n slack_pre_post_fence_ts TEXT,\n slack_post_ts TEXT,\n slack_recovered_post_ts TEXT,\n captured_at TEXT NOT NULL\n);\n\nCREATE UNIQUE INDEX artifact_recovery_idempotency\n ON artifacts(task_id, attempt_id, kind, content_hash)\n WHERE content_hash IS NOT NULL AND attempt_id IS NOT NULL;\n\nCREATE INDEX artifacts_task_kind_attempt_idx\n ON artifacts(task_id, kind, attempt_id);\n\nCREATE TABLE events (\n event_id INTEGER PRIMARY KEY AUTOINCREMENT,\n task_id TEXT NOT NULL REFERENCES tasks(task_id),\n attempt_id INTEGER REFERENCES attempts(attempt_id),\n event_type TEXT NOT NULL,\n from_state TEXT,\n to_state TEXT,\n payload_artifact_id INTEGER REFERENCES artifacts(artifact_id),\n occurred_at TEXT NOT NULL\n);\n\nCREATE INDEX events_task_occurred_idx ON events(task_id, occurred_at);\n" }, @@ -19,7 +19,7 @@ export const EMBEDDED_MIGRATIONS: readonly Migration[] = [ { name: "0011_orchestrator_handoffs.sql", sql: "-- Durable orchestrator handoff queue for tasks that enter\n-- awaiting-next-brief and need judgment outside `quay tick`.\n\nCREATE TABLE orchestrator_handoffs (\n handoff_id INTEGER PRIMARY KEY AUTOINCREMENT,\n task_id TEXT NOT NULL REFERENCES tasks(task_id),\n reason TEXT NOT NULL CHECK (\n reason IN (\n 'worker_blocker',\n 'budget_exhausted',\n 'human_reply_ingested',\n 'manual_resume'\n )\n ),\n state_event_id INTEGER NOT NULL REFERENCES events(event_id),\n idempotency_key TEXT NOT NULL,\n payload_json TEXT,\n status TEXT NOT NULL DEFAULT 'pending' CHECK (\n status IN ('pending', 'claimed', 'completed', 'cancelled')\n ),\n claim_id TEXT,\n claimed_at TEXT,\n completed_at TEXT,\n created_at TEXT NOT NULL,\n updated_at TEXT NOT NULL,\n UNIQUE (idempotency_key),\n UNIQUE (task_id, state_event_id, reason)\n);\n\nCREATE INDEX orchestrator_handoffs_status_created_idx\n ON orchestrator_handoffs(status, created_at, handoff_id);\n\nCREATE INDEX orchestrator_handoffs_task_status_idx\n ON orchestrator_handoffs(task_id, status, handoff_id);\n" }, { name: "0012_agent_model_selection.sql", sql: "-- First-class agent/model selection snapshots.\n--\n-- Repo columns are role defaults. Task columns are immutable snapshots taken\n-- at enqueue / synthetic review scheduling time, so later config changes do\n-- not alter already-queued work. Attempt column records the intended model\n-- that was passed to the agent invocation.\n\nALTER TABLE repos ADD COLUMN model_worker TEXT;\nALTER TABLE repos ADD COLUMN model_reviewer TEXT;\n\nALTER TABLE tasks ADD COLUMN worker_agent TEXT;\nALTER TABLE tasks ADD COLUMN worker_model TEXT;\nALTER TABLE tasks ADD COLUMN reviewer_agent TEXT;\nALTER TABLE tasks ADD COLUMN reviewer_model TEXT;\n\nALTER TABLE attempts ADD COLUMN agent_model TEXT;\n" }, { name: "0013_review_requests.sql", sql: "-- Durable review enrollment queue consumed by tick.\nCREATE TABLE review_requests (\n request_id INTEGER PRIMARY KEY AUTOINCREMENT,\n task_id TEXT NOT NULL REFERENCES tasks(task_id),\n repo_id TEXT NOT NULL REFERENCES repos(repo_id),\n pr_number INTEGER NOT NULL,\n head_sha TEXT NOT NULL,\n source TEXT NOT NULL DEFAULT 'review-pr',\n requested_by TEXT,\n delivery_id TEXT,\n tags_json TEXT,\n reviewer_agent TEXT,\n reviewer_model TEXT,\n status TEXT NOT NULL CHECK (\n status IN ('pending_ci', 'scheduled', 'superseded', 'discarded_terminal')\n ),\n scheduled_attempt_id INTEGER REFERENCES attempts(attempt_id),\n superseded_by_request_id INTEGER REFERENCES review_requests(request_id),\n created_at TEXT NOT NULL,\n updated_at TEXT NOT NULL,\n terminal_state TEXT\n);\n\nCREATE UNIQUE INDEX review_requests_unique_head\n ON review_requests(task_id, head_sha);\n\nCREATE INDEX review_requests_pending_idx\n ON review_requests(status, repo_id, pr_number, created_at);\n" }, - { name: "0014_task_objective_backfill.sql", sql: "-- Backfill task-level kind='task_objective' artifact rows for every task\n-- that existed before the shared code-worker prompt composer landed.\n--\n-- The composer's loadOriginalTaskObjective() requires a kind='task_objective'\n-- artifact with attempt_id IS NULL, written once at enqueue time. Without\n-- this backfill, any pre-existing active task would throw on its next CI /\n-- crash / stale / wall-clock / malformed retry, on review/conflict respawn,\n-- or on orchestrator submit-brief.\n--\n-- For legacy tasks, the raw original brief lives in the first attempt's\n-- (`attempt_number=1`, `reason='initial'`) brief artifact. The backfilled\n-- row points at the same on-disk file and copies the content_hash — no file\n-- writes are required. The `artifact_recovery_idempotency` unique index\n-- excludes `attempt_id IS NULL`, so the new task-level row never collides\n-- with the per-attempt brief it shadows.\n--\n-- The NOT EXISTS clause makes this migration safe to re-run.\n\nINSERT INTO artifacts (task_id, attempt_id, kind, file_path, content_hash, captured_at)\nSELECT\n ar.task_id,\n NULL,\n 'task_objective',\n ar.file_path,\n ar.content_hash,\n strftime('%Y-%m-%dT%H:%M:%fZ', 'now')\nFROM artifacts ar\nJOIN attempts a ON a.attempt_id = ar.attempt_id\nWHERE ar.kind = 'brief'\n AND a.attempt_number = 1\n AND a.reason = 'initial'\n AND NOT EXISTS (\n SELECT 1\n FROM artifacts ao\n WHERE ao.task_id = ar.task_id\n AND ao.kind = 'task_objective'\n AND ao.attempt_id IS NULL\n );\n" }, + { name: "0014_task_objective_backfill.sql", sql: "-- Backfill task-level kind='task_objective' artifact rows for every task\n-- that existed before the shared code-worker prompt composer landed.\n--\n-- The composer's loadOriginalTaskObjective() requires at least one\n-- kind='task_objective' artifact with attempt_id IS NULL. Without\n-- this backfill, any pre-existing active task would throw on its next CI /\n-- crash / stale / wall-clock / malformed retry, on review/conflict respawn,\n-- or on orchestrator submit-brief.\n--\n-- For legacy tasks, the raw original brief lives in the first attempt's\n-- (`attempt_number=1`, `reason='initial'`) brief artifact. The backfilled\n-- row points at the same on-disk file and copies the content_hash — no file\n-- writes are required. The `artifact_recovery_idempotency` unique index\n-- excludes `attempt_id IS NULL`, so the new task-level row never collides\n-- with the per-attempt brief it shadows.\n--\n-- The NOT EXISTS clause makes this migration safe to re-run.\n\nINSERT INTO artifacts (task_id, attempt_id, kind, file_path, content_hash, captured_at)\nSELECT\n ar.task_id,\n NULL,\n 'task_objective',\n ar.file_path,\n ar.content_hash,\n strftime('%Y-%m-%dT%H:%M:%fZ', 'now')\nFROM artifacts ar\nJOIN attempts a ON a.attempt_id = ar.attempt_id\nWHERE ar.kind = 'brief'\n AND a.attempt_number = 1\n AND a.reason = 'initial'\n AND NOT EXISTS (\n SELECT 1\n FROM artifacts ao\n WHERE ao.task_id = ar.task_id\n AND ao.kind = 'task_objective'\n AND ao.attempt_id IS NULL\n );\n" }, { name: "0015_task_goals.sql", sql: "-- Task-level goal worker mode.\n-- quay: foreign_keys_off\n--\n-- A goal is owned by its Quay task, not scheduled independently. The task\n-- stays the scheduling unit; task_goals carries durable objective/status and\n-- accounting state across normal attempts.\n\nALTER TABLE tasks ADD COLUMN worker_execution TEXT NOT NULL DEFAULT 'oneshot'\n CHECK (worker_execution IN ('oneshot', 'goal'));\n\nALTER TABLE attempts ADD COLUMN goal_id TEXT;\nALTER TABLE attempts ADD COLUMN goal_report_processed_at TEXT;\n\n-- Add the goal-mode no-progress handoff reason. SQLite cannot alter a CHECK\n-- constraint in place, so rebuild the table while preserving rows.\nALTER TABLE orchestrator_handoffs RENAME TO orchestrator_handoffs_old;\n\nCREATE TABLE orchestrator_handoffs (\n handoff_id INTEGER PRIMARY KEY AUTOINCREMENT,\n task_id TEXT NOT NULL REFERENCES tasks(task_id),\n reason TEXT NOT NULL CHECK (\n reason IN (\n 'worker_blocker',\n 'budget_exhausted',\n 'human_reply_ingested',\n 'manual_resume',\n 'no_progress'\n )\n ),\n state_event_id INTEGER NOT NULL REFERENCES events(event_id),\n idempotency_key TEXT NOT NULL,\n payload_json TEXT,\n status TEXT NOT NULL DEFAULT 'pending' CHECK (\n status IN ('pending', 'claimed', 'completed', 'cancelled')\n ),\n claim_id TEXT,\n claimed_at TEXT,\n completed_at TEXT,\n created_at TEXT NOT NULL,\n updated_at TEXT NOT NULL,\n UNIQUE (idempotency_key),\n UNIQUE (task_id, state_event_id, reason)\n);\n\nINSERT INTO orchestrator_handoffs (\n handoff_id, task_id, reason, state_event_id, idempotency_key,\n payload_json, status, claim_id, claimed_at, completed_at, created_at,\n updated_at\n)\nSELECT\n handoff_id, task_id, reason, state_event_id, idempotency_key,\n payload_json, status, claim_id, claimed_at, completed_at, created_at,\n updated_at\nFROM orchestrator_handoffs_old;\n\nDROP TABLE orchestrator_handoffs_old;\n\nCREATE INDEX orchestrator_handoffs_status_created_idx\n ON orchestrator_handoffs(status, created_at, handoff_id);\n\nCREATE INDEX orchestrator_handoffs_task_status_idx\n ON orchestrator_handoffs(task_id, status, handoff_id);\n\nCREATE TABLE task_goals (\n task_id TEXT PRIMARY KEY NOT NULL REFERENCES tasks(task_id),\n goal_id TEXT NOT NULL,\n objective TEXT NOT NULL,\n status TEXT NOT NULL CHECK (\n status IN ('active', 'blocked', 'budget_limited', 'complete')\n ),\n token_budget INTEGER,\n tokens_used INTEGER NOT NULL DEFAULT 0,\n time_used_seconds INTEGER NOT NULL DEFAULT 0,\n no_progress_active_count INTEGER NOT NULL DEFAULT 0,\n last_attempt_id INTEGER REFERENCES attempts(attempt_id),\n current_handoff_id INTEGER REFERENCES orchestrator_handoffs(handoff_id),\n created_at TEXT NOT NULL,\n updated_at TEXT NOT NULL,\n completed_at TEXT,\n CHECK (token_budget IS NULL OR token_budget > 0)\n);\n\nCREATE INDEX task_goals_status_idx ON task_goals(status);\n" }, { name: "0016_task_base_branch.sql", sql: "-- Task-level effective base branch.\n--\n-- Existing tasks are backfilled from their repo default so later repo config\n-- changes do not alter already-enqueued work. New enqueue paths write the\n-- effective branch explicitly.\n\nALTER TABLE tasks ADD COLUMN base_branch TEXT;\n\nUPDATE tasks\n SET base_branch = (\n SELECT repos.base_branch\n FROM repos\n WHERE repos.repo_id = tasks.repo_id\n )\n WHERE base_branch IS NULL;\n" }, { name: "0017_goal_completion_audit.sql", sql: "-- Goal completion audit gate.\n-- quay: foreign_keys_off\n--\n-- `completion_pending` is an internal status: the worker has made a terminal\n-- completion claim, but Quay has not yet accepted the claim and entered the\n-- PR lifecycle.\n\nALTER TABLE task_goals RENAME TO task_goals_old;\n\nCREATE TABLE task_goals (\n task_id TEXT PRIMARY KEY NOT NULL REFERENCES tasks(task_id),\n goal_id TEXT NOT NULL,\n objective TEXT NOT NULL,\n status TEXT NOT NULL CHECK (\n status IN (\n 'active',\n 'blocked',\n 'budget_limited',\n 'completion_pending',\n 'complete'\n )\n ),\n token_budget INTEGER,\n tokens_used INTEGER NOT NULL DEFAULT 0,\n time_used_seconds INTEGER NOT NULL DEFAULT 0,\n no_progress_active_count INTEGER NOT NULL DEFAULT 0,\n last_attempt_id INTEGER REFERENCES attempts(attempt_id),\n current_handoff_id INTEGER REFERENCES orchestrator_handoffs(handoff_id),\n created_at TEXT NOT NULL,\n updated_at TEXT NOT NULL,\n completed_at TEXT,\n CHECK (token_budget IS NULL OR token_budget > 0)\n);\n\nINSERT INTO task_goals (\n task_id, goal_id, objective, status, token_budget,\n tokens_used, time_used_seconds, no_progress_active_count,\n last_attempt_id, current_handoff_id, created_at, updated_at, completed_at\n)\nSELECT\n task_id, goal_id, objective, status, token_budget,\n tokens_used, time_used_seconds, no_progress_active_count,\n last_attempt_id, current_handoff_id, created_at, updated_at, completed_at\nFROM task_goals_old;\n\nDROP TABLE task_goals_old;\n\nCREATE INDEX task_goals_status_idx ON task_goals(status);\n" }, diff --git a/packages/cli/src/core/goals.ts b/packages/cli/src/core/goals.ts index c03ed19..125f598 100644 --- a/packages/cli/src/core/goals.ts +++ b/packages/cli/src/core/goals.ts @@ -130,7 +130,7 @@ export function loadGoalPromptContext( WHERE task_id = ? AND kind = 'task_objective' AND attempt_id IS NULL - ORDER BY artifact_id ASC + ORDER BY artifact_id DESC LIMIT 1`, ) .get(taskId); diff --git a/packages/cli/src/core/pr_review.ts b/packages/cli/src/core/pr_review.ts index e240fc6..0905808 100644 --- a/packages/cli/src/core/pr_review.ts +++ b/packages/cli/src/core/pr_review.ts @@ -1454,7 +1454,7 @@ function ensureTaskObjectiveArtifact( WHERE task_id = ? AND kind = 'task_objective' AND attempt_id IS NULL - ORDER BY artifact_id ASC + ORDER BY artifact_id DESC LIMIT 1`, ) .get(taskId); @@ -1641,7 +1641,7 @@ function loadReviewContextBrief(db: DB, taskId: string): string | null { WHERE task_id = ? AND kind = 'task_objective' AND attempt_id IS NULL - ORDER BY artifact_id ASC + ORDER BY artifact_id DESC LIMIT 1`, ) .get(taskId); diff --git a/packages/cli/src/core/resnapshot.ts b/packages/cli/src/core/resnapshot.ts index fc96307..b7d1463 100644 --- a/packages/cli/src/core/resnapshot.ts +++ b/packages/cli/src/core/resnapshot.ts @@ -1,20 +1,3 @@ -// `quay task resnapshot` — re-baseline a task's frozen `ticket_snapshot`. -// -// A task's `ticket_snapshot` is captured once at creation and is the -// definition-of-done the reviewer enforces. When an operator changes scope -// mid-flight by editing the live Linear ticket, that frozen snapshot never -// updates, so the reviewer keeps enforcing the stale acceptance criteria. -// -// `task_resnapshot` re-fetches the Linear issue, re-parses the quay-config -// block, re-composes the snapshot with the SAME code path enqueue uses -// (`fetchTicketContextWithIssue`), and replaces the single per-task -// `ticket_snapshot` artifact both worker and reviewer read. It records a -// `ticket_resnapshotted` audit event carrying a before/after diff and the -// required reason, and invalidates the latest review verdict so the next tick -// schedules a fresh review against the new snapshot (a stale -// `changes_requested` must not block re-review). Running it when the ticket is -// unchanged is a safe, still-audited no-op. - import { createHash } from "node:crypto"; import { readFileSync } from "node:fs"; import type { ArtifactStore } from "../artifacts/store.ts"; @@ -64,8 +47,9 @@ export interface ResnapshotValue { // Count of terminal review verdicts (`approved` / `changes_requested`) // superseded so the next tick re-reviews against the new snapshot. review_invalidated: number; - // The new `ticket_snapshot` artifact id, or null on a no-op. + // New artifacts written on a changed resnapshot, or null on a no-op. snapshot_artifact_id: number | null; + objective_artifact_id: number | null; event_id: number; } @@ -140,10 +124,11 @@ export async function task_resnapshot( task.external_ref, ); const freshSnapshot = fetched.ctx.ticket_snapshot; + const freshBrief = fetched.ctx.brief; const externalRef = task.external_ref; return deps.supervisorLock.run(() => - resnapshotUnderLock(deps, task, externalRef, freshSnapshot, reason), + resnapshotUnderLock(deps, task, externalRef, freshSnapshot, freshBrief, reason), ); } @@ -152,6 +137,7 @@ function resnapshotUnderLock( task: TaskRow, externalRef: string, freshSnapshot: string, + freshBrief: string, reason: string, ): ResnapshotResult { const now = deps.clock.nowISO(); @@ -168,48 +154,64 @@ function resnapshotUnderLock( oldContent === null || JSON.stringify(freshCore) !== JSON.stringify(oldCore); - const eventData: Record = { - reason, - external_ref: externalRef, - changed, - review_invalidated: 0, - snapshot_artifact_id: null, - before_snapshot_hash: oldContent === null ? null : sha256(oldContent), - after_snapshot_hash: sha256(freshSnapshot), - diff: diffCore(oldCore, freshCore), - }; - - let artifactId: number | null = null; + let snapshotArtifactId: number | null = null; + let objectiveArtifactId: number | null = null; let reviewInvalidated = 0; let eventId = -1; + let snapshotContentAfter = oldContent; deps.db.exec("BEGIN"); try { if (changed) { // Preserve non-core augmentation keys (and their original positions) // from the prior snapshot; overwrite only the core definition-of-done - // keys with the freshly fetched values. This is the single shared - // snapshot both worker and reviewer read — replaced in place, no - // version skew. + // keys with the freshly fetched values. const merged: Record = oldParsed === null ? {} : { ...oldParsed }; for (const key of SNAPSHOT_CORE_KEYS) { if (key in freshParsed) merged[key] = freshParsed[key]; else delete merged[key]; } - const written = deps.artifactStore.writeArtifact({ + const snapshotContent = JSON.stringify(merged, null, 2); + const writtenSnapshot = deps.artifactStore.writeArtifact({ taskId: task.task_id, attemptId: null, kind: "ticket_snapshot", - content: JSON.stringify(merged, null, 2), + content: snapshotContent, + extension: "md", + }); + snapshotArtifactId = writtenSnapshot.artifactId; + snapshotContentAfter = snapshotContent; + + const writtenObjective = deps.artifactStore.writeArtifact({ + taskId: task.task_id, + attemptId: null, + kind: "task_objective", + content: freshBrief, extension: "md", }); - artifactId = written.artifactId; + objectiveArtifactId = writtenObjective.artifactId; + + deps.db + .query(`UPDATE task_goals SET objective = ?, updated_at = ? WHERE task_id = ?`) + .run(freshBrief, now, task.task_id); + reviewInvalidated = invalidateLatestReview(deps.db, task.task_id); - eventData.review_invalidated = reviewInvalidated; - eventData.snapshot_artifact_id = artifactId; } + const eventData: Record = { + reason, + external_ref: externalRef, + changed, + review_invalidated: reviewInvalidated, + snapshot_artifact_id: snapshotArtifactId, + objective_artifact_id: objectiveArtifactId, + before_snapshot_hash: oldContent === null ? null : sha256(oldContent), + after_snapshot_hash: + snapshotContentAfter === null ? null : sha256(snapshotContentAfter), + diff: diffCore(oldCore, freshCore), + }; + const eventRow = deps.db .query<{ event_id: number }, [string, string, string, string, string]>( `INSERT INTO events ( @@ -237,7 +239,8 @@ function resnapshotUnderLock( external_ref: externalRef, changed, review_invalidated: reviewInvalidated, - snapshot_artifact_id: artifactId, + snapshot_artifact_id: snapshotArtifactId, + objective_artifact_id: objectiveArtifactId, event_id: eventId, }, }; diff --git a/packages/cli/src/core/tick.ts b/packages/cli/src/core/tick.ts index cfdeee8..dc8f424 100644 --- a/packages/cli/src/core/tick.ts +++ b/packages/cli/src/core/tick.ts @@ -3748,17 +3748,21 @@ function loadUmbrellaFinalPrExpectedSubtasks( ut.task_id, t.state AS task_state, t.pr_url, - ao.file_path AS objective_path + ( + SELECT ao.file_path + FROM artifacts ao + WHERE ao.task_id = ut.task_id + AND ao.kind = 'task_objective' + AND ao.attempt_id IS NULL + ORDER BY ao.artifact_id DESC + LIMIT 1 + ) AS objective_path FROM umbrella_expected_tasks uet LEFT JOIN umbrella_tasks ut ON ut.umbrella_workflow_id = uet.umbrella_workflow_id AND ut.external_ref = uet.external_ref LEFT JOIN tasks t ON t.task_id = ut.task_id - LEFT JOIN artifacts ao - ON ao.task_id = ut.task_id - AND ao.kind = 'task_objective' - AND ao.attempt_id IS NULL WHERE uet.umbrella_workflow_id = ? ORDER BY uet.external_ref`, ) diff --git a/packages/cli/src/core/worker_prompt.ts b/packages/cli/src/core/worker_prompt.ts index 43e9cc1..1aa0552 100644 --- a/packages/cli/src/core/worker_prompt.ts +++ b/packages/cli/src/core/worker_prompt.ts @@ -2,12 +2,12 @@ // // Every code-worker attempt — initial, deterministic retry, non-budget // respawn, orchestrator submit-brief — assembles its final prompt from the -// same conceptual sections so the original task objective remains first-class -// across the whole lifecycle. +// same conceptual sections so the task objective remains first-class across +// the whole lifecycle. // // Sections in render order: // 1. Output contract — the code-worker protocol preamble. -// 2. Stable task objective — original brief, capped if oversized, with a +// 2. Stable task objective — current brief, capped if oversized, with a // pointer to the full `task_objective` artifact. // 3. Reference repos — optional deployment/runtime context for // read-only sibling repo checkouts. @@ -131,9 +131,9 @@ export function composeReviewerPrompt( }; } -// Loads the canonical original task objective for a task. The objective is -// written once at enqueue time (kind='task_objective', attempt_id IS NULL) and -// referenced by every subsequent code-worker attempt. +// Loads the current task objective for a task. Enqueue writes the first +// task-level artifact; task resnapshot can append a newer one when operators +// re-baseline the ticket. export function loadOriginalTaskObjective( db: DB, taskId: string, @@ -145,13 +145,13 @@ export function loadOriginalTaskObjective( WHERE task_id = ? AND kind = 'task_objective' AND attempt_id IS NULL - ORDER BY artifact_id ASC + ORDER BY artifact_id DESC LIMIT 1`, ) .get(taskId); if (!row) { throw new Error( - `task_objective artifact not found for task ${taskId}; enqueue must write it once on task creation`, + `task_objective artifact not found for task ${taskId}; enqueue must write one on task creation`, ); } let body: string; @@ -212,7 +212,7 @@ function renderTaskObjective(obj: TaskObjectiveRef, cap: number): string { attrs.push(`excerpt-bytes="${utf8ByteLength(rendered)}"`); } const inner = truncated - ? `${escapeXmlText(rendered)}\n\n[Excerpt truncated. Read the full original task objective from artifact #${obj.artifactId} at ${obj.filePath}.]` + ? `${escapeXmlText(rendered)}\n\n[Excerpt truncated. Read the full task objective from artifact #${obj.artifactId} at ${obj.filePath}.]` : escapeXmlText(obj.body); return `\n${inner}\n`; } diff --git a/packages/cli/tests/adapters/test_github_adapter_merge_method.test.ts b/packages/cli/tests/adapters/test_github_adapter_merge_method.test.ts index 851d9b8..63fe9cb 100644 --- a/packages/cli/tests/adapters/test_github_adapter_merge_method.test.ts +++ b/packages/cli/tests/adapters/test_github_adapter_merge_method.test.ts @@ -1,8 +1,3 @@ -// BRIX-1920 / BRIX-1921: `mergePullRequest` must pick a merge method the -// target repo actually allows (instead of the historical hardcoded `--merge`), -// and a policy-rejected merge must be classified as a distinct NON-RETRYABLE -// kind rather than `unknown`. -// // Setup mirrors the other adapter tests: shadow the real `gh` binary with a // stub script on PATH. The stub reports a configurable merge-method policy for // `gh api repos/{owner}/{repo}` and logs the flag passed to `gh pr merge` so @@ -230,7 +225,7 @@ test("allowed merge method is cached per repo (single api read across merges)", test("policy-rejected merge throws GitHubMergeError kind=method_not_allowed", () => { const { apiLog, mergeLog } = newLogs(); // Method selection picks --merge (allowed per policy) but the merge itself - // is rejected by a branch/repo rule — the BRIX-1921 safety net. + // is rejected by a branch/repo rule. installGhStub( mergeMethodStub({ allow: { merge: true, squash: false, rebase: false }, diff --git a/packages/cli/tests/core/worker_prompt.test.ts b/packages/cli/tests/core/worker_prompt.test.ts index 95acec8..00e67da 100644 --- a/packages/cli/tests/core/worker_prompt.test.ts +++ b/packages/cli/tests/core/worker_prompt.test.ts @@ -314,7 +314,7 @@ test("objective over cap renders an excerpt plus pointer to full artifact", () = expect(composed.brief).toContain('objective-bytes="2048"'); expect(composed.brief).toContain("excerpt-bytes="); expect(composed.brief).toContain( - `[Excerpt truncated. Read the full original task objective from artifact #${SAFE_OBJECTIVE.artifactId} at ${SAFE_OBJECTIVE.filePath}.]`, + `[Excerpt truncated. Read the full task objective from artifact #${SAFE_OBJECTIVE.artifactId} at ${SAFE_OBJECTIVE.filePath}.]`, ); // Excerpt is a prefix of the body, never exceeds the cap. const matchExcerptBytes = composed.brief.match(/excerpt-bytes="(\d+)"/); @@ -341,7 +341,7 @@ test("default render cap is exposed as a constant", () => { expect(DEFAULT_OBJECTIVE_RENDER_CAP_BYTES).toBeGreaterThan(1024); }); -test("loadOriginalTaskObjective reads the task-level kind='task_objective' artifact", () => { +test("loadOriginalTaskObjective reads the current task-level kind='task_objective' artifact", () => { h = createHarness(); const taskId = insertTask(h.db, { taskId: "obj-task" }); const store = createArtifactStore({ @@ -363,6 +363,35 @@ test("loadOriginalTaskObjective reads the task-level kind='task_objective' artif expect(ref.body).toBe("Build the widget service."); }); +test("loadOriginalTaskObjective prefers the latest resnapshotted objective", () => { + h = createHarness(); + const taskId = insertTask(h.db, { taskId: "resnapshotted-objective" }); + const store = createArtifactStore({ + db: h.db, + artifactRoot: h.artifactRoot, + clock: h.clock, + }); + store.writeArtifact({ + taskId, + attemptId: null, + kind: "task_objective", + content: "Original objective.", + extension: "md", + }); + const latest = store.writeArtifact({ + taskId, + attemptId: null, + kind: "task_objective", + content: "Re-baselined objective.", + extension: "md", + }); + + const ref = loadOriginalTaskObjective(h.db, taskId); + expect(ref.artifactId).toBe(latest.artifactId); + expect(ref.filePath).toBe(latest.filePath); + expect(ref.body).toBe("Re-baselined objective."); +}); + test("loadOriginalTaskObjective throws when no task_objective artifact exists", () => { const harness = createHarness(); h = harness; diff --git a/packages/cli/tests/resnapshot/task_resnapshot.test.ts b/packages/cli/tests/resnapshot/task_resnapshot.test.ts index 3296f0b..30aa027 100644 --- a/packages/cli/tests/resnapshot/task_resnapshot.test.ts +++ b/packages/cli/tests/resnapshot/task_resnapshot.test.ts @@ -1,26 +1,17 @@ -// BRIX-1916 — `quay task resnapshot --reason `. -// -// Covers each acceptance criterion: -// * replaces the frozen ticket_snapshot from the current Linear ticket and -// re-parses the quay-config block (single shared snapshot, no version skew), -// * emits a ticket_resnapshotted event with a before/after diff + reason, -// * invalidates a stale changes_requested verdict so the next tick re-reviews, -// * preserves creation-time snapshot augmentations it does not recompute, -// * runs as a safe, still-audited no-op when the ticket is unchanged, -// * fails cleanly on unknown task / missing external_ref / missing reason / -// adapter-disabled. - import { afterEach, expect, test } from "bun:test"; +import { createHash } from "node:crypto"; import { readFileSync } from "node:fs"; import { dispatch } from "../../src/cli/dispatch.ts"; import { bufferIO } from "../../src/cli/io.ts"; +import { enterReview } from "../../src/core/pr_review.ts"; import { createRepoService } from "../../src/core/repos/service.ts"; import { fetchTicketContextWithIssue } from "../../src/core/ticket_context.ts"; +import { loadOriginalTaskObjective } from "../../src/core/worker_prompt.ts"; import type { DB } from "../../src/db/connection.ts"; import type { LinearIssue } from "../../src/ports/linear.ts"; import { createHarness, type Harness } from "../support/harness.ts"; import { buildCliDeps } from "../support/cli_deps.ts"; -import { insertPreamble } from "../support/fixtures.ts"; +import { insertPreamble, seedTaskObjective } from "../support/fixtures.ts"; let h: Harness | null = null; afterEach(() => { @@ -126,6 +117,10 @@ function latestSnapshot(harness: Harness, taskId: string): string { return readFileSync(row.file_path, "utf8"); } +function sha256(text: string): string { + return createHash("sha256").update(text).digest("hex"); +} + function snapshotCount(harness: Harness, taskId: string): number { return harness.db .query<{ n: number }, [string]>( @@ -198,6 +193,7 @@ test("resnapshot replaces the frozen snapshot from the current Linear ticket", a ); insertTask(h.db, { taskId: "task-1" }); seedSnapshot(built, "task-1", original); + seedTaskObjective(h, "task-1", "Original worker objective: Original strict AC."); // Operator relaxes the AC on the live ticket. built.linear.setIssue(makeIssue("Relaxed AC: the happy path is sufficient.")); @@ -219,9 +215,8 @@ test("resnapshot replaces the frozen snapshot from the current Linear ticket", a review_invalidated: 0, }); expect(payload.snapshot_artifact_id).not.toBeNull(); + expect(payload.objective_artifact_id).not.toBeNull(); - // Single shared snapshot replaced in place; latest content reflects the - // re-fetched ticket and a freshly re-parsed quay-config block. expect(snapshotCount(h, "task-1")).toBe(2); const parsed = JSON.parse(latestSnapshot(h, "task-1")); expect(parsed.linear_issue.body).toContain("Relaxed AC"); @@ -229,6 +224,11 @@ test("resnapshot replaces the frozen snapshot from the current Linear ticket", a expect(parsed.quay_config_block.repo).toBe(REPO_ID); expect(parsed.quay_config_block.tags).toEqual(["resnapshot"]); expect(built.linear.getIssueCalls).toContain(EXTERNAL_REF); + + const objective = loadOriginalTaskObjective(h.db, "task-1"); + expect(objective.artifactId).toBe(payload.objective_artifact_id); + expect(objective.body).toContain("Relaxed AC"); + expect(objective.body).not.toContain("Original strict AC"); }); test("resnapshot emits a ticket_resnapshotted event with a before/after diff and the reason", async () => { @@ -264,6 +264,77 @@ test("resnapshot emits a ticket_resnapshotted event with a before/after diff and expect(typeof data.before_snapshot_hash).toBe("string"); expect(typeof data.after_snapshot_hash).toBe("string"); expect(data.before_snapshot_hash).not.toBe(data.after_snapshot_hash); + expect(data.after_snapshot_hash).toBe(sha256(latestSnapshot(h, "task-1"))); +}); + +test("resnapshot updates the reviewer prompt context for the next review", async () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h); + + const original = await composeSnapshot(built, makeIssue("Original AC for review.")); + insertTask(h.db, { taskId: "task-1", state: "pr-open" }); + seedSnapshot(built, "task-1", original); + seedTaskObjective(h, "task-1", "Original reviewer objective: stale AC."); + built.linear.setIssue(makeIssue("Relaxed AC for review.")); + + const io = bufferIO(); + const result = await dispatch( + ["task", "resnapshot", "task-1", "--reason", "review should use latest ticket"], + built.deps, + io, + ); + expect(result.exitCode).toBe(0); + + built.github.setPrView(REPO_ID, 17, { + number: 17, + title: "Task PR", + body: "Body", + url: "https://github.example/repo/pull/17", + headRefName: "quay/task-1", + headSha: "head-review", + baseRef: "main", + isCrossRepository: false, + }); + built.github.setPrSnapshotByNumber(REPO_ID, 17, { + state: "open", + headSha: "head-review", + baseSha: "base-review", + mergeable: "mergeable", + latestReview: { decision: "NONE", latestReviewId: null, comments: "" }, + checks: { + checkSha: "head-review", + items: [{ name: "ci", workflow: null, bucket: "pass", required: true }], + }, + }); + + const review = enterReview( + { + db: h.db, + clock: h.clock, + github: built.github, + tmux: built.tmux, + artifactStore: built.deps.artifactStore, + }, + { + repoId: REPO_ID, + prNumber: 17, + reviewerEnabled: true, + gateQuayOwnedDone: true, + }, + ); + expect(review.scheduled).toBe(true); + expect(review.attempt_id).not.toBeNull(); + + const promptPath = h.db + .query<{ file_path: string }, [number]>( + `SELECT file_path FROM artifacts + WHERE attempt_id = ? AND kind = 'final_prompt'`, + ) + .get(review.attempt_id!)!.file_path; + const finalPrompt = readFileSync(promptPath, "utf8"); + expect(finalPrompt).toContain("Relaxed AC for review"); + expect(finalPrompt).not.toContain("stale AC"); }); test("resnapshot invalidates a stale changes_requested verdict so the next tick re-reviews", async () => { @@ -321,6 +392,9 @@ test("resnapshot preserves creation-time snapshot augmentations it does not reco expect(parsed.linear_issue.body).toContain("Relaxed AC"); expect(parsed.linear_blocked_by_relations).toEqual([{ identifier: "BRIX-1900" }]); expect(parsed.linear_hierarchy).toEqual({ parent: null, children: [] }); + + const data = JSON.parse(latestEvent(h, "task-1").event_data!); + expect(data.after_snapshot_hash).toBe(sha256(latestSnapshot(h, "task-1"))); }); test("resnapshot is a safe, still-audited no-op when the ticket is unchanged", async () => { @@ -380,6 +454,8 @@ test("resnapshot no-op ignores creation-time augmentations when comparing", asyn expect(result.exitCode).toBe(0); expect(JSON.parse(io.out()).changed).toBe(false); expect(snapshotCount(h, "task-1")).toBe(1); + const data = JSON.parse(latestEvent(h, "task-1").event_data!); + expect(data.before_snapshot_hash).toBe(data.after_snapshot_hash); }); test("resnapshot rejects an unknown task", async () => { diff --git a/packages/cli/tests/support/fixtures.ts b/packages/cli/tests/support/fixtures.ts index 926ec21..c2d6ab1 100644 --- a/packages/cli/tests/support/fixtures.ts +++ b/packages/cli/tests/support/fixtures.ts @@ -111,10 +111,9 @@ export function insertFinalPromptArtifact( }).artifactId; } -// Seeds the kind='task_objective' artifact that loadOriginalTaskObjective -// requires. Real enqueue writes this once per task; tests that bypass enqueue -// (any test that uses insertTask + retries/respawn/submit_brief) must call -// this helper explicitly. +// Seeds a task-level kind='task_objective' artifact. Tests that bypass enqueue +// (any test that uses insertTask + retries/respawn/submit_brief) must call this +// helper explicitly. export function seedTaskObjective( h: { db: DB; artifactRoot: string; clock: Clock }, taskId: string,