From bf12885a4ddde4f615941fce2e0506059fd70af9 Mon Sep 17 00:00:00 2001 From: Ralf Anton Beier Date: Sat, 25 Apr 2026 22:49:46 +0200 Subject: [PATCH] =?UTF-8?q?fix:=20AI=20review=20failed=20silently=20?= =?UTF-8?q?=E2=80=94=20octokit.issues=20namespace=20undefined=20at=20runti?= =?UTF-8?q?me?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Root cause On the deployed bot, every `pull_request.opened` event triggered an AI review that immediately failed with `Cannot read properties of undefined (reading 'listComments')` and `(reading 'createComment')`. Production logs show this hitting PRs #209, #210, … #213 in pulseengine/rivet. Zero AI review comments have appeared on any pulseengine PR since at least 2026-03. `src/ai-review.js` called `octokit.issues.listComments`, `.updateComment`, and `.createComment`. Probot v14's bundled Octokit (`@octokit/rest@22`) in the `pull_request.opened` event context does **not** expose the `.issues` namespace plugin. The same `context.octokit` works through `.request(route, params)` (which is what every other call site in the codebase uses). ## Fix Replace the three `octokit.issues.X` calls with the `.request()` form: | Was | Now | |---|---| | `octokit.issues.listComments({...})` | `octokit.request('GET /repos/{owner}/{repo}/issues/{issue_number}/comments', {...})` | | `octokit.issues.updateComment({...})` | `octokit.request('PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}', {...})` | | `octokit.issues.createComment({...})` | `octokit.request('POST /repos/{owner}/{repo}/issues/{issue_number}/comments', {...})` | The test mock retrofit dispatches `.request(route, params)` into the existing namespaced `jest.fn()`s when the route matches one of those three — so all 166 ai-review test assertions still apply unchanged. ## Test plan - [x] All 698 tests pass - [x] eslint clean - [ ] After merge + self-update: open a draft PR in pulseengine/temper (or any non-bot PR in another pulseengine repo). The AI review comment should appear within ~5 min. If it still doesn't, the second remaining cause is the Ollama timeout — `qwen2.5-coder:3b` on the netcup VPS sometimes exceeds the 300s `ai_review.timeout`. ## Risk & rollout - Risk: low. Pure call-shape change. Same endpoints, same params. - Rollout: self-update on merge. Confirmation = AI review comment lands on the next PR opened in any pulseengine repo. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 (1M context) --- __tests__/integration/ai-review.test.js | 29 +++++++++++++++--- src/ai-review.js | 40 +++++++++++++------------ 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/__tests__/integration/ai-review.test.js b/__tests__/integration/ai-review.test.js index c49e60f..06d8bee 100644 --- a/__tests__/integration/ai-review.test.js +++ b/__tests__/integration/ai-review.test.js @@ -40,12 +40,33 @@ import { getLogger } from '../../src/logger.js'; // --------------------------------------------------------------------------- function createMockOctokit() { + // Production code now calls octokit.request(route, params) for issue comments + // (was octokit.issues.X — that namespace turned out to be undefined at runtime + // in Probot v14's pull_request.opened context). Existing tests assert against + // octokit.issues.X mocks, so we retrofit the shape: route the request mock + // into the namespaced jest.fn()s for the three relevant routes. + const issuesListComments = jest.fn().mockResolvedValue({ data: [] }); + const issuesCreateComment = jest.fn().mockResolvedValue({ status: 201, data: {} }); + const issuesUpdateComment = jest.fn().mockResolvedValue({ status: 200, data: {} }); + + const routeDispatch = { + 'GET /repos/{owner}/{repo}/issues/{issue_number}/comments': issuesListComments, + 'POST /repos/{owner}/{repo}/issues/{issue_number}/comments': issuesCreateComment, + 'PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}': issuesUpdateComment + }; + + const request = jest.fn((route, params) => { + const dispatch = routeDispatch[route]; + if (dispatch) return dispatch(params); + return Promise.resolve({ status: 200, data: {} }); + }); + return { - request: jest.fn().mockResolvedValue({ status: 200, data: {} }), + request, issues: { - createComment: jest.fn().mockResolvedValue({ status: 201, data: {} }), - listComments: jest.fn().mockResolvedValue({ data: [] }), - updateComment: jest.fn().mockResolvedValue({ status: 200, data: {} }) + listComments: issuesListComments, + createComment: issuesCreateComment, + updateComment: issuesUpdateComment } }; } diff --git a/src/ai-review.js b/src/ai-review.js index aac4f00..7a742ac 100644 --- a/src/ai-review.js +++ b/src/ai-review.js @@ -285,13 +285,14 @@ function formatReviewComment(aiResponse, prNumber, headSha, meta) { */ async function supersedePreviousReviews(octokit, owner, repo, prNumber) { try { - // List all comments on the PR - const { data: comments } = await octokit.issues.listComments({ - owner, - repo, - issue_number: prNumber, - per_page: 100 - }); + // Use octokit.request() form (not .issues.listComments) because Probot v14's + // bundled Octokit does not always expose the .issues namespace plugin in the + // pull_request.opened event context \u2014 that mismatch was silently breaking + // every auto AI review with "Cannot read properties of undefined". + const { data: comments } = await octokit.request( + 'GET /repos/{owner}/{repo}/issues/{issue_number}/comments', + { owner, repo, issue_number: prNumber, per_page: 100 } + ); const outdatedPrefix = '> \u26a0\ufe0f **Outdated** \u2014 this review was for an earlier revision. See the latest review below.\n\n'; @@ -302,12 +303,15 @@ async function supersedePreviousReviews(octokit, owner, repo, prNumber) { comment.body.includes(AI_REVIEW_SIGNATURE) && !comment.body.startsWith('>') ) { - await octokit.issues.updateComment({ - owner, - repo, - comment_id: comment.id, - body: outdatedPrefix + comment.body - }); + await octokit.request( + 'PATCH /repos/{owner}/{repo}/issues/comments/{comment_id}', + { + owner, + repo, + comment_id: comment.id, + body: outdatedPrefix + comment.body + } + ); } } } catch (error) { @@ -396,12 +400,10 @@ async function reviewPullRequest(octokit, owner, repo, prNumber) { // Mark previous bot reviews as outdated await supersedePreviousReviews(octokit, owner, repo, prNumber); - await octokit.issues.createComment({ - owner, - repo, - issue_number: prNumber, - body: comment - }); + await octokit.request( + 'POST /repos/{owner}/{repo}/issues/{issue_number}/comments', + { owner, repo, issue_number: prNumber, body: comment } + ); // Store the review record storeReview({