From c7bad18fa40c1a8de1a8c070d1d533d52e08ec1f Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Tue, 16 Jun 2026 15:30:38 +0200 Subject: [PATCH 1/7] fix(unic-pr-review): fetch linked Work Items in ADO Fetcher Step 1.5 The pullrequests endpoint never includes workItemRefs; Work Item links live at a separate pullrequestworkitems endpoint that the fetcher was not calling. This caused discoverWorkItems() to always return [] and silently skip the Intent Check on every PR with linked Work Items. - Add Step 1.5 to ado-fetcher.md: fetch pullrequestworkitems via az devops invoke and set PR_METADATA.workItemRefs from the result - Update Step 6 output schema to document workItemRefs in prMetadata - Fix misleading 'native Work Item field' comment in provider.mjs - Add pullrequestworkitems to ado-cli-inventory.json - Add fetcher-contract tests asserting workItemRefs is always present Co-Authored-By: Claude Sonnet 4.6 --- .../unic-pr-review/agents/ado-fetcher.md | 27 ++++++++++++++++- .../fixtures/ado-cli-inventory.json | 6 ++++ .../providers/azure_devops/provider.mjs | 5 ++-- .../azure_devops/tests/provider.test.mjs | 29 +++++++++++++++++++ 4 files changed, 64 insertions(+), 3 deletions(-) diff --git a/apps/claude-code/unic-pr-review/agents/ado-fetcher.md b/apps/claude-code/unic-pr-review/agents/ado-fetcher.md index 127d6ec6..2341d591 100644 --- a/apps/claude-code/unic-pr-review/agents/ado-fetcher.md +++ b/apps/claude-code/unic-pr-review/agents/ado-fetcher.md @@ -38,6 +38,26 @@ If the command exits non-zero, emit this error and stop: { "error": "fetch-failed", "step": 1, "resource": "pullrequests", "message": "" } ``` +### Step 1.5 — Fetch linked Work Items + +The `pullrequests` endpoint does not include Work Item links. Fetch them from the dedicated endpoint: + +```sh +az devops invoke --area git --resource pullrequestworkitems \ + --route-parameters organization="" project="" repositoryId="" pullRequestId="" \ + --http-method GET --api-version 7.0 --output json +``` + +Store stdout as `WI_LIST`. Set `PR_METADATA.workItemRefs = WI_LIST.value ?? []`. + +If the command exits non-zero, set `PR_METADATA.workItemRefs = []` and add a warning: + +``` +"pullrequestworkitems fetch failed — Work Item discovery skipped, Intent Check will be omitted. Error: " +``` + +Do not stop; continue to Step 2. + ### Step 2 — Fetch Revisions (iterations) ```sh @@ -232,7 +252,10 @@ Emit exactly one JSON object — no prose, no markdown, no footer (replace each ```json { - "prMetadata": "", + "prMetadata": { + "pullRequestId": 42, + "workItemRefs": [{ "id": "101", "url": "https://dev.azure.com/org/project/_apis/wit/workitems/101" }] + }, "revisions": "", "threads": "", "changedFiles": ["path/to/file.ts"], @@ -247,4 +270,6 @@ Emit exactly one JSON object — no prose, no markdown, no footer (replace each } ``` +`prMetadata` is the raw ADO `pullrequests` response enriched with `workItemRefs` from Step 1.5. `workItemRefs` is always an array (empty `[]` when no Work Items are linked or the fetch failed). + `mode` is one of `"first-review"`, `"re-review"`, `"first-review-fallback"`. `priorRevisionId` and `priorIteration` are `null` except in `re-review` mode (where they carry `PRIOR_SIG.priorRevisionId` / `PRIOR_SIG.priorIteration`). `deltaRawDiff` is the delta diff string (empty in first-review modes). `priorFindings` is an array of `{ threadId, filePath, startLine, severity, title }` objects (empty except in `re-review` mode), where `threadId` is the number id of the ADO Thread carrying that prior finding's bot comment — it is what the Re-review Coordinator keys all thread mapping on. `diffUnavailable` is `false` when a real diff was computed (re-review always, first-review/first-review-fallback when inside a matching clone and the git diff succeeds) and `true` when a diff could not be obtained (no matching clone, missing commonRefCommit or sourceRefCommit, git diff failure, or an empty diff despite a non-empty changedFiles). `warnings` is an array of strings for any non-fatal issues. Never emit `hardStop` — the orchestrator handles all write decisions. diff --git a/apps/claude-code/unic-pr-review/providers/azure_devops/fixtures/ado-cli-inventory.json b/apps/claude-code/unic-pr-review/providers/azure_devops/fixtures/ado-cli-inventory.json index 422270f0..f03c10e7 100644 --- a/apps/claude-code/unic-pr-review/providers/azure_devops/fixtures/ado-cli-inventory.json +++ b/apps/claude-code/unic-pr-review/providers/azure_devops/fixtures/ado-cli-inventory.json @@ -1,6 +1,12 @@ { "invokeCommands": [ { "area": "git", "resource": "pullrequests", "httpMethod": "GET", "description": "PR metadata" }, + { + "area": "git", + "resource": "pullrequestworkitems", + "httpMethod": "GET", + "description": "Work Items linked to the PR (Step 1.5 — separate endpoint, not in pullrequests response)" + }, { "area": "git", "resource": "pullrequestiterations", diff --git a/apps/claude-code/unic-pr-review/providers/azure_devops/provider.mjs b/apps/claude-code/unic-pr-review/providers/azure_devops/provider.mjs index d8c6298e..f1bb8377 100644 --- a/apps/claude-code/unic-pr-review/providers/azure_devops/provider.mjs +++ b/apps/claude-code/unic-pr-review/providers/azure_devops/provider.mjs @@ -41,8 +41,9 @@ export const agents = { } /** - * Read the PR's native Work Item field (`workItemRefs`). Never regex-scrapes the - * description — this is the ADR-0001 amendment contract. + * Read the Work Item refs that the ADO Fetcher populated on `prMetadata` from the + * PR's linked work-item endpoint (`pullrequestworkitems`, Step 1.5). Never + * regex-scrapes the description — this is the ADR-0001 amendment contract. * * @param {{ workItemRefs?: Array<{ id: string, url: string }> }} prMetadata * @returns {Array<{ id: string, type: string, url: string, raw: object }>} diff --git a/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs b/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs index 4351fbe8..68e6cad7 100644 --- a/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs +++ b/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs @@ -84,3 +84,32 @@ describe('discoverWorkItems', () => { } }) }) + +// Fetcher contract: the ADO Fetcher (Step 1.5) must populate workItemRefs on prMetadata +// from the pullrequestworkitems endpoint. These tests assert that the fixtures reflect +// real fetcher output shape — workItemRefs is always present (never absent) because the +// fetcher sets it explicitly (to [] when no WIs are linked or the fetch fails). +describe('fetcher contract — prMetadata.workItemRefs is always fetcher-populated', () => { + it('pr-with-work-items fixture has workItemRefs with at least one entry', () => { + const meta = fixture('pr-with-work-items.json') + assert.ok(Array.isArray(meta.workItemRefs), 'workItemRefs must be an array (fetcher-populated)') + assert.ok(meta.workItemRefs.length > 0, 'fixture must have at least one work item ref') + }) + it('pr-with-multiple-work-items fixture has workItemRefs with multiple entries', () => { + const meta = fixture('pr-with-multiple-work-items.json') + assert.ok(Array.isArray(meta.workItemRefs), 'workItemRefs must be an array (fetcher-populated)') + assert.ok(meta.workItemRefs.length > 1, 'fixture must have more than one work item ref') + }) + it('pr-without-work-items fixture has workItemRefs as empty array (not absent)', () => { + const meta = fixture('pr-without-work-items.json') + assert.ok(Array.isArray(meta.workItemRefs), 'workItemRefs must be present even when empty') + assert.equal(meta.workItemRefs.length, 0) + }) + it('each workItemRef has string id and url', () => { + const meta = fixture('pr-with-work-items.json') + for (const ref of meta.workItemRefs) { + assert.equal(typeof ref.id, 'string', 'workItemRef.id must be a string') + assert.equal(typeof ref.url, 'string', 'workItemRef.url must be a string') + } + }) +}) From 97ad720a95ea30a7f9e3dfa49da4f85ee4fbd999 Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Tue, 16 Jun 2026 15:30:58 +0200 Subject: [PATCH 2/7] chore(unic-pr-review): add CHANGELOG entry for #247 Co-Authored-By: Claude Sonnet 4.6 --- apps/claude-code/unic-pr-review/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/claude-code/unic-pr-review/CHANGELOG.md b/apps/claude-code/unic-pr-review/CHANGELOG.md index 38656549..2cf2dd21 100644 --- a/apps/claude-code/unic-pr-review/CHANGELOG.md +++ b/apps/claude-code/unic-pr-review/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - (none) ### Fixed -- (none) +- ADO Fetcher now fetches linked Work Items via `pullrequestworkitems` endpoint (Step 1.5) and populates `prMetadata.workItemRefs`; previously the fetcher never called this endpoint so `discoverWorkItems()` always returned `[]` and the Intent Check was silently skipped on every PR with linked Work Items (#247) ## [2.1.9] — 2026-06-09 From 38eca8d226836615a81b346877fbb9f090c557c3 Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Tue, 16 Jun 2026 15:37:30 +0200 Subject: [PATCH 3/7] chore(unic-pr-review): bump to 2.1.10 for Work Item discovery fix (#247) Co-Authored-By: Claude Sonnet 4.6 --- .../unic-pr-review/.claude-plugin/marketplace.json | 2 +- .../unic-pr-review/.claude-plugin/plugin.json | 2 +- apps/claude-code/unic-pr-review/CHANGELOG.md | 11 +++++++++++ apps/claude-code/unic-pr-review/package.json | 2 +- 4 files changed, 14 insertions(+), 3 deletions(-) diff --git a/apps/claude-code/unic-pr-review/.claude-plugin/marketplace.json b/apps/claude-code/unic-pr-review/.claude-plugin/marketplace.json index 281c8749..208ee0af 100644 --- a/apps/claude-code/unic-pr-review/.claude-plugin/marketplace.json +++ b/apps/claude-code/unic-pr-review/.claude-plugin/marketplace.json @@ -21,7 +21,7 @@ "name": "unic-pr-review", "source": "./", "tags": ["productivity", "code-review", "azure-devops"], - "version": "2.1.9" + "version": "2.1.10" } ] } diff --git a/apps/claude-code/unic-pr-review/.claude-plugin/plugin.json b/apps/claude-code/unic-pr-review/.claude-plugin/plugin.json index dbe8bc73..d677f75b 100644 --- a/apps/claude-code/unic-pr-review/.claude-plugin/plugin.json +++ b/apps/claude-code/unic-pr-review/.claude-plugin/plugin.json @@ -15,5 +15,5 @@ "keywords": ["pr-review", "azure-devops", "jira", "confluence", "code-review", "unic"], "license": "LGPL-3.0-or-later", "name": "unic-pr-review", - "version": "2.1.9" + "version": "2.1.10" } diff --git a/apps/claude-code/unic-pr-review/CHANGELOG.md b/apps/claude-code/unic-pr-review/CHANGELOG.md index 2cf2dd21..3a86ab64 100644 --- a/apps/claude-code/unic-pr-review/CHANGELOG.md +++ b/apps/claude-code/unic-pr-review/CHANGELOG.md @@ -13,6 +13,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - (none) +### Fixed +- (none) + +## [2.1.10] — 2026-06-16 + +### Breaking +- (none) + +### Added +- (none) + ### Fixed - ADO Fetcher now fetches linked Work Items via `pullrequestworkitems` endpoint (Step 1.5) and populates `prMetadata.workItemRefs`; previously the fetcher never called this endpoint so `discoverWorkItems()` always returned `[]` and the Intent Check was silently skipped on every PR with linked Work Items (#247) diff --git a/apps/claude-code/unic-pr-review/package.json b/apps/claude-code/unic-pr-review/package.json index bcde4233..5cc2bc3e 100644 --- a/apps/claude-code/unic-pr-review/package.json +++ b/apps/claude-code/unic-pr-review/package.json @@ -22,5 +22,5 @@ "verify:changelog": "unic-verify-changelog" }, "type": "module", - "version": "2.1.9" + "version": "2.1.10" } From 02319e0e8d1a2e04c408fa945ba4da65c805d9f5 Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Tue, 16 Jun 2026 15:41:40 +0200 Subject: [PATCH 4/7] chore(unic-pr-review): apply Prettier to ADR-0016 to unblock CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR-0016 was committed directly to develop with non-conforming emphasis markers (`*x*` → `_x_`), turning the root Prettier check red on every PR. Formatting-only; unblocks #247 CI and repairs develop on merge. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../docs/adr/0016-human-threads-as-read-only-context.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apps/claude-code/unic-pr-review/docs/adr/0016-human-threads-as-read-only-context.md b/apps/claude-code/unic-pr-review/docs/adr/0016-human-threads-as-read-only-context.md index 21c9844d..33e44ef5 100644 --- a/apps/claude-code/unic-pr-review/docs/adr/0016-human-threads-as-read-only-context.md +++ b/apps/claude-code/unic-pr-review/docs/adr/0016-human-threads-as-read-only-context.md @@ -6,15 +6,15 @@ The ADO Fetcher fetches every PR Thread but classifies them on a single axis: does `comments[0].content` carry the Iteration Marker (a Bot Thread, ADR-0006) or not. In `first-review` mode the fetched Threads are then dropped — no step reads them — and in `re-review` only Bot Threads feed the Re-review Coordinator's `priorFindings`. Human review discussion is invisible to a Review in every Mode. -This surfaced in a head-to-head against the legacy `pr-review` plugin on ADO PR #5570. That plugin read the five Human Threads on the PR, cross-referenced a Finding to a still-open one, and noted a Thread marked *fixed* whose issue was in fact still present ("re-verify before merge"). `unic-pr-review` could surface none of this — a structural gap, not a fetch failure. +This surfaced in a head-to-head against the legacy `pr-review` plugin on ADO PR #5570. That plugin read the five Human Threads on the PR, cross-referenced a Finding to a still-open one, and noted a Thread marked _fixed_ whose issue was in fact still present ("re-verify before merge"). `unic-pr-review` could surface none of this — a structural gap, not a fetch failure. -The tempting fix — let a resolved Human Thread suppress or down-rank a matching Finding — is the mirror image of the work-item-discovery false negative (issue #247): where that bug made the Plugin *under-gather intent*, suppression would make it *under-report code issues*. A wrongly-resolved Thread would silently hide a real defect. +The tempting fix — let a resolved Human Thread suppress or down-rank a matching Finding — is the mirror image of the work-item-discovery false negative (issue #247): where that bug made the Plugin _under-gather intent_, suppression would make it _under-report code issues_. A wrongly-resolved Thread would silently hide a real defect. ## Decision Human Threads are **read-only context**, surfaced after the aspect fan-out and never used to suppress a Finding. -1. **Classification.** The Fetcher gains a human/system split alongside the existing bot detection: *Bot Thread* = Iteration Marker (ADR-0006); *System Thread* = `comments[0].commentType === "system"` (ref updates, votes, policy); *Human Thread* = neither. Only Human Threads are surfaced; System Threads are never shown to the Reviewer. +1. **Classification.** The Fetcher gains a human/system split alongside the existing bot detection: _Bot Thread_ = Iteration Marker (ADR-0006); _System Thread_ = `comments[0].commentType === "system"` (ref updates, votes, policy); _Human Thread_ = neither. Only Human Threads are surfaced; System Threads are never shown to the Reviewer. 2. **Context-only, never suppress.** A Finding is never dropped or down-ranked because a Human Thread exists. Confidence < 60 remains the sole filter (ADR-0002). From 216937dd9266a232ada1609342ccda4306679452 Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Tue, 16 Jun 2026 15:56:00 +0200 Subject: [PATCH 5/7] fix(unic-pr-review): address review findings for PR #249 Fixed: - Widen JSDoc @param type to id: string | number to match real ADO wire format - Update ADR-0001 amendment to correctly name pullrequestworkitems endpoint - Add comment to fetcher-contract test noting synthetic string ids vs real ADO integers Co-Authored-By: Claude Sonnet 4.6 --- ...rce-intent-with-shared-atlassian-credentials.md | 14 ++++++++------ .../providers/azure_devops/provider.mjs | 2 +- .../providers/azure_devops/tests/provider.test.mjs | 4 ++++ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/apps/claude-code/unic-pr-review/docs/adr/0001-multi-source-intent-with-shared-atlassian-credentials.md b/apps/claude-code/unic-pr-review/docs/adr/0001-multi-source-intent-with-shared-atlassian-credentials.md index d2de9d9f..c14bbc71 100644 --- a/apps/claude-code/unic-pr-review/docs/adr/0001-multi-source-intent-with-shared-atlassian-credentials.md +++ b/apps/claude-code/unic-pr-review/docs/adr/0001-multi-source-intent-with-shared-atlassian-credentials.md @@ -24,9 +24,11 @@ Reuse `~/.unic-confluence.json` as the shared Atlassian Credential File for both ## Amendment (2026-06) Work-item discovery is a **Provider contract**. Each Source Platform Provider exposes -`discoverWorkItems(prMetadata) → [{ id, type, url, raw }]`. For ADO, this reads the -PR's native `workItemRefs` field (not regex-scraping the description); for future -GitHub/GitLab Providers it will use their respective native linkages. The Intent Checker -stays Source-Platform-agnostic: it consumes the normalised list regardless of origin. -This separates "where did the Work Items come from?" (Provider contract) from "what -intent do they express?" (Intent Checker responsibility). +`discoverWorkItems(prMetadata) → [{ id, type, url, raw }]`. For ADO, the Fetcher +populates `prMetadata.workItemRefs` from the dedicated `pullrequestworkitems` endpoint +(Step 1.5 of the ADO Fetcher — the `pullrequests` response does not include Work Item +links); `discoverWorkItems` then reads that field (never regex-scraping the description). +For future GitHub/GitLab Providers it will use their respective native linkage endpoints. +The Intent Checker stays Source-Platform-agnostic: it consumes the normalised list +regardless of origin. This separates "where did the Work Items come from?" (Provider +contract) from "what intent do they express?" (Intent Checker responsibility). diff --git a/apps/claude-code/unic-pr-review/providers/azure_devops/provider.mjs b/apps/claude-code/unic-pr-review/providers/azure_devops/provider.mjs index f1bb8377..50739433 100644 --- a/apps/claude-code/unic-pr-review/providers/azure_devops/provider.mjs +++ b/apps/claude-code/unic-pr-review/providers/azure_devops/provider.mjs @@ -45,7 +45,7 @@ export const agents = { * PR's linked work-item endpoint (`pullrequestworkitems`, Step 1.5). Never * regex-scrapes the description — this is the ADR-0001 amendment contract. * - * @param {{ workItemRefs?: Array<{ id: string, url: string }> }} prMetadata + * @param {{ workItemRefs?: Array<{ id: string | number, url: string }> }} prMetadata * @returns {Array<{ id: string, type: string, url: string, raw: object }>} * @throws {Error} when `prMetadata` is not a PR-metadata object (guards against a * malformed stdin payload silently yielding zero Work Items) diff --git a/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs b/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs index 68e6cad7..023adb52 100644 --- a/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs +++ b/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs @@ -106,6 +106,10 @@ describe('fetcher contract — prMetadata.workItemRefs is always fetcher-populat assert.equal(meta.workItemRefs.length, 0) }) it('each workItemRef has string id and url', () => { + // Note: fixtures use synthetic string ids ("101") for simplicity. + // The real ADO pullrequestworkitems endpoint returns integer ids (101). + // discoverWorkItems normalises both via String(ref.id), so runtime is safe; + // this assertion matches the fixture shape, not the wire format. const meta = fixture('pr-with-work-items.json') for (const ref of meta.workItemRefs) { assert.equal(typeof ref.id, 'string', 'workItemRef.id must be a string') From fc73fc79c4b21509c398e9138f587ed28bf30124 Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Tue, 16 Jun 2026 16:17:18 +0200 Subject: [PATCH 6/7] fix(unic-pr-review): purge native-workItemRefs misconception from provider README, lock id normalisation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The provider README still documented discoverWorkItems as reading the PR's "native workItemRefs field" — the exact root-cause misconception #247 fixes — and claimed fixtures mirror the raw pullrequests response (which omits Work Item links). Both were reintroduction vectors for the bug this PR closes. Reword to match the corrected provider.mjs JSDoc / ADR-0001 amendment. Add a discoverWorkItems test feeding an integer wire-format id to lock the String(ref.id) coercion the JSDoc `id: string | number` widening opened; the existing fixture-shape tests only exercise string ids. Co-Authored-By: Claude Opus 4.8 --- .../unic-pr-review/providers/azure_devops/README.md | 9 ++++++--- .../providers/azure_devops/tests/provider.test.mjs | 12 ++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/apps/claude-code/unic-pr-review/providers/azure_devops/README.md b/apps/claude-code/unic-pr-review/providers/azure_devops/README.md index a828ec64..34f0b022 100644 --- a/apps/claude-code/unic-pr-review/providers/azure_devops/README.md +++ b/apps/claude-code/unic-pr-review/providers/azure_devops/README.md @@ -25,7 +25,9 @@ Throws `Not an ADO PR URL: ` when the URL does not match `prUrlPattern`. ## `discoverWorkItems(prMetadata)` -Reads the PR's **native** `workItemRefs` field and normalises each entry to +Reads the `workItemRefs` that the ADO Fetcher populates on `prMetadata` from the +dedicated `pullrequestworkitems` endpoint (Step 1.5 — the `pullrequests` response +does not carry Work Item links), and normalises each entry to `{ id, type: "ado-work-item", url, raw }`. It never regex-scrapes the PR description — work-item discovery is a Provider contract (ADR-0001 amendment). Returns `[]` when `workItemRefs` is empty or absent. @@ -39,8 +41,9 @@ Returns `[]` when `workItemRefs` is empty or absent. ## Adding fixtures -PR-metadata fixtures live in `fixtures/`. They mirror the shape returned by -`az devops invoke --area git --resource pullrequests`. Add a new fixture file and +PR-metadata fixtures live in `fixtures/`. They mirror the enriched `prMetadata` +shape — the `az devops invoke --area git --resource pullrequests` response with +`workItemRefs` grafted on by the Fetcher (Step 1.5). Add a new fixture file and reference it from `tests/provider.test.mjs` via the `fixture(name)` helper. The `fixtures/ado-cli-inventory.json` file catalogues every `az devops invoke` call the ADO Fetcher agent emits; the root `tests/ado-cli-smoke.test.mjs` asserts the diff --git a/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs b/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs index 023adb52..41db6cb0 100644 --- a/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs +++ b/apps/claude-code/unic-pr-review/providers/azure_devops/tests/provider.test.mjs @@ -83,6 +83,18 @@ describe('discoverWorkItems', () => { assert.equal(item.type, 'ado-work-item') } }) + it('normalises integer wire-format ids to strings', () => { + // The live pullrequestworkitems endpoint returns integer ids; fixtures use strings. + // discoverWorkItems must coerce via String(ref.id) so downstream id handling is stable + // regardless of wire shape — this exercises the integer branch the JSDoc `id: string | number` + // widening opened, which the fixture-shape tests below cannot reach. + const items = discoverWorkItems({ + workItemRefs: [{ id: 42622, url: 'https://dev.azure.com/FZAG/_apis/wit/workitems/42622' }], + }) + assert.equal(items.length, 1) + assert.equal(items[0].id, '42622') + assert.equal(typeof items[0].id, 'string') + }) }) // Fetcher contract: the ADO Fetcher (Step 1.5) must populate workItemRefs on prMetadata From 2e9eb448b732f7762c7164fde4fdba6a995b2a18 Mon Sep 17 00:00:00 2001 From: Oriol Torrent Florensa Date: Tue, 16 Jun 2026 16:22:07 +0200 Subject: [PATCH 7/7] fix(unic-pr-review): harden ado-fetcher Step 1.5 against zero-exit-malformed responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The original Step 1.5 used `WI_LIST.value ?? []`, which collapses a zero-exit response missing `.value` (auth-token race, proxy/HTML body, api-version drift) into an empty list with no warning — reintroducing the #247 silent false negative through a different door. Discriminate three outcomes: zero-exit + valid `value` (use it; empty is legitimate), non-zero exit (warn + []), and zero-exit-malformed (treat as fetch failure, warn + []). Also state explicitly that the warning joins the same `warnings` array emitted in Step 6, matching Steps 5a–5c. Markdown-only agent-prompt change; the ado-cli smoke test still passes. Co-Authored-By: Claude Opus 4.8 --- apps/claude-code/unic-pr-review/CHANGELOG.md | 2 +- apps/claude-code/unic-pr-review/agents/ado-fetcher.md | 10 +++++++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/apps/claude-code/unic-pr-review/CHANGELOG.md b/apps/claude-code/unic-pr-review/CHANGELOG.md index 3a86ab64..fa414a8f 100644 --- a/apps/claude-code/unic-pr-review/CHANGELOG.md +++ b/apps/claude-code/unic-pr-review/CHANGELOG.md @@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - (none) ### Fixed -- ADO Fetcher now fetches linked Work Items via `pullrequestworkitems` endpoint (Step 1.5) and populates `prMetadata.workItemRefs`; previously the fetcher never called this endpoint so `discoverWorkItems()` always returned `[]` and the Intent Check was silently skipped on every PR with linked Work Items (#247) +- ADO Fetcher now fetches linked Work Items via `pullrequestworkitems` endpoint (Step 1.5) and populates `prMetadata.workItemRefs`; previously the fetcher never called this endpoint so `discoverWorkItems()` always returned `[]` and the Intent Check was silently skipped on every PR with linked Work Items (#247). Step 1.5 discriminates three fetch outcomes so a zero-exit-but-malformed response can no longer masquerade as "no Work Items linked" ## [2.1.9] — 2026-06-09 diff --git a/apps/claude-code/unic-pr-review/agents/ado-fetcher.md b/apps/claude-code/unic-pr-review/agents/ado-fetcher.md index 2341d591..3a5b052c 100644 --- a/apps/claude-code/unic-pr-review/agents/ado-fetcher.md +++ b/apps/claude-code/unic-pr-review/agents/ado-fetcher.md @@ -48,12 +48,16 @@ az devops invoke --area git --resource pullrequestworkitems \ --http-method GET --api-version 7.0 --output json ``` -Store stdout as `WI_LIST`. Set `PR_METADATA.workItemRefs = WI_LIST.value ?? []`. +Store stdout as `WI_LIST` and capture the exit code. Distinguish three outcomes — never let a failure masquerade as the legitimate "no Work Items linked" case (the silent false negative this step exists to prevent): -If the command exits non-zero, set `PR_METADATA.workItemRefs = []` and add a warning: +- **Exit zero and stdout parses to an object with a `value` array** → set `PR_METADATA.workItemRefs = WI_LIST.value`. An empty `value` legitimately means no Work Items are linked — no warning. +- **Exit non-zero** → set `PR_METADATA.workItemRefs = []` and add the warning below (`` = `Error: `). +- **Exit zero but stdout is empty, unparseable, or has no `value` array** → treat as a fetch failure, not zero Work Items: set `PR_METADATA.workItemRefs = []` and add the warning below (`` = `unexpected response shape — do not assume zero Work Items`). + +In both failure branches add this warning (it belongs to the same `warnings` array emitted in Step 6): ``` -"pullrequestworkitems fetch failed — Work Item discovery skipped, Intent Check will be omitted. Error: " +"pullrequestworkitems fetch failed — Work Item discovery skipped, Intent Check will be omitted. " ``` Do not stop; continue to Step 2.