From 1f8e33b7e25e04af9b035349214c8896b347c2c2 Mon Sep 17 00:00:00 2001 From: Lince Mathew Date: Tue, 13 Oct 2026 01:58:45 +0530 Subject: [PATCH 01/17] Add Pull Request Review Links and Automated Bot Comments LiveReview Pre-Commit Check: ran (iter:1, coverage:0%) --- ...nks_comment_posted_at_to_pull_requests.sql | 9 + ..._review_commits_repository_fk_set_null.sql | 13 + db/schema.sql | 7 +- docs/pr-review-links-plan.md | 145 +++++++++++ internal/api/connectors.go | 19 +- internal/api/pull_request_review_trigger.go | 71 ++++++ internal/api/server.go | 18 ++ .../routes_guide/reviews/pr-review-link.md | 47 ++++ .../docs/routes_guide/reviews/reviews-list.md | 5 + internal/jobqueue/jobqueue.go | 4 +- internal/jobqueue/pr_policy_worker.go | 228 ++++++++++++++++++ internal/jobqueue/repo_sync_worker.go | 71 +++++- internal/jobqueue/review_worker.go | 8 +- internal/providers/github/pr_comment.go | 54 +++++ internal/providers/gitlab/gitlab_comment.go | 22 ++ ui/src/App.tsx | 2 + ui/src/api/pullRequests.ts | 19 ++ ui/src/api/reviews.ts | 1 + ui/src/hooks/useSwitchToPullRequestOrg.ts | 22 ++ .../Reviews/PullRequestReviewTrigger.tsx | 58 +++++ ui/src/pages/Reviews/Reviews.tsx | 52 +++- ui/src/types/explore.ts | 1 + ui/src/types/reviews.ts | 2 + 23 files changed, 860 insertions(+), 18 deletions(-) create mode 100644 db/migrations/20261012090000_add_links_comment_posted_at_to_pull_requests.sql create mode 100644 db/migrations/20261012100000_review_commits_repository_fk_set_null.sql create mode 100644 docs/pr-review-links-plan.md create mode 100644 internal/docindex/docs/routes_guide/reviews/pr-review-link.md create mode 100644 internal/jobqueue/pr_policy_worker.go create mode 100644 internal/providers/github/pr_comment.go create mode 100644 ui/src/hooks/useSwitchToPullRequestOrg.ts create mode 100644 ui/src/pages/Reviews/PullRequestReviewTrigger.tsx diff --git a/db/migrations/20261012090000_add_links_comment_posted_at_to_pull_requests.sql b/db/migrations/20261012090000_add_links_comment_posted_at_to_pull_requests.sql new file mode 100644 index 000000000..d9685e148 --- /dev/null +++ b/db/migrations/20261012090000_add_links_comment_posted_at_to_pull_requests.sql @@ -0,0 +1,9 @@ +-- migrate:up + +-- Set once the "Start a review / View history" links comment is posted on a new PR/MR, +-- so duplicate webhook deliveries don't post it twice. +ALTER TABLE pull_requests ADD COLUMN links_comment_posted_at TIMESTAMPTZ; + +-- migrate:down + +ALTER TABLE pull_requests DROP COLUMN IF EXISTS links_comment_posted_at; diff --git a/db/migrations/20261012100000_review_commits_repository_fk_set_null.sql b/db/migrations/20261012100000_review_commits_repository_fk_set_null.sql new file mode 100644 index 000000000..cf681934b --- /dev/null +++ b/db/migrations/20261012100000_review_commits_repository_fk_set_null.sql @@ -0,0 +1,13 @@ +-- migrate:up + +-- Deleting a connector cascades to its repositories; review_commits rows blocked that. +-- Keep the review's commit history and just clear the repository link instead. +ALTER TABLE review_commits DROP CONSTRAINT review_commits_repository_id_fkey; +ALTER TABLE review_commits ADD CONSTRAINT review_commits_repository_id_fkey + FOREIGN KEY (repository_id) REFERENCES repositories(id) ON DELETE SET NULL; + +-- migrate:down + +ALTER TABLE review_commits DROP CONSTRAINT review_commits_repository_id_fkey; +ALTER TABLE review_commits ADD CONSTRAINT review_commits_repository_id_fkey + FOREIGN KEY (repository_id) REFERENCES repositories(id); diff --git a/db/schema.sql b/db/schema.sql index 09d7a0e8e..3310934cd 100644 --- a/db/schema.sql +++ b/db/schema.sql @@ -1652,6 +1652,7 @@ CREATE TABLE public.pull_requests ( metadata jsonb DEFAULT '{}'::jsonb NOT NULL, created_at timestamp with time zone DEFAULT now() NOT NULL, updated_at timestamp with time zone DEFAULT now() NOT NULL, + links_comment_posted_at timestamp with time zone, CONSTRAINT pull_requests_last_synced_source_check CHECK ((last_synced_source = ANY (ARRAY['webhook'::text, 'poll'::text, 'backfill'::text]))), CONSTRAINT pull_requests_state_check CHECK ((state = ANY (ARRAY['open'::text, 'closed'::text, 'merged'::text]))) ); @@ -6081,7 +6082,7 @@ ALTER TABLE ONLY public.review_commits -- ALTER TABLE ONLY public.review_commits - ADD CONSTRAINT review_commits_repository_id_fkey FOREIGN KEY (repository_id) REFERENCES public.repositories(id); + ADD CONSTRAINT review_commits_repository_id_fkey FOREIGN KEY (repository_id) REFERENCES public.repositories(id) ON DELETE SET NULL; -- @@ -6548,4 +6549,6 @@ INSERT INTO public.schema_migrations (version) VALUES ('20260824190000'), ('20260825140000'), ('20260825190000'), - ('20260905090000'); + ('20260905090000'), + ('20261012090000'), + ('20261012100000'); diff --git a/docs/pr-review-links-plan.md b/docs/pr-review-links-plan.md new file mode 100644 index 000000000..17207d6f9 --- /dev/null +++ b/docs/pr-review-links-plan.md @@ -0,0 +1,145 @@ +# PR Review Links β€” Implementation Plan + +## Goal + +When a new PR/MR is opened, LiveReview posts one bot comment on it with two links: + +1. **Start a review** β€” opens LiveReview, triggers a review for that PR, and redirects to the + review details page. +2. **View review history** β€” opens the Reviews list filtered to that PR's reviews. + +Policy-based auto-approval now has a **static stand-in**: reviews started from the link +are checked against "no critical findings" by the `pr_policy_check` job +(`internal/jobqueue/pr_policy_worker.go`), which posts a policy comment and approves the +PR, or requests changes (GitHub) / comments only (GitLab). Org-defined policies (likely +built on CI/CD Gates jq rulesets) remain a later feature. + +## Decisions + +| Topic | Decision | +|---|---| +| Providers | GitHub and GitLab only (first phase) | +| Which repos | Every repo with the LiveReview webhook installed | +| Which PRs | Only newly opened PRs (GitHub action `opened`, GitLab action `open`). Existing open PRs, reopen, push, and edit events are ignored. | +| Where links go | One bot comment on the PR, posted once and never edited | +| Duplicate webhooks | Guarded by a "comment posted" flag on the `pull_requests` row | +| Auth | Both links require a LiveReview login. Logged-out users log in and are sent back to the link (existing `redirectAfterLogin` flow). | +| Org | The user is switched to the PR's org automatically. Non-members get "not found". | +| Trigger behavior | Opening the trigger link starts the review immediately (no confirm page, for now) | +| Repeat clicks | Each click starts a new review, **unless** a review for the PR is `created`/`in_progress` and started less than 10 minutes ago. Then it redirects to that review. | +| Existing triggers | Unchanged (reviewer assignment, @mentions) | +| History view | The existing Reviews list (`/reviews`) filtered by `pull_request_id` | + +## Link format + +The UI uses hash routing, so links look like: + +``` +/#/pr//review +/#/reviews?pull_request_id= +``` + +- `` is LiveReview's internal `pull_requests.id` (primary key), **not** the + provider PR number. It is unique across providers, repos, and orgs, so a GitHub #123 and a + GitLab !123 never collide. +- `` comes from the `livereview_prod_url` setting (the same public URL webhooks are + installed against). If it is not set, the comment is skipped with a logged warning. + +## Comment text + +``` +πŸ” LiveReview +- β–Ά Start a review: /#/pr//review +- πŸ“œ View review history: /#/reviews?pull_request_id= +``` + +## Backend changes + +### 1. Migration β€” `db/migrations/_add_links_comment_posted_at_to_pull_requests.sql` + +- Add `links_comment_posted_at TIMESTAMPTZ` (nullable) to `pull_requests`. +- Regenerate `db/schema.sql`. + +### 2. Post the comment β€” `internal/jobqueue/repo_sync_worker.go` (`PRStateSyncWorker.Work`) + +After `UpsertPullRequest` returns the PR row id: + +1. Skip unless `event.Metadata["action"]` is `opened` (GitHub) or `open` (GitLab). +2. Claim the post atomically: + `UPDATE pull_requests SET links_comment_posted_at = now() WHERE id = $1 AND links_comment_posted_at IS NULL`. + If no row changed, the comment was already posted β€” skip. +3. Load the repo token via the existing `repositoryConnectorInfo` lookup. +4. Read the public URL via the webhook store's `GetWebhookPublicEndpoint` (inject the store + into the worker in `jobqueue.go`). +5. Post the comment. On failure, reset `links_comment_posted_at` to `NULL` and return the + error so River retries. + +### 3. Comment helpers + +- `internal/providers/github/pr_comment.go` β€” `PostIssueComment(ctx, baseURL, token, ownerRepo, number, body)`, + using the existing `apiBaseURL` (works for GitHub Enterprise). +- GitLab: reuses the existing `NewHTTPClient(baseURL, token).CreateMRGeneralComment(projectID, iid, body)`. + +Not reusing `GitHubProvider.PostComment`: it hardcodes `api.github.com` and formats text as a +review finding. + +### 4. Org lookup β€” `GET /api/v1/pull-requests/:id/org` + +- Auth only (`RequireAuth`); the frontend doesn't know the PR's org yet, so the org + middleware chain can't be used here (approved exception). +- Handler looks up `pull_requests.org_id` and verifies the user is an active member of that + org. Returns `{ "org_id": ... }`, or **404** if the PR doesn't exist or the user isn't a + member (no existence leak). +- Returns nothing besides the org id. + +### 5. Reuse ongoing review β€” `internal/api/pull_request_review_trigger.go` (`createReview`) + +- Add optional body field `reuse_ongoing` (bool, default false). +- When true: if a review exists for this `pull_request_id` + `org_id` with status + `created`/`in_progress` and `created_at > now() - interval '10 minutes'`, return its id + without creating a new review. +- Existing callers don't send the field, so their behavior is unchanged. +- The actual review still goes through `POST /api/v1/reviews` with the full middleware chain + and LOC quota preflight. + +### 6. Reviews list filter β€” `getReviews` in `internal/api/server.go` + +- Add optional `pull_request_id` query param β†’ `AND pull_request_id = $n` (still scoped by + `org_id`). +- Add the field to the `ReviewsQuery` schema. + +## Frontend changes + +### 7. Trigger page β€” `ui/src/pages/Reviews/PullRequestReviewTrigger.tsx` + +Route `/pr/:id/review` in `ui/src/App.tsx`. On load: + +1. `GET /api/v1/pull-requests/:id/org`. +2. If the org differs from the current one, `switchToOrg(org_id)`. +3. `POST /api/v1/reviews { pull_request_id, reuse_ongoing: true }`. +4. Redirect (replace) to `/reviews/:reviewId`. + +States: loading spinner, "PR not found or you don't have access" (404), quota-exceeded +message. Use a ref guard so React StrictMode doesn't fire the POST twice in dev. + +### 8. Reviews list β€” `ui/src/pages/Reviews/Reviews.tsx`, `ui/src/api/reviews.ts` + +- Read `pull_request_id` from the URL and pass it to the reviews API. +- Show a small "Filtered to one pull request Β· Clear" chip. +- Like the trigger page, switch to the PR's org first (shared hook + `ui/src/hooks/useSwitchToPullRequestOrg.ts`), so multi-org users don't land on an empty list. + +## Docs & navigation + +- **Routes guide:** add `internal/docindex/docs/routes_guide/reviews/pr-review-link.md`, and + update the Reviews list doc for the `pull_request_id` filter. +- **Mega menu:** no entry. The trigger page is only reached from the PR comment, and the + filter is an option on the existing Reviews page. + +## Known limitations + +- Opening the trigger link starts a review, so chat/email link previewers (for a logged-in + user) could start reviews. The 10-minute ongoing-review reuse limits this. Revisit with a + confirm page if it becomes a problem. +- Only GitHub and GitLab. Bitbucket, Gitea, and Azure DevOps don't sync PR state into + `pull_requests` yet. diff --git a/internal/api/connectors.go b/internal/api/connectors.go index d66d6a8e2..5d96fe50f 100644 --- a/internal/api/connectors.go +++ b/internal/api/connectors.go @@ -506,7 +506,9 @@ func (s *Server) DeleteConnector(c echo.Context) error { }) } - // Fetch projects to queue webhook removal + // Look up projects now (needs the connector row), but only queue webhook removal + // after the delete succeeds, so a failed delete leaves webhooks untouched. + var projectsToUnhook []string repositoryData, err := s.fetchAndCacheRepositoryData(id, false, false) if err != nil { log.Printf("Failed to fetch repository data for connector %d during deletion: %v", id, err) @@ -515,13 +517,7 @@ func (s *Server) DeleteConnector(c echo.Context) error { log.Printf("Repository access error during deletion of connector %d: %s", id, repositoryData.Error) // Again, proceed to delete the connector } else { - // Queue webhook removal jobs for each project - ctx := c.Request().Context() - for _, projectPath := range repositoryData.Projects { - if err := s.jobQueue.QueueWebhookRemovalJob(ctx, id, projectPath, provider, providerURL, patToken, true); err != nil { - log.Printf("Failed to queue webhook removal job for %s: %v", projectPath, err) - } - } + projectsToUnhook = repositoryData.Projects } // Execute the delete query @@ -552,6 +548,13 @@ func (s *Server) DeleteConnector(c echo.Context) error { }) } + ctx := c.Request().Context() + for _, projectPath := range projectsToUnhook { + if err := s.jobQueue.QueueWebhookRemovalJob(ctx, id, projectPath, provider, providerURL, patToken, true); err != nil { + log.Printf("Failed to queue webhook removal job for %s: %v", projectPath, err) + } + } + // Drop this connector's cached blast-radius clones; best-effort, in the background. go repocache.RemoveScope(repocache.DefaultRoot, connectorOrgID, int64(id)) diff --git a/internal/api/pull_request_review_trigger.go b/internal/api/pull_request_review_trigger.go index d707b7cd2..a1015647d 100644 --- a/internal/api/pull_request_review_trigger.go +++ b/internal/api/pull_request_review_trigger.go @@ -5,8 +5,11 @@ import ( "fmt" "log" "net/http" + "strconv" "github.com/labstack/echo/v4" + "github.com/livereview/internal/api/auth" + "github.com/livereview/pkg/models" ) // CreateReviewRequest is the body of POST /api/v1/reviews - trigger a new @@ -14,6 +17,9 @@ import ( // POST /connectors/trigger-review's raw-URL flow. type CreateReviewRequest struct { PullRequestID int64 `json:"pull_request_id"` + // FromPRLink is set by the PR comment's "Start a review" link: reuse a review still + // running from the last 10 minutes, and run the approval policy check when it finishes. + FromPRLink bool `json:"from_pr_link"` } // createReview handles POST /api/v1/reviews. It resolves the PR/MR's web_url @@ -50,6 +56,30 @@ func (s *Server) createReview(c echo.Context) error { return c.JSON(http.StatusInternalServerError, ErrorResponse{Error: fmt.Sprintf("failed to look up pull request: %v", err)}) } + if req.FromPRLink { + var ongoingID int64 + err := s.db.QueryRow( + `SELECT id FROM reviews + WHERE pull_request_id = $1 AND org_id = $2 + AND status IN ('created', 'in_progress') + AND created_at > now() - interval '10 minutes' + ORDER BY created_at DESC LIMIT 1`, + req.PullRequestID, orgID, + ).Scan(&ongoingID) + if err == nil { + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{ + "message": "Review already in progress", + "url": webURL, + "reviewId": fmt.Sprintf("%d", ongoingID), + "pull_request_id": req.PullRequestID, + "reused": true, + }) + } + if err != sql.ErrNoRows { + return c.JSON(http.StatusInternalServerError, ErrorResponse{Error: fmt.Sprintf("failed to check ongoing review: %v", err)}) + } + } + // LOC Quota preflight check β€” block before creating any DB records, same // as TriggerReviewV2. if blocked, pfErr := s.preflightLOCQuota(c); blocked { @@ -68,6 +98,15 @@ func (s *Server) createReview(c echo.Context) error { return c.JSON(http.StatusInternalServerError, ErrorResponse{Error: err.Error()}) } s.enrichMetadata(ctx) + if req.FromPRLink { + // Marks the review so the worker runs the approval policy check once it completes. + if _, err := s.db.Exec( + `UPDATE reviews SET metadata = COALESCE(metadata, '{}'::jsonb) || '{"pr_link_policy": true}'::jsonb WHERE id = $1 AND org_id = $2`, + ctx.review.ID, orgID, + ); err != nil { + log.Printf("[WARN] createReview: failed to mark review %d for policy check: %v", ctx.review.ID, err) + } + } s.trackActivity(ctx) if err := s.launchBackgroundProcessing(ctx); err != nil { log.Printf("[ERROR] createReview: Failed to queue manual review: %v", err) @@ -81,3 +120,35 @@ func (s *Server) createReview(c echo.Context) error { "pull_request_id": req.PullRequestID, }) } + +// getPullRequestOrg handles GET /api/v1/pull-requests/:id/org for the PR comment's +// "Start a review" link, which doesn't know the PR's org yet. Auth-only route, so it +// checks membership itself and returns 404 for both "no such PR" and "not a member". +func (s *Server) getPullRequestOrg(c echo.Context) error { + user, ok := c.Get(string(auth.UserContextKey)).(*models.User) + if !ok || user == nil { + return c.JSON(http.StatusUnauthorized, ErrorResponse{Error: "authentication required"}) + } + prID, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil || prID <= 0 { + return c.JSON(http.StatusBadRequest, ErrorResponse{Error: "invalid pull request id"}) + } + + // Same access rule as ValidateOrgAccess: a role in the org, or super admin. + var orgID int64 + err = s.db.QueryRow( + `SELECT pr.org_id FROM pull_requests pr + WHERE pr.id = $1 AND EXISTS ( + SELECT 1 FROM user_roles ur JOIN roles r ON ur.role_id = r.id + WHERE ur.user_id = $2 AND (ur.org_id = pr.org_id OR r.name = $3) + )`, + prID, user.ID, auth.RoleSuperAdmin, + ).Scan(&orgID) + if err == sql.ErrNoRows { + return c.JSON(http.StatusNotFound, ErrorResponse{Error: "pull request not found"}) + } + if err != nil { + return c.JSON(http.StatusInternalServerError, ErrorResponse{Error: "failed to look up pull request"}) + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"org_id": orgID}) +} diff --git a/internal/api/server.go b/internal/api/server.go index b6508b33a..a6634c1c5 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -1509,6 +1509,10 @@ func (s *Server) setupRoutes() { pullRequestsGroup.Use(apimiddleware.BuildPlanContext()) pullRequestsGroup.GET("", s.ListPullRequests) + // Resolves a PR's org for the PR comment's "Start a review" link; the org isn't known + // yet, so this is auth-only and the handler checks org membership itself. + v1.GET("/pull-requests/:id/org", s.getPullRequestOrg, authMiddleware.RequireAuth()) + // GitLab profile validation endpoint v1.POST("/gitlab/validate-profile", s.ValidateGitLabProfile) @@ -2160,6 +2164,8 @@ type ReviewsQuery struct { Order string `form:"order" query:"order" json:"order,omitempty" jsonschema:"description=Sort direction: asc or desc (default desc)"` DateFrom string `form:"date_from" query:"date_from" json:"date_from,omitempty" jsonschema:"description=Only include reviews with last activity (completed/started/created) on or after this date. Accepts YYYY-MM-DD or RFC3339"` DateTo string `form:"date_to" query:"date_to" json:"date_to,omitempty" jsonschema:"description=Only include reviews with last activity (completed/started/created) on or before this date (inclusive of the whole day if bare YYYY-MM-DD). Accepts YYYY-MM-DD or RFC3339"` + + PullRequestID int64 `form:"pull_request_id" query:"pull_request_id" json:"pull_request_id,omitempty" jsonschema:"description=Only include reviews of this pull/merge request (internal pull_requests.id)"` } type ReviewsListResponse struct { @@ -2335,6 +2341,18 @@ func (s *Server) getReviews(c echo.Context) error { countQuery += clause } + if v := c.QueryParam("pull_request_id"); v != "" { + prID, err := strconv.ParseInt(v, 10, 64) + if err != nil { + return echo.NewHTTPError(http.StatusBadRequest, "Invalid pull_request_id") + } + clause := fmt.Sprintf(" AND pull_request_id = $%d", argIndex) + baseQuery += clause + countQuery += clause + args = append(args, prID) + argIndex++ + } + // Add search functionality search := c.QueryParam("search") if search != "" { diff --git a/internal/docindex/docs/routes_guide/reviews/pr-review-link.md b/internal/docindex/docs/routes_guide/reviews/pr-review-link.md new file mode 100644 index 000000000..1ce989283 --- /dev/null +++ b/internal/docindex/docs/routes_guide/reviews/pr-review-link.md @@ -0,0 +1,47 @@ +# PR Review Link (start a review from a PR comment) + +**Route:** `/pr/:id/review` +**Component:** `ui/src/pages/Reviews/PullRequestReviewTrigger.tsx` + +## Purpose + +When a new pull/merge request is opened on GitHub or GitLab in a repo with the +LiveReview webhook installed, LiveReview posts one comment on it with two links: + +- **Start a review** β†’ this page. It starts a LiveReview review of that PR and + opens the review's detail page. +- **View review history** β†’ the [Reviews list](reviews-list.md) filtered to that + PR (`/reviews?pull_request_id=`). + +`:id` is LiveReview's internal pull request id, not the GitHub/GitLab PR number. +The comment is posted once per PR, only when it's opened (not for PRs that +already existed, reopens, or pushes). + +## Who can access it + +Any authenticated member of the org the PR belongs to. Logged-out users are +asked to log in and then sent back to the link. If the user's active org is a +different one, LiveReview switches to the PR's org automatically. Users who +aren't members of that org see "pull request wasn't found". + +## Key actions + +- Opening the link starts a review right away; there is no confirm step. +- If a review of the same PR started less than 10 minutes ago and is still + running, the link opens that review instead of starting another one. +- Each later click starts a new review (counts against the org's review quota). +- When a review started from this link finishes, LiveReview checks it against a + fixed approval policy: **no critical findings**. It posts a policy comment on + the PR (counts of critical/warning/info findings, plus up to 5 critical + findings) and then: + - GitHub: approves the PR, or submits "Request changes" if it failed. If + GitHub refuses (e.g. the bot account authored the PR), only the comment is + posted. + - GitLab: approves the MR when it passes; a failure is comment-only. +- Reviews started any other way (reviewer assignment, @mentions, Explore) don't + run the policy check. + +## Related pages + +- [Reviews (list)](reviews-list.md) +- [Review Detail](review-detail.md) diff --git a/internal/docindex/docs/routes_guide/reviews/reviews-list.md b/internal/docindex/docs/routes_guide/reviews/reviews-list.md index 47debe750..54adc949e 100644 --- a/internal/docindex/docs/routes_guide/reviews/reviews-list.md +++ b/internal/docindex/docs/routes_guide/reviews/reviews-list.md @@ -20,10 +20,15 @@ Any authenticated org member. - Click a row to open [Review Detail](review-detail.md). - Start a new review via "New Review" (`/reviews/new`). - Navigate to scheduled reviews (`/reviews/scheduled`). +- View one pull request's review history: `/reviews?pull_request_id=` + (the "View review history" link LiveReview comments on new GitHub/GitLab + PRs). Switches to the PR's org if needed. A "Filtered to one pull request" + chip shows above the table; click **Clear** to see all reviews again. ## Related pages - [Review Detail](review-detail.md) +- [PR Review Link](pr-review-link.md) - [New Review](new-review.md) - [Scheduled Reviews](scheduled-reviews.md) - [CI/CD Gates](ci-cd-gates.md) diff --git a/internal/jobqueue/jobqueue.go b/internal/jobqueue/jobqueue.go index 9d8779c23..574166596 100644 --- a/internal/jobqueue/jobqueue.go +++ b/internal/jobqueue/jobqueue.go @@ -2418,7 +2418,8 @@ func NewJobQueue(databaseURL string, db *sql.DB) (*JobQueue, error) { diffWorker := &DiffReviewWorker{db: db, pool: pool} prStore := pullrequests.NewStore(db) repoPRSyncWorker := &RepoPRSyncWorker{db: db, store: prStore} - prStateSyncWorker := &PRStateSyncWorker{db: db, store: prStore} + prStateSyncWorker := &PRStateSyncWorker{db: db, store: prStore, webhookStore: store} + prPolicyCheckWorker := &PRPolicyCheckWorker{db: db, webhookStore: store} reconciliationWorker := &ReconciliationSweepWorker{db: db, pool: pool, stalenessThreshold: config.RepoSyncConfig.StalenessThreshold} scheduledReviewWorker := &ScheduledReviewWorker{db: db} preloadedChangesArchivalWorker := &PreloadedChangesArchivalWorker{db: db} @@ -2434,6 +2435,7 @@ func NewJobQueue(databaseURL string, db *sql.DB) (*JobQueue, error) { river.AddWorker(workers, &UpdateOrgUsageWorker{db: db, pool: pool}) river.AddWorker(workers, repoPRSyncWorker) river.AddWorker(workers, prStateSyncWorker) + river.AddWorker(workers, prPolicyCheckWorker) river.AddWorker(workers, reconciliationWorker) river.AddWorker(workers, preloadedChangesArchivalWorker) river.AddWorker(workers, preloadedChangesArchivalSweepWorker) diff --git a/internal/jobqueue/pr_policy_worker.go b/internal/jobqueue/pr_policy_worker.go new file mode 100644 index 000000000..a5a86e68b --- /dev/null +++ b/internal/jobqueue/pr_policy_worker.go @@ -0,0 +1,228 @@ +package jobqueue + +import ( + "context" + "database/sql" + "encoding/json" + "fmt" + "log" + "strings" + + providergithub "github.com/livereview/internal/providers/github" + providergitlab "github.com/livereview/internal/providers/gitlab" + storagejobqueue "github.com/livereview/storage/jobqueue" + "github.com/riverqueue/river" +) + +// PRPolicyCheckJobArgs runs the approval policy on a finished review that was started +// from the PR comment's "Start a review" link (metadata.pr_link_policy). +type PRPolicyCheckJobArgs struct { + OrgID int64 `json:"org_id"` + ReviewID int64 `json:"review_id"` +} + +func (PRPolicyCheckJobArgs) Kind() string { return "pr_policy_check" } + +type PRPolicyCheckWorker struct { + river.WorkerDefaults[PRPolicyCheckJobArgs] + db *sql.DB + webhookStore *storagejobqueue.WebhookStore +} + +// policyFinding is the subset of models.ReviewComment (stored without JSON tags) the policy reads. +type policyFinding struct { + FilePath string + Line int + Content string + Severity string + IsInternal bool +} + +type policyResult struct { + Policy string `json:"policy"` + Approved bool `json:"approved"` + Critical int `json:"critical"` + Warning int `json:"warning"` + Info int `json:"info"` +} + +// ponytail: one static policy (no critical findings); swap for org-defined policies later. +const staticPolicyName = "No critical findings" + +func (w *PRPolicyCheckWorker) Work(ctx context.Context, job *river.Job[PRPolicyCheckJobArgs]) error { + args := job.Args + + var status string + var pullRequestID sql.NullInt64 + var flagged bool + var commentsJSON []byte + err := w.db.QueryRowContext(ctx, ` + SELECT status, pull_request_id, + COALESCE((metadata->>'pr_link_policy')::boolean, false), + COALESCE(metadata->'review_result'->'comments', '[]'::jsonb) + FROM reviews WHERE id = $1 AND org_id = $2`, + args.ReviewID, args.OrgID, + ).Scan(&status, &pullRequestID, &flagged, &commentsJSON) + if err == sql.ErrNoRows { + return nil + } + if err != nil { + return fmt.Errorf("failed to load review %d: %w", args.ReviewID, err) + } + if status != "completed" || !pullRequestID.Valid || !flagged { + return nil + } + + var findings []policyFinding + if err := json.Unmarshal(commentsJSON, &findings); err != nil { + return fmt.Errorf("failed to parse findings for review %d: %w", args.ReviewID, err) + } + result := policyResult{Policy: staticPolicyName} + var critical []policyFinding + for _, f := range findings { + if f.IsInternal { + continue + } + switch strings.ToLower(f.Severity) { + case "critical": + result.Critical++ + critical = append(critical, f) + case "warning": + result.Warning++ + default: + result.Info++ + } + } + result.Approved = result.Critical == 0 + + // Store the result first; it doubles as a claim so River retries never approve twice. + resultJSON, _ := json.Marshal(result) + res, err := w.db.ExecContext(ctx, + `UPDATE reviews SET metadata = metadata || jsonb_build_object('policy_result', $2::jsonb) + WHERE id = $1 AND NOT (metadata ? 'policy_result')`, + args.ReviewID, string(resultJSON)) + if err != nil { + return fmt.Errorf("failed to store policy result for review %d: %w", args.ReviewID, err) + } + if n, _ := res.RowsAffected(); n == 0 { + return nil + } + + if err := w.publish(ctx, args, pullRequestID.Int64, result, critical); err != nil { + if _, resetErr := w.db.ExecContext(ctx, `UPDATE reviews SET metadata = metadata - 'policy_result' WHERE id = $1`, args.ReviewID); resetErr != nil { + log.Printf("[WARN] pr policy: failed to release claim for review %d: %v", args.ReviewID, resetErr) + } + return err + } + return nil +} + +// publish posts the policy comment and approves (or requests changes on) the PR/MR. +func (w *PRPolicyCheckWorker) publish(ctx context.Context, args PRPolicyCheckJobArgs, pullRequestID int64, result policyResult, critical []policyFinding) error { + var repositoryID int64 + var number int + if err := w.db.QueryRowContext(ctx, + `SELECT repository_id, number FROM pull_requests WHERE id = $1 AND org_id = $2`, + pullRequestID, args.OrgID, + ).Scan(&repositoryID, &number); err != nil { + return fmt.Errorf("failed to load pull request %d: %w", pullRequestID, err) + } + info, err := loadRepositoryConnectorInfo(w.db, repositoryID) + if err != nil { + return fmt.Errorf("failed to load connector for repository %d: %w", repositoryID, err) + } + host, err := w.webhookStore.GetWebhookPublicEndpoint(ctx) + if err != nil { + log.Printf("[WARN] pr policy: public URL not configured, posting without review link: %v", err) + } + body := buildPolicyComment(host, args.ReviewID, result, critical) + + switch info.Provider { + case "github", "github-com", "github-enterprise": + event := "REQUEST_CHANGES" + if result.Approved { + event = "APPROVE" + } + if err := providergithub.SubmitPullRequestReview(ctx, info.ProviderURL, info.token(), info.FullName, number, event, body); err != nil { + // GitHub refuses approve/request-changes when the bot authored the PR; still show the result. + log.Printf("[WARN] pr policy: %s failed for review %d, posting as comment: %v", event, args.ReviewID, err) + return providergithub.PostIssueComment(ctx, info.ProviderURL, info.token(), info.FullName, number, body) + } + return nil + case "gitlab", "gitlab-com", "gitlab-self-hosted": + client := providergitlab.NewHTTPClient(info.ProviderURL, info.token()) + if err := client.CreateMRGeneralComment(info.ProviderRepoID, number, body); err != nil { + return err + } + // GitLab Free has no "request changes", so a failed policy is comment-only. + if result.Approved { + if err := client.ApproveMR(info.ProviderRepoID, number); err != nil { + log.Printf("[WARN] pr policy: approve failed for review %d: %v", args.ReviewID, err) + } + } + return nil + default: + return nil + } +} + +const maxListedCriticalFindings = 5 + +func buildPolicyComment(host string, reviewID int64, result policyResult, critical []policyFinding) string { + var b strings.Builder + if result.Approved { + b.WriteString("## βœ… LiveReview policy: Approved\n\n") + } else { + b.WriteString("## ❌ LiveReview policy: Changes requested\n\n") + } + fmt.Fprintf(&b, "Review #%d Β· %d critical Β· %d warning Β· %d info\n\n", reviewID, result.Critical, result.Warning, result.Info) + b.WriteString("| Policy | Result |\n|---|---|\n") + if result.Approved { + fmt.Fprintf(&b, "| %s | βœ… Passed |\n", result.Policy) + } else { + fmt.Fprintf(&b, "| %s | ❌ %d critical |\n\n", result.Policy, result.Critical) + for i, f := range critical { + if i == maxListedCriticalFindings { + fmt.Fprintf(&b, "- …and %d more\n", len(critical)-maxListedCriticalFindings) + break + } + fmt.Fprintf(&b, "- `%s:%d` β€” %s\n", f.FilePath, f.Line, firstLine(f.Content, 120)) + } + } + if host != "" { + fmt.Fprintf(&b, "\n[View full review](%s/#/reviews/%d)\n", strings.TrimSuffix(host, "/"), reviewID) + } + return b.String() +} + +// firstLine returns the first non-empty line of s, cut to limit runes. +func firstLine(s string, limit int) string { + for _, line := range strings.Split(s, "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + if r := []rune(line); len(r) > limit { + return string(r[:limit]) + "…" + } + return line + } + return "" +} + +// QueuePRPolicyCheckIfFlagged enqueues the policy check only for reviews started from the PR link. +func (jq *JobQueue) QueuePRPolicyCheckIfFlagged(ctx context.Context, orgID, reviewID int64) error { + var flagged bool + if err := jq.db.QueryRowContext(ctx, + `SELECT COALESCE((metadata->>'pr_link_policy')::boolean, false) FROM reviews WHERE id = $1 AND org_id = $2`, + reviewID, orgID, + ).Scan(&flagged); err != nil || !flagged { + return err + } + _, err := jq.client.Insert(ctx, PRPolicyCheckJobArgs{OrgID: orgID, ReviewID: reviewID}, + &river.InsertOpts{Queue: "repo_sync", MaxAttempts: 5}) + if err != nil { + return fmt.Errorf("failed to queue PR policy check job: %w", err) + } + return nil +} diff --git a/internal/jobqueue/repo_sync_worker.go b/internal/jobqueue/repo_sync_worker.go index 7b695246b..242399710 100644 --- a/internal/jobqueue/repo_sync_worker.go +++ b/internal/jobqueue/repo_sync_worker.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "log" + "strings" "time" "github.com/jackc/pgx/v5/pgxpool" @@ -13,6 +14,7 @@ import ( providergithub "github.com/livereview/internal/providers/github" providergitlab "github.com/livereview/internal/providers/gitlab" "github.com/livereview/internal/prsync" + storagejobqueue "github.com/livereview/storage/jobqueue" "github.com/livereview/storage/providers/pullrequests" "github.com/riverqueue/river" ) @@ -196,8 +198,9 @@ func (PRStateSyncJobArgs) Kind() string { return "pr_state_sync" } type PRStateSyncWorker struct { river.WorkerDefaults[PRStateSyncJobArgs] - db *sql.DB - store *pullrequests.Store + db *sql.DB + store *pullrequests.Store + webhookStore *storagejobqueue.WebhookStore } func (w *PRStateSyncWorker) Work(ctx context.Context, job *river.Job[PRStateSyncJobArgs]) error { @@ -228,7 +231,7 @@ func (w *PRStateSyncWorker) Work(ctx context.Context, job *river.Job[PRStateSync args.ConnectorID, event.RepositoryProviderID, err) } - _, err = w.store.UpsertPullRequest(pullrequests.PullRequestUpsert{ + pullRequestID, err := w.store.UpsertPullRequest(pullrequests.PullRequestUpsert{ RepositoryID: repositoryID, OrgID: args.OrgID, Provider: args.Provider, @@ -252,9 +255,71 @@ func (w *PRStateSyncWorker) Work(ctx context.Context, job *river.Job[PRStateSync if err != nil { return fmt.Errorf("failed to upsert pull request from webhook event: %w", err) } + + if action, _ := event.Metadata["action"].(string); action == "opened" || action == "open" { + return w.postLinksComment(ctx, repositoryID, pullRequestID, event.Number) + } return nil } +// postLinksComment posts the "Start a review / View review history" comment once per new PR/MR. +// The links_comment_posted_at claim stops duplicate webhook deliveries from posting it twice. +func (w *PRStateSyncWorker) postLinksComment(ctx context.Context, repositoryID, pullRequestID int64, number int) error { + host, err := w.webhookStore.GetWebhookPublicEndpoint(ctx) + if err != nil || host == "" { + log.Printf("[WARN] pr links comment: skipping pull_request_id=%d, public URL not configured: %v", pullRequestID, err) + return nil + } + + res, err := w.db.ExecContext(ctx, + `UPDATE pull_requests SET links_comment_posted_at = now() WHERE id = $1 AND links_comment_posted_at IS NULL`, + pullRequestID) + if err != nil { + return fmt.Errorf("failed to claim links comment for pull_request_id=%d: %w", pullRequestID, err) + } + if n, _ := res.RowsAffected(); n == 0 { + return nil + } + + info, err := loadRepositoryConnectorInfo(w.db, repositoryID) + if err == nil { + body := buildPRLinksComment(host, pullRequestID) + switch info.Provider { + case "github", "github-com", "github-enterprise": + err = providergithub.PostIssueComment(ctx, info.ProviderURL, info.token(), info.FullName, number, body) + case "gitlab", "gitlab-com", "gitlab-self-hosted": + err = providergitlab.NewHTTPClient(info.ProviderURL, info.token()).CreateMRGeneralComment(info.ProviderRepoID, number, body) + default: + return nil + } + } + if err != nil { + // Release the claim so River's retry can post it. + if _, resetErr := w.db.ExecContext(ctx, `UPDATE pull_requests SET links_comment_posted_at = NULL WHERE id = $1`, pullRequestID); resetErr != nil { + log.Printf("[WARN] pr links comment: failed to release claim for pull_request_id=%d: %v", pullRequestID, resetErr) + } + return fmt.Errorf("failed to post links comment for pull_request_id=%d: %w", pullRequestID, err) + } + return nil +} + +func buildPRLinksComment(host string, pullRequestID int64) string { + host = strings.TrimSuffix(host, "/") + return fmt.Sprintf(`## πŸ” LiveReview is ready for this PR + +Run an AI code review on these changes. Findings appear in LiveReview and as comments on this PR. + +| Action | What it does | +|---|---| +| ▢️ **[Start a review](%s/#/pr/%d/review)** | Reviews all changes in this PR against its target branch | +| πŸ“œ **[Review history](%s/#/reviews?pull_request_id=%d)** | All past reviews of this PR | + +**Auto-approval policy:** this PR is approved automatically when a review started from the link above finds no critical issues. + +Links need a LiveReview login. +`, host, pullRequestID, host, pullRequestID) +} + // ReconciliationSweepJobArgs is the periodic coordinator job: it finds // repositories whose PR/MR data hasn't been synced recently (either never, or // staler than the configured threshold - which naturally excludes repos whose diff --git a/internal/jobqueue/review_worker.go b/internal/jobqueue/review_worker.go index c64927588..a9fff83ec 100644 --- a/internal/jobqueue/review_worker.go +++ b/internal/jobqueue/review_worker.go @@ -89,7 +89,7 @@ func (w *ManualReviewWorker) Work(ctx context.Context, job *river.Job[ManualRevi log.Error().Msg("[review_worker] Database connection not available on JobQueue") return fmt.Errorf("database connection not available") } - return reviewprocessor.ProcessManualReview(ctx, w.jq.db, args.OrgID, args.PlanCode, args.ActorUserID, args.ActorEmail, args.ReviewID, args.RequestJSON, + err := reviewprocessor.ProcessManualReview(ctx, w.jq.db, args.OrgID, args.PlanCode, args.ActorUserID, args.ActorEmail, args.ReviewID, args.RequestJSON, func(ctx context.Context, model string, batch license.QuotaBatchInput, extraMeta map[string]interface{}) error { operationID := fmt.Sprintf("manual-review:%d", args.ReviewID) idempotencyKey := operationID @@ -109,6 +109,12 @@ func (w *ManualReviewWorker) Work(ctx context.Context, job *river.Job[ManualRevi }) }, ) + if err == nil { + if qErr := w.jq.QueuePRPolicyCheckIfFlagged(ctx, args.OrgID, args.ReviewID); qErr != nil { + log.Warn().Err(qErr).Int64("review_id", args.ReviewID).Msg("[review_worker] failed to queue PR policy check") + } + } + return err } // DiffReviewJobArgs represents the arguments for an asynchronous diff review job. diff --git a/internal/providers/github/pr_comment.go b/internal/providers/github/pr_comment.go new file mode 100644 index 000000000..84c654db0 --- /dev/null +++ b/internal/providers/github/pr_comment.go @@ -0,0 +1,54 @@ +package github + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "time" +) + +// PostIssueComment posts a plain top-level comment on a PR (POST /repos/{owner}/{repo}/issues/{number}/comments). +// Unlike GitHubProvider.PostComment it honors GitHub Enterprise base URLs and posts body as-is. +func PostIssueComment(ctx context.Context, baseURL, pat, ownerRepo string, number int, body string) error { + path := fmt.Sprintf("/repos/%s/issues/%d/comments", ownerRepo, number) + return postGitHubJSON(ctx, baseURL, pat, path, map[string]string{"body": body}) +} + +// SubmitPullRequestReview submits a PR review with event "APPROVE" or "REQUEST_CHANGES" and body as its text. +// GitHub rejects both events when the token's user is the PR author. +func SubmitPullRequestReview(ctx context.Context, baseURL, pat, ownerRepo string, number int, event, body string) error { + path := fmt.Sprintf("/repos/%s/pulls/%d/reviews", ownerRepo, number) + return postGitHubJSON(ctx, baseURL, pat, path, map[string]string{"event": event, "body": body}) +} + +func postGitHubJSON(ctx context.Context, baseURL, pat, path string, payload interface{}) error { + data, err := json.Marshal(payload) + if err != nil { + return fmt.Errorf("failed to encode request: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, apiBaseURL(baseURL)+path, bytes.NewReader(data)) + if err != nil { + return fmt.Errorf("failed to create request: %w", err) + } + req.Header.Add("Authorization", "token "+pat) + req.Header.Add("Accept", "application/vnd.github.v3+json") + req.Header.Add("Content-Type", "application/json") + req.Header.Add("User-Agent", "LiveReview/1.0") + + client := &http.Client{Timeout: 30 * time.Second} + resp, err := client.Do(req) + if err != nil { + return fmt.Errorf("failed to execute request: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { + respBody, _ := io.ReadAll(resp.Body) + return fmt.Errorf("GitHub request %s failed with status %d: %s", path, resp.StatusCode, string(respBody)) + } + return nil +} diff --git a/internal/providers/gitlab/gitlab_comment.go b/internal/providers/gitlab/gitlab_comment.go index 24c8ebbd6..b4b2429dc 100644 --- a/internal/providers/gitlab/gitlab_comment.go +++ b/internal/providers/gitlab/gitlab_comment.go @@ -157,6 +157,28 @@ func (c *GitLabHTTPClient) CreateMRGeneralComment(projectID string, mrIID int, c return nil } +// ApproveMR approves a merge request as the token's user (POST /merge_requests/:iid/approve). +func (c *GitLabHTTPClient) ApproveMR(projectID string, mrIID int) error { + requestURL := fmt.Sprintf("%s/projects/%s/merge_requests/%d/approve", c.baseURL, url.PathEscape(projectID), mrIID) + req, err := http.NewRequest("POST", requestURL, nil) + if err != nil { + return fmt.Errorf("failed to create request: %w", err) + } + req.Header.Add("PRIVATE-TOKEN", c.token) + + resp, err := c.client.Do(req) + if err != nil { + return fmt.Errorf("failed to execute request: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { + body, _ := io.ReadAll(resp.Body) + return fmt.Errorf("approve request failed with status %d: %s", resp.StatusCode, string(body)) + } + return nil +} + // Enhanced implementation of CreateMRLineComment that uses the GitLab API documentation approach // This overrides the implementation in http_client.go func (c *GitLabHTTPClient) CreateMRLineComment(projectID string, mrIID int, filePath string, lineNum int, comment string, isDeletedLine ...bool) error { diff --git a/ui/src/App.tsx b/ui/src/App.tsx index 3bb59175a..5d7d46c71 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -22,6 +22,7 @@ const AIProviders = React.lazy(() => import('./pages/AIProviders/AIProviders')); const Settings = React.lazy(() => import('./pages/Settings/Settings')); const ReviewsRoutes = React.lazy(() => import('./pages/Reviews/ReviewsRoutes')); const ExploreRoutes = React.lazy(() => import('./pages/Explore/ExploreRoutes')); +const PullRequestReviewTrigger = React.lazy(() => import('./pages/Reviews/PullRequestReviewTrigger')); const Login = React.lazy(() => import('./pages/Auth/Login')); const SelfHosted = React.lazy(() => import('./pages/Auth/SelfHosted')); const Setup = React.lazy(() => import('./pages/Setup/Setup')); @@ -314,6 +315,7 @@ const AppContent: React.FC = () => { } /> } /> } /> + } /> } /> } /> } /> diff --git a/ui/src/api/pullRequests.ts b/ui/src/api/pullRequests.ts index aed9675e5..023f11700 100644 --- a/ui/src/api/pullRequests.ts +++ b/ui/src/api/pullRequests.ts @@ -58,6 +58,25 @@ export const triggerReviewForPullRequest = async (pullRequestId: number): Promis return apiClient.post('/api/v1/reviews', { pull_request_id: pullRequestId }); }; +/** + * Resolve which org a PR belongs to (404 if missing or the user isn't a member). + * Used by the PR comment's "Start a review" link before the org is known. + */ +export const getPullRequestOrg = async (pullRequestId: number): Promise<{ org_id: number }> => { + return apiClient.get<{ org_id: number }>(`/api/v1/pull-requests/${pullRequestId}/org`); +}; + +/** + * Start a review from the PR comment link: reuses one still running from the last + * 10 minutes, and runs the approval policy check when it finishes. + */ +export const startOrReuseReviewForPullRequest = async (pullRequestId: number): Promise => { + return apiClient.post('/api/v1/reviews', { + pull_request_id: pullRequestId, + from_pr_link: true, + }); +}; + /** * Human-friendly label for a PR/MR state. */ diff --git a/ui/src/api/reviews.ts b/ui/src/api/reviews.ts index 8a997726c..885c0f7ee 100644 --- a/ui/src/api/reviews.ts +++ b/ui/src/api/reviews.ts @@ -54,6 +54,7 @@ export const getReviews = async (filters: ReviewsFilters = {}): Promise { + const dispatch = useAppDispatch(); + const { currentOrgId, userOrganizations } = useAppSelector((state) => state.Organizations); + + return async (pullRequestId: number) => { + const { org_id: orgId } = await getPullRequestOrg(pullRequestId); + if (currentOrgId === orgId) return; + if (!userOrganizations.some((o) => o.id === orgId)) { + await dispatch(loadUserOrganizations()).unwrap(); + } + await dispatch(switchOrganization({ orgId })).unwrap(); + }; +}; diff --git a/ui/src/pages/Reviews/PullRequestReviewTrigger.tsx b/ui/src/pages/Reviews/PullRequestReviewTrigger.tsx new file mode 100644 index 000000000..b7a546436 --- /dev/null +++ b/ui/src/pages/Reviews/PullRequestReviewTrigger.tsx @@ -0,0 +1,58 @@ +import React, { useEffect, useRef, useState } from 'react'; +import { useNavigate, useParams } from 'react-router-dom'; +import { Button, EmptyState, Icons, Spinner } from '../../components/UIPrimitives'; +import { startOrReuseReviewForPullRequest } from '../../api/pullRequests'; +import { useSwitchToPullRequestOrg, PR_NOT_FOUND_MESSAGE } from '../../hooks/useSwitchToPullRequestOrg'; + +// Landing page for the "Start a review" link LiveReview posts on new PRs/MRs: +// switches to the PR's org, starts (or reuses a running) review, then opens it. +const PullRequestReviewTrigger: React.FC = () => { + const { id } = useParams<{ id: string }>(); + const navigate = useNavigate(); + const switchToPullRequestOrg = useSwitchToPullRequestOrg(); + const [error, setError] = useState(null); + const started = useRef(false); + + useEffect(() => { + // Guard against StrictMode's double effect firing two reviews. + if (started.current) return; + started.current = true; + + const pullRequestId = Number(id); + if (!Number.isInteger(pullRequestId) || pullRequestId <= 0) { + setError('This review link is invalid.'); + return; + } + + (async () => { + try { + await switchToPullRequestOrg(pullRequestId); + const res = await startOrReuseReviewForPullRequest(pullRequestId); + navigate(`/reviews/${res.reviewId}`, { replace: true }); + } catch (err: any) { + setError(err?.status === 404 ? PR_NOT_FOUND_MESSAGE : err?.message || 'Failed to start the review.'); + } + })(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [id]); + + if (error) { + return ( + } + title="Couldn't start the review" + description={error} + action={} + /> + ); + } + + return ( +
+ +

