Delete: only the filing agent may withdraw, rewrite artifacts before persistence with rollback, dedicated viewer renderer

strix/interface/viewer/frontend/src/components/live/tool-renderers/VulnReportDeleteRenderer.tsx

@@ -0,0 +1,46 @@
+"use client";
+
+import type { ToolRendererProps } from "@/types/events";
+import { TruncatedText } from "./ToolCard";
+
+const SEVERITY_COLORS: Record<string, string> = {
+  critical: "text-red-400", high: "text-orange-400", medium: "text-yellow-400",
+  low: "text-blue-400", info: "text-cyan-400",
+};
+
+/** A withdrawn finding: which report went, and what disproved it. */
+export default function VulnReportDeleteRenderer({ args, result, status }: ToolRendererProps) {
+  const reportId = (args.report_id as string) ?? "";
+  const reason = (args.delete_reason as string) ?? "";
+
+  const res = result && typeof result === "object" ? (result as Record<string, unknown>) : null;
+  const ok = res?.success === true;
+  const error = typeof res?.error === "string" ? res.error : "";
+  const title = typeof res?.title === "string" ? res.title : "";
+  const severity = typeof res?.severity === "string" ? res.severity.toLowerCase() : "";
+  const failed = res != null ? !ok : status === "failed" || status === "error";
+
+  return (
+    <div className="space-y-2">
+      <div className="flex items-center gap-2 flex-wrap">
+        <span className={`font-semibold text-sm ${failed ? "text-red-400/80" : "text-yellow-400/80"}`}>
+          {failed ? "report not withdrawn" : "report withdrawn"}
+        </span>
+        {reportId && <span className="text-[#555] font-mono text-[13px]">{reportId}</span>}
+        {severity && (
+          <span className={`text-[13px] line-through ${SEVERITY_COLORS[severity] ?? "text-[#888]"}`}>
+            {severity.toUpperCase()}
+          </span>
+        )}
+      </div>
+      {title && <div className="text-[15px] text-white/60 line-through">{title}</div>}
+      {reason && (
+        <div>
+          <span className="text-emerald-400/60 text-sm font-semibold">Why</span>
+          <div className="mt-1"><TruncatedText text={reason} maxLines={10} /></div>
+        </div>
+      )}
+      {error && <div className="text-red-400/80 text-[13px]">{error}</div>}
+    </div>
+  );
+}

strix/interface/viewer/frontend/src/components/live/tool-renderers/index.ts

@@ -12,6 +12,7 @@ import FileEditRenderer from "./FileEditRenderer";
 import ApplyPatchRenderer from "./ApplyPatchRenderer";
 import ViewImageRenderer from "./ViewImageRenderer";
 import VulnReportRenderer from "./VulnReportRenderer";
+import VulnReportDeleteRenderer from "./VulnReportDeleteRenderer";
 import ReportListRenderer from "./ReportListRenderer";
 import ProxyRenderer from "./ProxyRenderer";
 import ThinkRenderer from "./ThinkRenderer";
@@ -151,6 +152,7 @@ const RENDERER_OVERRIDES: Partial<Record<string, ComponentType<ToolRendererProps
   view_image: ViewImageRenderer,
   list_reports: ReportListRenderer,
   get_report: ReportListRenderer,
