Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions apps/claude-code/pr-review/CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,30 @@ A Thread Classification state. No action taken; the issue still exists in the ne
**obsolete**:
A Thread Classification state. The relevant code was deleted or moved; the comment no longer applies.

### Platform-failure handling

**Notice**:
A user-facing message emitted by an orchestration agent when a Review operation completed in a non-OK Notice Tier. Carries `severity` (`info` or `warning`), `kind` (a small enum identifying the failed operation), and a one-line `message`. Notices are merged across agents by the orchestrator, rendered in the Review Summary, included in the end-of-run Trailer, and (for Pre-PR mode) printed in the Claude interface before findings.
_Avoid_: warning, error, log line

**Notice Tier**:
A four-state classification of every Review operation outcome: **OK**, **EMPTY-BY-DESIGN**, **DEGRADED**, **ABORTED**. The tier choice IS the gating decision — there is no fifth "ask the user" tier. Failure modes that tempt one are reclassified as ABORTED.

**OK**:
A Notice Tier. The operation completed with a non-empty result. No Notice emitted.

**EMPTY-BY-DESIGN**:
A Notice Tier. The operation completed with an empty result that is a legitimate domain state (no work-items linked, no Confluence pages, no prior threads). Currently emits an `info` Notice only for the Doc Context family; other empty states are inherent to the Review type and stay silent.

**DEGRADED**:
A Notice Tier. The operation failed but the Review can still complete with reduced coverage. Emits a `warning` Notice; the Review still posts.

**ABORTED**:
A Notice Tier. The operation failed and continuing would corrupt cross-run state (Bot Signature drift, Summary thread desync, mode misdetection). The run stops before the Review Summary is composed; the failure goes to stderr plus the end-of-run Trailer.

**Trailer**:
A single end-of-run line printed by the orchestrator to the Claude interface, regardless of mode or success state. Carries findings count by severity, Notice counts by severity, and (for ADO modes) the PR URL. Designed for AFK skim: the invoker sees outcome status without opening the PR.

## Relationships

- A **Review** produces one **Review Summary**, zero or more **Inline Comments**, and zero or more **General Comments**
Expand All @@ -137,6 +161,7 @@ A Thread Classification state. The relevant code was deleted or moved; the comme
- The **ADO Fetcher** is invoked by first-review and re-review modes; **Pre-PR mode** skips it entirely and goes directly to Review Aspect agents
- The **Re-review Coordinator** is invoked only when the mode is re-review; first-review and pre-PR modes never load it
- The **ADO Writer** is invoked by first-review and re-review modes; **Pre-PR mode** does not write back to ADO
- Every operation in an orchestration agent terminates in one of the four **Notice Tiers**. **DEGRADED** and **EMPTY-BY-DESIGN**-with-message operations emit a **Notice** that flows from the agent's structured result block, through the orchestrator's merge step, into the **Review Summary** (for ADO modes) or the printed pre-findings block (for **Pre-PR mode**). The end-of-run **Trailer** carries Notice counts so the invoker sees them without opening the PR.

## Example dialogue

Expand Down
36 changes: 0 additions & 36 deletions docs/inbox/pr-review-ado-error-hardening-pass.md

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# A1. End-to-end Notice pipeline via Doc-Context info Notice + ADR-0014

**Status:** ready-for-agent
**Category:** enhancement
**Plugin:** `apps/claude-code/pr-review`
**Type:** AFK

## Parent

`docs/issues/pr-review-ado-fetcher-reliability/PRD.md`

## What to build

Deliver the foundational Notice pipeline end-to-end with one concrete emission site: the Doc-Context `info` Notice when the PR has no linked work items.

Implementation cuts through every layer:

