fix(CODEWIKI-005): CU-86akhf8u6 8 review findings across 4 files

[flamingo]

Closes 8 review findings across 4 files.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

#	Fix confidence	Finding	Location
1	🟢 95 high	codewiki/src/fe/routes.py never defines a module-level logger despite catching/discarding exceptions	codewiki/src/fe/routes.py:1
2	🟡 85 medium	routes.py uses bare except-pass blocks that silently discard failures instead of logging via module logger	codewiki/src/fe/routes.py:226
3	🟡 60 medium	serve_generated_docs constructs docs_path before validating job_id format, unlike serve_doc's stricter directory-traversal guard	codewiki/src/fe/routes.py:244
4	🟢 90 high	Bare except clauses in cloning.py swallow errors without any logging	codewiki/src/be/dependency_analyzer/analysis/cloning.py:109
5	🟢 90 high	Second bare except swallowing sparse-checkout setup errors without logging	codewiki/src/be/dependency_analyzer/analysis/cloning.py:161
6	🔴 55 low — review closely	clone_repository leaves temp_dir undeleted if the git executable check runs after mkdtemp but the initial 'longpaths' config subprocess.run raises an untyped exception	codewiki/src/be/dependency_analyzer/analysis/cloning.py:93
7	🔴 55 low — review closely	logging.basicConfig() called directly in a standalone script instead of using centralized setup_logging	test_clustering_forced.py:9
8	🟢 90 high	web_app.py main() uses print() instead of module logger for server startup diagnostics	codewiki/src/fe/web_app.py:113

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.

---

Run: https://product-hub.flamingo.so/admin/code-review
Run id: 4b0306d9-ca7f-413c-857e-fc323d1e9f21

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

ClickUp task: CU-86akhf8u6 CodeWiki review findings sweep (9 PRs)

[flamingo]

🦩 What this fix changed, finding by finding

8 finding(s) fixed in this draft — 8 explained inline on the diff; 2 low-confidence hunk(s) need close review before merging.

[flamingo]

🦩 🟠 codewiki/src/fe/routes.py never defines a module-level logger despite catching/discarding exceptions

Added import logging and logger = logging.getLogger(__name__) at module level in codewiki/src/fe/routes.py, immediately after the imports block, following the CODEWIKI-005/007-2 convention.

🤖 Prompt for AI agents

In codewiki/src/fe/routes.py around line 1, review and complete this code-review fix: codewiki/src/fe/routes.py never defines a module-level logger despite catching/discarding exceptions.
What the draft fix changed: Added `import logging` and `logger = logging.getLogger(__name__)` at module level in codewiki/src/fe/routes.py, immediately after the imports block, following the CODEWIKI-005/007-2 convention.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 95 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 routes.py uses bare except-pass blocks that silently discard failures instead of logging via module logger

Replaced the silent except Exception: pass blocks in serve_generated_docs (module_tree.json and metadata.json loading) with logger.exception(...) calls that record the failure and file path before continuing with module_tree/metadata left as None. Also added logger.exception(...) to the previously silent _normalize_github_url fallback except-block and to the index_post queueing exception and the serve_generated_docs file-read exception handler, giving every caught exception in the file a log trail while preserving existing control flow (HTTPException raising and pass-through fallback behavior unchanged).

🤖 Prompt for AI agents

In codewiki/src/fe/routes.py around line 226, review and complete this code-review fix: routes.py uses bare except-pass blocks that silently discard failures instead of logging via module logger.
What the draft fix changed: Replaced the silent `except Exception: pass` blocks in `serve_generated_docs` (module_tree.json and metadata.json loading) with `logger.exception(...)` calls that record the failure and file path before continuing with `module_tree`/`metadata` left as `None`. Also added `logger.exception(...)` to the previously silent `_normalize_github_url` fallback except-block and to the `index_post` queueing exception and the `serve_generated_docs` file-read exception handler, giving every caught exception in the file a log trail while preserving existing control flow (HTTPException raising and pass-through fallback behavior unchanged).
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 85 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 serve_generated_docs constructs docs_path before validating job_id format, unlike serve_doc's stricter directory-traversal guard

