Post the comment at runPipeline's no-code-change short-circuit #306

New issue

david · 2026-09-05T18:35:27Z

david commented

2026-09-05 18:35:27 +00:00

Summary

Wire the post-and-mark service into the no-code-change branch of runPipeline so that whenever the implement agent declares no code is needed, pi-loop deterministically comments on the source issue before returning success — with posting failures attributed to the write-back stage and aborting the run (ADR-021 §1–§5).

Background

Depends on: #303, #304, #305

Today the no-code-change short-circuit (the implementResult.noCodeChange !== undefined block in src/orchestrator/services/runPipeline.ts, ~line 543) returns ok: true without commenting. This step makes the comment happen there and adds the run-pipeline result field downstream consumers (summary, CLI outcome) will read in a later step.

closeIssue is never called and the existing closeIssueAfterAutoMerge machinery is untouched — comment-only, per ADR-021. defaultSkippedImplementResult (the resume helper in runPipeline.ts) needs no change: it already re-reads no-code-change.json, which is what lets a resumed run re-enter this branch (ADR-021 §7).

Implementation Details

In src/orchestrator/services/runPipeline.ts, inside the no-code-change branch (after the existing log at ~line 547):
1. Construct the comment client with the already-in-scope seam: const commentClient = makeCommentClient({ config, provider: issueRef.provider, originHost: origin.host }) — same shape as the existing post-MR use at ~line 774.
2. Set currentStage = 'write-back' before posting, so a thrown failure is attributed to write-back in error.json and the console (ADR-021 §5). Note the branch currently sits with currentStage === 'implement'.
3. Call postNoCodeChangeComment({ runDir, ref: issueRef, artifact: implementResult.noCodeChange, client: commentClient, now, logInfo }). It throws on failure (per the post service) or returns { posted: false } when the marker pre-exists on resume — no double post.
4. Thread the outcome into the returned pipeline result (see below).
In src/orchestrator/types/orchestrator.ts: extend the no-code-change variant of the run-pipeline result with noCodeChangeComment?: NoCodeChangeCommentArtifact | undefined (the marker artifact when posted this run; undefined when the marker pre-existed on resume). Extend RunPipelineInput with optional injectable seams (e.g. postNoCodeChangeComment? / readNoCodeChangeCommentSignal?-style, consistent with how makeCommentClient/doWriteback are seams today) so this wiring is unit-testable without a live client or fs.

Acceptance Criteria

No-code-change + no marker → comment client called once, marker written, result carries noCodeChangeComment.
No-code-change + marker already present (resume) → client not called, posted: false semantics, run still ok: true.
Comment client throws → pipeline result ok: false with an error report classifiable into error.json whose stage is write-back; marker NOT written.
Resume re-entry: implement skipped via defaultSkippedImplementResult returning the artifact → short-circuit fires and posts or skips per the marker.
closeIssue/closeIssueAfterAutoMerge untouched; no new Pi-Loop-Stage commit introduced on this path.
Unit tests in src/orchestrator/services/runPipeline.test.ts cover the four cases above.

Test Plan

Targeted unit tests per above; then npm test and npm run lint.

## Summary Wire the post-and-mark service into the no-code-change branch of `runPipeline` so that whenever the implement agent declares no code is needed, pi-loop deterministically comments on the source issue before returning success — with posting failures attributed to the `write-back` stage and aborting the run (ADR-021 §1–§5). ## Background **Depends on:** #303, #304, #305 Today the no-code-change short-circuit (the `implementResult.noCodeChange !== undefined` block in `src/orchestrator/services/runPipeline.ts`, ~line 543) returns `ok: true` without commenting. This step makes the comment happen there and adds the run-pipeline result field downstream consumers (summary, CLI outcome) will read in a later step. `closeIssue` is never called and the existing `closeIssueAfterAutoMerge` machinery is untouched — comment-only, per ADR-021. `defaultSkippedImplementResult` (the resume helper in runPipeline.ts) needs **no change**: it already re-reads `no-code-change.json`, which is what lets a resumed run re-enter this branch (ADR-021 §7). ## Implementation Details - In `src/orchestrator/services/runPipeline.ts`, inside the no-code-change branch (after the existing log at ~line 547): 1. Construct the comment client with the already-in-scope seam: `const commentClient = makeCommentClient({ config, provider: issueRef.provider, originHost: origin.host })` — same shape as the existing post-MR use at ~line 774. 2. Set `currentStage = 'write-back'` **before** posting, so a thrown failure is attributed to write-back in `error.json` and the console (ADR-021 §5). Note the branch currently sits with `currentStage === 'implement'`. 3. Call `postNoCodeChangeComment({ runDir, ref: issueRef, artifact: implementResult.noCodeChange, client: commentClient, now, logInfo })`. It throws on failure (per the post service) or returns `{ posted: false }` when the marker pre-exists on resume — no double post. 4. Thread the outcome into the returned pipeline result (see below). - In `src/orchestrator/types/orchestrator.ts`: extend the no-code-change variant of the run-pipeline result with `noCodeChangeComment?: NoCodeChangeCommentArtifact | undefined` (the marker artifact when posted this run; `undefined` when the marker pre-existed on resume). Extend `RunPipelineInput` with optional injectable seams (e.g. `postNoCodeChangeComment?` / `readNoCodeChangeCommentSignal?`-style, consistent with how `makeCommentClient`/`doWriteback` are seams today) so this wiring is unit-testable without a live client or fs. ## Acceptance Criteria - [ ] No-code-change + no marker → comment client called once, marker written, result carries `noCodeChangeComment`. - [ ] No-code-change + marker already present (resume) → client not called, `posted: false` semantics, run still `ok: true`. - [ ] Comment client throws → pipeline result `ok: false` with an error report classifiable into `error.json` whose `stage` is `write-back`; marker NOT written. - [ ] Resume re-entry: implement skipped via `defaultSkippedImplementResult` returning the artifact → short-circuit fires and posts or skips per the marker. - [ ] `closeIssue`/`closeIssueAfterAutoMerge` untouched; no new `Pi-Loop-Stage` commit introduced on this path. - [ ] Unit tests in `src/orchestrator/services/runPipeline.test.ts` cover the four cases above. ## Test Plan - Targeted unit tests per above; then `npm test` and `npm run lint`.

david added this to the No-code-change comment: pipeline wiring & resume milestone

2026-09-05 18:35:27 +00:00

david added the

enhancement

label

2026-09-05 18:35:27 +00:00

david referenced this issue

2026-09-05 18:35:41 +00:00

Surface the posted comment in the run summary and CLI outcome line #307

david referenced this issue

2026-09-05 18:35:50 +00:00

Resume mapping: treat a valid no-code-change.json as implement-complete evidence #308