feat(unic-pr-review): intent gathering via Atlassian (Pre-PR pasted URLs)

[orioltf]

Why

Pre-PR mode reviewed code quality alone — the review agents had no notion of what the changed code was supposed to do, so intent gaps went undetected. This wires Work Item / Confluence intent into the Pre-PR path (issue #147), closing that gap: the reviewer pastes the relevant Jira/Confluence URL(s), the plugin fetches them, and every aspect agent receives the extracted intent as context.

What

• scripts/atlassian-fetch.mjs (new) — routes pasted URLs by path (/browse/ → Jira, /wiki/ → Confluence), fetches Work Items + pages via the Atlassian REST APIs using built-in fetch (Node 22+), parses Story ACs and Bug repro/expected/actual from ADF, and extracts linked Confluence URLs. Credentials via lib/credentials.mjs (file + env-var override). fetch is injectable for tests.
• agents/intent-checker.md (new, Ariadne, yellow) — calls the fetch script via Bash, synthesises an Intent Brief, emits per-AC verdicts, and hard-stops when a promised Confluence page is unreachable (ADR-0004). No Bot Signature footer.
• commands/review-pr.md — Step 3.5 prompts for optional URLs (Enter to skip, US 30), Step 3.6 spawns the Intent Checker and aborts on hard-stop (US 29), Step 4 broadcasts the Brief verbatim to the code-reviewer, Step 5 forwards intentCheck via INTENT_CHECK_JSON.
• scripts/render-summary.mjs — parses + validates optional INTENT_CHECK_JSON, drops malformed items with a stderr note, forwards survivors so the renderer surfaces the Intent Check block above the Severity sections (PRD §10).
• agents/code-reviewer.md — Step 3 now treats a provided Intent Brief as the authoritative source of acceptance criteria.
• Tests — new tests/atlassian-fetch.test.mjs (routing, key/page-id extraction, credential resolution, Story/Bug ADF parsing, Confluence excerpt + link extraction, error classification, collectIntent/main shape) and extended tests/render-summary.test.mjs (Intent Check rendering / omission / malformed-input handling).

Acceptance criteria

All nine ACs from #147 are covered — see the per-AC mapping in the implementation report. Empty intent omits the block (US 30); unreachable promised intent hard-stops naming the URL + setup command (AC-7).

Validation

• pnpm --filter unic-pr-review typecheck ✅
• pnpm --filter unic-pr-review test ✅ 222 passed / 0 failed
• pnpm --filter unic-pr-review verify:changelog ✅
• pnpm ci:check (repo-wide) ✅
• Clean-slate doctrine respected — no code/prompts taken from apps/claude-code/pr-review/.

Fixes #147

🤖 Generated with Claude Code

[orioltf]

🔍 Comprehensive PR Review

PR: #159 — feat(unic-pr-review): intent gathering via Atlassian (Pre-PR pasted URLs)
Reviewed by: 5 specialized agents (code-review, error-handling, test-coverage, comment-quality, docs-impact)
Date: 2026-05-28

---

Summary

Well-architected intent-gathering feature with injectable fetch for testability, a typed FetchError discriminator, and an infallible collectIntent boundary. All 222 tests pass and CLAUDE.md compliance is strong. Three issues should be resolved before merge: two README gaps that leave unic-pr-review undiscoverable, and missing test coverage for fetchConfluencePage error paths that protects the ADR-0004 hard-stop logic. There are also a functional bug (relative linkedUrls) and a silent-failure risk in the collectIntent catch block worth addressing.

Verdict: REQUEST_CHANGES

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	3
🟡 MEDIUM	6
🟢 LOW	8

---

🟠 HIGH Issues (Should Fix Before Merge)

1. unic-pr-review missing from root README plugins table

📍 README.md — plugins table

The plugins table lists four plugins but omits unic-pr-review. Users evaluating which plugin to install won't discover it and may install the older pr-review instead.

View fix

Add one row:

| [`unic-pr-review`](apps/claude-code/unic-pr-review/) | Claude Code | AI-powered PR reviews with Atlassian intent checking, confidence-scored findings, and Approval Loop |

---

2. unic-pr-review missing from README install commands block

📍 README.md — install commands block

The install block lists four plugins but omits unic-pr-review. A user discovering the plugin from the table cannot find the install command.

View fix

Append one line:

/plugin install unic-pr-review@unic-agent-plugins

---

3. fetchConfluencePage missing auth-error and not-found test coverage

📍 tests/atlassian-fetch.test.mjs — describe('fetchConfluencePage')

fetchJiraIssue tests all three FetchError kinds. fetchConfluencePage only tests unreachable. The ADR-0004 hard-stop logic hinges on kind === 'auth-error' — a regression in the shared fetchJson helper would go undetected for Confluence callers. (Also flagged at LOW by code-review agent.)

View fix

it('throws FetchError with kind auth-error on 401', async () => {
  await assert.rejects(
    () => fetchConfluencePage('https://unic.atlassian.net/wiki/spaces/X/pages/1', CREDS, {
      fetch: fetchStatus(401),
    }),
    (err) => /** @type {any} */ (err).kind === 'auth-error'
  )
})

it('throws FetchError with kind not-found on 404', async () => {
  await assert.rejects(
    () => fetchConfluencePage('https://unic.atlassian.net/wiki/spaces/X/pages/1', CREDS, {
      fetch: fetchStatus(404),
    }),
    (err) => /** @type {any} */ (err).kind === 'not-found'
  )
})

---

🟡 MEDIUM Issues (Options for Author)

4. Relative linkedUrls break the follow-linked-pages step

📍 scripts/atlassian-fetch.mjs:552 (extractConfluenceLinks) + :707 (fetchConfluencePage)

extractConfluenceLinks returns relative paths like /wiki/spaces/Y/pages/9. These reach routeUrl() which calls new URL(url) — throws on relative paths — and silently skips the link. The intent-checker's follow-linked-pages step never fires for internal Confluence links (the common case). Options: fix now | create issue | skip.

View details

Recommended fix — one-liner in fetchConfluencePage (already has confluenceBase in scope):

linkedUrls: extractConfluenceLinks(htmlBody).map((href) =>
    href.startsWith('http') ? href : `${confluenceBase}${href}`
),

Add a companion test asserting /wiki/spaces/Y/pages/9 → https://unic.atlassian.net/wiki/spaces/Y/pages/9.

Note: extractAbsoluteWikiUrls (the Jira counterpart) already returns only absolute URLs — this brings ConfluenceItem.linkedUrls to the same guarantee.

---

5. Internal parser exceptions map to unreachable, triggering false hard-stops

📍 scripts/atlassian-fetch.mjs — collectIntent catch block

Non-FetchError exceptions (e.g., a TypeError in parseJiraACs) are classified as kind: 'unreachable'. The intent-checker aborts with a "run setup-confluence" message when the real cause is a code defect. Options: fix now | create issue | skip.

View details

Recommended fix — use parse-error for non-FetchErrors (soft failure, not hard-stop):

} catch (err) {
    if (err instanceof FetchError) {
        errors.push({ url, kind: err.kind, message: err.message })
    } else {
        const msg = err instanceof Error ? (err.stack ?? err.message) : String(err)
        stderr.write(`atlassian-fetch: internal error processing ${url}: ${msg}\n`)
        errors.push({ url, kind: 'parse-error', message: msg })
    }
}

