From c2b965c1e3aaaed33055ff36b2bfd2e55f262fac Mon Sep 17 00:00:00 2001 From: lanmower Date: Fri, 5 Jun 2026 11:19:59 +0200 Subject: [PATCH] @ fix(chain): name EXECUTE->PLAN re-entry on reshaping discovery + prd-add upsert Two coupled gaps let a mid-EXECUTE planning event strand the chain: 1. transitions.rs already allows transition to=PLAN from any phase (only to=COMPLETE is gated), but EXECUTE instruction prose only named the back-edge for a "new unknown" as a terse trailing clause, while the Re-expand-on-discovery section pushed every discovery toward prd-add-and-stay. So when a decision *reshaped* an existing row (not added a sibling), the prose conflicted and the agent narrated "I need to re-scope" without a routed move -- stranding in EXECUTE pointed at a plan that no longer matched the work. Add an explicit Planning-event re-entry section distinguishing additive discovery (prd-add, stay) from reshaping discovery (transition to=PLAN, re-cut the cover), and name the narration urge as the tell. 2. prd::handle_add always pushed, so re-adding an existing id created a duplicate row (two pending rows, one unresolvable by intent) -- there was no re-scope path. Make handle_add upsert by id: an existing id rewrites that row in place (response {"rescoped": id}), preserving position and the dependents that name it. This is the re-scope mechanism the re-entry prose now points at. plan.rs documents the upsert/re-scope contract and that PLAN re-entry is first-class. @ --- src/orchestrator/instructions/execute.rs | 8 +++++++- src/orchestrator/instructions/plan.rs | 2 ++ src/orchestrator/prd.rs | 20 ++++++++++++++++++-- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/orchestrator/instructions/execute.rs b/src/orchestrator/instructions/execute.rs index e21c59e..6fee43a 100644 --- a/src/orchestrator/instructions/execute.rs +++ b/src/orchestrator/instructions/execute.rs @@ -28,6 +28,12 @@ While executing, you discover every possible additional case the PLAN-phase expa Noticing-to-PRD is unchanged in EXECUTE — every observation that surfaces during work converts to a PRD row this turn. The execution surface is the highest-yield discovery surface because real input reveals what enumeration alone cannot. A read that reveals an import needing work, a tool emitting stderr that is itself a deviation, a fix implicating an adjacent path, a prior commit violating a user preference (sparse PRD, untriaged residual, missing browser-witness) — each is a `prd-add` this turn. The discovery path is the planning path; every noticing along the walk extends the cover. +## Planning-event re-entry — additive discovery vs reshaping discovery + +A discovery is one of two kinds, and they take different moves. **Additive discovery** adds a sibling the original cover missed (a new corner case, an adjacent file, an extra validation): you `prd-add` it this turn and stay in EXECUTE — the slice grew, its shape did not change. **Reshaping discovery** is a planning event: a decision or directive that changes the scope, approach, or dependency shape of an existing row (or the plan as a whole) — "this CPU mirror should be a real-GL renderer", "this row's approach is wrong, it needs X instead". That is not a sibling to append; it rewrites a node the DAG already contains, so the cover itself must be re-cut. The move is `transition to=PLAN` (always gate-legal from EXECUTE — only `to=COMPLETE` is gated), then re-scope in PLAN and walk forward again. Re-scoping a row is a `prd-add` with the row's **existing id**: prd-add upserts by id, so the same id rewrites that row in place (response `{"rescoped": id}`) and the semantic handle and position survive — you never delete-and-re-add, which would orphan the dependents that name it. + +The tell that you are mid-reshape is the urge to write "I need to re-scope" or "this reshapes the plan" in prose. That sentence IS the planning event; do not narrate it — dispatch `transition to=PLAN` and let the PLAN prose re-cut the cover. Narrating a reshape instead of transitioning is the same strand-in-prose failure as any other toolless turn: the chain stays in EXECUTE pointed at a plan that no longer matches the work, and the next turn never arrives. Additive → prd-add and stay; reshaping → transition to PLAN and re-cut. + ## Maturity-first Your first emit = closure of transform. Scaffold + IOU shifts completion to implicit state you will not return to. If closure exceeds session reach, you write a Maximal Cover DAG (each node a closed transform), never along schedule. @@ -48,5 +54,5 @@ You flip mutables by dispatching `mutable-resolve` with body `{"mutable_id": "", "witness_evidence": "<…>"}`. Bare text body (just the id) is also accepted but loses the witness audit trail. Do not pass `{prd_id, witness_evidence}` with the whole envelope nested as a string — the verb accepts `id` or `prd_id` at the top level alongside `witness_evidence`. A response with `deviation_kind: prd-resolve-unknown-id` means your id did not match a PRD row; you read the `hint` field and re-dispatch with the correct id, you do not retry blind. -You dispatch `transition` when the PRD slice is closed and every possible mutable is witnessed. On new unknown, you dispatch `transition` back to PLAN. +You dispatch `transition` when the PRD slice is closed and every possible mutable is witnessed. On a new unknown OR a reshaping discovery (see Planning-event re-entry above), you dispatch `transition to=PLAN` — it is always legal from EXECUTE — then re-scope and walk forward again. "#; diff --git a/src/orchestrator/instructions/plan.rs b/src/orchestrator/instructions/plan.rs index b9b6899..a5bab57 100644 --- a/src/orchestrator/instructions/plan.rs +++ b/src/orchestrator/instructions/plan.rs @@ -37,4 +37,6 @@ Between sub-steps of PLAN — between the orient fan-out and the PRD write, betw You dispatch: `recall`, `codesearch`, `prd-add`, `mutable-add`, `mutable-resolve`, `transition`. Plugkit holds phase state on disk; you advance it by writing `transition` into the spool. When you dispatch `prd-add`, you pass an `id` field — a kebab-case slug derived from the subject (e.g. `dedupe-update-error`, `route-fastgrnn-port`). Auto-generated `item-` ids appear when you omit it; those rows cannot be referenced by intent in recall or `prd-resolve`, so the chain loses the semantic handle the next turn would have used. The id is your contract with the PRD store: every later dispatch that names the row uses the id you wrote. + +`prd-add` upserts by id. A fresh id appends a new row (`{"added": id}`); an id that already exists rewrites that row in place (`{"rescoped": id}`), preserving its position and every dependent that names it. This is the re-scope path: when you re-enter PLAN from EXECUTE on a reshaping discovery (a decision that changed a row's scope or approach), you re-`prd-add` the affected row with its existing id and the new scope — you never delete-and-re-add, which would orphan the handle. Re-entry to PLAN is a first-class move, not a failure; the cover is meant to be re-cut whenever the work reveals the old shape was wrong. "#; diff --git a/src/orchestrator/prd.rs b/src/orchestrator/prd.rs index a966a4f..034092a 100644 --- a/src/orchestrator/prd.rs +++ b/src/orchestrator/prd.rs @@ -167,13 +167,28 @@ pub fn handle_add(content: &str) -> (String, String, i32) { } else { Value::Sequence(vec![]) }; + let mut upserted = false; if let Some(seq) = doc.as_sequence_mut() { let mut new_with_id = item_map.clone(); new_with_id.insert(Value::String("id".to_string()), Value::String(id.clone())); if !new_with_id.contains_key(&Value::String("status".to_string())) { new_with_id.insert(Value::String("status".to_string()), Value::String("pending".to_string())); } - seq.push(Value::Mapping(new_with_id)); + // Upsert by id: re-adding an existing id reshapes that row in place rather than pushing + // a duplicate. This is the re-scope path — a planning event that changes a row's + // scope/approach re-dispatches prd-add with the same id and the row is rewritten, + // preserving its position and semantic handle. Without this, re-adding an id duplicated + // the row (two pending rows sharing one id, one of which can never be resolved by intent). + let existing = seq.iter_mut().find(|it| { + it.as_mapping() + .and_then(|m| m.get(&Value::String("id".to_string()))) + .and_then(|v| v.as_str()) + == Some(id.as_str()) + }); + match existing { + Some(slot) => { *slot = Value::Mapping(new_with_id); upserted = true; } + None => seq.push(Value::Mapping(new_with_id)), + } } else { return (String::new(), "prd.yml is not a sequence".to_string(), 1); } @@ -181,7 +196,8 @@ pub fn handle_add(content: &str) -> (String, String, i32) { if !pkfs::write(&path_s, &new_raw) { return (String::new(), "write failed".to_string(), 1); } - (serde_json::json!({ "added": id }).to_string(), String::new(), 0) + let key = if upserted { "rescoped" } else { "added" }; + (serde_json::json!({ key: id }).to_string(), String::new(), 0) } fn parse_resolve_target(trimmed: &str) -> (String, Option) {