diff --git a/build/cli/index.js b/build/cli/index.js index 13ca39972..1b0232a98 100755 --- a/build/cli/index.js +++ b/build/cli/index.js @@ -71916,13 +71916,13 @@ async function runLinkPullRequestIssue(param, taskId, port, delay) { let retainedIssueReference = false; let alreadyLinked = false; if (!hasOwnedPendingOperation) { - alreadyLinked = await port.isLinked(param.pullRequestNumber); + alreadyLinked = await port.isLinked(param.pullRequestNumber, param.issueNumber); if (alreadyLinked) return []; } else { try { - alreadyLinked = await port.isLinked(param.pullRequestNumber); + alreadyLinked = await port.isLinked(param.pullRequestNumber, param.issueNumber); } catch (error) { primaryError = error; @@ -86701,6 +86701,7 @@ Object.defineProperty(exports, "__esModule", ({ value: true })); exports.PullRequestLifecycleRepository = void 0; const logger_1 = __nccwpck_require__(91151); const application_error_1 = __nccwpck_require__(75999); +const github_error_policy_1 = __nccwpck_require__(58791); class PullRequestLifecycleRepository { constructor(githubClient) { this.githubClient = githubClient; @@ -86762,26 +86763,69 @@ class PullRequestLifecycleRepository { throw semanticError; } }; - this.isLinked = async (owner, repository, pullRequestNumber, _token) => { - const pullRequestUrl = `https://github.com/${encodeURIComponent(owner)}/${encodeURIComponent(repository)}/pull/${pullRequestNumber}`; - const controller = new AbortController(); - const timeoutId = setTimeout(() => controller.abort(), PullRequestLifecycleRepository.IS_LINKED_FETCH_TIMEOUT_MS); + /** Reads at most 200 closing references through the credential-bound API. */ + this.isLinked = async (owner, repository, pullRequestNumber, issueNumber, token) => { try { - const res = await fetch(pullRequestUrl, { signal: controller.signal }); - if (!res.ok) { - throw new Error(`Pull-request linkage query returned HTTP ${res.status}.`); + if (![pullRequestNumber, issueNumber].every(number => Number.isSafeInteger(number) && number > 0)) { + throw new application_error_1.ApplicationError('validation.invalid-input', 'Linkage requires positive PR and issue numbers.'); } - const htmlContent = await res.text(); - return !htmlContent.includes('has_github_issues=false'); + const client = this.githubClient.getClient(token); + if (!client.graphql) + throw new application_error_1.ApplicationError('configuration.unsupported', 'Authenticated PR linkage queries are not supported by this installation.'); + const expectedRepository = `${owner}/${repository}`.toLowerCase(); + let cursor = null; + for (let page = 0; page < 2; page += 1) { + const result = await client.graphql(` + query PullRequestIssueLinkage($owner: String!, $repo: String!, $number: Int!, $cursor: String) { + repository(owner: $owner, name: $repo) { + nameWithOwner + pullRequest(number: $number) { + number + closingIssuesReferences(first: 100, after: $cursor) { + nodes { number repository { nameWithOwner } } + pageInfo { hasNextPage endCursor } + } + } + } + }`, { owner, repo: repository, number: pullRequestNumber, cursor }); + const resource = result?.repository; + if (resource === null || resource?.pullRequest === null) { + throw new application_error_1.ApplicationError('provider.not-found', 'The requested pull request is unavailable to the configured credential.'); + } + const pr = resource?.pullRequest; + const references = pr?.closingIssuesReferences; + if (typeof resource?.nameWithOwner !== 'string' || resource.nameWithOwner.toLowerCase() !== expectedRepository || pr?.number !== pullRequestNumber + || !Array.isArray(references?.nodes) || references.nodes.length > 100 + || typeof references.pageInfo?.hasNextPage !== 'boolean' + || references.nodes.some(node => !node || !Number.isSafeInteger(node.number) || node.number <= 0 + || typeof node.repository?.nameWithOwner !== 'string' || !node.repository.nameWithOwner)) { + throw new application_error_1.ApplicationError('provider.contract-invalid', 'GitHub did not return verifiable PR linkage evidence.'); + } + if (references.nodes.some(node => node.number === issueNumber + && node.repository.nameWithOwner.toLowerCase() === expectedRepository)) + return true; + if (!references.pageInfo.hasNextPage) + return false; + const nextCursor = references.pageInfo.endCursor; + if (typeof nextCursor !== 'string' || !nextCursor || nextCursor === cursor) { + throw new application_error_1.ApplicationError('provider.contract-invalid', 'GitHub returned incomplete linkage pagination.'); + } + cursor = nextCursor; + } + throw new application_error_1.ApplicationError('provider.contract-invalid', 'PR linkage exceeds the fixed 200-reference inspection bound.'); } catch (error) { - const semanticError = (0, application_error_1.toApplicationError)(error, 'provider.unavailable', `Unable to inspect issue linkage for pull request #${pullRequestNumber}.`); + const status = (0, github_error_policy_1.getGithubErrorStatus)(error); + const types = typeof error === 'object' && error !== null && 'errors' in error && Array.isArray(error.errors) + ? error.errors.map((entry) => entry?.type) : []; + const code = (0, github_error_policy_1.isGithubRateLimited)(error) || types.includes('RATE_LIMITED') ? 'provider.rate-limited' + : status === 401 ? 'authorization.credential-invalid' + : status === 403 || types.includes('FORBIDDEN') || types.includes('UNAUTHORIZED') ? 'authorization.denied' + : status === 404 || types.includes('NOT_FOUND') ? 'provider.not-found' : 'provider.unavailable'; + const semanticError = (0, application_error_1.toApplicationError)(error, code, `Unable to inspect issue linkage for pull request #${pullRequestNumber}.`); (0, logger_1.logError)(semanticError); throw semanticError; } - finally { - clearTimeout(timeoutId); - } }; this.updateBaseBranch = async (owner, repository, pullRequestNumber, branch, token) => { const octokit = this.githubClient.getClient(token); @@ -86850,8 +86894,6 @@ class PullRequestLifecycleRepository { } } exports.PullRequestLifecycleRepository = PullRequestLifecycleRepository; -/** Default timeout (ms) for isLinked fetch. */ -PullRequestLifecycleRepository.IS_LINKED_FETCH_TIMEOUT_MS = 10000; function toBugbotPullRequestIdentity(value) { const number = value.number; const state = value.state; @@ -94970,7 +95012,7 @@ function bindBranchWorkflow(port, binding) { } function bindPullRequestIssueLink(port, binding) { return Object.freeze({ - isLinked: (pullRequestNumber) => port.isLinked(binding.owner, binding.repository, pullRequestNumber, binding.token), + isLinked: (pullRequestNumber, issueNumber) => port.isLinked(binding.owner, binding.repository, pullRequestNumber, issueNumber, binding.token), getDetails: (pullRequestNumber) => port.getDetails(binding.owner, binding.repository, pullRequestNumber, binding.token), updateBaseBranch: (pullRequestNumber, baseBranch) => port.updateBaseBranch(binding.owner, binding.repository, pullRequestNumber, baseBranch, binding.token), updateDescription: (pullRequestNumber, description) => port.updateDescription(binding.owner, binding.repository, pullRequestNumber, description, binding.token), diff --git a/build/github_action/index.js b/build/github_action/index.js index 0f9bd128a..c82b2f11d 100644 --- a/build/github_action/index.js +++ b/build/github_action/index.js @@ -67599,13 +67599,13 @@ async function runLinkPullRequestIssue(param, taskId, port, delay) { let retainedIssueReference = false; let alreadyLinked = false; if (!hasOwnedPendingOperation) { - alreadyLinked = await port.isLinked(param.pullRequestNumber); + alreadyLinked = await port.isLinked(param.pullRequestNumber, param.issueNumber); if (alreadyLinked) return []; } else { try { - alreadyLinked = await port.isLinked(param.pullRequestNumber); + alreadyLinked = await port.isLinked(param.pullRequestNumber, param.issueNumber); } catch (error) { primaryError = error; @@ -77193,6 +77193,7 @@ Object.defineProperty(exports, "__esModule", ({ value: true })); exports.PullRequestLifecycleRepository = void 0; const logger_1 = __nccwpck_require__(91151); const application_error_1 = __nccwpck_require__(75999); +const github_error_policy_1 = __nccwpck_require__(58791); class PullRequestLifecycleRepository { constructor(githubClient) { this.githubClient = githubClient; @@ -77254,26 +77255,69 @@ class PullRequestLifecycleRepository { throw semanticError; } }; - this.isLinked = async (owner, repository, pullRequestNumber, _token) => { - const pullRequestUrl = `https://github.com/${encodeURIComponent(owner)}/${encodeURIComponent(repository)}/pull/${pullRequestNumber}`; - const controller = new AbortController(); - const timeoutId = setTimeout(() => controller.abort(), PullRequestLifecycleRepository.IS_LINKED_FETCH_TIMEOUT_MS); + /** Reads at most 200 closing references through the credential-bound API. */ + this.isLinked = async (owner, repository, pullRequestNumber, issueNumber, token) => { try { - const res = await fetch(pullRequestUrl, { signal: controller.signal }); - if (!res.ok) { - throw new Error(`Pull-request linkage query returned HTTP ${res.status}.`); + if (![pullRequestNumber, issueNumber].every(number => Number.isSafeInteger(number) && number > 0)) { + throw new application_error_1.ApplicationError('validation.invalid-input', 'Linkage requires positive PR and issue numbers.'); } - const htmlContent = await res.text(); - return !htmlContent.includes('has_github_issues=false'); + const client = this.githubClient.getClient(token); + if (!client.graphql) + throw new application_error_1.ApplicationError('configuration.unsupported', 'Authenticated PR linkage queries are not supported by this installation.'); + const expectedRepository = `${owner}/${repository}`.toLowerCase(); + let cursor = null; + for (let page = 0; page < 2; page += 1) { + const result = await client.graphql(` + query PullRequestIssueLinkage($owner: String!, $repo: String!, $number: Int!, $cursor: String) { + repository(owner: $owner, name: $repo) { + nameWithOwner + pullRequest(number: $number) { + number + closingIssuesReferences(first: 100, after: $cursor) { + nodes { number repository { nameWithOwner } } + pageInfo { hasNextPage endCursor } + } + } + } + }`, { owner, repo: repository, number: pullRequestNumber, cursor }); + const resource = result?.repository; + if (resource === null || resource?.pullRequest === null) { + throw new application_error_1.ApplicationError('provider.not-found', 'The requested pull request is unavailable to the configured credential.'); + } + const pr = resource?.pullRequest; + const references = pr?.closingIssuesReferences; + if (typeof resource?.nameWithOwner !== 'string' || resource.nameWithOwner.toLowerCase() !== expectedRepository || pr?.number !== pullRequestNumber + || !Array.isArray(references?.nodes) || references.nodes.length > 100 + || typeof references.pageInfo?.hasNextPage !== 'boolean' + || references.nodes.some(node => !node || !Number.isSafeInteger(node.number) || node.number <= 0 + || typeof node.repository?.nameWithOwner !== 'string' || !node.repository.nameWithOwner)) { + throw new application_error_1.ApplicationError('provider.contract-invalid', 'GitHub did not return verifiable PR linkage evidence.'); + } + if (references.nodes.some(node => node.number === issueNumber + && node.repository.nameWithOwner.toLowerCase() === expectedRepository)) + return true; + if (!references.pageInfo.hasNextPage) + return false; + const nextCursor = references.pageInfo.endCursor; + if (typeof nextCursor !== 'string' || !nextCursor || nextCursor === cursor) { + throw new application_error_1.ApplicationError('provider.contract-invalid', 'GitHub returned incomplete linkage pagination.'); + } + cursor = nextCursor; + } + throw new application_error_1.ApplicationError('provider.contract-invalid', 'PR linkage exceeds the fixed 200-reference inspection bound.'); } catch (error) { - const semanticError = (0, application_error_1.toApplicationError)(error, 'provider.unavailable', `Unable to inspect issue linkage for pull request #${pullRequestNumber}.`); + const status = (0, github_error_policy_1.getGithubErrorStatus)(error); + const types = typeof error === 'object' && error !== null && 'errors' in error && Array.isArray(error.errors) + ? error.errors.map((entry) => entry?.type) : []; + const code = (0, github_error_policy_1.isGithubRateLimited)(error) || types.includes('RATE_LIMITED') ? 'provider.rate-limited' + : status === 401 ? 'authorization.credential-invalid' + : status === 403 || types.includes('FORBIDDEN') || types.includes('UNAUTHORIZED') ? 'authorization.denied' + : status === 404 || types.includes('NOT_FOUND') ? 'provider.not-found' : 'provider.unavailable'; + const semanticError = (0, application_error_1.toApplicationError)(error, code, `Unable to inspect issue linkage for pull request #${pullRequestNumber}.`); (0, logger_1.logError)(semanticError); throw semanticError; } - finally { - clearTimeout(timeoutId); - } }; this.updateBaseBranch = async (owner, repository, pullRequestNumber, branch, token) => { const octokit = this.githubClient.getClient(token); @@ -77342,8 +77386,6 @@ class PullRequestLifecycleRepository { } } exports.PullRequestLifecycleRepository = PullRequestLifecycleRepository; -/** Default timeout (ms) for isLinked fetch. */ -PullRequestLifecycleRepository.IS_LINKED_FETCH_TIMEOUT_MS = 10000; function toBugbotPullRequestIdentity(value) { const number = value.number; const state = value.state; @@ -85470,7 +85512,7 @@ function bindBranchWorkflow(port, binding) { } function bindPullRequestIssueLink(port, binding) { return Object.freeze({ - isLinked: (pullRequestNumber) => port.isLinked(binding.owner, binding.repository, pullRequestNumber, binding.token), + isLinked: (pullRequestNumber, issueNumber) => port.isLinked(binding.owner, binding.repository, pullRequestNumber, issueNumber, binding.token), getDetails: (pullRequestNumber) => port.getDetails(binding.owner, binding.repository, pullRequestNumber, binding.token), updateBaseBranch: (pullRequestNumber, baseBranch) => port.updateBaseBranch(binding.owner, binding.repository, pullRequestNumber, baseBranch, binding.token), updateDescription: (pullRequestNumber, description) => port.updateDescription(binding.owner, binding.repository, pullRequestNumber, description, binding.token), diff --git a/docs/pull-requests/configuration.mdx b/docs/pull-requests/configuration.mdx index 5fbcb7c65..95efc655d 100644 --- a/docs/pull-requests/configuration.mdx +++ b/docs/pull-requests/configuration.mdx @@ -48,3 +48,21 @@ issue numbers must be positive safe integers, branch refs must be safe, and the provider target is derived from the configured repository rather than an event URL. The temporary marker format, default-branch observation delay, compensation order, and exact-body restoration cannot be weakened by workflow inputs. + +Linkage inspection uses the configured token with GitHub GraphQL +`closingIssuesReferences`; it never requests a public HTML page. Private and +public PRs follow the same path. The exact repository, PR and intended issue +must match; a link to another issue or repository does not skip linkage. Reads +are fixed at two pages of 100 references. Incomplete or malformed evidence stops +the operation rather than claiming the PR is unlinked. + +If inspection fails before temporary edits, the current base and description +remain unchanged. An interrupted owned operation still attempts compensation. +`authorization.credential-invalid` and `authorization.denied` require checking +the documented credential access (including GraphQL `UNAUTHORIZED` and +`FORBIDDEN` responses); `provider.not-found` requires checking the PR +identity and credential visibility. `configuration.unsupported` requires a +supported installation; `provider.contract-invalid` requires integration review +(including the fixed reference limit). Rate limits and transport outages retain +their retryable diagnostics. Inspect the run's retained-state guidance before +retrying an interrupted operation. No additional permission is introduced. diff --git a/specs/CATALOG.md b/specs/CATALOG.md index 9f98fe94b..a3026d298 100644 --- a/specs/CATALOG.md +++ b/specs/CATALOG.md @@ -25,7 +25,7 @@ debt or convert unknown historic intent into a design decision. | `comment-automation` | Implemented | Admit only explicit commands or exact mentions, then route them while protecting repository mutations | [Comment automation and authorization](./comment-automation-and-authorization.md) | 79 paths · 2026-10-11 | | `bugbot-analysis-and-autofix` | Implemented | Select one canonical PR, exhaustively analyze its bounded diff partitions, publish stable findings atomically, and apply authorized verified fixes | [Bugbot analysis, finding publication, and autofix](./bugbot-analysis-publication-and-autofix.md) + 2 companion | 109 paths · 2026-10-13 | | `branch-synchronization` | Implemented | Observe parent drift with one localized status card and transition-only notifications, then safely merge a parent branch into a linked working branch | [Branch synchronization and conflict recovery](./branch-synchronization-and-conflict-recovery.md) | 68 paths · 2026-10-18 | -| `pull-request-lifecycle` | Implemented | Enrich linked and unlinked pull requests with safe issue linkage, projects, metadata, reviewers, concise descriptions, and distinct workflow evidence | [Pull request lifecycle and enrichment](./pull-request-lifecycle-and-enrichment.md) | 75 paths · 2026-10-13 | +| `pull-request-lifecycle` | Implemented | Enrich linked and unlinked pull requests with safe issue linkage, projects, metadata, reviewers, concise descriptions, and distinct workflow evidence | [Pull request lifecycle and enrichment](./pull-request-lifecycle-and-enrichment.md) | 77 paths · 2026-10-20 | | `agent-runtime` | Implemented | Resolve, provision, authenticate, authorize, and execute only the agent roles reachable by a run | [Agent runtime, provider, model, and role routing](./agent-runtime-provider-and-model-routing.md) + 1 companion | 99 paths · 2026-10-17 | | `cli-and-single-actions` | As-built baseline | Expose bounded local commands and workflow-dispatched operations through the shared application core | [CLI and single-action execution](./cli-and-single-action-execution.md) | 44 paths · 2026-10-19 | | `configurable-issue-workflows` | Implemented | Select one canonical set of issue workflows and enforce its forms, dependencies, branch policy, runtime admission, migration, and diagnosis | [Configurable issue workflows and fail-closed admission](./configurable-issue-workflows-and-admission.md) | 89 paths · 2026-10-13 | @@ -202,11 +202,11 @@ debt or convert unknown historic intent into a design decision. ### `pull-request-lifecycle` — Pull request lifecycle and enrichment - Owner: Copilot maintainers -- Last verified: 2026-10-13 +- Last verified: 2026-10-20 - Specifications: [`specs/pull-request-lifecycle-and-enrichment.md`](./pull-request-lifecycle-and-enrichment.md) - Workflows: [`.github/workflows/copilot_pull_request.yml`](../.github/workflows/copilot_pull_request.yml) · [`.github/workflows/copilot_pull_request_merge_queue.yml`](../.github/workflows/copilot_pull_request_merge_queue.yml) · [`setup/workflows/copilot_pull_request.yml`](../setup/workflows/copilot_pull_request.yml) · [`setup/workflows/copilot_pull_request_merge_queue.yml`](../setup/workflows/copilot_pull_request_merge_queue.yml) · [`.github/workflows/copilot_issue_comment.yml`](../.github/workflows/copilot_issue_comment.yml) · [`.github/workflows/copilot_pull_request_comment.yml`](../.github/workflows/copilot_pull_request_comment.yml) · [`setup/workflows/copilot_issue_comment.yml`](../setup/workflows/copilot_issue_comment.yml) · [`setup/workflows/copilot_pull_request_comment.yml`](../setup/workflows/copilot_pull_request_comment.yml) - Entrypoints: [`src/application/usecases/pull_request_use_case.ts`](../src/application/usecases/pull_request_use_case.ts) · [`src/actions/common_action.ts`](../src/actions/common_action.ts) -- Core code: [`src/application/usecases/execution/execution_issue_number_policy.ts`](../src/application/usecases/execution/execution_issue_number_policy.ts) · [`src/application/usecases/execution/setup_execution_workflow.ts`](../src/application/usecases/execution/setup_execution_workflow.ts) · [`src/application/usecases/pull_request_workflow_context.ts`](../src/application/usecases/pull_request_workflow_context.ts) · [`src/application/usecases/pull_request_workflow.ts`](../src/application/usecases/pull_request_workflow.ts) · [`src/application/usecases/pull_request_workflow_steps.ts`](../src/application/usecases/pull_request_workflow_steps.ts) · [`src/application/ports/project_board_link_ports.ts`](../src/application/ports/project_board_link_ports.ts) · [`src/application/usecases/steps/common/project_content_link_workflow.ts`](../src/application/usecases/steps/common/project_content_link_workflow.ts) · [`src/application/usecases/steps/pull_request/link_pull_request_project_use_case.ts`](../src/application/usecases/steps/pull_request/link_pull_request_project_use_case.ts) · [`src/application/usecases/steps/pull_request/link_pull_request_issue_workflow.ts`](../src/application/usecases/steps/pull_request/link_pull_request_issue_workflow.ts) · [`src/application/usecases/steps/pull_request/update_pull_request_description_use_case.ts`](../src/application/usecases/steps/pull_request/update_pull_request_description_use_case.ts) · [`src/prompts/update_pull_request_description.ts`](../src/prompts/update_pull_request_description.ts) · [`src/application/policies/agent_response_schemas.ts`](../src/application/policies/agent_response_schemas.ts) · [`src/application/policies/pull_request_description_content_policy.ts`](../src/application/policies/pull_request_description_content_policy.ts) · [`src/data/repository/project/project_board_link_repository.ts`](../src/data/repository/project/project_board_link_repository.ts) · [`src/data/repository/project/project_board_command_repository.ts`](../src/data/repository/project/project_board_command_repository.ts) · [`src/infrastructure/composition/lifecycle_capability_port_binding.ts`](../src/infrastructure/composition/lifecycle_capability_port_binding.ts) · [`src/infrastructure/composition/shared_capability_port_binding.ts`](../src/infrastructure/composition/shared_capability_port_binding.ts) · [`src/domain/pull_request_description.ts`](../src/domain/pull_request_description.ts) · [`src/data/repository/pull_request/pull_request_lifecycle_repository.ts`](../src/data/repository/pull_request/pull_request_lifecycle_repository.ts) · [`src/application/usecases/steps/pull_request/update_pull_request_description_workflow.ts`](../src/application/usecases/steps/pull_request/update_pull_request_description_workflow.ts) · [`src/application/policies/agent_task_activation_policy.ts`](../src/application/policies/agent_task_activation_policy.ts) · [`src/application/policies/agent_activity_policy.ts`](../src/application/policies/agent_activity_policy.ts) · [`src/application/policies/lifecycle_state_policy.ts`](../src/application/policies/lifecycle_state_policy.ts) · [`src/application/usecases/push_single_action_contexts.ts`](../src/application/usecases/push_single_action_contexts.ts) · [`src/application/usecases/actions/synchronize_agent_activity_use_case.ts`](../src/application/usecases/actions/synchronize_agent_activity_use_case.ts) · [`src/application/policies/lifecycle_waiting_state_policy.ts`](../src/application/policies/lifecycle_waiting_state_policy.ts) · [`src/application/policies/copilot_evidence_policy.ts`](../src/application/policies/copilot_evidence_policy.ts) · [`src/actions/github_action_completion.ts`](../src/actions/github_action_completion.ts) · [`src/data/repository/project/project_board_item_query.ts`](../src/data/repository/project/project_board_item_query.ts) · [`src/data/repository/project/project_board_field_update.ts`](../src/data/repository/project/project_board_field_update.ts) +- Core code: [`src/application/usecases/execution/execution_issue_number_policy.ts`](../src/application/usecases/execution/execution_issue_number_policy.ts) · [`src/application/usecases/execution/setup_execution_workflow.ts`](../src/application/usecases/execution/setup_execution_workflow.ts) · [`src/application/usecases/pull_request_workflow_context.ts`](../src/application/usecases/pull_request_workflow_context.ts) · [`src/application/usecases/pull_request_workflow.ts`](../src/application/usecases/pull_request_workflow.ts) · [`src/application/usecases/pull_request_workflow_steps.ts`](../src/application/usecases/pull_request_workflow_steps.ts) · [`src/application/ports/project_board_link_ports.ts`](../src/application/ports/project_board_link_ports.ts) · [`src/application/usecases/steps/common/project_content_link_workflow.ts`](../src/application/usecases/steps/common/project_content_link_workflow.ts) · [`src/application/usecases/steps/pull_request/link_pull_request_project_use_case.ts`](../src/application/usecases/steps/pull_request/link_pull_request_project_use_case.ts) · [`src/application/usecases/steps/pull_request/link_pull_request_issue_workflow.ts`](../src/application/usecases/steps/pull_request/link_pull_request_issue_workflow.ts) · [`src/application/usecases/steps/pull_request/update_pull_request_description_use_case.ts`](../src/application/usecases/steps/pull_request/update_pull_request_description_use_case.ts) · [`src/prompts/update_pull_request_description.ts`](../src/prompts/update_pull_request_description.ts) · [`src/application/policies/agent_response_schemas.ts`](../src/application/policies/agent_response_schemas.ts) · [`src/application/policies/pull_request_description_content_policy.ts`](../src/application/policies/pull_request_description_content_policy.ts) · [`src/data/repository/project/project_board_link_repository.ts`](../src/data/repository/project/project_board_link_repository.ts) · [`src/data/repository/project/project_board_command_repository.ts`](../src/data/repository/project/project_board_command_repository.ts) · [`src/infrastructure/composition/lifecycle_capability_port_binding.ts`](../src/infrastructure/composition/lifecycle_capability_port_binding.ts) · [`src/infrastructure/composition/shared_capability_port_binding.ts`](../src/infrastructure/composition/shared_capability_port_binding.ts) · [`src/domain/pull_request_description.ts`](../src/domain/pull_request_description.ts) · [`src/data/repository/pull_request/pull_request_lifecycle_repository.ts`](../src/data/repository/pull_request/pull_request_lifecycle_repository.ts) · [`src/application/usecases/steps/pull_request/update_pull_request_description_workflow.ts`](../src/application/usecases/steps/pull_request/update_pull_request_description_workflow.ts) · [`src/application/policies/agent_task_activation_policy.ts`](../src/application/policies/agent_task_activation_policy.ts) · [`src/application/policies/agent_activity_policy.ts`](../src/application/policies/agent_activity_policy.ts) · [`src/application/policies/lifecycle_state_policy.ts`](../src/application/policies/lifecycle_state_policy.ts) · [`src/application/usecases/push_single_action_contexts.ts`](../src/application/usecases/push_single_action_contexts.ts) · [`src/application/usecases/actions/synchronize_agent_activity_use_case.ts`](../src/application/usecases/actions/synchronize_agent_activity_use_case.ts) · [`src/application/policies/lifecycle_waiting_state_policy.ts`](../src/application/policies/lifecycle_waiting_state_policy.ts) · [`src/application/policies/copilot_evidence_policy.ts`](../src/application/policies/copilot_evidence_policy.ts) · [`src/actions/github_action_completion.ts`](../src/actions/github_action_completion.ts) · [`src/data/repository/project/project_board_item_query.ts`](../src/data/repository/project/project_board_item_query.ts) · [`src/data/repository/project/project_board_field_update.ts`](../src/data/repository/project/project_board_field_update.ts) · [`src/application/ports/pull_request_issue_link_ports.ts`](../src/application/ports/pull_request_issue_link_ports.ts) · [`src/infrastructure/github/ports/github_pull_request_provider_ports.ts`](../src/infrastructure/github/ports/github_pull_request_provider_ports.ts) - Tests: [`src/application/usecases/execution/__tests__/execution_issue_number_policy.test.ts`](../src/application/usecases/execution/__tests__/execution_issue_number_policy.test.ts) · [`src/data/model/__tests__/execution.test.ts`](../src/data/model/__tests__/execution.test.ts) · [`src/actions/__tests__/common_action.test.ts`](../src/actions/__tests__/common_action.test.ts) · [`src/application/usecases/__tests__/issue_pull_request_context_projection.test.ts`](../src/application/usecases/__tests__/issue_pull_request_context_projection.test.ts) · [`src/application/usecases/__tests__/pull_request_use_case.test.ts`](../src/application/usecases/__tests__/pull_request_use_case.test.ts) · [`src/application/usecases/steps/pull_request/__tests__/link_pull_request_project_use_case.test.ts`](../src/application/usecases/steps/pull_request/__tests__/link_pull_request_project_use_case.test.ts) · [`src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts`](../src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts) · [`src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts`](../src/application/usecases/steps/pull_request/__tests__/update_pull_request_description_use_case.test.ts) · [`src/application/policies/__tests__/pull_request_description_content_policy.test.ts`](../src/application/policies/__tests__/pull_request_description_content_policy.test.ts) · [`src/application/policies/__tests__/agent_response_schemas.test.ts`](../src/application/policies/__tests__/agent_response_schemas.test.ts) · [`src/prompts/__tests__/update_pull_request_description.test.ts`](../src/prompts/__tests__/update_pull_request_description.test.ts) · [`src/data/repository/__tests__/project_board_link_repository.test.ts`](../src/data/repository/__tests__/project_board_link_repository.test.ts) · [`src/data/repository/project/__tests__/project_board_command_repository.test.ts`](../src/data/repository/project/__tests__/project_board_command_repository.test.ts) · [`src/infrastructure/composition/__tests__/shared_capability_port_binding.test.ts`](../src/infrastructure/composition/__tests__/shared_capability_port_binding.test.ts) · [`src/tooling/__tests__/validate_workflow_contract.test.ts`](../src/tooling/__tests__/validate_workflow_contract.test.ts) · [`src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts`](../src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts) · [`src/data/repository/__tests__/pull_request_lifecycle_repository.test.ts`](../src/data/repository/__tests__/pull_request_lifecycle_repository.test.ts) · [`src/application/policies/__tests__/agent_task_activation_policy.test.ts`](../src/application/policies/__tests__/agent_task_activation_policy.test.ts) · [`src/application/policies/__tests__/agent_activity_policy.test.ts`](../src/application/policies/__tests__/agent_activity_policy.test.ts) · [`src/application/policies/__tests__/lifecycle_state_policy.test.ts`](../src/application/policies/__tests__/lifecycle_state_policy.test.ts) · [`src/application/usecases/actions/__tests__/synchronize_agent_activity_use_case.test.ts`](../src/application/usecases/actions/__tests__/synchronize_agent_activity_use_case.test.ts) · [`src/application/usecases/actions/__tests__/synchronize_lifecycle_state_use_case.test.ts`](../src/application/usecases/actions/__tests__/synchronize_lifecycle_state_use_case.test.ts) · [`src/application/policies/__tests__/copilot_evidence_policy.test.ts`](../src/application/policies/__tests__/copilot_evidence_policy.test.ts) · [`src/actions/__tests__/github_action_completion.test.ts`](../src/actions/__tests__/github_action_completion.test.ts) · [`src/data/repository/project/__tests__/project_board_visibility_recovery.test.ts`](../src/data/repository/project/__tests__/project_board_visibility_recovery.test.ts) - User documentation: [`docs/pull-requests/capabilities.mdx`](../docs/pull-requests/capabilities.mdx) · [`docs/pull-requests/ai-description.mdx`](../docs/pull-requests/ai-description.mdx) · [`docs/pull-requests/configuration.mdx`](../docs/pull-requests/configuration.mdx) · [`docs/features.mdx`](../docs/features.mdx) · [`docs/bugbot/how-it-works.mdx`](../docs/bugbot/how-it-works.mdx) · [`docs/bugbot/configuration.mdx`](../docs/bugbot/configuration.mdx) · [`docs/bugbot/detection.mdx`](../docs/bugbot/detection.mdx) · [`docs/pull-requests/workflow-setup.mdx`](../docs/pull-requests/workflow-setup.mdx) · [`docs/pull-requests/examples.mdx`](../docs/pull-requests/examples.mdx) · [`docs/issues/comment-commands.mdx`](../docs/issues/comment-commands.mdx) diff --git a/specs/catalog.json b/specs/catalog.json index 3e0ac474d..949e0be71 100644 --- a/specs/catalog.json +++ b/specs/catalog.json @@ -1943,7 +1943,7 @@ "status": "implemented", "scope": "Enrich linked and unlinked pull requests with safe issue linkage, projects, metadata, reviewers, concise descriptions, and distinct workflow evidence", "owner": "Copilot maintainers", - "lastVerified": "2026-10-13", + "lastVerified": "2026-10-20", "specs": [ "specs/pull-request-lifecycle-and-enrichment.md" ], @@ -1991,7 +1991,9 @@ "src/application/policies/copilot_evidence_policy.ts", "src/actions/github_action_completion.ts", "src/data/repository/project/project_board_item_query.ts", - "src/data/repository/project/project_board_field_update.ts" + "src/data/repository/project/project_board_field_update.ts", + "src/application/ports/pull_request_issue_link_ports.ts", + "src/infrastructure/github/ports/github_pull_request_provider_ports.ts" ], "tests": [ "src/application/usecases/execution/__tests__/execution_issue_number_policy.test.ts", diff --git a/specs/pull-request-lifecycle-and-enrichment.md b/specs/pull-request-lifecycle-and-enrichment.md index f47f9217e..498515335 100644 --- a/specs/pull-request-lifecycle-and-enrichment.md +++ b/specs/pull-request-lifecycle-and-enrichment.md @@ -668,3 +668,61 @@ propagation timing is not asserted as a guaranteed consistency SLA. Primary schema references: [Issue projectItems](https://docs.github.com/en/graphql/reference/issues), [PullRequest projectItems](https://docs.github.com/en/graphql/reference/pulls), and [ProjectV2Item](https://docs.github.com/en/graphql/reference/projects). + + +## 22. Authenticated exact issue linkage (2026-10-20, #610) + +Observed defect: the HTML linkage query ignored the configured token and returned +404 for private PRs. The intentional contract now uses authenticated GitHub +GraphQL `closingIssuesReferences`, including manually linked issues, for the +exact repository and PR. Only the intended issue in that same repository permits +the already-linked no-op. No HTML request, public-repository workaround, or extra +permission is allowed. The semantic link port carries the intended issue number; +the credential-bound adapter owns provider DTOs and error mapping. + +```text +bound PR + intended issue -> authenticated references -> exact match: no-op + -> proven absence: existing link/restore flow + -> incomplete/error: retain or compensate state +``` + +Text equivalent: inspect authenticated references first; preserve the existing +link-and-compensate sequence only after proven absence. Missing PRs, denied or +invalid credentials, rate limiting, and transport errors remain distinct. +Unsupported clients and malformed/exhausted reference evidence are nonretryable. +The fixed limit is two pages of 100 references, with validated cursor progression; +no configuration, persistence migration, extra comment, or model call is added. +The base/body compensation and owned-marker recovery rules remain mandatory. + +Representative failed-state content uses the shared semantic error view: +“Unable to inspect issue linkage for pull request #12.” A missing resource shows +`provider.not-found`, “Verify the target resource and retry the operation”, and +`Retryable: No`. Temporary transport failure keeps `provider.unavailable` and +retry guidance. Completed project/review work remains completed. Normal successful +linkage adds no new publication. Existing locale, text-only, and noise rules apply. + +The regression budget is **at least 27 adapter cases** across authenticated +public/private access (2), exact/absent/paginated references (3), HTTP and GraphQL +error mapping (11), malformed/foreign/exhausted evidence (8), rate limiting (1), +unsupported capability (1), and input identity (1). Existing use-case restoration +and capability-binding suites remain required, including failure before edits +and interrupted compensation. Deterministic mocks verify no HTML call, credential +binding, and a maximum of two reads. Repository coverage and architecture gates +remain unchanged. Documentation: `docs/pull-requests/configuration.mdx`. + +Acceptance: authenticated private PR inspection succeeds; a foreign link cannot +skip the intended link; missing/incomplete evidence never proves absence; errors +before edits retain the body/base; owned pending edits are compensated on failure. +Validate with the lifecycle repository, linkage use-case and binding suites, +typecheck, lint, specification and documentation checks. The existing adapter and +composition architecture checks enforce dependency direction. Rollback restores +prior lookup behavior without data cleanup; private linkage would regress. +Live acceptance uses the managed correction PR, then the published patch in a +private consumer. Local verification does not claim consumer deployment. + +Primary schema: [GitHub PullRequest closingIssuesReferences](https://docs.github.com/en/graphql/reference/pulls). + +Review follow-up: an HTTP 200 GraphQL `UNAUTHORIZED` error is a nonretryable +`authorization.denied`, consistent with `FORBIDDEN` and the existing deployment +GraphQL error policy. A dedicated regression first reproduces the incorrect +retryable outage and then verifies terminal, sanitized credential guidance. diff --git a/src/application/ports/pull_request_issue_link_ports.ts b/src/application/ports/pull_request_issue_link_ports.ts index 1a6236fbc..3b0ad5fb2 100644 --- a/src/application/ports/pull_request_issue_link_ports.ts +++ b/src/application/ports/pull_request_issue_link_ports.ts @@ -1,5 +1,5 @@ export interface PullRequestIssueLinkPort { - isLinked(owner: string, repository: string, pullRequestNumber: number, token: string): Promise; + isLinked(owner: string, repository: string, pullRequestNumber: number, issueNumber: number, token: string): Promise; getDetails(owner: string, repository: string, pullRequestNumber: number, token: string): Promise; updateBaseBranch(owner: string, repository: string, pullRequestNumber: number, baseBranch: string, token: string): Promise; updateDescription(owner: string, repository: string, pullRequestNumber: number, description: string, token: string): Promise; @@ -13,7 +13,7 @@ export interface PullRequestLinkDetails { /** Repository-credential-bound PR linkage query and mutation authority. */ export interface BoundPullRequestIssueLinkPort { - isLinked(pullRequestNumber: number): Promise; + isLinked(pullRequestNumber: number, issueNumber: number): Promise; getDetails(pullRequestNumber: number): Promise; updateBaseBranch(pullRequestNumber: number, baseBranch: string): Promise; updateDescription(pullRequestNumber: number, description: string): Promise; diff --git a/src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts b/src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts index f7ffef4ff..c46c26cca 100644 --- a/src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts +++ b/src/application/usecases/steps/pull_request/__tests__/link_pull_request_issue_use_case.test.ts @@ -44,7 +44,7 @@ describe('LinkPullRequestIssueUseCase', () => { const results = await useCase.invoke(context()); expect(mockGetDetails).toHaveBeenCalledWith(10); - expect(mockIsLinked).toHaveBeenCalledWith(10); + expect(mockIsLinked).toHaveBeenCalledWith(10, 42); expect(mockUpdateDescription).toHaveBeenNthCalledWith(1, 10, expect.stringContaining('Resolves #42')); expect(mockUpdateBaseBranch).toHaveBeenNthCalledWith(1, 10, 'main'); expect(mockWait).toHaveBeenCalledWith(20_000); @@ -95,6 +95,17 @@ describe('LinkPullRequestIssueUseCase', () => { expect(results.every((result) => result.success)).toBe(true); }); + it('retains the exact base and body when the initial authenticated inspection fails', async () => { + const error = new Error('linkage inspection denied'); + mockIsLinked.mockRejectedValue(error); + const results = await useCase.invoke(context()); + expect(results).toEqual([expect.objectContaining({ success: false, executed: true })]); + expect(results[0].errors[0]).toMatchObject({ code: 'provider.unavailable' }); + expect(mockUpdateBaseBranch).not.toHaveBeenCalled(); + expect(mockUpdateDescription).not.toHaveBeenCalled(); + expect(mockWait).not.toHaveBeenCalled(); + }); + it('cleans an owned pending operation when linkage inspection fails', async () => { const pending = ''; mockGetDetails.mockResolvedValue({ body: `Original\n\nResolves #42\n\n${pending}`, baseBranch: 'main' }); @@ -279,7 +290,7 @@ describe('LinkPullRequestIssueUseCase', () => { baseBranch: 'develop', }); await useCase.invoke(context()); - expect(mockIsLinked).toHaveBeenCalledWith(10); + expect(mockIsLinked).toHaveBeenCalledWith(10, 42); expect(mockUpdateDescription.mock.calls[0][1]).toContain('pr=999'); }); diff --git a/src/application/usecases/steps/pull_request/link_pull_request_issue_workflow.ts b/src/application/usecases/steps/pull_request/link_pull_request_issue_workflow.ts index 2c3f6babd..c00f159ab 100644 --- a/src/application/usecases/steps/pull_request/link_pull_request_issue_workflow.ts +++ b/src/application/usecases/steps/pull_request/link_pull_request_issue_workflow.ts @@ -98,11 +98,11 @@ export async function runLinkPullRequestIssue( let alreadyLinked = false; if (!hasOwnedPendingOperation) { - alreadyLinked = await port.isLinked(param.pullRequestNumber); + alreadyLinked = await port.isLinked(param.pullRequestNumber, param.issueNumber); if (alreadyLinked) return []; } else { try { - alreadyLinked = await port.isLinked(param.pullRequestNumber); + alreadyLinked = await port.isLinked(param.pullRequestNumber, param.issueNumber); } catch (error) { primaryError = error; } diff --git a/src/data/repository/__tests__/pull_request_lifecycle_repository.test.ts b/src/data/repository/__tests__/pull_request_lifecycle_repository.test.ts index 049e43f9e..2c90e5157 100644 --- a/src/data/repository/__tests__/pull_request_lifecycle_repository.test.ts +++ b/src/data/repository/__tests__/pull_request_lifecycle_repository.test.ts @@ -4,9 +4,10 @@ import { OctokitPullRequestLifecycleClientAdapter } from "../../../infrastructur const mockList = jest.fn(); const mockUpdate = jest.fn(); const mockGet = jest.fn(); +const mockGraphql = jest.fn(); jest.mock("@actions/github", () => ({ - getOctokit: jest.fn(() => ({ rest: { pulls: { list: mockList, update: mockUpdate, get: mockGet } } })), + getOctokit: jest.fn(() => ({ graphql: mockGraphql, rest: { pulls: { list: mockList, update: mockUpdate, get: mockGet } } })), })); jest.mock("../../../utils/logger", () => ({ @@ -19,6 +20,7 @@ describe("PullRequestLifecycleRepository", () => { jest.clearAllMocks(); mockUpdate.mockResolvedValue({}); mockGet.mockReset(); + mockGraphql.mockReset(); }); it("lists open pull requests by head branch", async () => { @@ -183,37 +185,103 @@ describe("PullRequestLifecycleRepository", () => { expect(mockGet).toHaveBeenCalledWith({ owner: 'owner', repo: 'repo', pull_number: 12 }); }); - it("fails closed with a semantic error for non-success responses and network failures", async () => { - const fetchMock = jest.spyOn(global, "fetch") - .mockResolvedValueOnce(new Response("not linked", { status: 404 })) - .mockRejectedValueOnce(new Error("network unavailable")); + it.each(['private', 'public'])('authenticates %s PR linkage for the exact repository, PR and issue', async () => { + const fetchMock = jest.spyOn(global, 'fetch'); + mockGraphql.mockResolvedValue(linkage([42])); const repository = new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()); - - await expect(repository.isLinked('owner', 'repo', 12, 'token')).rejects.toMatchObject({ - code: 'provider.unavailable', - message: 'Unable to inspect issue linkage for pull request #12.', + await expect(repository.isLinked('owner', 'repo', 12, 42, 'PRIVATE_TOKEN')).resolves.toBe(true); + expect(jest.requireMock('@actions/github').getOctokit).toHaveBeenCalledWith('PRIVATE_TOKEN'); + expect(mockGraphql).toHaveBeenCalledWith(expect.stringContaining('closingIssuesReferences(first: 100'), { + owner: 'owner', repo: 'repo', number: 12, cursor: null, }); - await expect(repository.isLinked('owner', 'repo', 12, 'token')).rejects.toMatchObject({ - code: 'provider.unavailable', - message: 'Unable to inspect issue linkage for pull request #12.', - }); - expect(fetchMock).toHaveBeenCalledWith( - 'https://github.com/owner/repo/pull/12', - expect.objectContaining({ signal: expect.any(AbortSignal) }), - ); + expect(fetchMock).not.toHaveBeenCalled(); fetchMock.mockRestore(); }); - it("detects linked GitHub issue metadata from the HTML response", async () => { - const fetchMock = jest.spyOn(global, "fetch") - .mockResolvedValueOnce(new Response("has_github_issues=true", { status: 200 })) - .mockResolvedValueOnce(new Response("has_github_issues=false", { status: 200 })); - const repository = new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()); + it('does not confuse another issue or a foreign repository with the intended issue', async () => { + const response = linkage([41, 42]); + response.repository.pullRequest.closingIssuesReferences.nodes[1].repository.nameWithOwner = 'foreign/repo'; + mockGraphql.mockResolvedValue(response); + await expect(new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()) + .isLinked('owner', 'repo', 12, 42, 'token')).resolves.toBe(false); + }); - await expect(repository.isLinked('owner', 'repo', 12, 'token')).resolves.toBe(true); - await expect(repository.isLinked('owner', 'repo', 12, 'token')).resolves.toBe(false); - fetchMock.mockRestore(); + it('proves absence from an empty complete reference list', async () => { + mockGraphql.mockResolvedValue(linkage([])); + await expect(new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()) + .isLinked('owner', 'repo', 12, 42, 'token')).resolves.toBe(false); + }); + + it('follows one bounded cursor to find the intended issue', async () => { + mockGraphql.mockResolvedValueOnce(linkage([41], true, 'next')).mockResolvedValueOnce(linkage([42])); + await expect(new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()) + .isLinked('owner', 'repo', 12, 42, 'token')).resolves.toBe(true); + expect(mockGraphql).toHaveBeenNthCalledWith(2, expect.any(String), expect.objectContaining({ cursor: 'next' })); + }); + + it.each([ + [401, 'authorization.credential-invalid'], [403, 'authorization.denied'], + [404, 'provider.not-found'], [429, 'provider.rate-limited'], [503, 'provider.unavailable'], + [undefined, 'provider.unavailable'], + ])('maps HTTP %s without exposing provider details', async (status, code) => { + mockGraphql.mockRejectedValue(Object.assign(new Error('PRIVATE_SECRET'), { status })); + await expect(new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()) + .isLinked('owner', 'repo', 12, 42, 'token')).rejects.toMatchObject({ + code, message: 'Unable to inspect issue linkage for pull request #12.', + }); + }); + + it.each([['FORBIDDEN', 'authorization.denied'], ['NOT_FOUND', 'provider.not-found'], ['RATE_LIMITED', 'provider.rate-limited'], ['INTERNAL', 'provider.unavailable']])('maps GraphQL %s errors from HTTP 200', async (type, code) => { + mockGraphql.mockRejectedValue({ status: 200, errors: [{ type, message: 'PRIVATE_SECRET' }] }); + await expect(new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()) + .isLinked('owner', 'repo', 12, 42, 'token')).rejects.toMatchObject({ code }); }); + + it('treats GraphQL UNAUTHORIZED on HTTP 200 as a terminal access failure', async () => { + mockGraphql.mockRejectedValue({ status: 200, errors: [{ type: 'UNAUTHORIZED', message: 'PRIVATE_SECRET' }] }); + await expect(new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()) + .isLinked('owner', 'repo', 12, 42, 'token')).rejects.toMatchObject({ + code: 'authorization.denied', retryable: false, + message: 'Unable to inspect issue linkage for pull request #12.', + }); + }); + + it('distinguishes secondary rate limiting from denied permissions', async () => { + mockGraphql.mockRejectedValue({ status: 403, response: { headers: { 'retry-after': '30' } } }); + await expect(new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()) + .isLinked('owner', 'repo', 12, 42, 'token')).rejects.toMatchObject({ code: 'provider.rate-limited' }); + }); + + it.each(['missing PR', 'wrong PR', 'wrong repository', 'missing references', 'invalid node', 'missing page facts', 'missing cursor', 'exhausted pages'])('fails closed on %s', async scenario => { + const response = linkage([41], scenario === 'missing cursor' || scenario === 'exhausted pages', 'next'); + if (scenario === 'missing PR') Object.assign(response.repository, { pullRequest: null }); + if (scenario === 'wrong PR') response.repository.pullRequest.number = 13; + if (scenario === 'wrong repository') response.repository.nameWithOwner = 'foreign/repo'; + if (scenario === 'missing references') Object.assign(response.repository.pullRequest, { closingIssuesReferences: null }); + if (scenario === 'invalid node') Object.assign(response.repository.pullRequest.closingIssuesReferences, { nodes: [null] }); + if (scenario === 'missing page facts') Object.assign(response.repository.pullRequest.closingIssuesReferences, { pageInfo: {} }); + if (scenario === 'missing cursor') Object.assign(response.repository.pullRequest.closingIssuesReferences.pageInfo, { endCursor: null }); + mockGraphql.mockResolvedValue(response); + if (scenario === 'exhausted pages') mockGraphql.mockResolvedValueOnce(response).mockResolvedValueOnce(linkage([41], true, 'last')); + await expect(new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()) + .isLinked('owner', 'repo', 12, 42, 'token')).rejects.toMatchObject({ + code: scenario === 'missing PR' ? 'provider.not-found' : 'provider.contract-invalid', retryable: false, + }); + expect(mockGraphql.mock.calls.length).toBeLessThanOrEqual(2); + }); + + it('reports unsupported authenticated queries without falling back to HTML', async () => { + const repository = new PullRequestLifecycleRepository({ getClient: () => ({ rest: { pulls: { list: mockList, update: mockUpdate } } }) }); + await expect(repository.isLinked('owner', 'repo', 12, 42, 'token')) + .rejects.toMatchObject({ code: 'configuration.unsupported', retryable: false }); + }); + + it('rejects invalid identities before using credentials', async () => { + await expect(new PullRequestLifecycleRepository(new OctokitPullRequestLifecycleClientAdapter()) + .isLinked('owner', 'repo', 12, 0, 'token')).rejects.toMatchObject({ code: 'validation.invalid-input' }); + expect(mockGraphql).not.toHaveBeenCalled(); + }); + }); function pullRequestIdentity(number: number) { @@ -231,3 +299,12 @@ function pullRequestIdentity(number: number) { }, }; } + +function linkage(numbers: number[], hasNextPage = false, endCursor: string | null = null) { + return { repository: { nameWithOwner: 'owner/repo', pullRequest: { number: 12, + closingIssuesReferences: { + nodes: numbers.map(number => ({ number, repository: { nameWithOwner: 'owner/repo' } })), + pageInfo: { hasNextPage, endCursor }, + }, + } } }; +} diff --git a/src/data/repository/pull_request/pull_request_lifecycle_repository.ts b/src/data/repository/pull_request/pull_request_lifecycle_repository.ts index 54cf3419a..9475c4291 100644 --- a/src/data/repository/pull_request/pull_request_lifecycle_repository.ts +++ b/src/data/repository/pull_request/pull_request_lifecycle_repository.ts @@ -5,8 +5,10 @@ import type { GithubClientPort } from "../../../infrastructure/github/ports/gith import type { GithubPullRequestLifecycleClient, GithubPullRequestSummary, + GithubPullRequestLinkageResponse, } from "../../../infrastructure/github/ports/github_pull_request_provider_ports"; -import { toApplicationError } from '../../../application/errors/application_error'; +import { ApplicationError, toApplicationError } from '../../../application/errors/application_error'; +import { getGithubErrorStatus, isGithubRateLimited } from '../github/github_error_policy'; import type { BugbotPullRequestIdentity } from '../../../domain/bugbot/context'; export class PullRequestLifecycleRepository implements PullRequestHeadShaPort { @@ -114,35 +116,70 @@ export class PullRequestLifecycleRepository implements PullRequestHeadShaPort { throw new Error(`Open pull request pagination exceeded ${maximumPages} pages.`); } - /** Default timeout (ms) for isLinked fetch. */ - private static readonly IS_LINKED_FETCH_TIMEOUT_MS = 10000; - + /** Reads at most 200 closing references through the credential-bound API. */ isLinked = async ( owner: string, repository: string, pullRequestNumber: number, - _token: string, + issueNumber: number, + token: string, ): Promise => { - const pullRequestUrl = `https://github.com/${encodeURIComponent(owner)}/${encodeURIComponent(repository)}/pull/${pullRequestNumber}`; - const controller = new AbortController(); - const timeoutId = setTimeout(() => controller.abort(), PullRequestLifecycleRepository.IS_LINKED_FETCH_TIMEOUT_MS); try { - const res = await fetch(pullRequestUrl, { signal: controller.signal }); - if (!res.ok) { - throw new Error(`Pull-request linkage query returned HTTP ${res.status}.`); + if (![pullRequestNumber, issueNumber].every(number => Number.isSafeInteger(number) && number > 0)) { + throw new ApplicationError('validation.invalid-input', 'Linkage requires positive PR and issue numbers.'); } - const htmlContent = await res.text(); - return !htmlContent.includes('has_github_issues=false'); + const client = this.githubClient.getClient(token); + if (!client.graphql) throw new ApplicationError('configuration.unsupported', 'Authenticated PR linkage queries are not supported by this installation.'); + const expectedRepository = `${owner}/${repository}`.toLowerCase(); + let cursor: string | null = null; + for (let page = 0; page < 2; page += 1) { + const result: GithubPullRequestLinkageResponse = await client.graphql(` + query PullRequestIssueLinkage($owner: String!, $repo: String!, $number: Int!, $cursor: String) { + repository(owner: $owner, name: $repo) { + nameWithOwner + pullRequest(number: $number) { + number + closingIssuesReferences(first: 100, after: $cursor) { + nodes { number repository { nameWithOwner } } + pageInfo { hasNextPage endCursor } + } + } + } + }`, { owner, repo: repository, number: pullRequestNumber, cursor }); + const resource = result?.repository; + if (resource === null || resource?.pullRequest === null) { + throw new ApplicationError('provider.not-found', 'The requested pull request is unavailable to the configured credential.'); + } + const pr = resource?.pullRequest; + const references = pr?.closingIssuesReferences; + if (typeof resource?.nameWithOwner !== 'string' || resource.nameWithOwner.toLowerCase() !== expectedRepository || pr?.number !== pullRequestNumber + || !Array.isArray(references?.nodes) || references.nodes.length > 100 + || typeof references.pageInfo?.hasNextPage !== 'boolean' + || references.nodes.some(node => !node || !Number.isSafeInteger(node.number) || node.number <= 0 + || typeof node.repository?.nameWithOwner !== 'string' || !node.repository.nameWithOwner)) { + throw new ApplicationError('provider.contract-invalid', 'GitHub did not return verifiable PR linkage evidence.'); + } + if (references.nodes.some(node => node.number === issueNumber + && node.repository.nameWithOwner.toLowerCase() === expectedRepository)) return true; + if (!references.pageInfo.hasNextPage) return false; + const nextCursor = references.pageInfo.endCursor; + if (typeof nextCursor !== 'string' || !nextCursor || nextCursor === cursor) { + throw new ApplicationError('provider.contract-invalid', 'GitHub returned incomplete linkage pagination.'); + } + cursor = nextCursor; + } + throw new ApplicationError('provider.contract-invalid', 'PR linkage exceeds the fixed 200-reference inspection bound.'); } catch (error) { - const semanticError = toApplicationError( - error, - 'provider.unavailable', - `Unable to inspect issue linkage for pull request #${pullRequestNumber}.`, - ); + const status = getGithubErrorStatus(error); + const types = typeof error === 'object' && error !== null && 'errors' in error && Array.isArray(error.errors) + ? error.errors.map((entry: { type?: string }) => entry?.type) : []; + const code = isGithubRateLimited(error) || types.includes('RATE_LIMITED') ? 'provider.rate-limited' + : status === 401 ? 'authorization.credential-invalid' + : status === 403 || types.includes('FORBIDDEN') || types.includes('UNAUTHORIZED') ? 'authorization.denied' + : status === 404 || types.includes('NOT_FOUND') ? 'provider.not-found' : 'provider.unavailable'; + const semanticError = toApplicationError(error, code, `Unable to inspect issue linkage for pull request #${pullRequestNumber}.`); logError(semanticError); throw semanticError; - } finally { - clearTimeout(timeoutId); } }; diff --git a/src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts b/src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts index fe715dd5a..9001f0ef3 100644 --- a/src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts +++ b/src/infrastructure/composition/__tests__/lifecycle_capability_port_binding.test.ts @@ -118,7 +118,7 @@ describe('lifecycle capability repository bindings', () => { const updateBaseBranch = jest.fn().mockResolvedValue(undefined); const updateLinkDescription = jest.fn().mockResolvedValue(undefined); const link = bindPullRequestIssueLink({ isLinked, getDetails: getLinkDetails, updateBaseBranch, updateDescription: updateLinkDescription }, binding); - await link.isLinked(9); + await link.isLinked(9, 42); await link.getDetails(9); await link.updateBaseBranch(9, 'main'); await link.updateDescription(9, 'linked'); @@ -132,7 +132,7 @@ describe('lifecycle capability repository bindings', () => { await description.getDetails(9); await expect(description.getDiff(9)).resolves.toEqual(snapshot); - expect(isLinked).toHaveBeenCalledWith('acme', 'demo', 9, 'secret'); + expect(isLinked).toHaveBeenCalledWith('acme', 'demo', 9, 42, 'secret'); expect(getLinkDetails).toHaveBeenCalledWith('acme', 'demo', 9, 'secret'); expect(updateBaseBranch).toHaveBeenCalledWith('acme', 'demo', 9, 'main', 'secret'); expect(updateLinkDescription).toHaveBeenCalledWith('acme', 'demo', 9, 'linked', 'secret'); diff --git a/src/infrastructure/composition/lifecycle_capability_port_binding.ts b/src/infrastructure/composition/lifecycle_capability_port_binding.ts index 2bdc1d479..55450142a 100644 --- a/src/infrastructure/composition/lifecycle_capability_port_binding.ts +++ b/src/infrastructure/composition/lifecycle_capability_port_binding.ts @@ -270,10 +270,11 @@ export function bindPullRequestIssueLink( binding: RepositoryCredentialBinding, ): BoundPullRequestIssueLinkPort { return Object.freeze({ - isLinked: (pullRequestNumber) => port.isLinked( + isLinked: (pullRequestNumber, issueNumber) => port.isLinked( binding.owner, binding.repository, pullRequestNumber, + issueNumber, binding.token, ), getDetails: (pullRequestNumber) => port.getDetails( diff --git a/src/infrastructure/github/ports/github_pull_request_provider_ports.ts b/src/infrastructure/github/ports/github_pull_request_provider_ports.ts index 935abc9d9..959044508 100644 --- a/src/infrastructure/github/ports/github_pull_request_provider_ports.ts +++ b/src/infrastructure/github/ports/github_pull_request_provider_ports.ts @@ -27,7 +27,21 @@ export interface GithubPullRequestFile { patch?: string; } +export interface GithubPullRequestLinkageResponse { + repository?: { + nameWithOwner?: string; + pullRequest?: { + number?: number; + closingIssuesReferences?: { + nodes: { number: number; repository: { nameWithOwner: string } }[]; + pageInfo: { hasNextPage: boolean; endCursor?: string | null }; + } | null; + } | null; + } | null; +} + export interface GithubPullRequestLifecycleClient { + graphql?(query: string, variables: Record): Promise; rest: { pulls: { list(