From 906b20b16fa1f42b88b9e998f708538d9cfc9a76 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Tue, 25 Aug 2026 08:02:25 +0200 Subject: [PATCH 01/12] feat(live): Ask and Act are separate buttons, and the request says which MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One button meaning "hand this over and let the agent decide" left the decision in the wrong place. There are two now, and the request carries an `intent` rather than the instruction being mixed into the comment — the reader's text stays theirs, and the agent is told in a field. `await` still says it in plain words, because that is what an agent acts on: a question gets "Do not change code — they pressed Ask, not Act". An intent that is absent or unrecognised is a question, so a request that does not say what it wants gets an answer rather than an edit, and so does every request written before this. Act is not offered at all on a pull request somebody else wrote — the page asks the server, which derives it from who wrote it. Better than offering a button and then refusing it: a button that is there is a promise. Asking for a change there is still refused, since a request can arrive by other means than the button. Also: editing a comment no longer happens through a three-row letterbox. The box is sized to what is in it, between four rows and twenty-four, past which scrolling inside it is the lesser evil. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- packages/cli/src/agent.ts | 10 ++--- packages/cli/src/db.ts | 3 ++ packages/cli/src/live-intent.ts | 30 +++++++++++++ packages/cli/src/live.ts | 22 ++++++---- packages/cli/src/review-routes.ts | 9 ++-- packages/cli/src/server.ts | 2 + packages/cli/tests/live-intent.test.ts | 44 +++++++++++++++++++ packages/skills/diffity-live/SKILL.md | 18 ++++++-- .../components/comments/comment-bubble.tsx | 5 ++- .../components/comments/comment-form-row.tsx | 4 +- .../src/components/comments/comment-form.tsx | 41 ++++++++++++----- .../components/comments/comment-thread.tsx | 4 ++ .../src/components/comments/thread-card.tsx | 10 ++++- packages/ui/src/components/comments/types.ts | 2 + packages/ui/src/components/diff/diff-page.tsx | 36 +++++++++++++-- packages/ui/src/components/diff/diff-view.tsx | 6 +++ .../ui/src/components/diff/file-block.tsx | 6 ++- .../src/components/diff/hunk-block-split.tsx | 9 +++- .../ui/src/components/diff/hunk-block.tsx | 7 ++- .../ui/src/components/diff/hunk-with-gap.tsx | 8 +++- packages/ui/src/hooks/use-comment-actions.ts | 1 + packages/ui/src/lib/api.ts | 4 ++ packages/ui/src/lib/edit-box-size.ts | 18 ++++++++ packages/ui/src/queries/live.ts | 2 + packages/ui/tests/edit-box-size.test.ts | 32 ++++++++++++++ skills/diffity-live/SKILL.md | 18 ++++++-- 26 files changed, 298 insertions(+), 53 deletions(-) create mode 100644 packages/cli/src/live-intent.ts create mode 100644 packages/cli/tests/live-intent.test.ts create mode 100644 packages/ui/src/lib/edit-box-size.ts create mode 100644 packages/ui/tests/edit-box-size.test.ts diff --git a/packages/cli/src/agent.ts b/packages/cli/src/agent.ts index 91f17e8..c163d8f 100644 --- a/packages/cli/src/agent.ts +++ b/packages/cli/src/agent.ts @@ -16,6 +16,7 @@ import { } from './threads.js'; import { answerLiveRequest, type LiveRequest } from './live.js'; import { clampClientWait } from './live-wait.js'; +import { directiveFor } from './live-intent.js'; import { findInstanceForRepo, type RegistryEntry } from './registry.js'; import { createHash } from 'node:crypto'; import { createTour, addTourStep, updateTourStatus, deleteTour, deleteToursForSession } from './tours.js'; @@ -347,15 +348,10 @@ Examples: // stdout is the request, so a script can parse it. The directive goes to stderr, because the // turn this wakes up may be a long way from whatever armed the loop. - const mayChange = payload.request.mayChangeCode !== false; console.error( pc.cyan( - 'A request came back from the review page. ' - + (mayChange - ? 'Answer it in the thread, amend the finding it is about, or make the change' - : 'Answer it in the thread or amend the finding it is about — this pull request is ' - + 'somebody else\'s, so do not edit its code') - + ' — then re-arm with `agent await`. The diffity-live skill has the detail.', + `${directiveFor(payload.request.intent, payload.request.mayChangeCode !== false)}\n` + + 'Then re-arm with `agent await`. The diffity-live skill has the detail.', ), ); console.log(JSON.stringify(payload.request, null, 2)); diff --git a/packages/cli/src/db.ts b/packages/cli/src/db.ts index 5b23d76..87593b0 100644 --- a/packages/cli/src/db.ts +++ b/packages/cli/src/db.ts @@ -141,6 +141,9 @@ function migrateDb(db: DatabaseSync): void { // because "asked but not yet picked up" and "picked up but not answered" are both waiting, and // the page says different things about them. addColumn(db, 'comments', 'live_requested_at', 'TEXT'); + // Ask or act. Absent means ask: a request that does not say what it wants gets an answer rather + // than an edit, which is also what every request written before this said by omission. + addColumn(db, 'comments', 'live_intent', 'TEXT'); addColumn(db, 'comments', 'live_claimed_at', 'TEXT'); addColumn(db, 'comments', 'live_answered_at', 'TEXT'); addColumn(db, 'comment_threads', 'submitted_at', 'TEXT'); diff --git a/packages/cli/src/live-intent.ts b/packages/cli/src/live-intent.ts new file mode 100644 index 0000000..f4bb655 --- /dev/null +++ b/packages/cli/src/live-intent.ts @@ -0,0 +1,30 @@ +/** What the reader pressed: a question, or a request for a change. */ +export type LiveIntent = 'ask' | 'act'; + +/** + * Anything that does not plainly say `act` is a question. Least privilege, and it covers both + * requests written before intent existed and anything malformed arriving at the route. + */ +export function normaliseIntent(value: unknown): LiveIntent { + return value === 'act' ? 'act' : 'ask'; +} + +/** + * What the agent is told on waking. The intent is a field rather than words mixed into the comment, + * so the reader's text stays theirs — but the instruction still has to be said in plain language, + * because that is what the agent acts on. + */ +export function directiveFor(intent: LiveIntent, mayChangeCode: boolean): string { + if (intent === 'ask') { + return 'The reader asked a question. Answer it in the thread, or amend the finding it is about. ' + + 'Do not change code — they pressed Ask, not Act.'; + } + + if (!mayChangeCode) { + return 'The reader asked for a change, but this pull request is somebody else\'s. ' + + 'Do not change code. Answer in the thread, or amend the finding, and say that is why.'; + } + + return 'The reader asked for a change. Read it, make the change, and reply in the thread with what ' + + 'you did. Do not commit, push or merge.'; +} diff --git a/packages/cli/src/live.ts b/packages/cli/src/live.ts index cb4c85d..3473ec6 100644 --- a/packages/cli/src/live.ts +++ b/packages/cli/src/live.ts @@ -1,4 +1,5 @@ import { getDb, queryOne } from './db.js'; +import { normaliseIntent, type LiveIntent } from './live-intent.js'; /** * A question or an instruction the reader left for the agent, taken from the comment it was written @@ -25,6 +26,8 @@ export interface LiveRequest { * filled in by the route that hands the request over. */ mayChangeCode?: boolean; + /** What the reader pressed. A question must not turn into an edit. */ + intent: LiveIntent; } export interface LiveRequestStamp { @@ -34,10 +37,10 @@ export interface LiveRequestStamp { requestedAt: string | null; } -export function requestLive(commentId: string): LiveRequestStamp { +export function requestLive(commentId: string, intent: LiveIntent = 'ask'): LiveRequestStamp { getDb() - .prepare("UPDATE comments SET live_requested_at = datetime('now') WHERE id = ?") - .run(commentId); + .prepare("UPDATE comments SET live_requested_at = datetime('now'), live_intent = ? WHERE id = ?") + .run(intent, commentId); const row = queryOne<{ session_id: string; live_requested_at: string | null }>( `SELECT t.session_id, c.live_requested_at FROM comments c JOIN comment_threads t ON t.id = c.thread_id @@ -73,19 +76,20 @@ export function claimNextLiveRequest(sessionId: string): LiveRequest | null { return null; } - return ( - queryOne( - `SELECT c.id AS commentId, c.thread_id AS threadId, c.body AS body, - c.author_name AS authorName, t.file_path AS filePath, t.side AS side, + const request = queryOne( + `SELECT c.id AS commentId, c.thread_id AS threadId, c.body AS body, + c.author_name AS authorName, c.live_intent AS intent, + t.file_path AS filePath, t.side AS side, t.start_line AS startLine, t.end_line AS endLine, (SELECT f.body FROM comments f WHERE f.thread_id = t.id AND COALESCE(f.kind, 'review') = 'review' ORDER BY f.created_at ASC, f.rowid ASC LIMIT 1) AS findingBody FROM comments c JOIN comment_threads t ON t.id = c.thread_id WHERE c.id = ?`, - claimed.id, - ) ?? null + claimed.id, ); + + return request ? { ...request, intent: normaliseIntent(request.intent) } : null; } /** diff --git a/packages/cli/src/review-routes.ts b/packages/cli/src/review-routes.ts index 1f7e28c..236313b 100644 --- a/packages/cli/src/review-routes.ts +++ b/packages/cli/src/review-routes.ts @@ -13,6 +13,7 @@ import { type CommentKind, } from './threads.js'; import { requestLive, notifyLiveListeners } from './live.js'; +import { normaliseIntent } from './live-intent.js'; import { getCurrentSession, resolveSessionId } from './session.js'; import { sendJson, sendError, withJsonBody } from './http-utils.js'; @@ -50,7 +51,7 @@ export function handleReviewRoute(req: IncomingMessage, res: ServerResponse, pat if (pathname === '/api/threads' && req.method === 'POST') { withJsonBody(res, req, 'Failed to create thread', (body) => { - const { sessionId: sid, filePath, side, startLine, endLine, body: commentBody, author, anchorContent, kind, live } = body; + const { sessionId: sid, filePath, side, startLine, endLine, body: commentBody, author, anchorContent, kind, live, intent } = body; if (!sid || !filePath || !side || typeof startLine !== 'number' || typeof endLine !== 'number' || !commentBody || !author) { sendError(res, 400, 'Missing required fields'); return; @@ -63,7 +64,7 @@ export function handleReviewRoute(req: IncomingMessage, res: ServerResponse, pat threadKind, ); if (live === true && threadKind === 'aside') { - const stamp = requestLive(thread.comments[0].id); + const stamp = requestLive(thread.comments[0].id, normaliseIntent(intent)); thread.comments[0].liveRequestedAt = stamp.requestedAt; notifyLiveListeners(stamp.sessionId); } @@ -75,7 +76,7 @@ export function handleReviewRoute(req: IncomingMessage, res: ServerResponse, pat const threadReplyMatch = pathname.match(/^\/api\/threads\/([^/]+)\/reply$/); if (threadReplyMatch && req.method === 'POST') { withJsonBody(res, req, 'Failed to add reply', (body) => { - const { body: commentBody, author, kind, live } = body; + const { body: commentBody, author, kind, live, intent } = body; if (!commentBody || !author) { sendError(res, 400, 'Missing body or author'); return; @@ -91,7 +92,7 @@ export function handleReviewRoute(req: IncomingMessage, res: ServerResponse, pat // request's author, and it is going to the forge rather than to a listener here. let requestedAt: string | null = null; if (live === true && commentKind === 'aside') { - const stamp = requestLive(comment.id); + const stamp = requestLive(comment.id, normaliseIntent(intent)); requestedAt = stamp.requestedAt; notifyLiveListeners(stamp.sessionId); } diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index f441376..2d23760 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -373,6 +373,8 @@ export function startServer(options: ServerOptions): Promise { enabled: isLoopbackBind(getBindHost()), listening: sid ? liveListenerCount(sid) > 0 : false, waiting: sid ? pendingLiveCount(sid) : 0, + // So the page can decline to offer Act at all, rather than offering it and refusing. + mayChangeCode: mayChangeCode(authorship()), }); return; } diff --git a/packages/cli/tests/live-intent.test.ts b/packages/cli/tests/live-intent.test.ts new file mode 100644 index 0000000..12700d7 --- /dev/null +++ b/packages/cli/tests/live-intent.test.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from 'vitest'; +import { normaliseIntent, directiveFor } from '../src/live-intent.js'; + +describe('what the reader asked for', () => { + it('takes an explicit intent', () => { + expect(normaliseIntent('act')).toBe('act'); + expect(normaliseIntent('ask')).toBe('ask'); + }); + + // Least privilege: a request that does not say what it wants gets an answer, not an edit. That + // covers rows written before intent existed as well as anything malformed. + it('answers rather than acts when it does not say', () => { + expect(normaliseIntent(undefined)).toBe('ask'); + expect(normaliseIntent(null)).toBe('ask'); + expect(normaliseIntent('anything else')).toBe('ask'); + }); +}); + +describe('what the agent is told to do with it', () => { + it('forbids changing code for a question, in as many words', () => { + const directive = directiveFor('ask', true); + + expect(directive).toContain('Do not change code'); + }); + + it('allows a change when one was asked for and is permitted', () => { + const directive = directiveFor('act', true); + + expect(directive).toContain('make the change'); + expect(directive).not.toContain('Do not change code'); + }); + + // Asking for a change on somebody else's pull request is still not permission to make one. + it('refuses a change on a pull request the reader did not write', () => { + const directive = directiveFor('act', false); + + expect(directive).toContain('Do not change code'); + expect(directive).toContain('somebody else'); + }); + + it('says the same about a question either way', () => { + expect(directiveFor('ask', false)).toContain('Do not change code'); + }); +}); diff --git a/packages/skills/diffity-live/SKILL.md b/packages/skills/diffity-live/SKILL.md index 02349a7..6bec82e 100644 --- a/packages/skills/diffity-live/SKILL.md +++ b/packages/skills/diffity-live/SKILL.md @@ -31,9 +31,20 @@ never did is guessing. `findingBody` is usually the thing being asked about. Read it before the question. -## Choose one of three +## The reader already chose -Read `body` and decide. Do not do more than was asked. +The request carries an `intent`, because there are two buttons in the page and they mean different +things. `await` prints what it means in plain words before the payload; that line is the instruction, +not a summary of it. + +- **`ask`** — a question. Answer it, or amend the finding it is about. **Do not change code**, however + obvious the change looks. They pressed Ask. +- **`act`** — a request for a change. Make it, and say what you did. + +An `intent` that is absent or unrecognised is a question. If `mayChangeCode` is false the answer is +the same whatever they pressed: this pull request is somebody else's. + +Read `body` and decide *how* to do what was asked. Do not do more than was asked. **Answer it.** A question about the finding, the code, or your reasoning. @@ -56,7 +67,8 @@ does. Amend the **finding**, not the aside. If the finding has already been sent, `amend` tells you so — pass that on rather than letting the reader think the pull request has changed. -**Make the change.** Only on your own work. Read the rule below before you edit anything. +**Make the change.** Only when the intent is `act` and `mayChangeCode` is not false. Read the rule +below before you edit anything. ``` # make the edit, then: diff --git a/packages/ui/src/components/comments/comment-bubble.tsx b/packages/ui/src/components/comments/comment-bubble.tsx index 1d40a1a..7badb67 100644 --- a/packages/ui/src/components/comments/comment-bubble.tsx +++ b/packages/ui/src/components/comments/comment-bubble.tsx @@ -4,6 +4,7 @@ import { PencilIcon } from '../icons/pencil-icon'; import { TrashIcon } from '../icons/trash-icon'; import { MarkdownContent } from '../layout/markdown-content'; import { isAside, requestStateOf, type RequestState } from '../../lib/live-mode'; +import { rowsForBody } from '../../lib/edit-box-size'; interface CommentBubbleProps { comment: Comment; @@ -170,8 +171,8 @@ export function CommentBubble(props: CommentBubbleProps) { value={editBody} onChange={(e) => setEditBody(e.target.value)} onKeyDown={handleKeyDown} - rows={3} - className="w-full px-3 py-2 text-sm bg-bg-tertiary text-text resize-y outline-none rounded-md min-h-[60px]" + rows={rowsForBody(editBody)} + className="w-full px-3 py-2 text-sm bg-bg-tertiary text-text resize-y outline-none rounded-md" />
diff --git a/packages/ui/src/components/comments/comment-form-row.tsx b/packages/ui/src/components/comments/comment-form-row.tsx index d200bb1..a4366dd 100644 --- a/packages/ui/src/components/comments/comment-form-row.tsx +++ b/packages/ui/src/components/comments/comment-form-row.tsx @@ -13,11 +13,12 @@ interface CommentFormRowProps { viewMode?: 'unified' | 'split'; /** Hands a brand-new comment to the agent — asking about code nobody has commented on yet. */ onAsk?: (filePath: string, side: CommentSide, startLine: number, endLine: number, body: string, author: CommentAuthor) => void; + onAct?: (filePath: string, side: CommentSide, startLine: number, endLine: number, body: string, author: CommentAuthor) => void; askIsHeard?: boolean; } export function CommentFormRow(props: CommentFormRowProps) { - const { colSpan, filePath, side, startLine, endLine, currentAuthor, onSubmit, onCancel, viewMode, onAsk, askIsHeard } = props; + const { colSpan, filePath, side, startLine, endLine, currentAuthor, onSubmit, onCancel, viewMode, onAsk, onAct, askIsHeard } = props; const lineLabel = startLine === endLine ? `${startLine}` @@ -28,6 +29,7 @@ export function CommentFormRow(props: CommentFormRowProps) { onSubmit(filePath, side, startLine, endLine, body, currentAuthor)} onAsk={onAsk && ((body) => onAsk(filePath, side, startLine, endLine, body, currentAuthor))} + onAct={onAct && ((body) => onAct(filePath, side, startLine, endLine, body, currentAuthor))} askIsHeard={askIsHeard} onCancel={onCancel} lineLabel={`Add a comment on line${startLine !== endLine ? 's' : ''} ${lineLabel}`} diff --git a/packages/ui/src/components/comments/comment-form.tsx b/packages/ui/src/components/comments/comment-form.tsx index aa8edd1..6a274a5 100644 --- a/packages/ui/src/components/comments/comment-form.tsx +++ b/packages/ui/src/components/comments/comment-form.tsx @@ -8,18 +8,23 @@ interface CommentFormProps { autoFocus?: boolean; lineLabel?: string; /** - * Hands the text to the agent instead of leaving it for whoever wrote the code. Absent when no - * agent can be reached, so the button is never offered where it would do nothing. + * Hands the text to the agent as a question. Absent when no agent can be reached, so the button + * is never offered where it would do nothing. */ onAsk?: (body: string) => void; - /** Whether an agent is waiting right now, which changes what the button promises. */ + /** + * Asks the agent to make the change. Absent when the diff is somebody else's pull request, so it + * is not offered and then refused. + */ + onAct?: (body: string) => void; + /** Whether an agent is waiting right now, which changes what the buttons promise. */ askIsHeard?: boolean; } export function CommentForm(props: CommentFormProps) { const { onSubmit, onCancel, placeholder = 'Leave a comment', submitLabel = 'Comment', - autoFocus = true, lineLabel, onAsk, askIsHeard, + autoFocus = true, lineLabel, onAsk, onAct, askIsHeard, } = props; const [body, setBody] = useState(''); const textareaRef = useRef(null); @@ -39,12 +44,12 @@ export function CommentForm(props: CommentFormProps) { setBody(''); }; - const handleAsk = () => { + const handOver = (to?: (body: string) => void) => () => { const trimmed = body.trim(); - if (!trimmed || !onAsk) { + if (!trimmed || !to) { return; } - onAsk(trimmed); + to(trimmed); setBody(''); }; @@ -88,16 +93,30 @@ export function CommentForm(props: CommentFormProps) { {onAsk && ( + )} + {onAct && ( + )} - +
+ + +
+
); } diff --git a/packages/ui/src/components/layout/unseen-answers.tsx b/packages/ui/src/components/layout/unseen-answers.tsx new file mode 100644 index 0000000..f7eaedd --- /dev/null +++ b/packages/ui/src/components/layout/unseen-answers.tsx @@ -0,0 +1,39 @@ +import type { AnswerAlert } from '../../lib/answer-alerts'; +import { ArrowUpIcon } from '../icons/arrow-up-icon'; + +interface UnseenAnswersProps { + alerts: AnswerAlert[]; + onGo: (threadId: string) => void; +} + +/** + * What a note leaves behind once its time is up: a count, and a way back to the answer. Sits at the + * top of the diff over the gutter column, straddling the file header — close enough to the code to + * be noticed while reading, far enough left not to sit on it. + * + * Goes to the oldest first, so following it repeatedly walks through them in the order they arrived. + */ +export function UnseenAnswers(props: UnseenAnswersProps) { + const { alerts, onGo } = props; + + if (alerts.length === 0) { + return null; + } + + const oldest = alerts[0]; + + return ( + + ); +} diff --git a/packages/ui/src/lib/answer-alerts.ts b/packages/ui/src/lib/answer-alerts.ts index 930896a..90b6214 100644 --- a/packages/ui/src/lib/answer-alerts.ts +++ b/packages/ui/src/lib/answer-alerts.ts @@ -1,5 +1,6 @@ import type { CommentThread } from '../components/comments/types'; import { isAside } from './live-mode'; +import type { ThreadPosition } from './thread-visibility'; export interface AnswerAlert { threadId: string; @@ -74,3 +75,30 @@ export function dropSeenAlerts( const kept = alerts.filter(alert => !isOnScreen(alert.threadId)); return kept.length === alerts.length ? alerts : kept; } + +/** + * Which edge the note belongs on. A measurement is best, but a thread far from the reader is not + * rendered at all — and treating that as "ahead of you" is how a note about a thread above ended up + * in the bottom corner. Where the file sits in the reading order answers it without the DOM. + */ +export function positionForAlert( + alertFilePath: string, + activeFilePath: string | null, + orderedPaths: string[], + measured: ThreadPosition | null, +): ThreadPosition { + if (measured) { + return measured; + } + if (!activeFilePath) { + return 'below'; + } + + const alertAt = orderedPaths.indexOf(alertFilePath); + const readerAt = orderedPaths.indexOf(activeFilePath); + if (alertAt === -1 || readerAt === -1) { + return 'below'; + } + + return alertAt < readerAt ? 'above' : 'below'; +} diff --git a/packages/ui/tests/alert-position.test.ts b/packages/ui/tests/alert-position.test.ts new file mode 100644 index 0000000..2433999 --- /dev/null +++ b/packages/ui/tests/alert-position.test.ts @@ -0,0 +1,27 @@ +import { describe, it, expect } from 'vitest'; +import { positionForAlert } from '../src/lib/answer-alerts'; + +const order = ['a.ts', 'b.ts', 'c.ts', 'd.ts']; + +describe('which edge a note about an off-screen answer belongs on', () => { + it('trusts the measurement when the thread is rendered', () => { + expect(positionForAlert('d.ts', 'a.ts', order, 'above')).toBe('above'); + expect(positionForAlert('a.ts', 'd.ts', order, 'below')).toBe('below'); + }); + + // A thread far from the reader is not rendered at all, and null used to fall through to "below" — + // which is how a note about a thread above ended up in the bottom corner. + it('falls back to file order when the thread is not rendered', () => { + expect(positionForAlert('a.ts', 'c.ts', order, null)).toBe('above'); + expect(positionForAlert('d.ts', 'b.ts', order, null)).toBe('below'); + }); + + it('treats the same file as ahead, since the thread is further down it', () => { + expect(positionForAlert('b.ts', 'b.ts', order, null)).toBe('below'); + }); + + it('says below when it cannot tell', () => { + expect(positionForAlert('unknown.ts', 'b.ts', order, null)).toBe('below'); + expect(positionForAlert('a.ts', null, order, null)).toBe('below'); + }); +}); diff --git a/packages/ui/tests/answer-bubble.test.tsx b/packages/ui/tests/answer-bubble.test.tsx index 6070d3c..51f3d43 100644 --- a/packages/ui/tests/answer-bubble.test.tsx +++ b/packages/ui/tests/answer-bubble.test.tsx @@ -1,65 +1,89 @@ -import { describe, it, expect, afterEach, vi } from 'vitest'; -import { render, cleanup, screen } from '@testing-library/react'; -import { AnswerBubble } from '../src/components/layout/answer-bubble'; +import { describe, it, expect, afterEach, vi, beforeEach } from 'vitest'; +import { render, cleanup, screen, act } from '@testing-library/react'; +import { AnswerBubble, SHOW_FOR_MS } from '../src/components/layout/answer-bubble'; afterEach(cleanup); const one = [{ threadId: 't1', filePath: 'src/live.ts', authorName: 'Agent', preview: 'The stamp is written by the database.' }]; +function renderBubble(position: 'above' | 'below' = 'above', onExpire = vi.fn(), onGo = vi.fn()) { + render(); + return { onExpire, onGo }; +} + describe('an answer that arrived while you read on', () => { it('shows the reply and where it came from', () => { - render(); + renderBubble(); expect(screen.getByText(/The stamp is written by the database/)).toBeTruthy(); expect(screen.getByText(/live\.ts/)).toBeTruthy(); }); - it('sits at the top when the thread is behind you', () => { - render(); + // Over the old side, which is not the code the reader is reviewing. + it('sits on the left, near the top when the thread is behind you', () => { + renderBubble('above'); + const className = screen.getByRole('status').className; - expect(screen.getByRole('status').className).toContain('top-'); + expect(className).toContain('left-'); + expect(className).toContain('top-'); }); - it('sits at the bottom when the thread is ahead of you', () => { - render(); + it('sits low when the thread is ahead of you', () => { + renderBubble('below'); + const className = screen.getByRole('status').className; - expect(screen.getByRole('status').className).toContain('bottom-'); + expect(className).toContain('left-'); + expect(className).toContain('bottom-'); }); it('takes you there', () => { - const onGo = vi.fn(); - render(); + const { onGo } = renderBubble(); screen.getByRole('button', { name: /the stamp is written/i }).click(); expect(onGo).toHaveBeenCalledWith('t1'); }); - it('can be sent away', () => { - const onDismiss = vi.fn(); - render(); - - screen.getByRole('button', { name: /dismiss/i }).click(); - - expect(onDismiss).toHaveBeenCalled(); - }); - - // Two bubbles competing for the same corner is worse than one that counts. it('collapses several into one, showing the newest', () => { - const two = [ - one[0], - { threadId: 't2', filePath: 'src/agent.ts', authorName: 'Agent', preview: 'And the newest answer.' }, - ]; - render(); + render( + , + ); expect(screen.getAllByRole('status')).toHaveLength(1); expect(screen.getByText(/And the newest answer/)).toBeTruthy(); expect(screen.getByText(/1 more/)).toBeTruthy(); }); +}); + +describe('how long it stays', () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + it('goes on its own rather than sitting there', () => { + const onExpire = vi.fn(); + render(); + + expect(onExpire).not.toHaveBeenCalled(); + act(() => void vi.advanceTimersByTime(SHOW_FOR_MS + 50)); + + expect(onExpire).toHaveBeenCalled(); + }); + + // The bar is the only thing saying it is about to leave, so it has to be running from the start. + it('shows how much time is left', () => { + render(); - it('is not there when nothing has arrived', () => { - render(); + const bar = screen.getByTestId('answer-bubble-timer'); + const atStart = bar.style.width; + act(() => void vi.advanceTimersByTime(SHOW_FOR_MS / 2)); - expect(screen.queryByRole('status')).toBeNull(); + expect(atStart).toBe('100%'); + expect(parseFloat(bar.style.width)).toBeLessThan(60); }); }); diff --git a/packages/ui/tests/unseen-answers.test.tsx b/packages/ui/tests/unseen-answers.test.tsx new file mode 100644 index 0000000..7ec1d1f --- /dev/null +++ b/packages/ui/tests/unseen-answers.test.tsx @@ -0,0 +1,46 @@ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup, screen } from '@testing-library/react'; +import { UnseenAnswers } from '../src/components/layout/unseen-answers'; + +afterEach(cleanup); + +const two = [ + { threadId: 't1', filePath: 'a.ts', authorName: 'Agent', preview: 'first' }, + { threadId: 't2', filePath: 'b.ts', authorName: 'Agent', preview: 'second' }, +]; + +describe('what is left after a note has gone', () => { + it('is nothing at all when nothing is unseen', () => { + render(); + + expect(screen.queryByRole('button')).toBeNull(); + }); + + it('counts what is waiting', () => { + render(); + + expect(screen.getByText('2')).toBeTruthy(); + }); + + // The oldest, so following it repeatedly walks the reader through them in the order they arrived. + it('goes to the one that has been waiting longest', () => { + const onGo = vi.fn(); + render(); + + screen.getByRole('button').click(); + + expect(onGo).toHaveBeenCalledWith('t1'); + }); + + it('says what it will do', () => { + render(); + + expect(screen.getByRole('button').getAttribute('title')).toMatch(/2 answers/i); + }); + + it('counts one in the singular', () => { + render(); + + expect(screen.getByRole('button').getAttribute('title')).toMatch(/1 answer\b/i); + }); +}); From 3884c5ce94f8807cb7deeb7d67378cf170496d4b Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Tue, 25 Aug 2026 08:43:03 +0200 Subject: [PATCH 04/12] feat(live): the agent launching diffity says whether this is work or review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Authorship is a proxy for whose work this is, and it stops being one the moment work is handed over: take over a colleague's branch and the pull request still says they opened it, so Act disappeared and a change request was refused on your own work in progress. The agent bringing diffity up knows which it is doing. `--work` says you are working on this branch, `--review` says you are reviewing it — which also covers reviewing a pull request you wrote yourself, where authorship would have said yes. Neither still means derived from authorship, so nothing changes for anyone who says nothing. Not a setting. It lasts as long as that server, so there is nothing to leave switched on, and handing work over means restarting rather than remembering to untick something. diffity-review now launches with --review, since reviewing is the one thing it is for. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- packages/cli/src/index.ts | 3 ++ packages/cli/src/live-permissions.ts | 26 ++++++++++++++++ packages/cli/src/server.ts | 12 ++++++-- packages/cli/tests/change-permission.test.ts | 31 ++++++++++++++++++++ packages/skills/diffity-live/SKILL.md | 8 ++++- packages/skills/diffity-review/SKILL.md | 5 +++- skills/diffity-live/SKILL.md | 8 ++++- skills/diffity-review/SKILL.md | 5 +++- 8 files changed, 91 insertions(+), 7 deletions(-) create mode 100644 packages/cli/tests/change-permission.test.ts diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 36434ea..cc14bd4 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -72,6 +72,8 @@ program .option('--dark', 'Open in dark mode (default: light)') .option('--unified', 'Open in unified view (default: split)') .option('--new', 'Stop existing instance and start fresh') + .option('--work', 'You are working on this branch, so the agent may change code') + .option('--review', 'You are reviewing it, so the agent may not — even if you wrote it') .addHelpText('after', ` Common usage: $ diffity See all uncommitted changes @@ -331,6 +333,7 @@ range syntax (main..feature, main...feature) also work.`) prNumber: parsedPrNumber, version: pkg.version, registryInfo: { repoRoot, repoHash, repoName }, + purpose: opts.work ? 'work' : opts.review ? 'review' : undefined, }); const urlParams = new URLSearchParams({ ref: effectiveRef }); if (opts.dark) { diff --git a/packages/cli/src/live-permissions.ts b/packages/cli/src/live-permissions.ts index bdb3e6e..be7e6af 100644 --- a/packages/cli/src/live-permissions.ts +++ b/packages/cli/src/live-permissions.ts @@ -12,3 +12,29 @@ export function mayChangeCode(pullRequest: { viewerDidAuthor?: boolean } | null) } return pullRequest.viewerDidAuthor === true; } + +/** What the agent that launched diffity said it was here for. */ +export type SessionPurpose = 'work' | 'review'; + +export function normalisePurpose(value: unknown): SessionPurpose | undefined { + return value === 'work' || value === 'review' ? value : undefined; +} + +/** + * Authorship is a proxy for whose work this is, and it stops being one the moment work is handed + * over: take over a colleague's branch and the pull request still says they opened it. + * + * The agent launching diffity knows which it is doing, so it can say — once, at launch, for that + * server only. Not a setting: there is nothing to leave switched on, because it dies with the + * process. Unsaid still means derived from authorship. + */ +export function resolveMayChangeCode( + purpose: SessionPurpose | undefined, + pullRequest: { viewerDidAuthor?: boolean } | null, +): boolean { + const said = normalisePurpose(purpose); + if (said) { + return said === 'work'; + } + return mayChangeCode(pullRequest); +} diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index 2d23760..5086823 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -47,7 +47,7 @@ import { type ReviewEvent, } from '@diffity/github'; import { findOrCreateSession, resolveSessionId } from './session.js'; -import { mayChangeCode } from './live-permissions.js'; +import { resolveMayChangeCode, type SessionPurpose } from './live-permissions.js'; import { liveListenerCount, pendingLiveCount, @@ -190,6 +190,11 @@ function isSameOriginRequest(req: IncomingMessage): boolean { } interface ServerOptions { + /** + * What the agent launching this said it was here for. Unsaid means derived from who wrote the + * pull request, which is wrong exactly when work has been handed over. + */ + purpose?: SessionPurpose; port: number; portIsExplicit?: boolean; diffArgs: string[]; @@ -247,6 +252,7 @@ interface ServerResult { export function startServer(options: ServerOptions): Promise { const { + purpose, port, portIsExplicit, diffArgs, @@ -374,7 +380,7 @@ export function startServer(options: ServerOptions): Promise { listening: sid ? liveListenerCount(sid) > 0 : false, waiting: sid ? pendingLiveCount(sid) : 0, // So the page can decline to offer Act at all, rather than offering it and refusing. - mayChangeCode: mayChangeCode(authorship()), + mayChangeCode: resolveMayChangeCode(purpose, authorship()), }); return; } @@ -413,7 +419,7 @@ export function startServer(options: ServerOptions): Promise { } // Carried on the request rather than left for the agent to look up: a rule nobody // has to remember is a rule that holds. - sendJson(res, { request: { ...request, mayChangeCode: mayChangeCode(authorship()) } }); + sendJson(res, { request: { ...request, mayChangeCode: resolveMayChangeCode(purpose, authorship()) } }); }, err => { if (!res.writableEnded && !listenerGone.signal.aborted) { diff --git a/packages/cli/tests/change-permission.test.ts b/packages/cli/tests/change-permission.test.ts new file mode 100644 index 0000000..701ad87 --- /dev/null +++ b/packages/cli/tests/change-permission.test.ts @@ -0,0 +1,31 @@ +import { describe, it, expect } from 'vitest'; +import { resolveMayChangeCode } from '../src/live-permissions.js'; + +describe('whether the agent may change code here', () => { + const mine = { viewerDidAuthor: true }; + const theirs = { viewerDidAuthor: false }; + + // Nothing said: fall back to who wrote it, which is what it did before there was a flag. + it('derives it from authorship when nobody said', () => { + expect(resolveMayChangeCode(undefined, mine)).toBe(true); + expect(resolveMayChangeCode(undefined, theirs)).toBe(false); + expect(resolveMayChangeCode(undefined, null)).toBe(true); + }); + + // Taking over somebody else's branch is the case authorship gets wrong, and the agent that + // launched diffity is the one that knows. + it('lets the launcher say this is work rather than review', () => { + expect(resolveMayChangeCode('work', theirs)).toBe(true); + }); + + // And the other way: reviewing your own pull request should not invite edits mid-review. + it('lets the launcher say this is review rather than work', () => { + expect(resolveMayChangeCode('review', mine)).toBe(false); + expect(resolveMayChangeCode('review', null)).toBe(false); + }); + + it('ignores anything it does not recognise rather than guessing', () => { + expect(resolveMayChangeCode('sideways' as never, theirs)).toBe(false); + expect(resolveMayChangeCode('sideways' as never, mine)).toBe(true); + }); +}); diff --git a/packages/skills/diffity-live/SKILL.md b/packages/skills/diffity-live/SKILL.md index 6bec82e..df4d26c 100644 --- a/packages/skills/diffity-live/SKILL.md +++ b/packages/skills/diffity-live/SKILL.md @@ -80,7 +80,13 @@ merge — those wait to be asked for, here as everywhere. ### When you must not change code -Run `{{binary}} agent live-status` if you are unsure. Editing is off the table when the diff is +Run `{{binary}} agent live-status` if you are unsure. + +Whether changes are allowed is decided when diffity is launched. `--review` says you are reviewing +somebody's change, `--work` says you are working on the branch — which is the case authorship gets +wrong, since taking over a colleague's branch leaves their name on the pull request. Said neither +way, it falls back to who wrote it. It lasts as long as that server and no longer, so a reader who +hands work over restarts rather than unticking something. Editing is off the table when the diff is somebody else's pull request, however the conversation goes: reviewing is not editing, and a reader asking a follow-up has not asked you to rewrite their branch. Answer and amend instead, and say that is what you did. diff --git a/packages/skills/diffity-review/SKILL.md b/packages/skills/diffity-review/SKILL.md index e10fe02..bb0ffed 100644 --- a/packages/skills/diffity-review/SKILL.md +++ b/packages/skills/diffity-review/SKILL.md @@ -100,7 +100,10 @@ The review needs a running session whose ref matches the requested ref. A ref mi - If refs **don't match** → restart: run `{{binary}} --no-open --new` (or `{{binary}} --no-open --new` if no ref). The `--new` flag kills the old session and starts a fresh one. Use Bash tool with `run_in_background: true`. Wait 2 seconds, then verify with `{{binary}} list --json` and note the port. - If **no ref was requested** and the running session's ref is not `"work"` → restart with `{{binary}} --no-open --new` (the running session is for a named ref, but we need working-tree). 3. If **no session is running** for this repo, start one in the background: - - Command: `{{binary}} --no-open` (or `{{binary}} --no-open` if no ref) + - Command: `{{binary}} --no-open --review` (or `{{binary}} --no-open --review` if no ref) + - `--review` says what you are here for. Reviewing is not editing, so it keeps Act off the + comment box and refuses a change request even on a pull request the reader wrote themselves. + Authorship alone cannot tell the difference; you can. - Use Bash tool with `run_in_background: true` - Wait 2 seconds, then verify with `{{binary}} list --json` and note the port. diff --git a/skills/diffity-live/SKILL.md b/skills/diffity-live/SKILL.md index b484e5c..cb89182 100644 --- a/skills/diffity-live/SKILL.md +++ b/skills/diffity-live/SKILL.md @@ -80,7 +80,13 @@ merge — those wait to be asked for, here as everywhere. ### When you must not change code -Run `diffity agent live-status` if you are unsure. Editing is off the table when the diff is +Run `diffity agent live-status` if you are unsure. + +Whether changes are allowed is decided when diffity is launched. `--review` says you are reviewing +somebody's change, `--work` says you are working on the branch — which is the case authorship gets +wrong, since taking over a colleague's branch leaves their name on the pull request. Said neither +way, it falls back to who wrote it. It lasts as long as that server and no longer, so a reader who +hands work over restarts rather than unticking something. Editing is off the table when the diff is somebody else's pull request, however the conversation goes: reviewing is not editing, and a reader asking a follow-up has not asked you to rewrite their branch. Answer and amend instead, and say that is what you did. diff --git a/skills/diffity-review/SKILL.md b/skills/diffity-review/SKILL.md index 634e13e..f0ae9f5 100644 --- a/skills/diffity-review/SKILL.md +++ b/skills/diffity-review/SKILL.md @@ -100,7 +100,10 @@ The review needs a running session whose ref matches the requested ref. A ref mi - If refs **don't match** → restart: run `diffity --no-open --new` (or `diffity --no-open --new` if no ref). The `--new` flag kills the old session and starts a fresh one. Use Bash tool with `run_in_background: true`. Wait 2 seconds, then verify with `diffity list --json` and note the port. - If **no ref was requested** and the running session's ref is not `"work"` → restart with `diffity --no-open --new` (the running session is for a named ref, but we need working-tree). 3. If **no session is running** for this repo, start one in the background: - - Command: `diffity --no-open` (or `diffity --no-open` if no ref) + - Command: `diffity --no-open --review` (or `diffity --no-open --review` if no ref) + - `--review` says what you are here for. Reviewing is not editing, so it keeps Act off the + comment box and refuses a change request even on a pull request the reader wrote themselves. + Authorship alone cannot tell the difference; you can. - Use Bash tool with `run_in_background: true` - Wait 2 seconds, then verify with `diffity list --json` and note the port. From 3cde9e45dbf7a6b2ab7fd6a48f8ec93279485708 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Tue, 25 Aug 2026 08:45:58 +0200 Subject: [PATCH 05/12] refactor: cut the commentary back, and stop the note repeating itself on reload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Half the comments added on this branch went. What is left is the handful a reader could not get from the code: a non-obvious invariant, a choice that looks wrong until you know why. The rest was me narrating decisions. Also: threads are an empty array while the query loads, and recording that as "what I have seen" made every existing answer look new on the next poll — so a page rebuilt underneath the reader announced the whole conversation back at them. An empty first look now announces nothing. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- packages/cli/src/db.ts | 2 -- packages/cli/src/live-intent.ts | 11 ++--------- packages/cli/src/live-permissions.ts | 1 - packages/cli/src/server.ts | 3 --- .../ui/src/components/layout/answer-bubble.tsx | 9 ++------- .../src/components/layout/unseen-answers.tsx | 8 +------- packages/ui/src/lib/answer-alerts.ts | 10 ++++------ packages/ui/src/lib/edit-box-size.ts | 9 +-------- packages/ui/src/lib/live-mode.ts | 2 -- packages/ui/tests/answer-alerts.test.ts | 18 ++++++++++++++++++ 10 files changed, 28 insertions(+), 45 deletions(-) diff --git a/packages/cli/src/db.ts b/packages/cli/src/db.ts index 87593b0..1feccbf 100644 --- a/packages/cli/src/db.ts +++ b/packages/cli/src/db.ts @@ -141,8 +141,6 @@ function migrateDb(db: DatabaseSync): void { // because "asked but not yet picked up" and "picked up but not answered" are both waiting, and // the page says different things about them. addColumn(db, 'comments', 'live_requested_at', 'TEXT'); - // Ask or act. Absent means ask: a request that does not say what it wants gets an answer rather - // than an edit, which is also what every request written before this said by omission. addColumn(db, 'comments', 'live_intent', 'TEXT'); addColumn(db, 'comments', 'live_claimed_at', 'TEXT'); addColumn(db, 'comments', 'live_answered_at', 'TEXT'); diff --git a/packages/cli/src/live-intent.ts b/packages/cli/src/live-intent.ts index f4bb655..f3a84a8 100644 --- a/packages/cli/src/live-intent.ts +++ b/packages/cli/src/live-intent.ts @@ -1,19 +1,12 @@ /** What the reader pressed: a question, or a request for a change. */ export type LiveIntent = 'ask' | 'act'; -/** - * Anything that does not plainly say `act` is a question. Least privilege, and it covers both - * requests written before intent existed and anything malformed arriving at the route. - */ +/** Anything not plainly `act` is a question: least privilege, and it covers old and malformed alike. */ export function normaliseIntent(value: unknown): LiveIntent { return value === 'act' ? 'act' : 'ask'; } -/** - * What the agent is told on waking. The intent is a field rather than words mixed into the comment, - * so the reader's text stays theirs — but the instruction still has to be said in plain language, - * because that is what the agent acts on. - */ +/** Said in plain language, because that is what the agent acts on. */ export function directiveFor(intent: LiveIntent, mayChangeCode: boolean): string { if (intent === 'ask') { return 'The reader asked a question. Answer it in the thread, or amend the finding it is about. ' diff --git a/packages/cli/src/live-permissions.ts b/packages/cli/src/live-permissions.ts index be7e6af..f7278d7 100644 --- a/packages/cli/src/live-permissions.ts +++ b/packages/cli/src/live-permissions.ts @@ -13,7 +13,6 @@ export function mayChangeCode(pullRequest: { viewerDidAuthor?: boolean } | null) return pullRequest.viewerDidAuthor === true; } -/** What the agent that launched diffity said it was here for. */ export type SessionPurpose = 'work' | 'review'; export function normalisePurpose(value: unknown): SessionPurpose | undefined { diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index 5086823..73566ed 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -374,12 +374,9 @@ export function startServer(options: ServerOptions): Promise { if (pathname === '/api/live/status') { const sid = resolveSessionId(url.searchParams.get('session')); sendJson(res, { - // A comment box that drives an agent is only as safe as the loopback bind, so live - // mode is not offered at all when the server is reachable from elsewhere. enabled: isLoopbackBind(getBindHost()), listening: sid ? liveListenerCount(sid) > 0 : false, waiting: sid ? pendingLiveCount(sid) : 0, - // So the page can decline to offer Act at all, rather than offering it and refusing. mayChangeCode: resolveMayChangeCode(purpose, authorship()), }); return; diff --git a/packages/ui/src/components/layout/answer-bubble.tsx b/packages/ui/src/components/layout/answer-bubble.tsx index db8e081..c8f4421 100644 --- a/packages/ui/src/components/layout/answer-bubble.tsx +++ b/packages/ui/src/components/layout/answer-bubble.tsx @@ -18,12 +18,8 @@ interface AnswerBubbleProps { } /** - * An answer arrived and the thread it belongs to is off screen. Sits over the old side, near the - * edge the thread is on, so following it moves the way the note already suggests — and never over - * the new side, which is the code being reviewed. - * - * Leaves on its own, with a bar running down so that is not a surprise. Several answers collapse - * into one: two notes competing for the same corner is worse than one that counts. + * Sits over the old side, on the edge the thread is on — never over the code being reviewed. Leaves + * on its own, with a bar running down so that is not a surprise. */ export function AnswerBubble(props: AnswerBubbleProps) { const { alerts, position, onGo, onExpire, onDismiss } = props; @@ -50,7 +46,6 @@ export function AnswerBubble(props: AnswerBubbleProps) { }, TICK_MS); return () => clearInterval(timer); - // Restarted by a newer answer arriving, not by every render. }, [newest?.threadId, newest?.preview, onExpire]); if (!newest) { diff --git a/packages/ui/src/components/layout/unseen-answers.tsx b/packages/ui/src/components/layout/unseen-answers.tsx index f7eaedd..97ab5ed 100644 --- a/packages/ui/src/components/layout/unseen-answers.tsx +++ b/packages/ui/src/components/layout/unseen-answers.tsx @@ -6,13 +6,7 @@ interface UnseenAnswersProps { onGo: (threadId: string) => void; } -/** - * What a note leaves behind once its time is up: a count, and a way back to the answer. Sits at the - * top of the diff over the gutter column, straddling the file header — close enough to the code to - * be noticed while reading, far enough left not to sit on it. - * - * Goes to the oldest first, so following it repeatedly walks through them in the order they arrived. - */ +/** What a note leaves behind once its time is up. Goes to the oldest first. */ export function UnseenAnswers(props: UnseenAnswersProps) { const { alerts, onGo } = props; diff --git a/packages/ui/src/lib/answer-alerts.ts b/packages/ui/src/lib/answer-alerts.ts index 90b6214..be3664d 100644 --- a/packages/ui/src/lib/answer-alerts.ts +++ b/packages/ui/src/lib/answer-alerts.ts @@ -23,7 +23,9 @@ export function newAnswers( previous: CommentThread[] | null, current: CommentThread[], ): AnswerAlert[] { - if (!previous) { + // An empty previous is a page that has not loaded its threads yet, not a review with no comments + // — announcing against it turns a reload into the whole conversation arriving at once. + if (!previous || previous.length === 0) { return []; } @@ -76,11 +78,7 @@ export function dropSeenAlerts( return kept.length === alerts.length ? alerts : kept; } -/** - * Which edge the note belongs on. A measurement is best, but a thread far from the reader is not - * rendered at all — and treating that as "ahead of you" is how a note about a thread above ended up - * in the bottom corner. Where the file sits in the reading order answers it without the DOM. - */ +/** A thread far from the reader is not rendered, so file order answers it when the DOM cannot. */ export function positionForAlert( alertFilePath: string, activeFilePath: string | null, diff --git a/packages/ui/src/lib/edit-box-size.ts b/packages/ui/src/lib/edit-box-size.ts index 1867aac..8aa8b90 100644 --- a/packages/ui/src/lib/edit-box-size.ts +++ b/packages/ui/src/lib/edit-box-size.ts @@ -7,14 +7,7 @@ const MAX_EDIT_ROWS = 24; export const MIN_EDIT_HEIGHT = MIN_EDIT_ROWS * ROW_HEIGHT; export const MAX_EDIT_HEIGHT = MAX_EDIT_ROWS * ROW_HEIGHT; -/** - * How tall the box for editing a comment should be. The height comes from the element measuring its - * own wrapped content — a character count guesses, and guesses differently in split and unified view - * — and this only decides how far it is allowed to go. - * - * A fixed three rows meant editing a long finding through a letterbox; past the maximum the box - * would push the diff off screen, and scrolling inside it is the lesser evil. - */ +/** The element measures its own wrapped height; this only decides how far it may go. */ export function clampEditHeight(measured: number): number { return Math.min(MAX_EDIT_HEIGHT, Math.max(MIN_EDIT_HEIGHT, measured)); } diff --git a/packages/ui/src/lib/live-mode.ts b/packages/ui/src/lib/live-mode.ts index f8116d8..ee9f9b2 100644 --- a/packages/ui/src/lib/live-mode.ts +++ b/packages/ui/src/lib/live-mode.ts @@ -16,7 +16,6 @@ export function requestStateOf(comment: Comment): RequestState | null { return comment.liveClaimedAt ? 'working' : 'waiting'; } -/** What the reader asked for, for a chip that has to say more than "asked". */ export function intentOf(comment: Comment): 'ask' | 'act' { return comment.liveIntent === 'act' ? 'act' : 'ask'; } @@ -30,7 +29,6 @@ interface LiveStatusLike { mayChangeCode: boolean; } -/** Asking is available wherever live mode is, whether or not anyone is listening — it queues. */ export function canAskAgent(status: LiveStatusLike | undefined, reviewsEnabled: boolean): boolean { return !!status?.enabled && reviewsEnabled; } diff --git a/packages/ui/tests/answer-alerts.test.ts b/packages/ui/tests/answer-alerts.test.ts index 0dad4ef..00a5248 100644 --- a/packages/ui/tests/answer-alerts.test.ts +++ b/packages/ui/tests/answer-alerts.test.ts @@ -121,3 +121,21 @@ describe('dropping notes the reader has caught up with', () => { expect(dropSeenAlerts(alerts, () => true)).toEqual([]); }); }); + +describe('a page that has just been rebuilt', () => { + // Threads are an empty array while the query loads. Recording that as "what I have seen" makes + // every existing answer look new on the next poll — which is a page reload announcing the whole + // conversation back at you. + it('announces nothing when the first look was an empty load', () => { + const loaded = [thread('a', [{ id: 'c1', type: 'user' }, { id: 'c2', type: 'agent' }])]; + + expect(newAnswers([], loaded)).toEqual([]); + }); + + it('still announces an answer that arrives after a real look', () => { + const before = [thread('a', [{ id: 'c1', type: 'user' }])]; + const after = [thread('a', [{ id: 'c1', type: 'user' }, { id: 'c2', type: 'agent' }])]; + + expect(newAnswers(before, after)).toHaveLength(1); + }); +}); From 178164b013e30adbd8b20b0d8b33a14f0a38c330 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Tue, 25 Aug 2026 09:12:58 +0200 Subject: [PATCH 06/12] fix(live): the page and the listener now mean the same session MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The page said "No agent" while an agent was parked. The status route read `?session=` and the page sends `?ref=`, so it fell back to the shared current-session file — which every worktree using this data directory writes to — and answered about whichever review was opened last. Both live routes resolve the session from the ref, the way /api/info already did, and `agent await` asks the server which session it is serving instead of reading that file. That file was the ambient-session problem flagged earlier today; this is the path where it actually bit. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- packages/cli/src/agent.ts | 12 +++++++++--- packages/cli/src/server.ts | 15 +++++++++++++-- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/packages/cli/src/agent.ts b/packages/cli/src/agent.ts index c163d8f..7b0144e 100644 --- a/packages/cli/src/agent.ts +++ b/packages/cli/src/agent.ts @@ -314,9 +314,15 @@ Examples: const startedAt = Date.now(); let payload: { request: LiveRequest | null }; try { - const res = await fetch(`http://127.0.0.1:${instance.port}/api/live/claim?wait=${wait}`, { - method: 'POST', - }); + // Park on the session the server is serving, not on whatever the shared current-session + // file last named — those differ whenever another worktree has opened a review since. + const info = await fetch(`http://127.0.0.1:${instance.port}/api/info`); + const sessionId = info.ok + ? ((await info.json()) as { sessionId?: string }).sessionId + : undefined; + const claimUrl = `http://127.0.0.1:${instance.port}/api/live/claim?wait=${wait}` + + (sessionId ? `&session=${encodeURIComponent(sessionId)}` : ''); + const res = await fetch(claimUrl, { method: 'POST' }); if (!res.ok) { console.error(pc.red(`Could not wait for a request: ${res.status} ${await res.text()}`)); process.exitCode = 1; diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index 73566ed..6300776 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -371,8 +371,19 @@ export function startServer(options: ServerOptions): Promise { // as one arms and answers, and react-query keeps an unchanged object's identity — so // carrying this on the info payload made every consumer of it re-render each time a // listener came or went, which reads as the page reloading under you. + // The page asks about the session it is showing, which is the one for its ref — not + // whichever session the ambient current-session file last named, which is shared by every + // worktree using this data directory. + const liveSessionId = (): string => { + const asked = url.searchParams.get('session'); + if (asked) { + return resolveSessionId(asked); + } + return findOrCreateSession(url.searchParams.get('ref') || effectiveRef).id; + }; + if (pathname === '/api/live/status') { - const sid = resolveSessionId(url.searchParams.get('session')); + const sid = liveSessionId(); sendJson(res, { enabled: isLoopbackBind(getBindHost()), listening: sid ? liveListenerCount(sid) > 0 : false, @@ -387,7 +398,7 @@ export function startServer(options: ServerOptions): Promise { sendError(res, 403, 'Live mode is only available on a loopback bind'); return; } - const sid = resolveSessionId(url.searchParams.get('session')); + const sid = liveSessionId(); if (!sid) { sendError(res, 400, 'No review session'); return; From 2ac6f7eea104729c90bcd9c43a12d0ca9a1e3cf1 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Tue, 25 Aug 2026 09:25:35 +0200 Subject: [PATCH 07/12] fix(live): say "Agent working" instead of "No agent" while one is answering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Between taking a request and answering it the listener is not parked on the claim route, so presence alone reported nobody there — at exactly the moment an agent had the reader's question in hand. Which is the window a reader is most likely to be looking at it. Three states now: waiting, working, none. Working is derived from a request claimed and not yet answered, which is a fact about this review rather than about connections. Also: the test helper that drained the queue claimed without answering, so everything it touched counted as being worked on forever. It answers what it takes now, and the two new cases assert a delta — sessions on one branch share their threads, so an absolute count is about the whole file. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- packages/cli/src/live.ts | 15 ++++++ packages/cli/src/server.ts | 2 + packages/cli/tests/live-requests.test.ts | 48 +++++++++++++++++-- .../src/components/layout/live-indicator.tsx | 21 +++++--- packages/ui/src/components/layout/toolbar.tsx | 9 +++- packages/ui/src/queries/live.ts | 2 + 6 files changed, 84 insertions(+), 13 deletions(-) diff --git a/packages/cli/src/live.ts b/packages/cli/src/live.ts index 3473ec6..43c4759 100644 --- a/packages/cli/src/live.ts +++ b/packages/cli/src/live.ts @@ -103,6 +103,21 @@ export function answerLiveRequest(commentIdOrPrefix: string): boolean { return Number(result.changes ?? 0) > 0; } +/** + * Requests an agent has taken and not yet answered. Between the two it is not parked on the claim + * route, so presence alone would report nobody there while somebody is working on your question. + */ +export function liveWorkingCount(sessionId: string): number { + const row = queryOne<{ n: number }>( + `SELECT COUNT(*) AS n FROM comments c JOIN comment_threads t ON t.id = c.thread_id + WHERE t.session_id = ? + AND c.live_claimed_at IS NOT NULL + AND c.live_answered_at IS NULL`, + sessionId, + ); + return row?.n ?? 0; +} + /** How many requests are waiting for somebody to pick them up. */ export function pendingLiveCount(sessionId: string): number { const row = queryOne<{ n: number }>( diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index 6300776..89e88b2 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -50,6 +50,7 @@ import { findOrCreateSession, resolveSessionId } from './session.js'; import { resolveMayChangeCode, type SessionPurpose } from './live-permissions.js'; import { liveListenerCount, + liveWorkingCount, pendingLiveCount, reclaimStaleLiveRequests, waitForLiveRequest, @@ -387,6 +388,7 @@ export function startServer(options: ServerOptions): Promise { sendJson(res, { enabled: isLoopbackBind(getBindHost()), listening: sid ? liveListenerCount(sid) > 0 : false, + working: sid ? liveWorkingCount(sid) > 0 : false, waiting: sid ? pendingLiveCount(sid) : 0, mayChangeCode: resolveMayChangeCode(purpose, authorship()), }); diff --git a/packages/cli/tests/live-requests.test.ts b/packages/cli/tests/live-requests.test.ts index ce3f9bc..650905a 100644 --- a/packages/cli/tests/live-requests.test.ts +++ b/packages/cli/tests/live-requests.test.ts @@ -44,11 +44,14 @@ async function finding(body = 'P2: a finding') { async function drainRequests() { - const { claimNextLiveRequest } = await import('../src/live.js'); + const { claimNextLiveRequest, answerLiveRequest } = await import('../src/live.js'); const s = await session(); - while (claimNextLiveRequest(s.id)) { - // Sessions on one branch share their open threads by design, so earlier cases leave requests - // in this queue. A case about "the next one" has to start from empty. + // Answered, not merely claimed: sessions on one branch share their open threads, and a claim + // without an answer leaves the session looking like an agent is still working on it. + let taken = claimNextLiveRequest(s.id); + while (taken) { + answerLiveRequest(taken.commentId); + taken = claimNextLiveRequest(s.id); } } @@ -345,3 +348,40 @@ describe('a listener whose connection goes away', () => { expect(liveListenerCount(s.id)).toBe(0); }); }); + +describe('while an agent is busy with a request', () => { + // Between taking a request and answering it the listener is not parked, so "listening" is false + // — and the page said "No agent" at exactly the moment one was working on your question. + it('is reported as working, not as absent', async () => { + const { addReply } = await import('../src/threads.js'); + const { requestLive, claimNextLiveRequest, liveWorkingCount } = await import('../src/live.js'); + await drainRequests(); + const s = await session(); + const thread = await finding(); + const asked = addReply(thread.id, 'busy with this', you, 'aside'); + requestLive(asked.id); + + // A delta, not an absolute: other cases in this file claim without answering, and every + // session on this branch shares its threads. + const before = liveWorkingCount(s.id); + claimNextLiveRequest(s.id); + + expect(liveWorkingCount(s.id)).toBe(before + 1); + }); + + it('stops being busy once it has answered', async () => { + const { addReply } = await import('../src/threads.js'); + const { requestLive, claimNextLiveRequest, answerLiveRequest, liveWorkingCount } = await import('../src/live.js'); + await drainRequests(); + const s = await session(); + const thread = await finding(); + const asked = addReply(thread.id, 'answer me', you, 'aside'); + requestLive(asked.id); + claimNextLiveRequest(s.id); + const busy = liveWorkingCount(s.id); + + answerLiveRequest(asked.id); + + expect(liveWorkingCount(s.id)).toBe(busy - 1); + }); +}); diff --git a/packages/ui/src/components/layout/live-indicator.tsx b/packages/ui/src/components/layout/live-indicator.tsx index 68e0f93..2882cdd 100644 --- a/packages/ui/src/components/layout/live-indicator.tsx +++ b/packages/ui/src/components/layout/live-indicator.tsx @@ -1,4 +1,5 @@ interface LiveIndicatorProps { + working: boolean; /** False when the server is not on a loopback bind, where live mode is refused outright. */ enabled: boolean; /** Whether an agent is parked on the claim route right now. */ @@ -12,7 +13,7 @@ interface LiveIndicatorProps { * three comments with silence because the mode was off and nothing said so. */ export function LiveIndicator(props: LiveIndicatorProps) { - const { enabled, listening, waiting } = props; + const { enabled, listening, working, waiting } = props; if (!enabled) { return null; @@ -21,16 +22,22 @@ export function LiveIndicator(props: LiveIndicatorProps) { return ( - - {listening ? 'Agent waiting' : 'No agent'} + + {working ? 'Agent working' : listening ? 'Agent waiting' : 'No agent'} {waiting > 0 && ( {waiting} diff --git a/packages/ui/src/components/layout/toolbar.tsx b/packages/ui/src/components/layout/toolbar.tsx index 338103a..fe08396 100644 --- a/packages/ui/src/components/layout/toolbar.tsx +++ b/packages/ui/src/components/layout/toolbar.tsx @@ -21,7 +21,7 @@ import { isThreadResolved } from '../comments/types'; interface ToolbarProps { reviewInProgress?: boolean; - live?: { enabled: boolean; listening: boolean; waiting: number }; + live?: { enabled: boolean; listening: boolean; working: boolean; waiting: number }; viewMode: ViewMode; onViewModeChange: (mode: ViewMode) => void; hideWhitespace: boolean; @@ -183,7 +183,12 @@ export function Toolbar(props: ToolbarProps) {
{live && ( - + )} Date: Tue, 25 Aug 2026 09:28:50 +0200 Subject: [PATCH 08/12] fix(ui): the reading position is restored once the view exists to restore it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The restore fired on the first frame after the diff loaded, when the view had not mounted, so the scroll went nowhere — and the guard had already been set, so it never tried again. The reader was left at the top of the diff, which is where the general comments and their reply box are. That is the jump, not the bubble. It waits for the handle now, and gives up after sixty frames rather than spinning. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- packages/ui/src/components/diff/diff-page.tsx | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/packages/ui/src/components/diff/diff-page.tsx b/packages/ui/src/components/diff/diff-page.tsx index 1902047..3210c84 100644 --- a/packages/ui/src/components/diff/diff-page.tsx +++ b/packages/ui/src/components/diff/diff-page.tsx @@ -293,13 +293,31 @@ export function DiffPage() { if (restoredPositionRef.current || !orderedDiff || !repoRoot || typeof window === 'undefined') { return; } - restoredPositionRef.current = true; const wasReading = readReadingPosition(window.localStorage, repoRoot, refParam ?? ''); if (!wasReading || !orderedDiff.files.some(file => getFilePath(file) === wasReading)) { + restoredPositionRef.current = true; return; } - setActiveFile(wasReading); - requestAnimationFrame(() => diffViewRef.current?.scrollToFile(wasReading)); + + // The diff view mounts after this runs, and a single frame was not enough — the scroll went + // nowhere and the reader was left at the top of the diff, which is where the general comments + // are. Keep asking until the handle exists, then give up rather than spin. + let attempts = 0; + let frame = requestAnimationFrame(function restore() { + if (diffViewRef.current) { + restoredPositionRef.current = true; + setActiveFile(wasReading); + diffViewRef.current.scrollToFile(wasReading); + return; + } + if (attempts++ > 60) { + restoredPositionRef.current = true; + return; + } + frame = requestAnimationFrame(restore); + }); + + return () => cancelAnimationFrame(frame); }, [orderedDiff, repoRoot, refParam]); // Git reports a rename as `src/{old.ts => new.ts}`, which is never a path in the file list. Naming From b7bb5795557788b3fd6fb6fb5e2f1844f8d5559a Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Tue, 25 Aug 2026 09:38:13 +0200 Subject: [PATCH 09/12] feat(live): unread answers live in a bell, not floating over the diff MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The count was a badge hung over the gutter, and every attempt to place it was wrong in a new way: clipped by the container that scrolls, offset by the file list, on the wrong gutter in one view mode. A toolbar element has none of those problems, and a bell with a count is what everyone already knows how to read. Left of the agent indicator. Quiet with nothing unread, a red count when there is, and a list on click — who answered, which file, the first lines — oldest first, so working through them follows the order they arrived. Also: the note's edge now follows the order the reader sees rather than the raw diff order, which is why a note about a file above them arrived in the bottom corner while the walkthrough had reordered the files. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- packages/ui/src/components/diff/diff-page.tsx | 32 +++++--- .../ui/src/components/icons/bell-icon.tsx | 10 +++ .../components/layout/notification-bell.tsx | 73 +++++++++++++++++++ packages/ui/src/components/layout/toolbar.tsx | 9 +++ .../src/components/layout/unseen-answers.tsx | 33 --------- packages/ui/tests/notification-bell.test.tsx | 61 ++++++++++++++++ packages/ui/tests/unseen-answers.test.tsx | 46 ------------ 7 files changed, 174 insertions(+), 90 deletions(-) create mode 100644 packages/ui/src/components/icons/bell-icon.tsx create mode 100644 packages/ui/src/components/layout/notification-bell.tsx delete mode 100644 packages/ui/src/components/layout/unseen-answers.tsx create mode 100644 packages/ui/tests/notification-bell.test.tsx delete mode 100644 packages/ui/tests/unseen-answers.test.tsx diff --git a/packages/ui/src/components/diff/diff-page.tsx b/packages/ui/src/components/diff/diff-page.tsx index 3210c84..27c1db3 100644 --- a/packages/ui/src/components/diff/diff-page.tsx +++ b/packages/ui/src/components/diff/diff-page.tsx @@ -19,7 +19,6 @@ import { patchDiffFile } from '../../lib/patch-diff-file'; import { newAnswers, dropSeenAlerts, positionForAlert, type AnswerAlert } from '../../lib/answer-alerts'; import { whereIsThread, type ThreadPosition } from '../../lib/thread-visibility'; import { AnswerBubble } from '../layout/answer-bubble'; -import { UnseenAnswers } from '../layout/unseen-answers'; import { fetchDiffFile } from '../../lib/api'; import { diffOptions } from '../../queries/diff'; import { tourMarks, marksByPath, focusRangesFromMarks, type TourFocusRange } from '../../lib/tour-marks'; @@ -323,6 +322,13 @@ export function DiffPage() { // Git reports a rename as `src/{old.ts => new.ts}`, which is never a path in the file list. Naming // it would point the reader at a file they cannot find, so anything unmatched falls back to the // count — the whole-diff refresh still covers it. + // The order the reader sees, which is the walkthrough's when there is one — comparing against the + // raw diff order put a note about a file above them in the bottom corner. + const readingOrderPaths = useMemo( + () => (orderedDiff ? orderedDiff.files.map(file => getFilePath(file)) : []), + [orderedDiff], + ); + const namedStaleFiles = useMemo( () => staleFiles.filter(path => diffPaths.includes(path)), [staleFiles, diffPaths], @@ -576,7 +582,7 @@ export function DiffPage() { positionForAlert( newest.filePath, activeFile, - diffPaths, + readingOrderPaths, bounds ? whereIsThread(bounds.thread, bounds.viewport) : null, ), ); @@ -588,7 +594,7 @@ export function DiffPage() { settle(); container?.addEventListener('scroll', settle, { passive: true }); return () => container?.removeEventListener('scroll', settle); - }, [answerAlerts, activeFile, diffPaths]); + }, [answerAlerts, activeFile, readingOrderPaths]); const handleGoToAnswer = useCallback((threadId: string) => { const alert = [...answerAlerts, ...unseenAlerts].find(a => a.threadId === threadId); @@ -675,6 +681,8 @@ export function DiffPage() { githubDetails={githubDetails} reviewInProgress={!!info?.review?.inProgress} live={liveStatus} + unreadAnswers={unseenAlerts} + onGoToAnswer={handleGoToAnswer} sessionId={sessionId} onGitHubPulled={() => queryClient.invalidateQueries({ queryKey: ['threads'] })} /> @@ -691,14 +699,6 @@ export function DiffPage() { /> )}
- - + {/* Positioned against the diff rather than the row, which also holds the file list. */} +
+ {orderedDiff ? ( ) : null} +
{showHelp && setShowHelp(false)} />}
diff --git a/packages/ui/src/components/icons/bell-icon.tsx b/packages/ui/src/components/icons/bell-icon.tsx new file mode 100644 index 0000000..8df637d --- /dev/null +++ b/packages/ui/src/components/icons/bell-icon.tsx @@ -0,0 +1,10 @@ +import type { SVGProps } from 'react'; + +export function BellIcon(props: SVGProps) { + return ( + + + + + ); +} diff --git a/packages/ui/src/components/layout/notification-bell.tsx b/packages/ui/src/components/layout/notification-bell.tsx new file mode 100644 index 0000000..9d54582 --- /dev/null +++ b/packages/ui/src/components/layout/notification-bell.tsx @@ -0,0 +1,73 @@ +import { useEffect, useRef, useState } from 'react'; +import type { AnswerAlert } from '../../lib/answer-alerts'; +import { BellIcon } from '../icons/bell-icon'; + +interface NotificationBellProps { + alerts: AnswerAlert[]; + onGo: (threadId: string) => void; +} + +export function NotificationBell(props: NotificationBellProps) { + const { alerts, onGo } = props; + const [open, setOpen] = useState(false); + const rootRef = useRef(null); + + useEffect(() => { + if (!open) { + return; + } + const closeOnOutside = (event: MouseEvent) => { + if (!rootRef.current?.contains(event.target as Node)) { + setOpen(false); + } + }; + document.addEventListener('mousedown', closeOnOutside); + return () => document.removeEventListener('mousedown', closeOnOutside); + }, [open]); + + const label = alerts.length === 0 + ? 'No unread answers' + : `${alerts.length} unread answer${alerts.length === 1 ? '' : 's'}`; + + return ( +
+ + {open && ( +
+ {alerts.map(alert => ( + + ))} +
+ )} +
+ ); +} diff --git a/packages/ui/src/components/layout/toolbar.tsx b/packages/ui/src/components/layout/toolbar.tsx index fe08396..1ed0341 100644 --- a/packages/ui/src/components/layout/toolbar.tsx +++ b/packages/ui/src/components/layout/toolbar.tsx @@ -13,6 +13,8 @@ import { DiffStats } from '../diff/diff-stats'; import { GitHubDialog } from './github-dialog'; import { CommentToolbarActions } from '../comments/comment-toolbar-actions'; import { LiveIndicator } from './live-indicator'; +import { NotificationBell } from './notification-bell'; +import type { AnswerAlert } from '../../lib/answer-alerts'; import { OptionsMenu, menuItemClass } from './options-menu'; import { GENERAL_THREAD_FILE_PATH } from '../comments/types'; import type { ViewMode } from '../../lib/diff-utils'; @@ -22,6 +24,8 @@ import { isThreadResolved } from '../comments/types'; interface ToolbarProps { reviewInProgress?: boolean; live?: { enabled: boolean; listening: boolean; working: boolean; waiting: number }; + unreadAnswers?: AnswerAlert[]; + onGoToAnswer?: (threadId: string) => void; viewMode: ViewMode; onViewModeChange: (mode: ViewMode) => void; hideWhitespace: boolean; @@ -138,6 +142,8 @@ export function Toolbar(props: ToolbarProps) { githubDetails, reviewInProgress, live, + unreadAnswers, + onGoToAnswer, sessionId, onGitHubPulled, } = props; @@ -182,6 +188,9 @@ export function Toolbar(props: ToolbarProps) { )}
+ {onGoToAnswer && ( + + )} {live && ( void; -} - -/** What a note leaves behind once its time is up. Goes to the oldest first. */ -export function UnseenAnswers(props: UnseenAnswersProps) { - const { alerts, onGo } = props; - - if (alerts.length === 0) { - return null; - } - - const oldest = alerts[0]; - - return ( - - ); -} diff --git a/packages/ui/tests/notification-bell.test.tsx b/packages/ui/tests/notification-bell.test.tsx new file mode 100644 index 0000000..09edd7b --- /dev/null +++ b/packages/ui/tests/notification-bell.test.tsx @@ -0,0 +1,61 @@ +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { render, cleanup, screen, fireEvent } from '@testing-library/react'; +import { NotificationBell } from '../src/components/layout/notification-bell'; + +afterEach(cleanup); + +const two = [ + { threadId: 't1', filePath: 'packages/cli/src/live.ts', authorName: 'Agent', preview: 'first answer' }, + { threadId: 't2', filePath: 'packages/ui/src/api.ts', authorName: 'Agent', preview: 'second answer' }, +]; + +describe('the notification bell', () => { + it('is quiet with nothing unread', () => { + render(); + + expect(screen.queryByText('2')).toBeNull(); + expect(screen.getByRole('button', { name: /no unread/i })).toBeTruthy(); + }); + + it('counts what is unread', () => { + render(); + + expect(screen.getByText('2')).toBeTruthy(); + }); + + it('keeps the list closed until asked', () => { + render(); + + expect(screen.queryByText(/first answer/)).toBeNull(); + }); + + it('lists them oldest first, with where each came from', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: /2 unread/i })); + + const items = screen.getAllByRole('menuitem'); + expect(items).toHaveLength(2); + expect(items[0].textContent).toContain('first answer'); + expect(items[0].textContent).toContain('live.ts'); + }); + + it('goes to the one that is clicked', () => { + const onGo = vi.fn(); + render(); + + fireEvent.click(screen.getByRole('button', { name: /2 unread/i })); + fireEvent.click(screen.getAllByRole('menuitem')[1]); + + expect(onGo).toHaveBeenCalledWith('t2'); + }); + + it('closes once you have gone somewhere', () => { + render(); + + fireEvent.click(screen.getByRole('button', { name: /2 unread/i })); + fireEvent.click(screen.getAllByRole('menuitem')[0]); + + expect(screen.queryByRole('menuitem')).toBeNull(); + }); +}); diff --git a/packages/ui/tests/unseen-answers.test.tsx b/packages/ui/tests/unseen-answers.test.tsx deleted file mode 100644 index 7ec1d1f..0000000 --- a/packages/ui/tests/unseen-answers.test.tsx +++ /dev/null @@ -1,46 +0,0 @@ -import { describe, it, expect, afterEach, vi } from 'vitest'; -import { render, cleanup, screen } from '@testing-library/react'; -import { UnseenAnswers } from '../src/components/layout/unseen-answers'; - -afterEach(cleanup); - -const two = [ - { threadId: 't1', filePath: 'a.ts', authorName: 'Agent', preview: 'first' }, - { threadId: 't2', filePath: 'b.ts', authorName: 'Agent', preview: 'second' }, -]; - -describe('what is left after a note has gone', () => { - it('is nothing at all when nothing is unseen', () => { - render(); - - expect(screen.queryByRole('button')).toBeNull(); - }); - - it('counts what is waiting', () => { - render(); - - expect(screen.getByText('2')).toBeTruthy(); - }); - - // The oldest, so following it repeatedly walks the reader through them in the order they arrived. - it('goes to the one that has been waiting longest', () => { - const onGo = vi.fn(); - render(); - - screen.getByRole('button').click(); - - expect(onGo).toHaveBeenCalledWith('t1'); - }); - - it('says what it will do', () => { - render(); - - expect(screen.getByRole('button').getAttribute('title')).toMatch(/2 answers/i); - }); - - it('counts one in the singular', () => { - render(); - - expect(screen.getByRole('button').getAttribute('title')).toMatch(/1 answer\b/i); - }); -}); From 03fd0b3a53fd7d167a9e68f093871c12f9720168 Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Tue, 25 Aug 2026 09:44:27 +0200 Subject: [PATCH 10/12] docs(live): re-arm before answering, not after MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The reader is looking at the page having just asked, and between taking their request and answering it nobody is parked — so it said "No agent" at the moment they were most likely watching. Re-arming first closes that window, and a second request arriving while the answer is written is queued rather than lost. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- packages/skills/diffity-live/SKILL.md | 25 ++++++++++++++++++------- skills/diffity-live/SKILL.md | 25 ++++++++++++++++++------- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/packages/skills/diffity-live/SKILL.md b/packages/skills/diffity-live/SKILL.md index df4d26c..f884b71 100644 --- a/packages/skills/diffity-live/SKILL.md +++ b/packages/skills/diffity-live/SKILL.md @@ -31,6 +31,21 @@ never did is guessing. `findingBody` is usually the thing being asked about. Read it before the question. +## Re-arm first + +Before answering, put the loop back: + +``` +{{binary}} agent await --timeout 240 +``` + +Background command, as always. The reader is looking at the page having just asked, and between +taking their request and answering it there is nobody parked — so the page said "No agent" at exactly +the moment they were watching. Re-arming first closes that window, and a second request arriving +while you write is queued rather than lost. + +Nothing about answering changes; it just happens second. + ## The reader already chose The request carries an `intent`, because there are two buttons in the page and they mean different @@ -91,14 +106,10 @@ somebody else's pull request, however the conversation goes: reviewing is not ed asking a follow-up has not asked you to rewrite their branch. Answer and amend instead, and say that is what you did. -## Then go back to waiting - -``` -{{binary}} agent await --timeout 240 -``` +## When the wait ends on its own -Background command, same as before. Exit 3 means nothing was asked — re-arm without saying anything. -Anything else means the server is gone; say so once and stop, rather than looping on a dead port. +Exit 3 means nothing was asked — re-arm without saying anything. Anything else means the server is +gone; say so once and stop, rather than looping on a dead port. ## Keep it short diff --git a/skills/diffity-live/SKILL.md b/skills/diffity-live/SKILL.md index cb89182..13a2bd4 100644 --- a/skills/diffity-live/SKILL.md +++ b/skills/diffity-live/SKILL.md @@ -31,6 +31,21 @@ never did is guessing. `findingBody` is usually the thing being asked about. Read it before the question. +## Re-arm first + +Before answering, put the loop back: + +``` +diffity agent await --timeout 240 +``` + +Background command, as always. The reader is looking at the page having just asked, and between +taking their request and answering it there is nobody parked — so the page said "No agent" at exactly +the moment they were watching. Re-arming first closes that window, and a second request arriving +while you write is queued rather than lost. + +Nothing about answering changes; it just happens second. + ## The reader already chose The request carries an `intent`, because there are two buttons in the page and they mean different @@ -91,14 +106,10 @@ somebody else's pull request, however the conversation goes: reviewing is not ed asking a follow-up has not asked you to rewrite their branch. Answer and amend instead, and say that is what you did. -## Then go back to waiting - -``` -diffity agent await --timeout 240 -``` +## When the wait ends on its own -Background command, same as before. Exit 3 means nothing was asked — re-arm without saying anything. -Anything else means the server is gone; say so once and stop, rather than looping on a dead port. +Exit 3 means nothing was asked — re-arm without saying anything. Anything else means the server is +gone; say so once and stop, rather than looping on a dead port. ## Keep it short From 78df62c90e01897f8c44f01395ee1d76a2445fff Mon Sep 17 00:00:00 2001 From: "Fredrik Liljegren (Claude Code Claude Opus 5)" Date: Tue, 25 Aug 2026 09:55:37 +0200 Subject: [PATCH 11/12] fix(live): the note stops just short of the middle gutter Right-aligned against the midpoint in split view, so it covers the right of the old side and never the new code being read. Unified keeps to the left, having no midpoint to sit against. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018PkYQzbsnMihHesafWvXKs --- packages/ui/src/components/diff/diff-page.tsx | 1 + .../src/components/layout/answer-bubble.tsx | 8 +++++-- packages/ui/tests/answer-bubble.test.tsx | 24 ++++++++++++------- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/ui/src/components/diff/diff-page.tsx b/packages/ui/src/components/diff/diff-page.tsx index 27c1db3..eb74e35 100644 --- a/packages/ui/src/components/diff/diff-page.tsx +++ b/packages/ui/src/components/diff/diff-page.tsx @@ -721,6 +721,7 @@ export function DiffPage() { void; /** Its time ran out. The count that replaces it is the caller's business. */ onExpire: () => void; @@ -22,7 +24,7 @@ interface AnswerBubbleProps { * on its own, with a bar running down so that is not a surprise. */ export function AnswerBubble(props: AnswerBubbleProps) { - const { alerts, position, onGo, onExpire, onDismiss } = props; + const { alerts, position, viewMode, onGo, onExpire, onDismiss } = props; const [remaining, setRemaining] = useState(1); const newest = alerts[alerts.length - 1]; const expiredRef = useRef(false); @@ -58,7 +60,9 @@ export function AnswerBubble(props: AnswerBubbleProps) { return (
{open && ( -
+
{alerts.map(alert => (