diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1f22c84..72adafb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,14 @@
# Changelog
+## [0.9.48] - 2026-09-01
+
+### Added
+- **Queue/TaskStore integration for webhook-triggered reviews**: MR reviews triggered by GitLab webhooks are now visible in the live queue and history. Each webhook creates a task entry that tracks `pending` → `running` → `completed`/`failed`, broadcasts SSE events, and surfaces on the Queue Monitor and Recent Reviews dashboard. (`src/server/state.rs`, `src/server/gitlab/hooks.rs`, `src/server/task_queue.rs`, `src/server/api/dashboard.rs`, `src/server/api/queue.rs`)
+- **System hook verification fallback for single platforms**: when a GitLab System Hook payload carries no URL, or its URL does not match any configured platform, verification succeeds if exactly one platform has webhook credentials. This handles the GitLab "Test" button and payloads from instances with a single reachable endpoint. (`src/server/gitlab/handler.rs`)
+
+### Fixed
+- **Queue Monitor no longer appears blank when only completed tasks exist**: completed tasks are now shown under a "Recently completed" section, and the empty-state check counts all tasks instead of only active ones. (`frontend/src/views/QueueMonitor.vue`)
+
## [0.9.47] - 2026-09-01
### Added
diff --git a/Cargo.lock b/Cargo.lock
index a1dc2a0..2bfa3c7 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1840,7 +1840,7 @@ dependencies = [
[[package]]
name = "review-engine"
-version = "0.9.47"
+version = "0.9.48"
dependencies = [
"anyhow",
"async-trait",
diff --git a/Cargo.toml b/Cargo.toml
index 0506cf8..aabe854 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "review-engine"
-version = "0.9.47"
+version = "0.9.48"
license = "Apache-2.0"
edition = "2021"
diff --git a/frontend/src/i18n/locales/en.ts b/frontend/src/i18n/locales/en.ts
index bd75990..9ac3c21 100644
--- a/frontend/src/i18n/locales/en.ts
+++ b/frontend/src/i18n/locales/en.ts
@@ -293,6 +293,7 @@ export default {
queued: 'Queued Tasks',
failed: 'Failed Tasks',
cancelled: 'Cancelled Tasks',
+ completed: 'Recently Completed Tasks',
},
empty: 'No tasks in queue',
emptyHint: 'The queue is currently empty. New tasks will appear here.',
diff --git a/frontend/src/i18n/locales/fr.ts b/frontend/src/i18n/locales/fr.ts
index 806dcf0..cf677ac 100644
--- a/frontend/src/i18n/locales/fr.ts
+++ b/frontend/src/i18n/locales/fr.ts
@@ -285,6 +285,7 @@ export default {
queued: 'Tâches en file',
failed: 'Tâches en échec',
cancelled: 'Tâches annulées',
+ completed: 'Tâches récemment terminées',
},
empty: 'Aucune tâche dans la file',
emptyHint: 'La file est actuellement vide. Les nouvelles tâches apparaîtront ici.',
diff --git a/frontend/src/i18n/locales/ja.ts b/frontend/src/i18n/locales/ja.ts
index 0d2ac0d..065ac09 100644
--- a/frontend/src/i18n/locales/ja.ts
+++ b/frontend/src/i18n/locales/ja.ts
@@ -284,6 +284,7 @@ export default {
queued: '待機タスク',
failed: '失敗タスク',
cancelled: 'キャンセル済みタスク',
+ completed: '最近完了タスク',
},
empty: 'キューにタスクがありません',
emptyHint: 'キューは現在空です。新しいタスクはここに表示されます。',
diff --git a/frontend/src/i18n/locales/ko.ts b/frontend/src/i18n/locales/ko.ts
index 03c5ad2..3f9def8 100644
--- a/frontend/src/i18n/locales/ko.ts
+++ b/frontend/src/i18n/locales/ko.ts
@@ -283,6 +283,7 @@ export default {
queued: '대기 중 태스크',
failed: '실패한 태스크',
cancelled: '취소된 태스크',
+ completed: '최근 완료된 태스크',
},
empty: '대기열에 태스크가 없습니다',
emptyHint: '대기열이 비어 있습니다. 새 태스크가 여기에 표시됩니다.',
diff --git a/frontend/src/i18n/locales/zh-CN.ts b/frontend/src/i18n/locales/zh-CN.ts
index ae9cd06..0829315 100644
--- a/frontend/src/i18n/locales/zh-CN.ts
+++ b/frontend/src/i18n/locales/zh-CN.ts
@@ -279,6 +279,7 @@ export default {
queued: '排队任务',
failed: '失败任务',
cancelled: '已取消任务',
+ completed: '最近完成任务',
},
empty: '队列中没有任务',
emptyHint: '队列当前为空。新任务将显示在这里。',
diff --git a/frontend/src/i18n/locales/zh-TW.ts b/frontend/src/i18n/locales/zh-TW.ts
index b49b869..042558c 100644
--- a/frontend/src/i18n/locales/zh-TW.ts
+++ b/frontend/src/i18n/locales/zh-TW.ts
@@ -279,6 +279,7 @@ export default {
queued: '排隊任務',
failed: '失敗任務',
cancelled: '已取消任務',
+ completed: '最近完成任務',
},
empty: '佇列中沒有任務',
emptyHint: '佇列目前為空。新任務會顯示在這裡。',
diff --git a/frontend/src/views/QueueMonitor.vue b/frontend/src/views/QueueMonitor.vue
index 386b0f5..4a4ba53 100644
--- a/frontend/src/views/QueueMonitor.vue
+++ b/frontend/src/views/QueueMonitor.vue
@@ -59,7 +59,13 @@ const activeTasks = computed(() => queue.items.value.filter((t: QueueTask) => t.
const queuedTasks = computed(() => queue.items.value.filter((t: QueueTask) => t.status === 'queued'))
const failedTasks = computed(() => queue.items.value.filter((t: QueueTask) => t.status === 'failed'))
const cancelledTasks = computed(() => queue.items.value.filter((t: QueueTask) => t.status === 'cancelled'))
-const allTasks = computed(() => queue.items.value)
+const completedTasks = computed(() => queue.items.value.filter((t: QueueTask) => t.status === 'completed'))
+// The backend returns every status (including `completed`) when no status
+// filter is passed. Completed tasks are rendered in their own section below,
+// so the "no tasks" placeholder must cover the full list — otherwise a
+// completed-only result suppresses the placeholder while the four active
+// sections stay empty, leaving a blank page.
+const hasAnyTasks = computed(() => queue.items.value.length > 0)
// --- Load queue data ---
const loadQueueData = async () => {
@@ -461,8 +467,33 @@ onUnmounted(() => {
+
+
+
+
+
+
+
+
-
+
diff --git a/src/cli/app.rs b/src/cli/app.rs
index 52e01c1..c734894 100644
--- a/src/cli/app.rs
+++ b/src/cli/app.rs
@@ -261,7 +261,6 @@ pub async fn run() -> Result<()> {
.or_else(|| std::env::var("GITLAB_WEBHOOK_SIGNING_SECRET").ok())
.filter(|s| !s.is_empty());
let mut app_state = review_engine::server::AppState::new(config.llm.clone());
- app_state.task_store = Some(Arc::new(review_engine::server::task_queue::TaskStore::new()));
app_state.app_config = std::sync::RwLock::new(Some(Arc::new(config.clone())));
app_state.registry = Some(review_engine::metrics::REGISTRY.clone());
app_state.progress_map = Some(progress_map.clone());
diff --git a/src/server/api/dashboard.rs b/src/server/api/dashboard.rs
index f03bca3..060f262 100644
--- a/src/server/api/dashboard.rs
+++ b/src/server/api/dashboard.rs
@@ -262,4 +262,43 @@ mod tests {
assert_eq!(by_id["00000000-0000-0000-0000-000000000004"], "failed");
assert_eq!(by_id["00000000-0000-0000-0000-000000000005"], "cancelled");
}
+
+ /// `None` fallback: without a store the dashboard serves documented
+ /// defaults (zero KPIs, empty recent reviews, offline health) instead of
+ /// 503 — a pure guard, unreachable in production where the store is
+ /// always present.
+ #[tokio::test]
+ async fn dashboard_without_store_serves_defaults() {
+ let mut state = AppState::new(vec![]);
+ state.task_store = None;
+ let resp = get_dashboard(State(std::sync::Arc::new(state))).await.into_response();
+ assert_eq!(resp.status(), axum::http::StatusCode::OK);
+ let body = axum::body::to_bytes(resp.into_body(), 1024 * 1024).await.unwrap();
+ let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
+ assert_eq!(json["kpis"]["reviewsThisWeek"], 0);
+ assert_eq!(json["kpis"]["activeQueue"], 0);
+ assert!(json["recentReviews"].as_array().unwrap().is_empty());
+ assert_eq!(json["health"]["overall"], "offline");
+ }
+
+ /// With a store the dashboard reflects the real task store — completed
+ /// reviews from the webhook path surface as recentReviews (the exact
+ /// defect this fix closes).
+ #[tokio::test]
+ async fn dashboard_with_store_shows_webhook_recorded_review() {
+ let state = AppState::new(vec![]);
+ let store = state.task_store.clone().unwrap();
+ let id = store.create(None).await;
+ store.update(id, TaskState::Completed, None, None).await;
+
+ let resp = get_dashboard(State(std::sync::Arc::new(state))).await.into_response();
+ assert_eq!(resp.status(), axum::http::StatusCode::OK);
+ let body = axum::body::to_bytes(resp.into_body(), 1024 * 1024).await.unwrap();
+ let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
+ assert_eq!(json["kpis"]["reviewsThisWeek"], 1);
+ let recent = json["recentReviews"].as_array().unwrap();
+ assert_eq!(recent.len(), 1);
+ assert_eq!(recent[0]["id"], id.to_string());
+ assert_eq!(recent[0]["status"], "completed");
+ }
}
diff --git a/src/server/api/queue.rs b/src/server/api/queue.rs
index 014b0ee..b481194 100644
--- a/src/server/api/queue.rs
+++ b/src/server/api/queue.rs
@@ -227,3 +227,110 @@ fn task_to_queue_task(entry: &TaskEntry) -> serde_json::Value {
"errorMessage": entry.error,
})
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use axum::response::IntoResponse;
+
+ /// AppState with the store deliberately cleared — the only way the
+ /// `None` fallback paths are reachable now that `AppState::new`
+ /// initialises a store eagerly.
+ fn state_without_store() -> Arc {
+ let mut state = AppState::new(vec![]);
+ state.task_store = None;
+ Arc::new(state)
+ }
+
+ fn state_with_store() -> Arc {
+ Arc::new(AppState::new(vec![]))
+ }
+
+ async fn body_of(response: impl IntoResponse) -> serde_json::Value {
+ let resp = response.into_response();
+ let bytes = axum::body::to_bytes(resp.into_body(), 1024 * 1024).await.unwrap();
+ serde_json::from_slice(&bytes).unwrap()
+ }
+
+ /// `None` fallback: the stats endpoint keeps serving the documented
+ /// defaults instead of 503 — a pure guard, unreachable in production
+ /// where the store is always present.
+ #[tokio::test]
+ async fn stats_without_store_falls_back_to_defaults() {
+ let json = body_of(get_queue_stats(State(state_without_store())).await).await;
+ assert_eq!(json["active"], 0);
+ assert_eq!(json["queued"], 0);
+ assert_eq!(json["failed"], 0);
+ assert_eq!(json["totalDepth"], 0);
+ assert_eq!(json["maxConcurrent"], 8);
+ assert_eq!(json["queueCapacity"], 16);
+ assert_eq!(json["failedLast24h"], 0);
+ assert_eq!(json["totalLast24h"], 0);
+ assert_eq!(json["isPaused"], false);
+ }
+
+ /// With a store the stats reflect the real task store — the fix that makes
+ /// the queue page show actual activity instead of hardcoded zeros.
+ #[tokio::test]
+ async fn stats_with_store_reflects_real_entries() {
+ let state = state_with_store();
+ let store = state.task_store.clone().unwrap();
+ let id = store.create(None).await;
+ store.update(id, TaskState::Running, None, None).await;
+ let failed = store.create(None).await;
+ store
+ .update(failed, TaskState::Failed, None, Some("boom".to_string()))
+ .await;
+
+ let json = body_of(get_queue_stats(State(state)).await).await;
+ assert_eq!(json["active"], 1);
+ assert_eq!(json["queued"], 0);
+ assert_eq!(json["failed"], 1);
+ assert_eq!(json["totalDepth"], 1);
+ assert_eq!(json["maxConcurrent"], 8);
+ assert_eq!(json["queueCapacity"], 16);
+ assert_eq!(json["failedLast24h"], 1);
+ assert_eq!(json["totalLast24h"], 2);
+ }
+
+ #[tokio::test]
+ async fn tasks_without_store_returns_503() {
+ let resp = get_queue_tasks(
+ State(state_without_store()),
+ Query(QueueTaskParams {
+ status: None,
+ page: None,
+ per_page: None,
+ }),
+ )
+ .await
+ .into_response();
+ assert_eq!(resp.status(), StatusCode::SERVICE_UNAVAILABLE);
+ let json = body_of(resp).await;
+ assert_eq!(json["error"], "task store not initialized");
+ }
+
+ #[tokio::test]
+ async fn tasks_with_store_returns_real_entries() {
+ let state = state_with_store();
+ let store = state.task_store.clone().unwrap();
+ let id = store.create(None).await;
+ store.update(id, TaskState::Completed, None, None).await;
+
+ let json = body_of(
+ get_queue_tasks(
+ State(state),
+ Query(QueueTaskParams {
+ status: None,
+ page: None,
+ per_page: None,
+ }),
+ )
+ .await,
+ )
+ .await;
+ assert_eq!(json["total"], 1);
+ assert_eq!(json["items"][0]["id"], id.to_string());
+ assert_eq!(json["items"][0]["status"], "completed");
+ }
+}
diff --git a/src/server/api/review/tests.rs b/src/server/api/review/tests.rs
index 65b039c..ba8e775 100644
--- a/src/server/api/review/tests.rs
+++ b/src/server/api/review/tests.rs
@@ -456,6 +456,51 @@ async fn test_list_reviews_merges_camelcase_and_keeps_snakecase() {
assert!(item.get("rawComment").is_none(), "list items must not carry rawComment");
}
+/// The exact webhook defect: a webhook-dispatched review records a task entry
+/// (via `record_task_started` / `record_task_outcome` — the same helpers the
+/// GitLab hook path uses) that then surfaces in `/api/v1/reviews`. Before the
+/// TaskStore wiring, webhook reviews never created an entry, so this list was
+/// empty no matter how many MRs were reviewed.
+#[tokio::test]
+async fn webhook_recorded_task_surfaces_in_list_reviews() {
+ let state = state_with_store();
+ let store = state.task_store.clone().unwrap();
+
+ // Enqueue-time knowledge is only the MR URL + commit SHA (the dispatch
+ // dedup key); title/branch/author are absent for webhook tasks.
+ let mr_url = "http://gitlab.internal:8929/group/proj/-/merge_requests/7";
+ let sha = "abc123";
+ let id = crate::server::gitlab::record_task_started(&store, mr_url, sha).await;
+ let outcome: anyhow::Result<()> = Ok(());
+ crate::server::gitlab::record_task_outcome(&store, id, mr_url, sha, &outcome).await;
+
+ let params = ListParams {
+ status: None,
+ page: None,
+ per_page: None,
+ q: None,
+ project: None,
+ repository: None,
+ date_from: None,
+ date_to: None,
+ };
+ let resp = list_reviews(State(state), Query(params)).await.into_response();
+ assert_eq!(resp.status(), StatusCode::OK);
+ let body = axum::body::to_bytes(resp.into_body(), 1024 * 1024).await.unwrap();
+ let json: serde_json::Value = serde_json::from_slice(&body).unwrap();
+
+ assert_eq!(json["total"], 1, "the webhook-recorded task must be listed");
+ let item = &json["items"][0];
+ assert_eq!(item["task_id"], id.to_string());
+ assert_eq!(item["status"], "completed");
+ assert_eq!(item["gitlab_mr_url"], mr_url);
+ assert_eq!(item["commit_sha"], sha);
+ assert_eq!(
+ item["gitlabMrUrl"], mr_url,
+ "camelCase key must mirror the snake_case one"
+ );
+}
+
#[tokio::test]
async fn test_absent_metadata_is_null_in_both_naming_schemes() {
let state = state_with_store();
diff --git a/src/server/gitlab/handler.rs b/src/server/gitlab/handler.rs
index df68269..19f5e87 100644
--- a/src/server/gitlab/handler.rs
+++ b/src/server/gitlab/handler.rs
@@ -5,6 +5,7 @@ use base64::Engine;
use hmac::{Hmac, Mac};
use serde_json::Value;
use sha2::Sha256;
+use std::sync::Arc;
use super::super::dispatcher::MrDispatcher;
use super::super::webhook::WebhookHandler;
@@ -72,6 +73,25 @@ pub(crate) fn system_hook_event_name(body: &str) -> String {
.to_string()
}
+/// The "unique verification platform" fallback: when EXACTLY ONE configured
+/// git platform can verify webhooks ([`crate::models::GitPlatformConfig::has_webhook_verification`]),
+/// return it; `None` when zero or multiple entries are verification-capable
+/// (or the list is empty). Used when a payload carries no instance URL —
+/// GitLab System Hooks' **Test** button sends a sample payload with no
+/// `project`/URL at all — or its URL matched no platform: a single
+/// verification-capable entry is unambiguous, so it safely takes over; zero
+/// or multiple entries keep the runtime default — never guess.
+pub(crate) fn unique_verification_platform(
+ platforms: &[crate::models::GitPlatformConfig],
+) -> Option {
+ let mut verification = platforms.iter().filter(|p| p.has_webhook_verification());
+ let hit = verification.next()?.clone();
+ if verification.next().is_some() {
+ return None;
+ }
+ Some(hit)
+}
+
impl GitLabWebhookHandler {
/// Return the effective runtime config: if the global was initialised
/// (e.g. from the UI) use that, otherwise fall back to `self.*` so
@@ -88,50 +108,86 @@ impl GitLabWebhookHandler {
.unwrap_or_else(|| GitLabRuntimeConfig::from_handler(self))
}
- /// Return the effective config for a specific webhook body: when the
- /// payload's project/repository URL host matches a configured git
- /// platform, THAT platform's webhook_secret / signing_secret / token
- /// take over both verification and review dispatch; otherwise the
- /// runtime default applies.
+ /// Return the effective config for a specific webhook body.
///
- /// A matched platform WITHOUT any webhook verification credential
- /// (token-only entry, configured for REST review routing) deliberately
- /// does not take over: webhooks for its host keep the runtime default,
- /// so adding an instance for routing never breaks an existing webhook
- /// setup for the same host.
+ /// Resolution order:
+ /// 1. **URL match** — when the payload carries an instance URL
+ /// (`project.web_url` / `project.homepage` / `repository.homepage` /
+ /// `object_attributes.url`) and it matches a configured git platform
+ /// (strict host[:port], with `find_git_platform_for_url`'s unique-host
+ /// port fold), THAT platform's webhook_secret / signing_secret / token
+ /// take over both verification and review dispatch. Multi-platform
+ /// semantics are unchanged. A URL-matched entry WITHOUT webhook
+ /// verification (token-only, configured for REST review routing)
+ /// deliberately does not take over — webhooks for its host keep the
+ /// runtime default, so adding an instance for routing never breaks an
+ /// existing webhook setup for the same host.
+ /// 2. **Unique verification platform fallback** — when the payload has no
+ /// instance URL, or its URL matched no platform (host mismatch, e.g.
+ /// `base_url` = `host.docker.internal` while the payload carries
+ /// `localhost`), exactly ONE verification-capable platform
+ /// (`unique_verification_platform`) unambiguously takes over. This is
+ /// what makes GitLab System Hooks' **Test** button work: it sends a
+ /// sample payload with no URL, which used to resolve to no platform →
+ /// 403 "no verification configured". Zero or multiple
+ /// verification-capable entries keep the runtime default — never guess.
pub(crate) fn effective_config_for_body(&self, body: &str) -> GitLabRuntimeConfig {
let fallback = self.effective_config();
let Some(state) = self.app_state.as_ref().and_then(|w| w.upgrade()) else {
return fallback;
};
- let Some(url) = payload_instance_url(body) else {
- return fallback;
- };
let platforms = state.git_platforms.read().unwrap().clone();
- match crate::models::find_git_platform_for_url(&platforms, &url) {
- Some(platform) if platform.has_webhook_verification() => {
- tracing::debug!(platform = %platform.name, "gitlab webhook matched configured git platform");
- GitLabRuntimeConfig::from_platform(platform)
+ // A payload carrying an instance URL resolves strictly by URL: the
+ // URL-matched verification-capable platform wins regardless of how
+ // many entries exist; a token-only URL match keeps the runtime default
+ // (the host has no verification setup on purpose — never the fallback).
+ if let Some(url) = payload_instance_url(body) {
+ if let Some(platform) = crate::models::find_git_platform_for_url(&platforms, &url) {
+ if platform.has_webhook_verification() {
+ tracing::debug!(platform = %platform.name, "gitlab webhook matched configured git platform");
+ return GitLabRuntimeConfig::from_platform(platform);
+ }
+ return fallback;
}
- _ => fallback,
}
+ // No URL, or the URL matched no platform: when exactly ONE platform can
+ // verify webhooks it unambiguously takes over; zero or multiple keep
+ // the runtime default — never guess.
+ if let Some(platform) = unique_verification_platform(&platforms) {
+ tracing::debug!(platform = %platform.name, "gitlab webhook fell back to unique verification platform");
+ return GitLabRuntimeConfig::from_platform(&platform);
+ }
+ fallback
}
- /// Return the configured git platform matched by `body`'s instance URL,
- /// but only when that platform can verify webhooks (same rule as
- /// `effective_config_for_body`: token-only entries do not take over).
- /// `None` when the body carries no instance URL, no platform matches, or
- /// the match lacks verification credentials. Used by `handle_event` to
- /// resolve the matched platform's `allowed_projects` allowlist AND its
- /// `base_url` (for rewriting payload MR URLs onto the reachable endpoint
- /// before review dispatch).
+ /// Return the configured git platform for `body`'s webhook, or `None`.
+ ///
+ /// Mirrors `effective_config_for_body`'s resolution order so the platform
+ /// driving the `allowed_projects` allowlist AND its `base_url` (rewriting
+ /// payload MR URLs onto the reachable endpoint before review dispatch) is
+ /// the same one whose credentials verified the body:
+ /// 1. URL-matched verification-capable platform (`None` when the match is
+ /// token-only — it must not take over);
+ /// 2. else the unique verification platform fallback
+ /// (`unique_verification_platform`), so a no-URL payload (System Hooks
+ /// **Test** button) or a host-mismatch payload (localhost vs
+ /// `host.docker.internal`) still resolves to the single
+ /// verification-capable entry and its allowlist + reachable URL.
+ /// `None` when there is no AppState, no platform matched and no unique
+ /// verification platform exists → empty allowlist (every project allowed)
+ /// and the payload URL kept verbatim (legacy).
pub(crate) fn matched_platform(&self, body: &str) -> Option {
let state = self.app_state.as_ref()?.upgrade()?;
- let url = payload_instance_url(body)?;
let platforms = state.git_platforms.read().unwrap().clone();
- crate::models::find_git_platform_for_url(&platforms, &url)
- .filter(|p| p.has_webhook_verification())
- .cloned()
+ if let Some(url) = payload_instance_url(body) {
+ if let Some(platform) = crate::models::find_git_platform_for_url(&platforms, &url) {
+ if platform.has_webhook_verification() {
+ return Some(platform.clone());
+ }
+ return None;
+ }
+ }
+ unique_verification_platform(&platforms)
}
/// Create a new GitLab webhook handler.
@@ -313,13 +369,14 @@ impl GitLabWebhookHandler {
body: &str,
token: &str,
platform: Option,
+ task_store: Option>,
) -> Result, (StatusCode, Json)> {
let event_name = system_hook_event_name(body);
match event_name.as_str() {
- "merge_request" => super::handle_mr_hook(body, &self.dispatcher, token, platform)
+ "merge_request" => super::handle_mr_hook(body, &self.dispatcher, token, platform, task_store.clone())
.await
.map_err(|status| (status, Json(serde_json::json!({"error": "request failed"})))),
- "note" => super::handle_note_hook(body, &self.dispatcher, token, platform)
+ "note" => super::handle_note_hook(body, &self.dispatcher, token, platform, task_store.clone())
.await
.map_err(|status| (status, Json(serde_json::json!({"error": "request failed"})))),
"push" => super::handle_push_hook(body)
@@ -399,18 +456,26 @@ impl WebhookHandler for GitLabWebhookHandler {
// no platform matched (or none can verify) → empty allowlist (every
// project allowed) and the payload URL kept verbatim (legacy).
let platform = self.matched_platform(body);
+ // The shared task store (weak-handled via AppState) so webhook-dispatched
+ // reviews record a task entry: create → running → completed/failed. `None`
+ // in tests and legacy paths — the review still runs, just without a record.
+ let task_store = self
+ .app_state
+ .as_ref()
+ .and_then(|w| w.upgrade())
+ .and_then(|s| s.task_store.clone());
match event {
- "Merge Request Hook" => super::handle_mr_hook(body, &self.dispatcher, &token, platform)
+ "Merge Request Hook" => super::handle_mr_hook(body, &self.dispatcher, &token, platform, task_store.clone())
.await
.map_err(|status| (status, Json(serde_json::json!({"error": "request failed"})))),
- "Note Hook" => super::handle_note_hook(body, &self.dispatcher, &token, platform)
+ "Note Hook" => super::handle_note_hook(body, &self.dispatcher, &token, platform, task_store.clone())
.await
.map_err(|status| (status, Json(serde_json::json!({"error": "request failed"})))),
"Push Hook" => super::handle_push_hook(body)
.await
.map_err(|status| (status, Json(serde_json::json!({"error": "request failed"})))),
- "System Hook" => self.handle_system_hook(body, &token, platform).await,
+ "System Hook" => self.handle_system_hook(body, &token, platform, task_store).await,
_ => {
tracing::debug!("Ignoring unsupported GitLab event: {}", event);
Ok(Json(serde_json::json!({ "status": "ignored" })))
diff --git a/src/server/gitlab/hooks.rs b/src/server/gitlab/hooks.rs
index 4e3eeb6..3499258 100644
--- a/src/server/gitlab/hooks.rs
+++ b/src/server/gitlab/hooks.rs
@@ -1,7 +1,10 @@
use axum::{http::StatusCode, Json};
use serde_json::Value;
+use std::sync::Arc;
+use uuid::Uuid;
use super::super::dispatcher::MrDispatcher;
+use crate::server::task_queue::{SourceMeta, TaskState, TaskStore};
/// Parsed payload from a GitLab Merge Request webhook event.
pub struct MrHookPayload {
@@ -54,14 +57,104 @@ pub fn parse_mr_hook_payload(body: &str, gitlab_token: &str) -> Result Uuid {
+ let meta = SourceMeta {
+ gitlab_mr_url: Some(mr_url.to_string()),
+ commit_sha: Some(sha.to_string()),
+ ..SourceMeta::default()
+ };
+ let task_id = store.create(Some(meta)).await;
+ store.start(task_id).await;
+ task_id
+}
+
+/// Record the outcome of a webhook-dispatched MR review in the task store.
+///
+/// `Ok` → `Completed` with a result summary (MR URL + SHA); `Err` → `Failed`
+/// with the error message. `completed_at` is set by [`TaskStore::update`].
+pub async fn record_task_outcome(
+ store: &TaskStore,
+ task_id: Uuid,
+ mr_url: &str,
+ sha: &str,
+ outcome: &anyhow::Result<()>,
+) {
+ match outcome {
+ Ok(()) => {
+ store
+ .update(
+ task_id,
+ TaskState::Completed,
+ Some(serde_json::json!({
+ "mr_url": mr_url,
+ "sha": sha,
+ "summary": "review completed",
+ })),
+ None,
+ )
+ .await;
+ }
+ Err(e) => {
+ store
+ .update(task_id, TaskState::Failed, None, Some(format!("{e:#}")))
+ .await;
+ }
+ }
+}
+
+/// Execute a webhook-dispatched MR review on a detached task, recording its
+/// lifecycle in the task store when one is available.
+///
+/// With a store: creates a Running task entry (MR URL + commit SHA), runs the
+/// review, then marks it Completed (with a summary result) or Failed (with the
+/// error message). Without a store this is exactly the legacy behavior — run,
+/// log, and release the dispatcher's dedup on failure.
+async fn run_webhook_review(
+ task_store: Option>,
+ dispatcher: &MrDispatcher,
+ mr_url: String,
+ sha: String,
+ gitlab_token: String,
+ mr_iid: u64,
+) {
+ let task_id = if let Some(store) = task_store.as_ref() {
+ Some(record_task_started(store, &mr_url, &sha).await)
+ } else {
+ None
+ };
+
+ let outcome = run_review_for_mr(&mr_url, &gitlab_token, Some(dispatcher), Some(&mr_url), Some(&sha)).await;
+ if let Err(e) = &outcome {
+ tracing::error!("Review failed for MR !{}: {:?}", mr_iid, e);
+ dispatcher.reset(&mr_url).await;
+ }
+
+ if let (Some(store), Some(id)) = (task_store.as_ref(), task_id) {
+ record_task_outcome(store, id, &mr_url, &sha, &outcome).await;
+ }
+}
+
+/// Spawn a background task that runs the full review for an MR, recording its
+/// lifecycle in the task store when one is available.
+pub fn spawn_mr_review_task(
+ dispatcher: &MrDispatcher,
+ mr_url: String,
+ sha: String,
+ gitlab_token: String,
+ mr_iid: u64,
+ task_store: Option>,
+) {
let d = dispatcher.clone();
tokio::spawn(async move {
- if let Err(e) = run_review_for_mr(&mr_url, &gitlab_token, Some(&d), Some(&mr_url), Some(&sha)).await {
- tracing::error!("Review failed for MR !{}: {:?}", mr_iid, e);
- d.reset(&mr_url).await;
- }
+ run_webhook_review(task_store, &d, mr_url, sha, gitlab_token, mr_iid).await;
});
}
@@ -72,6 +165,7 @@ pub async fn handle_mr_in_progress(
sha: &str,
gitlab_token: &str,
mr_iid: u64,
+ task_store: Option>,
) {
tracing::info!("MR !{} review in progress, waiting...", mr_iid);
dispatcher.wait(mr_url).await;
@@ -84,6 +178,7 @@ pub async fn handle_mr_in_progress(
sha.to_string(),
gitlab_token.to_string(),
mr_iid,
+ task_store,
);
}
_ => {
@@ -94,7 +189,14 @@ pub async fn handle_mr_in_progress(
/// Dispatch an MR webhook event to start or defer a review based on the
/// dispatcher state.
-pub async fn dispatch_mr_event(dispatcher: &MrDispatcher, mr_url: &str, sha: &str, gitlab_token: &str, mr_iid: u64) {
+pub async fn dispatch_mr_event(
+ dispatcher: &MrDispatcher,
+ mr_url: &str,
+ sha: &str,
+ gitlab_token: &str,
+ mr_iid: u64,
+ task_store: Option>,
+) {
match dispatcher.try_start(mr_url, sha).await {
super::super::dispatcher::ShouldStart::Go => {
spawn_mr_review_task(
@@ -103,13 +205,14 @@ pub async fn dispatch_mr_event(dispatcher: &MrDispatcher, mr_url: &str, sha: &st
sha.to_string(),
gitlab_token.to_string(),
mr_iid,
+ task_store,
);
}
super::super::dispatcher::ShouldStart::AlreadyReviewed => {
tracing::info!("Skipping MR !{}: already reviewed at SHA {}", mr_iid, sha);
}
super::super::dispatcher::ShouldStart::InProgress => {
- handle_mr_in_progress(dispatcher, mr_url, sha, gitlab_token, mr_iid).await;
+ handle_mr_in_progress(dispatcher, mr_url, sha, gitlab_token, mr_iid, task_store).await;
}
}
}
@@ -167,6 +270,7 @@ pub async fn handle_mr_hook(
dispatcher: &MrDispatcher,
gitlab_token: &str,
platform: Option,
+ task_store: Option>,
) -> Result, StatusCode> {
let payload = parse_mr_hook_payload(body, gitlab_token)?;
@@ -224,6 +328,7 @@ pub async fn handle_mr_hook(
&payload.sha,
&payload.gitlab_token,
payload.mr_iid,
+ task_store,
)
.await;
}
@@ -281,6 +386,7 @@ pub async fn handle_note_hook(
dispatcher: &MrDispatcher,
gitlab_token: &str,
platform: Option,
+ task_store: Option>,
) -> Result, StatusCode> {
let parsed: Value = serde_json::from_str(body).map_err(|e| {
tracing::error!("Failed to parse Note hook: {}", e);
@@ -344,11 +450,9 @@ pub async fn handle_note_hook(
let d = dispatcher.clone();
let u = url;
let s = sha;
+ let note_iid = mr_iid;
tokio::spawn(async move {
- if let Err(e) = run_review_for_mr(&u, &token, Some(&d), Some(&u), Some(&s)).await {
- tracing::error!("Review from note failed: {:?}", e);
- d.reset(&u).await;
- }
+ run_webhook_review(task_store, &d, u, s, token, note_iid).await;
});
}
_ => {
@@ -376,3 +480,80 @@ pub async fn handle_push_hook(body: &str) -> Result, StatusCode> {
"status": "received",
})))
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use crate::server::task_queue::TaskStore;
+
+ const MR_URL: &str = "http://gitlab.internal:8929/group/proj/-/merge_requests/7";
+ const SHA: &str = "abc123";
+
+ #[tokio::test]
+ async fn record_task_started_creates_running_entry_with_mr_meta() {
+ let store = TaskStore::new();
+ let id = record_task_started(&store, MR_URL, SHA).await;
+
+ let entry = store.get(id).await.expect("task must exist");
+ assert_eq!(entry.state, TaskState::Running, "started review must be running");
+ assert!(entry.started_at.is_some(), "started review must record started_at");
+ assert_eq!(entry.source_meta.gitlab_mr_url.as_deref(), Some(MR_URL));
+ assert_eq!(entry.source_meta.commit_sha.as_deref(), Some(SHA));
+ // Enqueue-time metadata beyond URL+SHA is unknown for webhooks.
+ assert!(entry.source_meta.mr_title.is_none());
+ assert!(entry.source_meta.project.is_none());
+ }
+
+ #[tokio::test]
+ async fn record_task_outcome_success_marks_completed_with_summary() {
+ let store = TaskStore::new();
+ let id = record_task_started(&store, MR_URL, SHA).await;
+ let outcome: anyhow::Result<()> = Ok(());
+
+ record_task_outcome(&store, id, MR_URL, SHA, &outcome).await;
+
+ let entry = store.get(id).await.expect("task must exist");
+ assert_eq!(entry.state, TaskState::Completed);
+ assert!(entry.completed_at.is_some(), "completed task must record completed_at");
+ let result = entry
+ .result
+ .expect("completed webhook task must carry a result summary");
+ assert_eq!(result["mr_url"], MR_URL);
+ assert_eq!(result["sha"], SHA);
+ assert_eq!(result["summary"], "review completed");
+ assert!(entry.error.is_none());
+ }
+
+ #[tokio::test]
+ async fn record_task_outcome_failure_marks_failed_with_error() {
+ let store = TaskStore::new();
+ let id = record_task_started(&store, MR_URL, SHA).await;
+ let outcome: anyhow::Result<()> = Err(anyhow::anyhow!("provider unreachable"));
+
+ record_task_outcome(&store, id, MR_URL, SHA, &outcome).await;
+
+ let entry = store.get(id).await.expect("task must exist");
+ assert_eq!(entry.state, TaskState::Failed);
+ assert!(entry.completed_at.is_some(), "failed task must record completed_at");
+ assert!(entry.result.is_none());
+ let error = entry.error.expect("failed task must carry an error message");
+ assert!(error.contains("provider unreachable"), "got error: {error}");
+ }
+
+ /// The dispatcher dedups by URL+SHA before `spawn_mr_review_task`, so a
+ /// single actually-started review must record exactly one entry through
+ /// the full start → outcome cycle (never two).
+ #[tokio::test]
+ async fn full_cycle_records_exactly_one_entry() {
+ let store = TaskStore::new();
+ let id = record_task_started(&store, MR_URL, SHA).await;
+ let outcome: anyhow::Result<()> = Ok(());
+ record_task_outcome(&store, id, MR_URL, SHA, &outcome).await;
+
+ let (items, total) = store.list(None, 1, 100, None, None, None, None, None).await;
+ assert_eq!(total, 1, "one dispatch must record exactly one entry");
+ assert_eq!(items[0].task_id, id);
+ assert_eq!(items[0].state, TaskState::Completed);
+ assert_eq!(items[0].source_meta.commit_sha.as_deref(), Some(SHA));
+ }
+}
diff --git a/src/server/gitlab/mod.rs b/src/server/gitlab/mod.rs
index 5707c36..f1f32d1 100644
--- a/src/server/gitlab/mod.rs
+++ b/src/server/gitlab/mod.rs
@@ -17,7 +17,8 @@ pub use super::webhook::WebhookHandler;
pub use handler::GitLabWebhookHandler;
pub use hooks::{
dispatch_mr_event, handle_mr_hook, handle_mr_in_progress, handle_note_hook, handle_push_hook,
- note_starts_with_command, parse_mr_hook_payload, spawn_mr_review_task, MrHookPayload,
+ note_starts_with_command, parse_mr_hook_payload, record_task_outcome, record_task_started, spawn_mr_review_task,
+ MrHookPayload,
};
use std::sync::{OnceLock, RwLock};
diff --git a/src/server/gitlab/tests.rs b/src/server/gitlab/tests.rs
index 244bd23..29bff85 100644
--- a/src/server/gitlab/tests.rs
+++ b/src/server/gitlab/tests.rs
@@ -1,6 +1,7 @@
use super::super::dispatcher::{MrDispatcher, ShouldStart};
use super::handler::payload_instance_url;
use super::handler::system_hook_event_name;
+use super::handler::unique_verification_platform;
use super::handler::HmacSha256;
use super::hooks::{mr_iid_from_url, rewrite_url_to_platform};
use super::*;
@@ -703,17 +704,29 @@ async fn webhook_platform_match_selects_platform_config() {
}
/// An unmatched payload host (or a body without an instance URL) keeps the
-/// runtime default — the pre-multi-platform behavior.
+/// runtime default whenever the unique-verification fallback cannot decide:
+/// with MULTIPLE verification-capable platforms there is no single entry to
+/// fall back to, so the pre-multi-platform default applies (never guess).
#[tokio::test]
async fn webhook_unmatched_host_keeps_default_config() {
let _lock = super::RUNTIME_TEST_LOCK.lock().await;
let _guard = EmptyRuntimeGuard::new();
- let state = state_with_platforms(vec![platform_entry(
- "testbed",
- "http://gitlab.internal:8929",
- "glpat-platform",
- "platform-secret",
- )]);
+ // Two verification-capable platforms on unrelated hosts: an unmatched URL
+ // (or no URL at all) is ambiguous → runtime default, not either platform.
+ let state = state_with_platforms(vec![
+ platform_entry(
+ "testbed",
+ "http://gitlab.internal:8929",
+ "glpat-platform",
+ "platform-secret",
+ ),
+ platform_entry(
+ "other",
+ "http://gitlab-other.internal:8929",
+ "glpat-other",
+ "other-secret",
+ ),
+ ]);
let handler = GitLabWebhookHandler::new(
"default-secret".to_string(),
None,
@@ -779,17 +792,29 @@ async fn webhook_without_app_state_uses_default_config() {
/// End-to-end verification per matched platform: the platform's webhook
/// secret verifies requests for its host; the default secret is rejected
-/// there (and vice versa for an unmatched host).
+/// there (and vice versa for an unmatched host — with TWO verification
+/// platforms the unique-verification fallback cannot guess, so the default
+/// applies there).
#[tokio::test]
async fn webhook_verify_uses_matched_platform_secret() {
let _lock = super::RUNTIME_TEST_LOCK.lock().await;
let _guard = EmptyRuntimeGuard::new();
- let state = state_with_platforms(vec![platform_entry(
- "testbed",
- "http://gitlab.internal:8929",
- "glpat-platform",
- "platform-secret",
- )]);
+ // Two verification-capable platforms on unrelated hosts: PLATFORM_BODY
+ // strictly matches "testbed"; the unmatched host is ambiguous → default.
+ let state = state_with_platforms(vec![
+ platform_entry(
+ "testbed",
+ "http://gitlab.internal:8929",
+ "glpat-platform",
+ "platform-secret",
+ ),
+ platform_entry(
+ "other",
+ "http://gitlab-other.internal:8929",
+ "glpat-other",
+ "other-secret",
+ ),
+ ]);
let handler = GitLabWebhookHandler::new(
"default-secret".to_string(),
None,
@@ -845,8 +870,12 @@ async fn webhook_signing_verify_uses_matched_platform_key() {
headers.insert("webhook-signature", sig.parse().unwrap());
assert!(handler.verify(&headers, PLATFORM_BODY).await.is_ok());
- // The same signature against a different body (unmatched host) must not
- // verify — the default config has no signing secret at all.
+ // Unmatched host with a SINGLE verification-capable platform: the unique
+ // verification platform fallback takes over, so the same platform signing
+ // key verifies the re-hosted body — the host-mismatch path (base_url vs
+ // payload localhost) that the fallback exists to fix. The legacy default
+ // token in the header is irrelevant: the platform config has signing
+ // configured (and no legacy secret), so the signature path verifies.
let other_body = PLATFORM_BODY.replace("gitlab.internal:8929", "other.internal:8929");
let message_id = "msg-2";
let sig = sign_message(raw_key, message_id, unix_now(), &other_body);
@@ -856,13 +885,163 @@ async fn webhook_signing_verify_uses_matched_platform_key() {
headers.insert("webhook-signature", sig.parse().unwrap());
let mut headers_legacy = headers.clone();
headers_legacy.insert("X-Gitlab-Token", "default-secret".parse().unwrap());
- // With the legacy default secret present, the unmatched host falls back
- // to the legacy check when the signature header... is present — but the
- // default has no signing secret configured, so the legacy token path
- // applies and verifies.
assert!(handler.verify(&headers_legacy, &other_body).await.is_ok());
}
+// ── Unique verification platform fallback (System Hooks Test button) ──
+
+/// GitLab System Hooks' **Test** button sends a SAMPLE payload with no URL at
+/// all (`event_name`/`project_id`/`changes`/`refs` only — no `project`
+/// object, no `repository`, nothing to match). With exactly ONE
+/// verification-capable platform configured, that platform unambiguously
+/// takes over both verification config and `matched_platform` — the fix for
+/// the Test button 403.
+#[tokio::test]
+async fn no_url_payload_falls_back_to_unique_verification_platform() {
+ let _lock = super::RUNTIME_TEST_LOCK.lock().await;
+ let _guard = EmptyRuntimeGuard::new();
+ let state = state_with_platforms(vec![platform_entry(
+ "testbed",
+ "http://host.docker.internal:8929",
+ "glpat-platform",
+ "platform-secret",
+ )]);
+ let handler = GitLabWebhookHandler::new(
+ "default-secret".to_string(),
+ None,
+ MrDispatcher::new(),
+ "glpat-default".to_string(),
+ )
+ .with_app_state(&state);
+
+ // Real captured shape of the System Hooks Test button payload.
+ let body = r#"{"event_name":"merge_request","project_id":123,"changes":[],"refs":[]}"#;
+ assert_eq!(payload_instance_url(body), None);
+
+ let cfg = handler.effective_config_for_body(body);
+ assert_eq!(cfg.webhook_secret, "platform-secret");
+ assert_eq!(cfg.token, "glpat-platform");
+
+ let platform = handler.matched_platform(body).unwrap();
+ assert_eq!(platform.name, "testbed");
+ assert_eq!(platform.base_url, "http://host.docker.internal:8929");
+
+ // End-to-end: the platform's secret verifies the Test payload.
+ let mut headers = HeaderMap::new();
+ headers.insert("X-Gitlab-Token", "platform-secret".parse().unwrap());
+ assert!(handler.verify(&headers, body).await.is_ok());
+}
+
+/// No-URL payload + TWO verification-capable platforms → the fallback cannot
+/// guess: the runtime default applies and `matched_platform` is `None`.
+#[tokio::test]
+async fn no_url_payload_two_verification_platforms_keeps_default() {
+ let _lock = super::RUNTIME_TEST_LOCK.lock().await;
+ let _guard = EmptyRuntimeGuard::new();
+ let state = state_with_platforms(vec![
+ platform_entry("a", "http://gitlab-a.internal:8929", "glpat-a", "secret-a"),
+ platform_entry("b", "http://gitlab-b.internal:8929", "glpat-b", "secret-b"),
+ ]);
+ let handler = GitLabWebhookHandler::new(
+ "default-secret".to_string(),
+ None,
+ MrDispatcher::new(),
+ "glpat-default".to_string(),
+ )
+ .with_app_state(&state);
+
+ let body = r#"{"event_name":"merge_request","project_id":123}"#;
+ let cfg = handler.effective_config_for_body(body);
+ assert_eq!(cfg.webhook_secret, "default-secret");
+ assert_eq!(cfg.token, "glpat-default");
+ assert!(handler.matched_platform(body).is_none());
+}
+
+/// No-URL payload + a single token-only platform (no webhook verification
+/// credentials) → the fallback ignores it (it cannot verify): runtime default.
+#[tokio::test]
+async fn no_url_payload_token_only_platform_keeps_default() {
+ let _lock = super::RUNTIME_TEST_LOCK.lock().await;
+ let _guard = EmptyRuntimeGuard::new();
+ let state = state_with_platforms(vec![platform_entry(
+ "routing-only",
+ "http://gitlab.internal:8929",
+ "glpat-platform",
+ "", // no webhook secret
+ )]);
+ let handler = GitLabWebhookHandler::new(
+ "default-secret".to_string(),
+ None,
+ MrDispatcher::new(),
+ "glpat-default".to_string(),
+ )
+ .with_app_state(&state);
+
+ let body = r#"{"event_name":"merge_request","project_id":123}"#;
+ let cfg = handler.effective_config_for_body(body);
+ assert_eq!(cfg.webhook_secret, "default-secret");
+ assert_eq!(cfg.token, "glpat-default");
+ assert!(handler.matched_platform(body).is_none());
+}
+
+/// A payload WITH an instance URL still resolves strictly by URL first (the
+/// fallback must never override a URL match — multi-platform URL semantics
+/// unchanged), and the single-platform host mismatch — payload carries
+/// `localhost`, `base_url` is `host.docker.internal` — is exactly the case the
+/// unique fallback fixes.
+#[tokio::test]
+async fn url_payload_prefers_strict_match_and_host_mismatch_falls_back() {
+ let _lock = super::RUNTIME_TEST_LOCK.lock().await;
+ let _guard = EmptyRuntimeGuard::new();
+ let state = state_with_platforms(vec![platform_entry(
+ "testbed",
+ "http://host.docker.internal:8929",
+ "glpat-platform",
+ "platform-secret",
+ )]);
+ let handler = GitLabWebhookHandler::new(
+ "default-secret".to_string(),
+ None,
+ MrDispatcher::new(),
+ "glpat-default".to_string(),
+ )
+ .with_app_state(&state);
+
+ // URL matches the platform's base_url → strict match wins over the
+ // fallback (regression: multi-platform URL semantics unchanged).
+ let matching_body = PLATFORM_BODY.replace("gitlab.internal:8929", "host.docker.internal:8929");
+ let cfg = handler.effective_config_for_body(&matching_body);
+ assert_eq!(cfg.webhook_secret, "platform-secret");
+ assert_eq!(handler.matched_platform(&matching_body).unwrap().name, "testbed");
+
+ // Host mismatch: the payload's external_url is `localhost`, the platform's
+ // base_url is `host.docker.internal` → URL matches no platform → the
+ // unique verification platform still takes over (verification AND
+ // allowlist/rewrite resolution).
+ let localhost_body = PLATFORM_BODY.replace("gitlab.internal:8929", "localhost:8929");
+ let cfg = handler.effective_config_for_body(&localhost_body);
+ assert_eq!(cfg.webhook_secret, "platform-secret");
+ assert_eq!(handler.matched_platform(&localhost_body).unwrap().name, "testbed");
+}
+
+/// `unique_verification_platform` boundary: exactly one verification-capable
+/// entry wins regardless of surrounding token-only entries; zero or multiple
+/// verification-capable entries (or an empty list) yield `None`.
+#[test]
+fn unique_verification_platform_requires_exactly_one() {
+ let verifying = |name: &str| platform_entry(name, "http://gitlab.internal:8929", "tok", "wh-secret");
+ let token_only = |name: &str| platform_entry(name, "http://gitlab.internal:8929", "tok", "");
+ // Exactly one verification-capable entry → it (token-only entries ignored).
+ let hit = unique_verification_platform(&[token_only("routing"), verifying("verifying"), token_only("routing-2")]);
+ assert_eq!(hit.map(|p| p.name), Some("verifying".to_string()));
+ // Two verification-capable entries → ambiguous → None.
+ assert!(unique_verification_platform(&[verifying("a"), verifying("b")]).is_none());
+ // Zero verification-capable entries (all token-only) → None.
+ assert!(unique_verification_platform(&[token_only("a"), token_only("b")]).is_none());
+ // Empty list → None.
+ assert!(unique_verification_platform(&[]).is_none());
+}
+
// ── System Hooks (admin-level) ─────────────────────────────────────
/// System-hook MR payload shape: `event_name` lives in the body, there is NO
diff --git a/src/server/state.rs b/src/server/state.rs
index 577b8df..93a2211 100644
--- a/src/server/state.rs
+++ b/src/server/state.rs
@@ -246,14 +246,18 @@ pub struct AppState {
impl AppState {
/// Create a new `AppState` with the given LLM configs.
///
- /// All optional fields are initialised to `None`; set them directly
- /// or with builder-style methods as needed.
+ /// Optional fields except `task_store` are initialised to `None`; set
+ /// them directly or with builder-style methods as needed. The task store
+ /// is created eagerly so EVERY `AppState` can record review tasks — the
+ /// old `None` default left webhook-dispatched reviews (and every test
+ /// state) without a store, which is what made the dashboard / queue /
+ /// `/reviews` pages empty.
pub fn new(llm_configs: Vec) -> Self {
Self {
llm_configs: RwLock::new(llm_configs),
registry: None,
progress_map: None,
- task_store: None,
+ task_store: Some(Arc::new(TaskStore::new())),
app_config: RwLock::new(None),
log_collector: None,
ui_config: RwLock::new(UiConfig::default()),
@@ -277,7 +281,10 @@ mod tests {
assert!(state.llm_configs.read().unwrap().is_empty());
assert!(state.registry.is_none());
assert!(state.progress_map.is_none());
- assert!(state.task_store.is_none());
+ // The task store is initialized eagerly so every AppState can record
+ // review tasks (webhook + REST); the old `None` default is what made
+ // the dashboard / queue / reviews pages empty.
+ assert!(state.task_store.is_some());
assert!(state.app_config.read().unwrap().is_none());
assert!(state.log_collector.is_none());
assert!(state.feedback_store.is_none());
diff --git a/src/server/task_queue.rs b/src/server/task_queue.rs
index 4b2cfb8..3521516 100644
--- a/src/server/task_queue.rs
+++ b/src/server/task_queue.rs
@@ -101,20 +101,28 @@ impl TaskStore {
pub fn new() -> Self {
let (tx, _) = tokio::sync::broadcast::channel(256);
let inner: Arc>> = Arc::new(RwLock::new(HashMap::new()));
- let cleanup_inner = Arc::clone(&inner);
-
- tokio::spawn(async move {
- let mut interval = tokio::time::interval(std::time::Duration::from_secs(300));
- loop {
- interval.tick().await;
- let cutoff = chrono::Utc::now() - chrono::Duration::minutes(30);
- let mut map = cleanup_inner.write().await;
- map.retain(|_, entry| match entry.completed_at {
- Some(t) if t < cutoff => false,
- _ => true,
- });
- }
- });
+
+ // Only start the background reaper when a Tokio runtime is present.
+ // `TaskStore::new()` is also reached from `AppState::new()`, which sync
+ // unit tests construct without a runtime — `tokio::spawn` would panic
+ // there. The store is fully functional without the reaper (completed
+ // entries just aren't auto-reaped); `cleanup_expired()` covers the
+ // manual path.
+ if tokio::runtime::Handle::try_current().is_ok() {
+ let cleanup_inner = Arc::clone(&inner);
+ tokio::spawn(async move {
+ let mut interval = tokio::time::interval(std::time::Duration::from_secs(300));
+ loop {
+ interval.tick().await;
+ let cutoff = chrono::Utc::now() - chrono::Duration::minutes(30);
+ let mut map = cleanup_inner.write().await;
+ map.retain(|_, entry| match entry.completed_at {
+ Some(t) if t < cutoff => false,
+ _ => true,
+ });
+ }
+ });
+ }
Self {
inner,