diff --git a/apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-pr-review.md b/apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-pr-review.md new file mode 100644 index 00000000..be4d13d6 --- /dev/null +++ b/apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-pr-review.md @@ -0,0 +1,96 @@ +--- +description: Run the unic-archon-dlc PR review workflow — seven intent-grounded aspects fanned out, then a summary comment + inline comments posted/updated on the current PR, with iteration-aware re-review. +--- + +# /unic-dlc-pr-review + +Runs the `pr-review` box: composes a shared **Intent Brief** (from the linked work items, Confluence/MD +docs, the PR description, and `PRD.md`), fans out **seven review aspects** as parallel fresh nodes +(code-quality, test-coverage, silent-failure, type-design, comment-rot, code-simplification, and an +intent/AC-coverage check), synthesises the findings, **reconciles them against the prior iteration** +(new / still-present / fixed / regressed), and — after a config-gated human confirm — posts or updates a +single structured **summary comment** plus **inline comments** on the current PR. + +Generic and **self-contained**: it harvests `unic-pr-review`'s review learnings (confidence rubric, +structured summary, hidden-marker idempotency, conditional spawn table, two-surface posting) **without +its ADO code and without any runtime dependency** ([ADR-0016](../../docs/adr/0016-dlc-thin-process-layer.md)/ +[ADR-0017](../../docs/adr/0017-container-follows-structural-need.md)). Ported to the key-discriminated +Archon node schema ([ADR-0011](../../docs/adr/0011-archon-schema-target.md)); design in +[ADR-0026](../../docs/adr/0026-pr-review-generic-archon-harvest.md). + +## Usage + +``` +/unic-dlc-pr-review +``` + +`` is the session identifier from `/unic-archon-dlc:specs` / `tickets` / `build`. It scopes the +review artefacts (`//pr-review/`) and locates `PRD.md`. There is **no PRD +precondition** — intent is composed from whatever sources resolve. + +## What this workflow does + +1. **bootstrap** — parse the slug from `$ARGUMENTS`, read `.archon/unic-dlc.config.yaml` + (`artifacts_dir`, `gates.pr-review`, `pr-review.confidence_threshold`, `pr-review.inline_comments`, + `tracker.type`, `docs.*`, `project.branching`). Missing slug/config cancels cleanly. + +2. **prep** — identify the open PR + its description; compute the diff and **categorise** the changed + files (for the spawn gates); compose **one Intent Brief** from the linked work items, Confluence/MD + docs, the PR body, and `PRD.md` (recording any **contradictions across sources**); and detect the + **prior review iteration** by its hidden marker. Writes everything to `//pr-review/`. + +3. **7 aspect nodes** (parallel, fresh) — each reads the shared Intent Brief (**every aspect is + intent-grounded**) + the diff and emits findings scored on the **confidence rubric** (90–100 Critical + / 80–89 Important / 60–79 Minor / below the threshold dropped). **Spawn gates** run each aspect only + when meaningful: code-quality + intent-check always; tests/type-design/comment-rot/simplifier/ + silent-failure gated on the changed-file categories. + +4. **synthesize** — merge + dedupe this run's findings, bucket by severity, assemble the summary + sections + Intent Check + "What's good". + +5. **reconcile** — the re-review coordinator: classify each finding against the prior iteration + (new / still-present / fixed / regressed), compute the "since iteration N−1" delta, and finalise the + summary comment (with the `Iteration N` marker + footer) and the inline-comment plan. + +6. **review-gate** — **HITL by default** (`gates.pr-review`); skipped in AFK. Shows the composed summary, + the finding counts, and any contradiction warnings; APPROVE to post, REJECT to halt without posting. + +7. **post** — post/update the **summary comment** (matched by the `` + marker, never author identity) and, when `inline_comments` and the tracker supports inline threads, + reconcile **inline comments** per finding (still-present → update, fixed → resolve, regressed → + reopen, new → create). Trackers without inline threads (jira / local-markdown) degrade to + summary-only. + +## Gates & AFK + +`gates.pr-review` (default `hitl`) governs the review-gate. Set it to `afk` to run unattended — the gate +is skipped and both surfaces post directly. Posting is **advisory / non-blocking**: unlike `/qa`, +`/pr-review` never merges anything, so there is no fail-closed merge guard here — the real merge +checkpoint is `/qa`. + +## Prerequisites + +- The current branch has an open PR (for the summary + inline comments). +- `.archon/unic-dlc.config.yaml` is present (from `/unic-archon-dlc:setup`). +- The configured tracker CLI/MCP is reachable (`gh` / `az` / `jira`, or the `azure-devops-cli` skill). +- Archon ≥ 0.5.0. + +## Configuration reference + +Read from `.archon/unic-dlc.config.yaml`: + +| Field | Type | Default | Description | +| -------------------------------- | ------------ | ----------- | -------------------------------------------------------------------------------- | +| `gates.pr-review` | `hitl`/`afk` | `hitl` | HITL pauses at the review-gate before posting; AFK posts directly | +| `pr-review.confidence_threshold` | number | `60` | Findings below this confidence are dropped before posting | +| `pr-review.inline_comments` | boolean | `true` | Post inline per-finding comments in addition to the summary (where supported) | +| `artifacts_dir` | string | `workflows` | Session artefact home (`//pr-review/`) | +| `tracker.*` | object | — | Composed to read the PR, work items, and post comments (MCP-first, CLI-fallback) | +| `docs.*` | object | — | Composed to fetch Confluence/MD intent sources | +| `project.branching` | string | `gitflow` | `gitflow` → base `develop`; else `main` (for the merge-base diff) | + +## Runs + +``` +archon workflow run unic-dlc-pr-review --input +``` diff --git a/apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-review.md b/apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-review.md deleted file mode 100644 index 3f016f28..00000000 --- a/apps/claude-code/unic-archon-dlc/.archon/commands/unic-dlc-review.md +++ /dev/null @@ -1,82 +0,0 @@ ---- -description: Run the unic-archon-dlc self-contained code review workflow — four aspects, structured findings, single PR comment, idempotent re-run ---- - -# /unic-dlc-review - -Runs the `review` workflow: analyses the current PR diff across four review aspects -(code quality, test coverage, silent failures, type design), then posts — or updates — a -single structured comment on the PR. - -**No runtime dependency on pr-review-toolkit or any other plugin.** -All review logic is self-contained in `.archon/workflows/unic-dlc-review.yaml`. - -## When to use - -- After opening a PR and before requesting human review. -- As a final quality gate before merging. -- Re-run it after addressing feedback — it will **update** the prior comment rather than - posting a duplicate. - -## What it produces - -A single structured PR comment (or updated comment on re-run) with four sections: - -1. **Code quality** — readability, naming, function length, project-convention adherence -2. **Test coverage adequacy** — exercises public interfaces, output-focused assertions, no - internal-collaborator mocks -3. **Silent failure patterns** — swallowed exceptions, empty catch blocks, fallbacks that - hide bugs -4. **Type design quality** — encapsulation, invariants in types, illegal-state-unrepresentable - patterns - -Each section lists specific findings with `file:line` references, or an explicit -"No findings." line if nothing was detected. - -## Usage - -```sh -archon workflow run unic-dlc-review -``` - -Or invoke from Claude Code: - -``` -/unic-dlc-review -``` - -No arguments needed. The workflow reads `.archon/unic-dlc.config.json` to determine the -tracker and the current open PR. - -## Workflow structure - -``` -code-review ──▶ structured comment posted / updated on PR -``` - -Single-node DAG. The node reads CLAUDE.md / AGENTS.md for project conventions, runs the -four review aspects, and calls the tracker adapter to post or update the comment. - -## Re-run behaviour - -When you run `/unic-dlc-review` again on the same PR: - -- The workflow searches for a prior comment whose body contains the sentinel marker - ``. -- If found, it **updates** that comment in-place. -- If not found, it creates a new comment. - -This prevents duplicate review threads accumulating over multiple runs. - -## Inspiration - -- `apps/claude-code/pr-review/` — multi-aspect analysis structure, compact finding - schema (`severity / filePath / startLine / endLine / title / body`), re-review - detection via sentinel, and the discipline of posting a single summary comment rather - than many inline threads. -- Matt Pocock's review skill - (https://github.com/mattpocock/skills/blob/main/skills/in-progress/review/SKILL.md) — - aspect-driven review structure, explicit "no findings" lines, and the principle that - every run should produce actionable output even when everything looks good. -- This plugin's `lib/tracker-adapter.mjs` — used to post or update the comment via the - configured tracker backend (github / ado / jira / local-markdown). diff --git a/apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-pr-review.yaml b/apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-pr-review.yaml new file mode 100644 index 00000000..ea2950f3 --- /dev/null +++ b/apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-pr-review.yaml @@ -0,0 +1,448 @@ +name: unic-dlc-pr-review +description: > + Generic, self-contained PR review. Fans out seven review aspects (code-quality, test-coverage, + silent-failure, type-design, comment-rot, code-simplification, and intent/AC-coverage) as parallel + fresh Archon nodes, each grounded in a shared Intent Brief composed once in prep from the linked work + items, Confluence/MD docs, the PR description, and PRD.md. Synthesises this run's findings, reconciles + them against the prior iteration (new / still-present / fixed / regressed), then — after a config-gated + human confirm — posts/updates one structured summary comment plus inline comments on the current PR via + the configured tracker (MCP-first, CLI-fallback). Harvests unic-pr-review's review learnings without its + ADO code or any dependency (ADR-0016/0017). Ported to the key-discriminated node schema (ADR-0011); + inherits /build's self-contained-node conventions (ADR-0023 §5). See ADR-0026. + +# Required at the workflow level so the review-gate approval message reaches the user (ADR-0011). +interactive: true + +nodes: + # --------------------------------------------------------------------------------------------- + # bootstrap — parse the slug from $ARGUMENTS, read config. A prompt node (NOT a script) so it needs + # no plugin-lib import and no $CLAUDE_PLUGIN_ROOT (ADR-0023 §5): it reads .archon/unic-dlc.config.yaml + # with its own tools. Posting is AFK-safe; the human checkpoint here is the review-gate, and the real + # merge gate lives in /qa. + # --------------------------------------------------------------------------------------------- + - id: bootstrap + depends_on: [] + context: fresh + allowed_tools: [Read, Bash, Glob] + prompt: | + You are the bootstrap node of the unic-archon-dlc /pr-review workflow. + + 1. Parse the FIRST whitespace-delimited token of "$ARGUMENTS" as the slug (kebab-case). + If $ARGUMENTS is empty → status "no-slug". + 2. Read `.archon/unic-dlc.config.yaml` at the repo root. + - If it is absent or unreadable → status "no-config". + - Otherwise read: + `artifacts_dir` (default "workflows") + `gates.pr-review` (default "hitl") + `pr-review.confidence_threshold` (default 60) + `pr-review.inline_comments` (default true) + `tracker.type` (e.g. github / ado / jira / local-markdown) + `docs.type` / `docs.access` (for intent composition from docs) + `project.branching` (default "gitflow") + 3. If slug and config are both present → status "ready". (There is no PRD precondition: intent is + composed from whatever sources resolve — see prep.) + + Compute `expected_base` from branching: "develop" when branching == "gitflow", else "main". + ALWAYS emit EVERY required field, including in the no-slug / no-config branches, so the object + validates before `guard-not-ready` can cancel. When a value cannot be read, fall back to: slug "" + (if no-slug), artifacts_dir "workflows", gate "hitl", confidence_threshold 60, inline_comments + "true", tracker_type "", branching "gitflow", expected_base "develop". `inline_comments` is emitted + as the STRING "true"/"false" (enum) so downstream `when` comparisons are reliable. Set `message` to + a human-readable reason for any non-ready status. + output_format: + type: object + properties: + status: + type: string + enum: [ready, no-slug, no-config] + slug: { type: string } + artifacts_dir: { type: string } + gate: + type: string + enum: [hitl, afk] + confidence_threshold: { type: number } + inline_comments: + type: string + enum: ['true', 'false'] + tracker_type: { type: string } + branching: { type: string } + expected_base: { type: string } + message: { type: string } + required: [status, slug, artifacts_dir, gate, confidence_threshold, inline_comments, tracker_type, branching, expected_base, message] + + # --------------------------------------------------------------------------------------------- + # guard — expected precondition failures cancel cleanly (ADR-0011: cancel vs fail). + # --------------------------------------------------------------------------------------------- + - id: guard-not-ready + depends_on: [bootstrap] + when: "$bootstrap.output.status != 'ready'" + cancel: > + /pr-review preconditions not met — see the bootstrap output for whether the slug or + .archon/unic-dlc.config.yaml is missing. Pass a slug (`/unic-dlc-pr-review `) and run + /unic-archon-dlc:setup first if there is no config. + + # --------------------------------------------------------------------------------------------- + # prep — the shared context builder. Identifies the open PR, computes the diff, categorises the + # changed files (for the spawn gates), composes ONE Intent Brief from every available source, detects + # the prior review iteration, and writes everything to //pr-review/ so the fanned + # aspect nodes read it off disk (fresh nodes share no memory — ADR-0012/0015). Composing intent once + # and injecting it into every aspect is the primary quality lever (ADR-0026): each aspect judges the + # diff against what the PR is SUPPOSED to do. + # --------------------------------------------------------------------------------------------- + - id: prep + depends_on: [bootstrap] + when: "$bootstrap.output.status == 'ready'" + context: fresh + allowed_tools: [Read, Bash, Glob, Grep] + prompt: | + You are the prep node of /pr-review for slug "$bootstrap.output.slug". + + Constants: + SLUG = $bootstrap.output.slug + ARTIFACTS_DIR = $bootstrap.output.artifacts_dir + SESSION = //pr-review/ + TRACKER = $bootstrap.output.tracker_type + Create SESSION if it does not exist. + + ## 1. Identify the open PR + read its description + Compose the configured tracker (MCP-first, CLI-fallback — never hardcode a host, ADR-0016), reading + `tracker` from `.archon/unic-dlc.config.yaml`: + - github → `gh pr view --json number,title,body,headRefName,baseRefName` + - ado → `az repos pr list --status active` then `az repos pr show` for the current branch + - jira / local-markdown / other → there may be no queryable PR; note it and continue. + Store PR_ID, PR_TITLE, and the PR DESCRIPTION BODY (an intent source). + + ## 2. Compute the diff + categorise changed files + BASE_REF = the upstream base (merge-base of the PR base branch and HEAD). Try + `git merge-base origin/$bootstrap.output.expected_base HEAD`; fall back to `HEAD~1`. + Write the unified diff to `SESSION/diff.patch` (`git diff ...HEAD`) and the changed-file + list to `SESSION/changed-files.txt` (`git diff --name-only ...HEAD`). + Categorise the changed files into these STRING booleans ("true"/"false"): + has_source — any non-test source file (.mjs/.js/.ts/.py/… excluding tests/docs) + has_tests — any test file (*.test.*, *.spec.*, test/, tests/, __tests__/) + has_types — any type/JSDoc-type change (.ts/.d.ts, or added `@param`/`@returns`/`@type`) + has_docs — any .md/.mdx/doc file OR added/removed comment lines + touches_error_handling — the diff adds/changes try/catch, `.catch(`, `??`/`||` fallbacks, or early returns on error + + ## 3. Compose ONE Intent Brief (inject into every aspect) + Gather intent from EVERY source that resolves — none is individually required: + a. Linked work items — the PR's linked User Story / Bug / Jira ticket (from the PR body, branch + name, or tracker links). Fetch via the configured tracker (MCP-first, CLI-fallback). + b. Confluence / MD docs — pages referenced in the work item or PR body, or discoverable via the + configured docs system-skill (`docs.type`/`docs.access`). + c. The PR description body (from step 1). + d. `//PRD.md` if it exists (what /specs wrote). + Write `SESSION/intent-brief.md`: a short narrative of the intended behaviour + a numbered + Acceptance Criteria list (AC-1, AC-2, …) distilled across the sources. + Set `intent_available` = "true" only if at least one source resolved; else "false" (and write a + one-line brief saying no intent sources were found). + **Contradictions:** when sources disagree (e.g. the work item says one thing, the PR body another), + record each as a short string in `contradictions` AND note it at the top of intent-brief.md. Do not + silently pick a winner. + + ## 4. Detect the prior review iteration + Scan the PR's existing comments/threads for the hidden marker `` + (match on the MARKER, never on comment author). Take the HIGHEST N found. Set `iteration` = N+1 + (or 1 if none). Parse the prior findings from the prior summary + inline markers + (``) into `prior_findings` (each: hash, aspect, file, + title) so reconcile can classify against them. If the tracker cannot list comments, set iteration 1 + and prior_findings []. Write `SESSION/prep.json` mirroring the emitted object for durability. + + ALWAYS emit EVERY required field so the object validates (downstream nodes and `prep.json` assume + they exist). When no PR is queryable, use `pr_id` "" and `pr_title` "". `contradictions` and + `prior_findings` are ALWAYS arrays — `[]` when there is nothing to report. Emit the structured object. + output_format: + type: object + properties: + pr_id: { type: string } + pr_title: { type: string } + base_ref: { type: string } + has_source: { type: string, enum: ['true', 'false'] } + has_tests: { type: string, enum: ['true', 'false'] } + has_types: { type: string, enum: ['true', 'false'] } + has_docs: { type: string, enum: ['true', 'false'] } + touches_error_handling: { type: string, enum: ['true', 'false'] } + intent_available: { type: string, enum: ['true', 'false'] } + contradictions: { type: array, items: { type: string } } + iteration: { type: number } + prior_findings: + type: array + items: + type: object + properties: + hash: { type: string } + aspect: { type: string } + file: { type: string } + title: { type: string } + required: [hash, aspect, file, title] + required: [pr_id, pr_title, base_ref, has_source, has_tests, has_types, has_docs, touches_error_handling, intent_available, contradictions, iteration, prior_findings] + + # ============================================================================================= + # ASPECT FAN-OUT — seven parallel fresh nodes, each depends_on [prep]. Every aspect reads the shared + # SESSION/intent-brief.md (intent-grounded), reads SESSION/diff.patch + SESSION/changed-files.txt, and + # WRITES its findings to SESSION/findings/.json so synthesize reads them off disk (a skipped + # aspect simply writes nothing — no skipped-node-output reference). Conditional spawn gates + # (harvested SPAWN_TABLE, biased to over-spawn): code-quality + intent-check always run. + # + # Every aspect applies the SAME finding contract: + # - Confidence 0–100 → severity: 90–100 critical, 80–89 important, 60–79 minor, + # < $bootstrap.output.confidence_threshold → DROP before writing. + # - Finding shape: { aspect, severity, confidence, file, line, title (≤80 chars, no trailing period), + # body (1–4 sentences), suggestion? (only when a mechanical drop-in fix exists), + # hash (stable: sha1-ish of aspect+file+normalised-title, NOT the line number) } + # - Calibrate against the Intent Brief: do NOT flag a spec-mandated choice as a defect; DO flag a + # documented requirement the diff misses. + # - Also collect `positives` (things done well) for the "What's good" section. + # Write {"findings": [...], "positives": [...]} to SESSION/findings/.json. + # ============================================================================================= + + - id: code-quality + depends_on: [prep] + context: fresh + allowed_tools: [Read, Bash, Glob, Grep] + prompt: | + Aspect: code-quality. Read the shared finding contract from the workflow (severity rubric, finding + shape, intent calibration). SLUG=$bootstrap.output.slug, SESSION=//pr-review/, + threshold=$bootstrap.output.confidence_threshold. + Read SESSION/intent-brief.md, SESSION/diff.patch, SESSION/changed-files.txt, and the project's + conventions (CLAUDE.md / AGENTS.md). Review the diff for readability, naming that communicates + intent, over-long/multi-purpose functions, duplicated logic, and project-convention adherence + (indentation, quotes, semicolons, early returns). Always runs (baseline aspect). Write + SESSION/findings/code-quality.json. + + - id: tests + depends_on: [prep] + when: "$prep.output.has_tests == 'true'" + context: fresh + allowed_tools: [Read, Bash, Glob, Grep] + prompt: | + Aspect: tests (test-coverage adequacy). Apply the shared finding contract. + SESSION=/$bootstrap.output.slug/pr-review/, threshold=$bootstrap.output.confidence_threshold. + Read SESSION/intent-brief.md, SESSION/diff.patch, SESSION/changed-files.txt. Flag: new branches/ + functions with no test, happy-path-only tests missing sad/edge paths, assertions on implementation + detail rather than behaviour, mocked internal collaborators (only true external boundaries should + be mocked), async under test with no rejection assertion, and Acceptance Criteria (from the Intent + Brief) with no visible test coverage. Write SESSION/findings/tests.json. + + - id: silent-failure + depends_on: [prep] + when: "$prep.output.has_source == 'true' || $prep.output.touches_error_handling == 'true'" + context: fresh + allowed_tools: [Read, Bash, Glob, Grep] + prompt: | + Aspect: silent-failure. Apply the shared finding contract. + SESSION=/$bootstrap.output.slug/pr-review/, threshold=$bootstrap.output.confidence_threshold. + Read SESSION/intent-brief.md, SESSION/diff.patch, SESSION/changed-files.txt. Flag: empty catch + blocks, `.catch(() => {})`, swallowed exceptions (caught + logged but not rethrown/propagated), + inappropriate `?? default` / `|| fallback` masking missing config or corrupt state, retries that + exhaust then proceed silently, wrong error-property access, ignored error-signalling return values, + and lost-signal / observability gaps (a fallback or early-return branch that skips a telemetry / + analytics / Sentry event the normal path emits — especially one the Intent Brief requires). Write + SESSION/findings/silent-failure.json. + + - id: type-design + depends_on: [prep] + when: "$prep.output.has_types == 'true'" + context: fresh + allowed_tools: [Read, Bash, Glob, Grep] + prompt: | + Aspect: type-design. Apply the shared finding contract. + SESSION=/$bootstrap.output.slug/pr-review/, threshold=$bootstrap.output.confidence_threshold. + Read SESSION/intent-brief.md, SESSION/diff.patch, SESSION/changed-files.txt. Flag: types too wide + (`any`, `{}`, bare `string` where a union/branded type fits), types admitting illegal states, + primitive obsession, anemic data-bag types, assertions that should be narrowing guards, missing + exhaustiveness checks, exposed mutable internals, invariants enforced by convention rather than the + type/constructor, and (for `.mjs` `// @ts-check` files) missing JSDoc on exported params/returns. + Write SESSION/findings/type-design.json. + + - id: comment-rot + depends_on: [prep] + when: "$prep.output.has_docs == 'true'" + context: fresh + allowed_tools: [Read, Bash, Glob, Grep] + prompt: | + Aspect: comment-rot. Apply the shared finding contract. + SESSION=/$bootstrap.output.slug/pr-review/, threshold=$bootstrap.output.confidence_threshold. + Read SESSION/intent-brief.md, SESSION/diff.patch, SESSION/changed-files.txt. Flag: comments + describing behaviour the diff changed, JSDoc `@param`/`@returns` mismatching the signature, + over-documentation restating code, stale TODO/FIXME, markdown docs contradicted by the diff, and + "temporary/for now" comments the diff makes permanent. Write SESSION/findings/comment-rot.json. + + - id: simplifier + depends_on: [prep] + when: "$prep.output.has_source == 'true'" + context: fresh + allowed_tools: [Read, Bash, Glob, Grep] + prompt: | + Aspect: simplifier (unnecessary complexity). Apply the shared finding contract. + SESSION=/$bootstrap.output.slug/pr-review/, threshold=$bootstrap.output.confidence_threshold. + Read SESSION/intent-brief.md, SESSION/diff.patch, SESSION/changed-files.txt. Flag: nested + conditionals flattenable to guard clauses, over-long multi-purpose functions, duplicated logic, + clever one-liners hurting readability, boolean-flag params, `if (x) return true else return false`, + needless `async`, `.then` chains that should be `await`, and nested ternaries. Prefer findings that + preserve behaviour exactly. Write SESSION/findings/simplifier.json. + + - id: intent-check + depends_on: [prep] + context: fresh + allowed_tools: [Read, Bash, Glob, Grep] + prompt: | + Aspect: intent-check (AC-coverage). Apply the shared finding contract for any quality findings, but + your PRIMARY output is per-AC verdicts. SESSION=/$bootstrap.output.slug/pr-review/. + Read SESSION/intent-brief.md, SESSION/diff.patch, SESSION/changed-files.txt. + - If $prep.output.intent_available == "false": write SESSION/findings/intent-check.json as + {"findings": [], "positives": [], "ac_verdicts": [], "note": "no intent sources"} and stop. + - Otherwise, for each Acceptance Criterion in the Intent Brief, judge diff coverage and emit a + verdict: "addressed" | "partially addressed" | "unaddressed" (this is a coverage judgment, not a + code-quality one). Echo any `contradictions` from prep as findings so they surface in the summary. + Write SESSION/findings/intent-check.json as + {"findings": [...], "positives": [...], "ac_verdicts": [{"id","text","verdict"}...]}. + + # --------------------------------------------------------------------------------------------- + # synthesize — THIS RUN ONLY. Reads every SESSION/findings/.json that exists (skipped aspects + # wrote none), dedupes overlaps, buckets by severity, assigns each finding its stable hash marker, and + # assembles the severity-grouped body sections + the Intent Check section + the contradictions callout. + # No prior-iteration awareness here — that is reconcile's job. trigger_rule: all_done so skipped + # aspects (when=false) do not block it. + # --------------------------------------------------------------------------------------------- + - id: synthesize + depends_on: [code-quality, tests, silent-failure, type-design, comment-rot, simplifier, intent-check] + trigger_rule: all_done + context: fresh + allowed_tools: [Read, Bash, Glob] + prompt: | + You are the synthesize node of /pr-review for slug "$bootstrap.output.slug". + SESSION = //pr-review/ (artifacts_dir=$bootstrap.output.artifacts_dir). + + 1. Read every file in SESSION/findings/*.json that exists (a skipped aspect wrote none). + 2. Merge all `findings`, DEDUPE overlaps (same file + near-identical title across aspects → keep the + highest-confidence one, note the other aspects). Ensure each finding has its stable `hash`. + 3. Bucket by severity (critical / important / minor). Collect `positives` across aspects. + 4. Collect the intent-check `ac_verdicts` and any contradictions. + 5. Assemble the body SECTIONS (no iteration header / delta yet — reconcile adds those): + - `> ⚠ Intent sources disagree: …` callout (only if contradictions) + - `### Intent Check` with `- AC-N: ` lines (omit only if intent unavailable) + - `### 🔴 Critical (N)` / `### 🟠 Important (N)` / `### 🟡 Minor / Suggestions` — each finding a + bold `` `file:line` `` + title, body beneath; omit an empty severity section. + - `### ✅ What's good` — always present, from `positives`. + Write the deduped this-run set to SESSION/findings.json (each finding incl. its hash) and the body + sections to SESSION/summary-sections.md. Emit the counts. + output_format: + type: object + properties: + critical: { type: number } + important: { type: number } + minor: { type: number } + contradictions: { type: array, items: { type: string } } + required: [critical, important, minor] + + # --------------------------------------------------------------------------------------------- + # reconcile — THE RE-REVIEW COORDINATOR. Classifies this run's findings against the prior iteration and + # finalises the comment + inline plan. Keyed on the finding hash (marker), tolerating small line drift. + # --------------------------------------------------------------------------------------------- + - id: reconcile + depends_on: [synthesize] + context: fresh + allowed_tools: [Read, Bash, Glob] + prompt: | + You are the reconcile node of /pr-review for slug "$bootstrap.output.slug" — the re-review + coordinator. SESSION = //pr-review/. ITERATION = $prep.output.iteration. + + 1. Read SESSION/findings.json (this run) and SESSION/prep.json → `prior_findings` (prior iteration). + 2. Classify each finding by matching its `hash` against prior_findings (tolerate small line drift — + match on aspect + file + semantic title, NOT the exact line number): + - new — no prior match + - still-present — matched a prior finding still valid this run + - fixed — a prior finding with NO match this run (it no longer reproduces) + - regressed — a prior finding that was reported fixed, reappearing this run + 3. Compute the "since iteration N−1" delta: X fixed · Y still present · Z regressed · W new + (skip the delta line entirely when ITERATION == 1). + 4. FINALISE the summary body → SESSION/comment.md, in this exact frame: + + + ## unic-dlc /pr-review — $prep.output.pr_title · Iteration + + + ### Since iteration (only when ITERATION > 1) + + + --- + 🤖 Generated by unic-archon-dlc · Iteration · re-run `/unic-dlc-pr-review $bootstrap.output.slug` to update. + + Tag each still-present / regressed finding inline in its section (e.g. "(still present since + iteration N−1)" / "(⚠ regressed)"). + 5. Build the inline-comment plan → SESSION/inline.json: an array of + { file, line, severity, hash, classification, body } — one per finding. `body` starts with the + hidden marker `` then the rendered finding. Include a + `fixed` entry for each prior finding now fixed (classification "fixed", body = a + "✅ resolved in iteration " reply) so `post` can reconcile the prior thread. + Emit iteration + delta counts. + output_format: + type: object + properties: + iteration: { type: number } + fixed: { type: number } + still_present: { type: number } + regressed: { type: number } + new: { type: number } + required: [iteration, fixed, still_present, regressed, new] + + # --------------------------------------------------------------------------------------------- + # review-gate — HITL by default; skipped when gates.pr-review == afk. Shows the finalised summary + + # delta + any contradiction warnings before anything is written. Posting is advisory/non-blocking. + # --------------------------------------------------------------------------------------------- + - id: review-gate + depends_on: [reconcile] + when: "$bootstrap.output.gate == 'hitl'" + approval: + message: > + /pr-review for "$bootstrap.output.slug" — iteration $reconcile.output.iteration. Findings: + $synthesize.output.critical critical · $synthesize.output.important important · + $synthesize.output.minor minor. Since last: $reconcile.output.fixed fixed · + $reconcile.output.still_present still present · $reconcile.output.regressed regressed · + $reconcile.output.new new. Review SESSION/comment.md (the composed summary) and SESSION/inline.json + (the inline plan). APPROVE to post/update the summary + inline comments on the PR. REJECT to halt + without posting. + + # --------------------------------------------------------------------------------------------- + # post — write the review to the PR. trigger_rule: all_done so it runs when review-gate is SKIPPED in + # AFK (skipped counts as terminal); a rejected gate cancels the run before this node. Composes the + # configured tracker; matches by hidden marker so a re-run UPDATES in place rather than duplicating. + # --------------------------------------------------------------------------------------------- + - id: post + depends_on: [review-gate] + trigger_rule: all_done + context: fresh + allowed_tools: [Read, Bash] + prompt: | + You are the post node of /pr-review for slug "$bootstrap.output.slug". + SESSION = //pr-review/. ITERATION = $reconcile.output.iteration. + PR_ID = $prep.output.pr_id. inline_comments = $bootstrap.output.inline_comments. + + Compose the configured tracker from `.archon/unic-dlc.config.yaml` (`tracker.type`; MCP-first, + CLI-fallback — never hardcode a host, ADR-0016). + + ## Summary comment (always) + Read SESSION/comment.md. Search the PR's comments for the FIRST comment containing the marker + `` marker and reconcile + by `classification`: + - still-present → update the existing thread comment in place. + - fixed → reply "✅ resolved in iteration " and resolve the thread where the + tracker supports a resolved status (best-effort; otherwise just reply). + - regressed → reopen the thread (or open a new one) noting the regression. + - new → create a new inline thread at file:line. + (github: `gh api` review comments; ado: `az repos pr thread create/update` with threadContext.) + + Finally print: "pr-review: iteration posted for \"$bootstrap.output.slug\" — + , inline ( new · updated · resolved). ✓" diff --git a/apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-review.yaml b/apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-review.yaml deleted file mode 100644 index b085baa3..00000000 --- a/apps/claude-code/unic-archon-dlc/.archon/workflows/unic-dlc-review.yaml +++ /dev/null @@ -1,239 +0,0 @@ -name: unic-dlc-review -description: > - Self-contained code review across four aspects — code quality, test coverage adequacy, - silent failure patterns, and type design quality. Posts (or updates) a single structured - comment on the current PR via the configured tracker adapter. - No runtime dependency on pr-review-toolkit or any other plugin. - -nodes: - - id: code-review - name: Code Review — Four Aspects - type: prompt - depends_on: [] - prompt: | - You are running the unic-archon-dlc review workflow — code-review node. - - ## Setup - - 1. Read `.archon/unic-dlc.config.json` to determine: - - `tracker` (github | ado | jira | local-markdown; default: github) - - `branching` strategy if present - - 2. Read project conventions from whichever file exists (in order): - - `CLAUDE.md` - - `AGENTS.md` - - `.claude/CLAUDE.md` - Note all style rules, naming conventions, tab/space preferences, and - explicit "do not" rules. - - 3. Identify the current open PR: - - github: `gh pr view --json number,title,body,headRefName` - - ado: `az repos pr list --status active --output json | head -1` - - local-markdown: use the most recent `docs/workflow/*/report.md` as the review target - Store `PR_ID` (issue/PR number or identifier) and `PR_TITLE`. - - 4. Compute the diff to review: - - github / ado / local-markdown: - `git diff origin/$(git rev-parse --abbrev-ref --symbolic-full-name @{u} 2>/dev/null | sed 's|.*/||' || echo main)...HEAD` - - If the above fails, fall back to: `git diff HEAD~1...HEAD` - Store the result as `DIFF_TEXT`. - - 5. List all source files touched in the diff: - `git diff --name-only origin/$(git symbolic-ref --short HEAD 2>/dev/null)...HEAD 2>/dev/null || git diff --name-only HEAD~1` - Store as `CHANGED_FILES` (newline-separated list). - - 6. Read each file in `CHANGED_FILES` in full (skip deleted files). - - --- - - ## Aspect 1 — Code Quality - - Analyse the diff and changed file contents for: - - **Readability**: Is each function easy to understand in isolation? - - **Naming**: Do variable, function, and module names communicate intent without - needing comments? - - **Function length**: Are any functions longer than ~40 lines? Could they be split? - - **Project-convention adherence**: Check against the rules you read from CLAUDE.md / - AGENTS.md. Examples include tab vs. space indentation, quote style, semicolons, - trailing commas, explicit returns, early returns over nested conditionals. - - For each problem found, record a finding in this exact format: - - `file:line — [code-quality] ` - - If no problems are found, record: - - `No findings.` - - Store all findings under the heading `### Code Quality`. - - --- - - ## Aspect 2 — Test Coverage Adequacy - - Locate test files in `CHANGED_FILES` or in `test/`, `tests/`, `__tests__/`, - `*.test.*`, `*.spec.*` directories. For each changed source module, check whether: - - - Tests exercise the module through its **public interface** (exported functions), - not internal helpers. - - Assertions verify **outputs and side-effects**, not internal state or call counts - on internal collaborators. - - No internal collaborators are mocked — only true external boundaries (network, - disk, third-party CLIs) should be mocked. - - Every exported function has at least one happy-path test and at least one - edge-case or error-path test. - - For each gap found, record a finding: - - `file:line — [test-coverage] ` - - If no gaps are found, record: - - `No findings.` - - Store all findings under the heading `### Test Coverage Adequacy`. - - --- - - ## Aspect 3 — Silent Failure Patterns - - Scan the diff for patterns that swallow errors and hide bugs: - - - **Empty catch blocks**: `catch { }` or `catch (e) { /* ignored */ }` - - **Swallowed exceptions**: `catch` that logs but does not rethrow, return a default, - or propagate the error to the caller. - - **Silent fallbacks**: `?? defaultValue` or `|| fallback` on values that should - never be absent — where the fallback masks a missing configuration or corrupt state. - - **Unhandled promise rejections**: `.then(…)` without `.catch(…)` or `await` outside - try/catch in async functions. - - **Ignored return values**: calling a function whose return value signals an error - or state change, then discarding it. - - For each pattern found, record a finding: - - `file:line — [silent-failure] ` - - If no patterns are found, record: - - `No findings.` - - Store all findings under the heading `### Silent Failure Patterns`. - - --- - - ## Aspect 4 — Type Design Quality - - Analyse type annotations, JSDoc types, or TypeScript types introduced or changed - in the diff: - - - **Encapsulation**: Are types narrow enough to prevent invalid usage? Or are they - overly broad (e.g. `string` where a union literal would be more precise)? - - **Invariants in types**: Do types encode business rules? E.g. a `NonEmptyArray` - instead of `T[]` where empty is invalid. - - **Illegal-state-unrepresentable**: Are there mutually exclusive fields that could - be unified into a discriminated union? Are there optional fields that should be - required, or vice-versa? - - **JSDoc completeness** (for `.mjs` / `// @ts-check` files): Are all exported - function parameters and return types annotated? - - For each weakness found, record a finding: - - `file:line — [type-design] ` - - If no weaknesses are found, record: - - `No findings.` - - Store all findings under the heading `### Type Design Quality`. - - --- - - ## Compose the review comment - - Assemble a Markdown comment with this exact structure: - - ``` - - ## unic-dlc-review - - > PR: - - ### Code Quality - - - ### Test Coverage Adequacy - - - ### Silent Failure Patterns - - - ### Type Design Quality - - - --- - *Generated by unic-archon-dlc · re-run `/unic-dlc-review` to update this comment.* - ``` - - The `` HTML comment at the very first line is the sentinel - used for update detection. Do not remove or alter it. - - Store the assembled Markdown as `REVIEW_BODY`. - - --- - - ## Post or update the comment - - Search for an existing review comment on this PR: - - **github:** - ``` - gh pr view "$PR_ID" --json comments --jq '.comments[] | select(.body | startswith("")) | .databaseId' - ``` - If a comment ID is found (call it `EXISTING_ID`): - Update: `gh api repos/{owner}/{repo}/issues/comments/$EXISTING_ID -X PATCH -f body="$REVIEW_BODY"` - Print: "review: updated existing comment $EXISTING_ID on PR #$PR_ID" - If no comment exists: - Create: `gh pr comment "$PR_ID" --body "$REVIEW_BODY"` - Print: "review: posted new comment on PR #$PR_ID" - - **ado:** - Fetch threads: `az repos pr thread list --id "$PR_ID" --output json` - Search for a thread whose first comment starts with ``. - If found (`THREAD_ID`, `COMMENT_ID`): - Update: `az repos pr thread comment update --id "$PR_ID" --thread-id "$THREAD_ID" --comment-id "$COMMENT_ID" --content "$REVIEW_BODY"` - Print: "review: updated existing thread $THREAD_ID on PR $PR_ID" - If not found: - Create: `az repos pr thread create --id "$PR_ID" --comment "$REVIEW_BODY"` - Print: "review: posted new thread on PR $PR_ID" - - **local-markdown:** - Look for `docs/workflow//review-comment.md` (where slug is the most recent - workflow directory under docs/workflow/). - If it exists: - Overwrite the file with `REVIEW_BODY`. - Print: "review: updated docs/workflow//review-comment.md" - If it does not exist: - Write `REVIEW_BODY` to `docs/workflow//review-comment.md`. - Print: "review: created docs/workflow//review-comment.md" - - **jira:** - Search for a comment whose body starts with ``: - `jira issue comment list "$PR_ID" --output json` - If found (`COMMENT_ID`): - `jira issue comment update "$PR_ID" "$COMMENT_ID" --body "$REVIEW_BODY"` - Print: "review: updated existing comment $COMMENT_ID on issue $PR_ID" - If not found: - `jira issue comment add "$PR_ID" --body "$REVIEW_BODY"` - Print: "review: posted new comment on issue $PR_ID" - - --- - - ## Summary - - After posting or updating, print a human-readable summary: - - ``` - unic-dlc-review complete. - - Aspects reviewed: Code Quality · Test Coverage Adequacy · Silent Failure Patterns · Type Design Quality - PR: () - Action: review comment - - Findings: - Code Quality: - Test Coverage Adequacy: - Silent Failure Patterns: - Type Design Quality: - ``` diff --git a/apps/claude-code/unic-archon-dlc/.claude-plugin/marketplace.json b/apps/claude-code/unic-archon-dlc/.claude-plugin/marketplace.json index 380bb7d7..2134fa87 100644 --- a/apps/claude-code/unic-archon-dlc/.claude-plugin/marketplace.json +++ b/apps/claude-code/unic-archon-dlc/.claude-plugin/marketplace.json @@ -13,7 +13,7 @@ "name": "Unic AG" }, "category": "productivity", - "description": "A complete Archon-powered AI development lifecycle — explore, plan, build, qa, cleanup, and triage workflows as installable YAML DAGs with human approval gates.", + "description": "A complete Archon-powered AI development lifecycle — explore, specs, tickets, build, pr-review, qa, triage, and cleanup workflows as installable YAML DAGs with human approval gates.", "displayName": "Unic Archon DLC", "homepage": "https://github.com/unic/unic-agents-plugins", "keywords": ["archon", "workflow", "lifecycle", "tdd", "unic"], @@ -21,7 +21,7 @@ "name": "unic-archon-dlc", "source": "./", "tags": ["productivity", "workflow", "ai-development"], - "version": "0.7.0" + "version": "0.8.0" } ] } diff --git a/apps/claude-code/unic-archon-dlc/.claude-plugin/plugin.json b/apps/claude-code/unic-archon-dlc/.claude-plugin/plugin.json index 4bbf7bb9..200f48d6 100644 --- a/apps/claude-code/unic-archon-dlc/.claude-plugin/plugin.json +++ b/apps/claude-code/unic-archon-dlc/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "unic-archon-dlc", - "version": "0.7.0", - "description": "A complete Archon-powered AI development lifecycle as an installable DLC pack — explore, plan, build, qa, cleanup, and triage workflows.", + "version": "0.8.0", + "description": "A complete Archon-powered AI development lifecycle as an installable DLC pack — explore, specs, tickets, build, pr-review, qa, triage, and cleanup workflows.", "author": { "name": "Unic AG", "url": "https://www.unic.com" diff --git a/apps/claude-code/unic-archon-dlc/AGENTS.md b/apps/claude-code/unic-archon-dlc/AGENTS.md index 804266a5..965e4c7d 100644 --- a/apps/claude-code/unic-archon-dlc/AGENTS.md +++ b/apps/claude-code/unic-archon-dlc/AGENTS.md @@ -63,6 +63,7 @@ Load-bearing invariants. These either originate in a Plugin ADR or are policy de - **Slopcheck before build.** Every new package referenced in `package.json` is verified against the npm registry before any RED/GREEN/REFACTOR phase runs. Packages that fail are flagged `[ASSUMED]` and require explicit human approval. - **Nyquist map gates the build.** Every issue in Issues JSON must carry a `test_command` (or `test_command_planned`) before `/build` consumes it — the gate runs in `/tickets` ([ADR-0022](docs/adr/0022-tickets-slice-to-build.md)); `/build` reads the build-ready `issues.json` directly ([ADR-0023](docs/adr/0023-build-generic-red-green-refactor-loop.md)). - **`/qa` gates on `gates.qa` and fail-closes the merge.** The QA Archon pipeline (e2e → coverage → UAT → verify-pr-base → merge) has two `approval:` gates (UAT + merge), both HITL by default and skipped in AFK; downstream nodes use `trigger_rule: all_done` so AFK auto-merges a clean build, while the merge node's fail-closed `when` blocks auto-merging a red e2e/coverage or a wrong PR base. `/qa` is also an **issue-producing on-ramp**: a UAT rejection files each defect directly as a `ready-for-agent` tracker issue (composing the configured tracker + `classification.labels`, Matt's `qa` brief shape, AI disclaimer) that feeds `/tickets` — it does not just halt. Never `lib/tracker-adapter.mjs` (dissolved). See [ADR-0025](docs/adr/0025-qa-pipeline-onramp.md). +- **`/pr-review` is a self-contained fan-out Archon review that harvests, never depends.** The PR-review Archon pipeline (bootstrap → guard → prep → seven parallel aspect nodes → synthesize → reconcile → review-gate → post) reviews the open PR and posts a summary comment + inline comments via the configured tracker (MCP-first, CLI-fallback). It **harvests `unic-pr-review`'s review learnings** (confidence→severity rubric, structured summary, hidden-marker idempotency, conditional spawn table, two-surface posting) with **no ADO code and no runtime dependency** on that plugin. `prep` composes **one Intent Brief** (linked work items + Confluence/MD docs + PR description + `PRD.md`) that is **injected into every aspect** (the quality lever) and surfaces cross-source contradictions; intent-check is skipped only when no source resolves. Re-review is first-class — a dedicated `reconcile` node keyed on the `` marker (never author identity) classifies findings new / still-present / fixed / regressed. The `review-gate` confirms before posting `when gates.pr-review == hitl` (AFK posts directly); posting is advisory — the real merge gate is `/qa`. Never `lib/tracker-adapter.mjs` (dissolved). See [ADR-0026](docs/adr/0026-pr-review-generic-archon-harvest.md). - **Dogfooding note.** This monorepo has had Setup run against it; the generated artefacts live under [`docs/agents/`](../../../docs/agents/) at the repo root and are managed by the marker-delimited block in the root [`AGENTS.md`](../../../AGENTS.md). Those files describe the target two-axis architecture (see [ADR-0016](docs/adr/0016-dlc-thin-process-layer.md)–[ADR-0018](docs/adr/0018-generic-core-config-compose.md)) with the tracker as the single source of truth, and should be treated as the canonical agent guidance for this repo, not as a current-practice snapshot. ## External dependencies diff --git a/apps/claude-code/unic-archon-dlc/CHANGELOG.md b/apps/claude-code/unic-archon-dlc/CHANGELOG.md index eab3aa71..435d1379 100644 --- a/apps/claude-code/unic-archon-dlc/CHANGELOG.md +++ b/apps/claude-code/unic-archon-dlc/CHANGELOG.md @@ -11,6 +11,22 @@ ### Fixed - (none) +## [0.8.0] — 2026-07-03 + +### Breaking +- (none) + +### Added +- **`/pr-review` generic fan-out Archon workflow** (ADR-0026) — reviews the open PR by composing **one Intent Brief** (linked work items + Confluence/MD docs + PR description + `PRD.md`) in `prep` and **injecting it into every aspect**, then fanning out **seven parallel fresh aspect nodes** (code-quality, test-coverage, silent-failure, type-design, comment-rot, code-simplification, intent/AC-coverage) — conditionally spawned by the changed-file categories and scored on a confidence→severity rubric. Findings are synthesised, **reconciled against the prior iteration** (new / still-present / fixed / regressed) in a dedicated `reconcile` node keyed on a hidden `` marker (never author identity), then — after a `gates.pr-review` confirm (AFK posts directly) — posted as a **summary comment + inline comments** via the configured tracker (MCP-first, CLI-fallback). It **harvests `unic-pr-review`'s review learnings with no ADO code and no runtime dependency** on that plugin; posting is advisory (the real merge gate is `/qa`). **Not** via `lib/tracker-adapter.mjs` (dissolved). +- **`pr-review` config block** in `defaultConfig()` — `{ confidence_threshold: 60, inline_comments: true }`. `mergeConfig` auto-fills it for existing configs (no `/setup` change). The `gates.pr-review` key already existed. +- **New ADR-0026** recording the self-contained harvest-not-depend decision, the fan-out schema, intent-composed-once-injected-everywhere, the confidence rubric + spawn gates, first-class re-review, and the confirm-before-post gate. + +### Fixed +- (none) + +### Changed +- **Renamed + ported the `unic-dlc-review` workflow to `unic-dlc-pr-review`** on the key-discriminated node schema (ADR-0011) — the shipped single monolithic `type: prompt` node ran inert. `git mv`'d the workflow YAML + command stub, moved config reads to `.archon/unic-dlc.config.yaml`, artefact paths to `//pr-review/` (ADR-0015), replaced the four-aspect single-comment node with the seven-aspect fan-out DAG, and dropped the stale `lib/tracker-adapter.mjs` + `apps/claude-code/pr-review/` references. Updated the plugin/marketplace descriptions to enumerate the current boxes (added `pr-review`, dropped the retired `plan`). + ## [0.7.0] — 2026-07-02 ### Breaking diff --git a/apps/claude-code/unic-archon-dlc/CONTEXT.md b/apps/claude-code/unic-archon-dlc/CONTEXT.md index 0abb4f20..2dd53fb6 100644 --- a/apps/claude-code/unic-archon-dlc/CONTEXT.md +++ b/apps/claude-code/unic-archon-dlc/CONTEXT.md @@ -154,6 +154,30 @@ Architecture Decision Record. Written to `docs/adr/NNNN-*.md` only after explici approval in the `/improve-architecture` command/skill (which also supersedes stale ADRs). _Avoid_: decision doc, architecture note +### PR-review artifacts + +**Review comment**: +The `/pr-review` output — one structured **summary comment** on the open PR (severity-grouped findings + +an Intent Check + "What's good"), plus **inline comments** per finding where the tracker supports threads. +It is keyed by a hidden `` marker so a re-run **updates in place** +(and increments the iteration) rather than duplicating. Distinct from **Findings** (the `/explore` +research doc at `workflows//findings.md`) and from **arch-review** (the `/improve-architecture` +architecture-drift report) — this is diff-level PR feedback. +_Avoid_: review report, findings.md (the /explore artifact), code review (the arch-review sense) + +**Intent Brief**: +The single narrative + numbered Acceptance Criteria that `/pr-review`'s `prep` node composes once from the +linked work items, Confluence/MD docs, the PR description, and `PRD.md`, then injects into every review +aspect so each judges the diff against intended behaviour. Contradictions across sources are surfaced, not +silently resolved. +_Avoid_: spec, PRD (which is one input source, not the brief) + +**Review aspect**: +One of the seven parallel fresh nodes `/pr-review` fans out — code-quality, test-coverage, silent-failure, +type-design, comment-rot, code-simplification, and intent/AC-coverage — each conditionally spawned by the +changed-file categories and scoring findings on the confidence→severity rubric. +_Avoid_: reviewer, agent, check + ## Relationships - A **Session** is scoped by a **Slug** and produces **Findings**, a **PRD**, **Issues JSON**, and a `build-.yaml`, all under `workflows//` diff --git a/apps/claude-code/unic-archon-dlc/docs/adr/0026-pr-review-generic-archon-harvest.md b/apps/claude-code/unic-archon-dlc/docs/adr/0026-pr-review-generic-archon-harvest.md new file mode 100644 index 00000000..dcf95aaf --- /dev/null +++ b/apps/claude-code/unic-archon-dlc/docs/adr/0026-pr-review-generic-archon-harvest.md @@ -0,0 +1,126 @@ +# 0026. `/pr-review` is a generic fan-out Archon workflow that harvests unic-pr-review's learnings, not its code + +**Status:** Accepted (2026-07-03) + +## Context + +`/pr-review` is the main-line box between `/build` and `/qa` ([ADR-0014](0014-workflow-per-box-decomposition.md)): +it reviews the open PR and posts feedback. It is an **Archon workflow** because it is AFK-isolatable work +([ADR-0017](0017-container-follows-structural-need.md)) — but unlike `/qa` it merges nothing, so its human +checkpoint is a **confirm-before-post** gate, not a merge gate. + +The shipped `unic-dlc-review` workflow used the inert `type:`-style schema — a blocking migration +([ADR-0011](0011-archon-schema-target.md)): a single monolithic `type: prompt` node that posted one +sentinel-guarded comment, read a flat `.archon/unic-dlc.config.json`, and used stale `docs/workflow//` +paths. Its command stub also referenced the **dissolved** `lib/tracker-adapter.mjs` +([ADR-0016](0016-dlc-thin-process-layer.md)/[ADR-0018](0018-generic-core-config-compose.md)) — the same +stale reference [ADR-0025](0025-qa-pipeline-onramp.md) had to correct for `/qa`. + +Three questions had to be resolved: + +1. **Relationship to the separate `unic-pr-review` plugin.** That plugin is a rich but cautionary PR + reviewer welded to Azure DevOps (an ~880-line procedural orchestrator, 16 interdependent ADRs of + iteration-state machinery). It holds real, hard-won review _learnings_ — but they are buried in + ADO-specific IO. Does `/pr-review` delegate to it, share a module with it, or stay self-contained? +2. **How rich a review**, and in what node shape — one prompt or a fan-out? +3. **How much re-review awareness** — none, a counter, or full finding classification? + +These were grilled with the maintainer (2026-07-03); the decisions below are that outcome. + +## Decision + +### 1. Self-contained — harvest the learnings, depend on nothing + +`/pr-review` **stays deliberately self-contained** (PLAN #7). It re-derives, in generic config-composed +form, the _portable learnings_ of `unic-pr-review` — the six review aspects and what each hunts for, the +confidence→severity rubric, the structured grouped summary, the hidden-marker (never author-identity) +idempotency, the conditional spawn table, and the summary+inline two-surface model — but takes **none of +its ADO code and no runtime dependency on it** ([ADR-0016](0016-dlc-thin-process-layer.md)/ +[ADR-0017](0017-container-follows-structural-need.md)). `unic-pr-review`'s fate stays deferred; the two +are not coupled. What makes this _not_ a second cautionary tale is that the review lives in a declarative +Archon DAG over a config-composed tracker (MCP-first, CLI-fallback), not a procedural orchestrator with a +host welded in. + +### 2. Node graph — a fan-out ported to the key-discriminated schema + +`bootstrap → guard-not-ready → prep → {code-quality, tests, silent-failure, type-design, comment-rot, +simplifier, intent-check} → synthesize → reconcile → review-gate → post`, with `interactive: true` at the +workflow level so the gate message reaches the user ([ADR-0011](0011-archon-schema-target.md) §2). +Following [ADR-0023](0023-build-generic-red-green-refactor-loop.md) §5, every node is a self-contained +`prompt:` node reading files with its own tools — **no plugin-`lib/` import, no `$CLAUDE_PLUGIN_ROOT`** — +and artefacts live under `//pr-review/` ([ADR-0015](0015-workflows-slug-artifact-home.md)). + +The **seven aspects fan out as parallel fresh nodes** rather than one prompt: each judges the diff through +a focused lens in its own context. Fresh nodes share no memory, so `prep` writes the diff, changed-file +list, and Intent Brief to disk and each aspect **writes its findings to `findings/.json`**; +`synthesize` reads whatever files exist. A **skipped aspect writes nothing**, so `synthesize` never +references a skipped node's output — and it uses `trigger_rule: all_done` so the skips don't block it. + +### 3. Intent composed once in prep, injected into every aspect + +`prep` composes **one Intent Brief** from every source that resolves — the linked work items (User Story / +Bug / Jira ticket, via the configured tracker), Confluence/MD docs (via the configured docs system-skill), +the **PR description body**, and `PRD.md` if present — and every aspect reads it. This is the primary +quality lever: an intent-grounded aspect raises **precision** (a spec-mandated fallback or wide type is not +flagged as a defect) and **recall** (a missing AC path or a dropped spec-required telemetry event _is_ +flagged), and composing once keeps intent consistent across the seven aspects instead of re-derived seven +ways. Intent-check (the dedicated AC-coverage aspect) is skipped **only when no source resolves** — a +missing `PRD.md` alone is not a reason to skip. When sources **contradict**, `prep` records each and the +contradiction surfaces both at the gate and in the summary — the review never silently picks a winner. + +### 4. Conditional spawn gates, confidence rubric + +Aspects are spawned only when meaningful (the harvested SPAWN_TABLE, biased to over-spawn — a false spawn +is a cheap empty result, a false skip silently drops a finding set): code-quality + intent-check always +run; `tests` when the diff touches tests; `type-design` when it touches types; `comment-rot` when it +touches docs/comments; `silent-failure` when it touches source or error-handling; `simplifier` when it +touches source. Every aspect scores findings on the same rubric — confidence 90–100 Critical, 80–89 +Important, 60–79 Minor, below `pr-review.confidence_threshold` **dropped before writing**. + +### 5. First-class re-review — iteration counter + finding classification + +Re-review is a dedicated **`reconcile`** node, not folded into synthesize. Every bot comment carries a +hidden `` marker + an `Iteration N` footer; `prep` detects the +**highest** prior N on the PR (the PR itself is the source of truth, not the session dir — detection keys +on the marker, **never author identity**, so a human comment is never mistaken for a prior review) and +sets this run to N+1. `reconcile` matches this run's findings against the prior iteration by their stable +per-finding marker (tolerating small line drift — matched on aspect + file + semantic title, not the exact +line) and **classifies each new / still-present / fixed / regressed**, computes the "since iteration N−1" +delta, and finalises the summary + inline plan. `post` reconciles per classification: still-present → +update in place, fixed → resolve, regressed → reopen, new → new thread. + +### 6. Confirm-before-post gate; two write surfaces + +`review-gate` is an `approval:` node gated `when gates.pr-review == 'hitl'` (default HITL). In HITL it +shows the finalised summary + counts + contradiction warnings and pauses; in AFK it is skipped and `post` +runs via `trigger_rule: all_done`. Posting is **advisory and non-blocking** — unlike `/qa` there is no +fail-closed merge guard, because `/pr-review` merges nothing. `post` writes **two surfaces**: one +structured **summary comment** (matched/updated by its iteration marker) and, when +`pr-review.inline_comments` is set and the tracker supports inline threads, **inline comments** per +finding. Trackers without inline threads (jira / local-markdown) degrade to summary-only with a note. + +### 7. Config: a `pr-review` block that back-fills + +A `'pr-review': { confidence_threshold: 60, inline_comments: true }` block joins `defaultConfig()`. +`mergeConfig` auto-fills it for configs that predate it, so **no `/setup` change is required** this step +(same pattern as [ADR-0024](0024-triage-intake-on-ramp.md)/[ADR-0025](0025-qa-pipeline-onramp.md) §2). The +`gates.pr-review` key already existed in `defaultConfig()`. + +## Consequences + +- **`/pr-review` gains a real, AFK-capable review** it lacked (the old single node ran the inert schema). + Behavioural validation — the gate pauses in HITL, AFK skips-and-posts, the summary + inline update in + place, a second run increments the iteration and classifies findings — is required beyond + `archon validate` ([ADR-0011](0011-archon-schema-target.md) §6), which passes inert forms too. +- **New `pr-review` config block** in `config-schema.mjs` + tests; `mergeConfig` back-fills existing + configs. +- **The rename** `unic-dlc-review` → `unic-dlc-pr-review` propagates to the workflow `name:`, the command + stub, the sentinel (``), and the plugin/marketplace descriptions + (which also drop the retired `plan`). +- **Self-containment keeps the two PR reviewers decoupled:** improvements to one do not obligate the other, + and the DLC never inherits ADO-specific coupling. +- **Known item, not fixed here:** the re-review matching is deliberately generic — there is **no + delta-diff engine** for exact line-drift tracking (we match on aspect+file+semantic title), and + ADO-style inline thread _status_ transitions (resolve/reopen) are best-effort where the tracker exposes + them. Full end-to-end behavioural validation (including a second-iteration run) needs a real Archon run + against a Consumer PR; it is logged as a manual follow-up, not asserted by CI. diff --git a/apps/claude-code/unic-archon-dlc/docs/adr/README.md b/apps/claude-code/unic-archon-dlc/docs/adr/README.md index 008f457b..bf81cf9d 100644 --- a/apps/claude-code/unic-archon-dlc/docs/adr/README.md +++ b/apps/claude-code/unic-archon-dlc/docs/adr/README.md @@ -31,3 +31,4 @@ Plugin-scoped architectural decisions. Repo-wide decisions live in `docs/adr/` a | 0023 | `/build` is one generic red/green/refactor loop; dag-builder dissolved | Accepted | | 0024 | `/triage` is the intake on-ramp; thin wrapper binds Matt's method to DLC config | Accepted | | 0025 | `/qa` is an Archon pipeline with two config-gated approvals + an issue-producing on-ramp | Accepted | +| 0026 | `/pr-review` is a generic fan-out Archon workflow harvesting unic-pr-review's learnings | Accepted | diff --git a/apps/claude-code/unic-archon-dlc/docs/redesign/README.md b/apps/claude-code/unic-archon-dlc/docs/redesign/README.md index cdba9cf4..a3e48024 100644 --- a/apps/claude-code/unic-archon-dlc/docs/redesign/README.md +++ b/apps/claude-code/unic-archon-dlc/docs/redesign/README.md @@ -10,22 +10,22 @@ This directory drives the refactor of `unic-archon-dlc` into a **thin, Matt-alig > > Legend: ✅ done · ⏭️ skip (not a build) · ▶️ next · ⬜ todo -| # | Step | Container | Status | PR / notes | -| --- | ----------------------- | ----------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 00 | Archon schema pre-work | — | ✅ done | ADR-0011 (#262) | -| 01 | Foundations | — | ✅ done | ADRs 0012–0021 + AGENTS/CONTEXT sweep (#262) | -| 02 | `/handoff` | Matt's, ref | ⏭️ skip | referenced verbatim, not shipped; dependency declared in `/setup` | -| 03 | `/setup` | skill | ✅ done | conversational skill + `config-schema` lib; rich YAML config (#263) | -| 04 | `/specs` | skill | ✅ done | branch-on-input command; template → `templates.prd`; `specs.{discuss_mode,gate}` (#264) | -| 05 | `/tickets` | skill | ✅ done | command; stops at build-ready `issues.json` (no DAG gen); `tickets` config; ADR-0022 (#265) | -| 06 | `/build` | Archon | ✅ done | keystone — ported to key-discriminated schema as one generic red/green/refactor loop over `issues.json`; dag-builder dissolved; ADR-0023 (PR #266) | -| 07 | `/triage` | skill | ✅ done | thin wrapper over Matt's `triage` method; single-source label binding (DLC config); retires old triage workflow; ADR-0024 (PR #267) | -| 08 | `/qa` | Archon | ✅ done | ported to key-discriminated schema; e2e → coverage → UAT + merge gates (`gates.qa`, `all_done` AFK auto-merge, fail-closed merge); UAT-reject files agent-ready issues; `qa` config block; ADR-0025 (PR #268) | -| 09 | `/pr-review` | Archon | ▶️ next | new — harvest unic-pr-review learnings | -| 10 | `/improve-architecture` | skill | ⬜ todo | + ADR superseding | -| 11 | `/cleanup` | command | ⬜ todo | new — repo-global janitor | -| 12 | `/explore` | Archon | ⬜ todo | `/prototype` NOT built (Matt's referenced skill) | -| 13 | Finalize | — | ⬜ todo | reduced — `Unic-dlc.mmd` diagram + dogfood dry-run only (sweep done) | +| # | Step | Container | Status | PR / notes | +| --- | ----------------------- | ----------- | ------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 00 | Archon schema pre-work | — | ✅ done | ADR-0011 (#262) | +| 01 | Foundations | — | ✅ done | ADRs 0012–0021 + AGENTS/CONTEXT sweep (#262) | +| 02 | `/handoff` | Matt's, ref | ⏭️ skip | referenced verbatim, not shipped; dependency declared in `/setup` | +| 03 | `/setup` | skill | ✅ done | conversational skill + `config-schema` lib; rich YAML config (#263) | +| 04 | `/specs` | skill | ✅ done | branch-on-input command; template → `templates.prd`; `specs.{discuss_mode,gate}` (#264) | +| 05 | `/tickets` | skill | ✅ done | command; stops at build-ready `issues.json` (no DAG gen); `tickets` config; ADR-0022 (#265) | +| 06 | `/build` | Archon | ✅ done | keystone — ported to key-discriminated schema as one generic red/green/refactor loop over `issues.json`; dag-builder dissolved; ADR-0023 (PR #266) | +| 07 | `/triage` | skill | ✅ done | thin wrapper over Matt's `triage` method; single-source label binding (DLC config); retires old triage workflow; ADR-0024 (PR #267) | +| 08 | `/qa` | Archon | ✅ done | ported to key-discriminated schema; e2e → coverage → UAT + merge gates (`gates.qa`, `all_done` AFK auto-merge, fail-closed merge); UAT-reject files agent-ready issues; `qa` config block; ADR-0025 (PR #268) | +| 09 | `/pr-review` | Archon | ✅ done | new generic fan-out workflow (7 intent-grounded aspects → synthesize → reconcile → gate → post); harvests unic-pr-review learnings, no dependency; iteration-aware re-review; `pr-review` config; ADR-0026 (PR #269) | +| 10 | `/improve-architecture` | skill | ▶️ next | + ADR superseding | +| 11 | `/cleanup` | command | ⬜ todo | new — repo-global janitor | +| 12 | `/explore` | Archon | ⬜ todo | `/prototype` NOT built (Matt's referenced skill) | +| 13 | Finalize | — | ⬜ todo | reduced — `Unic-dlc.mmd` diagram + dogfood dry-run only (sweep done) | ## How to run a step diff --git a/apps/claude-code/unic-archon-dlc/lib/config-schema.mjs b/apps/claude-code/unic-archon-dlc/lib/config-schema.mjs index e32a086c..9e440687 100644 --- a/apps/claude-code/unic-archon-dlc/lib/config-schema.mjs +++ b/apps/claude-code/unic-archon-dlc/lib/config-schema.mjs @@ -84,6 +84,7 @@ export function defaultConfig() { tickets: { gate: 'open-pr' }, triage: { out_of_scope_dir: '.out-of-scope', external_prs: 'auto' }, qa: { e2e_command: null, coverage_threshold: null }, + 'pr-review': { confidence_threshold: 60, inline_comments: true }, gates: { build: 'hitl', qa: 'hitl', 'pr-review': 'hitl', explore: 'hitl' }, build: { fresh_context_red_green: true, diff --git a/apps/claude-code/unic-archon-dlc/package.json b/apps/claude-code/unic-archon-dlc/package.json index 717f8533..c10abc23 100644 --- a/apps/claude-code/unic-archon-dlc/package.json +++ b/apps/claude-code/unic-archon-dlc/package.json @@ -1,6 +1,6 @@ { "name": "unic-archon-dlc", - "version": "0.7.0", + "version": "0.8.0", "private": true, "license": "LGPL-3.0-or-later", "type": "module", diff --git a/apps/claude-code/unic-archon-dlc/test/config-schema.test.mjs b/apps/claude-code/unic-archon-dlc/test/config-schema.test.mjs index de44cfe4..fa6fa961 100644 --- a/apps/claude-code/unic-archon-dlc/test/config-schema.test.mjs +++ b/apps/claude-code/unic-archon-dlc/test/config-schema.test.mjs @@ -230,6 +230,29 @@ test('mergeConfig preserves a team override of qa, filling untouched sub-keys', assert.equal(qa.coverage_threshold, null, 'untouched sub-key filled from default') }) +test('defaultConfig ships pr-review defaults: confidence_threshold=60, inline_comments=true', () => { + const prReview = /** @type {any} */ (defaultConfig()['pr-review']) + assert.equal(prReview.confidence_threshold, 60) + assert.equal(prReview.inline_comments, true) +}) + +test('mergeConfig auto-fills the pr-review block for an existing config that predates it', () => { + // A config written before the pr-review block existed (e.g. the qa-era dogfood config). + const merged = mergeConfig( + { tracker: { type: 'github' }, project: { branching: 'gitflow', pr_strategy: 'merge' } }, + {} + ) + const prReview = /** @type {any} */ (merged['pr-review']) + assert.deepEqual(prReview, { confidence_threshold: 60, inline_comments: true }, 'pr-review block filled from default') +}) + +test('mergeConfig preserves a team override of pr-review, filling untouched sub-keys', () => { + const merged = mergeConfig({ 'pr-review': { confidence_threshold: 80 } }, {}) + const prReview = /** @type {any} */ (merged['pr-review']) + assert.equal(prReview.confidence_threshold, 80, 'existing override wins') + assert.equal(prReview.inline_comments, true, 'untouched sub-key filled from default') +}) + test('mergeConfig preserves a team override of triage, filling untouched sub-keys', () => { const merged = mergeConfig({ triage: { external_prs: 'never' } }, {}) const triage = /** @type {any} */ (merged.triage)