Feature/mcp science alias

[j-z10]

User description

• add mcp-science alias
• rename tinydb-server to tinydb

---

PR Type

Enhancement, Other

---

Description

• Add mcp-science launcher with click-based CLI
• Rename tinydb-server to mcp-tinydb for consistency
• Implement complete TinyDB MCP server functionality
• Add comprehensive test suite for database operations

---

Changes walkthrough 📝

	Relevant files
Enhancement	3 files __init__.py Add module exports for server main                                              [link]    main.py Implement complete TinyDB MCP server                                          [link]    __init__.py Create mcp-science launcher with CLI                                          +66/-0
Tests	1 files test_main.py Add comprehensive test suite for TinyDB                                    [link]
Miscellaneous	1 files __init__.py Remove old mcp-servers implementation                                        +0/-2
Configuration changes	3 files pyproject.toml Rename package and add click dependency                                    +4/-3      .python-version Set Python version to 3.12                                                              [link]    pyproject.toml Rename script from tinydb-server to mcp-tinydb                      +1/-1
Documentation	1 files README.md Add comprehensive TinyDB server documentation                        [link]

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 Security concerns Command injection: The mcp-science launcher in src/mcp_science/__init__.py passes additional command line arguments directly to subprocess without validation (line 50: uvx_cmd.extend(sys.argv[2:])). This could allow malicious arguments to be injected into the uvx command execution, potentially leading to arbitrary command execution.

⚡ Recommended focus areas for review Global State The use of global variables db and _db_file_path_for_get_db creates potential thread safety issues and makes the code harder to test and maintain. The complex logic in get_db() function with global state management could lead to unexpected behavior in concurrent scenarios. # Global db instance db = None # This will hold the db file path received from argparse or set by tests _db_file_path_for_get_db = None def get_db(db_file_path_from_arg=None): """ Gets the TinyDB instance. Initializes or re-initializes if the target db_file_path changes or if it's not initialized. The path for the DB is determined by: 1. db_file_path_from_arg (if provided, typically from CLI or direct test call) 2. _db_file_path_for_get_db (global, set by main() from CLI or by tests) """ global db global _db_file_path_for_get_db target_db_path = db_file_path_from_arg if db_file_path_from_arg else _db_file_path_for_get_db if not target_db_path: # This case should ideally not be hit if main() correctly sets _db_file_path_for_get_db logger.error("Database file path not set. Using default 'db.json'.") target_db_path = 'db.json' # Update the global path if a new one was explicitly passed via argument (e.g. from test setup) if db_file_path_from_arg: _db_file_path_for_get_db = db_file_path_from_arg current_storage_path = None if db and db._storage: current_storage_path = db._storage.path if db is None or current_storage_path != target_db_path: if db: db.close() logger.info(f"Initializing TinyDB with file: {target_db_path}") db = TinyDB(target_db_path) return db Command Injection The code constructs subprocess commands using user input without proper validation. The server_name parameter is validated against a whitelist, but additional arguments from sys.argv[2:] are passed directly to subprocess without sanitization, potentially allowing command injection. # Add any additional arguments if len(sys.argv) > 2: uvx_cmd.extend(sys.argv[2:]) Test Isolation Tests manipulate global state and rely on manual cleanup of the global db instance. The tearDown method directly accesses and modifies module-level variables, which could lead to test interdependencies and flaky tests if cleanup fails. """Tear down after test methods.""" # Access the global db instance from the imported module and manage it if tinydb_main_module.db: tinydb_main_module.db.close() tinydb_main_module.db = None # Ensure it's reset for the next test try: os.remove(self.test_db_file) except FileNotFoundError: pass # File might not have been created by all tests

[qodo-code-review]

CI Feedback 🧐

(Feedback updated until commit 11adae9)

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: codex

Failed stage: Run Codex [❌]

Failure summary: The action failed because the qodo-merge-pro[bot] user does not have sufficient permissions on the repository. The script checks if the bot has "admin" or "write" permissions, but it appears to have neither, causing the script to exit with code 1 at line 128.

