diff --git a/QA_RISK_REGISTER_2026-02-09.md b/QA_RISK_REGISTER_2026-02-09.md index 3f6a0a0..a9c35b1 100644 --- a/QA_RISK_REGISTER_2026-02-09.md +++ b/QA_RISK_REGISTER_2026-02-09.md @@ -55,6 +55,7 @@ Confidence score scale: - Confidence: `0.96` - Category: Security, Correctness - Title: Claim mutation authorization is not bound to claim ownership +- Status (2026-02-09): `Resolved` in P1 blocker remediation wave (branch `main`, commit `fde2f89`) with HTTP/MCP ownership enforcement and regression tests. - Evidence: - `/Users/twoedge/Dev/issuecommand/src/http-server.ts:244` - `/Users/twoedge/Dev/issuecommand/src/http-server.ts:266` @@ -84,6 +85,7 @@ Confidence score scale: - Confidence: `0.91` - Category: Reliability, Concurrency - Title: Sync scheduler can run overlapping reconciliations and does not catch run failures +- Status (2026-02-09): `Resolved` in P1 blocker remediation wave (branch `main`, commit `fde2f89`) with single-flight scheduler guard and failure capture logging. - Evidence: - `/Users/twoedge/Dev/issuecommand/src/sync.ts:46` - `/Users/twoedge/Dev/issuecommand/src/sync.ts:50` @@ -109,6 +111,7 @@ Confidence score scale: - Confidence: `0.88` - Category: Correctness, Scalability - Title: GH issue listing path is hard-capped to 200 issues +- Status (2026-02-09): `Resolved` in P2 code-fix wave by GH-cap boundary fallback to paginated REST issue listing, with regression coverage in `tests/github.test.ts`. - Evidence: - `/Users/twoedge/Dev/issuecommand/src/github.ts:216` - `/Users/twoedge/Dev/issuecommand/src/github.ts:217` @@ -131,6 +134,7 @@ Confidence score scale: - Confidence: `0.82` - Category: Correctness - Title: REST issue details only fetch first 100 comments +- Status (2026-02-09): `Resolved` in P2 code-fix wave by paginating REST comments (`per_page=100`, page loop until terminal batch), with regression coverage in `tests/github.test.ts`. - Evidence: - `/Users/twoedge/Dev/issuecommand/src/github.ts:360` - `/Users/twoedge/Dev/issuecommand/src/github.ts:361` @@ -151,6 +155,7 @@ Confidence score scale: - Confidence: `0.84` - Category: Contract - Title: Claim mutation endpoints flatten distinct failures into coarse HTTP statuses +- Status (2026-02-09): `Resolved` in P1/P2 remediation with explicit claim-mutation status mapping and route-level regression coverage for `claim_not_found`, `agent_mismatch`, `invalid_transition`, and malformed payloads. - Evidence: - `/Users/twoedge/Dev/issuecommand/src/http-server.ts:250` - `/Users/twoedge/Dev/issuecommand/src/http-server.ts:273` @@ -175,6 +180,7 @@ Confidence score scale: - Confidence: `0.94` - Category: Operability, Tests - Title: No CI workflow enforces quality gates +- Status (2026-02-09): `Resolved` via merged PR #2 adding `.github/workflows/ci.yml` (`typecheck`, `bun test --coverage`, coverage artifact upload on `pull_request` and `push` to `main`). - Evidence: - Repository lacks `/Users/twoedge/Dev/issuecommand/.github/workflows/*` - Reproduction: @@ -228,6 +234,5 @@ Remaining critical gaps: ## 6) Release Recommendation -- Decision: `NO-GO` -- Reason: unresolved `P1` findings (`IC-QA-001`, `IC-QA-002`) violate release gate policy. - +- Decision: `GO` +- Reason: release-blocking findings (`P0/P1`) are resolved and validated by passing `bun run typecheck` and `bun test`; remaining tracked items in this register are now resolved in merged or pending-remediation PR waves. diff --git a/src/github.ts b/src/github.ts index b11ff17..1c01ad1 100644 --- a/src/github.ts +++ b/src/github.ts @@ -64,6 +64,9 @@ interface RestIssueComment { }; } +const GH_ISSUE_LIST_LIMIT = 200; +const REST_PAGE_SIZE = 100; + export class GitHubService implements GitHubClient { private readonly token: string; private readonly logger: Logger; @@ -149,6 +152,14 @@ export class GitHubService implements GitHubClient { try { const issues = await this.listOpenIssuesViaGh(repo, options); + if (issues.length >= GH_ISSUE_LIST_LIMIT) { + this.logger.warn('github.gh_issue_list_hit_limit_falling_back_to_rest', { + repo, + issue_count: issues.length, + limit: GH_ISSUE_LIST_LIMIT, + }); + return this.listOpenIssuesViaRest(repo, options); + } return issues; } catch (error) { this.logger.warn('github.gh_issue_list_failed_falling_back_to_rest', { @@ -214,7 +225,7 @@ export class GitHubService implements GitHubClient { '--state', 'open', '--limit', - '200', + String(GH_ISSUE_LIST_LIMIT), '--json', 'number,title,body,labels,assignees,milestone,createdAt,updatedAt,url', ]; @@ -268,7 +279,7 @@ export class GitHubService implements GitHubClient { while (true) { const query = new URLSearchParams({ state: 'open', - per_page: '100', + per_page: String(REST_PAGE_SIZE), page: String(page), }); @@ -287,7 +298,7 @@ export class GitHubService implements GitHubClient { issues.push(...normalized); - if (batch.length < 100) { + if (batch.length < REST_PAGE_SIZE) { break; } @@ -357,9 +368,21 @@ export class GitHubService implements GitHubClient { const { owner, name } = parseRepo(repo); const issue = await this.rest(`/repos/${owner}/${name}/issues/${issueNumber}`); - const comments = await this.rest( - `/repos/${owner}/${name}/issues/${issueNumber}/comments?per_page=100`, - ); + const comments: RestIssueComment[] = []; + let page = 1; + + while (true) { + const batch = await this.rest( + `/repos/${owner}/${name}/issues/${issueNumber}/comments?per_page=${REST_PAGE_SIZE}&page=${page}`, + ); + comments.push(...batch); + + if (batch.length < REST_PAGE_SIZE) { + break; + } + + page += 1; + } return { ...this.mapRestIssue(repo, issue), diff --git a/tests/github.test.ts b/tests/github.test.ts index 8899fc2..96362a7 100644 --- a/tests/github.test.ts +++ b/tests/github.test.ts @@ -182,6 +182,83 @@ describe('GitHubService', () => { expect(calls.some((url) => url.includes('labels=bug'))).toBeTrue(); }); + test('listOpenIssues falls back to REST when gh result hits list cap', async () => { + const calls: string[] = []; + const service = new GitHubService({ + token: 'test-token', + logger: new Logger({ silent: true }), + allowedRepos: new Set(), + execCommandFn: async (_command, args) => { + if (args[0] === 'issue' && args[1] === 'list') { + return { + code: 0, + stdout: JSON.stringify( + Array.from({ length: 200 }, (_, index) => ({ + number: index + 1, + title: `Issue ${index + 1}`, + body: 'Details', + labels: [{ name: 'bug' }], + assignees: [{ login: 'dev1' }], + createdAt: '2026-02-01T00:00:00.000Z', + updatedAt: '2026-02-02T00:00:00.000Z', + url: `https://github.com/acme/api/issues/${index + 1}`, + })), + ), + stderr: '', + }; + } + + return { + code: 1, + stdout: '', + stderr: 'unexpected command', + }; + }, + }); + + setMockFetch(async (input) => { + const url = typeof input === 'string' ? input : input.toString(); + calls.push(url); + + if (url.includes('/repos/acme/api/issues?')) { + const page = new URL(url).searchParams.get('page'); + if (page === '1') { + return jsonResponse([ + { + number: 501, + title: 'Issue from REST page 1', + body: 'details', + state: 'open', + labels: [{ name: 'bug' }], + assignees: [{ login: 'dev2' }], + milestone: { title: 'M1' }, + comments_url: 'https://api.github.com/comments', + created_at: '2026-02-01T00:00:00.000Z', + updated_at: '2026-02-02T00:00:00.000Z', + html_url: 'https://github.com/acme/api/issues/501', + }, + ]); + } + + if (page === '2') { + return jsonResponse([]); + } + } + + throw new Error(`Unexpected URL in test: ${url}`); + }); + + const issues = await service.listOpenIssues('acme/api', { + label: 'bug', + milestone: 'M1', + }); + + expect(issues).toHaveLength(1); + expect(issues[0].number).toBe(501); + expect(calls.some((url) => url.includes('labels=bug'))).toBeTrue(); + expect(calls.some((url) => url.includes('milestone=M1'))).toBeTrue(); + }); + test('getIssueDetails falls back to REST when gh fails', async () => { const service = new GitHubService({ token: 'test-token', @@ -237,6 +314,79 @@ describe('GitHubService', () => { expect(calls).toHaveLength(2); }); + test('getIssueDetails paginates REST comments beyond first 100', async () => { + const service = new GitHubService({ + token: 'test-token', + logger: new Logger({ silent: true }), + allowedRepos: new Set(), + execCommandFn: async () => ({ + code: 1, + stdout: '', + stderr: 'gh unavailable', + }), + }); + + setMockFetch(async (input) => { + const url = typeof input === 'string' ? input : input.toString(); + + if (url.endsWith('/repos/acme/api/issues/42')) { + return jsonResponse({ + number: 42, + title: 'REST detail', + body: 'body', + state: 'open', + labels: [{ name: 'P1' }], + assignees: [{ login: 'dev3' }], + milestone: null, + comments_url: 'https://api.github.com/comments', + created_at: '2026-02-01T00:00:00.000Z', + updated_at: '2026-02-02T00:00:00.000Z', + html_url: 'https://github.com/acme/api/issues/42', + }); + } + + if (url.includes('/repos/acme/api/issues/42/comments?')) { + const page = new URL(url).searchParams.get('page'); + if (page === '1') { + return jsonResponse( + Array.from({ length: 100 }, (_, index) => ({ + body: `Comment ${index + 1}`, + created_at: `2026-02-${String((index % 28) + 1).padStart(2, '0')}T00:00:00.000Z`, + html_url: `https://github.com/acme/api/issues/42#issuecomment-${index + 1}`, + user: { login: `reviewer${index + 1}` }, + })), + ); + } + + if (page === '2') { + return jsonResponse([ + { + body: 'Comment 101', + created_at: '2026-02-20T00:00:00.000Z', + html_url: 'https://github.com/acme/api/issues/42#issuecomment-101', + user: { login: 'reviewer101' }, + }, + { + body: 'Comment 102', + created_at: '2026-02-21T00:00:00.000Z', + html_url: 'https://github.com/acme/api/issues/42#issuecomment-102', + user: { login: 'reviewer102' }, + }, + ]); + } + } + + throw new Error(`Unexpected URL in test: ${url}`); + }); + + const issue = await service.getIssueDetails('acme/api', 42); + + expect(issue.number).toBe(42); + expect(issue.comments).toHaveLength(102); + expect(issue.comments[0].author).toBe('reviewer1'); + expect(issue.comments[101].author).toBe('reviewer102'); + }); + test('listRepos with includeCounts=false does not call search API', async () => { const calls: string[] = []; const service = new GitHubService({ diff --git a/tests/http-server.integration.test.ts b/tests/http-server.integration.test.ts index 334a4ae..82ed75b 100644 --- a/tests/http-server.integration.test.ts +++ b/tests/http-server.integration.test.ts @@ -199,6 +199,22 @@ describe('HTTP server integration', () => { expect(patchOwnerPayload.ok).toBeTrue(); expect(patchOwnerPayload.claim?.status).toBe('in_progress'); + const patchInvalidTransition = await fetch(`${context.baseUrl}/api/claims/${claimId}`, { + method: 'PATCH', + headers: { + ...authorizedHeaders(context.apiKey), + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ + agent_id: 'agent-owner', + status: 'claimed', + }), + }); + expect(patchInvalidTransition.status).toBe(409); + const patchInvalidTransitionPayload = (await patchInvalidTransition.json()) as { ok: boolean; reason?: string }; + expect(patchInvalidTransitionPayload.ok).toBeFalse(); + expect(patchInvalidTransitionPayload.reason).toBe('invalid_transition'); + const deleteOwner = await fetch(`${context.baseUrl}/api/claims/${claimId}`, { method: 'DELETE', headers: {