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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion PRIVACY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 11 additions & 0 deletions apps/desktop/src/renderer/styles/tasks.css
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
329 changes: 329 additions & 0 deletions apps/desktop/src/renderer/tasks/agent-changes-tab.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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";
Expand Down Expand Up @@ -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<void> | 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: {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<HTMLInputElement>('.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<HTMLButtonElement>('button[aria-label="Changes actions"]');
act(() => options?.click());
const refresh = [...document.querySelectorAll<HTMLButtonElement>('[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<ActOutcome>();
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<void>();
viewedHeld = reading.promise;
const options = document.querySelector<HTMLButtonElement>('button[aria-label="Changes actions"]');
act(() => options?.click());
const refresh = [...document.querySelectorAll<HTMLButtonElement>('[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<HTMLButtonElement>('[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<HTMLButtonElement>('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],
]);
});
Loading
Loading