mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-30 16:17:25 +00:00
feat: review git sync pull requests with wmill review (#12055)
* feat: add ai review to deployment requests Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: bound ai review fetches and surface unreadable items Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat: add fix in ai session and model picker to deployment ai review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat: add workspace instructions for deployment ai review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: read review instructions from the parent and bind ai review to the fork Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: pin the fix-all session preview to the fork's compare page Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: show the protection-rule bypass toggle inline Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: make the bypass toggle small and red Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor: drop workspace-settings instructions from deployment ai review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat: folder-scoped review instructions for the deployment ai review Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * feat: review the selected items with ai on compare and deploy Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: keep reviewing when review instructions cannot be listed Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: say the review instruction floor is a prompt request, not enforcement Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: mark an ai review out of date when its items change Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee-repo-ref to 77703cf32e58964906d87ee83ddda44b5b8d1b7e This commit updates the EE repository reference after PR #926 was merged in windmill-ee-private. Previous ee-repo-ref: e30ab9e69d1854fcaed5620c934ace32acb0a729 New ee-repo-ref: 77703cf32e58964906d87ee83ddda44b5b8d1b7e Automated by sync-ee-ref workflow. * feat: review git sync pull requests with wmill review * fix: withhold retyped resources and encryption keys, fall back from the responses route * fix: withhold fileset children, workspace-specific resources and pathspec-named files * feat: include the ready-to-post review in wmill review --json * feat: review a workspace synced into a repository subfolder * docs: say what wmill review takes from flags rather than wmill.yaml --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
Ruben Fiszel
co-authored by
Claude Opus 5.5
windmill-internal-app[bot]
Claude Opus 5.5
windmill-internal-app[bot]
parent
60d07dc0d9
commit
fbe776d611
@@ -8,6 +8,7 @@ pub mod credentials;
|
||||
pub mod decision;
|
||||
pub mod image_handler;
|
||||
pub mod model_context;
|
||||
pub mod one_shot;
|
||||
pub mod providers;
|
||||
pub mod proxy;
|
||||
pub mod query_builder;
|
||||
|
||||
@@ -0,0 +1,207 @@
|
||||
//! A single text completion with no tools and no streaming reader: agent-step history
|
||||
//! compaction and the `/ai/complete` endpoint both send one through the provider's query
|
||||
//! builder, so they share its request shape, credential headers and error handling.
|
||||
|
||||
use async_trait::async_trait;
|
||||
use windmill_common::{client::AuthedClient, error::Error};
|
||||
|
||||
use crate::{
|
||||
ai_providers::AIProvider,
|
||||
credentials::ProviderCredentials,
|
||||
proxy::{common_outbound_headers, retain_effective_credentials},
|
||||
providers::{create_chat_completions_query_builder, remember_chat_completions_only},
|
||||
query_builder::{
|
||||
is_context_length_error, BuildRequestArgs, ParsedResponse, QueryBuilder, StreamEventSink,
|
||||
},
|
||||
types::{OpenAIMessage, OutputType, StreamingEvent, TokenUsage},
|
||||
utils::pinned_ai_client_for,
|
||||
};
|
||||
|
||||
pub struct OneShotRequest<'a> {
|
||||
pub query_builder: &'a dyn QueryBuilder,
|
||||
/// For Bedrock, the keys an assumed OIDC role yielded are expected here already.
|
||||
pub credentials: &'a ProviderCredentials,
|
||||
pub model: &'a str,
|
||||
pub max_tokens: Option<u32>,
|
||||
pub reasoning_effort: Option<&'a str>,
|
||||
pub timeout: std::time::Duration,
|
||||
/// Only reached for attachments, which a one-shot request never carries.
|
||||
pub client: &'a AuthedClient,
|
||||
pub workspace_id: &'a str,
|
||||
/// An already exchanged Anthropic federation token, sent in place of the api key.
|
||||
pub federated_token: Option<&'a str>,
|
||||
/// Whether to send the usage-tracking request shape. A gateway that rejects it is
|
||||
/// retried once without it either way.
|
||||
pub include_usage: bool,
|
||||
}
|
||||
|
||||
struct DiscardEvents;
|
||||
|
||||
#[async_trait]
|
||||
impl StreamEventSink for DiscardEvents {
|
||||
async fn send(&self, _event: StreamingEvent, _events_str: &mut String) -> Result<(), Error> {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
pub async fn complete_text(
|
||||
messages: &[OpenAIMessage],
|
||||
request: &OneShotRequest<'_>,
|
||||
) -> Result<(String, Option<TokenUsage>), Error> {
|
||||
let build_args = BuildRequestArgs {
|
||||
messages,
|
||||
tools: None,
|
||||
model: request.model,
|
||||
// OpenAI's reasoning models reject `temperature` alongside any effort but their
|
||||
// own default, so it is left to the provider rather than tracked per model.
|
||||
temperature: None,
|
||||
reasoning_effort: request.reasoning_effort,
|
||||
// Nothing reads this call's reasoning, and a summary of it would be billed.
|
||||
reasoning_summary: false,
|
||||
max_tokens: request.max_tokens,
|
||||
output_schema: None,
|
||||
output_type: &OutputType::Text,
|
||||
system_prompt: None,
|
||||
user_message: "",
|
||||
attachments: None,
|
||||
has_websearch: false,
|
||||
prompt_cache_key: None,
|
||||
};
|
||||
|
||||
let parsed = if request.credentials.provider == AIProvider::AWSBedrock {
|
||||
#[cfg(feature = "bedrock")]
|
||||
{
|
||||
let credentials = request.credentials;
|
||||
crate::providers::bedrock::BedrockQueryBuilder::default()
|
||||
.execute_request(
|
||||
messages,
|
||||
None,
|
||||
request.model,
|
||||
None,
|
||||
build_args.reasoning_effort,
|
||||
build_args.max_tokens,
|
||||
credentials.api_key.as_deref().unwrap_or(""),
|
||||
credentials
|
||||
.region
|
||||
.as_deref()
|
||||
.unwrap_or(crate::ai_providers::USE_ENV_REGION),
|
||||
Some(Box::new(DiscardEvents)),
|
||||
request.client,
|
||||
request.workspace_id,
|
||||
None,
|
||||
credentials.aws_access_key_id.as_deref(),
|
||||
credentials.aws_secret_access_key.as_deref(),
|
||||
credentials.aws_session_token.as_deref(),
|
||||
)
|
||||
.await?
|
||||
}
|
||||
#[cfg(not(feature = "bedrock"))]
|
||||
{
|
||||
return Err(Error::internal_err(
|
||||
"AWS Bedrock support is not enabled. Build with 'bedrock' feature.".to_string(),
|
||||
));
|
||||
}
|
||||
} else {
|
||||
send_http(&build_args, request).await?
|
||||
};
|
||||
|
||||
match parsed.without_nul() {
|
||||
ParsedResponse::Text { content, usage, .. } => Ok((content.unwrap_or_default(), usage)),
|
||||
ParsedResponse::Image { .. } => Err(Error::internal_err(
|
||||
"The completion came back as an image".to_string(),
|
||||
)),
|
||||
}
|
||||
}
|
||||
|
||||
async fn send_http(
|
||||
build_args: &BuildRequestArgs<'_>,
|
||||
request: &OneShotRequest<'_>,
|
||||
) -> Result<ParsedResponse, Error> {
|
||||
let credentials = request.credentials;
|
||||
let base_url = &credentials.base_url;
|
||||
let api_key = credentials.api_key.as_deref().unwrap_or("");
|
||||
// `endpoint` derives from the user-controlled provider base_url, so DNS is pinned to
|
||||
// the SSRF-validated address.
|
||||
let client = pinned_ai_client_for(base_url).await?;
|
||||
|
||||
// Azure serves the Responses API for only some models and regions, so a deployment
|
||||
// that rejects the route is asked again on `/chat/completions`, as the agent step does.
|
||||
let mut fallback: Option<Box<dyn QueryBuilder>> = None;
|
||||
let mut route_absent = false;
|
||||
let mut include_usage = request.include_usage;
|
||||
loop {
|
||||
let query_builder = fallback.as_deref().unwrap_or(request.query_builder);
|
||||
let endpoint = query_builder.get_endpoint(base_url, request.model, &OutputType::Text);
|
||||
let auth_headers = match request.federated_token {
|
||||
Some(token) => crate::providers::anthropic::federated_auth_headers(token),
|
||||
None => retain_effective_credentials(
|
||||
credentials,
|
||||
query_builder.get_auth_headers(api_key, base_url, &OutputType::Text),
|
||||
),
|
||||
};
|
||||
let body = if include_usage {
|
||||
query_builder
|
||||
.build_request(build_args, request.client, request.workspace_id)
|
||||
.await?
|
||||
} else {
|
||||
query_builder
|
||||
.build_request_without_usage(build_args, request.client, request.workspace_id)
|
||||
.await?
|
||||
};
|
||||
let mut http_request = client
|
||||
.post(&endpoint)
|
||||
.timeout(request.timeout)
|
||||
.header("Content-Type", "application/json");
|
||||
for (header_name, header_value) in &auth_headers {
|
||||
http_request = http_request.header(*header_name, header_value.clone());
|
||||
}
|
||||
for (header_name, header_value) in common_outbound_headers(credentials) {
|
||||
http_request = http_request.header(header_name.as_str(), header_value.as_str());
|
||||
}
|
||||
let resp = http_request
|
||||
.body(body)
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| Error::internal_err(format!("Failed to call API: {e}")))?;
|
||||
|
||||
if let Err(e) = resp.error_for_status_ref() {
|
||||
let status = resp.status();
|
||||
let text = resp
|
||||
.text()
|
||||
.await
|
||||
.unwrap_or_else(|_| "<failed to read body>".to_string());
|
||||
// Not every OpenAI-compatible gateway accepts `stream_options`.
|
||||
if include_usage
|
||||
&& query_builder.supports_retry_without_usage()
|
||||
&& status.as_u16() == 400
|
||||
&& (text.contains("stream_options")
|
||||
|| text.contains("include_usage")
|
||||
|| text.contains("Additional properties are not allowed"))
|
||||
{
|
||||
include_usage = false;
|
||||
continue;
|
||||
}
|
||||
if fallback.is_none()
|
||||
&& request.query_builder.supports_chat_completions_fallback(base_url)
|
||||
&& matches!(status.as_u16(), 400 | 404)
|
||||
&& !is_context_length_error(&text)
|
||||
{
|
||||
// Only a 404 says the route is absent; a 400 re-routes this call alone.
|
||||
route_absent = status.as_u16() == 404;
|
||||
fallback = Some(create_chat_completions_query_builder(credentials));
|
||||
include_usage = true;
|
||||
continue;
|
||||
}
|
||||
return Err(Error::internal_err(format!(
|
||||
"API error calling {endpoint}: {e} - {text}"
|
||||
)));
|
||||
}
|
||||
|
||||
if route_absent {
|
||||
remember_chat_completions_only(base_url, request.model);
|
||||
}
|
||||
return query_builder
|
||||
.parse_streaming_response(resp, Box::new(DiscardEvents))
|
||||
.await;
|
||||
}
|
||||
}
|
||||
@@ -14005,6 +14005,53 @@ paths:
|
||||
items:
|
||||
$ref: "#/components/schemas/FlowConversationMessage"
|
||||
|
||||
/w/{workspace}/ai/complete:
|
||||
post:
|
||||
summary: one text completion with the workspace's AI config
|
||||
description: >
|
||||
Sends a system and a user message to the configured provider and returns the text of
|
||||
the answer, with no tools and no streaming. For callers that do not speak a provider's
|
||||
own API through the proxy, such as the CLI and jobs.
|
||||
operationId: completeAi
|
||||
tags:
|
||||
- ai
|
||||
parameters:
|
||||
- $ref: "#/components/parameters/WorkspaceId"
|
||||
requestBody:
|
||||
required: true
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
type: object
|
||||
properties:
|
||||
system:
|
||||
type: string
|
||||
user:
|
||||
type: string
|
||||
model:
|
||||
description: defaults to the AI config's default model
|
||||
$ref: "#/components/schemas/AIProviderModel"
|
||||
max_tokens:
|
||||
type: integer
|
||||
required:
|
||||
- system
|
||||
- user
|
||||
responses:
|
||||
"200":
|
||||
description: the answer, and the model that wrote it
|
||||
content:
|
||||
application/json:
|
||||
schema:
|
||||
type: object
|
||||
properties:
|
||||
text:
|
||||
type: string
|
||||
model:
|
||||
$ref: "#/components/schemas/AIProviderModel"
|
||||
required:
|
||||
- text
|
||||
- model
|
||||
|
||||
/w/{workspace}/ai_evals/datasets/list:
|
||||
get:
|
||||
summary: list eval datasets
|
||||
|
||||
+268
-165
@@ -25,6 +25,7 @@ use windmill_ai::ai_providers::{
|
||||
};
|
||||
use windmill_ai::ai_types::{validate_token_limit, CONTEXT_WINDOWS, MAX_MODEL_RATE, OUTPUT_LIMITS};
|
||||
use windmill_ai::credentials::{AnthropicFederation, ProviderCredentials};
|
||||
use windmill_ai::one_shot::{complete_text, OneShotRequest};
|
||||
use windmill_ai::providers::anthropic::{
|
||||
exchange_federated_token, FederatedAccessToken, ANTHROPIC_API_BASE_URL,
|
||||
};
|
||||
@@ -33,19 +34,23 @@ use windmill_ai::providers::bedrock::{
|
||||
handle_bedrock_proxy, BedrockProxyResponse, BedrockProxyResponseBody,
|
||||
};
|
||||
use windmill_ai::providers::{
|
||||
create_query_builder,
|
||||
create_chat_completions_query_builder, create_query_builder,
|
||||
google_ai::{
|
||||
handle_google_ai_chat_proxy, handle_google_ai_models_proxy, GoogleAIProxyResponse,
|
||||
GoogleAIProxyResponseBody,
|
||||
},
|
||||
is_chat_completions_only,
|
||||
};
|
||||
use windmill_ai::proxy::{
|
||||
fim::maybe_transform_fim_request, proxy_execution_mode, validate_proxy_path, ProxyBuildArgs,
|
||||
ProxyExecutionMode, ProxyRequest,
|
||||
};
|
||||
use windmill_ai::retry::send_with_retries;
|
||||
use windmill_ai::types::{OpenAIContent, OpenAIMessage};
|
||||
use windmill_audit::{audit_oss::audit_log, ActionKind};
|
||||
use windmill_common::client::AuthedClient;
|
||||
use windmill_common::db::UserDB;
|
||||
use windmill_common::error::JsonResult;
|
||||
use windmill_common::error::{to_anyhow, Error, Result};
|
||||
use windmill_common::utils::{configure_client, require_admin};
|
||||
use windmill_common::variables::{get_variable_or_self, get_variable_or_self_as};
|
||||
@@ -824,6 +829,105 @@ impl AIConfig {
|
||||
}
|
||||
}
|
||||
|
||||
/// The AI config requests in `w_id` are served from: the workspace's own when it
|
||||
/// configures a provider, else the instance's, with the workspace the configured resource
|
||||
/// paths live in and, for the instance config, the revision its cache entries are tied to.
|
||||
/// `None` when neither configures a provider.
|
||||
async fn effective_ai_config(
|
||||
db: &DB,
|
||||
w_id: &str,
|
||||
) -> Result<Option<(AIConfig, String, Option<u64>)>> {
|
||||
let workspace_ai_config = sqlx::query_scalar!(
|
||||
"SELECT ai_config FROM workspace_settings WHERE workspace_id = $1",
|
||||
w_id
|
||||
)
|
||||
.fetch_one(db)
|
||||
.await?;
|
||||
if let Some(config) = workspace_ai_config
|
||||
.and_then(|v| serde_json::from_value::<AIConfig>(v).ok())
|
||||
.filter(AIConfig::has_providers)
|
||||
{
|
||||
return Ok(Some((config, w_id.to_string(), None)));
|
||||
}
|
||||
let instance_config =
|
||||
sqlx::query_scalar!("SELECT value FROM global_settings WHERE name = 'ai_config'")
|
||||
.fetch_optional(db)
|
||||
.await?;
|
||||
// An instance `ai_config` row with no usable provider (e.g. `{}` or
|
||||
// `{"providers":{}}`) is treated as unconfigured, exactly as
|
||||
// build_copilot_settings_state does — otherwise its mere presence would suppress the
|
||||
// free-tier fallback.
|
||||
Ok(instance_config
|
||||
.and_then(|v| serde_json::from_value::<AIConfig>(v).ok())
|
||||
.filter(AIConfig::has_providers)
|
||||
.map(|config| {
|
||||
(
|
||||
config,
|
||||
"admins".to_string(),
|
||||
Some(current_instance_ai_config_revision()),
|
||||
)
|
||||
}))
|
||||
}
|
||||
|
||||
/// Credentials for `provider` from the effective AI config, or `None` when no provider is
|
||||
/// configured at all.
|
||||
async fn configured_credentials(
|
||||
db: &DB,
|
||||
w_id: &str,
|
||||
provider: &AIProvider,
|
||||
) -> Result<Option<ProviderCredentials>> {
|
||||
let cache_key = (w_id.to_string(), provider.clone());
|
||||
if let Some(cached) = AI_REQUEST_CACHE.get(&cache_key).filter(|c| !c.is_expired()) {
|
||||
return Ok(Some(cached.credentials));
|
||||
}
|
||||
let Some((mut ai_config, resource_workspace, instance_ai_config_revision)) =
|
||||
effective_ai_config(db, w_id).await?
|
||||
else {
|
||||
return Ok(None);
|
||||
};
|
||||
let provider_config = ai_config
|
||||
.providers
|
||||
.as_mut()
|
||||
.and_then(|providers| providers.remove(provider))
|
||||
.ok_or_else(|| Error::BadRequest(format!("Provider {:?} not configured", provider)))?;
|
||||
let resource_path = provider_config.resource_path;
|
||||
if resource_path.is_empty() {
|
||||
return Err(Error::BadRequest("Resource path is empty".to_string()));
|
||||
}
|
||||
// The resource path was validated by an admin/devops user when configuring the AI
|
||||
// config, so the raw pool is used.
|
||||
let res = sqlx::query_scalar::<_, Option<sqlx::types::Json<Box<RawValue>>>>(
|
||||
"SELECT value FROM resource WHERE path = $1 AND workspace_id = $2",
|
||||
)
|
||||
.bind(&resource_path)
|
||||
.bind(&resource_workspace)
|
||||
.fetch_optional(db)
|
||||
.await?;
|
||||
let resource = parse_ai_resource(res, &resource_path)?;
|
||||
let credentials =
|
||||
resolve_provider_credentials(provider, db, &resource_workspace, resource, None).await?;
|
||||
AI_REQUEST_CACHE.insert(
|
||||
cache_key,
|
||||
ExpiringProviderCredentials::new(credentials.clone(), instance_ai_config_revision),
|
||||
);
|
||||
Ok(Some(credentials))
|
||||
}
|
||||
|
||||
fn parse_ai_resource(
|
||||
row: Option<Option<sqlx::types::Json<Box<RawValue>>>>,
|
||||
resource_path: &str,
|
||||
) -> Result<AIResource> {
|
||||
let value = row
|
||||
.ok_or_else(|| {
|
||||
Error::NotFound(format!(
|
||||
"Could not find the resource {}, update the resource path in the workspace settings",
|
||||
resource_path
|
||||
))
|
||||
})?
|
||||
.ok_or_else(|| Error::BadRequest(format!("Empty resource value for {}", resource_path)))?;
|
||||
serde_json::from_str::<AIResource>(value.0.get()).map_err(|e| Error::BadRequest(e.to_string()))
|
||||
}
|
||||
|
||||
pub fn global_service() -> Router {
|
||||
Router::new().route("/proxy/{*ai}", post(global_proxy).get(global_proxy))
|
||||
}
|
||||
@@ -831,6 +935,7 @@ pub fn global_service() -> Router {
|
||||
pub fn workspaced_service() -> Router {
|
||||
let router = Router::new()
|
||||
.route("/proxy/{*ai}", post(proxy).get(proxy))
|
||||
.route("/complete", post(complete))
|
||||
.route(
|
||||
"/usage",
|
||||
post(record_ai_usage)
|
||||
@@ -1226,6 +1331,121 @@ fn var_references(value: &serde_json::Value) -> Vec<&str> {
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct CompleteRequest {
|
||||
system: String,
|
||||
user: String,
|
||||
/// Defaults to the AI config's `default_model`.
|
||||
#[serde(default)]
|
||||
model: Option<ProviderModel>,
|
||||
#[serde(default)]
|
||||
max_tokens: Option<u32>,
|
||||
}
|
||||
|
||||
#[derive(Serialize)]
|
||||
struct CompleteResponse {
|
||||
text: String,
|
||||
model: ProviderModel,
|
||||
}
|
||||
|
||||
/// One text completion with the workspace's AI config, for callers that are not a browser
|
||||
/// talking a provider's own API through the proxy (the CLI, jobs). Provider differences are
|
||||
/// handled by the same query builders AI agent steps use.
|
||||
async fn complete(
|
||||
authed: ApiAuthed,
|
||||
Extension(db): Extension<DB>,
|
||||
Path(w_id): Path<String>,
|
||||
Json(request): Json<CompleteRequest>,
|
||||
) -> JsonResult<CompleteResponse> {
|
||||
let not_configured =
|
||||
|| Error::BadRequest("AI is not configured for this workspace".to_string());
|
||||
let model = match request.model {
|
||||
Some(model) => model,
|
||||
None => effective_ai_config(&db, &w_id)
|
||||
.await?
|
||||
.ok_or_else(not_configured)?
|
||||
.0
|
||||
.default_model
|
||||
.ok_or_else(|| {
|
||||
Error::BadRequest(
|
||||
"No model given, and the AI settings set no default model".to_string(),
|
||||
)
|
||||
})?,
|
||||
};
|
||||
if model.provider.is_decision_provider() {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"{:?} serves decision models, which answer typed questions rather than messages",
|
||||
model.provider
|
||||
)));
|
||||
}
|
||||
let mut credentials = configured_credentials(&db, &w_id, &model.provider)
|
||||
.await?
|
||||
.ok_or_else(not_configured)?;
|
||||
#[cfg(feature = "bedrock")]
|
||||
resolve_bedrock_oidc_credentials(&db, &w_id, &authed, &mut credentials).await?;
|
||||
resolve_anthropic_federated_credentials(&db, &w_id, &authed, &mut credentials).await?;
|
||||
// The query builders send the api key; only a federated Anthropic token has a way in.
|
||||
if credentials.access_token.is_some() && credentials.anthropic_federation.is_none() {
|
||||
return Err(Error::BadRequest(format!(
|
||||
"The {:?} resource authenticates with OAuth, which this endpoint does not support. \
|
||||
Set an API key on the resource, or carry the provider's credential header in its \
|
||||
`headers`.",
|
||||
credentials.provider
|
||||
)));
|
||||
}
|
||||
|
||||
let base_url = credentials.base_url.clone();
|
||||
let mut query_builder = create_query_builder(&credentials, &model.model);
|
||||
if query_builder.supports_chat_completions_fallback(&base_url)
|
||||
&& is_chat_completions_only(&base_url, &model.model)
|
||||
{
|
||||
query_builder = create_chat_completions_query_builder(&credentials);
|
||||
}
|
||||
|
||||
let mut tx = db.begin().await?;
|
||||
audit_log(
|
||||
&mut *tx,
|
||||
&authed,
|
||||
"ai.request",
|
||||
ActionKind::Execute,
|
||||
&w_id,
|
||||
Some(&authed.email),
|
||||
Some([("ai_config_path", "complete")].into()),
|
||||
)
|
||||
.await?;
|
||||
tx.commit().await?;
|
||||
|
||||
let messages =
|
||||
[("system", request.system), ("user", request.user)].map(|(role, text)| OpenAIMessage {
|
||||
role: role.to_string(),
|
||||
content: Some(OpenAIContent::Text(text)),
|
||||
..Default::default()
|
||||
});
|
||||
// Attachments are the only thing a query builder reaches the API for, and a one-shot
|
||||
// request carries none.
|
||||
let client = AuthedClient::new(String::new(), w_id.clone(), String::new(), None);
|
||||
let (text, _usage) = complete_text(
|
||||
&messages,
|
||||
&OneShotRequest {
|
||||
query_builder: query_builder.as_ref(),
|
||||
credentials: &credentials,
|
||||
model: &model.model,
|
||||
max_tokens: request.max_tokens,
|
||||
reasoning_effort: None,
|
||||
timeout: Duration::from_secs(*AI_TIMEOUT_SECS),
|
||||
client: &client,
|
||||
workspace_id: &w_id,
|
||||
federated_token: credentials
|
||||
.anthropic_federation
|
||||
.as_ref()
|
||||
.and(credentials.access_token.as_deref()),
|
||||
include_usage: true,
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
Ok(Json(CompleteResponse { text, model }))
|
||||
}
|
||||
|
||||
async fn global_proxy(
|
||||
authed: ApiAuthed,
|
||||
Extension(db): Extension<DB>,
|
||||
@@ -1364,12 +1584,9 @@ async fn proxy(
|
||||
None => return Err(Error::BadRequest("Provider is required".to_string())),
|
||||
};
|
||||
|
||||
let workspace_cache = AI_REQUEST_CACHE.get(&(w_id.clone(), provider.clone()));
|
||||
|
||||
let forced_resource_path = headers
|
||||
.get("X-Resource-Path")
|
||||
.map(|v| v.to_str().unwrap_or("").to_string());
|
||||
let is_user_specified_resource = forced_resource_path.is_some();
|
||||
|
||||
// When the caller supplies X-Resource-Path, the resource is treated as if it
|
||||
// were being read through the normal resource API: scope and RLS checks must
|
||||
@@ -1414,168 +1631,54 @@ async fn proxy(
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
match workspace_cache {
|
||||
Some(request_cache)
|
||||
if !request_cache.is_expired() && forced_resource_path.is_none() =>
|
||||
{
|
||||
request_cache.credentials
|
||||
}
|
||||
_ => {
|
||||
let (resource_path, save_to_cache, resource_workspace, instance_ai_config_revision) =
|
||||
if let Some(resource_path) = forced_resource_path {
|
||||
// forced resource path
|
||||
(resource_path, false, w_id.clone(), None)
|
||||
} else {
|
||||
let workspace_ai_config = sqlx::query_scalar!(
|
||||
"SELECT ai_config FROM workspace_settings WHERE workspace_id = $1",
|
||||
&w_id
|
||||
)
|
||||
.fetch_one(&db)
|
||||
.await?;
|
||||
|
||||
let (ai_config_value, resource_workspace, instance_ai_config_revision) = {
|
||||
let ws_has_config = workspace_ai_config
|
||||
.as_ref()
|
||||
.and_then(|v| serde_json::from_value::<AIConfig>(v.clone()).ok())
|
||||
.is_some_and(|config| config.has_providers());
|
||||
|
||||
if ws_has_config {
|
||||
(workspace_ai_config.unwrap(), w_id.clone(), None)
|
||||
} else {
|
||||
let instance_config = sqlx::query_scalar!(
|
||||
"SELECT value FROM global_settings WHERE name = 'ai_config'"
|
||||
)
|
||||
.fetch_optional(&db)
|
||||
.await?;
|
||||
|
||||
let instance_has_config =
|
||||
instance_config.as_ref().is_some_and(|v| {
|
||||
serde_json::from_value::<AIConfig>(v.clone())
|
||||
.ok()
|
||||
.is_some_and(|c| c.has_providers())
|
||||
});
|
||||
match instance_config {
|
||||
// An instance `ai_config` row with no usable provider (e.g. `{}`
|
||||
// or `{"providers":{}}`) is treated as unconfigured, exactly as
|
||||
// build_copilot_settings_state does — otherwise its mere presence
|
||||
// would suppress the free-tier fallback below.
|
||||
Some(config) if instance_has_config => (
|
||||
config,
|
||||
"admins".to_string(),
|
||||
Some(current_instance_ai_config_revision()),
|
||||
),
|
||||
_ => {
|
||||
// Nothing configured: fall back to Windmill's free AI tier
|
||||
// (EE-only) if a lent key is set and both the user's
|
||||
// one-time grant and the instance's daily cap have room.
|
||||
// Errors once the grant is spent, the day is capped, or the
|
||||
// user already has a request in flight; None otherwise.
|
||||
// Ineligible identities (e.g. service accounts) are refused
|
||||
// inside the helper, so every path treats them alike.
|
||||
let free =
|
||||
crate::ai_free_tier_oss::resolve_free_tier_credentials(
|
||||
&provider,
|
||||
&db,
|
||||
&ai_path,
|
||||
&authed.email,
|
||||
&body,
|
||||
)
|
||||
.await?;
|
||||
if let Some((free_credentials, lease)) = free {
|
||||
free_lease = Some(lease);
|
||||
break 'cred free_credentials;
|
||||
}
|
||||
return Err(Error::internal_err(
|
||||
"AI resource not configured".to_string(),
|
||||
));
|
||||
}
|
||||
}
|
||||
}
|
||||
};
|
||||
|
||||
let mut ai_config = serde_json::from_value::<AIConfig>(ai_config_value)
|
||||
.map_err(|e| Error::BadRequest(e.to_string()))?;
|
||||
|
||||
let provider_config = ai_config
|
||||
.providers
|
||||
.as_mut()
|
||||
.and_then(|providers| providers.remove(&provider))
|
||||
.ok_or_else(|| {
|
||||
Error::BadRequest(format!("Provider {:?} not configured", provider))
|
||||
})?;
|
||||
|
||||
if provider_config.resource_path.is_empty() {
|
||||
return Err(Error::BadRequest("Resource path is empty".to_string()));
|
||||
}
|
||||
|
||||
(
|
||||
provider_config.resource_path,
|
||||
true,
|
||||
resource_workspace,
|
||||
instance_ai_config_revision,
|
||||
)
|
||||
};
|
||||
|
||||
// For user-specified resources, fetch through an RLS-scoped
|
||||
// connection so PostgreSQL row-level security enforces the same
|
||||
// folder/group boundaries as the regular resource API. For the
|
||||
// workspace/instance ai_config path, the resource_path was already
|
||||
// validated by an admin/devops user when configuring the workspace,
|
||||
// so the raw pool is used.
|
||||
let resource = if is_user_specified_resource {
|
||||
let mut tx = user_db.clone().begin(&authed).await?;
|
||||
let res = sqlx::query_scalar::<_, Option<sqlx::types::Json<Box<RawValue>>>>(
|
||||
"SELECT value FROM resource WHERE path = $1 AND workspace_id = $2",
|
||||
)
|
||||
.bind(&resource_path)
|
||||
.bind(&resource_workspace)
|
||||
.fetch_optional(&mut *tx)
|
||||
.await?;
|
||||
tx.commit().await?;
|
||||
res
|
||||
} else {
|
||||
sqlx::query_scalar::<_, Option<sqlx::types::Json<Box<RawValue>>>>(
|
||||
"SELECT value FROM resource WHERE path = $1 AND workspace_id = $2",
|
||||
)
|
||||
.bind(&resource_path)
|
||||
.bind(&resource_workspace)
|
||||
.fetch_optional(&db)
|
||||
.await?
|
||||
}
|
||||
.ok_or_else(|| Error::NotFound(format!("Could not find the resource {}, update the resource path in the workspace settings", resource_path)))?
|
||||
.ok_or_else(|| Error::BadRequest(format!("Empty resource value for {}", resource_path)))?;
|
||||
|
||||
let resource = serde_json::from_str::<AIResource>(resource.0.get())
|
||||
.map_err(|e| Error::BadRequest(e.to_string()))?;
|
||||
|
||||
// Enforce RLS on $var: resolution when the resource path was
|
||||
// user-specified (X-Resource-Path header) so users can only read
|
||||
// variables they have permission to access.
|
||||
let enforce_authed = if is_user_specified_resource {
|
||||
Some(&authed)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let credentials = resolve_provider_credentials(
|
||||
&provider,
|
||||
&db,
|
||||
&resource_workspace,
|
||||
resource,
|
||||
enforce_authed,
|
||||
)
|
||||
.await?;
|
||||
if save_to_cache {
|
||||
AI_REQUEST_CACHE.insert(
|
||||
(w_id.clone(), provider.clone()),
|
||||
ExpiringProviderCredentials::new(
|
||||
credentials.clone(),
|
||||
instance_ai_config_revision,
|
||||
),
|
||||
);
|
||||
}
|
||||
credentials
|
||||
}
|
||||
if let Some(resource_path) = forced_resource_path {
|
||||
// For user-specified resources, fetch through an RLS-scoped connection so
|
||||
// PostgreSQL row-level security enforces the same folder/group boundaries as
|
||||
// the regular resource API.
|
||||
let mut tx = user_db.clone().begin(&authed).await?;
|
||||
let res = sqlx::query_scalar::<_, Option<sqlx::types::Json<Box<RawValue>>>>(
|
||||
"SELECT value FROM resource WHERE path = $1 AND workspace_id = $2",
|
||||
)
|
||||
.bind(&resource_path)
|
||||
.bind(&w_id)
|
||||
.fetch_optional(&mut *tx)
|
||||
.await?;
|
||||
tx.commit().await?;
|
||||
let resource = parse_ai_resource(res, &resource_path)?;
|
||||
// RLS also applies to the resource's $var: references, so users can only
|
||||
// read variables they have permission to access.
|
||||
break 'cred resolve_provider_credentials(
|
||||
&provider,
|
||||
&db,
|
||||
&w_id,
|
||||
resource,
|
||||
Some(&authed),
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
if let Some(credentials) = configured_credentials(&db, &w_id, &provider).await? {
|
||||
break 'cred credentials;
|
||||
}
|
||||
// Nothing configured: fall back to Windmill's free AI tier (EE-only) if a lent
|
||||
// key is set and both the user's one-time grant and the instance's daily cap have
|
||||
// room. Errors once the grant is spent, the day is capped, or the user already has
|
||||
// a request in flight; None otherwise. Ineligible identities (e.g. service
|
||||
// accounts) are refused inside the helper, so every path treats them alike.
|
||||
let free = crate::ai_free_tier_oss::resolve_free_tier_credentials(
|
||||
&provider,
|
||||
&db,
|
||||
&ai_path,
|
||||
&authed.email,
|
||||
&body,
|
||||
)
|
||||
.await?;
|
||||
if let Some((free_credentials, lease)) = free {
|
||||
free_lease = Some(lease);
|
||||
break 'cred free_credentials;
|
||||
}
|
||||
return Err(Error::internal_err(
|
||||
"AI resource not configured".to_string(),
|
||||
));
|
||||
};
|
||||
|
||||
#[cfg(feature = "bedrock")]
|
||||
|
||||
@@ -11,15 +11,12 @@ use windmill_ai::{
|
||||
ai_providers::AIProvider,
|
||||
credentials::ProviderCredentials,
|
||||
model_context::is_openai_reasoning_model,
|
||||
proxy::{common_outbound_headers, retain_effective_credentials},
|
||||
query_builder::{BuildRequestArgs, ParsedResponse, QueryBuilder},
|
||||
types::{ContentPart, OpenAIContent, OpenAIMessage, OutputType, TokenUsage},
|
||||
utils::pinned_ai_client_for,
|
||||
one_shot::{complete_text, OneShotRequest},
|
||||
query_builder::QueryBuilder,
|
||||
types::{ContentPart, OpenAIContent, OpenAIMessage, TokenUsage},
|
||||
};
|
||||
use windmill_common::{client::AuthedClient, error::Error};
|
||||
|
||||
use crate::ai::stream_event_processor::StreamEventProcessor;
|
||||
|
||||
const SUMMARY_OUTPUT_RESERVE_TOKENS: usize = 8000;
|
||||
const MIN_SUMMARY_RESERVE_TOKENS: usize = 2000;
|
||||
const MAX_CONSECUTIVE_COMPACTION_FAILURES: usize = 3;
|
||||
@@ -548,174 +545,65 @@ async fn summarize_prefix(
|
||||
) -> Result<(String, Option<TokenUsage>), Error> {
|
||||
let summary_messages = summary_messages(prefix);
|
||||
|
||||
let build_args = BuildRequestArgs {
|
||||
messages: &summary_messages,
|
||||
tools: None,
|
||||
model: request.model,
|
||||
// The step's temperature is not carried over: the summary imposes its own
|
||||
// thinking mode (below), and OpenAI's reasoning models reject `temperature`
|
||||
// alongside any effort but their own default. A structured extraction does not
|
||||
// need a set temperature, so the internal call omits it rather than track which
|
||||
// model forbids which pairing.
|
||||
temperature: None,
|
||||
// Thinking shares the output budget with the handoff; minimize it so the
|
||||
// summary can finish even when the step itself requests extensive reasoning.
|
||||
reasoning_effort: lowest_reasoning_effort(&request.credentials.provider, request.model),
|
||||
// Nothing streams this call's reasoning, and a summary of it would be billed.
|
||||
reasoning_summary: false,
|
||||
// Exactly the room the split set aside. Asking for more lets a summary land the
|
||||
// conversation back over the trigger; leaving it to the provider default gives
|
||||
// 64000 on Anthropic (over several Claude models' output ceiling) and the model's
|
||||
// own small default on Bedrock, short enough to truncate the response.
|
||||
max_tokens: Some(reserve_tokens as u32),
|
||||
output_schema: None,
|
||||
output_type: &OutputType::Text,
|
||||
system_prompt: None,
|
||||
user_message: "",
|
||||
attachments: None,
|
||||
has_websearch: false,
|
||||
// The summarization request carries no tools, so its prefix does not match the
|
||||
// agent's own requests: routing it onto the step's cache key would only evict.
|
||||
prompt_cache_key: None,
|
||||
};
|
||||
|
||||
let parsed = if request.credentials.provider == AIProvider::AWSBedrock {
|
||||
#[cfg(feature = "bedrock")]
|
||||
{
|
||||
// A resource that authenticates through an OIDC role carries no keys of its
|
||||
// own. Summaries are rare enough that assuming the role afresh here costs
|
||||
// less than threading the agent loop's cached credentials through.
|
||||
let mut assumed_role = None;
|
||||
let assumed = windmill_ai::ai_bedrock::refresh_bedrock_oidc_credentials(
|
||||
request.credentials,
|
||||
&mut assumed_role,
|
||||
request.client,
|
||||
request.job_id,
|
||||
)
|
||||
.await?;
|
||||
let (access_key_id, secret_access_key, session_token) = match assumed {
|
||||
Some(assumed) => (
|
||||
Some(assumed.access_key_id.as_str()),
|
||||
Some(assumed.secret_access_key.as_str()),
|
||||
Some(assumed.session_token.as_str()),
|
||||
),
|
||||
None => (
|
||||
request.credentials.aws_access_key_id.as_deref(),
|
||||
request.credentials.aws_secret_access_key.as_deref(),
|
||||
request.credentials.aws_session_token.as_deref(),
|
||||
),
|
||||
};
|
||||
windmill_ai::providers::bedrock::BedrockQueryBuilder::default()
|
||||
.execute_request(
|
||||
&summary_messages,
|
||||
None,
|
||||
request.model,
|
||||
// No temperature, as on the HTTP path above.
|
||||
None,
|
||||
build_args.reasoning_effort,
|
||||
build_args.max_tokens,
|
||||
request.credentials.api_key.as_deref().unwrap_or(""),
|
||||
request
|
||||
.credentials
|
||||
.region
|
||||
.as_deref()
|
||||
.unwrap_or(windmill_ai::ai_providers::USE_ENV_REGION),
|
||||
Some(StreamEventProcessor::new_silent().boxed_sink()),
|
||||
request.client,
|
||||
request.workspace_id,
|
||||
None,
|
||||
access_key_id,
|
||||
secret_access_key,
|
||||
session_token,
|
||||
)
|
||||
.await?
|
||||
}
|
||||
#[cfg(not(feature = "bedrock"))]
|
||||
{
|
||||
return Err(Error::internal_err(
|
||||
"AWS Bedrock support is not enabled. Build with 'bedrock' feature.".to_string(),
|
||||
));
|
||||
}
|
||||
} else {
|
||||
let base_url = &request.credentials.base_url;
|
||||
let api_key = request.credentials.api_key.as_deref().unwrap_or("");
|
||||
let body = if request.include_usage {
|
||||
request
|
||||
.query_builder
|
||||
.build_request(&build_args, request.client, request.workspace_id)
|
||||
.await?
|
||||
} else {
|
||||
request
|
||||
.query_builder
|
||||
.build_request_without_usage(&build_args, request.client, request.workspace_id)
|
||||
.await?
|
||||
};
|
||||
let endpoint =
|
||||
request
|
||||
.query_builder
|
||||
.get_endpoint(base_url, request.model, &OutputType::Text);
|
||||
// Summaries are rare enough that exchanging afresh costs less than threading
|
||||
// the agent loop's cached token through.
|
||||
let mut federated_token = None;
|
||||
let auth_headers = match windmill_ai::providers::anthropic::refresh_federated_access_token(
|
||||
// A resource that authenticates through an OIDC role carries no keys of its own.
|
||||
// Summaries are rare enough that assuming the role afresh here costs less than
|
||||
// threading the agent loop's cached credentials through.
|
||||
#[cfg(feature = "bedrock")]
|
||||
let assumed_credentials = {
|
||||
let mut assumed_role = None;
|
||||
match windmill_ai::ai_bedrock::refresh_bedrock_oidc_credentials(
|
||||
request.credentials,
|
||||
&mut federated_token,
|
||||
&mut assumed_role,
|
||||
request.client,
|
||||
request.job_id,
|
||||
)
|
||||
.await?
|
||||
{
|
||||
Some(token) => windmill_ai::providers::anthropic::federated_auth_headers(token),
|
||||
None => retain_effective_credentials(
|
||||
request.credentials,
|
||||
request
|
||||
.query_builder
|
||||
.get_auth_headers(api_key, base_url, &OutputType::Text),
|
||||
),
|
||||
};
|
||||
|
||||
// As in the agent loop, `endpoint` derives from the user-controlled provider
|
||||
// base_url, so DNS is pinned to the SSRF-validated address.
|
||||
let mut http_request = pinned_ai_client_for(base_url)
|
||||
.await?
|
||||
.post(&endpoint)
|
||||
.timeout(request.timeout)
|
||||
.header("Content-Type", "application/json");
|
||||
for (header_name, header_value) in &auth_headers {
|
||||
http_request = http_request.header(*header_name, header_value.clone());
|
||||
Some(assumed) => Some(ProviderCredentials {
|
||||
aws_access_key_id: Some(assumed.access_key_id.clone()),
|
||||
aws_secret_access_key: Some(assumed.secret_access_key.clone()),
|
||||
aws_session_token: Some(assumed.session_token.clone()),
|
||||
..request.credentials.clone()
|
||||
}),
|
||||
None => None,
|
||||
}
|
||||
for (header_name, header_value) in common_outbound_headers(request.credentials) {
|
||||
http_request = http_request.header(header_name.as_str(), header_value.as_str());
|
||||
}
|
||||
|
||||
let resp = http_request
|
||||
.body(body)
|
||||
.send()
|
||||
.await
|
||||
.map_err(|e| Error::internal_err(format!("Failed to call API: {e}")))?;
|
||||
|
||||
if let Err(e) = resp.error_for_status_ref() {
|
||||
let text = resp
|
||||
.text()
|
||||
.await
|
||||
.unwrap_or_else(|_| "<failed to read body>".to_string());
|
||||
return Err(Error::internal_err(format!(
|
||||
"API error calling {endpoint}: {e} - {text}"
|
||||
)));
|
||||
}
|
||||
|
||||
request
|
||||
.query_builder
|
||||
.parse_streaming_response(resp, StreamEventProcessor::new_silent().boxed_sink())
|
||||
.await?
|
||||
.without_nul()
|
||||
};
|
||||
#[cfg(not(feature = "bedrock"))]
|
||||
let assumed_credentials: Option<ProviderCredentials> = None;
|
||||
let credentials = assumed_credentials.as_ref().unwrap_or(request.credentials);
|
||||
|
||||
match parsed {
|
||||
ParsedResponse::Text { content, usage, .. } => Ok((content.unwrap_or_default(), usage)),
|
||||
ParsedResponse::Image { .. } => Err(Error::internal_err(
|
||||
"Compaction summary came back as an image".to_string(),
|
||||
)),
|
||||
}
|
||||
// Summaries are rare enough that exchanging afresh costs less than threading the
|
||||
// agent loop's cached token through.
|
||||
let mut federated_token = None;
|
||||
let federated_token = windmill_ai::providers::anthropic::refresh_federated_access_token(
|
||||
credentials,
|
||||
&mut federated_token,
|
||||
request.client,
|
||||
)
|
||||
.await?;
|
||||
|
||||
complete_text(
|
||||
&summary_messages,
|
||||
&OneShotRequest {
|
||||
query_builder: request.query_builder,
|
||||
credentials,
|
||||
model: request.model,
|
||||
// Exactly the room the split set aside. Asking for more lets a summary land the
|
||||
// conversation back over the trigger; leaving it to the provider default gives
|
||||
// 64000 on Anthropic (over several Claude models' output ceiling) and the model's
|
||||
// own small default on Bedrock, short enough to truncate the response.
|
||||
max_tokens: Some(reserve_tokens as u32),
|
||||
// Thinking shares the output budget with the handoff; minimize it so the
|
||||
// summary can finish even when the step itself requests extensive reasoning.
|
||||
reasoning_effort: lowest_reasoning_effort(&credentials.provider, request.model),
|
||||
timeout: request.timeout,
|
||||
client: request.client,
|
||||
workspace_id: request.workspace_id,
|
||||
federated_token,
|
||||
include_usage: request.include_usage,
|
||||
},
|
||||
)
|
||||
.await
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -0,0 +1,69 @@
|
||||
/**
|
||||
* Vendors the AI review core into the CLI, for `wmill review`.
|
||||
*
|
||||
* The prompt, the size budget and the reading of the model's answer are the
|
||||
* review: the deployment request panel and a pull request review must ask the
|
||||
* same thing and read the answer the same way, so both run one source,
|
||||
* frontend/src/lib/components/deploymentRequest/aiReviewCore.ts, plus the
|
||||
* folder instruction helpers it imports. The modules are concatenated as
|
||||
* TypeScript rather than bundled, so the CLI keeps their types.
|
||||
*
|
||||
* Run `bun run gen:ai-review` after touching either module;
|
||||
* test/ai_review_bundle_unit.test.ts fails when the committed copy no longer
|
||||
* matches them.
|
||||
*/
|
||||
import { readFileSync, writeFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
const REPO_ROOT = join(import.meta.dir, "..");
|
||||
const COMPONENTS = join(REPO_ROOT, "frontend", "src", "lib", "components");
|
||||
const SCOPES = join(COMPONENTS, "copilot", "chat", "folderInstructionScopes.ts");
|
||||
const CORE = join(COMPONENTS, "deploymentRequest", "aiReviewCore.ts");
|
||||
export const OUT_FILE = join(
|
||||
import.meta.dir,
|
||||
"src",
|
||||
"commands",
|
||||
"review",
|
||||
"aiReviewCore.gen.ts",
|
||||
);
|
||||
|
||||
const HEADER = `// Generated by cli/generate-ai-review.ts. Do not edit.
|
||||
// Run \`bun run gen:ai-review\` from cli/ to rebuild it from
|
||||
// frontend/src/lib/components/deploymentRequest/aiReviewCore.ts and
|
||||
// frontend/src/lib/components/copilot/chat/folderInstructionScopes.ts.
|
||||
`;
|
||||
|
||||
const SCOPES_IMPORT =
|
||||
/^(import|export) \{[^}]*\} from '\.\.\/copilot\/chat\/folderInstructionScopes'\n/gm;
|
||||
|
||||
/** The vendored module, header included. Exported so the staleness test can
|
||||
* build it and compare. Line endings are normalized: a CRLF checkout is the
|
||||
* same source. */
|
||||
export function buildAiReviewCore(): string {
|
||||
const read = (p: string) => readFileSync(p, "utf-8").replace(/\r\n/g, "\n");
|
||||
const scopes = read(SCOPES);
|
||||
const core = read(CORE).replace(SCOPES_IMPORT, "");
|
||||
// Anything else either module imports would not resolve in the CLI.
|
||||
for (const [file, source] of [[SCOPES, scopes], [CORE, core]]) {
|
||||
if (/^(import|export) .* from /m.test(source)) {
|
||||
throw new Error(
|
||||
`${file} imports another module. Keep the review core self-contained, ` +
|
||||
"or teach this generator to vendor what it imports.",
|
||||
);
|
||||
}
|
||||
}
|
||||
return `${HEADER}\n${scopes}\n${core}`;
|
||||
}
|
||||
|
||||
if (import.meta.main) {
|
||||
const ts = buildAiReviewCore();
|
||||
const before = (() => {
|
||||
try {
|
||||
return readFileSync(OUT_FILE, "utf-8");
|
||||
} catch {
|
||||
return "";
|
||||
}
|
||||
})();
|
||||
writeFileSync(OUT_FILE, ts, "utf-8");
|
||||
console.log(`${before === ts ? "Unchanged" : "Wrote"} ${OUT_FILE}`);
|
||||
}
|
||||
@@ -11,6 +11,7 @@
|
||||
"build": "./build.sh",
|
||||
"gen:dev-recorder": "bun run generate-dev-recorder.ts",
|
||||
"gen:app-policy": "bun run generate-app-policy.ts",
|
||||
"gen:ai-review": "bun run generate-ai-review.ts",
|
||||
"test": "bun test test/",
|
||||
"test:unit": "UNIT_ONLY=1 bun test test/*_unit*",
|
||||
"check": "bunx tsc --noEmit",
|
||||
|
||||
+259
@@ -0,0 +1,259 @@
|
||||
// Generated by cli/generate-ai-review.ts. Do not edit.
|
||||
// Run `bun run gen:ai-review` from cli/ to rebuild it from
|
||||
// frontend/src/lib/components/deploymentRequest/aiReviewCore.ts and
|
||||
// frontend/src/lib/components/copilot/chat/folderInstructionScopes.ts.
|
||||
|
||||
// No imports: the CLI's review bundle carries this module (see cli/generate-ai-review.ts).
|
||||
|
||||
/** Bytes of one instruction body a tool result may carry. */
|
||||
export const MAX_FOLDER_INSTRUCTIONS_LENGTH = 32 * 1024
|
||||
/** Bytes all the bodies one tool result carries may add up to, so nested folders
|
||||
* cannot stack several full-size bodies onto a single result. */
|
||||
export const MAX_FOLDER_INSTRUCTIONS_TOTAL = 48 * 1024
|
||||
|
||||
export type FolderInstruction = {
|
||||
/** The instruction resource. */
|
||||
path: string
|
||||
/** The directory it covers, with a trailing `/`: `f/billing/AGENTS` covers `f/billing/`. */
|
||||
scope: string
|
||||
}
|
||||
|
||||
export function folderInstructionScope(path: string): string {
|
||||
return path.slice(0, path.lastIndexOf('/') + 1)
|
||||
}
|
||||
|
||||
/** The instructions covering any of `paths`, outermost scope first, so a nested
|
||||
* folder's instructions come last and read as the more specific ones. A path naming
|
||||
* the folder itself (`f/billing`, as a pipeline's is) is covered too. */
|
||||
export function instructionsCovering(
|
||||
instructions: readonly FolderInstruction[],
|
||||
paths: readonly string[]
|
||||
): FolderInstruction[] {
|
||||
return instructions
|
||||
.filter((i) => paths.some((p) => `${p}/`.startsWith(i.scope)))
|
||||
.sort((a, b) => a.scope.length - b.scope.length || a.path.localeCompare(b.path))
|
||||
}
|
||||
|
||||
/** Cut `text` to `maxBytes` of UTF-8, marking the cut so a reader (the model
|
||||
* included) can tell truncation from a body that simply ends there.
|
||||
*
|
||||
* For the body, whose cap is a byte budget: 64k CJK characters are ~192 KiB, so a
|
||||
* code-unit cut would let three times the intended payload through. */
|
||||
export function truncateForPrompt(text: string, maxBytes: number): string {
|
||||
const encoded = new TextEncoder().encode(text)
|
||||
if (encoded.byteLength <= maxBytes) return text
|
||||
// `fatal: false` replaces the partial code point a byte-aligned cut can leave
|
||||
// with U+FFFD; dropping it keeps the tail clean.
|
||||
const cut = new TextDecoder('utf-8').decode(encoded.slice(0, maxBytes)).replace(/�$/, '')
|
||||
return `${cut}… [truncated]`
|
||||
}
|
||||
|
||||
export function folderInstructionsOpeningTag(path: string): string {
|
||||
return `<folder_instructions path="${path}"`
|
||||
}
|
||||
|
||||
export function formatFolderInstructions(
|
||||
blocks: readonly { instruction: FolderInstruction; body: string }[]
|
||||
): string {
|
||||
// The budget goes innermost first: the most specific instructions are the last to
|
||||
// be cut. An outer body the budget no longer reaches keeps its tag, so it still
|
||||
// counts as delivered, with the cut marked.
|
||||
let remaining = MAX_FOLDER_INSTRUCTIONS_TOTAL
|
||||
const bodies = [...blocks].reverse().map(({ body }) => {
|
||||
const cut = truncateForPrompt(body, Math.min(MAX_FOLDER_INSTRUCTIONS_LENGTH, remaining))
|
||||
remaining = Math.max(0, remaining - new TextEncoder().encode(cut).byteLength)
|
||||
return cut
|
||||
})
|
||||
bodies.reverse()
|
||||
return blocks
|
||||
.map(
|
||||
({ instruction }, i) =>
|
||||
`${folderInstructionsOpeningTag(instruction.path)} scope="${instruction.scope}">\n${bodies[i]}\n</folder_instructions>`
|
||||
)
|
||||
.join('\n\n')
|
||||
}
|
||||
|
||||
// What the review asks of the model and how its answer is read, for every place it runs: the
|
||||
// deployment request panel here, and `wmill review` on a git sync pull request, which carries
|
||||
// this module through cli/generate-ai-review.ts. Keep it free of `$lib` imports; the
|
||||
// staleness test in cli/test/ai_review_bundle_unit.test.ts fails when the CLI's copy drifts.
|
||||
|
||||
|
||||
/**
|
||||
* Review instructions are resources of this type: a markdown file resource whose
|
||||
* `value.content` applies to every changed item under the directory it lives in.
|
||||
*/
|
||||
export const REVIEW_INSTRUCTIONS_RESOURCE_TYPE = 'ai_review_instruction'
|
||||
|
||||
export type AiReviewSeverity = 'high' | 'medium' | 'low'
|
||||
|
||||
export type AiReviewFinding = {
|
||||
id: string
|
||||
/** Empty when the finding names no changed item. */
|
||||
kind: string
|
||||
path: string
|
||||
severity: AiReviewSeverity
|
||||
comment: string
|
||||
/** Pull request reviews only: the changed file and line the finding is anchored to. */
|
||||
file?: string
|
||||
line?: number
|
||||
side?: 'LEFT' | 'RIGHT'
|
||||
}
|
||||
|
||||
// A variable's value is read decrypted and a resource's value holds credentials, so neither may
|
||||
// reach the model: they are reviewed by name and change type only.
|
||||
export const VALUE_WITHHELD_KINDS = new Set(['variable', 'resource'])
|
||||
|
||||
export const PER_SIDE_CHAR_CAP = 20_000
|
||||
export const TOTAL_CHAR_BUDGET = 160_000
|
||||
// Under the Anthropic SDK's non-streaming pre-flight max_tokens limit.
|
||||
export const MAX_OUTPUT_TOKENS = 8_000
|
||||
|
||||
/** What is being reviewed: a fork's deployment request, or a pull request on a git sync repo. */
|
||||
export type ReviewSubject = 'deployment_request' | 'pull_request'
|
||||
|
||||
const INTRO: Record<ReviewSubject, string> = {
|
||||
deployment_request:
|
||||
'You review a Windmill deployment request: a set of changes made in a fork workspace that will be merged into its parent workspace. Each changed item is given with its parent version (what is deployed now) and its fork version (what will be deployed).',
|
||||
pull_request:
|
||||
"You review a pull request on a git repository that Windmill's git sync keeps in step with a workspace: merging it deploys its changes. Each changed item is given with the files that make it up in the repository, each as a unified diff of the base branch (what is deployed now) against the pull request (what will be deployed). Every diff line starts with its line number: R<n> for line n of the pull request's version of the file, L<n> for line n of the base version, then + for an added line, - for a removed one and a space for unchanged context."
|
||||
}
|
||||
|
||||
const ANSWER_SHAPE: Record<ReviewSubject, string> = {
|
||||
deployment_request:
|
||||
'{"summary": "<one or two sentences on what the change does and its overall risk>", "findings": [{"kind": "<item kind as given>", "path": "<item path as given>", "severity": "high" | "medium" | "low", "comment": "<the issue and what to do about it, under 80 words>"}]}',
|
||||
pull_request:
|
||||
'{"summary": "<one or two sentences on what the change does and its overall risk>", "findings": [{"kind": "<item kind as given>", "path": "<item path as given>", "file": "<the file the issue is in, as given>", "line": "<the R or L line number the issue is on, as given, such as R12, or null for the file as a whole>", "severity": "high" | "medium" | "low", "comment": "<the issue and what to do about it, under 80 words>"}]}'
|
||||
}
|
||||
|
||||
const SOURCE: Record<ReviewSubject, string> = {
|
||||
deployment_request: 'The parent workspace',
|
||||
pull_request: "The repository's base branch"
|
||||
}
|
||||
|
||||
export function reviewSystemPrompt(subject: ReviewSubject): string {
|
||||
return `${INTRO[subject]} An item can be a script, flow, app, schedule, trigger, folder, resource type, datatable migration, variable or resource.
|
||||
|
||||
Report only issues a reviewer should act on before approving the merge:
|
||||
- bugs and logic errors introduced by the change
|
||||
- breaking changes: removed or renamed inputs, changed input types, changed outputs that callers may rely on
|
||||
- risky operational changes: schedules retimed, triggers pointed at a different script or flow, folder owners or permissions, app policies, destructive migrations
|
||||
- hardcoded secrets or credentials
|
||||
- deletions that look unintended
|
||||
|
||||
Do not comment on style, naming, formatting or lockfiles, and do not praise. Do not repeat a finding for every item it touches. An item with no issue gets no finding.
|
||||
|
||||
Answer with JSON only, no prose around it, in this shape:
|
||||
${ANSWER_SHAPE[subject]}`
|
||||
}
|
||||
|
||||
/** The system prompt addition carrying the review instructions that apply, or '' for none. */
|
||||
export function reviewInstructionsSection(
|
||||
subject: ReviewSubject,
|
||||
blocks: readonly { instruction: FolderInstruction; body: string }[]
|
||||
): string {
|
||||
if (blocks.length === 0) return ''
|
||||
// Hardcoded secrets and unintended deletions stay a floor: a folder's writers steer the
|
||||
// review of their own changes, so they must not be able to switch those checks off.
|
||||
return `\n\n${SOURCE[subject]} gives the review instructions below, each for the changed items under the folder in its scope. For those items they take precedence over the instructions above, and a nested folder's over its parent folder's. They may ask you to check more, check less or weigh severity differently, except that hardcoded secrets or credentials and deletions that look unintended are always reported. The answer is still the JSON above.\n\n${formatFolderInstructions(blocks)}`
|
||||
}
|
||||
|
||||
/** One side of a changed item as the model reads it, lockfiles dropped and capped. */
|
||||
export function renderItemValue(value: unknown): string {
|
||||
if (value == null) return '(none)'
|
||||
// Flows and apps nest their inline scripts' lockfiles, so they are dropped at any depth.
|
||||
const text =
|
||||
typeof value === 'string'
|
||||
? value
|
||||
: JSON.stringify(value, (key, v) => (key === 'lock' ? undefined : v), 2)
|
||||
return capText(text, PER_SIDE_CHAR_CAP)
|
||||
}
|
||||
|
||||
export function capText(text: string, cap: number): string {
|
||||
return text.length > cap
|
||||
? `${text.slice(0, cap)}\n… (truncated, ${text.length - cap} more characters)`
|
||||
: text
|
||||
}
|
||||
|
||||
/**
|
||||
* Packs item sections into the prompt in order. The first section that does not fit spends
|
||||
* the budget, and every item after it is listed by name only: `used` alone could stay under
|
||||
* the cap forever, and a caller that reads items lazily stops reading once `spent` is set.
|
||||
*/
|
||||
export class ReviewBudget {
|
||||
included: string[] = []
|
||||
/** `kind:path` of the changed items reviewed by name only. */
|
||||
omitted: string[] = []
|
||||
spent = false
|
||||
private used = 0
|
||||
|
||||
add(key: string, section: string): void {
|
||||
if (!this.spent && this.used + section.length <= TOTAL_CHAR_BUDGET) {
|
||||
this.included.push(section)
|
||||
this.used += section.length
|
||||
} else {
|
||||
this.spent = true
|
||||
this.omitted.push(key)
|
||||
}
|
||||
}
|
||||
|
||||
userPrompt(header: string): string {
|
||||
let user = `${header}\n\n${this.included.join('\n\n')}`
|
||||
if (this.omitted.length > 0) {
|
||||
user += `\n\nAlso changed, contents not shown for size:\n${this.omitted.map((o) => `- ${o}`).join('\n')}`
|
||||
}
|
||||
return user
|
||||
}
|
||||
}
|
||||
|
||||
/** What a finding may be anchored to. */
|
||||
export type ReviewAnchors = {
|
||||
/** `kind:path` of every changed item. */
|
||||
items: ReadonlySet<string>
|
||||
/** Pull request reviews: for each changed file, the line numbers a comment can be placed on. */
|
||||
lines?: ReadonlyMap<string, { LEFT: ReadonlySet<number>; RIGHT: ReadonlySet<number> }>
|
||||
}
|
||||
|
||||
export function parseReviewAnswer(
|
||||
text: string,
|
||||
anchors: ReviewAnchors
|
||||
): { summary: string; findings: AiReviewFinding[] } {
|
||||
const start = text.indexOf('{')
|
||||
const end = text.lastIndexOf('}')
|
||||
if (start === -1 || end <= start) {
|
||||
throw new Error('The AI answer was not in the expected format')
|
||||
}
|
||||
const parsed = JSON.parse(text.slice(start, end + 1)) as {
|
||||
summary?: unknown
|
||||
findings?: unknown
|
||||
}
|
||||
// Anchoring a comment needs an item, file and line the change actually has, so a finding
|
||||
// naming anything else keeps its comment but loses the anchor rather than being posted
|
||||
// against a phantom.
|
||||
const findings: AiReviewFinding[] = []
|
||||
for (const [i, raw] of (Array.isArray(parsed.findings) ? parsed.findings : []).entries()) {
|
||||
if (!raw || typeof raw.comment !== 'string' || !raw.comment.trim()) continue
|
||||
const anchored = anchors.items.has(`${raw.kind}:${raw.path}`)
|
||||
const finding: AiReviewFinding = {
|
||||
id: String(i),
|
||||
kind: anchored ? raw.kind : '',
|
||||
path: anchored ? raw.path : '',
|
||||
severity: ['high', 'medium', 'low'].includes(raw.severity) ? raw.severity : 'medium',
|
||||
comment: raw.comment.trim()
|
||||
}
|
||||
const fileLines = typeof raw.file === 'string' ? anchors.lines?.get(raw.file) : undefined
|
||||
if (fileLines) {
|
||||
finding.file = raw.file
|
||||
const line = /^([LR])(\d+)$/.exec(String(raw.line ?? '').trim())
|
||||
const side = line?.[1] === 'L' ? 'LEFT' : 'RIGHT'
|
||||
if (line && fileLines[side].has(Number(line[2]))) {
|
||||
finding.side = side
|
||||
finding.line = Number(line[2])
|
||||
}
|
||||
}
|
||||
findings.push(finding)
|
||||
}
|
||||
const rank: Record<AiReviewSeverity, number> = { high: 0, medium: 1, low: 2 }
|
||||
findings.sort((a, b) => rank[a.severity] - rank[b.severity])
|
||||
return { summary: typeof parsed.summary === 'string' ? parsed.summary : '', findings }
|
||||
}
|
||||
@@ -0,0 +1,601 @@
|
||||
import { Command } from "@cliffy/command";
|
||||
import { colors } from "@cliffy/ansi/colors";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { posix } from "node:path";
|
||||
|
||||
import * as log from "../../core/log.ts";
|
||||
import { requireLogin } from "../../core/auth.ts";
|
||||
import { resolveWorkspace } from "../../core/context.ts";
|
||||
import * as wmill from "../../../gen/services.gen.ts";
|
||||
import { GlobalOptions, getTypeStrFromPath, parseDatatableMigrationPath } from "../../types.ts";
|
||||
import {
|
||||
DBT_MODULE_SUFFIX,
|
||||
DOTTED_SUFFIXES,
|
||||
NON_DOTTED_SUFFIXES,
|
||||
} from "../../utils/resource_folders.ts";
|
||||
import { apiErrorMessage } from "../../utils/utils.ts";
|
||||
import { yamlParseContent } from "../../utils/yaml.ts";
|
||||
import {
|
||||
capText,
|
||||
folderInstructionScope,
|
||||
instructionsCovering,
|
||||
MAX_OUTPUT_TOKENS,
|
||||
parseReviewAnswer,
|
||||
PER_SIDE_CHAR_CAP,
|
||||
REVIEW_INSTRUCTIONS_RESOURCE_TYPE,
|
||||
ReviewBudget,
|
||||
reviewInstructionsSection,
|
||||
reviewSystemPrompt,
|
||||
VALUE_WITHHELD_KINDS,
|
||||
type AiReviewFinding,
|
||||
type AiReviewSeverity,
|
||||
type FolderInstruction,
|
||||
} from "./aiReviewCore.gen.ts";
|
||||
|
||||
/** Context GitHub's diff gives a pull request, and so the lines a review comment can sit on. */
|
||||
const GITHUB_DIFF_CONTEXT = 3;
|
||||
/** Context the model reads around each change. */
|
||||
const MODEL_DIFF_CONTEXT = 20;
|
||||
|
||||
type ReviewOptions = GlobalOptions & {
|
||||
base?: string;
|
||||
head?: string;
|
||||
model?: string;
|
||||
json?: boolean;
|
||||
post?: "github";
|
||||
pr?: number;
|
||||
failOn?: AiReviewSeverity;
|
||||
folder?: string;
|
||||
};
|
||||
|
||||
type ChangedFile = { file: string; status: "added" | "deleted" | "modified" };
|
||||
type ChangedItem = { kind: string; path: string; files: ChangedFile[] };
|
||||
|
||||
export type ReviewResult = {
|
||||
summary: string;
|
||||
findings: AiReviewFinding[];
|
||||
/** Changed items reviewed by name only, over the size budget. */
|
||||
omitted: string[];
|
||||
/** Changed items whose files could not be read as text, so the model did not review them. */
|
||||
unreadable: string[];
|
||||
/** Review instructions of the base branch that covered a changed item. */
|
||||
instructions: string[];
|
||||
/** Review instructions on the base branch whose body could not be read. */
|
||||
skippedInstructions: string[];
|
||||
model: string;
|
||||
};
|
||||
|
||||
function git(args: string[]): string {
|
||||
return execFileSync("git", ["-c", "core.quotePath=false", ...args], {
|
||||
encoding: "utf-8",
|
||||
maxBuffer: 512 * 1024 * 1024,
|
||||
stdio: ["ignore", "pipe", "pipe"],
|
||||
});
|
||||
}
|
||||
|
||||
/** The workspace item a repository file belongs to, as git sync lays a workspace out. Any
|
||||
* other file is its own item of kind `file`. */
|
||||
export function itemOf(file: string): { kind: string; path: string } {
|
||||
// The outermost container owns everything under it: a fileset's or a dbt project's
|
||||
// children are named freely, so a child called `x.flow/` or `legacy__mod/` is still
|
||||
// part of the resource or script around it.
|
||||
const containers: [string, string][] = [
|
||||
[".fileset/", "resource"],
|
||||
["__mod/", "script"],
|
||||
[`${DBT_MODULE_SUFFIX}/`, "script"],
|
||||
...[DOTTED_SUFFIXES, NON_DOTTED_SUFFIXES].flatMap((suffixes) =>
|
||||
Object.entries(suffixes).map(([kind, suffix]): [string, string] => [`${suffix}/`, kind])
|
||||
),
|
||||
];
|
||||
let outermost: { kind: string; path: string } | undefined;
|
||||
for (const [marker, kind] of containers) {
|
||||
const i = file.indexOf(marker);
|
||||
if (i > 0 && (!outermost || i < outermost.path.length)) {
|
||||
outermost = { kind, path: file.slice(0, i) };
|
||||
}
|
||||
}
|
||||
if (outermost) return outermost;
|
||||
if (parseDatatableMigrationPath(file)) return { kind: "datatable_migration", path: file };
|
||||
const sidecar = file.indexOf(".resource.file.");
|
||||
if (sidecar > 0) return { kind: "resource", path: file.slice(0, sidecar) };
|
||||
let kind: string;
|
||||
try {
|
||||
kind = getTypeStrFromPath(file);
|
||||
} catch {
|
||||
return { kind: "file", path: file };
|
||||
}
|
||||
if (kind === "folder") return { kind, path: posix.dirname(file) };
|
||||
const dir = posix.dirname(file);
|
||||
const name = posix.basename(file).split(".")[0];
|
||||
if (!/^[ufg]\//.test(file) || !name) return { kind, path: file };
|
||||
return { kind, path: dir === "." ? name : `${dir}/${name}` };
|
||||
}
|
||||
|
||||
/** Lockfiles are left out of the review, as the deployment request review drops `lock`. */
|
||||
function isLockfile(file: string): boolean {
|
||||
return posix.basename(file) === "wmill-lock.yaml" || file.endsWith(".lock");
|
||||
}
|
||||
|
||||
/**
|
||||
* A unified diff with every line prefixed by its line number, R<n> on the pull request's
|
||||
* side and L<n> on the base's, and the numbers each side shows. `undefined` for a file git
|
||||
* reports as binary.
|
||||
*/
|
||||
export function numberDiff(
|
||||
diff: string,
|
||||
): { text: string; left: Set<number>; right: Set<number> } | undefined {
|
||||
if (/^Binary files .* differ$/m.test(diff)) return undefined;
|
||||
const out: string[] = [];
|
||||
const left = new Set<number>();
|
||||
const right = new Set<number>();
|
||||
let l = 0;
|
||||
let r = 0;
|
||||
let inHunk = false;
|
||||
for (const line of diff.split("\n")) {
|
||||
const hunk = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line);
|
||||
if (hunk) {
|
||||
l = Number(hunk[1]);
|
||||
r = Number(hunk[2]);
|
||||
inHunk = true;
|
||||
out.push(line);
|
||||
continue;
|
||||
}
|
||||
if (!inHunk || line.startsWith("\\")) continue;
|
||||
const mark = line[0];
|
||||
const content = line.slice(1);
|
||||
if (mark === "+") {
|
||||
right.add(r);
|
||||
out.push(`R${r++} +${content}`);
|
||||
} else if (mark === "-") {
|
||||
left.add(l);
|
||||
out.push(`L${l++} -${content}`);
|
||||
} else if (mark === " ") {
|
||||
right.add(r);
|
||||
out.push(`R${r++} ${content}`);
|
||||
l++;
|
||||
}
|
||||
}
|
||||
return { text: out.join("\n"), left, right };
|
||||
}
|
||||
|
||||
/** `root` is the directory the workspace is synced into, as a prefix ('' or `dir/`). Item
|
||||
* paths are read relative to it; file paths stay repository-relative, as the host's are. */
|
||||
function changedItems(mergeBase: string, head: string, root: string): ChangedItem[] {
|
||||
const out = git(["diff", "--name-status", "--no-renames", "-z", mergeBase, head]);
|
||||
const fields = out.split("\0").filter((f) => f !== "");
|
||||
const items = new Map<string, ChangedItem>();
|
||||
for (let i = 0; i + 1 < fields.length; i += 2) {
|
||||
const [code, file] = [fields[i], fields[i + 1]];
|
||||
if (isLockfile(file) || !file.startsWith(root)) continue;
|
||||
const { kind, path } = itemOf(file.slice(root.length));
|
||||
const key = `${kind}:${path}`;
|
||||
const item = items.get(key) ?? { kind, path, files: [] };
|
||||
item.files.push({
|
||||
file,
|
||||
status: code === "A" ? "added" : code === "D" ? "deleted" : "modified",
|
||||
});
|
||||
items.set(key, item);
|
||||
}
|
||||
return [...items.values()];
|
||||
}
|
||||
|
||||
function changeOf(item: ChangedItem): string {
|
||||
if (item.files.every((f) => f.status === "added")) return "added";
|
||||
if (item.files.every((f) => f.status === "deleted")) return "deleted";
|
||||
return "modified";
|
||||
}
|
||||
|
||||
/** Review instructions are read from the base branch, never the pull request: its author
|
||||
* controls the head, and a change to an instruction is itself one of the changes reviewed. */
|
||||
function baseInstructions(
|
||||
baseSha: string,
|
||||
itemPaths: string[],
|
||||
root: string,
|
||||
): { blocks: { instruction: FolderInstruction; body: string }[]; skipped: string[] } {
|
||||
let listing = "";
|
||||
try {
|
||||
listing = git([
|
||||
"grep",
|
||||
"-l",
|
||||
"-E",
|
||||
`resource_type['"]?:[[:space:]]*['"]?${REVIEW_INSTRUCTIONS_RESOURCE_TYPE}`,
|
||||
baseSha,
|
||||
"--",
|
||||
`${root}*.resource.yaml`,
|
||||
`${root}*.resource.json`,
|
||||
]);
|
||||
} catch (e) {
|
||||
// `git grep` exits 1 when nothing matches.
|
||||
if ((e as { status?: number }).status !== 1) throw e;
|
||||
}
|
||||
const listed = listing
|
||||
.split("\n")
|
||||
.filter(Boolean)
|
||||
.map((line) => line.slice(baseSha.length + 1))
|
||||
.map((file) => {
|
||||
const path = file.slice(root.length).replace(/\.resource\.(yaml|json)$/, "");
|
||||
return { file, instruction: { path, scope: folderInstructionScope(path) } };
|
||||
});
|
||||
const fileOf = new Map(listed.map((l) => [l.instruction.path, l.file]));
|
||||
const blocks: { instruction: FolderInstruction; body: string }[] = [];
|
||||
const skipped: string[] = [];
|
||||
for (const instruction of instructionsCovering(listed.map((l) => l.instruction), itemPaths)) {
|
||||
const file = fileOf.get(instruction.path)!;
|
||||
try {
|
||||
const raw = git(["show", `${baseSha}:${file}`]);
|
||||
const parsed = (file.endsWith(".json") ? JSON.parse(raw) : yamlParseContent(file, raw)) as {
|
||||
resource_type?: unknown;
|
||||
value?: { content?: unknown };
|
||||
};
|
||||
const content = parsed.value?.content;
|
||||
if (parsed.resource_type !== REVIEW_INSTRUCTIONS_RESOURCE_TYPE) continue;
|
||||
if (typeof content !== "string") throw new Error("no string content");
|
||||
const inline = /^!inline (.+)$/.exec(content);
|
||||
blocks.push({
|
||||
instruction,
|
||||
body: inline ? git(["show", `${baseSha}:${root}${inline[1].trim()}`]) : content,
|
||||
});
|
||||
} catch (e) {
|
||||
log.warn(`Could not read review instructions ${instruction.path}: ${e}`);
|
||||
skipped.push(instruction.path);
|
||||
}
|
||||
}
|
||||
return { blocks, skipped };
|
||||
}
|
||||
|
||||
/** A synced workspace encryption key is plaintext, so it is withheld like a secret value. */
|
||||
const WITHHELD_KINDS = new Set([...VALUE_WITHHELD_KINDS, "encryption_key"]);
|
||||
|
||||
/** Instruction resources hold prose rather than credentials, so a change to one is shown to
|
||||
* the model like any other: a pull request rewriting the review's own instructions is
|
||||
* something the reviewer should hear about. */
|
||||
const INSTRUCTION_RESOURCE_TYPES = new Set([REVIEW_INSTRUCTIONS_RESOURCE_TYPE, "ai_instruction"]);
|
||||
|
||||
/** Whether `item` is an instruction resource on every revision that defines it. Each side is
|
||||
* judged on its own: a resource retyped on either side holds credentials on the other. Any
|
||||
* changed file beside the resource's own definition and body (a workspace-specific variant, a
|
||||
* fileset child) may hold credentials of its own, so it withholds the whole item. */
|
||||
function isInstructionResource(item: ChangedItem, revs: string[], root: string): boolean {
|
||||
if (item.kind !== "resource") return false;
|
||||
const definition = `${root}${item.path}.resource.`;
|
||||
const ownFile = (file: string) => {
|
||||
if (!file.startsWith(definition)) return false;
|
||||
const rest = file.slice(definition.length);
|
||||
return rest === "yaml" || rest === "json" || /^file\.[^./]+$/.test(rest);
|
||||
};
|
||||
if (!item.files.every((f) => ownFile(f.file))) return false;
|
||||
let defined = false;
|
||||
for (const rev of revs) {
|
||||
for (const file of [`${definition}yaml`, `${definition}json`]) {
|
||||
let raw: string;
|
||||
try {
|
||||
raw = git(["show", `${rev}:${file}`]);
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
try {
|
||||
const parsed = (file.endsWith(".json") ? JSON.parse(raw) : yamlParseContent(file, raw)) as {
|
||||
resource_type?: unknown;
|
||||
};
|
||||
if (!INSTRUCTION_RESOURCE_TYPES.has(parsed.resource_type as string)) return false;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
defined = true;
|
||||
}
|
||||
}
|
||||
return defined;
|
||||
}
|
||||
|
||||
function parseModel(model: string | undefined) {
|
||||
if (!model) return undefined;
|
||||
const i = model.indexOf(":");
|
||||
if (i <= 0) throw new Error(`--model takes provider:model, got ${model}`);
|
||||
return { provider: model.slice(0, i), model: model.slice(i + 1) } as any;
|
||||
}
|
||||
|
||||
/** What the model is asked, and what its answer may anchor to, for the changes `headSha`
|
||||
* makes on top of its merge base with `baseSha`. Reads the repository in the working
|
||||
* directory. */
|
||||
export function buildReviewPrompt(baseSha: string, headSha: string, folder = "") {
|
||||
const trimmed = folder.replace(/^\/+|\/+$/g, "");
|
||||
const root = trimmed ? `${trimmed}/` : "";
|
||||
let mergeBase: string;
|
||||
try {
|
||||
mergeBase = git(["merge-base", baseSha, headSha]).trim();
|
||||
} catch {
|
||||
throw new Error(
|
||||
`No common ancestor between ${baseSha} and ${headSha}. Fetch the full history (actions/checkout with fetch-depth: 0).`,
|
||||
);
|
||||
}
|
||||
const items = changedItems(mergeBase, headSha, root);
|
||||
const { blocks, skipped } = baseInstructions(
|
||||
baseSha,
|
||||
items.map((i) => i.path),
|
||||
root,
|
||||
);
|
||||
|
||||
const budget = new ReviewBudget();
|
||||
const unreadable: string[] = [];
|
||||
const lines = new Map<string, { LEFT: Set<number>; RIGHT: Set<number> }>();
|
||||
for (const item of items) {
|
||||
const key = `${item.kind}:${item.path}`;
|
||||
if (budget.spent) {
|
||||
budget.omitted.push(key);
|
||||
continue;
|
||||
}
|
||||
const header = `### ${item.kind} ${item.path} (${changeOf(item)})`;
|
||||
if (WITHHELD_KINDS.has(item.kind) && !isInstructionResource(item, [mergeBase, headSha], root)) {
|
||||
budget.add(key, `${header}\nValue withheld from review.`);
|
||||
continue;
|
||||
}
|
||||
const files: string[] = [];
|
||||
let failed = false;
|
||||
for (const { file, status } of item.files) {
|
||||
const shown = numberDiff(
|
||||
git(["diff", "--no-color", "--no-ext-diff", `-U${MODEL_DIFF_CONTEXT}`, mergeBase, headSha, "--", `:(literal)${file}`]),
|
||||
);
|
||||
const commentable = numberDiff(
|
||||
git(["diff", "--no-color", "--no-ext-diff", `-U${GITHUB_DIFF_CONTEXT}`, mergeBase, headSha, "--", `:(literal)${file}`]),
|
||||
);
|
||||
if (!shown || !commentable) {
|
||||
failed = true;
|
||||
files.push(`#### ${file} (${status})\nContents could not be read.`);
|
||||
continue;
|
||||
}
|
||||
lines.set(file, { LEFT: commentable.left, RIGHT: commentable.right });
|
||||
files.push(`#### ${file} (${status})\n${capText(shown.text, 2 * PER_SIDE_CHAR_CAP)}`);
|
||||
}
|
||||
if (failed) unreadable.push(key);
|
||||
budget.add(key, `${header}\n${files.join("\n")}`);
|
||||
}
|
||||
|
||||
return {
|
||||
system: reviewSystemPrompt("pull_request") + reviewInstructionsSection("pull_request", blocks),
|
||||
user: budget.userPrompt(`Base: ${baseSha}\nPull request head: ${headSha}`),
|
||||
anchors: { items: new Set(items.map((i) => `${i.kind}:${i.path}`)), lines },
|
||||
omitted: budget.omitted,
|
||||
unreadable,
|
||||
instructions: blocks.map((b) => b.instruction.path),
|
||||
skippedInstructions: skipped,
|
||||
};
|
||||
}
|
||||
|
||||
export async function reviewChanges(
|
||||
workspaceId: string,
|
||||
model: string | undefined,
|
||||
baseSha: string,
|
||||
headSha: string,
|
||||
folder?: string,
|
||||
): Promise<ReviewResult> {
|
||||
const { system, user, anchors, ...reported } = buildReviewPrompt(baseSha, headSha, folder);
|
||||
const response = await wmill.completeAi({
|
||||
workspace: workspaceId,
|
||||
requestBody: { system, user, model: parseModel(model), max_tokens: MAX_OUTPUT_TOKENS },
|
||||
});
|
||||
return {
|
||||
...parseReviewAnswer(response.text, anchors),
|
||||
...reported,
|
||||
model: `${response.model.provider}:${response.model.model}`,
|
||||
};
|
||||
}
|
||||
|
||||
const SEVERITY_RANK: Record<AiReviewSeverity, number> = { high: 0, medium: 1, low: 2 };
|
||||
|
||||
function findingLine(f: AiReviewFinding): string {
|
||||
const where = [f.path && `\`${f.path}\``, f.file && f.file !== f.path && `\`${f.file}\``]
|
||||
.filter(Boolean)
|
||||
.join(" ");
|
||||
return `- **${f.severity}**${where ? ` ${where}` : ""}: ${f.comment}`;
|
||||
}
|
||||
|
||||
/** The review body: everything a line comment does not carry. */
|
||||
export function reviewBody(
|
||||
result: ReviewResult,
|
||||
marker: string,
|
||||
inBody: AiReviewFinding[],
|
||||
): string {
|
||||
const sections = [marker, "### Windmill AI review", result.summary || "No summary."];
|
||||
if (result.findings.length === 0) sections.push("No issues found.");
|
||||
if (inBody.length > 0) sections.push(inBody.map(findingLine).join("\n"));
|
||||
const list = (title: string, entries: string[]) =>
|
||||
entries.length > 0 && sections.push(`${title}\n${entries.map((e) => `- \`${e}\``).join("\n")}`);
|
||||
list("**Not reviewed**, the files could not be read as text:", result.unreadable);
|
||||
list("**Reviewed by name only**, over the size budget:", result.omitted);
|
||||
list("Review instructions applied, from the base branch:", result.instructions);
|
||||
list("**Review instructions skipped**, their body could not be read:", result.skippedInstructions);
|
||||
sections.push(`<sub>${result.model}</sub>`);
|
||||
return sections.join("\n\n");
|
||||
}
|
||||
|
||||
type GitHubContext = { api: string; repo: string; token: string; pr: number };
|
||||
|
||||
async function github(ctx: GitHubContext, method: string, path: string, body?: unknown) {
|
||||
const res = await fetch(`${ctx.api}/repos/${ctx.repo}${path}`, {
|
||||
method,
|
||||
headers: {
|
||||
Authorization: `Bearer ${ctx.token}`,
|
||||
Accept: "application/vnd.github+json",
|
||||
"X-GitHub-Api-Version": "2022-11-28",
|
||||
...(body ? { "Content-Type": "application/json" } : {}),
|
||||
},
|
||||
body: body ? JSON.stringify(body) : undefined,
|
||||
});
|
||||
if (!res.ok) {
|
||||
const err = new Error(`GitHub ${method} ${path}: ${res.status} ${await res.text()}`);
|
||||
(err as { status?: number }).status = res.status;
|
||||
throw err;
|
||||
}
|
||||
return res.json();
|
||||
}
|
||||
|
||||
async function alreadyReviewed(ctx: GitHubContext, marker: string): Promise<boolean> {
|
||||
for (let page = 1; page <= 20; page++) {
|
||||
const reviews = (await github(
|
||||
ctx,
|
||||
"GET",
|
||||
`/pulls/${ctx.pr}/reviews?per_page=100&page=${page}`,
|
||||
)) as { body?: string }[];
|
||||
if (reviews.some((r) => r.body?.includes(marker))) return true;
|
||||
if (reviews.length < 100) return false;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* What a pull request gets: a review whose line comments carry the anchored findings, and the
|
||||
* body to post instead if the host refuses them. Printed with `--json`, so a server posting the
|
||||
* review through its own credential sends exactly what `--post github` would.
|
||||
*/
|
||||
export function reviewPayload(result: ReviewResult, headSha: string) {
|
||||
const marker = `<!-- windmill-ai-review:${headSha} -->`;
|
||||
const anchored = result.findings.filter((f) => f.file && f.line);
|
||||
return {
|
||||
/** One review per head commit: a review whose body carries this one is already posted. */
|
||||
marker,
|
||||
body: reviewBody(result, marker, result.findings.filter((f) => !(f.file && f.line))),
|
||||
comments: anchored.map((f) => ({
|
||||
path: f.file!,
|
||||
line: f.line!,
|
||||
side: f.side!,
|
||||
body: `**${f.severity}**: ${f.comment}`,
|
||||
})),
|
||||
/** Every finding in the body, for a host that refuses a comment or has no line comments. */
|
||||
fallback_body: reviewBody(result, marker, result.findings),
|
||||
};
|
||||
}
|
||||
|
||||
async function postReview(ctx: GitHubContext, headSha: string, result: ReviewResult) {
|
||||
const { marker, body, comments, fallback_body } = reviewPayload(result, headSha);
|
||||
if (await alreadyReviewed(ctx, marker)) {
|
||||
log.info(`This commit already has a Windmill AI review, not posting another.`);
|
||||
return;
|
||||
}
|
||||
try {
|
||||
await github(ctx, "POST", `/pulls/${ctx.pr}/reviews`, {
|
||||
commit_id: headSha,
|
||||
event: "COMMENT",
|
||||
body,
|
||||
comments,
|
||||
});
|
||||
} catch (e) {
|
||||
// A comment GitHub cannot place fails the whole review, so the findings go in its body.
|
||||
if ((e as { status?: number }).status !== 422 || comments.length === 0) throw e;
|
||||
log.warn(`GitHub refused the line comments, posting them in the review body: ${e}`);
|
||||
await github(ctx, "POST", `/pulls/${ctx.pr}/reviews`, {
|
||||
commit_id: headSha,
|
||||
event: "COMMENT",
|
||||
body: fallback_body,
|
||||
});
|
||||
}
|
||||
log.info(colors.green(`Posted the review on pull request #${ctx.pr}.`));
|
||||
}
|
||||
|
||||
function githubEvent(): any {
|
||||
const path = process.env.GITHUB_EVENT_PATH;
|
||||
if (!path) return undefined;
|
||||
try {
|
||||
return JSON.parse(readFileSync(path, "utf-8"));
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
function githubContext(opts: ReviewOptions, event: any): GitHubContext {
|
||||
const token = process.env.GITHUB_TOKEN;
|
||||
const repo = process.env.GITHUB_REPOSITORY;
|
||||
const pr = opts.pr ?? event?.pull_request?.number;
|
||||
if (!token || !repo || !pr) {
|
||||
throw new Error(
|
||||
"--post github needs GITHUB_TOKEN and GITHUB_REPOSITORY in the environment, and the pull request number (--pr, or a pull_request event)",
|
||||
);
|
||||
}
|
||||
return { api: process.env.GITHUB_API_URL ?? "https://api.github.com", repo, token, pr };
|
||||
}
|
||||
|
||||
function printResult(result: ReviewResult) {
|
||||
log.info(colors.bold(result.summary || "No summary."));
|
||||
for (const f of result.findings) {
|
||||
const where = [f.path, f.file && f.line ? `${f.file}:${f.side === "LEFT" ? "L" : "R"}${f.line}` : f.file]
|
||||
.filter(Boolean)
|
||||
.join(" ");
|
||||
console.log(`${f.severity.toUpperCase()} ${where}\n ${f.comment}`);
|
||||
}
|
||||
if (result.findings.length === 0) log.info("No issues found.");
|
||||
if (result.unreadable.length) log.warn(`Not reviewed, unreadable: ${result.unreadable.join(", ")}`);
|
||||
if (result.omitted.length) log.warn(`Reviewed by name only, over budget: ${result.omitted.join(", ")}`);
|
||||
if (result.instructions.length) log.info(`Review instructions applied: ${result.instructions.join(", ")}`);
|
||||
if (result.skippedInstructions.length) {
|
||||
log.warn(`Review instructions skipped: ${result.skippedInstructions.join(", ")}`);
|
||||
}
|
||||
}
|
||||
|
||||
async function review(opts: ReviewOptions) {
|
||||
if (opts.json) log.setSilent(true);
|
||||
// The checkout is the pull request's head, which its author controls, so nothing is read
|
||||
// from its wmill.yaml or the local profiles it could select: the instance, workspace and
|
||||
// token come from the command line only, and the token never reaches a URL a branch names.
|
||||
if (!opts.baseUrl || !opts.workspace || !opts.token) {
|
||||
throw new Error("wmill review needs --base-url, --workspace and --token");
|
||||
}
|
||||
const event = githubEvent();
|
||||
const base = opts.base ?? event?.pull_request?.base?.sha;
|
||||
const head = opts.head ?? event?.pull_request?.head?.sha ?? "HEAD";
|
||||
if (!base) throw new Error("--base is required outside a GitHub pull_request event");
|
||||
const baseSha = git(["rev-parse", "--verify", `${base}^{commit}`]).trim();
|
||||
const headSha = git(["rev-parse", "--verify", `${head}^{commit}`]).trim();
|
||||
const ctx = opts.post === "github" ? githubContext(opts, event) : undefined;
|
||||
|
||||
await requireLogin(opts);
|
||||
const { workspaceId } = await resolveWorkspace(opts);
|
||||
let result: ReviewResult;
|
||||
try {
|
||||
result = await reviewChanges(workspaceId, opts.model, baseSha, headSha, opts.folder);
|
||||
} catch (e) {
|
||||
if (ctx) {
|
||||
await github(ctx, "POST", `/pulls/${ctx.pr}/reviews`, {
|
||||
commit_id: headSha,
|
||||
event: "COMMENT",
|
||||
body: `### Windmill AI review\n\nThe review could not be completed: ${apiErrorMessage(e) ?? (e instanceof Error ? e.message : e)}`,
|
||||
}).catch((postError) => log.error(`Could not report the failure on the pull request: ${postError}`));
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
|
||||
if (opts.json) console.log(JSON.stringify({ ...result, post: reviewPayload(result, headSha) }));
|
||||
else printResult(result);
|
||||
if (ctx) await postReview(ctx, headSha, result);
|
||||
if (
|
||||
opts.failOn &&
|
||||
result.findings.some((f) => SEVERITY_RANK[f.severity] <= SEVERITY_RANK[opts.failOn!])
|
||||
) {
|
||||
process.exitCode = 1;
|
||||
}
|
||||
}
|
||||
|
||||
const command = new Command()
|
||||
.description(
|
||||
"Review a pull request on a git sync repository with the workspace's AI, the way deployment requests are reviewed. Reads the changes and the base branch's review instructions from git; the instance, workspace and token come from the flags, never from the checkout's wmill.yaml.",
|
||||
)
|
||||
.option("--base <ref:string>", "The base branch commit. Defaults to the pull_request event's base.")
|
||||
.option("--head <ref:string>", "The pull request's commit. Defaults to the event's head, else HEAD.")
|
||||
.option("--model <model:string>", "provider:model to review with. Defaults to the workspace's default model.")
|
||||
.option("--folder <dir:string>", "The repository directory the workspace is synced into, when it is not the root.")
|
||||
.option("--json", "Print the review as JSON")
|
||||
.option("--post <target:string>", "Post the review on the pull request. Only `github` is supported.", {
|
||||
value: (v: string) => {
|
||||
if (v !== "github") throw new Error(`--post supports github only, got ${v}`);
|
||||
return v as "github";
|
||||
},
|
||||
})
|
||||
.option("--pr <number:number>", "The pull request number. Defaults to the pull_request event's.")
|
||||
.option("--fail-on <severity:string>", "Exit with 1 when a finding is at least this severe: high, medium or low.", {
|
||||
value: (v: string) => {
|
||||
if (!["high", "medium", "low"].includes(v)) throw new Error(`--fail-on takes high, medium or low, got ${v}`);
|
||||
return v as AiReviewSeverity;
|
||||
},
|
||||
})
|
||||
.action(review as any);
|
||||
|
||||
export default command;
|
||||
Generated
+14
@@ -9077,6 +9077,20 @@ resource type related commands
|
||||
- \`resource-type push <file_path:string> <name:string>\` - push a local resource spec. This overrides any remote versions.
|
||||
- \`resource-type generate-namespace\` - Create a TypeScript definition file with the RT namespace generated from the resource types
|
||||
|
||||
### review
|
||||
|
||||
Review a pull request on a git sync repository with the workspace's AI, the way deployment requests are reviewed. Reads the changes and the base branch's review instructions from git; the instance, workspace and token come from the flags, never from the checkout's wmill.yaml.
|
||||
|
||||
**Options:**
|
||||
- \`--base <ref:string>\` - The base branch commit. Defaults to the pull_request event's base.
|
||||
- \`--head <ref:string>\` - The pull request's commit. Defaults to the event's head, else HEAD.
|
||||
- \`--model <model:string>\` - provider:model to review with. Defaults to the workspace's default model.
|
||||
- \`--folder <dir:string>\` - The repository directory the workspace is synced into, when it is not the root.
|
||||
- \`--json\` - Print the review as JSON
|
||||
- \`--post <target:string>\` - Post the review on the pull request. Only \`github\` is supported.
|
||||
- \`--pr <number:number>\` - The pull request number. Defaults to the pull_request event's.
|
||||
- \`--fail-on <severity:string>\` - Exit with 1 when a finding is at least this severe: high, medium or low.
|
||||
|
||||
### schedule
|
||||
|
||||
schedule related commands
|
||||
|
||||
@@ -26,6 +26,7 @@ import instance from "./commands/instance/instance.ts";
|
||||
import workerGroups from "./commands/worker-groups/worker-groups.ts";
|
||||
import lint from "./commands/lint/lint.ts";
|
||||
import digest from "./commands/digest/digest.ts";
|
||||
import review from "./commands/review/review.ts";
|
||||
|
||||
import dev from "./commands/dev/dev.ts";
|
||||
import { GlobalOptions } from "./types.ts";
|
||||
@@ -205,6 +206,7 @@ const command = new Command()
|
||||
.command("sync", sync)
|
||||
.command("lint", lint)
|
||||
.command("digest", digest)
|
||||
.command("review", review)
|
||||
.command("gitsync-settings", gitsyncSettings)
|
||||
.command("protection-rules", protectionRules)
|
||||
.command("instance", instance)
|
||||
|
||||
@@ -21,14 +21,14 @@ export type FolderResourceType = "flow" | "app" | "raw_app";
|
||||
|
||||
// Configuration for folder suffixes - can be switched between dot and dunder prefixes
|
||||
// The default uses dot-prefixed names (.flow, .app, .raw_app)
|
||||
const DOTTED_SUFFIXES = {
|
||||
export const DOTTED_SUFFIXES = {
|
||||
flow: ".flow",
|
||||
app: ".app",
|
||||
raw_app: ".raw_app",
|
||||
} as const;
|
||||
|
||||
// Alternative dunder-prefixed names (__flow, __app, __raw_app)
|
||||
const NON_DOTTED_SUFFIXES = {
|
||||
export const NON_DOTTED_SUFFIXES = {
|
||||
flow: "__flow",
|
||||
app: "__app",
|
||||
raw_app: "__raw_app",
|
||||
|
||||
@@ -0,0 +1,182 @@
|
||||
/**
|
||||
* `wmill review` carries the frontend's AI review core, vendored by
|
||||
* cli/generate-ai-review.ts into src/commands/review/aiReviewCore.gen.ts.
|
||||
*
|
||||
* If that copy drifts, a pull request is reviewed with a different prompt, budget
|
||||
* or answer parsing than the deployment request panel uses. Fix by running
|
||||
* `bun run gen:ai-review` from cli/.
|
||||
*
|
||||
* No backend required.
|
||||
*/
|
||||
|
||||
import { describe, expect, test } from "bun:test";
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { dirname, join } from "node:path";
|
||||
import { buildAiReviewCore, OUT_FILE } from "../generate-ai-review.ts";
|
||||
import { buildReviewPrompt, itemOf, numberDiff } from "../src/commands/review/review.ts";
|
||||
|
||||
describe("ai review core", () => {
|
||||
test("the committed copy matches the frontend source", () => {
|
||||
const lf = (s: string) => s.replace(/\r\n/g, "\n");
|
||||
expect(lf(readFileSync(OUT_FILE, "utf-8"))).toBe(lf(buildAiReviewCore()));
|
||||
});
|
||||
});
|
||||
|
||||
describe("wmill review", () => {
|
||||
// Grouping decides which files the model reads as one item, the item a finding anchors to,
|
||||
// and which review instructions cover it.
|
||||
test("maps synced files to the workspace item they make up", () => {
|
||||
expect(
|
||||
[
|
||||
"f/a/flow.flow/flow.yaml",
|
||||
"f/a/flow__flow/step.inline_script.ts",
|
||||
"f/a/s.py",
|
||||
"f/a/s.script.yaml",
|
||||
"f/a/s__mod/helper.py",
|
||||
"f/a/sched.schedule.yaml",
|
||||
"f/a/folder.meta.yaml",
|
||||
"f/a/REVIEW.resource.file.md",
|
||||
"f/a/creds.variable.yaml",
|
||||
"f/a/files.fileset/x.flow/flow.yaml",
|
||||
"f/a/proj__dbt/models/legacy__mod/a.sql",
|
||||
".github/workflows/review.yml",
|
||||
].map((f) => Object.values(itemOf(f)).join(":")),
|
||||
).toEqual([
|
||||
"flow:f/a/flow",
|
||||
"flow:f/a/flow",
|
||||
"script:f/a/s",
|
||||
"script:f/a/s",
|
||||
"script:f/a/s",
|
||||
"schedule:f/a/sched",
|
||||
"folder:f/a",
|
||||
"resource:f/a/REVIEW",
|
||||
"variable:f/a/creds",
|
||||
"resource:f/a/files",
|
||||
"script:f/a/proj",
|
||||
"file:.github/workflows/review.yml",
|
||||
]);
|
||||
});
|
||||
|
||||
test("numbers each diff line by the side it is on", () => {
|
||||
const diff = [
|
||||
"diff --git a/x b/x",
|
||||
"@@ -10,3 +10,3 @@ fn",
|
||||
" keep",
|
||||
"-old",
|
||||
"+new",
|
||||
" tail",
|
||||
].join("\n");
|
||||
const numbered = numberDiff(diff)!;
|
||||
expect(numbered.text.split("\n").slice(1)).toEqual([
|
||||
"R10 keep",
|
||||
"L11 -old",
|
||||
"R11 +new",
|
||||
"R12 tail",
|
||||
]);
|
||||
expect([...numbered.left]).toEqual([11]);
|
||||
expect([...numbered.right]).toEqual([10, 11, 12]);
|
||||
expect(numberDiff("Binary files a/x and b/x differ")).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
/** Commits `base` on main and `head` on top of it in a fresh repository, and calls `run`
|
||||
* from inside it with both commits. */
|
||||
function withRepo<T>(
|
||||
base: Record<string, string>,
|
||||
head: Record<string, string>,
|
||||
run: (baseSha: string, headSha: string) => T,
|
||||
): T {
|
||||
const repo = mkdtempSync(join(tmpdir(), "wmill-review-"));
|
||||
const cwd = process.cwd();
|
||||
const write = (files: Record<string, string>) => {
|
||||
for (const [path, content] of Object.entries(files)) {
|
||||
mkdirSync(join(repo, dirname(path)), { recursive: true });
|
||||
writeFileSync(join(repo, path), content);
|
||||
}
|
||||
};
|
||||
const git = (...args: string[]) =>
|
||||
execFileSync("git", args, { cwd: repo, encoding: "utf-8" }).trim();
|
||||
try {
|
||||
git("init", "-q", "-b", "main");
|
||||
git("config", "user.email", "t@t");
|
||||
git("config", "user.name", "t");
|
||||
write(base);
|
||||
git("add", "-A");
|
||||
git("commit", "-qm", "base");
|
||||
const baseSha = git("rev-parse", "HEAD");
|
||||
write(head);
|
||||
git("add", "-A");
|
||||
git("commit", "-qm", "pr");
|
||||
process.chdir(repo);
|
||||
return run(baseSha, git("rev-parse", "HEAD"));
|
||||
} finally {
|
||||
process.chdir(cwd);
|
||||
rmSync(repo, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
describe("wmill review prompt", () => {
|
||||
// The pull request's head is written by its author, so nothing it does may put a
|
||||
// credential in front of the model or steer which instructions apply.
|
||||
test("withholds secrets whatever the head claims, and reads instructions from the base", () => {
|
||||
withRepo(
|
||||
{
|
||||
"f/a/REVIEW.resource.yaml":
|
||||
"resource_type: ai_review_instruction\nvalue:\n content: '!inline f/a/REVIEW.resource.file.md'\n",
|
||||
"f/a/REVIEW.resource.file.md": "BASE-RULE\n",
|
||||
"f/a/db.resource.yaml": "resource_type: postgresql\nvalue:\n password: OLD-PASSWORD\n",
|
||||
"f/a/creds.variable.yaml": "value: VARIABLE-SECRET\nis_secret: false\n",
|
||||
"encryption_key.yaml": "BASE-ENCRYPTION-KEY\n",
|
||||
"f/a/notes.resource.yaml": "resource_type: ai_review_instruction\nvalue:\n content: hi\n",
|
||||
"f/a/files.fileset/x.flow/flow.yaml": "password: FILESET-PASSWORD\n",
|
||||
},
|
||||
{
|
||||
"f/a/REVIEW.resource.file.md": "HEAD-RULE\n",
|
||||
"f/a/db.resource.yaml": "resource_type: ai_review_instruction\nvalue:\n content: hi\n",
|
||||
"f/a/creds.resource.yaml": "resource_type: ai_review_instruction\nvalue:\n content: hi\n",
|
||||
"f/a/creds.variable.yaml": "value: VARIABLE-SECRET-2\nis_secret: false\n",
|
||||
"encryption_key.yaml": "HEAD-ENCRYPTION-KEY\n",
|
||||
"f/a/notes.prod.resource.yaml": "resource_type: postgresql\nvalue:\n password: PROD-PASSWORD\n",
|
||||
"f/a/files.fileset/x.flow/flow.yaml": "password: FILESET-PASSWORD-2\n",
|
||||
// A pathspec matching every changed file, if git were let read it as one.
|
||||
"*": "star\n",
|
||||
},
|
||||
(base, head) => {
|
||||
const { system, user, instructions } = buildReviewPrompt(base, head);
|
||||
const prompt = system + user;
|
||||
for (const secret of ["PASSWORD", "VARIABLE-SECRET", "ENCRYPTION-KEY"]) {
|
||||
expect(prompt).not.toContain(secret);
|
||||
}
|
||||
expect(system).toContain("BASE-RULE");
|
||||
expect(system).not.toContain("HEAD-RULE");
|
||||
// The instruction change itself is shown, as a change.
|
||||
expect(user).toContain("HEAD-RULE");
|
||||
expect(instructions).toEqual(["f/a/notes", "f/a/REVIEW"]);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
// A workspace synced into a subfolder names its items from there, while the host anchors
|
||||
// comments to repository paths.
|
||||
test("reads a workspace synced into a subfolder", () => {
|
||||
withRepo(
|
||||
{
|
||||
"ws/f/a/REVIEW.resource.yaml":
|
||||
"resource_type: ai_review_instruction\nvalue:\n content: '!inline f/a/REVIEW.resource.file.md'\n",
|
||||
"ws/f/a/REVIEW.resource.file.md": "BASE-RULE\n",
|
||||
"ws/f/a/s.ts": "export const a = 1\n",
|
||||
},
|
||||
{ "ws/f/a/s.ts": "export const a = 2\n", "other/notes.md": "OUTSIDE\n" },
|
||||
(base, head) => {
|
||||
const { system, user, anchors, instructions } = buildReviewPrompt(base, head, "ws/");
|
||||
expect(instructions).toEqual(["f/a/REVIEW"]);
|
||||
expect(system).toContain("BASE-RULE");
|
||||
expect([...anchors.items]).toEqual(["script:f/a/s"]);
|
||||
expect([...anchors.lines.keys()]).toEqual(["ws/f/a/s.ts"]);
|
||||
expect(user).not.toContain("OUTSIDE");
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -734,6 +734,35 @@ 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
|
||||
|
||||
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 source for the review.** Prompt, size budget, withheld kinds and answer parsing live
|
||||
in `frontend/src/lib/components/deploymentRequest/aiReviewCore.ts`, which imports nothing
|
||||
but `folderInstructionScopes.ts`. `bun run gen:ai-review` vendors both into the CLI;
|
||||
`cli/test/ai_review_bundle_unit.test.ts` fails on drift.
|
||||
- **One provider path.** The CLI calls `POST /api/w/{ws}/ai/complete`, which sends one
|
||||
request through the `windmill-ai` query builders (`one_shot::complete_text`, shared with
|
||||
agent-step compaction). A client of its own would have to copy the per-model handling the
|
||||
frontend's `copilot/lib.ts` carries. A provider key held in CI is not supported for that
|
||||
reason.
|
||||
- **Nothing from the head steers the review.** Review instructions (`ai_review_instruction`
|
||||
resources) are read from the base branch's tip, never the pull request; a change to one is
|
||||
reviewed as a change. The checkout's `wmill.yaml` and local profiles are never read, since
|
||||
a `workspaces:` entry there could point the token at another server: base URL, workspace
|
||||
and token come from flags only. Folder suffixes are matched in both formats rather than
|
||||
read from config.
|
||||
- **Anchors.** The model reads each file as a numbered diff (`R<n>` head, `L<n>` base) with
|
||||
20 lines of context. A comment is placed on a line only if GitHub's own 3-line-context diff
|
||||
shows it, since one unplaceable comment fails the whole review; anything else goes in the
|
||||
review body, as does everything after a 422.
|
||||
- **Reruns.** One review per head commit, found by a `<!-- windmill-ai-review:<sha> -->`
|
||||
marker. A rerun still calls the model so `--fail-on` keeps gating, and skips the post.
|
||||
|
||||
## 16. Alternatives considered
|
||||
|
||||
**Portal as webhook proxy (the rejected "option 2").** Subscribe the managed app
|
||||
|
||||
@@ -0,0 +1,69 @@
|
||||
// No imports: the CLI's review bundle carries this module (see cli/generate-ai-review.ts).
|
||||
|
||||
/** Bytes of one instruction body a tool result may carry. */
|
||||
export const MAX_FOLDER_INSTRUCTIONS_LENGTH = 32 * 1024
|
||||
/** Bytes all the bodies one tool result carries may add up to, so nested folders
|
||||
* cannot stack several full-size bodies onto a single result. */
|
||||
export const MAX_FOLDER_INSTRUCTIONS_TOTAL = 48 * 1024
|
||||
|
||||
export type FolderInstruction = {
|
||||
/** The instruction resource. */
|
||||
path: string
|
||||
/** The directory it covers, with a trailing `/`: `f/billing/AGENTS` covers `f/billing/`. */
|
||||
scope: string
|
||||
}
|
||||
|
||||
export function folderInstructionScope(path: string): string {
|
||||
return path.slice(0, path.lastIndexOf('/') + 1)
|
||||
}
|
||||
|
||||
/** The instructions covering any of `paths`, outermost scope first, so a nested
|
||||
* folder's instructions come last and read as the more specific ones. A path naming
|
||||
* the folder itself (`f/billing`, as a pipeline's is) is covered too. */
|
||||
export function instructionsCovering(
|
||||
instructions: readonly FolderInstruction[],
|
||||
paths: readonly string[]
|
||||
): FolderInstruction[] {
|
||||
return instructions
|
||||
.filter((i) => paths.some((p) => `${p}/`.startsWith(i.scope)))
|
||||
.sort((a, b) => a.scope.length - b.scope.length || a.path.localeCompare(b.path))
|
||||
}
|
||||
|
||||
/** Cut `text` to `maxBytes` of UTF-8, marking the cut so a reader (the model
|
||||
* included) can tell truncation from a body that simply ends there.
|
||||
*
|
||||
* For the body, whose cap is a byte budget: 64k CJK characters are ~192 KiB, so a
|
||||
* code-unit cut would let three times the intended payload through. */
|
||||
export function truncateForPrompt(text: string, maxBytes: number): string {
|
||||
const encoded = new TextEncoder().encode(text)
|
||||
if (encoded.byteLength <= maxBytes) return text
|
||||
// `fatal: false` replaces the partial code point a byte-aligned cut can leave
|
||||
// with U+FFFD; dropping it keeps the tail clean.
|
||||
const cut = new TextDecoder('utf-8').decode(encoded.slice(0, maxBytes)).replace(/�$/, '')
|
||||
return `${cut}… [truncated]`
|
||||
}
|
||||
|
||||
export function folderInstructionsOpeningTag(path: string): string {
|
||||
return `<folder_instructions path="${path}"`
|
||||
}
|
||||
|
||||
export function formatFolderInstructions(
|
||||
blocks: readonly { instruction: FolderInstruction; body: string }[]
|
||||
): string {
|
||||
// The budget goes innermost first: the most specific instructions are the last to
|
||||
// be cut. An outer body the budget no longer reaches keeps its tag, so it still
|
||||
// counts as delivered, with the cut marked.
|
||||
let remaining = MAX_FOLDER_INSTRUCTIONS_TOTAL
|
||||
const bodies = [...blocks].reverse().map(({ body }) => {
|
||||
const cut = truncateForPrompt(body, Math.min(MAX_FOLDER_INSTRUCTIONS_LENGTH, remaining))
|
||||
remaining = Math.max(0, remaining - new TextEncoder().encode(cut).byteLength)
|
||||
return cut
|
||||
})
|
||||
bodies.reverse()
|
||||
return blocks
|
||||
.map(
|
||||
({ instruction }, i) =>
|
||||
`${folderInstructionsOpeningTag(instruction.path)} scope="${instruction.scope}">\n${bodies[i]}\n</folder_instructions>`
|
||||
)
|
||||
.join('\n\n')
|
||||
}
|
||||
@@ -1,5 +1,20 @@
|
||||
import { ResourceService } from '$lib/gen'
|
||||
import { truncateForPrompt } from './skills/skillResources'
|
||||
import {
|
||||
folderInstructionScope,
|
||||
folderInstructionsOpeningTag as openingTag,
|
||||
formatFolderInstructions,
|
||||
instructionsCovering,
|
||||
type FolderInstruction
|
||||
} from './folderInstructionScopes'
|
||||
|
||||
export {
|
||||
MAX_FOLDER_INSTRUCTIONS_LENGTH,
|
||||
MAX_FOLDER_INSTRUCTIONS_TOTAL,
|
||||
folderInstructionScope,
|
||||
formatFolderInstructions,
|
||||
instructionsCovering,
|
||||
type FolderInstruction
|
||||
} from './folderInstructionScopes'
|
||||
|
||||
/**
|
||||
* Folder instructions are resources of this type: a markdown file resource
|
||||
@@ -8,27 +23,10 @@ import { truncateForPrompt } from './skills/skillResources'
|
||||
*/
|
||||
export const FOLDER_INSTRUCTIONS_RESOURCE_TYPE = 'ai_instruction'
|
||||
|
||||
/** Bytes of one instruction body a tool result may carry. */
|
||||
export const MAX_FOLDER_INSTRUCTIONS_LENGTH = 32 * 1024
|
||||
/** Bytes all the bodies one tool result carries may add up to, so nested folders
|
||||
* cannot stack several full-size bodies onto a single result. */
|
||||
export const MAX_FOLDER_INSTRUCTIONS_TOTAL = 48 * 1024
|
||||
|
||||
/** Scopes the system prompt names. Past this the list is cut, not the delivery:
|
||||
* a scope left out still has its instructions delivered on first touch. */
|
||||
export const MAX_LISTED_FOLDER_INSTRUCTIONS = 50
|
||||
|
||||
export type FolderInstruction = {
|
||||
/** The `ai_instruction` resource. */
|
||||
path: string
|
||||
/** The directory it covers, with a trailing `/`: `f/billing/AGENTS` covers `f/billing/`. */
|
||||
scope: string
|
||||
}
|
||||
|
||||
export function folderInstructionScope(path: string): string {
|
||||
return path.slice(0, path.lastIndexOf('/') + 1)
|
||||
}
|
||||
|
||||
const PAGE_SIZE = 100
|
||||
/** Guard against a paging bug looping forever, not a product cap. */
|
||||
const MAX_PAGES = 20
|
||||
@@ -69,18 +67,6 @@ export function workspacePathsInArgs(args: unknown, depth = 0): string[] {
|
||||
)
|
||||
}
|
||||
|
||||
/** The instructions covering any of `paths`, outermost scope first, so a nested
|
||||
* folder's instructions come last and read as the more specific ones. A path naming
|
||||
* the folder itself (`f/billing`, as a pipeline's is) is covered too. */
|
||||
export function instructionsCovering(
|
||||
instructions: readonly FolderInstruction[],
|
||||
paths: readonly string[]
|
||||
): FolderInstruction[] {
|
||||
return instructions
|
||||
.filter((i) => paths.some((p) => `${p}/`.startsWith(i.scope)))
|
||||
.sort((a, b) => a.scope.length - b.scope.length || a.path.localeCompare(b.path))
|
||||
}
|
||||
|
||||
/** What a chat hands the tool loop: the instructions in play, and which tool call
|
||||
* delivered which of them.
|
||||
*
|
||||
@@ -182,31 +168,6 @@ export async function readFolderInstructionBody(workspace: string, path: string)
|
||||
return value.content
|
||||
}
|
||||
|
||||
function openingTag(path: string): string {
|
||||
return `<folder_instructions path="${path}"`
|
||||
}
|
||||
|
||||
export function formatFolderInstructions(
|
||||
blocks: readonly { instruction: FolderInstruction; body: string }[]
|
||||
): string {
|
||||
// The budget goes innermost first: the most specific instructions are the last to
|
||||
// be cut. An outer body the budget no longer reaches keeps its tag, so it still
|
||||
// counts as delivered, with the cut marked.
|
||||
let remaining = MAX_FOLDER_INSTRUCTIONS_TOTAL
|
||||
const bodies = [...blocks].reverse().map(({ body }) => {
|
||||
const cut = truncateForPrompt(body, Math.min(MAX_FOLDER_INSTRUCTIONS_LENGTH, remaining))
|
||||
remaining = Math.max(0, remaining - new TextEncoder().encode(cut).byteLength)
|
||||
return cut
|
||||
})
|
||||
bodies.reverse()
|
||||
return blocks
|
||||
.map(
|
||||
({ instruction }, i) =>
|
||||
`${openingTag(instruction.path)} scope="${instruction.scope}">\n${bodies[i]}\n</folder_instructions>`
|
||||
)
|
||||
.join('\n\n')
|
||||
}
|
||||
|
||||
/** The system prompt section naming the scopes that carry instructions. */
|
||||
export function getFolderInstructionsPromptSection(
|
||||
instructions: readonly FolderInstruction[]
|
||||
|
||||
@@ -92,19 +92,7 @@ export function truncateChars(text: string, maxChars: number): string {
|
||||
return points.length <= maxChars ? text : `${points.slice(0, maxChars).join('')}… [truncated]`
|
||||
}
|
||||
|
||||
/** Cut `text` to `maxBytes` of UTF-8, marking the cut so a reader (the model
|
||||
* included) can tell truncation from a body that simply ends there.
|
||||
*
|
||||
* For the body, whose cap is a byte budget: 64k CJK characters are ~192 KiB, so a
|
||||
* code-unit cut would let three times the intended payload through. */
|
||||
export function truncateForPrompt(text: string, maxBytes: number): string {
|
||||
const encoded = new TextEncoder().encode(text)
|
||||
if (encoded.byteLength <= maxBytes) return text
|
||||
// `fatal: false` replaces the partial code point a byte-aligned cut can leave
|
||||
// with U+FFFD; dropping it keeps the tail clean.
|
||||
const cut = new TextDecoder('utf-8').decode(encoded.slice(0, maxBytes)).replace(/�$/, '')
|
||||
return `${cut}… [truncated]`
|
||||
}
|
||||
export { truncateForPrompt } from '../folderInstructionScopes'
|
||||
|
||||
/** The SKILL.md body of one skill. Throws rather than returning `''` when the
|
||||
* resource holds no readable body: an empty string reaches the model as a
|
||||
|
||||
@@ -49,7 +49,7 @@ vi.mock('$lib/gen', async (importOriginal) => ({
|
||||
}
|
||||
}))
|
||||
|
||||
const { parseReviewAnswer, reviewDeployment } = await import('./aiReview')
|
||||
const { reviewDeployment } = await import('./aiReview')
|
||||
|
||||
function diff(path: string): WorkspaceItemDiff {
|
||||
return {
|
||||
@@ -69,26 +69,6 @@ beforeEach(() => {
|
||||
h.listed = []
|
||||
})
|
||||
|
||||
describe('parseReviewAnswer', () => {
|
||||
it('anchors only findings on items in the diff, and ranks by severity', () => {
|
||||
const answer =
|
||||
'```json\n' +
|
||||
JSON.stringify({
|
||||
summary: 's',
|
||||
findings: [
|
||||
{ kind: 'flow', path: 'f/not/changed', severity: 'low', comment: 'phantom' },
|
||||
{ kind: 'script', path: 'f/a/b', severity: 'high', comment: 'real' }
|
||||
]
|
||||
}) +
|
||||
'\n```'
|
||||
const { findings } = parseReviewAnswer(answer, [diff('f/a/b')])
|
||||
expect(findings.map((f) => [f.comment, f.kind, f.path])).toEqual([
|
||||
['real', 'script', 'f/a/b'],
|
||||
['phantom', '', '']
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
describe('reviewDeployment', () => {
|
||||
it('reports an item whose read failed instead of reviewing it as emptied', async () => {
|
||||
const result = await reviewDeployment(
|
||||
|
||||
@@ -3,28 +3,28 @@ import { getNonStreamingCompletion } from '$lib/components/copilot/lib'
|
||||
import { getItemValue } from '$lib/utils_workspace_deploy'
|
||||
import type { Kind } from '$lib/utils_deployable'
|
||||
import {
|
||||
formatFolderInstructions,
|
||||
instructionsCovering,
|
||||
listFolderInstructions,
|
||||
readFolderInstructionBody,
|
||||
type FolderInstruction
|
||||
readFolderInstructionBody
|
||||
} from '$lib/components/copilot/chat/folderInstructions'
|
||||
import {
|
||||
instructionsCovering,
|
||||
MAX_OUTPUT_TOKENS,
|
||||
parseReviewAnswer,
|
||||
renderItemValue,
|
||||
REVIEW_INSTRUCTIONS_RESOURCE_TYPE,
|
||||
reviewInstructionsSection,
|
||||
ReviewBudget,
|
||||
reviewSystemPrompt,
|
||||
VALUE_WITHHELD_KINDS,
|
||||
type AiReviewFinding,
|
||||
type FolderInstruction
|
||||
} from './aiReviewCore'
|
||||
|
||||
/**
|
||||
* Review instructions are resources of this type in the parent workspace: a markdown file
|
||||
* resource whose `value.content` applies to every changed item under the directory it lives in.
|
||||
*/
|
||||
export const REVIEW_INSTRUCTIONS_RESOURCE_TYPE = 'ai_review_instruction'
|
||||
|
||||
export type AiReviewSeverity = 'high' | 'medium' | 'low'
|
||||
|
||||
export type AiReviewFinding = {
|
||||
id: string
|
||||
kind: string
|
||||
path: string
|
||||
severity: AiReviewSeverity
|
||||
comment: string
|
||||
}
|
||||
export {
|
||||
REVIEW_INSTRUCTIONS_RESOURCE_TYPE,
|
||||
type AiReviewFinding,
|
||||
type AiReviewSeverity
|
||||
} from './aiReviewCore'
|
||||
|
||||
export type AiReviewResult = {
|
||||
summary: string
|
||||
@@ -41,29 +41,7 @@ export type AiReviewResult = {
|
||||
instructionsUnavailable: boolean
|
||||
}
|
||||
|
||||
// A variable's value is read decrypted and a resource's value holds credentials, so neither may
|
||||
// reach the model: they are reviewed by name and change type only.
|
||||
const VALUE_WITHHELD_KINDS = new Set(['variable', 'resource'])
|
||||
|
||||
const PER_SIDE_CHAR_CAP = 20_000
|
||||
const TOTAL_CHAR_BUDGET = 160_000
|
||||
const FETCH_BATCH = 6
|
||||
// Under the Anthropic SDK's non-streaming pre-flight max_tokens limit.
|
||||
const MAX_OUTPUT_TOKENS = 8_000
|
||||
|
||||
const SYSTEM_PROMPT = `You review a Windmill deployment request: a set of changes made in a fork workspace that will be merged into its parent workspace. Each changed item is given with its parent version (what is deployed now) and its fork version (what will be deployed). An item can be a script, flow, app, schedule, trigger, folder, resource type, datatable migration, variable or resource.
|
||||
|
||||
Report only issues a reviewer should act on before approving the merge:
|
||||
- bugs and logic errors introduced by the change
|
||||
- breaking changes: removed or renamed inputs, changed input types, changed outputs that callers may rely on
|
||||
- risky operational changes: schedules retimed, triggers pointed at a different script or flow, folder owners or permissions, app policies, destructive migrations
|
||||
- hardcoded secrets or credentials
|
||||
- deletions that look unintended
|
||||
|
||||
Do not comment on style, naming, formatting or lockfiles, and do not praise. Do not repeat a finding for every item it touches. An item with no issue gets no finding.
|
||||
|
||||
Answer with JSON only, no prose around it, in this shape:
|
||||
{"summary": "<one or two sentences on what the change does and its overall risk>", "findings": [{"kind": "<item kind as given>", "path": "<item path as given>", "severity": "high" | "medium" | "low", "comment": "<the issue and what to do about it, under 80 words>"}]}`
|
||||
|
||||
// Read from the parent, never the fork: the fork is the author's side, and its copy of an
|
||||
// instruction is itself one of the changes under review. A failed listing reviews without
|
||||
@@ -99,14 +77,8 @@ async function reviewInstructions(
|
||||
const blocks = reads.filter(
|
||||
(r): r is { instruction: FolderInstruction; body: string } => r.body !== undefined
|
||||
)
|
||||
// Hardcoded secrets and unintended deletions stay a floor: a folder's writers steer the
|
||||
// review of their own changes, so they must not be able to switch those checks off.
|
||||
const text =
|
||||
blocks.length === 0
|
||||
? ''
|
||||
: `\n\nThe parent workspace gives the review instructions below, each for the changed items under the folder in its scope. For those items they take precedence over the instructions above, and a nested folder's over its parent folder's. They may ask you to check more, check less or weigh severity differently, except that hardcoded secrets or credentials and deletions that look unintended are always reported. The answer is still the JSON above.\n\n${formatFolderInstructions(blocks)}`
|
||||
return {
|
||||
text,
|
||||
text: reviewInstructionsSection('deployment_request', blocks),
|
||||
applied: blocks.map((b) => b.instruction.path),
|
||||
skipped: reads.filter((r) => r.body === undefined).map((r) => r.instruction.path),
|
||||
unavailable: false
|
||||
@@ -119,18 +91,6 @@ function changeType(diff: WorkspaceItemDiff): 'added' | 'deleted' | 'modified' {
|
||||
return 'modified'
|
||||
}
|
||||
|
||||
function render(value: unknown): string {
|
||||
if (value == null) return '(none)'
|
||||
// Flows and apps nest their inline scripts' lockfiles, so they are dropped at any depth.
|
||||
const text =
|
||||
typeof value === 'string'
|
||||
? value
|
||||
: JSON.stringify(value, (key, v) => (key === 'lock' ? undefined : v), 2)
|
||||
return text.length > PER_SIDE_CHAR_CAP
|
||||
? `${text.slice(0, PER_SIDE_CHAR_CAP)}\n… (truncated, ${text.length - PER_SIDE_CHAR_CAP} more characters)`
|
||||
: text
|
||||
}
|
||||
|
||||
async function itemSection(
|
||||
diff: WorkspaceItemDiff,
|
||||
forkWorkspaceId: string,
|
||||
@@ -151,7 +111,7 @@ async function itemSection(
|
||||
return { text: `${header}\nContents could not be read.`, unreadable: true }
|
||||
}
|
||||
return {
|
||||
text: `${header}\n#### Parent version\n${render(parent)}\n#### Fork version\n${render(fork)}`,
|
||||
text: `${header}\n#### Parent version\n${renderItemValue(parent)}\n#### Fork version\n${renderItemValue(fork)}`,
|
||||
unreadable: false
|
||||
}
|
||||
}
|
||||
@@ -160,39 +120,6 @@ function isEmptyRead(value: unknown): boolean {
|
||||
return value != null && typeof value === 'object' && Object.keys(value as object).length === 0
|
||||
}
|
||||
|
||||
export function parseReviewAnswer(
|
||||
text: string,
|
||||
diffs: WorkspaceItemDiff[]
|
||||
): Pick<AiReviewResult, 'summary' | 'findings'> {
|
||||
const start = text.indexOf('{')
|
||||
const end = text.lastIndexOf('}')
|
||||
if (start === -1 || end <= start) {
|
||||
throw new Error('The AI answer was not in the expected format')
|
||||
}
|
||||
const parsed = JSON.parse(text.slice(start, end + 1)) as {
|
||||
summary?: unknown
|
||||
findings?: unknown
|
||||
}
|
||||
// Anchoring a comment needs a (kind, path) the fork's diff actually has, so a finding naming
|
||||
// anything else is kept but unanchored rather than posted against a phantom item.
|
||||
const known = new Set(diffs.map((d) => `${d.kind}:${d.path}`))
|
||||
const findings: AiReviewFinding[] = []
|
||||
for (const [i, raw] of (Array.isArray(parsed.findings) ? parsed.findings : []).entries()) {
|
||||
if (!raw || typeof raw.comment !== 'string' || !raw.comment.trim()) continue
|
||||
const anchored = known.has(`${raw.kind}:${raw.path}`)
|
||||
findings.push({
|
||||
id: String(i),
|
||||
kind: anchored ? raw.kind : '',
|
||||
path: anchored ? raw.path : '',
|
||||
severity: ['high', 'medium', 'low'].includes(raw.severity) ? raw.severity : 'medium',
|
||||
comment: raw.comment.trim()
|
||||
})
|
||||
}
|
||||
const rank: Record<AiReviewSeverity, number> = { high: 0, medium: 1, low: 2 }
|
||||
findings.sort((a, b) => rank[a.severity] - rank[b.severity])
|
||||
return { summary: typeof parsed.summary === 'string' ? parsed.summary : '', findings }
|
||||
}
|
||||
|
||||
/** Reviews the changes `diffs` carry from the fork into its parent with the workspace's AI model. */
|
||||
export async function reviewDeployment(
|
||||
diffs: WorkspaceItemDiff[],
|
||||
@@ -205,45 +132,37 @@ export async function reviewDeployment(
|
||||
}
|
||||
): Promise<AiReviewResult> {
|
||||
const instructionsPromise = reviewInstructions(diffs, parentWorkspaceId)
|
||||
const included: string[] = []
|
||||
const omitted: string[] = []
|
||||
const budget = new ReviewBudget()
|
||||
const unreadable: string[] = []
|
||||
let used = 0
|
||||
// Fetched a batch at a time so a large fork stops reading items once the budget is spent.
|
||||
// The first section that does not fit spends it: `used` alone can stay under the cap forever.
|
||||
let budgetSpent = false
|
||||
for (let i = 0; i < diffs.length; i += FETCH_BATCH) {
|
||||
const batch = diffs.slice(i, i + FETCH_BATCH)
|
||||
if (budgetSpent) {
|
||||
omitted.push(...batch.map((d) => `${d.kind}:${d.path}`))
|
||||
if (budget.spent) {
|
||||
budget.omitted.push(...batch.map((d) => `${d.kind}:${d.path}`))
|
||||
continue
|
||||
}
|
||||
const sections = await Promise.all(
|
||||
batch.map((d) => itemSection(d, forkWorkspaceId, parentWorkspaceId))
|
||||
)
|
||||
if (abortController.signal.aborted) throw new Error('aborted')
|
||||
sections.forEach(({ text: section, unreadable: failed }, j) => {
|
||||
if (failed) unreadable.push(`${batch[j].kind}:${batch[j].path}`)
|
||||
if (!budgetSpent && used + section.length <= TOTAL_CHAR_BUDGET) {
|
||||
included.push(section)
|
||||
used += section.length
|
||||
} else {
|
||||
budgetSpent = true
|
||||
omitted.push(`${batch[j].kind}:${batch[j].path}`)
|
||||
}
|
||||
sections.forEach(({ text, unreadable: failed }, j) => {
|
||||
const key = `${batch[j].kind}:${batch[j].path}`
|
||||
if (failed) unreadable.push(key)
|
||||
budget.add(key, text)
|
||||
})
|
||||
}
|
||||
let user = `Fork workspace: ${forkWorkspaceId}\nParent workspace: ${parentWorkspaceId}\n\n${included.join('\n\n')}`
|
||||
if (omitted.length > 0) {
|
||||
user += `\n\nAlso changed, contents not shown for size:\n${omitted.map((o) => `- ${o}`).join('\n')}`
|
||||
}
|
||||
|
||||
const instructions = await instructionsPromise
|
||||
if (abortController.signal.aborted) throw new Error('aborted')
|
||||
const answer = await getNonStreamingCompletion(
|
||||
[
|
||||
{ role: 'system', content: SYSTEM_PROMPT + instructions.text },
|
||||
{ role: 'user', content: user }
|
||||
{ role: 'system', content: reviewSystemPrompt('deployment_request') + instructions.text },
|
||||
{
|
||||
role: 'user',
|
||||
content: budget.userPrompt(
|
||||
`Fork workspace: ${forkWorkspaceId}\nParent workspace: ${parentWorkspaceId}`
|
||||
)
|
||||
}
|
||||
],
|
||||
abortController,
|
||||
// Bound to the fork: without `workspace` the call goes through whichever workspace the
|
||||
@@ -255,8 +174,10 @@ export async function reviewDeployment(
|
||||
}
|
||||
)
|
||||
return {
|
||||
...parseReviewAnswer(answer ?? '', diffs),
|
||||
omitted,
|
||||
...parseReviewAnswer(answer ?? '', {
|
||||
items: new Set(diffs.map((d) => `${d.kind}:${d.path}`))
|
||||
}),
|
||||
omitted: budget.omitted,
|
||||
unreadable,
|
||||
instructions: instructions.applied,
|
||||
skippedInstructions: instructions.skipped,
|
||||
|
||||
@@ -0,0 +1,43 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { parseReviewAnswer } from './aiReviewCore'
|
||||
|
||||
function answer(findings: object[]): string {
|
||||
return '```json\n' + JSON.stringify({ summary: 's', findings }) + '\n```'
|
||||
}
|
||||
|
||||
describe('parseReviewAnswer', () => {
|
||||
it('anchors only findings on items in the diff, and ranks by severity', () => {
|
||||
const { findings } = parseReviewAnswer(
|
||||
answer([
|
||||
{ kind: 'flow', path: 'f/not/changed', severity: 'low', comment: 'phantom' },
|
||||
{ kind: 'script', path: 'f/a/b', severity: 'high', comment: 'real' }
|
||||
]),
|
||||
{ items: new Set(['script:f/a/b']) }
|
||||
)
|
||||
expect(findings.map((f) => [f.comment, f.kind, f.path])).toEqual([
|
||||
['real', 'script', 'f/a/b'],
|
||||
['phantom', '', '']
|
||||
])
|
||||
})
|
||||
|
||||
// A pull request review is refused whole when one comment sits off the diff, so a line
|
||||
// the diff does not show must fall back to the file, and an unknown file to no anchor.
|
||||
it('keeps a line anchor only on a line the diff shows, on the side named', () => {
|
||||
const lines = new Map([['f/a/b.ts', { LEFT: new Set([3]), RIGHT: new Set([4, 5]) }]])
|
||||
const { findings } = parseReviewAnswer(
|
||||
answer([
|
||||
{ kind: 'script', path: 'f/a/b', file: 'f/a/b.ts', line: 'R5', comment: 'head' },
|
||||
{ kind: 'script', path: 'f/a/b', file: 'f/a/b.ts', line: 'L3', comment: 'base' },
|
||||
{ kind: 'script', path: 'f/a/b', file: 'f/a/b.ts', line: 'R3', comment: 'off diff' },
|
||||
{ kind: 'script', path: 'f/a/b', file: 'f/a/c.ts', line: 'R4', comment: 'other file' }
|
||||
]),
|
||||
{ items: new Set(['script:f/a/b']), lines }
|
||||
)
|
||||
expect(findings.map((f) => [f.comment, f.file, f.side, f.line])).toEqual([
|
||||
['head', 'f/a/b.ts', 'RIGHT', 5],
|
||||
['base', 'f/a/b.ts', 'LEFT', 3],
|
||||
['off diff', 'f/a/b.ts', undefined, undefined],
|
||||
['other file', undefined, undefined, undefined]
|
||||
])
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,193 @@
|
||||
// What the review asks of the model and how its answer is read, for every place it runs: the
|
||||
// deployment request panel here, and `wmill review` on a git sync pull request, which carries
|
||||
// this module through cli/generate-ai-review.ts. Keep it free of `$lib` imports; the
|
||||
// staleness test in cli/test/ai_review_bundle_unit.test.ts fails when the CLI's copy drifts.
|
||||
import {
|
||||
formatFolderInstructions,
|
||||
type FolderInstruction
|
||||
} from '../copilot/chat/folderInstructionScopes'
|
||||
|
||||
export {
|
||||
folderInstructionScope,
|
||||
instructionsCovering,
|
||||
type FolderInstruction
|
||||
} from '../copilot/chat/folderInstructionScopes'
|
||||
|
||||
/**
|
||||
* Review instructions are resources of this type: a markdown file resource whose
|
||||
* `value.content` applies to every changed item under the directory it lives in.
|
||||
*/
|
||||
export const REVIEW_INSTRUCTIONS_RESOURCE_TYPE = 'ai_review_instruction'
|
||||
|
||||
export type AiReviewSeverity = 'high' | 'medium' | 'low'
|
||||
|
||||
export type AiReviewFinding = {
|
||||
id: string
|
||||
/** Empty when the finding names no changed item. */
|
||||
kind: string
|
||||
path: string
|
||||
severity: AiReviewSeverity
|
||||
comment: string
|
||||
/** Pull request reviews only: the changed file and line the finding is anchored to. */
|
||||
file?: string
|
||||
line?: number
|
||||
side?: 'LEFT' | 'RIGHT'
|
||||
}
|
||||
|
||||
// A variable's value is read decrypted and a resource's value holds credentials, so neither may
|
||||
// reach the model: they are reviewed by name and change type only.
|
||||
export const VALUE_WITHHELD_KINDS = new Set(['variable', 'resource'])
|
||||
|
||||
export const PER_SIDE_CHAR_CAP = 20_000
|
||||
export const TOTAL_CHAR_BUDGET = 160_000
|
||||
// Under the Anthropic SDK's non-streaming pre-flight max_tokens limit.
|
||||
export const MAX_OUTPUT_TOKENS = 8_000
|
||||
|
||||
/** What is being reviewed: a fork's deployment request, or a pull request on a git sync repo. */
|
||||
export type ReviewSubject = 'deployment_request' | 'pull_request'
|
||||
|
||||
const INTRO: Record<ReviewSubject, string> = {
|
||||
deployment_request:
|
||||
'You review a Windmill deployment request: a set of changes made in a fork workspace that will be merged into its parent workspace. Each changed item is given with its parent version (what is deployed now) and its fork version (what will be deployed).',
|
||||
pull_request:
|
||||
"You review a pull request on a git repository that Windmill's git sync keeps in step with a workspace: merging it deploys its changes. Each changed item is given with the files that make it up in the repository, each as a unified diff of the base branch (what is deployed now) against the pull request (what will be deployed). Every diff line starts with its line number: R<n> for line n of the pull request's version of the file, L<n> for line n of the base version, then + for an added line, - for a removed one and a space for unchanged context."
|
||||
}
|
||||
|
||||
const ANSWER_SHAPE: Record<ReviewSubject, string> = {
|
||||
deployment_request:
|
||||
'{"summary": "<one or two sentences on what the change does and its overall risk>", "findings": [{"kind": "<item kind as given>", "path": "<item path as given>", "severity": "high" | "medium" | "low", "comment": "<the issue and what to do about it, under 80 words>"}]}',
|
||||
pull_request:
|
||||
'{"summary": "<one or two sentences on what the change does and its overall risk>", "findings": [{"kind": "<item kind as given>", "path": "<item path as given>", "file": "<the file the issue is in, as given>", "line": "<the R or L line number the issue is on, as given, such as R12, or null for the file as a whole>", "severity": "high" | "medium" | "low", "comment": "<the issue and what to do about it, under 80 words>"}]}'
|
||||
}
|
||||
|
||||
const SOURCE: Record<ReviewSubject, string> = {
|
||||
deployment_request: 'The parent workspace',
|
||||
pull_request: "The repository's base branch"
|
||||
}
|
||||
|
||||
export function reviewSystemPrompt(subject: ReviewSubject): string {
|
||||
return `${INTRO[subject]} An item can be a script, flow, app, schedule, trigger, folder, resource type, datatable migration, variable or resource.
|
||||
|
||||
Report only issues a reviewer should act on before approving the merge:
|
||||
- bugs and logic errors introduced by the change
|
||||
- breaking changes: removed or renamed inputs, changed input types, changed outputs that callers may rely on
|
||||
- risky operational changes: schedules retimed, triggers pointed at a different script or flow, folder owners or permissions, app policies, destructive migrations
|
||||
- hardcoded secrets or credentials
|
||||
- deletions that look unintended
|
||||
|
||||
Do not comment on style, naming, formatting or lockfiles, and do not praise. Do not repeat a finding for every item it touches. An item with no issue gets no finding.
|
||||
|
||||
Answer with JSON only, no prose around it, in this shape:
|
||||
${ANSWER_SHAPE[subject]}`
|
||||
}
|
||||
|
||||
/** The system prompt addition carrying the review instructions that apply, or '' for none. */
|
||||
export function reviewInstructionsSection(
|
||||
subject: ReviewSubject,
|
||||
blocks: readonly { instruction: FolderInstruction; body: string }[]
|
||||
): string {
|
||||
if (blocks.length === 0) return ''
|
||||
// Hardcoded secrets and unintended deletions stay a floor: a folder's writers steer the
|
||||
// review of their own changes, so they must not be able to switch those checks off.
|
||||
return `\n\n${SOURCE[subject]} gives the review instructions below, each for the changed items under the folder in its scope. For those items they take precedence over the instructions above, and a nested folder's over its parent folder's. They may ask you to check more, check less or weigh severity differently, except that hardcoded secrets or credentials and deletions that look unintended are always reported. The answer is still the JSON above.\n\n${formatFolderInstructions(blocks)}`
|
||||
}
|
||||
|
||||
/** One side of a changed item as the model reads it, lockfiles dropped and capped. */
|
||||
export function renderItemValue(value: unknown): string {
|
||||
if (value == null) return '(none)'
|
||||
// Flows and apps nest their inline scripts' lockfiles, so they are dropped at any depth.
|
||||
const text =
|
||||
typeof value === 'string'
|
||||
? value
|
||||
: JSON.stringify(value, (key, v) => (key === 'lock' ? undefined : v), 2)
|
||||
return capText(text, PER_SIDE_CHAR_CAP)
|
||||
}
|
||||
|
||||
export function capText(text: string, cap: number): string {
|
||||
return text.length > cap
|
||||
? `${text.slice(0, cap)}\n… (truncated, ${text.length - cap} more characters)`
|
||||
: text
|
||||
}
|
||||
|
||||
/**
|
||||
* Packs item sections into the prompt in order. The first section that does not fit spends
|
||||
* the budget, and every item after it is listed by name only: `used` alone could stay under
|
||||
* the cap forever, and a caller that reads items lazily stops reading once `spent` is set.
|
||||
*/
|
||||
export class ReviewBudget {
|
||||
included: string[] = []
|
||||
/** `kind:path` of the changed items reviewed by name only. */
|
||||
omitted: string[] = []
|
||||
spent = false
|
||||
private used = 0
|
||||
|
||||
add(key: string, section: string): void {
|
||||
if (!this.spent && this.used + section.length <= TOTAL_CHAR_BUDGET) {
|
||||
this.included.push(section)
|
||||
this.used += section.length
|
||||
} else {
|
||||
this.spent = true
|
||||
this.omitted.push(key)
|
||||
}
|
||||
}
|
||||
|
||||
userPrompt(header: string): string {
|
||||
let user = `${header}\n\n${this.included.join('\n\n')}`
|
||||
if (this.omitted.length > 0) {
|
||||
user += `\n\nAlso changed, contents not shown for size:\n${this.omitted.map((o) => `- ${o}`).join('\n')}`
|
||||
}
|
||||
return user
|
||||
}
|
||||
}
|
||||
|
||||
/** What a finding may be anchored to. */
|
||||
export type ReviewAnchors = {
|
||||
/** `kind:path` of every changed item. */
|
||||
items: ReadonlySet<string>
|
||||
/** Pull request reviews: for each changed file, the line numbers a comment can be placed on. */
|
||||
lines?: ReadonlyMap<string, { LEFT: ReadonlySet<number>; RIGHT: ReadonlySet<number> }>
|
||||
}
|
||||
|
||||
export function parseReviewAnswer(
|
||||
text: string,
|
||||
anchors: ReviewAnchors
|
||||
): { summary: string; findings: AiReviewFinding[] } {
|
||||
const start = text.indexOf('{')
|
||||
const end = text.lastIndexOf('}')
|
||||
if (start === -1 || end <= start) {
|
||||
throw new Error('The AI answer was not in the expected format')
|
||||
}
|
||||
const parsed = JSON.parse(text.slice(start, end + 1)) as {
|
||||
summary?: unknown
|
||||
findings?: unknown
|
||||
}
|
||||
// Anchoring a comment needs an item, file and line the change actually has, so a finding
|
||||
// naming anything else keeps its comment but loses the anchor rather than being posted
|
||||
// against a phantom.
|
||||
const findings: AiReviewFinding[] = []
|
||||
for (const [i, raw] of (Array.isArray(parsed.findings) ? parsed.findings : []).entries()) {
|
||||
if (!raw || typeof raw.comment !== 'string' || !raw.comment.trim()) continue
|
||||
const anchored = anchors.items.has(`${raw.kind}:${raw.path}`)
|
||||
const finding: AiReviewFinding = {
|
||||
id: String(i),
|
||||
kind: anchored ? raw.kind : '',
|
||||
path: anchored ? raw.path : '',
|
||||
severity: ['high', 'medium', 'low'].includes(raw.severity) ? raw.severity : 'medium',
|
||||
comment: raw.comment.trim()
|
||||
}
|
||||
const fileLines = typeof raw.file === 'string' ? anchors.lines?.get(raw.file) : undefined
|
||||
if (fileLines) {
|
||||
finding.file = raw.file
|
||||
const line = /^([LR])(\d+)$/.exec(String(raw.line ?? '').trim())
|
||||
const side = line?.[1] === 'L' ? 'LEFT' : 'RIGHT'
|
||||
if (line && fileLines[side].has(Number(line[2]))) {
|
||||
finding.side = side
|
||||
finding.line = Number(line[2])
|
||||
}
|
||||
}
|
||||
findings.push(finding)
|
||||
}
|
||||
const rank: Record<AiReviewSeverity, number> = { high: 0, medium: 1, low: 2 }
|
||||
findings.sort((a, b) => rank[a.severity] - rank[b.severity])
|
||||
return { summary: typeof parsed.summary === 'string' ? parsed.summary : '', findings }
|
||||
}
|
||||
@@ -536,6 +536,20 @@ resource type related commands
|
||||
- `resource-type push <file_path:string> <name:string>` - push a local resource spec. This overrides any remote versions.
|
||||
- `resource-type generate-namespace` - Create a TypeScript definition file with the RT namespace generated from the resource types
|
||||
|
||||
### review
|
||||
|
||||
Review a pull request on a git sync repository with the workspace's AI, the way deployment requests are reviewed. Reads the changes and the base branch's review instructions from git; the instance, workspace and token come from the flags, never from the checkout's wmill.yaml.
|
||||
|
||||
**Options:**
|
||||
- `--base <ref:string>` - The base branch commit. Defaults to the pull_request event's base.
|
||||
- `--head <ref:string>` - The pull request's commit. Defaults to the event's head, else HEAD.
|
||||
- `--model <model:string>` - provider:model to review with. Defaults to the workspace's default model.
|
||||
- `--folder <dir:string>` - The repository directory the workspace is synced into, when it is not the root.
|
||||
- `--json` - Print the review as JSON
|
||||
- `--post <target:string>` - Post the review on the pull request. Only `github` is supported.
|
||||
- `--pr <number:number>` - The pull request number. Defaults to the pull_request event's.
|
||||
- `--fail-on <severity:string>` - Exit with 1 when a finding is at least this severe: high, medium or low.
|
||||
|
||||
### schedule
|
||||
|
||||
schedule related commands
|
||||
|
||||
@@ -4407,6 +4407,20 @@ resource type related commands
|
||||
- \`resource-type push <file_path:string> <name:string>\` - push a local resource spec. This overrides any remote versions.
|
||||
- \`resource-type generate-namespace\` - Create a TypeScript definition file with the RT namespace generated from the resource types
|
||||
|
||||
### review
|
||||
|
||||
Review a pull request on a git sync repository with the workspace's AI, the way deployment requests are reviewed. Reads the changes and the base branch's review instructions from git; the instance, workspace and token come from the flags, never from the checkout's wmill.yaml.
|
||||
|
||||
**Options:**
|
||||
- \`--base <ref:string>\` - The base branch commit. Defaults to the pull_request event's base.
|
||||
- \`--head <ref:string>\` - The pull request's commit. Defaults to the event's head, else HEAD.
|
||||
- \`--model <model:string>\` - provider:model to review with. Defaults to the workspace's default model.
|
||||
- \`--folder <dir:string>\` - The repository directory the workspace is synced into, when it is not the root.
|
||||
- \`--json\` - Print the review as JSON
|
||||
- \`--post <target:string>\` - Post the review on the pull request. Only \`github\` is supported.
|
||||
- \`--pr <number:number>\` - The pull request number. Defaults to the pull_request event's.
|
||||
- \`--fail-on <severity:string>\` - Exit with 1 when a finding is at least this severe: high, medium or low.
|
||||
|
||||
### schedule
|
||||
|
||||
schedule related commands
|
||||
|
||||
@@ -541,6 +541,20 @@ resource type related commands
|
||||
- `resource-type push <file_path:string> <name:string>` - push a local resource spec. This overrides any remote versions.
|
||||
- `resource-type generate-namespace` - Create a TypeScript definition file with the RT namespace generated from the resource types
|
||||
|
||||
### review
|
||||
|
||||
Review a pull request on a git sync repository with the workspace's AI, the way deployment requests are reviewed. Reads the changes and the base branch's review instructions from git; the instance, workspace and token come from the flags, never from the checkout's wmill.yaml.
|
||||
|
||||
**Options:**
|
||||
- `--base <ref:string>` - The base branch commit. Defaults to the pull_request event's base.
|
||||
- `--head <ref:string>` - The pull request's commit. Defaults to the event's head, else HEAD.
|
||||
- `--model <model:string>` - provider:model to review with. Defaults to the workspace's default model.
|
||||
- `--folder <dir:string>` - The repository directory the workspace is synced into, when it is not the root.
|
||||
- `--json` - Print the review as JSON
|
||||
- `--post <target:string>` - Post the review on the pull request. Only `github` is supported.
|
||||
- `--pr <number:number>` - The pull request number. Defaults to the pull_request event's.
|
||||
- `--fail-on <severity:string>` - Exit with 1 when a finding is at least this severe: high, medium or low.
|
||||
|
||||
### schedule
|
||||
|
||||
schedule related commands
|
||||
|
||||
Reference in New Issue
Block a user