Skip to content

P fix list_directory_from_gh in case no directory is returned from API #76

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.

Sign up for GitHub

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

Merged
p- merged 11 commits into from
Apr 14, 2026
+407 −3
Merged

P fix list_directory_from_gh in case no directory is returned from API #76

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b7ca36c
add tests for gh_file_viewer
p- Apr 10, 2026
6e34530
return not a directory in case of a file
p- Apr 10, 2026
c90b70e
fix list_directory_from_gh in case no directory is returned from API
p- Apr 10, 2026
53ba24d
resolve linting issues
p- Apr 10, 2026
be371c0
only create new tables if they don't exist
p- Apr 10, 2026
1b14374
don't execute test_gh_file_viewer in parallel
p- Apr 10, 2026
3ca7990
log error database already exists
p- Apr 10, 2026
f4d8c4f
fix linting error
p- Apr 10, 2026
5408be3
fix another linter error
p- Apr 10, 2026
8eead38
pytest-asyncio as dep
p- Apr 10, 2026
6a9f6df
only catch OperationalError
p- Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ Issues = "https://github.com/GitHubSecurityLab/seclab-taskflows/issues"
[tool.hatch.version]
path = "src/seclab_taskflows/__about__.py"

[tool.hatch.envs.hatch-test]
extra-dependencies = [
"pytest-asyncio==1.3.0",
]

[tool.hatch.envs.types]
extra-dependencies = [
"mypy>=1.0.0",
Expand Down Expand Up @@ -120,3 +125,8 @@ ignore = [
"S101", # Use of assert (standard in pytest)
"SLF001", # Private member accessed (tests legitimately access module internals)
]

[tool.pytest.ini_options]
markers = [
"xdist_group: Group tests to run on the same xdist worker",
]
14 changes: 11 additions & 3 deletions src/seclab_taskflows/mcp_servers/gh_file_viewer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import os
from sqlalchemy.orm import DeclarativeBase, mapped_column, Mapped
from sqlalchemy import create_engine
import sqlalchemy.exc
from sqlalchemy.orm import Session
from typing import Optional
from pathlib import Path
Expand Down Expand Up @@ -53,7 +54,11 @@ def __repr__(self):
SEARCH_RESULT_DIR = mcp_data_dir("seclab-taskflows", "gh_file_viewer", "SEARCH_RESULTS_DIR")

engine = create_engine(f"sqlite:///{os.path.abspath(SEARCH_RESULT_DIR)}/search_result.db", echo=False)
Base.metadata.create_all(engine, tables=[SearchResults.__table__])

try:
Base.metadata.create_all(engine, tables=[SearchResults.__table__])
except sqlalchemy.exc.OperationalError as e:
logging.exception(f"Database/Tables already exist(s)") # only log here, as this error likely only happens in test
Comment on lines +58 to +61
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Catching all sqlalchemy.exc.OperationalError from Base.metadata.create_all(...) and then continuing can mask real DB init failures (e.g., permission errors, missing/readonly directory, DB locked/corrupt). Consider only suppressing the specific “table already exists” race case (inspect the exception message/orig), and re-raise otherwise (and/or ensure SEARCH_RESULT_DIR exists before creating the engine). Also logging.exception here is very noisy for an expected condition; prefer a warning/info log that includes the actual exception details.

See below for a potential fix:

os.makedirs(SEARCH_RESULT_DIR, exist_ok=True)


def _is_table_already_exists_error(error: sqlalchemy.exc.OperationalError) -> bool:
    error_messages = [str(error).lower()]
    if getattr(error, "orig", None) is not None:
        error_messages.append(str(error.orig).lower())

    return any(
        "already exists" in message and "table" in message
        for message in error_messages
    )


engine = create_engine(f"sqlite:///{os.path.abspath(SEARCH_RESULT_DIR)}/search_result.db", echo=False)

try:
    Base.metadata.create_all(engine, tables=[SearchResults.__table__])
except sqlalchemy.exc.OperationalError as e:
    if _is_table_already_exists_error(e):
        logging.warning("Search results table already exists during initialization: %s", e)
    else:
        raise

Copilot uses AI. Check for mistakes.


async def call_api(url: str, params: dict) -> str:
Expand Down Expand Up @@ -283,10 +288,13 @@ async def list_directory_from_gh(
r = await call_api(url=f"https://api.github.com/repos/{owner}/{repo}/contents/{path}", params={})
if isinstance(r, str):
return r
if not r.json():
data = r.json()
if not data:
return json.dumps([], indent=2)
if not isinstance(data, list):
return f"Path '{path}' is not a directory."

content = [item["path"] for item in r.json()]
content = [item["path"] for item in data]
return json.dumps(content, indent=2)


Expand Down
Loading
Loading