fix: detach Ghidra enrichment on reveal — target_reveal_dir hung 2+ hours (#275)

* fix: detach Ghidra enrichment on reveal — target_reveal_dir hung 2+ hours

Revealing a firmware child materializes its recon nodes AND (if Ghidra
headless enrichment is enabled) runs Ghidra's full-inventory analysis on it —
previously synchronous, inline, inside the reveal call. reveal_dir loops over
every hidden child under a directory prefix and calls this per child with no
aggregate bound. A real incident: revealing an Erlang erts/bin directory (a
dozen-plus binaries — erl, beam.smp, dialyzer, run_erl, ...) ran a cold
headless Ghidra full-analysis on each one sequentially, hanging the MCP call
for 2+ hours. Same architectural bug class as promote_file (fixed in #274
earlier tonight) — a bulk operation fanning out N synchronous heavy per-item
calls inside one blocking request, no detachment, no bound.

Split _materialize_on_reveal: node materialization from already-stored recon
facts stays synchronous (fast, no re-run). Ghidra enrichment now runs
DETACHED via a new `ghidra_enrich` Task type, reusing the
create_task/spawn_detached_task infrastructure built for the promote_file
fix. Idempotent + self-healing, same pattern: a target already marked
enriched or with a queued/running enrichment task is skipped; a task that
ended without marking success (died, or a soft failure) gets retried on the
next reveal call rather than silently staying unenriched forever.

set_visible/reveal_dir now report enrichment_queued so callers (MCP, REST,
the FilesystemBrowser UI) know background work is still in flight rather
than assuming a fast return means nothing happened.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix: batch reveal_dir's Ghidra enrichment into one detached process

Review of #275 flagged a real gap: reveal_dir spawned one INDEPENDENT
detached process per revealed binary needing enrichment. A directory with a
dozen-plus binaries (the incident's own erts/bin case) would launch that
many CONCURRENT cold headless Ghidra containers (--memory 2g --cpus 2.0
each) — genuine new resource contention the original blocking code never
had (it ran them one at a time, just synchronously).

reveal_dir now collects every target that needs enrichment across the whole
call and queues ONE `ghidra_enrich_batch` task that processes them
sequentially in a single detached process — still off the request path, but
no concurrency blowup. set_visible's single-target path is unaffected (spawning
one process for one target was never the problem). This also incidentally
restores reveal_dir's prior single-commit-point atomicity (the per-child
commit the review flagged as a second, smaller issue) as a side effect of
consolidating the spawn to one place.

Also: mark the Task failed (not stuck "queued" forever) if spawn_detached_task
itself raises, in both the single and batch paths — a permanently-queued task
would wrongly satisfy the already-running self-heal check and block every
future retry.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: branover

frontend/src/api.ts

