Code review fixes: wardrobe migration, response validation, path traversal guard, deduplication
- Migrate 11 character JSONs from old wardrobe keys to _BODY_GROUP_KEYS format - Add is_favourite/is_nsfw columns to Preset model - Add HTTP response validation and timeouts to ComfyUI client - Add path traversal protection on replace cover route - Deduplicate services/mcp.py (4 functions → 2 generic + 2 wrappers) - Extract apply_library_filters() and clean_html_text() shared helpers - Add named constants for 17 ComfyUI workflow node IDs - Fix bare except clauses in services/llm.py - Fix tags schema in ensure_default_outfit() (list → dict) - Convert f-string logging to lazy % formatting - Add 5-minute polling timeout to frontend waitForJob() - Improve migration error handling (non-duplicate errors log at WARNING) - Update CLAUDE.md to reflect all changes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Aodhan Collins
176
services/mcp.py
176
services/mcp.py
@@ -12,147 +12,83 @@ CHAR_MCP_COMPOSE_DIR = os.path.join(MCP_TOOLS_DIR, 'character-mcp')
|
||||
CHAR_MCP_REPO_URL = 'https://git.liveaodh.com/aodhan/character-mcp.git'
|
||||
|
||||
|
||||
def _ensure_mcp_repo():
|
||||
"""Clone or update the danbooru-mcp source repository inside tools/.
|
||||
def _ensure_repo(compose_dir, repo_url, name):
|
||||
"""Clone or update an MCP source repository inside tools/.
|
||||
|
||||
- If ``tools/danbooru-mcp/`` does not exist, clone from MCP_REPO_URL.
|
||||
- If the directory does not exist, clone from repo_url.
|
||||
- If it already exists, run ``git pull`` to fetch the latest changes.
|
||||
Errors are non-fatal.
|
||||
"""
|
||||
os.makedirs(MCP_TOOLS_DIR, exist_ok=True)
|
||||
try:
|
||||
if not os.path.isdir(MCP_COMPOSE_DIR):
|
||||
logger.info('Cloning danbooru-mcp from %s …', MCP_REPO_URL)
|
||||
if not os.path.isdir(compose_dir):
|
||||
logger.info('Cloning %s from %s …', name, repo_url)
|
||||
subprocess.run(
|
||||
['git', 'clone', MCP_REPO_URL, MCP_COMPOSE_DIR],
|
||||
['git', 'clone', repo_url, compose_dir],
|
||||
timeout=120, check=True,
|
||||
)
|
||||
logger.info('danbooru-mcp cloned successfully.')
|
||||
logger.info('%s cloned successfully.', name)
|
||||
else:
|
||||
logger.info('Updating danbooru-mcp via git pull …')
|
||||
logger.info('Updating %s via git pull …', name)
|
||||
subprocess.run(
|
||||
['git', 'pull'],
|
||||
cwd=MCP_COMPOSE_DIR,
|
||||
cwd=compose_dir,
|
||||
timeout=60, check=True,
|
||||
)
|
||||
logger.info('danbooru-mcp updated.')
|
||||
logger.info('%s updated.', name)
|
||||
except FileNotFoundError:
|
||||
logger.warning('git not found on PATH — danbooru-mcp repo will not be cloned/updated.')
|
||||
logger.warning('git not found on PATH — %s repo will not be cloned/updated.', name)
|
||||
except subprocess.CalledProcessError as e:
|
||||
logger.warning('git operation failed for danbooru-mcp: %s', e)
|
||||
logger.warning('git operation failed for %s: %s', name, e)
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.warning('git timed out while cloning/updating danbooru-mcp.')
|
||||
logger.warning('git timed out while cloning/updating %s.', name)
|
||||
except Exception as e:
|
||||
logger.warning('Could not clone/update danbooru-mcp repo: %s', e)
|
||||
logger.warning('Could not clone/update %s repo: %s', name, e)
|
||||
|
||||
|
||||
def _ensure_server_running(compose_dir, repo_url, container_name, name):
|
||||
"""Ensure an MCP repo is present/up-to-date, then start the Docker
|
||||
container if it is not already running.
|
||||
|
||||
Uses ``docker compose up -d`` so the image is built automatically on first
|
||||
run. Errors are non-fatal — the app will still start even if Docker is
|
||||
unavailable.
|
||||
|
||||
Skipped when ``SKIP_MCP_AUTOSTART=true`` (set by docker-compose, where the
|
||||
MCP service is managed by compose instead).
|
||||
"""
|
||||
if os.environ.get('SKIP_MCP_AUTOSTART', '').lower() == 'true':
|
||||
logger.info('SKIP_MCP_AUTOSTART set — skipping %s auto-start.', name)
|
||||
return
|
||||
_ensure_repo(compose_dir, repo_url, name)
|
||||
try:
|
||||
result = subprocess.run(
|
||||
['docker', 'ps', '--filter', f'name={container_name}', '--format', '{{.Names}}'],
|
||||
capture_output=True, text=True, timeout=10,
|
||||
)
|
||||
if container_name in result.stdout:
|
||||
logger.info('%s container already running.', name)
|
||||
return
|
||||
logger.info('Starting %s container via docker compose …', name)
|
||||
subprocess.run(
|
||||
['docker', 'compose', 'up', '-d'],
|
||||
cwd=compose_dir,
|
||||
timeout=120,
|
||||
)
|
||||
logger.info('%s container started.', name)
|
||||
except FileNotFoundError:
|
||||
logger.warning('docker not found on PATH — %s will not be started automatically.', name)
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.warning('docker timed out while starting %s.', name)
|
||||
except Exception as e:
|
||||
logger.warning('Could not ensure %s is running: %s', name, e)
|
||||
|
||||
|
||||
def ensure_mcp_server_running():
|
||||
"""Ensure the danbooru-mcp repo is present/up-to-date, then start the
|
||||
Docker container if it is not already running.
|
||||
|
||||
Uses ``docker compose up -d`` so the image is built automatically on first
|
||||
run. Errors are non-fatal — the app will still start even if Docker is
|
||||
unavailable.
|
||||
|
||||
Skipped when ``SKIP_MCP_AUTOSTART=true`` (set by docker-compose, where the
|
||||
danbooru-mcp service is managed by compose instead).
|
||||
"""
|
||||
if os.environ.get('SKIP_MCP_AUTOSTART', '').lower() == 'true':
|
||||
logger.info('SKIP_MCP_AUTOSTART set — skipping danbooru-mcp auto-start.')
|
||||
return
|
||||
_ensure_mcp_repo()
|
||||
try:
|
||||
result = subprocess.run(
|
||||
['docker', 'ps', '--filter', 'name=danbooru-mcp', '--format', '{{.Names}}'],
|
||||
capture_output=True, text=True, timeout=10,
|
||||
)
|
||||
if 'danbooru-mcp' in result.stdout:
|
||||
logger.info('danbooru-mcp container already running.')
|
||||
return
|
||||
# Container not running — start it via docker compose
|
||||
logger.info('Starting danbooru-mcp container via docker compose …')
|
||||
subprocess.run(
|
||||
['docker', 'compose', 'up', '-d'],
|
||||
cwd=MCP_COMPOSE_DIR,
|
||||
timeout=120,
|
||||
)
|
||||
logger.info('danbooru-mcp container started.')
|
||||
except FileNotFoundError:
|
||||
logger.warning('docker not found on PATH — danbooru-mcp will not be started automatically.')
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.warning('docker timed out while starting danbooru-mcp.')
|
||||
except Exception as e:
|
||||
logger.warning('Could not ensure danbooru-mcp is running: %s', e)
|
||||
|
||||
|
||||
def _ensure_character_mcp_repo():
|
||||
"""Clone or update the character-mcp source repository inside tools/.
|
||||
|
||||
- If ``tools/character-mcp/`` does not exist, clone from CHAR_MCP_REPO_URL.
|
||||
- If it already exists, run ``git pull`` to fetch the latest changes.
|
||||
Errors are non-fatal.
|
||||
"""
|
||||
os.makedirs(MCP_TOOLS_DIR, exist_ok=True)
|
||||
try:
|
||||
if not os.path.isdir(CHAR_MCP_COMPOSE_DIR):
|
||||
logger.info('Cloning character-mcp from %s …', CHAR_MCP_REPO_URL)
|
||||
subprocess.run(
|
||||
['git', 'clone', CHAR_MCP_REPO_URL, CHAR_MCP_COMPOSE_DIR],
|
||||
timeout=120, check=True,
|
||||
)
|
||||
logger.info('character-mcp cloned successfully.')
|
||||
else:
|
||||
logger.info('Updating character-mcp via git pull …')
|
||||
subprocess.run(
|
||||
['git', 'pull'],
|
||||
cwd=CHAR_MCP_COMPOSE_DIR,
|
||||
timeout=60, check=True,
|
||||
)
|
||||
logger.info('character-mcp updated.')
|
||||
except FileNotFoundError:
|
||||
logger.warning('git not found on PATH — character-mcp repo will not be cloned/updated.')
|
||||
except subprocess.CalledProcessError as e:
|
||||
logger.warning('git operation failed for character-mcp: %s', e)
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.warning('git timed out while cloning/updating character-mcp.')
|
||||
except Exception as e:
|
||||
logger.warning('Could not clone/update character-mcp repo: %s', e)
|
||||
"""Ensure the danbooru-mcp Docker container is running."""
|
||||
_ensure_server_running(MCP_COMPOSE_DIR, MCP_REPO_URL, 'danbooru-mcp', 'danbooru-mcp')
|
||||
|
||||
|
||||
def ensure_character_mcp_server_running():
|
||||
"""Ensure the character-mcp repo is present/up-to-date, then start the
|
||||
Docker container if it is not already running.
|
||||
|
||||
Uses ``docker compose up -d`` so the image is built automatically on first
|
||||
run. Errors are non-fatal — the app will still start even if Docker is
|
||||
unavailable.
|
||||
|
||||
Skipped when ``SKIP_MCP_AUTOSTART=true`` (set by docker-compose, where the
|
||||
character-mcp service is managed by compose instead).
|
||||
"""
|
||||
if os.environ.get('SKIP_MCP_AUTOSTART', '').lower() == 'true':
|
||||
logger.info('SKIP_MCP_AUTOSTART set — skipping character-mcp auto-start.')
|
||||
return
|
||||
_ensure_character_mcp_repo()
|
||||
try:
|
||||
result = subprocess.run(
|
||||
['docker', 'ps', '--filter', 'name=character-mcp', '--format', '{{.Names}}'],
|
||||
capture_output=True, text=True, timeout=10,
|
||||
)
|
||||
if 'character-mcp' in result.stdout:
|
||||
logger.info('character-mcp container already running.')
|
||||
return
|
||||
# Container not running — start it via docker compose
|
||||
logger.info('Starting character-mcp container via docker compose …')
|
||||
subprocess.run(
|
||||
['docker', 'compose', 'up', '-d'],
|
||||
cwd=CHAR_MCP_COMPOSE_DIR,
|
||||
timeout=120,
|
||||
)
|
||||
logger.info('character-mcp container started.')
|
||||
except FileNotFoundError:
|
||||
logger.warning('docker not found on PATH — character-mcp will not be started automatically.')
|
||||
except subprocess.TimeoutExpired:
|
||||
logger.warning('docker timed out while starting character-mcp.')
|
||||
except Exception as e:
|
||||
logger.warning('Could not ensure character-mcp is running: %s', e)
|
||||
"""Ensure the character-mcp Docker container is running."""
|
||||
_ensure_server_running(CHAR_MCP_COMPOSE_DIR, CHAR_MCP_REPO_URL, 'character-mcp', 'character-mcp')
|
||||
|
||||
Reference in New Issue
Block a user