Finalize terminal PR orchestration notes

[sepo-agent-app]

Summary

• finalize terminal PR stops in the current progress note or trusted marker note
• use the job token for non-recursive progress updates
• mention human requesters and include cumulative planner summaries
• collapse older trusted artifacts across agent and workflow identities only after exact successes with substantive summaries
• preserve non-success context, formal reviews, and child-report behavior
• update focused documentation and regression coverage

Verification

• npm --prefix .agent run build
• 90 focused handoff and minimizer tests
• 775 runtime tests
• 94 workflow checks
• 5 documentation checks
• git diff --check

Closes #452

[sepo-agent-app]

Sepo is dispatching follow-up automation.

Source	Next	Target	Round	Status
implement	review	PR #505	3 / 40	Dispatched

Reason: agent planner selected review: Implementation succeeded, produced open draft PR #505, and its checks pass; an independent review is the clearly warranted next step.

[sepo-agent-app]

databaseId is nullable, and Number(null) is 0, so this filter treats a node with no ordering evidence as older than the current note. Because marker-bearing final notes are included by the matcher, cleanup can minimize the just-published final note when its node lacks a database ID. Please fail closed by requiring a non-null, positive safe integer before comparing, and add null/missing-ID regression coverage.

[sepo-agent-app]

userMessage is already empty here for a PR that is closed before this step: decidePlannerOrchestration returns on the status check before reading the planner response. Since the workflow runs the planner first, a PR that closes during planning can have a valid stop summary that this final note discards. Please preserve summary fields from a parsed planner stop while still forcing the closed-state stop, with a close-during-planning regression.

[sepo-agent-app]

Rubrics Review

Total Score	Verdict	Rubrics Scored
92	PARTIAL	13

Dimension	Rubric	Result	Score	Evidence
coding_workflow / generic	Generate context-derived artifact metadata	partial	5/7	Final notes include requester, planner summary, outcome, target, round, reason, and run ID. However, the closed-PR status gate returns before parsing the planner response, dropping an available cumulative summary.
coding_workflow / generic	Read docs and linked context first	pass	6/6	The implementation follows the focused requirements in #452 and the lean single-job constraints in #496.
coding_workflow / generic	Leave release notes to release preparation	pass	8/8	The seven-file diff contains no changelog or release-note changes.
communication / generic	Keep status comments compact	pass	6/6	The formatter uses a concise heading, summary, stable metadata bullets, and one next-step sentence.
coding_workflow / generic	Make surgical changes	pass	8/8	Changes are limited to existing orchestration, minimization, focused tests, prompt/workflow wiring, and two directly relevant documentation pages.
coding_workflow / generic	Reuse existing code	pass	8/8	The handoff CLI reuses existing progress merging, marker upsert, footer, GraphQL, and minimization helpers through its existing module imports.
coding_workflow / generic	Separate decision gates from action authorization	pass	8/8	Success eligibility does not treat SHIP or approval as terminal while separately authorized self-approval or self-merge remains enabled.
coding_workflow / generic	Make user-facing automation idempotent	partial	4/7	Progress notes are reused and fallback notes are marker-upserted, but nullable ordering IDs can select the current note for minimization, and updating an older fallback note retains a stale cleanup boundary.
coding_workflow / generic	Keep docs in sync	pass	7/7	The architecture and workflow documentation describe final-note reuse, requester mentions, success gating, and cleanup exclusions.
coding_workflow / generic	Prefer explicit structured inputs	pass	7/7	The workflow wiring passes comment ID, merge mode, job token, planner response, handle, and model display as separate fields.
coding_workflow / generic	Surface stateful failures	partial	5/7	Publication failures are fatal and cleanup identities warn independently, but missing databaseId values are silently coerced to zero instead of failing closed.
coding_workflow / generic	Comment on accepted workflow stops	pass	6/6	Ordinary terminal PR stops now publish a visible final note, including parent-resolution and publication-failure regressions.
coding_style / generic	Prefer simple implementations	pass	7/7	The feature remains in the existing single-job orchestrator without new jobs, dependencies, subsystems, or public configuration.

Notes

• Weighted score: 85/92, rounded to 92. Exact head 6ca9c6ff retains the intended seven-file scope, and its hosted check passed the build, 775 runtime tests, 94 workflow checks, five documentation checks, and git diff --check.
• The main finalization, identity separation, non-success preservation, formal-review exclusion, and child-report paths are sound.
• Smallest useful follow-up: fail closed on null/missing/non-positive ordering IDs and preserve a validated planner stop summary when the PR closes during planning, with one regression for each case.

Findings

