Skip to content

Support orchestrated implement from PRs and discussions#302

Open
lolipopshock wants to merge 4 commits into
mainfrom
agent/orchestrate-nonissue-implement-minimal
Open

Support orchestrated implement from PRs and discussions#302
lolipopshock wants to merge 4 commits into
mainfrom
agent/orchestrate-nonissue-implement-minimal

Conversation

@lolipopshock

Copy link
Copy Markdown
Contributor

Summary

  • Allow /orchestrate on discussion targets and enable planner mode for discussion orchestration.
  • Allow planner-selected implement from PR/discussion orchestration by creating a focused tracking issue, dispatching agent-implement.yml against that issue, and linking back to the original surface.
  • Keep explicit /implement unchanged; this PR only wires the orchestrated path. Adds focused coverage for policy, preflight, and handoff behavior.

Supersedes #294 with a smaller focused implementation.
Closes #227

Tests

  • npm --prefix .agent run build
  • node --test .agent/dist/__tests__/handoff.test.js .agent/dist/__tests__/orchestrate-handoff-cli.test.js .agent/dist/__tests__/orchestrator-preflight-cli.test.js .agent/dist/__tests__/triage.test.js
  • YAML parse for agent-router.yml and agent-orchestrator.yml
  • git diff --check

@lolipopshock

Copy link
Copy Markdown
Contributor Author

@sepo-agent /review compare with #294 is it better?

@sepo-agent-app sepo-agent-app Bot added the agent Handled by the agent label May 20, 2026
@sepo-agent-app

This comment has been minimized.

}

const trackingKey = buildOrchestratedImplementationTrackingKey(metadata);
const existingIssueNumber = findExistingOrchestratedImplementationIssue(repo, trackingKey);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reuse path only checks GitHub issue-body search for the new tracking marker before creating the tracking issue. Issue search can lag, and the PR/discussion link-back has no marker fallback to discover the already-created issue, so a rerun can create a duplicate tracking issue. Please add a trusted link-back marker/comment lookup before creation, or reuse the shared implementation-tracking helper shape from #294.

@sepo-agent-app

This comment has been minimized.

@lolipopshock

Copy link
Copy Markdown
Contributor Author

@sepo-agent /fix-pr can we launch a minimal fix?

@sepo-agent-app

This comment has been minimized.

@lolipopshock

Copy link
Copy Markdown
Contributor Author

@sepo-agent /fix-pr there's a failed test? fix it?

@sepo-agent-app

This comment has been minimized.

@lolipopshock

Copy link
Copy Markdown
Contributor Author

@sepo-agent /review

@sepo-agent-app

Copy link
Copy Markdown

Rubrics Review

Total Score Verdict Rubrics Scored
96 PASS 13
Dimension Rubric Result Score Evidence
coding_workflow / generic Validate delegated route authorization pass 8/8 Initial orchestrate preflight still checks delegated route capability, and tests cover delegated access before dispatch.
coding_workflow / generic Generate context-derived artifact metadata pass 7/7 Non-issue implement handoffs create tracking issues from source target metadata, requester, request text, planner reason/context, target title/body, and base inputs.
coding_workflow / generic Reuse existing code partial 6/8 The PR reuses existing GitHub/comment/discussion helpers and marker patterns, but still keeps substantial tracking-issue orchestration logic inside orchestrate-handoff.ts rather than a shared explicit-implement tracking helper.
coding_workflow / generic Minimal defaults, native escape hatches pass 8/8 Behavior is limited to planner-selected PR/discussion implement, normalizing through a tracking issue and defaulting base selection to the existing implement workflow rules.
coding_workflow / generic Prefer explicit structured inputs pass 7/7 Keeps target_kind, target_number, base_branch, and base_pr as separate validated fields; conflicting base inputs stop visibly.
coding_workflow / generic Prefer agent-derived decisions pass 7/7 Planner policy and runtime validation choose whether PR/discussion orchestration should implement, answer, stop, or block without adding new repo settings.
coding_workflow / generic Separate decision gates from action authorization pass 8/8 Discussion targets can only dispatch implement; PR targets can dispatch review, fix-pr, or implement; invalid planner choices stop before privileged workflow dispatch.
coding_workflow / generic Keep docs in sync pass 7/7 README, action docs, workflow architecture docs, and orchestrator technical docs now describe discussion orchestration and PR/discussion implement tracking issues.
coding_workflow / generic Make user-facing automation idempotent pass 7/7 Tracking issue reuse checks trusted source link-backs and issue markers; discussion stop comments now use trusted exact-body dedupe.
coding_workflow / generic Surface stateful failures pass 7/7 Tracking issue creation failures set stop outputs, log the error, and attempt a visible stop comment.
coding_workflow / generic Comment on accepted workflow stops pass 6/6 Initial discussion orchestrate stops can post visible comments, with regression coverage for duplicate suppression.
coding_workflow / generic Surgical changes pass 8/8 The diff is focused on route policy, orchestration handoff logic, docs/prompts, and targeted tests for issue #227.
coding_style / generic Prefer simple implementations partial 6/7 The implementation is scoped and direct, though the added non-issue tracking path concentrates a fair amount of logic in one CLI.

