Skip to content

Where people and agents ship software together. The open-source git platform for the whole job: issues, agents, checks and deploys to the edge.

Commit

Web: a work item links each pull request in the repository that has it

Its decisions and merge-queue findings used the item's first repository as the base for every pull request, so a target's #44 was linked as the origin's. Each number is now looked up in the item's repositories and their pull requests; one none of them has, or two do, is not linked.

G1T-Session:
ses_01m56vafhge7ttrgs5288dpxvb
syntaqxcommitted Parent5440b27Browse files
4 files+90−240/4 viewed
+42−20
6969 sessionTree,
7070 titleHint,
7171 titleIsProvisional,
72+ type PullHref,
7273 } from "../lib/coordination";
73−import { sessionPath } from "../lib/work-items";
74+import { sessionPath, sessionsPath } from "../lib/work-items";
7475 import { AgentSetup } from "./agent-setup";
7576 import { Input, SubmitButton, TimeAgo } from "./ui";
7677 import { Alert, AlertDescription, AlertTitle } from "./ui/alert";
119120 }
120121
121122 /** Text with each `#N` linked to what it names, read in the repository at `base` (lib/refs.ts). */
122−export function NumberedText({ text, base }: { text: string; base: string }) {
123+export function NumberedText({ text, base, pullHref }: { text: string; base: string; pullHref?: PullHref }) {
123124 return (
124125 <>
125126 {numberedParts(text).map((part, index) =>
126127 typeof part === "number" ? (
127− <Link key={index} to={`${base}/ref/${part}`} className="font-medium text-fg hover:text-accent">
128+ <NumberLink key={index} to={pullHref ? pullHref(part) : `${base}/ref/${part}`} className="font-medium text-fg hover:text-accent">
128129 #{part}
129− </Link>
130+ </NumberLink>
130131 ) : (
131132 <Fragment key={index}>{part}</Fragment>
132133 ),
571572 note: <MessageSquare size={14} />,
572573 };
573574
575+/** A pull request's number, linked where it is known; plain text where it is not (`pullHref` said null). */
576+function NumberLink({ to, className, children }: { to: string | null; className?: string; children: ReactNode }) {
577+ if (!to) return <span className={className}>{children}</span>;
578+ return (
579+ <Link to={to} className={className}>
580+ {children}
581+ </Link>
582+ );
583+}
584+
585+/** Where pull request `number` is: `pullHref`'s answer when given (a work item's, across its repositories), else under `base`. */
586+function pullAt(base: string, pullHref: PullHref | undefined, number: number): string | null {
587+ return pullHref ? pullHref(number) : `${base}/pull/${number}`;
588+}
589+
574590 /** The pull requests tested together, in queue order: `#41 + #44`. */
575−export function Combination({ numbers, base, culprit }: { numbers: number[]; base: string; culprit?: number | null }) {
591+export function Combination({ numbers, base, culprit, pullHref }: { numbers: number[]; base: string; culprit?: number | null; pullHref?: PullHref }) {
576592 return (
577593 <span className="inline-flex flex-wrap items-center gap-1">
578594 {numbers.map((number, index) => (
579595 <Fragment key={number}>
580596 {index > 0 && <span className="text-faint">+</span>}
581− <Link to={`${base}/pull/${number}`}>
597+ <NumberLink to={pullAt(base, pullHref, number)}>
582598 <Badge shape="tag" tone={number === culprit ? "danger" : "neutral"} className="font-mono hover:border-line-strong">
583599 #{number}
584600 </Badge>
585− </Link>
601+ </NumberLink>
586602 </Fragment>
587603 ))}
588604 </span>
610626 }
611627
612628 /** One decision record: its kind, how sure it is, the sentence, who and when, and what it rests on. */
613−export function DecisionLine({ decision, base }: { decision: Decision; base: string }) {
629+export function DecisionLine({ decision, base, pullHref }: { decision: Decision; base: string; pullHref?: PullHref }) {
614630 const evidence = evidenceOf(decision);
615631 return (
616632 <div className="flex items-start gap-2.5">
620636 <span className="font-medium text-fg">{DECISION_KINDS[decision.kind]}</span>
621637 <ConfidenceBadge confidence={decision.confidence} />
622638 {decision.pull != null && (
623− <Link to={`${base}/pull/${decision.pull}`} className="text-muted hover:text-fg">
639+ <NumberLink to={pullAt(base, pullHref, decision.pull)} className="text-muted hover:text-fg">
624640 #{decision.pull}
625− </Link>
641+ </NumberLink>
626642 )}
627643 </p>
628644 <p className="mt-1 text-sm wrap-break-word">
629− <NumberedText text={decision.summary} base={base} />
645+ <NumberedText text={decision.summary} base={base} pullHref={pullHref} />
630646 </p>
631647 <p className="mt-1 flex flex-wrap items-center gap-x-1.5 gap-y-1 text-xs text-muted">
632648 <span>{decision.actor ?? "g1t"}</span>
635651 </span>
636652 {evidence.combination.length > 1 && (
637653 <span className="inline-flex items-center gap-1.5">
638− · tested as <Combination numbers={evidence.combination} base={base} culprit={decision.kind === "integration_failure" ? decision.pull : null} />
654+ · tested as{" "}
655+ <Combination numbers={evidence.combination} base={base} pullHref={pullHref} culprit={decision.kind === "integration_failure" ? decision.pull : null} />
639656 </span>
640657 )}
641658 {evidence.check && decision.kind === "integration_failure" && (
656673 }
657674
658675 /** Decision records, as a list in a card. */
659−export function DecisionList({ decisions, base }: { decisions: Decision[]; base: string }) {
676+export function DecisionList({ decisions, base, pullHref }: { decisions: Decision[]; base: string; pullHref?: PullHref }) {
660677 return (
661678 <Card asChild divided>
662679 <ol>
663680 {decisions.map((decision) => (
664681 <li key={decision.id} className="px-4 py-3">
665− <DecisionLine decision={decision} base={base} />
682+ <DecisionLine decision={decision} base={base} pullHref={pullHref} />
666683 </li>
667684 ))}
668685 </ol>
681698 fixed,
682699 branch = "main",
683700 className,
701+ pullHref,
684702 }: {
685703 failure: Decision;
686704 base: string;
705+ /** Where each pull request is, for a work item's, whose pull requests are in several repositories. */
706+ pullHref?: PullHref;
687707 /** The branch the combination was built on. */
688708 branch?: string;
689709 /** A fix or a passing combination since, when there is one. */
697717 {failure.pull != null ? (
698718 <span>
699719 <TriangleAlert size={15} className={`mr-2 inline align-[-3px] ${fixed ? "text-info" : "text-danger"}`} />
700− <Link to={`${base}/pull/${failure.pull}`} className="hover:underline">
720+ <NumberLink to={pullAt(base, pullHref, failure.pull)} className="hover:underline">
701721 #{failure.pull}
702− </Link>{" "}
722+ </NumberLink>{" "}
703723 failed in the merge queue {evidence.combination.length > 1 ? "together with others" : "on its own"}
704724 </span>
705725 ) : (
714734 {evidence.combination.length > 0 && (
715735 <p className="flex flex-wrap items-center gap-1.5 text-xs">
716736 Tested as <span className="font-mono text-fg">{branch}</span> <span className="text-faint">+</span>
717− <Combination numbers={evidence.combination} base={base} culprit={failure.pull} />
737+ <Combination numbers={evidence.combination} base={base} pullHref={pullHref} culprit={failure.pull} />
718738 </p>
719739 )}
720740 {evidence.check && (
726746 {evidence.suspects.length > 0 ? (
727747 <>
728748 Fails with{" "}
729− <NumberedText text={evidence.suspects.map((n) => `#${n}`).join(", ")} base={base} />
749+ <NumberedText text={evidence.suspects.map((n) => `#${n}`).join(", ")} base={base} pullHref={pullHref} />
730750 {evidence.bisected ? ", narrowed down by building smaller combinations." : "."}
731751 </>
732752 ) : (
738758 <CircleCheck size={13} className="mr-1.5 inline align-[-2px] text-success" />
739759 {fixed.kind === "integration_passed" ? "Passed together since" : "Fixed since"}:{" "}
740760 <span className="text-muted">
741− <NumberedText text={fixed.summary} base={base} />
761+ <NumberedText text={fixed.summary} base={base} pullHref={pullHref} />
742762 </span>
743763 </p>
744764 )}
917937 )}
918938 {trees.length > sessions.length && (
919939 <p className="mt-2 text-xs text-muted">
920− {trees.length - sessions.length} more on its repositories' Sessions pages
940+ <Link to={sessionsPath(work.item.workspace, { state: "recent", item: String(work.item.number) })} className="hover:text-fg hover:underline">
941+ {trees.length - sessions.length} more in Work's Sessions
942+ </Link>
921943 </p>
922944 )}
923945 </PanelPart>
+17−0
2020 mergeSessions,
2121 numberedParts,
2222 parseBlockedBy,
23+ pullHrefs,
2324 pendingLines,
2425 resolutionOf,
2526 sessionTree,
388389 assert.equal(lines[0].who, "bo");
389390 assert.match(lines[0].text, /claimed #4/);
390391 });
392+
393+test("a work item's pull requests are linked in the repository that has them, and not at all when none or two do", () => {
394+ const href = pullHrefs([
395+ { path: "acme/web", pulls: [{ number: 41 }] },
396+ { path: "acme/api", pulls: [{ number: 44 }, { number: 7 }] },
397+ { path: "acme/docs", pulls: [{ number: 7 }] },
398+ // A repository deleted before work items kept no path.
399+ { path: "", pulls: [{ number: 50 }] },
400+ ]);
401+ assert.equal(href(41), "/acme/web/pull/41");
402+ // A target's pull request is the target's, not the origin's.
403+ assert.equal(href(44), "/acme/api/pull/44");
404+ assert.equal(href(7), null, "two repositories have a #7: neither is guessed");
405+ assert.equal(href(99), null, "unknown");
406+ assert.equal(href(50), null, "a deleted repository");
407+});
+25−0
672672 if (words.some((word) => !/^\d+$/.test(word) || Number(word) <= 0)) return null;
673673 return [...new Set(words.map(Number))];
674674 }
675+
676+/** Where a pull request a work item's decisions name is, by its number: null when it is not known. */
677+export type PullHref = (number: number) => string | null;
678+
679+/**
680+ * Where each of a work item's pull requests is, from its repositories and
681+ * their pull requests: `/<owner>/<repo>/pull/<n>` in the one repository
682+ * that has it. A number none of them has (or that two have, from before
683+ * numbers were the workspace's) is not linked, rather than linked to the
684+ * wrong repository. A deleted repository, with no path, has none.
685+ */
686+export function pullHrefs(repos: readonly { path: string; pulls: readonly { number: number }[] }[]): PullHref {
687+ const where = new Map<number, string | null>();
688+ for (const repo of repos) {
689+ if (!repo.path) continue;
690+ for (const pull of repo.pulls) {
691+ const seen = where.get(pull.number);
692+ where.set(pull.number, seen === undefined || seen === repo.path ? repo.path : null);
693+ }
694+ }
695+ return (number) => {
696+ const path = where.get(number);
697+ return path ? `/${path}/pull/${number}` : null;
698+ };
699+}
+6−4
4242 import { assertSameOrigin, getViewer, requireUser, roleIn } from "../../../lib/session.server";
4343 import { useRefreshWhile } from "../../../lib/refresh";
4444 import { isFailed, part } from "../../../lib/partial";
45−import { parseBlockedBy, resolutionOf } from "../../../lib/coordination";
45+import { parseBlockedBy, pullHrefs, resolutionOf } from "../../../lib/coordination";
4646 import { refPath } from "../../../lib/refs.server";
4747 import { itemAbilities } from "../../../lib/work-access";
4848 import { LINK_ROLES, ROLE_LABELS, itemPath, originOf, repoFilter, sessionPath, shortPath, workPath } from "../../../lib/work-items";
530530 const decisions = coordination ? [...coordination.decisions].reverse() : [];
531531 const failure = coordination?.lastIntegrationFailure ?? null;
532532 const resolution = failure ? resolutionOf(coordination!.decisions, failure) : null;
533− const failureBase = failure ? `/${item.repos[0]?.path ?? workspace}` : "";
533+ // A decision or the merge queue names a pull request by number: link it in the repository that has it, or not at all.
534+ const pullHref = pullHrefs(repoRows.map(({ repo, pulls: repoPulls }) => ({ path: repo.path, pulls: repoPulls })));
535+ const failureBase = `/${originOf(item.repos)?.path || workspace}`;
534536 // Where a pull request or a session would be opened: a target still missing, else where it was filed.
535537 const workIn = missing[0] ?? item.repos.find((repo) => repo.role === "target")?.path ?? originOf(item.repos)?.path ?? null;
536538 const linkError = actionData && "action" in actionData && actionData.action === "link" ? actionData.error : null;
667669 </div>
668670 )}
669671
670− {failure && <IntegrationFailure className="mt-4" failure={failure} base={failureBase} fixed={resolution?.passed ?? resolution?.fix} />}
672+ {failure && <IntegrationFailure className="mt-4" failure={failure} base={failureBase} pullHref={pullHref} fixed={resolution?.passed ?? resolution?.fix} />}
671673
672674 {isFailed(loaderData.coordination) ? (
673675 <>
684686 <p className="text-sm text-muted">Why the work is the way it is, and how sure that is</p>
685687 </div>
686688 <div className="mt-3">
687− <DecisionList decisions={decisions} base={failureBase || `/${item.repos[0]?.path ?? workspace}`} />
689+ <DecisionList decisions={decisions} base={failureBase} pullHref={pullHref} />
688690 </div>
689691 </>
690692 )

Comments

Sign in to comment.