Starting review…

+
+ ); +}; + +export default PullRequestReviewTrigger; diff --git a/ui/src/pages/Reviews/Reviews.tsx b/ui/src/pages/Reviews/Reviews.tsx index a00a78382..2762794c7 100644 --- a/ui/src/pages/Reviews/Reviews.tsx +++ b/ui/src/pages/Reviews/Reviews.tsx @@ -48,6 +48,7 @@ import { getStatusText, } from '../../api/reviews'; import { Review, ReviewsFilters, ReviewsSort } from '../../types/reviews'; +import { useSwitchToPullRequestOrg, PR_NOT_FOUND_MESSAGE } from '../../hooks/useSwitchToPullRequestOrg'; const pageSizeOptions = [20, 50, 100]; @@ -87,6 +88,7 @@ const initialColumnFiltersFromURL = (params: URLSearchParams): ColumnFiltersStat const Reviews: React.FC = () => { const navigate = useNavigate(); const [searchParams, setSearchParams] = useSearchParams(); + const switchToPullRequestOrg = useSwitchToPullRequestOrg(); const [reviews, setReviews] = useState([]); const [total, setTotal] = useState(0); @@ -107,6 +109,13 @@ const Reviews: React.FC = () => { null ); + // Set by the PR comment's "View review history" link; kept apart from column filters + // since there's no PR column. The ref lets the stable fetch helpers read it. + const [pullRequestId, setPullRequestId] = useState( + () => Number(searchParams.get('pull_request_id')) || undefined + ); + const pullRequestIdRef = useRef(pullRequestId); + // Mirrors of the three state values above, read by refetchCurrent (used by // the 15s auto-refresh poll) so that callback can stay stable rather than // resetting its interval on every keystroke/sort/page change. @@ -139,6 +148,7 @@ const Reviews: React.FC = () => { provider: getFilter('source') || undefined, sort: sortEntry ? (sortEntry.id as ReviewsSort) : undefined, order: sortEntry ? (sortEntry.desc ? 'desc' : 'asc') : undefined, + pullRequestId: pullRequestIdRef.current, }; }; @@ -160,9 +170,22 @@ const Reviews: React.FC = () => { } }, []); - // Initial load. + // Initial load. A PR history link may point at another org, so switch to it first. useEffect(() => { - fetchReviews(buildFetchParams(sorting, columnFilters, pagination)); + (async () => { + if (pullRequestId) { + try { + await switchToPullRequestOrg(pullRequestId); + } catch (err: any) { + setLoading(false); + setError(err?.status === 404 + ? PR_NOT_FOUND_MESSAGE + : err?.message || 'Failed to open pull request history'); + return; + } + } + fetchReviews(buildFetchParams(sorting, columnFilters, pagination)); + })(); // eslint-disable-next-line react-hooks/exhaustive-deps }, []); @@ -204,9 +227,19 @@ const Reviews: React.FC = () => { if (search) next.set('search', search); if (source) next.set('source', source); if (status) next.set('status', status); + if (pullRequestIdRef.current) next.set('pull_request_id', String(pullRequestIdRef.current)); setSearchParams(next, { replace: true }); }; + const clearPullRequestFilter = () => { + pullRequestIdRef.current = undefined; + setPullRequestId(undefined); + syncFiltersToURL(columnFilters); + const nextPagination = { ...pagination, pageIndex: 0 }; + setPagination(nextPagination); + fetchReviews(buildFetchParams(sorting, columnFilters, nextPagination)); + }; + const handleColumnFiltersChange: OnChangeFn = ( updater ) => { @@ -670,6 +703,19 @@ const Reviews: React.FC = () => { + {pullRequestId && ( +
+ Filtered to one pull request + +
+ )} + { error={error} onRetry={refetchCurrent} // Don't hide the search/filter controls when a filter causes zero results. - isEmpty={total === 0 && columnFilters.length === 0} + isEmpty={total === 0 && columnFilters.length === 0 && !pullRequestId} empty={{ title: 'No review previews found', description: diff --git a/ui/src/types/explore.ts b/ui/src/types/explore.ts index b71b348c4..4020ca57b 100644 --- a/ui/src/types/explore.ts +++ b/ui/src/types/explore.ts @@ -133,4 +133,5 @@ export interface CreateReviewFromPullRequestResponse { url: string; reviewId: string; pull_request_id: number; + reused?: boolean; } diff --git a/ui/src/types/reviews.ts b/ui/src/types/reviews.ts index d1f85b0ee..2ea7128c3 100644 --- a/ui/src/types/reviews.ts +++ b/ui/src/types/reviews.ts @@ -159,6 +159,8 @@ export interface ReviewsFilters { order?: 'asc' | 'desc'; page?: number; perPage?: number; + /** Internal pull_requests.id; limits the list to that PR's reviews. */ + pullRequestId?: number; } export interface CreateReviewRequest { From 1c88ca6e53c0f3aa5f44fe89d1b97f07c8ac52fc Mon Sep 17 00:00:00 2001 From: Shrijith Venkatramana Date: Tue, 13 Oct 2026 14:16:34 +0000 Subject: [PATCH 02/17] Add risk-based auto-approve policy engine Implements a layered funnel (eligibility -> static jq gates -> trained risk score -> LLM classification -> decision) for the PR-link auto-approve flow, replacing the previous "no critical findings" placeholder. Includes: - Org/repo-scoped policy config (auto_approve_policies) with an online-learning risk model (internal/riskscore) anchored to a common-sense prior, category-level weight overrides, and daily + on-demand retraining. - Static gate reuse via internal/canonicaldoc (shared with CI/CD rulesets). - Eligibility by per-user review level, with identity mapping for PR authors and a whitelist bypass. - Review detail "Auto Approve" tab, Settings policy editor (Risk Model / Static Rules / Eligibility / General / Production Feedback sub-tabs), and repository-level overrides reusing the same field components. - Force Approve: an audited, level-gated override for a specific blocked review, and manual/webhook production-outcome reporting that feeds the model's online update. Fixes found during review and testing: a malformed claim-UPDATE string that broke Force Approve and landing-delay publish on every attempt, a cold-start 500 on a never-scored org's risk-model endpoint, and a missing org-ownership check on the manual outcome endpoint. Added regression tests for all three plus policy resolution and canonical-doc scoping. LiveReview Pre-Commit Check: skipped (iter:1, coverage:0%) --- ...261013090000_create_risk_policy_engine.sql | 168 +++++ docs/risk-policy-engine.md | 685 ++++++++++++++++++ go.mod | 2 + go.sum | 2 + internal/ai/gemini/batch.go | 1 + internal/ai/gemini/gemini.go | 10 +- internal/ai/langchain/provider.go | 14 +- internal/api/auto_approve_policy_handlers.go | 424 +++++++++++ internal/api/ci_rulesets_handler.go | 163 +---- internal/api/identity_handlers.go | 175 +++++ internal/api/organizations/org_service.go | 4 +- internal/api/outcome_handlers.go | 116 +++ internal/api/review_policy_actions.go | 260 +++++++ internal/api/review_policy_actions_test.go | 134 ++++ internal/api/risk_model_handlers.go | 83 +++ internal/api/server.go | 36 + internal/api/users/user_service.go | 27 +- internal/batch/batch.go | 42 +- internal/canonicaldoc/canonicaldoc.go | 188 +++++ internal/canonicaldoc/canonicaldoc_test.go | 102 +++ .../explore/repository-settings.md | 3 + .../routes_guide/reviews/review-detail.md | 11 + .../routes_guide/settings/auto-approve.md | 65 ++ .../settings/settings-overview.md | 4 +- .../routes_guide/settings/user-management.md | 5 +- internal/jobqueue/jobqueue.go | 15 +- internal/jobqueue/pr_policy_claim_sql_test.go | 85 +++ internal/jobqueue/pr_policy_force_approve.go | 118 +++ internal/jobqueue/pr_policy_worker.go | 248 ++++--- internal/jobqueue/review_worker.go | 11 +- internal/jobqueue/risk_model_refit_worker.go | 42 ++ internal/policyengine/eligibility.go | 107 +++ internal/policyengine/engine.go | 426 +++++++++++ internal/policyengine/policy.go | 114 +++ internal/policyengine/policy_test.go | 141 ++++ internal/prompts/registry_stub.go | 1 + internal/prompts/templates.go | 28 +- internal/review/service.go | 2 + internal/riskscore/calibration.go | 90 +++ internal/riskscore/features.go | 118 +++ internal/riskscore/model.go | 174 +++++ internal/riskscore/refit.go | 213 ++++++ internal/riskscore/riskscore_test.go | 127 ++++ internal/riskscore/scorer.go | 96 +++ internal/riskscore/store.go | 181 +++++ pkg/models/models.go | 15 + ui/src/api/riskPolicy.ts | 148 ++++ ui/src/api/users.ts | 1 + .../AutoApprove/PolicyFieldsEditor.tsx | 116 +++ .../Explore/RepoAutoApproveSection.tsx | 164 +++++ ui/src/components/Navbar/megaMenuData.ts | 1 + ui/src/components/UserManagement/UserForm.tsx | 23 + .../UserManagement/UserIdentitiesEditor.tsx | 100 +++ .../components/reviews/AutoApprovePanel.tsx | 148 ++++ ui/src/pages/Explore/RepositorySettings.tsx | 11 +- ui/src/pages/Reviews/ReviewDetail.tsx | 10 +- .../pages/Settings/AutoApprovePolicyTab.tsx | 268 +++++++ ui/src/pages/Settings/Settings.tsx | 11 + 58 files changed, 5808 insertions(+), 269 deletions(-) create mode 100644 db/migrations/20261013090000_create_risk_policy_engine.sql create mode 100644 docs/risk-policy-engine.md create mode 100644 internal/api/auto_approve_policy_handlers.go create mode 100644 internal/api/identity_handlers.go create mode 100644 internal/api/outcome_handlers.go create mode 100644 internal/api/review_policy_actions.go create mode 100644 internal/api/review_policy_actions_test.go create mode 100644 internal/api/risk_model_handlers.go create mode 100644 internal/canonicaldoc/canonicaldoc.go create mode 100644 internal/canonicaldoc/canonicaldoc_test.go create mode 100644 internal/docindex/docs/routes_guide/settings/auto-approve.md create mode 100644 internal/jobqueue/pr_policy_claim_sql_test.go create mode 100644 internal/jobqueue/pr_policy_force_approve.go create mode 100644 internal/jobqueue/risk_model_refit_worker.go create mode 100644 internal/policyengine/eligibility.go create mode 100644 internal/policyengine/engine.go create mode 100644 internal/policyengine/policy.go create mode 100644 internal/policyengine/policy_test.go create mode 100644 internal/riskscore/calibration.go create mode 100644 internal/riskscore/features.go create mode 100644 internal/riskscore/model.go create mode 100644 internal/riskscore/refit.go create mode 100644 internal/riskscore/riskscore_test.go create mode 100644 internal/riskscore/scorer.go create mode 100644 internal/riskscore/store.go create mode 100644 ui/src/api/riskPolicy.ts create mode 100644 ui/src/components/AutoApprove/PolicyFieldsEditor.tsx create mode 100644 ui/src/components/Explore/RepoAutoApproveSection.tsx create mode 100644 ui/src/components/UserManagement/UserIdentitiesEditor.tsx create mode 100644 ui/src/components/reviews/AutoApprovePanel.tsx create mode 100644 ui/src/pages/Settings/AutoApprovePolicyTab.tsx diff --git a/db/migrations/20261013090000_create_risk_policy_engine.sql b/db/migrations/20261013090000_create_risk_policy_engine.sql new file mode 100644 index 000000000..3561d9828 --- /dev/null +++ b/db/migrations/20261013090000_create_risk_policy_engine.sql @@ -0,0 +1,168 @@ +-- migrate:up + +-- Risk-based auto-approve policy engine. See docs/risk-policy-engine.md for the +-- full design. Replaces the placeholder "no critical findings" policy in +-- internal/jobqueue/pr_policy_worker.go with a layered funnel +-- (eligibility -> static jq gates -> trained risk score -> LLM classification -> decision). + +-- 1. The seniority dial that gates eligibility: a plain integer owners bump per +-- user (no roles/tiers table). Default 1 (lowest). +ALTER TABLE users ADD COLUMN review_level INTEGER NOT NULL DEFAULT 1; + +-- 2. Pull-request author email, captured opportunistically during PR sync so the +-- identity map (below) can match an author to a LiveReview user by email as +-- well as by provider username. Often absent on GitHub (commit author email +-- may be used); nullable. +ALTER TABLE pull_requests ADD COLUMN author_email TEXT; + +-- 3. User identities: the "alternative emails / handles" each org member can hold, +-- used to map a PR author (provider username or email) back to a LiveReview +-- user so eligibility uses their real review_level instead of the default. +-- Org-scoped; a member manages their own, owner/admin manage others'. +CREATE TABLE user_identities ( + id BIGSERIAL PRIMARY KEY, + org_id BIGINT NOT NULL REFERENCES orgs(id), + user_id BIGINT NOT NULL REFERENCES users(id), + kind TEXT NOT NULL CHECK (kind IN ('email', 'username')), + value TEXT NOT NULL, -- normalized (lowercased) email or handle + created_by BIGINT, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + UNIQUE (org_id, kind, value) +); + +CREATE INDEX idx_user_identities_org_user ON user_identities (org_id, user_id); +CREATE INDEX idx_user_identities_lookup ON user_identities (org_id, kind, value); + +-- 4. Per-org / per-repo auto-approve policy. repository_id NULL = org default. +-- 'inherit' is only valid on repo-scoped rows (explicit "use the org default"). +CREATE TABLE auto_approve_policies ( + id BIGSERIAL PRIMARY KEY, + org_id BIGINT NOT NULL REFERENCES orgs(id), + repository_id BIGINT REFERENCES repositories(id), + mode TEXT NOT NULL DEFAULT 'custom' + CHECK (mode IN ('inherit', 'custom', 'always_approve', 'never_approve')), + min_author_level INTEGER NOT NULL DEFAULT 1, + eligibility_whitelist_user_ids BIGINT[] NOT NULL DEFAULT '{}', + risk_threshold_percentile INTEGER NOT NULL DEFAULT 20, + require_llm_confidence INTEGER NOT NULL DEFAULT 8, + blocking_ruleset_ids BIGINT[] NOT NULL DEFAULT '{}', + landing_delay_seconds INTEGER NOT NULL DEFAULT 0, + daily_approve_cap INTEGER, + restrict_outcome_reporting_to_admins BOOLEAN NOT NULL DEFAULT false, + force_approve_min_level INTEGER NOT NULL DEFAULT 3, + created_by BIGINT, + created_at TIMESTAMPTZ NOT NULL DEFAULT now(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE UNIQUE INDEX idx_auto_approve_policies_org_default ON auto_approve_policies (org_id) WHERE repository_id IS NULL; +CREATE UNIQUE INDEX idx_auto_approve_policies_org_repo ON auto_approve_policies (org_id, repository_id) WHERE repository_id IS NOT NULL; +CREATE INDEX idx_auto_approve_policies_org ON auto_approve_policies (org_id); + +-- 5. Append-only policy evaluation trace: one row per stage per review, so the +-- UI can render the full funnel (eligibility -> static -> risk -> llm -> final), +-- not just the final approve/reject. +CREATE TABLE review_policy_log ( + id BIGSERIAL PRIMARY KEY, + review_id BIGINT NOT NULL REFERENCES reviews(id), + org_id BIGINT NOT NULL REFERENCES orgs(id), + stage TEXT NOT NULL, + verdict TEXT NOT NULL, + detail JSONB NOT NULL DEFAULT '{}', + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE INDEX idx_review_policy_log_review ON review_policy_log (review_id); +CREATE INDEX idx_review_policy_log_org ON review_policy_log (org_id, created_at DESC); + +-- 6. Production-truth outcomes (revert / incident / confirmed-safe) that feed the +-- risk model's feedback loop. reason is mandatory. 'test' source is excluded +-- from training. +CREATE TABLE review_outcomes ( + id BIGSERIAL PRIMARY KEY, + review_id BIGINT NOT NULL REFERENCES reviews(id), + org_id BIGINT NOT NULL REFERENCES orgs(id), + reported_by_user_id BIGINT, + outcome_type TEXT NOT NULL CHECK (outcome_type IN ('revert', 'incident', 'confirmed_safe')), + source TEXT NOT NULL CHECK (source IN ('webhook', 'manual', 'ci', 'test')), + reason TEXT NOT NULL, + detail JSONB NOT NULL DEFAULT '{}', + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE INDEX idx_review_outcomes_review ON review_outcomes (review_id); +CREATE INDEX idx_review_outcomes_org_created ON review_outcomes (org_id, created_at DESC); + +-- 7. Human force-approve/override of a specific review's verdict, fully audited. +CREATE TABLE review_policy_overrides ( + id BIGSERIAL PRIMARY KEY, + review_id BIGINT NOT NULL REFERENCES reviews(id), + org_id BIGINT NOT NULL REFERENCES orgs(id), + actor_user_id BIGINT NOT NULL REFERENCES users(id), + original_verdict TEXT NOT NULL, + forced_verdict TEXT NOT NULL, + reason TEXT NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE INDEX idx_review_policy_overrides_review ON review_policy_overrides (review_id); +CREATE INDEX idx_review_policy_overrides_org ON review_policy_overrides (org_id, created_at DESC); + +-- 8. Feature snapshots written at every policy evaluation: the training set once +-- review_outcomes rows attach to the same review_ids. Recomputed-after-the-fact +-- features would drift, so they're snapshotted at evaluation time. +CREATE TABLE risk_feature_snapshots ( + id BIGSERIAL PRIMARY KEY, + review_id BIGINT NOT NULL REFERENCES reviews(id), + org_id BIGINT NOT NULL REFERENCES orgs(id), + model_version INTEGER NOT NULL, + features JSONB NOT NULL, + score DOUBLE PRECISION NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE INDEX idx_risk_feature_snapshots_review ON risk_feature_snapshots (review_id); +CREATE INDEX idx_risk_feature_snapshots_org ON risk_feature_snapshots (org_id, created_at DESC); + +-- 9. The trained risk model. Weights are a few dozen floats, so the row IS the +-- model (no blob store). org_id NULL = the shared seed/default every org +-- starts from. Append-only: any version can be reactivated as an instant rollback. +CREATE TABLE risk_model_versions ( + id BIGSERIAL PRIMARY KEY, + org_id BIGINT REFERENCES orgs(id), + version INTEGER NOT NULL, + weights_json JSONB NOT NULL, + trained_at TIMESTAMPTZ NOT NULL DEFAULT now(), + training_sample_count INTEGER NOT NULL DEFAULT 0, + holdout_auc DOUBLE PRECISION, + update_kind TEXT NOT NULL CHECK (update_kind IN ('seed', 'online_update', 'batch_refit')), + is_active BOOLEAN NOT NULL DEFAULT false +); + +CREATE INDEX idx_risk_model_versions_org_active ON risk_model_versions (org_id) WHERE is_active; + +-- 10. Coarse, category-level admin weight overrides (blast_radius / security_findings / +-- critical_findings / diff_size): an instant multiplier on live scoring, +-- decoupled from the nightly retrain cadence. +CREATE TABLE risk_factor_overrides ( + id BIGSERIAL PRIMARY KEY, + org_id BIGINT NOT NULL REFERENCES orgs(id), + feature_category TEXT NOT NULL, + multiplier DOUBLE PRECISION NOT NULL DEFAULT 1.0, + updated_at TIMESTAMPTZ NOT NULL DEFAULT now(), + UNIQUE (org_id, feature_category) +); + +-- migrate:down + +DROP TABLE IF EXISTS risk_factor_overrides; +DROP TABLE IF EXISTS risk_model_versions; +DROP TABLE IF EXISTS risk_feature_snapshots; +DROP TABLE IF EXISTS review_policy_overrides; +DROP TABLE IF EXISTS review_outcomes; +DROP TABLE IF EXISTS review_policy_log; +DROP TABLE IF EXISTS auto_approve_policies; +DROP TABLE IF EXISTS user_identities; +ALTER TABLE pull_requests DROP COLUMN IF EXISTS author_email; +ALTER TABLE users DROP COLUMN IF EXISTS review_level; diff --git a/docs/risk-policy-engine.md b/docs/risk-policy-engine.md new file mode 100644 index 000000000..eb294fffb --- /dev/null +++ b/docs/risk-policy-engine.md @@ -0,0 +1,685 @@ +# Risk-Based Auto-Approve Policy Engine + +## Context + +A colleague already shipped the plumbing for this feature on `feat/auto-approve`: +a PR comment with a "Start a review" link (`internal/jobqueue/repo_sync_worker.go:265-321`), +a trigger endpoint that launches a review from that link +(`internal/api/pull_request_review_trigger.go`), and a worker that posts an +approve/request-changes decision back to GitHub/GitLab +(`internal/jobqueue/pr_policy_worker.go`). The decision logic itself is a +placeholder β€” `result.Approved = result.Critical == 0` β€” explicitly marked +`// ponytail: one static policy; swap for org-defined policies later`. + +We're replacing that placeholder with a real, configurable, risk-based policy +engine, modeled on Meta's RADAR system (arxiv 2605.30208): a layered funnel of +**eligibility β†’ static/deterministic gates β†’ ML risk score β†’ LLM semantic +classification β†’ final decision**, rather than one opaque model. The paper's +core lessons we're deliberately carrying over: +- Risk thresholds are a per-org *dial*, not a fixed cutoff (their P5β†’P50 experiment). +- Safety signals (revert rate, incident rate) must feed back into calibration β€” this + doesn't exist in LiveReview today and needs new plumbing. +- The system must stay legible: every decision should show *why*, stage by stage, not just a verdict. + +LiveReview already has three of the five RADAR ingredients half-built, which +shapes this plan heavily: +- **Static gates** β†’ the CI/CD gate jq-ruleset system (`ci_rulesets` table, + `internal/api/ci_rulesets_handler.go`'s `runJQBool`/`buildCanonicalReviewDoc`) + is *exactly* RADAR's deterministic-validation stage, just not yet reusable + outside CI. We reuse it as-is rather than building a second rule engine. +- **LLM semantic signal** β†’ findings already carry `Severity`/`Confidence`/`Category` + (`pkg/models.ReviewComment`, stored in `ai_comments` JSONB). What's missing is an + explicit *diff-level* classification (safe-refactor vs logic-risk vs security-risk), + not per-finding severity. +- **Risk score** β†’ blast radius already scores *hunks* (`internal/blastradius`, + `blast_radius_hunks` table) but only aggregates via `MAX(combined)` per review + (`blast_radius_reviews` view) β€” a deliberately crude placeholder with + "no production caller today". We build the real diff-level score on top of this. +- **Eligibility** β†’ doesn't exist at all. No seniority/level field on users, no + org policy config table, no settings UI tab for any of this. + +## Non-goals for this plan + +No full automatic revert-detection (e.g. auto-recognizing a revert commit by +pattern) beyond a manual/webhook ingestion point, and no attempt to make the +online-learning update path literally streaming/sub-second β€” "near real +time" here means an immediate single-example gradient nudge on each incoming +labeled outcome, plus a daily stabilizing batch refit and an on-demand +button, which is what was asked for. + +## Architecture + +### 1. Data model + +**`users.review_level`** (new column, `INTEGER NOT NULL DEFAULT 1`) β€” the +"seniority" dial the user asked for. No roles/tiers table; just a plain +integer owners can bump per user from the existing Users settings tab +(`ui/src/pages/Settings/UsersSettingsTab.tsx` β€” add one numeric field + +`PUT /api/v1/orgs/:org_id/users/:user_id` already exists, extend its payload). + +**One table, with one deliberate exception**: the thing that must stay +strictly org-wide is *the trained classifier itself* β€” `risk_model_versions`' +learned weights and `risk_factor_overrides`' category multipliers (Β§3) β€” +because fitting/retraining a separate model per repo is the genuinely +expensive thing hundreds of repos can't each support. The *threshold* +compared against that one shared score is a different, cheap thing: just a +cutoff number, no model behind it, so it's fine β€” and per the user's +preference, correct β€” to let it vary per repo. `auto_approve_policies` +therefore carries `risk_threshold_percentile` alongside everything else, +resolved the exact same way as every other field on this table: + +```sql +CREATE TABLE auto_approve_policies ( + id BIGSERIAL PRIMARY KEY, + org_id BIGINT NOT NULL REFERENCES organizations(id), + repository_id BIGINT REFERENCES repositories(id), -- NULL = org default + mode TEXT NOT NULL DEFAULT 'custom' + CHECK (mode IN ('inherit','custom','always_approve','never_approve')), + -- 'inherit' only valid on repo-scoped rows (explicit "use the org default" marker, + -- distinct from simply having no row, so the repo settings UI in Β§9 can show it + -- as a selected choice rather than an absence); org-default rows are never 'inherit'. + min_author_level INTEGER NOT NULL DEFAULT 1, + eligibility_whitelist_user_ids BIGINT[] NOT NULL DEFAULT '{}', -- bypass min_author_level + risk_threshold_percentile INTEGER NOT NULL DEFAULT 20, -- P20 default; cheap to override per repo, see below + require_llm_confidence INTEGER NOT NULL DEFAULT 8, -- out of 10, mirrors RADAR's ACR gate + blocking_ruleset_ids BIGINT[] NOT NULL DEFAULT '{}', -- reuses ci_rulesets.id + landing_delay_seconds INTEGER NOT NULL DEFAULT 0, -- post-approve human-rejection window + daily_approve_cap INTEGER, -- NULL = unlimited + restrict_outcome_reporting_to_admins BOOLEAN NOT NULL DEFAULT false, -- see Β§10 + force_approve_min_level INTEGER NOT NULL DEFAULT 3, -- see Β§7 + created_by, created_at, updated_at +); +``` +Resolution order for a given PR: repo-scoped row if present and `mode != +'inherit'`, else org-default row (`repository_id IS NULL`), else feature is +off β€” every field on this table, including the threshold, resolves per-repo +this way, pre-filled from the org default when a repo admin opens +"Customize" (Β§9) so they start from the global calibration and only change +what they want. The risk *score* itself always comes from the one shared +model in `risk_model_versions` +regardless, looked up by `org_id` alone. `mode = 'always_approve'`/`'never_approve'` +on either level are the "people may already know the rules for this repo" +shortcuts from the user's ask β€” skipping the risk-score/eligibility/LLM +stages entirely for `always_approve` (clearly a low-stakes repo, no need to +run the model against it every time), but the static jq gate (Β§4) still +runs even under `always_approve` β€” a repo-wide "trust this repo" setting is +not the same thing as "ignore a secrets-leak detector," and should never be +able to silently become that by accident. Every decision made under either +mode still writes a `review_policy_log` row ("mode override: +always_approve"), so the Auto Approve tab (Β§7) shows *why* just as clearly +as a normal funnel run, not a silent skip. This mirrors `ci_rulesets`' +existing org-scoping pattern β€” reuse its storage/handler conventions +directly (`internal/api/ci_rulesets_handler.go` is the template). + +A new **`review_policy_log`** table (append-only) replaces the single +`metadata.policy_result` JSONB blob: one row per policy evaluation with +`review_id, stage, verdict, detail JSONB, created_at`, so the UI can render +the full funnel trace (eligibility β†’ static β†’ risk β†’ LLM β†’ final), not just +the final approve/reject. (`risk_feature_snapshots` and `risk_model_versions` +β€” the tables backing the trained risk model β€” are described in Β§3, since +they're part of that stage's design, not generic schema.) + +**`review_outcomes`** (new table) for the feedback loop: `review_id, org_id, +reported_by_user_id, outcome_type ('revert'|'incident'|'confirmed_safe'), +source ('webhook'|'manual'|'ci'), reason TEXT NOT NULL, detail JSONB, +created_at`. `reason` is mandatory, not optional free text β€” a one-line +"why" (e.g. "rolled back due to prod 500s") required on every manual mark +and every webhook payload. This is the deliberate friction point: a bare +toggle with no permission check (per Β§10) would be too easy to mis-click or +spam, but a required reason keeps the manual log readable/auditable and +gives the risk model's audit trail (Β§7, Β§11) something a human reviewing it +later can actually make sense of, without needing an approval workflow or +role restriction to prevent noise. Populated by a manual "Mark as +reverted" action in the review detail UI and a documented webhook endpoint +(`POST /api/v1/reviews/:id/outcomes`, `reason` required in the payload too) +for CI/CD or incident-tooling integrations to report rollbacks/incidents +against a known review id β€” real auto-detection (e.g. watching for +revert-commit patterns) is future work. Critically, every insert into this +table is also the trigger for the real-time online model update described +in Β§3 (path 1) β€” this table isn't just a read-only log, it's the live +feedback signal the risk model reacts to. + +**`review_policy_overrides`** (new table): `review_id, org_id, +actor_user_id, original_verdict, forced_verdict, reason TEXT NOT NULL, +created_at`. This backs **Force Approve** (Β§7) β€” a human explicitly +approving (or rejecting) a specific PR *despite* the policy engine's +verdict, with a mandatory reason, fully audited. This is distinct from +`review_outcomes`: an override is about one specific review's decision +being overridden by a person, while an outcome is a production-truth signal +(revert/incident) the model learns from β€” they can both exist for the same +review without conflating "a human overrode the gate" with "this PR turned +out to be risky in prod." + +### 2. Eligibility stage + +New `internal/policyengine/eligibility.go`: given a review + its PR, resolve +`users.review_level` for the PR author (join via `pull_requests.author_email` +or equivalent existing field). If the author's id is in +`eligibility_whitelist_user_ids`, pass immediately regardless of level β€” +this is the "if <L3, always requires a review, unless whitelisted" rule +from the user's ask, read literally: level is the default gate, the +whitelist is the explicit per-user exception to it. Otherwise require +`review_level >= min_author_level`. Bots (reviews with no human author / +from a known bot login) get `review_level = 0` unless explicitly +whitelisted β€” this is the human/bot split RADAR treats as the very first +fork in the funnel. + +### 3. Risk score stage β€” an online-learning model that updates itself + +**The hard requirement here is adaptivity, not model complexity**: new +incident/rollback data must move the risk score within the day it arrives +(ideally immediately), and an admin tweaking "weight security findings +higher" must take effect immediately too. That requirement rules out the +ONNX-file approach from the earlier draft of this plan β€” ONNX is a *serving* +format for a model trained elsewhere; re-running an offline training script +and re-exporting a file on every incoming webhook is the wrong shape for +"updates in near-real-time." So this stage is a genuine online-learning +model, not a periodically-refreshed static artifact. + +**Library**: `github.com/cdipaolo/goml`'s `linear.Logistic`, specifically its +**`StochasticGA` online-learning mode** β€” pure Go, no cgo, and built for +exactly this: each call to `OnlineLearn` takes one new labeled example and +nudges the existing weight vector by one gradient step, in milliseconds, no +retraining pass over the full dataset required. (`goml` also supports a full +batch fit over a dataset in one call, used below for the periodic +stabilizing pass.) This is a legitimate, citable ML library doing real +gradient-based learning β€” not hand-picked constants β€” while still being +light enough to update on every single incoming data point, which is the +actual thing the user asked for. + +**Feature vector** (model inputs, shared with the simulation/backtest panel in Β§8): +- Blast-radius diff-level aggregates computed fresh from `blast_radius_hunks` + rows for the review β€” `sum(combined)`, `count(tier >= high)`, + `max(combined)`, `weighted_avg(combined, by hunk LOC)` β€” replacing the + `MAX(combined)` placeholder's reasoning with real feature engineering. +- Finding-severity counts (critical/warning/info) from the same + `canonicalReviewDoc` the jq rulesets already build + (`buildCanonicalReviewDoc` in `ci_rulesets_handler.go` β€” reused directly, + not re-queried). +- Diff-shape: files changed, lines added/removed (already on the review/PR row). +- LLM semantic-classification confidence + class, once Β§5 lands (feature + vector is append-only across phases β€” new features don't break old model + versions, they just start at 0 until the next retrain includes them). + +**Cold start β€” common sense first, data-driven second.** Most orgs will +have zero or a handful of `review_outcomes` rows for a long time β€” "collect +enough data to train a model" cannot be the starting state, or the feature +is useless for exactly the orgs that most need good defaults. So the seed +model's weights aren't zero-initialized or randomly initialized before the +first fit β€” they're hand-set to encode the same domain reasoning already +embedded elsewhere in this codebase (blast-radius tiers, the current +critical-findings-block rule, RADAR's own safe/risk taxonomy): blast-radius +sum and critical-finding count dominate, security-category findings are +weighted up, info-level findings and small diffs are weighted near zero. +This prior is the actual fallback behavior with no data β€” not a "cold start +model" distinct from the "real model," but the starting *point* every org's +model begins from and is anchored back to. + +That anchor is enforced structurally, not just at initialization, via a +**prior-anchored regularization term** on every batch refit (Β§ path 2/3): +instead of plain L2-toward-zero, penalize weights for drifting from the +common-sense prior, scaled by `Ξ» / (1 + training_sample_count)` β€” so with 3 +labeled outcomes the fitted model stays close to common sense (data can't +yet overrule domain knowledge), and by the time an org has a few hundred +outcomes the anchor's pull is negligible and the weights are essentially +data-driven. This is one extra term in `goml`'s existing loss, not a second +model or a blending step at inference time β€” inference is still a single +dot product against one weight vector, just one that was fit with this +anchor. The real-time online nudge (path 1) uses the same anchored loss per +step, so even online updates can't swing wildly away from the prior on a +single noisy example. + +The UI should say this plainly rather than imply a hidden black box: the +risk-model section of Β§8/Β§11 shows a **"model confidence"** indicator derived +from `training_sample_count` (e.g. "Low β€” mostly common-sense defaults (3 +outcomes recorded)" vs "Calibrated to your repo (180 outcomes recorded)"), +so an org with no data sees an honest "this is sensible defaults, not yet +learned from your history" rather than a number that looks authoritative +before it's earned that. + +**`risk_feature_snapshots`** (new table): `review_id, org_id, model_version, +features JSONB, score, created_at` β€” written at every policy evaluation. +Makes the model auditable (UI can show "this review's risk score came from +these feature values") and is the training set once `review_outcomes` rows +attach to the same `review_id`s β€” recomputing features after the fact from +mutable tables would drift, so they're snapshotted at evaluation time instead. + +**`risk_model_versions`** (new table): `id, org_id (NULL = shared default), +version, weights_json, trained_at, training_sample_count, holdout_auc, +update_kind ('seed'|'online_update'|'batch_refit'), is_active`. Weights are +a few dozen floats, so unlike the ONNX design there's no blob store involved +at all β€” the row itself is the model. Cold start ships one seed model +(`org_id IS NULL`, `update_kind = 'seed'`, `training_sample_count = 0`) whose +weights are exactly the hand-set common-sense prior described below, not a +fit against any data β€” every org starts here and only moves away from it as +real `review_outcomes` accumulate. + +**Three update paths, all ending in a new `risk_model_versions` row (append-only, so any version can be reactivated as an instant rollback)**: +1. **Real-time online nudge** β€” the moment a `review_outcomes` row lands + (webhook or manual "mark as reverted"), the policy engine loads that + review's `risk_feature_snapshots` row as the labeled example and calls + `OnlineLearn` on the org's current weights, immediately persisting the + nudged weights as a new version and activating it. This is the actual + "learns in real time" behavior β€” no batch job, no delay, one gradient step. +2. **Daily batch refit** β€” a River periodic job (same registration pattern + as `blast_radius_worker.go`) re-fits a fresh `linear.Logistic` from + scratch over all of an org's `risk_feature_snapshots β‹ˆ review_outcomes`, + corrects for drift that a long sequence of noisy one-example online steps + can accumulate, evaluates holdout AUC, and only promotes if it's at least + as good as the current online-nudged version β€” the stabilizing + counterpart to (1), not a replacement for it. +3. **On-demand refit** β€” the same batch refit as (2), triggered by a + "Retrain now" button in the policy settings UI (Β§8) / `POST + /api/v1/risk-model/retrain`, for when an admin just changed a weight + override (below) and doesn't want to wait for the nightly job. + +**Admin weight overrides** (`risk_factor_overrides` table: `org_id, +feature_category, multiplier, updated_at`) are how "people may update the +weights for certain factors (security, etc.)" takes effect *instantly*, +decoupled from training cadence. These are deliberately **coarse, +category-level** sliders, not one per raw model feature β€” `blast_radius`, +`security_findings`, `critical_findings`, `diff_size` map to small groups +of underlying features (e.g. `blast_radius` covers `sum(combined)`, +`max(combined)`, `weighted_avg(combined)` together), so an admin adjusts a +handful of legible dials instead of a dozen raw coefficients that could +easily be set into a contradictory configuration. Scoring computes +`effective_weight_i = learned_weight_i * override_multiplier[category(i)]`, +so dragging the "security" slider changes live scoring the moment it's +saved, with no retrain needed. The next batch refit (path 2/3) treats each +category override as an L2-regularization anchor (penalize that category's +learned weights for drifting far from `override_multiplier * initial_weight`) +so the model doesn't fight or silently erase the admin's stated preference +on its next pass β€” the override is both an instant hard effect and a soft +prior for future learning. + +**Inference**: `internal/riskscore.Scorer` keeps the org's active +`risk_model_versions` weights cached in memory (invalidated whenever a new +version activates via any of the three paths above), computes +`sigmoid(Ξ£ effective_weight_i * feature_i)` directly β€” a dot product, no +runtime/session overhead, which is also why this design tolerates being +updated on every single webhook event without a performance concern. + +**Percentile calibration**: unchanged β€” per-org, a rolling window of recent +scores turns a given `risk_threshold_percentile` value into "this org's +actual P20," not a global magic number. Independent of which model version +produced the scores, so recalibration is cheap even right after an online +nudge changes the score distribution. The *calibration curve itself* is a +single org-wide thing for v1 (one model per org, per the per-org-only scope +decision) β€” but the `risk_threshold_percentile` cutoff read off that curve +is a plain field on `auto_approve_policies` (Β§1) and so *is* +repo-overridable, exactly like the rest of that table: a repo can decide +"P50 is fine for us" against the same shared score distribution without +needing its own curve. + +**ONNX note**: `github.com/yalue/onnxruntime_go` is already vendored +(serving the BGE embedder in `internal/docindex/embedder.go`) and remains a +good fit if a *future* heavier model β€” gradient-boosted trees, a small +MLP β€” trained offline on a slower cadence is ever wanted; `riskscore.Scorer` +should be a small interface (`Score(features) float64`) specifically so an +ONNX-backed implementation could be swapped in later for one org without +touching the policy engine. It's just not what satisfies "updates in near +real time" today β€” that's what `goml`'s online mode is for. + +### 4. Static/deterministic gate stage β€” reuse jq rulesets as-is + +No new rule engine. `auto_approve_policies.blocking_ruleset_ids` references +existing `ci_rulesets` rows. The policy worker calls the *same* +`buildCanonicalReviewDoc` + `runJQBool` already used by +`CIRulesetsHandler.Evaluate` β€” this is the literal reuse of "the existing jq +based blockers" the user asked for. The only new code is a thin loop: for +each configured ruleset id, evaluate, block on first truthy. The existing +`ui/src/pages/CiRulesets/` builder UI becomes dual-purpose automatically (a +ruleset can be attached to CI, to auto-approve, or both) β€” just needs a +checkbox/multi-select in the new policy editor listing existing rulesets by +name, no changes to the ruleset UI itself. + +### 5. LLM semantic classification stage β€” folded into the existing review call + +Rather than a second LLM round-trip, this rides inside the review-generation +call that already exists: `internal/prompts/templates.go`'s +`JSONStructureExample` (the schema the model is told to return, currently +`fileSummaries` + `comments`) gets one more top-level object, +`diffClassification`: +```json +"diffClassification": { + "class": "refactor-only|logic-change|security-sensitive|...", + "confidence": 7 +} +``` +alongside a short addition to `CommentRequirements`/`ReviewGuidelines` +describing RADAR's safe/risk taxonomy (refactor-only, dead-code-removal, +logic-change, security-sensitive, etc.) and how to pick `confidence` (0–10). +Same taxonomy-enforcement pattern already used for `category`/`subcategory` +(`TaxonomyClassificationRules`, templates.go:100) β€” a fixed, closed list, +not free text. Whatever code already parses the model's JSON response into +`fileSummaries`/`comments` (one extra field on the same struct) picks up +`diffClassification` too and stores it on the review alongside the rest of +`review_result` (same `metadata.review_result` JSONB it already writes to β€” +no new storage column needed, `buildCanonicalReviewDoc` already reads from +this blob and gains one more field to surface for jq rules). Trade-off +accepted deliberately: this couples the classification's prompt/schema to +the main review prompt, so a future change to one should check the other β€” +worth it here to avoid a second LLM call's cost/latency per review, since +findings generation already has full context on the diff. Policy requires +`confidence >= require_llm_confidence` AND no disqualifying class, mirroring +RADAR's "any risk signal disqualifies" rule. + +### 6. Rewiring `pr_policy_worker.go` + +Replace the body of `Work`/the fake `policyResult` with calls into +`internal/policyengine`: resolve policy β†’ eligibility β†’ static gates β†’ risk +score vs. threshold β†’ LLM classification gate β†’ final verdict. Each stage +writes a `review_policy_log` row. `buildPolicyComment` is extended to render +the funnel (stage-by-stage βœ…/❌ table) instead of just severity counts β€” +this is the "show why" requirement. `landing_delay_seconds` becomes an actual +delay before `publish()` runs (queue a follow-up River job instead of +publishing immediately), giving the "human rejection window" RADAR relies on +for safety. + +### 7. Review detail β€” "Auto Approve" tab + +Every review triggered through the PR-link flow should show its policy +funnel on its own detail page, not just in the GitHub/GitLab comment. Add +`'auto-approve'` as a fourth value to `ReviewDetail.tsx`'s existing +`activeTab` union (currently `'findings' | 'accounting' | 'events'`, +`ReviewDetail.tsx:84`) and a fourth entry in the `tabs={[...]}` array passed +to the existing `Tabs` component (`ReviewDetail.tsx:679-686`) β€” the tab only +appears when the review has `metadata.pr_link_policy = true` (same flag the +backend already uses to decide whether to run the policy at all), so plain +ad-hoc reviews don't show an empty tab. + +Content, backed entirely by `GET /api/v1/reviews/:id/policy-log` (new +endpoint, reading `review_policy_log` ordered by stage): +- A vertical stage list β€” Eligibility, Static Rules, Risk Score, LLM + Classification, Final Decision β€” each row showing βœ…/❌/⏭️ (skipped, e.g. + static rules skipped because eligibility already blocked) and a one-line + reason ("Author level 2 < required 3" / "Blocked by ruleset `no-secrets`" + / "Risk score 34 (P45) β‰₯ threshold P20" / "approved"). +- Expanding the Risk Score row shows the actual feature breakdown from that + evaluation's `risk_feature_snapshots` row as a small table (feature name, + value, effective weight, contribution) β€” directly answers "what parts of + the decision are what" for the one stage that isn't already self-explanatory + jq/eligibility logic, and doubles as the explainability view for the + model-confidence indicator from Β§3. +- A final summary banner matching the comment already posted to + GitHub/GitLab (reuse `buildPolicyComment`'s data, rendered as UI instead + of markdown, so the two never drift apart in wording). +- If `mode` was `always_approve`/`never_approve` (Β§1), the tab shows that + as the single reason, with a link to the policy settings β€” no funnel to + render since none ran. +- **Force Approve**: when the final verdict is "blocked," a visible "Force + approve anyway" button, gated by `users.review_level >= + force_approve_min_level` (a configurable floor, default 3, in Β§8's + Eligibility sub-tab β€” not tied to org role, consistent with seniority + being the dial that already governs eligibility in Β§2) rather than a + fixed owner/admin check, opens a small form requiring a reason, then + calls `POST /api/v1/reviews/:id/force-approve`. Only the blockedβ†’approved + direction exists β€” there's no symmetric "force request-changes" on a PR + the engine already approved, since a human can already just leave an + ordinary review comment or request-changes on any PR regardless of what + the bot posted; the override only needs to exist for the direction that's + actually dangerous (bypassing a safety gate), not the direction that's + already unrestricted. This posts the real GitHub/GitLab + approval via the same `publish()` path as a normal approve (Β§6), but the + comment is explicitly marked "⚠️ Force-approved by {user} despite policy + block β€” {reason}" so the PR history is honest about what happened, and + writes a `review_policy_overrides` row (Β§1) that the metrics dashboard + (Β§11) can later report on ("N force-approvals this month, M of which were + later marked as incidents" β€” the sharpest possible signal that a policy + threshold is miscalibrated). + +### 8. Settings UI β€” policy editor (sub-tabbed) + "what would've happened" simulation + +New Settings tab `ui/src/pages/Settings/AutoApprovePolicyTab.tsx` (outer +registration pattern: `StorageSettingsTab.tsx`, gated `requiresOwnerOrAdmin` +like the other admin tabs in `Settings.tsx`'s `tabs` array, ~line 292). The +page itself is **five horizontal sub-tabs** (Risk Model, Static Rules, +Eligibility, General, and Production Feedback β€” the last described in +Β§10), using the same `Tabs` component/pattern already used inside +`ReviewDetail.tsx` (Β§7) rather than a long single-scroll form β€” this +directly answers the "each thing probably can be a tab within that main +tab" ask. **The trained classifier itself is org-only** (`risk_model_versions`' +weights, `risk_factor_overrides`' category multipliers, retraining β€” all +with no `repository_id`, never per-repo, since that's the expensive part); +everything else, *including the threshold read off that model's score*, is +backed by `auto_approve_policies` and reappears, repo-scoped, in Β§9's +repository settings β€” same components, different `repository_id` on the +save call, per the "I'd prefer to have those as repository level overrides +as well" ask. + +**Sub-tab A β€” Risk Model (the classifier itself, org-wide only, not +repeated in Β§9)**: enabled toggle, the four coarse category weight sliders +(Blast Radius / Security Findings / Critical Findings / Diff Size, backed +by `risk_factor_overrides`, instant effect per Β§3), the model-confidence +indicator ("Low β€” 3 outcomes recorded" / "Calibrated β€” 180 outcomes +recorded"), a "Retrain now" button (`POST /api/v1/risk-model/retrain`), and +a read-only calibration chart (score distribution across the org's recent +reviews) that both this sub-tab's own org-default threshold field and every +repo's threshold override (Sub-tab D / Β§9) are plotted against, so an admin +tuning either always sees the same curve. The threshold field itself lives +on Sub-tab D below (org-default row included), not here β€” it's a plain +policy field, not part of the model. + +**Simulation/backtest panel** (shown on Sub-tab A, reused everywhere a +threshold is being edited): a read-only `POST +/api/v1/auto-approve-policies/simulate` that takes the draft policy (not +yet saved β€” optionally for a specific repo, passing a `repository_id` so +"what would this repo's *effective* policy have done" is answerable even +though the underlying score comes from the shared model) and replays it +against the org's (or that repo's) last N completed reviews β€” same +evaluation path as production, result discarded β€” showing a table of +{review, would-approve?, which stage blocked it, risk score}. Reuses 100% +of real evaluation code (Β§2-Β§6), so there's no separate preview logic to +keep in sync β€” the same pattern `CiRulesetEditor.tsx` already uses to +preview a jq expression against a past review. + +**Sub-tab B β€” Static Rules** *(repo-overridable, reused in Β§9)*: "Write and +test static jq rules" per the user's ask, reusing `ui/src/pages/CiRulesets/` +wholesale rather than building a second editor β€” this sub-tab is a thin +wrapper embedding `CiRulesetEditor.tsx`'s existing builder (expression +input, live preview against a past review, jq quick-help sidebar from +`jqQuickHelp.tsx`) plus a multi-select of which existing/newly-created +rulesets are attached to this policy as blockers (`blocking_ruleset_ids`). +A ruleset created or edited here is the exact same `ci_rulesets` row usable +for CI/CD gates β€” no fork, per Β§4. A repo override can attach a completely +different set of rulesets than the org default (e.g. a stricter ruleset +only on the payments repo). + +**Sub-tab C β€” Eligibility** *(repo-overridable, reused in Β§9)*: +`min_author_level` input, explanatory copy for the "below this level, +always requires human review unless whitelisted" rule, a user picker +(searching the org's members, same component pattern as +`UsersSettingsTab.tsx`'s member list) to manage +`eligibility_whitelist_user_ids` β€” add/remove with each user's current +`review_level` shown inline so it's clear *why* they'd otherwise be +blocked β€” and a separate `force_approve_min_level` input (Β§7) with its own +explanatory copy, since it's a distinct dial from `min_author_level` +(one governs whose *PRs* skip the funnel, the other governs who can +*override* the funnel's verdict on anyone's PR). A repo can reasonably want +a higher bar here than the org default (e.g. the payments repo requires +L4+ where the org default is L2+). + +**Sub-tab D β€” General** *(repo-overridable, reused in Β§9)*: +`risk_threshold_percentile` (shown as a percentile slider against the same +calibration chart from Sub-tab A, pre-filled from the org default on a +repo's first "Customize"), `require_llm_confidence` slider (the LLM +classification gate from Β§5), `landing_delay_seconds`, `daily_approve_cap`, +and `restrict_outcome_reporting_to_admins` β€” the threshold sits here rather +than on its own sub-tab specifically because it's a plain field on +`auto_approve_policies`, not model configuration, and belongs next to the +other per-policy knobs that need to be overridable per repo the same way. + +Mega menu: add an entry under the existing "Reviews" or "Settings" section +in `ui/src/components/Navbar/megaMenuData.ts` per this repo's standing rule +that every new settings tab needs one β€” mirror the `requiresOwnerOrAdmin` +gating from the tab itself. + +### 9. Repository-level overrides (Explore β†’ Repositories β†’ repository settings) + +`ui/src/pages/Explore/RepositorySettings.tsx` already has exactly the +layout this needs: a `SECTIONS` array (currently just `repo-cache`, +`RepositorySettings.tsx:10`) rendering a left-hand section list and the +selected section's component on the right, hash-routed +(`#repo-cache`/`#auto-approve`). Add one entry, `{ id: 'auto-approve', name: +'Auto-approve', icon: ... }`, and a new `RepoAutoApproveSection.tsx` +component (same shape as the existing `RepoCacheSection.tsx` it sits next +to) that: +- Shows the resolved org-default policy as read-only context ("Org default: + Custom Β· min level 2 Β· P20 threshold Β· 2 blocking rulesets") plus a line + making clear which part of that *can't* be changed per-repo: the trained + model behind the score itself (weights, retraining, category overrides β€” + Β§3) is always the one org-wide classifier, no matter what this repo's + policy says. The threshold compared against that score, though, is just + another field below. +- Offers the same four choices as `auto_approve_policies.mode`: + **Use org default** (`mode = 'inherit'`, the common case β€” most repos + need nothing here), **Always auto-approve this repo** + (`mode = 'always_approve'`, for the low-stakes repos the user mentioned β€” + docs sites, internal tools), **Never auto-approve this repo** + (`mode = 'never_approve'`, for the sensitive repos), or **Customize** + (`mode = 'custom'`, which expands into **Static Rules / Eligibility / + General** β€” Β§8's sub-tabs B, C, D, reusing those exact components with + `repository_id` set instead of building repo-scoped equivalents, General + including the `risk_threshold_percentile` field pre-filled from the org + default β€” but *not* a Risk Model sub-tab, since the classifier itself + stays global per the user's "keep the risk model simple and global" + decision, while the threshold and everything else becomes repo-overridable). +- This is the same `auto_approve_policies` table and + `internal/policyengine` resolution logic from Β§1 β€” a repo-scoped row is + not a different feature, just a different `repository_id` on the same + CRUD endpoints, so this section is mostly UI wiring, not new backend work. + The risk score itself is always computed once per review from the org's + single model (Β§3) regardless of which repo it's in β€” only the threshold + compared against it, and everything else on `auto_approve_policies`, can + differ per repo. + +### 10. Outcome ingestion UX β€” setup, test, and soft guidance + +This is the single most fragile link in the feedback loop: `review_outcomes` +is the only thing that makes the risk model real ML instead of a frozen seed +model, and most orgs have no existing process for tagging "this PR caused an +incident" β€” this is a brand-new habit, not a webhook they're already +sending. So the UI work here matters as much as the API/schema from Β§1. + +**Setup panel** (a fourth sub-tab on `AutoApprovePolicyTab.tsx`, Β§8 β€” +"Production Feedback" alongside Risk Model / Static Rules / Eligibility): a dedicated +"Production feedback" subsection, modeled directly on +`SMTPSettingsTab.tsx`'s `handleTest`/"Test Connection" pattern (same +loading/success/error state shape) and `StorageSettingsTab.tsx`'s connection +test, rather than inventing a new pattern: +- Shows the org's webhook URL (`POST /api/v1/orgs/:org_id/review-outcomes`, + authenticated the same way other inbound integration webhooks are β€” + reuse whatever bearer/signature scheme `ci_rulesets`' CI integration + already documents in `CiRulesetIntegration.tsx`/`shared.tsx` rather than + inventing a new auth story) plus a one-click "regenerate secret." +- A **"Send test event"** button that POSTs a synthetic outcome against a + real, recent review in that org (picked from a dropdown) marked + `source: 'test'` so it never pollutes the training set (`risk_feature_snapshots + β‹ˆ review_outcomes` queries used for refitting explicitly exclude + `source = 'test'`), then immediately shows the actual stored row back β€” + "βœ… received, this review is now tagged as reverted" β€” so the admin gets + the same end-to-end confidence as `handleTest` gives for SMTP: a round + trip against the real endpoint, not a dry validation. +- Copy-paste integration snippets for the common cases, same generator + pattern as `buildCurlSnippet`/`buildGithubActionsWorkflow` in + `CiRulesetIntegration.tsx`'s `shared.tsx`: a `curl` one-liner for CI/CD + rollback jobs, a GitHub Actions snippet for a `revert` label or a + `git revert` commit message pattern, and a generic incident-tool webhook + shape (for PagerDuty/Opsgenie-style outbound webhooks) β€” reusing that file's + existing snippet-builder functions rather than writing new ones. + +**Soft guidance, not a hard requirement**: most orgs reading this panel will +have *no* existing revert/incident tracking at all β€” this isn't "connect +your existing system," it's "here's a new habit worth starting," and the +copy should say so plainly rather than assume a mature practice exists. +Concretely: +- An inline help callout: *"Don't have incident tracking yet? Start with + the easiest signal: mark a PR as reverted whenever you `git revert` it or + roll back a deploy because of it. Even a few marks a month are enough for + the risk model to start learning your repo's real risk patterns β€” it + doesn't need to be exhaustive."* +- The **manual "Mark as reverted/incident" button** in the review detail + page (already in scope from Β§1) is positioned as the primary, always-available + path β€” not a fallback for when the webhook isn't wired up. The webhook is + the "once you're ready to automate this" upgrade, not a prerequisite for + the feature doing anything useful. +- **Permissions default to open, with a lockdown switch**: by default, any + org member can click "Mark as reverted/incident" (reporting an incident + is exactly the kind of thing you want zero friction for, and restricting + it by default would undercut "start the habit easily"). A toggle in the + Production Feedback sub-tab β€” `restrict_outcome_reporting_to_admins` + (Β§1) β€” lets an org tighten this to owner/admin-only once they're worried + about noisy or mistaken reports skewing the model; the webhook endpoint + is always authenticated by its own secret regardless of this setting, so + the toggle only affects the in-app button. +- A small "last 30 days" counter on the setup panel β€” "3 outcomes recorded + (2 manual, 1 webhook)" β€” so an org can see at a glance whether this + signal is flowing at all, since a silently-empty `review_outcomes` table + is the single biggest way this whole feedback loop quietly does nothing. +- Explicitly do *not* promise automatic revert detection in this UI copy β€” + the non-goal from earlier in this plan β€” so expectations match what's + actually built (manual + webhook, not "we'll figure it out for you"). + +### 11. Metrics dashboard (wait time, revert rate, incident rate) + +New read-only panel (either its own page or a card on the existing analytics +page) computing the paper's three headline metrics from data this plan +already creates: +- **Wait time**: `policy_result published_at - review started_at` (or PR-open + time), from `review_policy_log` timestamps β€” no new instrumentation needed. +- **Revert rate / incident rate**: `count(review_outcomes) / count(auto-approved + reviews)` over a trailing window, compared against the same ratio for + human-reviewed PRs in the same repo β€” directly the paper's "1/3 revert + rate" style comparison, but will read near-zero/meaningless until + `review_outcomes` has real data (expected β€” the ingestion point existing is + what matters now, not the chart being populated day one). +- **Force-approval audit log**: a simple table view over + `review_policy_overrides` (Β§1/Β§7) β€” who, when, which review, what reason, + and whether that review later got a `review_outcomes` row β€” the most + direct "is this policy too strict" signal available, since every force + approval is a human explicitly disagreeing with the engine. + +## Delivery order + +1. **Schema + eligibility + policy CRUD** (reuse `ci_rulesets_handler.go` + patterns): `users.review_level`, `auto_approve_policies`, + `review_policy_log`, `review_outcomes`, `review_policy_overrides`, + `risk_feature_snapshots`, `risk_model_versions` migrations; + `internal/policyengine` package; new API handler + routes (including + `POST /api/v1/reviews/:id/force-approve`). No UI yet β€” policy rows + settable via SQL/API for internal testing. +2. **`internal/riskscore`**: feature extraction, the `goml`-based + `linear.Logistic` scorer (online + batch modes), `risk_model_versions`/ + `risk_feature_snapshots`/`risk_factor_overrides` migrations, the + seed/cold-start model (batch-fit once against weak labels, since + `review_outcomes` starts empty), the online-update hook fired from + `review_outcomes` inserts, and the daily River batch-refit job + its + on-demand API trigger. +3. **Rewire `pr_policy_worker.go`** onto the real engine (eligibility + + reused jq-ruleset gates + the trained risk scorer) β€” ships a materially + better auto-approve than the current placeholder even before the LLM + classification stage lands. +4. **LLM semantic classification stage** (Β§5) β€” additive gate, doesn't block + shipping step 3 first; extends the feature vector for future retrains. +5. **Review detail "Auto Approve" tab** (Β§7) + **Settings UI** (Β§8): + policy editor sub-tabs and simulation panel. +6. **Repository-level overrides** (Β§9): `RepoAutoApproveSection.tsx` and + the repo-settings `mode` picker, reusing Β§8's sub-tab components. +7. **Outcome ingestion UX** (Β§10): manual mark-as-reverted action, the + webhook setup/test/guidance panel, and excluding `source = 'test'` rows + from training queries. +8. **Metrics dashboard** (Β§11). + +## Verification + +- Unit tests for `internal/riskscore` (feature extraction + scorer math) and + `internal/policyengine` (each stage's pass/fail logic), following existing + test patterns in `internal/api/ci_gate_script_e2e_test.go`. +- End-to-end: trigger a review via the existing PR-link flow against a real + test PR, confirm `review_policy_log` captures all stages and the posted + GitHub/GitLab comment renders the funnel correctly for both an approve and + a blocked case. +- Settings UI: start dev server, exercise the policy editor and simulation + panel against real historical reviews in a test org, verify `npx tsc + --noEmit` passes (no full build without separate approval, per standing + project rule). diff --git a/go.mod b/go.mod index 34888b769..3300291b7 100644 --- a/go.mod +++ b/go.mod @@ -69,6 +69,8 @@ require ( google.golang.org/api v0.272.0 ) +require github.com/cdipaolo/goml v0.0.0-20220715001353-00e0c845ae1c // indirect + require ( cel.dev/expr v0.25.2 // indirect cloud.google.com/go v0.123.0 // indirect diff --git a/go.sum b/go.sum index ae5e3ecf2..7fc425f84 100644 --- a/go.sum +++ b/go.sum @@ -162,6 +162,8 @@ github.com/bytedance/sonic v1.15.0 h1:/PXeWFaR5ElNcVE84U0dOHjiMHQOwNIx3K4ymzh/uS github.com/bytedance/sonic v1.15.0/go.mod h1:tFkWrPz0/CUCLEF4ri4UkHekCIcdnkqXw9VduqpJh0k= github.com/bytedance/sonic/loader v0.5.0 h1:gXH3KVnatgY7loH5/TkeVyXPfESoqSBSBEiDd5VjlgE= github.com/bytedance/sonic/loader v0.5.0/go.mod h1:AR4NYCk5DdzZizZ5djGqQ92eEhCCcdf5x77udYiSJRo= +github.com/cdipaolo/goml v0.0.0-20220715001353-00e0c845ae1c h1:uqJXOhayPfl/QruVBP6VF0KUWNDzO/F14X8CPEkkFD8= +github.com/cdipaolo/goml v0.0.0-20220715001353-00e0c845ae1c/go.mod h1:Ue8jgVLdBDCtsh1laikvraXqXzKCyKiruCcCcaeNDFE= github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= diff --git a/internal/ai/gemini/batch.go b/internal/ai/gemini/batch.go index 13f8abd42..da94b74a0 100644 --- a/internal/ai/gemini/batch.go +++ b/internal/ai/gemini/batch.go @@ -160,6 +160,7 @@ func (p *GeminiProvider) ReviewCodeBatch( FileSummary: trimmedSummary, TechnicalSummaries: summaries, Comments: result.Comments, + DiffClassification: result.DiffClassification, Error: nil, }, nil } diff --git a/internal/ai/gemini/gemini.go b/internal/ai/gemini/gemini.go index 8da843b2c..070cbb6a2 100644 --- a/internal/ai/gemini/gemini.go +++ b/internal/ai/gemini/gemini.go @@ -351,8 +351,9 @@ func (p *GeminiProvider) parseResponse(response string, diffs []*models.CodeDiff } type GeminiResponse struct { - FileSummary string `json:"fileSummary"` - Comments []GeminiComment `json:"comments"` + FileSummary string `json:"fileSummary"` + Comments []GeminiComment `json:"comments"` + DiffClassification *models.DiffClassification `json:"diffClassification"` } // Try to parse the JSON @@ -444,8 +445,9 @@ func (p *GeminiProvider) parseResponse(response string, diffs []*models.CodeDiff // Create the result result := &models.ReviewResult{ - Summary: jsonResponse.FileSummary, // Store fileSummary temporarily in Summary field - Comments: []*models.ReviewComment{}, + Summary: jsonResponse.FileSummary, // Store fileSummary temporarily in Summary field + Comments: []*models.ReviewComment{}, + DiffClassification: jsonResponse.DiffClassification, } // Convert comments diff --git a/internal/ai/langchain/provider.go b/internal/ai/langchain/provider.go index 03c6b9a5a..ab16f0577 100644 --- a/internal/ai/langchain/provider.go +++ b/internal/ai/langchain/provider.go @@ -1339,6 +1339,7 @@ func (p *LangchainProvider) reviewCodeBatchFormatted(ctx context.Context, diffs FileSummary: legacySummary, TechnicalSummaries: result.TechnicalSummaries, Comments: result.Comments, + DiffClassification: result.DiffClassification, Error: nil, BatchID: batchId, }, nil @@ -1515,6 +1516,9 @@ func (p *LangchainProvider) parseResponse(response string, diffs []models.CodeDi FileSummaries []aiResponseFileSummary `json:"fileSummaries"` FileSummary string `json:"fileSummary"` // legacy fallback Comments []Comment `json:"comments"` + // DiffClassification is the diff-level semantic classification + // (docs/risk-policy-engine.md Β§5). + DiffClassification *models.DiffClassification `json:"diffClassification"` } // Try to extract JSON from the response @@ -1589,14 +1593,16 @@ func (p *LangchainProvider) parseResponse(response string, diffs []models.CodeDi } return &ParsedResult{ - TechnicalSummaries: summaries, - Comments: comments, + TechnicalSummaries: summaries, + Comments: comments, + DiffClassification: resp.DiffClassification, }, nil } type ParsedResult struct { - TechnicalSummaries []prompts.TechnicalSummary - Comments []*models.ReviewComment + TechnicalSummaries []prompts.TechnicalSummary + Comments []*models.ReviewComment + DiffClassification *models.DiffClassification } // fallbackParsedResult builds a minimal, user-visible result when the model response is empty/invalid diff --git a/internal/api/auto_approve_policy_handlers.go b/internal/api/auto_approve_policy_handlers.go new file mode 100644 index 000000000..337e80512 --- /dev/null +++ b/internal/api/auto_approve_policy_handlers.go @@ -0,0 +1,424 @@ +package api + +import ( + "context" + "database/sql" + "net/http" + "strconv" + + "github.com/labstack/echo/v4" + "github.com/lib/pq" + "github.com/livereview/internal/api/auth" + "github.com/livereview/internal/policyengine" +) + +// AutoApprovePoliciesHandler implements the auto-approve policy CRUD, +// resolution, and simulation API. See docs/risk-policy-engine.md Β§1/Β§8. +type AutoApprovePoliciesHandler struct { + db *sql.DB + engine *policyengine.Engine +} + +func NewAutoApprovePoliciesHandler(db *sql.DB) *AutoApprovePoliciesHandler { + return &AutoApprovePoliciesHandler{db: db, engine: policyengine.NewEngine(db)} +} + +// PolicyDTO is the persisted/returned shape of an auto_approve_policies row. +type PolicyDTO struct { + ID int64 `json:"id"` + OrgID int64 `json:"org_id"` + RepositoryID *int64 `json:"repository_id,omitempty"` + Mode string `json:"mode"` + MinAuthorLevel int `json:"min_author_level"` + EligibilityWhitelistUserIDs []int64 `json:"eligibility_whitelist_user_ids"` + RiskThresholdPercentile int `json:"risk_threshold_percentile"` + RequireLLMConfidence int `json:"require_llm_confidence"` + BlockingRulesetIDs []int64 `json:"blocking_ruleset_ids"` + LandingDelaySeconds int `json:"landing_delay_seconds"` + DailyApproveCap *int `json:"daily_approve_cap,omitempty"` + RestrictOutcomeReportingToAdmins bool `json:"restrict_outcome_reporting_to_admins"` + ForceApproveMinLevel int `json:"force_approve_min_level"` +} + +type policyUpsertRequest struct { + RepositoryID *int64 `json:"repository_id"` + Mode *string `json:"mode"` + MinAuthorLevel *int `json:"min_author_level"` + EligibilityWhitelistUserIDs []int64 `json:"eligibility_whitelist_user_ids"` + RiskThresholdPercentile *int `json:"risk_threshold_percentile"` + RequireLLMConfidence *int `json:"require_llm_confidence"` + BlockingRulesetIDs []int64 `json:"blocking_ruleset_ids"` + LandingDelaySeconds *int `json:"landing_delay_seconds"` + DailyApproveCap *int `json:"daily_approve_cap"` + RestrictOutcomeReportingToAdmins *bool `json:"restrict_outcome_reporting_to_admins"` + ForceApproveMinLevel *int `json:"force_approve_min_level"` +} + +func scanPolicy(row interface{ Scan(...interface{}) error }) (*PolicyDTO, error) { + var p PolicyDTO + var repoID sql.NullInt64 + err := row.Scan( + &p.ID, &p.OrgID, &repoID, &p.Mode, &p.MinAuthorLevel, + pq.Array(&p.EligibilityWhitelistUserIDs), &p.RiskThresholdPercentile, + &p.RequireLLMConfidence, pq.Array(&p.BlockingRulesetIDs), &p.LandingDelaySeconds, + &p.DailyApproveCap, &p.RestrictOutcomeReportingToAdmins, &p.ForceApproveMinLevel, + ) + if err != nil { + return nil, err + } + if repoID.Valid { + p.RepositoryID = &repoID.Int64 + } + return &p, nil +} + +const policyColumns = `id, org_id, repository_id, mode, min_author_level, + eligibility_whitelist_user_ids, risk_threshold_percentile, + require_llm_confidence, blocking_ruleset_ids, landing_delay_seconds, + daily_approve_cap, restrict_outcome_reporting_to_admins, force_approve_min_level` + +// List returns all policies for the org (org-default + repo-scoped). +func (h *AutoApprovePoliciesHandler) List(c echo.Context) error { + orgID, ok := auth.GetOrgIDFromContext(c) + if !ok { + return JSONErrorWithEnvelope(c, http.StatusUnauthorized, "org context required") + } + rows, err := h.db.QueryContext(c.Request().Context(), + `SELECT `+policyColumns+` FROM auto_approve_policies WHERE org_id = $1 ORDER BY repository_id NULLS FIRST`, orgID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "list failed: "+err.Error()) + } + defer rows.Close() + + out := []*PolicyDTO{} + for rows.Next() { + p, err := scanPolicy(rows) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "scan failed: "+err.Error()) + } + out = append(out, p) + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"rows": out}) +} + +// Get returns one policy by id. +func (h *AutoApprovePoliciesHandler) Get(c echo.Context) error { + orgID, ok := auth.GetOrgIDFromContext(c) + if !ok { + return JSONErrorWithEnvelope(c, http.StatusUnauthorized, "org context required") + } + id, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid policy id") + } + row := h.db.QueryRowContext(c.Request().Context(), + `SELECT `+policyColumns+` FROM auto_approve_policies WHERE id = $1 AND org_id = $2`, id, orgID) + p, err := scanPolicy(row) + if err == sql.ErrNoRows { + return JSONErrorWithEnvelope(c, http.StatusNotFound, "policy not found") + } + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "get failed: "+err.Error()) + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"policy": p}) +} + +// Create inserts a new policy (org-default or repo-scoped). +func (h *AutoApprovePoliciesHandler) Create(c echo.Context) error { + pc := auth.MustGetPermissionContext(c) + if err := pc.RequireOrgOwner(); err != nil { + return JSONErrorWithEnvelope(c, http.StatusForbidden, err.Error()) + } + orgID := pc.GetOrgID() + + var body policyUpsertRequest + if err := c.Bind(&body); err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid body") + } + mode := "custom" + if body.Mode != nil && *body.Mode != "" { + mode = *body.Mode + } + + // A repo-scoped row that omits a field should inherit the org default for + // that field, not the bare schema default β€” otherwise picking "Customize" + // on a repo can silently loosen a stricter org-wide setting (e.g. eligibility). + defaults := policyUpsertRequest{ + MinAuthorLevel: policyIntPtr(1), + RiskThresholdPercentile: policyIntPtr(20), + RequireLLMConfidence: policyIntPtr(8), + LandingDelaySeconds: policyIntPtr(0), + ForceApproveMinLevel: policyIntPtr(3), + } + if body.RepositoryID != nil { + if orgDefault, err := loadOrgDefaultPolicy(c.Request().Context(), h.db, orgID); err == nil && orgDefault != nil { + defaults = policyUpsertRequest{ + MinAuthorLevel: &orgDefault.MinAuthorLevel, + EligibilityWhitelistUserIDs: orgDefault.EligibilityWhitelistUserIDs, + RiskThresholdPercentile: &orgDefault.RiskThresholdPercentile, + RequireLLMConfidence: &orgDefault.RequireLLMConfidence, + BlockingRulesetIDs: orgDefault.BlockingRulesetIDs, + LandingDelaySeconds: &orgDefault.LandingDelaySeconds, + DailyApproveCap: orgDefault.DailyApproveCap, + RestrictOutcomeReportingToAdmins: &orgDefault.RestrictOutcomeReportingToAdmins, + ForceApproveMinLevel: &orgDefault.ForceApproveMinLevel, + } + } + } + if body.MinAuthorLevel == nil { + body.MinAuthorLevel = defaults.MinAuthorLevel + } + if body.EligibilityWhitelistUserIDs == nil { + body.EligibilityWhitelistUserIDs = defaults.EligibilityWhitelistUserIDs + } + if body.RiskThresholdPercentile == nil { + body.RiskThresholdPercentile = defaults.RiskThresholdPercentile + } + if body.RequireLLMConfidence == nil { + body.RequireLLMConfidence = defaults.RequireLLMConfidence + } + if body.BlockingRulesetIDs == nil { + body.BlockingRulesetIDs = defaults.BlockingRulesetIDs + } + if body.LandingDelaySeconds == nil { + body.LandingDelaySeconds = defaults.LandingDelaySeconds + } + if body.DailyApproveCap == nil { + body.DailyApproveCap = defaults.DailyApproveCap + } + if body.RestrictOutcomeReportingToAdmins == nil { + body.RestrictOutcomeReportingToAdmins = defaults.RestrictOutcomeReportingToAdmins + } + if body.ForceApproveMinLevel == nil { + body.ForceApproveMinLevel = defaults.ForceApproveMinLevel + } + + var id int64 + err := h.db.QueryRowContext(c.Request().Context(), ` + INSERT INTO auto_approve_policies (org_id, repository_id, mode, min_author_level, + eligibility_whitelist_user_ids, risk_threshold_percentile, require_llm_confidence, + blocking_ruleset_ids, landing_delay_seconds, daily_approve_cap, + restrict_outcome_reporting_to_admins, force_approve_min_level, created_by) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13) RETURNING id`, + orgID, body.RepositoryID, mode, derefInt(body.MinAuthorLevel, 1), + pq.Array(body.EligibilityWhitelistUserIDs), derefInt(body.RiskThresholdPercentile, 20), + derefInt(body.RequireLLMConfidence, 8), pq.Array(body.BlockingRulesetIDs), + derefInt(body.LandingDelaySeconds, 0), body.DailyApproveCap, + derefBool(body.RestrictOutcomeReportingToAdmins, false), derefInt(body.ForceApproveMinLevel, 3), + pc.GetUserID(), + ).Scan(&id) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "create failed: "+err.Error()) + } + + row := h.db.QueryRowContext(c.Request().Context(), + `SELECT `+policyColumns+` FROM auto_approve_policies WHERE id = $1 AND org_id = $2`, id, orgID) + p, err := scanPolicy(row) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "created but failed to reload") + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"policy": p}) +} + +// Update modifies a policy. +func (h *AutoApprovePoliciesHandler) Update(c echo.Context) error { + pc := auth.MustGetPermissionContext(c) + if err := pc.RequireOrgOwner(); err != nil { + return JSONErrorWithEnvelope(c, http.StatusForbidden, err.Error()) + } + orgID := pc.GetOrgID() + id, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid policy id") + } + + var body policyUpsertRequest + if err := c.Bind(&body); err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid body") + } + + _, err = h.db.ExecContext(c.Request().Context(), ` + UPDATE auto_approve_policies SET + mode = COALESCE($1, mode), + min_author_level = COALESCE($2, min_author_level), + eligibility_whitelist_user_ids = $3, + risk_threshold_percentile = COALESCE($4, risk_threshold_percentile), + require_llm_confidence = COALESCE($5, require_llm_confidence), + blocking_ruleset_ids = $6, + landing_delay_seconds = COALESCE($7, landing_delay_seconds), + daily_approve_cap = $8, + restrict_outcome_reporting_to_admins = COALESCE($9, restrict_outcome_reporting_to_admins), + force_approve_min_level = COALESCE($10, force_approve_min_level), + updated_at = now() + WHERE id = $11 AND org_id = $12`, + body.Mode, body.MinAuthorLevel, + pq.Array(body.EligibilityWhitelistUserIDs), body.RiskThresholdPercentile, + body.RequireLLMConfidence, pq.Array(body.BlockingRulesetIDs), + body.LandingDelaySeconds, body.DailyApproveCap, + body.RestrictOutcomeReportingToAdmins, body.ForceApproveMinLevel, + id, orgID, + ) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "update failed: "+err.Error()) + } + + row := h.db.QueryRowContext(c.Request().Context(), + `SELECT `+policyColumns+` FROM auto_approve_policies WHERE id = $1 AND org_id = $2`, id, orgID) + p, err := scanPolicy(row) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusNotFound, "policy not found") + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"policy": p}) +} + +// Delete removes a policy. +func (h *AutoApprovePoliciesHandler) Delete(c echo.Context) error { + pc := auth.MustGetPermissionContext(c) + if err := pc.RequireOrgOwner(); err != nil { + return JSONErrorWithEnvelope(c, http.StatusForbidden, err.Error()) + } + orgID := pc.GetOrgID() + id, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid policy id") + } + res, err := h.db.ExecContext(c.Request().Context(), `DELETE FROM auto_approve_policies WHERE id = $1 AND org_id = $2`, id, orgID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "delete failed: "+err.Error()) + } + if n, _ := res.RowsAffected(); n == 0 { + return JSONErrorWithEnvelope(c, http.StatusNotFound, "policy not found") + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"status": "deleted"}) +} + +// Resolved returns the effective policy for a repository (repo-scoped or org +// default), used by the repository settings UI to show "org default" context. +func (h *AutoApprovePoliciesHandler) Resolved(c echo.Context) error { + orgID, ok := auth.GetOrgIDFromContext(c) + if !ok { + return JSONErrorWithEnvelope(c, http.StatusUnauthorized, "org context required") + } + var repoID int64 + if v := c.QueryParam("repository_id"); v != "" { + repoID, _ = strconv.ParseInt(v, 10, 64) + } + p, found, err := policyengine.ResolvePolicy(c.Request().Context(), h.db, orgID, repoID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "resolve failed: "+err.Error()) + } + if !found { + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"policy": nil, "configured": false}) + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"policy": p, "configured": true}) +} + +// Simulate replays a draft policy against the org's recent completed reviews, +// discarding all results (dry-run). See docs/risk-policy-engine.md Β§8. +func (h *AutoApprovePoliciesHandler) Simulate(c echo.Context) error { + orgID, ok := auth.GetOrgIDFromContext(c) + if !ok { + return JSONErrorWithEnvelope(c, http.StatusUnauthorized, "org context required") + } + var body struct { + RepositoryID *int64 `json:"repository_id"` + Policy policyUpsertRequest `json:"policy"` + ReviewIDs []int64 `json:"review_ids"` + Limit int `json:"limit"` + } + if err := c.Bind(&body); err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid body") + } + if body.Limit <= 0 || body.Limit > 100 { + body.Limit = 20 + } + + draft := &policyengine.Policy{ + OrgID: orgID, + RepositoryID: body.RepositoryID, + Mode: "custom", + MinAuthorLevel: derefInt(body.Policy.MinAuthorLevel, 1), + EligibilityWhitelistUserIDs: body.Policy.EligibilityWhitelistUserIDs, + RiskThresholdPercentile: derefInt(body.Policy.RiskThresholdPercentile, 20), + RequireLLMConfidence: derefInt(body.Policy.RequireLLMConfidence, 8), + BlockingRulesetIDs: body.Policy.BlockingRulesetIDs, + LandingDelaySeconds: derefInt(body.Policy.LandingDelaySeconds, 0), + DailyApproveCap: body.Policy.DailyApproveCap, + RestrictOutcomeReportingToAdmins: derefBool(body.Policy.RestrictOutcomeReportingToAdmins, false), + ForceApproveMinLevel: derefInt(body.Policy.ForceApproveMinLevel, 3), + } + if body.Policy.Mode != nil { + draft.Mode = *body.Policy.Mode + } + + reviewIDs := body.ReviewIDs + if len(reviewIDs) == 0 { + rows, err := h.db.QueryContext(c.Request().Context(), ` + SELECT id FROM reviews + WHERE org_id = $1 AND status = 'completed' + ORDER BY created_at DESC LIMIT $2`, orgID, body.Limit) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "failed to load reviews: "+err.Error()) + } + defer rows.Close() + for rows.Next() { + var id int64 + if err := rows.Scan(&id); err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "scan failed: "+err.Error()) + } + reviewIDs = append(reviewIDs, id) + } + } + + type simRow struct { + ReviewID int64 `json:"review_id"` + Approved bool `json:"would_approve"` + BlockedAt string `json:"blocked_at,omitempty"` + Score float64 `json:"score,omitempty"` + Stages []policyengine.StageResult `json:"stages"` + } + out := []simRow{} + for _, rid := range reviewIDs { + res, err := h.engine.RunWithPolicy(c.Request().Context(), orgID, rid, draft, true) + if err != nil { + continue + } + row := simRow{ReviewID: rid, Approved: res.Approved, Score: res.Score, Stages: res.Stages} + for _, st := range res.Stages { + if st.Verdict == policyengine.VerdictFail { + row.BlockedAt = st.Stage + break + } + } + out = append(out, row) + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"rows": out}) +} + +// loadOrgDefaultPolicy returns the org-default (repository_id IS NULL) policy +// row, or (nil, nil) if none is configured yet. +func loadOrgDefaultPolicy(ctx context.Context, db *sql.DB, orgID int64) (*PolicyDTO, error) { + row := db.QueryRowContext(ctx, + `SELECT `+policyColumns+` FROM auto_approve_policies WHERE org_id = $1 AND repository_id IS NULL`, orgID) + p, err := scanPolicy(row) + if err == sql.ErrNoRows { + return nil, nil + } + return p, err +} + +func policyIntPtr(v int) *int { return &v } + +func derefInt(p *int, d int) int { + if p == nil { + return d + } + return *p +} + +func derefBool(p *bool, d bool) bool { + if p == nil { + return d + } + return *p +} diff --git a/internal/api/ci_rulesets_handler.go b/internal/api/ci_rulesets_handler.go index f7ce58e4d..b0a9161da 100644 --- a/internal/api/ci_rulesets_handler.go +++ b/internal/api/ci_rulesets_handler.go @@ -3,7 +3,6 @@ package api import ( "context" "database/sql" - "encoding/json" "fmt" "net/http" "strconv" @@ -13,6 +12,7 @@ import ( "github.com/itchyny/gojq" "github.com/labstack/echo/v4" "github.com/livereview/internal/api/auth" + "github.com/livereview/internal/canonicaldoc" storagereviews "github.com/livereview/storage/reviews" ) @@ -54,157 +54,22 @@ type upsertCIRulesetRequest struct { // ---- canonical review document ---- -// canonicalFinding is one row of the "findings" array in the canonical doc. -type canonicalFinding struct { - Severity string `json:"severity"` - Confidence string `json:"confidence"` - Category string `json:"category"` - Subcategory string `json:"subcategory"` - Type string `json:"type"` - FilePath string `json:"file_path,omitempty"` - LineNumber *int `json:"line_number,omitempty"` -} - -// canonicalReviewDoc is the stable, provider-agnostic document every jq -// ruleset is evaluated against, regardless of whether the review was -// triggered by a webhook, `lrc`, or the web UI. -type canonicalReviewDoc struct { - ReviewID int64 `json:"review_id"` - OrgID int64 `json:"org_id"` - Repository string `json:"repository"` - Provider string `json:"provider"` - Status string `json:"status"` - Findings []canonicalFinding `json:"findings"` - Counts canonicalReviewDocCounts `json:"counts"` -} +// The canonical document types and jq evaluation are shared with the +// risk-policy engine's static gate stage via internal/canonicaldoc, so there +// is exactly one rule engine (see docs/risk-policy-engine.md Β§4). These +// aliases keep the existing handler API and its tests unchanged. +type canonicalFinding = canonicaldoc.Finding +type canonicalReviewDoc = canonicaldoc.ReviewDoc +type canonicalReviewDocCounts = canonicaldoc.Counts -type canonicalReviewDocCounts struct { - BySeverity map[string]int `json:"by_severity"` - ByCategory map[string]int `json:"by_category"` - Total int `json:"total"` -} - -// reviewGateHeader is the minimal review row needed to build the canonical -// doc's header fields (everything but findings). -type reviewGateHeader struct { - ID int64 - Repository string - Provider string - Status string -} - -// getReviewGateHeader fetches a review scoped strictly to orgID -- never -// trust a caller-supplied review_id without confirming org ownership first. -func (h *CIRulesetsHandler) getReviewGateHeader(ctx context.Context, orgID, reviewID int64) (*reviewGateHeader, error) { - var r reviewGateHeader - var provider sql.NullString - err := h.db.QueryRowContext(ctx, ` - SELECT id, repository, provider, status - FROM reviews - WHERE id = $1 AND org_id = $2`, reviewID, orgID).Scan(&r.ID, &r.Repository, &provider, &r.Status) - if err != nil { - return nil, err - } - r.Provider = provider.String - return &r, nil -} - -// knownSeverities seeds counts.by_severity with zero values so jq -// expressions like `.counts.by_severity.critical > 0` never fail with "null -// cannot be compared" on a clean review -- they just see 0. This is the -// actual 3-value severity model the AI review prompt emits (see -// internal/prompts/templates.go's "info|warning|critical" schema) -- not a -// generic 5-level guess. -var knownSeverities = []string{"critical", "warning", "info"} +var knownSeverities = canonicaldoc.KnownSeverities +var runJQBool = canonicaldoc.RunJQBool +// buildCanonicalReviewDoc delegates to internal/canonicaldoc.Build, scoping the +// review strictly to orgID -- never trust a caller-supplied review_id without +// confirming org ownership first. func (h *CIRulesetsHandler) buildCanonicalReviewDoc(ctx context.Context, orgID, reviewID int64) (*canonicalReviewDoc, error) { - header, err := h.getReviewGateHeader(ctx, orgID, reviewID) - if err != nil { - return nil, err - } - - doc := &canonicalReviewDoc{ - ReviewID: header.ID, - OrgID: orgID, - Repository: header.Repository, - Provider: header.Provider, - Status: header.Status, - Findings: []canonicalFinding{}, - Counts: canonicalReviewDocCounts{ - BySeverity: map[string]int{}, - ByCategory: map[string]int{}, - }, - } - for _, sev := range knownSeverities { - doc.Counts.BySeverity[sev] = 0 - } - - rows, _, err := h.taxonomy.ListFindings(ctx, storagereviews.TaxonomyFilter{OrgID: orgID, ReviewID: reviewID}, 500, 0, storagereviews.TaxonomyFindingsOptions{}) - if err != nil { - return nil, fmt.Errorf("loading findings: %w", err) - } - for _, r := range rows { - sev := strings.ToLower(r.Severity) - cat := strings.ToLower(r.Category) - f := canonicalFinding{ - Severity: sev, - Confidence: strings.ToLower(r.Confidence), - Category: cat, - Subcategory: strings.ToLower(r.Subcategory), - Type: strings.ToLower(r.IssueType), - LineNumber: r.LineNumber, - } - if r.FilePath != nil { - f.FilePath = *r.FilePath - } - doc.Findings = append(doc.Findings, f) - doc.Counts.BySeverity[sev]++ - if cat != "" { - doc.Counts.ByCategory[cat]++ - } - doc.Counts.Total++ - } - return doc, nil -} - -// runJQBool evaluates expr against doc and returns whether the first -// produced value is truthy (jq truthiness: everything except false/null). -// It also returns the raw first result for display/debugging. -func runJQBool(expr string, doc interface{}) (blocked bool, result interface{}, err error) { - query, err := gojq.Parse(expr) - if err != nil { - return false, nil, fmt.Errorf("invalid jq expression: %w", err) - } - - // Round-trip through JSON so the doc matches what gojq expects - // (map[string]interface{}/[]interface{}/plain scalars). - raw, err := json.Marshal(doc) - if err != nil { - return false, nil, fmt.Errorf("marshaling document: %w", err) - } - // nosemgrep: go.lang.security.deserialization.unsafe-deserialization-interface.go-unsafe-deserialization-interface -- raw is our own json.Marshal output from just above; gojq requires generic map/slice/scalar input, and encoding/json into interface{} can only produce those, never arbitrary types. - var input interface{} - if err := json.Unmarshal(raw, &input); err != nil { - return false, nil, fmt.Errorf("unmarshaling document: %w", err) - } - - iter := query.Run(input) - v, ok := iter.Next() - if !ok { - return false, nil, fmt.Errorf("jq expression produced no output") - } - if e, ok := v.(error); ok { - return false, nil, fmt.Errorf("jq evaluation error: %w", e) - } - switch t := v.(type) { - case bool: - blocked = t - case nil: - blocked = false - default: - blocked = true - } - return blocked, v, nil + return canonicaldoc.Build(ctx, h.db, orgID, reviewID) } // ---- CRUD ---- diff --git a/internal/api/identity_handlers.go b/internal/api/identity_handlers.go new file mode 100644 index 000000000..8ff4c6778 --- /dev/null +++ b/internal/api/identity_handlers.go @@ -0,0 +1,175 @@ +package api + +import ( + "net/http" + "strconv" + "strings" + + "github.com/labstack/echo/v4" + "github.com/livereview/internal/api/auth" +) + +// UserIdentityDTO is one user_identities row. +type UserIdentityDTO struct { + ID int64 `json:"id"` + OrgID int64 `json:"org_id"` + UserID int64 `json:"user_id"` + Kind string `json:"kind"` + Value string `json:"value"` + CreatedBy *int64 `json:"created_by,omitempty"` +} + +// listUserIdentities handles GET /api/v1/orgs/:org_id/users/:user_id/identities. +func (s *Server) listUserIdentities(c echo.Context) error { + orgID, ok := auth.GetOrgIDFromContext(c) + if !ok { + return JSONErrorWithEnvelope(c, http.StatusUnauthorized, "org context required") + } + userID, err := strconv.ParseInt(c.Param("user_id"), 10, 64) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid user id") + } + + rows, err := s.db.QueryContext(c.Request().Context(), ` + SELECT id, org_id, user_id, kind, value, created_by + FROM user_identities WHERE org_id = $1 AND user_id = $2 ORDER BY kind, value`, orgID, userID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "query failed: "+err.Error()) + } + defer rows.Close() + + out := []UserIdentityDTO{} + for rows.Next() { + var d UserIdentityDTO + if err := rows.Scan(&d.ID, &d.OrgID, &d.UserID, &d.Kind, &d.Value, &d.CreatedBy); err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "scan failed: "+err.Error()) + } + out = append(out, d) + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"rows": out}) +} + +// addUserIdentity handles POST /api/v1/orgs/:org_id/users/:user_id/identities. +// Any member can add an identity for themselves; attaching to another user +// requires owner/admin. Session-only (no API key). +func (s *Server) addUserIdentity(c echo.Context) error { + if err := requireSession(c); err != nil { + return err + } + pc := auth.MustGetPermissionContext(c) + orgID := pc.GetOrgID() + targetUserID, err := strconv.ParseInt(c.Param("user_id"), 10, 64) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid user id") + } + + if targetUserID != pc.GetUserID() { + if err := pc.RequireOrgOwner(); err != nil { + return JSONErrorWithEnvelope(c, http.StatusForbidden, "only owners/admins can attach identities to another user") + } + } + + var body struct { + Kind string `json:"kind"` + Value string `json:"value"` + } + if err := c.Bind(&body); err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid body") + } + body.Kind = strings.ToLower(strings.TrimSpace(body.Kind)) + body.Value = normalizeIdentityValue(body.Kind, body.Value) + if body.Kind != "email" && body.Kind != "username" { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "kind must be email or username") + } + if body.Value == "" { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "value is required") + } + + var id int64 + var createdBy interface{} + if pc.User != nil { + createdBy = pc.User.ID + } + err = s.db.QueryRowContext(c.Request().Context(), ` + INSERT INTO user_identities (org_id, user_id, kind, value, created_by) + VALUES ($1, $2, $3, $4, $5) + ON CONFLICT (org_id, kind, value) DO UPDATE SET user_id = EXCLUDED.user_id, updated_at = now() + RETURNING id`, orgID, targetUserID, body.Kind, body.Value, createdBy).Scan(&id) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusConflict, "identity already maps to a different user in this org") + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"id": id, "kind": body.Kind, "value": body.Value}) +} + +// removeUserIdentity handles DELETE /api/v1/orgs/:org_id/users/:user_id/identities/:id. +func (s *Server) removeUserIdentity(c echo.Context) error { + if err := requireSession(c); err != nil { + return err + } + pc := auth.MustGetPermissionContext(c) + orgID := pc.GetOrgID() + targetUserID, err := strconv.ParseInt(c.Param("user_id"), 10, 64) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid user id") + } + identityID, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid identity id") + } + + // Any member can remove their own identities; others require owner/admin. + var owner int64 + if err := s.db.QueryRowContext(c.Request().Context(), + `SELECT user_id FROM user_identities WHERE id = $1 AND org_id = $2`, identityID, orgID).Scan(&owner); err != nil { + return JSONErrorWithEnvelope(c, http.StatusNotFound, "identity not found") + } + if owner != pc.GetUserID() && targetUserID != pc.GetUserID() { + if err := pc.RequireOrgOwner(); err != nil { + return JSONErrorWithEnvelope(c, http.StatusForbidden, "only owners/admins can remove another user's identity") + } + } + + res, err := s.db.ExecContext(c.Request().Context(), + `DELETE FROM user_identities WHERE id = $1 AND org_id = $2 AND user_id = $3`, identityID, orgID, targetUserID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "delete failed: "+err.Error()) + } + if n, _ := res.RowsAffected(); n == 0 { + return JSONErrorWithEnvelope(c, http.StatusNotFound, "identity not found") + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"status": "deleted"}) +} + +// listOrgIdentities handles GET /api/v1/orgs/:org_id/identities β€” the full index +// used for author-mapping lookup/prefill. +func (s *Server) listOrgIdentities(c echo.Context) error { + orgID, ok := auth.GetOrgIDFromContext(c) + if !ok { + return JSONErrorWithEnvelope(c, http.StatusUnauthorized, "org context required") + } + rows, err := s.db.QueryContext(c.Request().Context(), ` + SELECT id, org_id, user_id, kind, value, created_by + FROM user_identities WHERE org_id = $1 ORDER BY kind, value`, orgID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "query failed: "+err.Error()) + } + defer rows.Close() + + out := []UserIdentityDTO{} + for rows.Next() { + var d UserIdentityDTO + if err := rows.Scan(&d.ID, &d.OrgID, &d.UserID, &d.Kind, &d.Value, &d.CreatedBy); err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "scan failed: "+err.Error()) + } + out = append(out, d) + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"rows": out}) +} + +func normalizeIdentityValue(kind, value string) string { + v := strings.TrimSpace(value) + if kind == "email" { + return strings.ToLower(v) + } + return strings.ToLower(v) +} diff --git a/internal/api/organizations/org_service.go b/internal/api/organizations/org_service.go index e0b149c65..5c2f7dca0 100644 --- a/internal/api/organizations/org_service.go +++ b/internal/api/organizations/org_service.go @@ -491,7 +491,8 @@ func (s *OrganizationService) GetOrganizationMembers(orgID int64, limit, offset obs.billing_period_end as license_expires_at, ur.active_subscription_id, s.razorpay_subscription_id, - u.onboarding_api_key + u.onboarding_api_key, + u.review_level FROM users u INNER JOIN user_roles ur ON u.id = ur.user_id INNER JOIN roles r ON ur.role_id = r.id @@ -538,6 +539,7 @@ func (s *OrganizationService) GetOrganizationMembers(orgID int64, limit, offset &activeSubscriptionID, &razorpaySubscriptionID, &onboardingKey, + &user.ReviewLevel, ) if err != nil { s.logger.Printf("Error scanning member row: %v", err) diff --git a/internal/api/outcome_handlers.go b/internal/api/outcome_handlers.go new file mode 100644 index 000000000..ccf08a173 --- /dev/null +++ b/internal/api/outcome_handlers.go @@ -0,0 +1,116 @@ +package api + +import ( + "encoding/json" + "net/http" + "strings" + "time" + + "github.com/labstack/echo/v4" + "github.com/livereview/internal/api/auth" +) + +// orgReviewOutcome handles POST /api/v1/orgs/:org_id/review-outcomes β€” the +// documented webhook endpoint CI/CD or incident tooling POSTs to report a +// rollback/incident against a known review id. Authenticated by API key or JWT +// via the standard org middleware chain. +func (s *Server) orgReviewOutcome(c echo.Context) error { + orgID, ok := auth.GetOrgIDFromContext(c) + if !ok { + return JSONErrorWithEnvelope(c, http.StatusUnauthorized, "org context required") + } + + var body struct { + ReviewID int64 `json:"review_id"` + OutcomeType string `json:"outcome_type"` + Reason string `json:"reason"` + Source string `json:"source"` + Detail map[string]interface{} `json:"detail"` + } + if err := c.Bind(&body); err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid body") + } + if body.ReviewID <= 0 { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "review_id is required") + } + if body.OutcomeType != "revert" && body.OutcomeType != "incident" && body.OutcomeType != "confirmed_safe" { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid outcome_type") + } + body.Reason = strings.TrimSpace(body.Reason) + if body.Reason == "" { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "reason is required") + } + source := body.Source + if source == "" { + source = "webhook" + } + if source != "webhook" && source != "ci" && source != "test" { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid source") + } + + // Confirm the review belongs to this org before writing. + var exists bool + if err := s.db.QueryRowContext(c.Request().Context(), + `SELECT EXISTS(SELECT 1 FROM reviews WHERE id = $1 AND org_id = $2)`, body.ReviewID, orgID).Scan(&exists); err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "lookup failed: "+err.Error()) + } + if !exists { + return JSONErrorWithEnvelope(c, http.StatusNotFound, "review not found in this organization") + } + + if err := s.insertOutcome(c, orgID, body.ReviewID, 0, body.OutcomeType, source, body.Reason, body.Detail); err != nil { + return err + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"status": "recorded", "outcome_type": body.OutcomeType}) +} + +// listReviewOutcomes handles GET /api/v1/orgs/:org_id/review-outcomes β€” the +// "last 30 days" counter on the setup panel. +func (s *Server) listReviewOutcomes(c echo.Context) error { + orgID, ok := auth.GetOrgIDFromContext(c) + if !ok { + return JSONErrorWithEnvelope(c, http.StatusUnauthorized, "org context required") + } + + rows, err := s.db.QueryContext(c.Request().Context(), ` + SELECT review_id, reported_by_user_id, outcome_type, source, reason, detail, created_at + FROM review_outcomes + WHERE org_id = $1 AND created_at >= now() - interval '30 days' + ORDER BY created_at DESC LIMIT 500`, orgID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "query failed: "+err.Error()) + } + defer rows.Close() + + type outcomeRow struct { + ReviewID int64 `json:"review_id"` + ReportedBy *int64 `json:"reported_by_user_id,omitempty"` + OutcomeType string `json:"outcome_type"` + Source string `json:"source"` + Reason string `json:"reason"` + Detail json.RawMessage `json:"detail"` + CreatedAt time.Time `json:"created_at"` + } + out := []outcomeRow{} + manualCount, webhookCount := 0, 0 + for rows.Next() { + var r outcomeRow + var detail []byte + if err := rows.Scan(&r.ReviewID, &r.ReportedBy, &r.OutcomeType, &r.Source, &r.Reason, &detail, &r.CreatedAt); err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "scan failed: "+err.Error()) + } + r.Detail = detail + if r.Source == "manual" { + manualCount++ + } else if r.Source != "test" { + webhookCount++ + } + out = append(out, r) + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{ + "rows": out, + "total": len(out), + "manual_count": manualCount, + "webhook_count": webhookCount, + }) +} diff --git a/internal/api/review_policy_actions.go b/internal/api/review_policy_actions.go new file mode 100644 index 000000000..99f641b3d --- /dev/null +++ b/internal/api/review_policy_actions.go @@ -0,0 +1,260 @@ +package api + +import ( + "encoding/json" + "net/http" + "strconv" + "strings" + + "github.com/labstack/echo/v4" + "github.com/livereview/internal/api/auth" + "github.com/livereview/internal/jobqueue" + "github.com/livereview/internal/policyengine" + "github.com/livereview/internal/riskscore" +) + +// requireSession rejects API-key-authenticated requests (sensitive account +// actions require a live JWT session per the scoping guardrails). +func requireSession(c echo.Context) error { + if c.Get(authMethodContextKey) != authMethodSession { + return JSONErrorWithEnvelope(c, http.StatusForbidden, "this action requires an active user session") + } + return nil +} + +// getReviewPolicyLog handles GET /api/v1/reviews/:id/policy-log. +func (s *Server) getReviewPolicyLog(c echo.Context) error { + orgID, ok := auth.GetOrgIDFromContext(c) + if !ok { + return JSONErrorWithEnvelope(c, http.StatusUnauthorized, "org context required") + } + reviewID, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid review id") + } + + rows, err := s.db.QueryContext(c.Request().Context(), ` + SELECT stage, verdict, detail FROM review_policy_log + WHERE review_id = $1 AND org_id = $2 ORDER BY id`, reviewID, orgID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "query failed: "+err.Error()) + } + defer rows.Close() + + type stageRow struct { + Stage string `json:"stage"` + Verdict string `json:"verdict"` + Detail json.RawMessage `json:"detail"` + } + out := []stageRow{} + for rows.Next() { + var r stageRow + if err := rows.Scan(&r.Stage, &r.Verdict, &r.Detail); err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "scan failed: "+err.Error()) + } + out = append(out, r) + } + + // The feature snapshot for the risk stage's explainability view. + var snapshot struct { + ModelVersion int `json:"model_version"` + Features []byte `json:"-"` + Score float64 `json:"score"` + } + features := []float64{} + var featuresRaw []byte + var snapshotScore float64 + var modelVersion int + if err := s.db.QueryRowContext(c.Request().Context(), ` + SELECT model_version, features, score FROM risk_feature_snapshots + WHERE review_id = $1 AND org_id = $2`, reviewID, orgID). + Scan(&modelVersion, &featuresRaw, &snapshotScore); err == nil { + _ = json.Unmarshal(featuresRaw, &features) + snapshot.ModelVersion = modelVersion + snapshot.Score = snapshotScore + } + + // Force-approve eligibility context: is the review blocked, and what's the floor? + blocked := false + for _, r := range out { + if r.Stage == policyengine.StageFinal && r.Verdict == policyengine.FinalBlocked { + blocked = true + } + } + forceApproveMinLevel := 3 + if p, found, _ := policyengine.ResolvePolicy(c.Request().Context(), s.db, orgID, 0); found { + forceApproveMinLevel = p.ForceApproveMinLevel + } + + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{ + "stages": out, + "blocked": blocked, + "force_approve_min_level": forceApproveMinLevel, + "features": features, + "feature_names": riskscore.FeatureSpecs, + "model_version": modelVersion, + "score": snapshotScore, + }) +} + +// forceApproveReview handles POST /api/v1/reviews/:id/force-approve (session-only, +// gated by review_level OR owner/admin). +func (s *Server) forceApproveReview(c echo.Context) error { + if err := requireSession(c); err != nil { + return err + } + pc := auth.MustGetPermissionContext(c) + orgID := pc.GetOrgID() + reviewID, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid review id") + } + + var body struct { + Reason string `json:"reason"` + } + if err := c.Bind(&body); err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid body") + } + body.Reason = strings.TrimSpace(body.Reason) + if body.Reason == "" { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "reason is required") + } + + // Resolve the policy floor for force-approve. + forceApproveMinLevel := 3 + if p, found, _ := policyengine.ResolvePolicy(c.Request().Context(), s.db, orgID, 0); found { + forceApproveMinLevel = p.ForceApproveMinLevel + } + + // Authorize: review_level >= floor OR owner/admin. + var actorLevel int + if err := s.db.QueryRowContext(c.Request().Context(), `SELECT review_level FROM users WHERE id = $1`, pc.GetUserID()).Scan(&actorLevel); err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "failed to load user level") + } + if actorLevel < forceApproveMinLevel && !pc.IsOwner && !pc.IsSuperAdmin { + return JSONErrorWithEnvelope(c, http.StatusForbidden, "insufficient review level or role to force-approve") + } + + // Only blocked -> approved; there is no symmetric force-request-changes. + var finalVerdict string + if err := s.db.QueryRowContext(c.Request().Context(), ` + SELECT verdict FROM review_policy_log + WHERE review_id = $1 AND org_id = $2 AND stage = 'final' ORDER BY id DESC LIMIT 1`, + reviewID, orgID).Scan(&finalVerdict); err != nil { + return JSONErrorWithEnvelope(c, http.StatusNotFound, "no policy evaluation found for this review") + } + if finalVerdict != policyengine.FinalBlocked { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "review is not blocked") + } + + _, err = s.db.ExecContext(c.Request().Context(), ` + INSERT INTO review_policy_overrides (review_id, org_id, actor_user_id, original_verdict, forced_verdict, reason) + VALUES ($1, $2, $3, $4, $5, $6)`, + reviewID, orgID, pc.GetUserID(), finalVerdict, policyengine.FinalApproved, body.Reason) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "failed to record override: "+err.Error()) + } + + if err := s.jobQueue.QueuePRPolicyForceApprove(c.Request().Context(), jobqueue.PRPolicyForceApproveJobArgs{ + OrgID: orgID, + ReviewID: reviewID, + ActorUserID: pc.GetUserID(), + Reason: body.Reason, + }); err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "failed to queue force-approve: "+err.Error()) + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"status": "force-approved"}) +} + +// markReviewOutcome handles POST /api/v1/reviews/:id/outcomes (manual in-app +// "Mark as reverted/incident" action). +func (s *Server) markReviewOutcome(c echo.Context) error { + pc := auth.MustGetPermissionContext(c) + orgID := pc.GetOrgID() + reviewID, err := strconv.ParseInt(c.Param("id"), 10, 64) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid review id") + } + + var body struct { + OutcomeType string `json:"outcome_type"` + Reason string `json:"reason"` + } + if err := c.Bind(&body); err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid body") + } + body.Reason = strings.TrimSpace(body.Reason) + if body.Reason == "" { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "reason is required") + } + if body.OutcomeType != "revert" && body.OutcomeType != "incident" && body.OutcomeType != "confirmed_safe" { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid outcome_type") + } + + // Confirm the review belongs to this org before writing (never trust a + // caller-supplied review id without checking ownership first). + var exists bool + if err := s.db.QueryRowContext(c.Request().Context(), + `SELECT EXISTS(SELECT 1 FROM reviews WHERE id = $1 AND org_id = $2)`, reviewID, orgID).Scan(&exists); err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "lookup failed: "+err.Error()) + } + if !exists { + return JSONErrorWithEnvelope(c, http.StatusNotFound, "review not found in this organization") + } + + if err := s.insertOutcome(c, orgID, reviewID, pc.GetUserID(), body.OutcomeType, "manual", body.Reason, nil); err != nil { + return err + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"status": "recorded", "outcome_type": body.OutcomeType}) +} + +// insertOutcome inserts a review_outcomes row and (for non-test sources) fires +// the real-time online model nudge. +func (s *Server) insertOutcome(c echo.Context, orgID, reviewID, userID int64, outcomeType, source, reason string, detail map[string]interface{}) error { + // Permission lockdown switch (default open): when set, only owner/admin may + // report an outcome in-app. Webhooks are always authenticated by API key. + if source == "manual" && s.restrictOutcomeReporting(orgID) { + pc := auth.MustGetPermissionContext(c) + if !pc.IsOwner && !pc.IsSuperAdmin { + return JSONErrorWithEnvelope(c, http.StatusForbidden, "outcome reporting is restricted to admins") + } + } + + detailJSON := "{}" + if detail != nil { + if b, err := json.Marshal(detail); err == nil { + detailJSON = string(b) + } + } + var reportedBy interface{} + if userID > 0 { + reportedBy = userID + } + if _, err := s.db.ExecContext(c.Request().Context(), ` + INSERT INTO review_outcomes (review_id, org_id, reported_by_user_id, outcome_type, source, reason, detail) + VALUES ($1, $2, $3, $4, $5, $6, $7)`, + reviewID, orgID, reportedBy, outcomeType, source, reason, detailJSON); err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "failed to record outcome: "+err.Error()) + } + + // Real-time online nudge (path 1), skipped for test events. + if source != "test" { + store := riskscore.NewStore(s.db) + if err := store.ApplyOutcome(c.Request().Context(), orgID, reviewID, outcomeType); err != nil { + // Never fail the request over a learning update. + _ = err + } + } + return nil +} + +// restrictOutcomeReporting returns the org's lockdown switch. +func (s *Server) restrictOutcomeReporting(orgID int64) bool { + var restrict bool + if err := s.db.QueryRow(`SELECT COALESCE(restrict_outcome_reporting_to_admins, false) + FROM auto_approve_policies WHERE org_id = $1 AND repository_id IS NULL`, orgID).Scan(&restrict); err != nil { + return false + } + return restrict +} diff --git a/internal/api/review_policy_actions_test.go b/internal/api/review_policy_actions_test.go new file mode 100644 index 000000000..c82a1453b --- /dev/null +++ b/internal/api/review_policy_actions_test.go @@ -0,0 +1,134 @@ +package api + +import ( + "database/sql" + "net/http" + "net/http/httptest" + "os" + "strconv" + "strings" + "testing" + + "github.com/labstack/echo/v4" + _ "github.com/lib/pq" + + "github.com/livereview/internal/api/auth" +) + +// Regression test: markReviewOutcome (POST /api/v1/reviews/:id/outcomes) used +// to write a review_outcomes row without ever checking that the review +// actually belongs to the caller's org β€” unlike its webhook twin +// (orgReviewOutcome), which does. A member of org B could report a +// revert/incident against any review id from org A just by guessing it. This +// boots the real route with the real middleware chain and a real API key, and +// confirms org B is now rejected (404) and no row is written. +func TestMarkReviewOutcomeRejectsCrossOrgReview(t *testing.T) { + if testing.Short() { + t.Skip("skipping database integration test") + } + dsn := os.Getenv("DATABASE_URL") + if dsn == "" { + dsn = "postgres://livereview:livereview_password_123@localhost:5432/livereview?sslmode=disable" + } + db, err := sql.Open("postgres", dsn) + if err != nil { + t.Fatalf("open db: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.Ping(); err != nil { + t.Skipf("db not reachable: %v", err) + } + + // The live dev server's background dashboard-cache refresh job may insert a + // row for a new org before cleanup runs; that FK has no cascade, so it must + // be cleared first or the org delete silently no-ops. + cleanupOrg := func(orgID int64) { + t.Cleanup(func() { + _, _ = db.Exec(`DELETE FROM dashboard_cache WHERE org_id = $1`, orgID) + _, _ = db.Exec(`DELETE FROM orgs WHERE id = $1`, orgID) + }) + } + + // ---- org A owns the review ---- + var orgAID int64 + if err := db.QueryRow(`INSERT INTO orgs (name) VALUES ('outcome-isolation-org-a') RETURNING id`).Scan(&orgAID); err != nil { + t.Fatalf("insert org a: %v", err) + } + cleanupOrg(orgAID) + + var reviewID int64 + if err := db.QueryRow(`INSERT INTO reviews (repository, status, org_id, metadata) VALUES ('a/repo', 'completed', $1, '{}'::jsonb) RETURNING id`, orgAID).Scan(&reviewID); err != nil { + t.Fatalf("insert review: %v", err) + } + t.Cleanup(func() { _, _ = db.Exec(`DELETE FROM reviews WHERE id = $1`, reviewID) }) + + // ---- org B is the attacker's own org, with its own owner + API key ---- + var orgBID int64 + if err := db.QueryRow(`INSERT INTO orgs (name) VALUES ('outcome-isolation-org-b') RETURNING id`).Scan(&orgBID); err != nil { + t.Fatalf("insert org b: %v", err) + } + cleanupOrg(orgBID) + + var userBID int64 + if err := db.QueryRow(`INSERT INTO users (email, password_hash) VALUES ('outcome-isolation-b@example.com', 'x') RETURNING id`).Scan(&userBID); err != nil { + t.Fatalf("insert user b: %v", err) + } + t.Cleanup(func() { _, _ = db.Exec(`DELETE FROM users WHERE id = $1`, userBID) }) + + const ownerRoleID = 2 // seeded fixed row, see roles table + if _, err := db.Exec(`INSERT INTO user_roles (user_id, org_id, role_id) VALUES ($1, $2, $3)`, userBID, orgBID, ownerRoleID); err != nil { + t.Fatalf("insert user_roles: %v", err) + } + // user_roles has no cascading FK from users/orgs, so it must be cleaned up + // explicitly (and before the user/org deletes it's registered ahead of, since + // t.Cleanup runs funcs in LIFO order) or a rerun collides on users_email_key. + t.Cleanup(func() { _, _ = db.Exec(`DELETE FROM user_roles WHERE user_id = $1 AND org_id = $2`, userBID, orgBID) }) + + apiKeyMgr := NewAPIKeyManager(db) + _, plainKey, err := apiKeyMgr.CreateAPIKey(userBID, orgBID, "outcome-isolation-b", []string{}, nil) + if err != nil { + t.Fatalf("create api key: %v", err) + } + + // ---- boot the real route with the real middleware chain ---- + s := &Server{db: db, echo: echo.New()} + authMiddleware := auth.NewAuthMiddleware(nil, db) + + reviewsGroup := s.echo.Group("/api/v1/reviews") + reviewsGroup.Use(RequireAuthOrAPIKey(nil, db)) + reviewsGroup.Use(authMiddleware.BuildOrgContextFromHeader()) + reviewsGroup.Use(authMiddleware.ValidateOrgAccess()) + reviewsGroup.Use(authMiddleware.BuildPermissionContext()) + reviewsGroup.POST("/:id/outcomes", s.markReviewOutcome) + + srv := httptest.NewServer(s.echo) + defer srv.Close() + + body := `{"outcome_type":"revert","reason":"cross-org isolation regression test"}` + req, err := http.NewRequest(http.MethodPost, + srv.URL+"/api/v1/reviews/"+strconv.FormatInt(reviewID, 10)+"/outcomes", strings.NewReader(body)) + if err != nil { + t.Fatalf("build request: %v", err) + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("X-API-Key", plainKey) + req.Header.Set("X-Org-Context", strconv.FormatInt(orgBID, 10)) + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("request failed: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusNotFound { + t.Fatalf("expected 404 (review not found in this organization), got %d", resp.StatusCode) + } + + var count int + if err := db.QueryRow(`SELECT COUNT(*) FROM review_outcomes WHERE review_id = $1`, reviewID).Scan(&count); err != nil { + t.Fatalf("query review_outcomes: %v", err) + } + if count != 0 { + t.Fatalf("expected no review_outcomes row to be written across orgs, found %d", count) + } +} diff --git a/internal/api/risk_model_handlers.go b/internal/api/risk_model_handlers.go new file mode 100644 index 000000000..939245ba7 --- /dev/null +++ b/internal/api/risk_model_handlers.go @@ -0,0 +1,83 @@ +package api + +import ( + "net/http" + + "github.com/labstack/echo/v4" + "github.com/livereview/internal/api/auth" + "github.com/livereview/internal/riskscore" +) + +// getRiskModel handles GET /api/v1/risk-model β€” the Risk Model sub-tab's data: +// the active version/sample-count (model-confidence indicator), the category +// overrides, and the recent score distribution for the calibration chart. +func (s *Server) getRiskModel(c echo.Context) error { + orgID, ok := auth.GetOrgIDFromContext(c) + if !ok { + return JSONErrorWithEnvelope(c, http.StatusUnauthorized, "org context required") + } + + scorer := riskscore.NewScorer(s.db) + info, err := scorer.Info(c.Request().Context(), orgID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "failed to load model: "+err.Error()) + } + + store := riskscore.NewStore(s.db) + overrides, err := store.LoadOverrides(c.Request().Context(), orgID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "failed to load overrides: "+err.Error()) + } + scores, err := riskscore.LoadRecentScores(c.Request().Context(), s.db, orgID, 2000) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "failed to load calibration: "+err.Error()) + } + + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{ + "model": info, + "overrides": overrides, + "scores": scores, + }) +} + +// putRiskModelOverrides handles PUT /api/v1/risk-model/overrides β€” save category +// multipliers (instant effect, decoupled from the retrain cadence). +func (s *Server) putRiskModelOverrides(c echo.Context) error { + pc := auth.MustGetPermissionContext(c) + if err := pc.RequireOrgOwner(); err != nil { + return JSONErrorWithEnvelope(c, http.StatusForbidden, err.Error()) + } + orgID := pc.GetOrgID() + + var body struct { + Overrides map[string]float64 `json:"overrides"` + } + if err := c.Bind(&body); err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid body") + } + + store := riskscore.NewStore(s.db) + for category, multiplier := range body.Overrides { + if err := store.UpsertOverride(c.Request().Context(), orgID, category, multiplier); err != nil { + return JSONErrorWithEnvelope(c, http.StatusBadRequest, "invalid override for "+category+": "+err.Error()) + } + } + overrides, _ := store.LoadOverrides(c.Request().Context(), orgID) + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"overrides": overrides}) +} + +// retrainRiskModel handles POST /api/v1/risk-model/retrain β€” the on-demand batch +// refit (same as the daily job). +func (s *Server) retrainRiskModel(c echo.Context) error { + pc := auth.MustGetPermissionContext(c) + if err := pc.RequireOrgOwner(); err != nil { + return JSONErrorWithEnvelope(c, http.StatusForbidden, err.Error()) + } + orgID := pc.GetOrgID() + + res, err := riskscore.NewStore(s.db).Refit(c.Request().Context(), orgID) + if err != nil { + return JSONErrorWithEnvelope(c, http.StatusInternalServerError, "retrain failed: "+err.Error()) + } + return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"result": res}) +} diff --git a/internal/api/server.go b/internal/api/server.go index a6634c1c5..25534b65a 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -1283,6 +1283,12 @@ func (s *Server) setupRoutes() { orgGroup.PUT("/users/:user_id/role", s.userHandlers.ChangeUserRole) orgGroup.POST("/users/:user_id/force-password-reset", s.userHandlers.ForcePasswordReset) orgGroup.GET("/users/:user_id/audit-log", s.userHandlers.GetUserAuditLog) + orgGroup.POST("/review-outcomes", s.orgReviewOutcome) + orgGroup.GET("/review-outcomes", s.listReviewOutcomes) + orgGroup.GET("/users/:user_id/identities", s.listUserIdentities) + orgGroup.POST("/users/:user_id/identities", s.addUserIdentity) + orgGroup.DELETE("/users/:user_id/identities/:id", s.removeUserIdentity) + orgGroup.GET("/identities", s.listOrgIdentities) // Super admin routes - Production adminGroup := v1.Group("/admin") @@ -1421,6 +1427,33 @@ func (s *Server) setupRoutes() { ciRulesetsGroup.DELETE("/:id", ciRulesetsHandler.Delete) ciRulesetsGroup.GET("/:id/evaluate", ciRulesetsHandler.Evaluate) + // Auto-approve policy (organization-scoped) β€” the risk-based policy engine + // (docs/risk-policy-engine.md). Reuses ci_rulesets as blocking gates. + autoApprovePoliciesHandler := NewAutoApprovePoliciesHandler(s.db) + autoApproveGroup := v1.Group("/auto-approve-policies") + autoApproveGroup.Use(RequireAuthOrAPIKey(s.tokenService, s.db)) + autoApproveGroup.Use(authMiddleware.BuildOrgContextFromHeader()) + autoApproveGroup.Use(authMiddleware.ValidateOrgAccess()) + autoApproveGroup.Use(authMiddleware.BuildPermissionContext()) + autoApproveGroup.GET("", autoApprovePoliciesHandler.List) + autoApproveGroup.POST("", autoApprovePoliciesHandler.Create) + autoApproveGroup.GET("/resolved", autoApprovePoliciesHandler.Resolved) + autoApproveGroup.POST("/simulate", autoApprovePoliciesHandler.Simulate) + autoApproveGroup.GET("/:id", autoApprovePoliciesHandler.Get) + autoApproveGroup.PUT("/:id", autoApprovePoliciesHandler.Update) + autoApproveGroup.DELETE("/:id", autoApprovePoliciesHandler.Delete) + + // Risk model (org-wide classifier): read info/calibration, set category + // overrides, trigger an on-demand refit. + riskModelGroup := v1.Group("/risk-model") + riskModelGroup.Use(RequireAuthOrAPIKey(s.tokenService, s.db)) + riskModelGroup.Use(authMiddleware.BuildOrgContextFromHeader()) + riskModelGroup.Use(authMiddleware.ValidateOrgAccess()) + riskModelGroup.Use(authMiddleware.BuildPermissionContext()) + riskModelGroup.GET("", s.getRiskModel) + riskModelGroup.PUT("/overrides", s.putRiskModelOverrides) + riskModelGroup.POST("/retrain", s.retrainRiskModel) + // Development mode: Org-scoped and Admin test endpoints if s.devMode { // TEST: Org-scoped test endpoint @@ -1678,6 +1711,9 @@ func (s *Server) setupRoutes() { reviewsGroup.GET("/:id/summary", reviewEventsHandler.GetReviewSummary) reviewsGroup.GET("/:id/accounting", reviewEventsHandler.GetReviewAccounting) reviewsGroup.GET("/:id/commits", s.GetReviewCommits) + reviewsGroup.GET("/:id/policy-log", s.getReviewPolicyLog) + reviewsGroup.POST("/:id/force-approve", s.forceApproveReview) + reviewsGroup.POST("/:id/outcomes", s.markReviewOutcome) // Subscription endpoints (organization scoped) subscriptionsHandler := NewSubscriptionsHandler(s.db) diff --git a/internal/api/users/user_service.go b/internal/api/users/user_service.go index c69f23ab6..94ed5b143 100644 --- a/internal/api/users/user_service.go +++ b/internal/api/users/user_service.go @@ -106,6 +106,7 @@ type UserWithRole struct { RoleID int64 `json:"role_id"` OrgID int64 `json:"org_id"` OnboardingAPIKey string `json:"onboarding_api_key,omitempty"` + ReviewLevel int `json:"review_level"` } // CreateUserRequest represents the request to create a new user @@ -119,11 +120,12 @@ type CreateUserRequest struct { // UpdateUserRequest represents the request to update a user type UpdateUserRequest struct { - FirstName *string `json:"first_name"` - LastName *string `json:"last_name"` - IsActive *bool `json:"is_active"` - RoleID *int64 `json:"role_id"` - Password *string `json:"password,omitempty"` + FirstName *string `json:"first_name"` + LastName *string `json:"last_name"` + IsActive *bool `json:"is_active"` + RoleID *int64 `json:"role_id"` + Password *string `json:"password,omitempty"` + ReviewLevel *int `json:"review_level"` } // CreateUserInOrg creates a new user in the specified organization @@ -654,7 +656,7 @@ func (us *UserService) GetUserInOrg(orgID, userID int64) (*UserWithRole, error) SELECT u.id, u.email, u.first_name, u.last_name, u.is_active, u.last_login_at, u.created_at, u.updated_at, u.created_by_user_id, u.deactivated_at, u.deactivated_by_user_id, u.password_reset_required, - r.name as role, r.id as role_id, ur.org_id, u.onboarding_api_key + r.name as role, r.id as role_id, ur.org_id, u.onboarding_api_key, u.review_level FROM users u JOIN user_roles ur ON u.id = ur.user_id JOIN roles r ON ur.role_id = r.id @@ -663,7 +665,7 @@ func (us *UserService) GetUserInOrg(orgID, userID int64) (*UserWithRole, error) &user.ID, &user.Email, &user.FirstName, &user.LastName, &user.IsActive, &user.LastLoginAt, &user.CreatedAt, &user.UpdatedAt, &user.CreatedByUserID, &user.DeactivatedAt, &user.DeactivatedByUserID, &user.PasswordResetRequired, - &user.Role, &user.RoleID, &user.OrgID, &onboardingKey, + &user.Role, &user.RoleID, &user.OrgID, &onboardingKey, &user.ReviewLevel, ) if err != nil { @@ -700,7 +702,7 @@ func (us *UserService) ListUsersInOrg(orgID int64, offset, limit int) ([]*UserWi SELECT u.id, u.email, u.first_name, u.last_name, u.is_active, u.last_login_at, u.created_at, u.updated_at, u.created_by_user_id, u.deactivated_at, u.deactivated_by_user_id, u.password_reset_required, - r.name as role, r.id as role_id, ur.org_id, u.onboarding_api_key + r.name as role, r.id as role_id, ur.org_id, u.onboarding_api_key, u.review_level FROM users u JOIN user_roles ur ON u.id = ur.user_id JOIN roles r ON ur.role_id = r.id @@ -722,7 +724,7 @@ func (us *UserService) ListUsersInOrg(orgID int64, offset, limit int) ([]*UserWi &user.ID, &user.Email, &user.FirstName, &user.LastName, &user.IsActive, &user.LastLoginAt, &user.CreatedAt, &user.UpdatedAt, &user.CreatedByUserID, &user.DeactivatedAt, &user.DeactivatedByUserID, &user.PasswordResetRequired, - &user.Role, &user.RoleID, &user.OrgID, &onboardingKey, + &user.Role, &user.RoleID, &user.OrgID, &onboardingKey, &user.ReviewLevel, ) if err != nil { return nil, 0, fmt.Errorf("failed to scan user: %w", err) @@ -772,6 +774,13 @@ func (us *UserService) UpdateUserInOrg(orgID, userID, updatedByUserID int64, req } } + if req.ReviewLevel != nil { + setParts = append(setParts, fmt.Sprintf("review_level = $%d", argIndex)) + args = append(args, *req.ReviewLevel) + auditDetails["review_level"] = *req.ReviewLevel + argIndex++ + } + if req.Password != nil && *req.Password != "" { hashedPassword, hashErr := bcrypt.GenerateFromPassword([]byte(*req.Password), bcrypt.DefaultCost) if hashErr != nil { diff --git a/internal/batch/batch.go b/internal/batch/batch.go index 2bb77f8d7..29429764c 100644 --- a/internal/batch/batch.go +++ b/internal/batch/batch.go @@ -361,6 +361,7 @@ type BatchResult struct { FileSummary string // New: file-level summary if present TechnicalSummaries []prompts.TechnicalSummary Comments []*models.ReviewComment + DiffClassification *models.DiffClassification // diff-level semantic classification (per-batch) Error error BatchID string } @@ -497,13 +498,46 @@ func (p *BatchProcessor) AggregateAndCombineOutputs(ctx context.Context, llm llm // Output: one general summary, only EXTERNAL comments for posting return &models.ReviewResult{ - Summary: generalSummary, - Comments: externalComments, - InternalComments: internalComments, - Quiz: quiz, + Summary: generalSummary, + Comments: externalComments, + InternalComments: internalComments, + Quiz: quiz, + DiffClassification: pickDiffClassification(results), }, nil } +// pickDiffClassification reduces per-batch diff classifications to a single +// review-level one: highest confidence wins, ties broken toward the riskier +// class. Returns nil when no batch produced one. +func pickDiffClassification(results []*BatchResult) *models.DiffClassification { + var best *models.DiffClassification + for _, r := range results { + if r == nil || r.DiffClassification == nil { + continue + } + dc := r.DiffClassification + if best == nil || dc.Confidence > best.Confidence || + (dc.Confidence == best.Confidence && classRiskOrder(dc.Class) > classRiskOrder(best.Class)) { + best = dc + } + } + return best +} + +// classRiskOrder ranks diff classes by risk for tie-breaking (higher = riskier). +func classRiskOrder(class string) int { + switch class { + case "security-sensitive": + return 3 + case "logic-change": + return 2 + case "config-change", "dependency-change", "data-migration": + return 1 + default: + return 0 + } +} + // quizJSONStart/quizJSONEnd are the exact sentinel lines the "quiz" prompt // instructions (prompts.QuizOutputDelimiters) tell the model to wrap the // quiz JSON array in, appended after the markdown summary in the same diff --git a/internal/canonicaldoc/canonicaldoc.go b/internal/canonicaldoc/canonicaldoc.go new file mode 100644 index 000000000..3f1b07740 --- /dev/null +++ b/internal/canonicaldoc/canonicaldoc.go @@ -0,0 +1,188 @@ +// Package canonicaldoc builds the stable, provider-agnostic "canonical review +// document" every jq ruleset is evaluated against, and evaluates jq boolean +// expressions against it. It is shared by the CI/CD gate API +// (internal/api/ci_rulesets_handler.go) and the risk-policy engine's static +// gate stage (internal/policyengine), both of which reuse the same +// build/eval primitives so there is exactly one rule engine. +// +// A truthy jq result (anything except false/null) means "block". +package canonicaldoc + +import ( + "context" + "database/sql" + "encoding/json" + "fmt" + "strings" + + "github.com/itchyny/gojq" + "github.com/livereview/pkg/models" + storagereviews "github.com/livereview/storage/reviews" +) + +// Finding is one row of the "findings" array in the canonical doc. +type Finding struct { + Severity string `json:"severity"` + Confidence string `json:"confidence"` + Category string `json:"category"` + Subcategory string `json:"subcategory"` + Type string `json:"type"` + FilePath string `json:"file_path,omitempty"` + LineNumber *int `json:"line_number,omitempty"` +} + +// Counts carries pre-aggregated severity/category counts, zero-filled for the +// known severities so jq expressions like `.counts.by_severity.critical > 0` +// never fail with "null cannot be compared" on a clean review. +type Counts struct { + BySeverity map[string]int `json:"by_severity"` + ByCategory map[string]int `json:"by_category"` + Total int `json:"total"` +} + +// ReviewDoc is the canonical document shape jq rulesets evaluate against. +type ReviewDoc struct { + ReviewID int64 `json:"review_id"` + OrgID int64 `json:"org_id"` + Repository string `json:"repository"` + Provider string `json:"provider"` + Status string `json:"status"` + Findings []Finding `json:"findings"` + Counts Counts `json:"counts"` + // DiffClassification is surfaced (when present) so jq rules can reference the + // diff-level semantic classification produced by the review LLM + // (see docs/risk-policy-engine.md Β§5). Absent for CLI/legacy reviews. + DiffClassification *models.DiffClassification `json:"diff_classification,omitempty"` +} + +// KnownSeverities is the fixed 3-value severity model the AI review prompt emits. +var KnownSeverities = []string{"critical", "warning", "info"} + +// header is the minimal review row needed to build the canonical doc's header. +type header struct { + ID int64 + Repository string + Provider string + Status string +} + +// Build constructs the canonical review document for a review scoped strictly +// to orgID - never trust a caller-supplied review_id without confirming org +// ownership first. +func Build(ctx context.Context, db *sql.DB, orgID, reviewID int64) (*ReviewDoc, error) { + var h header + var provider sql.NullString + err := db.QueryRowContext(ctx, ` + SELECT id, repository, provider, status + FROM reviews + WHERE id = $1 AND org_id = $2`, reviewID, orgID).Scan(&h.ID, &h.Repository, &provider, &h.Status) + if err != nil { + return nil, err + } + h.Provider = provider.String + + doc := &ReviewDoc{ + ReviewID: h.ID, + OrgID: orgID, + Repository: h.Repository, + Provider: h.Provider, + Status: h.Status, + Findings: []Finding{}, + Counts: Counts{ + BySeverity: map[string]int{}, + ByCategory: map[string]int{}, + }, + } + for _, sev := range KnownSeverities { + doc.Counts.BySeverity[sev] = 0 + } + + taxonomy := storagereviews.NewTaxonomyReportStore(db) + rows, _, err := taxonomy.ListFindings(ctx, storagereviews.TaxonomyFilter{OrgID: orgID, ReviewID: reviewID}, 500, 0, storagereviews.TaxonomyFindingsOptions{}) + if err != nil { + return nil, fmt.Errorf("loading findings: %w", err) + } + for _, r := range rows { + sev := strings.ToLower(r.Severity) + cat := strings.ToLower(r.Category) + f := Finding{ + Severity: sev, + Confidence: strings.ToLower(r.Confidence), + Category: cat, + Subcategory: strings.ToLower(r.Subcategory), + Type: strings.ToLower(r.IssueType), + LineNumber: r.LineNumber, + } + if r.FilePath != nil { + f.FilePath = *r.FilePath + } + doc.Findings = append(doc.Findings, f) + doc.Counts.BySeverity[sev]++ + if cat != "" { + doc.Counts.ByCategory[cat]++ + } + doc.Counts.Total++ + } + + // Surface the diff-level classification if the review carried one. + doc.DiffClassification = loadDiffClassification(ctx, db, orgID, reviewID) + + return doc, nil +} + +// loadDiffClassification reads the diff-level classification out of the review's +// persisted review_result metadata (written by the review LLM), if present. +func loadDiffClassification(ctx context.Context, db *sql.DB, orgID, reviewID int64) *models.DiffClassification { + var raw []byte + err := db.QueryRowContext(ctx, + `SELECT metadata->'review_result'->'diffClassification' FROM reviews WHERE id = $1 AND org_id = $2`, + reviewID, orgID).Scan(&raw) + if err != nil || len(raw) == 0 { + return nil + } + var dc models.DiffClassification + if err := json.Unmarshal(raw, &dc); err != nil { + return nil + } + return &dc +} + +// RunJQBool evaluates expr against doc and returns whether the first produced +// value is truthy (jq truthiness: everything except false/null), plus the raw +// first result for display/debugging. +func RunJQBool(expr string, doc interface{}) (blocked bool, result interface{}, err error) { + query, err := gojq.Parse(expr) + if err != nil { + return false, nil, fmt.Errorf("invalid jq expression: %w", err) + } + + // Round-trip through JSON so the doc matches what gojq expects + // (map[string]interface{}/[]interface{}/plain scalars). + raw, err := json.Marshal(doc) + if err != nil { + return false, nil, fmt.Errorf("marshaling document: %w", err) + } + // nosemgrep: go.lang.security.deserialization.unsafe-deserialization-interface.go-unsafe-deserialization-interface -- raw is our own json.Marshal output from just above; gojq requires generic map/slice/scalar input, and encoding/json into interface{} can only produce those, never arbitrary types. + var input interface{} + if err := json.Unmarshal(raw, &input); err != nil { + return false, nil, fmt.Errorf("unmarshaling document: %w", err) + } + + iter := query.Run(input) + v, ok := iter.Next() + if !ok { + return false, nil, fmt.Errorf("jq expression produced no output") + } + if e, ok := v.(error); ok { + return false, nil, fmt.Errorf("jq evaluation error: %w", e) + } + switch t := v.(type) { + case bool: + blocked = t + case nil: + blocked = false + default: + blocked = true + } + return blocked, v, nil +} diff --git a/internal/canonicaldoc/canonicaldoc_test.go b/internal/canonicaldoc/canonicaldoc_test.go new file mode 100644 index 000000000..a90e9af2e --- /dev/null +++ b/internal/canonicaldoc/canonicaldoc_test.go @@ -0,0 +1,102 @@ +package canonicaldoc + +import ( + "context" + "database/sql" + "os" + "testing" + + _ "github.com/lib/pq" +) + +func testDB(t *testing.T) *sql.DB { + if testing.Short() { + t.Skip("skipping database integration test") + } + dsn := os.Getenv("DATABASE_URL") + if dsn == "" { + dsn = "postgres://livereview:livereview_password_123@localhost:5432/livereview?sslmode=disable" + } + db, err := sql.Open("postgres", dsn) + if err != nil { + t.Fatalf("open db: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.Ping(); err != nil { + t.Skipf("db not reachable: %v", err) + } + return db +} + +// Build must never return a review that belongs to a different org, even +// when given a syntactically valid review id. +func TestBuildRejectsCrossOrgReview(t *testing.T) { + db := testDB(t) + + // The live dev server's background dashboard-cache refresh job may insert a + // row for a new org before the test ends; that FK has no cascade, so it + // must be cleared before the org delete or the delete silently no-ops. + cleanupOrg := func(orgID int64) { + t.Cleanup(func() { + _, _ = db.Exec(`DELETE FROM dashboard_cache WHERE org_id = $1`, orgID) + _, _ = db.Exec(`DELETE FROM orgs WHERE id = $1`, orgID) + }) + } + + var orgAID, orgBID int64 + if err := db.QueryRow(`INSERT INTO orgs (name) VALUES ('canonicaldoc-org-a') RETURNING id`).Scan(&orgAID); err != nil { + t.Fatalf("insert org a: %v", err) + } + cleanupOrg(orgAID) + if err := db.QueryRow(`INSERT INTO orgs (name) VALUES ('canonicaldoc-org-b') RETURNING id`).Scan(&orgBID); err != nil { + t.Fatalf("insert org b: %v", err) + } + cleanupOrg(orgBID) + + var reviewID int64 + if err := db.QueryRow(`INSERT INTO reviews (repository, status, org_id, metadata) VALUES ('a/repo', 'completed', $1, '{}'::jsonb) RETURNING id`, orgAID).Scan(&reviewID); err != nil { + t.Fatalf("insert review: %v", err) + } + t.Cleanup(func() { _, _ = db.Exec(`DELETE FROM reviews WHERE id = $1`, reviewID) }) + + if _, err := Build(context.Background(), db, orgBID, reviewID); err == nil { + t.Fatalf("expected Build to fail when orgID doesn't own the review") + } + + doc, err := Build(context.Background(), db, orgAID, reviewID) + if err != nil { + t.Fatalf("Build with the owning org failed: %v", err) + } + if doc.ReviewID != reviewID || doc.OrgID != orgAID { + t.Fatalf("unexpected doc: %+v", doc) + } + for _, sev := range KnownSeverities { + if _, ok := doc.Counts.BySeverity[sev]; !ok { + t.Fatalf("expected by_severity to be zero-filled for %q", sev) + } + } +} + +func TestRunJQBoolTruthiness(t *testing.T) { + doc := &ReviewDoc{Counts: Counts{BySeverity: map[string]int{"critical": 1}}} + + blocked, _, err := RunJQBool(".counts.by_severity.critical > 0", doc) + if err != nil { + t.Fatalf("RunJQBool: %v", err) + } + if !blocked { + t.Fatalf("expected blocked=true for a critical finding") + } + + blocked, _, err = RunJQBool(".counts.by_severity.critical > 5", doc) + if err != nil { + t.Fatalf("RunJQBool: %v", err) + } + if blocked { + t.Fatalf("expected blocked=false when the condition doesn't hold") + } + + if _, _, err := RunJQBool("not valid jq [[", doc); err == nil { + t.Fatalf("expected an error for an invalid jq expression") + } +} diff --git a/internal/docindex/docs/routes_guide/explore/repository-settings.md b/internal/docindex/docs/routes_guide/explore/repository-settings.md index b1d5af087..d6b70d719 100644 --- a/internal/docindex/docs/routes_guide/explore/repository-settings.md +++ b/internal/docindex/docs/routes_guide/explore/repository-settings.md @@ -23,6 +23,9 @@ admin. next PR review. If a review is using it right now, clearing is refused until it finishes. When the repo cache is turned off for the server, the section says so and admins get a link to Settings β†’ Storage β†’ Repo Cache. +- **Auto Approve** (`#auto-approve`) β€” override this repository's auto-approve + policy (inherit the org default, or set a repo-specific mode and risk + threshold percentile). See [Settings β†’ Auto Approve](../settings/auto-approve.md). ## Related pages diff --git a/internal/docindex/docs/routes_guide/reviews/review-detail.md b/internal/docindex/docs/routes_guide/reviews/review-detail.md index 8cd7b93b3..a6c4b5c54 100644 --- a/internal/docindex/docs/routes_guide/reviews/review-detail.md +++ b/internal/docindex/docs/routes_guide/reviews/review-detail.md @@ -25,6 +25,17 @@ Any authenticated org member; the review must belong to their org - View the commits included in this review (up to a preview limit, expandable). - Navigate back to the reviews list. +## Auto Approve tab + +Reviews started from a PR/MR "Start a review" link show an **Auto Approve** tab +that renders the risk-policy funnel stage-by-stage (eligibility β†’ static rules +β†’ risk score β†’ LLM classification β†’ final decision), each with its pass/fail +reason. For a blocked review, owners/admins (or users at/above the policy's +`force_approve_min_level`) can **force approve** with a reason β€” recorded in the +audit log and posted to the PR/MR. Any member can **mark a production outcome** +(revert / incident / confirmed-safe) with a reason, which feeds the risk model. +See [Settings β†’ Auto Approve](../settings/auto-approve.md). + ## Blast-radius scoring and ordering Reviews run through the git-lrc CLI upload their own blast-radius report. PR diff --git a/internal/docindex/docs/routes_guide/settings/auto-approve.md b/internal/docindex/docs/routes_guide/settings/auto-approve.md new file mode 100644 index 000000000..9d4fca42d --- /dev/null +++ b/internal/docindex/docs/routes_guide/settings/auto-approve.md @@ -0,0 +1,65 @@ +# Settings β†’ Auto Approve + +**Route:** `/settings#auto-approve` +**Who sees it:** org **owner** (or super_admin) +(`canManageCurrentOrg` in `ui/src/pages/Settings/Settings.tsx`). + +## Purpose + +Configure the risk-based auto-approve policy that decides, after every review +started from a PR/MR "Start a review" link, whether the review is automatically +approved or changes are requested β€” without a human clicking approve. See +`docs/risk-policy-engine.md` for the full design, and +`internal/policyengine` for the engine. + +The decision is a funnel, evaluated in order, each stage recorded in +`review_policy_log` and rendered in the review's **Auto Approve** tab: + +1. **Eligibility** β€” the PR author must map to a LiveReview user (via their + Git username/email, matched through `user_identities`) with a + `review_level` at or above the policy's `min_author_level`. Explicitly + whitelisted users pass regardless of level; bots default to level 0. +2. **Static rules** β€” the configured CI/CD rulesets (`ci_rulesets`) run as + hard blocking gates (they also run under "always approve"). +3. **Risk score** β€” a trained logistic model scores the review and blocks it + if the score is at or above the configured percentile of the org's score + distribution. +4. **LLM classification** β€” the review LLM's diff-level `diffClassification` + must have confidence at/above the required level and no disqualifying + class (security-sensitive). + +## Layout + +Five sub-tabs: + +- **Risk Model** β€” the active model version + training-sample count (model + confidence), category weight overrides (blast radius / security / critical / + diff size, instant effect), a manual **Refit model now** button, and a + **Simulate on recent reviews** replay of the draft policy. +- **Static Rules** β€” pick which CI rulesets are blocking gates. +- **Eligibility** β€” `min_author_level` and `force_approve_min_level`. +- **General** β€” `risk_threshold_percentile`, required LLM confidence, landing + delay (human-rejection window), daily approve cap, and the + "restrict outcome reporting to admins" switch. +- **Production Feedback** β€” the outcome webhook URL and the last-30-days + outcome counter. + +## Key actions + +- **Save policy** upserts the organization-default policy (a single row in + `auto_approve_policies` with `repository_id IS NULL`). Individual repos can + override it from [repository settings](../explore/repository-settings.md). +- **Refit model now** runs a batch refit over all labeled outcomes and + promotes it only if its AUC is at least as good as the current model. +- **Simulate** replays the draft against recent completed reviews (dry-run, + nothing persisted). +- Report production outcomes (revert / incident / confirmed-safe) manually + from a review's **Auto Approve** tab, or POST them to the webhook. Each + non-test outcome nudges the model in real time. + +## Related pages + +[Settings overview](settings-overview.md) Β· +[CI Rulesets](../reviews/ci-cd-gates.md) Β· +[Repository settings](../explore/repository-settings.md) Β· +[Review detail](../reviews/review-detail.md) diff --git a/internal/docindex/docs/routes_guide/settings/settings-overview.md b/internal/docindex/docs/routes_guide/settings/settings-overview.md index 6761dd02b..835505623 100644 --- a/internal/docindex/docs/routes_guide/settings/settings-overview.md +++ b/internal/docindex/docs/routes_guide/settings/settings-overview.md @@ -19,6 +19,7 @@ tabs. Which tabs are visible depends on the user's role | Deployment | `deployment` | super_admin or org owner | | License | `license` | super_admin or org owner, self-hosted only (tab absent in cloud) | | Prompts | `prompts` | super_admin, or org owner/member | +| Auto Approve | `auto-approve` | org owner (or super_admin) | | Learnings | `learnings` | any org member | | API Keys | `api-keys` | any org member | | MCP Integration | `mcp` | any org member | @@ -35,7 +36,8 @@ the `subscriptions` tab. Each tab has its own file in this folder: [Instance](instance.md), [SMTP](smtp.md), [Storage](storage.md), [Deployment](deployment.md), -[License](license.md), [Prompts](prompts.md), [Learnings](learnings.md), +[License](license.md), [Prompts](prompts.md), [Auto Approve](auto-approve.md), +[Learnings](learnings.md), [API Keys](api-keys.md), [MCP Integration](mcp-integration.md), [User Management](user-management.md), [Integrations](integrations.md), [Plan & Usage](plan-and-usage.md). diff --git a/internal/docindex/docs/routes_guide/settings/user-management.md b/internal/docindex/docs/routes_guide/settings/user-management.md index 2afb9af62..1e7892f19 100644 --- a/internal/docindex/docs/routes_guide/settings/user-management.md +++ b/internal/docindex/docs/routes_guide/settings/user-management.md @@ -22,7 +22,10 @@ License Assignment is part of onboarding someone. - Add a single user (`/settings/users/add`) or bulk-import users (`/settings/users/add/bulk`) β€” owner action. - Edit a user's role or details (`/settings/users/edit/:userId`) β€” owner - action. + action, including the user's **review level** (their seniority dial for + auto-approve eligibility; unmapped authors default to 1) and their + **alternative identities** (Git usernames/emails that map their provider + account to this LiveReview user). - Deactivate a user or force a password reset β€” owner action. - Non-owners see the roster read-only. diff --git a/internal/jobqueue/jobqueue.go b/internal/jobqueue/jobqueue.go index 574166596..91c04c3bd 100644 --- a/internal/jobqueue/jobqueue.go +++ b/internal/jobqueue/jobqueue.go @@ -36,6 +36,7 @@ import ( "github.com/jackc/pgx/v5/pgxpool" "github.com/livereview/internal/providers" "github.com/livereview/internal/providers/gitea" + "github.com/livereview/internal/policyengine" "github.com/livereview/internal/seed_demo" networkjobqueue "github.com/livereview/network/jobqueue" storagejobqueue "github.com/livereview/storage/jobqueue" @@ -2419,7 +2420,8 @@ func NewJobQueue(databaseURL string, db *sql.DB) (*JobQueue, error) { prStore := pullrequests.NewStore(db) repoPRSyncWorker := &RepoPRSyncWorker{db: db, store: prStore} prStateSyncWorker := &PRStateSyncWorker{db: db, store: prStore, webhookStore: store} - prPolicyCheckWorker := &PRPolicyCheckWorker{db: db, webhookStore: store} + prPolicyCheckWorker := &PRPolicyCheckWorker{db: db, webhookStore: store, engine: policyengine.NewEngine(db)} + prPolicyPublishWorker := &PRPolicyPublishWorker{db: db, webhookStore: store} reconciliationWorker := &ReconciliationSweepWorker{db: db, pool: pool, stalenessThreshold: config.RepoSyncConfig.StalenessThreshold} scheduledReviewWorker := &ScheduledReviewWorker{db: db} preloadedChangesArchivalWorker := &PreloadedChangesArchivalWorker{db: db} @@ -2436,12 +2438,15 @@ func NewJobQueue(databaseURL string, db *sql.DB) (*JobQueue, error) { river.AddWorker(workers, repoPRSyncWorker) river.AddWorker(workers, prStateSyncWorker) river.AddWorker(workers, prPolicyCheckWorker) + river.AddWorker(workers, prPolicyPublishWorker) + river.AddWorker(workers, &PRPolicyForceApproveWorker{db: db, webhookStore: store}) river.AddWorker(workers, reconciliationWorker) river.AddWorker(workers, preloadedChangesArchivalWorker) river.AddWorker(workers, preloadedChangesArchivalSweepWorker) river.AddWorker(workers, preloadedChangesArchivalPurgeWorker) river.AddWorker(workers, seedDemoWorker) river.AddWorker(workers, &BlastRadiusWorker{db: db}) + river.AddWorker(workers, &RiskModelRefitWorker{db: db}) coordinatorInterval := config.RepoSyncConfig.CoordinatorInterval if coordinatorInterval <= 0 { @@ -2543,6 +2548,13 @@ func NewJobQueue(databaseURL string, db *sql.DB) (*JobQueue, error) { }, &river.PeriodicJobOpts{RunOnStart: false}, ), + river.NewPeriodicJob( + river.PeriodicInterval(24*time.Hour), + func() (river.JobArgs, *river.InsertOpts) { + return RiskModelRefitJobArgs{}, &river.InsertOpts{Queue: "repo_sync", MaxAttempts: 3} + }, + &river.PeriodicJobOpts{ID: "risk_model_refit", RunOnStart: false}, + ), river.NewPeriodicJob( archivalSchedule, func() (river.JobArgs, *river.InsertOpts) { @@ -2576,6 +2588,7 @@ func NewJobQueue(databaseURL string, db *sql.DB) (*JobQueue, error) { diffWorker.jq = jq reconciliationWorker.jq = jq scheduledReviewWorker.jq = jq + prPolicyCheckWorker.jq = jq return jq, nil } diff --git a/internal/jobqueue/pr_policy_claim_sql_test.go b/internal/jobqueue/pr_policy_claim_sql_test.go new file mode 100644 index 000000000..fd19cb2e4 --- /dev/null +++ b/internal/jobqueue/pr_policy_claim_sql_test.go @@ -0,0 +1,85 @@ +package jobqueue + +import ( + "context" + "database/sql" + "os" + "testing" + + _ "github.com/lib/pq" + "github.com/riverqueue/river" +) + +// Regression test: PRPolicyPublishWorker and PRPolicyForceApproveWorker once +// built their idempotency-claim UPDATE with a malformed string +// (`'{"key":"'" || now() || "'"}'::jsonb`), which is a Postgres syntax error +// on every single invocation β€” Force Approve and the landing-delay publish +// path never actually posted anything, though the audit trail looked fine. +// This runs the real worker against a real Postgres DB so the claim +// statement is actually executed, not just read. +func testDB(t *testing.T) *sql.DB { + if testing.Short() { + t.Skip("skipping database integration test") + } + dsn := os.Getenv("DATABASE_URL") + if dsn == "" { + dsn = "postgres://livereview:livereview_password_123@localhost:5432/livereview?sslmode=disable" + } + db, err := sql.Open("postgres", dsn) + if err != nil { + t.Fatalf("open db: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.Ping(); err != nil { + t.Skipf("db not reachable: %v", err) + } + return db +} + +func seedPolicyTestOrgAndReview(t *testing.T, db *sql.DB) (orgID, reviewID int64) { + if err := db.QueryRow(`INSERT INTO orgs (name) VALUES ('pr-policy-claim-sql-test-org') RETURNING id`).Scan(&orgID); err != nil { + t.Fatalf("insert org: %v", err) + } + t.Cleanup(func() { + // The live dev server's background dashboard-cache refresh job may insert + // a row for this org before cleanup runs; that FK has no cascade, so it + // must be cleared first or the org delete silently no-ops. + _, _ = db.Exec(`DELETE FROM dashboard_cache WHERE org_id = $1`, orgID) + _, _ = db.Exec(`DELETE FROM orgs WHERE id = $1`, orgID) + }) + + if err := db.QueryRow(` + INSERT INTO reviews (repository, status, org_id, metadata) + VALUES ('test/repo', 'completed', $1, '{}'::jsonb) RETURNING id`, orgID).Scan(&reviewID); err != nil { + t.Fatalf("insert review: %v", err) + } + t.Cleanup(func() { _, _ = db.Exec(`DELETE FROM reviews WHERE id = $1`, reviewID) }) + return orgID, reviewID +} + +func TestPRPolicyPublishWorkerClaimSQLIsValid(t *testing.T) { + db := testDB(t) + orgID, reviewID := seedPolicyTestOrgAndReview(t, db) + + w := &PRPolicyPublishWorker{db: db} + job := &river.Job[PRPolicyPublishJobArgs]{Args: PRPolicyPublishJobArgs{OrgID: orgID, ReviewID: reviewID}} + + // No pull_request_id on this review, so Work runs the claim UPDATE (exactly + // the statement that used to be a SQL syntax error), then releases the + // claim and returns nil since there's nothing to publish to. + if err := w.Work(context.Background(), job); err != nil { + t.Fatalf("Work() returned an error (claim SQL likely broken again): %v", err) + } +} + +func TestPRPolicyForceApproveWorkerClaimSQLIsValid(t *testing.T) { + db := testDB(t) + orgID, reviewID := seedPolicyTestOrgAndReview(t, db) + + w := &PRPolicyForceApproveWorker{db: db} + job := &river.Job[PRPolicyForceApproveJobArgs]{Args: PRPolicyForceApproveJobArgs{OrgID: orgID, ReviewID: reviewID, ActorUserID: 0, Reason: "test"}} + + if err := w.Work(context.Background(), job); err != nil { + t.Fatalf("Work() returned an error (claim SQL likely broken again): %v", err) + } +} diff --git a/internal/jobqueue/pr_policy_force_approve.go b/internal/jobqueue/pr_policy_force_approve.go new file mode 100644 index 000000000..c6424a777 --- /dev/null +++ b/internal/jobqueue/pr_policy_force_approve.go @@ -0,0 +1,118 @@ +package jobqueue + +import ( + "context" + "database/sql" + "fmt" + "log" + + providergithub "github.com/livereview/internal/providers/github" + providergitlab "github.com/livereview/internal/providers/gitlab" + storagejobqueue "github.com/livereview/storage/jobqueue" + "github.com/riverqueue/river" +) + +// PRPolicyForceApproveJobArgs posts a force-approval to the PR/MR after a human +// overrode a blocked policy verdict (the review_policy_overrides row is written +// by the API handler before this job is enqueued). +type PRPolicyForceApproveJobArgs struct { + OrgID int64 `json:"org_id"` + ReviewID int64 `json:"review_id"` + ActorUserID int64 `json:"actor_user_id"` + Reason string `json:"reason"` +} + +func (PRPolicyForceApproveJobArgs) Kind() string { return "pr_policy_force_approve" } + +type PRPolicyForceApproveWorker struct { + river.WorkerDefaults[PRPolicyForceApproveJobArgs] + db *sql.DB + webhookStore *storagejobqueue.WebhookStore +} + +func (w *PRPolicyForceApproveWorker) Work(ctx context.Context, job *river.Job[PRPolicyForceApproveJobArgs]) error { + args := job.Args + + // Idempotency claim so River retries don't approve twice. + res, err := w.db.ExecContext(ctx, + `UPDATE reviews SET metadata = metadata || jsonb_build_object('policy_force_published_at', now()) + WHERE id = $1 AND org_id = $2 AND NOT (metadata ? 'policy_force_published_at')`, + args.ReviewID, args.OrgID) + if err != nil { + return err + } + if n, _ := res.RowsAffected(); n == 0 { + return nil + } + release := func() { + _, _ = w.db.ExecContext(ctx, `UPDATE reviews SET metadata = metadata - 'policy_force_published_at' WHERE id = $1`, args.ReviewID) + } + + var pullRequestID sql.NullInt64 + if err := w.db.QueryRowContext(ctx, + `SELECT pull_request_id FROM reviews WHERE id = $1 AND org_id = $2`, + args.ReviewID, args.OrgID).Scan(&pullRequestID); err != nil { + release() + return err + } + if !pullRequestID.Valid { + release() + return nil + } + + var repositoryID int64 + var number int + if err := w.db.QueryRowContext(ctx, + `SELECT repository_id, number FROM pull_requests WHERE id = $1 AND org_id = $2`, + pullRequestID.Int64, args.OrgID).Scan(&repositoryID, &number); err != nil { + release() + return err + } + info, err := loadRepositoryConnectorInfo(w.db, repositoryID) + if err != nil { + release() + return err + } + + actorName := fmt.Sprintf("user %d", args.ActorUserID) + if args.ActorUserID > 0 { + var email string + if err := w.db.QueryRowContext(ctx, `SELECT email FROM users WHERE id = $1`, args.ActorUserID).Scan(&email); err == nil && email != "" { + actorName = email + } + } + + body := fmt.Sprintf("## ⚠️ LiveReview policy: Force-approved\n\n"+ + "Force-approved by **%s** despite a policy block.\n\n"+ + "**Reason:** %s\n\n"+ + "This override is recorded in LiveReview's audit log.\n", actorName, args.Reason) + + switch info.Provider { + case "github", "github-com", "github-enterprise": + if err := providergithub.SubmitPullRequestReview(ctx, info.ProviderURL, info.token(), info.FullName, number, "APPROVE", body); err != nil { + log.Printf("[WARN] pr policy force-approve: APPROVE failed for review %d, posting as comment: %v", args.ReviewID, err) + return providergithub.PostIssueComment(ctx, info.ProviderURL, info.token(), info.FullName, number, body) + } + return nil + case "gitlab", "gitlab-com", "gitlab-self-hosted": + client := providergitlab.NewHTTPClient(info.ProviderURL, info.token()) + if err := client.CreateMRGeneralComment(info.ProviderRepoID, number, body); err != nil { + return err + } + if err := client.ApproveMR(info.ProviderRepoID, number); err != nil { + log.Printf("[WARN] pr policy force-approve: approve failed for review %d: %v", args.ReviewID, err) + } + return nil + default: + return nil + } +} + +// QueuePRPolicyForceApprove enqueues the force-approve publish. +func (jq *JobQueue) QueuePRPolicyForceApprove(ctx context.Context, args PRPolicyForceApproveJobArgs) error { + _, err := jq.client.Insert(ctx, args, &river.InsertOpts{Queue: "repo_sync", MaxAttempts: 5}) + if err != nil { + return fmt.Errorf("failed to queue force-approve: %w", err) + } + return nil +} diff --git a/internal/jobqueue/pr_policy_worker.go b/internal/jobqueue/pr_policy_worker.go index a5a86e68b..a18adc038 100644 --- a/internal/jobqueue/pr_policy_worker.go +++ b/internal/jobqueue/pr_policy_worker.go @@ -4,18 +4,22 @@ import ( "context" "database/sql" "encoding/json" + "errors" "fmt" "log" "strings" + "time" + "github.com/livereview/internal/policyengine" providergithub "github.com/livereview/internal/providers/github" providergitlab "github.com/livereview/internal/providers/gitlab" storagejobqueue "github.com/livereview/storage/jobqueue" "github.com/riverqueue/river" ) -// PRPolicyCheckJobArgs runs the approval policy on a finished review that was started -// from the PR comment's "Start a review" link (metadata.pr_link_policy). +// PRPolicyCheckJobArgs runs the risk-based approval policy on a finished review +// that was started from the PR comment's "Start a review" link +// (metadata.pr_link_policy). See docs/risk-policy-engine.md. type PRPolicyCheckJobArgs struct { OrgID int64 `json:"org_id"` ReviewID int64 `json:"review_id"` @@ -27,42 +31,22 @@ type PRPolicyCheckWorker struct { river.WorkerDefaults[PRPolicyCheckJobArgs] db *sql.DB webhookStore *storagejobqueue.WebhookStore + engine *policyengine.Engine + jq *JobQueue } -// policyFinding is the subset of models.ReviewComment (stored without JSON tags) the policy reads. -type policyFinding struct { - FilePath string - Line int - Content string - Severity string - IsInternal bool -} - -type policyResult struct { - Policy string `json:"policy"` - Approved bool `json:"approved"` - Critical int `json:"critical"` - Warning int `json:"warning"` - Info int `json:"info"` -} - -// ponytail: one static policy (no critical findings); swap for org-defined policies later. -const staticPolicyName = "No critical findings" - func (w *PRPolicyCheckWorker) Work(ctx context.Context, job *river.Job[PRPolicyCheckJobArgs]) error { args := job.Args var status string var pullRequestID sql.NullInt64 var flagged bool - var commentsJSON []byte err := w.db.QueryRowContext(ctx, ` SELECT status, pull_request_id, - COALESCE((metadata->>'pr_link_policy')::boolean, false), - COALESCE(metadata->'review_result'->'comments', '[]'::jsonb) + COALESCE((metadata->>'pr_link_policy')::boolean, false) FROM reviews WHERE id = $1 AND org_id = $2`, args.ReviewID, args.OrgID, - ).Scan(&status, &pullRequestID, &flagged, &commentsJSON) + ).Scan(&status, &pullRequestID, &flagged) if err == sql.ErrNoRows { return nil } @@ -73,52 +57,81 @@ func (w *PRPolicyCheckWorker) Work(ctx context.Context, job *river.Job[PRPolicyC return nil } - var findings []policyFinding - if err := json.Unmarshal(commentsJSON, &findings); err != nil { - return fmt.Errorf("failed to parse findings for review %d: %w", args.ReviewID, err) + // Claim: the policy_result key doubles as an idempotency guard so River + // retries never evaluate (or approve) twice. + claim := `{"status":"evaluating"}` + res0, err := w.db.ExecContext(ctx, + `UPDATE reviews SET metadata = metadata || jsonb_build_object('policy_result', $2::jsonb) + WHERE id = $1 AND NOT (metadata ? 'policy_result')`, + args.ReviewID, claim) + if err != nil { + return fmt.Errorf("failed to claim policy check for review %d: %w", args.ReviewID, err) } - result := policyResult{Policy: staticPolicyName} - var critical []policyFinding - for _, f := range findings { - if f.IsInternal { - continue - } - switch strings.ToLower(f.Severity) { - case "critical": - result.Critical++ - critical = append(critical, f) - case "warning": - result.Warning++ - default: - result.Info++ + if n, _ := res0.RowsAffected(); n == 0 { + return nil // already evaluated + } + release := func() { + if _, e := w.db.ExecContext(ctx, `UPDATE reviews SET metadata = metadata - 'policy_result' WHERE id = $1`, args.ReviewID); e != nil { + log.Printf("[WARN] pr policy: failed to release claim for review %d: %v", args.ReviewID, e) } } - result.Approved = result.Critical == 0 - // Store the result first; it doubles as a claim so River retries never approve twice. - resultJSON, _ := json.Marshal(result) - res, err := w.db.ExecContext(ctx, - `UPDATE reviews SET metadata = metadata || jsonb_build_object('policy_result', $2::jsonb) - WHERE id = $1 AND NOT (metadata ? 'policy_result')`, - args.ReviewID, string(resultJSON)) + res, err := w.engine.Run(ctx, args.OrgID, args.ReviewID, false) + if errors.Is(err, policyengine.ErrPolicyDisabled) { + release() + return nil // feature off; nothing to post + } if err != nil { + release() + return fmt.Errorf("policy evaluation failed for review %d: %w", args.ReviewID, err) + } + + // Persist the funnel result (the policy-log table is the rendering source of + // truth; this JSON is what the landing-delay publish job re-reads). + resultJSON, _ := json.Marshal(res) + if _, err := w.db.ExecContext(ctx, + `UPDATE reviews SET metadata = metadata || jsonb_build_object('policy_result', $2::jsonb) WHERE id = $1`, + args.ReviewID, string(resultJSON)); err != nil { + release() return fmt.Errorf("failed to store policy result for review %d: %w", args.ReviewID, err) } - if n, _ := res.RowsAffected(); n == 0 { + + // Landing delay: instead of publishing immediately, queue a follow-up publish + // job scheduled after the human-rejection window. + if res.Approved && res.LandingDelaySeconds > 0 { + if err := w.queuePublish(ctx, args.OrgID, args.ReviewID, time.Duration(res.LandingDelaySeconds)*time.Second); err != nil { + release() + return err + } return nil } - if err := w.publish(ctx, args, pullRequestID.Int64, result, critical); err != nil { - if _, resetErr := w.db.ExecContext(ctx, `UPDATE reviews SET metadata = metadata - 'policy_result' WHERE id = $1`, args.ReviewID); resetErr != nil { - log.Printf("[WARN] pr policy: failed to release claim for review %d: %v", args.ReviewID, resetErr) - } + if err := w.publish(ctx, args, pullRequestID.Int64, res); err != nil { + release() return err } return nil } +// queuePublish enqueues the landing-delay publish follow-up. +func (w *PRPolicyCheckWorker) queuePublish(ctx context.Context, orgID, reviewID int64, delay time.Duration) error { + if w.jq == nil { + return fmt.Errorf("job queue unavailable for landing-delay publish") + } + _, err := w.jq.client.Insert(ctx, PRPolicyPublishJobArgs{OrgID: orgID, ReviewID: reviewID}, + &river.InsertOpts{ + Queue: "repo_sync", + MaxAttempts: 5, + ScheduledAt: time.Now().Add(delay), + }) + if err != nil { + return fmt.Errorf("failed to queue landing-delay publish: %w", err) + } + return nil +} + // publish posts the policy comment and approves (or requests changes on) the PR/MR. -func (w *PRPolicyCheckWorker) publish(ctx context.Context, args PRPolicyCheckJobArgs, pullRequestID int64, result policyResult, critical []policyFinding) error { +func (w *PRPolicyCheckWorker) publish(ctx context.Context, args PRPolicyCheckJobArgs, pullRequestID int64, res *policyengine.FunnelResult) error { var repositoryID int64 var number int if err := w.db.QueryRowContext(ctx, @@ -135,12 +148,12 @@ func (w *PRPolicyCheckWorker) publish(ctx context.Context, args PRPolicyCheckJob if err != nil { log.Printf("[WARN] pr policy: public URL not configured, posting without review link: %v", err) } - body := buildPolicyComment(host, args.ReviewID, result, critical) + body := buildPolicyComment(host, args.ReviewID, res) switch info.Provider { case "github", "github-com", "github-enterprise": event := "REQUEST_CHANGES" - if result.Approved { + if res.Approved { event = "APPROVE" } if err := providergithub.SubmitPullRequestReview(ctx, info.ProviderURL, info.token(), info.FullName, number, event, body); err != nil { @@ -155,7 +168,7 @@ func (w *PRPolicyCheckWorker) publish(ctx context.Context, args PRPolicyCheckJob return err } // GitLab Free has no "request changes", so a failed policy is comment-only. - if result.Approved { + if res.Approved { if err := client.ApproveMR(info.ProviderRepoID, number); err != nil { log.Printf("[WARN] pr policy: approve failed for review %d: %v", args.ReviewID, err) } @@ -166,48 +179,111 @@ func (w *PRPolicyCheckWorker) publish(ctx context.Context, args PRPolicyCheckJob } } -const maxListedCriticalFindings = 5 +// PRPolicyPublishJobArgs re-publishes the policy comment after the landing delay. +type PRPolicyPublishJobArgs struct { + OrgID int64 `json:"org_id"` + ReviewID int64 `json:"review_id"` +} + +func (PRPolicyPublishJobArgs) Kind() string { return "pr_policy_publish" } + +type PRPolicyPublishWorker struct { + river.WorkerDefaults[PRPolicyPublishJobArgs] + db *sql.DB + webhookStore *storagejobqueue.WebhookStore +} + +func (w *PRPolicyPublishWorker) Work(ctx context.Context, job *river.Job[PRPolicyPublishJobArgs]) error { + args := job.Args + + // Guard against double-posting. + res0, err := w.db.ExecContext(ctx, + `UPDATE reviews SET metadata = metadata || jsonb_build_object('policy_published_at', now()) + WHERE id = $1 AND org_id = $2 AND NOT (metadata ? 'policy_published_at')`, + args.ReviewID, args.OrgID) + if err != nil { + return err + } + if n, _ := res0.RowsAffected(); n == 0 { + return nil // already published + } + release := func() { + _, _ = w.db.ExecContext(ctx, `UPDATE reviews SET metadata = metadata - 'policy_published_at' WHERE id = $1`, args.ReviewID) + } -func buildPolicyComment(host string, reviewID int64, result policyResult, critical []policyFinding) string { + var pullRequestID sql.NullInt64 + var resultJSON []byte + if err := w.db.QueryRowContext(ctx, + `SELECT pull_request_id, COALESCE(metadata->'policy_result', '{}'::jsonb) FROM reviews WHERE id = $1 AND org_id = $2`, + args.ReviewID, args.OrgID).Scan(&pullRequestID, &resultJSON); err != nil { + release() + return err + } + if !pullRequestID.Valid { + release() + return nil + } + var res policyengine.FunnelResult + if err := json.Unmarshal(resultJSON, &res); err != nil { + release() + return fmt.Errorf("failed to decode policy result for review %d: %w", args.ReviewID, err) + } + + checker := &PRPolicyCheckWorker{db: w.db, webhookStore: w.webhookStore} + if err := checker.publish(ctx, PRPolicyCheckJobArgs{OrgID: args.OrgID, ReviewID: args.ReviewID}, pullRequestID.Int64, &res); err != nil { + release() + return err + } + return nil +} + +// buildPolicyComment renders the funnel stage-by-stage so the posted comment +// shows *why* the policy reached its verdict, not just the verdict. +func buildPolicyComment(host string, reviewID int64, res *policyengine.FunnelResult) string { var b strings.Builder - if result.Approved { + if res.Approved { b.WriteString("## βœ… LiveReview policy: Approved\n\n") } else { b.WriteString("## ❌ LiveReview policy: Changes requested\n\n") } - fmt.Fprintf(&b, "Review #%d Β· %d critical Β· %d warning Β· %d info\n\n", reviewID, result.Critical, result.Warning, result.Info) - b.WriteString("| Policy | Result |\n|---|---|\n") - if result.Approved { - fmt.Fprintf(&b, "| %s | βœ… Passed |\n", result.Policy) - } else { - fmt.Fprintf(&b, "| %s | ❌ %d critical |\n\n", result.Policy, result.Critical) - for i, f := range critical { - if i == maxListedCriticalFindings { - fmt.Fprintf(&b, "- …and %d more\n", len(critical)-maxListedCriticalFindings) - break - } - fmt.Fprintf(&b, "- `%s:%d` β€” %s\n", f.FilePath, f.Line, firstLine(f.Content, 120)) + fmt.Fprintf(&b, "Review #%d\n\n", reviewID) + + b.WriteString("| Stage | Result | Detail |\n|---|---|---|\n") + for _, st := range res.Stages { + icon := "βœ…" + if st.Verdict == "fail" { + icon = "❌" + } else if st.Verdict == "skip" { + icon = "⏭️" } + reason := "" + if r, ok := st.Detail["reason"].(string); ok { + reason = r + } + fmt.Fprintf(&b, "| %s | %s | %s |\n", titleCaseStage(st.Stage), icon, reason) } + if host != "" { fmt.Fprintf(&b, "\n[View full review](%s/#/reviews/%d)\n", strings.TrimSuffix(host, "/"), reviewID) } return b.String() } -// firstLine returns the first non-empty line of s, cut to limit runes. -func firstLine(s string, limit int) string { - for _, line := range strings.Split(s, "\n") { - line = strings.TrimSpace(line) - if line == "" { - continue - } - if r := []rune(line); len(r) > limit { - return string(r[:limit]) + "…" - } - return line +func titleCaseStage(s string) string { + switch s { + case "eligibility": + return "Eligibility" + case "static": + return "Static rules" + case "risk": + return "Risk score" + case "llm": + return "LLM classification" + case "final": + return "Final decision" + default: + return s } - return "" } // QueuePRPolicyCheckIfFlagged enqueues the policy check only for reviews started from the PR link. diff --git a/internal/jobqueue/review_worker.go b/internal/jobqueue/review_worker.go index a9fff83ec..9998937df 100644 --- a/internal/jobqueue/review_worker.go +++ b/internal/jobqueue/review_worker.go @@ -332,6 +332,7 @@ func (w *DiffReviewWorker) Work(ctx context.Context, job *river.Job[DiffReviewJo summary := "" var comments []*models.ReviewComment var quiz []models.QuizQuestion + var diffClassification *models.DiffClassification failureReason := "" if result != nil { @@ -396,6 +397,7 @@ func (w *DiffReviewWorker) Work(ctx context.Context, job *river.Job[DiffReviewJo summary = result.Summary comments = result.Comments quiz = result.Quiz + diffClassification = result.DiffClassification } else { failureReason = "review processing returned no result" if logger != nil { @@ -406,11 +408,12 @@ func (w *DiffReviewWorker) Work(ctx context.Context, job *river.Job[DiffReviewJo // 10. Persist final results and update status type diffReviewResultPayload struct { - Summary string `json:"summary"` - Comments []*models.ReviewComment `json:"comments"` - Quiz []models.QuizQuestion `json:"quiz,omitempty"` + Summary string `json:"summary"` + Comments []*models.ReviewComment `json:"comments"` + Quiz []models.QuizQuestion `json:"quiz,omitempty"` + DiffClassification *models.DiffClassification `json:"diffClassification,omitempty"` } - payload := diffReviewResultPayload{Summary: summary, Comments: comments, Quiz: quiz} + payload := diffReviewResultPayload{Summary: summary, Comments: comments, Quiz: quiz, DiffClassification: diffClassification} meta := map[string]interface{}{"review_result": payload} if failureReason != "" { meta["failure_reason"] = failureReason diff --git a/internal/jobqueue/risk_model_refit_worker.go b/internal/jobqueue/risk_model_refit_worker.go new file mode 100644 index 000000000..08cfbc700 --- /dev/null +++ b/internal/jobqueue/risk_model_refit_worker.go @@ -0,0 +1,42 @@ +package jobqueue + +import ( + "context" + "database/sql" + + "github.com/livereview/internal/riskscore" + "github.com/riverqueue/river" +) + +// RiskModelRefitJobArgs runs the daily stabilizing batch refit over every org +// that has labeled outcomes (docs/risk-policy-engine.md Β§3, path 2). +type RiskModelRefitJobArgs struct{} + +func (RiskModelRefitJobArgs) Kind() string { return "risk_model_refit" } + +type RiskModelRefitWorker struct { + river.WorkerDefaults[RiskModelRefitJobArgs] + db *sql.DB +} + +func (w *RiskModelRefitWorker) Work(ctx context.Context, job *river.Job[RiskModelRefitJobArgs]) error { + rows, err := w.db.QueryContext(ctx, `SELECT DISTINCT org_id FROM risk_feature_snapshots`) + if err != nil { + return err + } + defer rows.Close() + + store := riskscore.NewStore(w.db) + for rows.Next() { + var orgID int64 + if err := rows.Scan(&orgID); err != nil { + return err + } + // Best-effort per org: one org failing shouldn't block the sweep. + if _, err := store.Refit(ctx, orgID); err != nil { + // log and continue + _ = err + } + } + return rows.Err() +} diff --git a/internal/policyengine/eligibility.go b/internal/policyengine/eligibility.go new file mode 100644 index 000000000..395f19732 --- /dev/null +++ b/internal/policyengine/eligibility.go @@ -0,0 +1,107 @@ +package policyengine + +import ( + "context" + "database/sql" + "strings" +) + +// AuthorInfo describes how the PR author was resolved to a review level. +type AuthorInfo struct { + Username string `json:"username,omitempty"` + Email string `json:"email,omitempty"` + Provider string `json:"provider,omitempty"` + IsBot bool `json:"is_bot"` + UserID int64 `json:"user_id,omitempty"` // 0 = unmapped to a LiveReview user + Level int `json:"level"` // resolved level (default 1 when unmapped) + Matched bool `json:"matched"` // a user_identities mapping was found +} + +// resolveAuthor maps the PR author to a LiveReview user (via user_identities) and +// resolves their review_level. Unmapped non-bot authors default to level 1 +// (fail-closed: the lowest bar); bots default to level 0. +func (e *Engine) resolveAuthor(ctx context.Context, orgID, reviewID int64) (*AuthorInfo, error) { + var prID sql.NullInt64 + if err := e.db.QueryRowContext(ctx, + `SELECT pull_request_id FROM reviews WHERE id = $1 AND org_id = $2`, reviewID, orgID).Scan(&prID); err != nil { + return nil, err + } + + var username, email, provider string + if prID.Valid { + var u, em, p sql.NullString + err := e.db.QueryRowContext(ctx, + `SELECT COALESCE(author_username, ''), COALESCE(author_email, ''), COALESCE(provider, '') + FROM pull_requests WHERE id = $1 AND org_id = $2`, prID.Int64, orgID).Scan(&u, &em, &p) + if err != nil && err != sql.ErrNoRows { + return nil, err + } + username, email, provider = u.String, em.String, p.String + } else { + // No linked PR: fall back to the review's own author fields. + var u, em, p sql.NullString + if err := e.db.QueryRowContext(ctx, + `SELECT COALESCE(author_username, ''), COALESCE(user_email, ''), COALESCE(provider, '') + FROM reviews WHERE id = $1 AND org_id = $2`, reviewID, orgID).Scan(&u, &em, &p); err != nil { + return nil, err + } + username, email, provider = u.String, em.String, p.String + } + + ai := &AuthorInfo{ + Username: strings.TrimSpace(username), + Email: strings.ToLower(strings.TrimSpace(email)), + Provider: provider, + Level: 1, // fail-closed default + } + ai.IsBot = isBotLogin(ai.Username) + + // Look up user_identities by username and (if present) email. + usernameKey := strings.ToLower(strings.TrimSpace(username)) + emailKey := ai.Email + + var userID sql.NullInt64 + err := e.db.QueryRowContext(ctx, ` + SELECT user_id FROM user_identities + WHERE org_id = $1 AND kind = 'username' AND value = $2 + LIMIT 1`, orgID, usernameKey).Scan(&userID) + if err == nil && userID.Valid { + return e.withMappedUser(ctx, ai, userID.Int64) + } + if emailKey != "" { + err = e.db.QueryRowContext(ctx, ` + SELECT user_id FROM user_identities + WHERE org_id = $1 AND kind = 'email' AND value = $2 + LIMIT 1`, orgID, emailKey).Scan(&userID) + if err == nil && userID.Valid { + return e.withMappedUser(ctx, ai, userID.Int64) + } + } + + return ai, nil +} + +// withMappedUser fills in the mapped user's level (bots keep level 0 unless the +// mapping makes them a real user, in which case their level applies). +func (e *Engine) withMappedUser(ctx context.Context, ai *AuthorInfo, userID int64) (*AuthorInfo, error) { + var level int + if err := e.db.QueryRowContext(ctx, `SELECT review_level FROM users WHERE id = $1`, userID).Scan(&level); err != nil { + return nil, err + } + ai.UserID = userID + ai.Matched = true + ai.Level = level + return ai, nil +} + +// isBotLogin detects provider bot accounts (GitHub's "[bot]" suffix, and common +// automation suffixes) from a login. +func isBotLogin(username string) bool { + u := strings.ToLower(strings.TrimSpace(username)) + if u == "" { + return false + } + return strings.HasSuffix(u, "[bot]") || + strings.HasSuffix(u, "-bot") || + strings.HasSuffix(u, "_bot") +} diff --git a/internal/policyengine/engine.go b/internal/policyengine/engine.go new file mode 100644 index 000000000..dc6677a96 --- /dev/null +++ b/internal/policyengine/engine.go @@ -0,0 +1,426 @@ +package policyengine + +import ( + "context" + "database/sql" + "encoding/json" + "errors" + "fmt" + + "github.com/livereview/internal/canonicaldoc" + "github.com/livereview/internal/riskscore" +) + +// ErrPolicyDisabled is returned when no auto-approve policy is configured for +// the review's org/repo (the feature is off). +var ErrPolicyDisabled = errors.New("auto-approve policy not configured") + +// StageResult is one funnel stage's outcome, as written to review_policy_log. +type StageResult struct { + Stage string `json:"stage"` + Verdict string `json:"verdict"` + Detail map[string]interface{} `json:"detail"` +} + +// FunnelResult is the full outcome of one policy evaluation. +type FunnelResult struct { + Approved bool `json:"approved"` + Mode string `json:"mode"` + ModeOverride bool `json:"mode_override"` + Stages []StageResult `json:"stages"` + Score float64 `json:"score,omitempty"` + Percentile int `json:"percentile,omitempty"` + ThresholdCutoff float64 `json:"threshold_cutoff,omitempty"` + Author *AuthorInfo `json:"author,omitempty"` + // LandingDelaySeconds is the post-approve human-rejection window, copied + // from the resolved policy so the worker can schedule the delayed publish. + LandingDelaySeconds int `json:"landing_delay_seconds,omitempty"` +} + +// Engine runs the policy funnel against a database. +type Engine struct { + db *sql.DB + scorer *riskscore.Scorer + store *riskscore.Store +} + +// NewEngine returns an Engine backed by db. +func NewEngine(db *sql.DB) *Engine { + return &Engine{db: db, scorer: riskscore.NewScorer(db), store: riskscore.NewStore(db)} +} + +func (e *Engine) reason(st StageResult) string { + if r, ok := st.Detail["reason"].(string); ok { + return r + } + return "" +} + +func skipStage(stage string) StageResult { + return StageResult{Stage: stage, Verdict: VerdictSkip, Detail: map[string]interface{}{"reason": "skipped (earlier stage blocked)"}} +} + +func skipModeStage(stage string) StageResult { + return StageResult{Stage: stage, Verdict: VerdictSkip, Detail: map[string]interface{}{"reason": "skipped: mode override"}} +} + +// reviewRepositoryID resolves the repository_id for a review (via its linked +// pull request), or 0 when there is none. +func (e *Engine) reviewRepositoryID(ctx context.Context, orgID, reviewID int64) int64 { + var repoID int64 + err := e.db.QueryRowContext(ctx, ` + SELECT COALESCE(pr.repository_id, 0) + FROM reviews r LEFT JOIN pull_requests pr ON pr.id = r.pull_request_id + WHERE r.id = $1 AND r.org_id = $2`, reviewID, orgID).Scan(&repoID) + if err != nil { + return 0 + } + return repoID +} + +// Run evaluates the policy funnel for a review. dryRun skips all writes +// (review_policy_log rows and the feature snapshot) for the simulate endpoint. +func (e *Engine) Run(ctx context.Context, orgID, reviewID int64, dryRun bool) (*FunnelResult, error) { + repoID := e.reviewRepositoryID(ctx, orgID, reviewID) + p, found, err := ResolvePolicy(ctx, e.db, orgID, repoID) + if err != nil { + return nil, err + } + if !found { + return nil, ErrPolicyDisabled + } + return e.runWithPolicy(ctx, orgID, reviewID, p, dryRun) +} + +// RunWithPolicy evaluates the funnel against an explicit policy (the simulate +// endpoint's draft policy), bypassing DB resolution. dryRun skips all writes. +func (e *Engine) RunWithPolicy(ctx context.Context, orgID, reviewID int64, p *Policy, dryRun bool) (*FunnelResult, error) { + return e.runWithPolicy(ctx, orgID, reviewID, p, dryRun) +} + +func (e *Engine) runWithPolicy(ctx context.Context, orgID, reviewID int64, p *Policy, dryRun bool) (*FunnelResult, error) { + res := &FunnelResult{Mode: p.Mode, Stages: []StageResult{}, LandingDelaySeconds: p.LandingDelaySeconds} + + doc, docErr := canonicaldoc.Build(ctx, e.db, orgID, reviewID) + + switch p.Mode { + case ModeNeverApprove: + res.ModeOverride = true + res.Stages = append(res.Stages, StageResult{Stage: StageFinal, Verdict: FinalBlocked, + Detail: map[string]interface{}{"reason": "mode override: never_approve"}}) + e.persist(ctx, orgID, reviewID, res, dryRun) + return res, nil + + case ModeAlwaysApprove: + res.ModeOverride = true + // The static jq gate still runs even under always_approve: a repo-wide + // "trust this repo" setting must never silence a secrets-leak detector. + var staticStage StageResult + if docErr == nil { + staticStage = e.staticStage(ctx, orgID, reviewID, p, doc) + } else { + staticStage = StageResult{Stage: StageStatic, Verdict: VerdictSkip, + Detail: map[string]interface{}{"reason": "no findings document"}} + } + res.Stages = append(res.Stages, staticStage) + res.Stages = append(res.Stages, skipModeStage(StageEligibility), skipModeStage(StageRisk), skipModeStage(StageLLM)) + + if staticStage.Verdict == VerdictFail { + res.Approved = false + res.Stages = append(res.Stages, StageResult{Stage: StageFinal, Verdict: FinalBlocked, + Detail: map[string]interface{}{"reason": "blocked by static ruleset despite always_approve: " + e.reason(staticStage)}}) + } else { + res.Approved = true + res.Stages = append(res.Stages, StageResult{Stage: StageFinal, Verdict: FinalApproved, + Detail: map[string]interface{}{"reason": "mode override: always_approve"}}) + } + e.persist(ctx, orgID, reviewID, res, dryRun) + return res, nil + } + + // custom (the full funnel) + author, authStage := e.eligibilityStage(ctx, orgID, reviewID, p) + res.Author = author + res.Stages = append(res.Stages, authStage) + blocked := authStage.Verdict == VerdictFail + + // static + if !blocked { + if docErr != nil { + st := StageResult{Stage: StageStatic, Verdict: VerdictFail, + Detail: map[string]interface{}{"reason": "failed to build findings document: " + docErr.Error()}} + res.Stages = append(res.Stages, st) + blocked = true + } else { + st := e.staticStage(ctx, orgID, reviewID, p, doc) + res.Stages = append(res.Stages, st) + blocked = st.Verdict == VerdictFail + } + } else { + res.Stages = append(res.Stages, skipStage(StageStatic)) + } + + // risk + if !blocked && docErr == nil { + rt := e.riskStage(ctx, orgID, reviewID, p, doc, dryRun) + res.Stages = append(res.Stages, rt.StageResult) + res.Score = rt.Score + res.Percentile = rt.Percentile + res.ThresholdCutoff = rt.Cutoff + blocked = rt.Verdict == VerdictFail + } else { + res.Stages = append(res.Stages, skipStage(StageRisk)) + } + + // llm + if !blocked && docErr == nil { + lt := e.llmStage(p, doc) + res.Stages = append(res.Stages, lt) + blocked = lt.Verdict == VerdictFail + } else { + res.Stages = append(res.Stages, skipStage(StageLLM)) + } + + // daily approve cap (only affects approval, recorded on the final stage) + finalReason := "approved" + if !blocked { + if capped, reason := e.dailyCapExceeded(ctx, orgID, p); capped { + blocked = true + finalReason = reason + } + } else { + finalReason = e.blockReason(res.Stages) + } + + res.Approved = !blocked + verdict := FinalBlocked + if res.Approved { + verdict = FinalApproved + } + res.Stages = append(res.Stages, StageResult{Stage: StageFinal, Verdict: verdict, + Detail: map[string]interface{}{"reason": finalReason}}) + + e.persist(ctx, orgID, reviewID, res, dryRun) + return res, nil +} + +// blockReason walks the stages for the first failing stage's reason. +func (e *Engine) blockReason(stages []StageResult) string { + for _, st := range stages { + if st.Verdict == VerdictFail { + return fmt.Sprintf("%s: %s", st.Stage, e.reason(st)) + } + } + return "blocked" +} + +// riskStageResult is the risk stage's outcome plus the score fields surfaced on +// the funnel result. +type riskStageResult struct { + StageResult + Score float64 + Percentile int + Cutoff float64 +} + +// eligibilityStage resolves the author and applies the whitelist + min-level rule. +func (e *Engine) eligibilityStage(ctx context.Context, orgID, reviewID int64, p *Policy) (*AuthorInfo, StageResult) { + author, err := e.resolveAuthor(ctx, orgID, reviewID) + if err != nil { + return nil, StageResult{Stage: StageEligibility, Verdict: VerdictFail, + Detail: map[string]interface{}{"reason": "failed to resolve author: " + err.Error()}} + } + + // Whitelist bypasses the level gate. + if author.Matched && containsInt64(p.EligibilityWhitelistUserIDs, author.UserID) { + return author, StageResult{Stage: StageEligibility, Verdict: VerdictPass, + Detail: map[string]interface{}{"reason": "author whitelisted", "level": author.Level, "matched": author.Matched}} + } + + level := author.Level + if author.IsBot && !author.Matched { + level = 0 + } + + if level >= p.MinAuthorLevel { + return author, StageResult{Stage: StageEligibility, Verdict: VerdictPass, + Detail: map[string]interface{}{"reason": fmt.Sprintf("author level %d >= required %d", level, p.MinAuthorLevel), "level": level, "matched": author.Matched}} + } + return author, StageResult{Stage: StageEligibility, Verdict: VerdictFail, + Detail: map[string]interface{}{ + "reason": fmt.Sprintf("author level %d < required %d", level, p.MinAuthorLevel), + "level": level, + "matched": author.Matched, + "unmapped": !author.Matched, + "author_name": author.Username, + }} +} + +// staticStage runs the configured blocking jq rulesets (reusing the CI/CD gate). +func (e *Engine) staticStage(ctx context.Context, orgID, reviewID int64, p *Policy, doc *canonicaldoc.ReviewDoc) StageResult { + if len(p.BlockingRulesetIDs) == 0 { + return StageResult{Stage: StageStatic, Verdict: VerdictPass, + Detail: map[string]interface{}{"reason": "no blocking rulesets configured"}} + } + for _, rsID := range p.BlockingRulesetIDs { + expr, name, err := e.loadRuleset(ctx, orgID, rsID) + if err != nil { + return StageResult{Stage: StageStatic, Verdict: VerdictFail, + Detail: map[string]interface{}{"reason": fmt.Sprintf("ruleset %d unavailable: %v", rsID, err)}} + } + blocked, _, evalErr := canonicaldoc.RunJQBool(expr, doc) + if evalErr != nil { + return StageResult{Stage: StageStatic, Verdict: VerdictFail, + Detail: map[string]interface{}{"reason": fmt.Sprintf("ruleset %q failed to evaluate: %v", name, evalErr)}} + } + if blocked { + return StageResult{Stage: StageStatic, Verdict: VerdictFail, + Detail: map[string]interface{}{"reason": fmt.Sprintf("blocked by ruleset %q", name), "ruleset_id": rsID, "ruleset_name": name}} + } + } + return StageResult{Stage: StageStatic, Verdict: VerdictPass, + Detail: map[string]interface{}{"reason": fmt.Sprintf("passed %d blocking ruleset(s)", len(p.BlockingRulesetIDs))}} +} + +// loadRuleset returns a ruleset's jq expression + name, org-scoped. +func (e *Engine) loadRuleset(ctx context.Context, orgID, rulesetID int64) (expr, name string, err error) { + err = e.db.QueryRowContext(ctx, + `SELECT jq_expr, name FROM ci_rulesets WHERE id = $1 AND org_id = $2`, rulesetID, orgID).Scan(&expr, &name) + return expr, name, err +} + +// riskStage scores the review against the trained model and compares it to the +// percentile threshold. +func (e *Engine) riskStage(ctx context.Context, orgID, reviewID int64, p *Policy, doc *canonicaldoc.ReviewDoc, dryRun bool) riskStageResult { + br, err := riskscore.LoadBlastStats(ctx, e.db, orgID, reviewID) + if err != nil { + br = riskscore.BlastStats{} + } + features := riskscore.ExtractFeatures(br, doc) + score, err := e.scorer.ScoreFor(ctx, orgID, features) + if err != nil { + return riskStageResult{StageResult: StageResult{Stage: StageRisk, Verdict: VerdictFail, + Detail: map[string]interface{}{"reason": "risk model unavailable: " + err.Error()}}} + } + + scores, _ := riskscore.LoadRecentScores(ctx, e.db, orgID, 1000) + cutoff := riskscore.PercentileCutoff(scores, p.RiskThresholdPercentile) + pct := riskscore.PercentileOf(score, scores) + + if !dryRun { + info, infoErr := e.scorer.Info(ctx, orgID) + if infoErr == nil { + _ = e.store.SaveFeatureSnapshot(ctx, reviewID, orgID, info.Version, features, score) + } + } + + detail := map[string]interface{}{ + "score": score, + "percentile": pct, + "cutoff": cutoff, + "threshold": p.RiskThresholdPercentile, + "features": featureTable(features), + } + if score >= cutoff { + detail["reason"] = fmt.Sprintf("risk score %.2f (P%d) >= threshold P%d (cutoff %.2f)", score, pct, p.RiskThresholdPercentile, cutoff) + return riskStageResult{StageResult: StageResult{Stage: StageRisk, Verdict: VerdictFail, Detail: detail}, Score: score, Percentile: pct, Cutoff: cutoff} + } + detail["reason"] = fmt.Sprintf("risk score %.2f (P%d) < threshold P%d (cutoff %.2f)", score, pct, p.RiskThresholdPercentile, cutoff) + return riskStageResult{StageResult: StageResult{Stage: StageRisk, Verdict: VerdictPass, Detail: detail}, Score: score, Percentile: pct, Cutoff: cutoff} +} + +// featureTable renders the feature vector as a human-readable list for the UI's +// explainability view. +func featureTable(features []float64) []map[string]interface{} { + out := make([]map[string]interface{}, 0, len(features)) + for i, v := range features { + if i >= len(riskscore.FeatureSpecs) { + break + } + out = append(out, map[string]interface{}{ + "feature": riskscore.FeatureSpecs[i].Name, + "value": v, + }) + } + return out +} + +// llmStage gates on the diff-level semantic classification (confidence + +// disqualifying class). +func (e *Engine) llmStage(p *Policy, doc *canonicaldoc.ReviewDoc) StageResult { + dc := doc.DiffClassification + if dc == nil { + return StageResult{Stage: StageLLM, Verdict: VerdictFail, + Detail: map[string]interface{}{"reason": "no diff classification available"}} + } + if dc.Confidence < p.RequireLLMConfidence { + return StageResult{Stage: StageLLM, Verdict: VerdictFail, + Detail: map[string]interface{}{ + "reason": fmt.Sprintf("classification confidence %d < required %d", dc.Confidence, p.RequireLLMConfidence), + "class": dc.Class, + "confidence": dc.Confidence, + }} + } + if isDisqualifyingClass(dc.Class) { + return StageResult{Stage: StageLLM, Verdict: VerdictFail, + Detail: map[string]interface{}{"reason": "disqualifying class: " + dc.Class, "class": dc.Class, "confidence": dc.Confidence}} + } + return StageResult{Stage: StageLLM, Verdict: VerdictPass, + Detail: map[string]interface{}{"reason": fmt.Sprintf("class %q, confidence %d", dc.Class, dc.Confidence), "class": dc.Class, "confidence": dc.Confidence}} +} + +// isDisqualifyingClass implements RADAR's "any risk signal disqualifies": a +// security-sensitive diff never auto-approves regardless of score. +func isDisqualifyingClass(class string) bool { + switch class { + case "security-sensitive": + return true + default: + return false + } +} + +// dailyCapExceeded reports whether the org's daily auto-approve cap has been +// reached (nil cap = unlimited). +func (e *Engine) dailyCapExceeded(ctx context.Context, orgID int64, p *Policy) (bool, string) { + if p.DailyApproveCap == nil { + return false, "" + } + var count int + err := e.db.QueryRowContext(ctx, ` + SELECT COUNT(*) FROM review_policy_log + WHERE org_id = $1 AND stage = 'final' AND verdict = 'approved' + AND created_at >= date_trunc('day', now())`, orgID).Scan(&count) + if err != nil { + return false, "" + } + if count >= *p.DailyApproveCap { + return true, fmt.Sprintf("daily approve cap reached (%d/%d)", count, *p.DailyApproveCap) + } + return false, "" +} + +// persist writes each stage to review_policy_log (skipped in dryRun). It +// replaces any prior evaluation of this review so a worker retry doesn't leave +// duplicate funnel rows. +func (e *Engine) persist(ctx context.Context, orgID, reviewID int64, res *FunnelResult, dryRun bool) { + if dryRun { + return + } + _, _ = e.db.ExecContext(ctx, `DELETE FROM review_policy_log WHERE review_id = $1 AND org_id = $2`, reviewID, orgID) + for _, st := range res.Stages { + detail, _ := json.Marshal(st.Detail) + _, _ = e.db.ExecContext(ctx, ` + INSERT INTO review_policy_log (review_id, org_id, stage, verdict, detail) + VALUES ($1, $2, $3, $4, $5)`, reviewID, orgID, st.Stage, st.Verdict, detail) + } +} + +func containsInt64(slice []int64, v int64) bool { + for _, x := range slice { + if x == v { + return true + } + } + return false +} diff --git a/internal/policyengine/policy.go b/internal/policyengine/policy.go new file mode 100644 index 000000000..764c4120b --- /dev/null +++ b/internal/policyengine/policy.go @@ -0,0 +1,114 @@ +// Package policyengine implements the risk-based auto-approve policy funnel: +// eligibility -> static jq gates -> trained risk score -> LLM classification +// -> final decision. See docs/risk-policy-engine.md. It reuses the CI/CD gate's +// canonical-document + jq evaluation (internal/canonicaldoc) for the static +// stage and the trained model (internal/riskscore) for the risk stage. +package policyengine + +import ( + "context" + "database/sql" + + "github.com/lib/pq" +) + +// Policy modes. +const ( + ModeInherit = "inherit" + ModeCustom = "custom" + ModeAlwaysApprove = "always_approve" + ModeNeverApprove = "never_approve" +) + +// Stage verdicts. +const ( + VerdictPass = "pass" + VerdictFail = "fail" + VerdictSkip = "skip" +) + +// Funnel stages. +const ( + StageEligibility = "eligibility" + StageStatic = "static" + StageRisk = "risk" + StageLLM = "llm" + StageFinal = "final" +) + +// Final verdicts. +const ( + FinalApproved = "approved" + FinalBlocked = "blocked" +) + +// Policy is one auto_approve_policies row (org-default or repo-scoped). +type Policy struct { + ID int64 + OrgID int64 + RepositoryID *int64 + Mode string + MinAuthorLevel int + EligibilityWhitelistUserIDs []int64 + RiskThresholdPercentile int + RequireLLMConfidence int + BlockingRulesetIDs []int64 + LandingDelaySeconds int + DailyApproveCap *int + RestrictOutcomeReportingToAdmins bool + ForceApproveMinLevel int +} + +// ResolvePolicy returns the effective policy for a review in orgID/repositoryID: +// the repo-scoped row if present and not 'inherit', else the org-default row, +// else (found=false) the feature is off. repositoryID 0 means "no repo scope". +func ResolvePolicy(ctx context.Context, db *sql.DB, orgID, repositoryID int64) (*Policy, bool, error) { + if repositoryID > 0 { + p, err := loadPolicy(ctx, db, orgID, &repositoryID) + if err != nil { + return nil, false, err + } + if p != nil && p.Mode != ModeInherit { + return p, true, nil + } + } + + p, err := loadPolicy(ctx, db, orgID, nil) + if err != nil { + return nil, false, err + } + if p == nil { + return nil, false, nil + } + return p, true, nil +} + +// loadPolicy loads one row; repositoryID nil selects the org-default row. +func loadPolicy(ctx context.Context, db *sql.DB, orgID int64, repositoryID *int64) (*Policy, error) { + row := db.QueryRowContext(ctx, ` + SELECT id, org_id, repository_id, mode, min_author_level, + eligibility_whitelist_user_ids, risk_threshold_percentile, + require_llm_confidence, blocking_ruleset_ids, landing_delay_seconds, + daily_approve_cap, restrict_outcome_reporting_to_admins, force_approve_min_level + FROM auto_approve_policies + WHERE org_id = $1 AND repository_id IS NOT DISTINCT FROM $2`, orgID, repositoryID) + + var p Policy + var repoID sql.NullInt64 + err := row.Scan( + &p.ID, &p.OrgID, &repoID, &p.Mode, &p.MinAuthorLevel, + pq.Array(&p.EligibilityWhitelistUserIDs), &p.RiskThresholdPercentile, + &p.RequireLLMConfidence, pq.Array(&p.BlockingRulesetIDs), &p.LandingDelaySeconds, + &p.DailyApproveCap, &p.RestrictOutcomeReportingToAdmins, &p.ForceApproveMinLevel, + ) + if err == sql.ErrNoRows { + return nil, nil + } + if err != nil { + return nil, err + } + if repoID.Valid { + p.RepositoryID = &repoID.Int64 + } + return &p, nil +} diff --git a/internal/policyengine/policy_test.go b/internal/policyengine/policy_test.go new file mode 100644 index 000000000..55cb838cd --- /dev/null +++ b/internal/policyengine/policy_test.go @@ -0,0 +1,141 @@ +package policyengine + +import ( + "context" + "database/sql" + "os" + "testing" + + _ "github.com/lib/pq" +) + +func testDB(t *testing.T) *sql.DB { + if testing.Short() { + t.Skip("skipping database integration test") + } + dsn := os.Getenv("DATABASE_URL") + if dsn == "" { + dsn = "postgres://livereview:livereview_password_123@localhost:5432/livereview?sslmode=disable" + } + db, err := sql.Open("postgres", dsn) + if err != nil { + t.Fatalf("open db: %v", err) + } + t.Cleanup(func() { _ = db.Close() }) + if err := db.Ping(); err != nil { + t.Skipf("db not reachable: %v", err) + } + return db +} + +func seedOrg(t *testing.T, db *sql.DB, name string) int64 { + var orgID int64 + if err := db.QueryRow(`INSERT INTO orgs (name) VALUES ($1) RETURNING id`, name).Scan(&orgID); err != nil { + t.Fatalf("insert org: %v", err) + } + t.Cleanup(func() { + // The live dev server's background dashboard-cache refresh job may have + // already inserted a row for this org by the time the test ends; its FK + // has no cascade, so it must be cleared first or the org delete leaks. + _, _ = db.Exec(`DELETE FROM dashboard_cache WHERE org_id = $1`, orgID) + _, _ = db.Exec(`DELETE FROM orgs WHERE id = $1`, orgID) + }) + return orgID +} + +func insertPolicy(t *testing.T, db *sql.DB, orgID int64, repoID *int64, mode string, minLevel int) int64 { + var id int64 + if err := db.QueryRow(` + INSERT INTO auto_approve_policies (org_id, repository_id, mode, min_author_level) + VALUES ($1, $2, $3, $4) RETURNING id`, orgID, repoID, mode, minLevel).Scan(&id); err != nil { + t.Fatalf("insert policy: %v", err) + } + // auto_approve_policies.org_id has no ON DELETE CASCADE, so this must be + // cleaned up before seedOrg's org delete (t.Cleanup runs LIFO, and this is + // registered after seedOrg's, so it runs first) or the org delete fails + // silently and leaks on every run. + t.Cleanup(func() { _, _ = db.Exec(`DELETE FROM auto_approve_policies WHERE id = $1`, id) }) + return id +} + +// No policy configured at all: ResolvePolicy reports the feature as off. +func TestResolvePolicyNoneConfigured(t *testing.T) { + db := testDB(t) + orgID := seedOrg(t, db, "policy-resolve-none") + + _, found, err := ResolvePolicy(context.Background(), db, orgID, 0) + if err != nil { + t.Fatalf("ResolvePolicy: %v", err) + } + if found { + t.Fatalf("expected found=false when no policy row exists") + } +} + +// Only the org-default row exists: a repo-scoped lookup falls back to it. +func TestResolvePolicyFallsBackToOrgDefault(t *testing.T) { + db := testDB(t) + orgID := seedOrg(t, db, "policy-resolve-fallback") + insertPolicy(t, db, orgID, nil, ModeCustom, 2) + + p, found, err := ResolvePolicy(context.Background(), db, orgID, 999) + if err != nil { + t.Fatalf("ResolvePolicy: %v", err) + } + if !found { + t.Fatalf("expected found=true via org-default fallback") + } + if p.RepositoryID != nil { + t.Fatalf("expected the org-default row (repository_id nil), got %+v", p.RepositoryID) + } + if p.MinAuthorLevel != 2 { + t.Fatalf("expected org default's min_author_level=2, got %d", p.MinAuthorLevel) + } +} + +// A repo row with mode='inherit' defers wholesale to the org default, even +// though a repo-scoped row exists. +func TestResolvePolicyInheritDefersToOrgDefault(t *testing.T) { + db := testDB(t) + orgID := seedOrg(t, db, "policy-resolve-inherit") + insertPolicy(t, db, orgID, nil, ModeCustom, 3) + repoID := int64(42) + insertPolicy(t, db, orgID, &repoID, ModeInherit, 1) + + p, found, err := ResolvePolicy(context.Background(), db, orgID, repoID) + if err != nil { + t.Fatalf("ResolvePolicy: %v", err) + } + if !found { + t.Fatalf("expected found=true") + } + if p.RepositoryID != nil { + t.Fatalf("expected the org-default row to win over an 'inherit' repo row, got repository_id=%v", p.RepositoryID) + } + if p.MinAuthorLevel != 3 { + t.Fatalf("expected org default's min_author_level=3, got %d", p.MinAuthorLevel) + } +} + +// A repo row with mode='custom' takes precedence over the org default. +func TestResolvePolicyCustomRepoRowWins(t *testing.T) { + db := testDB(t) + orgID := seedOrg(t, db, "policy-resolve-custom-repo") + insertPolicy(t, db, orgID, nil, ModeCustom, 1) + repoID := int64(43) + insertPolicy(t, db, orgID, &repoID, ModeCustom, 5) + + p, found, err := ResolvePolicy(context.Background(), db, orgID, repoID) + if err != nil { + t.Fatalf("ResolvePolicy: %v", err) + } + if !found { + t.Fatalf("expected found=true") + } + if p.RepositoryID == nil || *p.RepositoryID != repoID { + t.Fatalf("expected the repo-scoped row to win, got repository_id=%v", p.RepositoryID) + } + if p.MinAuthorLevel != 5 { + t.Fatalf("expected repo row's min_author_level=5, got %d", p.MinAuthorLevel) + } +} diff --git a/internal/prompts/registry_stub.go b/internal/prompts/registry_stub.go index 6eb39c08b..d777c8dcd 100644 --- a/internal/prompts/registry_stub.go +++ b/internal/prompts/registry_stub.go @@ -13,6 +13,7 @@ func PlaintextTemplates() []PlaintextTemplate { CommentRequirements + "\n\n" + JSONStructureExample + "\n\n" + TaxonomyClassificationRules + "\n\n" + + DiffClassificationRules + "\n\n" + CommentClassification + "\n\n" + LineNumberInstructions + "\n\n" + "{{VAR:style_guide}}\n\n{{VAR:security_guidelines}}"}, diff --git a/internal/prompts/templates.go b/internal/prompts/templates.go index a9b62984c..b73f320e9 100644 --- a/internal/prompts/templates.go +++ b/internal/prompts/templates.go @@ -87,7 +87,11 @@ const ( "suggestions": ["Specific improvement suggestion 1", "Specific improvement suggestion 2"], "isInternal": false } - ] + ], + "diffClassification": { + "class": "one of the classes from DIFF CLASSIFICATION RULES", + "confidence": 8 + } } ` + "```" ) @@ -121,7 +125,27 @@ TAXONOMY (category -> allowed subcategories): ` + "```" ) -// Issue detection checklist +// Diff classification rules +const ( + // DiffClassificationRules is a closed, fixed taxonomy for the diff-level + // semantic classification used by the auto-approve policy's LLM gate + // (docs/risk-policy-engine.md Β§5). "class" MUST be exactly one of these + // values, verbatim; "confidence" is an integer 0-10 expressing how sure the + // model is. This rides inside the same review JSON (no second LLM call). + DiffClassificationRules = `DIFF CLASSIFICATION RULES: +- After reviewing the whole diff, classify it with exactly one "class" from the closed list below, verbatim (no other values): + "refactor-only" (behavior-preserving rename/extract/cleanup; no logic change) + "dead-code-removal" (removing unused code/imports with no behavior change) + "docs-only" (documentation/comments/changelog changes only) + "test-only" (test additions or changes only) + "config-change" (build/dependency/CI/feature-flag or other configuration change) + "dependency-change" (adding/removing/upgrading third-party dependencies) + "data-migration" (schema or data migration changes) + "logic-change" (changes to runtime behavior, control flow, or business logic) + "security-sensitive" (changes touching auth, permissions, secrets, cryptography, input validation, or data exposure) +- "confidence" is an integer 0-10: how confident you are in the chosen class. Use 0 when genuinely unsure, 10 when unambiguous. +- Pick the single most dominant class for the whole diff. If the diff spans multiple concerns, choose the highest-risk one that applies.` +) const ( // IssueDetectionChecklist enumerates the categories and specific issues the reviewer must watch for IssueDetectionChecklist = `ISSUE DETECTION FOCUS AREAS: diff --git a/internal/review/service.go b/internal/review/service.go index 627c51f04..030a5d750 100644 --- a/internal/review/service.go +++ b/internal/review/service.go @@ -99,6 +99,7 @@ type ReviewResult struct { CommentsCount int Comments []*models.ReviewComment // Added to track actual comment details Quiz []models.QuizQuestion // Optional comprehension quiz for this review, nil if none generated + DiffClassification *models.DiffClassification // Diff-level semantic classification (docs/risk-policy-engine.md Β§5) BillableLOC int64 Provider string Model string @@ -346,6 +347,7 @@ func (s *Service) ProcessReview(ctx context.Context, request ReviewRequest) *Rev result.CommentsCount = len(reviewData.Result.Comments) result.Comments = reviewData.Result.Comments // Include actual comment details result.Quiz = reviewData.Result.Quiz + result.DiffClassification = reviewData.Result.DiffClassification result.BillableLOC = reviewData.BillableLOC providerName := strings.TrimSpace(request.AI.Type) if configuredProvider, ok := request.AI.Config["provider_name"].(string); ok && strings.TrimSpace(configuredProvider) != "" { diff --git a/internal/riskscore/calibration.go b/internal/riskscore/calibration.go new file mode 100644 index 000000000..a790a7dc4 --- /dev/null +++ b/internal/riskscore/calibration.go @@ -0,0 +1,90 @@ +package riskscore + +import ( + "context" + "database/sql" + "sort" +) + +// Percentile calibration turns a per-org risk_threshold_percentile into an +// actual score cutoff: the org's own recent score distribution defines "P20", +// not a global magic number. The curve is org-wide (one model per org), while +// the percentile read off it is repo-overridable. + +// defaultCutoff is the cold-start threshold (no scores yet): 0.5, i.e. block +// reviews whose risk score is at or above the neutral midpoint. With the seed +// prior (bias -0.5), a clean review scores ~0.38 (approves) and one critical +// finding scores ~0.6 (blocks). +const defaultCutoff = 0.5 + +// LoadRecentScores returns the org's most recent risk scores (newest first), +// from risk_feature_snapshots, limited to limit rows. +func LoadRecentScores(ctx context.Context, db *sql.DB, orgID int64, limit int) ([]float64, error) { + if limit <= 0 { + limit = 1000 + } + rows, err := db.QueryContext(ctx, + `SELECT score FROM risk_feature_snapshots WHERE org_id = $1 ORDER BY created_at DESC LIMIT $2`, orgID, limit) + if err != nil { + return nil, err + } + defer rows.Close() + + scores := make([]float64, 0, limit) + for rows.Next() { + var s float64 + if err := rows.Scan(&s); err != nil { + return nil, err + } + scores = append(scores, s) + } + return scores, rows.Err() +} + +// PercentileCutoff returns the score value at the p-th percentile of the +// distribution (nearest-rank). A review blocks when its score >= this value. +// Empty distribution returns defaultCutoff. +func PercentileCutoff(scores []float64, p int) float64 { + if len(scores) == 0 { + return defaultCutoff + } + if p <= 0 { + p = 1 + } + if p > 100 { + p = 100 + } + sorted := make([]float64, len(scores)) + copy(sorted, scores) + sort.Float64s(sorted) + + // Nearest-rank method: index = ceil(p/100 * n) - 1, clamped. + idx := (p*len(sorted)+99)/100 - 1 + if idx < 0 { + idx = 0 + } + if idx >= len(sorted) { + idx = len(sorted) - 1 + } + return sorted[idx] +} + +// PercentileOf returns the percentile rank (0-100) of score within the +// distribution (the percentage of scores <= score), for the UI's +// "risk score 34 (P45)" display. +func PercentileOf(score float64, scores []float64) int { + if len(scores) == 0 { + return 0 + } + sorted := make([]float64, len(scores)) + copy(sorted, scores) + sort.Float64s(sorted) + + le := 0 + for _, s := range sorted { + if s <= score { + le++ + } + } + return le * 100 / len(sorted) +} diff --git a/internal/riskscore/features.go b/internal/riskscore/features.go new file mode 100644 index 000000000..994e4eae8 --- /dev/null +++ b/internal/riskscore/features.go @@ -0,0 +1,118 @@ +// Package riskscore implements the trained risk model behind the auto-approve +// policy engine: feature extraction, an online-learning logistic regression +// (goml's linear.Logistic for batch gradient ascent, plus a custom +// prior-anchored regularizer and category-level weight overrides), and the +// percentile calibration that turns a per-org risk_threshold_percentile into a +// concrete cutoff. See docs/risk-policy-engine.md Β§3 for the design. +package riskscore + +import ( + "context" + "database/sql" + "math" + "strings" + + "github.com/livereview/internal/canonicaldoc" + "github.com/livereview/pkg/models" +) + +// FeatureSpec describes one model feature. The slice is append-only: never +// reorder or remove entries, because old model versions keep scoring with the +// same indices (a feature a model predates simply reads as 0). +type FeatureSpec struct { + Name string + Category string // override category; "" means no category-level override applies +} + +// FeatureSpecs is the canonical feature vector. goml adds its own bias term, so +// the feature vector itself carries no intercept; weights are stored as +// [bias, w0, w1, ...] where w_i corresponds to FeatureSpecs[i]. +var FeatureSpecs = []FeatureSpec{ + {"br_sum", "blast_radius"}, + {"br_max", "blast_radius"}, + {"br_weighted_avg", "blast_radius"}, + {"br_high_count", "blast_radius"}, + {"critical_count", "critical_findings"}, + {"warning_count", ""}, + {"security_count", "security_findings"}, + {"files_changed", "diff_size"}, + {"lines_added", "diff_size"}, + {"llm_class_risk", ""}, +} + +// FeatureCount returns the number of model features (excluding goml's bias). +func FeatureCount() int { return len(FeatureSpecs) } + +// BlastStats is the diff-level blast-radius aggregate the feature extractor +// consumes, loaded from blast_radius_hunks. Zero values mean "no blast radius +// data" (CLI review, disabled cache, or the scoring job hasn't finished yet); +// the policy engine treats that as zeros rather than risk. +type BlastStats struct { + Sum float64 + Max float64 + WeightedAvg float64 + HighCount int + FileCount int + LinesAdded int +} + +// LoadBlastStats reads the blast-radius aggregates for a review from +// blast_radius_hunks, scoped to orgID. +func LoadBlastStats(ctx context.Context, db *sql.DB, orgID, reviewID int64) (BlastStats, error) { + var br BlastStats + err := db.QueryRowContext(ctx, ` + SELECT + COALESCE(SUM(combined), 0), + COALESCE(MAX(combined), 0), + COALESCE(SUM(combined * new_lines) / NULLIF(SUM(new_lines), 0), 0), + COALESCE(COUNT(*) FILTER (WHERE tier = 'blast-radius-high'), 0), + COALESCE(COUNT(DISTINCT file_path), 0), + COALESCE(SUM(new_lines), 0) + FROM blast_radius_hunks + WHERE review_id = $1 AND org_id = $2`, reviewID, orgID). + Scan(&br.Sum, &br.Max, &br.WeightedAvg, &br.HighCount, &br.FileCount, &br.LinesAdded) + if err != nil { + return br, err + } + return br, nil +} + +// ExtractFeatures builds the length-FeatureCount() feature vector from blast +// radius aggregates, finding-severity counts from the canonical review doc, and +// (when present) the diff-level semantic classification. Feature values are +// lightly bounded/compressed (log1p on counts) so no single feature dominates +// the dot product. +func ExtractFeatures(br BlastStats, doc *canonicaldoc.ReviewDoc) []float64 { + f := make([]float64, FeatureCount()) + f[0] = br.Sum + f[1] = br.Max + f[2] = br.WeightedAvg + f[3] = float64(br.HighCount) + if doc != nil { + f[4] = float64(doc.Counts.BySeverity["critical"]) + f[5] = float64(doc.Counts.BySeverity["warning"]) + f[6] = float64(doc.Counts.ByCategory["security"]) + } + f[7] = math.Log1p(float64(br.FileCount)) + f[8] = math.Log1p(float64(br.LinesAdded)) + if doc != nil { + f[9] = classRisk(doc.DiffClassification) + } + return f +} + +// classRisk maps the diff-level semantic classification to a coarse risk +// ordinal (0 safe, 0.5 mixed, 1 risky). Absent/unknown classification reads 0. +func classRisk(dc *models.DiffClassification) float64 { + if dc == nil { + return 0 + } + switch strings.ToLower(strings.TrimSpace(dc.Class)) { + case "logic-change", "security-sensitive": + return 1 + case "config-change", "dependency-change", "data-migration": + return 0.5 + default: // refactor-only, dead-code-removal, docs-only, test-only, unknown + return 0 + } +} diff --git a/internal/riskscore/model.go b/internal/riskscore/model.go new file mode 100644 index 000000000..36358073c --- /dev/null +++ b/internal/riskscore/model.go @@ -0,0 +1,174 @@ +package riskscore + +import ( + "encoding/json" + "io" + "math" + + "github.com/cdipaolo/goml/base" + "github.com/cdipaolo/goml/linear" +) + +// Weights is one model's weight vector, laid out exactly as goml's +// Logistic.Parameters: index 0 is the bias, index i+1 is the weight for +// FeatureSpecs[i]. A row in risk_model_versions.weights_json is a []float64 of +// length FeatureCount()+1. +type Weights []float64 + +// AnchorLambda is the prior-anchor strength constant. The effective anchor on a +// batch refit is AnchorStrength(n) = lambda / (lambda + n), so with no labeled +// outcomes the fitted model stays at the common-sense prior, and by a few +// hundred outcomes the prior's pull is negligible. +const AnchorLambda = 1.0 + +// Prior returns the hand-set common-sense seed weights (docs/risk-policy-engine.md +// Β§3 "Cold start"). Blast-radius magnitude and critical-finding count dominate, +// security findings are weighted up, info-level findings and small diffs are +// weighted near zero. This is the starting point every org's model begins from +// and is anchored back to. +func Prior() Weights { + w := make(Weights, FeatureCount()+1) + w[0] = -0.5 // bias: slight default toward "safe" + w[1] = 0.02 // br_sum (0-100) + w[2] = 0.01 // br_max + w[3] = 0.01 // br_weighted_avg + w[4] = 0.20 // br_high_count + w[5] = 0.90 // critical_count (dominates) + w[6] = 0.18 // warning_count + w[7] = 0.55 // security_count (weighted up) + w[8] = 0.10 // files_changed (log1p) + w[9] = 0.05 // lines_added (log1p) + w[10] = 0.30 // llm_class_risk + return w +} + +// AnchorStrength returns the blend factor toward the prior for n labeled +// outcomes: 1.0 with no data (fully prior), decaying as n grows. +func AnchorStrength(n int) float64 { + return AnchorLambda / (AnchorLambda + float64(n)) +} + +func sigmoid(z float64) float64 { + if z >= 0 { + zExp := math.Exp(-z) + return 1 / (1 + zExp) + } + zExp := math.Exp(z) + return zExp / (1 + zExp) +} + +// categoryOverride returns the multiplier for a weight index (0 = bias, no +// category), defaulting to 1.0 when no override is set. +func categoryOverride(overrides map[string]float64, weightIndex int) float64 { + if overrides == nil || weightIndex == 0 { + return 1 + } + spec := FeatureSpecs[weightIndex-1] + if spec.Category == "" { + return 1 + } + if m, ok := overrides[spec.Category]; ok && m > 0 { + return m + } + return 1 +} + +// Score computes sigmoid(bias + Ξ£ effective_w_i * x_i), where +// effective_w_i = w_i * override_multiplier[category(i)]. A single dot product. +func Score(w Weights, x []float64, overrides map[string]float64) float64 { + var z float64 + if len(w) > 0 { + z = w[0] // bias has no category override + } + for i := 0; i < len(x) && i+1 < len(w); i++ { + z += w[i+1] * categoryOverride(overrides, i+1) * x[i] + } + return sigmoid(z) +} + +// scoreRaw is the raw dot product (no overrides), used during training and AUC. +func scoreRaw(w Weights, x []float64) float64 { + var z float64 + if len(w) > 0 { + z = w[0] + } + for i := 0; i < len(x) && i+1 < len(w); i++ { + z += w[i+1] * x[i] + } + return sigmoid(z) +} + +// BatchFit fits a fresh logistic model over (X, Y) using goml's batch gradient +// ascent (regularization disabled inside goml; the prior anchor is applied as a +// post-fit blend toward the common-sense prior). With no training data it +// returns the prior unchanged. +func BatchFit(X [][]float64, Y []float64, prior Weights, sampleCount int, maxIter int) Weights { + if len(X) == 0 || len(X) != len(Y) { + return cloneWeights(prior) + } + if maxIter <= 0 { + maxIter = 200 + } + + model := linear.NewLogistic(base.BatchGA, 1e-3, 0, maxIter, X, Y, FeatureCount()) + model.Output = io.Discard + if err := model.Learn(); err != nil { + // goml failed (diverged / bad input): keep the prior rather than a broken model. + return cloneWeights(prior) + } + + fitted := Weights(model.Parameters) + a := AnchorStrength(sampleCount) + out := make(Weights, len(fitted)) + for i := range fitted { + out[i] = (1-a)*fitted[i] + a*prior[i] + } + return out +} + +// OnlineStep applies one stochastic gradient ascent step for a single labeled +// example (the real-time nudge fired when a review_outcomes row lands), +// shrinking toward the prior so a single noisy example can't swing the weights +// away from common sense. Returns a new weight vector (the old one is not +// mutated). +func OnlineStep(w Weights, x []float64, y float64, prior Weights, sampleCount int, alpha float64) Weights { + if alpha <= 0 { + alpha = 0.01 + } + + p := scoreRaw(w, x) + err := y - p + + // Per-step anchor: strong when data is scarce, negligible once calibrated. + shrink := AnchorLambda / (1 + float64(sampleCount)) + + out := cloneWeights(w) + // bias term (x_0 = 1) + if len(out) > 0 { + out[0] += alpha * (err*1 - shrink*(out[0]-prior[0])) + } + for i := 0; i < len(x) && i+1 < len(out); i++ { + out[i+1] += alpha * (err*x[i] - shrink*(out[i+1]-prior[i+1])) + } + return out +} + +func cloneWeights(w Weights) Weights { + out := make(Weights, len(w)) + copy(out, w) + return out +} + +// MarshalWeights encodes a weight vector for risk_model_versions.weights_json. +func MarshalWeights(w Weights) ([]byte, error) { + return json.Marshal([]float64(w)) +} + +// UnmarshalWeights decodes a weights_json value. +func UnmarshalWeights(raw []byte) (Weights, error) { + var w Weights + if err := json.Unmarshal(raw, &w); err != nil { + return nil, err + } + return w, nil +} diff --git a/internal/riskscore/refit.go b/internal/riskscore/refit.go new file mode 100644 index 000000000..a905a45c7 --- /dev/null +++ b/internal/riskscore/refit.go @@ -0,0 +1,213 @@ +package riskscore + +import ( + "context" + "database/sql" + "encoding/json" + "fmt" +) + +// TrainingExample is one labeled (features, risky) pair for model fitting, +// built from risk_feature_snapshots β‹ˆ review_outcomes. +type TrainingExample struct { + ReviewID int64 + Features []float64 + Risky float64 // 1.0 risky (revert/incident), 0.0 safe (confirmed_safe) +} + +// LoadTrainingSet returns the org's labeled training examples, excluding +// 'test'-source outcomes so the setup-panel round-trip never pollutes the model. +func (s *Store) LoadTrainingSet(ctx context.Context, orgID int64) ([]TrainingExample, error) { + rows, err := s.db.QueryContext(ctx, ` + SELECT s.review_id, s.features, o.outcome_type + FROM risk_feature_snapshots s + JOIN review_outcomes o ON o.review_id = s.review_id AND o.org_id = s.org_id + WHERE s.org_id = $1 AND o.source <> 'test' + ORDER BY s.created_at`, orgID) + if err != nil { + return nil, err + } + defer rows.Close() + + var out []TrainingExample + for rows.Next() { + var reviewID int64 + var raw []byte + var outcomeType string + if err := rows.Scan(&reviewID, &raw, &outcomeType); err != nil { + return nil, err + } + var features []float64 + if err := json.Unmarshal(raw, &features); err != nil { + continue + } + out = append(out, TrainingExample{ + ReviewID: reviewID, + Features: features, + Risky: outcomeLabel(outcomeType), + }) + } + return out, rows.Err() +} + +// outcomeLabel maps an outcome_type to a training label. +func outcomeLabel(outcomeType string) float64 { + switch outcomeType { + case "revert", "incident": + return 1 + default: // confirmed_safe + return 0 + } +} + +// ApplyOutcome is the real-time online nudge (path 1): when a review_outcomes +// row lands, load that review's snapshot as the labeled example and take one +// gradient step on the org's current weights, persisting a new active version. +// source 'test' outcomes are skipped by the caller. +func (s *Store) ApplyOutcome(ctx context.Context, orgID, reviewID int64, outcomeType string) error { + var raw []byte + err := s.db.QueryRowContext(ctx, + `SELECT features FROM risk_feature_snapshots WHERE review_id = $1 AND org_id = $2`, + reviewID, orgID).Scan(&raw) + if err == sql.ErrNoRows { + return nil // no snapshot (e.g. policy never scored this review); nothing to learn + } + if err != nil { + return err + } + var features []float64 + if err := json.Unmarshal(raw, &features); err != nil { + return err + } + if len(features) != FeatureCount() { + return fmt.Errorf("feature vector length %d != %d; model drifted", len(features), FeatureCount()) + } + + _, weights, sampleCount, err := s.ActiveWeights(ctx, orgID) + if err != nil { + return err + } + + next := OnlineStep(weights, features, outcomeLabel(outcomeType), Prior(), sampleCount, 0.01) + _, err = s.SaveVersion(ctx, orgID, next, sampleCount+1, nil, "online_update") + return err +} + +// RefitResult summarizes a batch refit. +type RefitResult struct { + OrgID int64 `json:"org_id"` + TrainingCount int `json:"training_count"` + HoldoutAUC float64 `json:"holdout_auc"` + Promoted bool `json:"promoted"` + Version int `json:"version,omitempty"` + Reason string `json:"reason"` +} + +// Refit is the daily/on-demand batch refit (paths 2/3): fit a fresh logistic +// model over all labeled snapshots, evaluate AUC, and promote only if it is at +// least as good as the current active version (or the current version is the +// seed). Corrects drift a long sequence of noisy online steps can accumulate. +func (s *Store) Refit(ctx context.Context, orgID int64) (*RefitResult, error) { + examples, err := s.LoadTrainingSet(ctx, orgID) + if err != nil { + return nil, err + } + res := &RefitResult{OrgID: orgID, TrainingCount: len(examples), Promoted: false} + if len(examples) == 0 { + res.Reason = "no labeled outcomes yet" + return res, nil + } + + X := make([][]float64, 0, len(examples)) + Y := make([]float64, 0, len(examples)) + for _, ex := range examples { + X = append(X, padFeatures(ex.Features)) + Y = append(Y, ex.Risky) + } + + trainX, trainY, evalX, evalY := splitHoldout(X, Y) + fitted := BatchFit(trainX, trainY, Prior(), len(trainX), 200) + auc := rankAUC(scoreAll(evalX, fitted), evalY) + res.HoldoutAUC = auc + + _, currentWeights, currentSamples, err := s.ActiveWeights(ctx, orgID) + if err != nil { + return nil, err + } + currentAUC := rankAUC(scoreAll(evalX, currentWeights), evalY) + + if currentSamples == 0 || auc >= currentAUC { + version, err := s.NextVersion(ctx, orgID) + if err != nil { + return nil, err + } + if _, err := s.SaveVersion(ctx, orgID, fitted, len(examples), &auc, "batch_refit"); err != nil { + return nil, err + } + res.Promoted = true + res.Version = version + res.Reason = "promoted (AUC improved or replaced seed)" + } else { + res.Reason = fmt.Sprintf("not promoted (AUC %.3f < current %.3f)", auc, currentAUC) + } + return res, nil +} + +// scoreAll scores each row with the given (raw) weights. +func scoreAll(X [][]float64, w Weights) []float64 { + out := make([]float64, len(X)) + for i := range X { + out[i] = scoreRaw(w, X[i]) + } + return out +} + +// padFeatures returns a copy of f padded/truncated to FeatureCount(). +func padFeatures(f []float64) []float64 { + out := make([]float64, FeatureCount()) + for i := range out { + if i < len(f) { + out[i] = f[i] + } + } + return out +} + +// splitHoldout splits X/Y into 80% train / 20% eval when >= 20 samples, else +// returns all data as both train and eval (in-sample AUC). +func splitHoldout(X [][]float64, Y []float64) ([][]float64, []float64, [][]float64, []float64) { + if len(X) < 20 { + return X, Y, X, Y + } + n := len(X) + trainN := n * 4 / 5 + return X[:trainN], Y[:trainN], X[trainN:], Y[trainN:] +} + +// rankAUC ranks scores against binary labels (pairwise). Returns 0.5 when +// there are no positive or no negative examples. +func rankAUC(scores, labels []float64) float64 { + nPos, nNeg := 0, 0 + rank := 0.0 + for i, si := range scores { + if labels[i] == 1 { + nPos++ + for j, sj := range scores { + if labels[j] == 0 { + switch { + case si > sj: + rank++ + case si == sj: + rank += 0.5 + } + } + } + } else { + nNeg++ + } + } + if nPos == 0 || nNeg == 0 { + return 0.5 + } + return rank / float64(nPos*nNeg) +} diff --git a/internal/riskscore/riskscore_test.go b/internal/riskscore/riskscore_test.go new file mode 100644 index 000000000..f719b5ad6 --- /dev/null +++ b/internal/riskscore/riskscore_test.go @@ -0,0 +1,127 @@ +package riskscore + +import ( + "math" + "testing" +) + +func TestPriorLength(t *testing.T) { + if got := len(Prior()); got != FeatureCount()+1 { + t.Fatalf("Prior length = %d, want %d", got, FeatureCount()+1) + } +} + +func TestScoreInRange(t *testing.T) { + prior := Prior() + x := make([]float64, FeatureCount()) + for i := range x { + x[i] = 1 + } + for _, overrides := range []map[string]float64{nil, {"blast_radius": 3, "security_findings": 0.5}} { + s := Score(prior, x, overrides) + if s < 0 || s > 1 { + t.Fatalf("score %v out of [0,1]", s) + } + } +} + +func TestScoreOverrideEffect(t *testing.T) { + prior := Prior() + x := make([]float64, FeatureCount()) + x[0] = 50 // br_sum + x[4] = 1 // critical_count + + base := Score(prior, x, nil) + boosted := Score(prior, x, map[string]float64{"blast_radius": 10}) + if boosted <= base { + t.Fatalf("boosting blast_radius should raise risk score: base %v, boosted %v", base, boosted) + } +} + +func TestBatchFitEmptyReturnsPrior(t *testing.T) { + prior := Prior() + got := BatchFit(nil, nil, prior, 0, 100) + if len(got) != len(prior) { + t.Fatalf("length mismatch") + } + for i := range got { + if math.Abs(got[i]-prior[i]) > 1e-9 { + t.Fatalf("BatchFit(empty) drifted from prior at %d: %v vs %v", i, got[i], prior[i]) + } + } +} + +func TestBatchFitWithData(t *testing.T) { + prior := Prior() + // A dataset strongly predictive of risk: critical_count high -> risky. + X := [][]float64{} + Y := []float64{} + for i := 0; i < 40; i++ { + x := make([]float64, FeatureCount()) + x[4] = float64(i % 5) // critical_count varies 0..4 + X = append(X, x) + Y = append(Y, []float64{0, 0, 1, 1, 1}[i%5]) + } + fitted := BatchFit(X, Y, prior, len(X), 200) + if len(fitted) != len(prior) { + t.Fatalf("length mismatch") + } + // A high-critical example should score above a low-critical one. + low := make([]float64, FeatureCount()) + high := make([]float64, FeatureCount()) + high[4] = 4 + if Score(fitted, low, nil) >= Score(fitted, high, nil) { + t.Fatalf("high-critical example should score higher") + } +} + +func TestOnlineStepShrinksTowardPrior(t *testing.T) { + prior := Prior() + w := cloneWeights(prior) + w[5] = prior[5] + 5 // perturb critical_count weight far from prior + x := make([]float64, FeatureCount()) + next := OnlineStep(w, x, 0, prior, 0, 0.1) + if math.Abs(next[5]-prior[5]) >= math.Abs(w[5]-prior[5]) { + t.Fatalf("online step should shrink toward prior: before %v -> %v, prior %v", w[5], next[5], prior[5]) + } +} + +func TestAnchorStrength(t *testing.T) { + if AnchorStrength(0) != 1.0 { + t.Fatalf("AnchorStrength(0) = %v, want 1.0", AnchorStrength(0)) + } + if AnchorStrength(1000) >= 0.01 { + t.Fatalf("AnchorStrength(1000) should be negligible, got %v", AnchorStrength(1000)) + } +} + +func TestPercentileCutoff(t *testing.T) { + if got := PercentileCutoff(nil, 20); got != defaultCutoff { + t.Fatalf("empty distribution cutoff = %v, want %v", got, defaultCutoff) + } + scores := []float64{0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0} + if got := PercentileCutoff(scores, 20); math.Abs(got-0.2) > 1e-9 { + t.Fatalf("P20 of 0.1..1.0 = %v, want 0.2", got) + } + if got := PercentileCutoff(scores, 50); math.Abs(got-0.5) > 1e-9 { + t.Fatalf("P50 = %v, want 0.5", got) + } +} + +func TestPercentileOf(t *testing.T) { + scores := []float64{0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9, 1.0} + if got := PercentileOf(0.5, scores); got != 50 { + t.Fatalf("PercentileOf(0.5) = %d, want 50", got) + } + if got := PercentileOf(1.0, scores); got != 100 { + t.Fatalf("PercentileOf(1.0) = %d, want 100", got) + } +} + +func TestRankAUC(t *testing.T) { + scores := []float64{0.9, 0.8, 0.7, 0.3, 0.2, 0.1} + labels := []float64{1, 1, 1, 0, 0, 0} + if got := rankAUC(scores, labels); math.Abs(got-1.0) > 1e-9 { + t.Fatalf("perfect separation AUC = %v, want 1.0", got) + } +} diff --git a/internal/riskscore/scorer.go b/internal/riskscore/scorer.go new file mode 100644 index 000000000..cafb4d69d --- /dev/null +++ b/internal/riskscore/scorer.go @@ -0,0 +1,96 @@ +package riskscore + +import ( + "context" + "database/sql" + "sync" +) + +// The scorer keeps each org's active weights (and category overrides) cached in +// memory; a scoring call is a single dot product with no DB round-trip. The +// cache is invalidated whenever a new version activates (online nudge, batch +// refit) or an override is saved, via Invalidate. + +type cachedOrg struct { + version int + sampleCount int + weights Weights + overrides map[string]float64 +} + +var ( + cacheMu sync.Mutex + cache = map[int64]cachedOrg{} +) + +// Invalidate drops the cached model for orgID so the next ScoreFor reloads it. +func Invalidate(orgID int64) { + cacheMu.Lock() + delete(cache, orgID) + cacheMu.Unlock() +} + +// Scorer scores feature vectors against the org's active model. A thin +// interface wrapper around ScoreFor keeps the door open for an ONNX-backed +// implementation later (see docs/risk-policy-engine.md Β§3 ONNX note). +type Scorer struct { + db *sql.DB + store *Store +} + +// NewScorer returns a Scorer backed by db. +func NewScorer(db *sql.DB) *Scorer { return &Scorer{db: db, store: NewStore(db)} } + +// ModelInfo is the "model confidence" signal surfaced in the UI: which version +// is active and how many labeled outcomes trained it (0 = common-sense seed). +type ModelInfo struct { + Version int `json:"version"` + TrainingSamples int `json:"training_samples"` + IsSeed bool `json:"is_seed"` +} + +// Info returns the active model's version and training-sample count without +// scoring, for the settings UI's model-confidence indicator. +func (s *Scorer) Info(ctx context.Context, orgID int64) (ModelInfo, error) { + cacheMu.Lock() + c, ok := cache[orgID] + cacheMu.Unlock() + if ok { + return ModelInfo{Version: c.version, TrainingSamples: c.sampleCount, IsSeed: c.sampleCount == 0}, nil + } + // ActiveWeights lazily creates the shared seed model if it doesn't exist yet. + version, _, sampleCount, err := s.store.ActiveWeights(ctx, orgID) + if err != nil { + return ModelInfo{}, err + } + return ModelInfo{Version: version, TrainingSamples: sampleCount, IsSeed: sampleCount == 0}, nil +} + +// ScoreFor computes sigmoid(Ξ£ effective_w Β· x) against the org's active model, +// loading and caching it on first use. +func (s *Scorer) ScoreFor(ctx context.Context, orgID int64, x []float64) (float64, error) { + cacheMu.Lock() + c, ok := cache[orgID] + cacheMu.Unlock() + if ok { + return Score(c.weights, x, c.overrides), nil + } + + if err := s.store.EnsureSeedModel(ctx); err != nil { + return 0, err + } + version, weights, sampleCount, err := s.store.ActiveWeights(ctx, orgID) + if err != nil { + return 0, err + } + overrides, err := s.store.LoadOverrides(ctx, orgID) + if err != nil { + return 0, err + } + + cacheMu.Lock() + cache[orgID] = cachedOrg{version: version, sampleCount: sampleCount, weights: weights, overrides: overrides} + cacheMu.Unlock() + + return Score(weights, x, overrides), nil +} diff --git a/internal/riskscore/store.go b/internal/riskscore/store.go new file mode 100644 index 000000000..e8e188436 --- /dev/null +++ b/internal/riskscore/store.go @@ -0,0 +1,181 @@ +package riskscore + +import ( + "context" + "database/sql" + "encoding/json" + "fmt" +) + +// Store bundles the DB access for the trained risk model: seed/version +// management, feature snapshots, and category overrides. Mutating functions +// invalidate the scorer's in-memory cache for the affected org. +type Store struct { + db *sql.DB +} + +// NewStore returns a Store backed by db. +func NewStore(db *sql.DB) *Store { return &Store{db: db} } + +// EnsureSeedModel inserts the single shared seed model (org_id NULL, +// update_kind 'seed') if it doesn't already exist. Idempotent. +func (s *Store) EnsureSeedModel(ctx context.Context) error { + var exists bool + err := s.db.QueryRowContext(ctx, + `SELECT EXISTS(SELECT 1 FROM risk_model_versions WHERE org_id IS NULL AND update_kind = 'seed')`).Scan(&exists) + if err != nil { + return fmt.Errorf("checking seed model: %w", err) + } + if exists { + return nil + } + weights, err := MarshalWeights(Prior()) + if err != nil { + return err + } + _, err = s.db.ExecContext(ctx, ` + INSERT INTO risk_model_versions (org_id, version, weights_json, training_sample_count, update_kind, is_active) + VALUES (NULL, 1, $1, 0, 'seed', false)`, weights) + if err != nil { + return fmt.Errorf("inserting seed model: %w", err) + } + return nil +} + +// ActiveWeights returns the org's active weights, falling back to the shared +// seed when the org has no active version yet. Lazily creates the seed model +// if it doesn't exist yet (e.g. no review has ever been scored in this +// deployment), so callers never see a bare "no rows" error. +func (s *Store) ActiveWeights(ctx context.Context, orgID int64) (version int, weights Weights, sampleCount int, err error) { + var raw []byte + err = s.db.QueryRowContext(ctx, ` + SELECT version, weights_json, training_sample_count + FROM risk_model_versions + WHERE org_id = $1 AND is_active + ORDER BY id DESC LIMIT 1`, orgID).Scan(&version, &raw, &sampleCount) + if err == nil { + weights, err = UnmarshalWeights(raw) + return version, weights, sampleCount, err + } + if err != sql.ErrNoRows { + return 0, nil, 0, err + } + + if err := s.EnsureSeedModel(ctx); err != nil { + return 0, nil, 0, err + } + + // Fall back to the shared seed. + err = s.db.QueryRowContext(ctx, ` + SELECT version, weights_json, training_sample_count + FROM risk_model_versions + WHERE org_id IS NULL AND update_kind = 'seed' + ORDER BY id DESC LIMIT 1`).Scan(&version, &raw, &sampleCount) + if err != nil { + return 0, nil, 0, err + } + weights, err = UnmarshalWeights(raw) + return version, weights, sampleCount, err +} + +// NextVersion returns the next per-org version number (1 if none yet). +func (s *Store) NextVersion(ctx context.Context, orgID int64) (int, error) { + var v int + err := s.db.QueryRowContext(ctx, + `SELECT COALESCE(MAX(version), 0) + 1 FROM risk_model_versions WHERE org_id = $1`, orgID).Scan(&v) + return v, err +} + +// SaveVersion appends a new active version (deactivating the previous one) and +// returns the new row id. Append-only: any version can be reactivated later. +func (s *Store) SaveVersion(ctx context.Context, orgID int64, weights Weights, sampleCount int, auc *float64, kind string) (int64, error) { + raw, err := MarshalWeights(weights) + if err != nil { + return 0, err + } + version, err := s.NextVersion(ctx, orgID) + if err != nil { + return 0, err + } + + tx, err := s.db.BeginTx(ctx, nil) + if err != nil { + return 0, err + } + defer func() { _ = tx.Rollback() }() + + if _, err := tx.ExecContext(ctx, `UPDATE risk_model_versions SET is_active = false WHERE org_id = $1 AND is_active`, orgID); err != nil { + return 0, err + } + var id int64 + err = tx.QueryRowContext(ctx, ` + INSERT INTO risk_model_versions (org_id, version, weights_json, training_sample_count, holdout_auc, update_kind, is_active) + VALUES ($1, $2, $3, $4, $5, $6, true) RETURNING id`, + orgID, version, raw, sampleCount, auc, kind).Scan(&id) + if err != nil { + return 0, err + } + if err := tx.Commit(); err != nil { + return 0, err + } + + Invalidate(orgID) + return id, nil +} + +// SaveFeatureSnapshot records the feature vector and score at evaluation time +// (the training set once review_outcomes attach to the same review_id). One +// snapshot per review: a re-evaluation replaces the prior row. +func (s *Store) SaveFeatureSnapshot(ctx context.Context, reviewID, orgID int64, modelVersion int, features []float64, score float64) error { + raw, err := json.Marshal(features) + if err != nil { + return err + } + if _, err := s.db.ExecContext(ctx, + `DELETE FROM risk_feature_snapshots WHERE review_id = $1 AND org_id = $2`, reviewID, orgID); err != nil { + return err + } + _, err = s.db.ExecContext(ctx, ` + INSERT INTO risk_feature_snapshots (review_id, org_id, model_version, features, score) + VALUES ($1, $2, $3, $4, $5)`, reviewID, orgID, modelVersion, raw, score) + return err +} + +// LoadOverrides returns the org's category multipliers, defaulting missing +// categories to 1.0. +func (s *Store) LoadOverrides(ctx context.Context, orgID int64) (map[string]float64, error) { + rows, err := s.db.QueryContext(ctx, ` + SELECT feature_category, multiplier FROM risk_factor_overrides WHERE org_id = $1`, orgID) + if err != nil { + return nil, err + } + defer rows.Close() + + out := map[string]float64{} + for rows.Next() { + var cat string + var m float64 + if err := rows.Scan(&cat, &m); err != nil { + return nil, err + } + out[cat] = m + } + return out, rows.Err() +} + +// UpsertOverride sets a category multiplier. Takes effect immediately (the +// scorer cache is invalidated); the next batch refit treats it as an anchor. +func (s *Store) UpsertOverride(ctx context.Context, orgID int64, category string, multiplier float64) error { + if multiplier <= 0 { + return fmt.Errorf("multiplier must be > 0") + } + _, err := s.db.ExecContext(ctx, ` + INSERT INTO risk_factor_overrides (org_id, feature_category, multiplier) + VALUES ($1, $2, $3) + ON CONFLICT (org_id, feature_category) DO UPDATE SET multiplier = EXCLUDED.multiplier, updated_at = now()`, + orgID, category, multiplier) + if err == nil { + Invalidate(orgID) + } + return err +} diff --git a/pkg/models/models.go b/pkg/models/models.go index df35411f0..bb2366a07 100644 --- a/pkg/models/models.go +++ b/pkg/models/models.go @@ -55,6 +55,9 @@ type User struct { CreatedByUserID *int64 `json:"created_by_user_id,omitempty" db:"created_by_user_id"` PasswordResetRequired bool `json:"password_reset_required" db:"password_reset_required"` DefaultOrgID *int64 `json:"default_org_id,omitempty" db:"default_org_id"` + // ReviewLevel is the seniority dial gating auto-approve eligibility + // (docs/risk-policy-engine.md Β§1). Default 1; owners bump it per user. + ReviewLevel int `json:"review_level" db:"review_level"` } // FullName returns the user's display name (first + last name), falling back to @@ -164,6 +167,18 @@ type ReviewResult struct { Comments []*ReviewComment // External comments to be posted to the platform InternalComments []*ReviewComment // Internal comments used for synthesis only Quiz []QuizQuestion // Optional: 5-question comprehension quiz for this PR, nil if none was generated + // DiffClassification is the diff-level semantic classification the review + // LLM emits alongside findings (docs/risk-policy-engine.md Β§5). Nil for + // CLI/legacy reviews and for degraded responses where the field was absent. + DiffClassification *DiffClassification +} + +// DiffClassification is the closed-taxonomy diff-level classification used by +// the auto-approve policy's LLM gate: Class is one of the values enumerated in +// prompts.DiffClassificationTaxonomy, Confidence is 0-10. +type DiffClassification struct { + Class string `json:"class"` + Confidence int `json:"confidence"` } // QuizQuestion is one multiple-choice question in a review's comprehension diff --git a/ui/src/api/riskPolicy.ts b/ui/src/api/riskPolicy.ts new file mode 100644 index 000000000..835ff47f9 --- /dev/null +++ b/ui/src/api/riskPolicy.ts @@ -0,0 +1,148 @@ +import apiClient from './apiClient'; + +// ---- types mirroring internal/api/* and internal/policyengine --------------- + +export interface AutoApprovePolicy { + id: number; + org_id: number; + repository_id?: number; + mode: 'inherit' | 'custom' | 'always_approve' | 'never_approve'; + min_author_level: number; + eligibility_whitelist_user_ids: number[]; + risk_threshold_percentile: number; + require_llm_confidence: number; + blocking_ruleset_ids: number[]; + landing_delay_seconds: number; + daily_approve_cap?: number; + restrict_outcome_reporting_to_admins: boolean; + force_approve_min_level: number; +} + +export interface AutoApprovePolicyUpsert { + repository_id?: number; + mode?: string; + min_author_level?: number; + eligibility_whitelist_user_ids?: number[]; + risk_threshold_percentile?: number; + require_llm_confidence?: number; + blocking_ruleset_ids?: number[]; + landing_delay_seconds?: number; + daily_approve_cap?: number; + restrict_outcome_reporting_to_admins?: boolean; + force_approve_min_level?: number; +} + +export interface PolicyLogStage { + stage: string; + verdict: string; + detail: Record; +} + +export interface PolicyLogResponse { + stages: PolicyLogStage[]; + blocked: boolean; + force_approve_min_level: number; + features: number[]; + feature_names: { Name: string; Category: string }[]; + model_version: number; + score: number; +} + +export interface RiskModelInfo { + version: number; + training_samples: number; + is_seed: boolean; +} + +export interface RiskModelResponse { + model: RiskModelInfo; + overrides: Record; + scores: number[]; +} + +export interface OutcomeRow { + review_id: number; + outcome_type: string; + source: string; + reason: string; + created_at: string; +} + +export interface UserIdentity { + id: number; + org_id: number; + user_id: number; + kind: 'email' | 'username'; + value: string; +} + +export interface SimulateRow { + review_id: number; + would_approve: boolean; + blocked_at?: string; + score?: number; +} + +// ---- auto-approve policy CRUD ---------------------------------------------- + +export const listAutoApprovePolicies = (): Promise<{ rows: AutoApprovePolicy[] }> => + apiClient.get('/auto-approve-policies'); + +export const createAutoApprovePolicy = (body: AutoApprovePolicyUpsert): Promise<{ policy: AutoApprovePolicy }> => + apiClient.post('/auto-approve-policies', body); + +export const updateAutoApprovePolicy = (id: number, body: AutoApprovePolicyUpsert): Promise<{ policy: AutoApprovePolicy }> => + apiClient.put(`/auto-approve-policies/${id}`, body); + +export const deleteAutoApprovePolicy = (id: number): Promise<{ status: string }> => + apiClient.delete(`/auto-approve-policies/${id}`); + +export const getResolvedPolicy = (repositoryId?: number): Promise<{ policy: AutoApprovePolicy | null; configured: boolean }> => + apiClient.get(`/auto-approve-policies/resolved${repositoryId ? `?repository_id=${repositoryId}` : ''}`); + +export const simulatePolicy = (body: { + repository_id?: number; + policy: AutoApprovePolicyUpsert; + review_ids?: number[]; + limit?: number; +}): Promise<{ rows: SimulateRow[] }> => apiClient.post('/auto-approve-policies/simulate', body); + +// ---- review policy actions ------------------------------------------------- + +export const getReviewPolicyLog = (reviewId: number): Promise => + apiClient.get(`/reviews/${reviewId}/policy-log`); + +export const forceApproveReview = (reviewId: number, reason: string): Promise<{ status: string }> => + apiClient.post(`/reviews/${reviewId}/force-approve`, { reason }); + +export const markReviewOutcome = (reviewId: number, outcomeType: string, reason: string): Promise<{ status: string }> => + apiClient.post(`/reviews/${reviewId}/outcomes`, { outcome_type: outcomeType, reason }); + +// ---- risk model ------------------------------------------------------------ + +export const getRiskModel = (): Promise => apiClient.get('/risk-model'); + +export const putRiskModelOverrides = (overrides: Record): Promise<{ overrides: Record }> => + apiClient.put('/risk-model/overrides', { overrides }); + +export const retrainRiskModel = (): Promise<{ result: { promoted: boolean; training_count: number; holdout_auc: number; reason: string } }> => + apiClient.post('/risk-model/retrain', {}); + +// ---- outcomes -------------------------------------------------------------- + +export const listReviewOutcomes = (orgId: number): Promise<{ rows: OutcomeRow[]; total: number; manual_count: number; webhook_count: number }> => + apiClient.get(`/orgs/${orgId}/review-outcomes`); + +// ---- identities ------------------------------------------------------------ + +export const listUserIdentities = (orgId: number, userId: number): Promise<{ rows: UserIdentity[] }> => + apiClient.get(`/orgs/${orgId}/users/${userId}/identities`); + +export const addUserIdentity = (orgId: number, userId: number, kind: string, value: string): Promise => + apiClient.post(`/orgs/${orgId}/users/${userId}/identities`, { kind, value }); + +export const removeUserIdentity = (orgId: number, userId: number, identityId: number): Promise<{ status: string }> => + apiClient.delete(`/orgs/${orgId}/users/${userId}/identities/${identityId}`); + +export const listOrgIdentities = (orgId: number): Promise<{ rows: UserIdentity[] }> => + apiClient.get(`/orgs/${orgId}/identities`); diff --git a/ui/src/api/users.ts b/ui/src/api/users.ts index 11149f059..f70f13461 100644 --- a/ui/src/api/users.ts +++ b/ui/src/api/users.ts @@ -81,6 +81,7 @@ export interface Member { role_id: number; org_id: number; onboarding_api_key?: string; + review_level?: number; } export interface FetchUsersResponse { diff --git a/ui/src/components/AutoApprove/PolicyFieldsEditor.tsx b/ui/src/components/AutoApprove/PolicyFieldsEditor.tsx new file mode 100644 index 000000000..5f2b54f12 --- /dev/null +++ b/ui/src/components/AutoApprove/PolicyFieldsEditor.tsx @@ -0,0 +1,116 @@ +import React from 'react'; +import { Input, Toggle } from '../UIPrimitives'; +import { AutoApprovePolicyUpsert } from '../../api/riskPolicy'; +import { CIRuleset } from '../../pages/CiRulesets/shared'; + +// Shared field groups for the auto-approve policy editor, used both by the +// org-wide Settings tab (AutoApprovePolicyTab.tsx) and a repository's +// "Customize" override (RepoAutoApproveSection.tsx), so the two never drift +// out of sync β€” only the save target (org default vs. repository_id) differs. + +export interface PolicyFieldsProps { + draft: AutoApprovePolicyUpsert; + patch: (partial: Partial) => void; + readOnly: boolean; +} + +export const StaticRulesFields: React.FC = ({ draft, patch, readOnly, rulesets }) => ( +
+

+ Blocking rulesets always run (even under "always approve"). Manage them under{' '} + CI Rulesets. +

+ {rulesets.length === 0 &&

No rulesets defined yet.

} + {rulesets.map((r) => { + const on = (draft.blocking_ruleset_ids || []).includes(r.id); + return ( + + ); + })} +
+); + +export const EligibilityFields: React.FC = ({ draft, patch, readOnly }) => ( +
+ patch({ min_author_level: parseInt(e.target.value, 10) || 0 })} + disabled={readOnly} + helperText="Unmapped authors default to level 1; bots to level 0." + /> + patch({ force_approve_min_level: parseInt(e.target.value, 10) || 0 })} + disabled={readOnly} + helperText="Users at/above this level (or owners/admins) may force-approve a blocked review." + /> +
+); + +export const GeneralFields: React.FC = ({ draft, patch, readOnly }) => ( +
+
+ +
+ patch({ risk_threshold_percentile: parseInt(e.target.value, 10) })} + disabled={readOnly} + /> + P{draft.risk_threshold_percentile ?? 20} +
+
+ patch({ require_llm_confidence: parseInt(e.target.value, 10) || 0 })} + disabled={readOnly} + /> + patch({ landing_delay_seconds: parseInt(e.target.value, 10) || 0 })} + disabled={readOnly} + helperText="Human-rejection window before the approval is posted." + /> + { + const v = e.target.value === '' ? undefined : parseInt(e.target.value, 10); + patch({ daily_approve_cap: v }); + }} + disabled={readOnly} + /> + +
+); diff --git a/ui/src/components/Explore/RepoAutoApproveSection.tsx b/ui/src/components/Explore/RepoAutoApproveSection.tsx new file mode 100644 index 000000000..a4266dc99 --- /dev/null +++ b/ui/src/components/Explore/RepoAutoApproveSection.tsx @@ -0,0 +1,164 @@ +import React, { useEffect, useState } from 'react'; +import { Alert, Button, Select, Spinner, Tabs } from '../UIPrimitives'; +import { useOrgContext } from '../../hooks/useOrgContext'; +import { + AutoApprovePolicy, + AutoApprovePolicyUpsert, + listAutoApprovePolicies, + createAutoApprovePolicy, + updateAutoApprovePolicy, +} from '../../api/riskPolicy'; +import { CIRuleset } from '../../pages/CiRulesets/shared'; +import apiClient from '../../api/apiClient'; +import { StaticRulesFields, EligibilityFields, GeneralFields } from '../AutoApprove/PolicyFieldsEditor'; + +// Draft fields a repo can customize β€” everything on auto_approve_policies +// except the trained risk model itself (weights/category overrides stay +// org-wide only, see docs/risk-policy-engine.md Β§3/Β§9). +const draftFromPolicy = (p: AutoApprovePolicy | null): AutoApprovePolicyUpsert => ({ + min_author_level: p?.min_author_level ?? 1, + eligibility_whitelist_user_ids: p?.eligibility_whitelist_user_ids ?? [], + risk_threshold_percentile: p?.risk_threshold_percentile ?? 20, + require_llm_confidence: p?.require_llm_confidence ?? 8, + blocking_ruleset_ids: p?.blocking_ruleset_ids ?? [], + landing_delay_seconds: p?.landing_delay_seconds ?? 0, + daily_approve_cap: p?.daily_approve_cap, + restrict_outcome_reporting_to_admins: p?.restrict_outcome_reporting_to_admins ?? false, + force_approve_min_level: p?.force_approve_min_level ?? 3, +}); + +const RepoAutoApproveSection: React.FC<{ repositoryId: number }> = ({ repositoryId }) => { + const { currentOrg, canManageCurrentOrg } = useOrgContext(); + const readOnly = !canManageCurrentOrg && currentOrg?.role !== 'owner'; + + const [orgDefault, setOrgDefault] = useState(null); + const [repoPolicy, setRepoPolicy] = useState(null); + const [rulesets, setRulesets] = useState([]); + const [loading, setLoading] = useState(true); + const [saving, setSaving] = useState(false); + const [saved, setSaved] = useState(false); + const [error, setError] = useState(''); + + const [mode, setMode] = useState<'inherit' | 'custom' | 'always_approve' | 'never_approve'>('inherit'); + const [draft, setDraft] = useState(draftFromPolicy(null)); + const [subtab, setSubtab] = useState('static-rules'); + + useEffect(() => { + Promise.all([listAutoApprovePolicies(), apiClient.get<{ rows: CIRuleset[] }>('/ci-rulesets')]) + .then(([polRes, rsRes]) => { + const org = polRes.rows.find((p) => p.repository_id == null) || null; + const repo = polRes.rows.find((p) => p.repository_id === repositoryId) || null; + setOrgDefault(org); + setRepoPolicy(repo); + setRulesets(rsRes.rows || []); + setMode(repo?.mode ?? 'inherit'); + // A repo's "Customize" starts from its own saved values if it has any, + // else from the org default β€” never from bare schema defaults β€” so + // picking Customize can't silently loosen an org-wide setting. + setDraft(draftFromPolicy(repo ?? org)); + }) + .catch((e) => setError(String(e?.message || e))) + .finally(() => setLoading(false)); + }, [repositoryId]); + + const patch = (partial: Partial) => setDraft((d) => ({ ...d, ...partial })); + + const onModeChange = (next: typeof mode) => { + // Switching into custom for the first time: seed from the org default so + // the repo starts aligned with it rather than at hardcoded defaults. + if (next === 'custom' && !repoPolicy) { + setDraft(draftFromPolicy(orgDefault)); + } + setMode(next); + }; + + const save = async () => { + setSaving(true); + setError(''); + try { + const body: AutoApprovePolicyUpsert = { mode, ...(mode === 'custom' ? draft : { risk_threshold_percentile: draft.risk_threshold_percentile }) }; + if (repoPolicy) { + const res = await updateAutoApprovePolicy(repoPolicy.id, body); + setRepoPolicy(res.policy); + } else { + const res = await createAutoApprovePolicy({ repository_id: repositoryId, ...body }); + setRepoPolicy(res.policy); + } + setSaved(true); + setTimeout(() => setSaved(false), 2500); + } catch (e: any) { + setError(String(e?.message || e)); + } finally { + setSaving(false); + } + }; + + if (loading) { + return
; + } + + const effective = repoPolicy && repoPolicy.mode !== 'inherit' ? repoPolicy : orgDefault; + + return ( +
+
+

Auto Approve

+

Override this repository's risk-based approval policy.

+
+ + {error && {error}} + {readOnly && You need the owner role to edit this repository's policy.} + +
+ Effective: {effective ? effective.mode : 'off'} + {!effective && ' (no organization policy configured β€” visit Settings β†’ Auto Approve)'} +
+

+ The trained risk model (weights, category overrides, retraining) always stays the one org-wide classifier β€” + only the threshold compared against its score, and everything below, can differ per repository. +

+ + + )} + {(!isEditMode && !existsGlobally) || (isEditMode && currentUserRole === 'super_admin') ? ( @@ -1421,6 +1440,10 @@ const UserForm: React.FC = () => {
)} + + {isEditMode && user && currentOrgId && ( + + )} diff --git a/ui/src/components/UserManagement/UserIdentitiesEditor.tsx b/ui/src/components/UserManagement/UserIdentitiesEditor.tsx new file mode 100644 index 000000000..a1a3b31c0 --- /dev/null +++ b/ui/src/components/UserManagement/UserIdentitiesEditor.tsx @@ -0,0 +1,100 @@ +import React, { useEffect, useState } from 'react'; +import { Button, Card, Input, Select, Spinner } from '../UIPrimitives'; +import { notify } from '../../utils/notify'; +import { + listUserIdentities, + addUserIdentity, + removeUserIdentity, + UserIdentity, +} from '../../api/riskPolicy'; + +const UserIdentitiesEditor: React.FC<{ orgId: number; userId: number }> = ({ orgId, userId }) => { + const [identities, setIdentities] = useState([]); + const [loading, setLoading] = useState(true); + const [kind, setKind] = useState<'email' | 'username'>('username'); + const [value, setValue] = useState(''); + const [busy, setBusy] = useState(false); + + const load = () => { + listUserIdentities(orgId, userId) + .then((res) => setIdentities(res.rows || [])) + .catch((e) => notify.error(String(e?.message || e))) + .finally(() => setLoading(false)); + }; + + useEffect(load, [orgId, userId]); + + const add = async () => { + if (!value.trim()) return; + setBusy(true); + try { + await addUserIdentity(orgId, userId, kind, value.trim()); + setValue(''); + await listUserIdentities(orgId, userId).then((res) => setIdentities(res.rows || [])); + notify.success('Identity added.'); + } catch (e: any) { + notify.error(String(e?.message || e)); + } finally { + setBusy(false); + } + }; + + const remove = async (id: number) => { + setBusy(true); + try { + await removeUserIdentity(orgId, userId, id); + setIdentities((prev) => prev.filter((i) => i.id !== id)); + notify.success('Identity removed.'); + } catch (e: any) { + notify.error(String(e?.message || e)); + } finally { + setBusy(false); + } + }; + + return ( + +

Alternative identities

+

+ Match this user's Git provider handles/emails to their account for auto-approve eligibility. +

+ + {loading ? ( + + ) : ( +
+ {identities.length === 0 &&

No identities yet.

} + {identities.map((i) => ( +
+ + {i.kind}: {i.value} + + +
+ ))} +
+ )} + +
+ setValue(e.target.value)} placeholder="e.g. octocat or user@example.com" /> + +
+ + ); +}; + +export default UserIdentitiesEditor; diff --git a/ui/src/components/reviews/AutoApprovePanel.tsx b/ui/src/components/reviews/AutoApprovePanel.tsx new file mode 100644 index 000000000..1ebe26966 --- /dev/null +++ b/ui/src/components/reviews/AutoApprovePanel.tsx @@ -0,0 +1,148 @@ +import React, { useEffect, useState } from 'react'; +import { Alert, Button, Card, Spinner } from '../UIPrimitives'; +import { getReviewPolicyLog, forceApproveReview, markReviewOutcome, PolicyLogResponse } from '../../api/riskPolicy'; + +const STAGE_LABEL: Record = { + eligibility: 'Eligibility', + static: 'Static rules', + risk: 'Risk score', + llm: 'LLM classification', + final: 'Final decision', +}; + +const verdictIcon = (v: string) => (v === 'pass' ? 'βœ…' : v === 'fail' ? '❌' : '⏭️'); + +const AutoApprovePanel: React.FC<{ reviewId: number }> = ({ reviewId }) => { + const [log, setLog] = useState(null); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(''); + + const [forceReason, setForceReason] = useState(''); + const [forceBusy, setForceBusy] = useState(false); + const [forceDone, setForceDone] = useState(false); + + const [outcomeType, setOutcomeType] = useState<'revert' | 'incident' | 'confirmed_safe'>('revert'); + const [outcomeReason, setOutcomeReason] = useState(''); + const [outcomeBusy, setOutcomeBusy] = useState(false); + const [outcomeDone, setOutcomeDone] = useState(false); + + const load = () => { + setLoading(true); + setError(''); + getReviewPolicyLog(reviewId) + .then(setLog) + .catch((e) => setError(String(e?.message || e))) + .finally(() => setLoading(false)); + }; + + useEffect(load, [reviewId]); + + if (loading) { + return ( +
+ +
+ ); + } + + if (error) { + return {error}; + } + + if (!log || !log.stages.length) { + return ( +
+

No policy evaluation for this review.

+
+ ); + } + + const forceApprove = async () => { + setForceBusy(true); + setError(''); + try { + await forceApproveReview(reviewId, forceReason || 'Manual override'); + setForceDone(true); + } catch (e: any) { + setError(String(e?.message || e)); + } finally { + setForceBusy(false); + } + }; + + const mark = async () => { + setOutcomeBusy(true); + setError(''); + try { + await markReviewOutcome(reviewId, outcomeType, outcomeReason); + setOutcomeDone(true); + } catch (e: any) { + setError(String(e?.message || e)); + } finally { + setOutcomeBusy(false); + } + }; + + return ( +
+ {error && {error}} + + +
+ {log.stages.map((st) => ( +
+ {verdictIcon(st.verdict)} + {STAGE_LABEL[st.stage] || st.stage} + + {String((st.detail as any)?.reason ?? '')} + +
+ ))} +
+ + + {log.blocked && ( + +

Force approve

+

+ Requires review level β‰₯ {log.force_approve_min_level} or owner/admin. Recorded in the audit log. +

+