Notes

  • The follow-up fixes addressed the earlier rubric gaps: canonical docs are now updated, trusted PR/discussion link-back reuse exists before tracking issue creation, and discussion stop comments are deduped.
  • Verification: npm --prefix .agent run build, focused node --test ... after rebuild passed 158/158, and git diff --check origin/main...HEAD passed.
  • No rubric-specific follow-up needed beyond considering a later extraction of the non-issue tracking issue helper if this path grows again.

Findings

  • INFO: reuse-existing-code and simple-implementation are not perfect because the new tracking issue/link-back flow is implemented directly in orchestrate-handoff.ts, but current coverage and scope make this acceptable for the requested minimal implementation.

Final Rubric Verdict

PASS

@sepo-agent-app

Copy link
Copy Markdown

AI Review Synthesis

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

Summary of PR/Issue

Review

The current head is ready to ship. Codex, Claude, and rubrics review outputs agree there are no unresolved BLOCKING or WARNING findings; remaining notes are INFO-level polish only.

Issue Severity Description
No required branch-change issues remain INFO Prior review findings are addressed at current head d8f706d15ecb4688aca270d3aed71c20f286dece, and the GitHub check is passing. Optional notes about helper size, possible future test depth, and one stale PR rejection message are not required ship blockers.

Progress

  • Issue Orchestrator small bugs: #227’s requested minimal behavior is implemented for PR and discussion /orchestrate paths.
  • Prior tracking issue reuse risk is addressed by checking trusted PR/discussion link-backs before issue-body search.
  • Prior discussion stop-comment duplication is addressed with trusted exact-body dedupe.
  • Canonical .agent/docs route and orchestrator documentation now describes discussion orchestration and non-issue implement tracking issues.
  • Re-fetched inline comments and review threads; the remaining older same-agent thread is for the already-fixed tracking issue reuse concern, but viewerCanResolve is false, so no thread mutation was performed.
  • No new inline comments, replies, comment minimizations, or thread resolutions were added.

Recommended Next Step

NO_AUTOMATED_ACTION: no unresolved required branch-change work remains.

Final Verdict

SHIP

Action Items

  • No required branch-change work remains.

@lolipopshock

Copy link
Copy Markdown
Contributor Author

@sepo-agent /answer at a high level, does it implement the function:

  1. allow orchestrate -> implement in PRs (to create a new issue etc)
  2. allow discussion orchestrate -> implement a new issue (just like what we have in issue orchestrate)?

@sepo-agent-app

Copy link
Copy Markdown

Yes. At current PR head d8f706d, #302 implements both high-level functions.

  1. PR /orchestrate -> implement: yes. The planner can choose implement for PR orchestration, and the runtime creates or reuses a tracking issue first, then dispatches agent-implement.yml against that issue. Evidence: planner policy in agent-orchestrator.md, handoff validation in handoff.ts, tracking issue creation/reuse in orchestrate-handoff.ts, and dispatch to implement in orchestrate-handoff.ts.

  2. Discussion /orchestrate -> new implementation issue: yes. The router now allows /orchestrate on discussions, the planner can select only implement as a workflow handoff for discussions, and the runtime creates/reuses a tracking issue before dispatching implement. Evidence: discussion route allowance in agent-router.yml, discussion planner policy in agent-orchestrator.md, discussion-only handoff constraint in handoff.ts, and discussion metadata/link-back support in orchestrate-handoff.ts.

Important nuance: for issues, orchestrated implement can run against the current issue. For PRs and discussions, this PR intentionally normalizes implementation through a separate tracking issue first, then runs agent-implement.yml on that issue. Current tests cover both PR and discussion tracking-issue flows.

@lolipopshock

Copy link
Copy Markdown
Contributor Author

@sepo-agent /fix-pr resolve merge conflicts to main?

@sepo-agent-app

Copy link
Copy Markdown

Sepo pushed fixes for this PR. Branch: agent/orchestrate-nonissue-implement-minimal. Requested by @lolipopshock.

  • Merged current origin/main into the PR branch and resolved the only content conflict in .agent/docs/actions/agent-actions.md.
  • Kept this PR’s discussion-orchestration wording while preserving main’s newer self-approval/self-merge and install-route docs.
  • Confirmed no unmerged paths or conflict markers remain.
  • Checks passed: npm --prefix .agent run build, focused node --test ... route/orchestration suite, git diff --check, and git diff --cached --check.

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.

Orchestrator small bugs:

2 participants