@@ -294,9 +294,11 @@ export const api = {
   promoteFile: (pid: string, fwId: string, rel: string) => postJSON<any>(`/api/projects/${pid}/targets/${fwId}/promote-file`, { rel }),
   // Reveal/re-hide a target in the curated graph. Firmware ELF children are hidden by default
   // (unpack registers them all); revealing materializes the target's recon nodes from stored facts.
-  setTargetVisible: (pid: string, tid: string, visible: boolean) => postJSON<{ target_id: string; name: string; visible: boolean; materialized: boolean }>(`/api/projects/${pid}/targets/${tid}/visible`, { visible }),
+  setTargetVisible: (pid: string, tid: string, visible: boolean) => postJSON<{ target_id: string; name: string; visible: boolean; materialized: boolean; enrichment_queued: boolean }>(`/api/projects/${pid}/targets/${tid}/visible`, { visible }),
   // Bulk-reveal every hidden firmware child whose rootfs path is under `prefix` (e.g. "usr/sbin").
-  revealDir: (pid: string, fwId: string, prefix: string) => postJSON<{ firmware_target_id: string; prefix: string; revealed: number; target_ids: string[] }>(`/api/projects/${pid}/targets/${fwId}/reveal-dir`, { prefix }),
+  // `enrichment_queued` = how many revealed targets got a background Ghidra enrichment task
+  // (revealing/materializing itself is synchronous and fast; enrichment is not).
+  revealDir: (pid: string, fwId: string, prefix: string) => postJSON<{ firmware_target_id: string; prefix: string; revealed: number; target_ids: string[]; enrichment_queued: number }>(`/api/projects/${pid}/targets/${fwId}/reveal-dir`, { prefix }),
   // Target-level follow-up suggestions from enriched recon metadata (e.g. risky-sink → static-analyze).
   targetSuggestions: (tid: string) => getJSON<any[]>(`/api/targets/${tid}/suggestions`),
   // Source trees (Phase 1 — read-only IDE browse)

frontend/src/components/FilesystemBrowser.tsx

@@ -74,6 +74,12 @@ export default function FilesystemBrowser({ projectId, targetId, onChanged }: {
     try {
       const r = await api.revealDir(projectId, targetId, prefix);
       if (r.revealed === 0) alert("No hidden binaries to reveal under " + (prefix || "/"));
+      // Ghidra enrichment per binary (if enabled) runs in the background — a directory with
+      // many binaries would otherwise block this call for a long time (each is a cold headless
+      // Ghidra pass). The binaries are already revealed/searchable; enrichment just deepens them.
+      else if (r.enrichment_queued > 0)
+        alert(`Revealed ${r.revealed} binaries. Ghidra enrichment for ${r.enrichment_queued} of `
+          + `them is running in the background and may take a while.`);
       await load(); onChanged?.();
     } catch (e: any) { alert(String(e.message || e)); }
     finally { setBusy(""); }

src/hexgraph/agent/mcp_catalog.py

@@ -125,9 +125,9 @@
      {"type": "object", "properties": {"project_id": {"type": "string"}, "target_id": {"type": "string"}}, "required": ["project_id", "target_id"]}),
     ("write", "target_restore", _t.restore_target, "Un-archive a previously soft-removed target subtree (its nodes/findings reappear).",
      {"type": "object", "properties": {"project_id": {"type": "string"}, "target_id": {"type": "string"}}, "required": ["project_id", "target_id"]}),
-    ("write", "target_set_visible", _t.set_visible, "REVEAL (visible=true) or re-HIDE (visible=false) ONE target in the curated graph. Firmware ELF children are hidden by default — unpack registers each (searchable via target_list(include_hidden=true), addressable, recon-enriched) but they'd flood the graph, so they contribute NOTHING to it until revealed. Revealing materializes the target's recon symbol/string nodes from the already-stored facts (no re-run), so it joins the graph + Targets pane. Use after target_list(include_hidden=true)/fs_list surfaces a child worth analyzing. Returns {target_id, name, visible, materialized}.",
+    ("write", "target_set_visible", _t.set_visible, "REVEAL (visible=true) or re-HIDE (visible=false) ONE target in the curated graph. Firmware ELF children are hidden by default — unpack registers each (searchable via target_list(include_hidden=true), addressable, recon-enriched) but they'd flood the graph, so they contribute NOTHING to it until revealed. Revealing materializes the target's recon symbol/string nodes from the already-stored facts (no re-run) so it joins the graph + Targets pane immediately; optional Ghidra enrichment (if enabled) runs DETACHED in the background — a cold headless pass can take minutes, this call does not wait for it. Use after target_list(include_hidden=true)/fs_list surfaces a child worth analyzing. Returns {target_id, name, visible, materialized, enrichment_queued}.",
      {"type": "object", "properties": {"project_id": {"type": "string"}, "target_id": {"type": "string"}, "visible": {"type": "boolean", "description": "true to reveal (default), false to re-hide"}}, "required": ["project_id", "target_id"]}),
-    ("write", "target_reveal_dir", _t.reveal_dir, "REVEAL every HIDDEN child of a firmware whose rootfs path is under `prefix` (e.g. prefix='usr/sbin' reveals all ELFs in /usr/sbin) — the bulk counterpart to target_set_visible for bringing a whole directory of binaries into the curated graph at once. An empty prefix reveals ALL hidden children. Materializes each revealed child's recon nodes from stored facts (no re-run). `target_id` is the firmware. Returns {firmware_target_id, prefix, revealed, target_ids}.",
+    ("write", "target_reveal_dir", _t.reveal_dir, "REVEAL every HIDDEN child of a firmware whose rootfs path is under `prefix` (e.g. prefix='usr/sbin' reveals all ELFs in /usr/sbin) — the bulk counterpart to target_set_visible for bringing a whole directory of binaries into the curated graph at once. An empty prefix reveals ALL hidden children. Materializes each revealed child's recon nodes from stored facts (no re-run) — fast, even for a directory with many binaries; optional per-binary Ghidra enrichment (if enabled) runs DETACHED in the background instead of blocking this call (a dozen+ binaries analyzed sequentially can take hours). `target_id` is the firmware. Returns {firmware_target_id, prefix, revealed, target_ids, enrichment_queued}.",
      {"type": "object", "properties": {"project_id": {"type": "string"}, "target_id": {"type": "string", "description": "the firmware target whose children to reveal"}, "prefix": {"type": "string", "description": "rootfs directory prefix (e.g. 'usr/sbin'); empty reveals all hidden children"}}, "required": ["project_id", "target_id"]}),
 
     # ---- re: static reverse engineering ---------------------------------------------

src/hexgraph/agent/mcp_tools.py

@@ -1548,8 +1548,10 @@ def set_visible(project_id: str, target_id: str, visible: bool = True) -> dict:
     Firmware ELF children are HIDDEN by default (unpack registers each so it's searchable
     and addressable, but a 765-ELF firmware would otherwise flood the graph/Targets pane);
     recon already enriched them. Revealing materializes the target's recon nodes from the
-    already-stored facts (no re-run) so it joins the graph. Returns
-    {target_id, name, visible, materialized}."""
+    already-stored facts (no re-run) so it joins the graph immediately; optional Ghidra
+    enrichment (if enabled) runs DETACHED in the background — `enrichment_queued` says
+    whether a task was kicked off (a cold headless Ghidra pass can take minutes; this call
+    does not wait for it). Returns {target_id, name, visible, materialized, enrichment_queued}."""
     from hexgraph.engine.targets.reveal import set_visible as _set
 
     with session_scope() as s:
@@ -1566,8 +1568,13 @@ def reveal_dir(project_id: str, target_id: str, prefix: str = "") -> dict:
     (e.g. prefix='usr/sbin' reveals all ELFs in /usr/sbin) — the bulk counterpart to
     target_set_visible for bringing a whole directory of binaries into the curated graph
     at once. An empty prefix reveals ALL hidden children. Materializes each revealed
-    child's recon nodes from stored facts (no re-run). `target_id` is the firmware.
-    Returns {firmware_target_id, prefix, revealed, target_ids}."""
+    child's recon nodes from stored facts (no re-run) — fast, even for a directory with many
+    binaries. Optional Ghidra enrichment per binary (if enabled) runs DETACHED in the
+    background rather than blocking this call (a cold headless Ghidra pass on a dozen+
+    binaries sequentially can take hours; this call returns as soon as they're revealed).
+    `target_id` is the firmware. Returns {firmware_target_id, prefix, revealed, target_ids,
+    enrichment_queued} (enrichment_queued = how many revealed targets got a background
+    enrichment task)."""
     # NB: the catalog advertises this arg as `target_id` (the firmware), and the MCP
     # server dispatches by KEYWORD (`fn(**arguments)`), so this param name MUST match
     # the catalog schema — otherwise every MCP call raises TypeError.

src/hexgraph/agent/vr_skill.py

@@ -417,6 +417,10 @@
   `fs_list` entry that's `added` but not `revealed` already IS a target — reveal it (recon
   already enriched it; reveal materializes its nodes, no re-run) rather than re-promoting.
   Reveal the binaries worth analyzing, then decompile / `task_run` / fuzz them like any other.
+  If Ghidra enrichment is enabled, it runs DETACHED per revealed binary (`enrichment_queued`
+  in the response) — a directory can have a dozen-plus binaries, each a cold headless Ghidra
+  pass; don't expect it done by the time this call returns, and don't loop calling
+  `target_reveal_dir` back-to-back on overlapping prefixes waiting for it to "finish".
 - **target_promote_file(target_id, path)** — promote a NON-ELF file or one unpack didn't register
   (a CGI script, a helper) into its OWN child target (created visible), then analyze it. Use
   `target_set_visible`/`target_reveal_dir` for the ELF children that already exist hidden, and

src/hexgraph/engine/targets/reveal.py

@@ -45,47 +45,147 @@ def _recon_facts(session: Session, project: Project, target: Target) -> dict:
     return target.metadata_json or {}
 
 
-def _materialize_on_reveal(session: Session, project: Project, target: Target) -> None:
-    """Bring a just-revealed target's enrichment into the curated graph: its recon
-    symbol/string nodes (from stored facts) plus the optional Ghidra enrichment that
-    was deferred while it was hidden. Idempotent (materialize_* dedups)."""
+def _materialize_recon_only(session: Session, project: Project, target: Target) -> dict:
+    """The fast, synchronous half of revealing: materialize `target`'s recon symbol/string
+    nodes from already-stored facts (no re-run). Returns the facts dict so the caller can
+    decide whether Ghidra enrichment applies (see `_needs_ghidra_enrichment`)."""
     from hexgraph.engine.re.recon import materialize_recon_nodes
 
     facts = _recon_facts(session, project, target)
     materialize_recon_nodes(session, project.id, target, facts)
-    # Mirror analyze_target's optional Ghidra enrich pass (skipped while hidden).
-    try:
-        from hexgraph.engine.re.ghidra import enrich_enabled, enrich_target
+    return facts
+
+
+def _needs_ghidra_enrichment(target: Target) -> bool:
+    """Whether `target` still wants Ghidra enrichment: the feature is on and it hasn't
+    already been recorded (see the `ghidra_enrich`/`ghidra_enrich_batch` task dispatch,
+    which marks `ghidra_enriched` on success only — a soft failure or a killed task leaves
+    it unmarked so a later reveal retries instead of silently giving up forever)."""
+    from hexgraph.engine.re.ghidra import enrich_enabled
+
+    return enrich_enabled() and not (target.metadata_json or {}).get("ghidra_enriched")
 
-        if enrich_enabled() and (facts.get("kind") in ("executable", "shared_library")):
-            enrich_target(session, project, target)
+
+def _materialize_on_reveal(session: Session, project: Project, target: Target) -> bool:
+    """Bring a just-revealed target's enrichment into the curated graph: recon nodes (fast,
+    synchronous) plus optional Ghidra enrichment, kicked off DETACHED (see
+    `_ensure_ghidra_enrichment`) rather than run inline — a cold headless Ghidra full-analysis
+    can take many minutes. Used by `set_visible` (ONE target — see `reveal_dir` for the bulk
+    path, which batches enrichment instead of spawning one detached process per target).
+    Idempotent (materialize_* dedups). Returns True if Ghidra enrichment was (newly) queued."""
+    facts = _materialize_recon_only(session, project, target)
+    if facts.get("kind") not in ("executable", "shared_library"):
+        return False
+    try:
+        return _ensure_ghidra_enrichment(session, project, target) if _needs_ghidra_enrichment(target) else False
     except Exception:  # noqa: BLE001 — enrichment is an optional bonus pass
-        pass
+        return False
+
+
+def _ensure_ghidra_enrichment(session: Session, project: Project, target: Target) -> bool:
+    """Kick off `target`'s optional Ghidra enrichment (`engine.re.ghidra.enrich_target`) in a
+    DETACHED background OS process if it isn't already in flight — same pattern as
+    `engine.targets.filesystem._ensure_analysis`. Caller must have already checked
+    `_needs_ghidra_enrichment`. Safe to call repeatedly: no-ops while a `ghidra_enrich` Task
+    for this target is still queued/running. Returns True if a task was (newly) queued."""
+    from hexgraph.db.models import Task, TaskStatus
+
+    already_running = (
+        session.query(Task)
+        .filter(Task.target_id == target.id, Task.type == "ghidra_enrich",
+                Task.status.in_((TaskStatus.queued, TaskStatus.running)))
+        .first()
+    )
+    if already_running is not None:
+        return False
+
+    from hexgraph.engine.tasks import create_task
+    from hexgraph.engine.worker import spawn_detached_task
+    from sqlalchemy.orm.attributes import flag_modified
+
+    task = create_task(session, project=project, target_id=target.id, type="ghidra_enrich")
+    meta = dict(target.metadata_json or {})
+    meta["ghidra_enrich_task_id"] = task.id
+    target.metadata_json = meta
+    flag_modified(target, "metadata_json")
+    session.commit()
+    try:
+        spawn_detached_task(task.id)
+    except Exception:  # noqa: BLE001 — if the spawn itself fails (fork/exec resource
+        # exhaustion), mark the task failed rather than leaving it stuck "queued" forever —
+        # a permanently-queued task would wrongly satisfy the already_running check above
+        # and block every future reveal from ever retrying.
+        from hexgraph.engine.tasks import mark_failed
+
+        mark_failed(task, "failed to spawn detached enrichment process")
+        session.commit()
+        return False
+    return True
+
+
+def _ensure_batch_ghidra_enrichment(session: Session, project: Project, firmware: Target,
+                                    target_ids: list[str]) -> int:
+    """Kick off Ghidra enrichment for MANY targets in ONE detached process that runs them
+    SEQUENTIALLY, not one detached process per target. `reveal_dir` can reveal a dozen+
+    binaries in one call — each is its own cold headless Ghidra analysis (`--memory 2g
+    --cpus 2.0` per the sandbox spec), so spawning one INDEPENDENT process per target would
+    launch that many CONCURRENT containers and genuinely contend for host resources, unlike
+    `promote_file`'s single detached analysis. The Task is anchored on `firmware` (there's no
+    single natural target for a batch); `target_ids` travel in `params_json`. Returns how
+    many targets were queued (0 if none needed it or the feature is off)."""
+    if not target_ids:
+        return 0
+
+    from hexgraph.engine.tasks import create_task
+    from hexgraph.engine.worker import spawn_detached_task
+
+    task = create_task(session, project=project, target_id=firmware.id, type="ghidra_enrich_batch",
+                       params={"target_ids": target_ids})
+    session.commit()
+    try:
+        spawn_detached_task(task.id)
+    except Exception:  # noqa: BLE001 — enrichment is optional, must never break reveal_dir;
+        # same self-heal reasoning as _ensure_ghidra_enrichment above.
+        from hexgraph.engine.tasks import mark_failed
+
+        mark_failed(task, "failed to spawn detached enrichment process")
+        session.commit()
+        return 0
+    return len(target_ids)
 
 
 def set_visible(session: Session, project_id: str, target_id: str, visible: bool) -> dict:
     """Reveal (visible=True) or re-hide (visible=False) one target. Revealing
-    materializes its recon nodes from the already-stored facts (no re-run). Returns
-    {target_id, visible, materialized} (materialized = nodes were (re)materialized)."""
+    materializes its recon nodes from the already-stored facts (no re-run); optional Ghidra
+    enrichment runs detached (see `_materialize_on_reveal`). Returns {target_id, visible,
+    materialized} (materialized = nodes were (re)materialized)."""
     t = session.get(Target, target_id)
     if t is None or t.project_id != project_id:
         raise ValueError("target not found in project")
     was_visible = t.visible
     t.visible = visible
     materialized = False
+    enrichment_queued = False
     if visible and not was_visible:
         project = session.get(Project, project_id)
-        _materialize_on_reveal(session, project, t)
+        enrichment_queued = _materialize_on_reveal(session, project, t)
         materialized = True
     session.flush()
-    return {"target_id": t.id, "name": t.name, "visible": t.visible, "materialized": materialized}
+    return {"target_id": t.id, "name": t.name, "visible": t.visible, "materialized": materialized,
+            "enrichment_queued": enrichment_queued}
 
 
 def reveal_dir(session: Session, project_id: str, firmware_target_id: str, prefix: str) -> dict:
     """Reveal every HIDDEN child of a firmware whose rootfs-relative name (path) is
     under `prefix` (a directory prefix like "usr/sbin" or "/usr/sbin"). Materializes
-    each revealed child's recon nodes. Returns {firmware_target_id, prefix, revealed,
-    target_ids}. Already-visible children are left untouched."""
+    each revealed child's recon nodes (fast); optional Ghidra enrichment for the whole batch
+    runs as ONE detached background process working through them sequentially — not one
+    process per target (see `_ensure_batch_ghidra_enrichment`: a directory can have a dozen+
+    binaries, and spawning that many CONCURRENT cold headless Ghidra containers would
+    contend hard for host resources, unlike a single target's `set_visible`).
+    Returns {firmware_target_id, prefix, revealed, target_ids, enrichment_queued}
+    (enrichment_queued = how many revealed targets got queued into that background batch).
+    Already-visible children are left untouched."""
     fw = session.get(Target, firmware_target_id)
     if fw is None or fw.project_id != project_id:
         raise ValueError("firmware target not found in project")
@@ -115,17 +215,22 @@ def _under(rel: str) -> bool:
         .all()
     )
     revealed_ids: list[str] = []
+    to_enrich: list[str] = []
     for c in children:
         if c.visible:
             continue
         if c.id in ids_under_prefix or _under(c.name):   # any manifest path under prefix, or its own name
             c.visible = True
-            _materialize_on_reveal(session, project, c)
+            facts = _materialize_recon_only(session, project, c)
+            if facts.get("kind") in ("executable", "shared_library") and _needs_ghidra_enrichment(c):
+                to_enrich.append(c.id)
             revealed_ids.append(c.id)
     session.flush()
+    enrichment_queued = _ensure_batch_ghidra_enrichment(session, project, fw, to_enrich)
     return {
         "firmware_target_id": firmware_target_id,
         "prefix": prefix,
         "revealed": len(revealed_ids),
         "target_ids": revealed_ids,
+        "enrichment_queued": enrichment_queued,
     }