Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
-- migrate:up

-- Deleting a connector cascades to its repositories; repo override policies blocked that.
-- A repo's override policy is meaningless without the repo, so delete it too.
ALTER TABLE auto_approve_policies DROP CONSTRAINT auto_approve_policies_repository_id_fkey;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The existing foreign key constraint has been removed.

Suggestions:

  1. Consider adding a comment explaining why the existing constraint is being dropped.

ALTER TABLE auto_approve_policies ADD CONSTRAINT auto_approve_policies_repository_id_fkey

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

A new foreign key constraint has been added with ON DELETE CASCADE.

Suggestions:

  1. Ensure repositories.id is indexed for efficient cascade operations.

FOREIGN KEY (repository_id) REFERENCES repositories(id) ON DELETE CASCADE;

-- migrate:down

ALTER TABLE auto_approve_policies DROP CONSTRAINT auto_approve_policies_repository_id_fkey;
ALTER TABLE auto_approve_policies ADD CONSTRAINT auto_approve_policies_repository_id_fkey
FOREIGN KEY (repository_id) REFERENCES repositories(id);
26 changes: 26 additions & 0 deletions db/migrations/20261017100000_default_auto_approve_policy.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
-- migrate:up

-- Auto-approve is on by default: every org gets an org-level policy with the column defaults.
INSERT INTO auto_approve_policies (org_id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The default organization-level policy has been inserted.

Suggestions:

  1. Consider adding a unique constraint on (org_id, repository_id) to prevent duplicates.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Insert the default organization policy.

Suggestions:

  1. Ensure org_id is unique for repository_id IS NULL entries to prevent duplicates.

SELECT o.id FROM orgs o
WHERE NOT EXISTS (
SELECT 1 FROM auto_approve_policies p WHERE p.org_id = o.id AND p.repository_id IS NULL
);

CREATE FUNCTION create_default_auto_approve_policy() RETURNS trigger

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

A function for the default policy has been created.

Suggestions:

  1. Consider naming the function more descriptively, e.g., ensure_default_auto_approve_policy.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Create a trigger function for new organizations.

Suggestions:

  1. Consider using INSERT ... ON CONFLICT DO NOTHING if org_id uniqueness is not guaranteed.

LANGUAGE plpgsql
AS $$
BEGIN
INSERT INTO auto_approve_policies (org_id) VALUES (NEW.id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The default policy for new organizations has been inserted.

Suggestions:

  1. Ensure orgs.id is indexed for efficient trigger operation.

RETURN NEW;
END;
$$;

CREATE TRIGGER orgs_default_auto_approve_policy
AFTER INSERT ON orgs
FOR EACH ROW EXECUTE FUNCTION create_default_auto_approve_policy();

-- migrate:down

DROP TRIGGER IF EXISTS orgs_default_auto_approve_policy ON orgs;
DROP FUNCTION IF EXISTS create_default_auto_approve_policy();
27 changes: 25 additions & 2 deletions db/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,20 @@ END
$$;


--
-- Name: create_default_auto_approve_policy(); Type: FUNCTION; Schema: public; Owner: -
--

CREATE FUNCTION public.create_default_auto_approve_policy() RETURNS trigger

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

This is the function definition for the default policy.

Suggestions:

  1. Consider adding a comment explaining the purpose of this function.

LANGUAGE plpgsql
AS $$
BEGIN
INSERT INTO auto_approve_policies (org_id) VALUES (NEW.id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Insert the default organization policy.

Suggestions:

  1. Ensure org_id is unique for repository_id IS NULL entries to prevent duplicates.

RETURN NEW;
END;
$$;


--
-- Name: license_seat_assignments_set_updated_at(); Type: FUNCTION; Schema: public; Owner: -
--
Expand Down Expand Up @@ -6120,6 +6134,13 @@ CREATE TRIGGER trg_org_billing_state_audit AFTER UPDATE ON public.org_billing_st
CREATE TRIGGER trg_org_billing_state_updated_at BEFORE UPDATE ON public.org_billing_state FOR EACH ROW EXECUTE FUNCTION public.org_billing_state_set_updated_at();


--
-- Name: orgs orgs_default_auto_approve_policy; Type: TRIGGER; Schema: public; Owner: -
--

CREATE TRIGGER orgs_default_auto_approve_policy AFTER INSERT ON public.orgs FOR EACH ROW EXECUTE FUNCTION public.create_default_auto_approve_policy();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

A trigger has been added to create the default policy for new organizations.

Suggestions:

  1. Consider adding a comment explaining the purpose of this trigger.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Add a trigger for new organizations.

Suggestions:

  1. Consider using INSERT ... ON CONFLICT DO NOTHING if org_id uniqueness is not guaranteed.



--
-- Name: plan_catalog trg_plan_catalog_updated_at; Type: TRIGGER; Schema: public; Owner: -
--
Expand Down Expand Up @@ -6216,7 +6237,7 @@ ALTER TABLE ONLY public.auto_approve_policies
--

ALTER TABLE ONLY public.auto_approve_policies
ADD CONSTRAINT auto_approve_policies_repository_id_fkey FOREIGN KEY (repository_id) REFERENCES public.repositories(id);
ADD CONSTRAINT auto_approve_policies_repository_id_fkey FOREIGN KEY (repository_id) REFERENCES public.repositories(id) ON DELETE CASCADE;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

This is a foreign key constraint with ON DELETE CASCADE.

Suggestions:

  1. Verify that repositories.id is a primary key or has a unique index.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Add the ON DELETE CASCADE constraint.

Suggestions:

  1. This change ensures data integrity by automatically removing orphaned policies.



--
Expand Down Expand Up @@ -7258,4 +7279,6 @@ INSERT INTO public.schema_migrations (version) VALUES
('20260905090000'),
('20261012090000'),
('20261012100000'),
('20261013090000');
('20261013090000'),
('20261017090000'),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

New migration versions have been added.

Suggestions:

  1. Ensure migration versions are sequential and unique.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Add new migration versions.

Suggestions:

  1. Ensure migration versions are sequential and correctly ordered.

('20261017100000');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Add new migration versions.

Suggestions:

  1. Ensure migration versions are sequential and correctly ordered.

6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ require (
github.com/aws/aws-sdk-go-v2/service/s3 v1.102.2
github.com/bwmarrin/discordgo v0.29.0
github.com/d1vbyz3r0/typed v0.2.3
github.com/getkin/kin-openapi v0.145.0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Update the kin-openapi version.

Suggestions:

  1. Review changelog for kin-openapi v0.151.0 for any breaking changes.

github.com/getkin/kin-openapi v0.151.0
github.com/go-co-op/gocron/v2 v2.22.0
github.com/go-swiss/fonts v0.0.0-20230807175105-90067c2f5042
github.com/golang-jwt/jwt/v5 v5.3.1
Expand Down Expand Up @@ -69,7 +69,7 @@ require (
google.golang.org/api v0.272.0
)

require github.com/cdipaolo/goml v0.0.0-20220715001353-00e0c845ae1c // indirect

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Ensure that the goml dependency is required.

Suggestions:

  1. This change ensures github.com/cdipaolo/goml is explicitly managed.

require github.com/cdipaolo/goml v0.0.0-20220715001353-00e0c845ae1c

require (
cel.dev/expr v0.25.2 // indirect
Expand Down Expand Up @@ -212,7 +212,7 @@ require (
github.com/sagikazarmark/locafero v0.7.0 // indirect
github.com/sagikazarmark/slog-shim v0.1.0 // indirect
github.com/sahilm/fuzzy v0.1.3 // indirect
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Update the jsonschema version.

Suggestions:

  1. Review changelog for jsonschema v6.0.3 for any breaking changes.

github.com/santhosh-tekuri/jsonschema/v6 v6.0.3 // indirect
github.com/shopspring/decimal v1.4.0 // indirect
github.com/sirupsen/logrus v1.9.4 // indirect
github.com/sorairolake/lzip-go v0.3.8 // indirect
Expand Down
12 changes: 4 additions & 8 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,6 @@ github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapp
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.55.0/go.mod h1:Mf6O40IAyB9zR/1J8nGDDPirZQQPbYJni8Yisy7NTMc=
github.com/HexmosTech/deidentify v1.0.0 h1:yC+CMoFdkyGNgSU4gEyB/AAJbJ3KLjqNuNe3Km/LCDA=
github.com/HexmosTech/deidentify v1.0.0/go.mod h1:1+fAOPPy8g/7KoF+OOY673j+3BxAs8xTzjlkNKgMWjU=
github.com/HexmosTech/git-lrc/blastradius v0.0.0-20261009104557-0cc6cb4e1a6c h1:ARY/iCSKMhvEWX3nG5OJVKQEsKc97l/rAsHUICmbozQ=

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Remove the old blastradius checksums.

Suggestions:

  1. This cleans up stale checksums after dependency updates.

github.com/HexmosTech/git-lrc/blastradius v0.0.0-20261009104557-0cc6cb4e1a6c/go.mod h1:uAX2wgGb22UPGzmXVs6b8rKRzZnEkfFlnhxiv/ewp9M=

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Remove the old blastradius checksums.

Suggestions:

  1. This cleans up stale checksums after dependency updates.

github.com/HexmosTech/git-lrc/blastradius v0.0.0-20261009151730-cb9fce8104d0 h1:4RTFP63IZ6cJJ+ibkAQ9m9sQbCa4Kxu0lkuiKw2++bQ=

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Remove the old blastradius checksums.

Suggestions:

  1. This cleans up stale checksums after dependency updates.

github.com/HexmosTech/git-lrc/blastradius v0.0.0-20261009151730-cb9fce8104d0/go.mod h1:uAX2wgGb22UPGzmXVs6b8rKRzZnEkfFlnhxiv/ewp9M=

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Remove the old blastradius checksums.

Suggestions:

  1. This cleans up stale checksums after dependency updates.

github.com/HexmosTech/git-lrc/blastradius v0.1.0 h1:fiykl9PI7plYe6cnKYT6uojVlzr8/EJb3sZ5Om+W4L8=
github.com/HexmosTech/git-lrc/blastradius v0.1.0/go.mod h1:uAX2wgGb22UPGzmXVs6b8rKRzZnEkfFlnhxiv/ewp9M=
github.com/KyleBanks/depth v1.2.1 h1:5h8fQADFrWtarTdtDudMmGsC7GPbOAu6RVB3ffsVFHc=
Expand Down Expand Up @@ -227,8 +223,8 @@ github.com/frankban/quicktest v1.14.6 h1:7Xjx+VpznH+oBnejlPUj8oUpdxnVs4f8XU8WnHk
github.com/frankban/quicktest v1.14.6/go.mod h1:4ptaffx2x8+WTWXmUCuVU6aPUX1/Mz7zb5vbUoiM6w0=
github.com/fsnotify/fsnotify v1.10.1 h1:b0/UzAf9yR5rhf3RPm9gf3ehBPpf0oZKIjtpKrx59Ho=
github.com/fsnotify/fsnotify v1.10.1/go.mod h1:TLheqan6HD6GBK6PrDWyDPBaEV8LspOxvPSjC+bVfgo=
github.com/getkin/kin-openapi v0.145.0 h1:htBX+Q7SevVaCUqymFegUKzH2WCbewl9tsmyn2FMGWY=

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Update the kin-openapi checksums.

Suggestions:

  1. Checksums updated to match new version v0.151.0.

github.com/getkin/kin-openapi v0.145.0/go.mod h1:3BH9M9XDe/y9M5DSvEocVYAYq1w0qrhJHjC/vZi0AaY=

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Update the kin-openapi checksums.

Suggestions:

  1. Checksums updated to match new version v0.151.0.

github.com/getkin/kin-openapi v0.151.0 h1:p8NIstL53h620DnlO9R4pR4QRkEzJ3jbw9flb4kc1hg=
github.com/getkin/kin-openapi v0.151.0/go.mod h1:1+BHDzstro+P5CKtPy1X4PfofnFgmRe6uvMy9+r9fKY=
github.com/gitleaks/go-gitdiff v0.9.1 h1:ni6z6/3i9ODT685OLCTf+s/ERlWUNWQF4x1pvoNICw0=
github.com/gitleaks/go-gitdiff v0.9.1/go.mod h1:pKz0X4YzCKZs30BL+weqBIG7mx0jl4tF1uXV9ZyNvrA=
github.com/go-co-op/gocron/v2 v2.22.0 h1:uEuH2F7k7VoESb1BYSaffuuV+T0kkpzsC0aXk7/z79I=
Expand Down Expand Up @@ -510,8 +506,8 @@ github.com/sagikazarmark/slog-shim v0.1.0 h1:diDBnUNK9N/354PgrxMywXnAwEr1QZcOr6g
github.com/sagikazarmark/slog-shim v0.1.0/go.mod h1:SrcSrq8aKtyuqEI1uvTDTK1arOWRIczQRv+GVI1AkeQ=
github.com/sahilm/fuzzy v0.1.3 h1:juByESSS32nVD81vr6tHmKmA/8zde7gE+x5CLxrzXPU=
github.com/sahilm/fuzzy v0.1.3/go.mod h1:au6//VbVSqu6DFrkL2CfjlJ5iURpNCPeE+1GwY3XsT8=
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ=

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Update the jsonschema checksums.

Suggestions:

  1. Checksums updated to match new version v6.0.3.

github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU=

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Update the jsonschema checksums.

Suggestions:

  1. Checksums updated to match new version v6.0.3.

github.com/santhosh-tekuri/jsonschema/v6 v6.0.3 h1:1EYB5IzjZawrrnELUi78f9fPu57HuXjmddZPjrls/28=
github.com/santhosh-tekuri/jsonschema/v6 v6.0.3/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU=
github.com/shopspring/decimal v1.4.0 h1:bxl37RwXBklmTi0C79JfXCEBD1cqqHt0bbgBAGFp81k=
github.com/shopspring/decimal v1.4.0/go.mod h1:gawqmDU56v4yIKSwfBSFip1HdCCXN8/+DMd9qYNcwME=
github.com/shrsv/AgentLaws v0.2.1 h1:00pN3dEhsFFtz+qrcqBwVy8ieLzaivHbpyyuAk3+Lsk=
Expand Down
22 changes: 13 additions & 9 deletions internal/api/pull_request_review_trigger.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,15 +98,7 @@ func (s *Server) createReview(c echo.Context) error {
return c.JSON(http.StatusInternalServerError, ErrorResponse{Error: err.Error()})
}
s.enrichMetadata(ctx)
if req.FromPRLink {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The inline SQL has been removed and replaced by a helper function.

Suggestions:

  1. Add a comment to markForPolicyCheck explaining the pull_request_id IS NOT NULL condition.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Extract the policy check marking logic into a new method.

Suggestions:

  1. This improves readability and reusability of the logic.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Add a check to ensure pull_request_id is not null.

Suggestions:

  1. Ensures policy check is only marked for reviews actually linked to a PR.

// 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.markForPolicyCheck(ctx.review.ID, orgID)
s.trackActivity(ctx)
if err := s.launchBackgroundProcessing(ctx); err != nil {
log.Printf("[ERROR] createReview: Failed to queue manual review: %v", err)
Expand Down Expand Up @@ -152,3 +144,15 @@ func (s *Server) getPullRequestOrg(c echo.Context) error {
}
return JSONWithEnvelope(c, http.StatusOK, map[string]interface{}{"org_id": orgID})
}

// markForPolicyCheck flags a manual PR review so the worker runs the approval policy check once it completes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Document the new helper method.

Suggestions:

  1. Clear documentation helps future developers understand the function's purpose.

// Reviews not linked to a known PR are left alone.
func (s *Server) markForPolicyCheck(reviewID, orgID int64) {
if _, err := s.db.Exec(
`UPDATE reviews SET metadata = COALESCE(metadata, '{}'::jsonb) || '{"pr_link_policy": true}'::jsonb
WHERE id = $1 AND org_id = $2 AND pull_request_id IS NOT NULL`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

A condition has been added to only update reviews that are linked to a pull request.

Suggestions:

  1. Consider logging a warning if pull_request_id is NULL for a review that is being processed.

reviewID, orgID,
); err != nil {
log.Printf("[WARN] failed to mark review %d for policy check: %v", reviewID, err)
}
}
1 change: 1 addition & 0 deletions internal/api/review_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func (s *Server) TriggerReviewV2(c echo.Context) error {

// Phase 4: Enrich metadata (fetch MR details, update DB)
s.enrichMetadata(ctx)
s.markForPolicyCheck(ctx.review.ID, ctx.orgID)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

A call has been added to mark the review for a policy check.

Suggestions:

  1. Ensure ctx.orgID is always populated before this call.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Call the markForPolicyCheck method.

Suggestions:

  1. Ensures PR-linked reviews are correctly flagged for policy checks.


// Phase 5: Track activity (log the trigger event)
s.trackActivity(ctx)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ Any authenticated org member.

- Search/filter/sort PRs by provider, repo, state, etc.
- Trigger a review directly for a specific pull request
(`triggerReviewForPullRequest`).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The description has been updated to include information about the policy check.

Suggestions:

  1. Consider adding a link to the 'Auto Approve Policies' settings page for more details.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Clarify that the policy check runs upon completion.

Suggestions:

  1. This update improves user understanding of the review process.

(`triggerReviewForPullRequest`). It opens the new review's page right away,
and the review runs the auto-approve policy check when it finishes.

## Related pages

Expand Down
22 changes: 16 additions & 6 deletions internal/docindex/docs/routes_guide/reviews/new-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@

## Purpose

Form to manually trigger a code review (as opposed to it firing from a git

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Update the purpose description.

Suggestions:

  1. More accurately reflects the current functionality of the page.

webhook or the `lrc` CLI hook). Lets the user pick a connected AI provider

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The description for manual reviews has been updated.

Suggestions:

  1. Clarify what 'synced' means in the context of PR LiveReview.

and target repository/PR, then submits the review job.
Start a code review of one pull/merge request right away (as opposed to it
firing from a git webhook or the `lrc` CLI hook). Below the form, an "Other
ways to review" section lists the other ways reviews can start.

## Who can access it

Expand All @@ -18,15 +18,25 @@ self-hosted license limits.

## Key actions

- Select an AI connector and repository/PR to review.
- Submit the review (`triggerReview` API call); redirects to the new
review's detail page on success.
- **Back** (top left) returns to the previous page.
- Type a repository name, PR title or author to see matching open PRs. Click
one, or use ↑/↓ and Enter, to start its review.
- Or paste a PR/MR URL and click **Start review**. Either way the new review's
page opens right away, and for a PR LiveReview already synced the org's

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The policy check process for pull request reviews has been clarified.

Suggestions:

  1. Ensure the link to the 'Explore → Merge Requests' page is accurate.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Provide details on the policy check for pull request reviews.

Suggestions:

  1. Clarifies the behavior for PR reviews initiated via this form.

auto-approve policy runs when the review finishes.
- Hover a provider chip (GitHub, GitLab, Bitbucket, Gitea, Azure DevOps) to see
its URL format.
- **Other ways to review** cards: On the PR (assign the bot as reviewer,
@mention it, or use the PR description's Start a review link), Pre-commit CLI,
IDE / MCP, CI/CD & API (CI rulesets), Chatbot (opens Livi with a review prompt),
and Scheduled reviews.
- If blocked by quota, see an upgrade prompt (`UpgradePromptModal`) instead.

## Related pages

- [Reviews list](reviews-list.md)
- [Review Detail](review-detail.md)
- [Explore → Merge Requests](../explore/merge-requests.md)
- [Create Review via CLI](create-review-cli.md)
- [Create Review via MCP](create-review-mcp.md)
- [AI Providers](../ai/ai-providers.md)
14 changes: 11 additions & 3 deletions internal/docindex/docs/routes_guide/reviews/pr-review-link.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,19 @@
## Purpose

When a new pull/merge request is opened on GitHub, GitLab, Bitbucket, Gitea, or Azure DevOps in a repo with the
LiveReview webhook installed, LiveReview adds a section with two links to the end of its description:
LiveReview webhook installed, LiveReview adds a section to the end of its description:

- **Start a review** → this page. It starts a LiveReview review of that PR and
opens the review's detail page.
- **Review history** → the [Reviews list](reviews-list.md) filtered to that
PR (`/reviews?pull_request_id=<id>`).
- A line under the auto-approval decisions (or under the two links before any

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Information regarding policy customization links has been added.

Suggestions:

  1. Ensure the links to repository and organization settings are correct and accessible.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Add links for policy customization.

Suggestions:

  1. Provides users with direct access to relevant policy settings.

decision): "Customize risk-based approval policy for **this repository** or
**your organization**". The repository link opens the repo's settings page,
Auto Approve section (`/explore/repositories/<repo id>/settings#auto-approve`);
the organization link opens [Settings → Auto Approve Policies](../settings/auto-approve.md)
(`/settings#auto-approve`). Both pages are for org owners and admins, and
open in the user's current org.

`:id` is LiveReview's internal pull request id, not the GitHub/GitLab PR number.
The section is added once per PR, only when it's opened (not for PRs that
Expand Down Expand Up @@ -49,8 +56,9 @@ aren't members of that org see "pull request wasn't found".
- Azure DevOps: a comment thread, then the bot votes Approved or
"Waiting for author".
- The decisions table shows the last 10 policy-checked reviews.
- Reviews started any other way (reviewer assignment, @mentions, Explore) don't
run the policy check.
- Reviews started by hand for a known PR also run the policy check and get the

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

It has been clarified which review triggers the policy check.

Suggestions:

  1. Consider adding a note about how to explicitly trigger a policy check for other review types if needed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Clarify which pull request reviews trigger the policy check.

Suggestions:

  1. Distinguishes between reviews that trigger auto-approve and those that don't.

Auto Approve tab: pasting the PR URL on New Review, or Start review in the
Explore PR table. Reviewer assignment and @mention reviews don't.

## Related pages

Expand Down
14 changes: 8 additions & 6 deletions internal/jobqueue/repo_sync_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ const (
legacySectionEnd = "<!-- livereview:end -->"
)

// postLinksComment adds the "Start a review / Review history" links to the end of a new PR/MR's description.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The comment has been updated to reflect the new links.

Suggestions:

  1. Ensure the comment accurately describes all links added by the function.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Update the description for the comment function.

Suggestions:

  1. More accurately reflects the added policy customization links.

// postLinksComment adds the LiveReview links (review, history, repo/org policy) to the end of a new PR/MR's description.
// The links_comment_posted_at claim stops duplicate webhook deliveries from adding it twice.
func (w *PRStateSyncWorker) postLinksComment(ctx context.Context, repositoryID, pullRequestID int64, number int) error {
host, err := w.webhookStore.GetWebhookPublicEndpoint(ctx)
Expand All @@ -299,7 +299,7 @@ func (w *PRStateSyncWorker) postLinksComment(ctx context.Context, repositoryID,

info, err := loadRepositoryConnectorInfo(w.db, repositoryID)
if err == nil {
err = writePRSection(ctx, info, number, buildPRLinksSection(host, pullRequestID, ""))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The repositoryID has been passed to the buildPRLinksSection function.

Suggestions:

  1. Consider adding a unique constraint on (repository_id, pull_request_id) in the reviews table if not already present.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Pass the repositoryID to the buildPRLinksSection function.

Suggestions:

  1. Ensures the function receives all necessary parameters for its updated functionality.

err = writePRSection(ctx, info, number, buildPRLinksSection(host, pullRequestID, repositoryID, ""))
}
if err != nil {
// Release the claim so River's retry can add it.
Expand All @@ -312,7 +312,7 @@ func (w *PRStateSyncWorker) postLinksComment(ctx context.Context, repositoryID,
}

// buildPRLinksSection renders the LiveReview section; history is the review history table, or "".
func buildPRLinksSection(host string, pullRequestID int64, history string) string {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The repositoryID parameter has been added.

Suggestions:

  1. Ensure the repositoryID parameter is always valid and non-zero when passed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Add the repositoryID parameter.

Suggestions:

  1. Function signature updated to accommodate new link generation.

func buildPRLinksSection(host string, pullRequestID, repositoryID int64, history string) string {
host = strings.TrimSuffix(host, "/")
return fmt.Sprintf(`%s

Expand All @@ -322,8 +322,10 @@ func buildPRLinksSection(host string, pullRequestID int64, history string) strin
- ▶️ **[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

%s%s

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Links for policy customization have been added.

Suggestions:

  1. Ensure the URLs for repository and organization settings are correctly formatted and resolve to the intended pages.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Add repository and organization policy links.

Suggestions:

  1. This adds valuable links for users to customize their approval policies.

`, prSectionStart, host, pullRequestID, host, pullRequestID, history, prSectionEnd)
%s⚙️ Customize risk-based approval policy for [this repository](%s/#/explore/repositories/%d/settings#auto-approve) or [your organization](%s/#/settings#auto-approve).

%s
`, prSectionStart, host, pullRequestID, host, pullRequestID, history, host, repositoryID, host, prSectionEnd)
}

// refreshPRSection rewrites the LiveReview section with one history row per policy-checked review of this PR.
Expand Down Expand Up @@ -369,7 +371,7 @@ func refreshPRSection(ctx context.Context, db *sql.DB, info *repositoryConnector
return err
}
b.WriteString("\n")
return writePRSection(ctx, info, number, buildPRLinksSection(host, pullRequestID, b.String()))
return writePRSection(ctx, info, number, buildPRLinksSection(host, pullRequestID, info.RepositoryID, b.String()))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The repositoryID has been passed to the buildPRLinksSection function.

Suggestions:

  1. Verify that info.RepositoryID is correctly populated before this call.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Pass the repositoryID to the buildPRLinksSection function.

Suggestions:

  1. Ensures the function receives all necessary parameters for its updated functionality.

}

// decisionReason is the readable "why" for one decision row: each failing stage's reason, or that all passed.
Expand Down
14 changes: 14 additions & 0 deletions internal/policyengine/policy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,23 @@ func seedOrg(t *testing.T, db *sql.DB, name string) int64 {
// 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 auto_approve_policies WHERE org_id = $1`, orgID)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The default policy for the organization has been deleted.

Suggestions:

  1. Consider using TRUNCATE TABLE auto_approve_policies if it's safe for all tests.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Delete the default organization policy row.

Suggestions:

  1. Consider if this explicit deletion is necessary if the trigger logic guarantees a default row.

_, _ = 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 repoID == nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The default organization-level policy has been updated.

Suggestions:

  1. Ensure the repository_id IS NULL condition correctly identifies the default policy.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Update the default organization policy if repoID is null.

Suggestions:

  1. This logic duplicates the INSERT logic but with an UPDATE. Consider a single upsert pattern if possible.

// Every new org already gets a default org-level row from a trigger; reuse it.
if err := db.QueryRow(`
UPDATE auto_approve_policies SET mode = $2, min_author_level = $3
WHERE org_id = $1 AND repository_id IS NULL RETURNING id`, orgID, mode, minLevel).Scan(&id); err != nil {
t.Fatalf("update default policy: %v", err)
}
return id
}
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 {
Expand All @@ -62,6 +72,10 @@ func insertPolicy(t *testing.T, db *sql.DB, orgID int64, repoID *int64, mode str
func TestResolvePolicyNoneConfigured(t *testing.T) {
db := testDB(t)
orgID := seedOrg(t, db, "policy-resolve-none")
// Drop the default row the org-insert trigger created.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The default policy row has been deleted.

Suggestions:

  1. Verify that this deletion does not affect other tests that rely on the default policy.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Delete the default organization policy row.

Suggestions:

  1. Explicitly delete default policy for test isolation.

if _, err := db.Exec(`DELETE FROM auto_approve_policies WHERE org_id = $1`, orgID); err != nil {
t.Fatalf("delete default policy: %v", err)
}

_, found, err := ResolvePolicy(context.Background(), db, orgID, 0)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion ui/src/components/Connector/AzureDevOpsConnector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const AzureDevOpsConnector: React.FC = () => {
return (
<div className="space-y-4">
<div className="flex items-center">
<Button variant="ghost" icon={<Icons.Add />} onClick={() => navigate('/git')} iconPosition="left" className="text-sm">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The button icon has been changed to 'Back'.

Suggestions:

  1. Ensure the Icons.Back component is correctly imported and styled.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The icon has been changed from 'Add' to 'Back'.

Suggestions:

  1. Use consistent navigation icons across components.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Apply a negative margin for alignment purposes.

Suggestions:

  1. Ensure consistent spacing and alignment for navigation elements.

<Button variant="ghost" icon={<Icons.Back />} onClick={() => navigate('/git')} iconPosition="left" className="text-sm -ml-4">
Back to providers
</Button>
</div>
Expand Down
2 changes: 1 addition & 1 deletion ui/src/components/Connector/BitbucketConnector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const BitbucketConnector: React.FC = () => {
return (
<div className="space-y-4">
<div className="flex items-center">
<Button variant="ghost" icon={<Icons.Add />} onClick={() => navigate('/git')} iconPosition="left" className="text-sm">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The button icon has been changed to 'Back'.

Suggestions:

  1. Ensure the Icons.Back component is correctly imported and styled.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The icon has been changed from 'Add' to 'Back'.

Suggestions:

  1. Use consistent navigation icons across components.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Apply a negative margin for alignment purposes.

Suggestions:

  1. Ensure consistent spacing and alignment for navigation elements.

<Button variant="ghost" icon={<Icons.Back />} onClick={() => navigate('/git')} iconPosition="left" className="text-sm -ml-4">
Back to providers
</Button>
</div>
Expand Down
2 changes: 1 addition & 1 deletion ui/src/components/Connector/GitHubConnector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const GitHubConnector: React.FC = () => {
return (
<div className="space-y-4">
<div className="flex items-center">
<Button variant="ghost" icon={<Icons.Add />} onClick={() => navigate('/git')} iconPosition="left" className="text-sm">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The button icon has been changed to 'Back'.

Suggestions:

  1. Ensure the Icons.Back component is correctly imported and styled.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The icon has been changed from 'Add' to 'Back'.

Suggestions:

  1. Use consistent navigation icons across components.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Apply a negative margin for alignment purposes.

Suggestions:

  1. Ensure consistent spacing and alignment for navigation elements.

<Button variant="ghost" icon={<Icons.Back />} onClick={() => navigate('/git')} iconPosition="left" className="text-sm -ml-4">
Back to providers
</Button>
</div>
Expand Down
2 changes: 1 addition & 1 deletion ui/src/components/Connector/GitLabComConnector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const GitLabComConnector: React.FC = () => {
return (
<div className="space-y-4">
<div className="flex items-center">
<Button variant="ghost" icon={<Icons.Add />} onClick={() => navigate('/git')} iconPosition="left" className="text-sm">

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The button icon has been changed to 'Back'.

Suggestions:

  1. Ensure the Icons.Back component is correctly imported and styled.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

The icon has been changed from 'Add' to 'Back'.

Suggestions:

  1. Use consistent navigation icons across components.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Severity: info

Apply a negative margin for alignment purposes.

Suggestions:

  1. Ensure consistent spacing and alignment for navigation elements.

<Button variant="ghost" icon={<Icons.Back />} onClick={() => navigate('/git')} iconPosition="left" className="text-sm -ml-4">
Back to providers
</Button>
</div>
Expand Down
Loading
Loading