From f8232f20218ce8138cbbb4d61fb5289b59b13fe4 Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Tue, 21 Jul 2026 17:54:28 +0200 Subject: [PATCH 1/2] deliverer: treat done with empty NextPhase as in-place save, not task completion Enforce the Result.NextPhase contract (empty means stay in current phase): the Kafka deliverer and applyStatusFrontmatter no longer map Done+empty NextPhase to phase: done / status: completed. Thread ContinueToNext into AgentResultInfo. Healthcheck steps now request NextPhase: done explicitly so healthcheck tasks still complete. --- CHANGELOG.md | 6 ++ agent_runner.go | 9 +-- agent_runner_test.go | 24 ++++++++ agent_status.go | 14 ++++- delivery/content-generator.go | 41 +++++++++---- delivery/content-generator_test.go | 66 +++++++++++++++++++-- delivery/result-deliverer.go | 25 +++++--- delivery/result-deliverer_test.go | 62 +++++++++++++++++-- docs/task-flow-and-failure-semantics.md | 2 +- healthcheck/healthcheck-claude-step.go | 7 ++- healthcheck/healthcheck-claude-step_test.go | 2 + healthcheck/healthcheck-gemini-step.go | 7 ++- healthcheck/healthcheck-gemini-step_test.go | 2 + healthcheck/healthcheck-nop-step.go | 7 ++- healthcheck/healthcheck-nop-step_test.go | 2 + healthcheck/healthcheck-pi-step.go | 7 ++- 16 files changed, 236 insertions(+), 47 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 526ec3b3..d1c53eb2 100644 --- a/CHANGELOG.md +++ b/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.77.2 - Bump `golang.org/x/text` to v0.39.0 (CVE-2026-56852) diff --git a/agent_runner.go b/agent_runner.go index 82f3b676..4287de83 100644 --- a/agent_runner.go +++ b/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()) } diff --git a/agent_runner_test.go b/agent_runner_test.go index 8c9f510e..0742a3f7 100644 --- a/agent_runner_test.go +++ b/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{} diff --git a/agent_status.go b/agent_status.go index e117f79f..850de3e6 100644 --- a/agent_status.go +++ b/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 } diff --git a/delivery/content-generator.go b/delivery/content-generator.go index 5d91064e..bd5e36b2 100644 --- a/delivery/content-generator.go +++ b/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 diff --git a/delivery/content-generator_test.go b/delivery/content-generator_test.go index 5ec6c81b..56b0da52 100644 --- a/delivery/content-generator_test.go +++ b/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, diff --git a/delivery/result-deliverer.go b/delivery/result-deliverer.go index a03beaf4..85d273a2 100644 --- a/delivery/result-deliverer.go +++ b/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) diff --git a/delivery/result-deliverer_test.go b/delivery/result-deliverer_test.go index 3be69191..4ef78e10 100644 --- a/delivery/result-deliverer_test.go +++ b/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,14 +196,63 @@ 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) @@ -210,8 +260,8 @@ var _ = Describe("KafkaResultDeliverer", func() { 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( diff --git a/docs/task-flow-and-failure-semantics.md b/docs/task-flow-and-failure-semantics.md index 6e2ee059..745b99e9 100644 --- a/docs/task-flow-and-failure-semantics.md +++ b/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) diff --git a/healthcheck/healthcheck-claude-step.go b/healthcheck/healthcheck-claude-step.go index 81034461..98f1d21d 100644 --- a/healthcheck/healthcheck-claude-step.go +++ b/healthcheck/healthcheck-claude-step.go @@ -50,7 +50,10 @@ func (s *claudeStep) Run(ctx context.Context, _ *agentlib.Markdown) (*agentlib.R }, nil } return &agentlib.Result{ - Status: agentlib.AgentStatusDone, - Message: trimmed, + Status: agentlib.AgentStatusDone, + // Explicit terminal phase: Done with empty NextPhase is an in-place save + // (stay in current phase) — healthcheck tasks must actually complete. + NextPhase: "done", + Message: trimmed, }, nil } diff --git a/healthcheck/healthcheck-claude-step_test.go b/healthcheck/healthcheck-claude-step_test.go index e050ed96..03da08ff 100644 --- a/healthcheck/healthcheck-claude-step_test.go +++ b/healthcheck/healthcheck-claude-step_test.go @@ -51,6 +51,8 @@ var _ = Describe("NewClaudeStep", func() { Expect(err).To(BeNil()) Expect(result).NotTo(BeNil()) Expect(result.Status).To(Equal(agentlib.AgentStatusDone)) + Expect(result.NextPhase).To(Equal("done"), + "healthcheck must request explicit terminal phase — empty NextPhase is an in-place save") Expect(result.Message).To(Equal("ok")) }) diff --git a/healthcheck/healthcheck-gemini-step.go b/healthcheck/healthcheck-gemini-step.go index e477d11b..65db31ea 100644 --- a/healthcheck/healthcheck-gemini-step.go +++ b/healthcheck/healthcheck-gemini-step.go @@ -52,7 +52,10 @@ func (s *geminiStep) Run(ctx context.Context, _ *agentlib.Markdown) (*agentlib.R }, nil } return &agentlib.Result{ - Status: agentlib.AgentStatusDone, - Message: reply.OK, + Status: agentlib.AgentStatusDone, + // Explicit terminal phase: Done with empty NextPhase is an in-place save + // (stay in current phase) — healthcheck tasks must actually complete. + NextPhase: "done", + Message: reply.OK, }, nil } diff --git a/healthcheck/healthcheck-gemini-step_test.go b/healthcheck/healthcheck-gemini-step_test.go index 24229944..ae02dda0 100644 --- a/healthcheck/healthcheck-gemini-step_test.go +++ b/healthcheck/healthcheck-gemini-step_test.go @@ -53,6 +53,8 @@ var _ = Describe("NewGeminiStep", func() { Expect(err).To(BeNil()) Expect(result).NotTo(BeNil()) Expect(result.Status).To(Equal(agentlib.AgentStatusDone)) + Expect(result.NextPhase).To(Equal("done"), + "healthcheck must request explicit terminal phase — empty NextPhase is an in-place save") Expect(result.Message).To(Equal("pong")) }) diff --git a/healthcheck/healthcheck-nop-step.go b/healthcheck/healthcheck-nop-step.go index 98e184bb..2ad25f1c 100644 --- a/healthcheck/healthcheck-nop-step.go +++ b/healthcheck/healthcheck-nop-step.go @@ -27,7 +27,10 @@ func (s *nopStep) ShouldRun(_ context.Context, _ *agentlib.Markdown) (bool, erro func (s *nopStep) Run(_ context.Context, _ *agentlib.Markdown) (*agentlib.Result, error) { return &agentlib.Result{ - Status: agentlib.AgentStatusDone, - Message: "ok", + Status: agentlib.AgentStatusDone, + // Explicit terminal phase: Done with empty NextPhase is an in-place save + // (stay in current phase) — healthcheck tasks must actually complete. + NextPhase: "done", + Message: "ok", }, nil } diff --git a/healthcheck/healthcheck-nop-step_test.go b/healthcheck/healthcheck-nop-step_test.go index 4a485c7d..eb61700b 100644 --- a/healthcheck/healthcheck-nop-step_test.go +++ b/healthcheck/healthcheck-nop-step_test.go @@ -45,6 +45,8 @@ var _ = Describe("NewNopStep", func() { Expect(err).To(BeNil()) Expect(result).NotTo(BeNil()) Expect(result.Status).To(Equal(agentlib.AgentStatusDone)) + Expect(result.NextPhase).To(Equal("done"), + "healthcheck must request explicit terminal phase — empty NextPhase is an in-place save") Expect(result.Message).To(Equal("ok")) }) }) diff --git a/healthcheck/healthcheck-pi-step.go b/healthcheck/healthcheck-pi-step.go index 4ffa9f1d..324352f6 100644 --- a/healthcheck/healthcheck-pi-step.go +++ b/healthcheck/healthcheck-pi-step.go @@ -47,7 +47,10 @@ func (s *piStep) Run(ctx context.Context, _ *agentlib.Markdown) (*agentlib.Resul }, nil } return &agentlib.Result{ - Status: agentlib.AgentStatusDone, - Message: trimmed, + Status: agentlib.AgentStatusDone, + // Explicit terminal phase: Done with empty NextPhase is an in-place save + // (stay in current phase) — healthcheck tasks must actually complete. + NextPhase: "done", + Message: trimmed, }, nil } From eec46b024711fab947553108f3a35572ce52d839 Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Tue, 21 Jul 2026 18:18:12 +0200 Subject: [PATCH 2/2] kick CI