---

6. parse-error is a silent soft failure — missing AC coverage without warning

📍 agents/intent-checker.md Step 3 + scripts/atlassian-fetch.mjs:fetchJson

When Atlassian returns an HTML maintenance page with HTTP 200, fetchJson emits kind: 'parse-error'. The intent-checker silently omits the Work Item. The review runs and passes, but intent was never checked for that item. Options: fix now | create issue | skip.

View details

Recommended fix — update intent-checker.md Step 6 to surface parse-error items with a warning:

For each `errors` entry whose `kind` is `"parse-error"` or `"not-found"`, include a
warning line in the brief: "⚠️ <id|url>: could not be loaded (<message>)."

---

7. mapFetchError timeout branch untested

📍 scripts/atlassian-fetch.mjs:407–409

The TimeoutError path in mapFetchError is never exercised. A broken err.name === 'TimeoutError' check would show a generic JS error message instead of "Request timed out after 15s". Options: fix now | create issue.

View test

describe('mapFetchError', () => {
  it('returns a human-readable message for TimeoutError', () => {
    const err = Object.assign(new Error('The operation was aborted'), { name: 'TimeoutError' })
    assert.match(mapFetchError(err), /timed out/)
  })
})

---

8. classifyIssueType Edge cases (Epic, Defect, other) untested + no description

