Feat/zero config#36
Open
vi70x3 wants to merge 5 commits into
Open
Conversation
…aling - setup.ts: non-interactive default mode with --interactive flag for old behavior - deploy-proxy.ts: deploy llm-proxy to workers.dev and capture assigned URL - reconcile-topology.ts: standalone drift detection and repair for proxy topology - capacityService.ts: shared getRequiredWorkerCount() from database - topologyReconciliation.ts: active self-healing at startup, blocks on failure - llm-proxy router: dynamic endpoint derivation via Web Crypto API SHA-256 - Eliminated PROXY_IP_COUNT and ROUTER_DOMAIN user-facing configuration - Updated install.sh, install.ps1, verify-deploy.ts, README.md, .env.example - Deleted llm-proxy/src/generated/topology.ts (replaced by dynamic endpoint)
…single deploy, handle spawn errors
… parser, sort TOML keys, type-safe sortModels
Reviewer's GuideImplements zero-configuration deployment and dynamic proxy topology: setup now non-interactive and auto-deploys llm-proxy via a new deploy-proxy script, topology and IP capacity rely solely on dynamic topology (deprecating PROXY_IP_COUNT and ROUTER_DOMAIN), startup includes automatic topology reconciliation based on API key counts, analytics typings and README docs are updated, and new design/spec documentation is added. Sequence diagram for startup topology reconciliation and self-healingsequenceDiagram
participant Main as server index.ts
participant Topology as proxyTopology
participant ReconSvc as topologyReconciliation
participant Capacity as capacityService
participant ReconScript as reconcile-topology.ts
participant Deploy as deploy-proxy.ts
participant CF as CloudflareWorkers
Main->>Topology: initialize()
Main->>ReconSvc: reconcileTopology()
ReconSvc->>Topology: getWorkerCount()
ReconSvc->>Topology: isDynamicTopologyAvailable()
ReconSvc->>Capacity: getRequiredWorkerCount()
alt drift detected
ReconSvc->>ReconScript: spawn("npx tsx reconcile-topology.ts")
ReconScript->>Capacity: getRequiredWorkerCount()
ReconScript->>Deploy: deployProxy(requiredCount)
Deploy->>CF: runWranglerDeploy(proxy workers)
Deploy->>CF: runWranglerDeploy(router)
Deploy->>Deploy: updateEnvKey(LLM_PROXY_URL)
else no drift
ReconSvc-->>Main: no-op
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a Zero-Configuration Deployment flow for freellmapi, automating secret generation, Cloudflare Workers deployment, and topology reconciliation on startup based on enabled database API keys. The review feedback is highly actionable and identifies critical issues that must be addressed: a potential process leak in the reconciliation script due to an unclosed SQLite connection, a cross-platform spawn failure on Windows, a specification violation where ROUTER_DOMAIN is not written to the proxy environment, and several unused imports that could break compilation under strict TypeScript configurations.
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.
Comment on lines
+176
to
+183
| async function main(): Promise<void> { | ||
| try { | ||
| await reconcile(); | ||
| } catch (err) { | ||
| console.error("[reconcile] Fatal error:", err); | ||
| process.exit(1); | ||
| } | ||
| } |
There was a problem hiding this comment.
The script initializes the database connection via initDb(), which keeps the SQLite connection open. Without an explicit process.exit(0) on success, the process will hang indefinitely in the background when spawned by the server, leading to process leaks. Add process.exit(0) after await reconcile() to ensure clean termination.
Suggested change
| async function main(): Promise<void> { | |
| try { | |
| await reconcile(); | |
| } catch (err) { | |
| console.error("[reconcile] Fatal error:", err); | |
| process.exit(1); | |
| } | |
| } | |
| async function main(): Promise<void> { | |
| try { | |
| await reconcile(); | |
| process.exit(0); | |
| } catch (err) { | |
| console.error("[reconcile] Fatal error:", err); | |
| process.exit(1); | |
| } | |
| } |
Comment on lines
+47
to
+50
| const child = spawn("npx", ["tsx", scriptPath], { | ||
| cwd: projectRoot, | ||
| stdio: "inherit", | ||
| }); |
There was a problem hiding this comment.
On Windows platforms, spawning npx directly without shell: true will fail with an ENOENT error because npx is a batch file (npx.cmd). Since Windows support is explicitly required (via install.ps1), add shell: true to make the spawn command cross-platform.
Suggested change
| const child = spawn("npx", ["tsx", scriptPath], { | |
| cwd: projectRoot, | |
| stdio: "inherit", | |
| }); | |
| const child = spawn("npx", ["tsx", scriptPath], { | |
| cwd: projectRoot, | |
| stdio: "inherit", | |
| shell: true, | |
| }); |
Comment on lines
+379
to
+382
| // Write LLM_PROXY_URL to .env | ||
| console.log("\n📝 Writing LLM_PROXY_URL to .env..."); | ||
| updateEnvKey(frellmapiEnvPath, "LLM_PROXY_URL", routerUrl, false); | ||
| console.log(` ✅ LLM_PROXY_URL written to .env`); |
There was a problem hiding this comment.
According to the requirements (REQ-Z3.3 and REQ-Z3.5), the captured router URL must be written to both freellmapi-alpha/.env as LLM_PROXY_URL and to llm-proxy/.env as ROUTER_DOMAIN (without the https:// prefix). Currently, ROUTER_DOMAIN is not being written, which violates the specification and could break manual deployments or backward compatibility.
Suggested change
| // Write LLM_PROXY_URL to .env | |
| console.log("\n📝 Writing LLM_PROXY_URL to .env..."); | |
| updateEnvKey(frellmapiEnvPath, "LLM_PROXY_URL", routerUrl, false); | |
| console.log(` ✅ LLM_PROXY_URL written to .env`); | |
| // Write LLM_PROXY_URL to .env and ROUTER_DOMAIN to llm-proxy/.env | |
| console.log("\n📝 Writing LLM_PROXY_URL and ROUTER_DOMAIN to .env files..."); | |
| updateEnvKey(frellmapiEnvPath, "LLM_PROXY_URL", routerUrl, false); | |
| const bareDomain = routerUrl.replace(/^https?:\/\//, ""); | |
| updateEnvKey(llmProxyEnvPath, "ROUTER_DOMAIN", bareDomain, false); | |
| console.log(" ✅ LLM_PROXY_URL and ROUTER_DOMAIN written to .env files"); |
| import { fileURLToPath } from 'node:url'; | ||
| import readline from 'node:readline'; | ||
| import { parseEnvFile, readEnvFileRaw, writeEnvFile } from './lib/env.js'; | ||
| import { parseEnvFile, readEnvFileRaw, writeEnvFile, updateEnvKey } from './lib/env.js'; |
There was a problem hiding this comment.
updateEnvKey is imported from ./lib/env.js but is never used in this file. If noUnusedLocals is enabled in tsconfig.json, this unused import will cause a compilation error and break the build.
Suggested change
| import { parseEnvFile, readEnvFileRaw, writeEnvFile, updateEnvKey } from './lib/env.js'; | |
| import { parseEnvFile, readEnvFileRaw, writeEnvFile } from './lib/env.js'; |
Comment on lines
+12
to
+15
| import fs from "node:fs"; | ||
| import path from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { parseEnvFile } from "./lib/env.js"; |
There was a problem hiding this comment.
fs is imported from node:fs but is never used in this file. If noUnusedLocals is enabled in tsconfig.json, this unused import will cause a compilation error and break the build.
Suggested change
| import fs from "node:fs"; | |
| import path from "node:path"; | |
| import { fileURLToPath } from "node:url"; | |
| import { parseEnvFile } from "./lib/env.js"; | |
| import path from "node:path"; | |
| import { fileURLToPath } from "node:url"; | |
| import { parseEnvFile } from "./lib/env.js"; |
| import fs from "node:fs"; | ||
| import path from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { parseEnvFile, readEnvFileRaw, updateEnvKey } from "./lib/env.js"; |
There was a problem hiding this comment.
readEnvFileRaw is imported from ./lib/env.js but is never used in this file. If noUnusedLocals is enabled in tsconfig.json, this unused import will cause a compilation error and break the build.
Suggested change
| import { parseEnvFile, readEnvFileRaw, updateEnvKey } from "./lib/env.js"; | |
| import { parseEnvFile, updateEnvKey } from "./lib/env.js"; |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The reconciliation logic is now split between
scripts/reconcile-topology.tsandserver/src/services/topologyReconciliation.ts; consider extracting the core drift-detection/redeploy functionality into a shared module so the CLI script and startup hook stay in sync and avoid duplicated behavior. scripts/reconcile-topology.tsdeclaresllmProxyEnvPathand importsfs/llmProxyRootwithout using them; cleaning up these unused variables/imports will make the script easier to maintain.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The reconciliation logic is now split between `scripts/reconcile-topology.ts` and `server/src/services/topologyReconciliation.ts`; consider extracting the core drift-detection/redeploy functionality into a shared module so the CLI script and startup hook stay in sync and avoid duplicated behavior.
- `scripts/reconcile-topology.ts` declares `llmProxyEnvPath` and imports `fs`/`llmProxyRoot` without using them; cleaning up these unused variables/imports will make the script easier to maintain.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce zero-configuration deployment for llm-proxy and dynamic topology-based worker capacity, removing reliance on PROXY_IP_COUNT and simplifying setup and deployment flows.
Enhancements:
Build:
Documentation:
Tests: