fix(review): signal-safe cleanup + triage suffix for PR feedback#914
Merged
Conversation
Two QA follow-ups found while validating the release: - Route SIGINT/SIGTERM through process.exit() so the existing "exit" handlers actually run on Ctrl-C / termination. A signal death previously skipped them, leaking background PR-checkout warmup children and stale `git worktree` registrations. A second signal still force-quits if cleanup hangs. - Append the review-denied triage suffix for PR-mode feedback, not just local diffs. The old `!isPRMode` gate suppressed it for every PR review. Gate on whether the reviewer actually sent annotations instead: genuine feedback always carries annotations, while platform PR actions (approve/comment posted to the host) return an empty annotation set + status message and correctly get no suffix. Applied consistently across hook, OpenCode, and Pi.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two QA follow-ups found while validating the release. Both are small, both touch the review feedback path, and neither belongs in the in-flight Cursor PR (#912), so they're bundled here.
1. Signal-safe cleanup (lifecycle)
apps/hook/server/index.tsregisters its background-PR-checkout cleanup (kill warmup children,git worktree remove/prune, session unregister) on the"exit"event — which does not fire on SIGINT (Ctrl-C) or SIGTERM. So aborting a review mid-checkout leaked clone/fetch children and stalegit worktreeregistrations.Fix: route
SIGINT/SIGTERMthroughprocess.exit()so the existing handlers run. Usesonce, so a second signal still force-quits if cleanup ever hangs. The change is scoped to the standalone hook binary only (OpenCode/Pi run in-host and must not callprocess.exit).2. Triage suffix now reaches PR-mode feedback
The review-denied suffix (
getReviewDeniedSuffix— "triage it, verify against the code, don't change anything until we discuss") was gated behind!isPRMode, so every PR review skipped it. That gate predates the suffix-unification work — the content was unified across runtimes, but the PR-mode exclusion was never revisited.Root cause:
isPRModewas a blunt proxy. In PR mode there are twoapproved:falseoutcomes:Fix: gate on
result.annotations.length > 0instead of!isPRMode. Genuine feedback always carries annotations; platform actions always send[]. Now the suffix is appended for PR and local feedback, and never onto a platform status message. Applied consistently across hook, OpenCode, and Pi.Verification
bun test— 1434 pass / 0 failbunx tsc -p ...)build:opencodecompilesisPRReviewlocal removed (now dead)🤖 Generated with Claude Code