diff --git a/.env.example b/.env.example index f571b1c1..22ff28e4 100644 --- a/.env.example +++ b/.env.example @@ -5,10 +5,17 @@ GITHUB_APP_ID=your_app_id GITHUB_PRIVATE_KEY="-----BEGIN RSA PRIVATE KEY-----\n...\n-----END RSA PRIVATE KEY-----" GITHUB_WEBHOOK_SECRET=your_webhook_secret +# Optional: the bot's own GitHub login(s), comma-separated. Override when the App is deployed under a +# different slug (its bot login is [bot]); keeps loop protection, /resolve, summary dedup, +# and follow-up tracking recognizing the bot's own activity. +#GITHUB_BOT_LOGINS=thrillhousebot[bot],thrillhouse-bot[bot] # Optional: webhook deduplication window for GitHub redeliveries #WEBHOOK_DEDUP_TTL=24h # Optional: comma-separated allowlist of logins permitted to trigger manual /review without repo access #THRILLHOUSEBOT_REVIEW_MANUAL_TRIGGER_ALLOWED_LOGINS=alice,bob +# Optional: upper bound on the manual /review write-access check (token mint + permission call) on the +# webhook ack thread; fails closed (denies the trigger) if GitHub is slower. +#MANUAL_TRIGGER_AUTH_TIMEOUT=5s # Optional: automatic review trigger filters. Defaults review every PR; a manual /review always runs. #WEBHOOK_SKIP_DRAFTS=true #WEBHOOK_REQUIRED_LABELS=ai-review diff --git a/CHANGELOG.md b/CHANGELOG.md index 22c8365c..c5636789 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,20 +2,21 @@ All notable changes to ThrillhouseBot. -## [Unreleased] +## [0.2.0] — 2026-06-21 This release makes the bot interactive and controllable from the PR — conversational replies, comment commands, context-aware labels, and configurable triggers — and hardens startup and the manual-review path. ### Added -- **Conversational replies**: a maintainer can now reply to one of the bot's review findings, or `@thrillhousebot` it anywhere in a PR thread, and the bot answers in context — pulling in the original finding, the surrounding diff, and the prior thread replies — instead of having to re-run the whole review. Replies are posted back into the same review thread (or as a PR comment for top-level mentions), gated to the same write-access/allowlisted users as a manual `/review`, and can be turned off with `REVIEW_CONVERSATIONAL_REPLIES_ENABLED=false`. Requires subscribing the GitHub App to the new `pull_request_review_comment` event (added to `manifest.json`) (#31) -- **Comment commands**: drive the bot from a PR with `/help`, `/summary`, `/resolve`, `/pause`, and `/resume` (each also accepts the `@Thrillhousebot ` mention form). `/pause` silences the bot on a PR — skipping automatic reviews and ignoring `/review` and `/summary` — until `/resume`; `/resolve` resolves the bot's open finding threads; `/summary` posts the PR summary if one was not generated yet. Every command except `/help` requires repository write access (#32) +- **Conversational replies**: a maintainer can `@thrillhousebot` anywhere in a PR thread — including as a reply to one of the bot's review findings — and the bot answers in context, pulling in the original finding, the surrounding diff, and the prior thread replies instead of having to re-run the whole review. An explicit `@`-mention is required; a bare reply on a thread (even the bot's own finding) does not pull it in. Replies are posted back into the same review thread (or as a PR comment for top-level mentions), gated to the same write-access/allowlisted users as a manual `/review`, and can be turned off with `REVIEW_CONVERSATIONAL_REPLIES_ENABLED=false`. Requires subscribing the GitHub App to the new `pull_request_review_comment` event (added to `manifest.json`) (#31, #202) +- **Comment commands**: drive the bot from a PR with `/help`, `/summary`, `/resolve`, `/pause`, and `/resume` (each also accepts the `@Thrillhousebot ` mention form). `/pause` silences the bot on a PR — skipping automatic reviews and conversational replies, and ignoring `/review` and `/summary` — until `/resume`; `/resolve` resolves the bot's open finding threads; `/summary` posts the PR summary if one was not generated yet. Every command except `/help` requires repository write access (#32) - **Context-aware PR labels** (opt-in): the model is shown the repository's existing labels and picks the few that best describe the change. Off by default (`REVIEW_LABELS_ENABLED`); when on, it either posts a one-line suggestion comment or applies the labels (`REVIEW_LABELS_APPLY`), with optional creation of new labels (`REVIEW_LABELS_ALLOW_CREATE`) and a per-PR cap (`REVIEW_LABELS_MAX`, default 3). Labelling is best-effort and never blocks a review (#61) - **Configurable review triggers**: narrow which pull requests are auto-reviewed — skip drafts (`WEBHOOK_SKIP_DRAFTS`), gate on labels (`WEBHOOK_REQUIRED_LABELS` / `WEBHOOK_EXCLUDED_LABELS`), and filter by base-branch glob (`WEBHOOK_BASE_BRANCHES` / `WEBHOOK_IGNORED_BASE_BRANCHES`); base-branch globs are gitignore-style, so `*` does not cross `/` — use `**` to span slashes (e.g. `dependabot/**`, or `**` alone for every branch). Defaults review every PR, matching prior behavior; a manual `/review` always bypasses the filters (#40) - **Review on ready-for-review**: a draft PR marked "Ready for review" is reviewed immediately, pairing with `WEBHOOK_SKIP_DRAFTS` so drafts can be skipped until they are ready (#72) - **Fail-fast configuration validation**: required configuration (`GITHUB_APP_ID`, `GITHUB_PRIVATE_KEY`, `GITHUB_WEBHOOK_SECRET`, `AI_API_KEY`) is validated at startup, and the app refuses to boot with a single message naming every missing or malformed value — including a non-numeric App id or a private key that is not valid PEM RSA — instead of failing later on the first webhook (#27) -- **Configurable bot identity**: the bot's own account login(s) are configurable via `GITHUB_BOT_LOGINS`, so loop protection and `/resolve` keep working when the App is deployed under a different slug (#165) +- **Configurable bot identity**: the bot's own account login(s) are configurable via `GITHUB_BOT_LOGINS`, so loop protection, `/resolve`, summary deduplication, and follow-up finding tracking all keep recognizing the bot's own activity when the App is deployed under a different slug (#165, #201) - **Reviewer flags single-page collection fetches**: the review prompt now has a pagination/truncation dimension, so a diff that lists a paginated collection (a GitHub REST endpoint or a GraphQL connection) and then consumes the result as if complete — searched, counted, iterated, or used to drive an action like `/resolve` — without walking every page is reported as a silent-truncation finding. The bot had been catching one such case while missing analogous REST and GraphQL ones (including in the same PR) because no dimension prompted the pattern; severity scales with what is dropped and confidence stays calibrated for the page-size assumption (#166) +- **Reviewer rejects refuted runtime-crash claims**: the review prompt now traces an alleged runtime failure (`NullPointerException`, index-out-of-bounds, and the like) from the enclosing method's entry down to the flagged line, and discards the finding when an in-diff guard makes that line unreachable for the claimed input — an earlier return/continue/throw, or a null/range check on a value derived from the flagged one. This removes a recurring class of confident false-positive crash findings the reviewer raised against code that already guards the condition (#112) ### Changed @@ -29,8 +30,9 @@ This release makes the bot interactive and controllable from the PR — conversa - **AI prompts dropped every context variable but the first**: each AI service (`PrReviewer`, `ReplyAssistant`, `FindingVerifier`) declared `@UserMessage` on a method *parameter*, which makes quarkus-langchain4j send only that parameter's raw value as the user message and never render the prompt template. So reviews ran on the diff alone — silently ignoring the repository instructions (`.github/thrillhousebot.md`), project stack, PR title/description, base comparison, related tests, and previous findings — the finding verifier audited candidates without the diff, and conversational replies saw only the maintainer's question with no diff, finding, or thread. Moved `@UserMessage` to the method so every `@V` variable is interpolated, and reduced `PromptTemplateEscaper` to marker-neutralization (its Qute unparsed-section wrapper was never stripped for data-bound values and corrupted any content containing `|}`). Added end-to-end and structural regression tests that pin the rendered prompt (#186) - **Reviewer corrupted the marker-handling code it was reviewing**: the prompt-injection defense rewrote the diff-section delimiters (`<<>>` / `<<>>`) found *inside* the diff, so whenever the bot reviewed code that legitimately contains those markers — the escaper, the prompt templates, and any PR that edits them — it saw altered source. That produced false "contradictory assertion"/no-op findings and silently degraded review accuracy of exactly those files. Replaced the fixed delimiters with a per-review unguessable random fence around the diff (the "random sequence enclosure"/spotlighting defense) and now pass the diff byte-exact; the small prose context slots keep the lightweight marker-neutralization as defense-in-depth (#187) - **Large PRs were silently truncated to 30 files**: `getPullRequestFiles` fetched only GitHub's default first page, so any PR with more than 30 changed files was reviewed — and described / changelog'd / replied to — on a partial diff, with no warning. It now paginates (100 files per page, bounded at 30 pages) so the whole diff is assembled before review (#190) -- **Duplicate PR summary on busy PRs**: the summary-dedup check (#183) fetched only the first page of issue comments, so on a PR with enough comments to push the bot's summary off page 1 the check missed it and re-posted a duplicate summary on the next review. `listComments` now paginates (100 comments per page, bounded at 10 pages) so the existing summary is found before deciding to post another - **False "undefined / missing symbol" findings when the definition is just outside the diff**: a finding could confidently flag a variable, env var, import, or config key as undefined/unset when its definition sat in the same file a few unchanged lines outside the diff hunk's context window — GitHub serves only ~3 lines of context, so the definition was never in the reviewed material (a CRITICAL false positive on `release.yml` in PR #88 claimed `NEXT`/`TAG` were undefined when the step's `env:` block defined them). The reviewer now treats an unseen definition as unconfirmed rather than absent, and the verifier rejects an "undefined / missing symbol" finding only when the scope its definition would occupy isn't shown in the material (an unverifiable claim) — a genuinely missing symbol that the diff *does* demonstrate (e.g. the diff removes the definition) still stands (#192) +- **Approval gating ignored ruleset-based branch protection**: CI-aware approval gating resolved the required status checks only from *classic* branch protection, so a repository that protects its base branch with a repository/organization **ruleset** (the modern mechanism) silently fell back to gating approvals on every check instead of the actual required set. Required contexts are now unioned from rulesets and classic protection both (#178) +- **Duplicate "no issues, but CI pending" message on a clean first review**: when a PR had no findings but a required check was still pending or failing, the bot posted the held-back notice twice — once in the PR summary's CI-status table and again as a separate COMMENT review restating it. The redundant COMMENT review is now skipped when a first review is held back solely by CI; an unresolved prior finding, a follow-up review, or a `REQUEST_CHANGES` verdict still posts it (#175) ### Documentation diff --git a/README.md b/README.md index e324ef40..57e8f53f 100644 --- a/README.md +++ b/README.md @@ -80,9 +80,10 @@ the repository (or to be named in `THRILLHOUSEBOT_REVIEW_MANUAL_TRIGGER_ALLOWED_LOGINS`), since reviews spend the operator's AI budget. -**Pause** — while a PR is paused, ThrillhouseBot skips automatic reviews on new commits and -ignores `/review` and `/summary` (it replies once to say it is paused). `/resume` lifts the -pause. `/help` and `/resolve` keep working while paused. +**Pause** — while a PR is paused, ThrillhouseBot skips automatic reviews on new commits, +ignores `/review` and `/summary`, and does not answer `@thrillhousebot` mentions (it replies +once to say it is paused). `/resume` lifts the pause. `/help` and `/resolve` keep working while +paused. ## Quick start @@ -162,7 +163,7 @@ Create a GitHub App before starting the bot; you'll need its credentials for `.e | Webhook URL | `https:///api/webhook` | | Webhook Secret | Random string | | Repository Permissions | Pull Requests: R/W, Checks: R/W, Contents: Read, Issues: R/W, Actions: Read | -| Subscribe to Events | Pull Request, Issue comment | +| Subscribe to Events | Pull Request, Issue comment, Pull request review comment | | Identifying & authorizing users | Enabled (for dashboard login) | | Callback URL | `https:///api/auth/callback` | @@ -314,8 +315,9 @@ This is still an early-stage project; the current constraints are: - **GitHub only** — no GitLab or Bitbucket integration. - **Large diffs** — the model sees at most `thrillhousebot.review.max-diff-lines` diff - lines (default 5000). Later files are dropped; the last included file may be cut - mid-hunk. + lines (default 5000). Later files are dropped; the last included file is cut at a hunk + boundary with its code fence re-closed (only cut mid-hunk if a single hunk alone exceeds + the budget). - **Single process** — OAuth login sessions and the live WebSocket replay buffer are in-memory (lost on restart). Review history and cost totals persist in PostgreSQL. Multiple replicas are unsupported. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8bc9a7a0..d95dc122 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -171,12 +171,12 @@ sequenceDiagram | Package | Responsibility | Notable classes | |---|---|---| -| `webhook/` | Receives GitHub events, verifies the HMAC signature, decides whether an event triggers a review | `WebhookController`, `WebhookVerifier`, `TriggerDetector` | +| `webhook/` | Receives GitHub events, verifies the HMAC signature, decides whether an event triggers a review (trigger filters, per-PR pause state), and runs the comment commands (`/help`, `/summary`, `/resolve`, `/pause`, `/resume`) | `WebhookController`, `WebhookVerifier`, `TriggerDetector`, `ReviewTriggerFilter`, `CommentCommandService`, `PrPauseService` | | `review/` | Orchestrates a review: formats the diff, calls the AI layer, maps findings to a risk level and review state, writes the summary comment, optionally labels the PR, and answers maintainer replies/mentions in PR threads | `ReviewOrchestrator`, `ReviewDispatcher`, `ReviewDiffFormatter`, `FollowUpAnalyzer`, `PrSummaryGenerator`, `PrLabeler`, `MaintainerReplyService`, `MaintainerReplyDispatcher` | | `review/ai/` | The LangChain4j layer: streams the model response, parses it into findings, runs a second pass to verify them, and writes conversational replies | `PrReviewer`, `AiReviewService`, `FindingVerifier`, `FindingVerificationService`, `ReviewResponseParser`, `ReplyAssistant` | | `github/` | Talks to the GitHub REST and GraphQL APIs: app auth, pull requests, reviews, check runs, comments, labels, and reading the repo instructions file | `GitHubAuthClient`, `GitHubReviewClient`, `GitHubCheckRunClient`, `GitHubLabelClient`, `InstructionsResolver` | | `dashboard/` | The live UI backend: OAuth login (in-memory sessions), WebSocket broadcaster, and review session persistence | `AuthResource`, `DashboardSessionStore`, `SessionEventBroadcaster`, `ReviewSessionRepository` | -| `config/` | Wiring: the outbound HTTP client, the review thread pool, and typed config | `HttpClientProducer`, `ReviewExecutorProducer`, `ThrillhouseConfig` | +| `config/` | Wiring: the outbound HTTP client, the review thread pool, typed config, fail-fast startup validation of required configuration, and the shared bot-identity used to recognize the bot's own activity | `HttpClientProducer`, `ReviewExecutorProducer`, `ThrillhouseConfig`, `StartupConfigValidator`, `BotIdentity` | | `frontend/` | The Next.js dashboard, built to a static export and served by Quarkus | — | ## Notes diff --git a/pom.xml b/pom.xml index 6bb437ff..5cf9c87a 100644 --- a/pom.xml +++ b/pom.xml @@ -8,7 +8,7 @@ dev.thiagogonzaga.thrillhousebot thrillhousebot - 0.2.0-SNAPSHOT + 0.2.0 3.15.0