desktop: address final projects review findings

Ensure maintainer assignment decisions outrank self-service timestamps, repeated entity links reopen their details, and assignee profiles hydrate in project lists.

Signed-off-by: Thomas Petersen <thomasp@squareup.com>
This commit is contained in:
Thomas Petersen
2026-08-12 12:45:22 -04:00
parent fbe953e1e9
commit 2aed85392e
6 changed files with 169 additions and 42 deletions
@@ -84,24 +84,28 @@ function statusFromEvent(issue, statusEvent) {
}
/**
* Assignment state is reduced from trusted kind:1 operations in chronological
* order. `t: assignment` adds each `p` tag and `t: unassignment` removes it.
* The issue root's `p` tags are notification routing only.
* Assignment state is reduced from trusted kind:1 operations. `t: assignment`
* adds each `p` tag and `t: unassignment` removes it. The issue root's `p`
* tags are notification routing only.
*
* Trusted signers are the issue author and repo owner (who may change anyone),
* plus any community member whose operation names only themselves. Same-second
* events use their id as a deterministic tie-breaker because relay result order
* is not stable.
* plus any community member whose operation names only themselves. Self-service
* operations are applied first and authoritative operations last, so an author
* or owner decision wins regardless of signer-controlled timestamps. Within
* each authority class, same-second events use their id as a deterministic
* tie-breaker because relay result order is not stable.
*/
function assigneesForIssue(issue, issueCommentEvents) {
const allowedActors = allowedActorsForRoot(issue);
const assignees = new Set();
const operations = sortEvents(
const selfServiceOperations = [];
const authoritativeOperations = [];
const events = sortEvents(
issueCommentEvents.filter((event) =>
event.tags.some((tag) => tag[0] === "e" && tag[1] === issue.id),
),
);
for (const event of operations) {
for (const event of events) {
const labels = getAllTags(event, "t");
const isAssignment = labels.includes(ISSUE_ASSIGNMENT_LABEL);
const isUnassignment = labels.includes(ISSUE_UNASSIGNMENT_LABEL);
@@ -112,6 +116,17 @@ function assigneesForIssue(issue, issueCommentEvents) {
);
const isSelfOperation = pubkeys.length === 1 && pubkeys[0] === signer;
if (!allowedActors.has(signer) && !isSelfOperation) continue;
const operation = { isAssignment, pubkeys };
if (allowedActors.has(signer)) {
authoritativeOperations.push(operation);
} else {
selfServiceOperations.push(operation);
}
}
for (const { isAssignment, pubkeys } of [
...selfServiceOperations,
...authoritativeOperations,
]) {
for (const pubkey of pubkeys) {
if (isAssignment) {
assignees.add(pubkey);
@@ -46,6 +46,31 @@ function statusEvent({ kind, pubkey, createdAt }) {
};
}
function assignmentComment(
pubkey,
assignees,
id,
label = ISSUE_ASSIGNMENT_LABEL,
createdAt = 200,
) {
return {
id,
kind: 1,
pubkey,
created_at: createdAt,
content:
label === ISSUE_ASSIGNMENT_LABEL
? "Assigned this issue"
: "Unassigned this issue",
tags: [
["e", "e".repeat(64), "", "root"],
["a", REPO_ADDRESS],
...assignees.map((value) => ["p", value]),
["t", label],
],
};
}
test("ignores status events from a different pubkey", () => {
const attackerClosed = statusEvent({
kind: 1632,
@@ -157,28 +182,6 @@ test("assignees follow trusted assignment operations in deterministic order", ()
const assignee = "d".repeat(64);
const otherAssignee = "f".repeat(64);
const volunteer = "5".repeat(64);
const assignmentComment = (
pubkey,
assignees,
id,
label = ISSUE_ASSIGNMENT_LABEL,
createdAt = 200,
) => ({
id,
kind: 1,
pubkey,
created_at: createdAt,
content:
label === ISSUE_ASSIGNMENT_LABEL
? "Assigned this issue"
: "Unassigned this issue",
tags: [
["e", "e".repeat(64), "", "root"],
["a", REPO_ADDRESS],
...assignees.map((value) => ["p", value]),
["t", label],
],
});
const issue = eventToProjectIssue(
issueEvent(),
@@ -246,6 +249,52 @@ test("assignees follow trusted assignment operations in deterministic order", ()
assert.deepEqual(issue.assignees.sort(), [AUTHOR, assignee].sort());
});
test("owner unassignment overrides a future-dated self-assignment", () => {
const volunteer = "5".repeat(64);
const issue = eventToProjectIssue(
issueEvent(),
[],
[
assignmentComment(
volunteer,
[volunteer],
"future-self-assign",
undefined,
1_000,
),
assignmentComment(
OWNER,
[volunteer],
"owner-unassign",
ISSUE_UNASSIGNMENT_LABEL,
200,
),
],
);
assert.deepEqual(issue.assignees, []);
});
test("owner assignment overrides a future-dated self-unassignment", () => {
const volunteer = "5".repeat(64);
const issue = eventToProjectIssue(
issueEvent(),
[],
[
assignmentComment(
volunteer,
[volunteer],
"future-self-unassign",
ISSUE_UNASSIGNMENT_LABEL,
1_000,
),
assignmentComment(OWNER, [volunteer], "owner-assign", undefined, 200),
],
);
assert.deepEqual(issue.assignees, [volunteer]);
});
test("issue recipients remain notification routing, not assignments", () => {
const recipient = "d".repeat(64);
const otherRecipient = "f".repeat(64);
@@ -177,14 +177,14 @@ export function ProjectDetailScreen(props: ProjectDetailScreenProps) {
const [selectedPullRequestId, setSelectedPullRequestId] = React.useState<
string | null
>(pullRequestId ?? null);
React.useEffect(
() => setSelectedPullRequestId(pullRequestId ?? null),
[pullRequestId],
);
const [selectedIssueId, setSelectedIssueId] = React.useState<string | null>(
issueId ?? null,
);
React.useEffect(() => setSelectedIssueId(issueId ?? null), [issueId]);
// biome-ignore lint/correctness/useExhaustiveDependencies: the transient request ID deliberately reapplies an unchanged entity selection.
React.useEffect(() => {
setSelectedPullRequestId(pullRequestId ?? null);
setSelectedIssueId(issueId ?? null);
}, [entityNavigationId, issueId, pullRequestId]);
const [selectedCommitHash, setSelectedCommitHash] = React.useState<
string | null
>(commitHash ?? null);
@@ -224,6 +224,7 @@ export function ProjectsView() {
...(projectsWorkItemsQuery.data?.issues.items.flatMap(({ issue }) => [
issue.author,
...issue.recipients,
...issue.assignees,
...issue.comments.map((comment) => comment.author),
]) ?? []),
].map(normalizePubkey),
@@ -26,9 +26,9 @@ export function useOpenEntityLink(): (link: ParsedEntityLink) => void {
? link.tab
: undefined;
void goProject(entityLinkProjectRouteId(link), {
entityNavigationId: crypto.randomUUID(),
...(tab
? {
entityNavigationId: crypto.randomUUID(),
tab,
}
: {}),
@@ -17,6 +17,8 @@ const DEFAULT_MOCK_PUBKEY = "deadbeef".repeat(8);
const REPO_ADDRESS = `30617:${ALICE_PUBKEY}:relay-tools`;
const PR_ID = `e0${"ca4d".repeat(15)}ff`; // 64-hex event id
const PR_SUBJECT = "Restore recipient-side entity cards";
const ISSUE_ID = `f0${"1a2b".repeat(15)}ee`; // 64-hex event id
const ISSUE_SUBJECT = "Reopen identical issue links";
test("agent-style message with bare buzz:// links renders entity cards without snapshot tags", async ({
page,
@@ -167,14 +169,56 @@ test("desktop composer shows entity card and send is not blocked by missing snap
});
});
test("reopening the same entity link reapplies its workspace tab", async ({
test("reopening the same entity link reapplies its workspace state", async ({
page,
}) => {
const repoAddress = `30617:${DEFAULT_MOCK_PUBKEY}:buzz`;
await page.addInitScript(
({ issueId, issueSubject, prId, prSubject, repoAddress, owner }) => {
const createdAt = Math.floor(Date.now() / 1000) - 60;
window.__BUZZ_E2E_EXTRA_PROJECT_EVENTS__ = [
{
id: prId,
kind: 1618, // KIND_GIT_PULL_REQUEST
pubkey: owner,
created_at: createdAt,
content: "PR body",
tags: [
["a", repoAddress],
["subject", prSubject],
["c", "abc123".padEnd(40, "0")],
["branch-name", "fix/reopen-entity-link"],
],
},
{
id: issueId,
kind: 1621, // KIND_GIT_ISSUE
pubkey: owner,
created_at: createdAt,
content: "Issue body",
tags: [
["a", repoAddress],
["subject", issueSubject],
],
},
];
},
{
issueId: ISSUE_ID,
issueSubject: ISSUE_SUBJECT,
prId: PR_ID,
prSubject: PR_SUBJECT,
repoAddress,
owner: DEFAULT_MOCK_PUBKEY,
},
);
await installMockBridge(page);
await page.goto("/", { waitUntil: "domcontentloaded" });
await expect(page.getByTestId("open-projects-view")).toBeVisible();
const link = `buzz://repo?owner=${DEFAULT_MOCK_PUBKEY}&d=buzz&tab=prs`;
const emitEntityLink = async () => {
const repoLink = `buzz://repo?owner=${DEFAULT_MOCK_PUBKEY}&d=buzz&tab=prs`;
const prLink = `buzz://pr?id=${PR_ID}&owner=${DEFAULT_MOCK_PUBKEY}&d=buzz`;
const issueLink = `buzz://issue?id=${ISSUE_ID}&owner=${DEFAULT_MOCK_PUBKEY}&d=buzz`;
const emitEntityLink = async (link: string) => {
await page.waitForFunction(
() => typeof window.__TAURI_INTERNALS__?.invoke === "function",
);
@@ -188,7 +232,7 @@ test("reopening the same entity link reapplies its workspace tab", async ({
);
};
await emitEntityLink();
await emitEntityLink(repoLink);
const pullRequestsTab = page.getByRole("tab", {
name: "Pull Request",
exact: true,
@@ -204,13 +248,31 @@ test("reopening the same entity link reapplies its workspace tab", async ({
"true",
);
await emitEntityLink();
await emitEntityLink(repoLink);
await expect(pullRequestsTab).toHaveAttribute("aria-selected", "true");
const filesTab = page.getByRole("tab", { name: "Files", exact: true });
await filesTab.click();
await expect(filesTab).toHaveAttribute("aria-selected", "true");
await emitEntityLink();
await emitEntityLink(repoLink);
await expect(pullRequestsTab).toHaveAttribute("aria-selected", "true");
await emitEntityLink(prLink);
const prHeading = page.getByRole("heading", { name: PR_SUBJECT });
await expect(prHeading).toBeVisible();
await breadcrumb
.getByRole("button", { name: "Pull Request", exact: true })
.click();
await expect(prHeading).toHaveCount(0);
await emitEntityLink(prLink);
await expect(prHeading).toBeVisible();
await emitEntityLink(issueLink);
const issueHeading = page.getByRole("heading", { name: ISSUE_SUBJECT });
await expect(issueHeading).toBeVisible();
await breadcrumb.getByRole("button", { name: "Issues", exact: true }).click();
await expect(issueHeading).toHaveCount(0);
await emitEntityLink(issueLink);
await expect(issueHeading).toBeVisible();
});