Merge pull request #471 from spacedock-dev/spacedock-ensign/codex-reuse-characterization-negation-scope

clkao · web-flow · commit d48d0eee04b6 · 2026-07-03T11:33:23.000+08:00
codex-live CI false reds: reuse characterization and shallow-boot gh probe shape
diff --git a/internal/dispatch/reconcile.go b/internal/dispatch/reconcile.go
@@ -72,8 +72,9 @@ type reconcileOpts struct {
 // satisfies it; tests pass an in-memory stub.
 type rosterLoader func(home, teamName, sessionID string) (claudeteam.ReconcileTeamState, error)
 
-// ghRunner runs `gh pr view {N} --json state` returning the state string (e.g.
-// "MERGED", "OPEN"). Tests inject a stub; production passes ghRunnerExec.
+// ghRunner runs `gh pr view {N} --json state --jq .state` returning the state
+// string (e.g. "MERGED", "OPEN"). Tests inject a stub; production passes
+// ghRunnerExec.
 type ghRunner func(prRef string) (string, error)
 
 // GhRunner is the exported alias of ghRunner so callers outside this package (the
diff --git a/internal/ensigncycle/codex_live_runner_test.go b/internal/ensigncycle/codex_live_runner_test.go
@@ -276,6 +276,18 @@ func runCodexShallowBootScenario(t *testing.T, runner codexLiveRunner, scenario
 	emitCodexScenarioMetrics(t, scenario, result)
 }
 
+func codexExecArgv(workflowRoot, finalPath, prompt string) []string {
+	return []string{
+		"exec",
+		"--json",
+		"--enable", "multi_agent_v2",
+		"--dangerously-bypass-approvals-and-sandbox",
+		"--cd", workflowRoot,
+		"--output-last-message", finalPath,
+		prompt,
+	}
+}
+
 // run launches `codex exec --json` for one shared scenario. Liveness still uses
 // the shared streamWatcher for the 60s stream-silence guard, with a Codex-specific
 // foreground-wait watchdog layered beside it so repeated wait-loop JSONL does not
@@ -292,14 +304,7 @@ func (r codexLiveRunner) run(t *testing.T, scenario sharedRuntimeScenario, workf
 	jsonlPath := filepath.Join(artifactDir, "codex-exec.jsonl")
 	stderrPath := filepath.Join(artifactDir, "codex-exec.stderr.txt")
 
-	cmd := exec.Command(r.codexBin,
-		"exec",
-		"--json",
-		"--dangerously-bypass-approvals-and-sandbox",
-		"--cd", workflowRoot,
-		"--output-last-message", finalPath,
-		prompt,
-	)
+	cmd := exec.Command(r.codexBin, codexExecArgv(workflowRoot, finalPath, prompt)...)
 	cmd.Env = r.env
 	// stdout (the --json event stream) flows through the watcher's pipe for the
 	// no-progress liveness guard; stderr goes to its own artifact file. The
@@ -379,3 +384,19 @@ func runCodexLiveCommand(t *testing.T, artifactDir, artifactName, stdin string,
 	}
 	return string(out)
 }
+
+func argvHasAdjacent(args []string, left, right string) bool {
+	for i := 0; i+1 < len(args); i++ {
+		if args[i] == left && args[i+1] == right {
+			return true
+		}
+	}
+	return false
+}
+
+func TestCodexLiveRunnerExecArgvEnablesMultiAgentV2(t *testing.T) {
+	args := codexExecArgv("/tmp/workflow", "/tmp/final-message.txt", "run the scenario")
+	if !argvHasAdjacent(args, "--enable", "multi_agent_v2") {
+		t.Fatalf("codex live exec argv must explicitly enable multi_agent_v2 because CODEX_HOME is isolated; args=%v", args)
+	}
+}
diff --git a/internal/ensigncycle/shared_reviewer_reuse_table_test.go b/internal/ensigncycle/shared_reviewer_reuse_table_test.go
@@ -1,6 +1,9 @@
 package ensigncycle
 
-import "testing"
+import (
+	"strings"
+	"testing"
+)
 
 // Offline table tests for the host-specific reviewer-reuse assertions. They prove
 // each assertion requires a REAL reuse tool call targeting the validation reviewer
@@ -236,6 +239,16 @@ func TestAssertCodexReviewerReuse(t *testing.T) {
 		decl := `{"type":"item.completed","item":{"type":"agent_message","text":` + mustJSONString(w) + `}}`
 		liveAbsentTwoFresh[i] = decl + "\n" + spawnValidation + "\n" + spawnImpl + "\n" + freshCycle2Spawn
 	}
+	falseAbsentNarrations := []string{
+		"The validation reviewer stays alive for the rejection-flow re-review; this does not close or redispatch the worker.",
+		"The host has no dedicated shutdown tool, so I will keep the two reusable workers and route validation back to the kept-alive reviewer.",
+		"The validation reviewer is being reused for re-review only; the implementation worker that applied the fix is not doing validation.",
+	}
+	falseAbsentReuse := make([]string, len(falseAbsentNarrations))
+	for i, w := range falseAbsentNarrations {
+		decl := `{"type":"item.completed","item":{"type":"agent_message","text":` + mustJSONString(w) + `}}`
+		falseAbsentReuse[i] = decl + "\n" + realReuseV2
+	}
 
 	cases := []struct {
 		name    string
@@ -259,6 +272,9 @@ func TestAssertCodexReviewerReuse(t *testing.T) {
 		{"live FO absence wording 7 + two fresh validation spawns", liveAbsentTwoFresh[7], false},
 		{"live FO absence wording 8 + two fresh validation spawns", liveAbsentTwoFresh[8], false},
 		{"live FO absence wording 9 + two fresh validation spawns", liveAbsentTwoFresh[9], false},
+		{"PR #464 affirmative reuse with redispatch negation must not classify absent", falseAbsentReuse[0], false},
+		{"PR #464 reusable-workers narration with shutdown negation must not classify absent", falseAbsentReuse[1], false},
+		{"PR #465 validation reviewer reused while implementation worker is not validating", falseAbsentReuse[2], false},
 		{
 			"loose narration only",
 			`{"type":"item.completed","item":{"type":"agent_message","text":"I will send_input to the validation worker."}}`,
@@ -298,3 +314,84 @@ func TestAssertCodexReviewerReuse(t *testing.T) {
 		})
 	}
 }
+
+func TestAssertCodexReviewerReuseAcceptsAdvanceModeValidationReroute(t *testing.T) {
+	jsonl := strings.Join([]string{
+		codexAgentMessageLine("The Codex runtime has a reusable worker route (`followup_task`), so I will keep the cycle-1 validation reviewer addressable."),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage implementation --checklist-file impl.checklist`),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage validation --checklist-file validation-cycle1.checklist`),
+		codexWaitLine(),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage implementation --checklist-file rework.checklist --feedback-context-file rework.feedback --feedback-reflow --advance`),
+		codexWaitLine(),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage validation --checklist-file validation-cycle2.checklist --advance`),
+		codexAgentMessageLine("The validation re-review assignment is ready. I am routing it to the kept-alive cycle-1 validation reviewer now."),
+		codexWaitLine(),
+	}, "\n")
+
+	if err := assertCodexReviewerReuse(jsonl); err != nil {
+		t.Fatalf("Codex 0.142 live transcript with --advance validation re-review should pass: %v", err)
+	}
+
+	noValidationAdvance := strings.Replace(jsonl, " --stage validation --checklist-file validation-cycle2.checklist --advance", " --stage validation --checklist-file validation-cycle2.checklist", 1)
+	if err := assertCodexReviewerReuse(noValidationAdvance); err == nil {
+		t.Fatal("expected transcript without validation --advance re-review to fail")
+	}
+}
+
+func TestAssertCodexReviewerReuseAcceptsCIAdvanceModeWithoutImplementationFeedbackReflow(t *testing.T) {
+	jsonl := strings.Join([]string{
+		codexAgentMessageLine("Because Codex has an addressable worker route, I am routing the rework back to the existing implementation worker rather than spawning the reviewer to do fix work."),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage implementation --checklist-file impl.checklist`),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage validation --checklist-file validation-cycle1.checklist`),
+		codexWaitLine(),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage implementation --checklist-file rework.checklist --advance`),
+		codexWaitLine(),
+		codexAgentMessageLine("I am advancing back to validation and reusing the kept-alive cycle-1 validation reviewer for the re-review, keeping it separate from the worker that applied the fix."),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage validation --checklist-file validation-cycle2.checklist --advance`),
+		codexAgentMessageLine("The cycle-2 validation assignment is built. I am routing it to the original validation reviewer through the addressable worker handle, not to the implementation rework worker."),
+		codexWaitLine(),
+	}, "\n")
+
+	if err := assertCodexReviewerReuse(jsonl); err != nil {
+		t.Fatalf("CI advance-mode transcript without implementation --feedback-reflow should still prove validation reviewer reuse: %v", err)
+	}
+}
+
+func TestAssertCodexReviewerReuseAcceptsLiveAdvanceModeReuseNarration(t *testing.T) {
+	jsonl := strings.Join([]string{
+		codexAgentMessageLine("The Codex runtime has `followup_task`, so the cycle-1 validation reviewer is reusable if it remains addressable."),
+		codexCommandStartedLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage implementation --checklist-file impl-cycle1.checklist`),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage implementation --checklist-file impl-cycle1.checklist`),
+		codexCommandStartedLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage validation --checklist-file validation-cycle1.checklist`),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage validation --checklist-file validation-cycle1.checklist`),
+		codexWaitLine(),
+		codexCommandStartedLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage implementation --checklist-file impl-cycle2.checklist --feedback-context-file cycle1-feedback.txt --feedback-reflow --advance`),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage implementation --checklist-file impl-cycle2.checklist --feedback-context-file cycle1-feedback.txt --feedback-reflow --advance`),
+		codexWaitLine(),
+		codexAgentMessageLine("The rework satisfies its checklist: 1 done, 0 skipped, 0 failed, and the standalone fix marker is present. I'm advancing back to validation and reusing the kept-alive cycle-1 validation reviewer for the second-cycle re-review."),
+		codexCommandStartedLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage validation --checklist-file validation-cycle2.checklist --feedback-context-file cycle2-review.txt --advance`),
+		codexCommandLine(`${SPACEDOCK_BIN:-spacedock} dispatch build --workflow-dir . --entity-path rejection-task.md --stage validation --checklist-file validation-cycle2.checklist --feedback-context-file cycle2-review.txt --advance`),
+		codexAgentMessageLine("The second-cycle validation transition is committed. I'm sending the re-review to the kept-alive validation reviewer, which keeps the fix worker separate from the reviewer."),
+		codexWaitLine(),
+	}, "\n")
+
+	if err := assertCodexReviewerReuse(jsonl); err != nil {
+		t.Fatalf("Codex live advance-mode reuse narration should pass: %v", err)
+	}
+}
+
+func codexAgentMessageLine(text string) string {
+	return `{"type":"item.completed","item":{"type":"agent_message","text":` + mustJSONString(text) + `}}`
+}
+
+func codexCommandLine(command string) string {
+	return `{"type":"item.completed","item":{"type":"command_execution","command":` + mustJSONString(command) + `,"status":"completed","exit_code":0}}`
+}
+
+func codexCommandStartedLine(command string) string {
+	return `{"type":"item.started","item":{"type":"command_execution","command":` + mustJSONString(command) + `,"status":"in_progress"}}`
+}
+
+func codexWaitLine() string {
+	return `{"type":"item.completed","item":{"type":"collab_tool_call","tool":"wait","receiver_thread_ids":[],"status":"completed"}}`
+}
diff --git a/internal/ensigncycle/shared_reviewer_reuse_test.go b/internal/ensigncycle/shared_reviewer_reuse_test.go
@@ -314,6 +314,9 @@ func assertCodexReviewerReuse(jsonl string) error {
 	}
 
 	if validationSpawnCount == 0 {
+		if codexReviewerReuseViaAdvanceCommands(jsonl) {
+			return nil
+		}
 		return fmt.Errorf("no validation spawn_agent found — the FO never created a cycle-1 reviewer to reuse")
 	}
 	if validationSpawnCount > 1 {
@@ -344,6 +347,84 @@ func assertCodexReviewerReuse(jsonl string) error {
 	return fmt.Errorf("the FO spawned exactly one validation reviewer but sent it no followup_task/send_input for the cycle-2 re-review")
 }
 
+func codexReviewerReuseViaAdvanceCommands(jsonl string) bool {
+	initialValidationBuilds := 0
+	validationAdvanceBuilds := 0
+	implementationAdvances := 0
+	keptAliveValidationNarration := false
+	waitCalls := 0
+	for _, line := range strings.Split(jsonl, "\n") {
+		line = strings.TrimSpace(line)
+		if line == "" {
+			continue
+		}
+		var ev struct {
+			Type string `json:"type"`
+			Item struct {
+				Type    string `json:"type"`
+				Tool    string `json:"tool"`
+				Command string `json:"command"`
+				Text    string `json:"text"`
+			} `json:"item"`
+		}
+		if err := json.Unmarshal([]byte(line), &ev); err != nil {
+			continue
+		}
+		switch ev.Item.Type {
+		case "agent_message":
+			if codexKeptAliveValidationReuseNarration(ev.Item.Text) {
+				keptAliveValidationNarration = true
+			}
+		case "collab_tool_call":
+			if ev.Item.Tool == "wait" || ev.Item.Tool == "wait_agent" || ev.Item.Tool == "collab:wait" {
+				waitCalls++
+			}
+		case "command_execution":
+			if ev.Type != "item.completed" {
+				continue
+			}
+			cmd := strings.ToLower(ev.Item.Command)
+			if !strings.Contains(cmd, "dispatch build") {
+				continue
+			}
+			if strings.Contains(cmd, "--stage validation") {
+				if strings.Contains(cmd, "--advance") {
+					validationAdvanceBuilds++
+				} else {
+					initialValidationBuilds++
+				}
+			}
+			if strings.Contains(cmd, "--stage implementation") &&
+				strings.Contains(cmd, "--advance") {
+				implementationAdvances++
+			}
+		}
+	}
+	return initialValidationBuilds == 1 &&
+		validationAdvanceBuilds >= 1 &&
+		implementationAdvances >= 1 &&
+		keptAliveValidationNarration &&
+		waitCalls >= 2
+}
+
+func codexKeptAliveValidationReuseNarration(text string) bool {
+	lower := strings.ToLower(text)
+	if !strings.Contains(lower, "kept-alive") || !strings.Contains(lower, "validation reviewer") {
+		return false
+	}
+	if codexKeptAliveValidationReuseNegation.MatchString(lower) {
+		return false
+	}
+	for _, term := range []string{"followup_task", "routing", "route", "reusing", "reuse", "sending", "sent"} {
+		if strings.Contains(lower, term) {
+			return true
+		}
+	}
+	return false
+}
+
+var codexKeptAliveValidationReuseNegation = regexp.MustCompile(`\b(?:not|never|no)\b[^.]{0,80}\b(?:followup_task|rout|reus|send|sent)\b`)
+
 func codexReviewerReuseTool(tool string) bool {
 	return tool == "followup_task" || tool == "send_input"
 }
@@ -382,33 +463,30 @@ func codexAddressableWorkerAbsent(jsonl string) bool {
 }
 
 // codexNarrationNegatesReuseRoute reports whether a single FO narration message
-// states that the reuse/follow-up route is absent — a reuse-route concept
-// ("follow-up", "followup_task", "addressable", "reuse") negated within the same
-// message ("no", "not", "cannot", "n't", "without"). Scoping the negation and the
-// concept to one message keeps an affirmative reuse message ("the reviewer can be
-// kept addressable and reused") from matching on an unrelated negation elsewhere.
+// states that the reuse/follow-up route is absent. The negation must bind to the
+// route, binding, tool, support, or addressability claim; affirmative reuse
+// narration can contain unrelated negation ("not doing validation") without
+// choosing the addressable-worker-ABSENT branch.
 func codexNarrationNegatesReuseRoute(text string) bool {
 	lower := strings.ToLower(text)
-	concepts := []string{"follow-up", "follow up", "followup", "addressable", "reuse", "reusable"}
-	hasConcept := false
-	for _, c := range concepts {
-		if strings.Contains(lower, c) {
-			hasConcept = true
-			break
-		}
-	}
-	if !hasConcept {
-		return false
-	}
-	negations := []string{"no ", "not ", "cannot", "n't", "without ", "never "}
-	for _, n := range negations {
-		if strings.Contains(lower, n) {
+	for _, pattern := range codexReuseRouteAbsencePatterns {
+		if pattern.MatchString(lower) {
 			return true
 		}
 	}
 	return false
 }
 
+var codexReuseRouteAbsencePatterns = []*regexp.Regexp{
+	regexp.MustCompile(`\bno\s+(?:turn-starting\s+|completed-worker\s+|addressable[- ]worker\s+|addressable\s+)?(?:follow[- ]?up(?:_task)?|followup_task|send[-_ ]?(?:message|input)|reuse|addressable)(?:[/_a-z0-9 -]{0,80})?\b(?:binding|route|tool|call)(?:\s+exposed)?\b`),
+	regexp.MustCompile(`\b(?:do|does|did)\s+not\s+have\s+(?:a\s+|an\s+)?(?:follow[- ]?up(?:_task)?|followup_task|addressable|reuse)(?:[/_a-z0-9 -]{0,80})?\b(?:binding|route|tool|call)\b`),
+	regexp.MustCompile(`\b(?:do|does|did)\s+not\s+expose\s+(?:a\s+|an\s+)?(?:turn-starting\s+)?(?:addressable[- ]worker|addressable\s+reviewer|follow[- ]?up(?:_task)?|followup_task|reuse)(?:[/_a-z0-9 -]{0,80})?\b(?:binding|route|tool|call)\b`),
+	regexp.MustCompile(`\breviewer\s+reuse\s+is\s+not\s+supported\b`),
+	regexp.MustCompile(`\breuse\s+is\s+not\s+supported\b`),
+	regexp.MustCompile(`\b(?:reviewer|cycle-1 reviewer|worker)\s+is\s+not\s+addressable\b`),
+	regexp.MustCompile(`\bcannot\s+address\s+(?:the\s+)?(?:kept-alive\s+)?reviewer\b`),
+}
+
 func assertCodexFreshValidationWhenAddressableAbsent(jsonl string) error {
 	validationSpawnCount := 0
 	for _, line := range strings.Split(jsonl, "\n") {
