Skip to content

Route no-branch-work reviews to self-approval#320

Draft
sepo-agent-app[bot] wants to merge 1 commit into
mainfrom
agent/implement-issue-318/codex-26152378805
Draft

Route no-branch-work reviews to self-approval#320
sepo-agent-app[bot] wants to merge 1 commit into
mainfrom
agent/implement-issue-318/codex-26152378805

Conversation

@sepo-agent-app

Copy link
Copy Markdown

Summary

  • Add structured review required-branch-work metadata to orchestrator handoffs.
  • Route MINOR_ISSUES + NO_AUTOMATED_ACTION + no required branch work to agent-self-approve when self-approval is enabled.
  • Extend self-approval provenance checks, prompts, docs, and regression coverage for disabled, required-work, and blocking cases.

Verification

  • npm --prefix .agent run build
  • node --test .agent/dist/__tests__/prepare-self-approve-cli.test.js .agent/dist/__tests__/resolve-self-approve-cli.test.js .agent/dist/__tests__/self-approval.test.js .agent/dist/__tests__/handoff.test.js .agent/dist/__tests__/dispatch-agent-orchestrator-cli.test.js .agent/dist/__tests__/orchestrate-handoff-cli.test.js .agent/dist/__tests__/envelope.test.js
  • git diff --check

Closes #318

Part of #314

@sepo-agent-app

sepo-agent-app Bot commented May 20, 2026

Copy link
Copy Markdown
Author

Sepo is dispatching follow-up automation.

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

Reason: implementation succeeded; dispatching review

@sepo-agent-app

Copy link
Copy Markdown
Author

Rubrics Review

Total Score Verdict Rubrics Scored
95 PASS 8
Dimension Rubric Result Score Evidence
coding_workflow / generic Separate decision gates from action authorization pass 8/8 Review NO_AUTOMATED_ACTION now routes eligible no-required-work cases to agent-self-approve, while approval still depends on self-approval provenance and resolver checks: handoff.ts, self-approval.ts.
coding_workflow / generic Prefer explicit structured inputs pass 7/7 Adds a typed source_required_branch_work workflow input and passes it into both planner and deterministic dispatch paths: agent-orchestrator.yml, agent-orchestrator.yml.
review_quality / generic Ground review synthesis in current artifacts partial 6/8 Self-approval provenance still requires trusted current-head review synthesis before trusting the new gate: self-approval.ts. Minor caveat: deriveReviewRequiredBranchWork() returns false for any present Action Items section with no extracted required items, not only an explicit no-work sentinel: handoff.ts.
coding_workflow / generic Validate delegated route authorization pass 8/8 The new delegated route is constrained to review-originated MINOR_ISSUES + NO_AUTOMATED_ACTION + false required work and keeps required-work paths on fix-pr; tests cover allowed and denied planner handoffs: handoff.test.ts.
coding_workflow / generic Prefer agent-derived decisions pass 7/7 Uses existing planner/deterministic handoff policy and current source metadata rather than adding a new repo-level setting; the planner prompt receives the new source fact directly: agent-orchestrator.md.
coding_workflow / generic Keep docs in sync pass 7/7 Updates architecture and technical docs to clarify that review NO_AUTOMATED_ACTION can still mean an approval gate is useful: agent-orchestrator.md, agent-orchestrator.md.
coding_workflow / generic Reuse existing code pass 8/8 Extends existing handoff parsing/building helpers and self-approval provenance evaluation instead of creating a parallel parser or route mechanism: dispatch-agent-orchestrator.ts.
coding_workflow / generic Make surgical changes pass 8/8 Changes are scoped to orchestrator handoff metadata, self-approval provenance, prompts/docs, workflow plumbing, and focused regression tests for the requested behavior.

Notes

  • The PR satisfies the core Route no-more-branch-work review outcomes to self-approval #318 acceptance criteria: enabled self-approval receives eligible no-branch-work review outcomes, disabled self-approval stops cleanly, and required-work or blocking outcomes do not use the approval shortcut.
  • The strongest evidence is the combination of deterministic policy tests, planner validation tests, prepare/resolve self-approval provenance tests, and docs updates.
  • Smallest useful follow-up: tighten deriveReviewRequiredBranchWork() so false requires either an explicit no-required-work sentinel or another clearly validated no-work signal, rather than an empty Action Items section.

