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

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,9 @@ running past the end would otherwise be counted and highlighted with nothing to

## The review inbox

`diffity inbox` watches the pull requests awaiting your review and prepares each one ahead of time, so the review is ready the moment you look. It polls GitHub (`gh search prs --review-requested=@me`), and for each pull request worth your attention it cuts a worktree at the PR head, runs a diffity session over the diff, has an agent prepare a review with a walkthrough, and saves the result as a bundle. New commits redo a stale review; a merged, closed, or no-longer-requested PR is retired. The daemon prepares up to `maxPrepared` of them from the queue on its own — the rest wait, smallest first — and a prepared review leaves the inbox once you have posted it (GitHub withdraws the request) or dismissed it from the page — a dismissal holds until the pull request gets new commits, and dismissed pull requests stay listed at the bottom so one can be brought back. A queued, skipped, failed or dismissed pull request has a ↑ button: prepare this one now — at once and in parallel with whatever the daemon is preparing, past the auto-prepare count, with the filter, the title patterns and the CI hold set aside. Each bump is its own agent run, and the row says `preparing` while it runs. Each prepared review shows its findings by severity ("1 P1 · 2 P2"), and the page can notify you when one is ready — click "Turn on notifications" once to allow it, and ⟳ in the header polls GitHub now instead of at the next interval; `localhost` counts as a secure context, so this works from the pinned tab with nothing else set up.
`diffity inbox` watches the pull requests awaiting your review and prepares each one ahead of time, so the review is ready the moment you look. It polls GitHub (`gh search prs --review-requested=@me`), and for each pull request worth your attention it cuts a worktree at the PR head, runs a diffity session over the diff, has an agent prepare a review with a walkthrough, and saves the result as a bundle. New commits redo a stale review; a merged, closed, or no-longer-requested PR is retired. The daemon prepares up to `maxPrepared` of them from the queue on its own — the rest wait, smallest first — and a prepared review leaves the Ready list once you have posted it (GitHub withdraws the request, and the pull request moves to Handled, below) or dismissed it from the page — a dismissal holds until the pull request gets new commits, and dismissed pull requests stay listed at the bottom so one can be brought back. A queued, skipped, failed or dismissed pull request has a ↑ button: prepare this one now — at once and in parallel with whatever the daemon is preparing, past the auto-prepare count, with the filter, the title patterns and the CI hold set aside. Each bump is its own agent run, and the row says `preparing` while it runs. Each prepared review shows its findings by severity ("1 P1 · 2 P2"), and the page can notify you when one is ready — click "Turn on notifications" once to allow it, and ⟳ in the header polls GitHub now instead of at the next interval; `localhost` counts as a secure context, so this works from the pinned tab with nothing else set up.

Every review you post from diffity is noted against its pull request, with the commit it was posted against, and the pull request then stays under **Handled** instead of vanishing when GitHub withdraws the review request. The card links to the pull request itself — the worktree is reclaimed once the review is out — and says what you said: "you approved", "you requested changes", "you commented", and when. When the author pushes after your review, the card moves to the top of the list, reads "new commits since you approved" and is badged `updated`, so a pull request that has come back to you is not something you have to remember; ↑ prepares a fresh review of the current head, re-request or not, and × sets the row aside until the next push. A pull request leaves the list when the author asks for a new review — the search lists it again and it goes back in the queue like anything else — or when it is merged or closed. This counts reviews posted from any diffity, so one you posted from your own clone brings its pull request into the list at the next poll, at the cost of one `gh pr view`. `diffity inbox status` prints the same list.

The daemon never posts your prepared reviews to GitHub — they are local drafts you open and submit yourself — and it runs the review agent with your GitHub credentials stripped from its environment. Its own git calls run with hooks disabled, so a checkout's hook scripts — the author's code — never run with your credentials. That said, the agent executes the pull request's own repository code (see the warning below), so treat the "never posts" behaviour as the daemon's design, not a sandbox.

