Skip to content

allow list files to exit gracefully when there are too many files and add a non recursive version to limit return files #19

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
m-y-mo merged 5 commits into from
Dec 12, 2025
Merged

allow list files to exit gracefully when there are too many files and add a non recursive version to limit return files #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
49305f0
allow list files to exit gracefully when there are too many files and…
m-y-mo Dec 12, 2025
910557c
fix directory case
m-y-mo Dec 12, 2025
5475193
Update src/seclab_taskflows/mcp_servers/local_file_viewer.py
m-y-mo Dec 12, 2025
17079ab
Update src/seclab_taskflows/mcp_servers/local_file_viewer.py
m-y-mo Dec 12, 2025
716b7e0
Add trailing dash when returning subdirectories
m-y-mo Dec 12, 2025
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
29 changes: 28 additions & 1 deletion src/seclab_taskflows/mcp_servers/local_file_viewer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@

LINE_LIMIT_FOR_FETCHING_FILE_CONTENT = int(os.getenv('LINE_LIMIT_FOR_FETCHING_FILE_CONTENT', default=1000))

FILE_LIMIT_FOR_LIST_FILES = int(os.getenv('FILE_LIMIT_FOR_LIST_FILES', default=100))

def is_subdirectory(directory, potential_subdirectory):
directory_path = Path(directory)
potential_subdirectory_path = Path(potential_subdirectory)
Expand Down Expand Up @@ -69,16 +71,22 @@ def search_zipfile(database_path, term, search_dir = None):
results[filename].append(i+1)
return results

def _list_files(database_path, root_dir = None):
def _list_files(database_path, root_dir = None, recursive=True):
results = []
root_dir = strip_leading_dash(root_dir)
with zipfile.ZipFile(database_path) as z:
for entry in z.infolist():
if entry.is_dir():
if not recursive:
dirname = remove_root_dir(entry.filename)
if Path(dirname).parent == Path(root_dir):
results.append(dirname + '/')
continue
filename = remove_root_dir(entry.filename)
if root_dir and not is_subdirectory(root_dir, filename):
continue
if not recursive and Path(filename).parent != Path(root_dir):
continue
results.append(filename)
Comment on lines 79 to 90
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

Logic error in the _list_files function: when an entry is a directory, the code continues to process it as a file on line 84. When entry.is_dir() is True, the code should skip to the next iteration after handling the directory case, but currently it falls through and processes the directory entry again as if it were a file. The logic should add a 'continue' statement after line 83 to prevent processing directories as files. Additionally, line 84 onwards will execute for both files and directories in the current implementation.

Copilot uses AI. Check for mistakes.
return results

Expand Down Expand Up @@ -152,8 +160,27 @@ async def list_files(
if not source_path or not source_path.exists():
return f"Invalid {owner} and {repo}. Check that the input is correct or try to fetch the repo from gh first."
content = _list_files(source_path, path)
if len(content) > FILE_LIMIT_FOR_LIST_FILES:
return f"Too many files to display in {owner}/{repo} at path {path} ({len(content)} files). Try using `list_files_non_recursive` instead."
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The error message suggests using list_files_non_recursive but wraps the function name in backticks, which may not render properly in all contexts where this error is displayed. Consider using either plain text or ensuring the output format properly handles markdown formatting.

Suggested change
return f"Too many files to display in {owner}/{repo} at path {path} ({len(content)} files). Try using `list_files_non_recursive` instead."
return f"Too many files to display in {owner}/{repo} at path {path} ({len(content)} files). Try using list_files_non_recursive instead."

Copilot uses AI. Check for mistakes.
return json.dumps(content, indent=2)

@mcp.tool()
async def list_files_non_recursive(
owner: str = Field(description="The owner of the repository"),
repo: str = Field(description="The name of the repository"),
path: str = Field(description="The path to the directory in the repository")) -> str:
"""
List the files of a directory from a local GitHub repository non-recursively.
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

There's a spelling error in the docstring. "non recursively" should be hyphenated as "non-recursively" for proper English grammar.

Copilot uses AI. Check for mistakes.
Subdirectories will be listed and indicated with a trailing slash.
"""
source_path = Path(f"{LOCAL_GH_DIR}/{owner}/{repo}.zip")
source_path = sanitize_file_path(source_path, [LOCAL_GH_DIR])
if not source_path or not source_path.exists():
return f"Invalid {owner} and {repo}. Check that the input is correct or try to fetch the repo from gh first."
content = _list_files(source_path, path, recursive=False)
Copy link

Copilot AI Dec 12, 2025

Choose a reason for hiding this comment

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

The error message suggests using 'list_files_non_recursive' but this won't solve the problem if the non-recursive listing itself has more than FILE_LIMIT_FOR_LIST_FILES files in a single directory. The non-recursive function should also check and enforce the file limit. Consider applying the same limit check in list_files_non_recursive to prevent it from returning an excessive number of results.

Suggested change
content = _list_files(source_path, path, recursive=False)
content = _list_files(source_path, path, recursive=False)
if len(content) > FILE_LIMIT_FOR_LIST_FILES:
return f"Too many files to display in {owner}/{repo} at path {path} ({len(content)} files)."

Copilot uses AI. Check for mistakes.
return json.dumps(content, indent=2)


@mcp.tool()
async def search_repo(
owner: str = Field(description="The owner of the repository"),
Expand Down
1 change: 1 addition & 0 deletions src/seclab_taskflows/toolboxes/local_file_viewer.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ server_params:
env:
LOCAL_GH_DIR: "{{ env DATA_DIR }}"
LINE_LIMIT_FOR_FETCHING_FILE_CONTENT: "{{ env LINE_LIMIT_FOR_FETCHING_FILE_CONTENT }}"
FILE_LIMIT_FOR_LIST_FILES: "{{ env FILE_LIMIT_FOR_LIST_FILES }}"