diff --git a/PRIVACY.md b/PRIVACY.md index 1d91c327c6..9d70bf2aef 100644 --- a/PRIVACY.md +++ b/PRIVACY.md @@ -464,7 +464,10 @@ sent to GitHub by our service as you, on the token of your own GitHub sign-in, whoever started the agent, so it can do only what your GitHub account may; it is never sent as the Luke GitHub App, and where you hold no GitHub sign-in GitHub takes, nothing is sent and the tab asks you to sign in with GitHub -again. Wherever Luke names someone on +again. Files changed's Viewed boxes are GitHub's own, which GitHub keeps for +each account: our service reads which files you marked viewed, and marks or +unmarks one when you press its box, on the same token of your own GitHub +sign-in and never as the Luke GitHub App, and stores none of it. Wherever Luke names someone on GitHub, as the tab does who opened, reviewed, and commented on the pull request, it draws them by their GitHub avatar, which your Mac loads straight from GitHub's avatar host, as github.com does, so GitHub sees your Mac's IP diff --git a/apps/desktop/src/renderer/styles/tasks.css b/apps/desktop/src/renderer/styles/tasks.css index d788e744c2..cd0ac00223 100644 --- a/apps/desktop/src/renderer/styles/tasks.css +++ b/apps/desktop/src/renderer/styles/tasks.css @@ -1526,6 +1526,17 @@ text-overflow: ellipsis; } +/* A file's Viewed box, GitHub's own (viewed-files.ts), at the end of its + header's row. */ +.diff-list-viewed { + display: flex; + flex: none; + align-items: center; + margin-left: auto; + color: var(--luke-color-text-muted); + font-size: var(--luke-type-caption-size); +} + /* Why a file has no hunks to draw, in their place under its header in the list (diff-view/index.ts draws it across the file's width). */ .diff-list-notice { diff --git a/apps/desktop/src/renderer/tasks/agent-changes-tab.test.ts b/apps/desktop/src/renderer/tasks/agent-changes-tab.test.ts index 8525ddf4e6..1c18fc63b1 100644 --- a/apps/desktop/src/renderer/tasks/agent-changes-tab.test.ts +++ b/apps/desktop/src/renderer/tasks/agent-changes-tab.test.ts @@ -1,6 +1,10 @@ // @vitest-environment jsdom import assert from "node:assert/strict"; +import { + CODING_AGENT_CALL_FAILURE, + type CodingAgentPullRequestViewedAnswerView, +} from "@luke/features/coding-agent-view"; import { CHANGED_FILE_STATUS, CODING_AGENT_STATUS, @@ -9,6 +13,13 @@ import { DIFF_SIDE, PATCH_OMITTED, } from "@luke/features/coding-agent-wire"; +import { + FILE_VIEWED_STATE, + PULL_REQUEST_ACTION, + PULL_REQUEST_WRITE_OUTCOME, + type PullRequestActionRequest, + type PullRequestWriteAnswer, +} from "@luke/features/pull-request-write-wire"; import { ACT_KIND, ACT_OUTCOME_STATUS, type Act, type ActOutcome } from "@luke/host/acts"; import { appBridge } from "@luke/panel/app-bridge"; import { DIFF_LAYOUT } from "@luke/settings/diff-view"; @@ -97,10 +108,24 @@ let commits: ImageCommits = {}; /** The pull request's files, in the order GitHub lists them, which is not the tree's. */ let files: CodingAgentChangedFile[] = [changed("src/seats.ts")]; +/** How the service answers the read of the member's viewed marks; unanswered unless a test answers it. */ +let viewedAnswer: CodingAgentPullRequestViewedAnswerView = { + failure: CODING_AGENT_CALL_FAILURE.UNANSWERED, +}; +/** How many times the marks were read. */ +let viewedReads = 0; +/** What the next read of the marks waits on, where a test holds it; none answers at once. */ +let viewedHeld: Promise | undefined; + const control = codingAgentsControl({ agents: [AGENT], agentIds: [AGENT.id], models: [], + readPullRequestViewed: () => { + viewedReads += 1; + const answer = viewedAnswer; + return (viewedHeld ?? Promise.resolve()).then(() => answer); + }, readChanges: () => Promise.resolve({ pullRequest: { @@ -172,6 +197,9 @@ beforeEach(() => { treeOpens = []; files = [changed("src/seats.ts")]; commits = {}; + viewedAnswer = { failure: CODING_AGENT_CALL_FAILURE.UNANSWERED }; + viewedReads = 0; + viewedHeld = undefined; boxWidth = 900; vi.spyOn(HTMLElement.prototype, "getBoundingClientRect").mockImplementation(function ( this: HTMLElement, @@ -967,3 +995,304 @@ test("after Refresh changes an image far from the view reads nothing until it co `https://raw.githubusercontent.com/acme/relay/${"c".repeat(40)}/logo.png`, ); }); + +/** The viewed marks the service names: the seats file viewed, the plan viewed and changed since, the readme not. */ +const MARKED: CodingAgentPullRequestViewedAnswerView = { + outcome: PULL_REQUEST_WRITE_OUTCOME.DONE, + files: [ + { path: "a.ts", state: FILE_VIEWED_STATE.VIEWED }, + { path: "b.ts", state: FILE_VIEWED_STATE.DISMISSED }, + { path: "c.ts", state: FILE_VIEWED_STATE.UNVIEWED }, + ], +}; + +/** Every mark asked of the service, answered as `answer` says. */ +function answerMarks(answer: () => PullRequestWriteAnswer): PullRequestActionRequest[] { + const asked: PullRequestActionRequest[] = []; + fakeAppBridge({ + act: (request: Act) => { + if (request.kind !== ACT_KIND.CODING_AGENTS_PULL_REQUEST_ACTION) { + return Promise.resolve({ status: ACT_OUTCOME_STATUS.REFUSED, reason: "Not in this test." }); + } + asked.push(request.payload.request); + return Promise.resolve({ status: ACT_OUTCOME_STATUS.DONE, value: answer() }); + }, + }); + handSettings(); + return asked; +} + +/** The Viewed box in a file's header. */ +function viewedBox(path: string): HTMLInputElement { + return ( + header(path).querySelector('.diff-list-viewed input[type="checkbox"]') ?? + assert.fail(`${path} offers a Viewed box`) + ); +} + +test("the member's viewed marks tick their files, count toward the foot's progress, and fold them; a file changed since reads not viewed", async () => { + viewedAnswer = MARKED; + files = [changed("a.ts"), changed("b.ts"), changed("c.ts")]; + await drawTab(); + + assert.deepEqual( + ["a.ts", "b.ts", "c.ts"].map((path) => viewedBox(path).checked), + [true, false, false], + ); + assert.deepEqual(folds(), [ + ["a.ts", true], + ["b.ts", false], + ["c.ts", false], + ]); + assert.match(document.querySelector(".file-tree-foot")?.textContent ?? "", /1 \/ 3 viewed/u); +}); + +test("ticking a file's box marks it viewed on GitHub at once and folds it; one GitHub refuses goes back and says why", async () => { + viewedAnswer = MARKED; + let refuse = false; + const asked = answerMarks(() => + refuse + ? { outcome: PULL_REQUEST_WRITE_OUTCOME.REFUSED, words: "Pull request is locked." } + : { outcome: PULL_REQUEST_WRITE_OUTCOME.DONE }, + ); + files = [changed("a.ts"), changed("b.ts"), changed("c.ts")]; + await drawTab(); + + act(() => viewedBox("c.ts").click()); + await flush(); + assert.equal(viewedBox("c.ts").checked, true); + assert.deepEqual(folds()[2], ["c.ts", true]); + assert.match(document.querySelector(".file-tree-foot")?.textContent ?? "", /2 \/ 3 viewed/u); + + refuse = true; + act(() => viewedBox("a.ts").click()); + await flush(); + assert.equal(viewedBox("a.ts").checked, true, "a refused unmark leaves the mark standing"); + assert.match( + document.querySelector('[role="alert"]')?.textContent ?? "", + /Pull request is locked\./u, + ); + assert.deepEqual(asked, [ + { action: PULL_REQUEST_ACTION.MARK_FILE_VIEWED, path: "c.ts" }, + { action: PULL_REQUEST_ACTION.UNMARK_FILE_VIEWED, path: "a.ts" }, + ]); +}); + +test("V marks the file the keys stand on viewed and steps on to the next, and is no key at all until the marks are read", async () => { + files = [changed("a.ts"), changed("b.ts"), changed("c.ts")]; + const asked = answerMarks(() => ({ outcome: PULL_REQUEST_WRITE_OUTCOME.DONE })); + await drawTab(); + assert.equal(press(listBox(), { key: "v" }), false, "with no marks read V is not the list's"); + + viewedAnswer = { outcome: PULL_REQUEST_WRITE_OUTCOME.DONE, files: [] }; + await drawTab(); + press(listBox(), { key: "j" }); + press(listBox(), { key: "v" }); + await flush(); + + assert.deepEqual(asked, [{ action: PULL_REQUEST_ACTION.MARK_FILE_VIEWED, path: "a.ts" }]); + assert.ok(header("b.ts").hasAttribute("data-standing")); + assert.equal(viewedBox("a.ts").checked, true); +}); + +test("a member with no GitHub sign-in is offered one, and no Viewed box stands until the marks are read", async () => { + viewedAnswer = { outcome: PULL_REQUEST_WRITE_OUTCOME.SIGN_IN_REQUIRED }; + files = [changed("a.ts")]; + await drawTab(); + + assert.equal(header("a.ts").querySelector(".diff-list-viewed") === null, true); + assert.match(document.querySelector('[role="alert"]')?.textContent ?? "", /Sign in with GitHub/u); +}); + +test("the marks are read again with the files on Refresh changes, so the two describe the same push", async () => { + viewedAnswer = MARKED; + files = [changed("a.ts")]; + await drawTab(); + assert.equal(viewedReads, 1); + + const options = document.querySelector('button[aria-label="Changes actions"]'); + act(() => options?.click()); + const refresh = [...document.querySelectorAll('[role="menuitem"]')].find( + (each) => each.textContent?.includes("Refresh changes"), + ); + act(() => refresh?.click()); + await flush(); + + assert.equal(viewedReads, 2); +}); + +/** Every mark asked of the service, each held until the test answers it. */ +function holdMarks(): Array<{ + readonly request: PullRequestActionRequest; + readonly answer: (answer: PullRequestWriteAnswer) => void; +}> { + const held: Array<{ + readonly request: PullRequestActionRequest; + readonly answer: (answer: PullRequestWriteAnswer) => void; + }> = []; + fakeAppBridge({ + act: (request: Act) => { + if (request.kind !== ACT_KIND.CODING_AGENTS_PULL_REQUEST_ACTION) { + return Promise.resolve({ status: ACT_OUTCOME_STATUS.REFUSED, reason: "Not in this test." }); + } + const answered = Promise.withResolvers(); + held.push({ + request: request.payload.request, + answer: (value) => answered.resolve({ status: ACT_OUTCOME_STATUS.DONE, value }), + }); + return answered.promise; + }, + }); + handSettings(); + return held; +} + +const DONE: PullRequestWriteAnswer = { outcome: PULL_REQUEST_WRITE_OUTCOME.DONE }; + +test("presses on one file go to GitHub one at a time, the last of them winning, whatever order GitHub could have answered them in", async () => { + viewedAnswer = MARKED; + const held = holdMarks(); + files = [changed("c.ts")]; + await drawTab(); + + act(() => viewedBox("c.ts").click()); + act(() => viewedBox("c.ts").click()); + await flush(); + assert.equal(held.length, 1, "the second press waits for the first"); + assert.equal(viewedBox("c.ts").checked, false); + + held[0]?.answer(DONE); + await flush(); + assert.deepEqual( + held.map((each) => each.request), + [ + { action: PULL_REQUEST_ACTION.MARK_FILE_VIEWED, path: "c.ts" }, + { action: PULL_REQUEST_ACTION.UNMARK_FILE_VIEWED, path: "c.ts" }, + ], + ); + held[1]?.answer(DONE); + await flush(); + assert.equal(viewedBox("c.ts").checked, false); + assert.match(document.querySelector(".file-tree-foot")?.textContent ?? "", /0 \/ 1 viewed/u); +}); + +test("a read asked before a mark settled never undoes it, and a read that does not answer takes the boxes away", async () => { + viewedAnswer = MARKED; + const held = holdMarks(); + files = [changed("c.ts")]; + await drawTab(); + + // A refresh's read stays out while the member marks the file and GitHub takes it. + const reading = Promise.withResolvers(); + viewedHeld = reading.promise; + const options = document.querySelector('button[aria-label="Changes actions"]'); + act(() => options?.click()); + const refresh = [...document.querySelectorAll('[role="menuitem"]')].find( + (each) => each.textContent?.includes("Refresh changes"), + ); + act(() => refresh?.click()); + await flush(); + act(() => viewedBox("c.ts").click()); + await flush(); + held[0]?.answer(DONE); + await flush(); + reading.resolve(); + await flush(); + assert.equal(viewedBox("c.ts").checked, true, "the read from before the mark leaves it standing"); + + viewedHeld = undefined; + viewedAnswer = { outcome: PULL_REQUEST_WRITE_OUTCOME.SIGN_IN_REQUIRED }; + act(() => options?.click()); + const again = [...document.querySelectorAll('[role="menuitem"]')].find( + (each) => each.textContent?.includes("Refresh changes"), + ); + act(() => again?.click()); + await flush(); + assert.equal(header("c.ts").querySelector(".diff-list-viewed") === null, true); +}); + +test("unmarking a deleted file opens it, and a press GitHub refuses puts back the fold chosen before", async () => { + viewedAnswer = { + outcome: PULL_REQUEST_WRITE_OUTCOME.DONE, + files: [{ path: "old.ts", state: FILE_VIEWED_STATE.VIEWED }], + }; + let refuse = false; + answerMarks(() => + refuse ? { outcome: PULL_REQUEST_WRITE_OUTCOME.REFUSED, words: "Locked." } : DONE, + ); + files = [changed("old.ts", { status: CHANGED_FILE_STATUS.DELETED }), changed("new.ts")]; + await drawTab(); + assert.deepEqual(folds()[1], ["old.ts", true]); + + act(() => viewedBox("old.ts").click()); + await flush(); + assert.deepEqual(folds()[1], ["old.ts", false], "unmarked, the deleted file opens"); + + refuse = true; + act(() => viewedBox("new.ts").click()); + await flush(); + assert.deepEqual(folds()[0], ["new.ts", false], "a refused mark leaves the file as it was"); +}); + +const REFUSED: PullRequestWriteAnswer = { + outcome: PULL_REQUEST_WRITE_OUTCOME.REFUSED, + words: "Try again later.", +}; + +test("a mark GitHub took stands where the press queued after it is refused, as GitHub holds it", async () => { + viewedAnswer = MARKED; + const held = holdMarks(); + files = [changed("c.ts")]; + await drawTab(); + + act(() => viewedBox("c.ts").click()); + act(() => viewedBox("c.ts").click()); + held[0]?.answer(DONE); + await flush(); + held[1]?.answer(REFUSED); + await flush(); + + assert.equal(viewedBox("c.ts").checked, true); + assert.match(document.querySelector('[role="alert"]')?.textContent ?? "", /Try again later\./u); +}); + +test("a last press GitHub was never asked for is sent even where the press before it was refused", async () => { + viewedAnswer = MARKED; + const held = holdMarks(); + files = [changed("c.ts")]; + await drawTab(); + + act(() => viewedBox("c.ts").click()); + act(() => viewedBox("c.ts").click()); + act(() => viewedBox("c.ts").click()); + held[0]?.answer(REFUSED); + await flush(); + // The last press, a mark GitHub was never asked for, is taken where GitHub answers it. + held.at(-1)?.answer(DONE); + await flush(); + + assert.equal(viewedBox("c.ts").checked, true); + assert.equal(document.querySelector('[role="alert"]') === null, true); +}); + +test("a refused mark leaves a fold chosen since, as Collapse all chose it", async () => { + viewedAnswer = { outcome: PULL_REQUEST_WRITE_OUTCOME.DONE, files: [] }; + const held = holdMarks(); + files = [changed("a.ts"), changed("b.ts")]; + await drawTab(); + + act(() => viewedBox("a.ts").click()); + await flush(); + const foldAll = + document.querySelector('button[aria-label="Collapse all files"]') ?? + assert.fail("the bar offers Collapse all"); + act(() => foldAll.click()); + await flush(); + held[0]?.answer(REFUSED); + await flush(); + + assert.deepEqual(folds(), [ + ["a.ts", true], + ["b.ts", true], + ]); +}); diff --git a/apps/desktop/src/renderer/tasks/agent-changes-tab.tsx b/apps/desktop/src/renderer/tasks/agent-changes-tab.tsx index f5e135430f..d625765cc3 100644 --- a/apps/desktop/src/renderer/tasks/agent-changes-tab.tsx +++ b/apps/desktop/src/renderer/tasks/agent-changes-tab.tsx @@ -16,6 +16,7 @@ import { appPlatform } from "@luke/panel/app-platform"; import { DIFF_LAYOUT, INLINE_DIFF } from "@luke/settings/diff-view"; import { Button, + Checkbox, CopyIcon, DownloadIcon, EmptyState, @@ -77,7 +78,9 @@ import { ImageDiff, type ImageSides, imageDrawable } from "./image-diff"; import { LineComment, type PendingReview } from "./line-comment"; import { PullRequestMark } from "./pull-request-mark"; import { OpenedBy, PullRequestPeople, ReviewersLine } from "./pull-request-people"; +import { PullRequestWriteNote } from "./pull-request-writes"; import type { CodingAgentsControl, HeldPeople } from "./use-coding-agents"; +import { useViewedFiles } from "./viewed-files"; /** * agent-changes-tab.tsx -- the Files changed view of an agent's Pull request tab (`agent-pull-request-tab.tsx`): its pull request's files drawn as a File tab draws a file, the chosen file's unified diff under the same bar and beside the same tree. @@ -388,9 +391,14 @@ function entryOf( list.toggleSource(path)} /> - ) + <> + {image === undefined || fileNotice(file) !== undefined ? null : ( + list.toggleSource(path)} /> + )} + {list.viewed === undefined || list.markViewed === undefined ? null : ( + list.markViewed?.(path)} /> + )} + } collapsed={collapsed} standing={list.standing === path} @@ -475,6 +483,23 @@ function useListCommenting( return { open, note: { path, picked, content } }; } +/** A file's Viewed box at the end of its header, GitHub's own, as the member's marks hold it; V does the same for the file the keys stand on. */ +function ViewedBox({ + viewed, + onToggle, +}: { + viewed: boolean; + onToggle: () => void; +}): React.JSX.Element { + return ( + + + {LIST_WORDS.VIEWED} + + + ); +} + /** An SVG's turn between its picture and its source's lines, at the end of its header. */ function SourceToggle({ source, @@ -706,7 +731,13 @@ export function AgentChangesTab({ const paths = files.map((file) => file.path); return { files, paths, marks: changeMarks(files), folders: changedFolders(paths) }; }, [pullRequest]); - const list = useChangesList(changed.files); + // Note that the marks are read with each answer the changes read lands with, so the two describe the same push. + const viewedFiles = useViewedFiles( + agentId, + control.readPullRequestViewed, + pullRequest === null ? undefined : reads.current.count, + ); + const list = useChangesList(changed.files, viewedFiles.marks); const { closeOver } = room; const { go } = list; // A file chosen in the tree floating over the list takes the tree down, to show the file. @@ -758,6 +789,15 @@ export function AgentChangesTab({ <> {count} {count === 1 ? "file" : "files"} changed{" "} + {list.viewed === undefined ? null : ( + + {" · "} + {LIST_WORDS.viewedOf( + pullRequest.files.filter((file) => list.viewed?.(file.path) === true).length, + count, + )} + + )} ); return ( @@ -789,6 +829,7 @@ export function AgentChangesTab({ } tree={room.toggles} /> + {pullRequest.truncated ? (
diff --git a/apps/desktop/src/renderer/tasks/changes-list.tsx b/apps/desktop/src/renderer/tasks/changes-list.tsx index 10991114b4..959661c533 100644 --- a/apps/desktop/src/renderer/tasks/changes-list.tsx +++ b/apps/desktop/src/renderer/tasks/changes-list.tsx @@ -28,19 +28,22 @@ import type { DiffListDoor } from "./diff-list"; * The keys are stage-cli's own, a letter alone each: J and K step to the * next and the previous file, `;` collapses or expands the file stood on, * ⇧; collapses every file the filter keeps, or expands them all once all - * are collapsed, and Escape lets go of the file stood on. They answer only + * are collapsed, V marks the file stood on viewed on GitHub, or unmarks it, + * and steps on to the next, and Escape lets go of the file stood on. They answer only * where the press lands inside Files changed and is not typing, and go no * further once answered, so J and K keep their meaning everywhere else in - * the window (the Inbox's next and previous item, `#shared/shortcuts`), an + * the window (the Inbox's next and previous item, `@luke/settings/shortcuts`), an * Escape with no file stood on reaches the window's layers, and the tab's ⌘F * is never one of them. The file stood on is outlined in the list and named * in the bar; with none stood on, the bar names the file at the top of the * list, and a key starts from it. * - * A file's fold is its default, `startsCollapsed`, turned over for the files - * the developer folded the other way, so a file that starts collapsed and is - * opened stays open, and Collapse all and Expand all turn over exactly the - * files the filter keeps. A file the filter leaves out, chosen where every + * A file's fold is the one the developer chose for it, else its default: + * folded where it `startsCollapsed`, or where the member marked it viewed, + * as stage-cli folds a viewed file. So a file that starts collapsed and is + * opened stays open, Collapse all and Expand all fold exactly the files the + * filter keeps, and marking a file viewed folds it, or unmarking opens it, + * whatever was chosen for it before. A file the filter leaves out, chosen where every * changed file is listed, as the bar's crumbs list them, clears the filter * and is scrolled to once the list holds it. */ @@ -52,6 +55,7 @@ const FILE_KEY = { TOGGLE: "toggle", TOGGLE_ALL: "toggle-all", LET_GO: "let-go", + VIEWED: "viewed", } as const; type FileKey = (typeof FILE_KEY)[keyof typeof FILE_KEY]; @@ -71,6 +75,9 @@ export const LIST_WORDS = { EXPAND_ALL: "Expand all files", SHOW_SOURCE: "Show source", SHOW_IMAGE: "Show image", + VIEWED: "Viewed", + /** How far through the files the member is, as GitHub counts it. */ + viewedOf: (viewed: number, files: number) => `${viewed} / ${files} viewed`, } as const; /** Whether the press landed in something taking typing, which keeps its letters. */ @@ -93,6 +100,7 @@ function fileKeyOf(event: React.KeyboardEvent, standing: boolean): FileKey | und if (event.shiftKey) return undefined; const key = event.key.toLowerCase(); if (key === "j") return FILE_KEY.NEXT; + if (key === "v") return FILE_KEY.VIEWED; return key === "k" ? FILE_KEY.PREVIOUS : undefined; } @@ -110,20 +118,29 @@ function turned(paths: ReadonlySet, path: string): ReadonlySet { return next; } -/** The folds turned over, with `paths` folded as `collapsed` says. */ +/** The folds the developer chose, with `files` folded as `collapsed` says. */ function foldedAs( - overrides: ReadonlySet, + chosen: ReadonlyMap, files: readonly CodingAgentChangedFile[], collapsed: boolean, -): ReadonlySet { - const next = new Set(overrides); - for (const file of files) { - if (startsCollapsed(file) === collapsed) next.delete(file.path); - else next.add(file.path); - } +): ReadonlyMap { + const next = new Map(chosen); + for (const file of files) next.set(file.path, collapsed); return next; } +/** The fold a path is drawn with: the one the developer chose, else its default. */ +function chosenOr(chosen: ReadonlyMap, path: string, otherwise: boolean): boolean { + return chosen.get(path) ?? otherwise; +} + +/** Which of the files the member marked viewed on GitHub, and the door that marks or unmarks one (`viewed-files.ts`). */ +export interface ViewedMarks { + readonly viewed: (path: string) => boolean; + /** Marks or unmarks a file, answering once GitHub settled it whether the member's last press for it stands. */ + readonly toggle: (path: string) => Promise; +} + /** The list's state and doors: the files kept, each fold, the file stood on and the one at the top, and what the keys and controls do. */ export interface ChangesList { readonly query: string; @@ -147,12 +164,18 @@ export interface ChangesList { /** Whether an image that is text too, an SVG, is drawn as its source's lines rather than as a picture, and the door that turns it. */ readonly asSource: (path: string) => boolean; readonly toggleSource: (path: string) => void; + /** Whether a file is marked viewed, and the door that marks or unmarks it, which folds or opens it; none while the marks are not read. */ + readonly viewed: ((path: string) => boolean) | undefined; + readonly markViewed: ((path: string) => void) | undefined; } /** Files changed's list over `files`, every fold at its default and no file stood on until the developer acts. */ -export function useChangesList(files: readonly CodingAgentChangedFile[]): ChangesList { +export function useChangesList( + files: readonly CodingAgentChangedFile[], + marks?: ViewedMarks | undefined, +): ChangesList { const [query, onQuery] = useState(""); - const [overrides, setOverrides] = useState>(new Set()); + const [chosen, setChosen] = useState>(new Map()); const [standing, setStanding] = useState(undefined); const [top, setTop] = useState(undefined); const [sources, setSources] = useState>(new Set()); @@ -160,23 +183,62 @@ export function useChangesList(files: readonly CodingAgentChangedFile[]): Change const ordered = useMemo(() => treeOrder(files), [files]); const kept = useMemo(() => filterFiles(ordered, query), [ordered, query]); const paths = useMemo(() => kept.map((file) => file.path), [kept]); + const viewed = marks?.viewed; const folds = useMemo( () => - new Map(files.map((file) => [file.path, startsCollapsed(file) !== overrides.has(file.path)])), - [files, overrides], + new Map( + files.map((file) => [ + file.path, + chosenOr(chosen, file.path, startsCollapsed(file) || viewed?.(file.path) === true), + ]), + ), + [files, chosen, viewed], ); const collapsed = useCallback((path: string) => folds.get(path) === true, [folds]); const allCollapsed = kept.length > 0 && kept.every((file) => collapsed(file.path)); const keptStanding = standing !== undefined && paths.includes(standing) ? standing : undefined; const current = keptStanding ?? (top !== undefined && paths.includes(top) ? top : paths[0]); - const toggle = useCallback((path: string) => setOverrides((was) => turned(was, path)), []); + // How many times each file's fold was chosen, so a press undone puts back only a fold nothing chose since. + const foldChoices = useRef(new Map()); + const choose = useCallback((paths: readonly string[]) => { + for (const path of paths) + foldChoices.current.set(path, (foldChoices.current.get(path) ?? 0) + 1); + }, []); + const toggle = useCallback( + (path: string) => { + choose([path]); + setChosen((was) => new Map(was).set(path, folds.get(path) !== true)); + }, + [folds, choose], + ); + // A file marked viewed folds, and one unmarked opens, whatever was chosen for it before, which comes back where GitHub would not take the press. + const markViewed = useCallback( + (path: string) => { + if (marks === undefined) return; + const before = chosen.get(path); + const folding = !marks.viewed(path); + choose([path]); + const choice = foldChoices.current.get(path); + setChosen((was) => new Map(was).set(path, folding)); + void marks.toggle(path).then((stood) => { + if (stood || foldChoices.current.get(path) !== choice) return; + setChosen((was) => { + const next = new Map(was); + if (before === undefined) next.delete(path); + else next.set(path, before); + return next; + }); + }); + }, + [marks, chosen, choose], + ); const asSource = useCallback((path: string) => sources.has(path), [sources]); const toggleSource = useCallback((path: string) => setSources((was) => turned(was, path)), []); - const toggleAll = useCallback( - () => setOverrides((was) => foldedAs(was, kept, !allCollapsed)), - [kept, allCollapsed], - ); + const toggleAll = useCallback(() => { + choose(kept.map((file) => file.path)); + setChosen((was) => foldedAs(was, kept, !allCollapsed)); + }, [kept, allCollapsed, choose]); // A file the filter leaves out is chosen in the tree's crumbs, which list every changed file: the filter is cleared, and the list scrolled to it once it holds it again. const [revealing, setRevealing] = useState(undefined); const go = useCallback( @@ -197,11 +259,19 @@ export function useChangesList(files: readonly CodingAgentChangedFile[]): Change const onKeyDown = (event: React.KeyboardEvent) => { const key = fileKeyOf(event, keptStanding !== undefined); if (key === undefined || current === undefined) return; + // V is the list's only while the marks are read, so there is something to mark. + if (key === FILE_KEY.VIEWED && marks === undefined) return; // Note that an answered press goes no further, because the window's keymap would run the Inbox's J and K too. event.preventDefault(); event.stopPropagation(); if (key === FILE_KEY.LET_GO) return setStanding(undefined); if (key === FILE_KEY.TOGGLE_ALL) return toggleAll(); + if (key === FILE_KEY.VIEWED) { + // The file marked folds, and the keys step on to the next, so pressing V again marks it. + markViewed(current); + const next = keptStanding === undefined ? current : stepped(paths, current, 1); + return go(next ?? current); + } if (key === FILE_KEY.TOGGLE) { toggle(current); return go(current); @@ -229,6 +299,8 @@ export function useChangesList(files: readonly CodingAgentChangedFile[]): Change door, asSource, toggleSource, + viewed: marks?.viewed, + markViewed: marks === undefined ? undefined : markViewed, }; } diff --git a/apps/desktop/src/renderer/tasks/pull-request-writes.tsx b/apps/desktop/src/renderer/tasks/pull-request-writes.tsx index 4393141157..e6314e4ec9 100644 --- a/apps/desktop/src/renderer/tasks/pull-request-writes.tsx +++ b/apps/desktop/src/renderer/tasks/pull-request-writes.tsx @@ -1,6 +1,7 @@ import { CODING_AGENT_CALL_FAILURE, type CodingAgentCallFailure, + type CodingAgentPullRequestViewedAnswerView, type CodingAgentPullRequestWriteAnswerView, } from "@luke/features/coding-agent-view"; import { @@ -78,6 +79,8 @@ const ACTION_WORDS = { [PULL_REQUEST_ACTION.REPLY_TO_THREAD]: "Reply", [PULL_REQUEST_ACTION.RESOLVE_THREAD]: "Resolve conversation", [PULL_REQUEST_ACTION.UNRESOLVE_THREAD]: "Unresolve conversation", + [PULL_REQUEST_ACTION.MARK_FILE_VIEWED]: "Mark as viewed", + [PULL_REQUEST_ACTION.UNMARK_FILE_VIEWED]: "Unmark as viewed", [PULL_REQUEST_ACTION.MERGE]: "Merge", [PULL_REQUEST_ACTION.ENABLE_AUTO_MERGE]: "Enable auto-merge", [PULL_REQUEST_ACTION.DISABLE_AUTO_MERGE]: "Disable auto-merge", @@ -130,7 +133,7 @@ export const WRITE_NOTE = { WORDS: "words", } as const; -type WriteNote = +export type WriteNote = | { readonly kind: typeof WRITE_NOTE.SIGN_IN } | { readonly kind: typeof WRITE_NOTE.WORDS; readonly words: string }; @@ -164,7 +167,9 @@ export interface PullRequestWrites { } /** What an answer leaves to say: nothing where GitHub made the change. */ -function noteOf(answer: CodingAgentPullRequestWriteAnswerView): WriteNote | undefined { +export function noteOf( + answer: CodingAgentPullRequestWriteAnswerView | CodingAgentPullRequestViewedAnswerView, +): WriteNote | undefined { if ("failure" in answer) return { kind: WRITE_NOTE.WORDS, words: failureWords(answer.failure) }; switch (answer.outcome) { case PULL_REQUEST_WRITE_OUTCOME.DONE: @@ -313,7 +318,8 @@ export function PullRequestWriteNote({ writes, className = "pull-request-write-note", }: { - writes: PullRequestWrites; + /** What the last change left to say, and the door that lets it go: a tab's changes', or Files changed's viewed marks'. */ + writes: Pick; className?: string; }): React.JSX.Element | null { const { connect } = useGitHubConnection(); diff --git a/apps/desktop/src/renderer/tasks/use-coding-agents.ts b/apps/desktop/src/renderer/tasks/use-coding-agents.ts index 34db6f5a35..faf94fe7a0 100644 --- a/apps/desktop/src/renderer/tasks/use-coding-agents.ts +++ b/apps/desktop/src/renderer/tasks/use-coding-agents.ts @@ -8,6 +8,7 @@ import { type CodingAgentOverviewChecksAnswerView, type CodingAgentOverviewConversationAnswerView, type CodingAgentOverviewPullRequestAnswerView, + type CodingAgentPullRequestViewedAnswerView, type CodingAgentReviewOptionsAnswerView, } from "@luke/features/coding-agent-view"; import { @@ -288,6 +289,8 @@ export interface CodingAgentsControl { readOverviewChecks: (agentId: string) => Promise; /** One read of an agent's pull request's conversation and where its reviewers stand, as its overview draws them. */ readOverviewConversation: (agentId: string) => Promise; + /** One read of which of an agent's pull request's files the member marked viewed on GitHub, on their own GitHub sign-in. */ + readPullRequestViewed: (agentId: string) => Promise; /** The words a dictation into an agent's box is likely to say, read off the open task. */ keywords: readonly string[]; /** The skills of the open plan's repository a `/` in an agent's box offers, read once for the plan (`repository-skills.ts`), or of the agent's own where no plan is open; nothing while neither names a repository. */ @@ -1118,6 +1121,11 @@ export function useCodingAgents(input: { [act], ); + const readPullRequestViewed = useCallback( + (agentId) => act(ACT_KIND.CODING_AGENTS_PULL_REQUEST_VIEWED, { agentId }), + [act], + ); + const readInbox = useCallback( (params) => act(ACT_KIND.CODING_AGENTS_INBOX, params), [act], @@ -1213,6 +1221,7 @@ export function useCodingAgents(input: { readReviewOptions, readOverviewChecks, readOverviewConversation, + readPullRequestViewed, keywords: input.keywords ?? NO_KEYWORDS, readSkills, readInbox, diff --git a/apps/desktop/src/renderer/tasks/viewed-files.ts b/apps/desktop/src/renderer/tasks/viewed-files.ts new file mode 100644 index 0000000000..f843c3663a --- /dev/null +++ b/apps/desktop/src/renderer/tasks/viewed-files.ts @@ -0,0 +1,195 @@ +import { + CODING_AGENT_CALL_FAILURE, + type CodingAgentPullRequestViewedAnswerView, + type CodingAgentPullRequestWriteAnswerView, +} from "@luke/features/coding-agent-view"; +import { + FILE_VIEWED_STATE, + type FileViewedState, + PULL_REQUEST_ACTION, + PULL_REQUEST_WRITE_OUTCOME, +} from "@luke/features/pull-request-write-wire"; +import { ACT_KIND } from "@luke/host/acts"; +import { useAct } from "@luke/panel/act"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; +import type { ViewedMarks } from "./changes-list"; +import { noteOf, type WriteNote } from "./pull-request-writes"; + +/** + * viewed-files.ts -- which of an agent's pull request's files the member marked viewed, as GitHub keeps the mark for them alone, read and set from Files changed. + * + * The marks are GitHub's own, the ones its Viewed box sets, so they are read + * and written by the service on the member's own GitHub sign-in and never on + * the Luke GitHub App (`pull-request-write-wire.ts`): the read + * (`CODING_AGENTS_PULL_REQUEST_VIEWED`) each time the changes are read, so + * the two describe the same push, and a mark or an unmark as one of the + * pull request's changes (`CODING_AGENTS_PULL_REQUEST_ACTION`). A file marked + * viewed and changed since (GitHub's dismissed) reads as not viewed, as + * GitHub unticks it. + * + * A press is drawn at once. Each file's marks go to GitHub one at a time, + * so GitHub settles them in the order pressed: presses made while one is out + * are folded into the last of them, which is sent once the one out is + * answered, taken or refused, wherever it still differs from what GitHub + * holds. Every mark GitHub takes is its word on the file at once, so where + * GitHub would not take the last press, the mark goes back to what GitHub + * holds and the line under the bar says why, as any other change's does + * (`pull-request-writes.tsx`). A read never undoes a mark settled or pressed + * since it was asked; a read that does not answer takes the marks away + * rather than leave ones it can no longer vouch for, until one answers. + */ + +/** The member's viewed marks: the marks, once read, what the last read or press left to say, and the door that lets it go. */ +export interface ViewedFiles { + /** The marks and the door that sets one; none until they are read, or since a read did not answer. */ + readonly marks: ViewedMarks | undefined; + readonly note: WriteNote | undefined; + readonly dismiss: () => void; +} + +/** What a read of the marks leaves to say, as a change's answer would. */ +function readNote(answer: CodingAgentPullRequestViewedAnswerView): WriteNote | undefined { + if (!("failure" in answer) && answer.outcome === PULL_REQUEST_WRITE_OUTCOME.DONE) + return undefined; + return noteOf(answer); +} + +/** Whether a change's answer is GitHub having made it. */ +function took(answer: CodingAgentPullRequestWriteAnswerView): boolean { + return !("failure" in answer) && answer.outcome === PULL_REQUEST_WRITE_OUTCOME.DONE; +} + +/** A file's marks still to settle: the mark its last press asks for, how many presses there were, and the doors told once it settles. */ +interface Pressing { + readonly wanted: boolean; + readonly presses: number; + readonly told: ReadonlyArray<(stood: boolean) => void>; +} + +/** + * The member's marks on the agent's pull request's files, read anew each + * time `reading` counts another answer of the changes read, and marked or + * unmarked a file at a time, several files at once. + */ +export function useViewedFiles( + agentId: string, + read: (agentId: string) => Promise, + /** How many answers the changes read has landed with, a pull request named in the latest; none while none is. */ + reading: number | undefined, +): ViewedFiles { + const { act } = useAct(); + const [states, setStates] = useState | undefined>(); + const [pressed, setPressed] = useState>(new Map()); + const [note, setNote] = useState(undefined); + const latest = useRef({ read }); + latest.current = { read }; + // Note that the presses still to settle, and how far each file's marks have settled, are held in refs, because an answer reads them as they are when it lands. + const pressing = useRef(new Map()); + const settled = useRef({ count: 0, at: new Map() }); + // How many of a file's presses there had been when its mark out now was sent. + const sentAt = useRef(new Map()).current; + + useEffect(() => { + if (reading === undefined) return undefined; + let standing = true; + const asked = settled.current.count; + latest.current + .read(agentId) + .catch( + (): CodingAgentPullRequestViewedAnswerView => ({ + failure: CODING_AGENT_CALL_FAILURE.UNANSWERED, + }), + ) + .then((answer) => { + if (!standing) return; + setNote(readNote(answer)); + if ("failure" in answer || answer.outcome !== PULL_REQUEST_WRITE_OUTCOME.DONE) { + return setStates(undefined); + } + // A file settled or pressed since the read was asked keeps the mark it has now. + const since = (path: string) => + (settled.current.at.get(path) ?? 0) > asked || pressing.current.has(path); + setStates((was) => { + const next = new Map(answer.files.map((file) => [file.path, file.state])); + for (const [path, state] of was ?? []) if (since(path)) next.set(path, state); + return next; + }); + }); + return () => { + standing = false; + }; + }, [agentId, reading]); + + const viewed = useCallback( + (path: string) => pressed.get(path) ?? states?.get(path) === FILE_VIEWED_STATE.VIEWED, + [pressed, states], + ); + + // Sends one mark for the file, GitHub's last word on it being `confirmed`, and once GitHub answers, sends the last press where it still differs from GitHub's word, else settles the file. + const send = useCallback( + (path: string, wanted: boolean, confirmed: boolean) => { + const action = wanted + ? PULL_REQUEST_ACTION.MARK_FILE_VIEWED + : PULL_REQUEST_ACTION.UNMARK_FILE_VIEWED; + sentAt.set(path, pressing.current.get(path)?.presses ?? 0); + void act(ACT_KIND.CODING_AGENTS_PULL_REQUEST_ACTION, { agentId, request: { action, path } }) + .catch( + (): CodingAgentPullRequestWriteAnswerView => ({ + failure: CODING_AGENT_CALL_FAILURE.UNANSWERED, + }), + ) + .then((answer) => { + const now = pressing.current.get(path); + if (now === undefined) return; + const sent = sentAt.get(path) ?? 0; + // Every mark GitHub took is GitHub's word at once, so a later refusal shows what GitHub holds. + const word = took(answer) ? wanted : confirmed; + if (took(answer)) { + const state = wanted ? FILE_VIEWED_STATE.VIEWED : FILE_VIEWED_STATE.UNVIEWED; + setStates((was) => (was === undefined ? was : new Map(was).set(path, state))); + } + // A press made since this one was sent is sent, where GitHub does not yet hold what it asks, whatever became of this one. + if (now.wanted !== word && (now.presses > sent || now.wanted !== wanted)) { + return send(path, now.wanted, word); + } + pressing.current.delete(path); + settled.current.count += 1; + settled.current.at.set(path, settled.current.count); + const stood = now.wanted === word; + if (!stood) setNote(noteOf(answer)); + setPressed((was) => { + const next = new Map(was); + next.delete(path); + return next; + }); + for (const tell of now.told) tell(stood); + }); + }, + [act, agentId], + ); + + const toggle = useCallback( + (path: string) => { + const settles = Promise.withResolvers(); + const wanted = !viewed(path); + const out = pressing.current.get(path); + pressing.current.set(path, { + wanted, + presses: (out?.presses ?? 0) + 1, + told: [...(out?.told ?? []), settles.resolve], + }); + setPressed((was) => new Map(was).set(path, wanted)); + setNote(undefined); + if (out === undefined) send(path, wanted, states?.get(path) === FILE_VIEWED_STATE.VIEWED); + return settles.promise; + }, + [viewed, send, states], + ); + + const dismiss = useCallback(() => setNote(undefined), []); + const marks = useMemo( + () => (states === undefined ? undefined : { viewed, toggle }), + [states, viewed, toggle], + ); + return { marks, note, dismiss }; +} diff --git a/apps/desktop/src/testing/tasks-control.ts b/apps/desktop/src/testing/tasks-control.ts index 7bc9b2f261..4cc9bf10f2 100644 --- a/apps/desktop/src/testing/tasks-control.ts +++ b/apps/desktop/src/testing/tasks-control.ts @@ -133,6 +133,7 @@ export function codingAgentsControl( readOverviewChecks: () => Promise.resolve({ failure: CODING_AGENT_CALL_FAILURE.UNANSWERED }), readOverviewConversation: () => Promise.resolve({ failure: CODING_AGENT_CALL_FAILURE.UNANSWERED }), + readPullRequestViewed: () => Promise.resolve({ failure: CODING_AGENT_CALL_FAILURE.UNANSWERED }), changesPeople: new Map(), readInbox: () => Promise.resolve({ failure: CODING_AGENT_CALL_FAILURE.UNANSWERED }), answerInbox: () => Promise.resolve({ failure: CODING_AGENT_CALL_FAILURE.UNANSWERED }), diff --git a/apps/web/server/api-rewrites.json b/apps/web/server/api-rewrites.json index 33827cb6f8..96c1d15394 100644 --- a/apps/web/server/api-rewrites.json +++ b/apps/web/server/api-rewrites.json @@ -147,6 +147,10 @@ "src": "/api/agents/([^/]+)/pull-request/action", "dest": "/api/default.js?route=agents/pull-request-action&id=$1" }, + { + "src": "/api/agents/([^/]+)/pull-request/viewed", + "dest": "/api/default.js?route=agents/pull-request-viewed&id=$1" + }, { "src": "/api/agents/([^/]+)/changes", "dest": "/api/default.js?route=agents/changes&id=$1" diff --git a/apps/web/server/function-rewrites.ts b/apps/web/server/function-rewrites.ts index de35b652b0..11a71785eb 100644 --- a/apps/web/server/function-rewrites.ts +++ b/apps/web/server/function-rewrites.ts @@ -137,6 +137,11 @@ const SEGMENT_REWRITES: readonly SegmentRewrite[] = [ route: "agents/pull-request-action", query: "id=$1", }, + { + src: "/api/agents/([^/]+)/pull-request/viewed", + route: "agents/pull-request-viewed", + query: "id=$1", + }, { src: "/api/agents/([^/]+)/changes", route: "agents/changes", query: "id=$1" }, { src: "/api/agents/([^/]+)/overview/pull-request", diff --git a/apps/web/server/routes/agents/pull-request-viewed.ts b/apps/web/server/routes/agents/pull-request-viewed.ts new file mode 100644 index 0000000000..93abbaacfc --- /dev/null +++ b/apps/web/server/routes/agents/pull-request-viewed.ts @@ -0,0 +1,9 @@ +import { deploymentEveOrigin } from "@luke/service/agents/brain-host/eve-origin"; +import { codingAgentsApp } from "@luke/service/agents/coding-agents-app"; +import { resolveHostedUserId } from "@luke/service/auth/bearer"; +import { routeFromHttpRouter } from "../../route-effect.js"; + +/** Which of a coding agent's pull request's files the workspace member who asks marked viewed on GitHub (GET), read on their own GitHub sign-in. The logic lives in `packages/service/src/agents/coding-agents-app.ts`; this file only hands it the deployment's real seams. */ +export default routeFromHttpRouter( + codingAgentsApp({ resolveUserId: resolveHostedUserId, eveOrigin: deploymentEveOrigin }), +); diff --git a/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestActionParamsSchema.json b/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestActionParamsSchema.json index c4e0207031..329d6f1463 100644 --- a/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestActionParamsSchema.json +++ b/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestActionParamsSchema.json @@ -523,6 +523,48 @@ "action" ], "additionalProperties": false + }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "mark-file-viewed" + ] + }, + "path": { + "type": "string", + "minLength": 1, + "maxLength": 4096 + } + }, + "required": [ + "action", + "path" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "unmark-file-viewed" + ] + }, + "path": { + "type": "string", + "minLength": 1, + "maxLength": 4096 + } + }, + "required": [ + "action", + "path" + ], + "additionalProperties": false } ] } diff --git a/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestViewedAnswerViewSchema.json b/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestViewedAnswerViewSchema.json new file mode 100644 index 0000000000..68d6bf0d96 --- /dev/null +++ b/packages/features/fixtures/json-schema/coding-agent-view-codingAgentPullRequestViewedAnswerViewSchema.json @@ -0,0 +1,108 @@ +{ + "anyOf": [ + { + "anyOf": [ + { + "type": "object", + "properties": { + "outcome": { + "type": "string", + "enum": [ + "done" + ] + }, + "files": { + "type": "array", + "items": { + "type": "object", + "properties": { + "path": { + "type": "string" + }, + "state": { + "type": "string", + "enum": [ + "viewed", + "unviewed", + "dismissed" + ] + } + }, + "required": [ + "path", + "state" + ], + "additionalProperties": false + }, + "maxItems": 300 + } + }, + "required": [ + "outcome", + "files" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "outcome": { + "type": "string", + "enum": [ + "sign-in-required" + ] + } + }, + "required": [ + "outcome" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "outcome": { + "type": "string", + "enum": [ + "refused" + ] + }, + "words": { + "type": "string", + "maxLength": 1000 + } + }, + "required": [ + "outcome", + "words" + ], + "additionalProperties": false + } + ] + }, + { + "type": "object", + "properties": { + "failure": { + "type": "string", + "enum": [ + "unanswered", + "not-found", + "no-repository", + "repository-not-reachable", + "github-unanswered", + "invalid-choice", + "message-too-long", + "agent-not-ready", + "agent-retired", + "plan-archived" + ] + } + }, + "required": [ + "failure" + ], + "additionalProperties": false + } + ] +} diff --git a/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestActionRequestSchema.json b/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestActionRequestSchema.json index 72ac00502c..5751ea6475 100644 --- a/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestActionRequestSchema.json +++ b/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestActionRequestSchema.json @@ -516,6 +516,48 @@ "action" ], "additionalProperties": false + }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "mark-file-viewed" + ] + }, + "path": { + "type": "string", + "minLength": 1, + "maxLength": 4096 + } + }, + "required": [ + "action", + "path" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "action": { + "type": "string", + "enum": [ + "unmark-file-viewed" + ] + }, + "path": { + "type": "string", + "minLength": 1, + "maxLength": 4096 + } + }, + "required": [ + "action", + "path" + ], + "additionalProperties": false } ] } diff --git a/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestViewedAnswerSchema.json b/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestViewedAnswerSchema.json new file mode 100644 index 0000000000..ee32c81bbd --- /dev/null +++ b/packages/features/fixtures/json-schema/pull-request-write-wire-pullRequestViewedAnswerSchema.json @@ -0,0 +1,80 @@ +{ + "anyOf": [ + { + "type": "object", + "properties": { + "outcome": { + "type": "string", + "enum": [ + "done" + ] + }, + "files": { + "type": "array", + "items": { + "type": "object", + "properties": { + "path": { + "type": "string" + }, + "state": { + "type": "string", + "enum": [ + "viewed", + "unviewed", + "dismissed" + ] + } + }, + "required": [ + "path", + "state" + ], + "additionalProperties": false + }, + "maxItems": 300 + } + }, + "required": [ + "outcome", + "files" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "outcome": { + "type": "string", + "enum": [ + "sign-in-required" + ] + } + }, + "required": [ + "outcome" + ], + "additionalProperties": false + }, + { + "type": "object", + "properties": { + "outcome": { + "type": "string", + "enum": [ + "refused" + ] + }, + "words": { + "type": "string", + "maxLength": 1000 + } + }, + "required": [ + "outcome", + "words" + ], + "additionalProperties": false + } + ] +} diff --git a/packages/features/src/coding-agent-client.ts b/packages/features/src/coding-agent-client.ts index b07d1da350..a8687bf8cd 100644 --- a/packages/features/src/coding-agent-client.ts +++ b/packages/features/src/coding-agent-client.ts @@ -47,6 +47,7 @@ import { type CodingAgentOverviewPullRequestAnswerView, type CodingAgentPageAnswerView, type CodingAgentPullRequestAnswerView, + type CodingAgentPullRequestViewedAnswerView, type CodingAgentPullRequestWriteAnswerView, type CodingAgentReviewOptionsAnswerView, type CodingAgentWorkspaceListParams, @@ -127,6 +128,7 @@ import { import { type PullRequestActionRequest, pullRequestActionRequestSchema, + pullRequestViewedAnswerSchema, pullRequestWriteAnswerSchema, } from "./pull-request-write-wire.js"; import { type RepositoryUnreached, repositoryNotReachableSchema } from "./repository-reach-wire.js"; @@ -142,6 +144,7 @@ import { agentPicturePath, agentPullRequestActionPath, agentPullRequestPath, + agentPullRequestViewedPath, agentReviewOptionsPath, agentStopPath, agentSubagentMessagesPath, @@ -883,6 +886,16 @@ export class HostedCodingAgentClient { ); } + /** Which of the agent's pull request's files the member marked viewed on GitHub, read on their own sign-in. */ + pullRequestViewed( + agentId: string, + ): Effect.Effect { + return Effect.flatMap( + this.#call.send({ method: HTTP_METHOD.GET, path: agentPullRequestViewedPath(agentId) }), + (answer) => readAnswer(answer, pullRequestViewedAnswerSchema, ROW_REFUSALS), + ); + } + /** * Writes to a workspace's chat as the account's own member of it: a post, * an answer to an ask, a resolve, or how far the person has read, under diff --git a/packages/features/src/coding-agent-view.ts b/packages/features/src/coding-agent-view.ts index fa730f9ef1..f7f5493592 100644 --- a/packages/features/src/coding-agent-view.ts +++ b/packages/features/src/coding-agent-view.ts @@ -46,6 +46,7 @@ import { } from "./provider-key-wire.js"; import { pullRequestActionRequestSchema, + pullRequestViewedAnswerSchema, pullRequestWriteAnswerSchema, } from "./pull-request-write-wire.js"; import { repositoryUnreachedSchema } from "./repository-reach-wire.js"; @@ -403,6 +404,15 @@ export const codingAgentPullRequestWriteAnswerViewSchema = EffectSchema.Union([ export type CodingAgentPullRequestWriteAnswerView = typeof codingAgentPullRequestWriteAnswerViewSchema.Type; +/** Which of an agent's pull request's files the member marked viewed on GitHub, as the window hears it: each file's mark, the member to sign in with GitHub again, GitHub's refusal in its words, or why there is no answer. */ +export const codingAgentPullRequestViewedAnswerViewSchema = EffectSchema.Union([ + pullRequestViewedAnswerSchema, + failed, +]); + +export type CodingAgentPullRequestViewedAnswerView = + typeof codingAgentPullRequestViewedAnswerViewSchema.Type; + /** A dictated clip, as the window asks for its words: the clip once, its length, and the words to listen for. */ export const dictationTranscribeParamsSchema = dictationRequestSchema; diff --git a/packages/features/src/hosted-wire-schemas.test.ts b/packages/features/src/hosted-wire-schemas.test.ts index 634108b77d..1d9fb3a070 100644 --- a/packages/features/src/hosted-wire-schemas.test.ts +++ b/packages/features/src/hosted-wire-schemas.test.ts @@ -227,6 +227,8 @@ const EFFECT_MODULE_SCHEMAS = { codingAgentView.codingAgentPullRequestActionParamsSchema, codingAgentPullRequestWriteAnswerViewSchema: codingAgentView.codingAgentPullRequestWriteAnswerViewSchema, + codingAgentPullRequestViewedAnswerViewSchema: + codingAgentView.codingAgentPullRequestViewedAnswerViewSchema, codingAgentGitHubImageParamsSchema: codingAgentView.codingAgentGitHubImageParamsSchema, githubImageTypeSchema: codingAgentView.githubImageTypeSchema, codingAgentGitHubImageViewSchema: codingAgentView.codingAgentGitHubImageViewSchema, @@ -441,6 +443,7 @@ const EFFECT_MODULE_SCHEMAS = { mergeMethodSchema: pullRequestWriteWire.mergeMethodSchema, pullRequestActionRequestSchema: pullRequestWriteWire.pullRequestActionRequestSchema, pullRequestWriteAnswerSchema: pullRequestWriteWire.pullRequestWriteAnswerSchema, + pullRequestViewedAnswerSchema: pullRequestWriteWire.pullRequestViewedAnswerSchema, } satisfies RecordedEffectJsonSchemas, "repository-reach-wire": { repositoryUnreachedSchema: repositoryReachWire.repositoryUnreachedSchema, diff --git a/packages/features/src/pull-request-write-wire.ts b/packages/features/src/pull-request-write-wire.ts index d8e39c1b3b..4d05455698 100644 --- a/packages/features/src/pull-request-write-wire.ts +++ b/packages/features/src/pull-request-write-wire.ts @@ -12,7 +12,11 @@ import { Schema as EffectSchema } from "effect"; * change itself, in GitHub's own words: a branch protection, a conflict, or * a permission the member's account does not hold. A merge, and turning * auto-merge on, name the head commit the member saw, so GitHub refuses one - * made against a head pushed since. A refusal of Luke's own, + * made against a head pushed since. Which of the pull request's files the + * member marked viewed on GitHub is read the same way, on their own sign-in, + * since GitHub keeps the mark per account + * (`GET /api/agents/{id}/pull-request/viewed`), and set by two of the + * changes. A refusal of Luke's own, * an agent out of reach or a service that could not answer, travels as the * hosted vocabulary's refusal as every other call's does. */ @@ -32,6 +36,8 @@ export const PULL_REQUEST_WRITE_BOUNDS = { MAX_NODE_ID_CHARS: 256, /** The longest path a line comment names; GitHub's own paths stop well short of it. */ MAX_PATH_CHARS: 4_096, + /** The most files whose viewed marks one read carries, as many as a read of the changes lists. */ + MAX_VIEWED_FILES: 300, } as const; /** How a pull request is merged, as GitHub offers each where the repository allows it. */ @@ -115,6 +121,10 @@ export const PULL_REQUEST_ACTION = { ADD_REVIEW_COMMENT: "add-review-comment", /** The member's own pending review thrown away, the comments it held with it. */ DISCARD_REVIEW: "discard-review", + /** One of the pull request's files marked viewed by the member, as GitHub's own Viewed box does. */ + MARK_FILE_VIEWED: "mark-file-viewed", + /** One of the pull request's files the member marked viewed unmarked. */ + UNMARK_FILE_VIEWED: "unmark-file-viewed", } as const; export type PullRequestAction = (typeof PULL_REQUEST_ACTION)[keyof typeof PULL_REQUEST_ACTION]; @@ -189,6 +199,11 @@ const subjectIdSchema = EffectSchema.NonEmptyString.check( EffectSchema.isMaxLength(PULL_REQUEST_WRITE_BOUNDS.MAX_NODE_ID_CHARS), ); +/** A file of the pull request, by its path from the repository's root; GitHub's own paths stop well short of the bound. */ +const filePathSchema = EffectSchema.NonEmptyString.check( + EffectSchema.isMaxLength(PULL_REQUEST_WRITE_BOUNDS.MAX_PATH_CHARS), +); + /** A reply's or a comment's words, Markdown, never blank; GitHub's bound. */ const replyBodySchema = reviewBodySchema.check(EffectSchema.isPattern(/\S/u)); @@ -282,6 +297,14 @@ export const pullRequestActionRequestSchema = EffectSchema.Union([ headSha: headShaSchema, }), EffectSchema.Struct({ action: EffectSchema.Literal(PULL_REQUEST_ACTION.DISCARD_REVIEW) }), + EffectSchema.Struct({ + action: EffectSchema.Literal(PULL_REQUEST_ACTION.MARK_FILE_VIEWED), + path: filePathSchema, + }), + EffectSchema.Struct({ + action: EffectSchema.Literal(PULL_REQUEST_ACTION.UNMARK_FILE_VIEWED), + path: filePathSchema, + }), ]); export type PullRequestActionRequest = typeof pullRequestActionRequestSchema.Type; @@ -312,3 +335,44 @@ export const pullRequestWriteAnswerSchema = EffectSchema.Union([ ]); export type PullRequestWriteAnswer = typeof pullRequestWriteAnswerSchema.Type; + +/** + * Where a file stands for the member on GitHub: marked viewed, not, or + * marked viewed and changed since, which GitHub unticks until marked again. + */ +export const FILE_VIEWED_STATE = { + VIEWED: "viewed", + UNVIEWED: "unviewed", + DISMISSED: "dismissed", +} as const; + +export type FileViewedState = (typeof FILE_VIEWED_STATE)[keyof typeof FILE_VIEWED_STATE]; + +/** + * Which of the pull request's files the member marked viewed, as the service + * answers it (GET): each file by its path and where it stands for them, none + * where the agent has opened no pull request; or why there is no answer, as + * a change's. + */ +export const pullRequestViewedAnswerSchema = EffectSchema.Union([ + EffectSchema.Struct({ + outcome: EffectSchema.Literal(PULL_REQUEST_WRITE_OUTCOME.DONE), + files: EffectSchema.Array( + EffectSchema.Struct({ + path: EffectSchema.String, + state: EffectSchema.Literals(Object.values(FILE_VIEWED_STATE)), + }), + ).check(EffectSchema.isMaxLength(PULL_REQUEST_WRITE_BOUNDS.MAX_VIEWED_FILES)), + }), + EffectSchema.Struct({ + outcome: EffectSchema.Literal(PULL_REQUEST_WRITE_OUTCOME.SIGN_IN_REQUIRED), + }), + EffectSchema.Struct({ + outcome: EffectSchema.Literal(PULL_REQUEST_WRITE_OUTCOME.REFUSED), + words: EffectSchema.String.check( + EffectSchema.isMaxLength(PULL_REQUEST_WRITE_BOUNDS.MAX_REFUSAL_CHARS), + ), + }), +]); + +export type PullRequestViewedAnswer = typeof pullRequestViewedAnswerSchema.Type; diff --git a/packages/features/src/service-paths.ts b/packages/features/src/service-paths.ts index 3afa62eed8..6a688a9767 100644 --- a/packages/features/src/service-paths.ts +++ b/packages/features/src/service-paths.ts @@ -624,6 +624,11 @@ export function agentPullRequestActionPath(agentId: string): string { return `/api/agents/${encodeURIComponent(agentId)}/pull-request/action`; } +/** Which of an agent's pull request's files the asking member marked viewed on GitHub (GET), read on their own sign-in; `pull-request-write-wire.ts` declares the answer. */ +export function agentPullRequestViewedPath(agentId: string): string { + return `/api/agents/${encodeURIComponent(agentId)}/pull-request/viewed`; +} + /** What the reviewer and label pickers of an agent's Pull request tab offer: its repository's labels, collaborators, and teams. */ export function agentReviewOptionsPath(agentId: string): string { return `/api/agents/${encodeURIComponent(agentId)}/review-options`; diff --git a/packages/gateway/src/protocol.ts b/packages/gateway/src/protocol.ts index d7849bbbfe..c28ff48094 100644 --- a/packages/gateway/src/protocol.ts +++ b/packages/gateway/src/protocol.ts @@ -171,6 +171,8 @@ export const GATEWAY_METHOD = { CODING_AGENTS_GITHUB_IMAGE: "codingAgents.githubImage", /** One change to an agent's pull request, made on GitHub by the service as the account on its own GitHub sign-in: how GitHub took it. */ CODING_AGENTS_PULL_REQUEST_ACTION: "codingAgents.pullRequestAction", + /** Which of an agent's pull request's files the account marked viewed on GitHub, read by the service on its own GitHub sign-in. */ + CODING_AGENTS_PULL_REQUEST_VIEWED: "codingAgents.pullRequestViewed", /** The account's own provider keys an agent runs on, read from the service now as hints and statuses; never a key. */ PROVIDER_KEYS_READ: "providerKeys.read", /** One provider key saved on the service, the key carried once in the params; the keys as they then stand, or why nothing was kept. */ diff --git a/packages/host/src/act-router/coding-agent-acts.test.ts b/packages/host/src/act-router/coding-agent-acts.test.ts index ebd64d80a9..e928c17c33 100644 --- a/packages/host/src/act-router/coding-agent-acts.test.ts +++ b/packages/host/src/act-router/coding-agent-acts.test.ts @@ -20,6 +20,7 @@ import { import { DEFAULT_PLAN_TEMPLATE } from "@luke/features/plan-document-template"; import { PROVIDER_KEY_PROVIDER } from "@luke/features/provider-key-wire"; import { + FILE_VIEWED_STATE, PULL_REQUEST_ACTION, PULL_REQUEST_WRITE_OUTCOME, } from "@luke/features/pull-request-write-wire"; @@ -247,6 +248,14 @@ function fixture(answered: CodingAgentAgentAnswer = STARTED) { asked.push(`pull-request-action:${agentId}:${request.action}`); return { outcome: PULL_REQUEST_WRITE_OUTCOME.DONE } as const; }), + codingAgentPullRequestViewed: ({ agentId }) => + Effect.sync(() => { + asked.push(`pull-request-viewed:${agentId}`); + return { + outcome: PULL_REQUEST_WRITE_OUTCOME.DONE, + files: [{ path: "src/seats.ts", state: FILE_VIEWED_STATE.VIEWED }], + } as const; + }), codingAgentGitHubImage: ({ agentId }) => Effect.sync(() => { asked.push(`github-image:${agentId}`); @@ -516,6 +525,34 @@ it.effect( }), ); +it.effect( + "a panel's read of which files it viewed reaches the host and hears each file's mark, and the voice window's is refused before it", + () => + Effect.gen(function* () { + const { router, asked } = fixture(); + const act = { + kind: ACT_KIND.CODING_AGENTS_PULL_REQUEST_VIEWED, + payload: { agentId: AGENT_ID }, + } as const; + + const refused = yield* router.performAct(act, VOICE); + const read = yield* router.performAct(act, PANEL); + + assert.deepEqual(refused, { + status: ACT_OUTCOME_STATUS.REFUSED, + reason: ACT[ACT_KIND.CODING_AGENTS_PULL_REQUEST_VIEWED].refusal, + }); + assert.deepEqual(read, { + status: ACT_OUTCOME_STATUS.DONE, + value: { + outcome: PULL_REQUEST_WRITE_OUTCOME.DONE, + files: [{ path: "src/seats.ts", state: FILE_VIEWED_STATE.VIEWED }], + }, + }); + assert.deepEqual(asked, [`pull-request-viewed:${AGENT_ID}`]); + }), +); + it.effect( "a panel's message on an inbox item's thread reaches the host with its words, files, and key, and hears the item; the voice window's is refused before it", () => diff --git a/packages/host/src/act-router/coding-agent-acts.ts b/packages/host/src/act-router/coding-agent-acts.ts index 2bdf07a0dd..7c3cba39d8 100644 --- a/packages/host/src/act-router/coding-agent-acts.ts +++ b/packages/host/src/act-router/coding-agent-acts.ts @@ -39,6 +39,7 @@ import { type CodingAgentPullRequestActionParams, type CodingAgentPullRequestAnswerView, type CodingAgentPullRequestParams, + type CodingAgentPullRequestViewedAnswerView, type CodingAgentPullRequestWriteAnswerView, type CodingAgentRenameParams, type CodingAgentReviewOptionsAnswerView, @@ -133,6 +134,9 @@ export interface CodingAgentActsDependencies { codingAgentPullRequestAction( params: CodingAgentPullRequestActionParams, ): Effect.Effect; + codingAgentPullRequestViewed( + params: CodingAgentPullRequestParams, + ): Effect.Effect; providerKeysRead(): Effect.Effect; providerKeySave(params: ProviderKeySaveParams): Effect.Effect; providerKeyRemove(params: ProviderKeyRemoveParams): Effect.Effect; @@ -216,6 +220,7 @@ export type CodingAgentActKind = | typeof ACT_KIND.CODING_AGENTS_OVERVIEW_CONVERSATION | typeof ACT_KIND.CODING_AGENTS_GITHUB_IMAGE | typeof ACT_KIND.CODING_AGENTS_PULL_REQUEST_ACTION + | typeof ACT_KIND.CODING_AGENTS_PULL_REQUEST_VIEWED | typeof ACT_KIND.PROVIDER_KEYS_READ | typeof ACT_KIND.PROVIDER_KEYS_SAVE | typeof ACT_KIND.PROVIDER_KEYS_REMOVE @@ -380,6 +385,10 @@ export function codingAgentActRows( refuseUnlessPanel(ACT_KIND.CODING_AGENTS_PULL_REQUEST_ACTION, sender); return host.codingAgentPullRequestAction(params); }, + [ACT_KIND.CODING_AGENTS_PULL_REQUEST_VIEWED]: (params, sender) => { + refuseUnlessPanel(ACT_KIND.CODING_AGENTS_PULL_REQUEST_VIEWED, sender); + return host.codingAgentPullRequestViewed(params); + }, [ACT_KIND.PROVIDER_KEYS_READ]: (_payload, sender) => { refuseUnlessPanel(ACT_KIND.PROVIDER_KEYS_READ, sender); return host.providerKeysRead(); diff --git a/packages/host/src/acts.ts b/packages/host/src/acts.ts index cda819b8cb..c7b8776b9b 100644 --- a/packages/host/src/acts.ts +++ b/packages/host/src/acts.ts @@ -37,6 +37,7 @@ import { type CodingAgentOverviewPullRequestAnswerView, type CodingAgentPageAnswerView, type CodingAgentPullRequestAnswerView, + type CodingAgentPullRequestViewedAnswerView, type CodingAgentPullRequestWriteAnswerView, type CodingAgentReviewOptionsAnswerView, codingAgentAgentAnswerSchema, @@ -79,6 +80,7 @@ import { codingAgentPullRequestActionParamsSchema, codingAgentPullRequestAnswerViewSchema, codingAgentPullRequestParamsSchema, + codingAgentPullRequestViewedAnswerViewSchema, codingAgentPullRequestWriteAnswerViewSchema, codingAgentRenameParamsSchema, codingAgentReviewOptionsAnswerViewSchema, @@ -485,6 +487,8 @@ export const ACT_KIND = { CODING_AGENTS_GITHUB_IMAGE: "codingAgents.githubImage", /** One change to an agent's pull request, such as marking it ready for review, made on GitHub as the account on its own GitHub sign-in. */ CODING_AGENTS_PULL_REQUEST_ACTION: "codingAgents.pullRequestAction", + /** Which of an agent's pull request's files the account marked viewed on GitHub, read on its own GitHub sign-in. */ + CODING_AGENTS_PULL_REQUEST_VIEWED: "codingAgents.pullRequestViewed", /** The account's automations: listed, made, changed, deleted, run now, and one's runs read. */ AUTOMATIONS_LIST: "automations.list", AUTOMATIONS_CREATE: "automations.create", @@ -1359,6 +1363,13 @@ export const ACT = { ), refusal: "Could not change the pull request on this system.", }, + [ACT_KIND.CODING_AGENTS_PULL_REQUEST_VIEWED]: { + payload: actSchema(codingAgentPullRequestParamsSchema), + result: wireResult( + isReadable(codingAgentPullRequestViewedAnswerViewSchema), + ), + refusal: "Could not read which files you viewed on this system.", + }, [ACT_KIND.AUTOMATIONS_LIST]: { payload: actSchema(automationListParamsSchema), result: wireResult(isReadable(automationListViewSchema)), diff --git a/packages/host/src/compose-coding-agents.test.ts b/packages/host/src/compose-coding-agents.test.ts index cbc50ede12..420970929d 100644 --- a/packages/host/src/compose-coding-agents.test.ts +++ b/packages/host/src/compose-coding-agents.test.ts @@ -337,6 +337,11 @@ function fakeService() { asked.push(`pull-request-action:${agentId}:${request.action}`); return { outcome: PULL_REQUEST_WRITE_OUTCOME.DONE }; }), + pullRequestViewed: (agentId) => + Effect.sync(() => { + asked.push(`pull-request-viewed:${agentId}`); + return { outcome: PULL_REQUEST_WRITE_OUTCOME.DONE, files: [] }; + }), githubImage: (agentId, src) => Effect.sync(() => { asked.push(`github-image:${agentId}:${src}`); @@ -645,6 +650,10 @@ it.effect("each method asks the service once and answers what it said, whole", ( }), { outcome: PULL_REQUEST_WRITE_OUTCOME.DONE }, ); + assert.deepEqual( + yield* call(GATEWAY_METHOD.CODING_AGENTS_PULL_REQUEST_VIEWED, { agentId: AGENT_ID }), + { outcome: PULL_REQUEST_WRITE_OUTCOME.DONE, files: [] }, + ); assert.deepEqual( yield* call(GATEWAY_METHOD.CODING_AGENTS_INBOX, { taskId: TASK_ID, @@ -768,6 +777,7 @@ it.effect("each method asks the service once and answers what it said, whole", ( `overview-conversation:${AGENT_ID}`, `github-image:${AGENT_ID}:https://camo.githubusercontent.com/abc`, `pull-request-action:${AGENT_ID}:${PULL_REQUEST_ACTION.READY_FOR_REVIEW}`, + `pull-request-viewed:${AGENT_ID}`, `inbox:${TASK_ID}:${INBOX_CURSOR_START}`, `attachment:${ITEM.id}:${ATTACHMENT_ID}`, `picture:${AGENT_ID}:${PICTURE_ID}:call-worker`, diff --git a/packages/host/src/compose-coding-agents.ts b/packages/host/src/compose-coding-agents.ts index 51ce253243..f0a4c520e3 100644 --- a/packages/host/src/compose-coding-agents.ts +++ b/packages/host/src/compose-coding-agents.ts @@ -22,6 +22,7 @@ import { type CodingAgentOverviewPullRequestAnswerView, type CodingAgentPageAnswerView, type CodingAgentPullRequestAnswerView, + type CodingAgentPullRequestViewedAnswerView, type CodingAgentPullRequestWriteAnswerView, type CodingAgentReviewOptionsAnswerView, codingAgentAccountInboxParamsSchema, @@ -118,6 +119,7 @@ export type CodingAgentClient = Pick< | "overviewConversation" | "githubImage" | "pullRequestAction" + | "pullRequestViewed" | "readProviderKeys" | "saveProviderKey" | "removeProviderKey" @@ -468,6 +470,16 @@ function composed( const answer = yield* asked(client.pullRequestAction(agentId, request)); return carried(answer); }), + [GATEWAY_METHOD.CODING_AGENTS_PULL_REQUEST_VIEWED]: (params) => + Effect.gen(function* () { + const { agentId } = yield* read( + codingAgentPullRequestParamsSchema, + params, + "reading which files of an agent's pull request were viewed names the agent", + ); + const answer = yield* asked(client.pullRequestViewed(agentId)); + return carried(answer); + }), [GATEWAY_METHOD.CODING_AGENTS_GITHUB_IMAGE]: (params) => Effect.gen(function* () { const { agentId, src } = yield* read( diff --git a/packages/host/src/host-operator.ts b/packages/host/src/host-operator.ts index 4371cc92e1..6b119c4a7b 100644 --- a/packages/host/src/host-operator.ts +++ b/packages/host/src/host-operator.ts @@ -62,6 +62,7 @@ import { type CodingAgentPullRequestActionParams, type CodingAgentPullRequestAnswerView, type CodingAgentPullRequestParams, + type CodingAgentPullRequestViewedAnswerView, type CodingAgentPullRequestWriteAnswerView, type CodingAgentRenameParams, type CodingAgentReviewOptionsAnswerView, @@ -90,6 +91,7 @@ import { codingAgentOverviewPullRequestAnswerViewSchema, codingAgentPageAnswerViewSchema, codingAgentPullRequestAnswerViewSchema, + codingAgentPullRequestViewedAnswerViewSchema, codingAgentPullRequestWriteAnswerViewSchema, codingAgentReviewOptionsAnswerViewSchema, type DictationTranscribeAnswerView, @@ -571,6 +573,10 @@ export interface HostOperator { codingAgentPullRequestAction( params: CodingAgentPullRequestActionParams, ): Effect.Effect; + /** Which of one agent's pull request's files the account marked viewed on GitHub, read on its own GitHub sign-in; or why there is no answer. */ + codingAgentPullRequestViewed( + params: CodingAgentPullRequestParams, + ): Effect.Effect; /** One agent's pull request itself as its overview draws it: its description, branches, mergeability, and people as GitHub holds them; or why there is none. */ codingAgentOverviewPullRequest( params: CodingAgentPullRequestParams, @@ -1628,6 +1634,11 @@ export function createHostOperator(options: HostOperatorOptions): HostOperator { }), codingAgentAnswer(codingAgentPullRequestWriteAnswerViewSchema), ), + codingAgentPullRequestViewed: (params) => + Effect.map( + client.call(GATEWAY_METHOD.CODING_AGENTS_PULL_REQUEST_VIEWED, { agentId: params.agentId }), + codingAgentAnswer(codingAgentPullRequestViewedAnswerViewSchema), + ), codingAgentOverviewPullRequest: (params) => Effect.map( client.call(GATEWAY_METHOD.CODING_AGENTS_OVERVIEW_PULL_REQUEST, { diff --git a/packages/service/src/agents/coding-agents-app.test.ts b/packages/service/src/agents/coding-agents-app.test.ts index 1658e811d0..91336f6ae4 100644 --- a/packages/service/src/agents/coding-agents-app.test.ts +++ b/packages/service/src/agents/coding-agents-app.test.ts @@ -1330,6 +1330,7 @@ it.layer(getTestDB())("the coding-agent routes", (it) => { }); const AGENT_PULL_REQUEST_ACTION = "/api/agents/pull-request-action"; +const AGENT_PULL_REQUEST_VIEWED = "/api/agents/pull-request-viewed"; const AGENT_PULL_REQUEST = "/api/agents/pull-request"; /** The agent's changes, the files Files changed draws. */ const AGENT_CHANGES = "/api/agents/changes"; @@ -1462,6 +1463,37 @@ function threadActionAs(bearer: string, agentId: string, body: WireBoundaryInput return request(AGENT_PULL_REQUEST_ACTION, bearer, { method: "POST", id: agentId, body }); } +/** + * GitHub answering the viewed read a page at a time, the first page's + * cursor naming the second, and the pull request's id and an empty answer + * to each mutation. + */ +function viewedAnswering(call: GraphqlCall): Response { + if (call.query.includes("mutation")) return Response.json({ data: {} }); + if (!call.query.includes("viewerViewedState")) { + return Response.json({ data: { repository: { pullRequest: { id: PULL_REQUEST_NODE_ID } } } }); + } + const first = call.variables.after === null; + const nodes = first + ? [ + { path: "src/seats.ts", viewerViewedState: "VIEWED" }, + { path: "src/plan.ts", viewerViewedState: "DISMISSED" }, + ] + : [{ path: "README.md", viewerViewedState: "UNVIEWED" }]; + return Response.json({ + data: { + repository: { + pullRequest: { + files: { + nodes, + pageInfo: { hasNextPage: first, endCursor: first ? "cursor-2" : null }, + }, + }, + }, + }, + }); +} + /** GitHub answering a thread's calls: the pull request's id, every thread named as one of the pull request given, and an empty answer to each mutation. */ function threadsAnswering(threadPullRequestId: string): (call: GraphqlCall) => Response { return (call) => { @@ -2386,6 +2418,85 @@ it.layer(getTestDB())("changing an agent's pull request", (it) => { }), ); + it.effect( + "a teammate reads which files they viewed on GitHub, every page of them, and marks and unmarks one, each on their own GitHub token", + () => + Effect.gen(function* () { + const { other, agent, calls, ask } = yield* published({ + otherToken: "ghu_teammate", + answer: viewedAnswering, + }); + + const viewed = yield* ask(request(AGENT_PULL_REQUEST_VIEWED, other, { id: agent.id })); + const marked = yield* ask( + threadActionAs(other, agent.id, { action: "mark-file-viewed", path: "README.md" }), + ); + const unmarked = yield* ask( + threadActionAs(other, agent.id, { action: "unmark-file-viewed", path: "src/seats.ts" }), + ); + + assert.deepEqual(viewed, { + status: HOSTED_HTTP_STATUS.OK, + body: { + outcome: "done", + files: [ + { path: "src/seats.ts", state: "viewed" }, + { path: "src/plan.ts", state: "dismissed" }, + { path: "README.md", state: "unviewed" }, + ], + }, + }); + assert.deepEqual(marked, { status: HOSTED_HTTP_STATUS.OK, body: { outcome: "done" } }); + assert.deepEqual(unmarked, { status: HOSTED_HTTP_STATUS.OK, body: { outcome: "done" } }); + assert.deepEqual( + calls + .filter((call) => call.query.includes("viewerViewedState")) + .map((call) => call.variables.after), + [null, "cursor-2"], + ); + assert.deepEqual( + calls + .filter((call) => call.query.includes("mutation")) + .map((call) => [ + call.query.includes("unmarkFileAsViewed") ? "unmark" : "mark", + call.variables, + ]), + [ + ["mark", { id: PULL_REQUEST_NODE_ID, path: "README.md" }], + ["unmark", { id: PULL_REQUEST_NODE_ID, path: "src/seats.ts" }], + ], + ); + assert.ok(calls.every((call) => call.authorization === "Bearer ghu_teammate")); + }), + ); + + it.effect( + "a member with no GitHub sign-in, or one GitHub refuses, is asked to sign in before any viewed mark is read, never on the App's token", + () => + Effect.gen(function* () { + const unsigned = yield* published({ otherToken: null }); + assert.deepEqual( + yield* unsigned.ask( + request(AGENT_PULL_REQUEST_VIEWED, unsigned.other, { id: unsigned.agent.id }), + ), + { status: HOSTED_HTTP_STATUS.OK, body: { outcome: "sign-in-required" } }, + ); + assert.deepEqual(unsigned.calls, []); + + const revoked = yield* published({ + otherToken: "ghu_revoked", + answer: () => Response.json({ message: "Bad credentials" }, { status: 401 }), + }); + assert.deepEqual( + yield* revoked.ask( + request(AGENT_PULL_REQUEST_VIEWED, revoked.other, { id: revoked.agent.id }), + ), + { status: HOSTED_HTTP_STATUS.OK, body: { outcome: "sign-in-required" } }, + ); + assert.ok(revoked.calls.every((call) => call.authorization === "Bearer ghu_revoked")); + }), + ); + it.effect( "an account outside the agent's workspace is answered as if the agent were none, and a change the wire does not name is refused, neither reaching GitHub", () => diff --git a/packages/service/src/agents/coding-agents-app.ts b/packages/service/src/agents/coding-agents-app.ts index 5293e7d7a4..08e7466d43 100644 --- a/packages/service/src/agents/coding-agents-app.ts +++ b/packages/service/src/agents/coding-agents-app.ts @@ -63,7 +63,9 @@ import { modelChoiceSchema } from "@luke/features/models-wire"; import { PULL_REQUEST_ACTION, PULL_REQUEST_WRITE_BOUNDS, + PULL_REQUEST_WRITE_OUTCOME, type PullRequestActionRequest, + type PullRequestViewedAnswer, pullRequestActionRequestSchema, } from "@luke/features/pull-request-write-wire"; import { readEither } from "@luke/wire/effect"; @@ -88,9 +90,11 @@ import { type GitHubWriteFailure, mergePullRequest, pullRequestNodeId, + pullRequestViewedStates, reactOnPullRequest, replyToReviewThread, setAutoMerge, + setFileViewed, setLabelled, setPullRequestDraft, setPullRequestOpen, @@ -263,7 +267,10 @@ import { * published answer is let go, so the next read asks GitHub afresh. What the * reviewer and label pickers offer (`GET /api/agents/{id}/review-options`), * the repository's labels, collaborators, and teams, is read on a token for - * the agent's repository, as the overview is. An image + * the agent's repository, as the overview is. Which of its files the member + * marked viewed on GitHub, a mark GitHub keeps for each account, is read + * through the member's own sign-in (`GET /api/agents/{id}/pull-request/viewed`), + * and set by two of the changes. An image * those words name (`GET /api/agents/{id}/github-image`) is * read on a token for the agent's repository too, an upload re-signed by its * asset's id (`github/github-images.ts`), and answered as bytes. @@ -301,6 +308,7 @@ const AGENTS_PATH = { OVERVIEW_CONVERSATION: "/api/agents/overview-conversation", /** POST makes one change to the agent's pull request as the asking member; the rewrite moves the path's agent id into the `id` query. */ PULL_REQUEST_ACTION: "/api/agents/pull-request-action", + PULL_REQUEST_VIEWED: "/api/agents/pull-request-viewed", /** GET reads what the reviewer and label pickers of the agent's Pull request tab offer; the rewrite moves the path's agent id into the `id` query. */ REVIEW_OPTIONS: "/api/agents/review-options", /** GET reads one image its pull request's overview names, by its address in the `src` query; the rewrite moves the path's agent id into the `id` query. */ @@ -1198,6 +1206,14 @@ function writeOf( } case PULL_REQUEST_ACTION.DISCARD_REVIEW: return discardPendingReview(token, id); + case PULL_REQUEST_ACTION.MARK_FILE_VIEWED: + case PULL_REQUEST_ACTION.UNMARK_FILE_VIEWED: + return setFileViewed( + token, + id, + request.path, + request.action === PULL_REQUEST_ACTION.MARK_FILE_VIEWED, + ); } } @@ -1268,6 +1284,40 @@ const pullRequestActionEndpoint = /* @__PURE__ */ Effect.fn("web/agentPullReques }, ); +/** + * GET reads which of the files of the pull request the agent's kept + * published answer names the asking member marked viewed on GitHub, on + * their own GitHub sign-in (`pull-request-write.ts`), since GitHub keeps the + * mark for each account alone and the App sees none of it; no files where it + * names no pull request yet. Nothing is let go, as nothing changed. + */ +const pullRequestViewedEndpoint = /* @__PURE__ */ Effect.fn("web/agentPullRequestViewedEndpoint")( + function* ( + seams: CodingAgentsAppSeams, + published: PublishedCache, + ): Effect.fn.Return { + const incoming = yield* HttpServerRequest.HttpServerRequest; + if (incoming.method !== HTTP_METHOD.GET) { + return yield* Effect.fail(HOSTED_REFUSAL.METHOD_NOT_ALLOWED); + } + const request = yield* Effect.orDie(HttpServerRequest.toWeb(incoming)); + const { userId, agent } = yield* reachedAgent(seams, request); + const { pullRequest } = yield* keptPublished(published, userId, agent); + if (pullRequest === null) { + const none: PullRequestViewedAnswer = { outcome: PULL_REQUEST_WRITE_OUTCOME.DONE, files: [] }; + return hostedJsonResponse(HOSTED_HTTP_STATUS.OK, none); + } + const read = yield* memberPullRequestWrite(userId, agent, pullRequest.number, (member) => + pullRequestViewedStates(member.token, member.repository, member.number), + ); + const answer: PullRequestViewedAnswer = + read.outcome === PULL_REQUEST_WRITE_OUTCOME.DONE + ? { outcome: read.outcome, files: read.value } + : read; + return hostedJsonResponse(HOSTED_HTTP_STATUS.OK, answer); + }, +); + /** * GET reads one image the agent's pull request's overview names, by the * address GitHub's rendering spelled (`src`): an upload re-signed on a token @@ -1749,6 +1799,11 @@ export function codingAgentsApp(seams: CodingAgentsAppSeams): WebRoutes( const githubWrite = ( token: Redacted.Redacted, document: string, - variables: Readonly>, + variables: Readonly>, data: Schema.Codec, ): Effect.Effect => Effect.scoped( @@ -765,3 +769,118 @@ export const reactOnPullRequest = /* @__PURE__ */ Effect.fn("web/reactOnPullRequ Schema.Unknown, ); }); + +const MARK_VIEWED_MUTATION = `mutation($id: ID!, $path: String!) { + markFileAsViewed(input: { pullRequestId: $id, path: $path }) { clientMutationId } +}`; + +const UNMARK_VIEWED_MUTATION = `mutation($id: ID!, $path: String!) { + unmarkFileAsViewed(input: { pullRequestId: $id, path: $path }) { clientMutationId } +}`; + +/** One of the pull request's files marked viewed, or unmarked where `viewed` is false, for the member alone, on their token. */ +export const setFileViewed = /* @__PURE__ */ Effect.fn("web/setFileViewed")(function* ( + token: Redacted.Redacted, + pullRequestId: string, + path: string, + viewed: boolean, +): Effect.fn.Return { + yield* githubWrite( + token, + viewed ? MARK_VIEWED_MUTATION : UNMARK_VIEWED_MUTATION, + { id: pullRequestId, path }, + Schema.Unknown, + ); +}); + +/** GitHub lists a pull request's files a hundred to a page. */ +const VIEWED_PER_PAGE = 100; + +const VIEWED_QUERY = `query($owner: String!, $name: String!, $number: Int!, $after: String) { + repository(owner: $owner, name: $name) { + pullRequest(number: $number) { + files(first: ${VIEWED_PER_PAGE}, after: $after) { + nodes { path viewerViewedState } + pageInfo { hasNextPage endCursor } + } + } + } +}`; + +/** GitHub's own word for where a file stands for the viewer, as `FileViewedState` spells it. */ +const GRAPHQL_VIEWED_STATE = { + VIEWED: FILE_VIEWED_STATE.VIEWED, + UNVIEWED: FILE_VIEWED_STATE.UNVIEWED, + DISMISSED: FILE_VIEWED_STATE.DISMISSED, +} as const satisfies Record; + +/** GitHub's words for where a file stands, each one `GRAPHQL_VIEWED_STATE` reads. */ +const GRAPHQL_VIEWED_STATES = [ + "VIEWED", + "UNVIEWED", + "DISMISSED", +] as const satisfies readonly (keyof typeof GRAPHQL_VIEWED_STATE)[]; + +const ViewedPageSchema = Schema.Struct({ + repository: Schema.NullOr( + Schema.Struct({ + pullRequest: Schema.NullOr( + Schema.Struct({ + files: Schema.NullOr( + Schema.Struct({ + nodes: Schema.Array( + Schema.Struct({ + path: Schema.String, + viewerViewedState: Schema.Literals(GRAPHQL_VIEWED_STATES), + }), + ), + pageInfo: Schema.Struct({ + hasNextPage: Schema.Boolean, + endCursor: Schema.NullOr(Schema.String), + }), + }), + ), + }), + ), + }), + ), +}); + +/** One file and where it stands for the member. */ +export interface FileViewed { + readonly path: string; + readonly state: FileViewedState; +} + +/** + * Where each of the pull request's files stands for the member on GitHub, + * read on their token a page at a time, up to as many as a read of the + * changes lists; a pull request out of their sight is GitHub's refusal. + */ +export const pullRequestViewedStates = /* @__PURE__ */ Effect.fn("web/pullRequestViewedStates")( + function* ( + token: Redacted.Redacted, + fullName: string, + number: number, + ): Effect.fn.Return { + const [owner = "", name = ""] = fullName.split("/"); + const files: FileViewed[] = []; + let after: string | null = null; + while (files.length < PULL_REQUEST_WRITE_BOUNDS.MAX_VIEWED_FILES) { + const page: typeof ViewedPageSchema.Type = yield* githubWrite( + token, + VIEWED_QUERY, + { owner, name, number, after }, + ViewedPageSchema, + ); + const listed = page.repository?.pullRequest?.files; + if (listed === undefined || listed === null) return yield* refusalOf([NOT_RESOLVED]); + for (const node of listed.nodes) { + files.push({ path: node.path, state: GRAPHQL_VIEWED_STATE[node.viewerViewedState] }); + } + if (!listed.pageInfo.hasNextPage || listed.pageInfo.endCursor === null) break; + after = listed.pageInfo.endCursor; + } + return files.slice(0, PULL_REQUEST_WRITE_BOUNDS.MAX_VIEWED_FILES); + }, +); diff --git a/vercel.json b/vercel.json index 7d471314af..9f5f23c76d 100644 --- a/vercel.json +++ b/vercel.json @@ -161,6 +161,10 @@ "src": "/api/agents/([^/]+)/pull-request/action", "dest": "/api/default.js?route=agents/pull-request-action&id=$1" }, + { + "src": "/api/agents/([^/]+)/pull-request/viewed", + "dest": "/api/default.js?route=agents/pull-request-viewed&id=$1" + }, { "src": "/api/agents/([^/]+)/changes", "dest": "/api/default.js?route=agents/changes&id=$1"