From 45cb5a980cd108178e9fd51b84124485477f2c04 Mon Sep 17 00:00:00 2001 From: Zain Gulel <60632602+zaingulel@users.noreply.github.com> Date: Sun, 16 Aug 2026 14:45:58 +0100 Subject: [PATCH 1/6] Reduce tracker reconciliation GraphQL cost --- scripts/lib/rentcottage-gh-source.mjs | 654 ++++++-- scripts/lib/rentcottage-github-adapter.mjs | 26 +- scripts/lib/rentcottage-github-schema.mjs | 11 +- scripts/rentcottage-gh-source.test.mjs | 1543 ++++++++++++------- scripts/rentcottage-github-adapter.test.mjs | 155 +- 5 files changed, 1615 insertions(+), 774 deletions(-) diff --git a/scripts/lib/rentcottage-gh-source.mjs b/scripts/lib/rentcottage-gh-source.mjs index 91b8a52..d69da94 100644 --- a/scripts/lib/rentcottage-gh-source.mjs +++ b/scripts/lib/rentcottage-gh-source.mjs @@ -17,6 +17,11 @@ import { export { runGh }; +const CONNECTION_PAGE_SIZE = 100; +const CONNECTION_ITEM_LIMIT = 1_000; +const PROJECT_FIELD_VALUE_PAGE_SIZE = 20; +const LINKED_PULL_REQUEST_PAGE_SIZE = 20; + function parseJson(serialized, context) { try { return JSON.parse(serialized); @@ -78,6 +83,280 @@ function hasFreshItemCoordinates(items, project, operation, repository) { ); } +const PROJECT_FIELD_NAMES = ["Area", "Status", "Linked pull requests"]; + +const FIELD_COORDINATE_SELECTION = `field { ... on ProjectV2FieldCommon { id name } }`; +const FIELD_VALUE_SELECTION = ` + __typename + ... on ProjectV2ItemFieldValueCommon { ${FIELD_COORDINATE_SELECTION} } + ... on ProjectV2ItemFieldLabelValue { ${FIELD_COORDINATE_SELECTION} } + ... on ProjectV2ItemFieldMilestoneValue { ${FIELD_COORDINATE_SELECTION} } + ... on ProjectV2ItemFieldPullRequestValue { + ${FIELD_COORDINATE_SELECTION} + pullRequests(first: ${LINKED_PULL_REQUEST_PAGE_SIZE}) { + totalCount nodes { id number url repository { nameWithOwner } } + pageInfo { hasNextPage endCursor } + } + } + ... on ProjectV2ItemFieldRepositoryValue { ${FIELD_COORDINATE_SELECTION} } + ... on ProjectV2ItemFieldReviewerValue { ${FIELD_COORDINATE_SELECTION} } + ... on ProjectV2ItemFieldSingleSelectValue { name optionId } + ... on ProjectV2ItemFieldUserValue { ${FIELD_COORDINATE_SELECTION} } + ... on ProjectV2ItemIssueFieldValue { ${FIELD_COORDINATE_SELECTION} } +`; + +function graphqlArgs(query, variables = {}) { + const args = ["api", "graphql", "-f", `query=${query}`]; + for (const [name, value] of Object.entries(variables)) { + if (value === null || value === undefined) continue; + args.push(Number.isInteger(value) ? "-F" : "-f", `${name}=${value}`); + } + return args; +} + +function requireConnection(connection, context) { + if ( + !isRecord(connection) || + !Number.isInteger(connection.totalCount) || + connection.totalCount < 0 || + !Array.isArray(connection.nodes) || + !isRecord(connection.pageInfo) || + typeof connection.pageInfo.hasNextPage !== "boolean" || + (connection.pageInfo.endCursor !== null && + typeof connection.pageInfo.endCursor !== "string") + ) { + throw new Error(`${context} pagination evidence is invalid`); + } + if (connection.totalCount > CONNECTION_ITEM_LIMIT) + throw new Error( + `${context} exceeds the ${CONNECTION_ITEM_LIMIT}-item safety limit`, + ); + if (connection.pageInfo.hasNextPage && !connection.pageInfo.endCursor) + throw new Error(`${context} pagination cursor is unavailable`); + return connection; +} + +function createConnectionState( + connection, + context, + identityFrom, + pageSize = CONNECTION_PAGE_SIZE, +) { + const first = requireConnection(connection, context); + const state = { + context, + totalCount: first.totalCount, + nodes: [], + identities: new Set(), + cursors: new Set(), + cursor: null, + hasNextPage: false, + pages: 0, + pageSize, + }; + appendConnectionPage(state, first, identityFrom); + return state; +} + +function appendConnectionPage(state, connection, identityFrom) { + const page = requireConnection(connection, state.context); + if (page.totalCount !== state.totalCount) + throw new Error(`${state.context} totalCount changed during pagination`); + if (state.pages >= Math.ceil(CONNECTION_ITEM_LIMIT / state.pageSize)) + throw new Error( + `${state.context} pagination exceeded the page safety limit`, + ); + for (const node of page.nodes) { + const identity = identityFrom(node); + if (typeof identity !== "string" || identity.length === 0) + throw new Error(`${state.context} returned an invalid identity`); + if (state.identities.has(identity)) + throw new Error(`${state.context} returned a duplicate identity`); + state.identities.add(identity); + state.nodes.push(node); + } + state.pages += 1; + if (state.nodes.length > state.totalCount) + throw new Error(`${state.context} returned more nodes than totalCount`); + state.hasNextPage = page.pageInfo.hasNextPage; + if (state.hasNextPage && state.cursors.has(page.pageInfo.endCursor)) + throw new Error(`${state.context} pagination cursor was repeated`); + if (state.hasNextPage) state.cursors.add(page.pageInfo.endCursor); + state.cursor = page.pageInfo.endCursor; + if (!state.hasNextPage && state.nodes.length !== state.totalCount) + throw new Error(`${state.context} pagination was truncated`); +} + +function requireProjectAnchor( + project, + { projectOwner, projectNumber, projectId }, + context, +) { + if ( + !isRecord(project) || + typeof project.id !== "string" || + (projectId !== undefined && project.id !== projectId) || + project.number !== projectNumber || + project.closed !== false || + !isRecord(project.owner) || + project.owner.login !== projectOwner + ) { + throw new Error(`${context} Project identity changed during pagination`); + } +} + +function requireItemAnchor(item, expected, repository, context) { + if ( + !isRecord(item) || + typeof item.id !== "string" || + item.id !== expected.id || + !isRecord(item.content) || + item.content.__typename !== "Issue" || + typeof item.content.id !== "string" || + item.content.id !== expected.content.id || + !Number.isInteger(item.content.number) || + item.content.number !== expected.content.number || + item.content.repository?.nameWithOwner !== repository + ) { + throw new Error( + `${context} item or issue identity changed during pagination`, + ); + } +} + +function linkedPullRequestIdentity(pullRequest) { + if ( + !isRecord(pullRequest) || + typeof pullRequest.id !== "string" || + !Number.isInteger(pullRequest.number) || + typeof pullRequest.url !== "string" || + typeof pullRequest.repository?.nameWithOwner !== "string" + ) { + return null; + } + return `${pullRequest.repository.nameWithOwner}#${pullRequest.number}`; +} + +function requireTrackedFields(fields) { + if ( + !fields.every(isProjectFieldRecord) || + !hasUniqueProjectFieldCoordinates(fields) + ) { + throw new Error("Project field coordinates are invalid or ambiguous"); + } + const coordinates = new Map(); + for (const name of PROJECT_FIELD_NAMES) { + const matches = fields.filter((field) => field.name === name); + if (matches.length !== 1) + throw new Error( + `Project ${name} field coordinate is unavailable or ambiguous`, + ); + if (name !== "Linked pull requests" && !Array.isArray(matches[0].options)) + throw new Error(`Project ${name} field options are unavailable`); + coordinates.set(name, matches[0]); + } + return coordinates; +} + +function fieldValueIdentity(value) { + return value?.field?.id; +} + +function requireItemFieldValues(item, coordinates, projectFields) { + const valuesById = new Map(); + const projectFieldsById = new Map( + projectFields.map((coordinate) => [coordinate.id, coordinate]), + ); + for (const value of item.fieldValues) { + if ( + !isRecord(value) || + typeof value.__typename !== "string" || + !isRecord(value.field) || + typeof value.field.id !== "string" || + typeof value.field.name !== "string" + ) { + throw new Error( + `Project item ${item.id} field value identity is invalid`, + ); + } + if (valuesById.has(value.field.id)) + throw new Error(`Project item ${item.id} has a duplicate field value`); + const coordinateByName = coordinates.get(value.field.name); + const coordinateById = projectFieldsById.get(value.field.id); + if ( + !coordinateById || + coordinateById.name !== value.field.name || + (coordinateByName && coordinateByName.id !== value.field.id) + ) { + throw new Error( + `Project item ${item.id} ${coordinateById?.name ?? value.field.name} field identity changed`, + ); + } + valuesById.set(value.field.id, value); + } + + const normalized = {}; + for (const name of PROJECT_FIELD_NAMES) { + const coordinate = coordinates.get(name); + const value = valuesById.get(coordinate.id); + if (!value) { + normalized[name] = name === "Linked pull requests" ? [] : null; + continue; + } + if (value.field.name !== name) + throw new Error(`Project item ${item.id} ${name} field identity changed`); + const expectedType = + name === "Linked pull requests" + ? "ProjectV2ItemFieldPullRequestValue" + : "ProjectV2ItemFieldSingleSelectValue"; + if (value.__typename !== expectedType) + throw new Error( + `Project item ${item.id} ${name} field value type is invalid`, + ); + if (name === "Linked pull requests") { + normalized[name] = value.pullRequests; + } else { + if (typeof value.name !== "string" || typeof value.optionId !== "string") + throw new Error(`Project item ${item.id} ${name} value is invalid`); + const matchingOptions = coordinate.options.filter( + (option) => option.name === value.name && option.id === value.optionId, + ); + if (matchingOptions.length !== 1) + throw new Error( + `Project item ${item.id} ${name} option identity is invalid`, + ); + normalized[name] = value.name; + } + } + return normalized; +} + +const PROJECT_EVIDENCE_QUERY = `query($login: String!, $number: Int!) { + user(login: $login) { + login + projectV2(number: $number) { + id number closed owner { ... on User { login } } + fields(first: ${CONNECTION_PAGE_SIZE}) { + totalCount nodes { __typename ... on ProjectV2FieldCommon { id name } ... on ProjectV2SingleSelectField { id name options { id name } } } + pageInfo { hasNextPage endCursor } + } + items(first: ${CONNECTION_PAGE_SIZE}) { + totalCount nodes { + id + content { __typename ... on Issue { id number repository { nameWithOwner } labels(first: ${CONNECTION_PAGE_SIZE}) { totalCount nodes { id name } pageInfo { hasNextPage endCursor } } } } + fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}) { + totalCount nodes { + ${FIELD_VALUE_SELECTION} + } + pageInfo { hasNextPage endCursor } + } + } + pageInfo { hasNextPage endCursor } + } + } + } +}`; + function requireGraphqlSuccess(serialized, context) { const response = parseJson(serialized, context); const errors = graphqlResponseErrors(response, context); @@ -85,66 +364,20 @@ function requireGraphqlSuccess(serialized, context) { return response; } -const CONNECTION_PAGE_SIZE = 100; -const CONNECTION_ITEM_LIMIT = 1_000; - function readBoundedConnection({ context, readPage, connectionFrom, identityFrom, - emptyWhen = () => false, }) { - const nodes = []; - const identities = new Set(); - let cursor = null; - let expectedTotal = null; - for ( - let page = 0; - page < CONNECTION_ITEM_LIMIT / CONNECTION_PAGE_SIZE; - page += 1 - ) { - const value = readPage(cursor); - if (emptyWhen(value)) return { value, nodes: [] }; + let state = null; + while (!state || state.hasNextPage) { + const value = readPage(state?.cursor ?? null); const connection = connectionFrom(value); - if ( - !isRecord(connection) || - !Number.isInteger(connection.totalCount) || - connection.totalCount < 0 || - !Array.isArray(connection.nodes) || - !isRecord(connection.pageInfo) || - typeof connection.pageInfo.hasNextPage !== "boolean" || - (connection.pageInfo.endCursor !== null && - typeof connection.pageInfo.endCursor !== "string") - ) { - throw new Error(`${context} pagination evidence is invalid`); - } - if (connection.totalCount > CONNECTION_ITEM_LIMIT) - throw new Error( - `${context} exceeds the ${CONNECTION_ITEM_LIMIT}-item safety limit`, - ); - expectedTotal ??= connection.totalCount; - if (connection.totalCount !== expectedTotal) - throw new Error(`${context} totalCount changed during pagination`); - for (const node of connection.nodes) { - const identity = identityFrom(node); - if (identities.has(identity)) - throw new Error(`${context} returned a duplicate identity`); - identities.add(identity); - nodes.push(node); - } - if (nodes.length > expectedTotal) - throw new Error(`${context} returned more nodes than totalCount`); - if (!connection.pageInfo.hasNextPage) { - if (nodes.length !== expectedTotal) - throw new Error(`${context} pagination was truncated`); - return { value, nodes }; - } - if (!connection.pageInfo.endCursor) - throw new Error(`${context} pagination cursor is unavailable`); - cursor = connection.pageInfo.endCursor; + if (state) appendConnectionPage(state, connection, identityFrom); + else state = createConnectionState(connection, context, identityFrom); } - throw new Error(`${context} pagination exceeded the page safety limit`); + return state.nodes; } export function createRentCottageGhSource({ @@ -153,39 +386,262 @@ export function createRentCottageGhSource({ projectNumber, run = runGh, }) { - const projectArgs = [String(projectNumber), "--owner", projectOwner]; const source = { async assertSupported() { assertSupportedGhVersion(run(["--version"])); }, - async readProject() { - return parseJson( - run(["project", "view", ...projectArgs, "--format", "json"]), - "Project", + async readProjectEvidence() { + const read = (query, variables, context) => + requireGraphqlSuccess(run(graphqlArgs(query, variables)), context); + const response = read( + PROJECT_EVIDENCE_QUERY, + { login: projectOwner, number: projectNumber }, + "Project evidence", + ); + const user = response.data?.user; + const project = user?.projectV2; + if (!isRecord(user) || user.login !== projectOwner) + throw new Error("Project owner identity is invalid"); + requireProjectAnchor( + project, + { projectOwner, projectNumber }, + "Project evidence", ); - }, - async readProjectFields() { - return parseJson( - run(["project", "field-list", ...projectArgs, "--format", "json"]), + const fields = createConnectionState( + project.fields, "Project fields", + (field) => field?.id, ); - }, + while (fields.hasNextPage) { + const query = `query($login: String!, $number: Int!, $cursor: String!) { + user(login: $login) { login projectV2(number: $number) { + id number closed owner { ... on User { login } } + fields(first: ${CONNECTION_PAGE_SIZE}, after: $cursor) { + totalCount nodes { __typename ... on ProjectV2FieldCommon { id name } ... on ProjectV2SingleSelectField { id name options { id name } } } + pageInfo { hasNextPage endCursor } + } + } } + }`; + const page = read( + query, + { login: projectOwner, number: projectNumber, cursor: fields.cursor }, + "Project fields", + ); + if (page.data?.user?.login !== projectOwner) + throw new Error( + "Project fields owner identity changed during pagination", + ); + const pageProject = page.data?.user?.projectV2; + requireProjectAnchor( + pageProject, + { projectOwner, projectNumber, projectId: project.id }, + "Project fields", + ); + appendConnectionPage(fields, pageProject.fields, (field) => field?.id); + } + const coordinates = requireTrackedFields(fields.nodes); - async readProjectItems() { - return parseJson( - run([ - "project", - "item-list", - ...projectArgs, - "--format", - "json", - "--limit", - "100", - ]), + const items = createConnectionState( + project.items, "Project items", + (item) => item?.id, ); + while (items.hasNextPage) { + const query = `query($login: String!, $number: Int!, $cursor: String!) { + user(login: $login) { login projectV2(number: $number) { + id number closed owner { ... on User { login } } + items(first: ${CONNECTION_PAGE_SIZE}, after: $cursor) { + totalCount nodes { + id + content { __typename ... on Issue { id number repository { nameWithOwner } labels(first: ${CONNECTION_PAGE_SIZE}) { totalCount nodes { id name } pageInfo { hasNextPage endCursor } } } } + fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}) { + totalCount nodes { ${FIELD_VALUE_SELECTION} } + pageInfo { hasNextPage endCursor } + } + } + pageInfo { hasNextPage endCursor } + } + } } + }`; + const page = read( + query, + { login: projectOwner, number: projectNumber, cursor: items.cursor }, + "Project items", + ); + if (page.data?.user?.login !== projectOwner) + throw new Error( + "Project items owner identity changed during pagination", + ); + const pageProject = page.data?.user?.projectV2; + requireProjectAnchor( + pageProject, + { projectOwner, projectNumber, projectId: project.id }, + "Project items", + ); + appendConnectionPage(items, pageProject.items, (item) => item?.id); + } + + const normalizedItems = []; + for (const item of items.nodes) { + const context = `Project item ${item?.id ?? "unknown"}`; + requireItemAnchor(item, item, repository, context); + + const labels = createConnectionState( + item.content.labels, + `${context} labels`, + (label) => label?.id, + ); + while (labels.hasNextPage) { + const query = `query($itemId: ID!, $cursor: String!) { + node(id: $itemId) { ... on ProjectV2Item { + id content { __typename ... on Issue { id number repository { nameWithOwner } labels(first: ${CONNECTION_PAGE_SIZE}, after: $cursor) { totalCount nodes { id name } pageInfo { hasNextPage endCursor } } } } + } } + }`; + const page = read( + query, + { itemId: item.id, cursor: labels.cursor }, + `${context} labels`, + ); + requireItemAnchor(page.data?.node, item, repository, context); + appendConnectionPage( + labels, + page.data.node.content.labels, + (label) => label?.id, + ); + } + if (labels.nodes.some((label) => typeof label?.name !== "string")) + throw new Error(`${context} label evidence is invalid`); + + const fieldValues = createConnectionState( + item.fieldValues, + `${context} field values`, + fieldValueIdentity, + PROJECT_FIELD_VALUE_PAGE_SIZE, + ); + const fieldValuePageCursor = new Map( + item.fieldValues.nodes.map((value) => [ + fieldValueIdentity(value), + null, + ]), + ); + while (fieldValues.hasNextPage) { + const pageStartCursor = fieldValues.cursor; + const query = `query($itemId: ID!, $cursor: String!) { + node(id: $itemId) { ... on ProjectV2Item { + id content { __typename ... on Issue { id number repository { nameWithOwner } } } + fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}, after: $cursor) { + totalCount nodes { ${FIELD_VALUE_SELECTION} } + pageInfo { hasNextPage endCursor } + } + } } + }`; + const page = read( + query, + { itemId: item.id, cursor: pageStartCursor }, + `${context} field values`, + ); + requireItemAnchor(page.data?.node, item, repository, context); + appendConnectionPage( + fieldValues, + page.data.node.fieldValues, + fieldValueIdentity, + ); + for (const value of page.data.node.fieldValues.nodes) + fieldValuePageCursor.set( + fieldValueIdentity(value), + pageStartCursor, + ); + } + item.fieldValues = fieldValues.nodes; + + const linkedCoordinate = coordinates.get("Linked pull requests"); + const linkedValue = item.fieldValues.find( + (value) => value?.field?.id === linkedCoordinate.id, + ); + if (linkedValue) { + if ( + linkedValue.__typename !== "ProjectV2ItemFieldPullRequestValue" || + linkedValue.field?.name !== linkedCoordinate.name + ) { + throw new Error( + `${context} Linked pull requests field value type is invalid`, + ); + } + const pullRequests = createConnectionState( + linkedValue.pullRequests, + `${context} linked pull requests`, + linkedPullRequestIdentity, + LINKED_PULL_REQUEST_PAGE_SIZE, + ); + while (pullRequests.hasNextPage) { + const query = `query($itemId: ID!, $fieldCursor: String, $pullRequestCursor: String!) { + node(id: $itemId) { ... on ProjectV2Item { + id content { __typename ... on Issue { id number repository { nameWithOwner } } } + fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}, after: $fieldCursor) { + nodes { __typename ... on ProjectV2ItemFieldPullRequestValue { field { ... on ProjectV2FieldCommon { id name } } pullRequests(first: ${LINKED_PULL_REQUEST_PAGE_SIZE}, after: $pullRequestCursor) { totalCount nodes { id number url repository { nameWithOwner } } pageInfo { hasNextPage endCursor } } } } + } + } } + }`; + const page = read( + query, + { + itemId: item.id, + fieldCursor: fieldValuePageCursor.get(linkedCoordinate.id), + pullRequestCursor: pullRequests.cursor, + }, + `${context} linked pull requests`, + ); + requireItemAnchor(page.data?.node, item, repository, context); + const matches = page.data.node.fieldValues?.nodes?.filter( + (value) => value?.field?.id === linkedCoordinate.id, + ); + if ( + matches?.length !== 1 || + matches[0].field.name !== linkedCoordinate.name || + matches[0].__typename !== "ProjectV2ItemFieldPullRequestValue" + ) { + throw new Error( + `${context} linked pull-request field identity changed`, + ); + } + appendConnectionPage( + pullRequests, + matches[0].pullRequests, + linkedPullRequestIdentity, + ); + } + linkedValue.pullRequests = pullRequests.nodes; + } + + const values = requireItemFieldValues(item, coordinates, fields.nodes); + normalizedItems.push({ + id: item.id, + area: values.Area, + status: values.Status, + content: { + type: item.content.__typename, + number: item.content.number, + repository: item.content.repository.nameWithOwner, + }, + labels: labels.nodes.map(({ name }) => name), + "linked pull requests": values["Linked pull requests"], + }); + } + + return { + project: { + id: project.id, + number: project.number, + owner: { login: user.login }, + closed: project.closed, + items: { totalCount: items.totalCount }, + fields: { totalCount: fields.totalCount }, + }, + fields: { totalCount: fields.totalCount, fields: fields.nodes }, + items: { totalCount: items.totalCount, items: normalizedItems }, + }; }, async listIssues() { @@ -212,50 +668,6 @@ export function createRentCottageGhSource({ ); }, - async listLinkedPullRequests(itemId) { - const query = `query($itemId: ID!, $cursor: String) { - node(id: $itemId) { - ... on ProjectV2Item { - fieldValueByName(name: "Linked pull requests") { - ... on ProjectV2ItemFieldPullRequestValue { - pullRequests(first: ${CONNECTION_PAGE_SIZE}, after: $cursor) { - totalCount - nodes { number url repository { nameWithOwner } } - pageInfo { hasNextPage endCursor } - } - } - } - } - } - }`; - const { nodes } = readBoundedConnection({ - context: `Project item ${itemId} linked pull requests`, - readPage(cursor) { - const args = [ - "api", - "graphql", - "-f", - `query=${query}`, - "-F", - `itemId=${itemId}`, - ]; - if (cursor) args.push("-f", `cursor=${cursor}`); - const response = requireGraphqlSuccess( - run(args), - `Project item ${itemId} linked pull requests`, - ); - if (!isRecord(response.data?.node)) - throw new Error(`Project item ${itemId} is unavailable`); - return response.data.node.fieldValueByName ?? null; - }, - connectionFrom: (fieldValue) => fieldValue.pullRequests, - identityFrom: (pullRequest) => - `${pullRequest.repository?.nameWithOwner}#${pullRequest.number}`, - emptyWhen: (fieldValue) => fieldValue === null, - }); - return nodes; - }, - async readPullRequest(pullRequestNumber) { const [owner, name] = repository.split("/"); const query = `query($owner: String!, $name: String!, $pullRequestNumber: Int!, $cursor: String) { @@ -271,7 +683,7 @@ export function createRentCottageGhSource({ } }`; let firstPullRequest = null; - const { nodes } = readBoundedConnection({ + const nodes = readBoundedConnection({ context: `Pull request #${pullRequestNumber} closing references`, readPage(cursor) { const args = [ @@ -315,7 +727,7 @@ export function createRentCottageGhSource({ async execute(operation) { if (operation.type === "add-project-item") { - const project = await source.readProject(); + const { project } = await source.readProjectEvidence(); if ( !hasFreshProjectCoordinates(project, { projectOwner, projectNumber }) ) @@ -340,15 +752,13 @@ export function createRentCottageGhSource({ } if (operation.type === "set-project-field") { - const project = await source.readProject(); + const { project, fields, items } = await source.readProjectEvidence(); if ( !hasFreshProjectCoordinates(project, { projectOwner, projectNumber }) ) throw new Error("Fresh Project response is invalid"); - const fields = await source.readProjectFields(); if (!hasFreshFieldCoordinates(fields, project)) throw new Error("Fresh Project fields response is invalid"); - const items = await source.readProjectItems(); if (!hasFreshItemCoordinates(items, project, operation, repository)) throw new Error("Fresh Project items response is invalid"); const matchingFields = fields.fields.filter( diff --git a/scripts/lib/rentcottage-github-adapter.mjs b/scripts/lib/rentcottage-github-adapter.mjs index 9b09d35..13b2df1 100644 --- a/scripts/lib/rentcottage-github-adapter.mjs +++ b/scripts/lib/rentcottage-github-adapter.mjs @@ -146,12 +146,12 @@ export function createRentCottageGitHubAdapter({ source, policy }) { let rawIssues; try { await source.assertSupported(); - [project, rawFields, rawItems, rawIssues] = await Promise.all([ - source.readProject(), - source.readProjectFields(), - source.readProjectItems(), + const [projectEvidence, issues] = await Promise.all([ + source.readProjectEvidence(), source.listIssues(), ]); + ({ project, fields: rawFields, items: rawItems } = projectEvidence); + rawIssues = issues; } catch (error) { return incompleteObservation(policy, [ `GitHub evidence unavailable: ${errorDiagnostic(error)}`, @@ -232,21 +232,9 @@ export function createRentCottageGitHubAdapter({ source, policy }) { const linkedIssuesByPullRequest = new Map(); if (intent.pullRequestNumber) pullRequestNumbers.add(intent.pullRequestNumber); - let linkedPullRequestsByItem; - try { - linkedPullRequestsByItem = await Promise.all( - rawItems.items - .filter((item) => item.content.type === "Issue") - .map(async (item) => [ - item, - await source.listLinkedPullRequests(item.id), - ]), - ); - } catch (error) { - return incompleteObservation(policy, [ - `GitHub evidence unavailable: ${errorDiagnostic(error)}`, - ]); - } + const linkedPullRequestsByItem = rawItems.items + .filter((item) => item.content.type === "Issue") + .map((item) => [item, item["linked pull requests"] ?? []]); if ( linkedPullRequestsByItem.some( ([, pullRequests]) => diff --git a/scripts/lib/rentcottage-github-schema.mjs b/scripts/lib/rentcottage-github-schema.mjs index 9bea69f..352e60b 100644 --- a/scripts/lib/rentcottage-github-schema.mjs +++ b/scripts/lib/rentcottage-github-schema.mjs @@ -20,7 +20,7 @@ export function isProjectFieldRecord(field) { export function hasUniqueProjectFieldCoordinates(fields) { const trackedFields = fields.filter(({ name }) => - ["Area", "Status"].includes(name), + ["Area", "Status", "Linked pull requests"].includes(name), ); const options = trackedFields.flatMap((field) => field.options ?? []); const hasDuplicates = (values) => new Set(values).size !== values.length; @@ -52,6 +52,13 @@ export function isProjectItemRecord(item) { typeof item.status === "string") && (item["linked pull requests"] === undefined || (Array.isArray(item["linked pull requests"]) && - item["linked pull requests"].every((url) => typeof url === "string"))) + item["linked pull requests"].every( + (pullRequest) => + isRecord(pullRequest) && + Number.isInteger(pullRequest.number) && + typeof pullRequest.url === "string" && + isRecord(pullRequest.repository) && + typeof pullRequest.repository.nameWithOwner === "string", + ))) ); } diff --git a/scripts/rentcottage-gh-source.test.mjs b/scripts/rentcottage-gh-source.test.mjs index 2261e1f..1b21e14 100644 --- a/scripts/rentcottage-gh-source.test.mjs +++ b/scripts/rentcottage-gh-source.test.mjs @@ -4,10 +4,182 @@ import { runGh, } from "./lib/rentcottage-gh-source.mjs"; -function expectVariableFlag(args, variable, flag) { - const index = args.findIndex((arg) => arg.startsWith(`${variable}=`)); - expect(index).toBeGreaterThan(0); - expect(args[index - 1]).toBe(flag); +const repository = "zaingulel/RentCottage"; + +function connection(nodes, options = {}) { + return { + totalCount: options.totalCount ?? nodes.length, + nodes, + pageInfo: { + hasNextPage: options.hasNextPage ?? false, + endCursor: options.endCursor ?? null, + }, + }; +} + +function connectionPage(nodes, pageIndex, pageSize, cursorPrefix) { + const start = pageIndex * pageSize; + const pageNodes = nodes.slice(start, start + pageSize); + const hasNextPage = start + pageNodes.length < nodes.length; + return connection(pageNodes, { + totalCount: nodes.length, + hasNextPage, + endCursor: hasNextPage ? `${cursorPrefix}-${pageIndex + 1}` : null, + }); +} + +function fields() { + return [ + { + __typename: "ProjectV2SingleSelectField", + id: "field-area", + name: "Area", + options: [{ id: "area-foundation", name: "Foundation & quality" }], + }, + { + __typename: "ProjectV2SingleSelectField", + id: "field-status", + name: "Status", + options: [ + { id: "status-backlog", name: "Backlog" }, + { id: "status-progress", name: "In progress" }, + ], + }, + { + __typename: "ProjectV2Field", + id: "field-linked", + name: "Linked pull requests", + }, + { + __typename: "ProjectV2Field", + id: "field-notes", + name: "Notes", + }, + ]; +} + +function singleSelect(field, name) { + return { + __typename: "ProjectV2ItemFieldSingleSelectValue", + field: { id: field.id, name: field.name }, + name, + optionId: field.options.find((option) => option.name === name)?.id, + }; +} + +function linkedPullRequests(nodes = [], options = {}) { + return { + __typename: "ProjectV2ItemFieldPullRequestValue", + field: { id: "field-linked", name: "Linked pull requests" }, + pullRequests: connection(nodes, options), + }; +} + +function textValue(text = "Internal delivery note") { + return { + __typename: "ProjectV2ItemFieldTextValue", + field: { id: "field-notes", name: "Notes" }, + text, + }; +} + +function projectItem(number = 55, overrides = {}) { + const projectFields = fields(); + return { + id: `item-${number}`, + content: { + __typename: "Issue", + id: `issue-${number}`, + number, + repository: { nameWithOwner: repository }, + labels: connection([{ id: `label-${number}`, name: "ready-for-agent" }]), + }, + fieldValues: connection([ + singleSelect(projectFields[0], "Foundation & quality"), + singleSelect(projectFields[1], "Backlog"), + linkedPullRequests(), + textValue(), + ]), + ...overrides, + }; +} + +function projectResponse({ fieldConnection, itemConnection } = {}) { + return { + data: { + user: { + login: "zaingulel", + projectV2: { + id: "project-4", + number: 4, + closed: false, + owner: { login: "zaingulel" }, + fields: fieldConnection ?? connection(fields()), + items: itemConnection ?? connection([projectItem()]), + }, + }, + }, + }; +} + +function fieldValuePage(item, fieldValues) { + return { + data: { + node: { + ...item, + fieldValues, + }, + }, + }; +} + +function sourceWith(run) { + return createRentCottageGhSource({ + repository, + projectOwner: "zaingulel", + projectNumber: 4, + run, + }); +} + +function variables(args) { + return Object.fromEntries( + args + .filter((arg) => /^[A-Za-z][A-Za-z0-9]*=/.test(arg)) + .map((arg) => { + const separator = arg.indexOf("="); + return [arg.slice(0, separator), arg.slice(separator + 1)]; + }), + ); +} + +function queryFrom(args) { + const query = args.find((arg) => arg.startsWith("query=")); + expect(query).toBeDefined(); + return query.slice("query=".length); +} + +function connectionFirst(query, name, { required = true } = {}) { + const match = query.match(new RegExp(`${name}\\(first:\\s*(\\d+)`)); + if (!required && !match) return 0; + expect(match, `${name} connection is missing`).not.toBeNull(); + return Number(match[1]); +} + +function projectQueryMaximumPossibleNodes(query) { + const fields = connectionFirst(query, "fields", { required: false }); + const items = connectionFirst(query, "items"); + const labelsPerItem = connectionFirst(query, "labels"); + const fieldValuesPerItem = connectionFirst(query, "fieldValues"); + const pullRequestsPerFieldValue = connectionFirst(query, "pullRequests"); + + return ( + fields + + items + + items * labelsPerItem + + items * fieldValuesPerItem + + items * fieldValuesPerItem * pullRequestsPerFieldValue + ); } describe("RentCottage gh source", () => { @@ -49,45 +221,6 @@ describe("RentCottage gh source", () => { expect(message.length).toBeLessThan(1_500); }); - it("summarizes GraphQL semantic errors without exposing response messages", async () => { - const privateBody = "private-graphql-body"; - const secret = "ghp_abcdefghijklmnopqrstuvwxyz1234567890"; - const codeSecret = "github_pat_abcdefghijklmnopqrstuvwxyz1234567890"; - const run = vi.fn(() => - JSON.stringify({ - errors: [ - { - message: `${privateBody} Bearer ${secret} ${"detail".repeat(5_000)}`, - extensions: { code: "FORBIDDEN" }, - }, - { message: "secondary failure", extensions: { code: codeSecret } }, - ], - }), - ); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, - }); - - let message; - try { - await source.listLinkedPullRequests("item-55"); - } catch (error) { - message = error.message; - } - - expect(message).toContain( - "Project item item-55 linked pull requests failed", - ); - expect(message).toContain("FORBIDDEN"); - expect(message).not.toContain(privateBody); - expect(message).not.toContain(secret); - expect(message).not.toContain(codeSecret); - expect(message.length).toBeLessThan(1_500); - }); - it.each([ ["object", { message: "reviewer-reproduced-object" }], ["string", "malformed-errors-string"], @@ -109,22 +242,16 @@ describe("RentCottage gh source", () => { : errors, }), ); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, - }); let message; try { - await source.listLinkedPullRequests("item-55"); + await sourceWith(run).readProjectEvidence(); } catch (error) { message = error.message; } expect(message).toContain( - "Project item item-55 linked pull requests returned malformed GraphQL errors evidence", + "Project evidence returned malformed GraphQL errors evidence", ); expect(message).not.toContain(secret); expect(message.length).toBeLessThan(1_500); @@ -133,18 +260,12 @@ describe("RentCottage gh source", () => { it("bounds and redacts provider-derived malformed GraphQL context", async () => { const secret = "ghp_abcdefghijklmnopqrstuvwxyz1234567890"; - const itemId = `item-55\u0000 Bearer ${secret} ${"context".repeat(5_000)}`; + const pullRequestNumber = `70\u0000 Bearer ${secret} ${"context".repeat(5_000)}`; const run = vi.fn(() => JSON.stringify({ errors: {} })); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, - }); let message; try { - await source.listLinkedPullRequests(itemId); + await sourceWith(run).readPullRequest(pullRequestNumber); } catch (error) { message = error.message; } @@ -155,620 +276,862 @@ describe("RentCottage gh source", () => { expect(message.length).toBeLessThan(500); }); - it.each([ - { - identity: "database ID", - duplicate: { id: 550, number: 56 }, - }, - { - identity: "repository issue number", - duplicate: { id: 551, number: 55 }, - }, - ])( - "rejects contradictory duplicate REST issue $identity across pages", - async ({ duplicate }) => { - const issue = { - id: 550, - number: 55, - state: "open", - body: "approved body", - }; - const run = vi - .fn() - .mockReturnValue( - JSON.stringify([ - [issue], - [{ ...issue, ...duplicate, state: "closed", body: "changed body" }], - ]), - ); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, - }); + it("reads multiple Project items with one lean GraphQL call and no broad or per-item query", async () => { + const run = vi.fn(() => + JSON.stringify( + projectResponse({ + itemConnection: connection([projectItem(55), projectItem(63)]), + }), + ), + ); + + const evidence = await sourceWith(run).readProjectEvidence(); + + expect(evidence.items.items.map(({ content }) => content.number)).toEqual([ + 55, 63, + ]); + expect(run).toHaveBeenCalledTimes(1); + expect(run.mock.calls[0][0].slice(0, 2)).toEqual(["api", "graphql"]); + const invocation = run.mock.calls[0][0].join(" "); + expect(invocation).not.toMatch(/project (view|field-list|item-list)/); + expect(invocation).not.toContain("fieldValueByName"); + expect(invocation).toContain("ProjectV2ItemFieldValueCommon"); + for (const typename of [ + "ProjectV2ItemFieldLabelValue", + "ProjectV2ItemFieldMilestoneValue", + "ProjectV2ItemFieldPullRequestValue", + "ProjectV2ItemFieldRepositoryValue", + "ProjectV2ItemFieldReviewerValue", + "ProjectV2ItemFieldUserValue", + "ProjectV2ItemIssueFieldValue", + ]) { + expect(invocation).toContain(typename); + } + }); + + it("keeps every Project-items query within GitHub's possible-node limit", async () => { + const providerMaximumPossibleNodes = 500_000; + const first = projectResponse({ + itemConnection: connection([projectItem(55)], { + totalCount: 2, + hasNextPage: true, + endCursor: "items-1", + }), + }); + const overflow = projectResponse({ + itemConnection: connection([projectItem(63)], { totalCount: 2 }), + }); + const run = vi + .fn() + .mockReturnValueOnce(JSON.stringify(first)) + .mockReturnValueOnce(JSON.stringify(overflow)); - await expect(source.listIssues()).rejects.toThrow( - "Issue pagination returned a duplicate stable identity", + await sourceWith(run).readProjectEvidence(); + + const itemQueries = run.mock.calls.map(([args]) => queryFrom(args)); + expect(itemQueries).toHaveLength(2); + for (const query of itemQueries) { + expect(connectionFirst(query, "items")).toBe(100); + expect(projectQueryMaximumPossibleNodes(query)).toBeLessThanOrEqual( + providerMaximumPossibleNodes, ); - }, - ); + } + }); + + it("tolerates a populated unconsumed Text field without normalizing it", async () => { + const response = projectResponse(); + + const evidence = await sourceWith( + vi.fn(() => JSON.stringify(response)), + ).readProjectEvidence(); + + expect(evidence.items.items[0]).toMatchObject({ + area: "Foundation & quality", + status: "Backlog", + "linked pull requests": [], + }); + expect(evidence.items.items[0]).not.toHaveProperty("Notes"); + }); it.each([ - { - identity: "database ID", - duplicate: { id: 520, number: 53 }, - }, - { - identity: "repository issue number", - duplicate: { id: 521, number: 52 }, - }, + [ + "missing coordinate", + (value) => { + delete value.field; + }, + "field values returned an invalid identity", + ], + [ + "unknown coordinate", + (value) => { + value.field.id = "field-notes-unknown"; + }, + "Notes field identity changed", + ], ])( - "rejects contradictory duplicate REST dependency $identity across pages", - async ({ duplicate }) => { - const dependency = { id: 520, number: 52, state: "open" }; - const run = vi - .fn() - .mockReturnValue( - JSON.stringify([ - [dependency], - [{ ...dependency, ...duplicate, state: "closed" }], - ]), - ); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, - }); + "rejects an unconsumed Text value with a $name", + async (_name, mutate, message) => { + const response = projectResponse(); + mutate(response.data.user.projectV2.items.nodes[0].fieldValues.nodes[3]); - await expect(source.listBlockedBy(55)).rejects.toThrow( - "#55 dependency pagination returned a duplicate stable identity", - ); + await expect( + sourceWith(vi.fn(() => JSON.stringify(response))).readProjectEvidence(), + ).rejects.toThrow(message); }, ); - it("paginates pull-request closing references until totalCount is proven", async () => { - const base = { - number: 70, - state: "OPEN", - isDraft: false, - mergedAt: null, - url: "https://github.com/zaingulel/RentCottage/pull/70", - }; + it("paginates only Project fields when only fields overflow", async () => { + const allFields = fields(); + const first = projectResponse({ + fieldConnection: connection([allFields[0]], { + totalCount: 4, + hasNextPage: true, + endCursor: "fields-1", + }), + }); + const overflow = projectResponse({ + fieldConnection: connection(allFields.slice(1), { totalCount: 4 }), + }); const run = vi .fn() - .mockReturnValueOnce( - JSON.stringify({ - data: { - repository: { - pullRequest: { - ...base, - closingIssuesReferences: { - totalCount: 2, - nodes: [ - { - number: 55, - repository: { - name: "RentCottage", - owner: { login: "zaingulel" }, - }, - }, - ], - pageInfo: { hasNextPage: true, endCursor: "cursor-1" }, - }, - }, - }, - }, - }), - ) - .mockReturnValueOnce( - JSON.stringify({ - data: { - repository: { - pullRequest: { - ...base, - closingIssuesReferences: { - totalCount: 2, - nodes: [ - { - number: 64, - repository: { - name: "RentCottage", - owner: { login: "zaingulel" }, - }, - }, - ], - pageInfo: { hasNextPage: false, endCursor: null }, - }, - }, - }, - }, - }), - ); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, + .mockReturnValueOnce(JSON.stringify(first)) + .mockReturnValueOnce(JSON.stringify(overflow)); + + const evidence = await sourceWith(run).readProjectEvidence(); + + expect(evidence.fields.fields).toHaveLength(4); + expect(run).toHaveBeenCalledTimes(2); + expect(variables(run.mock.calls[1][0])).toMatchObject({ + cursor: "fields-1", }); + expect(run.mock.calls[1][0].join(" ")).not.toContain("items(first:"); + }); - const pullRequest = await source.readPullRequest(70); + it("paginates only Project items when only items overflow", async () => { + const first = projectResponse({ + itemConnection: connection([projectItem(55)], { + totalCount: 2, + hasNextPage: true, + endCursor: "items-1", + }), + }); + const overflow = projectResponse({ + itemConnection: connection([projectItem(63)], { totalCount: 2 }), + }); + const run = vi + .fn() + .mockReturnValueOnce(JSON.stringify(first)) + .mockReturnValueOnce(JSON.stringify(overflow)); - expect(pullRequest.closingIssuesReferences).toEqual([ - { - number: 55, - repository: { - name: "RentCottage", - owner: { login: "zaingulel" }, - }, - }, - { - number: 64, - repository: { - name: "RentCottage", - owner: { login: "zaingulel" }, - }, - }, + const evidence = await sourceWith(run).readProjectEvidence(); + + expect(evidence.items.items.map(({ content }) => content.number)).toEqual([ + 55, 63, ]); - expect(run).toHaveBeenCalledTimes(2); - const secondArgs = run.mock.calls[1][0]; - expectVariableFlag(secondArgs, "owner", "-f"); - expectVariableFlag(secondArgs, "name", "-f"); - expectVariableFlag(secondArgs, "pullRequestNumber", "-F"); - expectVariableFlag(secondArgs, "cursor", "-f"); + expect(variables(run.mock.calls[1][0])).toMatchObject({ + cursor: "items-1", + }); + expect(run.mock.calls[1][0].join(" ")).not.toContain("fields(first:"); }); - it("paginates Project linked pull requests until totalCount is proven", async () => { - const connection = (nodes, hasNextPage, endCursor) => ({ + it("paginates item field values independently without replaying completed connections", async () => { + const item = projectItem(); + const [area, status, linked, notes] = item.fieldValues.nodes; + item.fieldValues = connection([area], { + totalCount: 4, + hasNextPage: true, + endCursor: "field-values-1", + }); + const initial = projectResponse({ itemConnection: connection([item]) }); + const overflow = { data: { node: { - fieldValueByName: { - pullRequests: { - totalCount: 2, - nodes, - pageInfo: { hasNextPage, endCursor }, - }, - }, + ...item, + fieldValues: connection([status, linked, notes], { totalCount: 4 }), }, }, - }); + }; const run = vi .fn() - .mockReturnValueOnce( - JSON.stringify( - connection( - [ - { - number: 70, - url: "https://github.com/zaingulel/RentCottage/pull/70", - repository: { nameWithOwner: "zaingulel/RentCottage" }, - }, - ], - true, - "cursor-1", - ), - ), - ) - .mockReturnValueOnce( - JSON.stringify( - connection( - [ - { - number: 71, - url: "https://github.com/zaingulel/RentCottage/pull/71", - repository: { nameWithOwner: "zaingulel/RentCottage" }, - }, - ], - false, - null, - ), - ), - ); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, - }); + .mockReturnValueOnce(JSON.stringify(initial)) + .mockReturnValueOnce(JSON.stringify(overflow)); - const pullRequests = await source.listLinkedPullRequests("item-55"); + const evidence = await sourceWith(run).readProjectEvidence(); - expect(pullRequests.map(({ number }) => number)).toEqual([70, 71]); + expect(evidence.items.items[0]).toMatchObject({ + area: "Foundation & quality", + status: "Backlog", + "linked pull requests": [], + }); + expect(variables(run.mock.calls[1][0])).toMatchObject({ + itemId: "item-55", + cursor: "field-values-1", + }); + const overflowQuery = run.mock.calls[1][0].join(" "); + expect(overflowQuery).toContain("ProjectV2ItemFieldValueCommon"); + expect(overflowQuery).not.toContain("labels(first:"); + expect(overflowQuery).not.toContain("items(first:"); expect(run).toHaveBeenCalledTimes(2); - const secondArgs = run.mock.calls[1][0]; - expectVariableFlag(secondArgs, "itemId", "-F"); - expectVariableFlag(secondArgs, "cursor", "-f"); }); - it("rejects duplicate linked pull-request identities across pages", async () => { - const linkedPullRequest = { + it("normalizes field values after more than ten provider-sized pages", async () => { + const item = projectItem(); + const extraFields = Array.from({ length: 197 }, (_, index) => ({ + __typename: "ProjectV2Field", + id: `field-extra-${index + 1}`, + name: `Extra ${index + 1}`, + })); + const allFields = [...fields(), ...extraFields]; + const allValues = [ + ...item.fieldValues.nodes, + ...extraFields.map((field) => ({ + __typename: "ProjectV2ItemFieldTextValue", + field: { id: field.id, name: field.name }, + text: "Unconsumed evidence", + })), + ]; + item.fieldValues = connectionPage(allValues, 0, 20, "field-values"); + const initial = projectResponse({ + fieldConnection: connectionPage(allFields, 0, 100, "fields"), + itemConnection: connection([item]), + }); + const fieldPages = [1, 2].map((pageIndex) => + projectResponse({ + fieldConnection: connectionPage(allFields, pageIndex, 100, "fields"), + }), + ); + const fieldValuePages = Array.from({ length: 10 }, (_, index) => + fieldValuePage( + item, + connectionPage(allValues, index + 1, 20, "field-values"), + ), + ); + const run = vi + .fn() + .mockReturnValueOnce(JSON.stringify(initial)) + .mockReturnValueOnce(JSON.stringify(fieldPages[0])) + .mockReturnValueOnce(JSON.stringify(fieldPages[1])); + for (const page of fieldValuePages) + run.mockReturnValueOnce(JSON.stringify(page)); + + const evidence = await sourceWith(run).readProjectEvidence(); + + expect(evidence.items.items[0]).toMatchObject({ + area: "Foundation & quality", + status: "Backlog", + "linked pull requests": [], + }); + expect(variables(run.mock.calls.at(-1)[0])).toMatchObject({ + itemId: "item-55", + cursor: "field-values-10", + }); + }); + + it("uses the field-value page cursor when a later linked-PR value overflows", async () => { + const item = projectItem(); + const [area, status, , notes] = item.fieldValues.nodes; + const firstPullRequest = { + id: "pr-70", number: 70, url: "https://github.com/zaingulel/RentCottage/pull/70", - repository: { nameWithOwner: "zaingulel/RentCottage" }, + repository: { nameWithOwner: repository }, }; - const response = (hasNextPage, endCursor) => - JSON.stringify({ + const secondPullRequest = { + ...firstPullRequest, + id: "pr-71", + number: 71, + url: "https://github.com/zaingulel/RentCottage/pull/71", + }; + item.fieldValues = connection([area], { + totalCount: 4, + hasNextPage: true, + endCursor: "field-values-1", + }); + const linkedOverflow = linkedPullRequests([firstPullRequest], { + totalCount: 2, + hasNextPage: true, + endCursor: "pull-requests-1", + }); + const fieldOverflow = fieldValuePage( + item, + connection([status, linkedOverflow, notes], { totalCount: 4 }), + ); + const pullRequestOverflow = fieldValuePage(item, { + nodes: [ + linkedPullRequests([secondPullRequest], { + totalCount: 2, + }), + ], + }); + const run = vi + .fn() + .mockReturnValueOnce( + JSON.stringify(projectResponse({ itemConnection: connection([item]) })), + ) + .mockReturnValueOnce(JSON.stringify(fieldOverflow)) + .mockReturnValueOnce(JSON.stringify(pullRequestOverflow)); + + const evidence = await sourceWith(run).readProjectEvidence(); + + expect( + evidence.items.items[0]["linked pull requests"].map( + ({ number }) => number, + ), + ).toEqual([70, 71]); + expect(variables(run.mock.calls[2][0])).toMatchObject({ + itemId: "item-55", + fieldCursor: "field-values-1", + pullRequestCursor: "pull-requests-1", + }); + }); + + it("normalizes linked pull requests after more than ten provider-sized pages", async () => { + const item = projectItem(); + const pullRequests = Array.from({ length: 201 }, (_, index) => ({ + id: `pr-${index + 1}`, + number: index + 1, + url: `https://github.com/zaingulel/RentCottage/pull/${index + 1}`, + repository: { nameWithOwner: repository }, + })); + const firstPage = connectionPage(pullRequests, 0, 20, "pull-requests"); + item.fieldValues.nodes[2] = linkedPullRequests(firstPage.nodes, { + totalCount: firstPage.totalCount, + hasNextPage: firstPage.pageInfo.hasNextPage, + endCursor: firstPage.pageInfo.endCursor, + }); + const overflowPages = Array.from({ length: 10 }, (_, index) => { + const page = connectionPage(pullRequests, index + 1, 20, "pull-requests"); + return { data: { node: { - fieldValueByName: { - pullRequests: { - totalCount: 2, - nodes: [linkedPullRequest], - pageInfo: { hasNextPage, endCursor }, - }, + ...item, + fieldValues: { + nodes: [ + linkedPullRequests(page.nodes, { + totalCount: page.totalCount, + hasNextPage: page.pageInfo.hasNextPage, + endCursor: page.pageInfo.endCursor, + }), + ], }, }, }, - }); + }; + }); const run = vi .fn() - .mockReturnValueOnce(response(true, "cursor-1")) - .mockReturnValueOnce(response(false, null)); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, + .mockReturnValueOnce( + JSON.stringify(projectResponse({ itemConnection: connection([item]) })), + ); + for (const page of overflowPages) + run.mockReturnValueOnce(JSON.stringify(page)); + + const evidence = await sourceWith(run).readProjectEvidence(); + + expect( + evidence.items.items[0]["linked pull requests"].map( + ({ number }) => number, + ), + ).toEqual(pullRequests.map(({ number }) => number)); + expect(variables(run.mock.calls.at(-1)[0])).toMatchObject({ + itemId: "item-55", + pullRequestCursor: "pull-requests-10", + }); + }); + + it.each([ + { + name: "cross-page truncation", + page(values) { + return connection(values.slice(0, 2), { totalCount: 4 }); + }, + message: "field values pagination was truncated", + }, + { + name: "duplicate field ID", + page(values, area) { + return connection([area, ...values], { totalCount: 4 }); + }, + message: "field values returned a duplicate identity", + }, + { + name: "changed total", + page(values) { + return connection(values, { totalCount: 5 }); + }, + message: "field values totalCount changed during pagination", + }, + { + name: "repeated cursor", + page(values) { + return connection([values[0]], { + totalCount: 4, + hasNextPage: true, + endCursor: "field-values-1", + }); + }, + message: "field values pagination cursor was repeated", + }, + ])("rejects field-value $name", async ({ page, message }) => { + const item = projectItem(); + const [area, ...remaining] = item.fieldValues.nodes; + item.fieldValues = connection([area], { + totalCount: 4, + hasNextPage: true, + endCursor: "field-values-1", }); + const run = vi + .fn() + .mockReturnValueOnce( + JSON.stringify(projectResponse({ itemConnection: connection([item]) })), + ) + .mockReturnValueOnce( + JSON.stringify(fieldValuePage(item, page(remaining, area))), + ); - await expect(source.listLinkedPullRequests("item-55")).rejects.toThrow( - "Project item item-55 linked pull requests returned a duplicate identity", + await expect(sourceWith(run).readProjectEvidence()).rejects.toThrow( + message, ); }); - it("rejects duplicate closing-issue identities across pages", async () => { - const closingIssue = { - number: 55, - repository: { - name: "RentCottage", - owner: { login: "zaingulel" }, + it("rejects missing field-value cursors and changed item or repository anchors", async () => { + const missingCursorItem = projectItem(); + missingCursorItem.fieldValues.pageInfo.hasNextPage = true; + await expect( + sourceWith( + vi.fn(() => + JSON.stringify( + projectResponse({ + itemConnection: connection([missingCursorItem]), + }), + ), + ), + ).readProjectEvidence(), + ).rejects.toThrow("field values pagination cursor is unavailable"); + + for (const changeAnchor of [ + (item) => { + item.id = "item-changed"; + }, + (item) => { + item.content.repository.nameWithOwner = "other/repository"; + }, + ]) { + const item = projectItem(); + const [area, ...remaining] = item.fieldValues.nodes; + item.fieldValues = connection([area], { + totalCount: 4, + hasNextPage: true, + endCursor: "field-values-1", + }); + const overflowItem = structuredClone(item); + changeAnchor(overflowItem); + const run = vi + .fn() + .mockReturnValueOnce( + JSON.stringify( + projectResponse({ itemConnection: connection([item]) }), + ), + ) + .mockReturnValueOnce( + JSON.stringify( + fieldValuePage( + overflowItem, + connection(remaining, { totalCount: 4 }), + ), + ), + ); + + await expect(sourceWith(run).readProjectEvidence()).rejects.toThrow( + "item or issue identity changed during pagination", + ); + } + }); + + it("keeps independent nested cursors for labels and linked pull requests on different items", async () => { + const labelItem = projectItem(55); + labelItem.content.labels = connection([{ id: "label-a", name: "one" }], { + totalCount: 2, + hasNextPage: true, + endCursor: "labels-55", + }); + const pullRequest = { + id: "pr-70", + number: 70, + url: "https://github.com/zaingulel/RentCottage/pull/70", + repository: { nameWithOwner: repository }, + }; + const pullRequestTwo = { + ...pullRequest, + id: "pr-71", + number: 71, + url: "https://github.com/zaingulel/RentCottage/pull/71", + }; + const pullRequestItem = projectItem(63); + pullRequestItem.fieldValues.nodes[2] = linkedPullRequests([pullRequest], { + totalCount: 2, + hasNextPage: true, + endCursor: "prs-63", + }); + const initial = projectResponse({ + itemConnection: connection([labelItem, pullRequestItem]), + }); + const labelPage = { + data: { + node: { + ...labelItem, + content: { + ...labelItem.content, + labels: connection([{ id: "label-b", name: "two" }], { + totalCount: 2, + }), + }, + }, }, }; - const response = (hasNextPage, endCursor) => - JSON.stringify({ - data: { - repository: { - pullRequest: { - number: 70, - state: "OPEN", - isDraft: false, - mergedAt: null, - url: "https://github.com/zaingulel/RentCottage/pull/70", - closingIssuesReferences: { - totalCount: 2, - nodes: [closingIssue], - pageInfo: { hasNextPage, endCursor }, - }, - }, + const pullRequestPage = { + data: { + node: { + ...pullRequestItem, + fieldValues: { + nodes: [linkedPullRequests([pullRequestTwo], { totalCount: 2 })], }, }, - }); + }, + }; const run = vi .fn() - .mockReturnValueOnce(response(true, "cursor-1")) - .mockReturnValueOnce(response(false, null)); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, + .mockReturnValueOnce(JSON.stringify(initial)) + .mockReturnValueOnce(JSON.stringify(labelPage)) + .mockReturnValueOnce(JSON.stringify(pullRequestPage)); + + const evidence = await sourceWith(run).readProjectEvidence(); + + expect(evidence.items.items[0].labels).toEqual(["one", "two"]); + expect( + evidence.items.items[1]["linked pull requests"].map( + ({ number }) => number, + ), + ).toEqual([70, 71]); + expect(variables(run.mock.calls[1][0])).toMatchObject({ + itemId: "item-55", + cursor: "labels-55", + }); + expect(variables(run.mock.calls[2][0])).toMatchObject({ + itemId: "item-63", + pullRequestCursor: "prs-63", }); + }); - await expect(source.readPullRequest(70)).rejects.toThrow( - "Pull request #70 closing references returned a duplicate identity", - ); + it.each([ + [ + "malformed pageInfo", + (response) => + delete response.data.user.projectV2.items.pageInfo.hasNextPage, + "pagination evidence is invalid", + ], + [ + "missing cursor", + (response) => { + response.data.user.projectV2.items.pageInfo.hasNextPage = true; + }, + "pagination cursor is unavailable", + ], + [ + "truncation", + (response) => { + response.data.user.projectV2.items.totalCount = 2; + }, + "pagination was truncated", + ], + [ + "duplicate field coordinate", + (response) => { + response.data.user.projectV2.fields.nodes[1].id = "field-area"; + }, + "Project fields returned a duplicate identity", + ], + [ + "duplicate item field value", + (response) => { + const values = response.data.user.projectV2.items.nodes[0].fieldValues; + values.nodes.push(structuredClone(values.nodes[0])); + values.totalCount += 1; + }, + "duplicate identity", + ], + [ + "wrong field name", + (response) => { + response.data.user.projectV2.items.nodes[0].fieldValues.nodes[0].field.name = + "Status"; + }, + "Area field identity changed", + ], + [ + "wrong field ID", + (response) => { + response.data.user.projectV2.items.nodes[0].fieldValues.nodes[0].field.id = + "field-area-changed"; + }, + "Area field identity changed", + ], + [ + "wrong field type", + (response) => { + response.data.user.projectV2.items.nodes[0].fieldValues.nodes[0].__typename = + "ProjectV2ItemFieldTextValue"; + }, + "Area field value type is invalid", + ], + [ + "wrong option identity", + (response) => { + response.data.user.projectV2.items.nodes[0].fieldValues.nodes[0].optionId = + "area-changed"; + }, + "Area option identity is invalid", + ], + [ + "safety limit", + (response) => { + response.data.user.projectV2.items.totalCount = 1_001; + }, + "1000-item safety limit", + ], + ])("fails closed for %s evidence", async (_name, mutate, message) => { + const response = projectResponse(); + mutate(response); + const source = sourceWith(vi.fn(() => JSON.stringify(response))); + + await expect(source.readProjectEvidence()).rejects.toThrow(message); }); - it("rejects provider connections over the safety cap", async () => { - const run = vi.fn().mockReturnValue( - JSON.stringify({ - data: { - node: { - fieldValueByName: { - pullRequests: { - totalCount: 1_001, - nodes: [], - pageInfo: { hasNextPage: true, endCursor: "cursor-1" }, - }, - }, - }, - }, + it.each([ + [ + "missing Area coordinate", + (response) => { + const fieldsConnection = response.data.user.projectV2.fields; + fieldsConnection.nodes = fieldsConnection.nodes.filter( + ({ name }) => name !== "Area", + ); + fieldsConnection.totalCount -= 1; + }, + "Project Area field coordinate is unavailable or ambiguous", + ], + [ + "duplicate Status coordinate", + (response) => { + const fieldsConnection = response.data.user.projectV2.fields; + fieldsConnection.nodes.push({ + ...structuredClone(fieldsConnection.nodes[1]), + id: "field-status-two", + }); + fieldsConnection.totalCount += 1; + }, + "coordinates are invalid or ambiguous", + ], + ])("rejects %s", async (_name, mutate, message) => { + const response = projectResponse(); + mutate(response); + + await expect( + sourceWith(vi.fn(() => JSON.stringify(response))).readProjectEvidence(), + ).rejects.toThrow(message); + }); + + it("treats missing Area, Status, and linked-pull-request values as null or empty", async () => { + const response = projectResponse(); + response.data.user.projectV2.items.nodes[0].fieldValues = connection([]); + + const evidence = await sourceWith( + vi.fn(() => JSON.stringify(response)), + ).readProjectEvidence(); + + expect(evidence.items.items[0]).toMatchObject({ + area: null, + status: null, + "linked pull requests": [], + }); + }); + + it("rejects repeated identities, changed totals, and wrong Project anchors", async () => { + const first = projectResponse({ + itemConnection: connection([projectItem(55)], { + totalCount: 2, + hasNextPage: true, + endCursor: "items-1", }), + }); + const repeated = projectResponse({ + itemConnection: connection([projectItem(55)], { totalCount: 2 }), + }); + const duplicateRun = vi + .fn() + .mockReturnValueOnce(JSON.stringify(first)) + .mockReturnValueOnce(JSON.stringify(repeated)); + await expect( + sourceWith(duplicateRun).readProjectEvidence(), + ).rejects.toThrow("Project items returned a duplicate identity"); + + const changed = projectResponse({ + itemConnection: connection([projectItem(63)], { totalCount: 3 }), + }); + const totalRun = vi + .fn() + .mockReturnValueOnce(JSON.stringify(first)) + .mockReturnValueOnce(JSON.stringify(changed)); + await expect(sourceWith(totalRun).readProjectEvidence()).rejects.toThrow( + "Project items totalCount changed during pagination", ); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, + + const wrongAnchor = projectResponse({ + itemConnection: connection([projectItem(63)], { totalCount: 2 }), + }); + wrongAnchor.data.user.projectV2.id = "project-changed"; + const anchorRun = vi + .fn() + .mockReturnValueOnce(JSON.stringify(first)) + .mockReturnValueOnce(JSON.stringify(wrongAnchor)); + await expect(sourceWith(anchorRun).readProjectEvidence()).rejects.toThrow( + "Project items Project identity changed during pagination", + ); + }); + + it("rejects a repeated pagination cursor before replaying a completed page", async () => { + const first = projectResponse({ + itemConnection: connection([projectItem(55)], { + totalCount: 3, + hasNextPage: true, + endCursor: "items-repeated", + }), + }); + const repeatedCursor = projectResponse({ + itemConnection: connection([projectItem(63)], { + totalCount: 3, + hasNextPage: true, + endCursor: "items-repeated", + }), }); + const run = vi + .fn() + .mockReturnValueOnce(JSON.stringify(first)) + .mockReturnValueOnce(JSON.stringify(repeatedCursor)); - await expect(source.listLinkedPullRequests("item-55")).rejects.toThrow( - "Project item item-55 linked pull requests exceeds the 1000-item safety limit", + await expect(sourceWith(run).readProjectEvidence()).rejects.toThrow( + "Project items pagination cursor was repeated", ); - expect(run).toHaveBeenCalledTimes(1); + expect(run).toHaveBeenCalledTimes(2); }); - it("rejects a provider connection that ends before totalCount", async () => { - const run = vi.fn().mockReturnValue( + it("summarizes GraphQL errors without exposing provider messages", async () => { + const privateBody = "private-graphql-body"; + const secret = "ghp_abcdefghijklmnopqrstuvwxyz1234567890"; + const codeSecret = "github_pat_abcdefghijklmnopqrstuvwxyz1234567890"; + const run = vi.fn(() => JSON.stringify({ - data: { - node: { - fieldValueByName: { - pullRequests: { - totalCount: 2, - nodes: [ - { - number: 70, - url: "https://github.com/zaingulel/RentCottage/pull/70", - repository: { nameWithOwner: "zaingulel/RentCottage" }, - }, - ], - pageInfo: { hasNextPage: false, endCursor: null }, - }, - }, + errors: [ + { + message: `${privateBody} Bearer ${secret} ${"detail".repeat(5_000)}`, + extensions: { code: "FORBIDDEN" }, }, - }, + { message: "secondary failure", extensions: { code: codeSecret } }, + ], }), ); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, - }); - await expect(source.listLinkedPullRequests("item-55")).rejects.toThrow( - "Project item item-55 linked pull requests pagination was truncated", + try { + await sourceWith(run).readProjectEvidence(); + throw new Error("expected Project evidence to fail"); + } catch (error) { + expect(error.message).toContain("Project evidence failed"); + expect(error.message).toContain("FORBIDDEN"); + expect(error.message).not.toContain(privateBody); + expect(error.message).not.toContain(secret); + expect(error.message).not.toContain(codeSecret); + expect(error.message.length).toBeLessThan(1_500); + } + }); + + it("rejects contradictory duplicate REST issue identities across pages", async () => { + const issue = { id: 550, number: 55, state: "open", body: "approved" }; + const run = vi.fn(() => + JSON.stringify([[issue], [{ ...issue, id: 551, state: "closed" }]]), + ); + + await expect(sourceWith(run).listIssues()).rejects.toThrow( + "Issue pagination returned a duplicate stable identity", ); }); - it("resolves fresh Project item, field, and option IDs before a field write", async () => { + it("paginates pull-request closing references until totalCount is proven", async () => { + const pullRequest = (nodes, hasNextPage, endCursor) => ({ + data: { + repository: { + pullRequest: { + number: 70, + state: "OPEN", + isDraft: false, + mergedAt: null, + url: "https://github.com/zaingulel/RentCottage/pull/70", + closingIssuesReferences: connection(nodes, { + totalCount: 2, + hasNextPage, + endCursor, + }), + }, + }, + }, + }); + const closingIssue = (number) => ({ + number, + repository: { name: "RentCottage", owner: { login: "zaingulel" } }, + }); const run = vi .fn() .mockReturnValueOnce( - JSON.stringify({ - id: "project-fresh", - number: 4, - owner: { login: "zaingulel" }, - closed: false, - items: { totalCount: 1 }, - fields: { totalCount: 2 }, - }), + JSON.stringify(pullRequest([closingIssue(55)], true, "closing-1")), ) .mockReturnValueOnce( - JSON.stringify({ - totalCount: 2, - fields: [ - { - id: "status-field-fresh", - name: "Status", - options: [{ id: "in-progress-fresh", name: "In progress" }], - }, - { id: "area-field", name: "Area", options: [] }, - ], - }), - ) + JSON.stringify(pullRequest([closingIssue(63)], false, null)), + ); + + const result = await sourceWith(run).readPullRequest(70); + + expect(result.closingIssuesReferences.map(({ number }) => number)).toEqual([ + 55, 63, + ]); + expect(variables(run.mock.calls[1][0])).toMatchObject({ + cursor: "closing-1", + }); + }); + + it("uses freshly read lean evidence coordinates before a field mutation", async () => { + const run = vi + .fn() + .mockReturnValueOnce(JSON.stringify(projectResponse())) .mockReturnValueOnce( JSON.stringify({ - totalCount: 1, - items: [ - { - id: "item-55-fresh", - content: { - number: 55, - type: "Issue", - repository: "zaingulel/RentCottage", - }, - }, - ], + data: { update: { projectV2Item: { id: "item-55" } } }, }), - ) - .mockReturnValueOnce(JSON.stringify({ data: { update: {} } })); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, - }); + ); - await source.execute({ + await sourceWith(run).execute({ type: "set-project-field", issueNumber: 55, field: "Status", value: "In progress", }); - expect(run).toHaveBeenCalledTimes(4); - expect(run.mock.calls[3][0]).toEqual( - expect.arrayContaining([ - "projectId=project-fresh", - "itemId=item-55-fresh", - "fieldId=status-field-fresh", - "optionId=in-progress-fresh", - ]), - ); - const mutationArgs = run.mock.calls[3][0]; - expectVariableFlag(mutationArgs, "projectId", "-F"); - expectVariableFlag(mutationArgs, "itemId", "-F"); - expectVariableFlag(mutationArgs, "fieldId", "-F"); - expectVariableFlag(mutationArgs, "optionId", "-f"); - }); - - it("rejects malformed fresh Project coordinates before a field mutation", async () => { - const run = vi.fn().mockReturnValueOnce(JSON.stringify({ number: 4 })); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, + expect(run).toHaveBeenCalledTimes(2); + expect(variables(run.mock.calls[1][0])).toMatchObject({ + projectId: "project-4", + itemId: "item-55", + fieldId: "field-status", + optionId: "status-progress", }); - - await expect( - source.execute({ - type: "set-project-field", - issueNumber: 55, - field: "Status", - value: "In progress", - }), - ).rejects.toThrow("Fresh Project response is invalid"); - expect(run).toHaveBeenCalledTimes(1); + expect(run.mock.calls[0][0].join(" ")).not.toMatch( + /project (view|field-list|item-list)/, + ); }); - - it.each([ - { - name: "wrong Project identity", - responses: [ - { - id: "project-fresh", - number: 5, - owner: { login: "someone-else" }, - closed: false, - items: { totalCount: 1 }, - fields: { totalCount: 1 }, - }, - ], - message: "Fresh Project response is invalid", - calls: 1, - }, - { - name: "truncated fields", - responses: [ - { - id: "project-fresh", - number: 4, - owner: { login: "zaingulel" }, - closed: false, - items: { totalCount: 1 }, - fields: { totalCount: 2 }, - }, - { - totalCount: 2, - fields: [ - { - id: "status-field", - name: "Status", - options: [{ id: "in-progress", name: "In progress" }], - }, - ], - }, - ], - message: "Fresh Project fields response is invalid", - calls: 2, - }, - { - name: "malformed field options", - responses: [ - { - id: "project-fresh", - number: 4, - owner: { login: "zaingulel" }, - closed: false, - items: { totalCount: 1 }, - fields: { totalCount: 1 }, - }, - { - totalCount: 1, - fields: [ - { - id: "status-field", - name: "Status", - options: [{ id: 7, name: "In progress" }], - }, - ], - }, - ], - message: "Fresh Project fields response is invalid", - calls: 2, - }, - { - name: "duplicate field coordinates", - responses: [ - { - id: "project-fresh", - number: 4, - owner: { login: "zaingulel" }, - closed: false, - items: { totalCount: 1 }, - fields: { totalCount: 2 }, - }, - { - totalCount: 2, - fields: [ - { - id: "duplicate-field", - name: "Status", - options: [{ id: "in-progress", name: "In progress" }], - }, - { - id: "duplicate-field", - name: "Area", - options: [{ id: "foundation", name: "Foundation & quality" }], - }, - ], - }, - ], - message: "Fresh Project fields response is invalid", - calls: 2, - }, - { - name: "duplicate items", - responses: [ - { - id: "project-fresh", - number: 4, - owner: { login: "zaingulel" }, - closed: false, - items: { totalCount: 2 }, - fields: { totalCount: 1 }, - }, - { - totalCount: 1, - fields: [ - { - id: "status-field", - name: "Status", - options: [{ id: "in-progress", name: "In progress" }], - }, - ], - }, - { - totalCount: 2, - items: ["a", "b"].map((suffix) => ({ - id: `item-${suffix}`, - content: { - number: 55, - type: "Issue", - repository: "zaingulel/RentCottage", - }, - })), - }, - ], - message: "Fresh Project items response is invalid", - calls: 3, - }, - ])( - "rejects $name before a field mutation", - async ({ responses, message, calls }) => { - const run = vi.fn(); - for (const response of responses) - run.mockReturnValueOnce(JSON.stringify(response)); - const source = createRentCottageGhSource({ - repository: "zaingulel/RentCottage", - projectOwner: "zaingulel", - projectNumber: 4, - run, - }); - - await expect( - source.execute({ - type: "set-project-field", - issueNumber: 55, - field: "Status", - value: "In progress", - }), - ).rejects.toThrow(message); - expect(run).toHaveBeenCalledTimes(calls); - }, - ); }); diff --git a/scripts/rentcottage-github-adapter.test.mjs b/scripts/rentcottage-github-adapter.test.mjs index 6bf1430..3413cf4 100644 --- a/scripts/rentcottage-github-adapter.test.mjs +++ b/scripts/rentcottage-github-adapter.test.mjs @@ -23,18 +23,17 @@ function policy() { } function completeSource() { - return { - assertSupported: vi.fn().mockResolvedValue(undefined), - readProject: vi.fn().mockResolvedValue({ + const evidence = { + project: { id: "project-4", number: 4, owner: { login: "zaingulel" }, closed: false, items: { totalCount: 1 }, - fields: { totalCount: 2 }, - }), - readProjectFields: vi.fn().mockResolvedValue({ - totalCount: 2, + fields: { totalCount: 3 }, + }, + fields: { + totalCount: 3, fields: [ { id: "field-area", @@ -52,9 +51,13 @@ function completeSource() { { id: "status-done", name: "Done" }, ], }, + { + id: "field-linked", + name: "Linked pull requests", + }, ], - }), - readProjectItems: vi.fn().mockResolvedValue({ + }, + items: { totalCount: 1, items: [ { @@ -69,7 +72,11 @@ function completeSource() { "linked pull requests": [], }, ], - }), + }, + }; + return { + assertSupported: vi.fn().mockResolvedValue(undefined), + readProjectEvidence: vi.fn().mockResolvedValue(evidence), listIssues: vi.fn().mockResolvedValue([ { id: 550, @@ -83,7 +90,6 @@ function completeSource() { }, ]), listBlockedBy: vi.fn().mockResolvedValue([]), - listLinkedPullRequests: vi.fn().mockResolvedValue([]), readPullRequest: vi.fn(), execute: vi.fn(), }; @@ -133,10 +139,9 @@ describe("RentCottage GitHub adapter", () => { it("reads native blockers for new issues discovered from Project 4", async () => { const source = completeSource(); - const project = await source.readProject(); + const evidence = await source.readProjectEvidence(); + const { project, items } = evidence; project.items.totalCount = 2; - source.readProject.mockResolvedValue(project); - const items = await source.readProjectItems(); items.totalCount = 2; items.items.push({ id: "item-63", @@ -149,7 +154,6 @@ describe("RentCottage GitHub adapter", () => { }, "linked pull requests": [], }); - source.readProjectItems.mockResolvedValue(items); const issues = await source.listIssues(); issues.push({ id: 630, @@ -229,13 +233,14 @@ describe("RentCottage GitHub adapter", () => { it("preserves the Project item to pull-request association", async () => { const source = completeSource(); - source.listLinkedPullRequests.mockResolvedValue([ + const { items } = await source.readProjectEvidence(); + items.items[0]["linked pull requests"] = [ { number: 70, url: "https://github.com/zaingulel/RentCottage/pull/70", repository: { nameWithOwner: "zaingulel/RentCottage" }, }, - ]); + ]; source.readPullRequest.mockResolvedValue({ number: 70, state: "OPEN", @@ -253,18 +258,86 @@ describe("RentCottage GitHub adapter", () => { expect(observed.pullRequests).toEqual([ expect.objectContaining({ number: 70, linkedIssues: [55] }), ]); - expect(source.listLinkedPullRequests).toHaveBeenCalledWith("item-55"); + expect(source.readProjectEvidence).toHaveBeenCalledTimes(2); + }); + + it("observes multiple items once and reads each distinct linked or explicit pull request once", async () => { + const source = completeSource(); + const evidence = await source.readProjectEvidence(); + evidence.project.items.totalCount = 2; + evidence.items.totalCount = 2; + evidence.items.items[0]["linked pull requests"] = [ + { + number: 70, + url: "https://github.com/zaingulel/RentCottage/pull/70", + repository: { nameWithOwner: "zaingulel/RentCottage" }, + }, + ]; + evidence.items.items.push({ + ...structuredClone(evidence.items.items[0]), + id: "item-63", + content: { + number: 63, + type: "Issue", + repository: "zaingulel/RentCottage", + }, + "linked pull requests": [ + { + number: 70, + url: "https://github.com/zaingulel/RentCottage/pull/70", + repository: { nameWithOwner: "zaingulel/RentCottage" }, + }, + { + number: 71, + url: "https://github.com/zaingulel/RentCottage/pull/71", + repository: { nameWithOwner: "zaingulel/RentCottage" }, + }, + ], + }); + const issues = await source.listIssues(); + issues.push({ + ...structuredClone(issues[0]), + id: 630, + node_id: "issue-node-63", + number: 63, + title: "Second ticket", + }); + source.readPullRequest.mockImplementation(async (number) => ({ + number, + state: "OPEN", + isDraft: false, + mergedAt: null, + closingIssuesReferences: [], + })); + source.readProjectEvidence.mockClear(); + source.listIssues.mockClear(); + const github = createRentCottageGitHubAdapter({ + source, + policy: policy(), + }); + + const observed = await github.observe({ + type: "audit", + pullRequestNumber: 70, + }); + + expect(observed.complete).toBe(true); + expect(source.readProjectEvidence).toHaveBeenCalledTimes(1); + expect(source.readPullRequest.mock.calls.map(([number]) => number)).toEqual( + [70, 71], + ); }); it("rejects a Project item link to a pull request in another repository", async () => { const source = completeSource(); - source.listLinkedPullRequests.mockResolvedValue([ + const { items } = await source.readProjectEvidence(); + items.items[0]["linked pull requests"] = [ { number: 70, url: "https://github.com/other/repository/pull/70", repository: { nameWithOwner: "other/repository" }, }, - ]); + ]; const github = createRentCottageGitHubAdapter({ source, policy: policy(), @@ -283,13 +356,14 @@ describe("RentCottage GitHub adapter", () => { it("rejects a closing issue reference from another repository", async () => { const source = completeSource(); - source.listLinkedPullRequests.mockResolvedValue([ + const { items } = await source.readProjectEvidence(); + items.items[0]["linked pull requests"] = [ { number: 70, url: "https://github.com/zaingulel/RentCottage/pull/70", repository: { nameWithOwner: "zaingulel/RentCottage" }, }, - ]); + ]; source.readPullRequest.mockResolvedValue({ number: 70, state: "OPEN", @@ -322,11 +396,10 @@ describe("RentCottage GitHub adapter", () => { it("marks evidence incomplete when a required Project option is missing", async () => { const source = completeSource(); - const fields = await source.readProjectFields(); + const { fields } = await source.readProjectEvidence(); fields.fields.find(({ name }) => name === "Status").options = [ { id: "status-backlog", name: "Backlog" }, ]; - source.readProjectFields.mockResolvedValue(fields); const github = createRentCottageGitHubAdapter({ source, policy: policy(), @@ -348,8 +421,8 @@ describe("RentCottage GitHub adapter", () => { ...structuredClone(fields.fields[0]), id: "field-area-duplicate", }); - fields.totalCount = 3; - project.fields.totalCount = 3; + fields.totalCount = 4; + project.fields.totalCount = 4; }, }, { @@ -375,11 +448,8 @@ describe("RentCottage GitHub adapter", () => { }, ])("rejects $name before field normalization", async ({ change }) => { const source = completeSource(); - const fields = await source.readProjectFields(); - const project = await source.readProject(); + const { fields, project } = await source.readProjectEvidence(); change(fields, project); - source.readProjectFields.mockResolvedValue(fields); - source.readProject.mockResolvedValue(project); const github = createRentCottageGitHubAdapter({ source, policy: policy(), @@ -397,7 +467,8 @@ describe("RentCottage GitHub adapter", () => { it("marks draft or foreign Project items as incomplete evidence", async () => { const source = completeSource(); - source.readProjectItems.mockResolvedValue({ + const evidence = await source.readProjectEvidence(); + evidence.items = { totalCount: 1, items: [ { @@ -407,7 +478,7 @@ describe("RentCottage GitHub adapter", () => { status: "Backlog", }, ], - }); + }; const github = createRentCottageGitHubAdapter({ source, policy: policy(), @@ -425,9 +496,8 @@ describe("RentCottage GitHub adapter", () => { it("rejects an Issue item whose repository provenance is unavailable", async () => { const source = completeSource(); - const items = await source.readProjectItems(); + const { items } = await source.readProjectEvidence(); delete items.items[0].content.repository; - source.readProjectItems.mockResolvedValue(items); const github = createRentCottageGitHubAdapter({ source, policy: policy(), @@ -445,14 +515,15 @@ describe("RentCottage GitHub adapter", () => { it("marks the wrong Project identity as incomplete evidence", async () => { const source = completeSource(); - source.readProject.mockResolvedValue({ + const evidence = await source.readProjectEvidence(); + evidence.project = { id: "project-5", number: 5, owner: { login: "someone-else" }, closed: false, items: { totalCount: 1 }, - fields: { totalCount: 2 }, - }); + fields: { totalCount: 3 }, + }; const github = createRentCottageGitHubAdapter({ source, policy: policy(), @@ -468,7 +539,8 @@ describe("RentCottage GitHub adapter", () => { it("turns malformed provider JSON into explicit incomplete evidence", async () => { const source = completeSource(); - source.readProject.mockResolvedValue({ id: "project-4", owner: null }); + const evidence = await source.readProjectEvidence(); + evidence.project = { id: "project-4", owner: null }; const github = createRentCottageGitHubAdapter({ source, policy: policy(), @@ -489,7 +561,9 @@ describe("RentCottage GitHub adapter", () => { it("turns an unavailable GitHub API into explicit incomplete evidence", async () => { const source = completeSource(); - source.readProject.mockRejectedValue(new Error("permission denied")); + source.readProjectEvidence.mockRejectedValue( + new Error("permission denied"), + ); const github = createRentCottageGitHubAdapter({ source, policy: policy(), @@ -505,9 +579,8 @@ describe("RentCottage GitHub adapter", () => { it("marks an unknown Project item Status as incomplete evidence", async () => { const source = completeSource(); - const items = await source.readProjectItems(); + const { items } = await source.readProjectEvidence(); items.items[0].status = "Unexpected"; - source.readProjectItems.mockResolvedValue(items); const github = createRentCottageGitHubAdapter({ source, policy: policy(), From 8dfa4e11c7610fc47f6c356f32778b01fc3c1f01 Mon Sep 17 00:00:00 2001 From: Zain Gulel <60632602+zaingulel@users.noreply.github.com> Date: Sun, 16 Aug 2026 14:54:47 +0100 Subject: [PATCH 2/6] Tighten reconciliation call-budget test --- scripts/lib/rentcottage-gh-source.mjs | 1 + scripts/rentcottage-github-adapter.test.mjs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/lib/rentcottage-gh-source.mjs b/scripts/lib/rentcottage-gh-source.mjs index d69da94..a0c56a4 100644 --- a/scripts/lib/rentcottage-gh-source.mjs +++ b/scripts/lib/rentcottage-gh-source.mjs @@ -486,6 +486,7 @@ export function createRentCottageGhSource({ const normalizedItems = []; for (const item of items.nodes) { const context = `Project item ${item?.id ?? "unknown"}`; + // Self-anchoring validates the initial item shape and repository before nested pagination. requireItemAnchor(item, item, repository, context); const labels = createConnectionState( diff --git a/scripts/rentcottage-github-adapter.test.mjs b/scripts/rentcottage-github-adapter.test.mjs index 3413cf4..6cd2736 100644 --- a/scripts/rentcottage-github-adapter.test.mjs +++ b/scripts/rentcottage-github-adapter.test.mjs @@ -252,13 +252,14 @@ describe("RentCottage GitHub adapter", () => { source, policy: policy(), }); + source.readProjectEvidence.mockClear(); const observed = await github.observe({ type: "audit" }); expect(observed.pullRequests).toEqual([ expect.objectContaining({ number: 70, linkedIssues: [55] }), ]); - expect(source.readProjectEvidence).toHaveBeenCalledTimes(2); + expect(source.readProjectEvidence).toHaveBeenCalledTimes(1); }); it("observes multiple items once and reads each distinct linked or explicit pull request once", async () => { From 5b2eb072465af1caa7a78199cc48f31a5e74e6a6 Mon Sep 17 00:00:00 2001 From: Zain Gulel <60632602+zaingulel@users.noreply.github.com> Date: Sun, 16 Aug 2026 15:02:34 +0100 Subject: [PATCH 3/6] Use lean project identity read for item writes --- scripts/lib/rentcottage-gh-source.mjs | 35 +++++++-- scripts/rentcottage-gh-source.test.mjs | 101 +++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 6 deletions(-) diff --git a/scripts/lib/rentcottage-gh-source.mjs b/scripts/lib/rentcottage-gh-source.mjs index a0c56a4..3579b4d 100644 --- a/scripts/lib/rentcottage-gh-source.mjs +++ b/scripts/lib/rentcottage-gh-source.mjs @@ -191,6 +191,7 @@ function requireProjectAnchor( project, { projectOwner, projectNumber, projectId }, context, + errorMessage = `${context} Project identity changed during pagination`, ) { if ( !isRecord(project) || @@ -201,7 +202,7 @@ function requireProjectAnchor( !isRecord(project.owner) || project.owner.login !== projectOwner ) { - throw new Error(`${context} Project identity changed during pagination`); + throw new Error(errorMessage); } } @@ -386,6 +387,32 @@ export function createRentCottageGhSource({ projectNumber, run = runGh, }) { + const readProjectIdentity = () => { + const query = `query($login: String!, $number: Int!) { + user(login: $login) { + login + projectV2(number: $number) { + id number closed owner { ... on User { login } } + } + } + }`; + const response = requireGraphqlSuccess( + run(graphqlArgs(query, { login: projectOwner, number: projectNumber })), + "Fresh Project identity", + ); + const user = response.data?.user; + if (!isRecord(user) || user.login !== projectOwner) + throw new Error("Fresh Project owner identity is invalid"); + const project = user.projectV2; + requireProjectAnchor( + project, + { projectOwner, projectNumber }, + "Fresh", + "Fresh Project identity is invalid", + ); + return project; + }; + const source = { async assertSupported() { assertSupportedGhVersion(run(["--version"])); @@ -728,11 +755,7 @@ export function createRentCottageGhSource({ async execute(operation) { if (operation.type === "add-project-item") { - const { project } = await source.readProjectEvidence(); - if ( - !hasFreshProjectCoordinates(project, { projectOwner, projectNumber }) - ) - throw new Error("Fresh Project response is invalid"); + const project = readProjectIdentity(); const query = `mutation($projectId: ID!, $contentId: ID!) { addProjectV2ItemById(input: {projectId: $projectId, contentId: $contentId}) { item { id } } }`; diff --git a/scripts/rentcottage-gh-source.test.mjs b/scripts/rentcottage-gh-source.test.mjs index 1b21e14..d45136e 100644 --- a/scripts/rentcottage-gh-source.test.mjs +++ b/scripts/rentcottage-gh-source.test.mjs @@ -1106,6 +1106,107 @@ describe("RentCottage gh source", () => { }); }); + it("uses a fresh identity-only Project query before adding an item", async () => { + const run = vi + .fn() + .mockReturnValueOnce( + JSON.stringify({ + data: { + user: { + login: "zaingulel", + projectV2: { + id: "project-4", + number: 4, + closed: false, + owner: { login: "zaingulel" }, + }, + }, + }, + }), + ) + .mockReturnValueOnce( + JSON.stringify({ + data: { addProjectV2ItemById: { item: { id: "item-55" } } }, + }), + ); + + await sourceWith(run).execute({ + type: "add-project-item", + contentNodeId: "issue-55", + }); + + expect(run).toHaveBeenCalledTimes(2); + expect(queryFrom(run.mock.calls[0][0])).not.toMatch( + /\b(fields|items)\s*\(/, + ); + expect(variables(run.mock.calls[0][0])).toMatchObject({ + login: "zaingulel", + number: "4", + }); + expect(variables(run.mock.calls[1][0])).toMatchObject({ + projectId: "project-4", + contentId: "issue-55", + }); + }); + + it.each([ + [ + "changed lookup owner", + (response) => { + response.data.user.login = "another-owner"; + }, + "Fresh Project owner identity is invalid", + ], + [ + "changed Project owner", + (response) => { + response.data.user.projectV2.owner.login = "another-owner"; + }, + "Fresh Project identity is invalid", + ], + [ + "closed Project", + (response) => { + response.data.user.projectV2.closed = true; + }, + "Fresh Project identity is invalid", + ], + [ + "changed Project number", + (response) => { + response.data.user.projectV2.number = 5; + }, + "Fresh Project identity is invalid", + ], + ])( + "rejects %s before adding an item", + async (_name, mutate, errorMessage) => { + const response = { + data: { + user: { + login: "zaingulel", + projectV2: { + id: "project-4", + number: 4, + closed: false, + owner: { login: "zaingulel" }, + }, + }, + }, + }; + mutate(response); + const run = vi.fn(() => JSON.stringify(response)); + + await expect( + sourceWith(run).execute({ + type: "add-project-item", + contentNodeId: "issue-55", + }), + ).rejects.toThrow(errorMessage); + expect(run).toHaveBeenCalledTimes(1); + }, + ); + it("uses freshly read lean evidence coordinates before a field mutation", async () => { const run = vi .fn() From c44c22117bd303ddfe0eca3a399fea9791403c23 Mon Sep 17 00:00:00 2001 From: Zain Gulel <60632602+zaingulel@users.noreply.github.com> Date: Sun, 16 Aug 2026 15:12:34 +0100 Subject: [PATCH 4/6] Preserve reconciliation evidence failures --- scripts/lib/rentcottage-github-adapter.mjs | 19 +++---------------- scripts/lib/rentcottage-github-schema.mjs | 19 +++++++++++-------- scripts/rentcottage-gh-source.test.mjs | 2 +- scripts/rentcottage-github-adapter.test.mjs | 20 ++++++++++++++++++++ 4 files changed, 35 insertions(+), 25 deletions(-) diff --git a/scripts/lib/rentcottage-github-adapter.mjs b/scripts/lib/rentcottage-github-adapter.mjs index 13b2df1..0bbd15b 100644 --- a/scripts/lib/rentcottage-github-adapter.mjs +++ b/scripts/lib/rentcottage-github-adapter.mjs @@ -2,6 +2,7 @@ import { sameValues } from "./value-comparison.mjs"; import { hasUniqueRepositoryIssueIdentities } from "./github-pagination.mjs"; import { hasUniqueProjectFieldCoordinates, + isLinkedPullRequestRecord, isProjectFieldRecord, isProjectItemRecord, isRecord, @@ -127,16 +128,6 @@ function pullRequestLink(url) { return match ? { repository: match[1], number: Number(match[2]) } : null; } -function isLinkedPullRequestResponse(pullRequest) { - return ( - isRecord(pullRequest) && - Number.isInteger(pullRequest.number) && - typeof pullRequest.url === "string" && - isRecord(pullRequest.repository) && - typeof pullRequest.repository.nameWithOwner === "string" - ); -} - export function createRentCottageGitHubAdapter({ source, policy }) { return { async observe(intent) { @@ -234,12 +225,12 @@ export function createRentCottageGitHubAdapter({ source, policy }) { pullRequestNumbers.add(intent.pullRequestNumber); const linkedPullRequestsByItem = rawItems.items .filter((item) => item.content.type === "Issue") - .map((item) => [item, item["linked pull requests"] ?? []]); + .map((item) => [item, item["linked pull requests"]]); if ( linkedPullRequestsByItem.some( ([, pullRequests]) => !Array.isArray(pullRequests) || - !pullRequests.every(isLinkedPullRequestResponse), + !pullRequests.every(isLinkedPullRequestRecord), ) ) { return incompleteObservation(policy, [ @@ -306,8 +297,6 @@ export function createRentCottageGitHubAdapter({ source, policy }) { "Project identity does not match RentCottage Project 4", ); } - if (project.items.totalCount !== rawItems.totalCount) - evidenceErrors.push("Project item counts disagree between reads"); if (rawItems.totalCount !== rawItems.items.length) evidenceErrors.push("Project items pagination was truncated"); if ( @@ -322,8 +311,6 @@ export function createRentCottageGitHubAdapter({ source, policy }) { "Project contains a draft, pull request, foreign item, or unavailable item", ); } - if (project.fields.totalCount !== rawFields.totalCount) - evidenceErrors.push("Project field counts disagree between reads"); if (rawFields.totalCount !== rawFields.fields.length) evidenceErrors.push("Project fields pagination was truncated"); if (!hasUniqueProjectFieldCoordinates(rawFields.fields)) diff --git a/scripts/lib/rentcottage-github-schema.mjs b/scripts/lib/rentcottage-github-schema.mjs index 352e60b..cfce9f5 100644 --- a/scripts/lib/rentcottage-github-schema.mjs +++ b/scripts/lib/rentcottage-github-schema.mjs @@ -34,6 +34,16 @@ export function hasUniqueProjectFieldCoordinates(fields) { ); } +export function isLinkedPullRequestRecord(pullRequest) { + return ( + isRecord(pullRequest) && + Number.isInteger(pullRequest.number) && + typeof pullRequest.url === "string" && + isRecord(pullRequest.repository) && + typeof pullRequest.repository.nameWithOwner === "string" + ); +} + export function isProjectItemRecord(item) { return ( isRecord(item) && @@ -52,13 +62,6 @@ export function isProjectItemRecord(item) { typeof item.status === "string") && (item["linked pull requests"] === undefined || (Array.isArray(item["linked pull requests"]) && - item["linked pull requests"].every( - (pullRequest) => - isRecord(pullRequest) && - Number.isInteger(pullRequest.number) && - typeof pullRequest.url === "string" && - isRecord(pullRequest.repository) && - typeof pullRequest.repository.nameWithOwner === "string", - ))) + item["linked pull requests"].every(isLinkedPullRequestRecord))) ); } diff --git a/scripts/rentcottage-gh-source.test.mjs b/scripts/rentcottage-gh-source.test.mjs index d45136e..1164a8a 100644 --- a/scripts/rentcottage-gh-source.test.mjs +++ b/scripts/rentcottage-gh-source.test.mjs @@ -369,7 +369,7 @@ describe("RentCottage gh source", () => { "Notes field identity changed", ], ])( - "rejects an unconsumed Text value with a $name", + "rejects an unconsumed Text value with a %s", async (_name, mutate, message) => { const response = projectResponse(); mutate(response.data.user.projectV2.items.nodes[0].fieldValues.nodes[3]); diff --git a/scripts/rentcottage-github-adapter.test.mjs b/scripts/rentcottage-github-adapter.test.mjs index 6cd2736..1505970 100644 --- a/scripts/rentcottage-github-adapter.test.mjs +++ b/scripts/rentcottage-github-adapter.test.mjs @@ -355,6 +355,26 @@ describe("RentCottage GitHub adapter", () => { expect(source.readPullRequest).not.toHaveBeenCalled(); }); + it("rejects missing linked-pull-request evidence for an Issue item", async () => { + const source = completeSource(); + const { items } = await source.readProjectEvidence(); + delete items.items[0]["linked pull requests"]; + const github = createRentCottageGitHubAdapter({ + source, + policy: policy(), + }); + + const observed = await github.observe({ type: "audit" }); + + expect(observed).toMatchObject({ + complete: false, + evidenceErrors: [ + "Linked pull requests response does not match the expected GitHub schema", + ], + }); + expect(source.readPullRequest).not.toHaveBeenCalled(); + }); + it("rejects a closing issue reference from another repository", async () => { const source = completeSource(); const { items } = await source.readProjectEvidence(); From d222a5107f73239c9f1815fe028c8728c0f25949 Mon Sep 17 00:00:00 2001 From: Zain Gulel <60632602+zaingulel@users.noreply.github.com> Date: Sun, 16 Aug 2026 15:45:34 +0100 Subject: [PATCH 5/6] Address reconciliation review findings --- scripts/lib/rentcottage-gh-source.mjs | 167 +++++++++++++++++++- scripts/lib/rentcottage-github-adapter.mjs | 21 ++- scripts/rentcottage-gh-source.test.mjs | 132 +++++++++++++++- scripts/rentcottage-github-adapter.test.mjs | 95 +++++------ 4 files changed, 343 insertions(+), 72 deletions(-) diff --git a/scripts/lib/rentcottage-gh-source.mjs b/scripts/lib/rentcottage-gh-source.mjs index 3579b4d..340ab9e 100644 --- a/scripts/lib/rentcottage-gh-source.mjs +++ b/scripts/lib/rentcottage-gh-source.mjs @@ -104,6 +104,7 @@ const FIELD_VALUE_SELECTION = ` ... on ProjectV2ItemFieldUserValue { ${FIELD_COORDINATE_SELECTION} } ... on ProjectV2ItemIssueFieldValue { ${FIELD_COORDINATE_SELECTION} } `; +const FIELD_VALUE_ORDER = "orderBy: { field: POSITION, direction: ASC }"; function graphqlArgs(query, variables = {}) { const args = ["api", "graphql", "-f", `query=${query}`]; @@ -225,6 +226,41 @@ function requireItemAnchor(item, expected, repository, context) { } } +function isInitialRepositoryIssue(item, repository) { + return ( + isRecord(item) && + typeof item.id === "string" && + isRecord(item.content) && + item.content.__typename === "Issue" && + typeof item.content.id === "string" && + Number.isInteger(item.content.number) && + item.content.repository?.nameWithOwner === repository + ); +} + +function normalizeInitialItem(item) { + const content = isRecord(item.content) ? item.content : null; + const availableIssue = + content?.__typename === "Issue" && + typeof content.id === "string" && + Number.isInteger(content.number) && + typeof content.repository?.nameWithOwner === "string"; + const type = + content?.__typename === "Issue" && !availableIssue + ? "Unavailable" + : (content?.__typename ?? "Unavailable"); + return { + id: item.id, + content: { + type, + ...(Number.isInteger(content?.number) ? { number: content.number } : {}), + ...(typeof content?.repository?.nameWithOwner === "string" + ? { repository: content.repository.nameWithOwner } + : {}), + }, + }; +} + function linkedPullRequestIdentity(pullRequest) { if ( !isRecord(pullRequest) || @@ -345,7 +381,7 @@ const PROJECT_EVIDENCE_QUERY = `query($login: String!, $number: Int!) { totalCount nodes { id content { __typename ... on Issue { id number repository { nameWithOwner } labels(first: ${CONNECTION_PAGE_SIZE}) { totalCount nodes { id name } pageInfo { hasNextPage endCursor } } } } - fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}) { + fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}, ${FIELD_VALUE_ORDER}) { totalCount nodes { ${FIELD_VALUE_SELECTION} } @@ -413,6 +449,121 @@ export function createRentCottageGhSource({ return project; }; + const readFreshProjectCoordinates = () => { + const read = (query, variables, context) => + requireGraphqlSuccess(run(graphqlArgs(query, variables)), context); + const query = `query($login: String!, $number: Int!) { + user(login: $login) { login projectV2(number: $number) { + id number closed owner { ... on User { login } } + fields(first: ${CONNECTION_PAGE_SIZE}) { + totalCount nodes { __typename ... on ProjectV2FieldCommon { id name } ... on ProjectV2SingleSelectField { id name options { id name } } } + pageInfo { hasNextPage endCursor } + } + items(first: ${CONNECTION_PAGE_SIZE}) { + totalCount nodes { id content { __typename ... on Issue { id number repository { nameWithOwner } } } } + pageInfo { hasNextPage endCursor } + } + } } + }`; + const response = read( + query, + { login: projectOwner, number: projectNumber }, + "Fresh Project coordinates", + ); + const user = response.data?.user; + const project = user?.projectV2; + if (!isRecord(user) || user.login !== projectOwner) + throw new Error("Fresh Project owner identity is invalid"); + requireProjectAnchor( + project, + { projectOwner, projectNumber }, + "Fresh Project coordinates", + "Fresh Project response is invalid", + ); + + const fields = createConnectionState( + project.fields, + "Fresh Project fields", + (field) => field?.id, + ); + while (fields.hasNextPage) { + const pageQuery = `query($login: String!, $number: Int!, $cursor: String!) { + user(login: $login) { login projectV2(number: $number) { + id number closed owner { ... on User { login } } + fields(first: ${CONNECTION_PAGE_SIZE}, after: $cursor) { + totalCount nodes { __typename ... on ProjectV2FieldCommon { id name } ... on ProjectV2SingleSelectField { id name options { id name } } } + pageInfo { hasNextPage endCursor } + } + } } + }`; + const page = read( + pageQuery, + { login: projectOwner, number: projectNumber, cursor: fields.cursor }, + "Fresh Project fields", + ); + if (page.data?.user?.login !== projectOwner) + throw new Error( + "Fresh Project fields owner identity changed during pagination", + ); + const pageProject = page.data?.user?.projectV2; + requireProjectAnchor( + pageProject, + { projectOwner, projectNumber, projectId: project.id }, + "Fresh Project fields", + ); + appendConnectionPage(fields, pageProject.fields, (field) => field?.id); + } + + const items = createConnectionState( + project.items, + "Fresh Project items", + (item) => item?.id, + ); + while (items.hasNextPage) { + const pageQuery = `query($login: String!, $number: Int!, $cursor: String!) { + user(login: $login) { login projectV2(number: $number) { + id number closed owner { ... on User { login } } + items(first: ${CONNECTION_PAGE_SIZE}, after: $cursor) { + totalCount nodes { id content { __typename ... on Issue { id number repository { nameWithOwner } } } } + pageInfo { hasNextPage endCursor } + } + } } + }`; + const page = read( + pageQuery, + { login: projectOwner, number: projectNumber, cursor: items.cursor }, + "Fresh Project items", + ); + if (page.data?.user?.login !== projectOwner) + throw new Error( + "Fresh Project items owner identity changed during pagination", + ); + const pageProject = page.data?.user?.projectV2; + requireProjectAnchor( + pageProject, + { projectOwner, projectNumber, projectId: project.id }, + "Fresh Project items", + ); + appendConnectionPage(items, pageProject.items, (item) => item?.id); + } + + return { + project: { + id: project.id, + number: project.number, + owner: { login: user.login }, + closed: project.closed, + items: { totalCount: items.totalCount }, + fields: { totalCount: fields.totalCount }, + }, + fields: { totalCount: fields.totalCount, fields: fields.nodes }, + items: { + totalCount: items.totalCount, + items: items.nodes.map(normalizeInitialItem), + }, + }; + }; + const source = { async assertSupported() { assertSupportedGhVersion(run(["--version"])); @@ -483,7 +634,7 @@ export function createRentCottageGhSource({ totalCount nodes { id content { __typename ... on Issue { id number repository { nameWithOwner } labels(first: ${CONNECTION_PAGE_SIZE}) { totalCount nodes { id name } pageInfo { hasNextPage endCursor } } } } - fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}) { + fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}, ${FIELD_VALUE_ORDER}) { totalCount nodes { ${FIELD_VALUE_SELECTION} } pageInfo { hasNextPage endCursor } } @@ -513,8 +664,10 @@ export function createRentCottageGhSource({ const normalizedItems = []; for (const item of items.nodes) { const context = `Project item ${item?.id ?? "unknown"}`; - // Self-anchoring validates the initial item shape and repository before nested pagination. - requireItemAnchor(item, item, repository, context); + if (!isInitialRepositoryIssue(item, repository)) { + normalizedItems.push(normalizeInitialItem(item)); + continue; + } const labels = createConnectionState( item.content.labels, @@ -559,7 +712,7 @@ export function createRentCottageGhSource({ const query = `query($itemId: ID!, $cursor: String!) { node(id: $itemId) { ... on ProjectV2Item { id content { __typename ... on Issue { id number repository { nameWithOwner } } } - fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}, after: $cursor) { + fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}, after: $cursor, ${FIELD_VALUE_ORDER}) { totalCount nodes { ${FIELD_VALUE_SELECTION} } pageInfo { hasNextPage endCursor } } @@ -607,7 +760,7 @@ export function createRentCottageGhSource({ const query = `query($itemId: ID!, $fieldCursor: String, $pullRequestCursor: String!) { node(id: $itemId) { ... on ProjectV2Item { id content { __typename ... on Issue { id number repository { nameWithOwner } } } - fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}, after: $fieldCursor) { + fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}, after: $fieldCursor, ${FIELD_VALUE_ORDER}) { nodes { __typename ... on ProjectV2ItemFieldPullRequestValue { field { ... on ProjectV2FieldCommon { id name } } pullRequests(first: ${LINKED_PULL_REQUEST_PAGE_SIZE}, after: $pullRequestCursor) { totalCount nodes { id number url repository { nameWithOwner } } pageInfo { hasNextPage endCursor } } } } } } } @@ -776,7 +929,7 @@ export function createRentCottageGhSource({ } if (operation.type === "set-project-field") { - const { project, fields, items } = await source.readProjectEvidence(); + const { project, fields, items } = readFreshProjectCoordinates(); if ( !hasFreshProjectCoordinates(project, { projectOwner, projectNumber }) ) diff --git a/scripts/lib/rentcottage-github-adapter.mjs b/scripts/lib/rentcottage-github-adapter.mjs index 0bbd15b..d83f0ec 100644 --- a/scripts/lib/rentcottage-github-adapter.mjs +++ b/scripts/lib/rentcottage-github-adapter.mjs @@ -53,6 +53,14 @@ function isProjectItemsResponse(value) { ); } +function isRepositoryIssueItem(item, repository) { + return ( + item.content.type === "Issue" && + Number.isInteger(item.content.number) && + item.content.repository === repository + ); +} + function isIssueResponse(issue) { return ( isRecord(issue) && @@ -180,7 +188,7 @@ export function createRentCottageGitHubAdapter({ source, policy }) { const dependencyIssueNumbers = new Set([ ...policy.issues.keys(), ...rawItems.items - .filter((item) => item.content.type === "Issue") + .filter((item) => isRepositoryIssueItem(item, policy.repository)) .map((item) => item.content.number), ]); blockerEntries = await Promise.all( @@ -224,7 +232,7 @@ export function createRentCottageGitHubAdapter({ source, policy }) { if (intent.pullRequestNumber) pullRequestNumbers.add(intent.pullRequestNumber); const linkedPullRequestsByItem = rawItems.items - .filter((item) => item.content.type === "Issue") + .filter((item) => isRepositoryIssueItem(item, policy.repository)) .map((item) => [item, item["linked pull requests"]]); if ( linkedPullRequestsByItem.some( @@ -301,10 +309,7 @@ export function createRentCottageGitHubAdapter({ source, policy }) { evidenceErrors.push("Project items pagination was truncated"); if ( rawItems.items.some( - (item) => - item.content?.type !== "Issue" || - !Number.isInteger(item.content?.number) || - item.content.repository !== policy.repository, + (item) => !isRepositoryIssueItem(item, policy.repository), ) ) { evidenceErrors.push( @@ -350,7 +355,9 @@ export function createRentCottageGitHubAdapter({ source, policy }) { ) { evidenceErrors.push("Project Area options do not match the contract"); } - for (const item of rawItems.items) { + for (const item of rawItems.items.filter((item) => + isRepositoryIssueItem(item, policy.repository), + )) { if ( item.area !== null && fields.Area && diff --git a/scripts/rentcottage-gh-source.test.mjs b/scripts/rentcottage-gh-source.test.mjs index 1164a8a..be5c023 100644 --- a/scripts/rentcottage-gh-source.test.mjs +++ b/scripts/rentcottage-gh-source.test.mjs @@ -104,6 +104,18 @@ function projectItem(number = 55, overrides = {}) { }; } +function freshCoordinateItem(number = 55) { + return { + id: `item-${number}`, + content: { + __typename: "Issue", + id: `issue-${number}`, + number, + repository: { nameWithOwner: repository }, + }, + }; +} + function projectResponse({ fieldConnection, itemConnection } = {}) { return { data: { @@ -338,6 +350,39 @@ describe("RentCottage gh source", () => { } }); + it.each([ + ["draft", { __typename: "DraftIssue" }, { type: "DraftIssue" }], + ["pull request", { __typename: "PullRequest" }, { type: "PullRequest" }], + [ + "foreign Issue", + { + __typename: "Issue", + id: "issue-55", + number: 55, + repository: { nameWithOwner: "other/repository" }, + }, + { type: "Issue", number: 55, repository: "other/repository" }, + ], + ["unavailable", null, { type: "Unavailable" }], + ])( + "preserves an initial %s item for project-evidence classification", + async (_name, content, expectedContent) => { + const item = projectItem(); + item.content = content; + const response = projectResponse({ + itemConnection: connection([item]), + }); + + const evidence = await sourceWith( + vi.fn(() => JSON.stringify(response)), + ).readProjectEvidence(); + + expect(evidence.items.items).toEqual([ + { id: "item-55", content: expectedContent }, + ]); + }, + ); + it("tolerates a populated unconsumed Text field without normalizing it", async () => { const response = projectResponse(); @@ -431,7 +476,11 @@ describe("RentCottage gh source", () => { expect(variables(run.mock.calls[1][0])).toMatchObject({ cursor: "items-1", }); - expect(run.mock.calls[1][0].join(" ")).not.toContain("fields(first:"); + const overflowQuery = queryFrom(run.mock.calls[1][0]); + expect(overflowQuery).not.toContain("fields(first:"); + expect(overflowQuery).toMatch( + /fieldValues\(first:\s*20,\s*orderBy:\s*\{\s*field:\s*POSITION,\s*direction:\s*ASC\s*\}\)/, + ); }); it("paginates item field values independently without replaying completed connections", async () => { @@ -583,6 +632,11 @@ describe("RentCottage gh source", () => { fieldCursor: "field-values-1", pullRequestCursor: "pull-requests-1", }); + for (const call of run.mock.calls) { + expect(queryFrom(call[0])).toMatch( + /fieldValues\(first:\s*20(?:,\s*after:\s*\$\w+)?\s*,\s*orderBy:\s*\{\s*field:\s*POSITION,\s*direction:\s*ASC\s*\}\)/, + ); + } }); it("normalizes linked pull requests after more than ten provider-sized pages", async () => { @@ -1210,7 +1264,13 @@ describe("RentCottage gh source", () => { it("uses freshly read lean evidence coordinates before a field mutation", async () => { const run = vi .fn() - .mockReturnValueOnce(JSON.stringify(projectResponse())) + .mockReturnValueOnce( + JSON.stringify( + projectResponse({ + itemConnection: connection([freshCoordinateItem()]), + }), + ), + ) .mockReturnValueOnce( JSON.stringify({ data: { update: { projectV2Item: { id: "item-55" } } }, @@ -1234,5 +1294,73 @@ describe("RentCottage gh source", () => { expect(run.mock.calls[0][0].join(" ")).not.toMatch( /project (view|field-list|item-list)/, ); + const freshnessQuery = queryFrom(run.mock.calls[0][0]); + expect(freshnessQuery).toMatch(/\bfields\(first:\s*100\)/); + expect(freshnessQuery).toMatch(/\bitems\(first:\s*100\)/); + expect(freshnessQuery).toMatch(/\.\.\. on Issue\s*\{\s*id\b/); + expect(freshnessQuery).not.toMatch( + /\b(labels|fieldValues|pullRequests)\s*\(/, + ); + }); + + it("paginates lean field and item coordinates before a field mutation", async () => { + const allFields = fields(); + const first = projectResponse({ + fieldConnection: connection([allFields[0]], { + totalCount: 4, + hasNextPage: true, + endCursor: "fresh-fields-1", + }), + itemConnection: connection([freshCoordinateItem(55)], { + totalCount: 2, + hasNextPage: true, + endCursor: "fresh-items-1", + }), + }); + const fieldOverflow = projectResponse({ + fieldConnection: connection(allFields.slice(1), { totalCount: 4 }), + }); + const itemOverflow = projectResponse({ + itemConnection: connection([freshCoordinateItem(63)], { totalCount: 2 }), + }); + const run = vi + .fn() + .mockReturnValueOnce(JSON.stringify(first)) + .mockReturnValueOnce(JSON.stringify(fieldOverflow)) + .mockReturnValueOnce(JSON.stringify(itemOverflow)) + .mockReturnValueOnce( + JSON.stringify({ + data: { update: { projectV2Item: { id: "item-55" } } }, + }), + ); + + await sourceWith(run).execute({ + type: "set-project-field", + issueNumber: 55, + field: "Status", + value: "In progress", + }); + + expect(run).toHaveBeenCalledTimes(4); + expect(variables(run.mock.calls[1][0])).toMatchObject({ + cursor: "fresh-fields-1", + }); + expect(variables(run.mock.calls[2][0])).toMatchObject({ + cursor: "fresh-items-1", + }); + for (const call of run.mock.calls.slice(0, 3)) { + expect(queryFrom(call[0])).not.toMatch( + /\b(labels|fieldValues|pullRequests)\s*\(/, + ); + } + for (const call of [run.mock.calls[0], run.mock.calls[2]]) { + expect(queryFrom(call[0])).toMatch(/\.\.\. on Issue\s*\{\s*id\b/); + } + expect(variables(run.mock.calls[3][0])).toMatchObject({ + projectId: "project-4", + itemId: "item-55", + fieldId: "field-status", + optionId: "status-progress", + }); }); }); diff --git a/scripts/rentcottage-github-adapter.test.mjs b/scripts/rentcottage-github-adapter.test.mjs index 1505970..7118988 100644 --- a/scripts/rentcottage-github-adapter.test.mjs +++ b/scripts/rentcottage-github-adapter.test.mjs @@ -75,6 +75,7 @@ function completeSource() { }, }; return { + evidence, assertSupported: vi.fn().mockResolvedValue(undefined), readProjectEvidence: vi.fn().mockResolvedValue(evidence), listIssues: vi.fn().mockResolvedValue([ @@ -139,7 +140,7 @@ describe("RentCottage GitHub adapter", () => { it("reads native blockers for new issues discovered from Project 4", async () => { const source = completeSource(); - const evidence = await source.readProjectEvidence(); + const { evidence } = source; const { project, items } = evidence; project.items.totalCount = 2; items.totalCount = 2; @@ -233,7 +234,7 @@ describe("RentCottage GitHub adapter", () => { it("preserves the Project item to pull-request association", async () => { const source = completeSource(); - const { items } = await source.readProjectEvidence(); + const { items } = source.evidence; items.items[0]["linked pull requests"] = [ { number: 70, @@ -252,8 +253,6 @@ describe("RentCottage GitHub adapter", () => { source, policy: policy(), }); - source.readProjectEvidence.mockClear(); - const observed = await github.observe({ type: "audit" }); expect(observed.pullRequests).toEqual([ @@ -264,7 +263,7 @@ describe("RentCottage GitHub adapter", () => { it("observes multiple items once and reads each distinct linked or explicit pull request once", async () => { const source = completeSource(); - const evidence = await source.readProjectEvidence(); + const { evidence } = source; evidence.project.items.totalCount = 2; evidence.items.totalCount = 2; evidence.items.items[0]["linked pull requests"] = [ @@ -310,7 +309,6 @@ describe("RentCottage GitHub adapter", () => { mergedAt: null, closingIssuesReferences: [], })); - source.readProjectEvidence.mockClear(); source.listIssues.mockClear(); const github = createRentCottageGitHubAdapter({ source, @@ -331,7 +329,7 @@ describe("RentCottage GitHub adapter", () => { it("rejects a Project item link to a pull request in another repository", async () => { const source = completeSource(); - const { items } = await source.readProjectEvidence(); + const { items } = source.evidence; items.items[0]["linked pull requests"] = [ { number: 70, @@ -357,7 +355,7 @@ describe("RentCottage GitHub adapter", () => { it("rejects missing linked-pull-request evidence for an Issue item", async () => { const source = completeSource(); - const { items } = await source.readProjectEvidence(); + const { items } = source.evidence; delete items.items[0]["linked pull requests"]; const github = createRentCottageGitHubAdapter({ source, @@ -377,7 +375,7 @@ describe("RentCottage GitHub adapter", () => { it("rejects a closing issue reference from another repository", async () => { const source = completeSource(); - const { items } = await source.readProjectEvidence(); + const { items } = source.evidence; items.items[0]["linked pull requests"] = [ { number: 70, @@ -417,7 +415,7 @@ describe("RentCottage GitHub adapter", () => { it("marks evidence incomplete when a required Project option is missing", async () => { const source = completeSource(); - const { fields } = await source.readProjectEvidence(); + const { fields } = source.evidence; fields.fields.find(({ name }) => name === "Status").options = [ { id: "status-backlog", name: "Backlog" }, ]; @@ -469,7 +467,7 @@ describe("RentCottage GitHub adapter", () => { }, ])("rejects $name before field normalization", async ({ change }) => { const source = completeSource(); - const { fields, project } = await source.readProjectEvidence(); + const { fields, project } = source.evidence; change(fields, project); const github = createRentCottageGitHubAdapter({ source, @@ -486,57 +484,42 @@ describe("RentCottage GitHub adapter", () => { }); }); - it("marks draft or foreign Project items as incomplete evidence", async () => { - const source = completeSource(); - const evidence = await source.readProjectEvidence(); - evidence.items = { - totalCount: 1, - items: [ + it.each([ + ["draft", { type: "DraftIssue" }], + ["pull request", { type: "PullRequest" }], + [ + "foreign Issue", + { type: "Issue", number: 55, repository: "other/repository" }, + ], + ["unavailable item", { type: "Unavailable" }], + ])( + "classifies an initial %s with the specific project-evidence error", + async (_name, content) => { + const source = completeSource(); + source.evidence.items.items = [ { - id: "draft-item", - content: { type: "DraftIssue" }, - area: "Foundation & quality", - status: "Backlog", + id: "unsupported-item", + content, }, - ], - }; - const github = createRentCottageGitHubAdapter({ - source, - policy: policy(), - }); + ]; + const github = createRentCottageGitHubAdapter({ + source, + policy: policy(), + }); - const observed = await github.observe({ type: "audit" }); + const observed = await github.observe({ type: "audit" }); - expect(observed).toMatchObject({ - complete: false, - evidenceErrors: [ + expect(observed.complete).toBe(false); + expect(observed.evidenceErrors).toEqual([ "Project contains a draft, pull request, foreign item, or unavailable item", - ], - }); - }); - - it("rejects an Issue item whose repository provenance is unavailable", async () => { - const source = completeSource(); - const { items } = await source.readProjectEvidence(); - delete items.items[0].content.repository; - const github = createRentCottageGitHubAdapter({ - source, - policy: policy(), - }); - - const observed = await github.observe({ type: "audit" }); - - expect(observed).toMatchObject({ - complete: false, - evidenceErrors: [ - "Project items response does not match the expected GitHub schema", - ], - }); - }); + ]); + expect(source.readPullRequest).not.toHaveBeenCalled(); + }, + ); it("marks the wrong Project identity as incomplete evidence", async () => { const source = completeSource(); - const evidence = await source.readProjectEvidence(); + const { evidence } = source; evidence.project = { id: "project-5", number: 5, @@ -560,7 +543,7 @@ describe("RentCottage GitHub adapter", () => { it("turns malformed provider JSON into explicit incomplete evidence", async () => { const source = completeSource(); - const evidence = await source.readProjectEvidence(); + const { evidence } = source; evidence.project = { id: "project-4", owner: null }; const github = createRentCottageGitHubAdapter({ source, @@ -600,7 +583,7 @@ describe("RentCottage GitHub adapter", () => { it("marks an unknown Project item Status as incomplete evidence", async () => { const source = completeSource(); - const { items } = await source.readProjectEvidence(); + const { items } = source.evidence; items.items[0].status = "Unexpected"; const github = createRentCottageGitHubAdapter({ source, From bf09bf9dda86253bfcfa97c4a605021299dc85fc Mon Sep 17 00:00:00 2001 From: Zain Gulel <60632602+zaingulel@users.noreply.github.com> Date: Sun, 16 Aug 2026 16:16:08 +0100 Subject: [PATCH 6/6] Validate reconciliation pagination evidence --- scripts/lib/rentcottage-gh-source.mjs | 55 ++++-- scripts/lib/rentcottage-github-adapter.mjs | 6 +- scripts/rentcottage-gh-source.test.mjs | 205 ++++++++++++++++++-- scripts/rentcottage-github-adapter.test.mjs | 15 +- 4 files changed, 233 insertions(+), 48 deletions(-) diff --git a/scripts/lib/rentcottage-gh-source.mjs b/scripts/lib/rentcottage-gh-source.mjs index 340ab9e..b3f2d86 100644 --- a/scripts/lib/rentcottage-gh-source.mjs +++ b/scripts/lib/rentcottage-gh-source.mjs @@ -480,6 +480,10 @@ export function createRentCottageGhSource({ "Fresh Project coordinates", "Fresh Project response is invalid", ); + const initialProjectTotals = Object.freeze({ + fields: project.fields?.totalCount, + items: project.items?.totalCount, + }); const fields = createConnectionState( project.fields, @@ -553,8 +557,8 @@ export function createRentCottageGhSource({ number: project.number, owner: { login: user.login }, closed: project.closed, - items: { totalCount: items.totalCount }, - fields: { totalCount: fields.totalCount }, + items: { totalCount: initialProjectTotals.items }, + fields: { totalCount: initialProjectTotals.fields }, }, fields: { totalCount: fields.totalCount, fields: fields.nodes }, items: { @@ -701,10 +705,14 @@ export function createRentCottageGhSource({ fieldValueIdentity, PROJECT_FIELD_VALUE_PAGE_SIZE, ); - const fieldValuePageCursor = new Map( + const fieldValuePageEvidence = new Map( item.fieldValues.nodes.map((value) => [ fieldValueIdentity(value), - null, + { + cursor: null, + totalCount: item.fieldValues.totalCount, + pageInfo: { ...item.fieldValues.pageInfo }, + }, ]), ); while (fieldValues.hasNextPage) { @@ -729,11 +737,13 @@ export function createRentCottageGhSource({ page.data.node.fieldValues, fieldValueIdentity, ); - for (const value of page.data.node.fieldValues.nodes) - fieldValuePageCursor.set( - fieldValueIdentity(value), - pageStartCursor, - ); + for (const value of page.data.node.fieldValues.nodes) { + fieldValuePageEvidence.set(fieldValueIdentity(value), { + cursor: pageStartCursor, + totalCount: page.data.node.fieldValues.totalCount, + pageInfo: { ...page.data.node.fieldValues.pageInfo }, + }); + } } item.fieldValues = fieldValues.nodes; @@ -757,11 +767,16 @@ export function createRentCottageGhSource({ LINKED_PULL_REQUEST_PAGE_SIZE, ); while (pullRequests.hasNextPage) { + const expectedFieldValuePage = fieldValuePageEvidence.get( + linkedCoordinate.id, + ); const query = `query($itemId: ID!, $fieldCursor: String, $pullRequestCursor: String!) { node(id: $itemId) { ... on ProjectV2Item { id content { __typename ... on Issue { id number repository { nameWithOwner } } } fieldValues(first: ${PROJECT_FIELD_VALUE_PAGE_SIZE}, after: $fieldCursor, ${FIELD_VALUE_ORDER}) { + totalCount nodes { __typename ... on ProjectV2ItemFieldPullRequestValue { field { ... on ProjectV2FieldCommon { id name } } pullRequests(first: ${LINKED_PULL_REQUEST_PAGE_SIZE}, after: $pullRequestCursor) { totalCount nodes { id number url repository { nameWithOwner } } pageInfo { hasNextPage endCursor } } } } + pageInfo { hasNextPage endCursor } } } } }`; @@ -769,13 +784,29 @@ export function createRentCottageGhSource({ query, { itemId: item.id, - fieldCursor: fieldValuePageCursor.get(linkedCoordinate.id), + fieldCursor: expectedFieldValuePage.cursor, pullRequestCursor: pullRequests.cursor, }, `${context} linked pull requests`, ); requireItemAnchor(page.data?.node, item, repository, context); - const matches = page.data.node.fieldValues?.nodes?.filter( + const pageFieldValues = requireConnection( + page.data.node.fieldValues, + `${context} linked pull-request field values`, + ); + if ( + pageFieldValues.totalCount !== + expectedFieldValuePage.totalCount || + pageFieldValues.pageInfo.hasNextPage !== + expectedFieldValuePage.pageInfo.hasNextPage || + pageFieldValues.pageInfo.endCursor !== + expectedFieldValuePage.pageInfo.endCursor + ) { + throw new Error( + `${context} linked pull-request field-value page changed during pagination`, + ); + } + const matches = pageFieldValues.nodes.filter( (value) => value?.field?.id === linkedCoordinate.id, ); if ( @@ -817,8 +848,6 @@ export function createRentCottageGhSource({ number: project.number, owner: { login: user.login }, closed: project.closed, - items: { totalCount: items.totalCount }, - fields: { totalCount: fields.totalCount }, }, fields: { totalCount: fields.totalCount, fields: fields.nodes }, items: { totalCount: items.totalCount, items: normalizedItems }, diff --git a/scripts/lib/rentcottage-github-adapter.mjs b/scripts/lib/rentcottage-github-adapter.mjs index d83f0ec..ebf4f55 100644 --- a/scripts/lib/rentcottage-github-adapter.mjs +++ b/scripts/lib/rentcottage-github-adapter.mjs @@ -20,11 +20,7 @@ function isProjectResponse(value) { Number.isInteger(value.number) && isRecord(value.owner) && typeof value.owner.login === "string" && - typeof value.closed === "boolean" && - isRecord(value.items) && - isNonnegativeInteger(value.items.totalCount) && - isRecord(value.fields) && - isNonnegativeInteger(value.fields.totalCount) + typeof value.closed === "boolean" ); } diff --git a/scripts/rentcottage-gh-source.test.mjs b/scripts/rentcottage-gh-source.test.mjs index be5c023..2704531 100644 --- a/scripts/rentcottage-gh-source.test.mjs +++ b/scripts/rentcottage-gh-source.test.mjs @@ -302,6 +302,8 @@ describe("RentCottage gh source", () => { expect(evidence.items.items.map(({ content }) => content.number)).toEqual([ 55, 63, ]); + expect(evidence.project).not.toHaveProperty("items"); + expect(evidence.project).not.toHaveProperty("fields"); expect(run).toHaveBeenCalledTimes(1); expect(run.mock.calls[0][0].slice(0, 2)).toEqual(["api", "graphql"]); const invocation = run.mock.calls[0][0].join(" "); @@ -605,13 +607,17 @@ describe("RentCottage gh source", () => { item, connection([status, linkedOverflow, notes], { totalCount: 4 }), ); - const pullRequestOverflow = fieldValuePage(item, { - nodes: [ - linkedPullRequests([secondPullRequest], { - totalCount: 2, - }), - ], - }); + const pullRequestOverflow = fieldValuePage( + item, + connection( + [ + status, + linkedPullRequests([secondPullRequest], { totalCount: 2 }), + notes, + ], + { totalCount: 4 }, + ), + ); const run = vi .fn() .mockReturnValueOnce( @@ -637,8 +643,115 @@ describe("RentCottage gh source", () => { /fieldValues\(first:\s*20(?:,\s*after:\s*\$\w+)?\s*,\s*orderBy:\s*\{\s*field:\s*POSITION,\s*direction:\s*ASC\s*\}\)/, ); } + expect(queryFrom(run.mock.calls[2][0])).toMatch( + /fieldValues[\s\S]*totalCount[\s\S]*pageInfo\s*\{\s*hasNextPage\s+endCursor\s*\}/, + ); }); + it.each([ + [ + "missing totalCount", + (fieldValues) => { + delete fieldValues.totalCount; + }, + "pagination evidence is invalid", + ], + [ + "malformed pageInfo", + (fieldValues) => { + fieldValues.pageInfo.hasNextPage = "yes"; + }, + "pagination evidence is invalid", + ], + [ + "changed totalCount", + (fieldValues) => { + fieldValues.totalCount += 1; + }, + "page changed during pagination", + ], + [ + "changed pageInfo", + (fieldValues) => { + fieldValues.pageInfo = { + hasNextPage: true, + endCursor: "changed-field-values", + }; + }, + "page changed during pagination", + ], + [ + "changed item ID", + (_fieldValues, item) => { + item.id = "item-changed"; + }, + "item or issue identity changed during pagination", + ], + [ + "changed Issue repository", + (_fieldValues, item) => { + item.content.repository.nameWithOwner = "other/repository"; + }, + "item or issue identity changed during pagination", + ], + [ + "changed linked field ID", + (fieldValues) => { + fieldValues.nodes[2].field.id = "field-changed"; + }, + "linked pull-request field identity changed", + ], + [ + "changed linked field name", + (fieldValues) => { + fieldValues.nodes[2].field.name = "Changed linked field"; + }, + "linked pull-request field identity changed", + ], + [ + "changed linked field type", + (fieldValues) => { + fieldValues.nodes[2].__typename = "ProjectV2ItemFieldTextValue"; + }, + "linked pull-request field identity changed", + ], + ])( + "rejects linked-PR outer reread with %s", + async (_name, mutate, message) => { + const item = projectItem(); + const pullRequest = (number) => ({ + id: `pr-${number}`, + number, + url: `https://github.com/zaingulel/RentCottage/pull/${number}`, + repository: { nameWithOwner: repository }, + }); + item.fieldValues.nodes[2] = linkedPullRequests([pullRequest(70)], { + totalCount: 2, + hasNextPage: true, + endCursor: "pull-requests-1", + }); + const outerFieldValues = structuredClone(item.fieldValues); + outerFieldValues.nodes[2] = linkedPullRequests([pullRequest(71)], { + totalCount: 2, + }); + const outerReread = fieldValuePage(item, outerFieldValues); + outerReread.data.node.content = structuredClone(item.content); + mutate(outerFieldValues, outerReread.data.node); + const run = vi + .fn() + .mockReturnValueOnce( + JSON.stringify( + projectResponse({ itemConnection: connection([item]) }), + ), + ) + .mockReturnValueOnce(JSON.stringify(outerReread)); + + await expect(sourceWith(run).readProjectEvidence()).rejects.toThrow( + message, + ); + }, + ); + it("normalizes linked pull requests after more than ten provider-sized pages", async () => { const item = projectItem(); const pullRequests = Array.from({ length: 201 }, (_, index) => ({ @@ -655,19 +768,17 @@ describe("RentCottage gh source", () => { }); const overflowPages = Array.from({ length: 10 }, (_, index) => { const page = connectionPage(pullRequests, index + 1, 20, "pull-requests"); + const fieldValueNodes = structuredClone(item.fieldValues.nodes); + fieldValueNodes[2] = linkedPullRequests(page.nodes, { + totalCount: page.totalCount, + hasNextPage: page.pageInfo.hasNextPage, + endCursor: page.pageInfo.endCursor, + }); return { data: { node: { ...item, - fieldValues: { - nodes: [ - linkedPullRequests(page.nodes, { - totalCount: page.totalCount, - hasNextPage: page.pageInfo.hasNextPage, - endCursor: page.pageInfo.endCursor, - }), - ], - }, + fieldValues: connection(fieldValueNodes, { totalCount: 4 }), }, }, }; @@ -843,13 +954,17 @@ describe("RentCottage gh source", () => { }, }, }; + const pullRequestFieldValues = structuredClone( + pullRequestItem.fieldValues.nodes, + ); + pullRequestFieldValues[2] = linkedPullRequests([pullRequestTwo], { + totalCount: 2, + }); const pullRequestPage = { data: { node: { ...pullRequestItem, - fieldValues: { - nodes: [linkedPullRequests([pullRequestTwo], { totalCount: 2 })], - }, + fieldValues: connection(pullRequestFieldValues, { totalCount: 4 }), }, }, }; @@ -1363,4 +1478,56 @@ describe("RentCottage gh source", () => { optionId: "status-progress", }); }); + + it.each([ + { + connectionName: "fields", + first: projectResponse({ + fieldConnection: connection([fields()[0]], { + totalCount: 4, + hasNextPage: true, + endCursor: "fresh-fields-1", + }), + itemConnection: connection([freshCoordinateItem()]), + }), + overflow: projectResponse({ + fieldConnection: connection(fields().slice(1), { totalCount: 5 }), + }), + }, + { + connectionName: "items", + first: projectResponse({ + itemConnection: connection([freshCoordinateItem(55)], { + totalCount: 2, + hasNextPage: true, + endCursor: "fresh-items-1", + }), + }), + overflow: projectResponse({ + itemConnection: connection([freshCoordinateItem(63)], { + totalCount: 3, + }), + }), + }, + ])( + "fails before a field mutation when fresh Project $connectionName totals drift", + async ({ connectionName, first, overflow }) => { + const run = vi + .fn() + .mockReturnValueOnce(JSON.stringify(first)) + .mockReturnValueOnce(JSON.stringify(overflow)); + + await expect( + sourceWith(run).execute({ + type: "set-project-field", + issueNumber: 55, + field: "Status", + value: "In progress", + }), + ).rejects.toThrow( + `Fresh Project ${connectionName} totalCount changed during pagination`, + ); + expect(run).toHaveBeenCalledTimes(2); + }, + ); }); diff --git a/scripts/rentcottage-github-adapter.test.mjs b/scripts/rentcottage-github-adapter.test.mjs index 7118988..390cc84 100644 --- a/scripts/rentcottage-github-adapter.test.mjs +++ b/scripts/rentcottage-github-adapter.test.mjs @@ -29,8 +29,6 @@ function completeSource() { number: 4, owner: { login: "zaingulel" }, closed: false, - items: { totalCount: 1 }, - fields: { totalCount: 3 }, }, fields: { totalCount: 3, @@ -141,8 +139,7 @@ describe("RentCottage GitHub adapter", () => { it("reads native blockers for new issues discovered from Project 4", async () => { const source = completeSource(); const { evidence } = source; - const { project, items } = evidence; - project.items.totalCount = 2; + const { items } = evidence; items.totalCount = 2; items.items.push({ id: "item-63", @@ -264,7 +261,6 @@ describe("RentCottage GitHub adapter", () => { it("observes multiple items once and reads each distinct linked or explicit pull request once", async () => { const source = completeSource(); const { evidence } = source; - evidence.project.items.totalCount = 2; evidence.items.totalCount = 2; evidence.items.items[0]["linked pull requests"] = [ { @@ -435,13 +431,12 @@ describe("RentCottage GitHub adapter", () => { it.each([ { name: "duplicate Area field", - change(fields, project) { + change(fields) { fields.fields.push({ ...structuredClone(fields.fields[0]), id: "field-area-duplicate", }); fields.totalCount = 4; - project.fields.totalCount = 4; }, }, { @@ -467,8 +462,8 @@ describe("RentCottage GitHub adapter", () => { }, ])("rejects $name before field normalization", async ({ change }) => { const source = completeSource(); - const { fields, project } = source.evidence; - change(fields, project); + const { fields } = source.evidence; + change(fields); const github = createRentCottageGitHubAdapter({ source, policy: policy(), @@ -525,8 +520,6 @@ describe("RentCottage GitHub adapter", () => { number: 5, owner: { login: "someone-else" }, closed: false, - items: { totalCount: 1 }, - fields: { totalCount: 3 }, }; const github = createRentCottageGitHubAdapter({ source,