+  delete_vulnerability_report: VulnReportDeleteRenderer,
 };
 
 /**

strix/interface/viewer/static/index.html

@@ -6,8 +6,8 @@
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
     <meta name="color-scheme" content="dark" />
     <title>Strix Results</title>
-    <script type="module" crossorigin src="./assets/index-B94ANU8d.js"></script>
-    <link rel="stylesheet" crossorigin href="./assets/index-DN__rVv3.css">
+    <script type="module" crossorigin src="./assets/index-DNPiqLUu.js"></script>
+    <link rel="stylesheet" crossorigin href="./assets/index-CrLE9rrV.css">
   </head>
   <body>
     <div id="root"></div>

strix/report/state.py

@@ -36,6 +36,19 @@ _global_report_state: Optional["ReportState"] = None
 _CONTROL_CHARS = re.compile(r"[\x00-\x1f\x7f]+")
 
 
+class ReportOwnershipError(PermissionError):
+    """Raised when an agent tries to withdraw a report another agent filed."""
+
+    def __init__(self, report_id: str, *, filed_by: str, caller: str | None) -> None:
+        self.report_id = report_id
+        self.filed_by = filed_by
+        self.caller = caller
+        super().__init__(
+            f"Report '{report_id}' was filed by agent {filed_by}, not by "
+            f"{caller or 'the caller'}; only the agent that filed a report can withdraw it"
+        )
+
+
 def _strix_version() -> str | None:
     """Best-effort package version for the SARIF tool.driver.version field."""
     try:
@@ -545,15 +558,27 @@ class ReportState:
     ) -> dict[str, Any] | None:
         """Withdraw a report from the run, keeping a record of the withdrawal.
 
-        Returns the removed report, or ``None`` when the id is unknown. The
-        report leaves ``vulnerability_reports`` and its rendered artifacts; the
-        run record keeps who withdrew it and why, so the id is never reissued.
+        Returns the removed report, or ``None`` when the id is unknown. Only the
+        agent that filed a report may withdraw it (:class:`ReportOwnershipError`
+        otherwise). The report leaves ``vulnerability_reports`` and its rendered
+        artifacts; the run record keeps who withdrew it and why, so the id is
+        never reissued.
+
+        The rewritten artifacts and the ``vulnerability_deleted_callback`` must
+        both accept the deletion first: if either fails the report is put back
+        and the error propagates, so the deletion can be retried.
         """
         report = next((r for r in self.vulnerability_reports if r.get("id") == report_id), None)
         if report is None:
             logger.warning("cannot delete unknown vulnerability report %s", report_id)
             return None
 
+        filed_by = report.get("agent_id")
+        if filed_by and deleted_by_agent_id != filed_by:
+            raise ReportOwnershipError(
+                report_id, filed_by=str(filed_by), caller=deleted_by_agent_id
+            )
+
         entry: dict[str, Any] = {
             "id": report_id,
             "title": report.get("title"),
@@ -569,17 +594,30 @@ class ReportState:
         if deleted_by_agent_name:
             entry["agent_name"] = deleted_by_agent_name
 
-        # Persistence must accept the deletion before local state changes. A
-        # failed callback leaves the report on file and the deletion retryable.
-        if self.vulnerability_deleted_callback:
-            self.vulnerability_deleted_callback({**report, "deletion": entry})
+        position = self.vulnerability_reports.index(report)
+        was_saved = report_id in self._saved_vuln_ids
+        history = self._deleted_vulnerability_reports()
 
         self.vulnerability_reports.remove(report)
         self._saved_vuln_ids.discard(report_id)
-        self.run_record["deleted_vulnerability_reports"] = [
-            *self._deleted_vulnerability_reports(),
-            entry,
-        ]
+        self.run_record["deleted_vulnerability_reports"] = [*history, entry]
+        try:
+            # Local artifacts first: they can be put back if persistence then
+            # refuses, whereas a row deleted elsewhere cannot.
+            self._sync_llm_usage_record()
+            self._write_artifacts()
+            if self.vulnerability_deleted_callback:
+                self.vulnerability_deleted_callback({**report, "deletion": entry})
+        except Exception:
+            self.vulnerability_reports.insert(position, report)
+            if was_saved:
+                self._saved_vuln_ids.add(report_id)
+            if history:
+                self.run_record["deleted_vulnerability_reports"] = history
+            else:
+                self.run_record.pop("deleted_vulnerability_reports", None)
+            self._save_artifacts()
+            raise
 
         md_path = self.get_run_dir() / "vulnerabilities" / f"{report_id}.md"
         try:
@@ -588,8 +626,6 @@ class ReportState:
             logger.exception("could not remove %s", md_path)
 
         logger.info("Deleted vulnerability report %s - %s", report_id, report.get("title"))
-
-        self.save_run_data()
         return report
 
     def get_existing_vulnerabilities(self) -> list[dict[str, Any]]:
@@ -775,49 +811,54 @@ class ReportState:
             return None
 
     def _save_artifacts(self) -> None:
-        """Write scan artifacts under ``run_dir``."""
-        run_dir = self.get_run_dir()
+        """Write scan artifacts under ``run_dir``; a write failure is logged."""
         try:
-            run_dir.mkdir(parents=True, exist_ok=True)
-
-            coverage = self._coverage_document()
-            if coverage is not None:
-                try:
-                    write_coverage(run_dir, coverage)
-                except OSError:
-                    logger.exception("coverage.json write failed (non-fatal)")
-
-            if self.final_scan_result:
-                write_executive_report(run_dir, self.final_scan_result)
-
-            # An index is written for an empty list too once a report was
-            # deleted, or the CSV/JSON on disk would still list it.
-            if self.vulnerability_reports or self._deleted_vulnerability_reports():
-                write_vulnerabilities(run_dir, self.vulnerability_reports, self._saved_vuln_ids)
-
-            # SARIF 2.1.0 emitter for CI / ASPM integration. Always emit (even
-            # empty) so a clean run overwrites a prior findings.sarif rather than
-            # leaving a stale one — codeql-action's "absent from new submission →
-            # fixed" needs the fresh empty doc to auto-resolve alerts. Isolated
-            # in its own try: a SARIF-build error must NEVER break the CSV/MD/
-            # run-record path (the emitter's own contract).
-            try:
-                write_sarif(
-                    run_dir,
-                    self.vulnerability_reports,
-                    tool_version=_strix_version(),
-                    repository_context=self._sarif_repository_context(),
-                    coverage=coverage,
-                )
-            except Exception:
-                logger.exception("SARIF emit failed (non-fatal; CSV/MD unaffected)")
-
-            write_run_record(run_dir, self.run_record)
-
-            logger.info("Essential scan data saved to: %s", run_dir)
+            self._write_artifacts()
         except (OSError, RuntimeError):
             logger.exception("Failed to save scan data")
 
+    def _write_artifacts(self) -> None:
+        """Write scan artifacts under ``run_dir``, raising when the index or
+        run record cannot be written."""
+        run_dir = self.get_run_dir()
+        run_dir.mkdir(parents=True, exist_ok=True)
+
+        coverage = self._coverage_document()
+        if coverage is not None:
+            try:
+                write_coverage(run_dir, coverage)
+            except OSError:
+                logger.exception("coverage.json write failed (non-fatal)")
+
+        if self.final_scan_result:
+            write_executive_report(run_dir, self.final_scan_result)
+
+        # An index is written for an empty list too once a report was
+        # deleted, or the CSV/JSON on disk would still list it.
+        if self.vulnerability_reports or self._deleted_vulnerability_reports():
+            write_vulnerabilities(run_dir, self.vulnerability_reports, self._saved_vuln_ids)
+
+        # SARIF 2.1.0 emitter for CI / ASPM integration. Always emit (even
+        # empty) so a clean run overwrites a prior findings.sarif rather than
+        # leaving a stale one — codeql-action's "absent from new submission →
+        # fixed" needs the fresh empty doc to auto-resolve alerts. Isolated
+        # in its own try: a SARIF-build error must NEVER break the CSV/MD/
+        # run-record path (the emitter's own contract).
+        try:
+            write_sarif(
+                run_dir,
+                self.vulnerability_reports,
+                tool_version=_strix_version(),
+                repository_context=self._sarif_repository_context(),
+                coverage=coverage,
+            )
+        except Exception:
+            logger.exception("SARIF emit failed (non-fatal; CSV/MD unaffected)")
+
+        write_run_record(run_dir, self.run_record)
+
+        logger.info("Essential scan data saved to: %s", run_dir)
+
     def _sarif_repository_context(self) -> dict[str, Any] | None:
         """Repo/commit/branch context for SARIF provenance (repo scans only).
 

strix/tools/reporting/tool.py

@@ -720,7 +720,7 @@ def _do_delete(
             ),
         }
 
-    from strix.report.state import get_global_report_state
+    from strix.report.state import ReportOwnershipError, get_global_report_state
 
     report_state = get_global_report_state()
     if report_state is None:
@@ -736,6 +736,15 @@ def _do_delete(
             deleted_by_agent_id=agent_id,
             deleted_by_agent_name=agent_name,
         )
+    except ReportOwnershipError as e:
+        return {
+            "success": False,
+            "error": (
+                f"{e!s}. If you have disproved another agent's finding, send that agent "
+                "your counterevidence so it can withdraw the report itself."
+            ),
+            "report_id": report_id,
+        }
     except Exception as e:
         logger.exception("delete_vulnerability_report persistence failed")
         return {
@@ -1645,7 +1654,8 @@ async def delete_vulnerability_report(
     impact and the counterevidence come down together. Do NOT rewrite a
     report into a "retracted" or "false positive" note either: delete it.
 
-    Only withdraw a report you can name and have re-tested yourself. Call
+    You can only withdraw a report you filed; a report filed by another
+    agent is refused, so send that agent your counterevidence instead. Call
     ``get_report`` first to read what it claims, then state in
     ``delete_reason`` exactly what disproved it, so the scan history shows
     why the finding is gone. A withdrawn report cannot be restored; if new

tests/test_reporting_fields.py

@@ -13,7 +13,7 @@ from strix.report.dedupe import (
     _prepare_report_for_comparison,
     check_duplicate,
 )
-from strix.report.state import ReportState, set_global_report_state
+from strix.report.state import ReportOwnershipError, ReportState, set_global_report_state
 from strix.tools.finish.tool import finish_scan
 from strix.tools.reporting import tool as reporting_tool
 from strix.tools.reporting.tool import (
@@ -2003,8 +2003,8 @@ def test_delete_withdraws_a_disproved_report_and_its_artifacts(
     result = _do_delete(
         report_id="vuln-0009",
         delete_reason="The cross-tenant read came from the harness reusing the victim's session.",
-        agent_id="834f79fb",
-        agent_name="Validation Agent",
+        agent_id="aaaa1111",
+        agent_name="Recon Agent",
     )
 
     assert result["success"] is True
@@ -2020,8 +2020,8 @@ def test_delete_withdraws_a_disproved_report_and_its_artifacts(
     assert persisted[0]["id"] == "vuln-0009"
     assert persisted[0]["agent_id"] == "aaaa1111", "the callback sees the finding as filed"
     deletion = persisted[0]["deletion"]
-    assert deletion["agent_id"] == "834f79fb"
-    assert deletion["agent_name"] == "Validation Agent"
+    assert deletion["agent_id"] == "aaaa1111"
+    assert deletion["agent_name"] == "Recon Agent"
     assert deletion["filed_by_agent_id"] == "aaaa1111"
     assert deletion["reason"].startswith("The cross-tenant read")
 
@@ -2035,7 +2035,9 @@ def test_delete_withdraws_a_disproved_report_and_its_artifacts(
 def test_delete_never_reissues_the_withdrawn_id(report_state: ReportState) -> None:
     """The next finding must not inherit a deleted report's id, file or history."""
     _seed_saved_report(report_state)
-    assert report_state.delete_vulnerability_report("vuln-0009", delete_reason="Disproved.")
+    assert report_state.delete_vulnerability_report(
+        "vuln-0009", delete_reason="Disproved.", deleted_by_agent_id="aaaa1111"
+    )
 
     new_id = report_state.add_vulnerability_report(title="A real finding", severity="high")
 
@@ -2043,26 +2045,151 @@ def test_delete_never_reissues_the_withdrawn_id(report_state: ReportState) -> No
     assert [r["id"] for r in report_state.vulnerability_reports] == ["vuln-0010"]
 
 
+def _assert_report_still_filed(
+    report_state: ReportState, run_dir: Path, original: dict[str, Any]
+) -> None:
+    """The report is in memory and in every artifact exactly as it was filed."""
+    assert report_state.vulnerability_reports == [original]
+    assert "vuln-0009" in report_state._saved_vuln_ids
+    assert (run_dir / "vulnerabilities" / "vuln-0009.md").exists()
+    indexed = json.loads((run_dir / "vulnerabilities.json").read_text(encoding="utf-8"))
+    assert [r["id"] for r in indexed] == ["vuln-0009"]
+    assert "vuln-0009" in (run_dir / "vulnerabilities.csv").read_text(encoding="utf-8")
+    run = json.loads((run_dir / "run.json").read_text(encoding="utf-8"))
+    assert "deleted_vulnerability_reports" not in run
+    assert "deleted_vulnerability_reports" not in report_state.run_record
+    assert report_state._next_report_id() == "vuln-0010"
+
+
+@pytest.mark.parametrize(
+    ("agent_id", "agent_name"),
+    [("834f79fb", "Validation Agent"), (None, None)],
+    ids=["another agent", "no caller identity"],
+)
+def test_delete_refuses_a_report_the_caller_did_not_file(
+    report_state: ReportState, agent_id: str | None, agent_name: str | None
+) -> None:
+    """Report ids are sequential and listed to every agent; knowing one is not owning it."""
+    run_dir = _seed_saved_report(report_state)
+    original = dict(report_state.vulnerability_reports[0])
+    calls: list[dict[str, Any]] = []
+    report_state.vulnerability_deleted_callback = calls.append
+
+    result = _do_delete(
+        report_id="vuln-0009",
+        delete_reason="Disproved.",
+        agent_id=agent_id,
+        agent_name=agent_name,
+    )
+
+    assert result["success"] is False
+    assert "filed by agent aaaa1111" in result["error"]
+    assert "send that agent your counterevidence" in result["error"]
+    assert calls == [], "a refused deletion never reaches persistence"
+    _assert_report_still_filed(report_state, run_dir, original)
+
+
+def test_delete_refuses_a_foreign_report_at_the_state_layer(report_state: ReportState) -> None:
+    _seed_saved_report(report_state)
+
+    with pytest.raises(ReportOwnershipError) as excinfo:
+        report_state.delete_vulnerability_report(
+            "vuln-0009", delete_reason="Disproved.", deleted_by_agent_id="834f79fb"
+        )
+
+    assert excinfo.value.filed_by == "aaaa1111"
+    assert excinfo.value.caller == "834f79fb"
+    assert len(report_state.vulnerability_reports) == 1
+
+
+def test_delete_allows_a_report_filed_without_an_agent(report_state: ReportState) -> None:
+    """A report with no filer on record (host-side or legacy) has no owner to check."""
+    _seed_saved_report(report_state)
+    del report_state.vulnerability_reports[0]["agent_id"]
+
+    deleted = report_state.delete_vulnerability_report(
+        "vuln-0009", delete_reason="Disproved.", deleted_by_agent_id="834f79fb"
+    )
+
+    assert deleted is not None
+    assert report_state.vulnerability_reports == []
+    history = report_state.run_record["deleted_vulnerability_reports"]
+    assert history[0]["agent_id"] == "834f79fb"
+    assert "filed_by_agent_id" not in history[0]
+
+
 def test_delete_reports_persistence_failure_and_keeps_the_report(
     report_state: ReportState,
 ) -> None:
+    """Persistence refused after the local indexes were rewritten: they are rewritten back."""
     run_dir = _seed_saved_report(report_state)
     original = dict(report_state.vulnerability_reports[0])
 
     def fail_persistence(_report: dict[str, Any]) -> None:
+        assert report_state.vulnerability_reports == [], "indexes are rewritten first"
         raise RuntimeError("persistence failed")
 
     report_state.vulnerability_deleted_callback = fail_persistence
 
-    result = _do_delete(report_id="vuln-0009", delete_reason="Disproved.")
+    result = _do_delete(report_id="vuln-0009", delete_reason="Disproved.", agent_id="aaaa1111")
 
     assert result["success"] is False
     assert "persistence failed" in result["error"]
     assert "still on file" in result["error"]
-    assert report_state.vulnerability_reports == [original]
-    assert (run_dir / "vulnerabilities" / "vuln-0009.md").exists()
-    assert "deleted_vulnerability_reports" not in report_state.run_record
-    assert report_state._next_report_id() == "vuln-0010"
+    _assert_report_still_filed(report_state, run_dir, original)
+
+
+def test_delete_reports_an_artifact_write_failure_and_keeps_the_report(
+    report_state: ReportState, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """A disk that cannot take the rewritten run record is a failed deletion, not a
+    success with stale artifacts; persistence is never told about it."""
+    run_dir = _seed_saved_report(report_state)
+    original = dict(report_state.vulnerability_reports[0])
+    calls: list[dict[str, Any]] = []
+    report_state.vulnerability_deleted_callback = calls.append
+    attempts = 0
+
+    def write_run_record_once_failing(run_dir_: Path, record: dict[str, Any]) -> None:
+        nonlocal attempts
+        attempts += 1
+        if attempts == 1:
+            raise OSError(28, "No space left on device")
+        (run_dir_ / "run.json").write_text(json.dumps(record), encoding="utf-8")
+
+    monkeypatch.setattr("strix.report.state.write_run_record", write_run_record_once_failing)
+
+    result = _do_delete(report_id="vuln-0009", delete_reason="Disproved.", agent_id="aaaa1111")
+
+    assert result["success"] is False
+    assert "No space left on device" in result["error"]
+    assert "still on file" in result["error"]
+    assert calls == []
+    assert attempts == 2, "the run record is written back after the failed rewrite"
+    _assert_report_still_filed(report_state, run_dir, original)
+
+
+def test_delete_can_be_retried_after_persistence_recovers(report_state: ReportState) -> None:
+    run_dir = _seed_saved_report(report_state)
+    outcomes = iter([RuntimeError("persistence failed"), None])
+
+    def flaky_persistence(_report: dict[str, Any]) -> None:
+        outcome = next(outcomes)
+        if outcome is not None:
+            raise outcome
+
+    report_state.vulnerability_deleted_callback = flaky_persistence
+
+    first = _do_delete(report_id="vuln-0009", delete_reason="Disproved.", agent_id="aaaa1111")
+    second = _do_delete(report_id="vuln-0009", delete_reason="Disproved.", agent_id="aaaa1111")
+
+    assert first["success"] is False
+    assert second["success"] is True
+    assert report_state.vulnerability_reports == []
+    assert not (run_dir / "vulnerabilities" / "vuln-0009.md").exists()
+    assert json.loads((run_dir / "vulnerabilities.json").read_text(encoding="utf-8")) == []
+    run = json.loads((run_dir / "run.json").read_text(encoding="utf-8"))
+    assert [e["id"] for e in run["deleted_vulnerability_reports"]] == ["vuln-0009"]
 
 
 @pytest.mark.parametrize(
@@ -2083,7 +2210,7 @@ def test_delete_rejects_a_call_it_cannot_act_on(
     calls: list[dict[str, Any]] = []
     report_state.vulnerability_deleted_callback = calls.append
 
-    result = _do_delete(report_id=report_id, delete_reason=delete_reason)
+    result = _do_delete(report_id=report_id, delete_reason=delete_reason, agent_id="aaaa1111")
 
     assert result["success"] is False
     assert expected in result["error"]
@@ -2093,9 +2220,9 @@ def test_delete_rejects_a_call_it_cannot_act_on(
 
 async def test_delete_tool_carries_the_caller_identity(report_state: ReportState) -> None:
     _seed_saved_report(report_state)
-    coordinator = type("Coordinator", (), {"names": {"834f79fb": "Validation Agent"}})()
+    coordinator = type("Coordinator", (), {"names": {"aaaa1111": "Recon Agent"}})()
     ctx = ToolContext(
-        context={"agent_id": "834f79fb", "coordinator": coordinator},
+        context={"agent_id": "aaaa1111", "coordinator": coordinator},
         tool_name="delete_vulnerability_report",
         tool_call_id="call-1",
         tool_arguments="{}",
@@ -2109,6 +2236,6 @@ async def test_delete_tool_carries_the_caller_identity(report_state: ReportState
 
     assert result["success"] is True
     history = report_state.run_record["deleted_vulnerability_reports"]
-    assert history[0]["agent_id"] == "834f79fb"
-    assert history[0]["agent_name"] == "Validation Agent"
+    assert history[0]["agent_id"] == "aaaa1111"
+    assert history[0]["agent_name"] == "Recon Agent"
     assert "update_vulnerability_report" in delete_vulnerability_report.description