From 2a3e3b377cd7a17f11d6aef4e8eb6a2fee45fcd3 Mon Sep 17 00:00:00 2001 From: idy Date: Tue, 4 Aug 2026 07:45:07 +0800 Subject: [PATCH] workflows: snapshot native Issue dependencies Issue Review omitted GitHub's native dependency connections, so reviewers could not verify real blocked-by ordering and readiness could remain unchanged when those relationships changed. - Capture bounded blockedBy and blocking connections in review and verification queries - Normalize, deduplicate, sort, hash, and fail closed on incomplete dependencies - Upgrade the Issue snapshot schema and cover live identity-changing boundaries Generated with [Codex](https://github.com/openai) --- .github/scripts/issue-review/common.mjs | 62 ++++++++++---- .github/scripts/issue-review/test.mjs | 71 +++++++++++++++ .github/scripts/pr-readiness/test.mjs | 100 ++++++++++++++++++++++ .github/scripts/pr-readiness/verify.mjs | 28 ++++++ .github/workflows/codex-openai-review.yml | 28 ++++++ README.md | 4 + 6 files changed, 275 insertions(+), 18 deletions(-) diff --git a/.github/scripts/issue-review/common.mjs b/.github/scripts/issue-review/common.mjs index 31a45d2..4de3f27 100644 --- a/.github/scripts/issue-review/common.mjs +++ b/.github/scripts/issue-review/common.mjs @@ -1,6 +1,6 @@ import crypto from "node:crypto"; -export const ISSUE_REVIEW_SCHEMA_VERSION = 3; +export const ISSUE_REVIEW_SCHEMA_VERSION = 4; export const PREFIXED_TITLE = /^[a-z][a-z0-9-]*(?:\/[a-z][a-z0-9-]*)*: \S.*$/; export function sha256(value) { @@ -11,6 +11,26 @@ function blocker(code, message) { return { source: "issue-format", code, message }; } +function normalizeIssueReferences(references, defaultRepository) { + return [...new Map((Array.isArray(references) ? references : []) + .map((reference) => ({ + repository: String(reference?.repository ?? defaultRepository), + number: Number(reference?.number), + state: String(reference?.state ?? "OPEN").toUpperCase(), + })) + .filter((reference) => ( + reference.repository + && Number.isSafeInteger(reference.number) + )) + .map((reference) => [ + `${reference.repository.toLowerCase()}#${reference.number}`, + reference, + ])).values()].sort((left, right) => ( + left.repository.localeCompare(right.repository) + || left.number - right.number + )); +} + export function issueSnapshot(issue) { const repository = String(issue.repository ?? ""); const rawSubIssues = Array.isArray(issue.sub_issues) @@ -20,23 +40,9 @@ export function issueSnapshot(issue) { number, state: "OPEN", })); - const subIssues = [...new Map(rawSubIssues - .map((subIssue) => ({ - repository: String(subIssue?.repository ?? repository), - number: Number(subIssue?.number), - state: String(subIssue?.state ?? "OPEN").toUpperCase(), - })) - .filter((subIssue) => ( - subIssue.repository - && Number.isSafeInteger(subIssue.number) - )) - .map((subIssue) => [ - `${subIssue.repository.toLowerCase()}#${subIssue.number}`, - subIssue, - ])).values()].sort((left, right) => ( - left.repository.localeCompare(right.repository) - || left.number - right.number - )); + const subIssues = normalizeIssueReferences(rawSubIssues, repository); + const blockedBy = normalizeIssueReferences(issue.blocked_by, repository); + const blocking = normalizeIssueReferences(issue.blocking, repository); return { repository, number: Number(issue.number), @@ -51,6 +57,14 @@ export function issueSnapshot(issue) { : Number(issue.sub_issue_count), sub_issue_numbers: subIssues.map((subIssue) => subIssue.number), sub_issues: subIssues, + blocked_by_count: issue.blocked_by_count == null + ? blockedBy.length + : Number(issue.blocked_by_count), + blocked_by: blockedBy, + blocking_count: issue.blocking_count == null + ? blocking.length + : Number(issue.blocking_count), + blocking, }; } @@ -73,6 +87,18 @@ export function analyzeIssue(issue) { "The workflow could not snapshot every native sub-issue and must fail closed.", )); } + if (snapshot.blocked_by_count > snapshot.blocked_by.length) { + blockers.push(blocker( + "blocked-by-truncated", + "The workflow could not snapshot every native blocking prerequisite and must fail closed.", + )); + } + if (snapshot.blocking_count > snapshot.blocking.length) { + blockers.push(blocker( + "blocking-truncated", + "The workflow could not snapshot every natively blocked Issue and must fail closed.", + )); + } return { schema_version: ISSUE_REVIEW_SCHEMA_VERSION, diff --git a/.github/scripts/issue-review/test.mjs b/.github/scripts/issue-review/test.mjs index 94a4725..2efc4a8 100644 --- a/.github/scripts/issue-review/test.mjs +++ b/.github/scripts/issue-review/test.mjs @@ -3,7 +3,9 @@ import assert from "node:assert/strict"; import { analyzeIssue, + issueSnapshot, issueSnapshotSha256, + ISSUE_REVIEW_SCHEMA_VERSION, } from "./common.mjs"; const validBody = [ @@ -39,7 +41,12 @@ const valid = { parent_number: null, sub_issue_numbers: [], sub_issues: [], + blocked_by_count: 0, + blocked_by: [], + blocking_count: 0, + blocking: [], }; +assert.equal(ISSUE_REVIEW_SCHEMA_VERSION, 4); assert.deepEqual(analyzeIssue(valid).deterministic_blockers, []); assert.equal(issueSnapshotSha256(valid), issueSnapshotSha256({ ...valid })); assert.notEqual( @@ -54,6 +61,56 @@ assert.notEqual( issueSnapshotSha256(valid), issueSnapshotSha256({ ...valid, state: "CLOSED" }), ); +const dependencies = { + ...valid, + blocked_by_count: 3, + blocked_by: [ + { repository: "zeta/example", number: 20, state: "open" }, + { repository: "Alpha/example", number: 30, state: "CLOSED" }, + { repository: "zeta/example", number: 20, state: "open" }, + { repository: "Alpha/example", number: 10, state: "open" }, + ], + blocking_count: 1, + blocking: [ + { repository: "GizClaw/example", number: 40, state: "open" }, + ], +}; +assert.deepEqual(issueSnapshot(dependencies).blocked_by, [ + { repository: "Alpha/example", number: 10, state: "OPEN" }, + { repository: "Alpha/example", number: 30, state: "CLOSED" }, + { repository: "zeta/example", number: 20, state: "OPEN" }, +]); +assert.deepEqual(issueSnapshot(dependencies).blocking, [ + { repository: "GizClaw/example", number: 40, state: "OPEN" }, +]); +assert.deepEqual(analyzeIssue(dependencies).deterministic_blockers, []); +assert.notEqual( + issueSnapshotSha256(dependencies), + issueSnapshotSha256({ + ...dependencies, + blocked_by: dependencies.blocked_by.map((dependency, index) => ( + index === 1 ? { ...dependency, state: "OPEN" } : dependency + )), + }), +); +assert.notEqual( + issueSnapshotSha256(dependencies), + issueSnapshotSha256({ + ...dependencies, + blocked_by_count: 2, + blocked_by: dependencies.blocked_by.slice(1), + }), +); +assert.notEqual( + issueSnapshotSha256(dependencies), + issueSnapshotSha256({ + ...dependencies, + blocked_by_count: 0, + blocked_by: [], + blocking_count: dependencies.blocked_by_count, + blocking: dependencies.blocked_by, + }), +); assert.ok(analyzeIssue({ ...valid, body_truncated: true }) .deterministic_blockers.some((item) => item.code === "issue-body-truncated")); assert.deepEqual( @@ -68,6 +125,20 @@ assert.ok(analyzeIssue({ }).deterministic_blockers.some( (item) => item.code === "sub-issues-truncated", )); +assert.ok(analyzeIssue({ + ...valid, + blocked_by_count: 2, + blocked_by: [{ repository: valid.repository, number: 20, state: "OPEN" }], +}).deterministic_blockers.some( + (item) => item.code === "blocked-by-truncated", +)); +assert.ok(analyzeIssue({ + ...valid, + blocking_count: 2, + blocking: [{ repository: valid.repository, number: 20, state: "OPEN" }], +}).deterministic_blockers.some( + (item) => item.code === "blocking-truncated", +)); assert.deepEqual(analyzeIssue({ ...valid, issue_type: "Task", diff --git a/.github/scripts/pr-readiness/test.mjs b/.github/scripts/pr-readiness/test.mjs index bc20033..81679db 100644 --- a/.github/scripts/pr-readiness/test.mjs +++ b/.github/scripts/pr-readiness/test.mjs @@ -46,6 +46,15 @@ const input = { issue_type: "Feature", sub_issue_numbers: [], sub_issues: [], + blocked_by_count: 2, + blocked_by: [ + { repository: "GizClaw/example", number: 9, state: "OPEN" }, + { repository: "Other/example", number: 30, state: "CLOSED" }, + ], + blocking_count: 1, + blocking: [ + { repository: "GizClaw/example", number: 20, state: "OPEN" }, + ], }], }; const context = { @@ -88,6 +97,10 @@ const workflowSource = fs.readFileSync( ); for (const source of [verifySource, workflowSource]) { assert.match(source, /closingIssuesReferences\(first: 100\)/); + assert.match(source, /blockedBy\(first: 100\)/); + assert.match(source, /blocking\(first: 100\)/); + assert.match(source, /blocked_by_count:\s*issue\.blockedBy\.totalCount/); + assert.match(source, /blocking_count:\s*issue\.blocking\.totalCount/); assert.doesNotMatch(source, /closingIssuesReferences\.nodes\s*\.slice\(/); } assert.deepEqual(context.readiness.deterministic_blockers, []); @@ -238,6 +251,30 @@ assert.ok(blockerCodes(analyzePullRequest({ sub_issue_numbers: Array.from({ length: 100 }, (_, index) => index + 1), }], })).includes("sub-issues-truncated")); +assert.ok(blockerCodes(analyzePullRequest({ + ...input, + linked_issues: [{ + ...input.linked_issues[0], + blocked_by_count: 101, + blocked_by: Array.from({ length: 100 }, (_, index) => ({ + repository: input.repository, + number: index + 1, + state: "OPEN", + })), + }], +})).includes("blocked-by-truncated")); +assert.ok(blockerCodes(analyzePullRequest({ + ...input, + linked_issues: [{ + ...input.linked_issues[0], + blocking_count: 101, + blocking: Array.from({ length: 100 }, (_, index) => ({ + repository: input.repository, + number: index + 1, + state: "OPEN", + })), + }], +})).includes("blocking-truncated")); assert.ok(blockerCodes(analyzePullRequest({ ...input, linked_issue_count: 2, @@ -309,6 +346,30 @@ assert.notEqual( }], }).snapshot_sha256, ); +assert.notEqual( + analyzePullRequest(input).snapshot_sha256, + analyzePullRequest({ + ...input, + linked_issues: [{ + ...input.linked_issues[0], + blocked_by: input.linked_issues[0].blocked_by.map( + (dependency, index) => index === 0 + ? { ...dependency, state: "CLOSED" } + : dependency, + ), + }], + }).snapshot_sha256, +); +assert.notEqual( + analyzePullRequest(input).snapshot_sha256, + analyzePullRequest({ + ...input, + linked_issues: [{ + ...input.linked_issues[0], + blocked_by_count: input.linked_issues[0].blocked_by_count + 1, + }], + }).snapshot_sha256, +); const cleanReview = { findings: [], @@ -441,6 +502,22 @@ try { issueType: { name: "Feature" }, parent: null, subIssues: { totalCount: 0, nodes: [] }, + blockedBy: { + totalCount: input.linked_issues[0].blocked_by_count, + nodes: input.linked_issues[0].blocked_by.map((dependency) => ({ + repository: { nameWithOwner: dependency.repository }, + number: dependency.number, + state: dependency.state, + })), + }, + blocking: { + totalCount: input.linked_issues[0].blocking_count, + nodes: input.linked_issues[0].blocking.map((dependency) => ({ + repository: { nameWithOwner: dependency.repository }, + number: dependency.number, + state: dependency.state, + })), + }, }], }, reviewThreads: { @@ -491,6 +568,22 @@ try { state: subIssue.state, })), }, + blockedBy: { + totalCount: issue.blocked_by_count ?? 0, + nodes: (issue.blocked_by ?? []).map((dependency) => ({ + repository: { nameWithOwner: dependency.repository }, + number: dependency.number, + state: dependency.state, + })), + }, + blocking: { + totalCount: issue.blocking_count ?? 0, + nodes: (issue.blocking ?? []).map((dependency) => ({ + repository: { nameWithOwner: dependency.repository }, + number: dependency.number, + state: dependency.state, + })), + }, })), }; fs.writeFileSync(fixtureFile, JSON.stringify(manyFixture)); @@ -532,6 +625,13 @@ try { }], }; }); + assertStale((pullRequest) => { + pullRequest.closingIssuesReferences.nodes[0].blockedBy.nodes[0].state = + "CLOSED"; + }); + assertStale((pullRequest) => { + pullRequest.closingIssuesReferences.nodes[0].blocking.totalCount += 1; + }); assertStale((pullRequest) => { pullRequest.reviewThreads.nodes.push({ isResolved: false, diff --git a/.github/scripts/pr-readiness/verify.mjs b/.github/scripts/pr-readiness/verify.mjs index 6c431c6..fd562cb 100644 --- a/.github/scripts/pr-readiness/verify.mjs +++ b/.github/scripts/pr-readiness/verify.mjs @@ -58,6 +58,22 @@ async function fetchPullRequest() { state } } + blockedBy(first: 100) { + totalCount + nodes { + repository { nameWithOwner } + number + state + } + } + blocking(first: 100) { + totalCount + nodes { + repository { nameWithOwner } + number + state + } + } } } reviewThreads(first: 100) { @@ -116,6 +132,18 @@ const linkedIssues = pullRequest.closingIssuesReferences.nodes number: item.number, state: item.state, })), + blocked_by_count: issue.blockedBy.totalCount, + blocked_by: issue.blockedBy.nodes.map((item) => ({ + repository: item.repository.nameWithOwner, + number: item.number, + state: item.state, + })), + blocking_count: issue.blocking.totalCount, + blocking: issue.blocking.nodes.map((item) => ({ + repository: item.repository.nameWithOwner, + number: item.number, + state: item.state, + })), })); const current = analyzePullRequest({ repository: data.repository.nameWithOwner, diff --git a/.github/workflows/codex-openai-review.yml b/.github/workflows/codex-openai-review.yml index c39f994..6bbe89d 100644 --- a/.github/workflows/codex-openai-review.yml +++ b/.github/workflows/codex-openai-review.yml @@ -410,6 +410,22 @@ jobs: state } } + blockedBy(first: 100) { + totalCount + nodes { + repository { nameWithOwner } + number + state + } + } + blocking(first: 100) { + totalCount + nodes { + repository { nameWithOwner } + number + state + } + } } } reviewThreads(first: 100) { @@ -468,6 +484,18 @@ jobs: number: subIssue.number, state: subIssue.state, })), + blocked_by_count: issue.blockedBy.totalCount, + blocked_by: issue.blockedBy.nodes.map((dependency) => ({ + repository: dependency.repository.nameWithOwner, + number: dependency.number, + state: dependency.state, + })), + blocking_count: issue.blocking.totalCount, + blocking: issue.blocking.nodes.map((dependency) => ({ + repository: dependency.repository.nameWithOwner, + number: dependency.number, + state: dependency.state, + })), })), unresolved_openai_thread_count: pullRequest.reviewThreads.nodes .filter((thread) => ( diff --git a/README.md b/README.md index 4082c8a..dfb87a6 100644 --- a/README.md +++ b/README.md @@ -140,6 +140,10 @@ The reviewer preserves every native closing Issue returned by GitHub's maximum 100-node GraphQL page. It fails closed when `totalCount` exceeds the collected nodes, rather than silently reviewing a truncated relationship set. Native sub-issue snapshots use the same 100-node fail-closed bound. +Each linked Issue also preserves its native `blockedBy` and `blocking` +relationships, including repository, Issue number, and state. Both dependency +directions use the same deterministic normalization and 100-node fail-closed +bound, and their normalized state is part of the Issue and readiness hashes. Caller policy can add trusted organization-level constraints. Repository-owned title formats, Issue Types, sections, relationships, ownership rules,