-
Notifications
You must be signed in to change notification settings - Fork 587
feat: persist scan lifecycle and model metadata #264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
52020d9
52a5a87
6e9f17d
9dfdb63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -79,6 +79,7 @@ | |
| SQLITE_RETRY_ATTEMPTS, | ||
| ) | ||
| from workbench_feedback import get_scan_feedback | ||
| from workbench_remediation import remediation_claim_is_active | ||
| from workbench_scan_start import ( | ||
| archive_scan, | ||
| compact_timestamp, | ||
|
|
@@ -137,23 +138,6 @@ def stale_claim_before(seconds: int = CLAIM_LEASE_SECONDS) -> str: | |
| ) | ||
|
|
||
|
|
||
| def remediation_claim_is_active(remediation: sqlite3.Row) -> bool: | ||
| if remediation["pending_action_claim_token"] is None: | ||
| return False | ||
| delivered_at = remediation["pending_action_delivered_at"] | ||
| claimed_at = delivered_at or remediation["pending_action_claimed_at"] | ||
| if not isinstance(claimed_at, str): | ||
| return True | ||
| try: | ||
| parsed = datetime.fromisoformat(claimed_at) | ||
| if parsed.tzinfo is None: | ||
| return True | ||
| except ValueError: | ||
| return True | ||
| lease_seconds = DELIVERED_ACTION_LEASE_SECONDS if delivered_at else CLAIM_LEASE_SECONDS | ||
| return parsed > datetime.now(timezone.utc) - timedelta(seconds=lease_seconds) | ||
|
|
||
|
|
||
| def state_dir() -> Path: | ||
| state_dir = os.environ.get("CODEX_SECURITY_STATE_DIR") | ||
| if state_dir: | ||
|
|
@@ -1172,6 +1156,8 @@ def start_scan(connection: sqlite3.Connection, args: argparse.Namespace) -> dict | |
| target_summary=target_summary, | ||
| scope_file_count=scope_file_count, | ||
| timestamp=timestamp, | ||
| model=args.model, | ||
| reasoning_effort=args.reasoning_effort, | ||
|
Comment on lines
+1159
to
+1160
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For the normal app-backed setup flow, the bundled MCP app invokes Useful? React with 👍 / 👎. |
||
| ) | ||
| if manages_transaction: | ||
| connection.commit() | ||
|
|
@@ -1309,6 +1295,8 @@ def start_prompt_only_scan( | |
| scope_file_count=scope_file_count, | ||
| timestamp=timestamp, | ||
| handoff_status="delivered", | ||
| model=args.model, | ||
| reasoning_effort=args.reasoning_effort, | ||
| ) | ||
| connection.commit() | ||
| except BaseException: | ||
|
|
@@ -3004,6 +2992,7 @@ def scan_result( | |
| progress_result["independentReviews"] = { | ||
| "active": independent_reviews["active"], | ||
| "completed": independent_reviews["completed"], | ||
| "consolidating": independent_reviews["consolidating"], | ||
| } | ||
| return { | ||
| "artifacts": artifacts, | ||
|
|
@@ -3024,8 +3013,10 @@ def scan_result( | |
| "handoffClaimToken": scan["handoff_claim_token"], | ||
| "handoffStatus": scan["handoff_status"], | ||
| "mode": scan["mode"], | ||
| "model": scan["model"], | ||
| "diffTarget": stored_diff_target(scan), | ||
| "progress": progress_result, | ||
| "reasoningEffort": scan["reasoning_effort"], | ||
| "remediationAvailable": remediation_available, | ||
| "remediationUnavailableReason": remediation_unavailable_reason, | ||
| "reportAvailable": "markdownReport" in artifacts, | ||
|
|
@@ -3647,13 +3638,9 @@ def main() -> None: | |
| read_coverage=coverage_for_comparison, | ||
| ) | ||
| elif args.command == "list-global-findings": | ||
| result = native_indexes.list_global_findings( | ||
| connection, args, read_coverage=coverage_for_comparison | ||
| ) | ||
| result = native_indexes.list_global_findings(connection, args) | ||
| elif args.command == "list-repositories": | ||
| result = native_indexes.list_repositories( | ||
| connection, args, read_coverage=coverage_for_comparison | ||
| ) | ||
| result = native_indexes.list_repositories(connection, args) | ||
| elif args.command == "list-findings": | ||
| result = list_findings(connection, args) | ||
| elif args.command == "update-progress": | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ | |
| import sqlite3 | ||
| import sys | ||
| from collections import Counter | ||
| from collections.abc import Callable, Iterator | ||
| from collections.abc import Iterator | ||
| from itertools import islice | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
@@ -19,14 +19,12 @@ | |
| def list_global_findings( | ||
| connection: sqlite3.Connection, | ||
| args: argparse.Namespace, | ||
| *, | ||
| read_coverage: Callable[[sqlite3.Row], dict[str, Any]], | ||
| ) -> dict[str, Any]: | ||
| limit = min(args.limit, FINDINGS_PAGE_MAX) | ||
| query = args.query.strip().casefold() if args.query else "" | ||
| findings = ( | ||
| row | ||
| for row in _active_findings(connection, read_coverage) | ||
| for row in _indexed_findings(connection) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a later completed scan covers a finding's location but no longer reports that finding, Useful? React with 👍 / 👎. |
||
| if (args.target_id is None or row["target_id"] == args.target_id) | ||
| and (args.severity is None or row["severity"] == args.severity) | ||
| and (args.status is None or row["status"] == args.status) | ||
|
|
@@ -72,23 +70,8 @@ def list_global_findings( | |
| } | ||
|
|
||
|
|
||
| def _active_findings( | ||
| connection: sqlite3.Connection, | ||
| read_coverage: Callable[[sqlite3.Row], dict[str, Any]], | ||
| ) -> Iterator[sqlite3.Row]: | ||
| completed_scans_by_target: dict[str, list[sqlite3.Row]] = {} | ||
| for scan in connection.execute( | ||
| """ | ||
| SELECT * | ||
| FROM scans | ||
| WHERE status = 'complete' AND seal_manifest_digest IS NOT NULL | ||
| ORDER BY started_at DESC, id DESC | ||
| """ | ||
| ): | ||
| completed_scans_by_target.setdefault(scan["target_id"], []).append(scan) | ||
|
|
||
| coverage_by_scan_id: dict[str, dict[str, Any]] = {} | ||
| rows = connection.execute( | ||
| def _indexed_findings(connection: sqlite3.Connection) -> Iterator[sqlite3.Row]: | ||
| yield from connection.execute( | ||
| """ | ||
| WITH ranked_findings AS ( | ||
| SELECT | ||
|
|
@@ -97,7 +80,6 @@ def _active_findings( | |
| occurrences.severity, | ||
| occurrences.created_at, | ||
| scans.id AS scan_id, | ||
| scans.started_at AS scan_started_at, | ||
| scans.target_id, | ||
| targets.current_path AS target_path, | ||
| scans.scope, | ||
|
|
@@ -146,35 +128,11 @@ def _active_findings( | |
| selected_findings.occurrence_id | ||
| """, | ||
| ) | ||
| for row in rows: | ||
| resolved = False | ||
| for scan in completed_scans_by_target.get(row["target_id"], ()): | ||
| if (scan["started_at"], scan["id"]) <= ( | ||
| row["scan_started_at"], | ||
| row["scan_id"], | ||
| ): | ||
| break | ||
| coverage = coverage_by_scan_id.get(scan["id"]) | ||
| if coverage is None: | ||
| coverage = read_coverage(scan) | ||
| coverage_by_scan_id[scan["id"]] = coverage | ||
| if scan_history.scan_covers_path( | ||
| scan, | ||
| target_id=row["target_id"], | ||
| path=row["location_path"], | ||
| coverage=coverage, | ||
| ): | ||
| resolved = True | ||
| break | ||
| if not resolved: | ||
| yield row | ||
|
|
||
|
|
||
| def list_repositories( | ||
| connection: sqlite3.Connection, | ||
| args: argparse.Namespace | None = None, | ||
| *, | ||
| read_coverage: Callable[[sqlite3.Row], dict[str, Any]], | ||
| ) -> dict[str, Any]: | ||
| scans = scan_history.list_scans(connection)["scans"] | ||
| scans_by_id = {scan["scanId"]: scan for scan in scans} | ||
|
|
@@ -190,9 +148,7 @@ def list_repositories( | |
| latest_scan_by_target.setdefault(row["target_id"], scans_by_id[row["id"]]) | ||
|
|
||
| open_findings_by_target = Counter( | ||
| row["target_id"] | ||
| for row in _active_findings(connection, read_coverage) | ||
| if row["status"] == "open" | ||
| row["target_id"] for row in _indexed_findings(connection) if row["status"] == "open" | ||
| ) | ||
| targets = {row["id"]: row for row in connection.execute("SELECT * FROM security_targets")} | ||
| repositories = [ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -225,6 +225,7 @@ def list_scans( | |
| "findingCount": row["finding_count"], | ||
| "handoffStatus": row["handoff_status"], | ||
| "mode": row["mode"], | ||
| "model": row["model"], | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For scans started through the public TypeScript SDK or CLI, Useful? React with 👍 / 👎. |
||
| "parentScanId": row["parent_scan_id"], | ||
| "progress": { | ||
| "candidates": {"reportable": row["reportable_findings_count"]}, | ||
|
|
@@ -238,6 +239,7 @@ def list_scans( | |
| "updatedAt": row["progress_updated_at"], | ||
| }, | ||
| "recipeAvailable": row["recipe_json"] is not None, | ||
| "reasoningEffort": row["reasoning_effort"], | ||
| "scanDir": row["scan_dir"], | ||
| "scanId": row["id"], | ||
| "scope": row["scope"], | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When an idempotent
start_codex_security_deep_scancall rejoins an already completed, failed, or canceled scan,require_owned_scanand the continuation check do not require the parent scan to be running, and this unrestricted update executes before the existing run is returned. A later retry from a turn using different model settings therefore rewrites the historical scan'smodelandreasoning_efforteven though no scan work runs. Restrict this update to running scans so terminal execution metadata remains immutable.Useful? React with 👍 / 👎.