Add plan review gate

Author: mekilis

.agents/archive/2026-06-12-002-add-plan-review-gate/decisions/2026-06-12T211520Z-direct-supervisor-edit-allowed-for-this-fix-because-agents-loop-could-not-make-p.md

@@ -0,0 +1,6 @@
+# Decision: Direct supervisor edit allowed for this fix because Agents loop could not make p
+
+Status: proposed
+By: smart
+
+Direct supervisor edit allowed for this fix because Agents loop could not make progress on the open finding: repeated implementer runs short-circuited or died while the plan gate itself routed the task away from the implementer.

.agents/archive/2026-06-12-002-add-plan-review-gate/summary.md

@@ -0,0 +1,38 @@
+# Add plan review gate
+
+status: archived
+previous_status: active
+archived_at: 2026-06-12T21:22:55Z
+
+Summary:
+- decisions: 1
+- handoffs: 8
+- verification records: 2
+- run activity records: 9
+- findings open: 0
+- findings fixed: 0
+- findings verified: 1
+
+## Scope
+
+Recent handoffs:
+- Please verify the fixed stale plan-review evidence finding and review the direct supervisor edit. Focus on plan Updated timestamp parsing, planReviewCurrent, the generic verification branch, and regression coverage. Tests passed locally with ...
+- Fix the open medium finding about stale plan-review evidence. Add regression coverage and preserve normal implementation verification behavior.
+
+Decisions:
+- Direct supervisor edit allowed for this fix because Agents loop could not make progress on the open finding: repeated implementer runs short-circuited or died while the plan gate itself routed the task ...
+
+Verification:
+- Reviewed plan review gate routing, plan Updated timestamp parsing, generic verification approval blocking, docs, and regression coverage. No unresolved issues found. Checks passed with isolated GOCACHE: go test ./internal/app and go test ...
+- Reviewed plan review gate fix: plan Updated metadata is tracked, planReviewCurrent requires verification newer than the current plan revision, generic verification no longer bypasses draft/stale plan approval states, and regression coverage verifies ...
+
+## Final Status
+
+Implemented plan review gate routing, reviewer prompt guidance, docs, and focused regression tests; go test ./... passes with isolated cache.
+
+## Reviewer Verification
+
+- Reviewed plan review gate routing, plan Updated timestamp parsing, generic verification approval blocking, docs, and regression coverage. No unresolved issues found. Checks passed with isolated GOCACHE: go test ./internal/app and go test ...
+- Reviewed plan review gate fix: plan Updated metadata is tracked, planReviewCurrent requires verification newer than the current plan revision, generic verification no longer bypasses draft/stale plan approval states, and regression coverage verifies ...
+
+Detailed transient chunks remain available in git history.

README.md

@@ -199,11 +199,11 @@ agents plan clear
 
 The canonical plan lives at `.agents/tasks/<task>/plan.md`. Runs may mention
 plan updates, but durable plan content belongs to the task. A draft or stale
-plan routes the loop to the coordinator for approval or refinement before Agents
-runs the implementer. Reviewers inspect the current task plan as part of review
-and should record findings when the implementation does not match the plan, or
-when the plan is stale, missing, shallow, or incorrect for the work under
-review.
+plan routes the loop to the reviewer for a plan review gate before Agents runs
+the implementer. Reviewers should record findings when the plan is stale,
+missing, shallow, incorrect, or missing enough evidence, validation, risks, or
+constraints for the work. Passing plan review still stops at the human approval
+boundary; `agents plan approve` is what allows implementation to proceed.
 
 `agents plan refine` accepts refinement feedback only when it can be handled
 honestly. Because current plans are free-form Markdown, the command refuses

internal/app/app.go

