feat: Phase 4 — Integration & CLI Hardening#85
Conversation
- Add gnat investigation / plugins / db CLI subcommand groups - Wire LineageTracker into IngestPipeline, ExportPipeline, ReportService - Bridge MetricsCollector to HookBus (register_metrics_hooks / unregister) - Add TUI InvestigationsScreen (F5) with DataTable, search, transitions - Wire InvestigationsScreen into GNATApp (F5 binding, db_url= param) - Extend test_tui, test_lineage, test_metrics; add test_cli_phase4 (65 tests) - Update CHANGELOG.md [Unreleased] with Phase 4 entries https://claude.ai/code/session_01BDoue9HxB83ijLzFARAugq
Signed-off-by: Bill <wrhalpin@gmail.com>
There was a problem hiding this comment.
Pull request overview
Implements Phase 4 integration features across CLI, TUI, lineage, and metrics by adding new subcommand groups, a new Investigations TUI screen, and wiring LineageTracker + MetricsCollector into core workflows.
Changes:
- Added CLI subcommand groups:
gnat investigation,gnat plugins, andgnat db, plus extendedgnat tuiscreen choices. - Added an Investigations TUI screen (F5) and wired it into the main GNAT TUI app (including
db_urlplumbing). - Wired optional lineage emission into ingest/export pipelines and report publishing; added a HookBus→MetricsCollector bridge.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_tui.py | Extends TUI tests for InvestigationsScreen, F5 binding, db_url, and 5-tab layout. |
| tests/unit/test_metrics.py | Adds tests for the new metrics HookBus bridge registration/unregistration behavior. |
| tests/unit/test_lineage.py | Adds/extends lineage wiring tests for ingest/export/report flows. |
| tests/unit/test_cli_phase4.py | Adds Phase 4 CLI unit tests for investigation/plugins/db subcommands and error paths. |
| gnat/tui/screens/investigations.py | Introduces the new Investigations TUI screen with search/filter/table/detail + transitions. |
| gnat/tui/app.py | Wires Investigations screen into GNATApp (F5 binding, tab, db_url passthrough). |
| gnat/reporting/service.py | Adds optional lineage= to ReportService and emits lineage on publish. |
| gnat/metrics/hooks.py | Adds HookBus bridge to record metrics from investigation/report/gap HookBus events. |
| gnat/metrics/init.py | Exports register_metrics_hooks / unregister_metrics_hooks. |
| gnat/ingest/pipeline/pipeline.py | Adds optional lineage tracking and emits lineage after successful object saves. |
| gnat/export/base.py | Adds optional lineage tracking and emits lineage after successful delivery. |
| gnat/cli/main.py | Adds new CLI parsers + command handlers for investigation/plugins/db; extends TUI choices. |
| CHANGELOG.md | Documents Phase 4 additions and test coverage updates under Unreleased. |
Comments suppressed due to low confidence (2)
gnat/cli/main.py:2012
config_pathis computed but never used. This is dead code and can be removed, or the value should actually be used to resolve DB URL / service configuration if intended.
from gnat.serve.config import WebUIConfig
gnat/export/base.py:519
- Lineage recording uses
getattr(obj, "type", "unknown"), but GNAT ORM objects (STIXBase) usestix_typefor the STIX object type. This will record "unknown" (or a non-STIX property namedtype) instead of the actual STIX object type. Useobj.stix_type/obj.to_dict()["type"]for the lineageobject_typevalue.
self.pipeline_id,
result.source_objects,
result.filtered_objects,
tr.object_count,
len(tr.payloads),
tr.total_bytes(),
)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if sub == "link": | ||
| indicators = [i.strip() for i in args.indicators.split(",") if i.strip()] | ||
| reports = [r.strip() for r in args.reports.split(",") if r.strip()] | ||
| try: | ||
| if indicators: |
There was a problem hiding this comment.
_cmd_plugins() instantiates PluginRegistry via PluginRegistry() instead of using the documented process-level singleton PluginRegistry.instance(). Since load_plugins() loads into the singleton registry, gnat plugins list will likely print “(no plugins loaded)” even when plugins were loaded successfully. Use PluginRegistry.instance() here (and for the load path) so list/load operate on the same registry instance used elsewhere (see gnat/plugins/loader.py:55 and gnat/plugins/registry.py:70).
| try: | ||
| self._lineage.record_ingest( | ||
| obj.id, | ||
| getattr(obj, "type", "unknown"), |
There was a problem hiding this comment.
Lineage recording uses getattr(obj, "type", "unknown"), but GNAT ORM objects (STIXBase) store the STIX object type on stix_type and do not expose it as an attribute named type. As a result, lineage will record object_type as "unknown" for most objects. Use obj.stix_type (or obj.to_dict()["type"]) when emitting lineage events.
| getattr(obj, "type", "unknown"), | |
| getattr(obj, "stix_type", "unknown"), |
| def register_metrics_hooks(collector: Any) -> None: | ||
| """ | ||
| Register HookBus handlers that forward events to *collector*. | ||
|
|
||
| Safe to call multiple times — duplicate registrations are ignored by | ||
| the bus because each call creates new closures. | ||
|
|
There was a problem hiding this comment.
The docstring claims register_metrics_hooks() is safe to call multiple times and that duplicate registrations are ignored, but HookBus.register() simply appends handlers (no de-dupe). Calling register_metrics_hooks() twice will double-count all metrics. Make registration idempotent (e.g., call unregister_metrics_hooks() first, or track a “registered” flag / per-collector registration) and/or update the docstring to reflect actual behavior.
| """Push a simple inline creation flow (uses Input widget).""" | ||
| # Simple approach: prompt for title via status bar | ||
| self._set_status( | ||
| "[yellow]Enter title in search box and press Ctrl+N again to create.[/]" | ||
| ) | ||
|
|
There was a problem hiding this comment.
action_new_dialog() and its status message claim “press Ctrl+N again to create”, and the module docstring says the screen allows creating new investigations, but no create path exists (Ctrl+N just updates the status label). Either implement creation (e.g., call self._service.create(...) using the Input value) or adjust the messaging/docs so users aren’t instructed to perform an action that does nothing.
| """Push a simple inline creation flow (uses Input widget).""" | |
| # Simple approach: prompt for title via status bar | |
| self._set_status( | |
| "[yellow]Enter title in search box and press Ctrl+N again to create.[/]" | |
| ) | |
| """Create a new investigation using the search box as the title input.""" | |
| if self._service is None: | |
| self._set_status("[red]Investigation storage not available.[/]") | |
| return | |
| title_input = self.query_one("#search-input", Input) | |
| title = title_input.value.strip() | |
| if not title: | |
| self._set_status( | |
| "[yellow]Enter title in search box and press Ctrl+N again to create.[/]" | |
| ) | |
| return | |
| try: | |
| inv = self._service.create(title=title) | |
| title_input.value = "" | |
| self._load_investigations() | |
| inv_id = getattr(inv, "id", None) | |
| if inv_id: | |
| self._set_status(f"[green]✓ Created investigation {inv_id[:8]}…[/]") | |
| else: | |
| self._set_status(f"[green]✓ Created investigation: {title}[/]") | |
| except Exception as exc: | |
| self._set_status(f"[red]Create failed: {exc}[/]") |
| logger.info("ReportService: published report %s (v%d)", report.id, report.version) | ||
|
|
||
| # Lineage: emit REPORTED event for each linked evidence object | ||
| if self._lineage is not None: | ||
| try: | ||
| stix_ref = report.stix_report_ref or f"report--{report.id}" | ||
| self._lineage.record_report( | ||
| stix_ref, "report", | ||
| source = "reporting-service", | ||
| actor = changed_by, | ||
| report_id = report.id, | ||
| ) |
There was a problem hiding this comment.
The comment says “emit REPORTED event for each linked evidence object”, but the implementation emits a single record_report(...) event for the report itself. Please align the comment with the behavior (or implement the per-evidence emission if that was the intent). Also, the broad except Exception: pass silently drops lineage failures; consider at least logger.debug(...) so lineage wiring issues are diagnosable.
| class TestPluginsSubcommand: | ||
| # _cmd_plugins imports PluginRegistry lazily from its source module | ||
| _REG_PATCH = "gnat.plugins.registry.PluginRegistry" | ||
|
|
||
| def test_plugins_list_no_plugins_returns_0(self): | ||
| from gnat.cli.main import _cmd_plugins | ||
|
|
||
| args = MagicMock() | ||
| args.plg_command = "list" | ||
|
|
||
| with patch(self._REG_PATCH) as MockReg: | ||
| MockReg.return_value.list.return_value = [] | ||
| result = _cmd_plugins(args) | ||
|
|
There was a problem hiding this comment.
These tests patch/instantiate PluginRegistry directly, which will still pass even if the CLI uses PluginRegistry() instead of the intended singleton PluginRegistry.instance(). Consider updating the tests to assert the CLI calls PluginRegistry.instance() (and that loaded plugins are visible via the singleton) so regressions like the current gnat plugins list behavior are caught.
| try: | ||
| from gnat.migrations.cli import run_db_command | ||
| except ImportError: | ||
| print(_red('Alembic is required. Run: pip install "gnat[migrations]"'), | ||
| file=sys.stderr) | ||
| return 1 | ||
|
|
There was a problem hiding this comment.
run_db_command() returns an exit code, but _cmd_db ignores the return value and always returns 0 on the success path. This can mask non-zero exit codes (e.g., if Alembic uses SystemExit codes internally). Return the value from run_db_command(alembic_args) instead of unconditionally returning 0.
https://claude.ai/code/session_01BDoue9HxB83ijLzFARAugq