(plan) update the plan to captures more

2026-08-18 20:04:29 -07:00
parent 5a30d0ffc6
commit 139f8419bb
11 changed files with 993 additions and 1 deletions
@@ -0,0 +1,101 @@
+# T10.6 — `plan-draft` workflow + `Suspended`-approval flow
+
+| Field | Value |
+|---|---|
+| Phase | P10 — Orchestration |
+| Size | M — 1 to 3 days |
+| Status | Not started |
+| Flags | — |
+| Spec | inlined below |
+| Blocks | — |
+
+## Goal
+
+Front door (b): a free-text problem statement produces a draft
+`WorkflowDef`, parked in `Suspended` for human review, resumed through the
+identical loader front door (a) uses.
+
+## Facts (inlined — no spec read needed)
+
+- `Suspended` is an exact fit, verified against source: "a run awaiting
+  human approval or a webhook must not hold an executor slot" (T1.3), lease
+  released outright with no heartbeat while parked (T7.3).
+- `plan-draft` is itself an ordinary poimen workflow: a `Model` step
+  producing a `WorkflowDef` draft, no new kernel machinery.
+- The draft is validated against T3.9's capability registry **before** a
+  human ever sees it — an unresolved verifier/judge/model id in the draft
+  fails here, at draft time, not after approval.
+- Approval feeds the (possibly human-edited) draft through the **same**
+  `WorkflowFormat`/builder + T3.1 canonicalization as door (a) — this is
+  what keeps both doors on one IR without new proof machinery.
+
+## Steps
+
+1. Define the built-in `plan-draft` workflow: one `Model` step (problem
+   statement → draft `WorkflowDef`), one validation step (registry check via
+   T3.9).
+2. On draft ready, transition the `plan-draft` run to `Suspended`, releasing
+   its lease (T7.3).
+3. `/v1/plans/{id}/approve` (T10.4 route): load the (possibly edited) draft,
+   run it through the exact loader door (a)'s `WorkflowFormat`/builder uses,
+   canonicalize (T3.1), resume — matching T1.3's resume-from-`Suspended`
+   semantics.
+4. `/v1/plans/{id}/reject`: cancel from `Suspended` (legal transition per
+   T1.3's cancel-from-any-non-terminal rule).
+5. Test: submit a problem statement, assert `Suspended`, edit the draft,
+   approve, assert the resulting `WorkflowVersion` hash matches
+   hand-authoring the same edited YAML directly.
+
+## Acceptance
+
+- A submitted problem statement produces a run parked in `Suspended` with no
+  held executor slot — lease released, per T7.3's own test shape.
+- Approving a draft (edited or not) produces a `WorkflowVersion`
+  byte-identical to loading the same content by hand through door (a).
+
+## Verify
+
+**Harness:** stub model for the `plan-draft` step, embedded `redb`, T7.3's
+lease-release assertion pattern reused.
+
+**Integration test** — `tests/it_plan_draft_approval.rs`:
+1. `POST /v1/problems` with a free-text statement; assert the run reaches
+   `Suspended`.
+2. Assert the lease is released and no heartbeat is emitted while
+   `Suspended` — reuse T7.3's assertion (its step 8).
+3. Fetch the draft; edit one step; `POST /v1/plans/{id}/approve` with the
+   edited draft.
+4. Assert the resulting `WorkflowVersion` hash equals hashing the same
+   edited content loaded directly via `WorkflowFormat`/builder — front-door
+   equivalence, extends T3.10.
+5. `POST /v1/plans/{id2}/reject` on a second draft; assert it reaches
+   `Cancelled`, not stuck in `Suspended`.
+6. Draft referencing an unregistered verifier id; assert `plan-draft`'s
+   validation step fails **before** ever reaching `Suspended` — the human
+   never sees an unloadable plan.
+
+**Command:** `cargo test -p orchestration plan_draft_approval`
+
+**False pass:**
+- Skipping step 4's hash comparison and only checking "run resumed" — a
+  subtly different canonicalization from door (a) would still resume, just
+  diverge from what T3.1's guarantee promises.
+- Not testing reject — an approval-only test suite misses that `Suspended`
+  must also support "no."
+
+## Traps
+
+- Having `/approve` construct a `WorkflowDef` directly in the HTTP handler
+  instead of routing through the same loader as door (a) — this is exactly
+  the two-IR-drift bug T3.10 exists to prevent, reintroduced one layer up.
+- Skipping the registry validation step "since a human will catch it." The
+  whole point of validating before `Suspended` is that load errors are cheap
+  pre-human, expensive post-human.
+
+---
+
+Background (not required to do this task):
+[../INDEX.md](../INDEX.md) (customization contract ~line 19-58) ·
+[T1.3-run-executor.md](T1.3-run-executor.md) ·
+[T7.3-leases-and-fencing.md](T7.3-leases-and-fencing.md) ·
+[T3.10-front-door-equivalence.md](T3.10-front-door-equivalence.md)