@@ -2587,6 +2587,7 @@ type activeTask struct {
 type taskStats struct {
 	Next             string
 	PlanState        string
+	PlanUpdatedStamp string
 	Decisions        int
 	Handoffs         int
 	LatestHandoffTo  string
@@ -2861,10 +2862,14 @@ func readTaskStats(cwd, activePath string) (taskStats, error) {
 	if err != nil {
 		return taskStats{}, err
 	}
-	planState, err := planStateForStats(cwd, activePath, latestHandoffName)
+	plan, err := readTaskPlan(cwd, activePath)
 	if err != nil {
 		return taskStats{}, err
 	}
+	planState := plan.State
+	if planState == planStateApproved && planStale(cwd, activePath, latestHandoffName) {
+		planState = planStateStale
+	}
 	verificationPass, verificationFail, latestVerificationName, err := readVerificationStats(filepath.Join(taskDir, "verification"))
 	if err != nil {
 		return taskStats{}, err
@@ -2880,6 +2885,7 @@ func readTaskStats(cwd, activePath string) (taskStats, error) {
 	return taskStats{
 		Next:                   next,
 		PlanState:              planState,
+		PlanUpdatedStamp:       plan.UpdatedStamp,
 		Decisions:              decisions,
 		Handoffs:               handoffs,
 		LatestHandoffTo:        latestHandoffTo,
@@ -4951,7 +4957,9 @@ func dispatchPacket(e env, role string) (dispatchInfo, error) {
 			info.Action = "implement the next task step, run relevant checks, then write a handoff"
 		}
 	case "reviewer":
-		if state.Stats.LatestHandoffTo != "reviewer" {
+		if state.NextAction == "review plan before implementation" {
+			info.Action = "review the draft or stale plan; add findings or record plan review evidence"
+		} else if state.Stats.LatestHandoffTo != "reviewer" {
 			info.Action = "wait for an implementer handoff before reviewing"
 			info.Boundary = "handoff required"
 		} else {
@@ -4980,7 +4988,11 @@ func analyzeLoopState(e env) (loopState, error) {
 	case stats.FindingsFixed > 0:
 		state.NextRole = "reviewer"
 		state.NextAction = "verify fixed findings independently"
-	case verificationCurrent(stats):
+	case planRequiresApproval(stats.PlanState) && planReviewCurrent(stats):
+		state.NextRole = "human"
+		state.NextAction = "approve reviewed plan before implementation"
+		state.Boundary = "plan approval required"
+	case verificationCurrent(stats) && !planRequiresApproval(stats.PlanState) && !(stats.PlanState == planStateApproved && stats.LatestHandoffTo == "implementer"):
 		state.NextRole = "human"
 		state.NextAction = "approve archive, commit, or next task"
 		state.Boundary = "human approval required"
@@ -4998,8 +5010,8 @@ func analyzeLoopState(e env) (loopState, error) {
 		state.NextAction = "start or continue implementation"
 	}
 	if state.NextRole == "implementer" && planRequiresApproval(stats.PlanState) {
-		state.NextRole = "coordinator"
-		state.NextAction = "approve or refine plan before implementation"
+		state.NextRole = "reviewer"
+		state.NextAction = "review plan before implementation"
 		state.Boundary = "plan approval required"
 	}
 	return state, nil
@@ -5030,6 +5042,16 @@ func verificationCurrent(stats taskStats) bool {
 	return chunkStamp(stats.LatestVerificationName) >= chunkStamp(stats.LatestHandoffName)
 }
 
+func planReviewCurrent(stats taskStats) bool {
+	if !verificationCurrent(stats) {
+		return false
+	}
+	if stats.PlanUpdatedStamp == "" {
+		return false
+	}
+	return chunkStamp(stats.LatestVerificationName) >= stats.PlanUpdatedStamp
+}
+
 // chunkStamp returns the leading UTC timestamp of a chunk filename
 // (e.g. "2026-06-10T112546Z"), or the whole name if it is shorter.
 func chunkStamp(name string) string {
@@ -6114,6 +6136,11 @@ func buildRolePrompt(e env, state loopState) (string, error) {
 		appendPromptDir(&b, e.cwd, state.Task.Path, "findings", "Findings")
 		appendPromptDir(&b, e.cwd, state.Task.Path, "verification", "Verification")
 		fmt.Fprintln(&b, "Working agreement:")
+		if state.NextAction == "review plan before implementation" {
+			fmt.Fprintln(&b, "- This is a plan review gate before implementation. Review `plan.md` for readiness; do not review or request implementation changes unless they are needed to make the plan actionable.")
+			fmt.Fprintln(&b, "- If the plan is shallow, stale, missing evidence, incorrect, or omits validation or constraints, record findings against the plan so the implementer can fix it before code changes.")
+			fmt.Fprintln(&b, "- If the plan is ready, record review evidence. The human must still approve the plan before implementation proceeds.")
+		}
 		fmt.Fprintln(&b, "- Review the working tree, tests, and task chunks; verify claims against the code, not the handoff text.")
 		fmt.Fprintln(&b, "- Review the current task plan in `plan.md` when present. Check for substantive objective, repo-relative evidence, assumptions or open questions, implementation shape, validation, risks, and constraints; do not require exact section headings.")
 		fmt.Fprintln(&b, "- Check whether the implementation and review scope match the approved or draft plan.")

internal/app/app_test.go

@@ -3429,7 +3429,7 @@ func TestRolePromptBoundsLongPlanContent(t *testing.T) {
 	}
 }
 
-func TestDraftPlanRoutesImplementerHandoffToCoordinator(t *testing.T) {
+func TestDraftPlanRoutesImplementerHandoffToReviewerPlanReview(t *testing.T) {
 	var out, errOut bytes.Buffer
 	code := withTempProcessEnv(t, func(home, cwd string) int {
 		e := env{cwd: cwd, home: home, trustPath: filepath.Join(home, ".agents", "trust.json")}
@@ -3454,14 +3454,149 @@ func TestDraftPlanRoutesImplementerHandoffToCoordinator(t *testing.T) {
 	if err := json.Unmarshal(out.Bytes(), &got); err != nil {
 		t.Fatalf("run JSON should decode: %v\n%s", err, out.String())
 	}
-	if got.NextRole != "coordinator" || got.NextAction != "approve or refine plan before implementation" || got.Boundary != "plan approval required" {
+	if got.NextRole != "reviewer" || got.NextAction != "review plan before implementation" || got.Boundary != "plan approval required" {
 		t.Fatalf("unexpected loop state: %+v", got)
 	}
 	if got.Task == nil || got.Task.Plan != "draft" {
 		t.Fatalf("expected draft plan in loop JSON, got %+v", got.Task)
 	}
 }
 
+func TestStalePlanRoutesImplementationToReviewerPlanReview(t *testing.T) {
+	code := withTempProcessEnv(t, func(home, cwd string) int {
+		e := env{cwd: cwd, home: home, trustPath: filepath.Join(home, ".agents", "trust.json")}
+		if _, err := openTask(e, []string{"Stale Plan Boundary"}, time.Date(2026, 6, 5, 10, 0, 0, 0, time.UTC)); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := addHandoff(e, []string{"--to", "implementer", "Build it."}, time.Date(2026, 6, 5, 10, 1, 0, 0, time.UTC)); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := writeActiveTaskPlan(e, []string{"Stale plan that needs reviewer attention."}, planStateStale); err != nil {
+			t.Fatal(err)
+		}
+		state, err := analyzeLoopState(e)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if state.NextRole != "reviewer" || state.NextAction != "review plan before implementation" || state.Boundary != "plan approval required" {
+			t.Fatalf("unexpected loop state: %+v", state)
+		}
+		return 0
+	})
+	if code != 0 {
+		t.Fatalf("withTempProcessEnv code=%d", code)
+	}
+}
+
+func TestReviewerPromptExplainsPlanReviewGate(t *testing.T) {
+	code := withTempProcessEnv(t, func(home, cwd string) int {
+		e := env{cwd: cwd, home: home, trustPath: filepath.Join(home, ".agents", "trust.json")}
+		task, err := openTask(e, []string{"Plan Review Prompt"}, time.Date(2026, 6, 5, 10, 0, 0, 0, time.UTC))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if _, err := writeActiveTaskPlan(e, []string{"Review this plan before implementation."}, planStateDraft); err != nil {
+			t.Fatal(err)
+		}
+		prompt, err := buildRolePrompt(e, loopState{
+			Task:       task,
+			NextRole:   "reviewer",
+			NextAction: "review plan before implementation",
+		})
+		if err != nil {
+			t.Fatal(err)
+		}
+		for _, want := range []string{
+			"This is a plan review gate before implementation",
+			"record findings against the plan",
+			"The human must still approve the plan before implementation proceeds",
+		} {
+			if !strings.Contains(prompt, want) {
+				t.Fatalf("reviewer prompt missing %q:\n%s", want, prompt)
+			}
+		}
+		return 0
+	})
+	if code != 0 {
+		t.Fatalf("withTempProcessEnv code=%d", code)
+	}
+}
+
+func writeTestPlanAt(t *testing.T, cwd, activePath, state, text string, updated time.Time) {
+	t.Helper()
+	taskDir := filepath.Join(cwd, stateDir, activePath)
+	body := fmt.Sprintf("# Plan\n\nState: %s\nUpdated: %s\n\n%s\n", state, updated.UTC().Format(time.RFC3339), text)
+	if err := os.WriteFile(filepath.Join(taskDir, "plan.md"), []byte(body), 0o644); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestPassingPlanReviewStopsForHumanPlanApproval(t *testing.T) {
+	code := withTempProcessEnv(t, func(home, cwd string) int {
+		e := env{cwd: cwd, home: home, trustPath: filepath.Join(home, ".agents", "trust.json")}
+		task, err := openTask(e, []string{"Plan Review Approval"}, time.Date(2026, 6, 5, 10, 0, 0, 0, time.UTC))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if _, err := addHandoff(e, []string{"--to", "implementer", "Build it after the plan is ready."}, time.Date(2026, 6, 5, 10, 1, 0, 0, time.UTC)); err != nil {
+			t.Fatal(err)
+		}
+		writeTestPlanAt(t, cwd, task.Path, planStateDraft, "Plan reviewed before implementation.", time.Date(2026, 6, 5, 10, 1, 30, 0, time.UTC))
+		if _, _, err := addVerification(e, []string{"reviewer checked plan readiness", "--by", "reviewer"}, time.Date(2026, 6, 5, 10, 2, 0, 0, time.UTC)); err != nil {
+			t.Fatal(err)
+		}
+		state, err := analyzeLoopState(e)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if state.NextRole != "human" || state.NextAction != "approve reviewed plan before implementation" || state.Boundary != "plan approval required" {
+			t.Fatalf("unexpected loop state before approval: %+v", state)
+		}
+		if _, err := approveActiveTaskPlan(e); err != nil {
+			t.Fatal(err)
+		}
+		state, err = analyzeLoopState(e)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if state.NextRole != "implementer" || state.NextAction != "act on latest handoff" {
+			t.Fatalf("approved reviewed plan should return to implementation, got %+v", state)
+		}
+		return 0
+	})
+	if code != 0 {
+		t.Fatalf("withTempProcessEnv code=%d", code)
+	}
+}
+
+func TestPlanReviewEvidenceMustBeNewerThanPlanRevision(t *testing.T) {
+	code := withTempProcessEnv(t, func(home, cwd string) int {
+		e := env{cwd: cwd, home: home, trustPath: filepath.Join(home, ".agents", "trust.json")}
+		task, err := openTask(e, []string{"Plan Review Freshness"}, time.Date(2026, 6, 5, 10, 0, 0, 0, time.UTC))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if _, err := addHandoff(e, []string{"--to", "implementer", "Build it after plan review."}, time.Date(2026, 6, 5, 10, 1, 0, 0, time.UTC)); err != nil {
+			t.Fatal(err)
+		}
+		if _, _, err := addVerification(e, []string{"reviewed the earlier plan", "--by", "reviewer"}, time.Date(2026, 6, 5, 10, 2, 0, 0, time.UTC)); err != nil {
+			t.Fatal(err)
+		}
+		writeTestPlanAt(t, cwd, task.Path, planStateDraft, "Newer plan that has not been reviewed.", time.Date(2026, 6, 5, 10, 3, 0, 0, time.UTC))
+		state, err := analyzeLoopState(e)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if state.NextRole != "reviewer" || state.NextAction != "review plan before implementation" || state.Boundary != "plan approval required" {
+			t.Fatalf("stale plan-review evidence must route back to reviewer, got %+v", state)
+		}
+		return 0
+	})
+	if code != 0 {
+		t.Fatalf("withTempProcessEnv code=%d", code)
+	}
+}
+
 func TestRunRoutesHandoffToCoordinatorToCoordinator(t *testing.T) {
 	var out, errOut bytes.Buffer
 	code := withTempProcessEnv(t, func(home, cwd string) int {

internal/app/plan.go

@@ -17,9 +17,10 @@ const (
 )
 
 type taskPlan struct {
-	State string
-	Text  string
-	Path  string
+	State        string
+	Text         string
+	Path         string
+	UpdatedStamp string
 }
 
 func runPlanCommand(args []string, out, errOut io.Writer, e env) int {
@@ -106,12 +107,22 @@ func readTaskPlan(cwd, activePath string) (taskPlan, error) {
 		return taskPlan{}, err
 	}
 	state := planStateDraft
+	updatedStamp := ""
 	body := planBodyWithoutState(stripFirstMarkdownHeading(string(data)))
 	for _, line := range strings.Split(string(data), "\n") {
 		trimmed := strings.TrimSpace(line)
-		if strings.HasPrefix(strings.ToLower(trimmed), "state:") {
+		lower := strings.ToLower(trimmed)
+		if strings.HasPrefix(lower, "state:") {
 			state = strings.ToLower(strings.TrimSpace(trimmed[len("state:"):]))
-			break
+			continue
+		}
+		if strings.HasPrefix(lower, "updated:") {
+			updatedStamp = planUpdatedStamp(strings.TrimSpace(trimmed[len("updated:"):]))
+		}
+	}
+	if updatedStamp == "" {
+		if info, err := os.Stat(path); err == nil {
+			updatedStamp = info.ModTime().UTC().Format("2006-01-02T150405Z")
 		}
 	}
 	switch state {
@@ -120,9 +131,10 @@ func readTaskPlan(cwd, activePath string) (taskPlan, error) {
 		state = planStateDraft
 	}
 	return taskPlan{
-		State: state,
-		Text:  strings.TrimSpace(body),
-		Path:  filepath.Join(activePath, "plan.md"),
+		State:        state,
+		Text:         strings.TrimSpace(body),
+		Path:         filepath.Join(activePath, "plan.md"),
+		UpdatedStamp: updatedStamp,
 	}, nil
 }
 
@@ -241,6 +253,16 @@ func planBodyWithoutState(text string) string {
 	return strings.TrimSpace(strings.Join(lines, "\n"))
 }
 
+func planUpdatedStamp(updated string) string {
+	if updated == "" {
+		return ""
+	}
+	if t, err := time.Parse(time.RFC3339, updated); err == nil {
+		return t.UTC().Format("2006-01-02T150405Z")
+	}
+	return chunkStamp(updated)
+}
+
 func planStateForStats(cwd, activePath, latestHandoffName string) (string, error) {
 	plan, err := readTaskPlan(cwd, activePath)
 	if err != nil {