Findings

  • WARNING: The new no-required-work classifier can treat an empty present ## Action Items section as source_required_branch_work=false because deriveReviewRequiredBranchWork() returns false when the section exists and extractRequiredReviewActionItems() is empty. That is probably acceptable given the self-approval agent still performs the final decision, but it is slightly looser than the issue language requiring the synthesis/action items to say no required branch-change work remains.

Final Rubric Verdict

PASS

@sepo-agent-app

Copy link
Copy Markdown
Author

AI Review Synthesis

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

Summary of PR/Issue

PR #320 implements #318 as part of the stacked #314 batch. It teaches orchestration that review NO_AUTOMATED_ACTION means no more branch mutation/review loop is useful, while still allowing agent-self-approve when self-approval is enabled and a trusted current-head MINOR_ISSUES synthesis says no required branch-change work remains. The implementation threads source_required_branch_work through dispatch/orchestrator handoff, re-validates provenance in self-approval, and updates prompts, workflow wiring, docs, and focused regression tests. This preserves the key safety boundary: routing metadata can trigger the gate, but approval still depends on current-head trusted synthesis checks.

Review

The independent reviews converge on SHIP: Codex found no findings, Claude found only low-risk non-blocking notes, and the rubric review passed with one optional classifier-tightening caveat. I did not find a currently actionable branch-change issue supported by the review artifacts or the live PR state.

Issue Severity Description
No actionable issues INFO No required branch-change work remains. The only reviewer caveats are optional hardening notes around no-required-work classification breadth and extra conservative-path test coverage.

Progress

  • PR already addresses Route no-more-branch-work review outcomes to self-approval #318’s acceptance criteria for enabled self-approval, disabled self-approval, blocking outcomes, and required-work outcomes.
  • Review artifacts report targeted build/test verification passed on head 926f279393b2668b8542208494fc8b6fc1d306df.
  • Existing inline comments: none. Existing review threads: none.
  • Inline cleanup: no comments posted, no replies added, no comments minimized, and no threads resolved.

Recommended Next Step

NO_AUTOMATED_ACTION: No unresolved required branch-change work remains; further branch mutation automation is not warranted.

Final Verdict

SHIP

Action Items

  • No required branch-change work remains.

@lolipopshock

Copy link
Copy Markdown
Contributor

@sepo-agent /answer I don't get it why it turned to be such a big PR? Consider simplify it? Also check how we simplified the pr for human decision -> self approval? (Check related merged and closed PRs

@sepo-agent-app

Copy link
Copy Markdown
Author

I agree this PR can be simplified.

Why it got big: #320 adds a new derived signal, source_required_branch_work, and threads it through dispatch, orchestrator workflow inputs, prompts, handoff policy, self-approval provenance, docs, and tests. Most of the size is regression coverage: 454 of 641 additions are tests. But the structural growth comes from classifying action-item text in handoff.ts, deriving requiredBranchWork in dispatch, and adding a second self-approval provenance path.

The related HUMAN_DECISION -> self-approval work was simplified by closing the larger intermediate attempts (#264/#265) and merging #287. The key simplification in #287 was: route on one structured review signal, source_recommended_next_step=HUMAN_DECISION, then let agent-self-approve be the decision gate. See #287’s simple handoff branch in handoff.ts and trusted synthesis check in self-approval.ts.

Recommended simplification for #320:

  • Prefer changing review synthesis semantics instead of adding a new orchestrator signal.
  • When a review has no required branch-change work but still needs an approval/judgment gate, have synthesis emit Recommended Next Step: HUMAN_DECISION, not NO_AUTOMATED_ACTION.
  • Then Route HUMAN_DECISION reviews to self-approval #287’s existing route already handles it: review + HUMAN_DECISION + self-approval enabled -> agent-self-approve.
  • Keep NO_AUTOMATED_ACTION for true terminal states where no approval/judgment gate should run.
  • Drop source_required_branch_work, normalizeRequiredBranchWork, the orchestrator workflow input, and most of the new action-item classifier plumbing from Route no-branch-work reviews to self-approval #320.

If we still want to preserve MINOR_ISSUES + NO_AUTOMATED_ACTION as a supported route, I’d still simplify by making self-approval preflight the only place that verifies “trusted current-head synthesis says no required branch work.” The orchestrator should not need to parse action-item wording and carry that as a workflow input.

@sepo-agent-app sepo-agent-app Bot changed the base branch from agent/implement-issue-313/codex-26151433746 to main May 20, 2026 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent Handled by the agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Route no-more-branch-work review outcomes to self-approval

2 participants