- **New helper** `scripts/ado/notices.mjs` — pure functions `createNotice`, `mergeNotices` (dedupe by `kind`), `formatNoticesAsSummaryBlock`, `formatNoticesAsPrePrPreamble`, `formatTrailer`. With unit tests.
- **ADO Fetcher prompt** — `ADO_FETCHER_RESULT_START/END` block gains a `NOTICES: [...]` field. When `WORK_ITEM_IDS=[]`, the Fetcher appends an `info` Notice (`kind: doc-context`, message: "Reviewed without business context — no work items linked to this PR.").
- **Orchestrator** — parses `NOTICES` from the Fetcher result, merges via the new helper (no-op at this stage but the merge wiring must exist), passes a merged `NOTICES_JSON` to the ADO Writer prompt.
- **ADO Writer prompt** — accepts the new `NOTICES_JSON` input; renders a `## Notices` block (with `ℹ️` for `info`, `⚠` for `warning`) above the severity-grouped findings in the Summary content.
- **Trailer** — orchestrator prints a mandatory end-of-run line in the Claude interface for every run (ADO modes, Pre-PR mode, aborted). Carries findings counts, notice counts, and (for ADO modes) the PR URL.
- **ADR-0014** — new `apps/claude-code/pr-review/docs/adr/0014-notice-tier-doctrine-and-failure-classification-helpers.md`. Records the four-tier doctrine (OK / EMPTY-BY-DESIGN / DEGRADED / ABORTED), the no-fifth-ASK-tier rule, and the JS-helper-layer refinement to ADR 0013.
- **CHANGELOG** — `[Unreleased]` Added entries for the new helper and ADR; Changed entries for the Fetcher result block, the orchestrator merge wiring, and the Summary rendering.
- **`commands/review-pr.md`** stays ≤ 200 lines.

End-to-end demoable: invoke `/pr-review:review-pr` against an ADO PR with no linked work items. The Summary opens with `ℹ️ Reviewed without business context — no work items linked to this PR.` followed by the findings. The Claude interface ends with `✅ Review posted: <N> findings · 0 warning notices · 1 info notice → <PR URL>`.

## Acceptance criteria

- [ ] `scripts/ado/notices.mjs` exists with all five exported functions and passes `pnpm --filter pr-review test`.
- [ ] `ADO_FETCHER_RESULT_START/END` block emits a `NOTICES` field (JSON array; empty array if no notices).
- [ ] Orchestrator parses, merges, and passes `NOTICES` to the ADO Writer prompt.
- [ ] ADO Writer Summary content renders a `## Notices` block above findings when `NOTICES` is non-empty; no block when empty.
- [ ] Doc-Context info Notice appears on a real ADO PR with no linked work items.
- [ ] End-of-run Trailer line is printed in the Claude interface for every run (success, abort, pre-PR).
- [ ] ADR-0014 exists at the documented path.
- [ ] `commands/review-pr.md` is ≤ 200 lines.
- [ ] `pnpm format`, `pnpm check`, and `pnpm --filter pr-review verify:changelog` all pass.

## Blocked by

None — can start immediately.

---

## Triage Notes

> _This was generated by AI during triage._

Locked during the `/grill-with-docs` session of 2026-05-13: Q2 four-tier doctrine (OK / EMPTY-BY-DESIGN / DEGRADED / ABORTED, no fifth ASK tier), Q3 Doc-Context info-Notice carve-out, Q4 Option A Notice flow (per-agent `NOTICES` array, orchestrator merges with `kind`-based dedup, Writer renders `## Notices` block above findings), and the mandatory Trailer convention. ADR-0014 content is fully specified in PRD A's "Implementation Decisions → Helper layer". No outstanding questions; ready for an AFK agent to implement.
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# A2. `classify-http-error` + `fetch-work-items` refactor → DEGRADED tier + ADR-0015

**Status:** ready-for-agent
**Category:** enhancement
**Plugin:** `apps/claude-code/pr-review`
**Type:** AFK

## Parent

`docs/issues/pr-review-ado-fetcher-reliability/PRD.md`

## What to build

Wire the DEGRADED tier end-to-end by introducing the canonical HTTP-tier mapping and applying it to the ADO Fetcher's work-item fetch.

Implementation cuts through every layer:

