forked from farhoodlabs/paperclip
fix(server): respect externally set PAPERCLIP_API_URL env var (#3472)
## Thinking Path > - Paperclip server starts up and sets internal `PAPERCLIP_API_URL` for downstream services and adapters > - The server startup code was unconditionally overwriting `PAPERCLIP_API_URL` with `http://localhost:3100` (or equivalent based on `config.host`) > - In Kubernetes deployments, `PAPERCLIP_API_URL` is set via a ConfigMap to the externally accessible load balancer URL (e.g. `https://paperclip.example.com`) > - Because the env var was unconditionally set after loading the ConfigMap value, the ConfigMap-provided URL was ignored and replaced with the internal localhost address > - This caused downstream services (adapter env building) to use the wrong URL, breaking external access > - This pull request makes the assignment conditional — only set if not already provided by the environment > - External deployments can now supply `PAPERCLIP_API_URL` and it will be respected; local development continues to work without setting it ## What Changed - `server/src/index.ts`: Wrapped `PAPERCLIP_API_URL` assignment in `if (!process.env.PAPERCLIP_API_URL)` guard so externally provided values are preserved - `server/src/__tests__/server-startup-feedback-export.test.ts`: Added tests verifying external `PAPERCLIP_API_URL` is respected and fallback behavior is correct - `docs/deploy/environment-variables.md`: Updated `PAPERCLIP_API_URL` description to clarify it can be externally provided and the load balancer/reverse proxy use case ## Verification - Run the existing test suite: `pnpm test:run server/src/__tests__/server-startup-feedback-export.test.ts` — all 3 tests pass - Manual verification: Set `PAPERCLIP_API_URL` to a custom value before starting the server and confirm it is not overwritten ## Risks - Low risk — purely additive conditional check; existing behavior for unset env var is unchanged ## Model Used MiniMax M2.7 — reasoning-assisted for tracing the root cause through the startup chain (`buildPaperclipEnv` → `startServer` → `config.host` → `HOST` env var) ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Pawla Abdul (Bot) <pawla@groombook.dev> Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -180,3 +180,27 @@ describe("startServer feedback export wiring", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("startServer PAPERCLIP_API_URL handling", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
process.env.BETTER_AUTH_SECRET = "test-secret";
|
||||
delete process.env.PAPERCLIP_API_URL;
|
||||
});
|
||||
|
||||
it("uses the externally set PAPERCLIP_API_URL when provided", async () => {
|
||||
process.env.PAPERCLIP_API_URL = "http://custom-api:3100";
|
||||
|
||||
const started = await startServer();
|
||||
|
||||
expect(started.apiUrl).toBe("http://custom-api:3100");
|
||||
expect(process.env.PAPERCLIP_API_URL).toBe("http://custom-api:3100");
|
||||
});
|
||||
|
||||
it("falls back to host-based URL when PAPERCLIP_API_URL is not set", async () => {
|
||||
const started = await startServer();
|
||||
|
||||
expect(started.apiUrl).toBe("http://127.0.0.1:3210");
|
||||
expect(process.env.PAPERCLIP_API_URL).toBe("http://127.0.0.1:3210");
|
||||
});
|
||||
});
|
||||
|
||||
+4
-2
@@ -554,7 +554,9 @@ export async function startServer(): Promise<StartedServer> {
|
||||
: runtimeListenHost;
|
||||
process.env.PAPERCLIP_LISTEN_HOST = runtimeListenHost;
|
||||
process.env.PAPERCLIP_LISTEN_PORT = String(listenPort);
|
||||
process.env.PAPERCLIP_API_URL = `http://${runtimeApiHost}:${listenPort}`;
|
||||
if (!process.env.PAPERCLIP_API_URL) {
|
||||
process.env.PAPERCLIP_API_URL = `http://${runtimeApiHost}:${listenPort}`;
|
||||
}
|
||||
|
||||
setupLiveEventsWebSocketServer(server, db as any, {
|
||||
deploymentMode: config.deploymentMode,
|
||||
@@ -792,7 +794,7 @@ export async function startServer(): Promise<StartedServer> {
|
||||
server,
|
||||
host: config.host,
|
||||
listenPort,
|
||||
apiUrl: process.env.PAPERCLIP_API_URL ?? `http://${runtimeApiHost}:${listenPort}`,
|
||||
apiUrl: process.env.PAPERCLIP_API_URL!,
|
||||
databaseUrl: activeDatabaseConnectionString,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user