Fix mcp_data_dir instances#39
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to fix directory/path handling for MCP data so that alert results and local GitHub ZIPs are consistently stored and consumed from the same locations across servers.
Changes:
- Centralizes
ALERT_RESULTS_DIRinalert_results_modelsand updatesreport_alert_stateto use this shared directory instead of its ownMEMORYpath. - Makes
local_file_viewerreuseLOCAL_GH_DIRfromlocal_gh_resources, so it looks for repository ZIP files in the same directory as the fetcher. - Starts wiring
gh_code_scanningto the sharedALERT_RESULTS_DIRconstant, though a local definition still overrides it.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/seclab_taskflows/mcp_servers/alert_results_models.py |
Introduces a shared ALERT_RESULTS_DIR (via mcp_data_dir) for alert results storage, to be reused by multiple MCP servers. |
src/seclab_taskflows/mcp_servers/report_alert_state.py |
Switches the backend to use the shared ALERT_RESULTS_DIR from alert_results_models so alert state reads/writes from the common directory. |
src/seclab_taskflows/mcp_servers/local_file_viewer.py |
Removes the per-module LOCAL_GH_DIR definition and instead relies on LOCAL_GH_DIR from local_gh_resources, aligning the viewer’s ZIP location with the fetcher. |
src/seclab_taskflows/mcp_servers/gh_code_scanning.py |
Imports the shared ALERT_RESULTS_DIR constant for alert result persistence, though the existing local constant currently still overrides it. |
Comments suppressed due to low confidence (1)
src/seclab_taskflows/mcp_servers/alert_results_models.py:7
log_file_nameis imported but never used in this file; it should be removed to keep dependencies minimal and avoid confusion about logging behavior here.
from seclab_taskflow_agent.path_utils import mcp_data_dir, log_file_name
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/seclab_taskflows/mcp_servers/alert_results_models.py:7
log_file_nameis imported here but never used in this module; consider removing it from the import to avoid an unused import and keep dependencies minimal.
from seclab_taskflow_agent.path_utils import mcp_data_dir, log_file_name
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
20ff191 to
e7e3013
Compare
e7e3013 to
04eaef4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/seclab_taskflows/mcp_servers/alert_results_models.py:7
log_file_nameis imported but never used in this module; consider removing the unused import to keep dependencies minimal and avoid confusion about its purpose here.
from seclab_taskflow_agent.path_utils import mcp_data_dir, log_file_name
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
src/seclab_taskflows/mcp_servers/alert_results_models.py:7
log_file_nameis imported but never used in this module; consider removing it from the import to avoid an unused-import warning and keep dependencies minimal.
from seclab_taskflow_agent.path_utils import mcp_data_dir, log_file_name
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9ef7c39 to
c6eeee6
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
I noticed that
local_file_viewer.pywasn't working because it was looking for the zip file in the wrong directory. It's supposed to use the same directory aslocal_gh_resources.py. This bug doesn't happen if you explicitly set theLOCAL_GH_DIRenvironment variable, but it's a problem if you letmcp_data_diruse the default location.ALERT_RESULTS_DIRhas the same bug.I accidentally introduced this bug in #11.