In serve_generated_docs, added an explicit '..' in filename rejection check right after job_id validation, and replaced the manual docs_path_resolved != file_path and docs_path_resolved not in file_path.parents check with file_path.is_relative_to(docs_path_resolved), matching the stricter guard style used in visualise_docs.py. This closes the gap where filename was unchecked before path construction and tightens the containment check. Residual risk: is_relative_to requires Python 3.9+ (assumed available given repo's existing usage patterns elsewhere), and this does not address potential symlink escapes inside docs_path itself (a symlinked file within the docs directory could still resolve outside it) — a complete fix would need docs_path.resolve(strict=True) validation plus symlink auditing, which is out of scope for a minimal fix.

🤖 Prompt for AI agents

In codewiki/src/fe/routes.py around line 244, review and complete this code-review fix: serve_generated_docs constructs docs_path before validating job_id format, unlike serve_doc's stricter directory-traversal guard.
What the draft fix changed: In `serve_generated_docs`, added an explicit `'..' in filename` rejection check right after job_id validation, and replaced the manual `docs_path_resolved != file_path and docs_path_resolved not in file_path.parents` check with `file_path.is_relative_to(docs_path_resolved)`, matching the stricter guard style used in visualise_docs.py. This closes the gap where `filename` was unchecked before path construction and tightens the containment check. Residual risk: `is_relative_to` requires Python 3.9+ (assumed available given repo's existing usage patterns elsewhere), and this does not address potential symlink escapes inside `docs_path` itself (a symlinked file within the docs directory could still resolve outside it) — a complete fix would need `docs_path.resolve(strict=True)` validation plus symlink auditing, which is out of scope for a minimal fix.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟡 60 medium — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 Bare except clauses in cloning.py swallow errors without any logging

In clone_repository, the bare except: pass around the core.longpaths git config subprocess call was changed to except Exception as e: logger.debug(f"Non-fatal git config step failed: {e}"), giving debug-level trace of any failure per CODEWIKI-005.

🤖 Prompt for AI agents

In codewiki/src/be/dependency_analyzer/analysis/cloning.py around line 109, review and complete this code-review fix: Bare except clauses in cloning.py swallow errors without any logging.
What the draft fix changed: In `clone_repository`, the bare `except: pass` around the `core.longpaths` git config subprocess call was changed to `except Exception as e: logger.debug(f"Non-fatal git config step failed: {e}")`, giving debug-level trace of any failure per CODEWIKI-005.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 Second bare except swallowing sparse-checkout setup errors without logging

In clone_repository, the bare except: pass around the sparse-checkout setup block (config, file write, read-tree) was changed to except Exception as e: logger.debug(f"Non-fatal sparse-checkout setup failed: {e}"), providing a debug log trace instead of silent swallowing.

🤖 Prompt for AI agents

In codewiki/src/be/dependency_analyzer/analysis/cloning.py around line 161, review and complete this code-review fix: Second bare except swallowing sparse-checkout setup errors without logging.
What the draft fix changed: In `clone_repository`, the bare `except: pass` around the sparse-checkout setup block (config, file write, read-tree) was changed to `except Exception as e: logger.debug(f"Non-fatal sparse-checkout setup failed: {e}")`, providing a debug log trace instead of silent swallowing.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 clone_repository leaves temp_dir undeleted if the git executable check runs after mkdtemp but the initial 'longpaths' config subprocess.run raises an untyped exception

Added a final except Exception: clause in clone_repository's outer try/except (after the existing TimeoutExpired, CalledProcessError, FileNotFoundError handlers) that cleans up temp_dir via cleanup_repository_safe if it exists and then re-raises the original exception, closing the resource leak for untyped exceptions (e.g. OSError, KeyboardInterrupt) raised during the clone attempt. Note: KeyboardInterrupt is a BaseException, not Exception, so it is still not caught by this handler and temp_dir would still leak in that specific case; a fully complete fix would need a finally-based cleanup keyed on success/failure state or a broader except BaseException, which was avoided here to keep the change minimal and not swallow interrupts silently.

🤖 Prompt for AI agents

In codewiki/src/be/dependency_analyzer/analysis/cloning.py around line 93, review and complete this code-review fix: clone_repository leaves temp_dir undeleted if the git executable check runs after mkdtemp but the initial 'longpaths' config subprocess.run raises an untyped exception.
What the draft fix changed: Added a final `except Exception:` clause in `clone_repository`'s outer try/except (after the existing `TimeoutExpired`, `CalledProcessError`, `FileNotFoundError` handlers) that cleans up `temp_dir` via `cleanup_repository_safe` if it exists and then re-raises the original exception, closing the resource leak for untyped exceptions (e.g. OSError, KeyboardInterrupt) raised during the clone attempt. Note: KeyboardInterrupt is a BaseException, not Exception, so it is still not caught by this handler and temp_dir would still leak in that specific case; a fully complete fix would need a `finally`-based cleanup keyed on success/failure state or a broader `except BaseException`, which was avoided here to keep the change minimal and not swallow interrupts silently.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 logging.basicConfig() called directly in a standalone script instead of using centralized setup_logging

Replaced the direct logging.basicConfig(level=logging.INFO, format='[%(levelname)s] %(message)s', force=True) call at module import time with a call to the centralized setup_logging from codewiki.src.be.dependency_analyzer.utils.logging_config, passing the same level, format, and force arguments. This assumes setup_logging accepts these keyword arguments (as implied by CODEWIKI-005/008-2 describing it as the centralized replacement for basicConfig); if its actual signature differs, the call site would need adjusting — this is unverified since the module's source wasn't provided.

🤖 Prompt for AI agents

In test_clustering_forced.py around line 9, review and complete this code-review fix: logging.basicConfig() called directly in a standalone script instead of using centralized setup_logging.
What the draft fix changed: Replaced the direct `logging.basicConfig(level=logging.INFO, format='[%(levelname)s] %(message)s', force=True)` call at module import time with a call to the centralized `setup_logging` from `codewiki.src.be.dependency_analyzer.utils.logging_config`, passing the same `level`, `format`, and `force` arguments. This assumes `setup_logging` accepts these keyword arguments (as implied by CODEWIKI-005/008-2 describing it as the centralized replacement for `basicConfig`); if its actual signature differs, the call site would need adjusting — this is unverified since the module's source wasn't provided.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 55 low — review closely — react 👍/👎 to teach the reviewer

[flamingo]

🦩 🟠 web_app.py main() uses print() instead of module logger for server startup diagnostics

Replaced all five print() calls in main() (startup banner, server URL, cache dir, temp dir, and the "Press Ctrl+C" hint) with logger.info() calls on a module-level logger created via logging.getLogger(__name__); also replaced the KeyboardInterrupt handler's print("\n👋 Server stopped") with logger.info(...). Added import logging and the logger = logging.getLogger(__name__) module-level declaration, consistent with the established convention referenced in the finding.

🤖 Prompt for AI agents

In codewiki/src/fe/web_app.py around line 113, review and complete this code-review fix: web_app.py main() uses print() instead of module logger for server startup diagnostics.
What the draft fix changed: Replaced all five print() calls in main() (startup banner, server URL, cache dir, temp dir, and the "Press Ctrl+C" hint) with logger.info() calls on a module-level logger created via `logging.getLogger(__name__)`; also replaced the KeyboardInterrupt handler's print("\n👋 Server stopped") with logger.info(...). Added `import logging` and the `logger = logging.getLogger(__name__)` module-level declaration, consistent with the established convention referenced in the finding.
Verify the change is correct and complete; do not refactor unrelated code.

fix confidence: 🟢 90 high — react 👍/👎 to teach the reviewer