Relevant error logs: 1: ##[group]Runner Image Provisioner 2: Hosted Compute Agent ... 119: ##[endgroup] 120: ##[group]Run set -euo pipefail 121: �[36;1mset -euo pipefail�[0m 122: �[36;1m�[0m 123: �[36;1mPERMISSION=$(gh api \�[0m 124: �[36;1m "/repos/${GITHUB_REPOSITORY}/collaborators/qodo-merge-pro[bot]/permission" \�[0m 125: �[36;1m | jq -r '.permission')�[0m 126: �[36;1m�[0m 127: �[36;1mif [[ "$PERMISSION" != "admin" && "$PERMISSION" != "write" ]]; then�[0m 128: �[36;1m exit 1�[0m 129: �[36;1mfi�[0m 130: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 131: env: 132: GH_TOKEN: *** 133: ##[endgroup] 134: ##[error]Process completed with exit code 1. 135: Post job cleanup.

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Handle private attribute access safely The function accesses the private _storage attribute of TinyDB which may not be stable across versions. Use the public API or handle potential AttributeError exceptions to prevent crashes when the internal structure changes. servers/tinydb/src/tinydb_server/main.py [22-53] def get_db(db_file_path_from_arg=None): """ Gets the TinyDB instance. Initializes or re-initializes if the target db_file_path changes or if it's not initialized. The path for the DB is determined by: 1. db_file_path_from_arg (if provided, typically from CLI or direct test call) 2. _db_file_path_for_get_db (global, set by main() from CLI or by tests) """ global db global _db_file_path_for_get_db target_db_path = db_file_path_from_arg if db_file_path_from_arg else _db_file_path_for_get_db if not target_db_path: # This case should ideally not be hit if main() correctly sets _db_file_path_for_get_db logger.error("Database file path not set. Using default 'db.json'.") target_db_path = 'db.json' # Update the global path if a new one was explicitly passed via argument (e.g. from test setup) if db_file_path_from_arg: _db_file_path_for_get_db = db_file_path_from_arg current_storage_path = None - if db and db._storage: - current_storage_path = db._storage.path + if db: + try: + current_storage_path = db._storage.path + except AttributeError: + # Handle case where _storage attribute doesn't exist or has different structure + current_storage_path = None if db is None or current_storage_path != target_db_path: if db: db.close() logger.info(f"Initializing TinyDB with file: {target_db_path}") db = TinyDB(target_db_path) return db Apply / Chat Suggestion importance[1-10]: 6 __ Why: The suggestion correctly points out the use of a private attribute _storage, which is a potential source of instability if the tinydb library changes its internal API. Wrapping the access in a try...except block is a good defensive programming practice that improves the code's robustness.	Low
	Remove redundant function argument The function calls get_db(args.db_file) after already setting the global _db_file_path_for_get_db. This is redundant since get_db() without arguments would use the global path that was just set. servers/tinydb/src/tinydb_server/main.py [200-217] def server_main(): parser = argparse.ArgumentParser(description="MCP TinyDB Server") parser.add_argument( "--db-file", type=str, default="db.json", help="Path to the TinyDB JSON file.", ) args = parser.parse_args(sys.argv[1:]) # Use only arguments after script name global _db_file_path_for_get_db _db_file_path_for_get_db = args.db_file logger.info(f"Starting tinydb-server. Database file: {args.db_file}") - get_db(args.db_file) + get_db() mcp.run('stdio') Apply / Chat Suggestion importance[1-10]: 3 __ Why: The suggestion is correct; the call to get_db(args.db_file) is redundant because the global variable _db_file_path_for_get_db is set on the preceding line. While this change improves code clarity by removing the unnecessary argument, it has no functional impact.	Low
Update

[github-actions]

Summary

Renames the root package/CLI from mcp-servers to mcp.science, introduces a new Click-based launcher that proxies to individual server sub-packages via uvx, and aligns the tinydb server folder/entrypoint with the new naming (servers/tinydb).

Notable changes

• pyproject.toml: package renamed, version bump to 0.1.3, adds click and updates console script to mcp-science.
• Adds src/mcp_science/__init__.py with the Click CLI wrapper.
• Removes placeholder src/mcp_servers.
• Renames servers/tinydb-server → servers/tinydb (files moved unchanged).
• Updates uv.lock accordingly.

Review

Nice cleanup—unifying the project name and adding a real launcher makes the repo easier to use! A couple of follow-ups would tighten it up:

• Consider leaving a thin compatibility stub (mcp_servers module & old CLI entry) to avoid breaking existing users.
• README / docs still reference the old mcp-servers name—worth updating.
• A quick smoke test in CI for the new mcp-science <server> command would catch regressions.

Great work overall 🚀

---

View workflow run

[niechen]

why do we want a mcp- prefix?

[j-z10]

This was the previous convention, and most of the existing servers in mcp.science already follow this rule.

[niechen]

is there a way to dynamically build this list by listing directories in /servers/?
so contributing new servers doesn't require manually updating this list.

[j-z10]

Then we need to package the entire /servers/ directory, so that when running uvx mcp-science, it can detect all available servers under that folder at runtime.