Skip to content

Support orchestrated implement from PRs and discussions#294

Closed
sepo-agent-app[bot] wants to merge 7 commits into
mainfrom
agent/implement-issue-227/codex-26126733366
Closed

Support orchestrated implement from PRs and discussions#294
sepo-agent-app[bot] wants to merge 7 commits into
mainfrom
agent/implement-issue-227/codex-26126733366

Conversation

@sepo-agent-app

Copy link
Copy Markdown

Summary

  • Allow /orchestrate on discussions and enable planner preflight for discussion targets.
  • Let planner-selected implement from PRs/discussions create a tracking issue first, then dispatch agent-implement.yml against it.
  • Update orchestrator prompts/docs and add focused regression coverage.

Tests

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

Closes #227

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

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

Comment thread .agent/src/cli/orchestrate-handoff.ts Outdated
Comment thread .agent/src/cli/orchestrate-handoff.ts
Comment thread .agent/src/cli/orchestrate-handoff.ts
@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@lolipopshock

Copy link
Copy Markdown
Contributor

why we added so many lines in .agent/src/cli/orchestrate-handoff.ts? we can just have a simple extension based on what we have?

@sepo-agent /answer

@sepo-agent-app

Copy link
Copy Markdown
Author

Short answer: yes, conceptually this should be a simple extension. The line count grew because the PR implemented the missing “PR/discussion /orchestrate -> implement” path inside orchestrate-handoff.ts instead of extracting the already-existing non-issue /implement tracking-issue pattern into a shared helper.

Why some extra logic is still needed:

  • agent-implement.yml is issue-based: it requires issue_number, not a PR/discussion target. See agent-implement.yml.
  • Existing explicit /implement from PR/discussion already creates a tracking issue, dispatches implement against that issue, then links back. See agent-router.yml.
  • Orchestrated handoff dedupe/status comments are issue-comment based, so once PR/discussion orchestration selects implement, we need a stable tracking issue before the normal handoff marker flow can work.
  • The later added lines were mostly hardening: reuse an existing tracking issue on retry, avoid duplicate link-backs, escape copied Sepo control markers, and support discussion comment lookup. That lives around orchestrate-handoff.ts.

I agree the shape can be cleaner. The better version is:

  1. Keep the small policy changes in triage.ts, orchestrator-preflight.ts, and handoff.ts.
  2. Move the non-issue implementation tracking issue creation/reuse/link-back logic into a shared helper, e.g. .agent/src/implementation-tracking.ts.
  3. Reuse that helper from both explicit /implement and orchestrated implement.
  4. Leave orchestrate-handoff.ts with only the orchestration-specific call:
if (decision.nextAction === "implement" && targetKind !== "issue") {
  decision.targetNumber = ensureImplementationTrackingIssueForTarget(...);
}

So I would not remove the tracking issue/dedupe/marker safety entirely, but I do think we should refactor this PR so orchestrate-handoff.ts gets a small extension rather than owning all of that glue directly.

@lolipopshock

Copy link
Copy Markdown
Contributor

Short answer: yes, conceptually this should be a simple extension. The line count grew because the PR implemented the missing “PR/discussion /orchestrate -> implement” path inside orchestrate-handoff.ts instead of extracting the already-existing non-issue /implement tracking-issue pattern into a shared helper.

Why some extra logic is still needed:

  • agent-implement.yml is issue-based: it requires issue_number, not a PR/discussion target. See agent-implement.yml.
  • Existing explicit /implement from PR/discussion already creates a tracking issue, dispatches implement against that issue, then links back. See agent-router.yml.
  • Orchestrated handoff dedupe/status comments are issue-comment based, so once PR/discussion orchestration selects implement, we need a stable tracking issue before the normal handoff marker flow can work.
  • The later added lines were mostly hardening: reuse an existing tracking issue on retry, avoid duplicate link-backs, escape copied Sepo control markers, and support discussion comment lookup. That lives around orchestrate-handoff.ts.

