Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
c7eb11d
test(dashboard): cover the capture-boundary reject branch for implaus…
spencermarx Jun 13, 2026
b5bc110
refact(shared): extract persistence and config into shared source-onl…
spencermarx Jun 14, 2026
278b308
feat(verdict): canonical 3-state verdict contract enforced end to end
spencermarx Jun 14, 2026
4481077
feat(dashboard): process supervision and database integrity hardening
spencermarx Jun 14, 2026
7648c90
test(dashboard-ui-e2e): eliminate flaky parallelism and networkidle race
spencermarx Jun 14, 2026
fb1f6eb
refact(config): repoint team-models e2e to @open-code-review/config/m…
spencermarx Jun 14, 2026
8113f4c
spec: add verdict-contract, supervision, and shared-package change pr…
spencermarx Jun 14, 2026
211cd50
spec: archive verdict-contract, supervision, and shared-package changes
spencermarx Jun 14, 2026
e4148b6
spec: add directional verdict↔blocker-count consistency proposal
spencermarx Jun 15, 2026
928365e
spec: add forward-resume of a stranded mid-pipeline review proposal
spencermarx Jun 15, 2026
a2c829a
feat(state): enforce directional verdict↔blocker-count consistency
spencermarx Jun 15, 2026
a65ea7b
feat(state): forward-resume core for stranded mid-pipeline runs
spencermarx Jun 15, 2026
6fb6541
feat(cli): forward-only, lease-guarded review --resume + status forwa…
spencermarx Jun 15, 2026
5785ffd
docs(agents): forward-resume control loop + don't-strand-the-pipeline…
spencermarx Jun 15, 2026
d85333f
feat(dashboard): auto-forward-resume sweep + exhausted-state recovery UI
spencermarx Jun 15, 2026
6343c35
test(cli-e2e): review --resume no-vendor-id now hands off (exit 0), n…
spencermarx Jun 15, 2026
23a338f
spec: sync apply checklists for both forward-resume changes
spencermarx Jun 15, 2026
a74cc97
spec: archive forward-resume + verdict-count-direction changes
spencermarx Jun 15, 2026
0017779
spec: address round-1 review feedback (5 blockers + should-fixes)
spencermarx Jun 15, 2026
ddd1c77
refactor: address round-1 code-review should-fixes (PR #49, APPROVE'd)
spencermarx Jun 15, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .ocr/cli-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
"claude",
"windsurf"
],
"lastUpdated": "2026-06-11T19:58:24.757Z",
"lastUpdated": "2026-06-13T12:06:54.298Z",
"cliVersion": "2.2.1"
}
2 changes: 1 addition & 1 deletion .ocr/reviewers-meta.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"schema_version": 1,
"generated_at": "2026-06-11T19:58:24.752Z",
"generated_at": "2026-06-13T12:06:54.294Z",
"reviewers": [
{
"id": "accessibility",
Expand Down
10 changes: 10 additions & 0 deletions .ocr/skills/references/final-template.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,16 @@ The Tech Lead determines the verdict based on simple rules:

**Important**: The Tech Lead does NOT override blockers. If any reviewer flags a blocker, the verdict is REQUEST CHANGES regardless of other opinions.

**Verdict and blocker count must point the same direction (CLI-enforced).** The verdict is now cross-checked against the deduplicated blocker count at `complete-round`:

- `REQUEST CHANGES` **requires at least one blocker** — if nothing is a blocker, the change is mergeable, so the verdict is `APPROVE` (carry the residual work as `should_fix`/`suggestion`/`style`).
- `APPROVE` **requires zero blockers** — a mergeable gate cannot coexist with a must-fix. If something truly must be fixed before merge, categorize it `blocker` and use `REQUEST CHANGES`.
- `NEEDS DISCUSSION` is unconstrained on blockers.

The CLI **rejects** a contradictory pair (exit 7, nothing written), so pick the verdict and the blocker categorization together.

**The verdict is the merge gate — one axis, three values.** It answers only "can this land?" Residual work is a *separate* axis: follow-ups (`should_fix`) and suggestions are finding **categories**, never verdict states. An `APPROVE` with open should-fix items is the normal, correct outcome — the work is tracked in the counts, not by bending the verdict into a composite like "approve with suggestions". Never emit a verdict outside the three canonical values.

---

## Final Review Template
Expand Down
25 changes: 25 additions & 0 deletions .ocr/skills/references/workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Complete 8-phase process for multi-agent code review.

> **CRITICAL**: You MUST call `ocr state advance` **BEFORE starting work** on each phase. The `ocr progress` CLI reads session state for real-time tracking. Transition the `current_phase` and `phase_number` immediately when entering a new phase—do not wait until the phase is complete.

> **DON'T STRAND THE PIPELINE**: Once you start the reviewers (Phase 4), drive the workflow all the way to `ocr state complete-round` **within the same turn**. Do **not** voluntarily end the turn between phases (e.g. spawning reviewers in the background and ending the turn to "wait" for them) — a turn that ends mid-pipeline leaves the round incomplete until it is forward-resumed. This is host-neutral guidance: it does not require or forbid any specific spawning primitive; it just keeps phases 4→7 in one continuous turn. (If a turn does end early anyway, recovery is the forward-resume control loop in Phase 0.)

> **PREREQUISITE**: The `ocr` CLI must be installed (`npm install -g @open-code-review/cli`) or accessible via `npx`. Every phase transition calls `ocr state advance`, which requires the CLI.

---
Expand Down Expand Up @@ -90,6 +92,25 @@ When starting a new round (CURRENT_ROUND > 1), pass the `--current-round` flag t
- **State and files mismatch**: Ask user which to trust
- **No session exists**: Create session directory and start Phase 1

> **Forward-resume control loop (a stranded mid-pipeline run).** If a prior turn
> ended between phases (crash, token limit, disconnect, `Ctrl-C`, or a host that
> finalized the turn on its own), the session is left `active` with no
> `round_completed`. To recover it, run `ocr state status --json` and **act on
> `next_action`** — do not infer state from the filesystem:
> - `next_action = "forward_resume"`: re-enter `current_phase` and continue
> **forward** through `remaining_phases` to `complete-round`. **Never regress**
> to an earlier phase and **never re-run a phase whose artifact already
> exists** — Phase 4 re-spawns only the reviewers whose output files are
> missing; aggregation/discourse/synthesis pick up from what's on disk.
> - `next_action = "abort_or_fresh"`: automatic recovery is exhausted — tell the
> user to start a fresh review or `ocr state finish --abort`.
> - `next_action = "finish"` / `"none"`: the round is complete; just
> `ocr state finish` (or nothing).
>
> Do **NOT** call `ocr state begin` on an active, incomplete session — it is for
> starting the *next* round and will be refused (it would reset the phase to
> `context` and lose progress).

### Step 5: Report to user

Before proceeding, tell the user:
Expand Down Expand Up @@ -799,13 +820,17 @@ See `references/discourse.md` for detailed instructions.

**`synthesis_counts`**: Count the actual numbered items (`### 1.`, `### 2.`, etc.) under each section of `final.md`. This is the **deduplicated** count after merging cross-reviewer duplicates.

**`verdict`** — the **merge gate**, exactly one of three values (uppercase, verbatim): `"APPROVE"` | `"REQUEST CHANGES"` | `"NEEDS DISCUSSION"`. The verdict expresses **one** thing — can this land? — and nothing else. Do **not** invent composite verdicts like `accept_with_followups` or `approve_with_suggestions`: residual work is **not** a gate state. Follow-ups and suggestions are carried by finding `category` and surfaced as counts; an APPROVE with open `should_fix` items is normal and correct. The CLI **rejects** any off-vocabulary verdict (exit 7, writes nothing) so you must re-emit a canonical value.

**Finding categories**: `"blocker"` | `"should_fix"` | `"suggestion"` | `"style"`
**Finding severity**: `"critical"` | `"high"` | `"medium"` | `"low"` | `"info"`

Optional flags: `--session-id <id>` (auto-detects active session), `--round <number>` (auto-detects current round).

> **Do NOT write `round-meta.json` directly** — always pipe through the CLI so the schema is validated and the event is recorded atomically.

> **The CLI fails fast (exit 7, nothing written) — self-correct and re-pipe** if: the `verdict` is not one of the three canonical values; any finding `title` is shorter than 8 characters (a degenerate title like `"s"` carries no information); a `synthesis_counts` value **exceeds** the number of findings of that category present (you cannot dedup to *more* than you started with — a count ≤ the tally is fine, that's the legitimate cross-reviewer dedup case); or the `verdict` contradicts the deduplicated **blocker count** — `APPROVE` requires **0** blockers and `REQUEST CHANGES` requires **≥ 1** (`NEEDS DISCUSSION` is unconstrained). If nothing is a blocker, use `APPROVE` and carry the work as `should_fix`/`suggestion`; if something must block merge, categorize it `blocker` and use `REQUEST CHANGES`.

8. **Write the final review file**:
```bash
# OUTPUT FILE - must be exactly this path:
Expand Down
3 changes: 3 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ Keep this managed block so 'openspec update' can refresh the instructions.
- **TypeScript only**: Do not create raw `.js` or `.mjs` files unless they serve a config purpose (e.g., `vite.config.mjs`, `eslint.config.mjs`). All project code, scripts, and utilities must be written in TypeScript.
- **Nx-native automation**: Release process automation must use Nx extension points (e.g., `VersionActions`, `preVersionCommand`), not npm lifecycle scripts or standalone scripts.
- **Agent assets — edit source, then sync**: Agent docs, skills, commands, references, and other agent-related files have their **source of truth in `packages/agents/`**. ALWAYS edit them there, then run `nx run cli:update` to write the changes out to the local project's `.ocr/` directory. Never hand-edit the generated `.ocr/` copies directly — they will be overwritten on the next sync and your edits will drift from source.
- **Shared layers live in `packages/shared/*`, apps never depend on apps**: `cli` and `dashboard` are application packages and MUST NOT depend on one another. Code both apps need (persistence, domain/state, config, cross-platform utilities) lives in dedicated library packages under `packages/shared/*` that each app depends on directly. The current shared packages are `@open-code-review/platform` (cross-platform/runtime utilities), `@open-code-review/persistence` (the `node:sqlite` adapter `db` + workflow `state` lifecycle + `test-support` + `vendor-resume` + the `node:sqlite` runtime precondition `runtime-checks` — kept in **one** package because `db` and `state` have a mutually-recursive *type* cycle (`db/types.ts` ↔ `state/types.ts`); a package boundary between them would form a dependency cycle. The single-module-instance connection-cache singleton is a *consequence* of that co-location, not its root cause), and `@open-code-review/config` (`runtime-config` + `team-config` + `models`).
- **Shared packages are source-only, private, and inlined — never published**: each `packages/shared/*` package mirrors `platform` exactly — `private: true`, `version 0.0.0`, every `exports` condition (`types`/`source`/`default`) points at `./src/*.ts` (no `build.mjs`, no `dist`), and it is declared by its **consumers** as a `devDependency: workspace:*` (consumer-side rule). A shared package still declares its own runtime third-party deps in its `dependencies` — they are inlined into the consumer's bundle, so they must resolve at build time. esbuild inlines the `.ts` source into each app's published bundle, so these packages are **excluded from the release set** (`!packages/shared/*` in `nx.json`) and do not join the fixed `cli`+`agents` release group. Do NOT give a shared package a `build` target or a `dist` — that machinery was removed in the cutover and must not return.
- **Graduation is by cause, not by count**: a slice graduates from an app package into a `packages/shared/*` package the moment it is consumed across a package boundary (by the other app, an e2e package, or another shared package) rather than only by its owning app's own code. There is no subpath-count trigger. A genuinely app-internal module stays in its app; the goal is to keep the dependency graph a DAG of `app → shared → shared`, never `app → app`.

## Release Process (GitHub + npm)

Expand Down
Loading
Loading