diff --git a/apps/frontend/src/pages/moderation/technical-review/rules.vue b/apps/frontend/src/pages/moderation/technical-review/rules.vue index 66035cd75e..ec444cfc23 100644 --- a/apps/frontend/src/pages/moderation/technical-review/rules.vue +++ b/apps/frontend/src/pages/moderation/technical-review/rules.vue @@ -10,6 +10,19 @@ placeholder="Known-safe obfuscated bootstrap" /> + + +

+ Higher-priority rules run first. Rules with the same priority run in creation order. +

+

{{ rule.name }}

-

Revision {{ rule.revision }}

+

+ Priority {{ rule.priority }} ยท Revision {{ rule.revision }} +

@@ -474,6 +489,7 @@ const expandedAffectedDetails = reactive( const loadingAffectedRuleIds = reactive(new Set()) const form = reactive({ name: '', + priority: 0 as number | undefined, rule: DEFAULT_RULE, }) let ruleTestRequestId = 0 @@ -753,6 +769,7 @@ function openCreateModal() { if (isScanning.value) return editingRuleId.value = null form.name = '' + form.priority = 0 form.rule = DEFAULT_RULE isRuleModalOpen.value = true ruleModal.value?.show() @@ -765,6 +782,7 @@ function openEditModal(rule: Labrinth.TechReview.Internal.DelphiRule) { if (isScanning.value) return editingRuleId.value = rule.id form.name = rule.name + form.priority = rule.priority form.rule = rule.rule isRuleModalOpen.value = true ruleModal.value?.show() @@ -787,11 +805,19 @@ function handleRuleModalHide() { async function saveRule() { if (isSaving.value || isScanning.value) return - if (!form.name.trim() || !form.rule.trim()) { + const priority = form.priority + if ( + !form.name.trim() || + !form.rule.trim() || + typeof priority !== 'number' || + !Number.isInteger(priority) || + priority < -2147483648 || + priority > 2147483647 + ) { addNotification({ type: 'error', title: 'Invalid rule', - text: 'Enter a name and a CEL expression.', + text: 'Enter a name, an integer priority, and a CEL expression.', }) return } @@ -799,6 +825,7 @@ async function saveRule() { isSaving.value = true const payload = { name: form.name, + priority, rule: form.rule, } diff --git a/apps/labrinth/migrations/20260724120000_delphi_rule_priority.sql b/apps/labrinth/migrations/20260724120000_delphi_rule_priority.sql new file mode 100644 index 0000000000..344cac1832 --- /dev/null +++ b/apps/labrinth/migrations/20260724120000_delphi_rule_priority.sql @@ -0,0 +1,2 @@ +ALTER TABLE delphi_rules +ADD COLUMN priority INTEGER NOT NULL DEFAULT 0; diff --git a/apps/labrinth/migrations/20260726120000_delphi_tech_review_queue.sql b/apps/labrinth/migrations/20260726120000_delphi_tech_review_queue.sql new file mode 100644 index 0000000000..f4f5030fa2 --- /dev/null +++ b/apps/labrinth/migrations/20260726120000_delphi_tech_review_queue.sql @@ -0,0 +1,28 @@ +CREATE TABLE delphi_tech_review_queue ( + project_id BIGINT PRIMARY KEY REFERENCES mods(id) + ON DELETE CASCADE +); + +INSERT INTO delphi_tech_review_queue (project_id) +SELECT DISTINCT didws.project_id +FROM delphi_issue_details_with_statuses didws +INNER JOIN delphi_report_issues dri ON dri.id = didws.issue_id +WHERE + ( + dri.issue_type = '__dummy' + AND didws.status = 'pending' + ) + OR ( + dri.issue_type != '__dummy' + AND didws.status IN ('pending', 'unsafe') + AND NOT didws.hidden + ); + +DELETE FROM delphi_report_issue_details detail +USING delphi_report_issues issue +WHERE + detail.issue_id = issue.id + AND issue.issue_type = '__dummy'; + +DELETE FROM delphi_report_issues +WHERE issue_type = '__dummy'; diff --git a/apps/labrinth/src/routes/internal/delphi/mod.rs b/apps/labrinth/src/routes/internal/delphi/mod.rs index ed275dfb6c..f414a68d79 100644 --- a/apps/labrinth/src/routes/internal/delphi/mod.rs +++ b/apps/labrinth/src/routes/internal/delphi/mod.rs @@ -32,7 +32,7 @@ use crate::{ }; pub mod rescan; -pub mod tech_review_sync; +pub mod tech_review_queue; pub fn config(cfg: &mut actix_web::web::ServiceConfig) { cfg.service( @@ -257,9 +257,8 @@ async fn ingest_report_deserialized( .await .wrap_internal_err("failed to apply delphi rules to new issue details")?; - tech_review_sync::sync_project_tech_review_state( + tech_review_queue::add_projects_with_review_details( &[DBProjectId::from(report.project_id)], - tech_review_sync::TechReviewExitReason::Resolved, &mut transaction, ) .await?; diff --git a/apps/labrinth/src/routes/internal/delphi/rescan.rs b/apps/labrinth/src/routes/internal/delphi/rescan.rs index cbaefd58f7..bfe40edfc6 100644 --- a/apps/labrinth/src/routes/internal/delphi/rescan.rs +++ b/apps/labrinth/src/routes/internal/delphi/rescan.rs @@ -97,28 +97,23 @@ async fn fetch_unreviewed_tech_review_project_ids( r#" SELECT DISTINCT m.id FROM mods m + INNER JOIN delphi_tech_review_queue queue ON queue.project_id = m.id WHERE EXISTS( SELECT 1 FROM delphi_issue_details_with_statuses didws - INNER JOIN delphi_report_issues dri ON dri.id = didws.issue_id WHERE didws.project_id = m.id AND didws.status = 'pending' AND NOT didws.hidden - -- see delphi.rs todo comment - AND dri.issue_type != '__dummy' ) AND NOT EXISTS( SELECT 1 FROM delphi_issue_details_with_statuses didws - INNER JOIN delphi_report_issues dri ON dri.id = didws.issue_id WHERE didws.project_id = m.id AND didws.status IN ('safe', 'unsafe') AND NOT didws.hidden - -- see delphi.rs todo comment - AND dri.issue_type != '__dummy' ) "#, ) diff --git a/apps/labrinth/src/routes/internal/delphi/tech_review_queue.rs b/apps/labrinth/src/routes/internal/delphi/tech_review_queue.rs new file mode 100644 index 0000000000..cde08d1581 --- /dev/null +++ b/apps/labrinth/src/routes/internal/delphi/tech_review_queue.rs @@ -0,0 +1,203 @@ +//! Maintains explicit project membership in the technical review queue. +//! +//! Queue membership is represented by a row in `delphi_tech_review_queue`. +//! Enter and exit thread messages are emitted only when an insert or delete +//! actually changes that membership, in the same transaction. + +use itertools::Itertools; + +use crate::{ + database::{ + PgTransaction, + models::{DBProjectId, DBThreadId, thread_item::ThreadMessageBuilder}, + }, + models::threads::MessageBody, + routes::ApiError, + util::error::Context, +}; + +#[derive(Debug, Clone, Copy)] +pub enum TechReviewRemovalReason { + RulesChanged, + FileDeleted, +} + +pub async fn add_projects( + project_ids: &[DBProjectId], + txn: &mut PgTransaction<'_>, +) -> Result<(), ApiError> { + let project_ids = project_ids.iter().copied().unique().collect::>(); + if project_ids.is_empty() { + return Ok(()); + } + + let rows = sqlx::query!( + r#" + WITH inserted AS ( + INSERT INTO delphi_tech_review_queue (project_id) + SELECT unnest($1::bigint[]) + ON CONFLICT (project_id) DO NOTHING + RETURNING project_id + ) + SELECT + inserted.project_id AS "project_id!: DBProjectId", + ( + SELECT thread.id + FROM threads thread + WHERE thread.mod_id = inserted.project_id + ORDER BY thread.id + LIMIT 1 + ) AS "thread_id!: DBThreadId" + FROM inserted + "#, + &project_ids.iter().map(|id| id.0).collect::>(), + ) + .fetch_all(&mut *txn) + .await + .wrap_internal_err("failed to add projects to technical review queue")?; + + for row in rows { + ThreadMessageBuilder { + author_id: None, + body: MessageBody::TechReviewEntered, + thread_id: row.thread_id, + hide_identity: false, + } + .insert(txn) + .await + .wrap_internal_err("failed to add entering technical review message")?; + } + + Ok(()) +} + +pub async fn remove_projects( + project_ids: &[DBProjectId], + reason: TechReviewRemovalReason, + txn: &mut PgTransaction<'_>, +) -> Result<(), ApiError> { + let project_ids = project_ids.iter().copied().unique().collect::>(); + if project_ids.is_empty() { + return Ok(()); + } + + let rows = sqlx::query!( + r#" + WITH removed AS ( + DELETE FROM delphi_tech_review_queue + WHERE project_id = ANY($1::bigint[]) + RETURNING project_id + ) + SELECT + removed.project_id AS "project_id!: DBProjectId", + ( + SELECT thread.id + FROM threads thread + WHERE thread.mod_id = removed.project_id + ORDER BY thread.id + LIMIT 1 + ) AS "thread_id!: DBThreadId" + FROM removed + "#, + &project_ids.iter().map(|id| id.0).collect::>(), + ) + .fetch_all(&mut *txn) + .await + .wrap_internal_err( + "failed to remove projects from technical review queue", + )?; + + let body = match reason { + TechReviewRemovalReason::RulesChanged => MessageBody::TechReviewExited, + TechReviewRemovalReason::FileDeleted => { + MessageBody::TechReviewExitFileDeleted + } + }; + + for row in rows { + ThreadMessageBuilder { + author_id: None, + body: body.clone(), + thread_id: row.thread_id, + hide_identity: false, + } + .insert(txn) + .await + .wrap_internal_err("failed to add exiting technical review message")?; + } + + Ok(()) +} + +pub async fn add_projects_with_review_details( + project_ids: &[DBProjectId], + txn: &mut PgTransaction<'_>, +) -> Result<(), ApiError> { + let project_ids = project_ids.iter().copied().unique().collect::>(); + if project_ids.is_empty() { + return Ok(()); + } + + let rows = sqlx::query!( + r#" + SELECT DISTINCT detail.project_id AS "project_id!: DBProjectId" + FROM delphi_issue_details_with_statuses detail + WHERE + detail.project_id = ANY($1::bigint[]) + AND detail.status IN ('pending', 'unsafe') + AND NOT detail.hidden + "#, + &project_ids.iter().map(|id| id.0).collect::>(), + ) + .fetch_all(&mut *txn) + .await + .wrap_internal_err("failed to find projects requiring technical review")?; + + add_projects( + &rows + .into_iter() + .map(|row| row.project_id) + .collect::>(), + txn, + ) + .await +} + +pub async fn remove_projects_without_details( + project_ids: &[DBProjectId], + reason: TechReviewRemovalReason, + txn: &mut PgTransaction<'_>, +) -> Result<(), ApiError> { + let project_ids = project_ids.iter().copied().unique().collect::>(); + if project_ids.is_empty() { + return Ok(()); + } + + let rows = sqlx::query!( + r#" + SELECT requested.project_id AS "project_id!: DBProjectId" + FROM unnest($1::bigint[]) AS requested(project_id) + WHERE NOT EXISTS ( + SELECT 1 + FROM delphi_issue_details_with_statuses detail + WHERE detail.project_id = requested.project_id + ) + "#, + &project_ids.iter().map(|id| id.0).collect::>(), + ) + .fetch_all(&mut *txn) + .await + .wrap_internal_err( + "failed to find projects without technical review details", + )?; + + remove_projects( + &rows + .into_iter() + .map(|row| row.project_id) + .collect::>(), + reason, + txn, + ) + .await +} diff --git a/apps/labrinth/src/routes/internal/delphi/tech_review_sync.rs b/apps/labrinth/src/routes/internal/delphi/tech_review_sync.rs deleted file mode 100644 index 974db515d0..0000000000 --- a/apps/labrinth/src/routes/internal/delphi/tech_review_sync.rs +++ /dev/null @@ -1,460 +0,0 @@ -//! Synchronizes moderation thread messages and dummy queue blockers with the -//! current computed tech review state of affected projects. -//! -//! When a project has a Delphi report submitted for it, or when a moderator -//! updates one of its issue details' rows (like flagging a detail as *globally* -//! safe or unsafe), we will need to recheck if the project still belongs in the -//! tech review queue or if it needs to exit now. -//! -//! Side-note: "entering the queue" or "exiting the queue" right now just means -//! adding a new message to the project's moderation thread which indicates if -//! it entered/exited. In the future this should be replaced with a more proper -//! audit log table, or "is project currently in tech review" table. -//! -//! A project is considered to need tech review when it has at least one -//! non-dummy issue detail whose effective status is pending or unsafe, or when -//! it already has a dummy pending detail blocking the final review submission. -//! Effective status is just the local detail's verdict (from -//! `delphi_issue_detail_verdicts`), or if it's null then the global verdict for -//! the same `drid.key` (from `delphi_global_detail_verdicts`). -//! -//! Some examples of how this behavior manifests: let's assume you have projects -//! _A_ and _B_ currently in tech review. They each have one (unresolved) issue -//! detail with key _K_. -//! - If you mark _K_ on _A_ as locally safe/unsafe, then _A_ is fully resolved, -//! but we still have the `__dummy` detail, which means it's still in the -//! queue until the moderator submits the actual report. _B_ is entirely -//! unaffected. -//! - If you mark _K_ on _A_ as globally safe, then _A_ and _B_ both get fully -//! resolved, but both still have the `__dummy` detail, so they also still -//! need the final report to be submitted by the moderator. -//! -//! In practice, this means that some projects may have e.g. "100/100 traces -//! are safe" reported, but they will just be waiting for final moderator -//! approval. -//! -//! The logic for checking whether a project is now in tech review or not, and -//! sending the appropriate message, is complex! That's why this module exists: -//! to act as a single chokepoint which (correctly) syncs all the state, instead -//! of having each mutation run its own ad-hoc update logic. - -use itertools::Itertools; - -use crate::{ - database::{ - PgTransaction, - models::{ - DBProjectId, DBThreadId, DelphiReportId, - delphi_report_item::DelphiVerdict, - thread_item::ThreadMessageBuilder, - }, - }, - models::threads::MessageBody, - routes::ApiError, - util::error::Context, -}; - -const DUMMY_ISSUE_TYPE: &str = "__dummy"; - -#[derive(Debug, Clone, Copy)] -pub enum TechReviewExitReason { - Resolved, - FileDeleted, - RulesChanged, -} - -struct ProjectTechReviewState { - project_id: DBProjectId, - has_pending_detail: bool, - has_unsafe_detail: bool, - has_dummy: bool, - thread_id: Option, - report_id: Option, - last_tech_review_message_type: Option, -} - -pub async fn sync_project_tech_review_state( - project_ids: &[DBProjectId], - exit_reason: TechReviewExitReason, - txn: &mut PgTransaction<'_>, -) -> Result<(), ApiError> { - let project_ids = project_ids.iter().copied().unique().collect::>(); - if project_ids.is_empty() { - return Ok(()); - } - - let project_ids_raw = project_ids.iter().map(|id| id.0).collect::>(); - let tech_review_message_types = tech_review_message_types(); - - let rows = sqlx::query!( - r#" - WITH project_ids AS ( - SELECT unnest($1::bigint[]) AS project_id - ), - project_detail_state AS ( - SELECT - p.project_id, - COALESCE(BOOL_OR( - didws.status = 'pending' - AND NOT didws.hidden - AND dri.issue_type != $3 - ), FALSE) AS has_pending_detail, - COALESCE(BOOL_OR( - didws.status = 'unsafe' - AND NOT didws.hidden - AND dri.issue_type != $3 - ), FALSE) AS has_unsafe_detail, - COALESCE(BOOL_OR( - didws.status = 'pending' - AND dri.issue_type = $3 - ), FALSE) AS has_dummy - FROM project_ids p - LEFT JOIN delphi_issue_details_with_statuses didws - ON didws.project_id = p.project_id - LEFT JOIN delphi_report_issues dri ON dri.id = didws.issue_id - GROUP BY p.project_id - ) - SELECT - p.project_id AS "project_id!: DBProjectId", - p.has_pending_detail AS "has_pending_detail!", - p.has_unsafe_detail AS "has_unsafe_detail!", - p.has_dummy AS "has_dummy!", - ( - SELECT t.id - FROM threads t - WHERE t.mod_id = p.project_id - ORDER BY t.id - LIMIT 1 - ) AS "thread_id: DBThreadId", - ( - SELECT tm.body->>'type' - FROM threads t - INNER JOIN threads_messages tm ON tm.thread_id = t.id - WHERE - t.mod_id = p.project_id - AND tm.body->>'type' = ANY($2::text[]) - ORDER BY tm.created DESC, tm.id DESC - LIMIT 1 - ) AS "last_tech_review_message_type", - ( - SELECT dr.id - FROM versions v - INNER JOIN files f ON f.version_id = v.id - INNER JOIN delphi_reports dr ON dr.file_id = f.id - WHERE v.mod_id = p.project_id - ORDER BY dr.created DESC, dr.id DESC - LIMIT 1 - ) AS "report_id: DelphiReportId" - FROM project_detail_state p - "#, - &project_ids_raw, - &tech_review_message_types, - DUMMY_ISSUE_TYPE, - ) - .fetch_all(&mut *txn) - .await - .wrap_internal_err("failed to fetch project tech review state")?; - - for row in rows { - let state = ProjectTechReviewState { - project_id: row.project_id, - has_pending_detail: row.has_pending_detail, - has_unsafe_detail: row.has_unsafe_detail, - has_dummy: row.has_dummy, - thread_id: row.thread_id, - report_id: row.report_id, - last_tech_review_message_type: row.last_tech_review_message_type, - }; - - sync_one_project_tech_review_state(state, exit_reason, txn).await?; - } - - Ok(()) -} - -pub async fn sync_detail_key_tech_review_state( - detail_keys: &[String], - exit_reason: TechReviewExitReason, - txn: &mut PgTransaction<'_>, -) -> Result<(), ApiError> { - let detail_keys = detail_keys.iter().cloned().unique().collect::>(); - - if detail_keys.is_empty() { - return Ok(()); - } - - let rows = sqlx::query!( - r#" - SELECT DISTINCT didws.project_id AS "project_id!: DBProjectId" - FROM delphi_issue_details_with_statuses didws - WHERE didws.key = ANY($1::text[]) - "#, - &detail_keys, - ) - .fetch_all(&mut *txn) - .await - .wrap_internal_err("failed to fetch projects affected by detail keys")?; - - let project_ids = rows - .into_iter() - .map(|row| row.project_id) - .collect::>(); - - sync_project_tech_review_state(&project_ids, exit_reason, txn).await -} - -pub async fn sync_deleted_project_tech_review_exit( - project_id: DBProjectId, - txn: &mut PgTransaction<'_>, -) -> Result<(), ApiError> { - let tech_review_message_types = tech_review_message_types(); - - let row = sqlx::query!( - r#" - SELECT - ( - SELECT t.id - FROM threads t - WHERE t.mod_id = $1 - ORDER BY t.id - LIMIT 1 - ) AS "thread_id: DBThreadId", - ( - SELECT tm.body->>'type' - FROM threads t - INNER JOIN threads_messages tm ON tm.thread_id = t.id - WHERE - t.mod_id = $1 - AND tm.body->>'type' = ANY($2::text[]) - ORDER BY tm.created DESC, tm.id DESC - LIMIT 1 - ) AS "last_tech_review_message_type" - "#, - project_id as DBProjectId, - &tech_review_message_types, - ) - .fetch_one(&mut *txn) - .await - .wrap_internal_err("failed to fetch deleted project tech review state")?; - - if let Some(thread_id) = row.thread_id - && should_send_exit(row.last_tech_review_message_type.as_deref()) - { - insert_exit_message(thread_id, TechReviewExitReason::FileDeleted, txn) - .await?; - } - - Ok(()) -} - -async fn sync_one_project_tech_review_state( - state: ProjectTechReviewState, - exit_reason: TechReviewExitReason, - txn: &mut PgTransaction<'_>, -) -> Result<(), ApiError> { - let has_review_detail = state.has_pending_detail || state.has_unsafe_detail; - - if matches!(exit_reason, TechReviewExitReason::RulesChanged) - && !has_review_detail - { - remove_dummy_issue_details(state.project_id, txn).await?; - - if let Some(thread_id) = state.thread_id - && should_send_exit(state.last_tech_review_message_type.as_deref()) - { - insert_exit_message(thread_id, exit_reason, txn).await?; - } - - return Ok(()); - } - - let needs_tech_review = has_review_detail || state.has_dummy; - - if needs_tech_review { - if (state.has_pending_detail || state.has_unsafe_detail) - && !state.has_dummy - && let Some(report_id) = state.report_id - { - // TODO: Currently, the queue query determines whether a project is - // in tech review by checking whether it has any pending issue - // details. If all visible issue details are marked safe or unsafe - // before the final report is submitted, the project would otherwise - // leave the tech review queue without a final tech review verdict - // message. - // - // This should be replaced with explicit tech review state, such as - // an append-only project tech review event table where the latest - // enter/exit event is the current state. Until then, this dummy - // issue detail acts as the pending queue blocker. - ensure_dummy_issue_detail(report_id, txn).await?; - } - - if let Some(thread_id) = state.thread_id - && state.last_tech_review_message_type.as_deref() - != Some(MessageBody::TechReviewEntered.as_ref()) - { - ThreadMessageBuilder { - author_id: None, - body: MessageBody::TechReviewEntered, - thread_id, - hide_identity: false, - } - .insert(txn) - .await - .wrap_internal_err("failed to add entering tech review message")?; - } - - return Ok(()); - } - - if matches!(exit_reason, TechReviewExitReason::Resolved) - && state.last_tech_review_message_type.as_deref() - == Some(MessageBody::TechReviewEntered.as_ref()) - { - if let Some(report_id) = state.report_id { - ensure_dummy_issue_detail(report_id, txn).await?; - } - - return Ok(()); - } - - if let Some(thread_id) = state.thread_id - && should_send_exit(state.last_tech_review_message_type.as_deref()) - { - insert_exit_message(thread_id, exit_reason, txn).await?; - } - - Ok(()) -} - -fn should_send_exit(last_tech_review_message_type: Option<&str>) -> bool { - matches!(last_tech_review_message_type, Some(message_type) if !matches!( - message_type, - message_type if message_type == MessageBody::TechReviewExited.as_ref() - || message_type == MessageBody::TechReviewExitFileDeleted.as_ref() - || message_type == tech_review_completed_message_type() - )) -} - -async fn insert_exit_message( - thread_id: DBThreadId, - exit_reason: TechReviewExitReason, - txn: &mut PgTransaction<'_>, -) -> Result<(), ApiError> { - let body = match exit_reason { - TechReviewExitReason::Resolved | TechReviewExitReason::RulesChanged => { - MessageBody::TechReviewExited - } - TechReviewExitReason::FileDeleted => { - MessageBody::TechReviewExitFileDeleted - } - }; - - ThreadMessageBuilder { - author_id: None, - body, - thread_id, - hide_identity: false, - } - .insert(txn) - .await - .wrap_internal_err("failed to add exiting tech review message")?; - - Ok(()) -} - -async fn remove_dummy_issue_details( - project_id: DBProjectId, - txn: &mut PgTransaction<'_>, -) -> Result<(), ApiError> { - sqlx::query!( - r#" - DELETE FROM delphi_report_issue_details detail - USING - delphi_report_issues issue, - delphi_reports report, - files file, - versions version - WHERE - detail.issue_id = issue.id - AND issue.issue_type = $2 - AND issue.report_id = report.id - AND report.file_id = file.id - AND file.version_id = version.id - AND version.mod_id = $1 - "#, - project_id as DBProjectId, - DUMMY_ISSUE_TYPE, - ) - .execute(&mut *txn) - .await - .wrap_internal_err("failed to remove dummy Delphi issue details")?; - - Ok(()) -} - -async fn ensure_dummy_issue_detail( - report_id: DelphiReportId, - txn: &mut PgTransaction<'_>, -) -> Result<(), ApiError> { - sqlx::query!( - r#" - WITH dummy_issue AS ( - INSERT INTO delphi_report_issues (report_id, issue_type) - VALUES ($1, $2) - ON CONFLICT (report_id, issue_type) - DO UPDATE SET issue_type = EXCLUDED.issue_type - RETURNING id - ) - INSERT INTO delphi_report_issue_details ( - issue_id, - key, - jar, - file_path, - decompiled_source, - data, - severity - ) - SELECT - id, - '', - NULL, - '', - NULL, - '{}'::jsonb, - 'low'::delphi_severity - FROM dummy_issue - WHERE NOT EXISTS ( - SELECT 1 - FROM delphi_report_issue_details drid - WHERE drid.issue_id = dummy_issue.id - ) - "#, - report_id as DelphiReportId, - DUMMY_ISSUE_TYPE, - ) - .execute(&mut *txn) - .await - .wrap_internal_err("failed to ensure dummy Delphi report issue detail")?; - - Ok(()) -} - -fn tech_review_message_types() -> Vec { - [ - MessageBody::TechReviewEntered.as_ref(), - MessageBody::TechReviewExited.as_ref(), - MessageBody::TechReviewExitFileDeleted.as_ref(), - tech_review_completed_message_type(), - ] - .into_iter() - .map(|message_type| message_type.to_string()) - .collect() -} - -fn tech_review_completed_message_type() -> &'static str { - MessageBody::TechReview { - verdict: DelphiVerdict::Safe, - } - .as_ref() -} diff --git a/apps/labrinth/src/routes/internal/moderation/tech_review.rs b/apps/labrinth/src/routes/internal/moderation/tech_review.rs index 853d0b05fb..a43c40eb73 100644 --- a/apps/labrinth/src/routes/internal/moderation/tech_review.rs +++ b/apps/labrinth/src/routes/internal/moderation/tech_review.rs @@ -33,10 +33,7 @@ use crate::{ queue::session::AuthQueue, routes::{ ApiError, - internal::{ - delphi::tech_review_sync::{self, TechReviewExitReason}, - moderation::Ownership, - }, + internal::{delphi::tech_review_queue, moderation::Ownership}, }, search::SearchState, util::error::Context, @@ -318,7 +315,6 @@ pub async fn get_report( ON didws.issue_id = severity_issue.id WHERE severity_issue.report_id = dr.id - AND severity_issue.issue_type != '__dummy' AND NOT didws.hidden ), 'low'::delphi_severity), -- TODO: replace with `json_array` in Postgres 16 @@ -352,8 +348,6 @@ pub async fn get_report( FROM delphi_report_issues dri WHERE dri.report_id = dr.id - -- see delphi.rs todo comment - AND dri.issue_type != '__dummy' AND EXISTS ( SELECT 1 FROM delphi_issue_details_with_statuses visible_detail @@ -655,10 +649,6 @@ async fn fetch_project_reports( let mut file_issues = Vec::new(); for issue_row in report_issues { - if issue_row.issue_type == "__dummy" { - continue; - } - let issue_details = details_by_issue .get(&issue_row.id) .unwrap_or(&empty_details); @@ -770,13 +760,15 @@ pub async fn search_projects( m.id AS "project_id: DBProjectId", MIN(t.id) AS "thread_id!: DBThreadId" FROM mods m + INNER JOIN delphi_tech_review_queue trq ON trq.project_id = m.id INNER JOIN threads t ON t.mod_id = m.id - INNER JOIN versions v ON v.mod_id = m.id - INNER JOIN files f ON f.version_id = v.id - INNER JOIN delphi_reports dr ON dr.file_id = f.id - INNER JOIN delphi_report_issues dri ON dri.report_id = dr.id - INNER JOIN delphi_issue_details_with_statuses didws + LEFT JOIN versions v ON v.mod_id = m.id + LEFT JOIN files f ON f.version_id = v.id + LEFT JOIN delphi_reports dr ON dr.file_id = f.id + LEFT JOIN delphi_report_issues dri ON dri.report_id = dr.id + LEFT JOIN delphi_issue_details_with_statuses didws ON didws.issue_id = dri.id + AND NOT didws.hidden LEFT JOIN threads_messages tm_last ON tm_last.thread_id = t.id AND tm_last.id = ( @@ -820,9 +812,25 @@ pub async fn search_projects( ) AND m.status NOT IN ('draft', 'rejected', 'withheld') AND (cardinality($6::text[]) = 0 OR m.status = ANY($6::text[])) - AND (cardinality($7::text[]) = 0 OR dri.issue_type = ANY($7::text[])) - AND didws.status = 'pending' - AND NOT didws.hidden + AND ( + cardinality($7::text[]) = 0 + OR EXISTS ( + SELECT 1 + FROM versions issue_version + INNER JOIN files issue_file + ON issue_file.version_id = issue_version.id + INNER JOIN delphi_reports issue_report + ON issue_report.file_id = issue_file.id + INNER JOIN delphi_report_issues issue + ON issue.report_id = issue_report.id + INNER JOIN delphi_issue_details_with_statuses detail + ON detail.issue_id = issue.id + WHERE + issue_version.mod_id = m.id + AND issue.issue_type = ANY($7::text[]) + AND NOT detail.hidden + ) + ) AND ( $5::text IS NULL OR ($5::text = 'unreplied' AND (tm_last.id IS NULL OR u_last.role IS NULL OR u_last.role NOT IN ('moderator', 'admin'))) @@ -832,8 +840,8 @@ pub async fn search_projects( ORDER BY CASE WHEN $3 = 'created_asc' THEN MIN(dr.created) ELSE TO_TIMESTAMP(0) END ASC, CASE WHEN $3 = 'created_desc' THEN MIN(dr.created) ELSE TO_TIMESTAMP(0) END DESC, - CASE WHEN $3 = 'severity_asc' THEN MAX(didws.severity) ELSE 'low'::delphi_severity END ASC, - CASE WHEN $3 = 'severity_desc' THEN MAX(didws.severity) ELSE 'low'::delphi_severity END DESC, + CASE WHEN $3 = 'severity_asc' THEN COALESCE(MAX(didws.severity), 'low'::delphi_severity) ELSE 'low'::delphi_severity END ASC, + CASE WHEN $3 = 'severity_desc' THEN COALESCE(MAX(didws.severity), 'low'::delphi_severity) ELSE 'low'::delphi_severity END DESC, -- tie-breaker: oldest reports MIN(dr.created) ASC LIMIT $1 OFFSET $2 @@ -1079,8 +1087,6 @@ pub async fn submit_report( m.id = $1 AND didws.status = 'pending' AND NOT didws.hidden - -- see delphi.rs todo comment - AND dri.issue_type != '__dummy' "#, project_id as _, ) @@ -1099,24 +1105,18 @@ pub async fn submit_report( }); } - sqlx::query!( - " - DELETE FROM delphi_report_issue_details drid - WHERE issue_id IN ( - SELECT dri.id - FROM mods m - INNER JOIN versions v ON v.mod_id = m.id - INNER JOIN files f ON f.version_id = v.id - INNER JOIN delphi_reports dr ON dr.file_id = f.id - INNER JOIN delphi_report_issues dri ON dri.report_id = dr.id - WHERE m.id = $1 AND dri.issue_type = '__dummy' - ) - ", - project_id as _, + sqlx::query_scalar!( + r#" + DELETE FROM delphi_tech_review_queue + WHERE project_id = $1 + RETURNING project_id AS "project_id: DBProjectId" + "#, + project_id as DBProjectId, ) - .execute(&mut txn) + .fetch_optional(&mut txn) .await - .wrap_internal_err("failed to delete dummy issue")?; + .wrap_internal_err("failed to remove project from technical review queue")? + .ok_or(ApiError::NotFound)?; let record = sqlx::query!( r#" @@ -1301,10 +1301,6 @@ pub async fn update_issue_details( i.verdict FROM incoming i INNER JOIN delphi_issue_details_with_statuses didws ON didws.id = i.detail_id - INNER JOIN delphi_report_issues dri ON dri.id = didws.issue_id - WHERE - -- see delphi.rs todo comment - dri.issue_type != '__dummy' ), validated AS ( SELECT @@ -1364,10 +1360,7 @@ pub async fn update_issue_details( r#" SELECT DISTINCT didws.project_id AS "project_id!: DBProjectId" FROM delphi_issue_details_with_statuses didws - INNER JOIN delphi_report_issues dri ON dri.id = didws.issue_id - WHERE - didws.id = ANY($1::bigint[]) - AND dri.issue_type != '__dummy' + WHERE didws.id = ANY($1::bigint[]) "#, &detail_ids, ) @@ -1382,9 +1375,8 @@ pub async fn update_issue_details( .map(|row| row.project_id) .collect::>(); - tech_review_sync::sync_project_tech_review_state( + tech_review_queue::add_projects_with_review_details( &affected_project_ids, - TechReviewExitReason::Resolved, &mut txn, ) .await?; @@ -1492,9 +1484,25 @@ pub async fn update_global_issue_details( .await .wrap_internal_err("failed to update global issue details")?; - tech_review_sync::sync_detail_key_tech_review_state( + let affected_projects = sqlx::query!( + r#" + SELECT DISTINCT detail.project_id AS "project_id!: DBProjectId" + FROM delphi_issue_details_with_statuses detail + WHERE detail.key = ANY($1::text[]) + "#, &detail_keys, - TechReviewExitReason::Resolved, + ) + .fetch_all(&mut txn) + .await + .wrap_internal_err( + "failed to fetch projects affected by global detail updates", + )?; + + tech_review_queue::add_projects_with_review_details( + &affected_projects + .into_iter() + .map(|row| row.project_id) + .collect::>(), &mut txn, ) .await?; diff --git a/apps/labrinth/src/routes/internal/moderation/tech_review/global.rs b/apps/labrinth/src/routes/internal/moderation/tech_review/global.rs index 09f47d3ea3..34ab60f7fc 100644 --- a/apps/labrinth/src/routes/internal/moderation/tech_review/global.rs +++ b/apps/labrinth/src/routes/internal/moderation/tech_review/global.rs @@ -191,7 +191,6 @@ pub async fn search_global_issue_details( AND NOT didws.hidden LEFT JOIN delphi_report_issues dri ON dri.id = didws.issue_id - AND dri.issue_type != '__dummy' WHERE ( $1::text IS NULL OR dgdv.detail_key ILIKE '%' || $1 || '%' @@ -250,7 +249,6 @@ pub async fn search_global_issue_details( WHERE didws.key = ANY($1::text[]) AND NOT didws.hidden - AND dri.issue_type != '__dummy' ) SELECT detail_key AS "detail_key!", @@ -372,7 +370,6 @@ pub async fn get_global_issue_detail( AND NOT didws.hidden LEFT JOIN delphi_report_issues dri ON dri.id = didws.issue_id - AND dri.issue_type != '__dummy' WHERE dgdv.detail_key = $1 GROUP BY dgdv.detail_key, dgdv.verdict "#, @@ -416,7 +413,6 @@ pub async fn get_global_issue_detail( didws.key = $1 AND ($2::bigint IS NULL OR didws.id > $2) AND NOT didws.hidden - AND dri.issue_type != '__dummy' ORDER BY didws.id LIMIT $3 "#, diff --git a/apps/labrinth/src/routes/internal/moderation/tech_review/rules.rs b/apps/labrinth/src/routes/internal/moderation/tech_review/rules.rs index 00c2f4a91a..5c71eaa080 100644 --- a/apps/labrinth/src/routes/internal/moderation/tech_review/rules.rs +++ b/apps/labrinth/src/routes/internal/moderation/tech_review/rules.rs @@ -45,6 +45,7 @@ pub struct DelphiRule { pub id: i64, pub name: String, pub rule: String, + pub priority: i32, pub revision: i64, pub created_at: DateTime, pub updated_at: DateTime, @@ -77,6 +78,8 @@ pub struct DelphiRuleAffectedDetail { pub struct WriteDelphiRule { pub name: String, pub rule: String, + #[serde(default)] + pub priority: i32, } #[derive(Debug, Deserialize, utoipa::ToSchema)] @@ -112,6 +115,7 @@ pub struct DelphiRuleEffect { struct ValidatedRule { name: String, rule: String, + priority: i32, } impl WriteDelphiRule { @@ -142,7 +146,11 @@ impl WriteDelphiRule { ApiError::Request(eyre!("invalid cel expression: {error}")) })?; - Ok(ValidatedRule { name, rule }) + Ok(ValidatedRule { + name, + rule, + priority: self.priority, + }) } } @@ -265,6 +273,7 @@ pub async fn get_rules( delphi_rule.id, delphi_rule.name, delphi_rule.rule, + delphi_rule.priority, delphi_rule.revision, delphi_rule.created_at, delphi_rule.updated_at, @@ -323,7 +332,10 @@ pub async fn get_rules( LIMIT 3 ) preview ON TRUE WHERE NOT delphi_rule.delete_on_next_revision - ORDER BY delphi_rule.id, preview.detail_id DESC + ORDER BY + delphi_rule.priority DESC, + delphi_rule.id, + preview.detail_id DESC "#, ) .fetch_all(&***ro_pool) @@ -340,6 +352,7 @@ pub async fn get_rules( id: rule.id, name: rule.name, rule: rule.rule, + priority: rule.priority, revision: rule.revision, created_at: rule.created_at, updated_at: rule.updated_at, @@ -513,6 +526,7 @@ pub async fn create_rule( INSERT INTO delphi_rules ( name, rule, + priority, revision, created_by, updated_by @@ -520,14 +534,16 @@ pub async fn create_rule( VALUES ( $1, $2, - (SELECT revision + 1 FROM delphi_rule_revisions LIMIT 1), $3, - $3 + (SELECT revision + 1 FROM delphi_rule_revisions LIMIT 1), + $4, + $4 ) RETURNING id, name, rule, + priority, revision, created_at, updated_at, @@ -536,6 +552,7 @@ pub async fn create_rule( "#, rule.name, rule.rule, + rule.priority, user_id, ) .fetch_one(&**pool) @@ -546,6 +563,7 @@ pub async fn create_rule( id: rule.id, name: rule.name, rule: rule.rule, + priority: rule.priority, revision: rule.revision, created_at: rule.created_at, updated_at: rule.updated_at, @@ -591,16 +609,18 @@ pub async fn update_rule( SET name = $2, rule = $3, + priority = $4, revision = ( SELECT revision + 1 FROM delphi_rule_revisions LIMIT 1 ), updated_at = CURRENT_TIMESTAMP, - updated_by = $4 + updated_by = $5 WHERE id = $1 AND NOT delete_on_next_revision RETURNING id, name, rule, + priority, revision, created_at, updated_at, @@ -610,6 +630,7 @@ pub async fn update_rule( id, rule.name, rule.rule, + rule.priority, user_id, ) .fetch_optional(&**pool) @@ -621,6 +642,7 @@ pub async fn update_rule( id: rule.id, name: rule.name, rule: rule.rule, + priority: rule.priority, revision: rule.revision, created_at: rule.created_at, updated_at: rule.updated_at, diff --git a/apps/labrinth/src/routes/internal/moderation/tech_review/rules_scan.rs b/apps/labrinth/src/routes/internal/moderation/tech_review/rules_scan.rs index c3f9d79d21..0594027df3 100644 --- a/apps/labrinth/src/routes/internal/moderation/tech_review/rules_scan.rs +++ b/apps/labrinth/src/routes/internal/moderation/tech_review/rules_scan.rs @@ -12,8 +12,8 @@ use tokio_stream::wrappers::UnboundedReceiverStream; use utoipa::{PartialSchema, ToSchema}; use super::rules::DelphiRuleEffect; -use crate::routes::internal::delphi::tech_review_sync::{ - self, TechReviewExitReason, +use crate::routes::internal::delphi::tech_review_queue::{ + self, TechReviewRemovalReason, }; use crate::{ auth::check_is_moderator_from_headers, @@ -32,8 +32,6 @@ use crate::{ const RULE_SCAN_LOCK_ID: i64 = 0x6465_6c70_6869_7275; const PROGRESS_INTERVAL: usize = 50; -const DUMMY_ISSUE_TYPE: &str = "__dummy"; - pub fn config(cfg: &mut actix_web::web::ServiceConfig) { cfg.service(get_rule_schema) .service(get_detail_rule_input) @@ -212,12 +210,9 @@ pub async fn get_detail_rule_input( FROM hashes WHERE hashes.file_id = file.id ) file_hashes ON TRUE - WHERE - detail.id = $1 - AND issue.issue_type != $2 + WHERE detail.id = $1 "#, detail_id as DelphiReportIssueDetailsId, - DUMMY_ISSUE_TYPE, ) .fetch_optional(&***ro_pool) .await, @@ -384,11 +379,8 @@ async fn run_scan( let total = sqlx::query_scalar!( r#" SELECT COUNT(*) AS "count!" - FROM delphi_report_issue_details detail - INNER JOIN delphi_report_issues issue ON issue.id = detail.issue_id - WHERE issue.issue_type != $1 + FROM delphi_report_issue_details "#, - DUMMY_ISSUE_TYPE, ) .fetch_one(&mut transaction) .await @@ -423,10 +415,8 @@ async fn run_scan( FROM hashes GROUP BY file_id ) file_hashes ON file_hashes.file_id = file.id - WHERE issue.issue_type != $1 ORDER BY detail.id "#, - DUMMY_ISSUE_TYPE, ) .fetch(&mut transaction); @@ -508,28 +498,25 @@ async fn run_scan( detail.project_id, BOOL_OR( detail.status IN ('pending', 'unsafe') - AND issue.issue_type != $2 AND NOT detail.hidden ) AS old_needs_review, BOOL_OR( detail.status IN ('pending', 'unsafe') - AND issue.issue_type != $2 AND NOT COALESCE(new_effect.hidden, FALSE) ) AS new_needs_review FROM delphi_issue_details_with_statuses detail - INNER JOIN delphi_report_issues issue - ON issue.id = detail.issue_id LEFT JOIN delphi_rule_effects new_effect ON new_effect.revision = $1 AND new_effect.detail_id = detail.id GROUP BY detail.project_id ) - SELECT project_id AS "project_id!: DBProjectId" + SELECT + project_id AS "project_id!: DBProjectId", + new_needs_review AS "new_needs_review!" FROM project_membership WHERE old_needs_review IS DISTINCT FROM new_needs_review "#, revision, - DUMMY_ISSUE_TYPE, ) .fetch_all(&mut transaction) .await @@ -547,19 +534,25 @@ async fn run_scan( .await .wrap_err("failed to publish the delphi rule revision")?; - tech_review_sync::sync_project_tech_review_state( + tech_review_queue::add_projects( &affected_projects .iter() + .filter(|project| project.new_needs_review) .map(|project| project.project_id) .collect::>(), - TechReviewExitReason::RulesChanged, &mut transaction, ) - .await - .map_err(|error| { - eyre!(error) - .wrap_err("failed to sync projects affected by delphi rule changes") - })?; + .await?; + tech_review_queue::remove_projects( + &affected_projects + .iter() + .filter(|project| !project.new_needs_review) + .map(|project| project.project_id) + .collect::>(), + TechReviewRemovalReason::RulesChanged, + &mut transaction, + ) + .await?; sqlx::query!( "DELETE FROM delphi_rule_effects WHERE revision <> $1", @@ -634,13 +627,10 @@ pub(crate) async fn materialize_current_rule_effects( FROM hashes WHERE hashes.file_id = file.id ) file_hashes ON TRUE - WHERE - detail.id = ANY($1::bigint[]) - AND issue.issue_type != $2 + WHERE detail.id = ANY($1::bigint[]) ORDER BY detail.id "#, &detail_ids.iter().map(|id| id.0).collect::>(), - DUMMY_ISSUE_TYPE, ) .fetch_all(&mut *transaction) .await @@ -702,7 +692,7 @@ async fn fetch_compiled_rules( SELECT id, rule FROM delphi_rules WHERE NOT delete_on_next_revision - ORDER BY id + ORDER BY priority DESC, id "#, ) .fetch_all(&mut *transaction) diff --git a/apps/labrinth/src/routes/v3/projects.rs b/apps/labrinth/src/routes/v3/projects.rs index 578cd74e3f..7fe539fc98 100644 --- a/apps/labrinth/src/routes/v3/projects.rs +++ b/apps/labrinth/src/routes/v3/projects.rs @@ -2746,8 +2746,9 @@ pub async fn project_delete_internal( .begin() .await .wrap_internal_err("failed to start transaction")?; - delphi::tech_review_sync::sync_deleted_project_tech_review_exit( - project.inner.id, + delphi::tech_review_queue::remove_projects( + &[project.inner.id], + delphi::tech_review_queue::TechReviewRemovalReason::FileDeleted, &mut transaction, ) .await?; diff --git a/apps/labrinth/src/routes/v3/version_file.rs b/apps/labrinth/src/routes/v3/version_file.rs index ad206ab352..045c924136 100644 --- a/apps/labrinth/src/routes/v3/version_file.rs +++ b/apps/labrinth/src/routes/v3/version_file.rs @@ -934,9 +934,9 @@ pub async fn delete_file( database::models::version_item::cleanup_unused_attribution_files_and_groups(&mut transaction) .await?; - delphi::tech_review_sync::sync_project_tech_review_state( + delphi::tech_review_queue::remove_projects_without_details( &[row.project_id], - delphi::tech_review_sync::TechReviewExitReason::FileDeleted, + delphi::tech_review_queue::TechReviewRemovalReason::FileDeleted, &mut transaction, ) .await?; diff --git a/apps/labrinth/src/routes/v3/versions.rs b/apps/labrinth/src/routes/v3/versions.rs index 9d4f33e602..5dd7b51299 100644 --- a/apps/labrinth/src/routes/v3/versions.rs +++ b/apps/labrinth/src/routes/v3/versions.rs @@ -1237,9 +1237,9 @@ pub async fn version_delete( ) .await?; - delphi::tech_review_sync::sync_project_tech_review_state( + delphi::tech_review_queue::remove_projects_without_details( &[version.inner.project_id], - delphi::tech_review_sync::TechReviewExitReason::FileDeleted, + delphi::tech_review_queue::TechReviewRemovalReason::FileDeleted, &mut transaction, ) .await?; diff --git a/packages/api-client/src/modules/labrinth/types.ts b/packages/api-client/src/modules/labrinth/types.ts index 3089099050..ddb1743be5 100644 --- a/packages/api-client/src/modules/labrinth/types.ts +++ b/packages/api-client/src/modules/labrinth/types.ts @@ -2230,6 +2230,7 @@ export namespace Labrinth { id: number name: string rule: string + priority: number revision: number created_at: string updated_at: string @@ -2260,6 +2261,7 @@ export namespace Labrinth { export type WriteDelphiRule = { name: string rule: string + priority: number } export type TestDelphiRuleTrace = {