I agree the shape can be cleaner. The better version is:

  1. Keep the small policy changes in triage.ts, orchestrator-preflight.ts, and handoff.ts.
  2. Move the non-issue implementation tracking issue creation/reuse/link-back logic into a shared helper, e.g. .agent/src/implementation-tracking.ts.
  3. Reuse that helper from both explicit /implement and orchestrated implement.
  4. Leave orchestrate-handoff.ts with only the orchestration-specific call:
if (decision.nextAction === "implement" && targetKind !== "issue") {
  decision.targetNumber = ensureImplementationTrackingIssueForTarget(...);
}

So I would not remove the tracking issue/dedupe/marker safety entirely, but I do think we should refactor this PR so orchestrate-handoff.ts gets a small extension rather than owning all of that glue directly.

@sepo-agent /orchestrate

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

Comment thread .agent/src/implementation-tracking.ts
@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@lolipopshock

Copy link
Copy Markdown
Contributor

@sepo-agent /answer

I thought the goal of #294 (comment) is to make the PR simpler, but it adds more lines??

@sepo-agent-app

Copy link
Copy Markdown
Author

Yes, your read is fair.

The refactor made the architecture simpler, but not the net diff smaller.

At current PR head 3b256955:

  • .agent/src/cli/orchestrate-handoff.ts got much smaller versus the previous heavy version: about 2124 lines -> 1739 lines.
  • Compared to main, though, it is still slightly larger: about 1662 -> 1739.
  • The total PR grew because the logic moved into a new shared helper:

So the change did address the specific concern from your comment: orchestrate-handoff.ts no longer owns most of the tracking issue logic. The orchestrated path is now basically the intended small call:

if (decision.nextAction === "implement" && targetKind !== "issue") {
  dispatchTargetNumber = ensureImplementationTrackingIssueForTarget(...)
}

See orchestrate-handoff.ts.

But if the goal is “make the whole PR smaller”, then yes, this still misses that bar. It centralized behavior, but kept all the hardening: marker escaping, retry reuse, duplicate link-back prevention, discussion support, and tests.

The smallest follow-up direction would be: keep the shared helper for tracking issue creation/reuse, but avoid moving every link-back/threading behavior into it. The latest review already found this tradeoff: explicit /implement lost original response-threading, so preserving the old post-response.js path for explicit /implement may be simpler than expanding the helper further.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

Comment thread .agent/src/implementation-tracking.ts
@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

Comment thread .agent/src/implementation-tracking.ts
@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

This comment has been minimized.

@sepo-agent-app

Copy link
Copy Markdown
Author

Sepo pushed fixes for this PR. Branch: agent/implement-issue-227/codex-26126733366. Requested by @sepo-agent.

  • Escaped every <!-- sepo- opener in copied tracking issue content, including incomplete marker openers without a closing -->.
  • Added generated tracking issue regressions for incomplete Sepo marker openers in request text and target body.
  • Verified with npm run build, focused node --test ..., and git diff --check.

@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
fix-pr review PR #294 11 / 40 Dispatched

Reason: agent planner selected review: fix-pr succeeded after addressing the latest review action item, so one automated review should verify the marker-escaping fix and regressions before stopping.

@sepo-agent-app

Copy link
Copy Markdown
Author

Rubrics Review

