deliverer: empty NextPhase means stay-in-phase, not task completion

CHANGELOG.md

@@ -8,6 +8,12 @@ Please choose versions by [Semantic Versioning](http://semver.org/).
 * MINOR version when you add functionality in a backwards-compatible manner, and
 * PATCH version when you make backwards-compatible bug fixes.
 
+## Unreleased
+
+- deliverer: `AgentStatusDone` with empty `NextPhase` is now an in-place save (`status: in_progress`, phase preserved) instead of terminating the task (`phase: done`, `status: completed`) — enforces the documented `Result.NextPhase` contract ("Empty means stay in current phase"). Fixes multi-step agents whose Done+ContinueToNext preflight steps marked live tasks completed mid-run (observed: github-update-go-agent planning preflight republish, ~13 min false-completed window). Applies to both the Kafka deliverer and the content generators (`applyStatusFrontmatter`), which previously clobbered phase to `done` unconditionally.
+- **Semantic change (minor bump):** steps that relied on the empty→`done` fallback to complete tasks must now return an explicit `NextPhase: "done"`. In-repo call sites updated: all four `healthcheck` steps (claude, gemini, nop, pi) now emit `NextPhase: "done"`. Config-driven steps (`claude.NewAgentStep`, `pi.NewStep`, `agentlib.NewParseStep`) and single-shot LLM results (`claude.TaskRunner` JSON without `next_phase`) inherit the new semantics — terminal steps must configure/emit `next_phase: "done"` explicitly.
+- `AgentResultInfo` gains `ContinueToNext` (forwarded from `Result.ContinueToNext` by `StepRunner`), so deliverers can distinguish mid-run preflight saves.
+
 ## v0.78.0
 
 - launch-agent: default new agents to stateless LLM token auth (Agent Design Guide §7.2c) instead of the OAuth-PVC shape; interview Part 2 runtime tier + Part 7 security now cover GitHub App naming (§7.2a) and per-stage App pairs; config-crd-template demotes the PVC to an opt-in exception

agent_runner.go

@@ -68,10 +68,11 @@ func (r *StepRunner) Run(ctx context.Context, md *Markdown) (*Result, error) {
 		}
 
 		if err := r.deliverer.DeliverResult(ctx, AgentResultInfo{
-			Status:    result.Status,
-			Output:    newContent,
-			Message:   result.Message,
-			NextPhase: result.NextPhase,
+			Status:         result.Status,
+			Output:         newContent,
+			Message:        result.Message,
+			NextPhase:      result.NextPhase,
+			ContinueToNext: result.ContinueToNext,
 		}); err != nil {
 			return result, errors.Wrapf(ctx, err, "step %q deliver", s.Name())
 		}

agent_runner_test.go

@@ -41,6 +41,30 @@ var _ = Describe("StepRunner", func() {
 			Expect(deliverer.DeliverResultCallCount()).To(Equal(1))
 		})
 
+		It("forwards ContinueToNext to the deliverer", func() {
+			deliverer := &mocks.AgentResultDeliverer{}
+
+			step := &mocks.AgentStep{}
+			step.NameReturns("preflight-step")
+			step.ShouldRunReturns(true, nil)
+			step.RunReturns(&lib.Result{
+				Status:         lib.AgentStatusDone,
+				ContinueToNext: true,
+			}, nil)
+
+			md := &lib.Markdown{}
+			runner := lib.NewStepRunner(deliverer, step)
+
+			_, err := runner.Run(ctx, md)
+			Expect(err).To(BeNil())
+			Expect(deliverer.DeliverResultCallCount()).To(Equal(1))
+			_, info := deliverer.DeliverResultArgsForCall(0)
+			Expect(info.Status).To(Equal(lib.AgentStatusDone))
+			Expect(info.NextPhase).To(Equal(""))
+			Expect(info.ContinueToNext).To(BeTrue(),
+				"deliverer must see ContinueToNext so Done+empty NextPhase preflight saves are distinguishable")
+		})
+
 		It("returns error when step.Run returns error", func() {
 			deliverer := &mocks.AgentResultDeliverer{}
 

agent_status.go

@@ -36,8 +36,16 @@ type AgentResultInfo struct {
 	Message string // human-readable status; used by failure/needs_input paths
 	// NextPhase is the task phase the agent requests the controller to write
 	// when Status == AgentStatusDone. Ignored on Failed/NeedsInput (failure
-	// paths always escalate to human_review). Empty means "use default"
-	// (phase: done on Status: done). Valid values are vault-cli TaskPhase
-	// enum strings: planning, in_progress, ai_review, human_review, done.
+	// paths always escalate to human_review). Empty means "stay in current
+	// phase" — an in-place save between steps of a multi-step phase; the
+	// task keeps status: in_progress and its phase untouched. Terminating a
+	// task requires an explicit NextPhase: "done". Valid values are vault-cli
+	// TaskPhase enum strings: planning, execution, ai_review, human_review,
+	// done ("in_progress" is a legacy alias for execution).
 	NextPhase string
+	// ContinueToNext mirrors Result.ContinueToNext: whether the StepRunner
+	// proceeds to the next step in the same Job invocation. Informational
+	// for deliverers — a Done result with empty NextPhase is an in-place
+	// save regardless of this flag.
+	ContinueToNext bool
 }

delivery/content-generator.go

@@ -35,7 +35,7 @@ func (g *fallbackContentGenerator) Generate(
 	originalContent string,
 	result agentlib.AgentResultInfo,
 ) (string, error) {
-	updated := applyStatusFrontmatter(originalContent, result.Status)
+	updated := applyStatusFrontmatter(originalContent, result)
 	if result.Status == agentlib.AgentStatusFailed {
 		section := buildFailureSection(result)
 		return ReplaceOrAppendSection(updated, "## Failure", section), nil
@@ -47,12 +47,29 @@ func (g *fallbackContentGenerator) Generate(
 	return ReplaceOrAppendSection(updated, "## Result", section), nil
 }
 
-// applyStatusFrontmatter updates status+phase frontmatter fields based on agent result status.
-func applyStatusFrontmatter(content string, status agentlib.AgentStatus) string {
-	switch status {
+// applyStatusFrontmatter updates status+phase frontmatter fields based on agent result
+// status and requested NextPhase.
+func applyStatusFrontmatter(content string, result agentlib.AgentResultInfo) string {
+	switch result.Status {
 	case agentlib.AgentStatusDone:
-		content = SetFrontmatterField(content, "status", "completed")
-		content = SetFrontmatterField(content, "phase", "done")
+		if result.NextPhase == "" {
+			// Done without NextPhase is an in-place save (see agentlib.Result.NextPhase:
+			// "Empty means stay in current phase"): keep status: in_progress, preserve
+			// phase from existing content. Terminating a task requires an explicit
+			// NextPhase: "done".
+			content = SetFrontmatterField(content, "status", "in_progress")
+			// phase intentionally not modified — preserves the current phase
+			break
+		}
+		resolvedPhase := resolveNextPhase("", result.NextPhase)
+		content = SetFrontmatterField(content, "phase", resolvedPhase)
+		// Only mark the task completed when the resolved phase is terminal (done) —
+		// mirrors the kafkaResultDeliverer frontmatter override.
+		if resolvedPhase == "done" {
+			content = SetFrontmatterField(content, "status", "completed")
+		} else {
+			content = SetFrontmatterField(content, "status", "in_progress")
+		}
 	case agentlib.AgentStatusNeedsInput:
 		// task-level failure: agent ran cleanly but task is impossible/underspecified.
 		// Clear assignee so the task surfaces in the operator inbox; preserve phase from
@@ -121,9 +138,11 @@ func buildMinimalResultSection(result agentlib.AgentResultInfo) string {
 // the agent-produced content directly.
 //
 // Status/phase frontmatter is still applied here so file delivery sets
-// status: completed / phase: done on success without each agent having to
-// mutate the frontmatter map manually. The Kafka deliverer overrides
-// status/phase again after this generator runs (same end state).
+// status: completed / phase: done on success (Done + explicit NextPhase:
+// "done") without each agent having to mutate the frontmatter map manually;
+// Done without NextPhase is an in-place save that leaves the phase untouched.
+// The Kafka deliverer overrides status/phase again after this generator runs
+// (same end state).
 //
 // On AgentStatusFailed or AgentStatusNeedsInput, the passthrough generator
 // splices a ## Failure section into result.Output so operators always see the
@@ -140,7 +159,7 @@ func (g *passthroughContentGenerator) Generate(
 	_ string,
 	result agentlib.AgentResultInfo,
 ) (string, error) {
-	updated := applyStatusFrontmatter(result.Output, result.Status)
+	updated := applyStatusFrontmatter(result.Output, result)
 	if result.Status == agentlib.AgentStatusFailed ||
 		result.Status == agentlib.AgentStatusNeedsInput {
 		// result.Output is unreliable on early-step failures — agents return
@@ -172,7 +191,7 @@ func (g *sectionContentGenerator) Generate(
 	originalContent string,
 	result agentlib.AgentResultInfo,
 ) (string, error) {
-	updated := applyStatusFrontmatter(originalContent, result.Status)
+	updated := applyStatusFrontmatter(originalContent, result)
 	if result.Status == agentlib.AgentStatusFailed {
 		section := buildFailureSection(result)
 		return ReplaceOrAppendSection(updated, "## Failure", section), nil

delivery/content-generator_test.go

@@ -27,19 +27,55 @@ var _ = Describe("FallbackContentGenerator", func() {
 	})
 
 	Context("with frontmatter and body", func() {
-		It("sets status=completed and phase=done for done result", func() {
+		It("sets status=completed and phase=done for done result with NextPhase=done", func() {
 			original := "---\ntitle: My Task\nstatus: in_progress\n---\n\n## Task\n\nRun a backtest.\n"
 			result := agentlib.AgentResultInfo{
-				Status: agentlib.AgentStatusDone,
-				Output: "## Result\n\n- Strategy: foo\n",
+				Status:    agentlib.AgentStatusDone,
+				NextPhase: "done",
+				Output:    "## Result\n\n- Strategy: foo\n",
 			}
 			generated, err := generator.Generate(ctx, original, result)
 			Expect(err).NotTo(HaveOccurred())
 			Expect(generated).To(ContainSubstring("status: completed"))
+			Expect(generated).To(ContainSubstring("phase: done"))
 			Expect(generated).To(ContainSubstring("## Result"))
 			Expect(generated).To(ContainSubstring("Strategy: foo"))
 		})
 
+		It(
+			"treats done result with empty NextPhase as in-place save (status in_progress, phase preserved)",
+			func() {
+				original := "---\ntitle: My Task\nstatus: in_progress\nphase: planning\n---\n\n## Task\n\nRun a backtest.\n"
+				result := agentlib.AgentResultInfo{
+					Status: agentlib.AgentStatusDone,
+					Output: "## Result\n\n- Strategy: foo\n",
+				}
+				generated, err := generator.Generate(ctx, original, result)
+				Expect(err).NotTo(HaveOccurred())
+				fm, _ := delivery.ParseMarkdownFrontmatter(generated)
+				Expect(fm["status"]).To(Equal("in_progress"))
+				Expect(fm["phase"]).To(Equal("planning"))
+				Expect(fm["phase"]).NotTo(Equal("done"))
+			},
+		)
+
+		It(
+			"sets phase=execution and status=in_progress for done result with NextPhase=execution",
+			func() {
+				original := "---\ntitle: My Task\nstatus: in_progress\nphase: planning\n---\n\n## Task\n\nRun a backtest.\n"
+				result := agentlib.AgentResultInfo{
+					Status:    agentlib.AgentStatusDone,
+					NextPhase: "execution",
+					Output:    "## Result\n\n- Strategy: foo\n",
+				}
+				generated, err := generator.Generate(ctx, original, result)
+				Expect(err).NotTo(HaveOccurred())
+				fm, _ := delivery.ParseMarkdownFrontmatter(generated)
+				Expect(fm["status"]).To(Equal("in_progress"))
+				Expect(fm["phase"]).To(Equal("execution"))
+			},
+		)
+
 		It(
 			"sets status=in_progress, clears assignee, preserves phase for failed result with ## Failure section",
 			func() {
@@ -317,11 +353,12 @@ var _ = Describe("PassthroughContentGenerator", func() {
 	})
 
 	It(
-		"returns result.Output verbatim with status=completed frontmatter on AgentStatusDone",
+		"returns result.Output verbatim with status=completed frontmatter on AgentStatusDone with NextPhase=done",
 		func() {
 			result := agentlib.AgentResultInfo{
-				Status: agentlib.AgentStatusDone,
-				Output: "---\ntitle: My Task\n---\n\n## Review\n\nLooks good.\n",
+				Status:    agentlib.AgentStatusDone,
+				NextPhase: "done",
+				Output:    "---\ntitle: My Task\n---\n\n## Review\n\nLooks good.\n",
 			}
 			generated, err := generator.Generate(ctx, "", result)
 			Expect(err).NotTo(HaveOccurred())
@@ -332,6 +369,23 @@ var _ = Describe("PassthroughContentGenerator", func() {
 		},
 	)
 
+	It(
+		"treats AgentStatusDone with empty NextPhase as in-place save (status in_progress, phase preserved)",
+		func() {
+			result := agentlib.AgentResultInfo{
+				Status: agentlib.AgentStatusDone,
+				Output: "---\ntitle: My Task\nstatus: in_progress\nphase: planning\n---\n\n## Review\n\nDraft.\n",
+			}
+			generated, err := generator.Generate(ctx, "", result)
+			Expect(err).NotTo(HaveOccurred())
+			fm, _ := delivery.ParseMarkdownFrontmatter(generated)
+			Expect(fm["status"]).To(Equal("in_progress"))
+			Expect(fm["phase"]).To(Equal("planning"))
+			Expect(fm["phase"]).NotTo(Equal("done"))
+			Expect(generated).NotTo(ContainSubstring("## Failure"))
+		},
+	)
+
 	It("preserves phase from input and keeps status=in_progress on AgentStatusInProgress", func() {
 		result := agentlib.AgentResultInfo{
 			Status: agentlib.AgentStatusInProgress,

delivery/result-deliverer.go

@@ -139,10 +139,21 @@ func (d *kafkaResultDeliverer) DeliverResult(
 	// max_triggers, not a phase loop.
 	switch result.Status {
 	case agentlib.AgentStatusDone:
+		if result.NextPhase == "" {
+			// Done without NextPhase is an in-place save (see agentlib.Result.NextPhase:
+			// "Empty means stay in current phase"): keep status: in_progress and preserve
+			// the phase from incoming frontmatter (already copied from fmMap above) —
+			// exactly like the AgentStatusInProgress branch. Terminating a task requires
+			// an explicit NextPhase: "done". Without this, mid-phase saves from
+			// Done+ContinueToNext preflight steps marked live tasks completed.
+			frontmatter["status"] = "in_progress"
+			// phase intentionally not modified — preserves incoming phase
+			break
+		}
 		resolvedPhase := resolveNextPhase(d.taskID, result.NextPhase)
 		frontmatter["phase"] = resolvedPhase
 		// Only mark the task completed when the resolved phase is terminal (done).
-		// Requested transitions to planning/in_progress/ai_review/human_review keep
+		// Requested transitions to planning/execution/ai_review/human_review keep
 		// the task at status: in_progress so the controller re-triggers on the
 		// new phase. Without this, multi-phase agents stall after their first phase.
 		if resolvedPhase == "done" {
@@ -207,17 +218,15 @@ func (d *kafkaResultDeliverer) DeliverResult(
 	return nil
 }
 
-// resolveNextPhase returns the validated phase string for a done agent result.
-// An empty NextPhase defaults to "done" (existing behavior). An invalid value
-// is logged with task-id context and also falls back to "done" — we never refuse
-// to write a result just because the agent requested a bogus phase.
+// resolveNextPhase returns the validated phase string for a done agent result
+// with a non-empty requested NextPhase (empty NextPhase is an in-place save and
+// never reaches this function). An invalid value is logged with task-id context
+// and falls back to "done" — we never refuse to write a result just because the
+// agent requested a bogus phase.
 func resolveNextPhase(
 	taskID agentlib.TaskIdentifier,
 	requested string,
 ) string {
-	if requested == "" {
-		return "done"
-	}
 	canonical, ok := domain.NormalizeTaskPhase(requested)
 	if !ok {
 		glog.Warningf("task %s: ignoring invalid NextPhase %q: unknown phase", taskID, requested)

delivery/result-deliverer_test.go

@@ -135,7 +135,8 @@ var _ = Describe("KafkaResultDeliverer", func() {
 			nil,
 		)
 		err := deliverer.DeliverResult(ctx, agentlib.AgentResultInfo{
-			Status: agentlib.AgentStatusDone,
+			Status:    agentlib.AgentStatusDone,
+			NextPhase: "done",
 		})
 		Expect(err).NotTo(HaveOccurred())
 		Expect(sender.SendCommandObjectCallCount()).To(Equal(1))
@@ -195,23 +196,72 @@ var _ = Describe("KafkaResultDeliverer", func() {
 		},
 	)
 
-	It("sets phase=done when done result has empty NextPhase", func() {
+	It(
+		"treats done result with empty NextPhase as in-place save (phase preserved, status in_progress)",
+		func() {
+			generator.GenerateReturns(
+				"---\nstatus: in_progress\nphase: planning\n---\n\nBody.\n",
+				nil,
+			)
+			err := deliverer.DeliverResult(ctx, agentlib.AgentResultInfo{
+				Status:    agentlib.AgentStatusDone,
+				NextPhase: "",
+			})
+			Expect(err).NotTo(HaveOccurred())
+			_, cmdObj := sender.SendCommandObjectArgsForCall(0)
+			frontmatter, ok := cmdObj.Command.Data["frontmatter"]
+			Expect(ok).To(BeTrue())
+			fm, ok := frontmatter.(map[string]interface{})
+			Expect(ok).To(BeTrue())
+			// Empty NextPhase means "stay in current phase" (agentlib.Result contract) —
+			// never terminal. Regression: preflight steps publishing Done+ContinueToNext
+			// with empty NextPhase marked live tasks phase: done / status: completed.
+			Expect(fm["phase"]).To(Equal("planning"))
+			Expect(fm["phase"]).NotTo(Equal("done"))
+			Expect(fm["status"]).To(Equal("in_progress"))
+		},
+	)
+
+	It(
+		"treats done result with empty NextPhase and ContinueToNext as in-place save (phase preserved, status in_progress)",
+		func() {
+			generator.GenerateReturns(
+				"---\nstatus: in_progress\nphase: execution\n---\n\nBody.\n",
+				nil,
+			)
+			err := deliverer.DeliverResult(ctx, agentlib.AgentResultInfo{
+				Status:         agentlib.AgentStatusDone,
+				NextPhase:      "",
+				ContinueToNext: true,
+			})
+			Expect(err).NotTo(HaveOccurred())
+			_, cmdObj := sender.SendCommandObjectArgsForCall(0)
+			frontmatter, ok := cmdObj.Command.Data["frontmatter"]
+			Expect(ok).To(BeTrue())
+			fm, ok := frontmatter.(map[string]interface{})
+			Expect(ok).To(BeTrue())
+			Expect(fm["phase"]).To(Equal("execution"))
+			Expect(fm["status"]).To(Equal("in_progress"))
+		},
+	)
+
+	It("sets phase=execution when done result requests NextPhase=execution", func() {
 		generator.GenerateReturns(
-			"---\nstatus: completed\nphase: done\n---\n\nBody.\n",
+			"---\nstatus: in_progress\nphase: execution\n---\n\nBody.\n",
 			nil,
 		)
 		err := deliverer.DeliverResult(ctx, agentlib.AgentResultInfo{
 			Status:    agentlib.AgentStatusDone,
-			NextPhase: "",
+			NextPhase: "execution",
 		})
 		Expect(err).NotTo(HaveOccurred())
 		_, cmdObj := sender.SendCommandObjectArgsForCall(0)
 		frontmatter, ok := cmdObj.Command.Data["frontmatter"]
 		Expect(ok).To(BeTrue())
 		fm, ok := frontmatter.(map[string]interface{})
 		Expect(ok).To(BeTrue())
-		Expect(fm["phase"]).To(Equal("done"))
-		Expect(fm["status"]).To(Equal("completed"))
+		Expect(fm["phase"]).To(Equal("execution"))
+		Expect(fm["status"]).To(Equal("in_progress"))
 	})
 
 	It(

docs/task-flow-and-failure-semantics.md

@@ -108,7 +108,7 @@ default (failed):
 
 ### Happy path
 
-1. Task `phase: in_progress`, agent emits `done` → `phase: done`, `status: completed`. Terminal.
+1. Task `phase: in_progress`, agent emits `done` with explicit `NextPhase: "done"` → `phase: done`, `status: completed`. Terminal. (`done` with empty `NextPhase` is an in-place save: `status: in_progress`, phase unchanged — used between steps of a multi-step phase.)
 
 ### Agent emits `needs_input` (spec 010)