fix: distinguish completed review requests in merge request status
This commit is contained in:
@@ -0,0 +1,76 @@
|
|||||||
|
import type {
|
||||||
|
MergeRequestDetail,
|
||||||
|
MergeRequestThreadEvent,
|
||||||
|
} from "./api/merge-requests.ts";
|
||||||
|
|
||||||
|
function isCurrentSourceEvent(
|
||||||
|
event: MergeRequestThreadEvent,
|
||||||
|
source: string,
|
||||||
|
): boolean {
|
||||||
|
return event.subject_ref === source;
|
||||||
|
}
|
||||||
|
|
||||||
|
function requestHasTerminalOutcome(
|
||||||
|
thread: MergeRequestThreadEvent[],
|
||||||
|
requestEventId: unknown,
|
||||||
|
): boolean {
|
||||||
|
return thread.some((event) =>
|
||||||
|
(event.kind === "review" || event.kind === "review_cancelled") &&
|
||||||
|
event.request_event_id === requestEventId
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function sourceReviewFreshness(
|
||||||
|
mergeRequest: MergeRequestDetail,
|
||||||
|
): string {
|
||||||
|
const source = mergeRequest.source.ref;
|
||||||
|
if (!source) return "Source review unavailable: selector_from is unresolved.";
|
||||||
|
|
||||||
|
const effectiveReview = [...mergeRequest.thread].reverse().find((event) => {
|
||||||
|
if (event.kind !== "review" || !isCurrentSourceEvent(event, source)) {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return !mergeRequest.thread.some(
|
||||||
|
(candidate) =>
|
||||||
|
candidate.kind === "review_revoked" &&
|
||||||
|
candidate.review_event_id === event.event_id,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
if (effectiveReview) {
|
||||||
|
return effectiveReview.decision === "approve"
|
||||||
|
? `Current source approved at exact ref ${source}.`
|
||||||
|
: `Current source requests changes at exact ref ${source}.`;
|
||||||
|
}
|
||||||
|
|
||||||
|
const latestEvidence = [...mergeRequest.thread].reverse().find(
|
||||||
|
(event) =>
|
||||||
|
(event.kind === "review" || event.kind === "review_requested") &&
|
||||||
|
typeof event.subject_ref === "string",
|
||||||
|
);
|
||||||
|
if (latestEvidence?.subject_ref && latestEvidence.subject_ref !== source) {
|
||||||
|
return `Fresh source review required: selector_from moved from ${latestEvidence.subject_ref} to ${source}.`;
|
||||||
|
}
|
||||||
|
|
||||||
|
const pendingRequest = [...mergeRequest.thread].reverse().find((event) =>
|
||||||
|
event.kind === "review_requested" &&
|
||||||
|
isCurrentSourceEvent(event, source) &&
|
||||||
|
!requestHasTerminalOutcome(mergeRequest.thread, event.event_id)
|
||||||
|
);
|
||||||
|
if (pendingRequest) {
|
||||||
|
return `Current source review pending for exact ref ${source}.`;
|
||||||
|
}
|
||||||
|
|
||||||
|
return `Fresh source review required: no effective verdict exists for ${source}.`;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function targetIntegrationStatus(
|
||||||
|
mergeRequest: MergeRequestDetail,
|
||||||
|
): string {
|
||||||
|
if (mergeRequest.state === "merged") {
|
||||||
|
return "Target integration recorded by CompleteMergeRequest.";
|
||||||
|
}
|
||||||
|
if (!mergeRequest.target.ref) {
|
||||||
|
return "Target integration unavailable: selector_to is unresolved.";
|
||||||
|
}
|
||||||
|
return `Target integration awaits Orchestrator action at ${mergeRequest.target.ref}. Target-only movement refreshes integration evidence; it does not invalidate approval for an unchanged source.`;
|
||||||
|
}
|
||||||
@@ -37,6 +37,9 @@ const detailPage = await Deno.readTextFile(
|
|||||||
import.meta.url,
|
import.meta.url,
|
||||||
),
|
),
|
||||||
);
|
);
|
||||||
|
const statusProjection = await Deno.readTextFile(
|
||||||
|
new URL("../merge-request-status.ts", import.meta.url),
|
||||||
|
);
|
||||||
const sidebar = await Deno.readTextFile(
|
const sidebar = await Deno.readTextFile(
|
||||||
new URL("../sidebar/WorkspaceSidebar.svelte", import.meta.url),
|
new URL("../sidebar/WorkspaceSidebar.svelte", import.meta.url),
|
||||||
);
|
);
|
||||||
@@ -72,7 +75,12 @@ Deno.test("Workspace exposes Merge Request collection and detail pages", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
Deno.test("Merge Request UI separates source review freshness from target integration", () => {
|
Deno.test("Merge Request UI separates source review freshness from target integration", () => {
|
||||||
for (const source of [detailPage, ticketPage]) {
|
assert(
|
||||||
|
detailPage.includes("sourceReviewFreshness") &&
|
||||||
|
detailPage.includes("targetIntegrationStatus"),
|
||||||
|
"MR detail page does not render authority status projections",
|
||||||
|
);
|
||||||
|
for (const source of [`${detailPage}\n${statusProjection}`, ticketPage]) {
|
||||||
assert(
|
assert(
|
||||||
source.includes("Fresh source review required"),
|
source.includes("Fresh source review required"),
|
||||||
"missing source-review freshness diagnostic",
|
"missing source-review freshness diagnostic",
|
||||||
@@ -88,11 +96,11 @@ Deno.test("Merge Request UI separates source review freshness from target integr
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
assert(
|
assert(
|
||||||
detailPage.includes("selector_from moved from"),
|
statusProjection.includes("selector_from moved from"),
|
||||||
"source ref mismatch is not explained",
|
"source ref mismatch is not explained",
|
||||||
);
|
);
|
||||||
assert(
|
assert(
|
||||||
detailPage.includes("CompleteMergeRequest"),
|
statusProjection.includes("CompleteMergeRequest"),
|
||||||
"target integration authority is not named",
|
"target integration authority is not named",
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|||||||
+4
-48
@@ -1,8 +1,9 @@
|
|||||||
<script lang="ts">
|
<script lang="ts">
|
||||||
|
import { mergeRequestPagePath } from "$lib/workspace/api/merge-requests";
|
||||||
import {
|
import {
|
||||||
mergeRequestPagePath,
|
sourceReviewFreshness,
|
||||||
type MergeRequestDetail,
|
targetIntegrationStatus,
|
||||||
} from "$lib/workspace/api/merge-requests";
|
} from "$lib/workspace/merge-request-status";
|
||||||
import type { PageData } from "./$types";
|
import type { PageData } from "./$types";
|
||||||
|
|
||||||
let { data }: { data: PageData } = $props();
|
let { data }: { data: PageData } = $props();
|
||||||
@@ -17,51 +18,6 @@
|
|||||||
return typeof value === "string" && value.length > 0 ? value : null;
|
return typeof value === "string" && value.length > 0 ? value : null;
|
||||||
}
|
}
|
||||||
|
|
||||||
function sourceReviewFreshness(mergeRequest: MergeRequestDetail): string {
|
|
||||||
const source = mergeRequest.source.ref;
|
|
||||||
if (!source) return "Source review unavailable: selector_from is unresolved.";
|
|
||||||
|
|
||||||
const effectiveReview = [...mergeRequest.thread].reverse().find((event) => {
|
|
||||||
if (event.kind !== "review" || event.subject_ref !== source) return false;
|
|
||||||
return !mergeRequest.thread.some(
|
|
||||||
(candidate) =>
|
|
||||||
candidate.kind === "review_revoked" &&
|
|
||||||
candidate.review_event_id === event.event_id,
|
|
||||||
);
|
|
||||||
});
|
|
||||||
if (effectiveReview) {
|
|
||||||
return effectiveReview.decision === "approve"
|
|
||||||
? `Current source approved at exact ref ${source}.`
|
|
||||||
: `Current source requests changes at exact ref ${source}.`;
|
|
||||||
}
|
|
||||||
|
|
||||||
const latestEvidence = [...mergeRequest.thread].reverse().find(
|
|
||||||
(event) =>
|
|
||||||
(event.kind === "review" || event.kind === "review_requested") &&
|
|
||||||
typeof event.subject_ref === "string",
|
|
||||||
);
|
|
||||||
if (latestEvidence?.subject_ref && latestEvidence.subject_ref !== source) {
|
|
||||||
return `Fresh source review required: selector_from moved from ${latestEvidence.subject_ref} to ${source}.`;
|
|
||||||
}
|
|
||||||
if (
|
|
||||||
mergeRequest.thread.some(
|
|
||||||
(event) => event.kind === "review_requested" && event.subject_ref === source,
|
|
||||||
)
|
|
||||||
) {
|
|
||||||
return `Current source review pending for exact ref ${source}.`;
|
|
||||||
}
|
|
||||||
return `Fresh source review required: no effective verdict exists for ${source}.`;
|
|
||||||
}
|
|
||||||
|
|
||||||
function targetIntegrationStatus(mergeRequest: MergeRequestDetail): string {
|
|
||||||
if (mergeRequest.state === "merged") {
|
|
||||||
return "Target integration recorded by CompleteMergeRequest.";
|
|
||||||
}
|
|
||||||
if (!mergeRequest.target.ref) {
|
|
||||||
return "Target integration unavailable: selector_to is unresolved.";
|
|
||||||
}
|
|
||||||
return `Target integration awaits Orchestrator action at ${mergeRequest.target.ref}. Target-only movement refreshes integration evidence; it does not invalidate approval for an unchanged source.`;
|
|
||||||
}
|
|
||||||
</script>
|
</script>
|
||||||
|
|
||||||
<svelte:head><title>Merge Request · Yoi</title></svelte:head>
|
<svelte:head><title>Merge Request · Yoi</title></svelte:head>
|
||||||
|
|||||||
@@ -0,0 +1,142 @@
|
|||||||
|
/// <reference lib="deno.ns" />
|
||||||
|
|
||||||
|
import type {
|
||||||
|
MergeRequestDetail,
|
||||||
|
MergeRequestThreadEvent,
|
||||||
|
} from "../src/lib/workspace/api/merge-requests.ts";
|
||||||
|
import { sourceReviewFreshness } from "../src/lib/workspace/merge-request-status.ts";
|
||||||
|
|
||||||
|
function assertEquals(actual: unknown, expected: unknown): void {
|
||||||
|
if (actual !== expected) {
|
||||||
|
throw new Error(
|
||||||
|
`expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function event(
|
||||||
|
kind: string,
|
||||||
|
fields: Record<string, unknown>,
|
||||||
|
): MergeRequestThreadEvent {
|
||||||
|
return { kind, sequence: 1, at: "2026-09-01T00:00:00Z", ...fields };
|
||||||
|
}
|
||||||
|
|
||||||
|
function detail(thread: MergeRequestThreadEvent[]): MergeRequestDetail {
|
||||||
|
return {
|
||||||
|
merge_request_id: "MR-1",
|
||||||
|
workspace_id: "W",
|
||||||
|
repository_id: "main",
|
||||||
|
ticket_ids: ["T-1"],
|
||||||
|
selector_from: "work/ticket",
|
||||||
|
selector_to: "develop",
|
||||||
|
state: "open",
|
||||||
|
opened_by: {
|
||||||
|
runtime_id: "runtime",
|
||||||
|
worker_id: "worker",
|
||||||
|
assignment_id: "assignment",
|
||||||
|
},
|
||||||
|
created_at: "2026-09-01T00:00:00Z",
|
||||||
|
updated_at: "2026-09-01T00:00:00Z",
|
||||||
|
source: {
|
||||||
|
status: "known",
|
||||||
|
ref: "source-2",
|
||||||
|
observed_at: "2026-09-01T00:00:00Z",
|
||||||
|
},
|
||||||
|
target: {
|
||||||
|
status: "known",
|
||||||
|
ref: "target-2",
|
||||||
|
observed_at: "2026-09-01T00:00:00Z",
|
||||||
|
},
|
||||||
|
linked_tickets: [{ ticket_id: "T-1", key: "T-1" }],
|
||||||
|
thread,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
Deno.test("revoked review requires a fresh review instead of appearing pending", () => {
|
||||||
|
const mergeRequest = detail([
|
||||||
|
event("review_requested", {
|
||||||
|
event_id: "request-1",
|
||||||
|
subject_ref: "source-2",
|
||||||
|
}),
|
||||||
|
event("review", {
|
||||||
|
event_id: "review-1",
|
||||||
|
request_event_id: "request-1",
|
||||||
|
subject_ref: "source-2",
|
||||||
|
decision: "approve",
|
||||||
|
}),
|
||||||
|
event("review_revoked", {
|
||||||
|
event_id: "revoke-1",
|
||||||
|
review_event_id: "review-1",
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
|
||||||
|
assertEquals(
|
||||||
|
sourceReviewFreshness(mergeRequest),
|
||||||
|
"Fresh source review required: no effective verdict exists for source-2.",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
Deno.test("unresolved review request for the current source is pending", () => {
|
||||||
|
const mergeRequest = detail([
|
||||||
|
event("review_requested", {
|
||||||
|
event_id: "request-2",
|
||||||
|
subject_ref: "source-2",
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
|
||||||
|
assertEquals(
|
||||||
|
sourceReviewFreshness(mergeRequest),
|
||||||
|
"Current source review pending for exact ref source-2.",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
Deno.test("completed or cancelled request is not projected as pending", () => {
|
||||||
|
const approved = detail([
|
||||||
|
event("review_requested", {
|
||||||
|
event_id: "request-3",
|
||||||
|
subject_ref: "source-2",
|
||||||
|
}),
|
||||||
|
event("review", {
|
||||||
|
event_id: "review-3",
|
||||||
|
request_event_id: "request-3",
|
||||||
|
subject_ref: "source-2",
|
||||||
|
decision: "approve",
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
assertEquals(
|
||||||
|
sourceReviewFreshness(approved),
|
||||||
|
"Current source approved at exact ref source-2.",
|
||||||
|
);
|
||||||
|
|
||||||
|
const cancelled = detail([
|
||||||
|
event("review_requested", {
|
||||||
|
event_id: "request-4",
|
||||||
|
subject_ref: "source-2",
|
||||||
|
}),
|
||||||
|
event("review_cancelled", {
|
||||||
|
event_id: "cancel-4",
|
||||||
|
request_event_id: "request-4",
|
||||||
|
subject_ref: "source-2",
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
assertEquals(
|
||||||
|
sourceReviewFreshness(cancelled),
|
||||||
|
"Fresh source review required: no effective verdict exists for source-2.",
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
Deno.test("source movement explains the exact stale and current refs", () => {
|
||||||
|
const mergeRequest = detail([
|
||||||
|
event("review", {
|
||||||
|
event_id: "review-old",
|
||||||
|
request_event_id: "request-old",
|
||||||
|
subject_ref: "source-1",
|
||||||
|
decision: "approve",
|
||||||
|
}),
|
||||||
|
]);
|
||||||
|
|
||||||
|
assertEquals(
|
||||||
|
sourceReviewFreshness(mergeRequest),
|
||||||
|
"Fresh source review required: selector_from moved from source-1 to source-2.",
|
||||||
|
);
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user