fix: persist total_components count for EI analysis when job terminates [Backport release/0.6.z] - #2574
Merged
Merged
Conversation
(cherry picked from commit 155a5c9)
Contributor
Reviewer's GuideThis backport ensures exploit intelligence jobs persist and expose accurate total_components and related component counters when jobs complete or fail, and wires that through service APIs, runners, and tests. Sequence diagram for EI product job completion/failure total_components persistencesequenceDiagram
actor Runner
participant EiService
participant Db
Runner->>EiService: poll_for_product_result(job_id, product_id, db)
EiService->>Db: fetch_components(job_id, db)
Db-->>EiService: components
EiService->>EiService: [compute total = components.len()]
alt has_finding or all_excluded
EiService->>Db: update_job_completed(job_id, total, db)
Db-->>EiService: Result
else no findings and not all_excluded
EiService->>Db: update_job_failed(job_id, msg, total, db)
Db-->>EiService: Result
end
EiService-->>Runner: Result
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
poll_for_result,update_job_completedis called with a hard-codedSome(1)fortotal_components; consider deriving the count fromfetch_componentsas inpoll_for_product_resultto avoid incorrect totals if the flow ever becomes multi-component. - The new
update_job_completed/update_job_failedsemantics leavetotal_componentsunchanged when called withNone; verify whether callers ever expect the value to be cleared instead of preserved, and if so, consider setting it explicitly toSet(None)in those cases to avoid stale counts.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `poll_for_result`, `update_job_completed` is called with a hard-coded `Some(1)` for `total_components`; consider deriving the count from `fetch_components` as in `poll_for_product_result` to avoid incorrect totals if the flow ever becomes multi-component.
- The new `update_job_completed`/`update_job_failed` semantics leave `total_components` unchanged when called with `None`; verify whether callers ever expect the value to be cleared instead of preserved, and if so, consider setting it explicitly to `Set(None)` in those cases to avoid stale counts.
## Individual Comments
### Comment 1
<location path="modules/exploit-intelligence/src/endpoints/test.rs" line_range="475" />
<code_context>
let db_rw = db::ReadWrite::new(ctx.db.clone());
ei_service
- .update_job_failed(job_id, "analysis timed out after 1800s", &db_rw)
+ .update_job_failed(job_id, "analysis timed out after 1800s", None, &db_rw)
.await?;
</code_context>
<issue_to_address>
**suggestion (testing):** Assert `total_components` on the timeout failure endpoint test
This timeout test only checks status and error message. Since `update_job_failed` is now called with `total_components = None`, please also assert that the GET response exposes `total_components == None`. Consider a variant where the job already has a `total_components` value to confirm it remains unchanged under timeout failures, so the endpoint is clearly tied to the new failure semantics for component counts.
Suggested implementation:
```rust
let ei_service = test_service();
let db_rw = db::ReadWrite::new(ctx.db.clone());
// Assert that timeout failures expose `total_components == None`
let timeout_status = ei_service
.get_job_status(job_id, &db_rw)
.await
.expect("failed to fetch job status after timeout");
assert_eq!(timeout_status.total_components, None);
// Variant: job with pre-existing `total_components` keeps its value on timeout
let pre_existing_total_components = Some(42u32);
let pre_existing_job_id = ei_service
.create_job(
&ctx,
"CVE-2024-9999",
ExploitIntelligenceJobStatus::Running,
pre_existing_total_components,
None,
)
.await
.expect("failed to create job with pre-existing total_components");
ei_service
.update_job_failed(
pre_existing_job_id,
"analysis timed out after 1800s",
None,
&db_rw,
)
.await
.expect("failed to mark pre-existing job as timed out");
let pre_existing_timeout_status = ei_service
.get_job_status(pre_existing_job_id, &db_rw)
.await
.expect("failed to fetch pre-existing job status after timeout");
assert_eq!(
pre_existing_timeout_status.total_components,
pre_existing_total_components
);
ei_service
```
I only see a small fragment of the test file, so you will likely need to adapt the above to your actual helpers and types:
1. Replace `get_job_status` with the actual GET endpoint helper used in this test (e.g. `get_exploit_intelligence_job`, `get_job`, or whatever returns the job DTO that includes `total_components`).
2. If the timeout test currently drives the endpoint via HTTP (e.g. using `ctx.get(...)`), move the `assert_eq!(..., None)` assertions to use the decoded HTTP response body instead of `ei_service.get_job_status(...)`.
3. Replace `create_job` with the existing job creation helper you already use for `"CVE-2024-5678"` and `"CVE-2024-9999"` in this file, matching its signature (status, `total_components`, `completed_components`).
4. Ensure the `job_id` used in the first assertion is the same one that was marked failed with `update_job_failed(job_id, "analysis timed out after 1800s", None, &db_rw)` in the timeout test.
5. If the job DTO nests `total_components` (e.g. `response.body.total_components` or `job.metrics.total_components`), adjust the field access in the assertions accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| let db_rw = db::ReadWrite::new(ctx.db.clone()); | ||
| ei_service | ||
| .update_job_failed(job_id, "analysis timed out after 1800s", &db_rw) | ||
| .update_job_failed(job_id, "analysis timed out after 1800s", None, &db_rw) |
Contributor
There was a problem hiding this comment.
suggestion (testing): Assert total_components on the timeout failure endpoint test
This timeout test only checks status and error message. Since update_job_failed is now called with total_components = None, please also assert that the GET response exposes total_components == None. Consider a variant where the job already has a total_components value to confirm it remains unchanged under timeout failures, so the endpoint is clearly tied to the new failure semantics for component counts.
Suggested implementation:
let ei_service = test_service();
let db_rw = db::ReadWrite::new(ctx.db.clone());
// Assert that timeout failures expose `total_components == None`
let timeout_status = ei_service
.get_job_status(job_id, &db_rw)
.await
.expect("failed to fetch job status after timeout");
assert_eq!(timeout_status.total_components, None);
// Variant: job with pre-existing `total_components` keeps its value on timeout
let pre_existing_total_components = Some(42u32);
let pre_existing_job_id = ei_service
.create_job(
&ctx,
"CVE-2024-9999",
ExploitIntelligenceJobStatus::Running,
pre_existing_total_components,
None,
)
.await
.expect("failed to create job with pre-existing total_components");
ei_service
.update_job_failed(
pre_existing_job_id,
"analysis timed out after 1800s",
None,
&db_rw,
)
.await
.expect("failed to mark pre-existing job as timed out");
let pre_existing_timeout_status = ei_service
.get_job_status(pre_existing_job_id, &db_rw)
.await
.expect("failed to fetch pre-existing job status after timeout");
assert_eq!(
pre_existing_timeout_status.total_components,
pre_existing_total_components
);
ei_serviceI only see a small fragment of the test file, so you will likely need to adapt the above to your actual helpers and types:
- Replace
get_job_statuswith the actual GET endpoint helper used in this test (e.g.get_exploit_intelligence_job,get_job, or whatever returns the job DTO that includestotal_components). - If the timeout test currently drives the endpoint via HTTP (e.g. using
ctx.get(...)), move theassert_eq!(..., None)assertions to use the decoded HTTP response body instead ofei_service.get_job_status(...). - Replace
create_jobwith the existing job creation helper you already use for"CVE-2024-5678"and"CVE-2024-9999"in this file, matching its signature (status,total_components,completed_components). - Ensure the
job_idused in the first assertion is the same one that was marked failed withupdate_job_failed(job_id, "analysis timed out after 1800s", None, &db_rw)in the timeout test. - If the job DTO nests
total_components(e.g.response.body.total_componentsorjob.metrics.total_components), adjust the field access in the assertions accordingly.
Strum355
approved these changes
Aug 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Backport of #2570 to
release/0.6.z.Summary by Sourcery
Persist and expose accurate total component counts for exploit intelligence analysis jobs, including when jobs complete or fail, and strengthen tests around job details and endpoint behaviour.
Bug Fixes:
Enhancements: