mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-15 14:19:13 +02:00
fix(issues): don't call an intermediate stage final in child-done comment (MUL-4062) (#4932)
The staged child-done system comment derived its "final stage vs next stage" wording from stageProgressSummary over the sub-issues that currently exist. The server has no declarative workflow model — stages are agent-driven and often created lazily (stage N+1's sub-issues are written only after stage N produces the inputs they depend on), so an intermediate stage reaches nextStage==0 exactly like a true final stage. The old else branch then asserted "This was the final stage. Wrap up the parent", pushing leaders/humans to wrap up mid-workflow (GH #4927). Extract the trailing instruction into stageAdvanceInstruction and, when no later stage exists among the created sub-issues, stop asserting finality: name both possibilities (create the next stage, or wrap up) and hand the decision back to the leader. Add a unit test locking in that the nextStage==0 message never claims a definitive final stage. Co-authored-by: J <j@multica.ai> Co-authored-by: multica-agent <github@multica.ai>
This commit is contained in:
@@ -144,15 +144,7 @@ func (h *Handler) notifyParentOfChildDone(ctx context.Context, prev, issue db.Is
|
||||
if staged {
|
||||
closedStage := issue.Stage.Int32
|
||||
summary, nextStage := stageProgressSummary(children, closedStage)
|
||||
var advance string
|
||||
if nextStage > 0 {
|
||||
advance = fmt.Sprintf(
|
||||
" Stage %d is next. Review the full layout with `multica issue children %s`, and if Stage %d's dependencies are satisfied promote its `backlog` sub-issues to `todo` to continue. Read each sub-issue's description first and only promote items whose stated dependencies are already met — do not rely on this parent's higher-level breakdown alone. If a description conflicts with that breakdown, leave it `backlog` and post a comment to confirm first.",
|
||||
nextStage, parentID, nextStage,
|
||||
)
|
||||
} else {
|
||||
advance = " This was the final stage. Wrap up the parent — synthesize the results and move it forward, or close it out if nothing remains."
|
||||
}
|
||||
advance := stageAdvanceInstruction(nextStage, parentID)
|
||||
content = fmt.Sprintf(
|
||||
"%sStage %d of this issue is complete — its last sub-issue [%s](mention://issue/%s) — \"%s\" — just finished. Stage progress — %s.%s",
|
||||
mentionPrefix, closedStage, identifier, childID, title, summary, advance,
|
||||
@@ -300,6 +292,32 @@ func stageProgressSummary(children []db.Issue, closedStage int32) (summary strin
|
||||
return strings.Join(parts, "; "), nextStage
|
||||
}
|
||||
|
||||
// stageAdvanceInstruction returns the trailing instruction appended to a
|
||||
// staged child-done system comment, given the next stage with pending work
|
||||
// among the sub-issues that currently exist (nextStage, 0 = none).
|
||||
//
|
||||
// - nextStage > 0: a later stage with unfinished work already exists, so
|
||||
// point the leader at it.
|
||||
// - nextStage == 0: no later stage exists *among the sub-issues created so
|
||||
// far*. This deliberately does NOT assert that the workflow is finished.
|
||||
// The server has no declarative workflow model — stages are agent-driven
|
||||
// and often created lazily (stage N+1's sub-issues are only written after
|
||||
// stage N produces the inputs they depend on), so an intermediate stage in
|
||||
// such a pipeline reaches nextStage == 0 exactly like a true final stage
|
||||
// does. The old wording ("This was the final stage. Wrap up the parent")
|
||||
// asserted a finality the server cannot know and pushed leaders to wrap up
|
||||
// mid-workflow (MUL-4062 / #4927). The message now names both possibilities
|
||||
// and hands the create-next-vs-wrap-up decision back to the leader.
|
||||
func stageAdvanceInstruction(nextStage int32, parentID string) string {
|
||||
if nextStage > 0 {
|
||||
return fmt.Sprintf(
|
||||
" Stage %d is next. Review the full layout with `multica issue children %s`, and if Stage %d's dependencies are satisfied promote its `backlog` sub-issues to `todo` to continue. Read each sub-issue's description first and only promote items whose stated dependencies are already met — do not rely on this parent's higher-level breakdown alone. If a description conflicts with that breakdown, leave it `backlog` and post a comment to confirm first.",
|
||||
nextStage, parentID, nextStage,
|
||||
)
|
||||
}
|
||||
return " Completing this stage does not mean the whole issue is done. Decide whether the issue is actually complete — if so, wrap up the parent (synthesize the results and move it forward, or close it out) — or whether the next stage still needs to be created, in which case create that stage and its sub-issues now."
|
||||
}
|
||||
|
||||
// sanitizeChildTitleForSystemComment removes mention-style markdown from a
|
||||
// child issue's title before it is embedded into the parent's system
|
||||
// comment. Smuggled mentions are already harmless on the listener path
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package handler
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
@@ -146,6 +147,40 @@ func TestStageProgressSummary_SkipsUnstaged(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// stageAdvanceInstruction must point at a known next stage when one exists,
|
||||
// and — the core of MUL-4062 — must NOT assert finality when no later stage
|
||||
// exists yet, because a lazily-created intermediate stage reaches nextStage==0
|
||||
// exactly like a true final stage does.
|
||||
func TestStageAdvanceInstruction(t *testing.T) {
|
||||
const parentID = "parent-uuid"
|
||||
|
||||
t.Run("a known next stage points the leader at it", func(t *testing.T) {
|
||||
got := stageAdvanceInstruction(3, parentID)
|
||||
if !strings.Contains(got, "Stage 3 is next") {
|
||||
t.Fatalf("expected next-stage instruction, got %q", got)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("no created next stage does not assert finality", func(t *testing.T) {
|
||||
got := stageAdvanceInstruction(0, parentID)
|
||||
// Regression guard for MUL-4062: an intermediate stage in a lazily
|
||||
// created workflow also reaches nextStage==0, so the message must not
|
||||
// claim this was definitively the final stage.
|
||||
if strings.Contains(got, "This was the final stage") {
|
||||
t.Fatalf("must not assert finality when the workflow shape is unknown, got %q", got)
|
||||
}
|
||||
// It must make clear that finishing the stage != the whole issue is
|
||||
// done, and hand both paths (wrap up / create the next stage) to the
|
||||
// leader.
|
||||
if !strings.Contains(got, "does not mean the whole issue is done") {
|
||||
t.Fatalf("expected stage-done != issue-done framing, got %q", got)
|
||||
}
|
||||
if !strings.Contains(got, "next stage") {
|
||||
t.Fatalf("expected create-next-stage guidance, got %q", got)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
// A stage can close because its last open child is *cancelled*, not only
|
||||
// done — a cancelled sibling never finishes, so it must not hold the stage open.
|
||||
func TestStageBarrierClosed_CancelledClosesStage(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user