- **New helper** `scripts/ado/classify-http-error.mjs` — pure function `({ status, body, exitCode }) → { tier: 'ok' | 'degraded' | 'aborted', kind, message }`. Encodes the canonical mapping (200/201/404/409 → ok; 401/403 → aborted; 5xx/network/4xx → degraded). With unit tests covering every row of the mapping table from PRD A's Implementation Decisions, plus malformed-body and network-exit-code paths.
- **New helper** `scripts/ado/fetch-work-items.mjs` — wraps the work-items fetch (currently inline in the Fetcher prompt). Returns `{ ok: true, ids } | { ok: false, reason, message }`. Subsumes today's `parseWorkItemIds`. Empty array on successful fetch → `{ ok: true, ids: [] }` (legitimate EMPTY-BY-DESIGN). Auth/5xx/network → `{ ok: false }` with a kind and message ready for Notice creation. With unit tests covering the discriminated-union behaviour.
- **ADO Fetcher prompt** — Step 5 (work-item fetch) refactored to `await import(...)` the new helper. On `{ ok: false }`, emit a DEGRADED Notice (`kind: work-items`, message from the helper) into the Fetcher's `NOTICES` array (the channel wired by A1). The Doc-Context info Notice from A1 still fires when `{ ok: true, ids: [] }`.
- **ADR-0015** — new `apps/claude-code/pr-review/docs/adr/0015-canonical-http-tier-mapping.md`. Records the HTTP-tier mapping, the 401/403 abort rule, and the no-retries-in-v1 stance.
- **CHANGELOG** — `[Unreleased]` Added entries for the two new helpers and ADR-0015; Changed entry for the Fetcher prompt's work-item step.

Subsumption: today's `parseWorkItemIds` is removed (subsumed into `fetch-work-items.mjs`), and its existing test file is replaced by the new helper's tests.

End-to-end demoable: invoke `/pr-review:review-pr` against an ADO PR while the local Azure CLI is unauthenticated (e.g. revoke the token). The Summary renders `## Notices` containing `⚠ work-items: Failed to fetch linked work items (auth error). Review proceeded without business context.` The Trailer reports `· 1 warning notice`. Restore auth and the same PR shows the Doc-Context info Notice from A1 (or no Notice at all if work items are populated).

## Acceptance criteria

- [ ] `scripts/ado/classify-http-error.mjs` exists with full HTTP-tier-mapping unit tests (≥ 10 cases).
- [ ] `scripts/ado/fetch-work-items.mjs` exists with discriminated-union return shape and unit tests; subsumes `parseWorkItemIds`.
- [ ] Old `parseWorkItemIds` exports and tests are removed.
- [ ] ADO Fetcher prompt's work-item step calls the new helper via `await import(...)`.
- [ ] A simulated work-item-fetch failure (e.g. wrong PROJECT name, revoked auth) produces a DEGRADED Notice with `kind: work-items` in the Summary.
- [ ] ADR-0015 exists at the documented path.
- [ ] `commands/review-pr.md` is ≤ 200 lines.
- [ ] `pnpm format`, `pnpm check`, `pnpm --filter pr-review test`, `pnpm --filter pr-review verify:changelog` all pass.

## Blocked by

`docs/issues/pr-review-ado-fetcher-reliability/01-end-to-end-notice-pipeline.md`

---

## Triage Notes

> _This was generated by AI during triage._

Locked during the `/grill-with-docs` session of 2026-05-13: Q7 canonical HTTP-tier mapping (200/201/404/409 → OK; 401/403 → ABORTED; 5xx/network/4xx → DEGRADED; no retries in v1). Helper-API discriminated-union refactor for `parseWorkItemIds` verified breaking-change-free (zero consumers outside the plugin, confirmed by `grep` across `apps/`, `packages/`, `docs/`). ADR-0015 content is fully specified in PRD A's "Implementation Decisions → Canonical HTTP-tier mapping". No outstanding questions.
Loading
Loading