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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ 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 next, ahead of the queue and past the limit, with the title patterns and the filter set aside; the daemon picks it up at once. 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 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.

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.27",
"version": "0.10.28",
"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.27",
"version": "0.10.28",
"description": "Agent-agnostic, GitHub-style diff viewer and code review tool with a live agent loop",
"type": "module",
"bin": {
Expand Down
47 changes: 29 additions & 18 deletions packages/cli/src/inbox/daemon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@ import { createServer, type IncomingMessage, type Server, type ServerResponse }
import { MAX_SETTINGS_TEXT, parseSettingsPatch } from './settings.js';
import { existsSync, readdirSync, readFileSync, rmSync } from 'node:fs';
import { basename, join } from 'node:path';
import { getViewerLogin, searchReviewRequested, viewPr } from '@diffity/github';
import { getViewerLogin, searchReviewRequested, viewPr, type PrSnapshot } from '@diffity/github';
import { saveInboxSettings, type InboxConfig, type InboxSettings } from './config.js';
import { inboxDir } from './paths.js';
import { logsDir, preparePr, type PrepareDeps } from './prepare.js';
import { realAttendantDeps, realPrepareDeps, type Inflight } from './runtime.js';
import { logsDir, preparePr, type PrepareDeps, type PrepareResult } from './prepare.js';
import { noneInflight, realAttendantDeps, realPrepareDeps, type Inflight } from './runtime.js';
import { Attendants, type AttendedPr } from './attendant.js';
import { removeWorktree, cloneDir } from './worktree.js';
import { localHhMm } from './runs.js';
import { findInstanceForRepo, killInstance } from '../registry.js';
import { repoHash } from './open-session.js';
import { InboxStore } from './store.js';
import { runTick, type Forge } from './tick.js';
import { prepareBumped, runTick, type Forge } from './tick.js';
import { buildView } from './view.js';
import { resolveBump, resolveDismiss, resolveOpen } from './open.js';
import { openPreparedSession, realOpenSessionDeps, type OpenSessionDeps } from './open-session.js';
Expand Down Expand Up @@ -44,15 +44,19 @@ export interface DaemonOptions {
openDeps?: OpenSessionDeps;
/** Who parks on an opened review; defaults to the real attendants. Tests override it. */
attendants?: AttendantHost;
/** How one pull request is prepared; defaults to the real preparation. Tests override it. */
prepare?: (snapshot: PrSnapshot, opts: { bumped: boolean }) => Promise<PrepareResult>;
/** Where the prepares register what they have running, for the shutdown to stop; its own by default. */
inflight?: Inflight;
/** Where the page's settings are written; without it they change the running daemon only. */
configPath?: string;
}

/** What the page may ask of the daemon beyond the store: park agents, tick, settings, and how the tick is doing. */
export interface ServerHooks {
attendants?: AttendantHost | null;
/** A bump wants a tick now, or right after the one in flight. */
onBump?: (() => void) | null;
/** A bump wants this pull request prepared now, whatever else is being prepared. */
onBump?: ((id: string) => void) | null;
/** The page's ⟳: the same tick, asked for by hand. */
onTick?: (() => void) | null;
settings?: SettingsHost | null;
Expand Down Expand Up @@ -118,18 +122,20 @@ export async function runDaemon(
let ticking = false;
let lastPollAt: string | null = null;

const inflight: Inflight = {};
const inflight = options.inflight ?? noneInflight();
const prepareDeps: PrepareDeps = realPrepareDeps(nodePath, entry, inboxDataDir, config, log, inflight);
// The pause outlives this process: a session limit is the reviewer's, not the daemon's, so it is
// kept in the store and a restart does not spend a run rediscovering it.
const pausedUntil = () => store.pausedUntil(new Date().toISOString());
const deps = {
forge: options.forge ?? realForge,
prepare: (snapshot: Parameters<typeof preparePr>[0], opts: { bumped: boolean }) => preparePr(snapshot, config, prepareDeps, opts),
prepare: options.prepare ?? ((snapshot: PrSnapshot, opts: { bumped: boolean }) => preparePr(snapshot, config, prepareDeps, opts)),
removeWorktree: (worktree: string, repo: string) => reclaimWorktree(config, worktree, repo),
log,
now: () => new Date().toISOString(),
shouldContinue: () => !stopping,
// One set for the tick and the bumps: each knows what the other is already preparing.
inFlight: new Set<string>(),
// Read at each tick, not copied: the page can change it while the daemon runs.
get maxPrepared() { return config.maxPrepared; },
get waitForCi() { return config.waitForCi; },
Expand All @@ -144,7 +150,7 @@ export async function runDaemon(
pausedUntil,
};

// A bump arriving mid-tick is served by another tick right after, not by the next poll.
// A arriving mid-tick is served by another tick right after it, not by the next poll.
let tickWanted = false;
const tick = async () => {
if (ticking || stopping) {
Expand Down Expand Up @@ -192,7 +198,11 @@ export async function runDaemon(
};
const settings = settingsHost(config, options.configPath, armPoll);
const server = await bindInboxServer(store, config, log, openDeps, {
attendants, onBump: requestTick, onTick: requestTick, settings, status: () => ({ ticking, lastPollAt, pausedUntil: pausedUntil() }),
attendants, onTick: requestTick, settings, status: () => ({ ticking, lastPollAt, pausedUntil: pausedUntil() }),
// Not a tick: the ↑ prepares that one at once, beside whatever a tick is already preparing, and
// the page's next refresh finds it `preparing`.
onBump: id => void prepareBumped(store, deps, id)
.catch(err => log(`could not prepare ${id}: ${err instanceof Error ? err.message : err}`)),
});
reclaimLeftoverServers(log);
armPoll();
Expand All @@ -205,10 +215,11 @@ export async function runDaemon(
if (timer) {
clearInterval(timer);
}
// Kill whatever a prepare has running right now — the detached diffity server and the agent
// and its group — so nothing outlives the daemon.
inflight.agentKill?.();
inflight.serverStop?.();
// Kill whatever the prepares have running right now — every detached diffity server and every
// agent with its group — so nothing outlives the daemon.
for (const stop of [...inflight.stops]) {
stop();
}
attendants.stopAll();
server.close(() => {
store.close();
Expand Down Expand Up @@ -414,19 +425,19 @@ function stateChangingId(req: IncomingMessage, res: ServerResponse, prefix: stri
}
}

/** Puts a pull request at the front of the queue and asks for a tick, so it is prepared now. */
function handleBump(store: InboxStore, id: string, onBump: (() => void) | null, log: (message: string) => void, res: ServerResponse): void {
/** Puts a pull request at the front of the queue and has it prepared at once. */
function handleBump(store: InboxStore, id: string, onBump: ((id: string) => void) | null, log: (message: string) => void, res: ServerResponse): void {
const resolution = resolveBump(store, id);
if (!resolution.ok) {
res.writeHead(resolution.status, { 'Content-Type': 'text/plain; charset=utf-8' });
res.end(resolution.message);
return;
}
store.bump(resolution.pr.id, new Date().toISOString());
log(`${id} bumped to the front of the queue`);
log(`${id} bumped — preparing it now`);
res.writeHead(204);
res.end();
onBump?.();
onBump?.(resolution.pr.id);
}

/** Marks a pull request as one the reviewer will not review, and reclaims its worktree. */
Expand Down
33 changes: 29 additions & 4 deletions packages/cli/src/inbox/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,21 @@ export function inboxPage(): string {
.badge.stale { color: var(--stale); border: 1px solid var(--stale); }
.badge.work { color: var(--work); border: 1px solid var(--line); }
.badge.bad { color: var(--bad); border: 1px solid var(--bad); }
.badge.busy { color: var(--accent); border-color: var(--accent); }
/* The wider edge comes out of the padding, so a card being prepared is exactly as wide as the rest. */
.row.busy { border-left: 3px solid var(--accent); padding-left: 12px; }
@media (prefers-reduced-motion: no-preference) {
.badge.busy { animation: working 1.6s infinite ease-in-out; }
.row.busy { animation: edge 1.6s infinite ease-in-out; }
}
@keyframes working {
0%, 100% { color: var(--work); border-color: var(--line); }
50% { color: var(--accent); border-color: var(--accent); }
}
@keyframes edge {
0%, 100% { border-left-color: var(--line); }
50% { border-left-color: var(--accent); }
}
.open-hint { color: var(--accent); font-size: 12px; font-weight: 600; white-space: nowrap; }
.empty { color: var(--muted); padding: 12px 2px; }
.dot { width: 7px; height: 7px; border-radius: 50%; background: var(--ready); flex: none; }
Expand Down Expand Up @@ -236,9 +251,9 @@ export function inboxPage(): string {
return row;
}

function plainRow(r, badgeClass, badgeText) {
function plainRow(r, badgeClass, badgeText, busy) {
const row = document.createElement('div');
row.className = 'row';
row.className = busy ? 'row busy' : 'row';
row.innerHTML =
'<span class="size">' + sizeLabel(r) + '</span>' +
ciDot(r) +
Expand All @@ -249,6 +264,16 @@ export function inboxPage(): string {
return row;
}

// A queue row. One with an agent on it says so and pulses while it runs; the bump that asked for
// it is spent the moment it starts, so only a row still waiting its turn reads as bumped.
function workingRow(r) {
const busy = r.status === 'preparing';
const label = busy
? (r.bumped ? 'preparing \\u00b7 bumped' : 'preparing')
: (r.bumped && r.status === 'queued' ? 'bumped' : r.status);
return plainRow(r, busy ? 'work busy' : 'work', label, busy);
}

function withActions(row, r) {
if (!r.dismissUrl && !r.prepareUrl) return row;
const wrap = document.createElement('div');
Expand All @@ -258,7 +283,7 @@ export function inboxPage(): string {
const up = document.createElement('button');
up.type = 'button';
up.className = 'bump';
up.title = 'Prepare this one next: ahead of the queue, past the limit, the skips set aside';
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.textContent = '\\u2191';
up.onclick = () => bump(r);
wrap.append(up);
Expand Down Expand Up @@ -424,7 +449,7 @@ export function inboxPage(): string {
const view = await res.json();
announce(view);
fill('ready-section', 'ready', view.ready, r => withActions(readyRow(r), r));
fill('working-section', 'working', view.working, r => withActions(plainRow(r, 'work', r.bumped ? 'bumped' : r.status), r));
fill('working-section', 'working', view.working, r => withActions(workingRow(r), r));
fill('other-section', 'other', view.other, r => {
const bad = r.status === 'failed';
return withActions(plainRow(r, bad ? 'bad' : 'work', r.status), r);
Expand Down
Loading
Loading