📍 scripts/atlassian-fetch.mjs:413–422

Two related issues from two agents: (a) no prose description — the epic→story business rule is invisible to future maintainers; (b) only 'Story' and 'Bug' are tested — a regression dropping 'Epic' to 'other' would silently lose Epic ACs. Options: fix now | create issue.

View fixes

Add description:

/**
 * Map a Jira issue type name to the three canonical buckets used for
 * intent extraction. Epics are bucketed as `'story'` because they can
 * carry acceptance criteria in the same AC-heading format.
 */
function classifyIssueType(typeName) { … }

Add tests: fetchJiraIssue with issuetype: { name: 'Epic' } → type === 'story', { name: 'Defect' } → type === 'bug', { name: 'Task' } → type === 'other'.

---

🟢 LOW Issues

View 8 low-priority suggestions

Issue	Location	Agent	Suggestion
Exit 0 for per-URL auth errors is by design but undocumented	atlassian-fetch.mjs:813	code-review	Add comment explaining why exit 0 is correct for per-URL errors (JSON inspection is the intended path)
extractConfluencePageId null fallback produces opaque HTTP 404	fetchConfluencePage	error-handling	Guard: if (pageId === null) throw new FetchError(..., 'not-found', 'could not extract page ID')
JSON.stringify(result) in main() unguarded	atlassian-fetch.mjs:main	error-handling	Wrap with descriptive error for future non-serialisable shapes
collectIntent non-FetchError else branch is dead code	atlassian-fetch.mjs:538	test-coverage	Skip — unreachable through production paths; acknowledged defensive guard
parseJiraACs orderedList variant untested	atlassian-fetch.mjs:204	test-coverage	Add one test with orderedList instead of bulletList
customfield_10016 requested but never consumed	atlassian-fetch.mjs:435	comment-quality	Add comment if intentional (story points placeholder); remove if accidental
extractAbsoluteWikiUrls example too narrow	atlassian-fetch.mjs:330	comment-quality	Broaden: "...or plain-text descriptions with bare links"
docs/ structure section in README outdated	README.md	docs-impact	Replace docs/└── plans/ with actual 6-subdirectory layout

---

✅ What's Good

