feat(core): extract PR mutation handlers (approve, reject, edit)#167
feat(core): extract PR mutation handlers (approve, reject, edit)#167galligan wants to merge 1 commit into
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Add three transport-agnostic PR mutation handlers: - approveHandler: submit APPROVE review via REST API - rejectHandler: submit REQUEST_CHANGES review via REST API - editHandler: polymorphic — edit PR metadata (title, body, base, labels, milestone) or edit a comment body; routes by classifyId() All validate repo format, authenticate via detectAuth, and return Result<T, Error>. No transport-specific code. 🤘🏻 In-collaboration-with: [Claude Code](https://claude.com/claude-code)
galligan
force-pushed
the
fire-6/feedback-handlers
branch
from
07f873a to
c806561
Compare
galligan
force-pushed
the
fire-7/pr-handlers
branch
from
267df45 to
8889f7c
Compare
galligan
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const numericId = extractNumericId(entry.id); | ||
| if (!numericId) { | ||
| return Result.err( | ||
| new ValidationError({ | ||
| message: `Cannot determine numeric ID for comment: ${entry.id}`, | ||
| }) | ||
| ); | ||
| } |
There was a problem hiding this comment.
extractNumericId(entry.id) won't work reliably - entry.database_id is optional and not always populated. Should use the same approach as apps/cli/src/commands/edit.ts:168-175 which tries entry.database_id first, then falls back to base64 decoding:
| const numericId = extractNumericId(entry.id); | |
| if (!numericId) { | |
| return Result.err( | |
| new ValidationError({ | |
| message: `Cannot determine numeric ID for comment: ${entry.id}`, | |
| }) | |
| ); | |
| } | |
| // Prefer stored database_id (synced from GitHub API) | |
| const numericId = entry.database_id ?? extractNumericId(entry.id); | |
| if (!numericId) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/handlers/edit.ts
Line: 202-209
Comment:
`extractNumericId(entry.id)` won't work reliably - `entry.database_id` is optional and not always populated. Should use the same approach as `apps/cli/src/commands/edit.ts:168-175` which tries `entry.database_id` first, then falls back to base64 decoding:
```suggestion
// Prefer stored database_id (synced from GitHub API)
const numericId = entry.database_id ?? extractNumericId(entry.id);
if (!numericId) {
```
How can I resolve this? If you propose a fix, please make it concise.
| } | ||
|
|
||
| /** Extract numeric ID from various GitHub ID formats. */ | ||
| function extractNumericId(id: string): number | null { | ||
| // Try direct numeric | ||
| const num = Number(id); | ||
| if (!Number.isNaN(num) && num > 0) { | ||
| return num; | ||
| } | ||
| // Try extracting trailing number from node ID patterns |
There was a problem hiding this comment.
extractNumericId() is too simplistic - GitHub node IDs are base64-encoded (e.g., "IC_kwDO..."). The regex won't extract the numeric ID correctly. Use the base64 decoding approach from apps/cli/src/commands/edit.ts:148-162:
| } | |
| /** Extract numeric ID from various GitHub ID formats. */ | |
| function extractNumericId(id: string): number | null { | |
| // Try direct numeric | |
| const num = Number(id); | |
| if (!Number.isNaN(num) && num > 0) { | |
| return num; | |
| } | |
| // Try extracting trailing number from node ID patterns | |
| function extractNumericId(id: string): number | null { | |
| // Try direct numeric | |
| const num = Number(id); | |
| if (!Number.isNaN(num) && num > 0) { | |
| return num; | |
| } | |
| // Try base64 decoding for GraphQL node IDs | |
| try { | |
| const decoded = Buffer.from(id, "base64").toString("utf8"); | |
| const matches = decoded.match(/\d+/g); | |
| const last = matches?.at(-1); | |
| return last ? Number(last) : null; | |
| } catch { | |
| return null; | |
| } | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/handlers/edit.ts
Line: 222-231
Comment:
`extractNumericId()` is too simplistic - GitHub node IDs are base64-encoded (e.g., "IC_kwDO..."). The regex won't extract the numeric ID correctly. Use the base64 decoding approach from `apps/cli/src/commands/edit.ts:148-162`:
```suggestion
function extractNumericId(id: string): number | null {
// Try direct numeric
const num = Number(id);
if (!Number.isNaN(num) && num > 0) {
return num;
}
// Try base64 decoding for GraphQL node IDs
try {
const decoded = Buffer.from(id, "base64").toString("utf8");
const matches = decoded.match(/\d+/g);
const last = matches?.at(-1);
return last ? Number(last) : null;
} catch {
return null;
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
| const editResult = await client.editIssueComment( | ||
| owner, | ||
| repo, | ||
| numericId, |
There was a problem hiding this comment.
Only handles issue comments - missing review_comment support. Check entry.subtype and call client.editReviewComment() for review comments (see apps/cli/src/commands/edit.ts:622-626)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/core/src/handlers/edit.ts
Line: 211-214
Comment:
Only handles issue comments - missing `review_comment` support. Check `entry.subtype` and call `client.editReviewComment()` for review comments (see `apps/cli/src/commands/edit.ts:622-626`)
How can I resolve this? If you propose a fix, please make it concise.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8889f7c379
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const normalized = normalizeShortId(input.id); | ||
| const resolved = resolveShortId(normalized); |
There was a problem hiding this comment.
This path resolves @shortid via resolveShortId only, but it never loads entries or calls buildShortIdCache, so in a fresh process the map is empty and valid short IDs fail with Could not resolve short ID. That makes edit by short ID non-functional unless some previous command happened to warm the in-memory cache first.
Useful? React with 👍 / 👎.
| ); | ||
| } | ||
|
|
||
| const editResult = await client.editIssueComment( |
There was a problem hiding this comment.
The handler always calls editIssueComment, but GitHub review comments require the pull-review endpoint (editReviewComment). When the cached entry is a review_comment, this uses the wrong API route and edit requests fail for code-review comments, which are a primary feedback type in this tool.
Useful? React with 👍 / 👎.
| // Try extracting trailing number from node ID patterns | ||
| const match = id.match(/(\d+)$/); | ||
| return match ? Number(match[1]) : null; |
There was a problem hiding this comment.
Extracting the REST comment ID by regexing trailing digits from entry.id is unreliable because entry.id is an opaque GraphQL node ID, not the REST numeric ID. This causes frequent false failures (Cannot determine numeric ID) and can also produce an unrelated number when a node ID happens to end with digits, potentially editing the wrong comment.
Useful? React with 👍 / 👎.
1 participant
Summary
approveHandler,rejectHandler,editHandlerto coreTest plan
bun run checkpassesbun testpasses (288 tests)🤘🏻 In-collaboration-with: Claude Code
Greptile Summary
Extracts PR mutation handlers (
approveHandler,rejectHandler,editHandler) from CLI to core package for code reuse.approve.tsandreject.tsare clean, straightforward implementationsedit.tshas critical bugs in comment editing:extractNumericId()won't correctly decode GitHub node IDs (needs base64 decoding)entry.database_idfieldreview_commentsubtype (only handles issue comments)apps/cli/src/commands/edit.ts) has the correct approach that should be replicatedConfidence Score: 2/5
editHandlerhas three critical bugs that will cause failures when editing comments: incorrect numeric ID extraction, missing database_id fallback, and missing review_comment support. The approve and reject handlers are fine.Important Files Changed
Last reviewed commit: 8889f7c