diff --git a/CLAUDE.md b/CLAUDE.md index 6e5486e..3cde76a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co AEOS is a CLI that drives tickets through LLM-agent pipelines. A ticket is an **epic** or a **task**, and each has its own column sequence: -- **epic** — `BACKLOG → PRODUCT_SCOPING → TECH_SPEC → TASK_BREAKDOWN → DOD_GATE → DONE` +- **epic** — `BACKLOG → PRODUCT_SCOPING → TECH_SPEC → TASK_BREAKDOWN → INTEGRATION_REVIEW → DOD_GATE → DONE` - **task** — `BACKLOG → IMPLEMENTATION → CODE_REVIEW → QA → DONE` Each column has a worker agent, a reviewer agent with rubrics, and a human approval gate. An epic decomposes into child tasks and cannot leave `TASK_BREAKDOWN` until every child is `DONE`. `aeos orchestrator run ` drives the whole thing autonomously. See `README.md` for the full command surface, state machine table, and executor matrix — that document is current; prefer it over re-deriving behaviour from code. diff --git a/README.md b/README.md index 4f5af2e..39998bc 100644 --- a/README.md +++ b/README.md @@ -60,14 +60,15 @@ aeos ticket dod-approve MYPRJ-1 A ticket is either an **epic** or a **task**, and each follows its own pipeline. ``` -EPIC BACKLOG → PRODUCT_SCOPING → TECH_SPEC → TASK_BREAKDOWN ─────────→ DOD_GATE → DONE - PM Agent Architect Architect Human - ↓ ↓ ↓ ▲ - PRD Tech Spec tasks.md │ - ↓ ↓ ↓ │ - Reviewer Reviewer Reviewer │ - │ │ - fans out into child tasks all children DONE +EPIC BACKLOG → PRODUCT_SCOPING → TECH_SPEC → TASK_BREAKDOWN ──→ INTEGRATION_REVIEW → DOD_GATE → DONE + PM Agent Architect Architect Integration Reviewer Human + ↓ ↓ ↓ ↓ ▲ + PRD Tech Spec tasks.md integration-review.md │ + ↓ ↓ ↓ ↓ │ + Reviewer Reviewer Reviewer Reviewer │ + │ (whole-feature diff │ + fans out into child tasks vs PRD/tech spec) │ + all children DONE ────────────┘ ▼ │ TASK BACKLOG → IMPLEMENTATION → CODE_REVIEW → QA → DONE ───────────────────┘ Engineer Engineer QA @@ -79,7 +80,9 @@ TASK BACKLOG → IMPLEMENTATION → CODE_REVIEW → QA → DONE ──── An epic is scoped, specced, and decomposed — it never implements anything itself. Its child tasks do that, and the epic cannot leave `TASK_BREAKDOWN` until every -child reaches `DONE`. +child reaches `DONE`. It then runs `INTEGRATION_REVIEW`, which reviews the whole +assembled feature (the epic branch's `base..HEAD` diff) against the PRD and tech +spec — the check per-task review cannot make — before the human `DOD_GATE`. Each column follows the same cycle: @@ -124,7 +127,7 @@ The forward order depends on the ticket's kind: | Kind | Pipeline | | ------ | -------------------------------------------------------------------------- | -| `EPIC` | `BACKLOG → PRODUCT_SCOPING → TECH_SPEC → TASK_BREAKDOWN → DOD_GATE → DONE` | +| `EPIC` | `BACKLOG → PRODUCT_SCOPING → TECH_SPEC → TASK_BREAKDOWN → INTEGRATION_REVIEW → DOD_GATE → DONE` | | `TASK` | `BACKLOG → IMPLEMENTATION → CODE_REVIEW → QA → DONE` | `aeos ticket approve` uses the kind to pick the next column, so a task never @@ -181,6 +184,7 @@ Every non-terminal column allows the same set: | `IMPLEMENTATION` | task | all eight | | `CODE_REVIEW` | task | all eight | | `QA` | task | all eight | +| `INTEGRATION_REVIEW` | epic | all eight | | `DOD_GATE` | epic | all eight | | `DONE` | both | `null` only | @@ -264,7 +268,8 @@ Notes: | `aeos ticket approve ` | Advance a SIGNED_OFF ticket to the next column | | `aeos ticket sign-off ` | Manual override — set a ticket sub-state to SIGNED_OFF | | `aeos ticket move ` | Human override — move a ticket directly to any workflow status | -| `aeos ticket answer ` | Unblock a ticket after answering pre-flight questions | +| `aeos ticket answer ` | Unblock a `BLOCKED` ticket after answering pre-flight questions | +| `aeos ticket resolve ` | Resume an `ESCALATED` ticket after writing a decision in its `escalation.md` | | `aeos ticket dod-approve ` | Final human gate — mark ticket as DONE _(not yet implemented)_ | | `aeos dashboard` | Cross-project Kanban summary _(not yet implemented)_ | | `aeos costs [--project] [--ticket]` | LLM spend report _(not yet implemented)_ | @@ -293,7 +298,9 @@ aeos ticket approve AEOS-2 && aeos ticket run AEOS-2 # implementation ... # 5. Only once every task is DONE can the epic advance -aeos ticket approve AEOS-1 # → DOD_GATE +aeos ticket approve AEOS-1 # → INTEGRATION_REVIEW +aeos ticket run AEOS-1 # reviews the whole feature vs PRD/spec +aeos ticket approve AEOS-1 # → DOD_GATE (after human reads the verdict) ``` Nesting is one level deep: tasks hang off epics, and a task cannot itself have diff --git a/docs/aeos-pipeline-integrity-tech-spec.md b/docs/aeos-pipeline-integrity-tech-spec.md new file mode 100644 index 0000000..a74d0aa --- /dev/null +++ b/docs/aeos-pipeline-integrity-tech-spec.md @@ -0,0 +1,346 @@ +# Tech Spec — AEOS Pipeline Integrity Improvements + +**Date:** 2026-07-24 +**Status:** Implemented on `feat/pipeline-integrity` (all seven work items). See the status table below. +**Motivates:** `aeos-postmortem-stan1.md` — a full-epic run shipped two spec-fidelity defects that every per-task gate marked green. + +## Implementation status + +| WI | Status | Commit | Notes | +|----|--------|--------|-------| +| WI-1 | ✅ Done | `f9186d2` | Per-epic `aeos/` branch + `aeos-base/` tag; opt out with `AEOS_GIT_ISOLATION=none`. Delivered via env flag rather than `project.json` for v1. | +| WI-2 | ✅ Done | `e3e4bf3` | `INTEGRATION_REVIEW` epic column over `base..HEAD`; escalates to human (manual advance). Auto-remediation remains out of scope. | +| WI-3 | ✅ Done | `8bda398` | Parent PRD/tech-spec injected into child context; parent id parsed from ticket metadata (no new port). | +| WI-4 | ✅ Done | `b1964c7` | `spec-traceability.md` rubric on TASK_BREAKDOWN. | +| WI-5 | ✅ Done | `b1964c7` | Integration-seam criterion in `code-structure.md`. | +| WI-6 | ✅ Done | `e3e4bf3` | Folded into WI-2: deferred hand-offs verified by the integration reviewer + spec-fidelity rubric. | +| WI-7 | ✅ Done | `d856fa9` | `escalation.md` + `aeos ticket resolve`; response injected as a resolution artifact. | + +Deltas from the design below: WI-1's opt-out is an env var (not yet a +`project.json` field); the epic base is captured as a git **tag** rather than a +stored `base_ref` column, so no migration was needed and `ContextAssembler` +derives the range from the ticket id alone. + +## 1. Overview + +The postmortem identified six root causes (RC-1…RC-6). This spec designs the +changes that close them, against the current hexagonal architecture +(`cli/ → application/ → domain/ ← infrastructure/`). Nothing here is implemented; +each work item is scoped so it can become one or more child tasks. + +**Guiding principle:** the pipeline must be able to answer *"does the assembled +feature match the PRD and the tech-spec decisions?"* at least once, with the +whole feature in view — and no completed task's work may be silently lost. + +Scope in priority order (from the postmortem's ranking): + +| WI | Root cause | Title | Leverage | +|----|-----------|-------|----------| +| WI-1 | RC-1 | Durable per-task isolation (feature branch + task commits + intact-check) | Critical | +| WI-2 | RC-3 | Epic `INTEGRATION_REVIEW` stage over the assembled diff | Critical | +| WI-3 | RC-2 | Inject epic PRD/tech-spec into child-task context | High | +| WI-4 | RC-4 | Spec-traceability dimension in the `TASK_BREAKDOWN` rubric | Medium | +| WI-5 | RC-5 | Require a non-mocked integration test per integrating task | Medium | +| WI-6 | RC-6 | Make `Depends on:` a checkable contract | Medium | +| WI-7 | RC-7 | Resolve every escalation through an editable MD file, like preflight questions | High (ergonomics) | + +WI-1, WI-2, WI-3 each independently would have caught one of the two shipped +defects; they are the core of this spec. WI-7 is a human-in-the-loop ergonomics +change that makes every escalation (including WI-2's) respondable and resumable +without the operator hand-resetting state. + +## 2. Current-state facts this design depends on + +- **Columns** are an enum (`src/domain/model/column.ts`) with per-kind sequences + and transition rules enforced by `StateMachineService` + (`domain/services/state-machine.ts`) via `nextColumnFor(kind, column)`. +- **Column behavior is data**: `.aeos/column-specs/*.yaml` (worker/reviewer + agents, rubrics, `executorMode`, `requiresRepoDiff`, `advanceMode`, + `preflight`). Adding a column requires a template spec + agents + rubrics + (`templates/`), guarded by `template-source.test.ts`. +- **Context** is assembled by `ContextAssembler.assemble(ticketId, projectRoot, + column)`. `priorArtifacts` come **only** from + `listArtifacts(projectRoot, ticketId)` — the ticket's own directory. The + CODE_REVIEW diff is `gitGateway.diff(projectRoot)` = `git diff HEAD`. +- **The orchestrator** (`domain/services/orchestrator-policy.ts`, + `decideNextAction`) is a pure scheduler. The epic's "all children DONE" join + currently gates `TASK_BREAKDOWN → DOD_GATE`. +- **Git**: `GitGateway` now has `stageAll`, `commitAll`, `commit`, `commitFiles`, + `diff`. There is one working tree per project; there is no branch-per-task. + +## 3. Work items + +### WI-1 — Durable per-task isolation (RC-1) + +**Problem.** All task work accumulates in one uncommitted working tree; a task's +tracked-file edits can be reverted by later churn with no signal. T-001's +verified `id`-forwarding was lost this way. + +**Design.** + +1. **Epic feature branch.** When the orchestrator begins driving an epic (or on + the epic leaving `TASK_BREAKDOWN`), create/checkout a branch + `aeos/` off the current `HEAD` of the target repo. All task commits + land here. Extend `GitGateway` with: + - `currentBranch(dir): string` + - `checkoutBranch(dir, name, { createFrom?: string }): void` + - `mergeBase(dir, ref): string` (for the integration diff, WI-2) +2. **Commit at task DONE** (already implemented via `commitAll` in + `TicketApproveUseCase`) — keep, but target the feature branch. +3. **Intact-check at task start.** Before an agentic worker runs, assert the + working tree has **no uncommitted tracked modifications** it did not create + (i.e. the tree is clean except for what a re-run baseline expects). If dirty + with unexpected tracked changes, escalate `NEEDS_HUMAN` rather than proceed — + losing prior work must be loud. Reuses the baseline-commit machinery already + in `TicketRunUseCase.runAttempt`. +4. **Prior-work assertion (lightweight).** At epic `INTEGRATION_REVIEW` (WI-2), + the review runs against the full branch diff, which structurally surfaces any + task whose committed contribution is missing — no separate mechanism needed + for v1. + +**Ports/adapters touched.** `GitGateway` (+ `SimpleGitGateway`), +`TicketRunUseCase`, `OrchestratorUseCase` (branch setup), `container.ts`. + +**Alternatives considered.** Full `git worktree`-per-task (stronger isolation, +parallel tasks) — deferred; sequential tasks + feature branch is enough to stop +loss and is far simpler. Revisit if tasks ever run concurrently. + +**Risk.** Writing branches/commits to the user's *source* repo is a policy step +beyond `.aeos/.git`. Must be opt-outable (`project.json` flag, e.g. +`"gitIsolation": "branch" | "none"`) and must never touch a dirty repo without +consent. + +### WI-2 — Epic `INTEGRATION_REVIEW` stage (RC-3) + +**Problem.** No stage ever reviews the *assembled* feature against the PRD and +tech-spec decisions. Both shipped defects are integration/fidelity issues. + +**Design.** Add one epic column: + +``` +EPIC: BACKLOG → PRODUCT_SCOPING → TECH_SPEC → TASK_BREAKDOWN + → INTEGRATION_REVIEW → DOD_GATE → DONE +``` + +1. **Column** `INTEGRATION_REVIEW` in `column.ts`, EPIC sequence + transitions in + `state-machine.ts` / `nextColumnFor`. Migration note: existing epics parked in + `TASK_BREAKDOWN`/`DOD_GATE` are unaffected (additive enum + sequence). +2. **Join moves here.** The "all children DONE" gate that currently guards + `TASK_BREAKDOWN → DOD_GATE` now guards `TASK_BREAKDOWN → INTEGRATION_REVIEW` + (in `TicketApproveUseCase` and `orchestrator-policy.decideNextAction`). An + epic may only enter `INTEGRATION_REVIEW` once every child is DONE. +3. **Worker + reviewer.** Reuse the `reviewer-agent`; add an + `integration-reviewer` worker whose job is to produce an integration report, + reviewed against a new rubric `rubrics/drift/spec-fidelity.md`: + - Every PRD acceptance criterion is met by the assembled code. + - Every numbered tech-spec decision (D-1…D-N) is honored or has a logged, + signed-off deviation. + - Every task's `Touches`/deliverable is present in the branch (catches lost + work — RC-1/RC-6). + - Cross-component seams are exercised by a real (non-mocked) test (RC-5 hook). +4. **Context for this column.** `ContextAssembler` for `INTEGRATION_REVIEW` + injects: the epic PRD, the epic tech-spec, the task breakdown, and the + **full feature diff** = `git diff ..HEAD` on `aeos/` + (not `git diff HEAD`). Add a branch-aware diff to `GitGateway` + (`diffRange(dir, from, to)`). +5. **Verdict handling.** `REJECTED` escalates `NEEDS_HUMAN` with the findings + (same machinery as task escalation, now persisted per RC of the escalation + work already shipped). v1 does **not** auto-spawn fix tasks — the operator + decides (re-open a task, or accept). Auto-remediation is a future item. +6. **`advanceMode`.** Default `manual` — a human confirms the integration verdict + before `DOD_GATE`. + +**Templates required** (or a fresh `project init` / `project sync` breaks): +`templates/column-specs/integration-review.yaml`, +`templates/agents/integration-reviewer-agent.yaml`, +`templates/rubrics/drift/spec-fidelity.md`. `template-source.test.ts` will force +these to exist. + +**This is the highest-leverage, most invasive item.** It is the one stage that +would have caught *both* defects. + +### WI-3 — Epic PRD/tech-spec in child-task context (RC-2) + +**Problem.** A child task never sees the epic PRD/tech-spec, so it cannot check +its work against D-6/D-9.1. Confirmed in `ContextAssembler` — `priorArtifacts` +read only the ticket's own dir. + +**Design.** + +1. Give `ContextAssembler` the ability to resolve a task's parent: inject a + `TicketRepository` (it currently has `projectRepo`, `artifactStore`, + `gitGateway`), or have `TicketRunUseCase` pass the parent's key artifacts in. + Prefer injecting `TicketRepository` — keeps the caller thin. +2. When the ticket is a `TASK` with a `parentId`, additionally read the parent + epic's **PRD** and **tech-spec** artifacts from + `.aeos/tickets//` and add them to context as a distinct, + clearly-labeled block (new `AssembledContext.epicContext`, or labeled + entries in `priorArtifacts`, e.g. `EPIC PRD (STAN-1)`). +3. `buildPrompt` renders the epic block ahead of the task body, framed as + authoritative spec the task must not contradict. + +**Context-size guard.** PRD + tech-spec can be large. v1 injects both in full +(they are the load-bearing docs and the whole point). If token pressure +appears, add a `decisions-digest` extraction (the D-1…D-N list + acceptance +criteria) as a later refinement — do not prematurely truncate. + +**Ports/adapters touched.** `ContextAssembler` (+ its constructor wiring in +`container.ts`), `AssembledContext` model, `buildPrompt`. + +### WI-4 — Spec-traceability in the `TASK_BREAKDOWN` rubric (RC-4) + +**Problem.** The breakdown contradicted D-6/D-9.1 before any code existed, and +the `TASK_BREAKDOWN` review (intent-drift vs ticket/PRD) never checked against +the tech-spec's numbered decisions. + +**Design.** Add a dimension to the architect's reviewer rubric +(`rubrics/drift/intent-drift.md` or a new `rubrics/structure/spec-traceability.md`): + +- Every task traces to at least one tech-spec section/decision. +- **No task may contradict a numbered decision (D-N)**; a deliberate deviation + must be called out as such and flagged for sign-off (not silently baked in). +- The coverage table must map decisions → tasks, and unmapped decisions are a + finding. + +The `TASK_BREAKDOWN` reviewer already receives the epic tech-spec as a prior +artifact (same ticket dir), so this is primarily a rubric + prompt change, not +new plumbing. Purely data (`templates/rubrics/…`), shippable independently. + +### WI-5 — Require a non-mocked integration test (RC-5) + +**Problem.** Every task mocked its collaborators; the real `sink` (drops `id`) +and the real delivery path (`response_url` vs `chat.update`) were never +exercised. Green tests hid both defects. + +**Design.** + +1. Add a QA/CODE_REVIEW rubric requirement: a task that integrates two + components must include at least one test that exercises the **real seam** + (collaborator not mocked), or explicitly justify why it cannot. +2. Reinforce at `INTEGRATION_REVIEW` (WI-2): the spec-fidelity rubric asserts the + feature's critical seams have non-mocked coverage; absence is a finding. + +Rubric-only (`templates/rubrics/…`); no code. Weaker on its own — pairs with +WI-2, which is the backstop. + +### WI-6 — `Depends on:` as a checkable contract (RC-6) + +**Problem.** `feedback.ts` deferred behavior to T-001 ("out of scope here") with +nothing verifying T-001's contribution still existed. + +**Design (lightweight, v1).** + +1. Keep `Depends on:` in the task body (already present). No new schema. +2. `INTEGRATION_REVIEW` (WI-2) verifies deferred hand-offs: for each task that + defers behavior to another (`Depends on:` / "out of scope, see T-NNN"), the + spec-fidelity rubric checks the depended-on behavior is actually present in + the assembled branch. + +A structured dependency graph (enforced ordering, machine-checked contracts) is +a future item; v1 leans on the integration review to catch the class. + +### WI-7 — Resolve escalations through an editable MD file (RC-7) + +**Problem.** Preflight blockers already have an excellent round-trip: they write +`-questions.md`, the operator fills in the `### Answer` block, runs +`aeos ticket answer`, and the run resumes carrying the answer. Every *other* +escalation (`ITERATIONS_EXHAUSTED`, `NOT_CONVERGING`, `UNPARSEABLE_VERDICT`, and +WI-2's integration-review rejection) has no such flow. The operator must read the +review artifact, reverse-engineer what to change, edit code/docs by hand, and +`aeos ticket ready` to retry — with no structured way to *tell the pipeline what +they decided*. Every escalation in this session (STAN-4 scope creep, STAN-6/8 +preflight, the epic-level review) forced manual state archaeology. + +Generalize the questions.md pattern to all escalations so the human edits a file +and moves on, and their response becomes context for the retry. + +**Design.** + +1. **Escalation artifact.** When a run ends `ESCALATED`, write + `-escalation.md` alongside the persisted escalation (the reason is + already stored on the ticket — this is its editable, human-facing form). It + contains: + - `Reason:` (the `EscalationReason`) and the operator-facing message. + - `See:` pointer to the driving artifact (the review/QA report). + - The unresolved blockers/findings (blocker-topics), so the decision has the + evidence inline. + - A fenced `## Response` block for the operator, mirroring questions.md's + `### Answer`. +2. **Resolve command.** `aeos ticket resolve ` (or extend `ticket answer` to + cover both artifacts): reads the `## Response` block, clears the escalation, + and sets the ticket back to `READY`. Empty response ⇒ error ("nothing to + resolve"), same guard as `ticket answer`. +3. **Response feeds the retry.** The operator's response is promoted into the + ticket's context for the next run — as settled feedback the worker must honor + (e.g. *"accept the attributable-asker narrowing"*, *"use `response_url` per + D-6, not `chat.update`"*, *"T-001's id-forwarding was lost — re-add it"*). + Reuse the existing decisions/feedback path (`decisions.md` / + `settledDecisions` in `ContextAssembler`, `DecisionPromotionService`) so the + response is durable and injected on the retry, not just a one-off note. +4. **Uniform surface.** Preflight keeps `questions.md`; loop/integration + escalations use `escalation.md`; both resolve the same way ("edit the block, + run one command, resume"). `aeos ticket show` already surfaces the reason + (shipped) — it should also point at `escalation.md` when present. +5. **Applies to WI-2.** The epic `INTEGRATION_REVIEW` rejection escalates through + this same file, so the operator responds to a whole-feature review the same + way — decide per finding, resolve, and the epic resumes. + +**Ports/adapters touched.** `TicketRunUseCase.finishEscalatedRun` (write the +artifact), a `TicketResolve` use case + driving port + `resolve` command +(mirrors `TicketAnswer`), `ContextAssembler`/`DecisionPromotionService` (inject +the response), `container.ts`. Escalation persistence (reason on the ticket) is +already shipped, so this builds on it. + +**Why High-ergonomics.** It does not catch a defect, but it collapses the +operator's cost of *acting on* one from "read artifact → reason → hand-edit → +guess the reset command" to "write a response, run `resolve`." That directly +raises how usable the human-in-the-loop is — the entire point of escalation +being distinct from failure. + +## 4. Sequencing + +| Phase | Items | Rationale | +|-------|-------|-----------| +| 1 | WI-4, WI-5 | Pure rubric/data changes; no code; immediate value; low risk. | +| 2 | WI-3, WI-7 | Context injection + escalation ergonomics; self-contained; WI-7 builds on already-shipped escalation persistence and makes every later stage respondable. | +| 3 | WI-1 | Git isolation; prerequisite for a meaningful branch-wide integration diff. | +| 4 | WI-2, WI-6 | Integration review consumes WI-1's branch and WI-3's context, escalates through WI-7's file; WI-6 rides its rubric. | + +Phase 1 is shippable this week and would have flagged the decomposition drift. +WI-7 lands in Phase 2 so that by the time WI-2's integration review can reject a +whole feature, the operator already has a file-based way to respond to it. Phase +4 is the structural fix and depends on Phases 2–3. + +## 5. Testing strategy + +- **Domain (pure).** `nextColumnFor`/state-machine transitions for the new + `INTEGRATION_REVIEW` column; `decideNextAction` scheduling the epic into it + only after all children DONE. +- **Application.** `ContextAssembler` includes parent PRD/tech-spec for a TASK, + and does not for an EPIC; the integration-review context uses the branch-range + diff; the intact-check escalates on an unexpectedly dirty tree. +- **Infrastructure.** `GitGateway` branch/mergeBase/diffRange against a real temp + repo (as done for `commitAll`). +- **Templates.** `template-source.test.ts` must green with the new column spec, + agent, and rubrics — a fresh `project init` must fully load. +- **Regression.** A test that reproduces the STAN-1 shape: a task that "completes" + a contribution which is then absent from the branch is flagged by + `INTEGRATION_REVIEW`. + +## 6. Migration & compatibility + +- New column is additive; DB migration for tickets is not required (column lives + in the enum + specs). Existing epics mid-flight advance through the new stage + on their next transition. +- `project sync` (already shipped) is the delivery vehicle for the new + templates into existing projects; the postmortem's staleness lesson applies — + ship the templates and tell operators to `aeos project sync`. +- Git isolation (WI-1) must be opt-outable and must refuse to run against a + dirty source repo without consent. + +## 7. Explicitly out of scope (future) + +- Auto-remediation: `INTEGRATION_REVIEW` spawning fix-tasks instead of escalating. +- `git worktree`-per-task / concurrent task execution. +- A machine-enforced dependency graph with typed contracts. +- Token-budgeted "decisions digest" extraction for context (only if size bites). diff --git a/docs/aeos-postmortem-stan1.md b/docs/aeos-postmortem-stan1.md new file mode 100644 index 0000000..461a4ba --- /dev/null +++ b/docs/aeos-postmortem-stan1.md @@ -0,0 +1,159 @@ +# Postmortem — STAN-1 "Slack Bot Answer Feedback" shipped with two spec-fidelity defects + +**Date:** 2026-07-24 +**Subject:** First full-epic run of the AEOS pipeline on a real feature (project `Standin`, epic `STAN-1`, tasks `T-001`…`T-007`). +**Trigger:** A human/out-of-band code review of the assembled feature found two real functional defects that every AEOS per-task review and QA pass had marked green. + +This is a process postmortem for **AEOS itself**, not for the Standin feature. The +goal is to explain how a pipeline that reported success at every gate shipped +non-functional behavior, and to record hypotheses for fixing the pipeline. No +AEOS code changes are proposed here — see +`aeos-pipeline-integrity-tech-spec.md` for the design that acts on these +findings. + +--- + +## 1. What shipped broken + +The external review (`.aeos/tickets/STAN-1/STAN-1-code-review.md`) found: + +| # | Severity | Defect | Spec decision violated | +|---|----------|--------|------------------------| +| 1 | HIGH | Deterministic Langfuse score `id` is dropped by the real sink, so every 👍→👎 flip writes a **new** score instead of upserting. Eval data silently corrupts. | D-2 / §2.1, impl-plan Step 1 | +| 2 | MEDIUM–HIGH | Visual acknowledgement uses `chat.update` with stored `channel`+`ts`, which cannot edit a slash-command `response_url` message — the user never sees "logged" and buttons stay live. | D-6 | +| 3 | LOW | Migration adds `slack_team_id/channel_id/message_ts` columns that D-9.1 explicitly rejected; they exist only to feed the broken `chat.update` path. | D-9.1 | + +All 36 tests passed and `tsc` was clean. The review's own summary: *"the tests +mock past the broken seams (false confidence)."* + +--- + +## 2. How the pipeline ran + +`STAN-1` (epic) decomposed into seven tasks, each of which ran the task pipeline +(`IMPLEMENTATION → CODE_REVIEW → QA → DONE`) independently: + +| Task | Ticket | Scope | +|------|--------|-------| +| T-001 | STAN-2 | Add optional deterministic `id` to `ScoreRecord` and the Langfuse sink | +| T-002 | STAN-3 | Allow unattributed questions so every answer is rateable | +| T-003 | STAN-4 | `slack_answer_feedback` table migration | +| T-004 | STAN-5 | `lib/slack/feedback.ts` + unit tests | +| T-005 | STAN-6 | Render feedback actions on every answer | +| T-006 | STAN-7 | Handle feedback interactions in the interactive endpoint | +| T-007 | STAN-8 | Document the feedback loop | + +Every task reached `DONE` with a passing CODE_REVIEW and a `READY FOR DOD` QA +verdict. + +--- + +## 3. Root causes + +### 3.1 Defect #1 was completed work that AEOS lost (state-management failure) + +This is the most severe finding and the least expected. + +- T-001 (STAN-2) was exactly the task *"Add optional `id` to `ScoreRecord` and + the Langfuse sink."* +- STAN-2's implementation notes, CODE_REVIEW, and QA report all describe and + verify a **real diff**, line-cited: `sink.ts:16-17` adds `id?: string`; + `langfuse.ts:11-12` forwards it to `client.score()`. QA: *"verified against + the actual diff … READY FOR DOD."* +- The current code has **neither**. `id` is absent from both `sink.ts` and + `langfuse.ts`, and `git log -S id -- lib/llm/scores/langfuse.ts` shows it was + never committed. The only commit in the repo is `30c5b9f [STAN-8]` — the + per-task commit added late in the run — and T-001's work is not in it. + +**Conclusion:** the pipeline reviewed and QA'd this work correctly. The work was +then **silently reverted** in the shared working tree before anything committed +it. The exact git operation is unrecoverable, but the structural cause is clear: +for almost the entire epic there were **no per-task commits** — every task's +edits lived in one mutable, uncommitted working tree, so any task's tracked-file +changes could be (and were) lost by later re-runs, reverts, or `git add -A` +churn with zero signal. + +This is not a context or review gap. It is a durability gap. + +### 3.2 Defects #2/#3 drifted at decomposition and were never reconciled to the spec + +- The tech spec's D-6 chose `response_url` + `replace_original` *specifically + because* the answers are not bot-posted messages; D-9.1 rejected denormalized + Slack columns in favor of `question_id` lookup. +- The **task breakdown contradicted both** before any code was written: STAN-4's + ticket body (from `tasks.md`) says *"…plus the Slack coordinates … provide a + `chat.update` fallback."* Every downstream task then faithfully implemented a + plan that already violated the spec. +- Nothing reconciled the breakdown back to the spec. The `TASK_BREAKDOWN` review + checks intent-drift against the *ticket/PRD*, not against the tech spec's + specific load-bearing decisions, so the drift passed. + +### 3.3 The structural gaps that let all of it through + +1. **Child tasks never see the epic PRD/tech spec.** + `ContextAssembler.assemble()` builds `priorArtifacts` from + `listArtifacts(projectRoot, ticketId)` — the **current ticket's own + directory only**. A child task (`.aeos/tickets/STAN-5/`) never reads + `STAN-1-prd.md` or `STAN-1-tech-spec.md`. The engineer sees the task body's + *references* to "§2.1 / §4.1" but never the actual decisions, so it cannot + check its work against D-6/D-9.1 — the spec is not in the room. + +2. **No epic-level integration review.** The epic pipeline is + `…→TASK_BREAKDOWN→DOD_GATE→DONE`. There is no epic-level CODE_REVIEW or QA + over the *assembled* feature against the PRD. DOD_GATE is a human gate over + artifacts. Both shipped defects are integration/spec-fidelity issues that + only a whole-feature review catches — and that review happened outside AEOS. + +3. **Per-task unit tests + mocks give false green.** Each task tests its unit in + isolation with mocked collaborators. `feedback.test.ts` asserts `sink.record` + was called *with* `id` against a **mock** sink; the real sink that drops `id` + is never exercised. No test runs the integrated seam, so green tests actively + hid both defects. + +4. **Cross-task dependencies are invisible and unverified.** `feedback.ts` + comments that id-forwarding is *"out of scope here"* — deferring to T-001. + Nothing in AEOS models or checks that hand-off. T-004 built on a T-001 + contribution that no longer existed, and no stage noticed. + +**Cross-session observation (RC-7, ergonomics not correctness).** Every +escalation in this run — STAN-4 scope creep, STAN-6/STAN-8 preflight blocks, the +epic-level review — required the operator to read an artifact, reverse-engineer +what to change, hand-edit, and guess the reset command. Only preflight has a +structured "edit a block, run one command, resume" flow (`questions.md` + +`aeos ticket answer`). Escalation is meant to be distinct from failure — a +decision point, not an error — but acting on one is currently as costly as +debugging a failure. Generalizing the questions.md round-trip to all escalations +is a high-value usability fix even though it caught none of the defects. + +--- + +## 4. Findings mapped to fixes + +| # | Root cause | Severity to AEOS | Improvement hypothesis | +|---|------------|------------------|------------------------| +| RC-1 | No durable per-task isolation; completed work lost | **Critical** | Per-task branch/worktree + commit; verify upstream work intact at task start | +| RC-2 | Child tasks lack epic PRD/tech-spec context | High | Inject epic lineage (or a decisions digest) into child context | +| RC-3 | No epic-level integration review vs PRD/spec | High | Add an epic INTEGRATION_REVIEW stage before DOD_GATE | +| RC-4 | Breakdown not reconciled to spec decisions | Medium | Add a spec-traceability dimension to the TASK_BREAKDOWN rubric | +| RC-5 | Mocked unit tests hide integration defects | Medium | Require ≥1 non-mocked integration test per feature | +| RC-6 | Cross-task dependencies unmodeled | Medium | Make `Depends on:` a checkable contract verified at epic review | +| RC-7 | Escalations are hard to act on — no structured "respond and resume" for anything but preflight | High (ergonomics) | Resolve every escalation through an editable `escalation.md`, like preflight `questions.md` | + +Severity ranking for AEOS work: +**RC-1 ≈ RC-3 > RC-2 > RC-4 > RC-5 > RC-6.** RC-1, RC-2, and RC-3 would each +independently have caught one of the two shipped defects. + +--- + +## 5. What already changed during this session (partial, not sufficient) + +- **Per-task commits** were added mid-epic (baseline commit before an agentic + worker; a commit when a TASK reaches DONE). This reduces future accumulation + but arrived after the damage, and does not isolate a task's own review-loop + work or verify that upstream contributions survive. RC-1 is only partly + addressed. +- **Ticket body + lineage** — child tickets now carry their full task breakdown + and parent metadata. This makes the *task's own* scope legible but does **not** + inject the epic PRD/tech spec. RC-2 is not addressed. + +The remaining work is specified in `aeos-pipeline-integrity-tech-spec.md`. diff --git a/src/application/orchestrator.use-case.test.ts b/src/application/orchestrator.use-case.test.ts index 4327157..51125ad 100644 --- a/src/application/orchestrator.use-case.test.ts +++ b/src/application/orchestrator.use-case.test.ts @@ -14,6 +14,7 @@ import type { ColumnSpecLoader } from '../domain/ports/driven/column-spec-loader import type { OrchestratorStateRepository } from '../domain/ports/driven/orchestrator-state-repository.port.js'; import type { TicketRunPort } from '../domain/ports/driving/ticket-run.port.js'; import type { TicketApprovePort } from '../domain/ports/driving/ticket-approve.port.js'; +import type { GitGateway } from '../domain/ports/driven/git-gateway.port.js'; const PROJECT_ID = 'p'; const PROJECT_PATH = '/tmp/proj'; @@ -42,6 +43,7 @@ describe('OrchestratorUseCase', () => { let configStore: ConfigStore; let ticketRun: TicketRunPort; let ticketApprove: TicketApprovePort; + let gitGateway: GitGateway; let useCase: OrchestratorUseCase; beforeEach(() => { @@ -98,6 +100,20 @@ describe('OrchestratorUseCase', () => { }), }; + gitGateway = { + init: vi.fn(), + commit: vi.fn(), + commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + diff: vi.fn().mockReturnValue(''), + isRepo: vi.fn().mockReturnValue(false), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), + }; + useCase = new OrchestratorUseCase( ticketRepo, costRepo, @@ -106,6 +122,7 @@ describe('OrchestratorUseCase', () => { configStore, ticketRun, ticketApprove, + gitGateway, ); }); @@ -172,6 +189,29 @@ describe('OrchestratorUseCase', () => { expect(result.spentUsd).toBe(6); }); + it('sets up an isolated epic branch + base tag when the repo is git', async () => { + (gitGateway.isRepo as ReturnType).mockReturnValue(true); + (ticketRepo.findById as ReturnType).mockReturnValue( + epic({ column: Column.DOD_GATE, subState: SubState.READY }), + ); + + await useCase.run(PROJECT_ID, PROJECT_PATH, EPIC_ID); + + expect(gitGateway.ensureOnBranch).toHaveBeenCalledWith(PROJECT_PATH, 'aeos/AEOS-1'); + expect(gitGateway.tagHere).toHaveBeenCalledWith(PROJECT_PATH, 'aeos-base/AEOS-1'); + }); + + it('skips git isolation when the project path is not a repo', async () => { + (gitGateway.isRepo as ReturnType).mockReturnValue(false); + (ticketRepo.findById as ReturnType).mockReturnValue( + epic({ column: Column.DOD_GATE, subState: SubState.READY }), + ); + + await useCase.run(PROJECT_ID, PROJECT_PATH, EPIC_ID); + + expect(gitGateway.ensureOnBranch).not.toHaveBeenCalled(); + }); + it('refuses to run a paused epic', async () => { (stateRepo.find as ReturnType).mockReturnValue({ projectId: PROJECT_ID, diff --git a/src/application/orchestrator.use-case.ts b/src/application/orchestrator.use-case.ts index 4dae54c..e23cda5 100644 --- a/src/application/orchestrator.use-case.ts +++ b/src/application/orchestrator.use-case.ts @@ -23,6 +23,7 @@ import type { OrchestratorStateRepository } from '../domain/ports/driven/orchest import type { TicketRunPort } from '../domain/ports/driving/ticket-run.port.js'; import type { TicketRunObserver } from '../domain/model/ticket-run-event.js'; import type { TicketApprovePort } from '../domain/ports/driving/ticket-approve.port.js'; +import type { GitGateway } from '../domain/ports/driven/git-gateway.port.js'; import type { OrchestratorPort, OrchestratorObserver, @@ -59,8 +60,22 @@ export class OrchestratorUseCase implements OrchestratorPort { private readonly configStore: ConfigStore, private readonly ticketRun: TicketRunPort, private readonly ticketApprove: TicketApprovePort, + private readonly gitGateway: GitGateway, ) {} + /** + * Gives the epic its own feature branch and marks its base commit, so task + * commits accumulate in isolation and the whole-feature diff (base..HEAD) can + * be computed at INTEGRATION_REVIEW. No-op when the project path is not a git + * repo, or when isolation is disabled via AEOS_GIT_ISOLATION=none. + */ + private setUpGitIsolation(projectPath: string, epicId: string): void { + if (process.env.AEOS_GIT_ISOLATION === 'none') return; + if (!this.gitGateway.isRepo(projectPath)) return; + this.gitGateway.ensureOnBranch(projectPath, `aeos/${epicId}`); + this.gitGateway.tagHere(projectPath, `aeos-base/${epicId}`); + } + async run( projectId: string, projectPath: string, @@ -112,6 +127,9 @@ export class OrchestratorUseCase implements OrchestratorPort { this.writeState(projectId, epicId, OrchestratorStatus.RUNNING, null, null, budgetUsd); let settled = false; + // Isolate this epic's work on its own branch before any task runs. + this.setUpGitIsolation(projectPath, epicId); + try { for (let step = 0; step < maxSteps; step += 1) { // An interrupt that arrived between actions stops before scheduling the diff --git a/src/application/project-init.use-case.test.ts b/src/application/project-init.use-case.test.ts index 326c8ed..fb95f25 100644 --- a/src/application/project-init.use-case.test.ts +++ b/src/application/project-init.use-case.test.ts @@ -35,6 +35,13 @@ function createMockGitGateway(): GitGateway { init: vi.fn(), commit: vi.fn(), commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), diff: vi.fn().mockReturnValue(''), }; } diff --git a/src/application/services/context-assembler.test.ts b/src/application/services/context-assembler.test.ts index 98ae17f..6ff1499 100644 --- a/src/application/services/context-assembler.test.ts +++ b/src/application/services/context-assembler.test.ts @@ -34,6 +34,13 @@ function createMockGitGateway(): GitGateway { init: vi.fn(), commit: vi.fn(), commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), diff: vi.fn().mockReturnValue(''), }; } @@ -250,3 +257,105 @@ describe('ContextAssembler', () => { expect(truncatedContent).toHaveLength(MAX_DIFF_CHARS); }); }); + +describe('ContextAssembler — epic context (WI-3)', () => { + let artifactStore: ReturnType; + let projectRepo: ReturnType; + let gitGateway: ReturnType; + let assembler: ContextAssembler; + + const PROJECT_ROOT = '/projects/test'; + + // A child-task ticket document carries "- Parent: " in its metadata block. + const TASK_DOC = [ + '# Ticket: AEOS-2', + '', + '## AEOS Metadata', + '- Kind: TASK', + '- Parent: AEOS-1', + '', + '## Title', + 'Do a task', + ].join('\n'); + + beforeEach(() => { + artifactStore = createMockArtifactStore(); + projectRepo = createMockProjectRepo(); + gitGateway = createMockGitGateway(); + assembler = new ContextAssembler(artifactStore, projectRepo, gitGateway); + }); + + it("injects the parent epic's PRD and tech spec, PRD first", async () => { + (artifactStore.listArtifacts as ReturnType).mockImplementation( + (_root: string, ticketId: string) => + ticketId === 'AEOS-2' + ? ['AEOS-2-ticket.md'] + : ['AEOS-1-tech-spec.md', 'AEOS-1-prd.md', 'AEOS-1-tasks.md'], + ); + (artifactStore.readArtifact as ReturnType).mockImplementation( + (_root: string, _id: string, filename: string) => + filename === 'AEOS-2-ticket.md' + ? TASK_DOC + : filename === 'AEOS-1-prd.md' + ? '# PRD body' + : filename === 'AEOS-1-tech-spec.md' + ? '# Tech spec body' + : 'other', + ); + + const result = await assembler.assemble('AEOS-2', PROJECT_ROOT, Column.IMPLEMENTATION); + + expect(result.epicContext.map((a) => a.name)).toEqual(['AEOS-1-prd.md', 'AEOS-1-tech-spec.md']); + expect(result.epicContext[0].content).toBe('# PRD body'); + // The epic's tasks.md is NOT pulled in — only PRD + tech spec. + expect(result.epicContext.some((a) => a.name.endsWith('-tasks.md'))).toBe(false); + }); + + it('returns no epic context for an epic (no parent in metadata)', async () => { + (artifactStore.listArtifacts as ReturnType).mockReturnValue(['AEOS-1-ticket.md']); + (artifactStore.readArtifact as ReturnType).mockReturnValue( + '# Ticket: AEOS-1\n## AEOS Metadata\n- Kind: EPIC\n', + ); + + const result = await assembler.assemble('AEOS-1', PROJECT_ROOT, Column.IMPLEMENTATION); + + expect(result.epicContext).toEqual([]); + }); +}); + +describe('ContextAssembler — integration-review diff (WI-2)', () => { + let artifactStore: ReturnType; + let projectRepo: ReturnType; + let gitGateway: ReturnType; + let assembler: ContextAssembler; + + beforeEach(() => { + artifactStore = createMockArtifactStore(); + projectRepo = createMockProjectRepo(); + gitGateway = createMockGitGateway(); + (artifactStore.listArtifacts as ReturnType).mockReturnValue(['AEOS-1-ticket.md']); + (artifactStore.readArtifact as ReturnType).mockReturnValue('# Epic'); + assembler = new ContextAssembler(artifactStore, projectRepo, gitGateway); + }); + + it('diffs the epic base tag..HEAD when the base tag exists', async () => { + (gitGateway.refExists as ReturnType).mockReturnValue(true); + (gitGateway.diffRange as ReturnType).mockReturnValue('diff --git a/x b/x'); + + const result = await assembler.assemble('AEOS-1', '/root', Column.INTEGRATION_REVIEW); + + expect(gitGateway.refExists).toHaveBeenCalledWith('/root', 'aeos-base/AEOS-1'); + expect(gitGateway.diffRange).toHaveBeenCalledWith('/root', 'aeos-base/AEOS-1', 'HEAD'); + expect(result.codeDiff).toContain('diff --git'); + }); + + it('falls back to the working-tree diff when the base tag is missing', async () => { + (gitGateway.refExists as ReturnType).mockReturnValue(false); + (gitGateway.diff as ReturnType).mockReturnValue(''); + + const result = await assembler.assemble('AEOS-1', '/root', Column.INTEGRATION_REVIEW); + + expect(gitGateway.diffRange).not.toHaveBeenCalled(); + expect(result.codeDiff).toBe('No changes detected'); + }); +}); diff --git a/src/application/services/context-assembler.ts b/src/application/services/context-assembler.ts index 3b8ae89..a9570b3 100644 --- a/src/application/services/context-assembler.ts +++ b/src/application/services/context-assembler.ts @@ -3,12 +3,26 @@ import type { ArtifactStore } from '../../domain/ports/driven/artifact-store.port.js'; import type { ProjectRepository } from '../../domain/ports/driven/project-repository.port.js'; import type { GitGateway } from '../../domain/ports/driven/git-gateway.port.js'; -import type { AssembledContext } from '../../domain/model/assembled-context.js'; +import type { AssembledContext, PriorArtifact } from '../../domain/model/assembled-context.js'; import { Column } from '../../domain/model/column.js'; /** Maximum characters before a diff is truncated. */ export const MAX_DIFF_CHARS = 50_000; +/** + * The parent epic's specification artifacts a child task is assembled against. + * Suffix-matched against the parent's artifact filenames (e.g. `AEOS-1-prd.md`). + */ +const EPIC_SPEC_SUFFIXES = ['-prd.md', '-tech-spec.md'] as const; + +/** Pulls the parent epic id out of the ticket document's AEOS metadata block. */ +function parentEpicId(ticketContent: string): string | null { + // The metadata block renders "- Parent: " only for a task (see + // ticket-document.ts). Absent for an epic. + const match = ticketContent.match(/^-\s*Parent:\s*(\S+)\s*$/m); + return match ? match[1] : null; +} + export class ContextAssembler { constructor( private artifactStore: ArtifactStore, @@ -43,22 +57,64 @@ export class ContextAssembler { content: this.artifactStore.readArtifact(projectRoot, ticketId, filename), })); + // 3a. For a child task, inject the parent epic's spec (PRD, tech spec) so the + // task is built against — and does not contradict — the decisions that + // scoped it. The task's own artifact dir never contains these. + const epicContext = this.gatherEpicContext(ticketContent, projectRoot); + // 4. Read constraints const constraints = this.projectRepo.readConstraints(projectRoot); - // 5. Inject git diff for CODE_REVIEW column + // 5. Inject a git diff for the review columns. + // - CODE_REVIEW reviews the working-tree diff of one task. + // - INTEGRATION_REVIEW reviews the whole assembled feature: every task + // commit on the epic branch, i.e. the range from the epic's base tag to + // HEAD. Task work is committed by then, so a plain `git diff HEAD` would + // be empty. let codeDiff: string | null = null; if (column === Column.CODE_REVIEW) { - const rawDiff = this.gitGateway.diff(projectRoot); - if (rawDiff.trim() === '') { - codeDiff = 'No changes detected'; - } else if (rawDiff.length > MAX_DIFF_CHARS) { - codeDiff = `${rawDiff.slice(0, MAX_DIFF_CHARS)}\n\n[DIFF TRUNCATED — showing first 50,000 characters of ${rawDiff.length} total]`; - } else { - codeDiff = rawDiff; - } + codeDiff = this.clampDiff(this.gitGateway.diff(projectRoot)); + } else if (column === Column.INTEGRATION_REVIEW) { + const baseTag = `aeos-base/${ticketId}`; + const raw = this.gitGateway.refExists(projectRoot, baseTag) + ? this.gitGateway.diffRange(projectRoot, baseTag, 'HEAD') + : this.gitGateway.diff(projectRoot); + codeDiff = this.clampDiff(raw); } - return { ticketContent, settledDecisions, priorArtifacts, constraints, codeDiff }; + return { ticketContent, settledDecisions, priorArtifacts, epicContext, constraints, codeDiff }; + } + + /** Normalizes an empty diff to a marker and truncates an oversized one. */ + private clampDiff(rawDiff: string): string { + if (rawDiff.trim() === '') return 'No changes detected'; + if (rawDiff.length > MAX_DIFF_CHARS) { + return `${rawDiff.slice(0, MAX_DIFF_CHARS)}\n\n[DIFF TRUNCATED — showing first 50,000 characters of ${rawDiff.length} total]`; + } + return rawDiff; + } + + /** + * Reads the parent epic's PRD and tech-spec artifacts for a child task. + * Returns [] for an epic (no parent) or when the parent has no such artifacts. + */ + private gatherEpicContext(ticketContent: string, projectRoot: string): PriorArtifact[] { + const epicId = parentEpicId(ticketContent); + if (!epicId) return []; + + const parentFiles = this.artifactStore.listArtifacts(projectRoot, epicId); + const out: PriorArtifact[] = []; + // Preserve EPIC_SPEC_SUFFIXES order (PRD before tech spec) rather than the + // directory's order. + for (const suffix of EPIC_SPEC_SUFFIXES) { + const filename = parentFiles.find((name) => name.endsWith(suffix)); + if (filename) { + out.push({ + name: filename, + content: this.artifactStore.readArtifact(projectRoot, epicId, filename), + }); + } + } + return out; } } diff --git a/src/application/services/escalation-document.test.ts b/src/application/services/escalation-document.test.ts new file mode 100644 index 0000000..4172344 --- /dev/null +++ b/src/application/services/escalation-document.test.ts @@ -0,0 +1,63 @@ +import { describe, it, expect } from 'vitest'; + +import { + buildEscalationDocument, + parseEscalationResponse, + buildResolutionDocument, + escalationFilename, +} from './escalation-document.js'; + +describe('escalation-document', () => { + it('builds a document with an empty response block and a resolve hint', () => { + const doc = buildEscalationDocument({ + ticketId: 'STAN-4', + column: 'CODE_REVIEW', + reason: 'NOT_CONVERGING', + message: 'Attempt 2 reproduced the same blockers.', + artifactPath: 'STAN-4-code-review-review.md', + }); + + expect(doc).toContain('Reason: NOT_CONVERGING'); + expect(doc).toContain('Attempt 2 reproduced the same blockers.'); + expect(doc).toContain('See: STAN-4-code-review-review.md'); + expect(doc).toContain('aeos ticket resolve STAN-4'); + // Round-trips to null: nothing written yet. + expect(parseEscalationResponse(doc)).toBeNull(); + }); + + it('parses the operator response, ignoring guidance comments', () => { + const doc = buildEscalationDocument({ + ticketId: 'STAN-4', + column: 'CODE_REVIEW', + reason: 'NOT_CONVERGING', + message: 'stuck', + }); + const filled = doc.replace( + /```text\n\n```/, + '```text\nUse response_url per D-6, not chat.update.\n```', + ); + + expect(parseEscalationResponse(filled)).toBe('Use response_url per D-6, not chat.update.'); + }); + + it('returns null when the response block is only whitespace', () => { + const doc = buildEscalationDocument({ + ticketId: 'X-1', + column: 'QA', + reason: 'ITERATIONS_EXHAUSTED', + message: 'm', + }); + expect(parseEscalationResponse(doc.replace(/```text\n\n```/, '```text\n \n```'))).toBeNull(); + }); + + it('names the artifact file', () => { + expect(escalationFilename('STAN-4')).toBe('STAN-4-escalation.md'); + }); + + it('renders the resolution as authoritative guidance', () => { + const res = buildResolutionDocument('STAN-4', 'NOT_CONVERGING', 'do X'); + expect(res).toContain('Operator Resolution: STAN-4'); + expect(res).toContain('do X'); + expect(res).toContain('overrides earlier assumptions'); + }); +}); diff --git a/src/application/services/escalation-document.ts b/src/application/services/escalation-document.ts new file mode 100644 index 0000000..6c3f74f --- /dev/null +++ b/src/application/services/escalation-document.ts @@ -0,0 +1,89 @@ +// Application service — the human-facing escalation artifact and its response. +// +// Generalizes the preflight `questions.md` round-trip to every escalation: a run +// that escalates writes `-escalation.md` with the reason and a fenced +// `## Response` block; the operator writes their decision there and runs +// `aeos ticket resolve `, which folds the response back as context and +// resumes the ticket. + +export const ESCALATION_FILENAME_SUFFIX = '-escalation.md'; + +/** The filename of a ticket's escalation artifact. */ +export function escalationFilename(ticketId: string): string { + return `${ticketId}${ESCALATION_FILENAME_SUFFIX}`; +} + +export interface EscalationDocumentInput { + ticketId: string; + column: string; + reason: string; + message: string; + artifactPath?: string | null; +} + +/** Builds the initial `-escalation.md` with an empty response block. */ +export function buildEscalationDocument(input: EscalationDocumentInput): string { + const { ticketId, column, reason, message, artifactPath } = input; + const lines = [ + '# AEOS Escalation', + 'Format-Version: 1', + `Ticket: ${ticketId}`, + `Column: ${column}`, + `Reason: ${reason}`, + '', + '## Why', + message, + '', + ]; + if (artifactPath) { + lines.push(`See: ${artifactPath}`, ''); + } + lines.push( + '## Response', + '', + '', + '```text', + '', + '```', + '', + ); + return lines.join('\n'); +} + +/** + * Extracts the operator's response from the fenced block under `## Response`. + * Returns null when the block is absent or empty (nothing to resolve). + */ +export function parseEscalationResponse(content: string): string | null { + const responseIdx = content.indexOf('## Response'); + if (responseIdx === -1) return null; + + const after = content.slice(responseIdx); + const fence = after.match(/```(?:text)?\n([\s\S]*?)```/); + if (!fence) return null; + + const body = fence[1] + // Drop the HTML comment guidance lines if they landed inside the fence. + .replace(//g, '') + .trim(); + return body.length > 0 ? body : null; +} + +/** Renders the operator's resolution as a context artifact for the retry. */ +export function buildResolutionDocument( + ticketId: string, + reason: string, + response: string, +): string { + return [ + `# Operator Resolution: ${ticketId}`, + '', + `The prior run escalated (${reason}). A human resolved it with the following`, + 'authoritative guidance. Honor it in this run — it overrides earlier assumptions', + 'where they conflict.', + '', + '## Guidance', + response, + '', + ].join('\n'); +} diff --git a/src/application/services/preflight.test.ts b/src/application/services/preflight.test.ts index 67cad1e..67837ed 100644 --- a/src/application/services/preflight.test.ts +++ b/src/application/services/preflight.test.ts @@ -54,6 +54,7 @@ function makeContext(overrides: Partial = {}): AssembledContex ticketContent: '# Build a login page\nUsers should be able to log in with email and password.', settledDecisions: '# AEOS Decisions\nUse email/password auth only.', priorArtifacts: [{ name: 'AEOS-1-prd.md', content: '# PRD' }], + epicContext: [], constraints: 'Use TypeScript strict mode', codeDiff: null, ...overrides, diff --git a/src/application/services/prompt-builder.test.ts b/src/application/services/prompt-builder.test.ts index a900ff7..bafad53 100644 --- a/src/application/services/prompt-builder.test.ts +++ b/src/application/services/prompt-builder.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import { buildPrompt } from './prompt-builder.js'; +import { buildPrompt, buildContextSection } from './prompt-builder.js'; import type { AssembledContext } from '../../domain/model/assembled-context.js'; import type { AgentSpec } from '../../domain/model/agent-spec.js'; @@ -20,6 +20,7 @@ function createContext(overrides: Partial = {}): AssembledCont ticketContent: '# AEOS-1\nImplement feature X', settledDecisions: null, priorArtifacts: [], + epicContext: [], constraints: null, codeDiff: null, ...overrides, @@ -201,3 +202,24 @@ describe('buildPrompt', () => { expect(result).toContain('[DIFF TRUNCATED'); }); }); + +describe('buildContextSection — epic specification (WI-3)', () => { + it('renders epic context as an authoritative, do-not-contradict block before the ticket', () => { + const section = buildContextSection( + createContext({ + epicContext: [{ name: 'AEOS-1-tech-spec.md', content: 'D-6: use response_url' }], + }), + ); + + expect(section).toContain('## Epic Specification'); + expect(section).toContain('do not contradict'); + expect(section).toContain('D-6: use response_url'); + // It precedes the ticket block. + expect(section.indexOf('## Epic Specification')).toBeLessThan(section.indexOf('## Ticket')); + }); + + it('omits the epic block entirely when there is no epic context', () => { + const section = buildContextSection(createContext({ epicContext: [] })); + expect(section).not.toContain('## Epic Specification'); + }); +}); diff --git a/src/application/services/prompt-builder.ts b/src/application/services/prompt-builder.ts index a890136..ca48bb5 100644 --- a/src/application/services/prompt-builder.ts +++ b/src/application/services/prompt-builder.ts @@ -40,6 +40,18 @@ export function buildContextSection(context: AssembledContext): string { parts.push('[CONTEXT]'); + // Epic Specification — the parent epic's PRD/tech spec for a child task. + // Placed first and framed as authoritative: the task must implement against + // these decisions and must not contradict them. + if (context.epicContext.length > 0) { + const epicParts = context.epicContext + .map((artifact) => `### ${artifact.name}\n${artifact.content}`) + .join('\n\n'); + parts.push( + `## Epic Specification (authoritative — do not contradict its decisions)\n${epicParts}`, + ); + } + // Ticket parts.push(`## Ticket\n${context.ticketContent}`); diff --git a/src/application/ticket-answer.use-case.test.ts b/src/application/ticket-answer.use-case.test.ts index 320719e..097933b 100644 --- a/src/application/ticket-answer.use-case.test.ts +++ b/src/application/ticket-answer.use-case.test.ts @@ -40,6 +40,13 @@ function createMockGitGateway(): GitGateway { init: vi.fn(), commit: vi.fn(), commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), diff: vi.fn().mockReturnValue(''), }; } diff --git a/src/application/ticket-approve.use-case.test.ts b/src/application/ticket-approve.use-case.test.ts index deeb0dd..a5f9f89 100644 --- a/src/application/ticket-approve.use-case.test.ts +++ b/src/application/ticket-approve.use-case.test.ts @@ -28,6 +28,13 @@ function createMockGitGateway(): GitGateway { init: vi.fn(), commit: vi.fn(), commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), diff: vi.fn().mockReturnValue(''), }; } @@ -81,7 +88,7 @@ describe('TicketApproveUseCase', () => { artifactStore = createMockArtifactStore(); gitGateway = createMockGitGateway(); stateMachine = createMockStateMachine(); - useCase = new TicketApproveUseCase(ticketRepo, artifactStore, stateMachine); + useCase = new TicketApproveUseCase(ticketRepo, artifactStore, stateMachine, gitGateway); (ticketRepo.findById as ReturnType).mockReturnValue(signedOffTicket()); }); @@ -233,7 +240,7 @@ describe('TicketApproveUseCase', () => { expect(stateMachine.transition).not.toHaveBeenCalled(); }); - it('advances the epic to DOD_GATE once every task is DONE', () => { + it('advances the epic to INTEGRATION_REVIEW once every task is DONE', () => { (ticketRepo.findById as ReturnType).mockReturnValue(epicInBreakdown()); (ticketRepo.findChildren as ReturnType).mockReturnValue([ child('AEOS-2', 'DONE'), @@ -242,7 +249,7 @@ describe('TicketApproveUseCase', () => { const result = useCase.execute(PROJECT_ID, PROJECT_PATH, TICKET_ID); - expect(result).toMatchObject({ status: 'advanced', toColumn: 'DOD_GATE' }); + expect(result).toMatchObject({ status: 'advanced', toColumn: 'INTEGRATION_REVIEW' }); }); it('advances an epic that decomposed into no tasks', () => { @@ -251,7 +258,7 @@ describe('TicketApproveUseCase', () => { expect(useCase.execute(PROJECT_ID, PROJECT_PATH, TICKET_ID)).toMatchObject({ status: 'advanced', - toColumn: 'DOD_GATE', + toColumn: 'INTEGRATION_REVIEW', }); }); @@ -271,3 +278,48 @@ describe('TicketApproveUseCase', () => { }); }); }); + +describe('TicketApproveUseCase — per-task commit', () => { + let ticketRepo: ReturnType; + let artifactStore: ReturnType; + let gitGateway: ReturnType; + let stateMachine: ReturnType; + let useCase: TicketApproveUseCase; + + function task(column: Ticket['column']): Ticket { + return { ...signedOffTicket(column), kind: 'TASK', parentId: 'AEOS-9' }; + } + + beforeEach(() => { + ticketRepo = createMockTicketRepo(); + artifactStore = createMockArtifactStore(); + gitGateway = createMockGitGateway(); + stateMachine = createMockStateMachine(); + useCase = new TicketApproveUseCase(ticketRepo, artifactStore, stateMachine, gitGateway); + }); + + it('commits the source repo when a TASK reaches DONE', () => { + (ticketRepo.findById as ReturnType).mockReturnValue(task('QA')); + + const result = useCase.execute(PROJECT_ID, PROJECT_PATH, TICKET_ID); + + expect(result).toMatchObject({ status: 'advanced', toColumn: 'DONE' }); + expect(gitGateway.commitAll).toHaveBeenCalledWith(PROJECT_PATH, '[AEOS-1] Test ticket'); + }); + + it('does not commit on a mid-pipeline advance — CODE_REVIEW still needs the diff', () => { + (ticketRepo.findById as ReturnType).mockReturnValue(task('IMPLEMENTATION')); + + useCase.execute(PROJECT_ID, PROJECT_PATH, TICKET_ID); + + expect(gitGateway.commitAll).not.toHaveBeenCalled(); + }); + + it('does not commit for an epic — epics produce artifacts, not code', () => { + (ticketRepo.findById as ReturnType).mockReturnValue(signedOffTicket('DOD_GATE')); + + useCase.execute(PROJECT_ID, PROJECT_PATH, TICKET_ID); + + expect(gitGateway.commitAll).not.toHaveBeenCalled(); + }); +}); diff --git a/src/application/ticket-approve.use-case.ts b/src/application/ticket-approve.use-case.ts index 36c2988..1c5f655 100644 --- a/src/application/ticket-approve.use-case.ts +++ b/src/application/ticket-approve.use-case.ts @@ -4,6 +4,7 @@ import { Column, nextColumnFor } from '../domain/model/column.js'; import { SubState } from '../domain/model/sub-state.js'; import { TicketKind } from '../domain/model/ticket-kind.js'; import type { ArtifactStore } from '../domain/ports/driven/artifact-store.port.js'; +import type { GitGateway } from '../domain/ports/driven/git-gateway.port.js'; import type { TicketRepository } from '../domain/ports/driven/ticket-repository.port.js'; import type { StateMachineService } from '../domain/services/state-machine.js'; import type { @@ -17,6 +18,7 @@ export class TicketApproveUseCase implements TicketApprovePort { private readonly ticketRepo: TicketRepository, private readonly artifactStore: ArtifactStore, private readonly stateMachine: StateMachineService, + private readonly gitGateway: GitGateway, ) {} execute(projectId: string, projectPath: string, ticketId: string): TicketApproveResult { @@ -94,6 +96,15 @@ export class TicketApproveUseCase implements TicketApprovePort { subState: SubState.READY, }); + // A finished task's work becomes one commit in the source repo. This is the + // only point it is safe to commit: CODE_REVIEW reads `git diff HEAD`, so + // committing any earlier would leave the reviewer with nothing to review. + // One commit per task also keeps the next task's diff free of this one's + // changes. Epics are skipped — they produce artifacts, not code. + if (nextColumn === Column.DONE && ticket.kind === TicketKind.TASK) { + this.gitGateway.commitAll(projectPath, `[${ticketId}] ${ticket.title}`); + } + return { status: 'advanced', ticketId, fromColumn: currentColumn, toColumn: nextColumn }; } } diff --git a/src/application/ticket-create.use-case.test.ts b/src/application/ticket-create.use-case.test.ts index df24008..53f67f0 100644 --- a/src/application/ticket-create.use-case.test.ts +++ b/src/application/ticket-create.use-case.test.ts @@ -44,6 +44,13 @@ function createMockGitGateway(): GitGateway { init: vi.fn(), commit: vi.fn(), commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), diff: vi.fn().mockReturnValue(''), }; } diff --git a/src/application/ticket-dod-approve.use-case.test.ts b/src/application/ticket-dod-approve.use-case.test.ts index e29dddf..210b5d6 100644 --- a/src/application/ticket-dod-approve.use-case.test.ts +++ b/src/application/ticket-dod-approve.use-case.test.ts @@ -29,6 +29,13 @@ function createMockGitGateway(): GitGateway { init: vi.fn(), commit: vi.fn(), commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), diff: vi.fn().mockReturnValue(''), }; } diff --git a/src/application/ticket-move.use-case.test.ts b/src/application/ticket-move.use-case.test.ts index ce2001d..5b4f4d3 100644 --- a/src/application/ticket-move.use-case.test.ts +++ b/src/application/ticket-move.use-case.test.ts @@ -28,6 +28,13 @@ function createMockGitGateway(): GitGateway { init: vi.fn(), commit: vi.fn(), commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), diff: vi.fn().mockReturnValue(''), }; } diff --git a/src/application/ticket-ready.use-case.test.ts b/src/application/ticket-ready.use-case.test.ts index a89097c..de12c17 100644 --- a/src/application/ticket-ready.use-case.test.ts +++ b/src/application/ticket-ready.use-case.test.ts @@ -33,7 +33,19 @@ function createMockArtifactStore(): ArtifactStore { }; } function createMockGitGateway(): GitGateway { - return { init: vi.fn(), commit: vi.fn(), commitFiles: vi.fn(), diff: vi.fn() }; + return { + init: vi.fn(), + commit: vi.fn(), + commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), + diff: vi.fn(), + }; } function createMockStateMachine() { const mock: Pick = { diff --git a/src/application/ticket-resolve.use-case.test.ts b/src/application/ticket-resolve.use-case.test.ts new file mode 100644 index 0000000..c9f8057 --- /dev/null +++ b/src/application/ticket-resolve.use-case.test.ts @@ -0,0 +1,152 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { TicketResolveUseCase } from './ticket-resolve.use-case.js'; +import { buildEscalationDocument } from './services/escalation-document.js'; +import type { TicketRepository } from '../domain/ports/driven/ticket-repository.port.js'; +import type { ArtifactStore } from '../domain/ports/driven/artifact-store.port.js'; +import type { GitGateway } from '../domain/ports/driven/git-gateway.port.js'; +import type { StateMachineService } from '../domain/services/state-machine.js'; +import type { Ticket } from '../domain/model/ticket.js'; + +const PROJECT_ID = 'p'; +const PROJECT_PATH = '/proj'; +const TICKET_ID = 'STAN-4'; + +function mockArtifactStore(): ArtifactStore { + return { + artifactExists: vi.fn().mockReturnValue(true), + readArtifact: vi.fn().mockReturnValue(''), + getArtifactMtime: vi.fn().mockReturnValue(new Date('2030-01-01T00:00:00Z')), + writeArtifact: vi.fn(), + removeArtifact: vi.fn(), + listArtifacts: vi.fn().mockReturnValue([]), + }; +} + +function mockGit(): GitGateway { + return { + init: vi.fn(), + commit: vi.fn(), + commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), + diff: vi.fn().mockReturnValue(''), + }; +} + +function mockRepo(ticket: Ticket | null): TicketRepository { + return { + nextId: vi.fn(), + save: vi.fn(), + createAtomic: vi.fn(), + deleteById: vi.fn(), + findById: vi.fn().mockReturnValue(ticket), + findByProject: vi.fn().mockReturnValue([]), + findChildren: vi.fn().mockReturnValue([]), + updateColumn: vi.fn(), + updateSubState: vi.fn(), + setEscalation: vi.fn(), + }; +} + +function mockStateMachine(): StateMachineService { + return { + transition: vi.fn().mockReturnValue({ ok: true }), + setSubState: vi.fn().mockReturnValue({ ok: true }), + } as unknown as StateMachineService; +} + +function escalatedTicket(overrides: Partial = {}): Ticket { + return { + id: TICKET_ID, + projectId: PROJECT_ID, + title: 'A task', + kind: 'TASK', + parentId: 'STAN-1', + column: 'CODE_REVIEW', + subState: 'ESCALATED', + createdAt: '2020-01-01T00:00:00Z', + updatedAt: '2020-01-01T00:00:00Z', + escalation: { reason: 'NOT_CONVERGING', message: 'stuck', artifactPath: null }, + ...overrides, + }; +} + +const filledEscalation = buildEscalationDocument({ + ticketId: TICKET_ID, + column: 'CODE_REVIEW', + reason: 'NOT_CONVERGING', + message: 'stuck', +}).replace(/```text\n\n```/, '```text\nUse response_url per D-6.\n```'); + +describe('TicketResolveUseCase', () => { + let artifactStore: ReturnType; + let git: ReturnType; + let stateMachine: StateMachineService; + + beforeEach(() => { + artifactStore = mockArtifactStore(); + git = mockGit(); + stateMachine = mockStateMachine(); + }); + + function run(ticket: Ticket | null) { + const useCase = new TicketResolveUseCase(mockRepo(ticket), artifactStore, stateMachine, git); + return useCase.execute({ + projectId: PROJECT_ID, + projectPath: PROJECT_PATH, + ticketId: TICKET_ID, + }); + } + + it('rejects a ticket that is not escalated', () => { + const result = run(escalatedTicket({ subState: 'READY' })); + expect(result).toMatchObject({ ok: false }); + if (!result.ok && 'error' in result) expect(result.error).toContain('not escalated'); + }); + + it('writes the response as a resolution artifact and sets READY', () => { + (artifactStore.readArtifact as ReturnType).mockReturnValue(filledEscalation); + + const result = run(escalatedTicket()); + + expect(result).toEqual({ ok: true, ticketId: TICKET_ID }); + // Resolution artifact captured the operator's guidance. + expect(artifactStore.writeArtifact).toHaveBeenCalledWith( + PROJECT_PATH, + TICKET_ID, + 'STAN-4-resolution.md', + expect.stringContaining('Use response_url per D-6.'), + ); + // Sub-state returned to READY (which also clears the stored escalation). + expect(stateMachine.setSubState).toHaveBeenCalledWith(PROJECT_ID, TICKET_ID, 'READY'); + expect(git.commitFiles).toHaveBeenCalled(); + }); + + it('errors when the response block is empty', () => { + (artifactStore.readArtifact as ReturnType).mockReturnValue( + buildEscalationDocument({ ticketId: TICKET_ID, column: 'QA', reason: 'X', message: 'm' }), + ); + + const result = run(escalatedTicket()); + + expect(result).toMatchObject({ ok: false }); + if (!result.ok && 'error' in result) expect(result.error).toContain('No response found'); + expect(stateMachine.setSubState).not.toHaveBeenCalled(); + }); + + it('asks for confirmation when the file was not modified since escalation', () => { + (artifactStore.readArtifact as ReturnType).mockReturnValue(filledEscalation); + (artifactStore.getArtifactMtime as ReturnType).mockReturnValue( + new Date('2019-01-01T00:00:00Z'), // older than ticket.updatedAt + ); + + const result = run(escalatedTicket({ updatedAt: '2020-06-01T00:00:00Z' })); + + expect(result).toMatchObject({ ok: false, needsConfirmation: true }); + }); +}); diff --git a/src/application/ticket-resolve.use-case.ts b/src/application/ticket-resolve.use-case.ts new file mode 100644 index 0000000..531d0ff --- /dev/null +++ b/src/application/ticket-resolve.use-case.ts @@ -0,0 +1,111 @@ +// Use case — TicketResolve (unblock an ESCALATED ticket from its escalation.md) + +import * as path from 'node:path'; +import type { TicketRepository } from '../domain/ports/driven/ticket-repository.port.js'; +import type { ArtifactStore } from '../domain/ports/driven/artifact-store.port.js'; +import type { GitGateway } from '../domain/ports/driven/git-gateway.port.js'; +import type { StateMachineService } from '../domain/services/state-machine.js'; +import type { + TicketResolvePort, + TicketResolveInput, + TicketResolveResult, +} from '../domain/ports/driving/ticket-resolve.port.js'; +import { SubState } from '../domain/model/sub-state.js'; +import { syncTicketDocument } from './services/ticket-document.js'; +import { + escalationFilename, + parseEscalationResponse, + buildResolutionDocument, +} from './services/escalation-document.js'; + +/** The artifact the operator's guidance is written to, picked up as prior context. */ +function resolutionFilename(ticketId: string): string { + return `${ticketId}-resolution.md`; +} + +export class TicketResolveUseCase implements TicketResolvePort { + constructor( + private readonly ticketRepo: TicketRepository, + private readonly artifactStore: ArtifactStore, + private readonly stateMachine: StateMachineService, + private readonly gitGateway: GitGateway, + ) {} + + execute(input: TicketResolveInput): TicketResolveResult { + const { projectId, projectPath, ticketId, confirmed } = input; + + const ticket = this.ticketRepo.findById(projectId, ticketId); + if (!ticket) { + return { ok: false, error: `Ticket ${ticketId} not found` }; + } + + // Only ESCALATED tickets carry an escalation.md. (Preflight BLOCKED uses + // `aeos ticket answer` and questions.md instead.) + if (ticket.subState !== SubState.ESCALATED) { + return { + ok: false, + error: `Ticket ${ticketId} is not escalated (current state: ${ticket.subState ?? 'none'}). Only escalated tickets can be resolved; a preflight-blocked ticket uses \`aeos ticket answer\`.`, + }; + } + + const filename = escalationFilename(ticketId); + if (!this.artifactStore.artifactExists(projectPath, ticketId, filename)) { + return { ok: false, error: `Escalation file not found: ${filename}` }; + } + + // Guard against resolving a file the operator never edited. + if (!confirmed) { + const mtime = this.artifactStore.getArtifactMtime(projectPath, ticketId, filename); + const ticketUpdatedAt = new Date(ticket.updatedAt).getTime(); + const fileMtime = mtime ? mtime.getTime() : 0; + if (fileMtime <= ticketUpdatedAt) { + return { ok: false, needsConfirmation: true, reason: 'escalation file not modified' }; + } + } + + const content = this.artifactStore.readArtifact(projectPath, ticketId, filename); + const response = parseEscalationResponse(content); + if (response === null) { + return { + ok: false, + error: `No response found in ${filename}. Write your decision in the \`## Response\` block, then re-run.`, + }; + } + + // Persist the operator's guidance as a context artifact for the retry. + const reason = ticket.escalation?.reason ?? 'ESCALATED'; + this.artifactStore.writeArtifact( + projectPath, + ticketId, + resolutionFilename(ticketId), + buildResolutionDocument(ticketId, reason, response), + ); + + // READY (not ESCALATED/BLOCKED) — this also clears the stored escalation. + const result = this.stateMachine.setSubState(projectId, ticketId, SubState.READY); + if (!result.ok) { + return { ok: false, error: `Failed to set sub-state: ${result.reason}` }; + } + syncTicketDocument(this.artifactStore, projectPath, { ...ticket, subState: SubState.READY }); + + // Commit the resolution + escalation artifacts (state stays in SQLite). + const aeosDir = path.join(projectPath, '.aeos'); + try { + this.gitGateway.commitFiles( + aeosDir, + [ + path.join(aeosDir, 'tickets', ticketId, resolutionFilename(ticketId)), + path.join(aeosDir, 'tickets', ticketId, filename), + ], + `[${ticketId}][ESCALATION][v1][human][resolved]`, + ); + } catch (err) { + // Compensate: revert to ESCALATED so the state matches the uncommitted tree. + this.stateMachine.setSubState(projectId, ticketId, SubState.ESCALATED); + syncTicketDocument(this.artifactStore, projectPath, ticket); + throw err; + } + + return { ok: true, ticketId }; + } +} diff --git a/src/application/ticket-run.use-case.test.ts b/src/application/ticket-run.use-case.test.ts index 2946c9f..4e23079 100644 --- a/src/application/ticket-run.use-case.test.ts +++ b/src/application/ticket-run.use-case.test.ts @@ -54,6 +54,13 @@ function createMockGitGateway(): GitGateway { init: vi.fn(), commit: vi.fn(), commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), diff: vi.fn().mockReturnValue('diff --git a/src/file.ts b/src/file.ts'), }; } @@ -204,6 +211,7 @@ function defaultContext(): AssembledContext { ticketContent: 'ticket content', settledDecisions: null, priorArtifacts: [], + epicContext: [], constraints: null, codeDiff: null, }; @@ -569,6 +577,37 @@ describe('TicketRunUseCase', () => { ); }); + it('baselines pre-existing repo changes before an agentic worker runs', async () => { + // Otherwise the accumulated tree (a previous task's work, unrelated local + // edits) lands in this ticket's diff and CODE_REVIEW rejects files the + // ticket never claimed. + await useCase.execute(PROJECT_ID, PROJECT_PATH, TICKET_ID); + + expect(gitGateway.commitAll).toHaveBeenCalledWith( + PROJECT_PATH, + expect.stringContaining('[AEOS-1][BASELINE]'), + ); + }); + + it('stages the repo before diffing so newly created files count as changes', async () => { + // `git diff HEAD` ignores untracked files, so a task that only ADDS files + // (a migration, a new module) would read as "no changes" and fail — and the + // files would reach CODE_REVIEW/QA looking untracked. + const order: string[] = []; + (gitGateway.stageAll as ReturnType).mockImplementation(() => + order.push('stageAll'), + ); + (gitGateway.diff as ReturnType).mockImplementation(() => { + order.push('diff'); + return 'diff --git a/new.sql b/new.sql'; + }); + + await useCase.execute(PROJECT_ID, PROJECT_PATH, TICKET_ID); + + expect(gitGateway.stageAll).toHaveBeenCalledWith(PROJECT_PATH); + expect(order).toEqual(['stageAll', 'diff']); + }); + it('skips the repo-diff check for an agentic column that opts out', async () => { // TASK_BREAKDOWN is agentic (it calls the CLI to create tasks) but its work // is not a repo edit, so requiresRepoDiff:false must let a no-diff run pass. diff --git a/src/application/ticket-run.use-case.ts b/src/application/ticket-run.use-case.ts index 35971b3..29ada3c 100644 --- a/src/application/ticket-run.use-case.ts +++ b/src/application/ticket-run.use-case.ts @@ -51,6 +51,7 @@ import type { ExecutorChunkObserver } from '../domain/model/executor-invocation. import type { ReviewVerdict } from '../domain/model/review-verdict.js'; import type { Escalation } from '../domain/model/escalation.js'; import { EscalationReason } from '../domain/model/escalation.js'; +import { buildEscalationDocument, escalationFilename } from './services/escalation-document.js'; import { Column } from '../domain/model/column.js'; import { SubState } from '../domain/model/sub-state.js'; import { validateOutput } from '../domain/services/output-validation.js'; @@ -493,6 +494,20 @@ export class TicketRunUseCase implements TicketRunPort { } const prompt = this.buildPromptFn(workerContext, ctx.workerAgentSpec); + // Baseline the repo before an agentic worker touches it, on the first + // attempt only. Whatever is already uncommitted — a previous task's work, + // or unrelated local edits — becomes its own commit, so `git diff HEAD` + // afterwards contains this task's changes and nothing else. Without this, + // CODE_REVIEW reviews the accumulated tree and rejects files the ticket + // never claimed ("orphan unacknowledged file"). Retries skip it, or the + // rejected attempt's own work would be baselined away mid-loop. + if (workerMode === 'agentic' && columnSpec.requiresRepoDiff !== false && attempt === 1) { + this.gitGateway.commitAll( + ctx.projectPath, + `[${ctx.ticketId}][BASELINE] pre-existing changes before ${ctx.column}`, + ); + } + const attemptLabel = attempt > 1 ? ` (attempt ${attempt})` : ''; this.emitStageEvent( emitter, @@ -581,6 +596,12 @@ export class TicketRunUseCase implements TicketRunPort { // its work is not a repo edit (TASK_BREAKDOWN creates tickets). Undefined // means true, so IMPLEMENTATION keeps the guarantee without stating it. if (workerMode === 'agentic' && columnSpec.requiresRepoDiff !== false) { + // Stage first: `git diff HEAD` does not show untracked files, so a task + // that only adds files (a migration, a new module) would read as "no + // changes" and fail this guarantee. Staging also makes the new files + // tracked, so CODE_REVIEW and QA see them as part of the change set + // instead of flagging them as untracked. + this.gitGateway.stageAll(projectPath); const repoDiff = this.gitGateway.diff(projectPath).trim(); if (repoDiff.length === 0) { const error = 'Agentic implementation produced no repository changes'; @@ -965,6 +986,25 @@ export class TicketRunUseCase implements TicketRunPort { artifactPath: escalation.artifactPath ?? null, }); + // Write the human-facing escalation artifact so the operator can respond in + // a file and `aeos ticket resolve`, mirroring the preflight questions flow. + // The BLOCKED (preflight) path already has its own questions.md, so only the + // true ESCALATED path gets an escalation.md. + if (subState === SubState.ESCALATED) { + this.artifactStore.writeArtifact( + ctx.projectPath, + ctx.ticketId, + escalationFilename(ctx.ticketId), + buildEscalationDocument({ + ticketId: ctx.ticketId, + column: ctx.column, + reason: escalation.reason, + message: escalation.message, + artifactPath: escalation.artifactPath ?? null, + }), + ); + } + ctx.emitter.emit({ type: 'ticket-run.escalated', phase: 'complete', diff --git a/src/application/ticket-sign-off.use-case.test.ts b/src/application/ticket-sign-off.use-case.test.ts index 9a705f5..b9125e5 100644 --- a/src/application/ticket-sign-off.use-case.test.ts +++ b/src/application/ticket-sign-off.use-case.test.ts @@ -33,7 +33,19 @@ function createMockArtifactStore(): ArtifactStore { }; } function createMockGitGateway(): GitGateway { - return { init: vi.fn(), commit: vi.fn(), commitFiles: vi.fn(), diff: vi.fn() }; + return { + init: vi.fn(), + commit: vi.fn(), + commitFiles: vi.fn(), + stageAll: vi.fn(), + commitAll: vi.fn().mockReturnValue(false), + isRepo: vi.fn().mockReturnValue(true), + ensureOnBranch: vi.fn(), + tagHere: vi.fn(), + diffRange: vi.fn().mockReturnValue(''), + refExists: vi.fn().mockReturnValue(false), + diff: vi.fn(), + }; } function createMockStateMachine() { const mock: Pick = { diff --git a/src/cli/commands/ticket-resolve.command.ts b/src/cli/commands/ticket-resolve.command.ts new file mode 100644 index 0000000..e367eff --- /dev/null +++ b/src/cli/commands/ticket-resolve.command.ts @@ -0,0 +1,82 @@ +// CLI command — aeos ticket resolve + +import * as readline from 'node:readline'; +import type { Command } from 'commander'; +import type { TicketResolvePort } from '../../domain/ports/driving/ticket-resolve.port.js'; +import type { ProjectRepository } from '../../domain/ports/driven/project-repository.port.js'; + +function askConfirmation(question: string): Promise { + const rl = readline.createInterface({ input: process.stdin, output: process.stdout }); + return new Promise((resolve) => { + rl.question(question, (answer) => { + rl.close(); + resolve(answer.toLowerCase() === 'y'); + }); + }); +} + +export function registerTicketResolveCommand( + program: Command, + getTicketResolveUseCase: () => TicketResolvePort, + projectRepo: ProjectRepository, +): void { + const ticketCmd = + program.commands.find((c) => c.name() === 'ticket') ?? + program.command('ticket').description('Ticket management commands'); + + ticketCmd + .command('resolve') + .description('Resume an ESCALATED ticket after writing a decision in its escalation.md') + .argument('', 'Ticket ID (e.g. AEOS-1)') + .action(async (ticketId: string) => { + try { + const projectPath = projectRepo.findRoot(process.cwd()); + const successMsg = `✓ Ticket ${ticketId} resolved — sub-state set to READY; your guidance will be injected on the next run`; + + if (!projectPath) { + // eslint-disable-next-line no-console + console.error('Error: No AEOS project found. Run "aeos project init" first.'); + process.exitCode = 1; + return; + } + + const project = projectRepo.read(projectPath); + const run = (confirmed?: boolean) => + getTicketResolveUseCase().execute({ + projectId: project.id, + projectPath, + ticketId, + confirmed, + }); + + let result = run(); + + if (!result.ok && 'needsConfirmation' in result && result.needsConfirmation) { + const confirmed = await askConfirmation( + 'Warning: escalation file does not appear to have been modified. Continue anyway? [y/N] ', + ); + if (!confirmed) { + // eslint-disable-next-line no-console + console.error('Aborted.'); + return; + } + result = run(true); + } + + if (!result.ok) { + // eslint-disable-next-line no-console + console.error(`Error: ${'error' in result ? result.error : 'Unknown error'}`); + process.exitCode = 1; + return; + } + + // eslint-disable-next-line no-console + console.log(successMsg); + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + // eslint-disable-next-line no-console + console.error(`Error: ${message}`); + process.exitCode = 1; + } + }); +} diff --git a/src/cli/commands/ticket-show.command.ts b/src/cli/commands/ticket-show.command.ts index 8267d13..6e90daa 100644 --- a/src/cli/commands/ticket-show.command.ts +++ b/src/cli/commands/ticket-show.command.ts @@ -86,6 +86,18 @@ export function registerTicketShowCommand( // eslint-disable-next-line no-console console.log(` See: ${escalation.artifactPath}`); } + // Point the operator at the file-based resolve flow. + if (subState === 'ESCALATED') { + // eslint-disable-next-line no-console + console.log( + ` Respond in ${result.ticket.id}-escalation.md, then: aeos ticket resolve ${result.ticket.id}`, + ); + } else if (subState === 'BLOCKED') { + // eslint-disable-next-line no-console + console.log( + ` Answer ${result.ticket.id}-questions.md, then: aeos ticket answer ${result.ticket.id}`, + ); + } } if (result.children.length > 0) { diff --git a/src/cli/container.ts b/src/cli/container.ts index dc3e757..61aadac 100644 --- a/src/cli/container.ts +++ b/src/cli/container.ts @@ -7,6 +7,7 @@ import type { TicketCreatePort } from '../domain/ports/driving/ticket-create.por import type { TicketListPort } from '../domain/ports/driving/ticket-list.port.js'; import type { TicketShowPort } from '../domain/ports/driving/ticket-show.port.js'; import type { TicketAnswerPort } from '../domain/ports/driving/ticket-answer.port.js'; +import type { TicketResolvePort } from '../domain/ports/driving/ticket-resolve.port.js'; import type { TicketRunPort } from '../domain/ports/driving/ticket-run.port.js'; import type { TicketApprovePort } from '../domain/ports/driving/ticket-approve.port.js'; import type { TicketSignOffPort } from '../domain/ports/driving/ticket-sign-off.port.js'; @@ -33,6 +34,7 @@ import { TicketCreateUseCase } from '../application/ticket-create.use-case.js'; import { TicketListUseCase } from '../application/ticket-list.use-case.js'; import { TicketShowUseCase } from '../application/ticket-show.use-case.js'; import { TicketAnswerUseCase } from '../application/ticket-answer.use-case.js'; +import { TicketResolveUseCase } from '../application/ticket-resolve.use-case.js'; import { TicketRunUseCase } from '../application/ticket-run.use-case.js'; import { TicketApproveUseCase } from '../application/ticket-approve.use-case.js'; import { TicketSignOffUseCase } from '../application/ticket-sign-off.use-case.js'; @@ -65,6 +67,7 @@ export interface Container { ticketList: TicketListPort; ticketShow: TicketShowPort; ticketAnswer: TicketAnswerPort; + ticketResolve: TicketResolvePort; ticketRun: TicketRunPort; ticketApprove: TicketApprovePort; ticketSignOff: TicketSignOffPort; @@ -158,6 +161,14 @@ export function createContainer(): Container { new DecisionPromotionService(artifactStore), ); }, + get ticketResolve() { + return new TicketResolveUseCase( + getTicketRepo(), + artifactStore, + getStateMachine(), + gitGateway, + ); + }, get ticketRun() { const contextAssembler = new ContextAssembler(artifactStore, projectRepo, gitGateway); const columnSpecLoader = new YamlColumnSpecLoader(); @@ -183,7 +194,12 @@ export function createContainer(): Container { ); }, get ticketApprove() { - return new TicketApproveUseCase(getTicketRepo(), artifactStore, getStateMachine()); + return new TicketApproveUseCase( + getTicketRepo(), + artifactStore, + getStateMachine(), + gitGateway, + ); }, get ticketSignOff() { return new TicketSignOffUseCase(getTicketRepo(), artifactStore, getStateMachine()); @@ -203,6 +219,7 @@ export function createContainer(): Container { configStore, this.ticketRun, this.ticketApprove, + gitGateway, ); }, get ticketDodApprove() { diff --git a/src/cli/index.ts b/src/cli/index.ts index 8479823..db01cfa 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -13,6 +13,7 @@ import { registerTicketCreateCommand } from './commands/ticket-create.command.js import { registerTicketListCommand } from './commands/ticket-list.command.js'; import { registerTicketShowCommand } from './commands/ticket-show.command.js'; import { registerTicketAnswerCommand } from './commands/ticket-answer.command.js'; +import { registerTicketResolveCommand } from './commands/ticket-resolve.command.js'; import { registerTicketRunCommand } from './commands/ticket-run.command.js'; import { registerTicketApproveCommand } from './commands/ticket-approve.command.js'; import { registerTicketSignOffCommand } from './commands/ticket-sign-off.command.js'; @@ -30,6 +31,7 @@ export { registerTicketCreateCommand } from './commands/ticket-create.command.js export { registerTicketListCommand } from './commands/ticket-list.command.js'; export { registerTicketShowCommand } from './commands/ticket-show.command.js'; export { registerTicketAnswerCommand } from './commands/ticket-answer.command.js'; +export { registerTicketResolveCommand } from './commands/ticket-resolve.command.js'; export { registerTicketRunCommand } from './commands/ticket-run.command.js'; export { registerTicketApproveCommand } from './commands/ticket-approve.command.js'; export { registerTicketSignOffCommand } from './commands/ticket-sign-off.command.js'; @@ -80,6 +82,7 @@ export function buildProgram(): Command { registerTicketListCommand(program, () => container.ticketList, container.projectRepo); registerTicketShowCommand(program, () => container.ticketShow, container.projectRepo); registerTicketAnswerCommand(program, () => container.ticketAnswer, container.projectRepo); + registerTicketResolveCommand(program, () => container.ticketResolve, container.projectRepo); registerTicketRunCommand(program, () => container.ticketRun, container.projectRepo); registerTicketApproveCommand(program, () => container.ticketApprove, container.projectRepo); registerTicketSignOffCommand(program, () => container.ticketSignOff, container.projectRepo); diff --git a/src/domain/model/assembled-context.ts b/src/domain/model/assembled-context.ts index 3cfac47..4947351 100644 --- a/src/domain/model/assembled-context.ts +++ b/src/domain/model/assembled-context.ts @@ -9,6 +9,13 @@ export interface AssembledContext { ticketContent: string; settledDecisions: string | null; priorArtifacts: PriorArtifact[]; + /** + * The parent epic's authoritative specification artifacts (PRD, tech spec) for + * a child task, so the task can be implemented against — and not contradict — + * the decisions that scoped it. Empty for an epic, or a task whose parent has + * no such artifacts. + */ + epicContext: PriorArtifact[]; constraints: string | null; codeDiff: string | null; } diff --git a/src/domain/model/column-pipelines.test.ts b/src/domain/model/column-pipelines.test.ts index 9b28a04..2fc6239 100644 --- a/src/domain/model/column-pipelines.test.ts +++ b/src/domain/model/column-pipelines.test.ts @@ -17,11 +17,15 @@ describe('column pipelines', () => { 'PRODUCT_SCOPING', 'TECH_SPEC', 'TASK_BREAKDOWN', + 'INTEGRATION_REVIEW', 'DOD_GATE', 'DONE', ]); + // The epic reviews the assembled feature (INTEGRATION_REVIEW) but never does + // the per-task build columns itself. expect(isColumnInPipeline(TicketKind.EPIC, Column.IMPLEMENTATION)).toBe(false); expect(isColumnInPipeline(TicketKind.EPIC, Column.CODE_REVIEW)).toBe(false); + expect(isColumnInPipeline(TicketKind.EPIC, Column.INTEGRATION_REVIEW)).toBe(true); }); it('routes a task straight to building — it is already specified', () => { @@ -47,9 +51,11 @@ describe('column pipelines', () => { expect(nextColumnFor(TicketKind.EPIC, Column.TECH_SPEC)).toBe(Column.TASK_BREAKDOWN); }); - it('advances an epic from TASK_BREAKDOWN straight to DOD_GATE', () => { - // The build columns belong to its children, not to the epic. - expect(nextColumnFor(TicketKind.EPIC, Column.TASK_BREAKDOWN)).toBe(Column.DOD_GATE); + it('advances an epic from TASK_BREAKDOWN to INTEGRATION_REVIEW, then DOD_GATE', () => { + // The build columns belong to its children; the epic reviews the whole + // assembled feature before the human DoD gate. + expect(nextColumnFor(TicketKind.EPIC, Column.TASK_BREAKDOWN)).toBe(Column.INTEGRATION_REVIEW); + expect(nextColumnFor(TicketKind.EPIC, Column.INTEGRATION_REVIEW)).toBe(Column.DOD_GATE); }); it('advances a task from BACKLOG straight to IMPLEMENTATION', () => { diff --git a/src/domain/model/column.test.ts b/src/domain/model/column.test.ts index e19e1ba..cedd9d1 100644 --- a/src/domain/model/column.test.ts +++ b/src/domain/model/column.test.ts @@ -6,16 +6,16 @@ describe('Column', () => { expect(Column.BACKLOG).toBe('BACKLOG'); }); - it('all 9 column values are distinct strings', () => { + it('all 10 column values are distinct strings', () => { const values = Object.values(Column); - expect(values).toHaveLength(9); - expect(new Set(values).size).toBe(9); + expect(values).toHaveLength(10); + expect(new Set(values).size).toBe(10); }); }); describe('COLUMN_ORDER', () => { - it('contains all 9 columns in correct pipeline order', () => { - expect(COLUMN_ORDER).toHaveLength(9); + it('contains all 10 columns in correct pipeline order', () => { + expect(COLUMN_ORDER).toHaveLength(10); expect([...COLUMN_ORDER]).toEqual([ 'BACKLOG', 'PRODUCT_SCOPING', @@ -24,6 +24,7 @@ describe('COLUMN_ORDER', () => { 'IMPLEMENTATION', 'CODE_REVIEW', 'QA', + 'INTEGRATION_REVIEW', 'DOD_GATE', 'DONE', ]); diff --git a/src/domain/model/column.ts b/src/domain/model/column.ts index 78c0b06..95b7842 100644 --- a/src/domain/model/column.ts +++ b/src/domain/model/column.ts @@ -10,6 +10,7 @@ export const Column = { IMPLEMENTATION: 'IMPLEMENTATION', CODE_REVIEW: 'CODE_REVIEW', QA: 'QA', + INTEGRATION_REVIEW: 'INTEGRATION_REVIEW', DOD_GATE: 'DOD_GATE', DONE: 'DONE', } as const; @@ -31,6 +32,7 @@ export const COLUMN_ORDER = [ Column.IMPLEMENTATION, Column.CODE_REVIEW, Column.QA, + Column.INTEGRATION_REVIEW, Column.DOD_GATE, Column.DONE, ] as const; @@ -46,6 +48,9 @@ export const EPIC_COLUMN_ORDER = [ Column.PRODUCT_SCOPING, Column.TECH_SPEC, Column.TASK_BREAKDOWN, + // Once every child task is DONE, the epic reviews the assembled feature diff + // against its own PRD and tech spec before the human DoD gate. + Column.INTEGRATION_REVIEW, Column.DOD_GATE, Column.DONE, ] as const; diff --git a/src/domain/ports/driven/git-gateway.port.ts b/src/domain/ports/driven/git-gateway.port.ts index e508ef9..220902b 100644 --- a/src/domain/ports/driven/git-gateway.port.ts +++ b/src/domain/ports/driven/git-gateway.port.ts @@ -13,6 +13,49 @@ export interface GitGateway { */ commitFiles(dir: string, files: string[], message: string): void; + /** + * Stage every change in `dir` (`git add -A`) without committing. + * + * Used after an agentic implementation so newly created files become tracked: + * `git diff HEAD` ignores untracked files, so a task that only adds files + * (a migration, a new module) would otherwise look like it changed nothing, + * and downstream review/QA would see the work as "untracked". + */ + stageAll(dir: string): void; + + /** + * Stage everything in `dir` and commit it, if there is anything to commit. + * Returns true when a commit was created, false when the tree was clean. + * + * Unlike `commit`, this never creates an empty commit — it operates on a + * user's source repository, where empty commits are noise. + */ + commitAll(dir: string, message: string): boolean; + /** Return the output of `git diff HEAD` in the given directory. */ diff(dir: string): string; + + /** True when `dir` is inside a git working tree. */ + isRepo(dir: string): boolean; + + /** + * Ensures the working tree is on branch `name`, creating it from the current + * HEAD if it does not exist. Preserves uncommitted changes. No-op if already + * on it. Used to give an epic its own `aeos/` feature branch so task + * commits accumulate in isolation. + */ + ensureOnBranch(dir: string, name: string): void; + + /** + * Creates a lightweight tag at the current HEAD if it does not already exist. + * Used to mark an epic branch's base commit (`aeos-base/`) so the + * whole-feature diff can be computed later without stored state. + */ + tagHere(dir: string, name: string): void; + + /** Returns `git diff ` (e.g. a tag..HEAD range). */ + diffRange(dir: string, from: string, to: string): string; + + /** True when `ref` resolves in the repo (branch, tag, or commit). */ + refExists(dir: string, ref: string): boolean; } diff --git a/src/domain/ports/driving/ticket-resolve.port.ts b/src/domain/ports/driving/ticket-resolve.port.ts new file mode 100644 index 0000000..3838814 --- /dev/null +++ b/src/domain/ports/driving/ticket-resolve.port.ts @@ -0,0 +1,22 @@ +// Driving port — TicketResolve use case interface +// +// Resolves an ESCALATED ticket from the operator's response in +// `-escalation.md`: injects the response as context and returns the +// ticket to READY so the next run resumes with the human's guidance. + +export interface TicketResolveInput { + projectId: string; + projectPath: string; + ticketId: string; + /** Skip the "escalation file not modified since it was written" guard. */ + confirmed?: boolean; +} + +export type TicketResolveResult = + | { ok: true; ticketId: string } + | { ok: false; needsConfirmation: true; reason: string } + | { ok: false; error: string }; + +export interface TicketResolvePort { + execute(input: TicketResolveInput): TicketResolveResult; +} diff --git a/src/infrastructure/git/simple-git-gateway.adapter.ts b/src/infrastructure/git/simple-git-gateway.adapter.ts index d05d68c..0720b11 100644 --- a/src/infrastructure/git/simple-git-gateway.adapter.ts +++ b/src/infrastructure/git/simple-git-gateway.adapter.ts @@ -47,7 +47,74 @@ export class SimpleGitGateway implements GitGateway { } } + stageAll(dir: string): void { + execFileSync('git', ['add', '-A'], { cwd: dir, stdio: 'ignore' }); + } + + commitAll(dir: string, message: string): boolean { + this.stageAll(dir); + // `git diff --cached --quiet` exits 1 when something is staged. Checking + // first keeps us from creating an empty commit in a user's source repo. + try { + execFileSync('git', ['diff', '--cached', '--quiet'], { cwd: dir, stdio: 'ignore' }); + return false; // nothing staged — clean tree + } catch { + // non-zero exit means there are staged changes + } + + try { + execFileSync('git', ['commit', '-m', message], { cwd: dir, stdio: 'ignore' }); + return true; + } catch { + // A commit can still fail (e.g. a hook rejects it). Do not break the run + // over it — the changes remain staged and the pipeline continues. + return false; + } + } + diff(dir: string): string { return execFileSync('git', ['diff', 'HEAD'], { cwd: dir, encoding: 'utf-8' }); } + + isRepo(dir: string): boolean { + try { + execFileSync('git', ['rev-parse', '--is-inside-work-tree'], { cwd: dir, stdio: 'ignore' }); + return true; + } catch { + return false; + } + } + + ensureOnBranch(dir: string, name: string): void { + const current = execFileSync('git', ['rev-parse', '--abbrev-ref', 'HEAD'], { + cwd: dir, + encoding: 'utf-8', + }).trim(); + if (current === name) return; + + if (this.refExists(dir, `refs/heads/${name}`)) { + execFileSync('git', ['checkout', name], { cwd: dir, stdio: 'ignore' }); + } else { + // Create from current HEAD, carrying any uncommitted work with us. + execFileSync('git', ['checkout', '-b', name], { cwd: dir, stdio: 'ignore' }); + } + } + + tagHere(dir: string, name: string): void { + if (this.refExists(dir, `refs/tags/${name}`)) return; + execFileSync('git', ['tag', name], { cwd: dir, stdio: 'ignore' }); + } + + diffRange(dir: string, from: string, to: string): string { + return execFileSync('git', ['diff', from, to], { cwd: dir, encoding: 'utf-8' }); + } + + refExists(dir: string, ref: string): boolean { + try { + execFileSync('git', ['rev-parse', '--verify', '--quiet', ref], { cwd: dir, stdio: 'ignore' }); + return true; + } catch { + return false; + } + } } diff --git a/src/infrastructure/spec-loader/yaml-column-spec-loader.adapter.ts b/src/infrastructure/spec-loader/yaml-column-spec-loader.adapter.ts index 99c2964..8be95a0 100644 --- a/src/infrastructure/spec-loader/yaml-column-spec-loader.adapter.ts +++ b/src/infrastructure/spec-loader/yaml-column-spec-loader.adapter.ts @@ -18,6 +18,7 @@ const COLUMN_SPEC_FILENAMES: Partial> = { [Column.IMPLEMENTATION]: 'implementation', [Column.CODE_REVIEW]: 'code-review', [Column.QA]: 'qa', + [Column.INTEGRATION_REVIEW]: 'integration-review', [Column.DOD_GATE]: 'dod-gate', }; diff --git a/templates/agents/engineer-agent.yaml b/templates/agents/engineer-agent.yaml index 43780fd..dddb096 100644 --- a/templates/agents/engineer-agent.yaml +++ b/templates/agents/engineer-agent.yaml @@ -30,8 +30,26 @@ taskInstruction: | ### When running in IMPLEMENTATION + **Stay inside this task's scope.** This ticket is one task in a larger + breakdown — sibling tasks own the rest of the work. The ticket's `Touches:` + line lists the files this task may change, and its `Out of scope` / `Notes` + sections name work reserved for other tasks (often as "— T-0NN"). Treat those + as hard boundaries: + - Modify **only** the files in `Touches:` (plus files those changes strictly + force, e.g. a barrel re-export). Do not touch a file the ticket assigns to + another task, even if you believe it needs changing. + - If the task genuinely cannot be completed correctly without changing an + out-of-scope file, do **not** expand scope to "fix" it. Implement what is in + scope, and in your summary's Deviations section state exactly what + out-of-scope change is needed and which task owns it. If nothing in scope + can ship as a result, say so plainly — that routes the ticket to a human + rather than silently doing another task's work. + - Overreaching is not helpful: a sibling task will do that work, and the code + reviewer rejects out-of-scope changes as scope creep, wasting the whole + review loop (this is a common cause of non-convergence). + Implement the approved tech spec by modifying the repository directly. Deliver working code and tests, then emit a concise Markdown summary of what you changed. The implementation must: - 1. Modify only the files required to satisfy the ticket and tech spec. + 1. Modify only the files required to satisfy the ticket and tech spec, within the `Touches:` boundary above. 2. Add or update tests that verify the behavior introduced by each meaningful code change. 3. Respect existing codebase patterns unless the tech spec explicitly requires a different structure. 4. Keep the change set cohesive and minimal — avoid speculative refactors or unrelated cleanup. @@ -47,6 +65,12 @@ taskInstruction: | - Always include a rollback section. If the change is purely additive or low risk, say so explicitly and explain why rollback is straightforward. - If a new dependency would be required, do not invent it silently; call it out explicitly in the summary. - Do not claim a file was changed unless you actually changed it. + - Create new files normally — AEOS stages the whole working tree (`git add -A`) + after you finish, so new files are tracked and visible to the code reviewer + and QA. Do not run `git commit` yourself; committing is not your job, and a + commit would make the change set harder to review as a single diff. + - List every new file in the Code Changes table with Action = Create. A file + you add but never mention reads to the reviewer as an unexplained change. ### When running in CODE_REVIEW @@ -180,6 +204,8 @@ outputFormat: | [If no constraints apply or all are met: "All applicable constraints satisfied."] selfVerificationChecklist: + - The implementation is consistent with the epic's PRD and tech-spec decisions (see the "Epic Specification" context block); no settled decision (e.g. a chosen mechanism or delivery path) is silently contradicted + - Every file changed is within the ticket's `Touches:` scope; nothing reserved for another task (per `Out of scope`/`Notes`) was modified - Every implementation step references specific file paths and function/type names from the tech spec — no vague "update the module" language - The test plan covers every implementation step with at least one unit test and all edge cases from the PRD acceptance criteria - The implementation step order respects dependency direction — foundational types and interfaces come before consumers diff --git a/templates/agents/integration-reviewer-agent.yaml b/templates/agents/integration-reviewer-agent.yaml new file mode 100644 index 0000000..491ec8b --- /dev/null +++ b/templates/agents/integration-reviewer-agent.yaml @@ -0,0 +1,104 @@ +name: integration-reviewer-agent +role: worker +systemPrompt: | + You are a senior integration reviewer in an AI-assisted development pipeline. + You are the last technical check before a feature is declared done. + + The feature was built by decomposing an epic into independent tasks, each + implemented, reviewed, and QA'd in isolation. Per-task review cannot see the + assembled whole: a decision made in the epic's tech spec can be silently + dropped, a task's contribution can be lost, two tasks can disagree at their + seam, and unit tests that mock across a seam can stay green while the + integrated behaviour is broken. Your job is to catch exactly those failures. + + You review the ASSEMBLED FEATURE — the full base..HEAD diff of every task + commit on the epic branch, provided in the context as the Code Diff — against + the epic's PRD and tech spec (provided as prior artifacts / Epic + Specification). You do not rewrite code. You produce findings and a verdict. + + Principles: + - Judge the code that is actually in the diff, not what the task summaries + claim. If a spec decision's mechanism is not present in the diff, it was not + implemented — say so, and cite the file/line where it should be. + - Trace every load-bearing tech-spec decision (D-1…D-N) and PRD acceptance + criterion into the diff. A decision honoured in prose but contradicted in + code is a defect. + - Look hardest at seams: where two components meet (a caller and a real + collaborator, a producer and a store, a delivery path). Verify a real + (non-mocked) test exercises each critical seam; a green suite that mocks the + seam is false confidence, not coverage. + - Flag any behaviour a task deferred to another ("out of scope, see T-NNN") + that is absent from the assembled diff — it means nobody implemented it. + +taskInstruction: | + Read the epic PRD and tech spec, and the full feature diff in the Code Diff + section. Produce an integration review of the assembled feature. + + Do not modify the repository. Your printed output IS the artifact. + + For the review: + 1. Build a decision-trace table: every tech-spec decision (D-N) and PRD + acceptance criterion → is it realized in the diff? cite file/line, or mark + it missing/contradicted. + 2. Check each cross-component seam: is it exercised by a real, non-mocked test? + 3. Check that every deferred hand-off between tasks is actually present. + 4. Classify each finding HIGH / MEDIUM / LOW with a concrete failure scenario + and the fix. A HIGH is behaviour that diverges from a load-bearing decision + or is silently broken in production. + 5. State a bottom line: is the assembled feature faithful to the spec, or does + it need work before the DoD gate? + +outputFormat: | + # Integration Review: {epic title} + + ## 1. Summary + + [2-4 sentences: what the assembled feature does, how many task commits, and the + overall verdict — faithful to spec, or diverges.] + + ## 2. Decision & Acceptance-Criteria Trace + + | Spec ref | Decision / AC | In the diff? | Evidence (file:line) or gap | + |----------|---------------|--------------|-----------------------------| + | [e.g. D-2 / §2.1] | [deterministic score id upsert] | [✅ / ⚠️ / ❌] | [e.g. "❌ langfuse.ts:11 drops `id`"] | + + ## 3. Seam & Test-Reality Check + + | Seam | Exercised by a non-mocked test? | Notes | + |------|--------------------------------|-------| + | [e.g. feedback → real score sink] | [✅ / ❌] | [e.g. "only a mock sink asserts `id`; real sink never run"] | + + ## 4. Deferred Hand-offs + + [For each "out of scope, see T-NNN" deferral, confirm the behaviour is present + in the assembled diff, or flag it as owned by nobody.] + + ## 5. Findings + + ### HIGH + - [file:line — divergence from a load-bearing decision or a silently broken + path; concrete failure scenario; the fix.] + + [If none: "No high-severity findings."] + + ### MEDIUM + - [...] + + ### LOW + - [...] + + ## 6. Bottom Line + + [Is the assembled feature faithful to the PRD and tech spec? What, if anything, + must be fixed before the DoD gate. Be direct.] + +selfVerificationChecklist: + - Every load-bearing tech-spec decision (D-N) and PRD acceptance criterion appears in the trace table, marked present/missing/contradicted with file:line evidence + - Findings cite the actual diff, not task summaries or claims + - Each critical cross-component seam is assessed for a real (non-mocked) test + - Every deferred cross-task hand-off is confirmed present in the assembled diff or flagged as unimplemented + +executor: + type: claude-cli + model: claude-opus-4-8 + timeoutSeconds: 2340 diff --git a/templates/column-specs/integration-review.yaml b/templates/column-specs/integration-review.yaml new file mode 100644 index 0000000..eba90f2 --- /dev/null +++ b/templates/column-specs/integration-review.yaml @@ -0,0 +1,23 @@ +column: INTEGRATION_REVIEW +phase: VERIFY +# The epic's own column, reached once every child task is DONE. The worker +# (integration-reviewer) reviews the WHOLE assembled feature — the base..HEAD +# diff of the epic branch — against the epic's PRD and tech-spec decisions, then +# the standard reviewer grades that report. Artifact mode: it produces a report, +# not repo edits, so the agentic diff check does not apply. +workerAgentFile: agents/integration-reviewer-agent.yaml +reviewerAgentFile: agents/reviewer-agent.yaml +outputArtifact: integration-review.md +minWordCount: 80 +requiredSections: [] +reviewerRubrics: + - rubrics/drift/spec-fidelity.md + - rubrics/drift/intent-drift.md +maxIterations: 3 +escalation: escalate_to_human +# Manual: a human reads the integration verdict before the DoD gate, regardless +# of the reviewer's pass — this is the last check before the feature is called done. +advanceMode: manual +preflight: + enabled: false + questionsArtifact: questions.md diff --git a/templates/column-specs/task-breakdown.yaml b/templates/column-specs/task-breakdown.yaml index 4db49b7..7988f02 100644 --- a/templates/column-specs/task-breakdown.yaml +++ b/templates/column-specs/task-breakdown.yaml @@ -12,6 +12,7 @@ minWordCount: 100 requiredSections: [] reviewerRubrics: - rubrics/structure/tech-spec-structure.md + - rubrics/structure/spec-traceability.md - rubrics/drift/intent-drift.md # The breakdown is reviewed as ONE artifact, not per task. Atomicity, ordering, # and collective sufficiency are properties of the set — a per-task review diff --git a/templates/rubrics/drift/spec-fidelity.md b/templates/rubrics/drift/spec-fidelity.md new file mode 100644 index 0000000..355e6c7 --- /dev/null +++ b/templates/rubrics/drift/spec-fidelity.md @@ -0,0 +1,73 @@ +# Spec Fidelity Rubric + +Applied by the reviewer in **INTEGRATION_REVIEW** to grade the integration +review of an assembled feature. It checks that the review actually verified the +whole feature against the epic's PRD and tech spec — the check that per-task +review structurally cannot perform. + +The reviewer applies each criterion independently. Any **FAIL** blocks +advancement; **WARN** must be flagged but does not block. The epic PRD, tech +spec, and the base..HEAD feature diff are all in context. + +--- + +## 1. Decision Trace Completeness + +| Grade | Definition | +|-------|------------| +| **PASS** | The integration review traces every load-bearing tech-spec decision (`D-N`) and every PRD acceptance criterion to the assembled diff, marking each present / missing / contradicted with file:line evidence. Nothing load-bearing is left unaddressed. | +| **WARN** | The trace covers most decisions but omits one minor item, or cites evidence loosely. | +| **FAIL** | The review does not trace decisions/criteria to the diff, or asserts the feature is complete without citing where each decision is realized. A spec decision could be silently dropped and this review would not show it. | + +--- + +## 2. Evidence From the Diff, Not the Summaries + +| Grade | Definition | +|-------|------------| +| **PASS** | Findings are grounded in the actual feature diff (file:line), not in task summaries or implementation-notes claims. Where a mechanism is claimed but absent from the diff, the review flags the gap. | +| **WARN** | The review mixes diff evidence with unverified summary claims but reaches defensible conclusions. | +| **FAIL** | The review trusts task summaries over the diff — e.g. marks a decision "done" because a task said so, when the diff does not contain it. This is the exact failure that ships lost work. | + +--- + +## 3. Seam & Test-Reality Check + +| Grade | Definition | +|-------|------------| +| **PASS** | The review identifies the feature's cross-component seams and states, for each critical one, whether a real (non-mocked) test exercises it. It calls out green suites that mock the very seam a behaviour depends on. | +| **WARN** | Seams are discussed but the mocked-vs-real distinction is not made explicit. | +| **FAIL** | The review treats a passing unit-test suite as proof of integrated behaviour without examining whether the seams are actually exercised. | + +--- + +## 4. Deferred Hand-off Verification + +| Grade | Definition | +|-------|------------| +| **PASS** | For every behaviour a task deferred to another ("out of scope, see T-NNN"), the review confirms it is present in the assembled diff, or flags it as implemented by nobody. | +| **WARN** | Deferrals are noted but not all are confirmed present. | +| **FAIL** | Deferred hand-offs are ignored, so a behaviour every task assumed someone else built could be absent with no finding. | + +--- + +## Validation: Hypothetical Weak Integration Review + +**Assembled feature:** the score-`id` idempotency mechanism (`D-2`) is absent +from the diff — the real sink drops `id` — but task T-001's summary claims it was +implemented, and the unit suite mocks the sink and asserts `id` was passed. + +**Weak review excerpt:** + +> All 36 tests pass and every task reports its acceptance criteria met. D-2's +> idempotency is covered by T-001. The feature is faithful to the spec. + +| Criterion | Grade | Rationale | +|-----------|-------|-----------| +| 1. Decision Trace Completeness | **FAIL** | D-2 is asserted "covered" with no file:line evidence; the review never checks the diff for the `id` forwarding. | +| 2. Evidence From the Diff | **FAIL** | The conclusion rests on T-001's summary, not the diff — which drops `id`. | +| 3. Seam & Test-Reality Check | **FAIL** | Treats the green suite as proof, missing that the sink seam is mocked. | +| 4. Deferred Hand-off Verification | **WARN** | The T-004→T-001 deferral of id-forwarding is not verified against the diff. | + +**Result:** three FAILs — the rubric rejects an integration review that would +have let the STAN-1 defects ship. diff --git a/templates/rubrics/structure/code-structure.md b/templates/rubrics/structure/code-structure.md index 12ff35f..741125c 100644 --- a/templates/rubrics/structure/code-structure.md +++ b/templates/rubrics/structure/code-structure.md @@ -54,6 +54,23 @@ Evaluates whether the reviewer assessed that tests accompanying the code changes --- +## 4a. Integration Seam Exercised (not mocked past) + +Evaluates whether the tests exercise the **real** boundary between the changed +component and its collaborators, rather than mocking the collaborator and +asserting against the mock. Unit tests that mock the very seam a change depends +on can pass while the integrated behaviour is broken (a green test asserting +`sink.record` was called *with* an `id`, against a mock sink that the real, +`id`-dropping sink never stands in for). + +| Grade | Definition | +|-------|------------| +| **PASS** | Where the change integrates two components (a caller and a real collaborator — a sink, an adapter, a delivery path), at least one test drives the **real** collaborator end-to-end, or the review explicitly justifies why a non-mocked test is infeasible here and names what compensates. Assertions verify observable behaviour at the far side of the seam, not just that the mock was called. | +| **WARN** | The seam is covered only by mocks, and the review notes the gap but accepts it without an integration test or a stated reason. | +| **FAIL** | The change's core behaviour crosses a component boundary, every test mocks that collaborator, and the review does not flag it — so the tests would stay green if the real collaborator dropped or mishandled the data. The "false confidence" case. | + +--- + ## 5. No Dead Code or Debug Artifacts Evaluates whether the reviewer checked for dead code, debug statements, commented-out code, TODO/FIXME markers, and other artifacts that should not reach production. diff --git a/templates/rubrics/structure/spec-traceability.md b/templates/rubrics/structure/spec-traceability.md new file mode 100644 index 0000000..e6fd599 --- /dev/null +++ b/templates/rubrics/structure/spec-traceability.md @@ -0,0 +1,77 @@ +# Spec Traceability Rubric + +Applied by the reviewer in **TASK_BREAKDOWN**, after the structural rubric and +before intent drift. It checks that the task breakdown is a faithful +decomposition of the epic's **tech spec** — not just of the ticket — and that no +task silently contradicts a load-bearing spec decision. + +This rubric exists because a breakdown that drifts from the spec propagates that +drift into every child task, and each per-task review (which sees only its own +diff) cannot catch it. The breakdown is the last place the whole feature is in +view before code is written. + +The reviewer applies each criterion independently. Any **FAIL** blocks +advancement; **WARN** must be flagged but does not block. The tech spec is +supplied as a prior artifact — cite decision identifiers (e.g. `D-6`, `§4.1`) +when grading. + +--- + +## 1. Decision Coverage + +Every numbered decision and load-bearing requirement in the tech spec is realized +by at least one task. + +| Grade | Definition | +|-------|------------| +| **PASS** | Each tech-spec decision (`D-N`) and each numbered section that implies work maps to one or more tasks. The breakdown's coverage table names the decision/section for every task and leaves no spec decision unimplemented. | +| **WARN** | Coverage is largely complete but one minor decision is unmapped or only implied, without a stated reason. | +| **FAIL** | A load-bearing spec decision has no task implementing it (e.g. the spec's idempotency mechanism is never assigned to a task), so the assembled feature will be missing behavior the spec requires. | + +--- + +## 2. No Decision Contradiction + +No task contradicts a decision the tech spec settled. + +| Grade | Definition | +|-------|------------| +| **PASS** | Every task is consistent with the spec's decisions. Where a task must diverge, it is explicitly labeled a **deviation**, states which decision it overrides, and flags it for sign-off — it is not silently baked in. | +| **WARN** | A task diverges from a non-load-bearing detail without labeling it, but the divergence is minor and does not undermine a decision's intent. | +| **FAIL** | A task silently implements the opposite of a settled decision (e.g. the spec chose `response_url` + `replace_original` under `D-6`, but a task specifies `chat.update` with stored `channel`+`ts`; or the spec rejected denormalized columns under `D-9.1`, but a task adds them). Unlabeled contradiction of a load-bearing decision is always FAIL. | + +--- + +## 3. Cross-Task Dependency Integrity + +Behavior one task defers to another is actually assigned to that other task. + +| Grade | Definition | +|-------|------------| +| **PASS** | Every `Depends on:` and every "out of scope — see T-NNN" hand-off names a real sibling task whose scope genuinely includes the deferred behavior. The dependency ordering is acyclic and stated. | +| **WARN** | A dependency is named but its ordering or exact boundary is ambiguous, though the deferred behavior is assigned somewhere. | +| **FAIL** | A task defers behavior to a task that does not exist or whose scope does not include it, so the deferred behavior would be implemented by nobody (e.g. `feedback.ts` defers id-forwarding to T-001, but no task owns forwarding it in the sink). | + +--- + +## Validation: Hypothetical Drifted Breakdown + +**Tech spec decisions:** `D-2` deterministic score `id` for idempotent upsert; +`D-6` update the answer via `response_url` + `replace_original`; `D-9.1` reject +denormalized Slack columns, look up by `question_id`. + +**Drifted breakdown excerpt:** + +> - T-004: implement `feedback.ts`; compute the score `id` but forwarding it to +> the sink is out of scope here (see T-001). +> - T-003: add `slack_team_id / slack_channel_id / slack_message_ts` columns to +> support a `chat.update` fallback. + +| Criterion | Grade | Rationale | +|-----------|-------|-----------| +| 1. Decision Coverage | **WARN** | `D-2`'s upsert is referenced but forwarding is deferred to T-001 — acceptable only if T-001 actually owns it (see criterion 3). | +| 2. No Decision Contradiction | **FAIL** | T-003 adds the denormalized columns `D-9.1` rejected, and the `chat.update` fallback contradicts `D-6`'s `response_url` decision — both unlabeled. | +| 3. Cross-Task Dependency Integrity | **FAIL** | T-004 defers id-forwarding to T-001; if T-001's scope does not cover forwarding in the sink, the behavior is owned by nobody. | + +**Result:** Criteria 2 and 3 trigger FAIL — the rubric catches the exact drift +that shipped in STAN-1.