postNoCodeChangeComment — deterministic post-and-mark service #305

New issue

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

david commented

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

Summary

Add the one deterministic function that posts the no-code-change comment through the platform comment client and writes the marker artifact — at most once, with a fatal failure path (ADR-021 §5 + §6).

Background

Depends on: #303, #304

This service is the middle layer between the comment-body builder and the marker artifact module: check marker → build body → post → write marker → report. It is called only from runPipeline's no-code-change short-circuit (wired in a later step) but is designed, compiled, and unit-tested here in isolation with a fake client.

The platform IssueCommentClient (output of the makeCommentClient seam) already applies bounded retries (3 attempts, backoff) for transient errors, so this function must NOT add its own retry loop. Any post failure here must throw so the pipeline catch turns it into error.json + a non-zero exit — and the marker must not be written on failure, keeping resume's "no marker = not posted" invariant truthful.

Implementation Details

Create src/writeback/services/postNoCodeChangeComment.ts:
```
export async function postNoCodeChangeComment(input: {
  runDir: string;
  ref: IssueRef;                      // issue to comment on
  artifact: NoCodeChangeArtifact;     // schema-validated no-code-change
  client: IssueCommentClient;         // makeCommentClient output
  now?: () => string;
  logInfo?: (msg: string) => void;
}): Promise<{ posted: boolean; postedAt?: string }>
```
Logic:
1. If readNoCodeChangeCommentSignal(runDir) returns an artifact → return { posted: false } (already commented — resume fast-path; do not call the client).
2. Build the body via buildNoCodeChangeCommentBody(artifact).
3. client.addComment(ref, body) — transient errors already retried by the client.
4. On success, write the marker via writeNoCodeChangeCommentSignal(runDir, { issueKey: <issue key from ref>, postedAt: now() }), then return { posted: true, postedAt }.
5. On failure, throw. Do not write the marker.
now defaults to new Date().toISOString(); logInfo defaults to a no-op.
All fs/time access goes through the seams so the service stays deterministic and unit-testable.

Acceptance Criteria

src/writeback/services/postNoCodeChangeComment.ts exports the typed function above.
First post: client called exactly once with the builder's body; marker file written with the correct issueKey and postedAt; returns { posted: true, postedAt }.
Marker already present → returns { posted: false }; client NOT called; marker untouched.
Client throws → the function throws; marker file NOT written.
The now seam drives postedAt.
Unit tests in src/writeback/services/postNoCodeChangeComment.test.ts (fake IssueCommentClient + temp run dir) cover posted / marker-skip / throw.

Test Plan

Targeted unit tests as above: npx vitest run src/writeback/services/postNoCodeChangeComment.test.ts
npm run lint clean.

## Summary Add the one deterministic function that posts the no-code-change comment through the platform comment client and writes the marker artifact — at most once, with a fatal failure path (ADR-021 §5 + §6). ## Background **Depends on:** #303, #304 This service is the middle layer between the comment-body builder and the marker artifact module: check marker → build body → post → write marker → report. It is called only from runPipeline's no-code-change short-circuit (wired in a later step) but is designed, compiled, and unit-tested here in isolation with a fake client. The platform `IssueCommentClient` (output of the `makeCommentClient` seam) already applies bounded retries (3 attempts, backoff) for transient errors, so this function must NOT add its own retry loop. Any post failure here must **throw** so the pipeline catch turns it into `error.json` + a non-zero exit — and the marker must not be written on failure, keeping resume's "no marker = not posted" invariant truthful. ## Implementation Details - Create `src/writeback/services/postNoCodeChangeComment.ts`: ```typescript export async function postNoCodeChangeComment(input: { runDir: string; ref: IssueRef; // issue to comment on artifact: NoCodeChangeArtifact; // schema-validated no-code-change client: IssueCommentClient; // makeCommentClient output now?: () => string; logInfo?: (msg: string) => void; }): Promise<{ posted: boolean; postedAt?: string }> ``` Logic: 1. If `readNoCodeChangeCommentSignal(runDir)` returns an artifact → return `{ posted: false }` (already commented — resume fast-path; do **not** call the client). 2. Build the body via `buildNoCodeChangeCommentBody(artifact)`. 3. `client.addComment(ref, body)` — transient errors already retried by the client. 4. On success, write the marker via `writeNoCodeChangeCommentSignal(runDir, { issueKey: <issue key from ref>, postedAt: now() })`, then return `{ posted: true, postedAt }`. 5. On failure, **throw**. Do not write the marker. - `now` defaults to `new Date().toISOString()`; `logInfo` defaults to a no-op. - All fs/time access goes through the seams so the service stays deterministic and unit-testable. ## Acceptance Criteria - [ ] `src/writeback/services/postNoCodeChangeComment.ts` exports the typed function above. - [ ] First post: client called exactly once with the builder's body; marker file written with the correct `issueKey` and `postedAt`; returns `{ posted: true, postedAt }`. - [ ] Marker already present → returns `{ posted: false }`; client NOT called; marker untouched. - [ ] Client throws → the function throws; marker file NOT written. - [ ] The `now` seam drives `postedAt`. - [ ] Unit tests in `src/writeback/services/postNoCodeChangeComment.test.ts` (fake `IssueCommentClient` + temp run dir) cover posted / marker-skip / throw. ## Test Plan - Targeted unit tests as above: `npx vitest run src/writeback/services/postNoCodeChangeComment.test.ts` - `npm run lint` clean.

david added this to the No-code-change comment: core modules milestone

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

david added the

enhancement

label

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

david referenced this issue

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

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

david referenced this issue

2026-09-05 18:36:05 +00:00

Consolidated unit-test seams for the no-code-change comment feature #309