diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index f5d5dd8275..548dec8dc1 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -d535972f0b35c6a7f257720816bdaec18465adef +bdc4ba4af27f57dc978a7ddca2dccd5b0d67f627 diff --git a/backend/src/main.rs b/backend/src/main.rs index 7cdce97e7a..4ffc55aff5 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -311,6 +311,7 @@ async fn cache_hub_scripts(file_path: Option) -> anyhow::Result<()> { for git_sync_path in [ windmill_common::workspaces::LATEST_GIT_SYNC_SCRIPT_PATH, windmill_common::workspaces::GIT_SYNC_PULL_SCRIPT_PATH, + windmill_common::workspaces::GIT_SYNC_REVIEW_SCRIPT_PATH, ] { let git_sync_path = git_sync_path.to_string(); if !all_paths.contains(&git_sync_path) { diff --git a/backend/windmill-api-workspaces/src/workspaces.rs b/backend/windmill-api-workspaces/src/workspaces.rs index 7c4c649ad2..4f93c4f02d 100644 --- a/backend/windmill-api-workspaces/src/workspaces.rs +++ b/backend/windmill-api-workspaces/src/workspaces.rs @@ -1153,6 +1153,8 @@ async fn reject_parent_only_git_sync_settings_on_fork<'a>( Some("Promotion mode") } else if r.fork_open_prs { Some("Opening PRs for fork deploys") + } else if r.ai_review_prs { + Some("Reviewing pull requests with AI") } else { None } @@ -7988,7 +7990,7 @@ async fn update_workspace_settings( .filter(|r| !r.use_individual_branch.unwrap_or(false)) .take(1) .map(|mut r| { - // Auto-pull and fork PRs are parent-owned and must not be inherited: + // Auto-pull, fork PRs and PR reviews are parent-owned and must not be inherited: // the fork would otherwise carry the parent's webhook id (turning off // auto-pull on the fork would delete the parent's webhook). A fork // still inherits the push-direction config, the installation, and the @@ -7997,6 +7999,7 @@ async fn update_workspace_settings( // fork unqualified for managed features until its first check. r.auto_pull = None; r.fork_open_prs = false; + r.ai_review_prs = false; r.open_pr_error = None; r }) @@ -11341,6 +11344,7 @@ async fn attach_dev_workspace( } r.auto_pull = None; r.fork_open_prs = false; + r.ai_review_prs = false; r.open_pr_error = None; } let serialized = diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 9358628fed..4671a2c18e 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -38690,6 +38690,9 @@ components: type: boolean fork_open_prs: type: boolean + ai_review_prs: + type: boolean + description: review every pull request against the tracked branch with the workspace's AI open_pr_error: type: string description: server-owned, last failure opening a PR for a deploy branch of this repo diff --git a/backend/windmill-common/src/workspaces.rs b/backend/windmill-common/src/workspaces.rs index 898446f0c3..2a266ba813 100644 --- a/backend/windmill-common/src/workspaces.rs +++ b/backend/windmill-common/src/workspaces.rs @@ -216,6 +216,11 @@ pub const LATEST_GIT_SYNC_SCRIPT_PATH: &str = "hub/30052/sync-script-to-git-repo /// reverse proxies reject as double-encoding when the client re-encodes it). pub const GIT_SYNC_PULL_SCRIPT_PATH: &str = "hub/30051/git-sync-init-repository-windmill"; +/// Hub script that reviews a pull request with the workspace's AI (`ai_review_prs`): clones +/// the base branch, fetches the pull request's head and runs `wmill review --json` from the +/// `windmill-cli` it pins. The server posts what it returns. +pub const GIT_SYNC_REVIEW_SCRIPT_PATH: &str = "hub/30053/git-sync-review-pull-request-windmill"; + /// Prefix used to identify fork workspaces. A workspace whose id starts with this string is a /// fork of another workspace. pub const WM_FORK_PREFIX: &str = "wm-fork-"; @@ -378,6 +383,12 @@ pub struct GitRepositorySettings { /// deploy callback reads this from the parent). Off by default. #[serde(default, skip_serializing_if = "is_false")] pub fork_open_prs: bool, + /// Review every pull request against the tracked branch with the workspace's AI and post + /// the findings on it (`wmill review`, run by `GIT_SYNC_REVIEW_SCRIPT_PATH`). Arrives on + /// the pull request webhook, so it needs a credential the server holds. Parent-owned like + /// `fork_open_prs`; off by default. + #[serde(default, skip_serializing_if = "is_false")] + pub ai_review_prs: bool, /// Server-owned: the last failure opening a PR for a deploy branch of this /// repo (e.g. the GitHub App installation hasn't approved the pull-request /// permission). Written by the deploy completion hook, cleared on the next diff --git a/backend/windmill-git-sync/src/lib.rs b/backend/windmill-git-sync/src/lib.rs index d65e230988..a6043a5118 100644 --- a/backend/windmill-git-sync/src/lib.rs +++ b/backend/windmill-git-sync/src/lib.rs @@ -21,12 +21,13 @@ pub use git_sync_ee::{ tally_deployed_object_changes, }; -// The CI-test check exists only on enterprise builds; `private` alone (the CE image) +// The CI-test check and the pull request review exist only on enterprise builds; `private` alone (the CE image) // compiles git_sync_ee without them. #[cfg(all(feature = "private", feature = "enterprise"))] pub use git_sync_ee::{ - ensure_ci_test_check_for_pr, evaluate_and_conclude_ci_test_checks, - post_ci_test_check_not_applicable, resolve_pr_head_workspace, + enqueue_git_pr_review, ensure_ci_test_check_for_pr, evaluate_and_conclude_ci_test_checks, + post_ci_test_check_not_applicable, post_git_pr_review_result, resolve_pr_head_workspace, + GitPrReviewMarker, }; #[cfg(not(feature = "private"))] diff --git a/backend/windmill-worker/src/result_processor.rs b/backend/windmill-worker/src/result_processor.rs index e5b84f39b9..e82f1a1d93 100644 --- a/backend/windmill-worker/src/result_processor.rs +++ b/backend/windmill-worker/src/result_processor.rs @@ -1842,6 +1842,14 @@ pub async fn process_completed_job( .await; maybe_record_git_sync_pushed_head(db, &job_id, &workspace_id, result.get()).await; maybe_open_git_sync_deploy_pr(db, &job_id, &workspace_id, result.get()).await; + windmill_git_sync::post_git_pr_review_result( + db, + &job_id, + &workspace_id, + true, + result.get(), + ) + .await; } // A CI test job just finished: advance any open "Windmill CI tests" PR check for // its workspace. Detached, since concluding a check calls GitHub and this loop @@ -1953,6 +1961,8 @@ pub async fn process_completed_job( if job.kind == JobKind::DeploymentCallback { maybe_post_git_sync_check(db, &job.id, &job.workspace_id, false, result.get()).await; maybe_reconcile_git_sync_auto_pull(db, &job.id, &job.workspace_id, false, "").await; + windmill_git_sync::post_git_pr_review_result(db, &job.id, &job.workspace_id, false, "") + .await; } // A failed CI test job also settles its check; same detached advance as on success. #[cfg(all(feature = "enterprise", feature = "private"))] diff --git a/docs/feature-telemetry.md b/docs/feature-telemetry.md index 6ed1c40004..bf39baf201 100644 --- a/docs/feature-telemetry.md +++ b/docs/feature-telemetry.md @@ -4,13 +4,13 @@ anonymous usage-stats payload. It answers "does anyone use this, and which variant do they pick" without any identifying data leaving the instance. -It currently carries 89 registered actions across thirty-four features (`ai_session`, `ai_chat`, +It currently carries 91 registered actions across thirty-five features (`ai_session`, `ai_chat`, `ai_fix`, `ai_agent`, `ai_decision`, `ai_agent_eval`, `app_sandbox`, `datatable`, `db_manager`, `flow_editor`, `flow_run`, `flow_step`, `home`, `header_nav`, `run_form`, `debugger`, `trigger`, `command_script`, `mcp`, `mcp_workspace`, `hub_script`, `usage_meter`, `sso_groups_claim`, `cloud_trial_offer`, `job_token_scopes`, `worker_draining`, `worker_group`, `worker_resource_stats`, `dev_workspace_isolation`, `slack_agent`, `variable_expiration`, `anthropic_federation`, -`deployment_request`, `fork_roles`). Nearly all of the +`deployment_request`, `fork_roles`, `git_sync_review`). Nearly all of the product is uninstrumented, so new user-facing work is the opportunity to change that. ## When to instrument diff --git a/docs/git-sync-pull-design.md b/docs/git-sync-pull-design.md index bab7cccd3c..ab01e1d843 100644 --- a/docs/git-sync-pull-design.md +++ b/docs/git-sync-pull-design.md @@ -734,12 +734,27 @@ those PRs). A timeout on a repository pinned to a sync script older than the one pushed commits names that as the reason. Known limit (accepted for v1): the fork's status is workspace-wide (all its tested items), which for the one-fork-per-PR model equals the PR's scope. -### Phase 8: AI review of pull requests (`wmill review`), CLI implemented +### Phase 8: AI review of pull requests (`wmill review`), implemented The deployment request panel's AI review, run on a pull request against a synced repository. -One review, three runners: the panel, `wmill review` in a customer's CI, and later an -instance-side job on the `pull_request` webhook (a git sync setting next to `fork_open_prs`, -running `wmill review --json` from a pinned hub script and posting through the installation). +One review, three runners: the panel, `wmill review` in a customer's CI, and an instance-side +job on the pull request webhook, enabled per repository with `ai_review_prs`. + +- **Instance side.** On a pull request event against the tracked branch, the server enqueues + `GIT_SYNC_REVIEW_SCRIPT_PATH` as the read-only sync identity with an + `__git_sync_ai_review` marker (repository identity, number, head). The hub script clones the + base branch, fetches the host's head ref, returns `{skipped}` when the pull request moved + past that head, and otherwise runs `wmill review --json` from the `windmill-cli` it pins. + The completion hook (`post_git_pr_review_result`) posts the `post` payload that JSON + carries through the credential the server holds: a GitHub review per head, deduplicated by + the same marker `--post github` writes, or the one managed GitLab note; a failed job posts + a note saying so. The script is on the installation-token allowlist by slug. +- **Who gets reviewed.** Only a head in the base repository: a contributor fork could + otherwise spend the workspace's AI at will. The setting is parent-owned like + `fork_open_prs`, so forks never hold it. +- **Delivery.** A change to the review reaches the instance side as a `windmill-cli` release, + then a hub script version pinning it, then a bump of `GIT_SYNC_REVIEW_SCRIPT_PATH`, the same + chain as the sync scripts. - **One source for the review.** Prompt, size budget, withheld kinds and answer parsing live in `frontend/src/lib/components/deploymentRequest/aiReviewCore.ts`, which imports nothing diff --git a/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts b/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts index 97b79b7c1b..f742aa853e 100644 --- a/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts +++ b/frontend/src/lib/components/git_sync/GitSyncContext.svelte.ts @@ -191,7 +191,8 @@ export function createGitSyncContext(workspace: string) { exclude_types_override: repo.exclude_types_override, auto_pull: repo.auto_pull, promotion_open_prs: repo.promotion_open_prs, - fork_open_prs: repo.fork_open_prs + fork_open_prs: repo.fork_open_prs, + ai_review_prs: repo.ai_review_prs } } @@ -561,7 +562,8 @@ export function createGitSyncContext(workspace: string) { exclude_types_override: repoToSave.exclude_types_override, auto_pull: repoToSave.auto_pull, promotion_open_prs: repoToSave.promotion_open_prs, - fork_open_prs: repoToSave.fork_open_prs + fork_open_prs: repoToSave.fork_open_prs, + ai_review_prs: repoToSave.ai_review_prs } } }) diff --git a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte index 8c3af1c54d..27ae11e412 100644 --- a/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte +++ b/frontend/src/lib/components/git_sync/GitSyncRepositoryCard.svelte @@ -18,6 +18,7 @@ import { sendUserToast } from '$lib/toast' import { apiErrorMessage } from '$lib/utils' import { workspaceStore, userWorkspaces, enterpriseLicense } from '$lib/stores' + import { copilotInfo } from '$lib/aiStore' import type { GitSyncRepository } from './GitSyncContext.svelte' import GitSyncModeDisplay from './GitSyncModeDisplay.svelte' import Toggle from '$lib/components/Toggle.svelte' @@ -95,6 +96,9 @@ function setForkOpenPrs(v: boolean) { if (repo) repo.fork_open_prs = v } + function setAiReviewPrs(v: boolean) { + if (repo) repo.ai_review_prs = v + } function setPromotionOpenPrs(v: boolean) { if (repo) repo.promotion_open_prs = v } @@ -168,6 +172,20 @@ // Not the recorded status: that is keyed by resource path and outlives a // repoint, while the origin follows the repository the URL names now. let hasManagedCredential = $derived(isGithubApp || credentialOrigin !== undefined) + // Why the AI review cannot be turned on, or undefined when it can. Pull request events + // arrive on the auto-pull webhook, the review posts through the credential Windmill holds, + // and the job asks the workspace's default model. + const aiReviewBlocker = $derived( + !$enterpriseLicense + ? 'Requires an enterprise license.' + : !hasManagedCredential + ? 'Needs a credential Windmill holds for the repository: the GitHub App, or a GitLab project access token.' + : !repo?.auto_pull?.enabled + ? 'Requires automatic deploy from Git to be enabled above: pull request events arrive on its webhook.' + : !$copilotInfo.defaultModel + ? 'Set a default model in the workspace AI settings first.' + : undefined + ) const MS_PER_DAY = 86_400_000 @@ -893,6 +911,23 @@ on:change={(e) => setSyncForks(e.detail)} /> +
+ setAiReviewPrs(e.detail)} + > + {#snippet right()} + {#if !$enterpriseLicense}{/if} + {/snippet} + +
{/if} {#if !hasManagedCredential && !loadingResourceInfo}