Start policy evaluation timeout after security report ingestion
What does this MR do and why?
Merge request approval policies with scan_finding rules wait for security report ingestion before failing closed with "could not be evaluated within the specified timeframe", and evaluation no longer queues behind the policy-sync concurrency limit. Behind the policy_evaluation_after_reports_ingestion feature flag (default off).
With the flag off, behavior is unchanged.
Changes with the flag on:
- Add
Security::Scan.ingestion_pending?(pipeline). It is true when the pipeline hierarchy has latest scans still processing, or has security report artifacts but no scans yet. ProcessPipelineCompletionWorkerreturns onPipelineFinishedEventwhile ingestion is pending.ReportsIngestedEventtriggers evaluation later. It also callsSyncFindingsToApprovalRulesServiceinline instead of enqueuing the concurrency-limitedSyncFindingsToApprovalRulesWorker. The service only enqueues oneSyncMergeRequestApprovalsWorkerper merge request, so it is cheap.UnblockPendingMergeRequestViolationsWorkerdefers failing closed while ingestion is pending. It reschedules itself in 150 seconds. If scans were stored less than 150 seconds ago, it reschedules for the remaining time, measured from the newest latest-scanupdated_at. It stops deferring once the pipeline is more than 10 minutes past completion (MAX_INGESTION_WAIT), then fails closed as before. Completion is the pipelinefinished_at, orupdated_atfor pipelines blocked on manual jobs (statusmanual, which have nofinished_at). Before, such pipelines skipped the deferral entirely.- After
UnblockPendingMergeRequestViolationsWorkermarks violationsskipped, it enqueuesSyncMergeRequestApprovalsWorkerfor the merge request, unless report ingestion is still pending or the pipeline is running again. Nothing else re-runs evaluation in the case described in Cause 4, where a merge request stayedEVALUATION_SKIPPEDfor over 13 hours.SyncMergeRequestApprovalsWorkeris not concurrency-limited. While ingestion is pending it does not enqueue, because evaluation would see no findings.ReportsIngestedEventtriggers evaluation instead. A running pipeline is evaluated again when it completes. This re-evaluatesscan_findingrules only; skippedlicense_scanningandany_merge_requestviolations keep their own evaluation paths.
Why this happens
Symptom: policies with scan_finding rules intermittently fail closed with "Policy X could not be evaluated within the specified timeframe and, as a result, approvals are required". The pipeline succeeded and the reports have no new findings. The MR clears by itself later, when the delayed evaluation finally runs.
Cause 1: Security::UnenforceablePolicyRulesPipelineNotificationWorker schedules Security::ScanResultPolicies::UnblockPendingMergeRequestViolationsWorker 150 seconds (UNBLOCK_PENDING_VIOLATIONS_TIMEOUT) after pipeline completion. That worker marks running violations as skipped and requires approvals. But scan_finding rules can only be evaluated after Security::StoreScansWorker stores the scans. In an observed case, queueing plus running StoreScansWorker took about 137 seconds of the 150. The constant was already bumped from 10 minutes to 90 seconds to 150 seconds. No fixed value works because queue latency varies.
Cause 2: SyncFindingsToApprovalRulesWorker has concurrency_limit -> { 200 }, added to throttle policy-sync fan-out. Pipeline-completion evaluation shares that global limit. During fan-out bursts the job is parked (concurrency_limit: paused). The post-ingestion enqueue is deduplicated into the parked job (deduplicate :until_executing). In the observed case, evaluation started about 4.5 minutes after the timeout fired. In another production case the parked job stayed buffered for about 69 minutes (concurrency_limit_buffering_duration_s: 4125), and the worker logged about 40,000 concurrency-limit deferrals per 30 minutes during working hours.
Cause 3: Security::Scan.results_ready? returns true before StoreScansWorker creates any Security::Scan row, because it only checks for processing scans. On PipelineFinishedEvent, ProcessPipelineCompletionWorker therefore evaluates before ingestion. That premature job is usually parked by the limiter, so it ends up running after ingestion by accident. This change makes that ordering explicit.
Cause 4: the late evaluation does not always arrive. This is confirmed from Sidekiq logs of one production case (times UTC, 2026-09-30). Reports were ingested while the pipeline was still running (17:47:59). The evaluation that followed ran at 17:49:05 and logged "No security reports found for the pipeline", because Ci::Pipeline#has_security_reports? requires complete_or_manual?. The pipeline then finished in manual status (17:49:22). ProcessPipelineCompletionWorker returned without evaluating, because results_ready? calls all_security_jobs_complete?, which stays false while security jobs wait in created behind a manual job. The timeout skipped the violations at 17:51:56 and no later event re-evaluated them. This does not depend on the concurrency limit. The re-evaluation after a skip (item 4) resolves it.
References
- Prompted by a customer support escalation (no link).
- Closes #631508
- Rollout issue: #631506
- Introduced the timeout: fa0b192e
- Bumped the timeout to 150 seconds: af598851
- Added the concurrency limit: 257d49f1
- Introduced
ReportsIngestedEventand theresults_ready?gate: e2acc003
Database
All new queries are behind policy_evaluation_after_reports_ingestion and run on the Security::Scan model (security_scans, gitlab_sec). They reuse the pipeline_id IN (...) shape of the existing Security::Scan.processing? and are served by an existing index with leading pipeline_id (on production the planner picks index_security_scans_on_length_of_warnings). No migrations, no new indexes.
Security::Scan.ingestion_pending?(pipeline) runs on every PipelineFinishedEvent and on each reschedule of UnblockPendingMergeRequestViolationsWorker (at most 150 s apart, until 10 min after completion), and once per unblock run that skips violations. Where <ids> is the pipeline hierarchy from pipeline.self_and_project_descendants.pluck_primary_key (an existing recursive CTE on ci_sources_pipelines, then p_ci_pipelines):
-- 1. any latest scans in the hierarchy?
SELECT 1 FROM "security_scans" WHERE "security_scans"."pipeline_id" IN (<ids>) AND "security_scans"."latest" = TRUE LIMIT 1;
-- 2. any of them still processing? (only if 1 returned a row)
SELECT 1 FROM "security_scans" WHERE "security_scans"."pipeline_id" IN (<ids>) AND "security_scans"."latest" = TRUE AND "security_scans"."status" IN (0, 4) LIMIT 1;
-- 3. worker only, when nothing is processing: newest scan update
SELECT MAX("security_scans"."updated_at") FROM "security_scans" WHERE "security_scans"."pipeline_id" IN (<ids>) AND "security_scans"."latest" = TRUE;Query 1 with no matching scans falls through to Ci::Pipeline#has_self_or_descendant_reports?(Ci::JobArtifact.security_reports), an existing method and query on ci_builds and p_ci_job_artifacts.
Query plans (postgres.ai DBLab clone of gitlab-production-sec, EXPLAIN ANALYZE)
Run on the gitlab-production-sec thin clone (security_scans is on the sec database) with a realistic 6-pipeline hierarchy of recently created latest scans, taken from the clone itself. The CLI returned no shareable plan URLs.
- Any latest scans in the hierarchy: index scan, 1.7 ms execution (4 buffer reads, 1 hit).
- Any still processing: index scan, 0.46 ms execution (38 buffer hits, 18 rows removed by the filter).
- Newest scan update: index scan of 26 rows, aggregate, 0.68 ms execution (44 hits, 4 reads).
All three use a pipeline_id = ANY (...) index condition with latest and status as filters, and are bounded by the number of scans in one pipeline hierarchy.
-- 1
Limit (cost=0.57..1.65 rows=1) (actual time=1.595..1.597 rows=1 loops=1)
Buffers: shared hit=1 read=4
-> Index Scan using index_security_scans_on_length_of_warnings on security_scans
Index Cond: (pipeline_id = ANY ('{2896882010,2896882794,2896893645,2896850762,2896893404,2896895312}'::bigint[]))
Filter: latest
-- 2
Limit (cost=0.57..57.91 rows=1) (actual time=0.393..0.394 rows=1 loops=1)
Buffers: shared hit=38
-> Index Scan using index_security_scans_on_length_of_warnings on security_scans
Index Cond: (pipeline_id = ANY ('{...same 6 ids...}'::bigint[]))
Filter: (latest AND (status = ANY ('{0,4}'::integer[])))
Rows Removed by Filter: 18
-- 3
Aggregate (cost=57.91..57.92 rows=1) (actual time=0.584..0.585 rows=1 loops=1)
Buffers: shared hit=44 read=4 dirtied=1
-> Index Scan using index_security_scans_on_length_of_warnings on security_scans
(actual time=0.063..0.572 rows=26 loops=1)
Index Cond: (pipeline_id = ANY ('{...same 6 ids...}'::bigint[]))
Filter: latestScreenshots or screen recordings
Not applicable, backend worker timing change.
How to set up and validate locally
Run the specs:
bundle exec rspec ee/spec/models/security/scan_spec.rb -e '.ingestion_pending?'
bundle exec rspec ee/spec/workers/security/scan_result_policies/process_pipeline_completion_worker_spec.rb
bundle exec rspec ee/spec/workers/security/scan_result_policies/unblock_pending_merge_request_violations_worker_spec.rbManual check in GDK:
- Create a project with a
scan_findingmerge request approval policy. - Enable the flag in a Rails console:
Feature.enable(:policy_evaluation_after_reports_ingestion). - Run an MR pipeline that includes a security scanner.
- Right after the pipeline finishes, stop the Sidekiq process so
StoreScansWorkeris delayed past 150 seconds. Then restart it. - Check that the violation is not
skippedand the bot comment has no timeout error.
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Reviewable LOC: 88 (plus 256 in tests)
Context for LLM agents
Rejected alternatives:
- Bump
UNBLOCK_PENDING_VIOLATIONS_TIMEOUTagain. Queue latency is unbounded, so it keeps failing intermittently. - Only anchor the deadline on ingestion. It would not have saved the observed case, because the limiter parked evaluation about 5 minutes after ingestion.
- Only bypass the limiter. It leaves about 13 seconds of headroom when ingestion is slow.
- Change
results_ready?itself. It has other callers (Vulnerabilities::CompareSecurityReportsService,Resolvers::Security::EnabledScansResolver) whose behavior this MR keeps unchanged.
Invariants:
- The unblock worker must still eventually fail closed (the 10 minute cap), so violations never stay
runningindefinitely when ingestion never happens. - Inline evaluation must stay gated on ingestion completing. Otherwise it evaluates with zero findings, which is the race e2acc003 fixed.
- The re-evaluation enqueued after a skip must stay gated on ingestion having finished, for the same reason as inline evaluation.
- The worker reschedules with
perform_infrom insideperform. This works withdeduplicate :until_executing, including_scheduled: truebecause the lock is released when execution starts.