diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index f2cab74c..496cbe2a 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -809,6 +809,7 @@ components: - model_reviewer - preamble_worker - preamble_reviewer + - review_finding_linear_enabled - archived_at - created_at properties: @@ -861,6 +862,11 @@ components: - integer - "null" minimum: 1 + review_finding_linear_enabled: + description: Per-repo override for filing non-blocking review findings as Linear issues. null inherits the deployment default; true/false pin the repo. + type: + - boolean + - "null" archived_at: type: - string @@ -886,6 +892,7 @@ components: - model_reviewer - preamble_worker - preamble_reviewer + - review_finding_linear_enabled - archived_at - created_at - active_task_count @@ -945,6 +952,11 @@ components: - integer - "null" minimum: 1 + review_finding_linear_enabled: + description: Per-repo override for filing non-blocking review findings as Linear issues. null inherits the deployment default; true/false pin the repo. + type: + - boolean + - "null" archived_at: type: - string @@ -1079,6 +1091,7 @@ components: - operations - ci_policy - adapters + - review_findings - agents - preambles - retry_templates @@ -1104,6 +1117,15 @@ components: type: array items: $ref: "#/components/schemas/AdapterSummary" + review_findings: + type: object + additionalProperties: false + required: + - linear_enabled + properties: + linear_enabled: + description: Global default for filing non-blocking review findings as Linear issues (resolves ON when the deployment setting is unset). Policy flag, distinct from the Linear adapter connectivity toggle. + type: boolean agents: $ref: "#/components/schemas/AgentsReadModel" preambles: @@ -2053,6 +2075,11 @@ components: - integer - "null" minimum: 1 + review_finding_linear_enabled: + description: Per-repo override for filing non-blocking review findings as Linear issues. null inherits the deployment default; true/false pin the repo. Affects synthetic_review tasks only. + type: + - boolean + - "null" ci_ignore_mode: type: string enum: @@ -2125,6 +2152,11 @@ components: - string - "null" minLength: 1 + review_finding_linear_enabled: + description: Global default for filing non-blocking review findings as Linear issues. null leaves it unset (resolves to ON). Policy flag, distinct from the Linear adapter connectivity toggle. + type: + - boolean + - "null" TagsReplaceChange: type: object additionalProperties: false diff --git a/docs/quay-spec.md b/docs/quay-spec.md index 9067d8f4..acc65f18 100644 --- a/docs/quay-spec.md +++ b/docs/quay-spec.md @@ -552,7 +552,7 @@ The "single chokepoint" referenced in §4 invariant 7 guarantees **SQL atomicity Attempt rows have a two-phase lifecycle: **scheduled** (created when a respawn is queued; `spawned_at = NULL`, `tmux_session = NULL`) and **spawned** (filled in by tick at promotion). The "pending attempt" for a task in `queued` is the most recent attempt row with `spawned_at = NULL`. 1. **Schedule** (at every respawn trigger — submit-brief, deterministic retry detection, review/conflict respawn): within one SQL transaction, insert `attempts` row with `spawned_at = NULL`, `tmux_session = NULL`, `consumed_budget` set per the reason, `template_id` and brief artifact written, transition `tasks.state = queued`. (Enqueue follows this exact pattern for attempt #1.) - 2. **Promote** (tick, when capacity allows): resolve the worker GitHub token exactly as the worker pane will receive it, then run a spawn-time auth preflight against the target repository before any worker process starts. The preflight validates repository access, worker write access, and PR visibility for the task branch. If this fails with invalid, expired, missing, empty, or repo-inaccessible worker credentials, mark the pending attempt `exit_kind = 'worker_auth_invalid'`, retry once after re-reading the token source, and on a repeated failure transition to `awaiting-next-brief` with a `worker_auth_invalid` handoff. Then refresh the remote ref with `git -C fetch origin ` (handles the case where a prior attempt or external pusher advanced the remote). Read the remote SHA: `git -C rev-parse origin/` if the ref exists; NULL otherwise (first attempt for a brand-new branch). Snapshot whether a PR currently exists for the branch via the resolved worker token and `gh pr view ` (returns 0 → exists; non-zero / "no pull requests" → does not exist); store as `pr_existed_at_spawn` (1 or 0). On non-auth `gh` failure, log `tick_error` and skip promotion this tick (the PR-existence snapshot is required for correct progress detection). Within one SQL transaction, set `attempts.spawned_at = now()`, `attempts.remote_sha_at_spawn = `, `attempts.pr_existed_at_spawn = <0|1>`, increment `tasks.attempts_consumed` iff `attempts.consumed_budget = 1`, update `tasks.state = running`, write `spawned` event. + 2. **Promote** (tick, when capacity allows and `tasks.spawn_retry_next_eligible_at` is NULL or in the past): resolve the worker GitHub token exactly as the worker pane will receive it, then run a spawn-time auth preflight against the target repository before any worker process starts. The preflight validates repository access, worker write access, and PR visibility for the task branch. If this fails with invalid, expired, missing, empty, or repo-inaccessible worker credentials, mark the pending attempt `exit_kind = 'worker_auth_invalid'`, record `tasks.spawn_failure_reason`, set `tasks.spawn_retry_next_eligible_at` using spawn-failure backoff, retry once after re-reading the token source, and on a repeated failure transition to `awaiting-next-brief` with a `worker_auth_invalid` handoff. Then refresh the remote ref with `git -C fetch origin ` (handles the case where a prior attempt or external pusher advanced the remote). Read the remote SHA: `git -C rev-parse origin/` if the ref exists; NULL otherwise (first attempt for a brand-new branch). Snapshot whether a PR currently exists for the branch via the resolved worker token and `gh pr view ` (returns 0 → exists; non-zero / "no pull requests" → does not exist); store as `pr_existed_at_spawn` (1 or 0). On non-auth `gh` failure, log `tick_error` and skip promotion this tick (the PR-existence snapshot is required for correct progress detection). Within one SQL transaction, set `attempts.spawned_at = now()`, `attempts.remote_sha_at_spawn = `, `attempts.pr_existed_at_spawn = <0|1>`, increment `tasks.attempts_consumed` iff `attempts.consumed_budget = 1`, update `tasks.state = running`, write `spawned` event. 3. **Substrate work** (outside the transaction): write `.quay-prompt.md` to the worktree, create the tmux session, send the agent invocation. 4. **Record session**: update `attempts.tmux_session` with the session name. @@ -565,7 +565,7 @@ The "single chokepoint" referenced in §4 invariant 7 guarantees **SQL atomicity - Else, fetch `remote_sha_at_exit`, read `pr_exists_at_exit` via `gh pr view`, and apply the canonical progress predicate (`pr_existed_at_spawn` recorded at promotion is the spawn-time PR existence). If a PR exists at exit and progress was made → `exit_kind = 'pr_opened'`, transition → `pr-open` (budget preserved at promotion's accounting; this is a healthy outcome). - Else if a PR exists but no progress this attempt → `exit_kind = 'no_progress'`, schedule deterministic `crash` retry, transition → `queued` (or budget-exhausted handling). Budget is preserved at promotion's accounting; the retry consumes one unit at its own promotion. - Else (no PR, no signal file) → continue to step 4 (the genuine substrate-failed default). - 4. **Substrate-failed default (no worker evidence).** Set `attempts.exit_kind = 'spawn_failed'`, `ended_at = now()`. **Roll back budget**: if `attempts.consumed_budget = 1`, decrement `tasks.attempts_consumed` by 1 to offset the increment from step 2. (Non-budget respawns are not decremented.) Increment `tasks.spawn_failures_consecutive`. If `spawn_failures_consecutive >= max_spawn_failures` (default 3), transition task to `worktree_error` (parked, manual recovery). Otherwise, insert a fresh scheduled `attempts` row with the same `reason`, `consumed_budget`, and brief content (clean retry of the same logical attempt), transition the task back to `queued`. The failed `spawn_failed` row is retained for forensics. + 4. **Substrate-failed default (no worker evidence).** Set `attempts.exit_kind = 'spawn_failed'`, `ended_at = now()`. **Roll back budget**: if `attempts.consumed_budget = 1`, decrement `tasks.attempts_consumed` by 1 to offset the increment from step 2. (Non-budget respawns are not decremented.) Increment `tasks.spawn_failures_consecutive` and record `tasks.spawn_failure_reason`. If `spawn_failures_consecutive >= max_spawn_failures` (default 3), transition task to `worktree_error` (parked, manual recovery). Otherwise, insert a fresh scheduled `attempts` row with the same `reason`, `consumed_budget`, and brief content (clean retry of the same logical attempt), set `tasks.spawn_retry_next_eligible_at` using exponential backoff, and transition the task back to `queued`. The failed `spawn_failed` row is retained for forensics. - **Why budget is preserved on the evidence-found paths.** The whole point of the §5 budget rule is that a successful (or productive) spawn consumes one unit of budget at promotion time. If recovery finds that the worker actually started and did real work — opened a PR, advanced the remote, wrote a blocker — that's a productive (or at least observable) attempt, and budget accounting should match the equivalent dead-worker outcome. Only the genuine "tmux never came up, nothing happened" case rolls budget back, because there was no real attempt to charge for. - `spawn_failures_consecutive` resets to 0 on any successful spawn (i.e., the next attempt's worker actually starts logging) **and** on any evidence-found recovery outcome (since a worker provably started). @@ -1340,7 +1340,7 @@ Single config file (location configurable; default `~/.quay/config.toml`). Loade | `claim_timeout_seconds` | `1800` | Max age of an orchestrator claim before tick auto-releases the task back to `awaiting-next-brief`. | | `max_claim_expirations` | `3` | Consecutive `claim_expired` events before the task is parked in `orchestrator_loop`. | | `max_non_budget_respawns` | `20` | Count of allowed review-feedback + merge-conflict respawns per task (the two paths that can loop on stale GitHub signals). The Nth respawn schedules normally; the (N+1)th parks the task in `non_budget_loop`. **`advice_answered` is NOT counted** — it's bounded by human availability (one respawn per Slack reply) and is not a runaway-loop risk. | -| `max_spawn_failures` | `3` | Consecutive substrate-side spawn failures (tmux create errors, DB write failures during spawn) before the task is parked in `worktree_error`. Substrate spawn failures do not consume retry budget. | +| `max_spawn_failures` | `3` | Consecutive substrate-side spawn failures (tmux create errors, DB write failures during spawn) before the task is parked in `worktree_error`. Substrate spawn failures do not consume retry budget and retry with exponential spawn backoff. | | `tick_lock_path` | `${data_dir}/tick.lock` | **Supervisor lockfile** — held by `quay tick` for the duration of a cycle and by `quay cancel` for intent-write + finalizer. Name retained for compatibility; semantically protects all supervisor side effects (tmux, gh mutations, Slack, FS, branch ops). | | `supervisor_lock_stale_seconds` | `30` | Grace period after which a lockfile whose owning PID is no longer alive is considered stale and reclaimable. Prevents a hung-then-killed tick from indefinitely blocking `quay cancel`. | | `agent_invocation` | (e.g. `claude --prompt-file {prompt_file}`) | The CLI invocation pattern for spawning the worker. The literal token `{prompt_file}` is substituted with the path to `.quay-prompt.md` at spawn time. | @@ -1452,7 +1452,7 @@ Collisions across distinct `external_ref` values that slug to the same branch ar | **Budget-exhausted handoff** | Quay never forces a task into a terminal-failed state on budget exhaustion. The task parks in `awaiting-next-brief` with `budget_exhausted = true`. The orchestrator decides between `escalate-human` and `cancel`. Quay never owns "this task is hopeless" judgment. | | **Budget consumed at spawn/respawn time, not at trigger time** | Worker blockers, CI failures, etc. don't decrement budget on detection — only the act of spawning a new attempt does. The first spawn (tick promoting `queued → running` with `reason = initial`) also consumes one unit. `submit-brief --reason advice_answered`, `review`, and `conflict` respawns do not consume. `submit-brief --reason blocker_resolved` errors when `budget_exhausted = true`. | | **Enqueue does not spawn** | `quay enqueue` registers a task in `queued`; the next `quay tick` promotes to `running` when capacity allows. Capacity logic lives in tick only. | -| **Bootstrap is atomic at enqueue** | All git/install work happens synchronously inside `quay enqueue`. Any bootstrap failure aborts enqueue cleanly with no task row created. By the time a task is in `queued`, its worktree is fully ready for spawn. | +| **Bootstrap is atomic at enqueue** | All git/install work happens synchronously inside `quay enqueue`. Any bootstrap failure aborts enqueue cleanly with no task row created. By the time a newly enqueued task is in `queued`, its worktree is fully ready for spawn. Queued respawns also self-heal if the recorded worktree path is missing: tick recreates the worktree before promotion, preferring `origin/` when that remote branch exists and falling back to `origin/` while preserving the task branch name. Dependency installation is rerun and a `worktree_recreated` audit event records the selected recovery base. | | **SQL atomicity, side-effect eventual consistency** | The transition chokepoint guarantees one SQL transaction per state change. Side effects (tmux, gh, Slack, FS) happen outside the transaction with explicit ordering and idempotent recovery on the next tick. See §5 "Transition chokepoint and side-effect ordering." | | **Non-budget respawns are deduplicated** | Review feedback dedupes by `last_review_id_acted_on`; merge conflict by `last_conflict_observation`. A safety cap (`max_non_budget_respawns`, default 20) parks the task in `non_budget_loop` if the dedupe keys ever miss. Stale GitHub signals do not cause infinite respawns. | | **`pr-open` polls PR state** | `pr-open` checks for merged/closed PR state on every tick, not just CI. Humans merging or closing while CI is pending transitions to terminal cleanly. | @@ -1464,7 +1464,7 @@ Collisions across distinct `external_ref` values that slug to the same branch ar | **CI status semantics** | Defined precisely in §5 "CI status rules": stale-SHA filtering, any reported failure blocks, no reported checks = pass, unparseable = pending. | | **Read commands return JSON** | Collections → JSON array. Single records → JSON object. NDJSON only for `quay tick`. No `--json` flag. | | **Idempotent PR contract** | The worker creates a PR only if none exists for its branch; otherwise it pushes updates to the existing PR. Tick uses per-attempt `remote_sha_at_spawn` vs. `remote_sha_at_exit` (the **remote** branch SHA, fetched fresh) to detect "no progress." It also snapshots `pr_existed_at_spawn` at promotion: if no PR existed at spawn but one exists at exit, the attempt counts as progress even when the remote SHA didn't change during *this* attempt (handles the case where attempt N pushed but crashed before `gh pr create`, and attempt N+1 only opens the PR). Local-only commits do not count as progress; the PR is only updated by a successful push. | -| **Spawn-failure recovery is evidence-first** | If a task is in `running` with `spawned_at` set but `tmux_session = NULL`, that's a crash mid-spawn. Tick recovery is **NOT** an unconditional spawn-failed write — between substrate-step success and DB-step commit, the worker may have started, run, pushed, opened a PR, or written a blocker. Recovery (1) kills any orphan canonical-name tmux session, (2) collects the session log, (3) runs the same dead-worker evidence classifier as the normal `running` branch (blocker → `awaiting-next-brief`; PR with progress → `pr-open`; PR no progress → `crash` retry; no PR no signal → `spawn_failed` rollback). Budget is preserved on every evidence-found path; budget rollback applies only to the genuine no-evidence case. This guarantees that a real PR opened during the spawn-step-3-to-4 crash window is never killed and never causes false `no_progress` or budget-loss. | +| **Spawn-failure recovery is evidence-first** | If a task is in `running` with `spawned_at` set but `tmux_session = NULL`, that's a crash mid-spawn. Tick recovery is **NOT** an unconditional spawn-failed write — between substrate-step success and DB-step commit, the worker may have started, run, pushed, opened a PR, or written a blocker. Recovery (1) kills any orphan canonical-name tmux session, (2) collects the session log, (3) runs the same dead-worker evidence classifier as the normal `running` branch, but only evidence attributable to the current attempt prevents spawn-failure handling. Current-attempt evidence includes blocker/goal/ready-for-review signals, a PR opened during the attempt, or remote progress that can be attached to a PR. A stale PR that already existed at spawn with unchanged remote SHA is not current-attempt evidence and routes to `spawn_failed` rollback. Budget is preserved on every evidence-found path; budget rollback applies only to the genuine no-evidence case. This guarantees that a real PR opened during the spawn-step-3-to-4 crash window is never killed and never causes false `no_progress` or budget-loss. | | **Recovery-path artifacts always have an attempt_id** | `blocker`, `slack_reply`, `malformed_signal`, `slack_escalation_post` are linked to a specific attempt and set `content_hash`. The partial unique index excludes NULL `attempt_id` values to avoid SQLite NULL-non-distinct duplicates. For `slack_escalation_post` the hash also incorporates `escalation_seq` so a recovery retry collides while a legitimate second escalation on the same attempt does not. | | **Human advice is orchestrator-owned** | `escalate-human` records the question and preserves the claim; `record-human-reply` records the answer and returns the task to `claimed-by-orchestrator`; `submit-brief --reason advice_answered` schedules the next worker and completes the original handoff. Tick does not need workspace-specific Slack routing policy for the new flow. | | **Shared delivery outbox** | All Quay-originated orchestrator side effects are represented by `outbox_items`. `handler_class = 'workflow_intervention'` backs task-affecting human/advice handoffs; `handler_class = 'delivery'` is for notification-only work that can be claimed, completed, failed, and retried without changing task state. Generic outbox mutation commands reject workflow/intervention rows; those continue through the task claim/handoff ownership fence. Quay owns `idempotency_key` uniqueness, so downstream delivery systems do not have to infer duplicate emissions from Slack state. | @@ -1665,8 +1665,9 @@ The v1 test suite must cover the following cases. Each is a state-machine integr a) No worker evidence → marks `spawn_failed`, rolls back budget if `consumed_budget = 1`, schedules a fresh attempt, transitions back to `queued`. `spawn_failures_consecutive` increments. (See tests 45, 46, 46b.) b) Worker opened a PR → `exit_kind = 'pr_opened'`, transition → `pr-open`, budget preserved, `spawn_failures_consecutive` reset. (See test 46a.) c) Worker wrote `.quay-blocked.md` → `exit_kind = 'blocker_written'`, transition → `awaiting-next-brief`, budget preserved, `spawn_failures_consecutive` reset. (See test 46c.) - d) PR existed at spawn and remote did not advance → `exit_kind = 'no_progress'`, schedule `crash` retry, transition → `queued`, budget preserved at promotion's accounting (the retry consumes one unit at its own promotion). - Asserts: convergence to the right outcome per evidence; budget is rolled back **only** in sub-case (a); `spawn_failures_consecutive` increments only in (a) and resets in (b)/(c). + d) Adopted ready-for-review signal exists → `exit_kind = 'adopted_pr_ready_for_review'`, transition → `pr-open`, budget preserved, `spawn_failures_consecutive` reset. + e) Stale PR existed at spawn and remote did not advance, with no current-attempt signal → `exit_kind = 'spawn_failed'`, rollback budget if `consumed_budget = 1`, schedule a fresh attempt, transition back to `queued`. `spawn_failures_consecutive` increments. + Asserts: convergence to the right outcome per evidence; budget is rolled back **only** in sub-cases (a)/(e); `spawn_failures_consecutive` increments only in (a)/(e) and resets in (b)/(c)/(d). ### CI source-of-truth specifics diff --git a/docs/user/cli-reference.md b/docs/user/cli-reference.md index a0268884..a3b63cca 100644 --- a/docs/user/cli-reference.md +++ b/docs/user/cli-reference.md @@ -426,6 +426,8 @@ quay task events quay task claim quay task release-claim --claim-id quay task retarget --repo [--base-branch ] --yes +quay task resnapshot --reason +quay task recreate-worktree --yes [--force] ``` `task claim` only succeeds for `awaiting-next-brief` tasks. @@ -435,6 +437,15 @@ It also includes `authors`, parsed from the ticket's `quay-config.authors` block as `{name, slack_id}` objects. Legacy or malformed rows return `authors: []`. +`task recreate-worktree` recreates an existing task's recorded worktree. By +default it only runs when `task.worktree_path` is missing and no active attempt +is recorded. It uses `origin/` when that remote branch exists; +otherwise it rebuilds from `origin/` while restoring the task +branch name. The repo install command runs after recreation, and Quay records a +`worktree_recreated` event with the recovery base. Use `--force` only after +confirming no worker is live; it allows recreation when the path already exists +or an active attempt is still recorded. + `task list`, `task get`, and `task events` accept legacy `task_id` values and return the same task/run rows as before. JSON output now also includes run-aware compatibility fields: @@ -510,6 +521,22 @@ but before the source reaches terminal state, the next `quay tick` recovers the cancel intent and writes the same source-side `retargeted` audit context from the linked clone. +`task resnapshot` re-baselines a task's frozen `ticket_snapshot` and current +`task_objective` — the definition of done the reviewer enforces and the +objective worker/reviewer prompts load. It re-fetches the task's Linear ticket, +re-parses the `quay-config` block, and re-composes both artifacts with the same +code path enqueue uses. Use it when an operator has edited the live ticket to +change scope mid-flight: without a resnapshot, later prompts keep enforcing the +stale acceptance criteria. It emits a `ticket_resnapshotted` audit event +carrying the required `--reason` and a before/after diff of the snapshot, and it +invalidates the latest review verdict (superseding a standing `approved` / +`changes_requested`) so the next `quay tick` runs a fresh review against the new +snapshot and objective rather than being blocked by a stale verdict. Running it +when the ticket is unchanged is a safe, still-audited no-op: the event is +recorded but the artifacts are not rewritten and no verdict is invalidated. The +command requires the Linear adapter to be configured, and the task must have an +`external_ref`. + ## Submit Brief ```bash diff --git a/docs/user/concepts.md b/docs/user/concepts.md index 053c5902..594063c1 100644 --- a/docs/user/concepts.md +++ b/docs/user/concepts.md @@ -27,9 +27,10 @@ Before enqueueing work, the operator must: At enqueue time, Quay fetches the effective base branch, creates a local branch named `quay/`, creates a worktree from `origin/`, runs the repo `install_cmd`, stores the task-level `task_objective` artifact (the raw -original brief) plus the first attempt's `brief` and `final_prompt` artifacts, -and creates the first pending attempt. The effective base branch is the repo -default unless a task-level override is supplied. +initial brief) plus the first attempt's `brief` and `final_prompt` artifacts, +and creates the first pending attempt. `task resnapshot` can append a newer +task-level objective after the live ticket changes. The effective base branch is +the repo default unless a task-level override is supplied. ## Work Items, Runs, And Attempts @@ -61,8 +62,8 @@ Every attempt has: - A reason, such as `initial`, `ci_fail`, `crash`, `review`, or `blocker_resolved`. -- A `brief` artifact: a structured composed prompt body with the original - task objective (rendered from the task-level `task_objective` artifact), +- A `brief` artifact: a structured composed prompt body with the current + task objective (rendered from the newest task-level `task_objective` artifact), the current attempt's guidance, and any diagnostics for this attempt. - A `final_prompt` artifact: for code-worker attempts, Quay's worker preamble followed by the composed `brief`; for review attempts, Quay's static @@ -91,7 +92,7 @@ A supervisor lock prevents overlapping ticks and serializes side effects with Artifacts are snapshots of data that crosses a boundary, such as: - `ticket_snapshot` -- `task_objective` (task-level; the raw original brief, source of every later attempt's stable objective section) +- `task_objective` (task-level; the canonical brief used by later prompt objective sections) - `brief` (per-attempt composed body) - `final_prompt` (worker preamble + `brief`, or reviewer protocol + guidance + review `brief`) diff --git a/docs/user/configuration.md b/docs/user/configuration.md index 2dc895f1..0760dce3 100644 --- a/docs/user/configuration.md +++ b/docs/user/configuration.md @@ -63,9 +63,6 @@ supervisor_lock_stale_seconds = 30 [adapters.linear] enabled = true api_key_env = "LINEAR_API_KEY" -# Required when reviewer-created human follow-up findings should create -# Linear issues. -default_issue_team_key = "BRIX" # For OAuth/app-actor tokens, use Bearer mode. The token is resolved for each # Linear request, so a Hermes-owned helper can mint/cache/refresh it. # auth_mode = "bearer" @@ -254,10 +251,11 @@ are probed against the target repository before their attempts are promoted. Worker tokens are checked before tmux starts by exercising repo access, branch PR visibility, and worker write access. Invalid, expired, empty, missing, or repo-inaccessible worker tokens fail as `worker_auth_invalid`: Quay retries one -freshly resolved token source, then moves the task to `awaiting-next-brief` -with a `worker_auth_invalid` handoff if the preflight still fails. Reviewer -auth failures remain `spawn_substrate_failed` and stay out of the -`review_infra_failed` retry accounting. +freshly resolved token source after the task's spawn backoff expires, then moves +the task to `awaiting-next-brief` with a `worker_auth_invalid` handoff if the +preflight still fails. Reviewer auth failures remain `spawn_substrate_failed` +until the reviewer infrastructure failure threshold parks the task in +`non_budget_loop`. `worker.gh_token_file` is the preferred worker source when configured, even if `QUAY_WORKER_GH_TOKEN` is also present in the tick environment. diff --git a/docs/user/linear-and-slack.md b/docs/user/linear-and-slack.md index 6e33e294..f50b24e8 100644 --- a/docs/user/linear-and-slack.md +++ b/docs/user/linear-and-slack.md @@ -11,7 +11,6 @@ see [External Services Setup](external-services.md). [adapters.linear] enabled = true api_key_env = "LINEAR_API_KEY" -default_issue_team_key = "BRIX" [adapters.slack] enabled = true diff --git a/docs/user/monitoring-and-artifacts.md b/docs/user/monitoring-and-artifacts.md index 23a7656e..c7ba2864 100644 --- a/docs/user/monitoring-and-artifacts.md +++ b/docs/user/monitoring-and-artifacts.md @@ -81,9 +81,9 @@ binary and invalid UTF-8 artifacts such as `malformed_signal`. | Kind | Meaning | | --- | --- | -| `ticket_snapshot` | Snapshot of source ticket/context at enqueue time. | -| `task_objective` | Stable original task brief, written once at enqueue and reused by every later code-worker attempt as the canonical objective. Task-level (no `attempt_id`). | -| `brief` | Per-attempt composed prompt body: a structured `` block pointing at the task-level `task_objective` artifact, a `` block (initial instruction, retry/respawn template, or orchestrator-submitted brief), and an optional `` block (CI excerpt, review comments, conflict slice, etc.). The raw original brief lives in `task_objective`, not here. | +| `ticket_snapshot` | Snapshot of source ticket/context. Enqueue writes the first task-level row; `task resnapshot` can append a newer task-level snapshot after the live ticket changes. | +| `task_objective` | Canonical task brief reused by later code-worker and reviewer prompts. Enqueue writes the first task-level row; `task resnapshot` can append a newer task-level objective. | +| `brief` | Per-attempt composed prompt body: a structured `` block pointing at the current task-level `task_objective` artifact, a `` block (initial instruction, retry/respawn template, or orchestrator-submitted brief), and an optional `` block (CI excerpt, review comments, conflict slice, etc.). The current task objective lives in `task_objective`, not here. | | `final_prompt` | Code-worker attempts: worker preamble plus the attempt's composed `brief`. Review attempts: static reviewer protocol, reviewer guidance, then the review `brief`. | | `session_log` | Captured tmux output. | | `usage` | JSON usage envelope captured per attempt. `.quay-usage.json` is stored verbatim when present; otherwise Codex `--json` JSONL in `.quay-tool-trace.log` can synthesize normalized model/token totals. | @@ -111,5 +111,7 @@ a `tick_error` event and continues processing other tasks. A later successful tick path clears the task's `tick_error` field. Worker GitHub auth preflight failures are not `tick_error`: they are classified -as `worker_auth_invalid`, retried once with freshly resolved credentials, then -surfaced through an `awaiting-next-brief` handoff if the retry also fails. +as `worker_auth_invalid`, retried once after `spawn_retry_next_eligible_at` with +freshly resolved credentials, then surfaced through an `awaiting-next-brief` +handoff if the retry also fails. Spawn/reviewer infrastructure failures record +their latest operator-facing reason in `tasks.spawn_failure_reason`. diff --git a/docs/user/troubleshooting.md b/docs/user/troubleshooting.md index f4f24ef9..32ce325a 100644 --- a/docs/user/troubleshooting.md +++ b/docs/user/troubleshooting.md @@ -179,8 +179,9 @@ not consume retry budget. ## `worker_auth_invalid` The worker GitHub token failed Quay's spawn-time auth preflight. Quay retries -once after re-reading the configured token source. If the retry also fails, the -task moves to `awaiting-next-brief` with a `worker_auth_invalid` handoff. +once after re-reading the configured token source and waiting for +`spawn_retry_next_eligible_at`. If the retry also fails, the task moves to +`awaiting-next-brief` with a `worker_auth_invalid` handoff. Check the configured worker token source: @@ -201,7 +202,23 @@ quay task events quay artifact get session_log --path ``` -Manual recovery is currently cancellation: +`tasks.spawn_failure_reason` records the latest spawn or reviewer +infrastructure diagnostic, and `tasks.spawn_retry_next_eligible_at` shows when a +non-parked retry becomes eligible. + +If the task row still points at a missing `worktree_path`, recreate that +recorded worktree without DB surgery: + +```bash +quay task recreate-worktree --yes +``` + +Quay prefers `origin/` when it exists. If that branch is gone +from the remote, Quay rebuilds from `origin/` and checks out the +task branch name again. The command refuses existing paths and active attempts +unless `--force` is supplied. + +Cancellation remains available when the task should not continue: ```bash quay cancel --keep-worktree diff --git a/packages/admin-ui/src/api/quayAdmin.ts b/packages/admin-ui/src/api/quayAdmin.ts index 6a847d14..717aa587 100644 --- a/packages/admin-ui/src/api/quayAdmin.ts +++ b/packages/admin-ui/src/api/quayAdmin.ts @@ -47,6 +47,7 @@ export interface QuayAdminRepo { model_reviewer: string | null; preamble_worker: number | null; preamble_reviewer: number | null; + review_finding_linear_enabled: boolean | null; archived_at: string | null; created_at: string; } @@ -203,6 +204,9 @@ interface QuayAdminGlobal { ignored_workflow_names: string[]; }; adapters: QuayAdminAdapter[]; + review_findings: { + linear_enabled: boolean; + }; agents: { defaults: QuayAdminAgentDefaults; invocations: QuayAdminAgentInvocation[]; @@ -461,13 +465,17 @@ function toRepoSummary(repo: QuayAdminRepoDetail): RepoSummary { repo.preamble_worker, repo.preamble_reviewer, ].filter(Boolean); + // Tri-state override: both on (true) and off (false) count as overrides; + // only inherit (null) does not. `filter(Boolean)` above would drop `false`, + // so count this one by non-null instead. + const reviewFindingOverrides = repo.review_finding_linear_enabled != null ? 1 : 0; return { revision: repo.revision, id: repo.repo_id, active: repo.active_task_count, agent: repo.agent_worker ?? 'inherits', - overrides: overrideFields.length, + overrides: overrideFields.length + reviewFindingOverrides, url: repo.repo_url, baseBranch: repo.base_branch, createdAt: repo.created_at, @@ -482,6 +490,7 @@ function toRepoSummary(repo: QuayAdminRepoDetail): RepoSummary { modelReviewer: repo.model_reviewer, preambleWorker: repo.preamble_worker, preambleReviewer: repo.preamble_reviewer, + reviewFindingLinearEnabled: repo.review_finding_linear_enabled, ciPolicy: { ignoreMode: repo.ci_policy.ignore_mode, ignoredCheckNames: repo.ci_policy.ignored_check_names, @@ -537,6 +546,9 @@ function toGlobalSummary(global: QuayAdminGlobal): GlobalConfigSummary { ignoredWorkflowNames: global.ci_policy.ignored_workflow_names, }, adapters: global.adapters.map(toAdapterSummary), + reviewFindings: { + linearEnabled: global.review_findings.linear_enabled, + }, agents: { defaults: { worker: global.agents.defaults.worker, diff --git a/packages/admin-ui/src/screens/GlobalScreen.tsx b/packages/admin-ui/src/screens/GlobalScreen.tsx index f8e3c58b..49e43997 100644 --- a/packages/admin-ui/src/screens/GlobalScreen.tsx +++ b/packages/admin-ui/src/screens/GlobalScreen.tsx @@ -157,6 +157,7 @@ interface SettingsBodyProps { function SettingsBody({ global, changes, onChange, onOpenPreamble, active, setActive }: SettingsBodyProps) { const deploymentSettings = deploymentSettingsFields(global, changes, onChange); + const reviewFindings = reviewFindingLinearToggle(global, changes, onChange); return (
( ))} + +
+
+ + + File non-blocking review findings as Linear issues + + {reviewFindings.dirty && ( + + edited + + )} + + + Gates whether a synthetic_review finding is enqueued as a Linear issue. + Worker-authored (quay_owned) tasks never create issues regardless of this + switch. This is policy — distinct from the Linear adapter connectivity flag + above. Repos may override it under their own Review findings control. + +
+ reviewFindings.commit(next)} + /> +
+
; function deploymentSettingsFields( global: GlobalConfigSummary, @@ -570,6 +613,41 @@ function deploymentSettingsFields( }; } +const REVIEW_FINDING_LINEAR_CHANGE_ID = 'deployment_settings:review_finding_linear_enabled'; + +function reviewFindingLinearToggle( + global: GlobalConfigSummary, + changes: ChangeEntry[], + onChange: (entry: ChangeEntry) => void, +) { + const baseline = global.reviewFindings.linearEnabled; + + function pending(): boolean | undefined { + const entry = changes.find((change) => change.id === REVIEW_FINDING_LINEAR_CHANGE_ID); + if (entry?.change.type !== 'deployment_settings.update') return undefined; + const value = entry.change.patch.review_finding_linear_enabled; + return typeof value === 'boolean' ? value : undefined; + } + + return { + value: pending() ?? baseline, + dirty: changes.some((change) => change.id === REVIEW_FINDING_LINEAR_CHANGE_ID), + commit(next: boolean): void { + onChange({ + id: REVIEW_FINDING_LINEAR_CHANGE_ID, + scope: 'global', + label: 'deployment review_finding_linear_enabled', + before: baseline ? 'on' : 'off', + after: next ? 'on' : 'off', + change: { + type: 'deployment_settings.update', + patch: { review_finding_linear_enabled: next }, + }, + }); + }, + }; +} + function formatList(values: string[]): string { return values.length === 0 ? '[]' : values.join(', '); } diff --git a/packages/admin-ui/src/screens/RepoScreen.tsx b/packages/admin-ui/src/screens/RepoScreen.tsx index 8fd12223..54b5bfe4 100644 --- a/packages/admin-ui/src/screens/RepoScreen.tsx +++ b/packages/admin-ui/src/screens/RepoScreen.tsx @@ -2,6 +2,7 @@ import { useEffect, useState } from 'react'; import { Badge } from '../components/Badge'; import { Button } from '../components/Button'; import { Card } from '../components/Card'; +import { Segmented } from '../components/Segmented'; import { HStack } from '../components/Stack'; import { T } from '../components/Typography'; import { Icon } from '../icons/Icon'; @@ -117,6 +118,7 @@ interface BodyProps { function Body({ repo, global, changes, onChange, active, setActive }: BodyProps) { const field = createRepoFieldAccess(repo, changes, onChange); const guidance = createRepoGuidanceAccess(repo, changes, onChange); + const reviewFindings = createRepoReviewFindingAccess(repo, changes, onChange); const globalWorkerPreamble = global.preambles.find((preamble) => preamble.kind === 'code') ?? null; const globalReviewerPreamble = global.preambles.find((preamble) => preamble.kind === 'review') ?? null; const workerBasePreamble = effectiveWithPending( @@ -335,6 +337,47 @@ function Body({ repo, global, changes, onChange, active, setActive }: BodyProps) onCommit={(next) => field.commit('model_reviewer', next)} /> + +
+ + + File non-blocking findings as Linear issues + + + {reviewFindings.source} + + {reviewFindings.dirty && ( + + edited + + )} + + reviewFindings.commit(next)} + /> + + + Inherit follows the global default ( + {global.reviewFindings.linearEnabled ? 'on' : 'off'}). Worker-authored + (quay_owned) tasks never create issues regardless. Off suppresses the Linear + issue only; findings are still persisted and still posted in the PR review. + +
+
{/* 04 · Prompts */} @@ -414,7 +457,9 @@ function Body({ repo, global, changes, onChange, active, setActive }: BodyProps) } type RepoPatch = RepoUpdateChange['patch']; -type RepoPatchKey = keyof RepoPatch; +// The string/number-valued repo fields; the boolean review-finding override is +// handled separately by createRepoReviewFindingAccess. +type RepoPatchKey = keyof Omit; function createRepoFieldAccess( repo: RepoSummary, @@ -480,6 +525,62 @@ function createRepoFieldAccess( }; } +type ReviewFindingTriState = 'inherit' | 'on' | 'off'; + +function triStateFromValue(value: boolean | null): ReviewFindingTriState { + return value === null ? 'inherit' : value ? 'on' : 'off'; +} + +function valueFromTriState(state: ReviewFindingTriState): boolean | null { + return state === 'inherit' ? null : state === 'on'; +} + +// Per-repo tri-state override for filing non-blocking review findings as +// Linear issues. Wires through the same repo.update change machinery as the +// Agents overrides; `source` mirrors that block (override when set, else +// inherits) and feeds the Overview "N overrides from Global" counter. +function createRepoReviewFindingAccess( + repo: RepoSummary, + changes: ChangeEntry[], + onChange: (entry: ChangeEntry) => void, +) { + const changeId = `repo:${repo.id}:review_finding_linear_enabled`; + const baseline: boolean | null = repo.reviewFindingLinearEnabled ?? null; + + function pending(): boolean | null | undefined { + const entry = changes.find((change) => change.id === changeId); + if (entry?.change.type !== 'repo.update') return undefined; + if (!('review_finding_linear_enabled' in entry.change.patch)) return undefined; + return entry.change.patch.review_finding_linear_enabled ?? null; + } + + const current = (() => { + const p = pending(); + return p === undefined ? baseline : p; + })(); + + return { + state: triStateFromValue(current), + source: current === null ? 'inherits' : 'override', + dirty: changes.some((change) => change.id === changeId), + commit(state: ReviewFindingTriState): void { + const after = valueFromTriState(state); + onChange({ + id: changeId, + scope: repo.id, + label: `${repo.id} review_finding_linear_enabled`, + before: triStateFromValue(baseline), + after: triStateFromValue(after), + change: { + type: 'repo.update', + repo_id: repo.id, + patch: { review_finding_linear_enabled: after }, + }, + }); + }, + }; +} + type GuidanceRole = RepoGuidanceSetChange['role']; function createRepoGuidanceAccess( diff --git a/packages/admin-ui/src/screens/configurationAgentContext.ts b/packages/admin-ui/src/screens/configurationAgentContext.ts index 36b972dc..56b2d849 100644 --- a/packages/admin-ui/src/screens/configurationAgentContext.ts +++ b/packages/admin-ui/src/screens/configurationAgentContext.ts @@ -271,6 +271,7 @@ function repoPendingValue(repo: RepoSummary, changes: ChangeEntry[]) { if (entry?.change.type !== 'repo.update') return fallback; const pending = entry.change.patch[field as keyof typeof entry.change.patch]; if (typeof pending === 'number') return String(pending); + if (typeof pending === 'boolean') return pending ? 'on' : 'off'; return pending === undefined ? fallback : pending; }; } diff --git a/packages/admin-ui/src/store/data.ts b/packages/admin-ui/src/store/data.ts index 96494c8f..bbb2f599 100644 --- a/packages/admin-ui/src/store/data.ts +++ b/packages/admin-ui/src/store/data.ts @@ -20,6 +20,9 @@ export interface RepoSummary { modelReviewer?: string | null; preambleWorker?: number | null; preambleReviewer?: number | null; + // Per-repo override for filing non-blocking review findings as Linear + // issues. null = inherit the global default, true = on, false = off. + reviewFindingLinearEnabled?: boolean | null; ciPolicy: RepoCiPolicySummary; effectivePreambles: { worker: RepoEffectivePreamble; @@ -172,6 +175,11 @@ export interface GlobalConfigSummary { ignoredWorkflowNames: string[]; }; adapters: AdapterSummary[]; + reviewFindings: { + // Global default (policy) for filing non-blocking review findings as + // Linear issues. Distinct from the Linear adapter's connectivity flag. + linearEnabled: boolean; + }; agents: { defaults: AgentDefaults; invocations: AgentInvocation[]; diff --git a/packages/admin-ui/src/store/dirty.ts b/packages/admin-ui/src/store/dirty.ts index 0b5dd3e1..ab40d145 100644 --- a/packages/admin-ui/src/store/dirty.ts +++ b/packages/admin-ui/src/store/dirty.ts @@ -33,6 +33,7 @@ export interface RepoUpdateChange { model_reviewer: string | null; preamble_worker: string | number | null; preamble_reviewer: string | number | null; + review_finding_linear_enabled: boolean | null; }>; } @@ -62,6 +63,7 @@ export interface DeploymentSettingsUpdateChange { worker_model: string | null; reviewer_agent: string | null; reviewer_model: string | null; + review_finding_linear_enabled: boolean | null; }>; } diff --git a/packages/cli/migrations/0014_task_objective_backfill.sql b/packages/cli/migrations/0014_task_objective_backfill.sql index 9615e61c..5127c0ef 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/migrations/0040_review_finding_linear_toggle.sql b/packages/cli/migrations/0040_review_finding_linear_toggle.sql new file mode 100644 index 00000000..cd1ace53 --- /dev/null +++ b/packages/cli/migrations/0040_review_finding_linear_toggle.sql @@ -0,0 +1,18 @@ +-- Toggle for review-finding -> Linear issue creation (BRIX-1898). +-- +-- Global default lives on deployment_settings; per-repo override lives on +-- repos. Both are nullable INTEGER tri-states: +-- NULL = unset / inherit, 1 = on, 0 = off. +-- +-- Resolution at the enqueue gate (tick.ts persistReviewFindings -> +-- enqueueReviewFindingLinearIssues): repo value if non-NULL, else the global +-- default, else ON. Only `synthetic_review` tasks ever reach this gate, so the +-- switch never changes worker-authored (`quay_owned`) behavior. Turning it off +-- suppresses the `review_finding_linear_issue` outbox row only; findings are +-- still persisted and still posted in the PR review. + +ALTER TABLE deployment_settings ADD COLUMN review_finding_linear_enabled INTEGER + CHECK (review_finding_linear_enabled IN (0, 1)); + +ALTER TABLE repos ADD COLUMN review_finding_linear_enabled INTEGER + CHECK (review_finding_linear_enabled IN (0, 1)); diff --git a/packages/cli/migrations/0041_spawn_failure_backoff.sql b/packages/cli/migrations/0041_spawn_failure_backoff.sql new file mode 100644 index 00000000..80e77b35 --- /dev/null +++ b/packages/cli/migrations/0041_spawn_failure_backoff.sql @@ -0,0 +1,6 @@ +ALTER TABLE tasks ADD COLUMN spawn_retry_next_eligible_at TEXT; +ALTER TABLE tasks ADD COLUMN spawn_failure_reason TEXT; + +CREATE INDEX tasks_spawn_retry_eligible_idx + ON tasks(state, spawn_retry_next_eligible_at) + WHERE state IN ('queued', 'pr-review'); diff --git a/packages/cli/src/adapters/git.ts b/packages/cli/src/adapters/git.ts index c130f1ca..9f551a93 100644 --- a/packages/cli/src/adapters/git.ts +++ b/packages/cli/src/adapters/git.ts @@ -455,6 +455,15 @@ export class LocalGitAdapter implements GitPort { } } + worktreePrune(repoId: string): void { + const result = runIn(this.bareDir(repoId), ["git", "worktree", "prune"]); + if (result.exitCode !== 0) { + throw new Error( + `git worktree prune failed for ${repoId}: ${result.stderr.trim() || result.stdout.trim()}`, + ); + } + } + branchDelete(repoId: string, branch: string): void { const result = runIn(this.bareDir(repoId), [ "git", diff --git a/packages/cli/src/adapters/github.ts b/packages/cli/src/adapters/github.ts index f78d73e9..9ce67e5d 100644 --- a/packages/cli/src/adapters/github.ts +++ b/packages/cli/src/adapters/github.ts @@ -36,6 +36,7 @@ import { join, resolve } from "node:path"; import { GitHubMergeError } from "../ports/github.ts"; import type { + GitHubMergeErrorKind, GitHubPort, GitHubGraphqlRateLimit, OpenBranchPr, @@ -59,8 +60,15 @@ export interface RunResult { stderr: string; } +export type MergeMethod = "merge" | "squash" | "rebase"; + export class GitHubCliAdapter implements GitHubPort { private cachedLogin: string | null = null; + // Allowed merge method per repo_id. The repo's merge-method policy is stable + // across a run, so we read it once and reuse it for every subsequent merge + // (an umbrella's child PRs all merge into the same repo). See + // `allowedMergeMethod`. + private readonly cachedMergeMethod = new Map(); constructor(private readonly reposRoot: string) {} @@ -268,34 +276,73 @@ export class GitHubCliAdapter implements GitHubPort { prNumber: number, expectedHeadSha: string, ): void { + // Pick a merge method the target repo actually allows. The historical + // hardcoded `--merge` fails on squash-only repos ("Merge commits are not + // allowed on this repository"); read the repo's policy and pass the + // matching flag (BRIX-1920). + const methodFlag = MERGE_METHOD_FLAG[this.allowedMergeMethod(repoId)]; const result = this.run(repoId, [ "gh", "pr", "merge", String(prNumber), - "--merge", + methodFlag, "--match-head-commit", expectedHeadSha, ]); if (result.exitCode === 0) return; const msg = `${result.stdout}\n${result.stderr}`; - const lower = msg.toLowerCase(); - const kind = - lower.includes("head branch was modified") || - lower.includes("head sha") || - lower.includes("head commit") - ? "head_mismatch" - : lower.includes("not mergeable") || - lower.includes("merge conflict") || - lower.includes("cannot be merged") - ? "not_mergeable" - : "unknown"; throw new GitHubMergeError( `gh pr merge ${prNumber} failed: ${msg.trim()}`, - kind, + classifyMergeErrorKind(msg), ); } + // Resolve an allowed merge method for the repo by reading its merge-method + // policy (`allow_merge_commit` / `allow_squash_merge` / `allow_rebase_merge`) + // via `gh api repos/{owner}/{repo}` — gh resolves the `{owner}/{repo}` + // placeholders from the bare clone's `origin`, same as `probeTokenAccess`. + // + // GitHub exposes no single "default method" field, so we pick + // deterministically: prefer `merge` (byte-identical to the historical + // `--merge` wherever it's allowed), else `squash`, else `rebase`. If the + // repo allows none (shouldn't happen), throw a clear error. Cached per + // repo_id — the policy is stable and every umbrella child merge in the same + // repo would otherwise re-read it. + private allowedMergeMethod(repoId: string): MergeMethod { + const cached = this.cachedMergeMethod.get(repoId); + if (cached !== undefined) return cached; + const result = this.run(repoId, ["gh", "api", "repos/{owner}/{repo}"]); + if (result.exitCode !== 0) { + throw new Error( + `gh api repos/{owner}/{repo} (merge methods) failed: ${result.stderr.trim() || result.stdout.trim()}`, + ); + } + let parsed: Record; + try { + parsed = JSON.parse(result.stdout) as Record; + } catch (err) { + throw new Error( + `gh api repos/{owner}/{repo} returned unparseable JSON for merge methods: ${(err as Error).message}`, + ); + } + const method: MergeMethod | null = + parsed.allow_merge_commit === true + ? "merge" + : parsed.allow_squash_merge === true + ? "squash" + : parsed.allow_rebase_merge === true + ? "rebase" + : null; + if (method === null) { + throw new Error( + `repo "${repoId}" allows no merge method (merge, squash and rebase are all disabled); cannot auto-merge`, + ); + } + this.cachedMergeMethod.set(repoId, method); + return method; + } + getGraphqlRateLimit(repoId: string): GitHubGraphqlRateLimit | null { const result = this.run(repoId, ["gh", "api", "rate_limit"]); if (result.exitCode !== 0) return null; @@ -1508,6 +1555,50 @@ function mapMergeable(raw: unknown): PrMergeableState { return "unknown"; } +// `gh pr merge` flag for each resolved merge method. +const MERGE_METHOD_FLAG: Record = { + merge: "--merge", + squash: "--squash", + rebase: "--rebase", +}; + +// Classify a `gh pr merge` failure from its combined stdout+stderr. Pure and +// deterministic — exported so unit tests can exercise it without a `gh` +// binary. Order matters: the policy-rejection check runs first because a +// repo that forbids the chosen method is a distinct, NON-RETRYABLE failure +// (BRIX-1921) that must not be miscategorised as `unknown` and retried every +// tick. +export function classifyMergeErrorKind(message: string): GitHubMergeErrorKind { + const lower = message.toLowerCase(); + // Policy rejection: the repo disallows the merge method we used. gh surfaces + // this as "Merge commits are not allowed on this repository", "Squash merges + // are not allowed on this repository", "Rebase merges are not allowed on + // this repository", or a bare "merge method ... is not allowed" from the + // GraphQL merge mutation. + if ( + lower.includes("not allowed on this repository") || + lower.includes("merge method is not allowed") || + lower.includes("merge method not allowed") + ) { + return "method_not_allowed"; + } + if ( + lower.includes("head branch was modified") || + lower.includes("head sha") || + lower.includes("head commit") + ) { + return "head_mismatch"; + } + if ( + lower.includes("not mergeable") || + lower.includes("merge conflict") || + lower.includes("cannot be merged") + ) { + return "not_mergeable"; + } + return "unknown"; +} + // Stable key for matching a check row across the two `gh pr checks` calls // (unfiltered + `--required`). `gh` does not surface an opaque id, so we // identify a check by `(workflow, name)`. The `\x1f` separator ensures the diff --git a/packages/cli/src/adapters/linear.ts b/packages/cli/src/adapters/linear.ts index 7d7bbb36..60d96380 100644 --- a/packages/cli/src/adapters/linear.ts +++ b/packages/cli/src/adapters/linear.ts @@ -25,8 +25,6 @@ import { QuayError } from "../core/errors.ts"; import type { LinearBlockedByRelation, LinearComment, - LinearCreatedIssue, - LinearCreateIssueInput, LinearHierarchyIssue, LinearIssue, LinearIssueHierarchy, @@ -153,24 +151,6 @@ interface RawTeamStates { states: { nodes: RawTeamStateNode[] }; } -interface RawTeamNode { - id: string; - key: string; -} - -interface RawTeamsByKey { - teams: { nodes: RawTeamNode[] }; -} - -interface RawIssueCreatePayload { - success: boolean; - issue: { - id: string; - identifier: string; - url: string; - } | null; -} - interface GraphQLEnvelope { data?: T | null; errors?: Array<{ message?: string; extensions?: Record }>; @@ -183,7 +163,6 @@ export class LinearAdapter implements LinearPort { private readonly authMode: LinearAuthMode; private readonly tokenCommand: string | null; private readonly tokenProvider: LinearTokenProvider | null; - private readonly defaultIssueTeamKey: string | null; private readonly timeoutMs: number; private readonly transport: LinearTransport; // Per-team workflow state cache (state-name → state-id). Linear's workflow @@ -213,7 +192,6 @@ export class LinearAdapter implements LinearPort { authMode?: LinearAuthMode; tokenCommand?: string; tokenProvider?: LinearTokenProvider; - defaultIssueTeamKey?: string; endpoint?: string; timeoutMs?: number; transport?: LinearTransport; @@ -230,11 +208,6 @@ export class LinearAdapter implements LinearPort { ? opts.tokenCommand : null; this.tokenProvider = opts?.tokenProvider ?? null; - this.defaultIssueTeamKey = - opts?.defaultIssueTeamKey !== undefined && - opts.defaultIssueTeamKey.trim() !== "" - ? opts.defaultIssueTeamKey.trim() - : null; this.endpoint = opts?.endpoint ?? DEFAULT_LINEAR_ENDPOINT; this.timeoutMs = opts?.timeoutMs !== undefined && opts.timeoutMs > 0 @@ -408,42 +381,6 @@ export class LinearAdapter implements LinearPort { this.rememberSyncedState(identifier, stateName); } - async createIssue(input: LinearCreateIssueInput): Promise { - const teamKey = (input.teamKey ?? this.defaultIssueTeamKey)?.trim(); - if (teamKey === undefined || teamKey === "") { - throw new QuayError( - "adapter_not_configured", - "LinearAdapter requires adapters.linear.default_issue_team_key to create issues", - { adapter: "linear", config_key: "adapters.linear.default_issue_team_key" }, - ); - } - const teamId = await this.resolveTeamIdByKey(teamKey); - const response = await this.postGraphQL("issueCreate", CREATE_ISSUE_MUTATION, { - input: { - id: normalizeOptionalString(input.idempotencyKey), - teamId, - title: input.title, - description: input.body, - }, - }); - const parsed = this.parseGraphQLEnvelope<{ - issueCreate: RawIssueCreatePayload | null; - }>("issueCreate", response); - const payload = parsed?.issueCreate ?? null; - if (payload === null || payload.success !== true || payload.issue === null) { - throw new QuayError( - "adapter_error", - "Linear issueCreate returned no created issue", - { adapter: "linear", retryable: false }, - ); - } - return { - id: payload.issue.id, - identifier: payload.issue.identifier, - url: payload.issue.url, - }; - } - async updateIssueBody(identifier: string, body: string): Promise { const response = await this.postGraphQL( identifier, @@ -766,31 +703,6 @@ export class LinearAdapter implements LinearPort { return map; } - private async resolveTeamIdByKey(teamKey: string): Promise { - const response = await this.postGraphQL(teamKey, GET_TEAM_BY_KEY_QUERY, { - key: teamKey, - }); - const parsed = this.parseGraphQLEnvelope(teamKey, response); - const nodes = parsed?.teams.nodes ?? []; - if (nodes.length === 0) { - throw new QuayError( - "adapter_error", - `Linear team key ${teamKey} was not found`, - { adapter: "linear", retryable: false, team_key: teamKey }, - ); - } - if (nodes.length > 1) { - throw new QuayError( - "adapter_error", - `Linear team key ${teamKey} matched multiple teams`, - { adapter: "linear", retryable: false, team_key: teamKey }, - ); - } - const node = nodes[0]; - if (node === undefined) throw new Error("Linear team lookup invariant failed"); - return node.id; - } - private async runIssueUpdate( identifier: string, stateId: string, @@ -1058,12 +970,6 @@ const GET_TEAM_STATES_QUERY = `query GetTeamStates($teamId: String!, $statesFirs } }`; -const GET_TEAM_BY_KEY_QUERY = `query GetTeamByKey($key: String!) { - teams(filter: { key: { eq: $key } }, first: 2) { - nodes { id key } - } -}`; - // `issueUpdate` with a `stateId` input is Linear's canonical state move. The // adapter never persists the returned issue body — only `success` matters. const UPDATE_ISSUE_STATE_MUTATION = `mutation UpdateIssueState($id: String!, $stateId: String!) { @@ -1078,17 +984,6 @@ const UPDATE_ISSUE_BODY_MUTATION = `mutation UpdateIssueBody($id: String!, $desc } }`; -const CREATE_ISSUE_MUTATION = `mutation CreateIssue($input: IssueCreateInput!) { - issueCreate(input: $input) { - success - issue { - id - identifier - url - } - } -}`; - function isDraftIssue(issue: RawLinearIssue): boolean { // Defensive check per adapters spec §17: Linear's `issue()` query usually // returns 404 for drafts (drafts live on a separate `IssueDraft` entity in @@ -1177,11 +1072,6 @@ function truncate(s: string): string { return `${s.slice(0, 500)}... (truncated, ${s.length} bytes total)`; } -function normalizeOptionalString(value: string | null | undefined): string | undefined { - const trimmed = value?.trim(); - return trimmed === undefined || trimmed === "" ? undefined : trimmed; -} - function resolveTimeoutFromEnv(): number { const raw = process.env.QUAY_LINEAR_TIMEOUT_MS; if (raw === undefined || raw === "") return DEFAULT_LINEAR_TIMEOUT_MS; diff --git a/packages/cli/src/admin/api.ts b/packages/cli/src/admin/api.ts index 9452b562..2a0cbd1f 100644 --- a/packages/cli/src/admin/api.ts +++ b/packages/cli/src/admin/api.ts @@ -400,6 +400,7 @@ const deploymentSettingsPatchSchema = z worker_model: z.string().min(1).nullable().optional(), reviewer_agent: z.string().min(1).nullable().optional(), reviewer_model: z.string().min(1).nullable().optional(), + review_finding_linear_enabled: z.boolean().nullable().optional(), }) .strict(); @@ -876,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, ( @@ -885,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 @@ -1437,6 +1438,12 @@ function buildGlobalReadModel(runtime: AdminApiRuntime): Record ignored_workflow_names: ciPolicyFromConfig(runtime.config).ignoredWorkflowNames, }, adapters: buildAdapterSummaries(runtime), + review_findings: { + // Global default (policy, distinct from the Linear adapter's + // connectivity `enabled` flag). Gates the review-finding -> Linear + // issue enqueue for `synthetic_review` tasks only. + linear_enabled: effectiveReviewFindingLinearEnabled(runtime), + }, agents: { defaults: { worker: agentSelection.defaults.worker, @@ -2171,6 +2178,7 @@ const DEPLOYMENT_SETTINGS_PATCH_FIELDS = [ "worker_model", "reviewer_agent", "reviewer_model", + "review_finding_linear_enabled", ] as const satisfies readonly (keyof DeploymentSettingsPatch)[]; const REPO_PATCH_FIELDS = [ @@ -2187,6 +2195,7 @@ const REPO_PATCH_FIELDS = [ "model_reviewer", "preamble_worker", "preamble_reviewer", + "review_finding_linear_enabled", "ci_ignore_mode", "ignored_check_names", "ignored_workflow_names", @@ -2362,6 +2371,7 @@ function deploymentSettings(runtime: AdminApiRuntime): { worker_model: row?.worker_model ?? null, reviewer_agent: row?.reviewer_agent ?? null, reviewer_model: row?.reviewer_model ?? null, + review_finding_linear_enabled: row?.review_finding_linear_enabled ?? null, }, }; } @@ -2377,9 +2387,16 @@ function effectiveDeploymentSettings(runtime: AdminApiRuntime): DeploymentSettin worker_model: selection.defaultModels?.worker ?? null, reviewer_agent: selection.defaults.reviewer, reviewer_model: selection.defaultModels?.reviewer ?? null, + review_finding_linear_enabled: effectiveReviewFindingLinearEnabled(runtime), }; } +// Global default for filing non-blocking review findings as Linear issues: +// the stored deployment setting when present, else ON. +function effectiveReviewFindingLinearEnabled(runtime: AdminApiRuntime): boolean { + return deploymentSettingsRow(runtime)?.review_finding_linear_enabled ?? true; +} + function stableJson(value: unknown): string { if (value === null) return "null"; if (Array.isArray(value)) { diff --git a/packages/cli/src/build/embedded.generated.ts b/packages/cli/src/build/embedded.generated.ts index 8f17a571..2dd2a836 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+2084ad3"; +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" }, @@ -49,6 +49,8 @@ export const EMBEDDED_MIGRATIONS: readonly Migration[] = [ { name: "0039_repo_guidance.sql", sql: "-- Additive per-repo prompt guidance.\n--\n-- `preambles` remains the full global/pinned preamble stream. This table is\n-- a separate append-only stream for small repo-specific appendices that compose\n-- on top of the selected preamble instead of replacing it.\n\nCREATE TABLE repo_guidance (\n guidance_id INTEGER PRIMARY KEY AUTOINCREMENT,\n repo_id TEXT NOT NULL REFERENCES repos(repo_id),\n role TEXT NOT NULL CHECK (role IN ('worker', 'reviewer')),\n body TEXT NOT NULL,\n created_at TEXT NOT NULL\n);\n\nCREATE INDEX repo_guidance_repo_role_idx\n ON repo_guidance(repo_id, role, guidance_id DESC);\n\nALTER TABLE attempts ADD COLUMN repo_guidance_id INTEGER REFERENCES repo_guidance(guidance_id);\n\nCREATE INDEX attempts_repo_guidance_id_idx\n ON attempts(repo_guidance_id)\n WHERE repo_guidance_id IS NOT NULL;\n" }, { name: "0039_work_items_task_type.sql", sql: "ALTER TABLE work_items\n ADD COLUMN task_type TEXT\n CHECK (task_type IS NULL OR task_type IN ('bugfix', 'feature', 'chore', 'refactor'));\n" }, { name: "0039_worker_auth_invalid_handoff.sql", sql: "-- Add a dedicated orchestrator handoff reason for worker GitHub auth\n-- preflight failures that persisted after Quay's one fresh-auth retry.\n-- quay: foreign_keys_off\n-- SQLite cannot alter CHECK constraints in place, so rebuild while\n-- preserving existing rows.\nPRAGMA legacy_alter_table = ON;\n\nALTER TABLE orchestrator_handoffs RENAME TO orchestrator_handoffs_old;\n\nCREATE TABLE orchestrator_handoffs (\n handoff_id INTEGER PRIMARY KEY AUTOINCREMENT,\n outbox_item_id INTEGER REFERENCES outbox_items(outbox_item_id),\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 'worker_auth_invalid'\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 next_eligible_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, outbox_item_id, task_id, reason, state_event_id,\n idempotency_key, payload_json, status, claim_id, claimed_at,\n completed_at, next_eligible_at, created_at, updated_at\n)\nSELECT\n handoff_id, outbox_item_id, task_id, reason, state_event_id,\n idempotency_key, payload_json, status, claim_id, claimed_at,\n completed_at, next_eligible_at, created_at, 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 INDEX orchestrator_handoffs_pending_eligible_idx\n ON orchestrator_handoffs(status, next_eligible_at, created_at, handoff_id)\n WHERE status = 'pending';\n\nCREATE INDEX orchestrator_handoffs_outbox_item_idx\n ON orchestrator_handoffs(outbox_item_id);\n\nPRAGMA legacy_alter_table = OFF;\n" }, + { name: "0040_review_finding_linear_toggle.sql", sql: "-- Toggle for review-finding -> Linear issue creation (BRIX-1898).\n--\n-- Global default lives on deployment_settings; per-repo override lives on\n-- repos. Both are nullable INTEGER tri-states:\n-- NULL = unset / inherit, 1 = on, 0 = off.\n--\n-- Resolution at the enqueue gate (tick.ts persistReviewFindings ->\n-- enqueueReviewFindingLinearIssues): repo value if non-NULL, else the global\n-- default, else ON. Only `synthetic_review` tasks ever reach this gate, so the\n-- switch never changes worker-authored (`quay_owned`) behavior. Turning it off\n-- suppresses the `review_finding_linear_issue` outbox row only; findings are\n-- still persisted and still posted in the PR review.\n\nALTER TABLE deployment_settings ADD COLUMN review_finding_linear_enabled INTEGER\n CHECK (review_finding_linear_enabled IN (0, 1));\n\nALTER TABLE repos ADD COLUMN review_finding_linear_enabled INTEGER\n CHECK (review_finding_linear_enabled IN (0, 1));\n" }, + { name: "0041_spawn_failure_backoff.sql", sql: "ALTER TABLE tasks ADD COLUMN spawn_retry_next_eligible_at TEXT;\nALTER TABLE tasks ADD COLUMN spawn_failure_reason TEXT;\n\nCREATE INDEX tasks_spawn_retry_eligible_idx\n ON tasks(state, spawn_retry_next_eligible_at)\n WHERE state IN ('queued', 'pr-review');\n" }, ]; export const EMBEDDED_TICKET_SCHEMA = "# Quay default ticket_schema.toml\n# Override at ${QUAY_CONFIG_DIR:-$HOME/.quay}/ticket_schema.toml.\n# Field set is aligned 1:1 with the quay-config block (per\n# docs/quay-spec-deployment-adapters.md §10) plus body length sanity.\n\n[required.body]\ntype = \"string\"\nmin_length = 10\nmax_length = 50000\n\n[required.tags]\ntype = \"list\"\nitem_type = \"string\"\nmin_count = 1\ncharset = \"lowercase_alphanum_dash\"\nunique = true\n\n[optional.slack_thread]\ntype = \"string\"\npattern = '^[A-Z0-9]+:\\d+\\.\\d+$'\ndescription = \"Slack thread reference: :. Optional: tickets without an originating Slack discussion can omit it. Escalation degrades cleanly to a no-op when absent (per substrate spec, src/core/tick.ts:1019).\"\n\n[required.repo]\ntype = \"string\"\nmin_length = 1\npattern = '^[A-Za-z0-9._-]+$'\ndescription = \"Target repo ID. Must match a repo registered with `quay repo add`. Mirrors the quay-config block's `repo:` field 1:1, and the charset matches `repoIdSchema` in src/core/repos/schema.ts so existing deployments with uppercase, `.`, or `_` repo IDs are not silently locked out.\"\n\n[required.task_type]\ntype = \"enum\"\nallowed = [\"bugfix\", \"feature\", \"chore\", \"refactor\"]\ndescription = \"Canonical work classification from the quay-config block. Quay persists this on the durable work item.\"\n\n[required.authors]\ntype = \"list\"\nitem_type = \"object\"\nmin_count = 1\ndescription = \"Humans associated with the ticket, ordered by involvement (most-involved first). Mirrors the quay-config block's `authors:` list 1:1.\"\n\n[required.authors.fields.name]\ntype = \"string\"\nmin_length = 1\n\n[required.authors.fields.slack_id]\ntype = \"string\"\npattern = '^U[A-Z0-9]+$'\ndescription = \"Bare Slack user ID, e.g. U06TDC56VJB. Same format as the block.\"\n\n[optional.external_ref]\ntype = \"string\"\ndescription = \"Source-system identifier (e.g., 'ITRY-1276'). Stored opaquely by Quay as tasks.external_ref.\"\n\n[optional.worker_execution]\ntype = \"enum\"\nallowed = [\"oneshot\", \"goal\"]\ndescription = \"Worker execution mode. Defaults to oneshot; goal enables durable Quay goal worker mode.\"\n\n[optional.base_branch]\ntype = \"string\"\nmin_length = 1\nmax_length = 255\npattern = '^(?!refs/)(?!origin/)(?!@$)(?!.*@\\{)(?!.*\\.\\.)(?!.*//)(?!/)(?!.*[/.]$)(?!.*(?:^|/)\\.)(?!.*(?:^|/)[^/]*\\.lock(?:/|$))[A-Za-z0-9._/-]+$'\ndescription = \"Task-level PR base override. When present, Quay branches from origin/ and instructs the worker to open the PR into that same branch without changing the repo default.\"\n"; diff --git a/packages/cli/src/cli/config.ts b/packages/cli/src/cli/config.ts index 62ec168a..82cbdd86 100644 --- a/packages/cli/src/cli/config.ts +++ b/packages/cli/src/cli/config.ts @@ -35,7 +35,6 @@ const LinearAdapterConfigSchema = z api_key_env: z.string().min(1).optional(), bearer_token_env: z.string().min(1).optional(), token_command: z.string().min(1).optional(), - default_issue_team_key: z.string().min(1).optional(), }) .strict(); @@ -309,13 +308,11 @@ export function linearAdapterOptionsFromConfig( tokenEnvVar?: string; authMode?: "api_key" | "bearer"; tokenCommand?: string; - defaultIssueTeamKey?: string; } { const opts: { tokenEnvVar?: string; authMode?: "api_key" | "bearer"; tokenCommand?: string; - defaultIssueTeamKey?: string; } = {}; const linear = config.adapters?.linear; const authMode = @@ -333,9 +330,6 @@ export function linearAdapterOptionsFromConfig( if (linear?.token_command !== undefined) { opts.tokenCommand = linear.token_command; } - if (linear?.default_issue_team_key !== undefined) { - opts.defaultIssueTeamKey = linear.default_issue_team_key; - } return opts; } diff --git a/packages/cli/src/cli/dispatch.ts b/packages/cli/src/cli/dispatch.ts index 187841ef..f6db6142 100644 --- a/packages/cli/src/cli/dispatch.ts +++ b/packages/cli/src/cli/dispatch.ts @@ -57,6 +57,14 @@ import { task_retarget, type RetargetDeps, } from "../core/retarget.ts"; +import { + task_resnapshot, + type ResnapshotDeps, +} from "../core/resnapshot.ts"; +import { + recreate_task_worktree, + type RecreateWorktreeDeps, +} from "../core/recreate_worktree.ts"; import { claim_task, release_claim, @@ -102,9 +110,6 @@ import { type AdoptPrResult, type EnterReviewResult, } from "../core/pr_review.ts"; -import { - processReviewFindingLinearIssueOutboxItem, -} from "../core/review_finding_linear_outbox.ts"; export interface CliPaths { reposRoot: string; @@ -523,9 +528,6 @@ async function handleOutbox( case "fail": if (wantsHelp(rest)) return printHelp(io, ["outbox", "fail"]); return handleOutboxFail(rest, deps, io); - case "deliver": - if (wantsHelp(rest)) return printHelp(io, ["outbox", "deliver"]); - return await handleOutboxDeliver(rest, deps, io); default: return writeErrorWithUsage( io, @@ -536,34 +538,6 @@ async function handleOutbox( } } -async function handleOutboxDeliver( - argv: string[], - deps: CliDeps, - io: CliIO, -): Promise { - const validation = validateFlags(argv, { valued: [] }); - if (!validation.ok) { - return writeError(io, "usage_error", validation.message, validation.details); - } - const parsed = parsePositiveIntArg(positional(argv), "outbox deliver"); - if (!parsed.ok) return writeError(io, "usage_error", parsed.message); - const linear = pickLinearAdapter(deps); - if (linear === undefined) { - return writeError( - io, - "adapter_not_enabled", - "[adapters.linear] is not configured for this deployment", - { adapter: "linear" }, - ); - } - const row = await processReviewFindingLinearIssueOutboxItem( - { db: deps.db, clock: deps.clock, linear }, - { outboxItemId: parsed.value }, - ); - io.stdout(`${JSON.stringify(row)}\n`); - return { exitCode: 0 }; -} - function handleOutboxList( argv: string[], deps: CliDeps, @@ -764,6 +738,12 @@ async function handleTask( case "retarget": if (wantsHelp(rest)) return printHelp(io, ["task", "retarget"]); return await handleTaskRetarget(rest, deps, io); + case "resnapshot": + if (wantsHelp(rest)) return printHelp(io, ["task", "resnapshot"]); + return await handleTaskResnapshot(rest, deps, io); + case "recreate-worktree": + if (wantsHelp(rest)) return printHelp(io, ["task", "recreate-worktree"]); + return await handleTaskRecreateWorktree(rest, deps, io); default: // A typo'd subcommand benefits from the noun's usage block as much as // a missing one — surface it on stderr alongside the structured envelope. @@ -776,6 +756,42 @@ async function handleTask( } } +async function handleTaskRecreateWorktree( + argv: string[], + deps: CliDeps, + io: CliIO, +): Promise { + const validation = validateFlags(argv, { + boolean: ["--yes", "--force"], + }); + if (!validation.ok) { + return writeError(io, "usage_error", validation.message, validation.details); + } + const taskId = positional(argv); + if (!taskId) { + return writeError( + io, + "usage_error", + "task recreate-worktree requires ", + ); + } + const recreateDeps: RecreateWorktreeDeps = { + db: deps.db, + clock: deps.clock, + git: deps.git, + commandRunner: deps.commandRunner, + supervisorLock: deps.supervisorLock, + }; + return emitServiceResult( + await recreate_task_worktree(recreateDeps, { + taskId, + yes: argv.includes("--yes"), + force: argv.includes("--force"), + }), + io, + ); +} + async function handleTaskRetarget( argv: string[], deps: CliDeps, @@ -819,6 +835,49 @@ async function handleTaskRetarget( return emitServiceResult(await task_retarget(retargetDeps, input), io); } +async function handleTaskResnapshot( + argv: string[], + deps: CliDeps, + io: CliIO, +): Promise { + const validation = validateFlags(argv, { valued: ["--reason"] }); + if (!validation.ok) { + return writeError(io, "usage_error", validation.message, validation.details); + } + const taskId = positional(argv); + if (!taskId) { + return writeError(io, "usage_error", "task resnapshot requires "); + } + const reason = readFlag(argv, "--reason"); + if (reason === null) { + return writeError(io, "usage_error", "task resnapshot requires --reason "); + } + // Re-fetch reads the live Linear ticket; a deployment without the Linear + // adapter wired cannot re-baseline. Fail closed with the same usage-error + // shape the enqueue-linear path uses. + if (deps.linear === undefined || deps.adaptersConfig === undefined) { + return writeError( + io, + "adapter_not_enabled", + "[adapters.linear] is not configured for this deployment", + { adapter: "linear" }, + ); + } + const resnapshotDeps: ResnapshotDeps = { + db: deps.db, + clock: deps.clock, + artifactStore: deps.artifactStore, + supervisorLock: deps.supervisorLock, + linear: deps.linear, + slack: deps.slack, + adaptersConfig: deps.adaptersConfig, + }; + return emitServiceResult( + await task_resnapshot(resnapshotDeps, { taskId, reason }), + io, + ); +} + // Common "explicit --help" path for any command/subcommand: prints to stdout, // exits 0. Returns a no-op `{exitCode: 1}` if the path isn't registered, but // this should never fire in practice (the path is always one we control). @@ -2373,6 +2432,9 @@ function handleSettingsImport( worker_model: loaded.config.agents?.worker_model ?? null, reviewer_agent: loaded.config.agents?.reviewer ?? null, reviewer_model: loaded.config.agents?.reviewer_model ?? null, + // No config-file source for the review-finding toggle: leave it unset so + // it resolves to ON (the current intended behavior). + review_finding_linear_enabled: null, }; const onlyEmpty = argv.includes("--only-empty"); const next = onlyEmpty && current !== null ? current : imported; diff --git a/packages/cli/src/cli/help.ts b/packages/cli/src/cli/help.ts index 217da154..d6e2e01d 100644 --- a/packages/cli/src/cli/help.ts +++ b/packages/cli/src/cli/help.ts @@ -51,6 +51,8 @@ const COMMANDS: Record = { "task claim", "task release-claim", "task retarget", + "task resnapshot", + "task recreate-worktree", ], }, "task list": { @@ -98,6 +100,28 @@ const COMMANDS: Record = { { flag: "--yes", desc: "Required confirmation for the source task mutation." }, ], }, + "task resnapshot": { + path: "task resnapshot", + synopsis: "quay task resnapshot --reason ", + summary: + "Re-fetch the task's Linear ticket and replace its frozen ticket_snapshot, re-baselining the reviewer's definition of done.", + flags: [ + { flag: "--reason ", desc: "Required audit note recorded on the ticket_resnapshotted event." }, + ], + }, + "task recreate-worktree": { + path: "task recreate-worktree", + synopsis: + "quay task recreate-worktree --yes [--force]", + summary: + "Recreate a task's recorded worktree when the path is missing.", + details: + "Uses origin/ when that remote branch exists. Otherwise it rebuilds from origin/ while restoring the task branch name, then reruns the repo install command.", + flags: [ + { flag: "--yes", desc: "Required confirmation for git worktree mutation." }, + { flag: "--force", desc: "Allow recreation when the path exists or an active attempt is recorded." }, + ], + }, tick: { path: "tick", synopsis: "quay tick", @@ -143,7 +167,6 @@ const COMMANDS: Record = { "outbox claim", "outbox complete", "outbox fail", - "outbox deliver", ], }, "outbox list": { @@ -190,12 +213,6 @@ const COMMANDS: Record = { { flag: "--next-eligible-at ", desc: "Optional retry cooldown timestamp." }, ], }, - "outbox deliver": { - path: "outbox deliver", - synopsis: "quay outbox deliver ", - summary: - "Claim, execute, and complete a supported delivery outbox item with its production handler.", - }, enqueue: { path: "enqueue", synopsis: diff --git a/packages/cli/src/core/classifier.ts b/packages/cli/src/core/classifier.ts index dab4ec33..037e23bf 100644 --- a/packages/cli/src/core/classifier.ts +++ b/packages/cli/src/core/classifier.ts @@ -279,6 +279,18 @@ export function classifyAndApply( return transitionPrOpened(deps, task, attempt, remoteShaAtExit, exitInfo); } if (prExistsAtExit && noProgress) { + const adoptedReady = ingestAdoptedReadyForReviewSignal( + deps, + task, + attempt, + remoteShaAtExit, + exitInfo, + predicate, + ); + if (adoptedReady !== null) return adoptedReady; + if (options.spawnWindow && prExistedAtSpawn && remoteUnchanged) { + return { outcome: "spawn_window_no_evidence" }; + } if (task.pr_number === null) { const attached = reconcileExistingOpenPr( deps, @@ -290,15 +302,6 @@ export function classifyAndApply( ); if (attached !== null) return attached; } - const adoptedReady = ingestAdoptedReadyForReviewSignal( - deps, - task, - attempt, - remoteShaAtExit, - exitInfo, - predicate, - ); - if (adoptedReady !== null) return adoptedReady; return scheduleNoProgressRetry( deps, task, diff --git a/packages/cli/src/core/deployment_settings.ts b/packages/cli/src/core/deployment_settings.ts index 3f852438..a54e61b4 100644 --- a/packages/cli/src/core/deployment_settings.ts +++ b/packages/cli/src/core/deployment_settings.ts @@ -7,6 +7,9 @@ export interface DeploymentSettings { worker_model: string | null; reviewer_agent: string | null; reviewer_model: string | null; + // Global default for filing non-blocking review findings as Linear issues. + // NULL means unset, which resolves to ON (the current intended behavior). + review_finding_linear_enabled: boolean | null; } export interface DeploymentSettingsRow extends DeploymentSettings { @@ -15,6 +18,27 @@ export interface DeploymentSettingsRow extends DeploymentSettings { updated_at: string; } +// SQLite stores the tri-state toggle as a nullable INTEGER (NULL/0/1); the +// service surface speaks `boolean | null` so callers never juggle 0/1. +interface DeploymentSettingsDbRow { + singleton_id: 1; + worker_agent: string | null; + worker_model: string | null; + reviewer_agent: string | null; + reviewer_model: string | null; + review_finding_linear_enabled: number | null; + created_at: string; + updated_at: string; +} + +function intToBool(value: number | null): boolean | null { + return value === null ? null : value !== 0; +} + +function boolToInt(value: boolean | null): number | null { + return value === null ? null : value ? 1 : 0; +} + export type DeploymentSettingsPatch = { [K in keyof DeploymentSettings]?: DeploymentSettings[K] | undefined; }; @@ -37,14 +61,25 @@ export function createDeploymentSettingsService(deps: { const nowISO = () => deps.clock?.nowISO() ?? new Date().toISOString(); function getRow(): DeploymentSettingsRow | null { - return deps.db - .query( + const row = deps.db + .query( `SELECT singleton_id, worker_agent, worker_model, reviewer_agent, - reviewer_model, created_at, updated_at + reviewer_model, review_finding_linear_enabled, created_at, updated_at FROM deployment_settings WHERE singleton_id = 1`, ) - .get() ?? null; + .get(); + if (row === null || row === undefined) return null; + return { + singleton_id: row.singleton_id, + worker_agent: row.worker_agent, + worker_model: row.worker_model, + reviewer_agent: row.reviewer_agent, + reviewer_model: row.reviewer_model, + review_finding_linear_enabled: intToBool(row.review_finding_linear_enabled), + created_at: row.created_at, + updated_at: row.updated_at, + }; } function get(): DeploymentSettings { @@ -54,6 +89,7 @@ export function createDeploymentSettingsService(deps: { worker_model: row?.worker_model ?? null, reviewer_agent: row?.reviewer_agent ?? null, reviewer_model: row?.reviewer_model ?? null, + review_finding_linear_enabled: row?.review_finding_linear_enabled ?? null, }; } @@ -68,6 +104,10 @@ export function createDeploymentSettingsService(deps: { worker_model: valueOrCurrent(patch.worker_model, base?.worker_model ?? null), reviewer_agent: valueOrCurrent(patch.reviewer_agent, base?.reviewer_agent ?? null), reviewer_model: valueOrCurrent(patch.reviewer_model, base?.reviewer_model ?? null), + review_finding_linear_enabled: valueOrCurrent( + patch.review_finding_linear_enabled, + base?.review_finding_linear_enabled ?? null, + ), }; return replace(next); } @@ -79,13 +119,14 @@ export function createDeploymentSettingsService(deps: { .query( `INSERT INTO deployment_settings ( singleton_id, worker_agent, worker_model, reviewer_agent, - reviewer_model, created_at, updated_at - ) VALUES (1, ?, ?, ?, ?, ?, ?) + reviewer_model, review_finding_linear_enabled, created_at, updated_at + ) VALUES (1, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT(singleton_id) DO UPDATE SET worker_agent = excluded.worker_agent, worker_model = excluded.worker_model, reviewer_agent = excluded.reviewer_agent, reviewer_model = excluded.reviewer_model, + review_finding_linear_enabled = excluded.review_finding_linear_enabled, updated_at = excluded.updated_at`, ) .run( @@ -93,6 +134,7 @@ export function createDeploymentSettingsService(deps: { settings.worker_model, settings.reviewer_agent, settings.reviewer_model, + boolToInt(settings.review_finding_linear_enabled), current?.created_at ?? now, now, ); @@ -112,6 +154,8 @@ export function createDeploymentSettingsService(deps: { worker_model: config.agents?.worker_model ?? null, reviewer_agent: config.agents?.reviewer ?? null, reviewer_model: config.agents?.reviewer_model ?? null, + // No config-file source: leave the toggle unset so it resolves to ON. + review_finding_linear_enabled: null, }; if (opts.onlyEmpty !== true) return update(patch); @@ -123,6 +167,7 @@ export function createDeploymentSettingsService(deps: { worker_model: patch.worker_model ?? null, reviewer_agent: patch.reviewer_agent ?? null, reviewer_model: patch.reviewer_model ?? null, + review_finding_linear_enabled: patch.review_finding_linear_enabled ?? null, }); } diff --git a/packages/cli/src/core/goals.ts b/packages/cli/src/core/goals.ts index c03ed197..125f598b 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 cd95ccc9..09058085 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); @@ -1569,7 +1569,11 @@ function composeTaskReviewBrief( lines.push( "", renderReviewRequiredAction( - task.authoring_mode === "quay_owned" ? "quay_owned" : "non_quay_owned", + // adopted_external_pr is treated as quay_owned for verdict disposition: + // Quay owns the feedback loop (the worker respawns on changes_requested), + // so non-blocking findings are fixed in-loop rather than filed to Linear. + // Only synthetic_review (human owns the branch) stays non-quay-owned. + task.authoring_mode === "synthetic_review" ? "non_quay_owned" : "quay_owned", ), ); @@ -1637,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/recreate_worktree.ts b/packages/cli/src/core/recreate_worktree.ts new file mode 100644 index 00000000..7f5bfbb9 --- /dev/null +++ b/packages/cli/src/core/recreate_worktree.ts @@ -0,0 +1,258 @@ +import { existsSync, rmSync } from "node:fs"; +import type { DB } from "../db/connection.ts"; +import type { Clock } from "../ports/clock.ts"; +import type { CommandRunner } from "../ports/command_runner.ts"; +import type { GitPort } from "../ports/git.ts"; +import type { SupervisorLock } from "./supervisor_lock.ts"; +import { + installWorktreeDependencies, + loadWorktreeDependencyRepo, +} from "./worktree_dependencies.ts"; + +export type RecreateWorktreeErrorCode = + | "unknown_task" + | "confirmation_required" + | "worktree_exists" + | "active_task"; + +export interface RecreateWorktreeError { + code: RecreateWorktreeErrorCode; + message: string; + details?: Record; +} + +export type RecreateWorktreeResult = + | { ok: true; value: RecreateWorktreeValue } + | { ok: false; error: RecreateWorktreeError }; + +export interface RecreateWorktreeValue { + task_id: string; + repo_id: string; + branch_name: string; + base_branch: string; + worktree_path: string; + recovery_base: "remote_task_branch" | "remote_base_branch"; + recovery_ref: string; + forced: boolean; +} + +export interface RecreateWorktreeDeps { + db: DB; + clock: Clock; + git: GitPort; + commandRunner: CommandRunner; + supervisorLock: SupervisorLock; +} + +export interface RecreateWorktreeInput { + taskId: string; + yes?: boolean; + force?: boolean; +} + +interface TaskRow { + task_id: string; + repo_id: string; + state: string; + branch_name: string; + base_branch: string | null; + worktree_path: string; +} + +interface ActiveAttemptRow { + attempt_id: number; + tmux_session: string | null; +} + +export async function recreate_task_worktree( + deps: RecreateWorktreeDeps, + input: RecreateWorktreeInput, +): Promise { + return deps.supervisorLock.run(() => recreateUnderLock(deps, input)); +} + +function recreateUnderLock( + deps: RecreateWorktreeDeps, + input: RecreateWorktreeInput, +): RecreateWorktreeResult { + const task = loadTask(deps.db, input.taskId); + if (task === null) { + return { + ok: false, + error: { + code: "unknown_task", + message: `task ${input.taskId} not found`, + details: { task_id: input.taskId }, + }, + }; + } + + if (input.yes !== true) { + return { + ok: false, + error: { + code: "confirmation_required", + message: "task recreate-worktree mutates git state; rerun with --yes", + details: { task_id: task.task_id, worktree_path: task.worktree_path }, + }, + }; + } + + const activeAttempt = loadActiveAttempt(deps.db, task.task_id); + if (activeAttempt !== null && input.force !== true) { + return { + ok: false, + error: { + code: "active_task", + message: + `task ${task.task_id} has an active attempt; rerun with --force only if the worker is not live`, + details: { + task_id: task.task_id, + state: task.state, + attempt_id: activeAttempt.attempt_id, + tmux_session: activeAttempt.tmux_session, + }, + }, + }; + } + + const pathExists = existsSync(task.worktree_path); + if (pathExists && input.force !== true) { + return { + ok: false, + error: { + code: "worktree_exists", + message: + `task ${task.task_id} already has a worktree at ${task.worktree_path}; rerun with --force to recreate it`, + details: { + task_id: task.task_id, + worktree_path: task.worktree_path, + }, + }, + }; + } + + const repo = loadWorktreeDependencyRepo(deps.db, task.repo_id); + const baseBranch = task.base_branch ?? loadRepoBaseBranch(deps.db, task.repo_id); + const branchExistsRemotely = deps.git.hasRemoteBranch( + task.repo_id, + task.branch_name, + ); + const recoveryBase: RecreateWorktreeValue["recovery_base"] = + branchExistsRemotely ? "remote_task_branch" : "remote_base_branch"; + const recoveryRef = branchExistsRemotely + ? `origin/${task.branch_name}` + : `origin/${baseBranch}`; + + if (branchExistsRemotely) { + deps.git.fetch(task.repo_id, task.branch_name); + } else { + deps.git.fetch(task.repo_id, baseBranch); + } + + if (pathExists) { + deps.git.worktreeRemove(task.worktree_path); + } else { + deps.git.worktreePrune(task.repo_id); + } + + let worktreeCreated = false; + try { + deps.git.worktreeAddExistingBranch( + task.repo_id, + task.worktree_path, + task.branch_name, + recoveryRef, + ); + worktreeCreated = true; + installWorktreeDependencies(deps.commandRunner, repo, task.worktree_path); + } catch (err) { + if (worktreeCreated) { + try { + deps.git.worktreeRemove(task.worktree_path); + } catch { + try { + rmSync(task.worktree_path, { recursive: true, force: true }); + } catch {} + } + } + throw err; + } + + const now = deps.clock.nowISO(); + const eventData = { + worktree_path: task.worktree_path, + branch_name: task.branch_name, + base_branch: baseBranch, + recovery_base: recoveryBase, + recovery_ref: recoveryRef, + forced: input.force === true, + active_attempt_id: activeAttempt?.attempt_id ?? null, + }; + deps.db + .query( + `INSERT INTO events ( + task_id, event_type, from_state, to_state, occurred_at, event_data + ) VALUES (?, 'worktree_recreated', ?, ?, ?, ?)`, + ) + .run( + task.task_id, + task.state, + task.state, + now, + JSON.stringify(eventData), + ); + deps.db + .query(`UPDATE tasks SET tick_error = NULL, updated_at = ? WHERE task_id = ?`) + .run(now, task.task_id); + + return { + ok: true, + value: { + task_id: task.task_id, + repo_id: task.repo_id, + branch_name: task.branch_name, + base_branch: baseBranch, + worktree_path: task.worktree_path, + recovery_base: recoveryBase, + recovery_ref: recoveryRef, + forced: input.force === true, + }, + }; +} + +function loadTask(db: DB, taskId: string): TaskRow | null { + return db + .query( + `SELECT task_id, repo_id, state, branch_name, base_branch, worktree_path + FROM tasks + WHERE task_id = ?`, + ) + .get(taskId) ?? null; +} + +function loadActiveAttempt(db: DB, taskId: string): ActiveAttemptRow | null { + return db + .query( + `SELECT attempt_id, tmux_session + FROM attempts + WHERE task_id = ? + AND spawned_at IS NOT NULL + AND ended_at IS NULL + ORDER BY attempt_id DESC + LIMIT 1`, + ) + .get(taskId) ?? null; +} + +function loadRepoBaseBranch(db: DB, repoId: string): string { + const row = db + .query<{ base_branch: string }, [string]>( + `SELECT base_branch FROM repos WHERE repo_id = ?`, + ) + .get(repoId); + if (!row) { + throw new Error(`repo ${repoId} not found for task worktree recreation`); + } + return row.base_branch; +} diff --git a/packages/cli/src/core/repos/schema.ts b/packages/cli/src/core/repos/schema.ts index 868c12fc..19d6f62e 100644 --- a/packages/cli/src/core/repos/schema.ts +++ b/packages/cli/src/core/repos/schema.ts @@ -39,6 +39,10 @@ const preambleId = z.preprocess((value) => { return value; }, z.number().int().positive()); +// Per-repo override for filing non-blocking review findings as Linear issues. +// Tri-state: NULL/omitted = inherit the global default, true = on, false = off. +const reviewFindingLinearEnabled = z.boolean(); + export const repoAddInputSchema = z .object({ repo_id: repoIdSchema, @@ -55,6 +59,7 @@ export const repoAddInputSchema = z model_reviewer: modelName.optional(), preamble_worker: preambleId.optional(), preamble_reviewer: preambleId.optional(), + review_finding_linear_enabled: reviewFindingLinearEnabled.optional(), ci_ignore_mode: ciIgnoreMode.optional(), ignored_check_names: ciIgnoredNameList.optional(), ignored_workflow_names: ciIgnoredNameList.optional(), @@ -78,6 +83,7 @@ export const repoUpdateInputSchema = z model_reviewer: modelName.nullable().optional(), preamble_worker: preambleId.nullable().optional(), preamble_reviewer: preambleId.nullable().optional(), + review_finding_linear_enabled: reviewFindingLinearEnabled.nullable().optional(), ci_ignore_mode: ciIgnoreMode.optional(), ignored_check_names: ciIgnoredNameList.optional(), ignored_workflow_names: ciIgnoredNameList.optional(), @@ -107,6 +113,7 @@ export const repoImportInputSchema = z model_reviewer: modelName.nullable().optional(), preamble_worker: preambleId.nullable().optional(), preamble_reviewer: preambleId.nullable().optional(), + review_finding_linear_enabled: reviewFindingLinearEnabled.nullable().optional(), ci_ignore_mode: ciIgnoreMode.optional(), ignored_check_names: ciIgnoredNameList.optional(), ignored_workflow_names: ciIgnoredNameList.optional(), diff --git a/packages/cli/src/core/repos/service.ts b/packages/cli/src/core/repos/service.ts index 9581ea7c..434de56f 100644 --- a/packages/cli/src/core/repos/service.ts +++ b/packages/cli/src/core/repos/service.ts @@ -31,6 +31,9 @@ export interface RepoRow { model_reviewer: string | null; preamble_worker: number | null; preamble_reviewer: number | null; + // Per-repo override for filing non-blocking review findings as Linear + // issues. NULL = inherit the deployment default, true = on, false = off. + review_finding_linear_enabled: boolean | null; ci_ignore_mode: CiIgnoreMode; ignored_check_names: string[]; ignored_workflow_names: string[]; @@ -66,16 +69,26 @@ const SELECT_REPO_COLUMNS = ` repo_id, repo_url, base_branch, package_manager, install_cmd, test_cmd, ci_workflow_name, contribution_guide_path, agent_worker, agent_reviewer, model_worker, model_reviewer, - preamble_worker, preamble_reviewer, + preamble_worker, preamble_reviewer, review_finding_linear_enabled, ci_ignore_mode, ci_ignored_check_names, ci_ignored_workflow_names, archived_at, created_at `; -type RepoDbRow = Omit & { +type RepoDbRow = Omit< + RepoRow, + "ignored_check_names" | "ignored_workflow_names" | "review_finding_linear_enabled" +> & { ci_ignored_check_names: string; ci_ignored_workflow_names: string; + review_finding_linear_enabled: number | null; }; +// SQLite stores the toggle as a nullable INTEGER (NULL/0/1); the service +// surface speaks `boolean | null` so callers never juggle 0/1. +function reviewFindingLinearToDb(value: boolean | null | undefined): number | null { + return value === null || value === undefined ? null : value ? 1 : 0; +} + // Mirrors spec §10: repo removal blocks non-terminal, non-parked tasks only. // Parked and terminal tasks keep their FK for forensics after archival. const ACTIVE_TASK_STATES = [ @@ -94,10 +107,15 @@ function fromRepoDbRow(row: RepoDbRow): RepoRow { const { ci_ignored_check_names, ci_ignored_workflow_names, + review_finding_linear_enabled, ...rest } = row; return { ...rest, + review_finding_linear_enabled: + review_finding_linear_enabled === null + ? null + : review_finding_linear_enabled !== 0, ignored_check_names: parseCiIgnoreListJson(ci_ignored_check_names), ignored_workflow_names: parseCiIgnoreListJson(ci_ignored_workflow_names), }; @@ -134,6 +152,7 @@ export function createRepoService({ db, clock }: RepoServiceDeps): RepoService { agent_worker = ?, agent_reviewer = ?, model_worker = ?, model_reviewer = ?, preamble_worker = ?, preamble_reviewer = ?, + review_finding_linear_enabled = ?, ci_ignore_mode = ?, ci_ignored_check_names = ?, ci_ignored_workflow_names = ?, archived_at = NULL WHERE repo_id = ?`, @@ -151,6 +170,7 @@ export function createRepoService({ db, clock }: RepoServiceDeps): RepoService { parsed.model_reviewer ?? null, parsed.preamble_worker ?? null, parsed.preamble_reviewer ?? null, + reviewFindingLinearToDb(parsed.review_finding_linear_enabled), parsed.ci_ignore_mode ?? "inherit", JSON.stringify(parsed.ignored_check_names ?? []), JSON.stringify(parsed.ignored_workflow_names ?? []), @@ -162,10 +182,10 @@ export function createRepoService({ db, clock }: RepoServiceDeps): RepoService { repo_id, repo_url, base_branch, package_manager, install_cmd, test_cmd, ci_workflow_name, contribution_guide_path, agent_worker, agent_reviewer, model_worker, model_reviewer, - preamble_worker, preamble_reviewer, + preamble_worker, preamble_reviewer, review_finding_linear_enabled, ci_ignore_mode, ci_ignored_check_names, ci_ignored_workflow_names, created_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, ).run( parsed.repo_id, parsed.repo_url, @@ -181,6 +201,7 @@ export function createRepoService({ db, clock }: RepoServiceDeps): RepoService { parsed.model_reviewer ?? null, parsed.preamble_worker ?? null, parsed.preamble_reviewer ?? null, + reviewFindingLinearToDb(parsed.review_finding_linear_enabled), parsed.ci_ignore_mode ?? "inherit", JSON.stringify(parsed.ignored_check_names ?? []), JSON.stringify(parsed.ignored_workflow_names ?? []), @@ -212,6 +233,11 @@ export function createRepoService({ db, clock }: RepoServiceDeps): RepoService { values.push(JSON.stringify(value)); continue; } + if (key === "review_finding_linear_enabled") { + sets.push("review_finding_linear_enabled = ?"); + values.push(reviewFindingLinearToDb(value as boolean | null | undefined)); + continue; + } sets.push(`${key} = ?`); values.push(value); } @@ -312,6 +338,10 @@ export function createRepoService({ db, clock }: RepoServiceDeps): RepoService { parsed.preamble_reviewer !== undefined ? parsed.preamble_reviewer : (existing?.preamble_reviewer ?? null); + const reviewFindingLinearEnabled = + parsed.review_finding_linear_enabled !== undefined + ? parsed.review_finding_linear_enabled + : (existing?.review_finding_linear_enabled ?? null); const ciIgnoreMode = parsed.ci_ignore_mode !== undefined ? parsed.ci_ignore_mode @@ -336,6 +366,7 @@ export function createRepoService({ db, clock }: RepoServiceDeps): RepoService { agent_worker = ?, agent_reviewer = ?, model_worker = ?, model_reviewer = ?, preamble_worker = ?, preamble_reviewer = ?, + review_finding_linear_enabled = ?, ci_ignore_mode = ?, ci_ignored_check_names = ?, ci_ignored_workflow_names = ?, archived_at = ?, created_at = ? WHERE repo_id = ?`, @@ -353,6 +384,7 @@ export function createRepoService({ db, clock }: RepoServiceDeps): RepoService { modelReviewer, preambleWorker, preambleReviewer, + reviewFindingLinearToDb(reviewFindingLinearEnabled), ciIgnoreMode, JSON.stringify(ignoredCheckNames), JSON.stringify(ignoredWorkflowNames), @@ -366,10 +398,10 @@ export function createRepoService({ db, clock }: RepoServiceDeps): RepoService { repo_id, repo_url, base_branch, package_manager, install_cmd, test_cmd, ci_workflow_name, contribution_guide_path, agent_worker, agent_reviewer, model_worker, model_reviewer, - preamble_worker, preamble_reviewer, + preamble_worker, preamble_reviewer, review_finding_linear_enabled, ci_ignore_mode, ci_ignored_check_names, ci_ignored_workflow_names, archived_at, created_at - ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, + ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`, ).run( parsed.repo_id, parsed.repo_url, @@ -385,6 +417,7 @@ export function createRepoService({ db, clock }: RepoServiceDeps): RepoService { modelReviewer, preambleWorker, preambleReviewer, + reviewFindingLinearToDb(reviewFindingLinearEnabled), ciIgnoreMode, JSON.stringify(ignoredCheckNames), JSON.stringify(ignoredWorkflowNames), diff --git a/packages/cli/src/core/resnapshot.ts b/packages/cli/src/core/resnapshot.ts new file mode 100644 index 00000000..b7d14637 --- /dev/null +++ b/packages/cli/src/core/resnapshot.ts @@ -0,0 +1,356 @@ +import { createHash } from "node:crypto"; +import { readFileSync } from "node:fs"; +import type { ArtifactStore } from "../artifacts/store.ts"; +import type { DB } from "../db/connection.ts"; +import type { Clock } from "../ports/clock.ts"; +import type { LinearPort } from "../ports/linear.ts"; +import type { SlackPort } from "../ports/slack.ts"; +import type { SupervisorLock } from "./supervisor_lock.ts"; +import { fetchTicketContextWithIssue } from "./ticket_context.ts"; + +// The top-level keys `composeTicketSnapshot` emits (ticket_context.ts). Any +// other key in a stored snapshot — `linear_blocked_by_relations`, +// `linear_hierarchy`, `linear_umbrella_membership_override` — is a +// creation-time augmentation the enqueue-linear path bolts on and that +// resnapshot does NOT recompute. Those keys are preserved verbatim so +// re-baselining the definition-of-done never drops dependency / hierarchy +// context. +const SNAPSHOT_CORE_KEYS = [ + "linear_issue", + "quay_config_block", + "slack_thread_ref", + "slack_thread", +] as const; + +export type ResnapshotErrorCode = + | "unknown_task" + | "missing_external_ref" + | "missing_reason"; + +export interface ResnapshotError { + code: ResnapshotErrorCode; + message: string; + details?: Record; +} + +export type ResnapshotResult = + | { ok: true; value: ResnapshotValue } + | { ok: false; error: ResnapshotError }; + +export interface ResnapshotValue { + task_id: string; + external_ref: string; + // Whether the definition-of-done (core snapshot keys) actually changed. + // False means the run was a still-audited no-op: the event is recorded but + // the artifact is not rewritten and no review verdict is invalidated. + changed: boolean; + // Count of terminal review verdicts (`approved` / `changes_requested`) + // superseded so the next tick re-reviews against the new snapshot. + review_invalidated: number; + // 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; +} + +export interface ResnapshotDeps { + db: DB; + clock: Clock; + artifactStore: ArtifactStore; + supervisorLock: SupervisorLock; + linear: LinearPort; + slack: SlackPort; + adaptersConfig: { linearEnabled: boolean; slackEnabled: boolean }; +} + +export interface ResnapshotInput { + taskId: string; + reason: string; +} + +interface TaskRow { + task_id: string; + external_ref: string | null; + state: string; +} + +interface ArtifactRow { + file_path: string; +} + +export async function task_resnapshot( + deps: ResnapshotDeps, + input: ResnapshotInput, +): Promise { + const reason = input.reason.trim(); + if (reason.length === 0) { + return { + ok: false, + error: { + code: "missing_reason", + message: "task resnapshot requires a non-empty --reason", + }, + }; + } + + const task = loadTask(deps.db, input.taskId); + if (task === null) { + return { + ok: false, + error: { + code: "unknown_task", + message: `task ${input.taskId} not found`, + details: { task_id: input.taskId }, + }, + }; + } + if (task.external_ref === null) { + return { + ok: false, + error: { + code: "missing_external_ref", + message: `task ${input.taskId} has no external_ref; nothing to re-fetch`, + details: { task_id: input.taskId }, + }, + }; + } + + // Re-fetch + re-parse + re-compose via the exact enqueue code path. This is + // a pure read, so it runs before the supervisor lock. It throws QuayError + // (adapter_not_enabled, ticket_not_found, ticket_block_invalid, + // adapter_error) which the CLI maps to the stderr error contract. + const fetched = await fetchTicketContextWithIssue( + { linear: deps.linear, slack: deps.slack, config: deps.adaptersConfig }, + 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, freshBrief, reason), + ); +} + +function resnapshotUnderLock( + deps: ResnapshotDeps, + task: TaskRow, + externalRef: string, + freshSnapshot: string, + freshBrief: string, + reason: string, +): ResnapshotResult { + const now = deps.clock.nowISO(); + const oldContent = loadArtifactContent(deps.db, task.task_id, "ticket_snapshot"); + const freshParsed = parseJsonObject(freshSnapshot); + const oldParsed = oldContent === null ? null : parseJsonObject(oldContent); + + const freshCore = coreSubset(freshParsed); + const oldCore = coreSubset(oldParsed); + // "Unchanged" is measured on the definition-of-done (core keys) only, so a + // task whose stored snapshot carries creation-time augmentations still + // no-ops when the ticket body / config block are identical. + const changed = + oldContent === null || + JSON.stringify(freshCore) !== JSON.stringify(oldCore); + + 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. + 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 snapshotContent = JSON.stringify(merged, null, 2); + const writtenSnapshot = deps.artifactStore.writeArtifact({ + taskId: task.task_id, + attemptId: null, + kind: "ticket_snapshot", + 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", + }); + 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); + } + + 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 ( + task_id, event_type, from_state, to_state, occurred_at, event_data + ) VALUES (?, 'ticket_resnapshotted', ?, ?, ?, ?) + RETURNING event_id`, + ) + .get(task.task_id, task.state, task.state, now, JSON.stringify(eventData)); + if (!eventRow) { + throw new Error("ticket_resnapshotted event insert returned no row"); + } + eventId = eventRow.event_id; + deps.db.exec("COMMIT"); + } catch (err) { + try { + deps.db.exec("ROLLBACK"); + } catch {} + throw err; + } + + return { + ok: true, + value: { + task_id: task.task_id, + external_ref: externalRef, + changed, + review_invalidated: reviewInvalidated, + snapshot_artifact_id: snapshotArtifactId, + objective_artifact_id: objectiveArtifactId, + event_id: eventId, + }, + }; +} + +// Supersede every terminal reviewer verdict on the task's review-only +// attempts. The `enterReview` gate skips scheduling when a review-only attempt +// for the current head SHA already holds `approved` / `changes_requested` +// (`terminal_verdict_exists`); flipping those to `superseded` — the same +// marker enterReview and terminal cleanup already use — clears the gate so the +// next tick runs a fresh review against the re-baselined snapshot. Returns the +// number of verdicts invalidated. +function invalidateLatestReview(db: DB, taskId: string): number { + const result = db + .query( + `UPDATE attempts + SET review_verdict = 'superseded' + WHERE task_id = ? + AND reason = 'review_only' + AND review_verdict IN ('approved', 'changes_requested')`, + ) + .run(taskId); + return Number(result.changes); +} + +function loadTask(db: DB, taskId: string): TaskRow | null { + return ( + db + .query( + `SELECT task_id, external_ref, state FROM tasks WHERE task_id = ?`, + ) + .get(taskId) ?? null + ); +} + +function loadArtifactContent( + db: DB, + taskId: string, + kind: string, +): string | null { + const row = + db + .query( + `SELECT file_path + FROM artifacts + WHERE task_id = ? AND kind = ? AND attempt_id IS NULL + ORDER BY artifact_id DESC + LIMIT 1`, + ) + .get(taskId, kind) ?? null; + if (row === null) return null; + return readFileSync(row.file_path, "utf8"); +} + +function coreSubset( + obj: Record | null, +): Record { + const out: Record = {}; + if (obj === null) return out; + for (const key of SNAPSHOT_CORE_KEYS) { + if (key in obj) out[key] = obj[key]; + } + return out; +} + +// Field-level before/after diff of the core snapshot keys. Object values are +// expanded one level so, e.g., a changed `linear_issue.body` is reported on +// its own without dumping every unchanged sibling field. +function diffCore( + before: Record, + after: Record, +): Record { + const diff: Record = {}; + for (const key of SNAPSHOT_CORE_KEYS) { + const b = before[key]; + const a = after[key]; + if (JSON.stringify(b) === JSON.stringify(a)) continue; + diff[key] = diffValue(b, a); + } + return diff; +} + +function diffValue(before: unknown, after: unknown): unknown { + if (isPlainObject(before) && isPlainObject(after)) { + const sub: Record = {}; + const keys = new Set([...Object.keys(before), ...Object.keys(after)]); + for (const key of keys) { + if (JSON.stringify(before[key]) === JSON.stringify(after[key])) continue; + sub[key] = { before: before[key] ?? null, after: after[key] ?? null }; + } + return sub; + } + return { before: before ?? null, after: after ?? null }; +} + +function isPlainObject(v: unknown): v is Record { + return v !== null && typeof v === "object" && !Array.isArray(v); +} + +function parseJsonObject(s: string): Record { + try { + const parsed: unknown = JSON.parse(s); + if (isPlainObject(parsed)) return parsed; + } catch { + // Non-JSON stored snapshot (never produced by the enqueue path). Treat as + // structureless so the fresh snapshot fully replaces it. + } + return {}; +} + +function sha256(s: string): string { + return createHash("sha256").update(s).digest("hex"); +} diff --git a/packages/cli/src/core/review_finding_linear_outbox.ts b/packages/cli/src/core/review_finding_linear_outbox.ts index 3ac93105..f0cbf10c 100644 --- a/packages/cli/src/core/review_finding_linear_outbox.ts +++ b/packages/cli/src/core/review_finding_linear_outbox.ts @@ -1,16 +1,6 @@ -import { createHash } from "node:crypto"; import type { DB } from "../db/connection.ts"; import type { Clock } from "../ports/clock.ts"; -import type { LinearPort } from "../ports/linear.ts"; -import { QuayError } from "./errors.ts"; -import { - claimOutboxItem, - completeOutboxItem, - enqueueOutboxItem, - failOutboxItem, - type CompleteOutboxItemResult, - type OutboxItemRow, -} from "./outbox.ts"; +import { enqueueOutboxItem } from "./outbox.ts"; export const REVIEW_FINDING_LINEAR_ISSUE_OUTBOX_KIND = "delivery.review_finding_linear_issue"; @@ -101,113 +91,6 @@ export function enqueueReviewFindingLinearIssuesInOpenTxn( return outboxIds; } -export async function processReviewFindingLinearIssueOutboxItem( - deps: { db: DB; clock: Clock; linear: LinearPort }, - input: { outboxItemId: number }, -): Promise { - const claimed = claimOutboxItem(deps, { outboxItemId: input.outboxItemId }); - if (!claimed.ok) throw new Error(claimed.error.message); - const row = loadOutboxItem(deps.db, input.outboxItemId); - if (row === null) throw new Error(`outbox item ${input.outboxItemId} disappeared`); - - try { - await deliverClaimedReviewFindingLinearIssue(deps, row); - const completed = completeOutboxItem(deps, { - outboxItemId: row.outbox_item_id, - claimId: claimed.value.claim_id, - }); - if (!completed.ok) throw new Error(completed.error.message); - return completed.value; - } catch (err) { - const failed = failOutboxItem(deps, { - outboxItemId: row.outbox_item_id, - claimId: claimed.value.claim_id, - lastError: err instanceof Error ? err.message : String(err), - nextEligibleAt: nextEligibleAtFromError(deps.clock, err), - }); - if (!failed.ok) throw new Error(failed.error.message); - throw err; - } -} - -function nextEligibleAtFromError(clock: Clock, err: unknown): string | null { - if (!(err instanceof QuayError)) return null; - if (err.code !== "adapter_error") return null; - const retryAfter = err.details?.retry_after; - if ( - typeof retryAfter !== "number" || - !Number.isFinite(retryAfter) || - retryAfter <= 0 - ) { - return null; - } - const now = Date.parse(clock.nowISO()); - if (!Number.isFinite(now)) return null; - return new Date(now + retryAfter * 1000).toISOString(); -} - -async function deliverClaimedReviewFindingLinearIssue( - deps: { db: DB; clock: Clock; linear: LinearPort }, - row: OutboxItemRow, -): Promise { - if (row.kind !== REVIEW_FINDING_LINEAR_ISSUE_OUTBOX_KIND) { - throw new Error(`unsupported outbox kind ${row.kind}`); - } - const payload = parsePayload(row.payload_json); - const finding = loadCurrentFinding(deps.db, payload); - if (finding === null) return; - const existing = loadExternalLink( - deps.db, - finding.task_id, - finding.review_id, - finding.fingerprint, - ); - if (existing !== null && existing.provider_url !== "") { - deps.db - .query( - `UPDATE review_finding_external_links - SET finding_id = ?, - outbox_item_id = COALESCE(outbox_item_id, ?), - updated_at = ? - WHERE link_id = ?`, - ) - .run(finding.finding_id, row.outbox_item_id, deps.clock.nowISO(), existing.link_id); - return; - } - - const locations = loadLocations(deps.db, finding.finding_id); - const created = await deps.linear.createIssue({ - title: finding.title, - body: renderLinearIssueBody(finding, locations), - idempotencyKey: linearIssueProviderIdempotencyKey(finding), - }); - const now = deps.clock.nowISO(); - deps.db - .query( - `INSERT INTO review_finding_external_links ( - finding_id, task_id, review_id, fingerprint, provider, - provider_external_id, provider_url, outbox_item_id, created_at, updated_at - ) VALUES (?, ?, ?, ?, 'linear', ?, ?, ?, ?, ?) - ON CONFLICT(provider, task_id, review_id, fingerprint) DO UPDATE SET - finding_id = excluded.finding_id, - provider_external_id = excluded.provider_external_id, - provider_url = excluded.provider_url, - outbox_item_id = excluded.outbox_item_id, - updated_at = excluded.updated_at`, - ) - .run( - finding.finding_id, - finding.task_id, - finding.review_id, - finding.fingerprint, - created.id, - created.url, - row.outbox_item_id, - now, - now, - ); -} - function loadEligibleFindings( db: DB, taskId: string, @@ -225,41 +108,12 @@ function loadEligibleFindings( AND f.attempt_id = ? AND f.review_id = ? AND f.severity = 'non_blocking' - AND t.authoring_mode IN ('synthetic_review', 'adopted_external_pr') + AND t.authoring_mode = 'synthetic_review' ORDER BY f.ordinal`, ) .all(taskId, attemptId, reviewId); } -function loadCurrentFinding( - db: DB, - payload: { - finding_id: number; - task_id: string; - review_id: string; - fingerprint: string; - }, -): ReviewFindingOutboxRow | null { - return ( - db - .query( - `SELECT f.finding_id, f.task_id, f.review_id, f.head_sha, f.severity, - f.title, f.body_markdown, f.principle_text, f.fingerprint, - t.repo_id, t.authoring_mode, t.pr_number, t.pr_url - FROM review_findings f - JOIN tasks t ON t.task_id = f.task_id - WHERE f.task_id = ? - AND f.review_id = ? - AND f.fingerprint = ? - AND f.severity = 'non_blocking' - AND t.authoring_mode IN ('synthetic_review', 'adopted_external_pr') - ORDER BY f.finding_id DESC - LIMIT 1`, - ) - .get(payload.task_id, payload.review_id, payload.fingerprint) ?? null - ); -} - function loadLocations(db: DB, findingId: number): ReviewFindingLocationRow[] { return db .query( @@ -294,21 +148,6 @@ function loadExternalLink( ); } -function loadOutboxItem(db: DB, outboxItemId: number): OutboxItemRow | null { - return ( - db - .query( - `SELECT outbox_item_id, task_id, kind, handler_class, source_event_id, - idempotency_key, payload_json, route_hint_json, status, claim_id, - claimed_at, delivered_at, completed_at, last_error, - next_eligible_at, created_at, updated_at - FROM outbox_items - WHERE outbox_item_id = ?`, - ) - .get(outboxItemId) ?? null - ); -} - function linearIssueOutboxIdempotencyKey(finding: ReviewFindingOutboxRow): string { return [ REVIEW_FINDING_LINEAR_ISSUE_OUTBOX_KIND, @@ -317,86 +156,3 @@ function linearIssueOutboxIdempotencyKey(finding: ReviewFindingOutboxRow): strin finding.fingerprint, ].join(":"); } - -function linearIssueProviderIdempotencyKey(finding: ReviewFindingOutboxRow): string { - return stableUuid([ - REVIEW_FINDING_LINEAR_ISSUE_OUTBOX_KIND, - finding.task_id, - finding.review_id, - finding.fingerprint, - ].join(":")); -} - -function stableUuid(source: string): string { - const bytes = createHash("sha256").update(source).digest(); - bytes[6] = (bytes[6]! & 0x0f) | 0x50; - bytes[8] = (bytes[8]! & 0x3f) | 0x80; - const hex = bytes.subarray(0, 16).toString("hex"); - return [ - hex.slice(0, 8), - hex.slice(8, 12), - hex.slice(12, 16), - hex.slice(16, 20), - hex.slice(20, 32), - ].join("-"); -} - -function renderLinearIssueBody( - finding: ReviewFindingOutboxRow, - locations: ReviewFindingLocationRow[], -): string { - const lines = [ - finding.body_markdown, - "", - "## Quay Review Finding", - `- Task: ${finding.task_id}`, - `- Review: ${finding.review_id}`, - `- Head SHA: ${finding.head_sha}`, - ]; - if (finding.pr_url !== null) lines.push(`- PR: ${finding.pr_url}`); - else if (finding.pr_number !== null) lines.push(`- PR number: ${finding.pr_number}`); - if (finding.principle_text !== null) { - lines.push("", "## quay-principle", finding.principle_text); - } - if (locations.length > 0) { - lines.push("", "## Locations"); - for (const location of locations) { - const locus = [ - location.path, - formatLineRange(location.start_line, location.end_line), - ].filter((part) => part !== null && part !== "").join(":"); - lines.push(`- ${locus === "" ? "(review body)" : locus}`); - if (location.url !== null) lines.push(` ${location.url}`); - } - } - return lines.join("\n"); -} - -function formatLineRange(start: number | null, end: number | null): string | null { - if (start === null && end === null) return null; - if (start !== null && end !== null && start !== end) return `${start}-${end}`; - return String(start ?? end); -} - -function parsePayload(payloadJson: string | null): { - finding_id: number; - task_id: string; - review_id: string; - fingerprint: string; -} { - if (payloadJson === null) throw new Error("review finding outbox payload is empty"); - const parsed = JSON.parse(payloadJson) as Record; - const findingId = parsed.finding_id; - const taskId = parsed.task_id; - const reviewId = parsed.review_id; - const fingerprint = parsed.fingerprint; - if ( - typeof findingId !== "number" || - typeof taskId !== "string" || - typeof reviewId !== "string" || - typeof fingerprint !== "string" - ) { - throw new Error("review finding outbox payload is malformed"); - } - return { finding_id: findingId, task_id: taskId, review_id: reviewId, fingerprint }; -} diff --git a/packages/cli/src/core/review_finding_linear_policy.ts b/packages/cli/src/core/review_finding_linear_policy.ts new file mode 100644 index 00000000..d34ae68c --- /dev/null +++ b/packages/cli/src/core/review_finding_linear_policy.ts @@ -0,0 +1,68 @@ +// Resolves whether a repo's non-blocking review findings should be filed as +// Linear issues (BRIX-1898), and gates the enqueue accordingly. +// +// Precedence: per-repo override (repos.review_finding_linear_enabled) wins when +// set; otherwise the deployment default (deployment_settings. +// review_finding_linear_enabled); otherwise ON. Both columns are nullable +// INTEGER tri-states (NULL/0/1) where NULL means "inherit / unset". +// +// The gate lives at enqueue time only. When resolved off we skip placing the +// `review_finding_linear_issue` outbox row; the findings themselves are still +// persisted and still surface in the PR review. Only `synthetic_review` tasks +// reach the enqueue path (see enqueueReviewFindingLinearIssuesInOpenTxn), so +// worker-authored `quay_owned` tasks are unaffected regardless of the switch. + +import type { DB } from "../db/connection.ts"; +import type { Clock } from "../ports/clock.ts"; +import { enqueueReviewFindingLinearIssuesInOpenTxn } from "./review_finding_linear_outbox.ts"; + +interface ToggleRow { + review_finding_linear_enabled: number | null; +} + +export function resolveReviewFindingLinearEnabled(db: DB, repoId: string): boolean { + const repoRow = + db + .query( + `SELECT review_finding_linear_enabled FROM repos WHERE repo_id = ?`, + ) + .get(repoId) ?? null; + if (repoRow !== null && repoRow.review_finding_linear_enabled !== null) { + return repoRow.review_finding_linear_enabled !== 0; + } + + const globalRow = + db + .query( + `SELECT review_finding_linear_enabled + FROM deployment_settings + WHERE singleton_id = 1`, + ) + .get() ?? null; + if (globalRow !== null && globalRow.review_finding_linear_enabled !== null) { + return globalRow.review_finding_linear_enabled !== 0; + } + + // Unset at both scopes resolves to ON, preserving the current behavior. + return true; +} + +// Enqueue gate used by tick's review-finding persistence. Delegates to the +// pure enqueue when the resolved value is on; skips it (no outbox row) when +// off. Returns the enqueued outbox ids (empty when gated off). +export function enqueueReviewFindingLinearIssuesIfEnabledInOpenTxn( + deps: { db: DB; clock: Clock }, + input: { + taskId: string; + attemptId: number; + reviewId: string; + repoId: string; + }, +): number[] { + if (!resolveReviewFindingLinearEnabled(deps.db, input.repoId)) return []; + return enqueueReviewFindingLinearIssuesInOpenTxn(deps, { + taskId: input.taskId, + attemptId: input.attemptId, + reviewId: input.reviewId, + }); +} diff --git a/packages/cli/src/core/tick.ts b/packages/cli/src/core/tick.ts index 1a081a5f..dc8f424c 100644 --- a/packages/cli/src/core/tick.ts +++ b/packages/cli/src/core/tick.ts @@ -74,7 +74,7 @@ import { import { createIdentityMappingService } from "./identity_mappings.ts"; import { ensureWorkItemRunIdentity } from "./enqueue.ts"; import { enqueuePrReadyApprovedOutboxItem } from "./pr_ready_approved_outbox.ts"; -import { enqueueReviewFindingLinearIssuesInOpenTxn } from "./review_finding_linear_outbox.ts"; +import { enqueueReviewFindingLinearIssuesIfEnabledInOpenTxn } from "./review_finding_linear_policy.ts"; import { normalizeStoredSlackThreadRef } from "./slack_thread_ref.ts"; import { processGoalCompletionAudit, @@ -113,6 +113,8 @@ export const DEFAULT_MAX_NON_BUDGET_RESPAWNS = 20; export const DEFAULT_LOW_PRIORITY_PR_POLL_INTERVAL_MINUTES = 5; export const DEFAULT_PARKED_PR_POLL_INTERVAL_MINUTES = 15; export const DEFAULT_GITHUB_GRAPHQL_BACKOFF_MINUTES = 10; +export const DEFAULT_SPAWN_FAILURE_BACKOFF_BASE_SECONDS = 5 * 60; +export const DEFAULT_SPAWN_FAILURE_BACKOFF_MAX_SECONDS = 60 * 60; export const DEFAULT_RETAINED_CANCELLED_WORKTREE_RETENTION_HOURS = 24; export const DEFAULT_RETAINED_CANCELLED_WORKTREE_GC_BATCH_SIZE = 10; export const WORKER_GH_TOKEN_ENV = "QUAY_WORKER_GH_TOKEN"; @@ -255,6 +257,7 @@ export type TickAction = | "ci_passed" | "adopted_pr_reconciled" | "umbrella_final_pr_reconciled" + | "umbrella_retired" | "pr_merged" | "pr_closed_unmerged" | "review_respawn_scheduled" @@ -450,6 +453,12 @@ interface ReadyUmbrellaFinalPrWorkflowRow { final_pr_url: string | null; } +interface RetirableUmbrellaWorkflowRow { + umbrella_workflow_id: number; + external_ref: string; + repo_id: string; +} + interface UmbrellaFinalPrExpectedSubtaskRow { external_ref: string; title: string | null; @@ -1117,7 +1126,7 @@ export async function tick_once( const runningReviewSnapshot = readRunningReviewAttempts(deps.db).filter( (t) => !cancelledIds.has(t.task_id), ); - const pendingReviewSnapshot = readPendingReviewAttempts(deps.db).filter( + const pendingReviewSnapshot = readPendingReviewAttempts(deps.db, nowISO).filter( (t) => !cancelledIds.has(t.task_id), ); @@ -1131,7 +1140,7 @@ export async function tick_once( } } - const queuedSnapshot = readQueued(deps.db).filter( + const queuedSnapshot = readQueued(deps.db, nowISO).filter( (t) => !cancelledIds.has(t.task_id) && !closedUnmergedIds.has(t.task_id), ); @@ -1194,6 +1203,16 @@ export async function tick_once( } } + for (const workflow of readRetirableUmbrellaWorkflows(deps.db)) { + const auditTaskId = `umbrella-${workflow.umbrella_workflow_id}`; + try { + const result = retireUmbrellaWorkflow(deps, workflow); + if (result !== null) results.push(result); + } catch (err) { + results.push(recordTickError(deps, auditTaskId, err)); + } + } + for (const task of prOpenSnapshot) { const skipped = githubBackoffSkipResult(githubBackoff, task.task_id); if (skipped !== null) { @@ -1634,9 +1653,9 @@ function readCancelTargets(db: DB): CancelTargetRow[] { .all(); } -function readQueued(db: DB): QueuedTaskRow[] { +function readQueued(db: DB, nowISO: string): QueuedTaskRow[] { return db - .query( + .query( `SELECT t.task_id, t.repo_id, t.branch_name, COALESCE(t.base_branch, r.base_branch) AS base_branch, t.tmux_id, t.worktree_path, @@ -1644,9 +1663,13 @@ function readQueued(db: DB): QueuedTaskRow[] { t.worker_execution FROM tasks t JOIN repos r ON r.repo_id = t.repo_id WHERE t.state = 'queued' + AND ( + t.spawn_retry_next_eligible_at IS NULL + OR t.spawn_retry_next_eligible_at <= ? + ) ORDER BY t.created_at, t.task_id`, ) - .all(); + .all(nowISO); } function readRunning(db: DB): RunningTaskRow[] { @@ -1790,6 +1813,48 @@ function readReadyUmbrellaFinalPrWorkflows( .all(); } +// BRIX-1924: an umbrella can never reach its final PR once every expected +// child is linked to a Quay task that terminated as `cancelled` — those +// children can never reach `merged_to_feature_branch`, and none of the +// expected rows is still `expected` (enqueueable) or `complete_without_quay` +// (a success). Such an umbrella would otherwise linger `active` forever +// (observed on BRIX-1902). This is the exact inverse of the readiness gate in +// `readReadyUmbrellaFinalPrWorkflows`: retire when NOT EXISTS an expected task +// that is anything other than "linked to a cancelled task". Umbrellas with a +// child still in progress, already merged to the feature branch, complete +// without Quay, or still unlinked (`expected`) are left untouched. +function readRetirableUmbrellaWorkflows( + db: DB, +): RetirableUmbrellaWorkflowRow[] { + return db + .query( + `SELECT uw.umbrella_workflow_id, uw.external_ref, uw.repo_id + FROM umbrella_workflows uw + WHERE uw.state = 'active' + AND EXISTS ( + SELECT 1 + FROM umbrella_expected_tasks uet + WHERE uet.umbrella_workflow_id = uw.umbrella_workflow_id + ) + AND NOT EXISTS ( + SELECT 1 + 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 + WHERE uet.umbrella_workflow_id = uw.umbrella_workflow_id + AND NOT ( + ut.task_id IS NOT NULL + AND t.state = 'cancelled' + ) + ) + ORDER BY uw.created_at, uw.umbrella_workflow_id`, + ) + .all(); +} + function readSyntheticReviewLifecycle( db: DB, ): SyntheticReviewLifecycleTaskRow[] { @@ -1907,9 +1972,9 @@ function readRunningReviewAttempts(db: DB): ReviewAttemptTaskRow[] { .all(); } -function readPendingReviewAttempts(db: DB): ReviewAttemptTaskRow[] { +function readPendingReviewAttempts(db: DB, nowISO: string): ReviewAttemptTaskRow[] { return db - .query( + .query( `SELECT t.task_id, t.repo_id, t.branch_name, t.tmux_id, t.worktree_path, CASE WHEN t.authoring_mode = 'quay_owned' @@ -1930,9 +1995,13 @@ function readPendingReviewAttempts(db: DB): ReviewAttemptTaskRow[] { AND a.reason = 'review_only' AND a.spawned_at IS NULL AND a.ended_at IS NULL + AND ( + t.spawn_retry_next_eligible_at IS NULL + OR t.spawn_retry_next_eligible_at <= ? + ) ORDER BY a.attempt_id`, ) - .all(); + .all(nowISO); } function loadCurrentAttempt(db: DB, taskId: string): CurrentAttemptRow | null { @@ -2276,10 +2345,13 @@ function persistReviewFindingsInOpenTxn( now, rawReviewResult, }); - enqueueReviewFindingLinearIssuesInOpenTxn(deps, { + // Enqueue gate (BRIX-1898): only place the Linear-issue outbox row when the + // toggle resolves on for this repo. Findings persistence above is unchanged. + enqueueReviewFindingLinearIssuesIfEnabledInOpenTxn(deps, { taskId: task.task_id, attemptId: task.attempt_id, reviewId, + repoId: task.repo_id, }); } @@ -3069,12 +3141,64 @@ function processUmbrellaSubtaskAutoMerge( ); } } + // Non-retryable merge failure (e.g. the repo forbids the merge method): + // retrying the identical merge every tick can never succeed, so park the + // task with the reason recorded instead of looping (BRIX-1921). + if (err instanceof GitHubMergeError && err.kind === "method_not_allowed") { + return parkNonRetryableUmbrellaMerge( + deps, + task, + attempt, + `PR #${prNumber} auto-merge failed with a non-retryable error: ${err.message}`, + ); + } throw err; } return finalizePrTerminal(deps, task, attempt, "merged", "done"); } +// Park a done umbrella subtask whose auto-merge failed non-retryably. Moves +// the task into `non_budget_loop` (the same parked state the non-budget cap +// uses) with the reason recorded in `tick_error`, so the per-tick merge retry +// stops. Parked tasks are still polled by `processParkedPrTerminal`, which +// finalizes them if the PR later merges/closes externally but never +// re-attempts the doomed merge. +function parkNonRetryableUmbrellaMerge( + deps: TickDeps, + task: DoneTaskRow, + attempt: CurrentAttemptRow, + reason: string, +): TickTaskResult { + const now = deps.clock.nowISO(); + deps.db.exec("BEGIN IMMEDIATE"); + try { + deps.db + .query( + `UPDATE tasks + SET state = 'non_budget_loop', + tick_error = ?, + updated_at = ? + WHERE task_id = ? AND state = 'done' AND cancel_requested_at IS NULL`, + ) + .run(reason, now, task.task_id); + deps.db + .query( + `INSERT INTO events ( + task_id, attempt_id, event_type, from_state, to_state, occurred_at + ) VALUES (?, ?, 'non_budget_loop_parked', 'done', 'non_budget_loop', ?)`, + ) + .run(task.task_id, attempt.attempt_id, now); + deps.db.exec("COMMIT"); + } catch (err) { + try { + deps.db.exec("ROLLBACK"); + } catch {} + throw err; + } + return { task_id: task.task_id, action: "non_budget_loop_parked" }; +} + function reviewAppliesToHead(snapshot: PrSnapshot): boolean { return ( snapshot.latestReview.submittedHeadSha === undefined || @@ -3555,6 +3679,32 @@ function markFinalUmbrellaWorkflowCompleted( ).run(now, taskId); } +// BRIX-1924: transition an orphaned umbrella (all children cancelled, nothing +// left that could ever complete) to the terminal `cancelled` state. The +// `state = 'active'` predicate keeps this idempotent — a re-run whose UPDATE +// matches no row (already retired) returns null and emits no audit action. +function retireUmbrellaWorkflow( + deps: TickDeps, + workflow: RetirableUmbrellaWorkflowRow, +): TickTaskResult | null { + const now = deps.clock.nowISO(); + const upd = deps.db + .query( + `UPDATE umbrella_workflows + SET state = 'cancelled', + updated_at = ? + WHERE umbrella_workflow_id = ? + AND state = 'active'`, + ) + .run(now, workflow.umbrella_workflow_id); + const changed = (upd as { changes?: number }).changes ?? 0; + if (changed === 0) return null; + return { + task_id: `umbrella-${workflow.umbrella_workflow_id}`, + action: "umbrella_retired", + }; +} + function isUmbrellaTask(db: DB, taskId: string): boolean { const row = db .query<{ n: number }, [string]>( @@ -3598,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`, ) @@ -5246,6 +5400,7 @@ function markReviewInfraFailure( ? Math.max(observedFailures, 3) : observedFailures; const parking = promptMissedReviewResultProtocol || failures >= 3; + const nextEligibleAt = parking ? null : spawnFailureBackoffUntil(now, failures); deps.db.exec("BEGIN IMMEDIATE"); try { @@ -5283,6 +5438,8 @@ function markReviewInfraFailure( review_infra_failure_head_sha = ?, state = ?, tick_error = ?, + spawn_retry_next_eligible_at = ?, + spawn_failure_reason = ?, updated_at = ? WHERE task_id = ? AND state = 'pr-review' AND cancel_requested_at IS NULL`, @@ -5292,6 +5449,8 @@ function markReviewInfraFailure( task.head_sha, parking ? "non_budget_loop" : "pr-review", parking ? diagnostic : null, + nextEligibleAt, + diagnostic, now, task.task_id, ); @@ -5335,14 +5494,15 @@ function markReviewInfraFailure( deps.db .query( `INSERT INTO events ( - task_id, attempt_id, event_type, from_state, to_state, occurred_at - ) VALUES (?, ?, 'review_infra_failed', 'pr-review', ?, ?)`, + task_id, attempt_id, event_type, from_state, to_state, occurred_at, event_data + ) VALUES (?, ?, 'review_infra_failed', 'pr-review', ?, ?, ?)`, ) .run( task.task_id, task.attempt_id, parking ? "non_budget_loop" : "pr-review", now, + JSON.stringify({ failures, diagnostic, next_eligible_at: nextEligibleAt }), ); deps.db.exec("COMMIT"); } catch (err) { @@ -6370,26 +6530,52 @@ function handleSpawnFailure( ) .get(attempt.consumed_budget, now, task.task_id); const failures = updated?.n ?? 0; + const diagnostic = `worker spawn substrate failed ${failures} consecutive time(s) for repo ${task.repo_id}`; if (failures >= maxSpawnFailures) { deps.db - .query(`UPDATE tasks SET state = 'worktree_error' WHERE task_id = ?`) - .run(task.task_id); + .query( + `UPDATE tasks + SET state = 'worktree_error', + spawn_retry_next_eligible_at = NULL, + spawn_failure_reason = ? + WHERE task_id = ?`, + ) + .run(diagnostic, task.task_id); deps.db .query( `INSERT INTO events ( - task_id, attempt_id, event_type, from_state, to_state, occurred_at - ) VALUES (?, ?, 'worktree_error', 'running', 'worktree_error', ?)`, + task_id, attempt_id, event_type, from_state, to_state, occurred_at, event_data + ) VALUES (?, ?, 'worktree_error', 'running', 'worktree_error', ?, ?)`, ) - .run(task.task_id, attempt.attempt_id, now); + .run( + task.task_id, + attempt.attempt_id, + now, + JSON.stringify({ failures, diagnostic }), + ); } else { scheduleCleanSpawnRetry(deps, { taskId: task.task_id, prevAttempt: attempt }); + const nextEligibleAt = spawnFailureBackoffUntil(now, failures); + deps.db + .query( + `UPDATE tasks + SET spawn_retry_next_eligible_at = ?, + spawn_failure_reason = ? + WHERE task_id = ?`, + ) + .run(nextEligibleAt, diagnostic, task.task_id); deps.db .query( `INSERT INTO events ( - task_id, attempt_id, event_type, from_state, to_state, occurred_at - ) VALUES (?, ?, 'spawn_failed', 'running', 'queued', ?)`, + task_id, attempt_id, event_type, from_state, to_state, occurred_at, event_data + ) VALUES (?, ?, 'spawn_failed', 'running', 'queued', ?, ?)`, ) - .run(task.task_id, attempt.attempt_id, now); + .run( + task.task_id, + attempt.attempt_id, + now, + JSON.stringify({ failures, next_eligible_at: nextEligibleAt, diagnostic }), + ); } deps.db.exec("COMMIT"); return { task_id: task.task_id, action: "spawn_failed" }; @@ -6401,6 +6587,15 @@ function handleSpawnFailure( } } +function spawnFailureBackoffUntil(nowISO: string, failures: number): string { + const exponent = Math.max(0, failures - 1); + const seconds = Math.min( + DEFAULT_SPAWN_FAILURE_BACKOFF_MAX_SECONDS, + DEFAULT_SPAWN_FAILURE_BACKOFF_BASE_SECONDS * 2 ** exponent, + ); + return new Date(Date.parse(nowISO) + seconds * 1000).toISOString(); +} + function countRunning(db: DB): number { const row = db .query<{ n: number }, []>( @@ -6525,13 +6720,15 @@ function handleWorkerAuthInvalidPreflight( .query( `UPDATE tasks SET state = 'awaiting-next-brief', + spawn_retry_next_eligible_at = NULL, + spawn_failure_reason = ?, tick_error = NULL, updated_at = ? WHERE task_id = ? AND state = 'queued' AND cancel_requested_at IS NULL`, ) - .run(now, task.task_id); + .run(diagnostic, now, task.task_id); if (((taskUpdate as { changes?: number }).changes ?? 0) === 0) { deps.db.exec("ROLLBACK"); return { task_id: task.task_id, action: "skipped_predicate" }; @@ -6566,6 +6763,15 @@ function handleWorkerAuthInvalidPreflight( taskId: task.task_id, prevAttempt: pending, }); + const nextEligibleAt = spawnFailureBackoffUntil(now, 1); + deps.db + .query( + `UPDATE tasks + SET spawn_retry_next_eligible_at = ?, + spawn_failure_reason = ? + WHERE task_id = ?`, + ) + .run(nextEligibleAt, diagnostic, task.task_id); deps.db .query( `INSERT INTO events ( @@ -6576,7 +6782,11 @@ function handleWorkerAuthInvalidPreflight( task.task_id, pending.attempt_id, now, - JSON.stringify({ repeated: false, diagnostic }), + JSON.stringify({ + repeated: false, + diagnostic, + next_eligible_at: nextEligibleAt, + }), ); } deps.db.exec("COMMIT"); @@ -6674,6 +6884,67 @@ function refreshDependencyReleasedWorktreeIfNeeded( return null; } +function recreateMissingQueuedWorktreeIfNeeded( + deps: TickDeps, + task: QueuedTaskRow, + pending: PendingAttemptRow, + now: string, +): TickTaskResult | null { + if (pending.attempt_number <= 1) return null; + if (existsSync(task.worktree_path)) return null; + + let recoveryBaseBranch: string; + let recoveryBaseRef: string; + try { + const repo = loadWorktreeDependencyRepo(deps.db, task.repo_id); + if (deps.git.hasRemoteBranch(task.repo_id, task.branch_name)) { + recoveryBaseBranch = task.branch_name; + recoveryBaseRef = `origin/${task.branch_name}`; + } else { + recoveryBaseBranch = task.base_branch; + recoveryBaseRef = `origin/${task.base_branch}`; + } + deps.git.fetch(task.repo_id, recoveryBaseBranch); + deps.git.worktreePrune(task.repo_id); + deps.git.worktreeAddExistingBranch( + task.repo_id, + task.worktree_path, + task.branch_name, + recoveryBaseRef, + ); + try { + installWorktreeDependencies(deps.commandRunner, repo, task.worktree_path); + } catch (err) { + removeUmbrellaFinalPrWorktreeBestEffort(deps, task.worktree_path); + throw err; + } + } catch (err) { + return { + task_id: task.task_id, + action: "spawn_substrate_failed", + error: err instanceof Error ? err.message : String(err), + }; + } + + deps.db + .query( + `INSERT INTO events (task_id, event_type, occurred_at, event_data) + VALUES (?, 'worktree_recreated', ?, ?)`, + ) + .run( + task.task_id, + now, + JSON.stringify({ + reason: "missing_queued_worktree", + branch_name: task.branch_name, + recovery_base_branch: recoveryBaseBranch, + recovery_base_ref: recoveryBaseRef, + worktree_path: task.worktree_path, + }), + ); + return null; +} + function promoteAndSpawn( deps: TickDeps, task: QueuedTaskRow, @@ -6756,6 +7027,13 @@ function promoteAndSpawn( now, ); if (refreshResult !== null) return refreshResult; + const recreateResult = recreateMissingQueuedWorktreeIfNeeded( + deps, + task, + pending, + now, + ); + if (recreateResult !== null) return recreateResult; const spawnEnv = addCodexLaunchIsolation( githubToken.env, @@ -6831,7 +7109,13 @@ function promoteAndSpawn( ) .run(sessionName, agentIdentity, agentName, agentModel, pending.attempt_id); deps.db - .query(`UPDATE tasks SET spawn_failures_consecutive = 0 WHERE task_id = ?`) + .query( + `UPDATE tasks + SET spawn_failures_consecutive = 0, + spawn_retry_next_eligible_at = NULL, + spawn_failure_reason = NULL + WHERE task_id = ?`, + ) .run(task.task_id); linearSyncs.enqueue(task.external_ref, LINEAR_STATE_IN_PROGRESS); @@ -7000,6 +7284,76 @@ function probeGithubActorToken( return { ok: true }; } +function recordReviewerSpawnPreflightFailure( + deps: TickDeps, + task: ReviewAttemptTaskRow, + diagnostic: string, + options: TickOptions, +): TickTaskResult { + const now = deps.clock.nowISO(); + const sameSha = task.review_infra_failure_head_sha === task.head_sha; + const failures = sameSha + ? task.review_infra_failures_consecutive + 1 + : 1; + const maxSpawnFailures = + options.maxSpawnFailures ?? DEFAULT_MAX_SPAWN_FAILURES; + const parking = failures >= maxSpawnFailures; + const nextEligibleAt = parking ? null : spawnFailureBackoffUntil(now, failures); + + deps.db.exec("BEGIN"); + try { + deps.db + .query( + `UPDATE tasks + SET review_infra_failures_consecutive = ?, + review_infra_failure_head_sha = ?, + state = ?, + tick_error = ?, + spawn_retry_next_eligible_at = ?, + spawn_failure_reason = ?, + updated_at = ? + WHERE task_id = ? + AND state = 'pr-review' + AND cancel_requested_at IS NULL`, + ) + .run( + failures, + task.head_sha, + parking ? "non_budget_loop" : "pr-review", + parking ? diagnostic : null, + nextEligibleAt, + diagnostic, + now, + task.task_id, + ); + deps.db + .query( + `INSERT INTO events ( + task_id, attempt_id, event_type, from_state, to_state, occurred_at, event_data + ) VALUES (?, ?, 'review_infra_failed', 'pr-review', ?, ?, ?)`, + ) + .run( + task.task_id, + task.attempt_id, + parking ? "non_budget_loop" : "pr-review", + now, + JSON.stringify({ failures, diagnostic, next_eligible_at: nextEligibleAt }), + ); + deps.db.exec("COMMIT"); + } catch (err) { + try { + deps.db.exec("ROLLBACK"); + } catch {} + throw err; + } + + return { + task_id: task.task_id, + action: parking ? "non_budget_loop_parked" : "spawn_substrate_failed", + error: diagnostic, + }; +} + function addCodexLaunchIsolation( env: TmuxSpawnInput["env"], worktreePath: string, @@ -7129,11 +7483,12 @@ function promoteAndSpawnReviewer( options, ); if (!tokenPreflight.ok) { - return { - task_id: task.task_id, - action: "spawn_substrate_failed", - error: tokenPreflight.error, - }; + return recordReviewerSpawnPreflightFailure( + deps, + task, + tokenPreflight.error, + options, + ); } if (task.authoring_mode === "synthetic_review") { @@ -7247,6 +7602,14 @@ function promoteAndSpawnReviewer( WHERE attempt_id = ?`, ) .run(sessionName, agentIdentity, agentName, agentModel, task.attempt_id); + deps.db + .query( + `UPDATE tasks + SET spawn_retry_next_eligible_at = NULL, + spawn_failure_reason = NULL + WHERE task_id = ?`, + ) + .run(task.task_id); return { task_id: task.task_id, action: "spawned" }; } diff --git a/packages/cli/src/core/worker_prompt.ts b/packages/cli/src/core/worker_prompt.ts index 43e9cc1d..1aa05528 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/src/ports/git.ts b/packages/cli/src/ports/git.ts index 26ff9bb9..1bce4bb9 100644 --- a/packages/cli/src/ports/git.ts +++ b/packages/cli/src/ports/git.ts @@ -41,6 +41,7 @@ export interface GitPort { worktreeHeadSha(worktreePath: string): string | null; worktreeDetach(worktreePath: string): void; worktreeRemove(worktreePath: string): void; + worktreePrune(repoId: string): void; branchDelete(repoId: string, branch: string): void; // Returns the SHA at origin/ in the bare clone after a fetch, or null // if the remote ref does not exist. diff --git a/packages/cli/src/ports/github.ts b/packages/cli/src/ports/github.ts index cc84c215..7b545ab5 100644 --- a/packages/cli/src/ports/github.ts +++ b/packages/cli/src/ports/github.ts @@ -243,10 +243,23 @@ export interface PostedReviewAuthor { decision: PostedReview["decision"]; } +// `kind` classifies the merge failure so the tick can decide whether to retry. +// `head_mismatch` and `not_mergeable` are RETRYABLE (head moved / transient +// unmergeable — a later tick may succeed). `method_not_allowed` is +// NON-RETRYABLE: the repo forbids the merge method GitHub was asked to use +// ("Merge commits are not allowed on this repository" and similar), so +// retrying the identical merge every tick can never succeed — the tick parks +// the task instead of looping (BRIX-1921). `unknown` is the catch-all. +export type GitHubMergeErrorKind = + | "not_mergeable" + | "head_mismatch" + | "method_not_allowed" + | "unknown"; + export class GitHubMergeError extends Error { constructor( message: string, - readonly kind: "not_mergeable" | "head_mismatch" | "unknown", + readonly kind: GitHubMergeErrorKind, ) { super(message); this.name = "GitHubMergeError"; diff --git a/packages/cli/src/ports/linear.ts b/packages/cli/src/ports/linear.ts index e50ce690..671caa65 100644 --- a/packages/cli/src/ports/linear.ts +++ b/packages/cli/src/ports/linear.ts @@ -45,19 +45,6 @@ export interface LinearIssueHierarchy { children: LinearHierarchyIssue[]; } -export interface LinearCreatedIssue { - id: string; - identifier: string; - url: string; -} - -export interface LinearCreateIssueInput { - title: string; - body: string; - teamKey?: string | null; - idempotencyKey?: string | null; -} - export interface LinearPort { // Returns null on 404 (no such issue). // Throws `ticket_not_actionable` on draft issues, `adapter_error` with @@ -87,10 +74,4 @@ export interface LinearPort { // Updates the Linear issue markdown description. Used by enqueue-time // metadata writeback after Quay infers missing canonical config fields. updateIssueBody(identifier: string, body: string): Promise; - - // Creates a follow-up issue in the configured/default Linear team. When - // supplied, `idempotencyKey` is forwarded as Linear's client-generated issue - // id so provider retries converge even if Quay crashes before persisting the - // local provider-link row. - createIssue(input: LinearCreateIssueInput): Promise; } diff --git a/packages/cli/tests/adapters/linear_adapter.test.ts b/packages/cli/tests/adapters/linear_adapter.test.ts index af3fb973..e007b8de 100644 --- a/packages/cli/tests/adapters/linear_adapter.test.ts +++ b/packages/cli/tests/adapters/linear_adapter.test.ts @@ -189,60 +189,6 @@ test("test_linear_adapter_get_issue_returns_structured_payload", async () => { expect(handle.requests[0]!.headers.Authorization).toBe("test-token"); }); -test("test_linear_adapter_create_issue_uses_default_team_key", async () => { - const handle = recorder((req) => { - if (req.parsedBody.query.includes("GetTeamByKey")) { - expect(req.parsedBody.variables).toEqual({ key: "BRIX" }); - return jsonResponse({ - data: { - teams: { - nodes: [{ id: "team-brix", key: "BRIX" }], - }, - }, - }); - } - expect(req.parsedBody.query).toContain("issueCreate"); - expect(req.parsedBody.variables).toEqual({ - input: { - id: "11111111-1111-5111-9111-111111111111", - teamId: "team-brix", - title: "Follow-up title", - description: "Follow-up body", - }, - }); - return jsonResponse({ - data: { - issueCreate: { - success: true, - issue: { - id: "issue-uuid", - identifier: "BRIX-9999", - url: "https://linear.app/inverter/issue/BRIX-9999", - }, - }, - }, - }); - }); - const adapter = new LinearAdapter({ - token: "test-token", - defaultIssueTeamKey: "BRIX", - transport: handle.transport, - }); - - const created = await adapter.createIssue({ - title: "Follow-up title", - body: "Follow-up body", - idempotencyKey: "11111111-1111-5111-9111-111111111111", - }); - - expect(created).toEqual({ - id: "issue-uuid", - identifier: "BRIX-9999", - url: "https://linear.app/inverter/issue/BRIX-9999", - }); - expect(handle.requests).toHaveLength(2); -}); - test("linear adapter returns native blocked-by relations with blocker metadata", async () => { const handle = recorder((req) => { if (req.parsedBody.query.includes("inverseRelations")) { 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 new file mode 100644 index 00000000..63fe9cb7 --- /dev/null +++ b/packages/cli/tests/adapters/test_github_adapter_merge_method.test.ts @@ -0,0 +1,270 @@ +// 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 +// each case can assert which flag the adapter chose. + +import { + chmodSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, expect, test } from "bun:test"; +import { + GitHubCliAdapter, + classifyMergeErrorKind, +} from "../../src/adapters/github.ts"; +import { GitHubMergeError } from "../../src/ports/github.ts"; + +let cleanups: Array<() => void> = []; +let savedPath: string | undefined; + +beforeEach(() => { + savedPath = process.env.PATH; +}); + +afterEach(() => { + if (savedPath !== undefined) process.env.PATH = savedPath; + for (const fn of cleanups.splice(0)) { + try { + fn(); + } catch {} + } +}); + +function tempDir(prefix = "quay-gh-merge-"): string { + const d = mkdtempSync(join(tmpdir(), prefix)); + cleanups.push(() => rmSync(d, { recursive: true, force: true })); + return d; +} + +function installGhStub(body: string): string { + const bin = tempDir(); + writeFileSync(join(bin, "gh"), `#!/bin/sh\n${body}\n`); + chmodSync(join(bin, "gh"), 0o755); + process.env.PATH = `${bin}:${process.env.PATH ?? ""}`; + return bin; +} + +function makeBareDir(): { reposRoot: string; repoId: string } { + const reposRoot = tempDir("quay-gh-merge-repos-"); + const repoId = "fake-repo"; + mkdirSync(join(reposRoot, `${repoId}.git`), { recursive: true }); + return { reposRoot, repoId }; +} + +interface AllowPolicy { + merge: boolean; + squash: boolean; + rebase: boolean; +} + +// A `gh` stub that answers `gh api repos/...` with the given policy (appending +// one line to `apiLog` per read) and answers `gh pr merge ...` by appending the +// full argv to `mergeLog`, then exits with `mergeExit` (writing `mergeStderr`). +function mergeMethodStub(opts: { + allow: AllowPolicy; + apiLog: string; + mergeLog: string; + mergeExit?: number; + mergeStderr?: string; +}): string { + const { allow, apiLog, mergeLog } = opts; + const mergeExit = opts.mergeExit ?? 0; + const mergeStderr = opts.mergeStderr ?? ""; + return ` +if [ "$1" = "api" ]; then + case "$2" in + repos/*) + printf 'read\\n' >> '${apiLog}' + echo '{"allow_merge_commit":${allow.merge},"allow_squash_merge":${allow.squash},"allow_rebase_merge":${allow.rebase}}' + exit 0 + ;; + esac +fi +if [ "$1" = "pr" ] && [ "$2" = "merge" ]; then + printf '%s\\n' "$*" >> '${mergeLog}' + ${mergeStderr === "" ? "" : `echo '${mergeStderr}' 1>&2`} + exit ${mergeExit} +fi +echo "unexpected: $*" 1>&2 +exit 2 +`; +} + +function newLogs(): { apiLog: string; mergeLog: string } { + const d = tempDir("quay-gh-merge-logs-"); + return { apiLog: join(d, "api.log"), mergeLog: join(d, "merge.log") }; +} + +test("squash-only repo merges via --squash, not --merge", () => { + const { apiLog, mergeLog } = newLogs(); + installGhStub( + mergeMethodStub({ + allow: { merge: false, squash: true, rebase: false }, + apiLog, + mergeLog, + }), + ); + const { reposRoot, repoId } = makeBareDir(); + const adapter = new GitHubCliAdapter(reposRoot); + + adapter.mergePullRequest(repoId, 263, "deadbeef"); + + const logged = readFileSync(mergeLog, "utf8"); + expect(logged).toContain("--squash"); + expect(logged).not.toContain("--merge"); + expect(logged).toContain("--match-head-commit deadbeef"); +}); + +test("merge-commit repo still merges via --merge (unchanged behavior)", () => { + const { apiLog, mergeLog } = newLogs(); + // Multiple methods allowed → deterministic preference keeps `--merge`. + installGhStub( + mergeMethodStub({ + allow: { merge: true, squash: true, rebase: true }, + apiLog, + mergeLog, + }), + ); + const { reposRoot, repoId } = makeBareDir(); + const adapter = new GitHubCliAdapter(reposRoot); + + adapter.mergePullRequest(repoId, 42, "cafef00d"); + + const logged = readFileSync(mergeLog, "utf8"); + expect(logged).toContain("--merge"); + expect(logged).not.toContain("--squash"); + expect(logged).not.toContain("--rebase"); +}); + +test("rebase-only repo merges via --rebase", () => { + const { apiLog, mergeLog } = newLogs(); + installGhStub( + mergeMethodStub({ + allow: { merge: false, squash: false, rebase: true }, + apiLog, + mergeLog, + }), + ); + const { reposRoot, repoId } = makeBareDir(); + const adapter = new GitHubCliAdapter(reposRoot); + + adapter.mergePullRequest(repoId, 7, "abc123"); + + const logged = readFileSync(mergeLog, "utf8"); + expect(logged).toContain("--rebase"); + expect(logged).not.toContain("--merge"); + expect(logged).not.toContain("--squash"); +}); + +test("squash preferred over rebase when merge disallowed", () => { + const { apiLog, mergeLog } = newLogs(); + installGhStub( + mergeMethodStub({ + allow: { merge: false, squash: true, rebase: true }, + apiLog, + mergeLog, + }), + ); + const { reposRoot, repoId } = makeBareDir(); + const adapter = new GitHubCliAdapter(reposRoot); + + adapter.mergePullRequest(repoId, 9, "sha9"); + + const logged = readFileSync(mergeLog, "utf8"); + expect(logged).toContain("--squash"); + expect(logged).not.toContain("--rebase"); +}); + +test("repo allowing no merge method raises a clear error", () => { + const { apiLog, mergeLog } = newLogs(); + installGhStub( + mergeMethodStub({ + allow: { merge: false, squash: false, rebase: false }, + apiLog, + mergeLog, + }), + ); + const { reposRoot, repoId } = makeBareDir(); + const adapter = new GitHubCliAdapter(reposRoot); + + expect(() => adapter.mergePullRequest(repoId, 1, "sha")).toThrow( + /allows no merge method/i, + ); +}); + +test("allowed merge method is cached per repo (single api read across merges)", () => { + const { apiLog, mergeLog } = newLogs(); + installGhStub( + mergeMethodStub({ + allow: { merge: false, squash: true, rebase: false }, + apiLog, + mergeLog, + }), + ); + const { reposRoot, repoId } = makeBareDir(); + const adapter = new GitHubCliAdapter(reposRoot); + + adapter.mergePullRequest(repoId, 263, "sha263"); + adapter.mergePullRequest(repoId, 264, "sha264"); + adapter.mergePullRequest(repoId, 265, "sha265"); + + // Three merges, but the merge-method policy is read exactly once. + const reads = readFileSync(apiLog, "utf8").trim().split("\n").filter(Boolean); + expect(reads).toHaveLength(1); + const merges = readFileSync(mergeLog, "utf8").trim().split("\n").filter(Boolean); + expect(merges).toHaveLength(3); + expect(merges.every((m) => m.includes("--squash"))).toBe(true); +}); + +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. + installGhStub( + mergeMethodStub({ + allow: { merge: true, squash: false, rebase: false }, + apiLog, + mergeLog, + mergeExit: 1, + mergeStderr: "Merge commits are not allowed on this repository", + }), + ); + const { reposRoot, repoId } = makeBareDir(); + const adapter = new GitHubCliAdapter(reposRoot); + + let caught: unknown; + try { + adapter.mergePullRequest(repoId, 263, "sha263"); + } catch (err) { + caught = err; + } + expect(caught).toBeInstanceOf(GitHubMergeError); + expect((caught as GitHubMergeError).kind).toBe("method_not_allowed"); +}); + +test("classifyMergeErrorKind maps gh merge failure messages", () => { + expect( + classifyMergeErrorKind("Merge commits are not allowed on this repository"), + ).toBe("method_not_allowed"); + expect( + classifyMergeErrorKind("Squash merges are not allowed on this repository"), + ).toBe("method_not_allowed"); + expect( + classifyMergeErrorKind("Rebase merges are not allowed on this repository"), + ).toBe("method_not_allowed"); + expect( + classifyMergeErrorKind( + "GraphQL: Head branch was modified. Review and try the merge again.", + ), + ).toBe("head_mismatch"); + expect(classifyMergeErrorKind("Pull request is not mergeable")).toBe( + "not_mergeable", + ); + expect(classifyMergeErrorKind("some unrecognized gh error")).toBe("unknown"); +}); diff --git a/packages/cli/tests/admin/api.test.ts b/packages/cli/tests/admin/api.test.ts index 39b209a1..a21ca59a 100644 --- a/packages/cli/tests/admin/api.test.ts +++ b/packages/cli/tests/admin/api.test.ts @@ -2245,6 +2245,104 @@ test("POST /v1/changes/apply preserves effective defaults on first partial deplo }); }); +test("GET /v1/global exposes the review-finding Linear default (ON when unset)", async () => { + h = createHarness(); + const handler = createHandler(); + + const response = await handler(new Request("http://quay.local/v1/global")); + const body = await responseJson(response); + + expect(response.status).toBe(200); + expect(body).toMatchObject({ review_findings: { linear_enabled: true } }); +}); + +test("POST /v1/changes/apply toggles the global review-finding Linear default off", async () => { + h = createHarness(); + const handler = createHandler(); + const revision = await currentRevision(handler); + + const response = await handler(postJson("/v1/changes/apply", { + base_revision: revision, + changes: [ + { + type: "deployment_settings.update", + patch: { review_finding_linear_enabled: false }, + }, + ], + })); + + expect(response.status).toBe(200); + expect( + h.db + .query<{ review_finding_linear_enabled: number | null }, []>( + `SELECT review_finding_linear_enabled + FROM deployment_settings + WHERE singleton_id = 1`, + ) + .get()?.review_finding_linear_enabled, + ).toBe(0); + + const global = await responseJson( + await handler(new Request("http://quay.local/v1/global")), + ); + expect(global).toMatchObject({ review_findings: { linear_enabled: false } }); +}); + +test("GET /v1/repos/:id exposes the review-finding Linear override (inherit by default)", async () => { + h = createHarness(); + const repoService = createRepoService({ db: h.db, clock: h.clock }); + repoService.add({ + repo_id: "repo-toggle", + repo_url: "git@example.com:owner/repo-toggle.git", + base_branch: "main", + package_manager: "bun", + install_cmd: "bun install", + }); + const handler = createHandler({ repoService }); + + const body = await responseJson( + await handler(new Request("http://quay.local/v1/repos/repo-toggle")), + ); + + expect(body.review_finding_linear_enabled).toBeNull(); +}); + +test("POST /v1/changes/apply sets a per-repo review-finding Linear override", async () => { + h = createHarness(); + const repoService = createRepoService({ db: h.db, clock: h.clock }); + repoService.add({ + repo_id: "repo-toggle", + repo_url: "git@example.com:owner/repo-toggle.git", + base_branch: "main", + package_manager: "bun", + install_cmd: "bun install", + }); + const handler = createHandler({ repoService }); + const revision = await currentRevision(handler); + + const response = await handler(postJson("/v1/changes/apply", { + base_revision: revision, + changes: [ + { + type: "repo.update", + repo_id: "repo-toggle", + patch: { review_finding_linear_enabled: false }, + }, + ], + })); + + expect(response.status).toBe(200); + expect(JSON.stringify(await responseJson(response))).toContain( + "review_finding_linear_enabled", + ); + expect(repoService.get("repo-toggle")?.review_finding_linear_enabled).toBe(false); + + const detail = await responseJson( + await handler(new Request("http://quay.local/v1/repos/repo-toggle")), + ); + expect(detail.review_finding_linear_enabled).toBe(false); +}); + test("GET /v1/global includes identity mappings and unmapped contributor discovery", async () => { h = createHarness(); const repoId = insertRepo(h.db, "repo-identities"); diff --git a/packages/cli/tests/classifier/spawn_window_classifier.test.ts b/packages/cli/tests/classifier/spawn_window_classifier.test.ts index 82c7ad51..b7e0ccb9 100644 --- a/packages/cli/tests/classifier/spawn_window_classifier.test.ts +++ b/packages/cli/tests/classifier/spawn_window_classifier.test.ts @@ -83,3 +83,74 @@ test("test_spawn_window_null_session_uses_same_classifier", async () => { expect(evTypes).toContain("blocker_ingested"); expect(evTypes).not.toContain("spawn_failed"); }); + +test("spawn-window stale pre-existing PR without current evidence is spawn_failed", async () => { + h = createHarness(); + h.clock.set("2026-07-14T10:00:00.000Z"); + + const repoId = insertRepo(h.db, "repo-spawn-window-stale-pr"); + const worktreesRoot = join(h.dataDir, "worktrees"); + const t = insertRunningTask(h.db, { + taskId: "task-spawn-window-stale-pr", + repoId, + worktreesRoot, + tmuxSession: null, + remoteShaAtSpawn: "same-remote-head", + prExistedAtSpawn: 1, + attemptsConsumed: 1, + }); + + const built = buildTickDeps(h); + built.git.setRemoteHeadSha(repoId, t.branchName, "same-remote-head"); + built.github.setPrExists(repoId, t.branchName, true); + + const results = await tick_once(built.deps); + + expect(results).toEqual([{ task_id: t.taskId, action: "spawn_failed" }]); + + const task = h.db + .query< + { + state: string; + attempts_consumed: number; + spawn_failures_consecutive: number; + }, + [string] + >( + `SELECT state, attempts_consumed, spawn_failures_consecutive + FROM tasks WHERE task_id = ?`, + ) + .get(t.taskId); + expect(task).toEqual({ + state: "queued", + attempts_consumed: 0, + spawn_failures_consecutive: 1, + }); + + const attempts = h.db + .query<{ attempt_number: number; spawned_at: string | null; exit_kind: string | null }, [string]>( + `SELECT attempt_number, spawned_at, exit_kind + FROM attempts + WHERE task_id = ? + ORDER BY attempt_number`, + ) + .all(t.taskId); + expect(attempts).toEqual([ + { + attempt_number: 1, + spawned_at: "2026-01-01T00:00:00.000Z", + exit_kind: "spawn_failed", + }, + { attempt_number: 2, spawned_at: null, exit_kind: null }, + ]); + + const evTypes = h.db + .query<{ event_type: string }, [string]>( + `SELECT event_type FROM events WHERE task_id = ? ORDER BY event_id`, + ) + .all(t.taskId) + .map((r) => r.event_type); + expect(evTypes).toContain("spawn_failed"); + expect(evTypes).not.toContain("no_progress"); + expect(evTypes).not.toContain("existing_pr_attached"); +}); diff --git a/packages/cli/tests/cli/test_config_loader.test.ts b/packages/cli/tests/cli/test_config_loader.test.ts index 617a1614..f7440024 100644 --- a/packages/cli/tests/cli/test_config_loader.test.ts +++ b/packages/cli/tests/cli/test_config_loader.test.ts @@ -348,7 +348,6 @@ test("loads [adapters.linear] and [adapters.slack] sections", () => { `[adapters.linear] enabled = true api_key_env = "MY_LINEAR_KEY" -default_issue_team_key = "BRIX" [adapters.slack] enabled = true @@ -363,7 +362,6 @@ max_thread_messages = 400 }); expect(linearAdapterOptionsFromConfig(result.config)).toEqual({ tokenEnvVar: "MY_LINEAR_KEY", - defaultIssueTeamKey: "BRIX", }); expect(slackAdapterOptionsFromConfig(result.config)).toEqual({ tokenEnvVar: "MY_SLACK_TOKEN", diff --git a/packages/cli/tests/core/codex_launch_isolation.test.ts b/packages/cli/tests/core/codex_launch_isolation.test.ts index b040dc71..bf77d0ef 100644 --- a/packages/cli/tests/core/codex_launch_isolation.test.ts +++ b/packages/cli/tests/core/codex_launch_isolation.test.ts @@ -267,6 +267,7 @@ test("worker PR visibility permission failure is classified as auth invalid", as test("worker auth preflight retries once with freshly resolved token then spawns", async () => { h = createHarness(); + h.clock.set("2026-05-13T10:00:00.000Z"); const built = buildTickDeps(h); const repoId = insertRepo(h.db, "repo-worker-auth-refresh"); const taskId = insertTask(h.db, { @@ -305,6 +306,17 @@ test("worker auth preflight retries once with freshly resolved token then spawns expect(built.tmux.spawnCalls).toHaveLength(0); writeFileSync(tokenPath, "ghs_fresh_worker_token\n"); + expect( + await tick_once(built.deps, { + workerGhTokenFile: tokenPath, + env: {}, + }), + ).toEqual([]); + expect(built.github.tokenAccessCalls.map((c) => c.token)).toEqual([ + "ghs_stale_worker_token", + ]); + + h.clock.set("2026-05-13T10:05:00.000Z"); const second = await tick_once(built.deps, { workerGhTokenFile: tokenPath, env: {}, @@ -323,6 +335,7 @@ test("worker auth preflight retries once with freshly resolved token then spawns test("worker auth preflight escalates clearly after the fresh-auth retry fails", async () => { h = createHarness(); + h.clock.set("2026-05-13T10:00:00.000Z"); const built = buildTickDeps(h); const repoId = insertRepo(h.db, "repo-worker-auth-repeated"); const taskId = insertTask(h.db, { @@ -347,6 +360,14 @@ test("worker auth preflight escalates clearly after the fresh-auth retry fails", await tick_once(built.deps, { env: { [WORKER_GH_TOKEN_ENV]: token }, }); + expect( + await tick_once(built.deps, { + env: { [WORKER_GH_TOKEN_ENV]: token }, + }), + ).toEqual([]); + expect(built.github.tokenAccessCalls).toHaveLength(1); + + h.clock.set("2026-05-13T10:05:00.000Z"); const second = await tick_once(built.deps, { env: { [WORKER_GH_TOKEN_ENV]: token }, }); diff --git a/packages/cli/tests/core/test_agent_resolver.test.ts b/packages/cli/tests/core/test_agent_resolver.test.ts index 0ca10c66..40e3cb5b 100644 --- a/packages/cli/tests/core/test_agent_resolver.test.ts +++ b/packages/cli/tests/core/test_agent_resolver.test.ts @@ -196,6 +196,7 @@ test("resolver uses DB deployment settings over TOML defaults", () => { worker_model: "db-worker-model", reviewer_agent: "claude", reviewer_model: "db-reviewer-model", + review_finding_linear_enabled: null, }, }); @@ -240,7 +241,8 @@ test("resolver treats existing DB null settings as authoritative clears", () => }, deploymentSettings: h.db .query( - `SELECT worker_agent, worker_model, reviewer_agent, reviewer_model + `SELECT worker_agent, worker_model, reviewer_agent, reviewer_model, + review_finding_linear_enabled FROM deployment_settings WHERE singleton_id = 1`, ) @@ -249,6 +251,7 @@ test("resolver treats existing DB null settings as authoritative clears", () => worker_model: string | null; reviewer_agent: string | null; reviewer_model: string | null; + review_finding_linear_enabled: boolean | null; }, }); @@ -285,8 +288,10 @@ test("resolver reads deployment settings provider on each default resolve", () = worker_model: string | null; reviewer_agent: string | null; reviewer_model: string | null; + review_finding_linear_enabled: boolean | null; }, []>( - `SELECT worker_agent, worker_model, reviewer_agent, reviewer_model + `SELECT worker_agent, worker_model, reviewer_agent, reviewer_model, + review_finding_linear_enabled FROM deployment_settings WHERE singleton_id = 1`, ) diff --git a/packages/cli/tests/core/test_linear_state_sync_helpers.test.ts b/packages/cli/tests/core/test_linear_state_sync_helpers.test.ts index 23ebe365..10532323 100644 --- a/packages/cli/tests/core/test_linear_state_sync_helpers.test.ts +++ b/packages/cli/tests/core/test_linear_state_sync_helpers.test.ts @@ -93,9 +93,6 @@ test("test_linear_sync_queue_does_not_block_on_enqueue", async () => { async updateIssueBody() { throw new Error("not used"); }, - async createIssue() { - throw new Error("not used"); - }, }; const queue = new LinearSyncQueue(slowLinear); // Enqueue is sync — this line returns before `setIssueState` resolves. diff --git a/packages/cli/tests/core/worker_prompt.test.ts b/packages/cli/tests/core/worker_prompt.test.ts index 95acec8d..00e67dab 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/pr/test_053_pr_open_merged_transitions_terminal.test.ts b/packages/cli/tests/pr/test_053_pr_open_merged_transitions_terminal.test.ts index a9314db0..365c84e8 100644 --- a/packages/cli/tests/pr/test_053_pr_open_merged_transitions_terminal.test.ts +++ b/packages/cli/tests/pr/test_053_pr_open_merged_transitions_terminal.test.ts @@ -6,6 +6,7 @@ import { afterEach, expect, test } from "bun:test"; import { mkdirSync, existsSync } from "node:fs"; import { getTask } from "../../src/cli/format.ts"; import { tick_once } from "../../src/core/tick.ts"; +import { GitHubMergeError } from "../../src/ports/github.ts"; import { createTaskDependency } from "../../src/core/task_dependencies.ts"; import { createHarness, type Harness } from "../support/harness.ts"; import { insertAttempt, insertRepo, insertTask } from "../support/fixtures.ts"; @@ -499,6 +500,119 @@ test("approved green umbrella subtask auto-merges into feature branch", async () expect(existsSync(worktreePath)).toBe(false); }); +test("non-retryable umbrella auto-merge failure parks the task instead of looping", async () => { + // BRIX-1921: if the merge is rejected for a non-retryable reason (the repo + // forbids the chosen merge method), the tick must park the task with the + // reason recorded rather than retrying the identical doomed merge every tick. + h = createHarness(); + h.clock.set("2026-04-29T15:00:00.000Z"); + + const repoId = insertRepo(h.db, "repo-umbrella-merge-rejected"); + const taskId = insertTask(h.db, { + taskId: "task-umbrella-merge-rejected", + repoId, + state: "done", + }); + insertAttempt(h.db, { + taskId, + attemptNumber: 1, + spawnedAt: "2026-04-29T14:30:00.000Z", + }); + + const workflow = h.db + .query<{ umbrella_workflow_id: number }, [string, string, string, string, string, string]>( + `INSERT INTO umbrella_workflows ( + external_ref, repo_id, base_branch, feature_branch, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?) + RETURNING umbrella_workflow_id`, + ) + .get( + "BRIX-1902", + repoId, + "dev", + "feature/brix-1902", + "2026-04-29T14:20:00.000Z", + "2026-04-29T14:20:00.000Z", + ); + h.db + .query( + `INSERT INTO umbrella_tasks ( + umbrella_workflow_id, task_id, external_ref, created_at + ) VALUES (?, ?, ?, ?)`, + ) + .run(workflow!.umbrella_workflow_id, taskId, "BRIX-1903", "2026-04-29T14:25:00.000Z"); + + const worktreePath = h.db + .query<{ worktree_path: string }, [string]>( + `SELECT worktree_path FROM tasks WHERE task_id = ?`, + ) + .get(taskId)!.worktree_path; + mkdirSync(worktreePath, { recursive: true }); + + const built = buildTickDeps(h); + built.git.setLocalBranches(repoId, [`quay/${taskId}`]); + const snapshot = { + prNumber: 263, + state: "open" as const, + headSha: "head-rejected", + baseSha: "base-rejected", + baseRef: "feature/brix-1902", + mergeable: "mergeable" as const, + latestReview: { + decision: "APPROVED" as const, + latestReviewId: "R_rejected_approved", + submittedHeadSha: "head-rejected", + comments: "", + }, + checks: { + checkSha: "head-rejected", + items: [{ name: "build", workflow: null, bucket: "pass" as const, required: true }], + }, + }; + built.github.setPrSnapshot(repoId, `quay/${taskId}`, snapshot); + built.github.setPrSnapshotByNumber(repoId, 263, snapshot); + built.github.setPrView(repoId, 263, { + number: 263, + title: "fix: umbrella subtask", + body: "", + url: "https://github.example/pr/263", + headRefName: `quay/${taskId}`, + headSha: "head-rejected", + baseRef: "feature/brix-1902", + }); + // Simulate a policy-rejected merge (the adapter's non-retryable kind). + built.github.setMergePullRequestHandler(() => { + throw new GitHubMergeError( + "gh pr merge 263 failed: Merge commits are not allowed on this repository", + "method_not_allowed", + ); + }); + + const results = await tick_once(built.deps); + expect(results).toEqual([ + { task_id: taskId, action: "non_budget_loop_parked" }, + ]); + expect(built.github.mergePullRequestCalls).toHaveLength(1); + + const parked = h.db + .query<{ state: string; tick_error: string | null }, [string]>( + `SELECT state, tick_error FROM tasks WHERE task_id = ?`, + ) + .get(taskId); + expect(parked?.state).toBe("non_budget_loop"); + expect(parked?.tick_error).toContain("non-retryable"); + expect(parked?.tick_error).toContain("not allowed on this repository"); + + // A second tick must NOT re-attempt the doomed merge (no per-tick loop). + const secondResults = await tick_once(built.deps); + expect(secondResults).toEqual([]); + expect(built.github.mergePullRequestCalls).toHaveLength(1); + const stillParked = h.db + .query<{ state: string }, [string]>(`SELECT state FROM tasks WHERE task_id = ?`) + .get(taskId); + expect(stillParked?.state).toBe("non_budget_loop"); +}); + test("normal done task is not auto-merged even when approved and green", async () => { h = createHarness(); h.clock.set("2026-04-29T13:30:00.000Z"); diff --git a/packages/cli/tests/repo/repo_add.test.ts b/packages/cli/tests/repo/repo_add.test.ts index 7a20d778..3921feb6 100644 --- a/packages/cli/tests/repo/repo_add.test.ts +++ b/packages/cli/tests/repo/repo_add.test.ts @@ -89,6 +89,51 @@ test("test_repo_add_persists_optional_repo_config", () => { expect(row2.contribution_guide_path).toBe("CONTRIBUTING.md"); }); +test("test_repo_review_finding_linear_override_round_trips_tri_state", () => { + h = createHarness(); + const repos = createRepoService({ db: h.db, clock: h.clock }); + + // Default: unset (inherit). + const inherited = repos.add({ ...REQUIRED_FIELDS, repo_id: "repo-inherit" }); + expect(inherited.review_finding_linear_enabled).toBeNull(); + + // Explicit on / off persist as booleans. + const on = repos.add({ + ...REQUIRED_FIELDS, + repo_id: "repo-on", + review_finding_linear_enabled: true, + }); + expect(on.review_finding_linear_enabled).toBe(true); + + const off = repos.add({ + ...REQUIRED_FIELDS, + repo_id: "repo-off", + review_finding_linear_enabled: false, + }); + expect(off.review_finding_linear_enabled).toBe(false); + + // Update flips and clears the override. + expect( + repos.update("repo-on", { review_finding_linear_enabled: false }) + .review_finding_linear_enabled, + ).toBe(false); + expect( + repos.update("repo-on", { review_finding_linear_enabled: null }) + .review_finding_linear_enabled, + ).toBeNull(); + + // upsert preserves an omitted override on an existing row, and applies it + // on insert. + repos.upsert({ ...REQUIRED_FIELDS, repo_id: "repo-off" }); + expect(repos.get("repo-off")?.review_finding_linear_enabled).toBe(false); + repos.upsert({ + ...REQUIRED_FIELDS, + repo_id: "repo-new", + review_finding_linear_enabled: true, + }); + expect(repos.get("repo-new")?.review_finding_linear_enabled).toBe(true); +}); + test("test_repo_add_rejects_duplicate_id", () => { h = createHarness(); const repos = createRepoService({ db: h.db, clock: h.clock }); diff --git a/packages/cli/tests/resnapshot/task_resnapshot.test.ts b/packages/cli/tests/resnapshot/task_resnapshot.test.ts new file mode 100644 index 00000000..30aa0273 --- /dev/null +++ b/packages/cli/tests/resnapshot/task_resnapshot.test.ts @@ -0,0 +1,519 @@ +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, seedTaskObjective } from "../support/fixtures.ts"; + +let h: Harness | null = null; +afterEach(() => { + h?.cleanup(); + h = null; +}); + +const FENCE = "```"; +const REPO_ID = "repo-1"; +const EXTERNAL_REF = "BRIX-1907"; + +type Built = ReturnType; + +function addRepo(harness: Harness, repoId = REPO_ID): void { + createRepoService({ db: harness.db, clock: harness.clock }).add({ + repo_id: repoId, + repo_url: `git@example.com:owner/${repoId}.git`, + base_branch: "main", + package_manager: "bun", + install_cmd: "bun install", + }); +} + +function block(repo = REPO_ID): string { + return [ + `${FENCE}quay-config`, + `repo: ${repo}`, + "tags:", + " - resnapshot", + "authors:", + " - name: Fabian Scherer", + " slack_id: U06TDC56VJB", + FENCE, + ].join("\n"); +} + +function makeIssue(context: string, identifier = EXTERNAL_REF): LinearIssue { + return { + identifier, + url: `https://linear.app/inverter/issue/${identifier}`, + title: "Re-baseline the acceptance criteria", + body: `## Context\n\n${context}\n\n${block()}\n`, + comments: [], + }; +} + +function insertTask( + db: DB, + opts: { taskId: string; state?: string; externalRef?: string | null }, +): void { + db.query( + `INSERT INTO tasks ( + task_id, repo_id, external_ref, state, branch_name, tmux_id, worktree_path, + retry_budget, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?, 5, ?, ?)`, + ).run( + opts.taskId, + REPO_ID, + opts.externalRef === undefined ? EXTERNAL_REF : opts.externalRef, + opts.state ?? "waiting_external_changes", + `quay/${opts.taskId}`, + `quay-task-${opts.taskId}`, + `/tmp/${opts.taskId}`, + "2026-01-01T00:00:00.000Z", + "2026-01-01T00:00:00.000Z", + ); +} + +// Mirror what enqueue does: compose the snapshot from the (currently set) +// Linear issue via the exact fetch+parse+snapshot path, so the seeded artifact +// is byte-identical to a real creation-time snapshot. +async function composeSnapshot(built: Built, issue: LinearIssue): Promise { + built.linear.setIssue(issue); + const fetched = await fetchTicketContextWithIssue( + { + linear: built.linear, + slack: built.slack, + config: { linearEnabled: true, slackEnabled: true }, + }, + issue.identifier, + ); + return fetched.ctx.ticket_snapshot; +} + +function seedSnapshot(built: Built, taskId: string, content: string): void { + built.deps.artifactStore.writeArtifact({ + taskId, + attemptId: null, + kind: "ticket_snapshot", + content, + extension: "md", + }); +} + +function latestSnapshot(harness: Harness, taskId: string): string { + const row = harness.db + .query<{ file_path: string }, [string]>( + `SELECT file_path FROM artifacts + WHERE task_id = ? AND kind = 'ticket_snapshot' AND attempt_id IS NULL + ORDER BY artifact_id DESC LIMIT 1`, + ) + .get(taskId)!; + 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]>( + `SELECT count(*) AS n FROM artifacts + WHERE task_id = ? AND kind = 'ticket_snapshot'`, + ) + .get(taskId)!.n; +} + +function latestEvent( + harness: Harness, + taskId: string, +): { event_type: string; from_state: string | null; to_state: string | null; event_data: string | null } { + return harness.db + .query< + { event_type: string; from_state: string | null; to_state: string | null; event_data: string | null }, + [string] + >( + `SELECT event_type, from_state, to_state, event_data + FROM events WHERE task_id = ? + ORDER BY event_id DESC LIMIT 1`, + ) + .get(taskId)!; +} + +function seedReviewOnlyAttempt( + harness: Harness, + taskId: string, + verdict: string, + headSha = "sha-original", +): void { + const preambleId = insertPreamble(harness.db); + harness.db + .query( + `INSERT INTO attempts ( + task_id, attempt_number, preamble_id, reason, consumed_budget, + spawned_at, ended_at, head_sha, review_verdict + ) VALUES (?, 1, ?, 'review_only', 0, ?, ?, ?, ?)`, + ) + .run( + taskId, + preambleId, + "2026-01-01T00:00:00.000Z", + "2026-01-01T00:00:00.000Z", + headSha, + verdict, + ); +} + +function reviewVerdict(harness: Harness, taskId: string): string | null { + return ( + harness.db + .query<{ review_verdict: string | null }, [string]>( + `SELECT review_verdict FROM attempts + WHERE task_id = ? AND reason = 'review_only' + ORDER BY attempt_id DESC LIMIT 1`, + ) + .get(taskId)?.review_verdict ?? null + ); +} + +test("resnapshot replaces the frozen snapshot from the current Linear ticket", async () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h); + + const original = await composeSnapshot( + built, + makeIssue("Original strict AC: must handle every edge case."), + ); + 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.")); + + const io = bufferIO(); + const result = await dispatch( + ["task", "resnapshot", "task-1", "--reason", "operator relaxed AC in BRIX-1907"], + built.deps, + io, + ); + + expect(result.exitCode).toBe(0); + expect(io.err()).toBe(""); + const payload = JSON.parse(io.out()); + expect(payload).toMatchObject({ + task_id: "task-1", + external_ref: EXTERNAL_REF, + changed: true, + review_invalidated: 0, + }); + expect(payload.snapshot_artifact_id).not.toBeNull(); + expect(payload.objective_artifact_id).not.toBeNull(); + + expect(snapshotCount(h, "task-1")).toBe(2); + const parsed = JSON.parse(latestSnapshot(h, "task-1")); + expect(parsed.linear_issue.body).toContain("Relaxed AC"); + expect(parsed.linear_issue.body).not.toContain("Original strict AC"); + 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 () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h); + + const original = await composeSnapshot(built, makeIssue("Strict: reject empty input.")); + insertTask(h.db, { taskId: "task-1", state: "pr-review" }); + seedSnapshot(built, "task-1", original); + built.linear.setIssue(makeIssue("Relaxed: empty input is allowed.")); + + const io = bufferIO(); + const result = await dispatch( + ["task", "resnapshot", "task-1", "--reason", "widen accepted inputs"], + built.deps, + io, + ); + expect(result.exitCode).toBe(0); + + const event = latestEvent(h, "task-1"); + expect(event.event_type).toBe("ticket_resnapshotted"); + // No state transition — from/to mirror the task's current state. + expect(event.from_state).toBe("pr-review"); + expect(event.to_state).toBe("pr-review"); + + const data = JSON.parse(event.event_data!); + expect(data.reason).toBe("widen accepted inputs"); + expect(data.external_ref).toBe(EXTERNAL_REF); + expect(data.changed).toBe(true); + expect(data.diff.linear_issue.body.before).toContain("Strict: reject empty input."); + expect(data.diff.linear_issue.body.after).toContain("Relaxed: empty input is allowed."); + 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 () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h); + + const original = await composeSnapshot(built, makeIssue("Original AC.")); + insertTask(h.db, { taskId: "task-1", state: "waiting_external_changes" }); + seedSnapshot(built, "task-1", original); + seedReviewOnlyAttempt(h, "task-1", "changes_requested"); + built.linear.setIssue(makeIssue("Relaxed AC.")); + + const io = bufferIO(); + const result = await dispatch( + ["task", "resnapshot", "task-1", "--reason", "unblock BRIX-1907 loop"], + built.deps, + io, + ); + expect(result.exitCode).toBe(0); + const payload = JSON.parse(io.out()); + expect(payload.changed).toBe(true); + expect(payload.review_invalidated).toBe(1); + + // The terminal verdict is superseded, so enterReview's terminal_verdict_exists + // gate no longer blocks a fresh review of the same head SHA. + expect(reviewVerdict(h, "task-1")).toBe("superseded"); +}); + +test("resnapshot preserves creation-time snapshot augmentations it does not recompute", async () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h); + + const core = await composeSnapshot(built, makeIssue("Original AC.")); + // Simulate an enqueue-linear snapshot that carries dependency/hierarchy + // augmentation keys resnapshot must not recompute or drop. + const augmented = JSON.parse(core) as Record; + augmented.linear_blocked_by_relations = [{ identifier: "BRIX-1900" }]; + augmented.linear_hierarchy = { parent: null, children: [] }; + insertTask(h.db, { taskId: "task-1" }); + seedSnapshot(built, "task-1", JSON.stringify(augmented, null, 2)); + + built.linear.setIssue(makeIssue("Relaxed AC.")); + + const io = bufferIO(); + const result = await dispatch( + ["task", "resnapshot", "task-1", "--reason", "relax while keeping deps"], + built.deps, + io, + ); + expect(result.exitCode).toBe(0); + + const parsed = JSON.parse(latestSnapshot(h, "task-1")); + 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 () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h); + + const original = await composeSnapshot(built, makeIssue("Unchanged AC.")); + insertTask(h.db, { taskId: "task-1", state: "pr-review" }); + seedSnapshot(built, "task-1", original); + seedReviewOnlyAttempt(h, "task-1", "changes_requested"); + // Ticket is NOT edited between creation and resnapshot. + + const io = bufferIO(); + const result = await dispatch( + ["task", "resnapshot", "task-1", "--reason", "double-check no drift"], + built.deps, + io, + ); + expect(result.exitCode).toBe(0); + const payload = JSON.parse(io.out()); + expect(payload.changed).toBe(false); + expect(payload.review_invalidated).toBe(0); + expect(payload.snapshot_artifact_id).toBeNull(); + + // No new artifact, and the review verdict is untouched. + expect(snapshotCount(h, "task-1")).toBe(1); + expect(reviewVerdict(h, "task-1")).toBe("changes_requested"); + + // Still audited: the event is recorded with changed=false and an empty diff. + const event = latestEvent(h, "task-1"); + expect(event.event_type).toBe("ticket_resnapshotted"); + const data = JSON.parse(event.event_data!); + expect(data.changed).toBe(false); + expect(data.reason).toBe("double-check no drift"); + expect(data.diff).toEqual({}); +}); + +test("resnapshot no-op ignores creation-time augmentations when comparing", async () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h); + + const core = await composeSnapshot(built, makeIssue("Stable AC.")); + const augmented = JSON.parse(core) as Record; + augmented.linear_hierarchy = { parent: null, children: [] }; + insertTask(h.db, { taskId: "task-1" }); + seedSnapshot(built, "task-1", JSON.stringify(augmented, null, 2)); + // Ticket unchanged; only difference vs a fresh compose is the augmentation. + + const io = bufferIO(); + const result = await dispatch( + ["task", "resnapshot", "task-1", "--reason", "verify stability"], + built.deps, + io, + ); + 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 () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h); + + const io = bufferIO(); + const result = await dispatch( + ["task", "resnapshot", "nope", "--reason", "x"], + built.deps, + io, + ); + expect(result.exitCode).toBe(1); + expect(JSON.parse(io.err())).toMatchObject({ error: "unknown_task" }); +}); + +test("resnapshot rejects a task without an external_ref", async () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h); + insertTask(h.db, { taskId: "task-1", externalRef: null }); + + const io = bufferIO(); + const result = await dispatch( + ["task", "resnapshot", "task-1", "--reason", "x"], + built.deps, + io, + ); + expect(result.exitCode).toBe(1); + expect(JSON.parse(io.err())).toMatchObject({ error: "missing_external_ref" }); +}); + +test("resnapshot requires --reason", async () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h); + insertTask(h.db, { taskId: "task-1" }); + + const io = bufferIO(); + const result = await dispatch(["task", "resnapshot", "task-1"], built.deps, io); + expect(result.exitCode).toBe(1); + expect(JSON.parse(io.err())).toMatchObject({ error: "usage_error" }); +}); + +test("resnapshot fails closed when the Linear adapter is disabled", async () => { + h = createHarness(); + addRepo(h); + const built = buildCliDeps(h, { linearEnabled: false }); + await composeSnapshot(built, makeIssue("Any AC.")); + insertTask(h.db, { taskId: "task-1" }); + + const io = bufferIO(); + const result = await dispatch( + ["task", "resnapshot", "task-1", "--reason", "x"], + built.deps, + io, + ); + expect(result.exitCode).toBe(1); + expect(JSON.parse(io.err())).toMatchObject({ error: "adapter_not_enabled" }); +}); diff --git a/packages/cli/tests/review/pr_review_entry.test.ts b/packages/cli/tests/review/pr_review_entry.test.ts index 80be2f32..d56b70d4 100644 --- a/packages/cli/tests/review/pr_review_entry.test.ts +++ b/packages/cli/tests/review/pr_review_entry.test.ts @@ -1771,7 +1771,10 @@ test("review-pr gives Quay-owned tasks a request-changes verdict policy for any expect(brief).toContain("Choose the verdict according to the Verdict policy below."); }); -test("review-pr gives adopted external PRs the non-Quay-owned verdict policy", async () => { +test("review-pr gives adopted external PRs the Quay-owned verdict policy", async () => { + // Quay owns the feedback loop for adopted PRs (the worker respawns on + // changes_requested), so non-blocking-only findings must request changes and + // be fixed in-loop rather than approved-with-notes and filed to Linear. h = createHarness(); const built = buildCliDeps(h); built.deps.tickOptions = { reviewerEnabled: true, gateQuayOwnedDone: true }; @@ -1822,12 +1825,12 @@ test("review-pr gives adopted external PRs the non-Quay-owned verdict policy", a expect(briefRow).toBeDefined(); const brief = readFileSync(briefRow!.file_path, "utf8"); expect(brief).toContain("## Verdict policy"); - expect(brief).toContain("This is not a Quay-owned task."); + expect(brief).toContain("This is a Quay-owned task."); expect(brief).toContain( - "Non-blocking-only findings -> `approved` with the findings listed under `### Non-blocking`.", + "Non-blocking-only findings -> `changes_requested` with the findings listed under `### Non-blocking`.", ); expect(brief).not.toContain( - "Non-blocking-only findings -> `changes_requested`", + "Non-blocking-only findings -> `approved`", ); expect(brief).toContain("Choose the verdict according to the Verdict policy below."); }); diff --git a/packages/cli/tests/review/review_finding_linear_outbox.test.ts b/packages/cli/tests/review/review_finding_linear_outbox.test.ts index 9d5cc59d..a433043f 100644 --- a/packages/cli/tests/review/review_finding_linear_outbox.test.ts +++ b/packages/cli/tests/review/review_finding_linear_outbox.test.ts @@ -1,17 +1,11 @@ import { afterEach, expect, test } from "bun:test"; -import { dispatch } from "../../src/cli/dispatch.ts"; -import { bufferIO } from "../../src/cli/io.ts"; import { enqueueReviewFindingLinearIssuesInOpenTxn, - processReviewFindingLinearIssueOutboxItem, REVIEW_FINDING_LINEAR_ISSUE_OUTBOX_KIND, } from "../../src/core/review_finding_linear_outbox.ts"; import { persistStructuredReviewFindingsInOpenTxn } from "../../src/core/tick.ts"; import { createHarness, type Harness } from "../support/harness.ts"; import { insertAttempt, insertRepo, insertTask } from "../support/fixtures.ts"; -import { buildCliDeps } from "../support/cli_deps.ts"; -import { FakeLinearAdapter } from "../support/fakes/linear.ts"; -import { QuayError } from "../../src/core/errors.ts"; let h: Harness | null = null; afterEach(() => { @@ -19,7 +13,7 @@ afterEach(() => { h = null; }); -test("synthetic non-blocking findings enqueue and deliver one Linear issue", async () => { +test("synthetic non-blocking findings enqueue one Linear issue outbox row", () => { h = createHarness(); const seeded = seedReviewFindingTask("synthetic_review"); persistFindings(seeded, [ @@ -33,119 +27,27 @@ test("synthetic non-blocking findings enqueue and deliver one Linear issue", asy expect(outbox).toHaveLength(1); const item = outbox[0]!; expect(item.kind).toBe(REVIEW_FINDING_LINEAR_ISSUE_OUTBOX_KIND); - - const linear = new FakeLinearAdapter(); - await processReviewFindingLinearIssueOutboxItem( - { db: h.db, clock: h.clock, linear }, - { outboxItemId: item.outbox_item_id }, - ); - - expect(linear.createIssueCalls).toHaveLength(1); - const call = linear.createIssueCalls[0]!; - expect(call.title).toBe("Persist the follow-up"); - expect(call.body).toContain("src/a.ts:7-9"); - expect(call.body).toContain("quay-principle"); - expect(call.idempotencyKey).toMatch( - /^[0-9a-f]{8}-[0-9a-f]{4}-5[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/, - ); - expect(linkCount()).toBe(1); -}); - -test("outbox deliver command executes the Linear issue delivery handler", async () => { - h = createHarness(); - const built = buildCliDeps(h); - const seeded = seedReviewFindingTask("synthetic_review"); - persistFindings(seeded, [ - finding("non_blocking", "CLI follow-up", "Body text"), - ]); - const outbox = listFindingOutbox()[0]!; - const io = bufferIO(); - - const result = await dispatch( - ["outbox", "deliver", String(outbox.outbox_item_id)], - built.deps, - io, - ); - - expect(result.exitCode).toBe(0); - expect(io.err()).toBe(""); - expect(JSON.parse(io.out())).toMatchObject({ - outbox_item_id: outbox.outbox_item_id, - status: "completed", - }); - expect(built.linear.createIssueCalls).toHaveLength(1); - expect(linkCount()).toBe(1); + const payload = JSON.parse(item.payload_json ?? "{}"); + expect(payload.title).toBe("Persist the follow-up"); + expect(payload.principle_text).toBe("Prefer durable side effects."); + expect(payload.locations).toHaveLength(1); + expect(payload.locations[0].path).toBe("src/a.ts"); + expect(payload.locations[0].start_line).toBe(7); + expect(payload.locations[0].end_line).toBe(9); }); -test("outbox deliver command rejects disabled Linear adapter before delivery", async () => { - h = createHarness(); - const built = buildCliDeps(h, { linearEnabled: false }); - const seeded = seedReviewFindingTask("synthetic_review"); - persistFindings(seeded, [ - finding("non_blocking", "CLI follow-up", "Body text"), - ]); - const outbox = listFindingOutbox()[0]!; - const io = bufferIO(); - - const result = await dispatch( - ["outbox", "deliver", String(outbox.outbox_item_id)], - built.deps, - io, - ); - - expect(result.exitCode).toBe(1); - expect(io.out()).toBe(""); - expect(JSON.parse(io.err())).toMatchObject({ - error: "adapter_not_enabled", - adapter: "linear", - }); - expect(built.linear.createIssueCalls).toHaveLength(0); - expect(linkCount()).toBe(0); -}); - -test("outbox deliver normalizes Linear delivery failures through CLI errors", async () => { - h = createHarness(); - const built = buildCliDeps(h); - const seeded = seedReviewFindingTask("synthetic_review"); - persistFindings(seeded, [ - finding("non_blocking", "CLI failure", "Body text"), - ]); - const outbox = listFindingOutbox()[0]!; - built.linear.createIssue = async () => { - throw new QuayError("adapter_error", "Linear failed", { - adapter: "linear", - retryable: true, - }); - }; - const io = bufferIO(); - - const result = await dispatch( - ["outbox", "deliver", String(outbox.outbox_item_id)], - built.deps, - io, - ); - - expect(result.exitCode).toBe(1); - expect(io.out()).toBe(""); - expect(JSON.parse(io.err())).toMatchObject({ - error: "adapter_error", - message: "Linear failed", - adapter: "linear", - }); -}); - -test("adopted external PR findings are human-owned and blocking findings are skipped", () => { +test("adopted external PR findings do not create Linear follow-up outbox rows", () => { h = createHarness(); + // Quay owns the feedback loop for adopted PRs: the worker respawns on + // changes_requested and fixes non-blocking findings in-loop, so filing a + // Linear issue for them would be redundant. const seeded = seedReviewFindingTask("adopted_external_pr"); persistFindings(seeded, [ finding("non_blocking", "Adopted follow-up", "Body text"), finding("blocking", "Blocking review", "Do not ticket this path"), ]); - const outbox = listFindingOutbox(); - expect(outbox).toHaveLength(1); - const payload = JSON.parse(outbox[0]!.payload_json ?? "{}"); - expect(payload.title).toBe("Adopted follow-up"); + expect(listFindingOutbox()).toHaveLength(0); }); test("quay-owned review findings do not create Linear follow-up outbox rows", () => { @@ -158,119 +60,17 @@ test("quay-owned review findings do not create Linear follow-up outbox rows", () expect(listFindingOutbox()).toHaveLength(0); }); -test("retrying a Linear issue outbox row does not create duplicate issues", async () => { - h = createHarness(); - const seeded = seedReviewFindingTask("synthetic_review"); - persistFindings(seeded, [ - finding("non_blocking", "Retry follow-up", "Body text"), - ]); - const outbox = listFindingOutbox()[0]!; - const linear = new FakeLinearAdapter(); - - await processReviewFindingLinearIssueOutboxItem( - { db: h.db, clock: h.clock, linear }, - { outboxItemId: outbox.outbox_item_id }, - ); - - h.db - .query( - `UPDATE outbox_items - SET status = 'pending', - claim_id = NULL, - claimed_at = NULL - WHERE outbox_item_id = ?`, - ) - .run(outbox.outbox_item_id); - await processReviewFindingLinearIssueOutboxItem( - { db: h.db, clock: h.clock, linear }, - { outboxItemId: outbox.outbox_item_id }, - ); - - expect(linear.createIssueCalls).toHaveLength(1); - expect(linkCount()).toBe(1); -}); - -test("Linear rate limits preserve retry-after as outbox cooldown", async () => { - h = createHarness(); - h.clock.set("2026-06-11T16:00:00.000Z"); - const seeded = seedReviewFindingTask("synthetic_review"); - persistFindings(seeded, [ - finding("non_blocking", "Rate limited follow-up", "Body text"), - ]); - const outbox = listFindingOutbox()[0]!; - const linear = new FakeLinearAdapter(); - linear.createIssue = async () => { - throw new QuayError("adapter_error", "Linear rate-limited", { - adapter: "linear", - retryable: true, - retry_after: 90, - }); - }; - - await expect( - processReviewFindingLinearIssueOutboxItem( - { db: h.db, clock: h.clock, linear }, - { outboxItemId: outbox.outbox_item_id }, - ), - ).rejects.toThrow("Linear rate-limited"); - - const row = h.db - .query<{ next_eligible_at: string | null }, [number]>( - `SELECT next_eligible_at FROM outbox_items WHERE outbox_item_id = ?`, - ) - .get(outbox.outbox_item_id); - expect(row?.next_eligible_at).toBe("2026-06-11T16:01:30.000Z"); -}); - -test("provider idempotency converges when link persistence fails after Linear create", async () => { - h = createHarness(); - const seeded = seedReviewFindingTask("synthetic_review"); - persistFindings(seeded, [ - finding("non_blocking", "Crash-window follow-up", "Body text"), - ]); - const outbox = listFindingOutbox()[0]!; - const linear = new FakeLinearAdapter(); - - h.db.exec( - `CREATE TRIGGER review_finding_external_links_crash - BEFORE INSERT ON review_finding_external_links - BEGIN - SELECT RAISE(FAIL, 'simulated link write crash'); - END`, - ); - await expect( - processReviewFindingLinearIssueOutboxItem( - { db: h.db, clock: h.clock, linear }, - { outboxItemId: outbox.outbox_item_id }, - ), - ).rejects.toThrow("simulated link write crash"); - h.db.exec("DROP TRIGGER review_finding_external_links_crash"); - - await processReviewFindingLinearIssueOutboxItem( - { db: h.db, clock: h.clock, linear }, - { outboxItemId: outbox.outbox_item_id }, - ); - - expect(linear.createIssueCalls).toHaveLength(2); - expect(linear.createIssueCalls[0]!.idempotencyKey).toBe( - linear.createIssueCalls[1]!.idempotencyKey, - ); - expect(linkProviderExternalId()).toBe("linear-created-1"); - expect(linkCount()).toBe(1); -}); - -test("re-persisting the same review reuses outbox idempotency and reattaches the provider link", async () => { +test("re-persisting the same review reattaches an existing provider link to the new finding", () => { h = createHarness(); const seeded = seedReviewFindingTask("synthetic_review"); const rows = [finding("non_blocking", "Stable follow-up", "Body text")]; persistFindings(seeded, rows); const outbox = listFindingOutbox()[0]!; - const linear = new FakeLinearAdapter(); - await processReviewFindingLinearIssueOutboxItem( - { db: h.db, clock: h.clock, linear }, - { outboxItemId: outbox.outbox_item_id }, - ); const firstFindingId = currentFindingId(); + // Simulate the orchestrator recording the delivered Linear issue back into + // Quay's dedup ledger. Re-enqueue must then reattach the link to the newly + // persisted finding rather than minting a second outbox row. + seedExternalLink(firstFindingId, outbox.outbox_item_id); persistFindings(seeded, rows); @@ -371,6 +171,38 @@ function listFindingOutbox(): Array<{ .all(REVIEW_FINDING_LINEAR_ISSUE_OUTBOX_KIND); } +// Seed the Quay-owned dedup ledger row the orchestrator would write back after +// creating the Linear issue, so enqueue's reattachment path can be exercised +// without the (removed) in-process delivery handler. +function seedExternalLink(findingId: number, outboxItemId: number): void { + if (!h) throw new Error("missing harness"); + const finding = h.db + .query<{ task_id: string; review_id: string; fingerprint: string }, [number]>( + `SELECT task_id, review_id, fingerprint + FROM review_findings + WHERE finding_id = ?`, + ) + .get(findingId); + if (finding === null) throw new Error(`finding ${findingId} not found`); + const now = h.clock.nowISO(); + h.db + .query( + `INSERT INTO review_finding_external_links ( + finding_id, task_id, review_id, fingerprint, provider, + provider_external_id, provider_url, outbox_item_id, created_at, updated_at + ) VALUES (?, ?, ?, ?, 'linear', 'linear-created-1', 'https://linear.test/1', ?, ?, ?)`, + ) + .run( + findingId, + finding.task_id, + finding.review_id, + finding.fingerprint, + outboxItemId, + now, + now, + ); +} + function linkCount(): number { if (!h) throw new Error("missing harness"); return h.db @@ -397,12 +229,3 @@ function linkFindingId(): number | null { ) .get()!.finding_id; } - -function linkProviderExternalId(): string | null { - if (!h) throw new Error("missing harness"); - return h.db - .query<{ provider_external_id: string | null }, []>( - `SELECT provider_external_id FROM review_finding_external_links LIMIT 1`, - ) - .get()!.provider_external_id; -} diff --git a/packages/cli/tests/review/review_finding_linear_toggle.test.ts b/packages/cli/tests/review/review_finding_linear_toggle.test.ts new file mode 100644 index 00000000..685f8ac7 --- /dev/null +++ b/packages/cli/tests/review/review_finding_linear_toggle.test.ts @@ -0,0 +1,215 @@ +import { afterEach, expect, test } from "bun:test"; +import { + enqueueReviewFindingLinearIssuesIfEnabledInOpenTxn, + resolveReviewFindingLinearEnabled, +} from "../../src/core/review_finding_linear_policy.ts"; +import { REVIEW_FINDING_LINEAR_ISSUE_OUTBOX_KIND } from "../../src/core/review_finding_linear_outbox.ts"; +import { persistStructuredReviewFindingsInOpenTxn } from "../../src/core/tick.ts"; +import { createDeploymentSettingsService } from "../../src/core/deployment_settings.ts"; +import { createRepoService } from "../../src/core/repos/service.ts"; +import { createHarness, type Harness } from "../support/harness.ts"; +import { insertAttempt, insertRepo, insertTask } from "../support/fixtures.ts"; + +let h: Harness | null = null; +afterEach(() => { + h?.cleanup(); + h = null; +}); + +function setGlobal(enabled: boolean | null): void { + if (!h) throw new Error("missing harness"); + createDeploymentSettingsService({ db: h.db, clock: h.clock }).update({ + review_finding_linear_enabled: enabled, + }); +} + +function setRepoOverride(repoId: string, enabled: boolean | null): void { + if (!h) throw new Error("missing harness"); + createRepoService({ db: h.db, clock: h.clock }).update(repoId, { + review_finding_linear_enabled: enabled, + }); +} + +// --- resolution logic ----------------------------------------------------- + +test("resolution defaults to ON when unset at both scopes", () => { + h = createHarness(); + const repoId = insertRepo(h.db, "repo-default"); + expect(resolveReviewFindingLinearEnabled(h.db, repoId)).toBe(true); +}); + +test("resolution follows the global default when the repo inherits", () => { + h = createHarness(); + const repoId = insertRepo(h.db, "repo-inherits"); + + setGlobal(false); + expect(resolveReviewFindingLinearEnabled(h.db, repoId)).toBe(false); + + setGlobal(true); + expect(resolveReviewFindingLinearEnabled(h.db, repoId)).toBe(true); +}); + +test("resolution: repo override wins over the global default", () => { + h = createHarness(); + const repoId = insertRepo(h.db, "repo-override"); + + setGlobal(true); + setRepoOverride(repoId, false); + expect(resolveReviewFindingLinearEnabled(h.db, repoId)).toBe(false); + + setGlobal(false); + setRepoOverride(repoId, true); + expect(resolveReviewFindingLinearEnabled(h.db, repoId)).toBe(true); +}); + +test("resolution: clearing the repo override falls back to the global default", () => { + h = createHarness(); + const repoId = insertRepo(h.db, "repo-cleared"); + + setGlobal(false); + setRepoOverride(repoId, true); + expect(resolveReviewFindingLinearEnabled(h.db, repoId)).toBe(true); + + setRepoOverride(repoId, null); + expect(resolveReviewFindingLinearEnabled(h.db, repoId)).toBe(false); +}); + +// --- enqueue gate --------------------------------------------------------- + +test("enqueue gate off (repo) persists findings but skips the outbox row", () => { + h = createHarness(); + const seeded = seedSyntheticReviewTask("repo-gate-off"); + setRepoOverride(seeded.repoId, false); + + persistAndGatedEnqueue(seeded); + + expect(findingCount()).toBe(1); + expect(listFindingOutbox()).toHaveLength(0); +}); + +test("enqueue gate off (global) skips the outbox row", () => { + h = createHarness(); + const seeded = seedSyntheticReviewTask("repo-global-off"); + setGlobal(false); + + persistAndGatedEnqueue(seeded); + + expect(findingCount()).toBe(1); + expect(listFindingOutbox()).toHaveLength(0); +}); + +test("enqueue gate default ON enqueues the outbox row", () => { + h = createHarness(); + const seeded = seedSyntheticReviewTask("repo-gate-on"); + + persistAndGatedEnqueue(seeded); + + expect(findingCount()).toBe(1); + const outbox = listFindingOutbox(); + expect(outbox).toHaveLength(1); + expect(outbox[0]!.kind).toBe(REVIEW_FINDING_LINEAR_ISSUE_OUTBOX_KIND); +}); + +test("enqueue gate: repo ON overrides a global OFF default", () => { + h = createHarness(); + const seeded = seedSyntheticReviewTask("repo-gate-repo-on"); + setGlobal(false); + setRepoOverride(seeded.repoId, true); + + persistAndGatedEnqueue(seeded); + + expect(listFindingOutbox()).toHaveLength(1); +}); + +// --- helpers -------------------------------------------------------------- + +function seedSyntheticReviewTask(repoId: string): { + repoId: string; + taskId: string; + attemptId: number; +} { + if (!h) throw new Error("missing harness"); + insertRepo(h.db, repoId); + const taskId = insertTask(h.db, { + repoId, + taskId: `task-${repoId}`, + state: "pr-review", + }); + h.db + .query( + `UPDATE tasks + SET authoring_mode = 'synthetic_review', + pr_number = 21, + pr_url = 'https://github.test/acme/repo/pull/21', + head_sha = 'head-review' + WHERE task_id = ?`, + ) + .run(taskId); + const attemptId = insertAttempt(h.db, { + taskId, + reason: "review_only", + consumedBudget: 0, + spawnedAt: h.clock.nowISO(), + }); + return { repoId, taskId, attemptId }; +} + +function persistAndGatedEnqueue(seeded: { + repoId: string; + taskId: string; + attemptId: number; +}): void { + if (!h) throw new Error("missing harness"); + h.db.exec("BEGIN IMMEDIATE"); + try { + persistStructuredReviewFindingsInOpenTxn( + { db: h.db }, + { + taskId: seeded.taskId, + attemptId: seeded.attemptId, + reviewId: "review-1", + headSha: "head-review", + now: h.clock.nowISO(), + rawReviewResult: JSON.stringify({ + verdict: "changes_requested", + body: "review body", + findings: [ + { severity: "non_blocking", title: "Follow-up", body: "Body text" }, + ], + }), + }, + ); + enqueueReviewFindingLinearIssuesIfEnabledInOpenTxn( + { db: h.db, clock: h.clock }, + { + taskId: seeded.taskId, + attemptId: seeded.attemptId, + reviewId: "review-1", + repoId: seeded.repoId, + }, + ); + h.db.exec("COMMIT"); + } catch (err) { + h.db.exec("ROLLBACK"); + throw err; + } +} + +function findingCount(): number { + if (!h) throw new Error("missing harness"); + return h.db + .query<{ n: number }, []>(`SELECT COUNT(*) AS n FROM review_findings`) + .get()!.n; +} + +function listFindingOutbox(): Array<{ outbox_item_id: number; kind: string }> { + if (!h) throw new Error("missing harness"); + return h.db + .query<{ outbox_item_id: number; kind: string }, [string]>( + `SELECT outbox_item_id, kind + FROM outbox_items + WHERE kind = ? + ORDER BY outbox_item_id`, + ) + .all(REVIEW_FINDING_LINEAR_ISSUE_OUTBOX_KIND); +} diff --git a/packages/cli/tests/review/reviewer_gh_token_file.test.ts b/packages/cli/tests/review/reviewer_gh_token_file.test.ts index 987d103f..4c5505a2 100644 --- a/packages/cli/tests/review/reviewer_gh_token_file.test.ts +++ b/packages/cli/tests/review/reviewer_gh_token_file.test.ts @@ -205,6 +205,7 @@ test("reviewer env token wins over gh_token_file when both are present", async ( test("reviewer spawn fails before promotion when no reviewer token source exists", async () => { h = createHarness(); + h.clock.set("2026-05-12T10:00:00.000Z"); const built = buildTickDeps(h); const { taskId, attemptId } = seedPendingReview(h, "token-none"); @@ -220,6 +221,68 @@ test("reviewer spawn fails before promotion when no reviewer token source exists expect(built.tmux.spawnCalls).toHaveLength(0); expect(built.github.tokenAccessCalls).toHaveLength(0); expect(reviewAttemptSpawnedAt(h, attemptId)).toBeNull(); + expect(reviewSpawnBackoff(h, taskId)).toEqual({ + state: "pr-review", + review_infra_failures_consecutive: 1, + spawn_retry_next_eligible_at: "2026-05-12T10:05:00.000Z", + }); +}); + +test("reviewer spawn auth failure backs off and parks after repeated failures", async () => { + h = createHarness(); + h.clock.set("2026-05-12T10:00:00.000Z"); + const built = buildTickDeps(h); + const { repoId, taskId, attemptId } = seedPendingReview(h, "token-auth-backoff"); + const reviewerToken = "ghs_reviewer_without_repo_access"; + built.github.setTokenAccessHandler(() => { + throw new Error("HTTP 404: Not Found"); + }); + + const first = await tick_once(built.deps, { + reviewerEnabled: true, + env: { [REVIEWER_GH_TOKEN_ENV]: reviewerToken }, + }); + expect(first.find((r) => r.task_id === taskId)?.action).toBe( + "spawn_substrate_failed", + ); + expect(built.github.tokenAccessCalls).toEqual([ + { repoId, token: reviewerToken, actor: "reviewer" }, + ]); + expect(reviewAttemptSpawnedAt(h, attemptId)).toBeNull(); + expect(reviewSpawnBackoff(h, taskId)).toEqual({ + state: "pr-review", + review_infra_failures_consecutive: 1, + spawn_retry_next_eligible_at: "2026-05-12T10:05:00.000Z", + }); + + expect( + await tick_once(built.deps, { + reviewerEnabled: true, + env: { [REVIEWER_GH_TOKEN_ENV]: reviewerToken }, + }), + ).toEqual([]); + expect(built.github.tokenAccessCalls).toHaveLength(1); + + h.clock.set("2026-05-12T10:05:00.000Z"); + await tick_once(built.deps, { + reviewerEnabled: true, + env: { [REVIEWER_GH_TOKEN_ENV]: reviewerToken }, + }); + h.clock.set("2026-05-12T10:15:00.000Z"); + const third = await tick_once(built.deps, { + reviewerEnabled: true, + env: { [REVIEWER_GH_TOKEN_ENV]: reviewerToken }, + }); + + expect(third.find((r) => r.task_id === taskId)?.action).toBe( + "non_budget_loop_parked", + ); + expect(built.github.tokenAccessCalls).toHaveLength(3); + expect(reviewSpawnBackoff(h, taskId)).toEqual({ + state: "non_budget_loop", + review_infra_failures_consecutive: 3, + spawn_retry_next_eligible_at: null, + }); }); test("reviewer spawn fails when gh_token_file is missing", async () => { @@ -292,9 +355,35 @@ test("reviewer spawn validates non-empty gh_token_file credentials before promot ]); expect(built.tmux.spawnCalls).toHaveLength(0); expect(reviewAttemptSpawnedAt(h, attemptId)).toBeNull(); - expect(reviewInfraFailureEventCount(h, taskId)).toBe(0); + expect(reviewInfraFailureEventCount(h, taskId)).toBe(1); }); +function reviewSpawnBackoff( + harness: Harness, + taskId: string, +): { + state: string; + review_infra_failures_consecutive: number; + spawn_retry_next_eligible_at: string | null; +} { + const row = harness.db + .query< + { + state: string; + review_infra_failures_consecutive: number; + spawn_retry_next_eligible_at: string | null; + }, + [string] + >( + `SELECT state, review_infra_failures_consecutive, + spawn_retry_next_eligible_at + FROM tasks WHERE task_id = ?`, + ) + .get(taskId); + if (!row) throw new Error(`missing task ${taskId}`); + return row; +} + function reviewAttemptSpawnedAt(harness: Harness, attemptId: number): string | null { const row = harness.db .query<{ spawned_at: string | null }, [number]>( diff --git a/packages/cli/tests/schema/migrations.test.ts b/packages/cli/tests/schema/migrations.test.ts index 163181fe..2af76d7c 100644 --- a/packages/cli/tests/schema/migrations.test.ts +++ b/packages/cli/tests/schema/migrations.test.ts @@ -624,9 +624,43 @@ test("deployment_settings table stores mutable agent defaults", () => { "reviewer_model", "created_at", "updated_at", + "review_finding_linear_enabled", ]); }); +test("review-finding Linear toggle columns are tri-state (NULL/0/1)", () => { + h = createHarness(); + const db = h.db; + + const deploymentCols = db + .query<{ name: string }, []>(`PRAGMA table_info(deployment_settings)`) + .all() + .map((r) => r.name); + expect(deploymentCols).toContain("review_finding_linear_enabled"); + + const repoCols = db + .query<{ name: string }, []>(`PRAGMA table_info(repos)`) + .all() + .map((r) => r.name); + expect(repoCols).toContain("review_finding_linear_enabled"); + + // New columns default to NULL (inherit / unset) on existing rows. + insertRepo(db, "repo-toggle-default"); + const value = db + .query<{ review_finding_linear_enabled: number | null }, [string]>( + `SELECT review_finding_linear_enabled FROM repos WHERE repo_id = ?`, + ) + .get("repo-toggle-default"); + expect(value?.review_finding_linear_enabled).toBeNull(); + + // The CHECK constraint rejects out-of-range integers. + expect(() => + db + .query(`UPDATE repos SET review_finding_linear_enabled = 2 WHERE repo_id = ?`) + .run("repo-toggle-default"), + ).toThrow(); +}); + test("identity_mappings table stores Slack to GitHub assignee mappings", () => { h = createHarness(); const cols = h.db @@ -865,6 +899,28 @@ test("orchestrator handoffs carry next eligibility timestamp", () => { ).toContain("WHERE status = 'pending'"); }); +test("tasks table tracks spawn retry backoff and reason", () => { + h = createHarness(); + const cols = h.db + .query<{ name: string }, []>(`PRAGMA table_info(tasks)`) + .all() + .map((r) => r.name); + expect(cols).toContain("spawn_retry_next_eligible_at"); + expect(cols).toContain("spawn_failure_reason"); + + const indexes = h.db + .query<{ name: string; sql: string }, []>( + `SELECT name, sql + FROM sqlite_master + WHERE type = 'index' + AND tbl_name = 'tasks' + AND name = 'tasks_spawn_retry_eligible_idx'`, + ) + .all(); + expect(indexes).toHaveLength(1); + expect(indexes[0]!.sql).toContain("spawn_retry_next_eligible_at"); +}); + test("outbox items support delivery and workflow metadata", () => { h = createHarness(); const cols = h.db diff --git a/packages/cli/tests/spawn_failure/spawn_failure_slice5.test.ts b/packages/cli/tests/spawn_failure/spawn_failure_slice5.test.ts index db1bb02d..47e3367d 100644 --- a/packages/cli/tests/spawn_failure/spawn_failure_slice5.test.ts +++ b/packages/cli/tests/spawn_failure/spawn_failure_slice5.test.ts @@ -39,21 +39,61 @@ test("test_045_spawn_failure_no_evidence_rolls_back_budget_and_requeues", async ]); const task = h.db .query< - { state: string; attempts_consumed: number; spawn_failures_consecutive: number }, + { + state: string; + attempts_consumed: number; + spawn_failures_consecutive: number; + spawn_retry_next_eligible_at: string | null; + spawn_failure_reason: string | null; + }, [string] >( - `SELECT state, attempts_consumed, spawn_failures_consecutive + `SELECT state, attempts_consumed, spawn_failures_consecutive, + spawn_retry_next_eligible_at, spawn_failure_reason FROM tasks WHERE task_id = ?`, ) .get(t.taskId); - expect(task).toEqual({ - state: "queued", - attempts_consumed: 0, - spawn_failures_consecutive: 1, - }); + expect(task?.state).toBe("queued"); + expect(task?.attempts_consumed).toBe(0); + expect(task?.spawn_failures_consecutive).toBe(1); + expect(task?.spawn_retry_next_eligible_at).toBe("2026-04-28T13:05:00.000Z"); + expect(task?.spawn_failure_reason).toContain("repo-spawn-fail"); expect(pendingReason(t.taskId)).toBe("initial"); }); +test("spawn failure backoff skips queued retry before token preflight", async () => { + h = createHarness(); + h.clock.set("2026-04-28T13:00:00.000Z"); + const repoId = insertRepo(h.db, "repo-spawn-backoff"); + const t = insertRunningTask(h.db, { + taskId: "task-spawn-backoff", + repoId, + worktreesRoot: join(h.dataDir, "worktrees"), + tmuxSession: null, + attemptsConsumed: 1, + remoteShaAtSpawn: null, + }); + + const built = buildTickDeps(h); + built.git.setRemoteHeadSha(repoId, t.branchName, null); + built.github.setPrExists(repoId, t.branchName, false); + + expect(await tick_once(built.deps)).toEqual([ + { task_id: t.taskId, action: "spawn_failed" }, + ]); + expect(await tick_once(built.deps)).toEqual([]); + expect(built.github.tokenAccessCalls).toHaveLength(0); + expect(built.github.prExistsWithTokenCalls).toHaveLength(0); + expect(built.tmux.spawnAttempts).toHaveLength(0); + + h.clock.set("2026-04-28T13:05:00.000Z"); + expect(await tick_once(built.deps)).toEqual([ + { task_id: t.taskId, action: "spawned" }, + ]); + expect(built.github.tokenAccessCalls).toHaveLength(1); + expect(built.tmux.spawnAttempts).toHaveLength(1); +}); + test("test_046b_spawn_window_push_without_pr_takes_spawn_failed_default", async () => { h = createHarness(); const repoId = insertRepo(h.db, "repo-pushed-no-pr"); @@ -129,6 +169,7 @@ test("test_062_consecutive_substrate_failures_accumulate_across_ticks", async () for (let i = 0; i < 6; i++) { built.tmux.failSpawnNext(); await tick_once(built.deps); + h.clock.set(new Date(Date.parse(h.clock.nowISO()) + 60 * 60 * 1000).toISOString()); } const task = h.db .query<{ state: string; spawn_failures_consecutive: number }, [string]>( diff --git a/packages/cli/tests/support/fakes/git.ts b/packages/cli/tests/support/fakes/git.ts index b77b908f..5ec03c00 100644 --- a/packages/cli/tests/support/fakes/git.ts +++ b/packages/cli/tests/support/fakes/git.ts @@ -137,6 +137,11 @@ export class FakeGit implements GitPort { if (this.fail.worktreeAdd?.(worktreePath)) { throw new Error(`fake: worktreeAddExistingBranch failed for ${worktreePath}`); } + for (const checkout of this.worktreeBranches.values()) { + if (checkout.repoId === repoId && checkout.branch === branch) { + throw new Error(`fake: branch ${branch} is checked out in a worktree`); + } + } mkdirSync(worktreePath, { recursive: true }); this.worktrees.add(worktreePath); let set = this.localBranches.get(repoId); @@ -203,6 +208,17 @@ export class FakeGit implements GitPort { this.worktreeHeads.delete(worktreePath); } + worktreePrune(repoId: string): void { + this.record("worktreePrune", { repoId }); + for (const [path, checkout] of [...this.worktreeBranches.entries()]) { + if (checkout.repoId === repoId && !existsSync(path)) { + this.worktrees.delete(path); + this.worktreeBranches.delete(path); + this.worktreeHeads.delete(path); + } + } + } + branchDelete(repoId: string, branch: string): void { this.record("branchDelete", { repoId, branch }); if (this.fail.branchDelete?.(branch)) { diff --git a/packages/cli/tests/support/fakes/linear.ts b/packages/cli/tests/support/fakes/linear.ts index eaf30db3..6eaac9f1 100644 --- a/packages/cli/tests/support/fakes/linear.ts +++ b/packages/cli/tests/support/fakes/linear.ts @@ -1,8 +1,6 @@ import { QuayError } from "../../../src/core/errors.ts"; import type { LinearBlockedByRelation, - LinearCreatedIssue, - LinearCreateIssueInput, LinearIssueHierarchy, LinearIssue, LinearPort, @@ -24,8 +22,6 @@ export interface FakeLinearBodyUpdate { body: string; } -export interface FakeLinearCreatedIssueCall extends LinearCreateIssueInput {} - export class FakeLinearAdapter implements LinearPort { getIssueCalls: string[] = []; getBlockedByRelationsCalls: string[] = []; @@ -35,7 +31,6 @@ export class FakeLinearAdapter implements LinearPort { // duplicate writes when idempotency should have suppressed the call. setIssueStateCalls: FakeLinearStateChange[] = []; updateIssueBodyCalls: FakeLinearBodyUpdate[] = []; - createIssueCalls: FakeLinearCreatedIssueCall[] = []; private states = new Map(); private blockedByRelations = new Map(); @@ -45,8 +40,6 @@ export class FakeLinearAdapter implements LinearPort { // current value records nothing on `setIssueStateCalls` (skip), mirroring // the real adapter's read-before-write behaviour. private currentStates = new Map(); - private nextCreatedIssueNumber = 1; - private createdIssuesByIdempotencyKey = new Map(); // Errors queued via `failNextSetIssueState` are consumed in FIFO order; // calls past the queued count succeed normally. A queue (vs. a single // field) so two failures-in-a-row stay observable rather than silently @@ -181,23 +174,4 @@ export class FakeLinearAdapter implements LinearPort { }); } } - - async createIssue(input: LinearCreateIssueInput): Promise { - this.createIssueCalls.push({ ...input }); - const idempotencyKey = input.idempotencyKey?.trim(); - if (idempotencyKey !== undefined && idempotencyKey !== "") { - const existing = this.createdIssuesByIdempotencyKey.get(idempotencyKey); - if (existing !== undefined) return existing; - } - const n = this.nextCreatedIssueNumber++; - const created = { - id: `linear-created-${n}`, - identifier: `BRIX-${9000 + n}`, - url: `https://linear.app/inverter/issue/BRIX-${9000 + n}`, - }; - if (idempotencyKey !== undefined && idempotencyKey !== "") { - this.createdIssuesByIdempotencyKey.set(idempotencyKey, created); - } - return created; - } } diff --git a/packages/cli/tests/support/fixtures.ts b/packages/cli/tests/support/fixtures.ts index 926ec216..c2d6ab16 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, diff --git a/packages/cli/tests/task/recreate_worktree.test.ts b/packages/cli/tests/task/recreate_worktree.test.ts new file mode 100644 index 00000000..16615d64 --- /dev/null +++ b/packages/cli/tests/task/recreate_worktree.test.ts @@ -0,0 +1,271 @@ +import { afterEach, expect, test } from "bun:test"; +import { existsSync, rmSync } from "node:fs"; +import { dispatch } from "../../src/cli/dispatch.ts"; +import { bufferIO } from "../../src/cli/io.ts"; +import { enqueue } from "../../src/core/enqueue.ts"; +import { createRepoService } from "../../src/core/repos/service.ts"; +import { recreate_task_worktree } from "../../src/core/recreate_worktree.ts"; +import { createHarness, type Harness } from "../support/harness.ts"; +import { buildCliDeps, type BuiltCliDeps } from "../support/cli_deps.ts"; + +let h: Harness | null = null; +afterEach(() => { + h?.cleanup(); + h = null; +}); + +function setupTask(): { + h: Harness; + built: BuiltCliDeps; + task: { + task_id: string; + branch_name: string; + worktree_path: string; + }; +} { + const harness = createHarness(); + createRepoService({ db: harness.db, clock: harness.clock }).add({ + repo_id: "repo-a", + repo_url: "git@example.com:owner/repo-a.git", + base_branch: "main", + package_manager: "bun", + install_cmd: "bun install", + }); + const built = buildCliDeps(harness); + built.git.seedBareClone("repo-a"); + harness.ids.push("11111111aaaaaaaaaaaaaaaaaaaaaaaa"); + const task = enqueue( + { + db: harness.db, + clock: harness.clock, + ids: harness.ids, + git: built.git, + commandRunner: built.commandRunner, + artifactStore: built.deps.artifactStore, + paths: built.deps.paths, + agentResolver: built.deps.agentResolver, + }, + { + repo_id: "repo-a", + brief: "Recover this task worktree.", + external_ref: "BRIX-1923", + }, + ); + h = harness; + return { h: harness, built, task }; +} + +function removeRecordedWorktree(task: { worktree_path: string }): void { + rmSync(task.worktree_path, { recursive: true, force: true }); +} + +test("recreate_task_worktree restores from remote task branch and audits event", async () => { + const { h, built, task } = setupTask(); + removeRecordedWorktree(task); + built.git.setRemoteBranches("repo-a", [task.branch_name]); + + const result = await recreate_task_worktree( + { + db: h.db, + clock: h.clock, + git: built.git, + commandRunner: built.commandRunner, + supervisorLock: built.deps.supervisorLock, + }, + { taskId: task.task_id, yes: true }, + ); + + expect(result).toEqual({ + ok: true, + value: { + task_id: task.task_id, + repo_id: "repo-a", + branch_name: task.branch_name, + base_branch: "main", + worktree_path: task.worktree_path, + recovery_base: "remote_task_branch", + recovery_ref: `origin/${task.branch_name}`, + forced: false, + }, + }); + expect(existsSync(task.worktree_path)).toBe(true); + expect(built.commandRunner.calls.at(-1)).toEqual({ + command: "bun install", + cwd: task.worktree_path, + }); + expect(built.git.calls).toContainEqual({ + op: "worktreeAddExistingBranch", + args: { + repoId: "repo-a", + worktreePath: task.worktree_path, + branch: task.branch_name, + baseRef: `origin/${task.branch_name}`, + }, + }); + + const event = h.db + .query< + { + event_type: string; + from_state: string; + to_state: string; + event_data: string; + }, + [string] + >( + `SELECT event_type, from_state, to_state, event_data + FROM events + WHERE task_id = ? AND event_type = 'worktree_recreated'`, + ) + .get(task.task_id); + expect(event).toMatchObject({ + event_type: "worktree_recreated", + from_state: "queued", + to_state: "queued", + }); + expect(JSON.parse(event!.event_data)).toMatchObject({ + worktree_path: task.worktree_path, + branch_name: task.branch_name, + recovery_base: "remote_task_branch", + recovery_ref: `origin/${task.branch_name}`, + forced: false, + }); +}); + +test("recreate_task_worktree falls back to origin base branch when task branch is not remote", async () => { + const { built, task } = setupTask(); + removeRecordedWorktree(task); + built.git.setRemoteBranches("repo-a", []); + + const result = await recreate_task_worktree( + { + db: h!.db, + clock: h!.clock, + git: built.git, + commandRunner: built.commandRunner, + supervisorLock: built.deps.supervisorLock, + }, + { taskId: task.task_id, yes: true }, + ); + + expect(result.ok).toBe(true); + if (!result.ok) throw new Error("expected success"); + expect(result.value.recovery_base).toBe("remote_base_branch"); + expect(result.value.recovery_ref).toBe("origin/main"); + expect(built.git.calls).toContainEqual({ + op: "fetch", + args: { repoId: "repo-a", ref: "main" }, + }); + expect(built.git.worktreeBranches.get(task.worktree_path)).toEqual({ + repoId: "repo-a", + branch: task.branch_name, + }); +}); + +test("recreate_task_worktree refuses an existing path unless forced", async () => { + const { built, task } = setupTask(); + + const refused = await recreate_task_worktree( + { + db: h!.db, + clock: h!.clock, + git: built.git, + commandRunner: built.commandRunner, + supervisorLock: built.deps.supervisorLock, + }, + { taskId: task.task_id, yes: true }, + ); + expect(refused).toMatchObject({ + ok: false, + error: { code: "worktree_exists" }, + }); + + built.git.setRemoteBranches("repo-a", [task.branch_name]); + const forced = await recreate_task_worktree( + { + db: h!.db, + clock: h!.clock, + git: built.git, + commandRunner: built.commandRunner, + supervisorLock: built.deps.supervisorLock, + }, + { taskId: task.task_id, yes: true, force: true }, + ); + expect(forced.ok).toBe(true); + expect(built.git.countCalls("worktreeRemove")).toBe(1); +}); + +test("recreate_task_worktree refuses an active attempt unless forced", async () => { + const { built, task } = setupTask(); + removeRecordedWorktree(task); + h!.db + .query( + `UPDATE attempts + SET spawned_at = ?, tmux_session = ? + WHERE task_id = ? AND ended_at IS NULL`, + ) + .run(h!.clock.nowISO(), "quay-live", task.task_id); + + const refused = await recreate_task_worktree( + { + db: h!.db, + clock: h!.clock, + git: built.git, + commandRunner: built.commandRunner, + supervisorLock: built.deps.supervisorLock, + }, + { taskId: task.task_id, yes: true }, + ); + expect(refused).toMatchObject({ + ok: false, + error: { + code: "active_task", + details: { tmux_session: "quay-live" }, + }, + }); + + const forced = await recreate_task_worktree( + { + db: h!.db, + clock: h!.clock, + git: built.git, + commandRunner: built.commandRunner, + supervisorLock: built.deps.supervisorLock, + }, + { taskId: task.task_id, yes: true, force: true }, + ); + expect(forced.ok).toBe(true); +}); + +test("task recreate-worktree CLI validates confirmation and emits JSON result", async () => { + const { built, task } = setupTask(); + removeRecordedWorktree(task); + built.git.setRemoteBranches("repo-a", [task.branch_name]); + + const missingYesIo = bufferIO(); + const missingYes = await dispatch( + ["task", "recreate-worktree", task.task_id], + built.deps, + missingYesIo, + ); + expect(missingYes.exitCode).toBe(1); + expect(JSON.parse(missingYesIo.err())).toMatchObject({ + error: "confirmation_required", + }); + + const io = bufferIO(); + const result = await dispatch( + ["task", "recreate-worktree", task.task_id, "--yes"], + built.deps, + io, + ); + expect(result.exitCode).toBe(0); + expect(io.err()).toBe(""); + expect(JSON.parse(io.out())).toMatchObject({ + task_id: task.task_id, + repo_id: "repo-a", + recovery_base: "remote_task_branch", + recovery_ref: `origin/${task.branch_name}`, + forced: false, + }); +}); diff --git a/packages/cli/tests/tick/tick_001_promotes_queued_to_running.test.ts b/packages/cli/tests/tick/tick_001_promotes_queued_to_running.test.ts index 7518f472..e0efcde7 100644 --- a/packages/cli/tests/tick/tick_001_promotes_queued_to_running.test.ts +++ b/packages/cli/tests/tick/tick_001_promotes_queued_to_running.test.ts @@ -1,4 +1,6 @@ import { afterEach, expect, test } from "bun:test"; +import { existsSync } from "node:fs"; +import { join } from "node:path"; import { createTaskDependency } from "../../src/core/task_dependencies.ts"; import { tick_once } from "../../src/core/tick.ts"; import { createHarness, type Harness } from "../support/harness.ts"; @@ -139,6 +141,307 @@ test("tick does not spawn tasks waiting on dependencies", async () => { }); }); +test("tick recreates missing queued respawn worktree from remote task branch", async () => { + h = createHarness(); + h.clock.set("2026-07-14T11:00:00.000Z"); + + const repoId = insertRepo(h.db, "repo-missing-respawn-worktree"); + const taskId = insertTask(h.db, { + taskId: "task-missing-respawn-worktree", + repoId, + }); + const worktreePath = join(h.dataDir, "worktrees", taskId); + h.db + .query(`UPDATE tasks SET worktree_path = ? WHERE task_id = ?`) + .run(worktreePath, taskId); + + const firstAttemptId = insertAttempt(h.db, { + taskId, + attemptNumber: 1, + reason: "initial", + consumedBudget: 1, + spawnedAt: "2026-07-14T10:00:00.000Z", + }); + h.db + .query( + `UPDATE attempts + SET ended_at = '2026-07-14T10:05:00.000Z', + exit_kind = 'spawn_failed' + WHERE attempt_id = ?`, + ) + .run(firstAttemptId); + const retryAttemptId = insertAttempt(h.db, { + taskId, + attemptNumber: 2, + reason: "spawn_failed", + consumedBudget: 1, + }); + insertFinalPromptArtifact(h.db, h.artifactRoot, h.clock, taskId, retryAttemptId); + + const built = buildTickDeps(h); + const branchName = `quay/${taskId}`; + built.git.setRemoteBranches(repoId, [branchName]); + built.git.setRemoteHeadSha(repoId, branchName, "retry-remote-head"); + built.github.setPrExists(repoId, branchName, false); + + expect(existsSync(worktreePath)).toBe(false); + + const results = await tick_once(built.deps); + + expect(results).toEqual([{ task_id: taskId, action: "spawned" }]); + expect(existsSync(worktreePath)).toBe(true); + expect(built.git.calls).toContainEqual({ + op: "hasRemoteBranch", + args: { repoId, branch: branchName }, + }); + expect(built.git.calls).toContainEqual({ + op: "fetch", + args: { repoId, ref: branchName }, + }); + expect(built.git.calls).toContainEqual({ + op: "worktreeAddExistingBranch", + args: { + repoId, + worktreePath, + branch: branchName, + baseRef: `origin/${branchName}`, + }, + }); + expect(built.commandRunner.calls).toEqual([ + { command: "bun install", cwd: worktreePath }, + ]); + expect(built.tmux.spawnCalls).toHaveLength(1); + expect(built.tmux.spawnCalls[0]!.worktreePath).toBe(worktreePath); + + const event = h.db + .query<{ event_type: string; event_data: string | null }, [string]>( + `SELECT event_type, event_data + FROM events + WHERE task_id = ? AND event_type = 'worktree_recreated'`, + ) + .get(taskId); + expect(event?.event_type).toBe("worktree_recreated"); + expect(JSON.parse(event!.event_data!)).toEqual({ + reason: "missing_queued_worktree", + branch_name: branchName, + recovery_base_branch: branchName, + recovery_base_ref: `origin/${branchName}`, + worktree_path: worktreePath, + }); +}); + +test("tick prunes stale git metadata before recreating missing queued respawn worktree", async () => { + h = createHarness(); + h.clock.set("2026-07-14T11:03:00.000Z"); + + const repoId = insertRepo(h.db, "repo-missing-respawn-worktree-stale-git"); + const taskId = insertTask(h.db, { + taskId: "task-missing-respawn-worktree-stale-git", + repoId, + }); + const worktreePath = join(h.dataDir, "worktrees", taskId); + h.db + .query(`UPDATE tasks SET worktree_path = ? WHERE task_id = ?`) + .run(worktreePath, taskId); + + const firstAttemptId = insertAttempt(h.db, { + taskId, + attemptNumber: 1, + reason: "initial", + consumedBudget: 1, + spawnedAt: "2026-07-14T10:00:00.000Z", + }); + h.db + .query( + `UPDATE attempts + SET ended_at = '2026-07-14T10:05:00.000Z', + exit_kind = 'spawn_failed' + WHERE attempt_id = ?`, + ) + .run(firstAttemptId); + const retryAttemptId = insertAttempt(h.db, { + taskId, + attemptNumber: 2, + reason: "spawn_failed", + consumedBudget: 1, + }); + insertFinalPromptArtifact(h.db, h.artifactRoot, h.clock, taskId, retryAttemptId); + + const built = buildTickDeps(h); + const branchName = `quay/${taskId}`; + built.git.setRemoteBranches(repoId, [branchName]); + built.git.worktreeBranches.set(worktreePath, { repoId, branch: branchName }); + built.github.setPrExists(repoId, branchName, false); + + expect(existsSync(worktreePath)).toBe(false); + + const results = await tick_once(built.deps); + + expect(results).toEqual([{ task_id: taskId, action: "spawned" }]); + expect(built.git.calls).toContainEqual({ + op: "worktreePrune", + args: { repoId }, + }); + expect(built.git.calls).toContainEqual({ + op: "worktreeAddExistingBranch", + args: { + repoId, + worktreePath, + branch: branchName, + baseRef: `origin/${branchName}`, + }, + }); + expect(existsSync(worktreePath)).toBe(true); + expect(built.tmux.spawnCalls).toHaveLength(1); +}); + +test("tick recreates missing queued respawn worktree from base when task branch is absent", async () => { + h = createHarness(); + h.clock.set("2026-07-14T11:05:00.000Z"); + + const repoId = insertRepo(h.db, "repo-missing-respawn-worktree-base"); + const taskId = insertTask(h.db, { + taskId: "task-missing-respawn-worktree-base", + repoId, + }); + const worktreePath = join(h.dataDir, "worktrees", taskId); + h.db + .query(`UPDATE tasks SET worktree_path = ? WHERE task_id = ?`) + .run(worktreePath, taskId); + + const firstAttemptId = insertAttempt(h.db, { + taskId, + attemptNumber: 1, + reason: "initial", + consumedBudget: 1, + spawnedAt: "2026-07-14T10:00:00.000Z", + }); + h.db + .query( + `UPDATE attempts + SET ended_at = '2026-07-14T10:05:00.000Z', + exit_kind = 'spawn_failed' + WHERE attempt_id = ?`, + ) + .run(firstAttemptId); + const retryAttemptId = insertAttempt(h.db, { + taskId, + attemptNumber: 2, + reason: "spawn_failed", + consumedBudget: 1, + }); + insertFinalPromptArtifact(h.db, h.artifactRoot, h.clock, taskId, retryAttemptId); + + const built = buildTickDeps(h); + const branchName = `quay/${taskId}`; + built.git.setRemoteBranches(repoId, []); + built.github.setPrExists(repoId, branchName, false); + + const results = await tick_once(built.deps); + + expect(results).toEqual([{ task_id: taskId, action: "spawned" }]); + expect(built.git.calls).toContainEqual({ + op: "fetch", + args: { repoId, ref: "main" }, + }); + expect(built.git.calls).toContainEqual({ + op: "worktreeAddExistingBranch", + args: { + repoId, + worktreePath, + branch: branchName, + baseRef: "origin/main", + }, + }); + expect(built.commandRunner.calls).toEqual([ + { command: "bun install", cwd: worktreePath }, + ]); + + const event = h.db + .query<{ event_data: string | null }, [string]>( + `SELECT event_data + FROM events + WHERE task_id = ? AND event_type = 'worktree_recreated'`, + ) + .get(taskId); + expect(JSON.parse(event!.event_data!)).toMatchObject({ + reason: "missing_queued_worktree", + branch_name: branchName, + recovery_base_branch: "main", + recovery_base_ref: "origin/main", + worktree_path: worktreePath, + }); +}); + +test("tick removes partially recreated queued worktree when dependency install fails", async () => { + h = createHarness(); + h.clock.set("2026-07-14T11:07:00.000Z"); + + const repoId = insertRepo(h.db, "repo-missing-respawn-worktree-install-fails"); + const taskId = insertTask(h.db, { + taskId: "task-missing-respawn-worktree-install-fails", + repoId, + }); + const worktreePath = join(h.dataDir, "worktrees", taskId); + h.db + .query(`UPDATE tasks SET worktree_path = ? WHERE task_id = ?`) + .run(worktreePath, taskId); + + const firstAttemptId = insertAttempt(h.db, { + taskId, + attemptNumber: 1, + reason: "initial", + consumedBudget: 1, + spawnedAt: "2026-07-14T10:00:00.000Z", + }); + h.db + .query( + `UPDATE attempts + SET ended_at = '2026-07-14T10:05:00.000Z', + exit_kind = 'spawn_failed' + WHERE attempt_id = ?`, + ) + .run(firstAttemptId); + const retryAttemptId = insertAttempt(h.db, { + taskId, + attemptNumber: 2, + reason: "spawn_failed", + consumedBudget: 1, + }); + insertFinalPromptArtifact(h.db, h.artifactRoot, h.clock, taskId, retryAttemptId); + + const built = buildTickDeps(h); + const branchName = `quay/${taskId}`; + built.git.setRemoteBranches(repoId, [branchName]); + built.github.setPrExists(repoId, branchName, false); + built.commandRunner.failNext("install boom"); + + const failed = await tick_once(built.deps); + + expect(failed).toEqual([ + { + task_id: taskId, + action: "spawn_substrate_failed", + error: expect.stringContaining("install_cmd failed"), + }, + ]); + expect(existsSync(worktreePath)).toBe(false); + expect(built.git.calls).toContainEqual({ + op: "worktreeRemove", + args: { worktreePath }, + }); + expect(built.tmux.spawnCalls).toHaveLength(0); + + const retried = await tick_once(built.deps); + + expect(retried).toEqual([{ task_id: taskId, action: "spawned" }]); + expect(built.commandRunner.calls).toEqual([ + { command: "bun install", cwd: worktreePath }, + { command: "bun install", cwd: worktreePath }, + ]); + expect(built.tmux.spawnCalls).toHaveLength(1); +}); + test("tick reconciles waiting dependencies from local merged task state before promotion", async () => { h = createHarness(); h.clock.set("2026-04-26T10:00:00.000Z"); diff --git a/packages/cli/tests/tick/umbrella_retire.test.ts b/packages/cli/tests/tick/umbrella_retire.test.ts new file mode 100644 index 00000000..9db02f18 --- /dev/null +++ b/packages/cli/tests/tick/umbrella_retire.test.ts @@ -0,0 +1,269 @@ +import { afterEach, expect, test } from "bun:test"; +import { tick_once } from "../../src/core/tick.ts"; +import { createHarness, type Harness } from "../support/harness.ts"; +import { insertRepo, insertTask } from "../support/fixtures.ts"; +import { buildTickDeps } from "../support/tick_deps.ts"; + +// BRIX-1924: tick retires an umbrella workflow whose children can no longer +// complete — all linked child tasks cancelled and nothing left that could ever +// reach `merged_to_feature_branch` / `complete_without_quay` (observed on the +// orphaned-active BRIX-1902 umbrella). + +let h: Harness | null = null; +afterEach(() => { + h?.cleanup(); + h = null; +}); + +function insertUmbrella( + repoId: string, + opts: { externalRef?: string; state?: "active" | "completed" | "cancelled" } = {}, +): number { + if (h === null) throw new Error("harness not initialized"); + const externalRef = opts.externalRef ?? "BRIX-1902"; + const now = h.clock.nowISO(); + const row = h.db + .query<{ umbrella_workflow_id: number }, [string, string, string, string, string, string]>( + `INSERT INTO umbrella_workflows ( + external_ref, repo_id, base_branch, feature_branch, state, + created_at, updated_at + ) VALUES (?, ?, 'dev', ?, ?, ?, ?) + RETURNING umbrella_workflow_id`, + ) + .get( + externalRef, + repoId, + `quay/umbrella/${externalRef}`, + opts.state ?? "active", + now, + now, + ); + if (!row) throw new Error("umbrella insert failed"); + return row.umbrella_workflow_id; +} + +// Adds an expected subtask that has been linked to a Quay child task in the +// given state (the umbrella_expected_tasks row stays `linked`, mirroring the +// BRIX-1902 shape where operators cancelled the children by hand). +function insertLinkedChild( + workflowId: number, + repoId: string, + opts: { externalRef: string; taskId: string; taskState: string }, +): void { + if (h === null) throw new Error("harness not initialized"); + const now = h.clock.nowISO(); + insertTask(h.db, { taskId: opts.taskId, repoId, state: opts.taskState }); + h.db + .query(`UPDATE tasks SET external_ref = ? WHERE task_id = ?`) + .run(opts.externalRef, opts.taskId); + h.db + .query( + `INSERT INTO umbrella_expected_tasks ( + umbrella_workflow_id, external_ref, state, created_at, updated_at + ) VALUES (?, ?, 'linked', ?, ?)`, + ) + .run(workflowId, opts.externalRef, now, now); + h.db + .query( + `INSERT INTO umbrella_tasks ( + umbrella_workflow_id, task_id, external_ref, created_at + ) VALUES (?, ?, ?, ?)`, + ) + .run(workflowId, opts.taskId, opts.externalRef, now); +} + +// Adds an expected subtask with no linked Quay task (still `expected`, or a +// `complete_without_quay` success recorded outside Quay). +function insertUnlinkedExpected( + workflowId: number, + opts: { + externalRef: string; + state: "expected" | "complete_without_quay"; + completionSource?: "linear" | "manual"; + }, +): void { + if (h === null) throw new Error("harness not initialized"); + const now = h.clock.nowISO(); + h.db + .query( + `INSERT INTO umbrella_expected_tasks ( + umbrella_workflow_id, external_ref, state, completion_source, + completed_at, created_at, updated_at + ) VALUES (?, ?, ?, ?, ?, ?, ?)`, + ) + .run( + workflowId, + opts.externalRef, + opts.state, + opts.state === "complete_without_quay" ? opts.completionSource ?? "manual" : null, + opts.state === "complete_without_quay" ? now : null, + now, + now, + ); +} + +function umbrellaState(workflowId: number): string { + if (h === null) throw new Error("harness not initialized"); + const row = h.db + .query<{ state: string }, [number]>( + `SELECT state FROM umbrella_workflows WHERE umbrella_workflow_id = ?`, + ) + .get(workflowId); + if (!row) throw new Error("umbrella not found"); + return row.state; +} + +test("tick retires an umbrella whose linked children are all cancelled", async () => { + h = createHarness(); + h.clock.set("2026-06-01T12:00:00.000Z"); + const repoId = insertRepo(h.db, "repo-umbrella-retire"); + const workflowId = insertUmbrella(repoId, { externalRef: "BRIX-1902" }); + insertLinkedChild(workflowId, repoId, { + externalRef: "BRIX-1903", + taskId: "child-1903", + taskState: "cancelled", + }); + insertLinkedChild(workflowId, repoId, { + externalRef: "BRIX-1904", + taskId: "child-1904", + taskState: "cancelled", + }); + const built = buildTickDeps(h); + + const results = await tick_once(built.deps); + + expect(results).toEqual([ + { task_id: `umbrella-${workflowId}`, action: "umbrella_retired" }, + ]); + expect(umbrellaState(workflowId)).toBe("cancelled"); + // Retirement must never touch a final PR. + expect(built.github.createPullRequestCalls).toEqual([]); + const workflow = h.db + .query<{ final_pr_task_id: string | null; final_pr_number: number | null }, [number]>( + `SELECT final_pr_task_id, final_pr_number FROM umbrella_workflows WHERE umbrella_workflow_id = ?`, + ) + .get(workflowId); + expect(workflow).toEqual({ final_pr_task_id: null, final_pr_number: null }); +}); + +test("tick leaves an umbrella active while a child is still in progress", async () => { + h = createHarness(); + h.clock.set("2026-06-01T12:05:00.000Z"); + const repoId = insertRepo(h.db, "repo-umbrella-inprogress"); + const workflowId = insertUmbrella(repoId, { externalRef: "BRIX-2000" }); + insertLinkedChild(workflowId, repoId, { + externalRef: "BRIX-2001", + taskId: "child-2001", + taskState: "cancelled", + }); + insertLinkedChild(workflowId, repoId, { + externalRef: "BRIX-2002", + taskId: "child-2002", + taskState: "running", + }); + const built = buildTickDeps(h); + + const results = await tick_once(built.deps); + + expect(results).toEqual([]); + expect(umbrellaState(workflowId)).toBe("active"); +}); + +test("tick does not retire an umbrella with an unlinked expected child", async () => { + h = createHarness(); + h.clock.set("2026-06-01T12:07:00.000Z"); + const repoId = insertRepo(h.db, "repo-umbrella-expected"); + const workflowId = insertUmbrella(repoId, { externalRef: "BRIX-2100" }); + insertLinkedChild(workflowId, repoId, { + externalRef: "BRIX-2101", + taskId: "child-2101", + taskState: "cancelled", + }); + // Still enqueueable — it could yet become a real child. + insertUnlinkedExpected(workflowId, { + externalRef: "BRIX-2102", + state: "expected", + }); + const built = buildTickDeps(h); + + const results = await tick_once(built.deps); + + expect(results).toEqual([]); + expect(umbrellaState(workflowId)).toBe("active"); +}); + +test("tick does not retire an umbrella that has a success among cancelled children", async () => { + h = createHarness(); + h.clock.set("2026-06-01T12:09:00.000Z"); + const repoId = insertRepo(h.db, "repo-umbrella-mixed"); + const workflowId = insertUmbrella(repoId, { externalRef: "BRIX-2200" }); + insertLinkedChild(workflowId, repoId, { + externalRef: "BRIX-2201", + taskId: "child-2201", + taskState: "cancelled", + }); + // A child already merged to the feature branch is real, kept work. + insertLinkedChild(workflowId, repoId, { + externalRef: "BRIX-2202", + taskId: "child-2202", + taskState: "merged_to_feature_branch", + }); + // And an out-of-Quay completion is also a success. + insertUnlinkedExpected(workflowId, { + externalRef: "BRIX-2203", + state: "complete_without_quay", + completionSource: "manual", + }); + const built = buildTickDeps(h); + + const results = await tick_once(built.deps); + + expect(results).toEqual([]); + expect(umbrellaState(workflowId)).toBe("active"); +}); + +test("tick does not touch an already-completed umbrella", async () => { + h = createHarness(); + h.clock.set("2026-06-01T12:11:00.000Z"); + const repoId = insertRepo(h.db, "repo-umbrella-completed"); + const workflowId = insertUmbrella(repoId, { + externalRef: "BRIX-2300", + state: "completed", + }); + // Even if its recorded children happen to be cancelled, a completed umbrella + // is terminal and must be left alone. + insertLinkedChild(workflowId, repoId, { + externalRef: "BRIX-2301", + taskId: "child-2301", + taskState: "cancelled", + }); + const built = buildTickDeps(h); + + const results = await tick_once(built.deps); + + expect(results).toEqual([]); + expect(umbrellaState(workflowId)).toBe("completed"); +}); + +test("umbrella retirement is idempotent across ticks", async () => { + h = createHarness(); + h.clock.set("2026-06-01T12:13:00.000Z"); + const repoId = insertRepo(h.db, "repo-umbrella-idempotent"); + const workflowId = insertUmbrella(repoId, { externalRef: "BRIX-2400" }); + insertLinkedChild(workflowId, repoId, { + externalRef: "BRIX-2401", + taskId: "child-2401", + taskState: "cancelled", + }); + const built = buildTickDeps(h); + + const first = await tick_once(built.deps); + expect(first).toEqual([ + { task_id: `umbrella-${workflowId}`, action: "umbrella_retired" }, + ]); + expect(umbrellaState(workflowId)).toBe("cancelled"); + + const second = await tick_once(built.deps); + expect(second).toEqual([]); + expect(umbrellaState(workflowId)).toBe("cancelled"); +});