Expand Down
12 changes: 6 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/api/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@diffity/api",
"version": "0.10.28",
"version": "0.10.29",
"private": true,
"type": "module",
"main": "./dist/index.js",
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@naturalcycles/diffity",
"version": "0.10.28",
"version": "0.10.29",
"description": "Agent-agnostic, GitHub-style diff viewer and code review tool with a live agent loop",
"type": "module",
"bin": {
Expand Down
6 changes: 5 additions & 1 deletion packages/cli/src/commands/inbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ export function registerInboxCommand(program: Command): void {
return;
}

if (view.ready.length === 0 && view.working.length === 0 && view.other.length === 0 && view.dismissed.length === 0) {
if (view.ready.length === 0 && view.working.length === 0 && view.handled.length === 0
&& view.other.length === 0 && view.dismissed.length === 0) {
console.log(pc.dim('Nothing in the inbox yet. Run `diffity inbox` to start watching.'));
spent(view);
return;
Expand All @@ -99,6 +100,9 @@ export function registerInboxCommand(program: Command): void {
section('Queue', view.working.map(row =>
` ${pc.dim(row.status.padEnd(9))} ${row.repo}#${row.number} ${row.title} ${pc.dim(row.statusReason ?? '')}`,
));
section('Handled', view.handled.map(row =>
` ${pc.dim((row.handled?.updated ? 'updated' : 'handled').padEnd(9))} ${row.repo}#${row.number} ${row.title} ${pc.dim(row.statusReason ?? '')}`,
));
section('Other', view.other.map(row =>
` ${pc.dim(row.status.padEnd(9))} ${row.repo}#${row.number} ${pc.dim(row.statusReason ?? '')}`,
));
Expand Down
37 changes: 37 additions & 0 deletions packages/cli/src/inbox/handled.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import type { ReviewEvent } from '@diffity/api';
import { inboxStorePath } from './paths.js';
import { InboxStore, prId } from './store.js';

/**
* Notes that a review reached the forge, so the inbox keeps listing the pull request instead of
* losing it the moment GitHub withdraws the review request.
*
* The store is opened and closed around the one write: this runs in whichever diffity posted the
* review — a session the inbox prepared, or one the reviewer started on their own clone — and none
* of those hold the inbox open otherwise.
*/
export function recordHandledReview(input: {
owner: string;
repo: string;
number: number;
/** The head the review was posted against. */
headSha: string;
event: ReviewEvent;
reviewUrl: string | null;
now: string;
/** The reviewer's own inbox unless a test says otherwise. */
storePath?: string;
}): void {
const store = new InboxStore(input.storePath ?? inboxStorePath());
try {
store.recordHandled({
prId: prId(input),
headSha: input.headSha,
event: input.event,
reviewUrl: input.reviewUrl,
at: input.now,
});
} finally {
store.close();
}
}
8 changes: 4 additions & 4 deletions packages/cli/src/inbox/open.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { InboxPr, InboxStore } from './store.js';

export const BUMPABLE: ReadonlySet<InboxPr['status']> = new Set(['queued', 'skipped', 'failed', 'dismissed']);
export const BUMPABLE: ReadonlySet<InboxPr['status']> = new Set(['queued', 'skipped', 'failed', 'dismissed', 'handled']);

export type Resolution =
| { ok: true; pr: InboxPr }
Expand Down Expand Up @@ -42,16 +42,16 @@ export function resolveDismiss(store: InboxStore, id: string): Resolution {

/**
* Whether a pull request can be bumped to the front of the queue: one that is waiting, one a verdict
* or a failure set aside, or one the reviewer dismissed and wants back. A prepared, stale or
* in-flight one has nothing to gain.
* or a failure set aside, one the reviewer dismissed and wants back, or one already reviewed whose
* current head they want a fresh review of. A prepared, stale or in-flight one has nothing to gain.
*/
export function resolveBump(store: InboxStore, id: string): Resolution {
const pr = store.get(id);
if (!pr) {
return { ok: false, status: 404, message: `No pull request ${id} in the inbox.` };
}
if (!BUMPABLE.has(pr.status)) {
return { ok: false, status: 409, message: `${id} is ${pr.status}; only a queued, skipped, failed or dismissed pull request can be bumped.` };
return { ok: false, status: 409, message: `${id} is ${pr.status}; only a queued, skipped, failed, dismissed or handled pull request can be bumped.` };
}
return { ok: true, pr };
}
44 changes: 40 additions & 4 deletions packages/cli/src/inbox/page.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* The inbox page, served at `/`. Self-contained (no build step, no external requests): it polls
* `/api/inbox` and renders the three groups, opening a prepared review in a new tab via `/open/:id`.
* `/api/inbox` and renders its groups, opening a prepared review in a new tab via `/open/:id`.
*/
export function inboxPage(): string {
return `<!doctype html>
Expand Down Expand Up @@ -121,6 +121,10 @@ export function inboxPage(): string {
<h2>Queue</h2>
<div id="working"></div>
</section>
<section id="handled-section" hidden>
<h2>Handled</h2>
<div id="handled"></div>
</section>
<section id="other-section" hidden>
<h2>Other</h2>
<div id="other"></div>
Expand Down Expand Up @@ -251,6 +255,34 @@ export function inboxPage(): string {
return row;
}

const VERDICTS = { APPROVE: 'you approved', REQUEST_CHANGES: 'you requested changes', COMMENT: 'you commented' };

/** What the reviewer said about a pull request, and whether the author has pushed since. */
function verdict(h) {
const said = VERDICTS[h.event] || 'you reviewed';
return h.updated ? 'new commits since ' + said : said;
}

// A handled row has no worktree left to open — it was reclaimed when the review was posted — so
// it links to the pull request itself.
function handledRow(r) {
const h = r.handled;
const row = document.createElement('a');
row.className = 'row open';
row.href = r.url;
row.target = '_blank';
row.rel = 'noopener';
row.innerHTML =
'<span class="size">' + sizeLabel(r) + '</span>' +
ciDot(r) +
'<span class="title"><div><span class="repo">' + esc(r.repo) + '#' + r.number + '</span> ' +
'<span class="name">' + esc(r.title) + '</span></div>' +
metaLine(['by ' + esc(r.author), verdict(h), ago(h.at), esc(h.headSha.slice(0, 7))],
'reviewed at ' + h.headSha + (h.reviewUrl ? '\\n' + h.reviewUrl : '')) + '</span>' +
'<span class="badge ' + (h.updated ? 'alert' : 'work') + '">' + (h.updated ? 'updated' : 'handled') + '</span>';
return row;
}

function plainRow(r, badgeClass, badgeText, busy) {
const row = document.createElement('div');
row.className = busy ? 'row busy' : 'row';
Expand All @@ -274,7 +306,10 @@ export function inboxPage(): string {
return plainRow(r, busy ? 'work busy' : 'work', label, busy);
}

function withActions(row, r) {
const BUMP_TITLE = 'Prepare this one now: at once and beside whatever is being prepared, past the auto-prepare count, the skips and the CI hold set aside';
const REPREPARE_TITLE = 'Prepare a fresh review of the current head';

function withActions(row, r, bumpTitle) {
if (!r.dismissUrl && !r.prepareUrl) return row;
const wrap = document.createElement('div');
wrap.className = 'entry';
Expand All @@ -283,7 +318,7 @@ export function inboxPage(): string {
const up = document.createElement('button');
up.type = 'button';
up.className = 'bump';
up.title = 'Prepare this one now: at once and beside whatever is being prepared, past the auto-prepare count, the skips and the CI hold set aside';
up.title = bumpTitle || BUMP_TITLE;
up.textContent = '\\u2191';
up.onclick = () => bump(r);
wrap.append(up);
Expand Down Expand Up @@ -450,12 +485,13 @@ export function inboxPage(): string {
announce(view);
fill('ready-section', 'ready', view.ready, r => withActions(readyRow(r), r));
fill('working-section', 'working', view.working, r => withActions(workingRow(r), r));
fill('handled-section', 'handled', view.handled, r => withActions(handledRow(r), r, REPREPARE_TITLE));
fill('other-section', 'other', view.other, r => {
const bad = r.status === 'failed';
return withActions(plainRow(r, bad ? 'bad' : 'work', r.status), r);
});
fill('dismissed-section', 'dismissed', view.dismissed, r => withActions(plainRow(r, 'work', 'dismissed'), r));
const total = view.ready.length + view.working.length + view.other.length + view.dismissed.length;
const total = view.ready.length + view.working.length + view.handled.length + view.other.length + view.dismissed.length;
el('all-empty').hidden = total > 0;
el('status').textContent = view.ready.length + ' ready \\u00b7 ' + view.working.length + ' queued';
showReload(view.ticking === true);
Expand Down
51 changes: 46 additions & 5 deletions packages/cli/src/inbox/reconcile.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ciState, type PrCheck, type PrSnapshot } from '@diffity/github';
import type { InboxPr, InboxStatus } from './store.js';
import type { Handled, InboxPr, InboxStatus } from './store.js';

/** How many times preparation is retried at one head before the pull request is left as failed. */
export const MAX_PREPARE_ATTEMPTS = 3;
Expand All @@ -20,6 +20,11 @@ export interface ReconcileInput {
/** Whether this poll's search listed the PR as awaiting the reviewer. */
requested: boolean;
viewerLogin: string | null;
/**
* The last review diffity posted for this pull request; absent or null is none, as an unreviewed
* one has.
*/
handled?: Handled | null;
/**
* Whether a pull request waits for its CI before an agent is spent on it; absent is off, as the
* config's default is.
Expand All @@ -42,8 +47,9 @@ const TITLE_MATCHES = 'title matches';
* The status a pull request should move to, given what the forge now says and what the inbox
* already did — the whole decision in one pure function, so every branch is a plain test.
*
* Nothing prepares a draft, the reviewer's own pull request, or a bot's. A closed or merged one, or
* one no longer asking for the review, is retired but keeps whatever was prepared. A new commit
* Nothing prepares a draft, the reviewer's own pull request, or a bot's. A closed or merged one is
* retired but keeps whatever was prepared; one no longer asking for the review is retired too,
* unless a review was posted for it from diffity — that one stays listed as handled. A new commit
* makes a prepared review stale and worth redoing. One the reviewer dismissed stays dismissed until
* it gets new commits; one they bumped is prepared whatever else would have held it back, drafts
* apart. A title matching one of the reviewer's patterns is skipped before any agent is spent on
Expand Down Expand Up @@ -143,8 +149,31 @@ function decide(input: ReconcileInput): Transition | null {
if (!requested) {
if (snapshot.state === 'MERGED') return settled('done', 'merged');
if (snapshot.state === 'CLOSED') return settled('done', 'closed');
// Open, but no longer in the review-requested search: the request was withdrawn or already met.
return settled('hidden', 'review no longer requested');
const handled = input.handled ?? null;
if (!handled) {
// Open, but never reviewed from diffity and no longer in the review-requested search: the
// request was withdrawn, or met somewhere else.
return settled('hidden', 'review no longer requested');
}
// A dismissal covers this version of the pull request here as it does everywhere else.
if (existing?.status === 'dismissed' && existing.headSha === snapshot.headSha) {
return null;
}
// A bump on a handled pull request owns the row while its preparation runs, and the review it
// produced stays openable until that one is posted too.
if (existing?.status === 'queued' || existing?.status === 'preparing') {
return null;
}
// A preparation that failed says so until the head moves, as it does on a requested row: the
// reviewer asked for that review and wants to know it did not happen.
if (existing?.status === 'failed' && existing.headSha === snapshot.headSha) {
return null;
}
if ((existing?.status === 'prepared' || existing?.status === 'stale')
&& (existing.preparedAt ?? '') > handled.at) {
return null;
}
return settled('handled', handledReason(handled, snapshot));
}

// A dismissal is the reviewer's word on this version of the pull request; a new head is a new
Expand Down Expand Up @@ -198,6 +227,18 @@ function decide(input: ReconcileInput): Transition | null {
return { status: 'queued', reason: null, prepare: true };
}

/** What the reviewer said, and whether the author has pushed since they said it. */
function handledReason(handled: Handled, snapshot: PrSnapshot): string {
const verdict = HANDLED_VERDICTS[handled.event];
return handled.headSha === snapshot.headSha ? verdict : `new commits since ${verdict}`;
}

const HANDLED_VERDICTS: Record<Handled['event'], string> = {
APPROVE: 'you approved',
REQUEST_CHANGES: 'you requested changes',
COMMENT: 'you commented',
};

/** A resolved status that needs no preparation — a skip, a draft, or a retirement. */
function settled(status: InboxStatus, reason: string): Transition {
return { status, reason, prepare: false };
Expand Down
Loading
Loading