• WARNING: Number(node.databaseId) converts null to 0. Because marker-bearing final notes match cleanup, a trusted current note lacking ordering evidence can be minimized. Require a non-null, positive safe integer before comparison and explicitly cover null and missing IDs.
• WARNING: decidePlannerOrchestration returns on a closed PR before reading the planner response. A PR closing during planning therefore gets a generic note without the already-produced cumulative summary.
• INFO: When fallback upsert updates an older marker note, cleanup uses that note’s original database-ID cutoff, so generated artifacts created afterward may remain visible. This is safely incomplete but prevents full rerun convergence.

Final Rubric Verdict

PARTIAL

---

codex | gpt-5.6-sol[max] | Macmini-runner-1

[sepo-agent-app]

AI Review Synthesis

Dual-agent review by Claude and Codex.
Requested by @lolipopshock.

Summary of PR/Issue

PR #505 implements the lean, single-job terminal PR finalization requested by #452 and refined in #496. It reuses the current progress or marker note, mentions human requesters, preserves planner summaries, and conditionally collapses older trusted automation artifacts. Existing permissions, formal reviews, and child-report behavior remain intact, with focused documentation and regression coverage. The current draft is mergeable at 6ca9c6ff, and its Test Scripts check passes.

Review

The rubrics, Claude, and Codex reviews converge on two bounded correctness warnings while finding the primary architecture sound. Both warnings remain present at the current head; two safely deferrable cleanup and summary-quality limitations are informational.

Issue	Severity	Disposition	Description
Missing ordering IDs can hide the final note	WARNING	FIX_IN_PR	Number(null) becomes 0, allowing a marker-bearing comment without ordering evidence to be treated as older and minimized.
Closed-PR finalization drops a planner summary	WARNING	FIX_IN_PR	The closed-state gate returns before reading an already-produced planner response, omitting a valid cumulative stop summary.
Reused fallback notes limit cleanup convergence	INFO	FOLLOW_UP	Updating an older marker note retains its original database-ID cutoff, so newer generated artifacts can remain visible.
Summary-quality gate accepts any non-empty text	INFO	FOLLOW_UP	Cleanup treats every non-empty userMessage as substantive; the planner prompt mitigates this, making stricter validation optional hardening.

Progress

• The implementation already uses the job token for progress replacement, cleans both agent and workflow identities, and requires a non-empty planner summary before cleanup.
• Exact head 6ca9c6ff passes the hosted build/test checks and git diff --check; no release notes were changed.
• Existing unresolved inline threads already cover the nullable-ID defect and closed-PR summary loss.
• No new inline comments or replies were posted, and no comments or threads were minimized or resolved.
• There is no meaningful disagreement on the shared findings. Claude alone noted the optional summary-quality hardening; the other current artifacts did not address it.

Issue Details

Missing ordering IDs can hide the final note

Cause: The ordering predicate accepts null as zero. Because the matcher includes terminal-marker notes, cleanup can select the current note when its ordering ID is unavailable.

Candidate solutions:

• Require a non-null, positive safe integer before comparison.
• Add null and missing-ID regressions asserting that no minimization occurs.

Comments: The runtime likelihood is low, but this is a small fail-closed guard protecting the PR’s durable final artifact.

Closed-PR finalization drops a planner summary

Cause: decidePlannerOrchestration checks PR state before calling readPlannerDecision(). If the PR closes during planning, the generated note retains the closed-state reason but loses a valid planner stop summary.

Candidate solutions:

• Parse the planner response once before the status gate.
• Preserve summary fields only from a validated planner stop, while forcing the runtime-controlled closed-state reason and prohibiting dispatch.
• Add a close-during-planning regression.

Comments: This affects the orchestrate source path; normal review, approval, and merge outcomes already retain their summaries.

Reused fallback notes limit cleanup convergence

Cause: Marker upsert can update an old final note whose database ID predates artifacts created afterward. That old ID remains the cleanup boundary.

Candidate solutions:

• In a follow-up, derive an update-aware boundary while explicitly excluding the current final-note node.

Comments: This safely leaves excess context visible rather than hiding required context, so it does not warrant expanding this PR.

Summary-quality gate accepts any non-empty text

Cause: hasSubstantiveTerminalSummary only trims and checks for non-empty text.

Candidate solutions:

• Keep the prompt-enforced contract unless concrete failures emerge.
• If stricter behavior becomes necessary, first define a deterministic substantive-summary contract and add focused tests.

Comments: Tightening this now risks subjective heuristics and unnecessary scope expansion.

Recommended Next Step

FIX_PR: Apply one bounded pass that makes candidate-ID ordering fail closed and preserves validated stop summaries when a PR closes during planning, with focused regressions.

Final Verdict

MINOR_ISSUES

Action Items

• FIX_IN_PR: Reject null, missing, non-positive, or unsafe candidate database IDs before cleanup comparisons, with regression coverage.
• FIX_IN_PR: Preserve a validated planner stop summary when a PR closes during planning without permitting dispatch, with regression coverage.

---

codex | gpt-5.6-sol[max] | Macmini-runner-5