• Injectable fetch + deps pattern — clean, consistent with doctor.mjs, zero new testing infrastructure needed
• FetchError typed discriminator (unreachable | not-found | auth-error | parse-error) — intent-checker branches on kind without string-parsing
• collectIntent never throws — correct infallible boundary for a multi-URL dispatcher
• render-summary.mjs INTENT_CHECK_JSON — six-layer defensive validation, all tested
• Module docstring — names purpose, contract, credential source, HTTP approach, testability in under 10 lines
• ADR + AC back-references in inline comments (// AC-7, // ADR-0004) — exactly the right "why" style
• CONTEXT.md thorough — all domain terms pre-defined, zero documentation catch-up needed
• 222 tests pass including all new intent-check rendering cases

---

📋 Suggested Follow-up Issues

Issue Title	Priority
"Surface parse-error warnings in Intent Brief"	P2
"Classify internal errors in collectIntent as parse-error not unreachable"	P2
"Add fetchConfluencePage timeout + parse-error tests"	P3

---

Reviewed by Archon comprehensive-pr-review workflow · 5 parallel agents
Full artifacts: /Users/oriol.torrent/.archon/workspaces/unic/unic-agents-plugins/artifacts/runs/a9d5dfb5f5c1d6d5731cdc0c3457c672/review/

[orioltf]

⚡ Self-Fix Report (Aggressive)

Status: COMPLETE
Pushed: ✅ Changes pushed to archon/task-fix-issue-147
Commit: b81f3f5
Philosophy: Fix everything unless clearly a new concern

---

Fixes Applied (16 total)

Severity	Count
🔴 CRITICAL	0
🟠 HIGH	3
🟡 MEDIUM	6
🟢 LOW	7

View all fixes

HIGH

• ✅ unic-pr-review missing from root README plugins table (README.md) — Added row to plugins table
• ✅ unic-pr-review missing from README install commands block (README.md) — Added /plugin install unic-pr-review@unic-agent-plugins
• ✅ fetchConfluencePage missing auth-error and not-found tests (tests/atlassian-fetch.test.mjs) — Added 401 → auth-error and 404 → not-found test cases

MEDIUM

• ✅ Relative linkedUrls break follow-linked-pages step (scripts/atlassian-fetch.mjs:fetchConfluencePage) — Relative hrefs now resolved against confluenceBase; added absolute-URL test
• ✅ Internal parser exceptions map to unreachable → false hard-stops (scripts/atlassian-fetch.mjs:collectIntent) — Non-FetchError now emits to stderr and records as parse-error (soft)
• ✅ parse-error is silent soft failure — missing data without warning (agents/intent-checker.md:Step 6) — Step 6 now requires ⚠️ warning line + unaddressed intentCheck entry for parse-error/not-found
• ✅ mapFetchError timeout branch untested (tests/atlassian-fetch.test.mjs) — Added 4-case mapFetchError describe block
• ✅ classifyIssueType edge cases (Epic, Defect, Task) untested (tests/atlassian-fetch.test.mjs) — Added Epic→story, Defect→bug, Task→other tests
• ✅ classifyIssueType missing JSDoc prose (scripts/atlassian-fetch.mjs:classifyIssueType) — Added description explaining epic→story business rule

LOW

• ✅ Exit code 0 for per-URL auth errors undocumented (scripts/atlassian-fetch.mjs) — Added comment explaining exit-0 design and ADR-0004 intent
• ✅ extractConfluencePageId null fallback produces opaque HTTP error (scripts/atlassian-fetch.mjs:fetchConfluencePage) — Now throws FetchError('not-found', ...) immediately when pageId is null
• ✅ JSON.stringify(result) in main() not wrapped in try/catch (scripts/atlassian-fetch.mjs:main) — Wrapped with descriptive error message
• ✅ parseJiraACs orderedList variant untested (tests/atlassian-fetch.test.mjs) — Added orderedList test case
• ✅ customfield_10016 requested but never consumed (scripts/atlassian-fetch.mjs:fetchJiraIssue) — Added inline comment: story points, included for future caller use
• ✅ extractAbsoluteWikiUrls example too narrow (scripts/atlassian-fetch.mjs) — Broadened JSDoc example to mention bare links
• ✅ docs/ structure section in README outdated (README.md) — Replaced plans/ with accurate 6-entry subtree

---

Tests Added

• fetchConfluencePage: 401 → auth-error, 404 → not-found, relative hrefs → absolute URLs
• fetchJiraIssue: Epic → story, Defect → bug, Task → other
• parseJiraACs: orderedList variant
• mapFetchError: TimeoutError message, timeout seconds, generic Error, non-Error stringify

11 new test cases — total now 233 (all passing)

---

Skipped (1)

Finding	Reason
collectIntent non-FetchError else branch dead code	Test-coverage report itself recommends skipping — branch is unreachable through production paths

---

Suggested Follow-up Issues

(none — all findings fully addressed)

---

Validation

✅ Type check | ✅ Lint (Biome + Prettier) | ✅ Tests (233 passed)

---

Self-fix by Archon · aggressive mode · fixes pushed to archon/task-fix-issue-147

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.