Total Score Verdict Rubrics Scored
99 PASS 13
Dimension Rubric Result Score Evidence
coding_workflow / generic Validate delegated route authorization pass 8/8 Router passes requester/auth context into orchestrator dispatch, and preflight blocks planner use when delegated capability authorization fails: agent-router.yml, orchestrator-preflight.ts.
coding_workflow / generic Generate context-derived artifact metadata pass 7/7 Tracking issues are generated from target metadata, requester, planner reason, request/context, and base inputs rather than copying the slash command as metadata: implementation-tracking.ts.
coding_workflow / generic Reuse existing code pass 8/8 Explicit non-issue /implement and orchestrated non-issue implement both use the shared tracking helper/CLI: agent-router.yml, orchestrate-handoff.ts.
coding_workflow / generic Minimal defaults, native escape hatches pass 8/8 The PR keeps the common path minimal: non-issue implement normalizes through a tracking issue, while base_pr / base_branch remain explicit typed escape hatches.
coding_workflow / generic Prefer explicit structured inputs pass 7/7 The helper and handoff path use typed fields such as targetKind, trackingScope, baseBranch, basePr, response kind, and threaded IDs; conflicting base inputs stop clearly: implementation-tracking.ts, orchestrate-handoff.ts.
coding_workflow / generic Prefer agent-derived decisions pass 7/7 Planner policy and deterministic handoff validation decide PR/discussion orchestration behavior without adding a new repo setting: agent-orchestrator.md, handoff.ts.
coding_workflow / generic Separate decision gates from action authorization pass 8/8 Planner decisions remain constrained by target kind: discussions can dispatch only implement, while PRs can dispatch review, fix-pr, or separate implement: handoff.ts.
coding_workflow / generic Keep docs in sync pass 7/7 README and architecture docs now describe PR/discussion orchestration and shared tracking issue behavior: README.md, supported-workflows.md.
coding_workflow / generic Make user-facing automation idempotent pass 7/7 The helper checks trusted threaded/top-level link-backs and trusted issue-body markers before creating, then skips duplicate link-backs: implementation-tracking.ts, implementation-tracking.ts.
coding_workflow / generic Surface stateful failures pass 7/7 Tracking issue failures stop the handoff, set outputs, log the reason, and attempt a visible stop comment: orchestrate-handoff.ts.
coding_workflow / generic Comment on accepted workflow stops pass 6/6 Discussion stop dedupe has focused coverage for trusted final comments, avoiding silent stops and duplicate status noise: orchestrate-handoff-cli.test.ts.
coding_workflow / generic Surgical changes pass 8/8 The file list is scoped to orchestration policy, shared tracking helper/CLI, workflow wiring, docs/prompts, and focused tests for the PR/discussion implement path.
coding_style / generic Prefer simple implementations partial 6/7 The call sites are now simple and shared, matching the requested shape. The helper is still sizable because it centralizes create/reuse/link-back/threading/marker-safety behavior in one module.

Notes

  • The current PR head satisfies the main rubric concern from the request: tracking issue create/reuse/link-back logic moved into .agent/src/implementation-tracking.ts, and orchestrate-handoff.ts now makes a small route-specific call.
  • The prior idempotency and marker-safety gaps are covered by focused tests, including incomplete <!-- sepo- marker openers, threaded PR review-comment retries, and discussion link-back dedupe.
  • Verification run: npm run build and focused node --test ... passed with 230 passing tests.
  • No rubric-specific follow-up needed.

Findings

  • INFO: No blocking rubric-specific issues found. The only tradeoff is helper size, but the complexity is now centralized and reused by both explicit /implement and orchestrated implement.

Final Rubric Verdict

PASS

@lolipopshock

Copy link
Copy Markdown
Contributor

Superseded by the smaller focused replacement PR #302.

@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 Support orchestrated implement from PRs and discussions #294 addresses Orchestrator small bugs: #227 by allowing /orchestrate on discussion targets and letting planner-selected implement run from PRs and discussions.
  • Non-issue implement handoffs now create or reuse a shared implementation tracking issue before dispatching agent-implement.yml.
  • The PR updates route policy, planner validation, workflow wiring, prompts, docs, and focused regression tests.
  • Follow-up fixes addressed the earlier marker escaping, tracking issue reuse, threaded link-back, and discussion stop-comment dedupe concerns.

Review

The current PR head a33e26fec8b7923dc4b7919a85a23507b4638180 is ready to ship. The code review and rubrics review agree there are no unresolved BLOCKING or WARNING findings; the only remaining note is INFO-level helper size tradeoff.

Issue Severity Description
No actionable issues remain INFO Current reviewer outputs report SHIP/PASS, and the current diff supports that conclusion. Prior findings are fixed and covered by focused tests.

Progress

  • Issue Orchestrator small bugs: #227’s requested minimal behavior is implemented for PR and discussion /orchestrate paths.
  • Prior marker-safety issues are fixed by escaping every <!-- sepo- opener in copied tracking-issue content.
  • Tracking issue reuse and link-back dedupe now cover top-level and threaded PR/discussion surfaces.
  • Re-fetched inline comments and review threads; resolved stale same-agent thread PRRT_kwDOSOhEMM6DVLEr.
  • No new inline comments, replies, or comment minimizations 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.

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