Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ion issues Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-agent Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…split Records the decision to refactor review-pr.md into a thin orchestrator plus three focused agents (ado-fetcher, re-review-coordinator, ado-writer), and defines the three operating modes and orchestration agent terms in CONTEXT.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds the needs-triage PRD for refactoring review-pr.md into a thin orchestrator plus ADO Fetcher, Re-review Coordinator, and ADO Writer agents, covering the pre-PR mode, compact sub-agent output, and deferred toolkit normalisation decisions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds agent brief and updates status to ready-for-agent. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Breaks the orchestrator split PRD into 7 independently-grabbable vertical slices: ADO Fetcher, ADO Writer, and Re-review Coordinator agents (parallelizable), thin orchestrator refactor, pre-PR mode, compact sub-agent output guidance, and version bump. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-agent Adds agent briefs and updates status on all seven issues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Issues in pr-review-rereview, pr-review-doc-context-enrichment, pr-review-doc-context-spawn-reliability, and inbox-collision-check were completed through docs/plans/ specs but never advanced past resolved in the issue tracker. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Parallel terms to Spec / Spec Runner, grounded during the feature-runner grilling session. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three ADRs capturing decisions reached during the Feature Runner PRD grilling session: - 0027: context bundle injected into every /tdd sub-agent invocation, with domain-scoped ADR and CONTEXT.md selection - 0028: ## Blocked by as canonical sequencing signal; topological order over numerical filename order; halt on conflict - 0029: AFK invocation via Agent tool; acceptance criteria replace /tdd's interactive planning phase; LOOP_COMPLETE as stop signal Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Merges five decisions from the /grill-with-docs session into the Feature Runner PRD: - Context bundle: /tdd sub-agents receive issue + PRD + siblings + CONTEXT.md + scoped ADRs + recent commits, not just the issue file - ADR scoping: plugin ADRs for plugin features, root ADRs for tooling; inferred from PRD path references - AFK invocation: Agent tool is non-interactive; acceptance criteria replace /tdd's planning phase - Sequencing: ## Blocked by is the canonical dependency signal; topological order over numerical filename order; halt on conflict - Parallelism: serialisation is an explicit decision, not an omission; LOOP_COMPLETE named as the stop signal for /loop composability Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Split Phase 7 in development-workflow.md into Spec Runner and Feature Runner paths — the two runners are not interchangeable and the previous single-command entry was misleading. Adds /implement-feature, /loop composability, and LOOP_COMPLETE to the quick reference table. Adds ai-development.md: a narrative deep guide covering the mental model behind the workflow — two-runner architecture, pipeline quality gates, the correct-but-wrong failure mode, context bundle design, AFK trust chain, ## Blocked by sequencing, overnight draining, and CONTEXT.md/ADR maintenance. Complements the quick-reference workflow doc with the reasoning behind the decisions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Breaks the Feature Runner PRD into 8 vertical slices, all triaged to ready-for-agent: 01 — skill scaffold and minimal execution loop (tracer bullet) 02 — failure handling (blocks on 01) 03 — PR creation and worktree cleanup (blocks on 02) 04 — progress reporting (blocks on 01) 05 — full context bundle (blocks on 01) 06 — dependency graph and topological ordering (blocks on 01) 07 — auto-selection and LOOP_COMPLETE (blocks on 01) 08 — docs/agents/feature-runner.md (blocks on 03–07) ## Blocked by is the canonical dependency signal per ADR-0028. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Creates .claude/skills/implement-feature/SKILL.md — the tracer-bullet happy path for the Feature Runner: worktree creation, issue filtering by status, sequential /tdd sub-agent invocation, and resolved marking. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… (issue 02) On /tdd sub-agent failure: appends a failure note under ## Comments, leaves the issue at ready-for-agent, stops the runner, and surfaces a clear message to the user. The worktree is left in place for inspection. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After all issues resolve, the skill pushes the branch, opens a PR targeting develop via gh CLI (title from PRD frontmatter, body listing resolved issues), then removes the worktree. Failure path from 02 prevents this step from running on a broken feature. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ll (issue 04) Before each /tdd invocation the runner outputs "Implementing issue N of M: <title>". M is frozen at run start from the ready-for-agent queue count, so the progress line is accurate regardless of failures or mid-run state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…c (issue 05) Step 1 now detects plugin vs. repo scope from PRD path references and gathers CONTEXT.md, scoped ADRs, and last-5 commits once before the loop. Step 4 reads sibling issues at invocation time and assembles the full six-part context bundle for each /tdd sub-agent call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…dering (issue 06) Step 3 now reads all NN-*.md files (including resolved/closed), parses ## Blocked by edges, checks for conflicts where a blocker has a higher NN than the issue it blocks, and derives a topological execution order. Resolved/closed issues satisfy dependencies but are not re-executed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nal (issue 07) Step 0 handles the no-argument path: scans docs/issues/ for features where every NN-*.md is ready-for-agent, selects the first alphabetically, and runs it. Emits LOOP_COMPLETE on an empty queue so /loop terminates cleanly. Partial features (mixed statuses) are skipped. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ument (issue 08) Covers the full lifecycle (selection → worktree → tdd loop → PR → closed), the six-part context bundle with ADR scoping rationale, ## Blocked by ordering and conflict behaviour, failure handling, and the historical cleanup convention for Spec Runner overlap. No duplication of issue-tracker.md. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Grilled and rejected. Both failure modes that could trigger a retry are not worth automating: Agent tool errors have unpredictable resolution windows, and sub-agent failures mean /tdd already exhausted its own loop — both call for human review, not an automated retry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds two ready-for-agent issues identified during a post-implementation skill review and grilling session: extract verbatim output strings to a references file (10), and add a Quick start section to SKILL.md (11). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…es file (issue 10) Creates references/runner-output-formats.md with all five canonical strings (progress line, conflict error, failure note, PR body, LOOP_COMPLETE). SKILL.md references the file by name, dropping from 213 to 171 lines. Procedural logic is unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds three annotated invocation patterns (named run, auto-select, overnight loop) between the opening paragraph and ## Steps, so users can orient themselves before reading the full procedure. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two ready-for-agent issues identified after issues 10–11 were resolved: add heredoc wrapping note to the PR body template in references (12), and extract the /tdd prompt template to its own references file (13). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…issue 12) The PR body template section in runner-output-formats.md now shows the complete gh pr create command with heredoc wrapper so agents know how to pass the multiline body. No changes to SKILL.md or other sections. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…file (issue 13) Creates references/tdd-prompt-template.md with the full AFK prompt. Step 4 of SKILL.md replaces the 23-line inline block with a one-sentence reference. SKILL.md drops from 177 to 151 lines. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
PR #30 had `## ⚠ Notices` in PRD A's Notice-flow prose but `## Notices` in the slice files — Copilot flagged the inconsistency on PRD A:49. Reconcile to the bare heading. Justification: a mixed `info` + `warning` Notices list would force a single heading emoji to misrepresent one tier; per-item emoji prefixes (`ℹ️` for `info`, `⚠` for `warning`) correctly distinguish them without polluting the heading. Add an explicit doctrine sentence to PRD A's "Notice flow" subsection so future contributors don't relitigate the choice. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pilot K2) Prettier had collapsed the whitespace around several inline-code fragments in the End-to-end demoable paragraph of B5 (lines 26), making the example unreadable. Rewrite the paragraph with prose spacing instead of inline-code adjacency, so the example renders cleanly in both raw markdown and GitHub. Copilot caught this on PR #30 line 26 of the slice file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…opilot K3) The earlier wording read as if `git remote show origin HEAD branch` were a single command invocation (an invalid one — `git remote show` takes a remote name only). Reword to make clear we parse the `HEAD branch:` line out of `git remote show origin`'s output and pass the parsed branch name into the helper via a new `remoteHeadBranch` argument. Step A's prose mirrors the helper signature. Copilot caught this on PR #30 line 19 of the slice file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…Copilot K4) PRD B's "Test-scope choice" section stated MODIFY helpers receive no new unit tests, but slices B2/B3/B4/B6 each explicitly require 2–3 new test cases pinning the new behaviour branches. Copilot flagged this real contradiction on PR #30 line 81. Reconcile by clarifying the doctrine: the user's "NEW deep modules only" answer during grilling was about not writing whole new test suites for MODIFY helpers; the minimal branch-verification cases each slice requires are necessary to confirm the new behaviour landed and do not constitute new suites. Update Test-scope choice + Modules under test sections and two Out of Scope bullets in PRD B and its Agent Brief. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t K3 follow-up) The K3 commit (3a63d78) only fixed slice 06's description of detect-default-branch.mjs. PRD B itself carried four stale references to the same helper that contradicted the slice: - Implementation Decisions: helper signature missing `remoteHeadBranch` parameter; fallback chain described as `git remote show origin HEAD` (the exact invalid-command wording K3 was meant to remove). - Testing Decisions: listed test cases referenced `git remote show succeeds` without naming the `remoteHeadBranch` injection. - Agent Brief desired-behaviour: said the helper walks `develop → main → master` (omitting the `remote-show` first level). - Agent Brief acceptance criteria: said orchestrator wires the helper via injectable `branchExists` only, omitting `remoteHeadBranch`. Reconcile all four against slice 06's K3-corrected contract so an AFK agent picking up B6 reads one consistent signature across PRD and slice. No effect on the other 9 slices. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
docs(pr-review): prep ADO fetcher reliability — Notice Tier doctrine + PRD A/B triage
Lands the foundational tracer-bullet slice of PRD A (ADO Fetcher
reliability). Wires the four-tier Notice doctrine end-to-end with one
concrete emission site — the Doc-Context EMPTY-BY-DESIGN info Notice
fired when WORK_ITEM_IDS=[] — and adds the mandatory end-of-run Trailer
in the Claude interface. Subsequent slices (A2/A3/A4 and PRD B) extend
the same surface with DEGRADED + ABORTED emission sites on the Fetcher,
Writer, Coordinator, and Pre-PR paths.
What this slice ships:
- scripts/ado/notices.mjs — pure helpers (createNotice, mergeNotices,
formatNoticesAsSummaryBlock, formatNoticesAsPrePrPreamble,
formatTrailer). 14 unit-test cases pinning the dedup-by-kind merge,
the per-severity emoji rendering (ℹ️ for info, ⚠ for warning, bare
## Notices heading), and the four Trailer shapes (first-review,
re-review, pre-pr, aborted) including pluralisation.
- ADR 0014 — Notice Tier doctrine + failure-classification helper
layer; refines ADR 0013's testing posture (orchestration in agent
prompts, classification in pure-JS helpers).
- ADO Fetcher (.agents/ado-fetcher.md) — new Step 6 builds the NOTICES
array and emits the Doc-Context info Notice when WORK_ITEM_IDS=[].
ADO_FETCHER_RESULT block grows a NOTICES JSON-array field.
- Orchestrator (commands/review-pr.md) — parses NOTICES from the
Fetcher result block, sets NOTICES_JSON via mergeNotices (single
source today; subsequent slices add Coordinator/Writer sources),
threads it into the ADO Writer prompt, and prints one mandatory
formatTrailer line via a new Step 8. Pre-PR Step E's completion
message becomes a formatTrailer call (mode: 'pre-pr'). The 200-line
cap is held exactly by folding the documentation-only Constants
section into the lead paragraph (SIGNATURE_PREFIX value was already
inlined at both call sites; no behavioural change).
- ADO Writer (.agents/ado-writer.md) — accepts NOTICES_JSON input and
renders a ## Notices block above severity-grouped findings in the
Review Summary content. Empty NOTICES_JSON ⇒ no heading.
- Version: pr-review 1.0.0 → 1.1.0 (Added: user-visible Trailer +
Notices block). CHANGELOG dated 2026-05-13; verify:changelog clean.
Key decisions (locked during the 2026-05-13 /grill-with-docs session):
- Four tiers, no fifth ASK tier — AFK runs cannot block on user input.
Failures that would tempt ASK reclassified as ABORTED.
- Doc-Context info-Notice is the one EMPTY-BY-DESIGN carve-out — every
other empty state is silent.
- Notice shape: { severity, kind, message }; kind is an enum, not a
free-form string, so cross-agent dedup is structural rather than
textual.
- Trailer is mandatory for every run (success, abort, pre-pr); designed
for AFK skim — invoker sees outcome status without opening the PR.
- Helper layer refines (not replaces) ADR 0013: orchestration still in
agent prompts; failure classification moves to scripts/ado/.
Files: 12 changed (5 new, 7 modified). Issue
docs/issues/pr-review-ado-fetcher-reliability/01-end-to-end-notice-pipeline.md
flipped to Status: resolved with a Deviations section documenting the
sandbox-driven manual bump (no pnpm — Node 24 unreachable here, Node 20
runs node --test directly), the manual prettier/tsc feedback loops, and
the Biome layer left to CI (only darwin-arm64 cli binary is installed;
sandbox is linux-arm64).
Unblocks: A2 (work-items DEGRADED), A4 (DIFF_RANGE sentinel), B2
(parseAdoWriterResult discriminated-union), B4 (match-finding throw),
B6 (Pre-PR Notice surface). A3 and B-others remain blocked on A2/A4/B1.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…for Node Sandboxed AFK runs were failing two ways: pnpm's `useNodeVersion: 24.15.0` triggered an outbound `nodejs.org` fetch the firewall blocked, and Biome's platform binary was only installed for the host arch, so `pnpm check` couldn't run on Linux. Fix both by deferring Node version selection to `.nvmrc` (already consumed by `actions/setup-node` and every local version manager) and by declaring `supportedArchitectures` so all OS/arch pairs of platform-specific packages land in `node_modules` regardless of where `pnpm install` runs. Relaxing the engines range from `>=24` to `>=22` makes the existing CI matrix (Node 22 + 24) honest — `useNodeVersion` was silently forcing Node 24 even on the "Node 22" job. Docs (AGENTS.md, README, CONTRIBUTING, plugin CLAUDE.md/CONTRIBUTING, and the new-plugin scaffolding template) updated to point at `.nvmrc` and the relaxed engine range. Historical artifacts under docs/adr/ and docs/plans/ are left intact. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…er + ADR-0015 (v1.2.0)
Wires the DEGRADED tier end-to-end for the work-item fetch path. Two new
pure JS helpers replace the inline parseWorkItemIds helper and introduce
the canonical HTTP-tier mapping used by PRD B's write-side helpers.
New helpers:
- scripts/ado/classify-http-error.mjs — classifyHttpError({ status, body,
exitCode }) → { tier, kind, message }. Canonical mapping: 200/201/404/409
→ ok; 401/403 → aborted (kind: auth); 5xx → degraded (kind: transient);
other 4xx → degraded (kind: malformed-request); non-zero exit + no status
→ degraded (kind: network). 16 unit-test cases.
- scripts/ado/fetch-work-items.mjs — fetchWorkItems({ responseText, exitCode
}) → { ok: true, ids } | { ok: false, reason, message }. Distinguishes
EMPTY-BY-DESIGN (ok: true, ids: []) from fetch failure (ok: false). 9
unit-test cases.
Prompt change (.agents/ado-fetcher.md Step 5): delegates work-item fetch to
fetchWorkItems via await import. On { ok: false }, emits DEGRADED Notice
(kind: work-items) into NOTICES array. On { ok: true, ids: [] }, emits
EMPTY-BY-DESIGN info Notice (kind: doc-context) as before.
Cleanup: parseWorkItemIds removed from scripts/ado-fetcher.mjs and its
tests; ado-fetcher.test.mjs updated to assert fetchWorkItems usage instead.
ADR 0015 (docs/adr/0015-canonical-http-tier-mapping.md): records the
HTTP-tier mapping table, the 401/403 abort rule, and the no-retries-in-v1
stance. Consumed by PRD B's parse-write-response (B1).
Deviations: version bumped manually (unic-bump requires a clean working
tree; sandbox does not support committing mid-step). Version updated in
plugin.json + marketplace.json; CHANGELOG moved [Unreleased] to [1.2.0].
Unblocks: A3 (fetch-iterations ABORTED), B1 (parse-write-response + Writer
HTTP-tier mapping). A3 depends on classify-http-error; B1 depends on both.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Removes the invalid `iterationId=1` fallback and wires the ABORTED tier
for the iterations fetch path. An empty or failed iterations response now
aborts the run rather than silently signing comments with the wrong ID.
New helper:
- scripts/ado/fetch-iterations.mjs — fetchIterations({ responseText,
exitCode }) → { ok: true, latestIterationId, latestCommitSha } |
{ ok: false, reason: 'empty-iterations' | 'auth' | 'transient' |
'malformed', message }. Uses classifyHttpError for HTTP failures.
8 unit-test cases covering every branch.
Prompt changes:
- .agents/ado-fetcher.md Step 2: delegates to fetchIterations via
await import. On { ok: false }, exits non-zero with a clear stderr
message ('az devops login' hint for auth; explicit empty-iterations
message for that reason).
- commands/review-pr.md Step 5: detects absent result block (Fetcher
exited non-zero), infers abortKind from output, calls formatTrailer
and stops. Line count held at exactly 200.
Cleanup:
- scripts/ado-fetcher.mjs deleted (parseIterations was its only export;
all callers updated).
- tests/ado-fetcher.test.mjs: parseIterations import + describe block
removed; new assertions check for fetchIterations usage and verify
no iterationId=1 default remains in the agent prompt.
- package.json: tests/fetch-iterations.test.mjs added to test script.
Deviations: version bumped manually (unic-bump requires a clean working
tree; sandbox does not support committing mid-step).
Unblocks: A4 (diff-range sentinel) had no dependency on A3; A3 itself
had no dependents in the current issue queue.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ndment (v1.2.2)
Emits the DIFF_RANGE sentinel in the ADO Fetcher result block and fires
a DEGRADED Notice when the incremental diff fallback triggers.
ADO Fetcher (.agents/ado-fetcher.md):
- Step 4: sets DIFF_RANGE=incremental on successful incremental diff,
DIFF_RANGE=full + DIFF_RANGE_FALLBACK=true when prior commit is
unreachable, DIFF_RANGE=full on first-review, DIFF_RANGE=incremental
on no-new-commits branch.
- Step 6: passes DIFF_RANGE_FB into the Notice-building script; emits
warning Notice (kind: diff-range) when DIFF_RANGE_FALLBACK=true.
Removes the forward-looking A4 comment (now implemented).
- Output block: adds DIFF_RANGE: {DIFF_RANGE} field with Where: docs.
Orchestrator (commands/review-pr.md): adds DIFF_RANGE to the parsed
fields list in Step 5 with a note that B3 will consume it. 200 lines.
ADR 0004: adds "Degraded baseline" subsection documenting the γ-downgrade
rule (remap addressed/obsolete → pending when DIFF_RANGE=full). B3 impls.
Tests: adds 3 new ado-fetcher content assertions (DIFF_RANGE assignments,
DIFF_RANGE_FALLBACK flag, diff-range Notice). 185 tests, all passing.
Deviations: version bumped manually (unic-bump requires a clean working
tree; sandbox does not support committing mid-step).
Unblocks: B3 (Coordinator γ-downgrade consumes DIFF_RANGE from A4).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(v1.2.3)
Routes every ADO write call site in the ADO Writer through one canonical
helper. Fixes the H1 auth gap (401/403 on inline-POST now aborts the Writer
instead of log-and-continue), wires the DEGRADED Notice surface end-to-end
for write failures, and adds the NOTICES field to the Writer result block.
New helper:
- scripts/ado/parse-write-response.mjs — parseWriteResponse({ httpExit,
responseText, errStream }) → { ok: true, id } | { ok: false, tier, kind,
message }. Composes classifyHttpError with response-id extraction. 404/409
map to { ok: true, id: null } (canonical ok, no resource created); 200
without numeric id → { ok: false, tier: 'degraded', kind: 'malformed-
response' }. 13 unit cases covering all branches.
Prompt changes (.agents/ado-writer.md):
- New "Helper: parse-write-response" section shows the canonical invoke
pattern once; each of the four call sites (inline POST + fallback, summary
POST, delta reply, completion marker) references it.
- On tier: aborted → stream *.err to stderr + exit 1 at every call site.
- On tier: degraded → push typed Warning Notice to NOTICES='[]' and continue.
- ADO_WRITER_RESULT_START/END grows NOTICES: [...] field.
- Cleanup (Step 4) is unconditional (no retention based on counts).
Orchestrator (commands/review-pr.md):
- Step 8 renamed to "Merge Writer notices + Trailer"; parses NOTICES from
ADO_WRITER_RESULT_START/END and merges via mergeNotices before Trailer.
200 lines.
Helper changes (scripts/ado-writer.mjs):
- parseAdoWriterResult return type extended to { summaryThreadId,
findingsPosted, notices: Notice[] }. Legacy blocks without NOTICES return
notices: []. Tests updated (7 parseAdoWriterResult cases + 3 new
agent-content assertions for parse-write-response usage).
Deviations: version bumped manually (unic-bump requires clean working tree).
Unblocks: B5 (Coordinator PATCH-to-fixed mapping depends on parse-write-
response).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…1.2.4)
Distinguishes Writer crash from zero-success: the old `null` return could
not tell "block was never emitted" from "block parsed, zero findings."
Helper change (scripts/ado-writer.mjs):
- parseAdoWriterResult now returns
{ ok: true, summaryThreadId, findingsPosted, notices }
| { ok: false, reason: 'missing-block' | 'malformed' }
- missing-block: ADO_WRITER_RESULT_START/END absent from output
- malformed: block present but FINDINGS_POSTED field absent/non-numeric
- summaryThreadId remains nullable (empty SUMMARY_THREAD_ID is valid)
Tests (tests/ado-writer.test.mjs):
- All valid-block cases assert result.ok === true before field access
- Renamed "returns null for both numeric fields" → "returns { ok: false,
reason: 'missing-block' } when no result block is present"
- New case: "returns { ok: false, reason: 'malformed' } when block is
present but FINDINGS_POSTED is absent"
- 205 tests, all passing
ADO Writer prompt (.agents/ado-writer.md):
- Round-trip validation updated from
`if (parsed.summaryThreadId === null || parsed.findingsPosted === null)`
to `if (!parsed.ok)`, with reason included in the error message.
Orchestrator (commands/review-pr.md):
- Step 8 renamed "Parse Writer result + Trailer"; branches on result.ok.
On { ok: false }, emits ERROR stderr message (with reason) and Trailer
aborted line, then stops. On { ok: true }, extracts result.notices and
merges with fetcher notices as before.
- File remains at exactly 200 lines.
Deviations: version bumped manually (unic-bump requires clean working
tree; sandbox does not support committing mid-step).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… wiring (v1.2.5) Extends classify-thread with the γ-downgrade rule (ADR-0004) and wires DIFF_RANGE from the ADO Fetcher result block into every thread classification call in the Re-review Coordinator. Helper change (scripts/re-review/classify-thread.mjs): - classifyThread now accepts diffRange: 'full' | 'incremental' (default 'incremental') - When diffRange === 'full', outputs 'addressed' and 'obsolete' are remapped to 'pending' via a single post-processing branch; 'disputed' is unaffected (its derivation is reviewer-reply-based, not diff-position-based) Tests (tests/classify-thread.test.mjs): - Three new γ-downgrade cases: full+intersects→pending, full+absent→pending, full+disputed→still disputed; 208 tests, all passing Coordinator (.agents/re-review-coordinator.md): - Inputs: added DIFF_RANGE as a parsed field with description - Step 5: parses DIFF_RANGE from ADO_FETCHER_RESULT (default incremental), passes DIFF_R env var into the classifyThread invocation Orchestrator (commands/review-pr.md): - Step 5 note updated: "not yet consumed" removed; explains Coordinator parses DIFF_RANGE from ADO_FETCHER_RESULT to apply γ-downgrade - File remains at exactly 200 lines Deviations: version bumped manually (unic-bump requires clean working tree). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ress-addressed-reply/01)
Removes the cosmetic "Resolved — thanks!" reply comment from the addressed
branch of the Re-review Coordinator. The thread status PATCH to fixed (status 2),
ADDRESSED_COUNT, and FINDINGS_POSTED counts are all unchanged.
Motivation: the reply generated an ADO notification for every thread participant.
Developers often self-resolve threads before the bot runs, causing the bot to
comment on already-closed threads (notification spam).
Changes:
- .agents/re-review-coordinator.md: removed Reply POST az devops invoke block,
updated section heading ("post resolution confirmation and" removed), updated
Step 8 description for addressed count
- docs/adr/0006-reply-not-duplicate-auto-resolve.md: updated Decision bullet for
addressed threads; added Revised note (2026-05-14) with reason
- docs/issues/done/01-remove-addressed-reply.md: moved from ready-for-agent to resolved
Note: version bump and CHANGELOG covered by suppress-addressed-reply/02.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ed-reply/02) Bumped manually (unic-bump requires a clean working tree; sandbox does not support committing mid-step). Changes: - plugin.json + marketplace.json: 1.2.5 → 1.2.6 - CHANGELOG.md: [Unreleased] promoted to [1.2.6] — 2026-05-14 with the addressed-thread reply removal entry; fresh empty [Unreleased] added - docs/issues/done/02-version-bump.md: moved from ready-for-agent to resolved Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…1.2.7)
Changes match-finding's null-vs-throw contract so parse errors are
distinguishable from legitimate no-match, and wires the Coordinator
to surface a DEGRADED Notice instead of silently posting duplicates.
Helper change (scripts/re-review/match-finding.mjs):
- Throws TypeError when priorThreads is not an array
- Throws TypeError when finding is null or missing typed fields
- Returns null only for legitimate no-match (contract unchanged)
Tests (tests/match-finding.test.mjs):
- Three new throw-path cases: non-array priorThreads, null finding,
wrong field types; 211 tests, all passing
Coordinator (.agents/re-review-coordinator.md):
- Initialises NOTICES='[]' alongside FRESH_FINDINGS_JSON in Step 6
- Step 6a captures Node exit code (MATCH_EXIT); on non-zero, pushes
DEGRADED Notice (kind: thread-match) and falls through to no-match
- Result block gains NOTICES: {NOTICES} field; Step 8 description updated
Orchestrator (commands/review-pr.md):
- Step 7: extracts coordinatorNotices from RE_REVIEW_COORDINATOR_RESULT
- Step 8: merges coordinatorNotices into combined mergeNotices call
- File remains at exactly 200 lines
Deviations: version bumped manually (unic-bump requires clean working tree).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…(v1.2.8) Routes the PATCH-to-fixed call site in the Re-review Coordinator through the canonical parse-write-response helper, replacing the old 409-only catch-all with uniform HTTP-tier handling. Coordinator (.agents/re-review-coordinator.md): - addressed branch: captures PATCH_RESP + PATCH_EXIT separately; routes through parseWriteResponse via Node heredoc (same PWR_JSON pattern as ADO Writer); on ok: true → continue; on tier: aborted → stderr + exit 1; on tier: degraded → push patch-to-fixed Notice + continue - 404/409 map to ok: true (canonical mapping) — silent continue as before - Old 409-only catch-all removed Orchestrator (commands/review-pr.md): - Step 7: added coordinator-abort handling — missing result block infers abortKind from output and calls formatTrailer before stopping - File remains at exactly 200 lines Deviations: version bumped manually (unic-bump requires clean working tree). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… (v1.2.9)
Gives Pre-PR mode the same Notice surface as ADO modes. Detects malformed
diff inputs. Replaces the hardcoded 'main' fallback with a Gitflow-aware
fallback chain that emits visible Notices.
New helper (scripts/pre-pr/detect-default-branch.mjs):
- detectDefaultBranch({ branchExists, remoteHeadBranch })
→ { branch, source, notice? }
- Chain: remote-show → develop-fallback → main-fallback → master-fallback → none
- Warning Notice (kind: default-branch) for every fallback level
- branch: null means no detectable branch → caller aborts
- 7 unit cases (tests/detect-default-branch.test.mjs)
Helper change (scripts/pre-pr.mjs):
- buildPrePrContext returns notices: Notice[] alongside existing fields
- Suspicious-shape detection: non-empty diff with diff --git headers but
zero parsed paths → DEGRADED Notice (kind: diff-parse)
- 4 new test cases in tests/pre-pr.test.mjs
Orchestrator (commands/review-pr.md):
- Pre-PR Step A: calls detectDefaultBranch; on branch: null aborts with Trailer;
any fallback notice pushed to PRE_PR_NOTICES
- Pre-PR Step B: merges buildPrePrContext().notices via mergeNotices
- Pre-PR Step E: formatNoticesAsPrePrPreamble before findings;
formatTrailer receives PRE_PR_NOTICES (notice count reflected in Trailer)
- File is 187 lines (≤ 200)
package.json: added tests/detect-default-branch.test.mjs to test script.
Deviations: version bumped manually (unic-bump requires clean working tree).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…n malformed-JSON httpExit:0 test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… override Tracks F7 from Copilot review of PR #31 — valid improvement but out of scope of any current PRD. Deferred until after the orchestrator-split branch ships. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…d + sentinel doc - B3: map malformed-request kind to 'malformed' reason instead of 'transient' - B1: filter null array elements before reduce; treat all-null array as empty-iterations - B4: document latestCommitSha '' sentinel in JSDoc comment - D1: add tests for missing value key and HTTP 400 → malformed paths Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…t guard Route non-zero exit codes through classifyHttpError for consistent tier mapping (auth/transient/malformed) matching fetch-iterations.mjs pattern. Guard against null/non-object elements in ADO value array to prevent TypeError. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…c + test coverage Corrects the classify-thread JSDoc to accurately list 5 ordered rules (ADR-0004) instead of collapsing status-check and intersection-check into one; documents minor-findings omission from the trailer in notices.mjs; adds γ-downgrade/status=fixed test, full RESOLVED_STATUSES coverage, re-review trailer test, and mergeNotices null-source tolerance test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…am in classified write failures
- A1: parseAdoWriterResult returns { ok: false, reason: 'malformed' } on malformed NOTICES JSON
instead of silently falling back to empty notices; broadened NOTICES regex to detect
unclosed arrays too
- A3: parseWriteResponse appends errStream detail to classified failure messages (same
pattern already used for malformed-response path)
- D4: add HTTP 400 and 422 test cases to parse-write-response.test.mjs
- D10: update malformed JSON (non-zero exit) test to assert kind: 'network'
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…e, add none-notice - Import Notice from notices.mjs instead of local typedef (kind was string, not NoticeKind) - Trim whitespace from remoteHeadBranch before truthy check (whitespace-only input now falls through to fallback chain) - Add warning Notice to source='none' result for consistency with levels 2–4 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…otice assertions Add whitespace-only remoteHeadBranch test (D6) and update source='none' assertions to expect a warning notice (pairs with A4 fix in 3ee5c6a). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
feat(pr-review): orchestrator split + platform failure handling + suppress addressed reply (v1.1.0–v1.2.9)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.