From 466d56280a37cb3dae9cdce8788937964824a3d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Fri, 23 Oct 2026 22:25:47 +0300 Subject: [PATCH 1/4] fix(eval): retain pending completion notifications (#3700) Claude Code can announce a background completion before the foreground result. Carry the notification across that boundary and consume it once when validating the continuation, preserving complete session accounting. Cover both event orderings with the pinned CLI and reject notification reuse. Reproduces the failure from Actions run 39948470804. Co-authored-by: Gergo Magyar --- eval/tests/test_workflow_bench_sessions.py | 120 +++++++++++++++++++++ eval/workflow_bench/runner_sessions.py | 23 ++-- 2 files changed, 131 insertions(+), 12 deletions(-) diff --git a/eval/tests/test_workflow_bench_sessions.py b/eval/tests/test_workflow_bench_sessions.py index f5a82beda..f203f0d43 100644 --- a/eval/tests/test_workflow_bench_sessions.py +++ b/eval/tests/test_workflow_bench_sessions.py @@ -7,6 +7,7 @@ import os import shutil import subprocess import sys +import threading from contextlib import nullcontext from dataclasses import replace from pathlib import Path @@ -1352,6 +1353,125 @@ def test_unverifiable_background_continuation_stays_excluded(monkeypatch, tmp_pa assert rec["transcript_missing"] is True +def test_background_notification_before_first_result_remains_pending(monkeypatch, tmp_path): + events = background_completion_events() + events.insert(0, events.pop(1)) + proc = replace(fake_cli_result("\n".join(map(json.dumps, events))), duration_s=12.5) + monkeypatch.setattr(runner_sessions, "run_managed", lambda *a, **k: proc) + + rec = runner.run_claude("task", tmp_path, claude_bin="claude", timeout=30, transcript_output_dir=tmp_path) + + assert rec["ok"] is True, rec["error_detail"] + assert rec["transcript_missing"] is False + assert rec["cost_usd"] == 0.15 + assert rec["num_turns"] == 6 + assert [rec[field] for field in runner_sessions.USAGE_FIELDS] == [2, 4, 6, 8] + assert rec["duration_s"] == 12.5 + + +def test_background_notification_cannot_authorize_two_continuations(monkeypatch, tmp_path): + events = background_completion_events() + events.insert(0, events.pop(1)) + events += [ + {"type": "system", "subtype": "init", "session_id": "s"}, + {"type": "assistant", "message": {"content": [{"type": "text", "text": "Another reply"}]}}, + json.loads(report_variant(uuid="extra", subtype="success", is_error=False, total_cost_usd=0.2)), + ] + monkeypatch.setattr( + runner_sessions, "run_managed", lambda *a, **k: fake_cli_result("\n".join(map(json.dumps, events))) + ) + + rec = runner.run_claude("task", tmp_path, claude_bin="claude", timeout=30, transcript_output_dir=tmp_path) + + assert rec["ok"] is False + assert rec["error_kind"] == "session-error" + assert rec["transcript_missing"] is True + assert rec["cost_usd"] is None + + +def test_real_cli_notification_before_first_result_is_complete_evidence(monkeypatch, tmp_path): + from workflow_bench.mock_provider import MockProvider, Reply + + claude = os.environ.get("CLAUDE_CANARY_BIN") + if not claude or not Path(claude).is_file(): + if os.environ.get("GITNEXUS_REQUIRE_CLAUDE_CANARY") == "1": + pytest.fail("required pinned CLAUDE_CANARY_BIN is missing") + pytest.skip("the containment job supplies the pinned CLAUDE_CANARY_BIN") + home = tmp_path / "home" + home.mkdir() + notification_seen = threading.Event() + observe_line = runner_sessions.SessionProgress._observe_line + captured = [] + + def observe_notification(self, line): + observe_line(self, line) + event = json.loads(line) + captured.append(event) + if event.get("type") == "system" and event.get("subtype") == "task_notification": + notification_seen.set() + + class BoundaryProvider(MockProvider): + def next_reply(self): + if len(self.requests) == 2: + # Finish the real background command while the first foreground + # response is pending, and wait for its notification to arrive. + (home / "completion-ready").touch() + if not notification_seen.wait(10): + raise AssertionError("CLI did not emit the background completion notification") + return super().next_reply() + + monkeypatch.setattr(runner_sessions.SessionProgress, "_observe_line", observe_notification) + replies = [ + Reply( + tools=[ + { + "name": "Bash", + "input": { + "command": "while [ ! -f completion-ready ]; do sleep 0.01; done; printf done", + "run_in_background": True, + }, + } + ], + input_tokens=101, + output_tokens=11, + ), + Reply(text="First completion", input_tokens=102, output_tokens=12), + Reply(text="Background completion acknowledged", input_tokens=103, output_tokens=13), + ] + with BoundaryProvider(replies) as provider: + rec = runner.run_claude( + "Run the command", + home, + claude_bin=claude, + timeout=40, + env={ + "PATH": os.environ["PATH"], + "HOME": str(home), + "ANTHROPIC_API_KEY": "offline-probe", + "ANTHROPIC_BASE_URL": provider.base_url, + "CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC": "1", + "DISABLE_AUTOUPDATER": "1", + "HTTPS_PROXY": "http://127.0.0.1:9", + "HTTP_PROXY": "http://127.0.0.1:9", + "NO_PROXY": "127.0.0.1,localhost", + }, + allowed_tools=["Bash"], + strict_mcp_config=True, + transcript_output_dir=tmp_path, + progress_label="offline-notification-boundary", + ) + + notification = next(i for i, e in enumerate(captured) if e.get("subtype") == "task_notification") + results = [i for i, e in enumerate(captured) if e.get("type") == "result"] + assert len(results) == 2 and notification < results[0] + assert rec["ok"] is True, rec["error_detail"] + assert rec["transcript_missing"] is False + assert len(provider.requests) == 3 + assert rec["input_tokens"] == 306 and rec["output_tokens"] == 36 + assert rec["num_turns"] == 3 + assert rec["cost_usd"] == captured[results[-1]]["total_cost_usd"] + + def test_real_cli_background_completion_is_one_complete_measurement(monkeypatch, tmp_path): """Only the provider is scripted; the pinned CLI performs a real background Bash task.""" from workflow_bench.mock_provider import MockProvider, Reply diff --git a/eval/workflow_bench/runner_sessions.py b/eval/workflow_bench/runner_sessions.py index a20e2ec87..f45b2dad8 100644 --- a/eval/workflow_bench/runner_sessions.py +++ b/eval/workflow_bench/runner_sessions.py @@ -643,6 +643,7 @@ def _session_result(events: list[dict[str, Any]], duration_s: float) -> tuple[di turns = 0 cost = 0.0 result_ids: set[str] = set() + pending_notifications: deque[int] = deque() previous_index = -1 for index in indexes: result = events[index] @@ -668,27 +669,25 @@ def _session_result(events: list[dict[str, Any]], duration_s: float) -> tuple[di segment = events[previous_index + 1 : index] if any(event.get("session_id", session_id) != session_id for event in segment): raise ValueError("background continuation changed session identity") + pending_notifications.extend( + previous_index + 1 + i + for i, event in enumerate(segment) + if event.get("type") == "system" and event.get("subtype") == "task_notification" + ) if previous_index >= 0: - # Reject duplicate/replayed results and unrelated continuations. - # A task notification must precede a fresh foreground init and reply. - notification = next( - ( - i - for i, e in enumerate(segment) - if e.get("type") == "system" and e.get("subtype") == "task_notification" - ), - None, - ) + # The CLI can announce completion before the preceding result. + # Consume each pending notification only once, before a fresh turn. init = next( (i for i, e in enumerate(segment) if e.get("type") == "system" and e.get("subtype") == "init"), None ) if ( - notification is None + not pending_notifications or init is None - or notification >= init + or pending_notifications[0] >= previous_index + 1 + init or not any(e.get("type") == "assistant" for e in segment[init + 1 :]) ): raise ValueError("additional result lacks a background completion turn") + pending_notifications.popleft() for field in USAGE_FIELDS: total_usage[field] += usage[field] turns += count From 69a3ab1d58d703e89778f642faa0c44e7d574cf8 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 23 Oct 2026 21:02:27 +0100 Subject: [PATCH 2/4] chore(deps)(deps): bump dompurify (#3699) Bumps the npm_and_yarn group with 1 update in the /gitnexus-web directory: [dompurify](https://github.com/cure53/DOMPurify). Updates `dompurify` from 3.4.16 to 3.4.17 - [Release notes](https://github.com/cure53/DOMPurify/releases) - [Commits](https://github.com/cure53/DOMPurify/compare/3.4.16...3.4.17) --- updated-dependencies: - dependency-name: dompurify dependency-version: 3.4.17 dependency-type: direct:production dependency-group: npm_and_yarn ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- gitnexus-web/package-lock.json | 8 ++++---- gitnexus-web/package.json | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gitnexus-web/package-lock.json b/gitnexus-web/package-lock.json index c94752826..62adbae7f 100644 --- a/gitnexus-web/package-lock.json +++ b/gitnexus-web/package-lock.json @@ -18,7 +18,7 @@ "@tailwindcss/vite": "^4.3.3", "axios": "^1.20.0", "d3": "^7.9.0", - "dompurify": "^3.4.16", + "dompurify": "^3.4.17", "gitnexus-shared": "file:../gitnexus-shared", "graphology": "^0.26.0", "graphology-indices": "^0.17.0", @@ -4701,9 +4701,9 @@ "peer": true }, "node_modules/dompurify": { - "version": "3.4.16", - "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.16.tgz", - "integrity": "sha512-sqo+pNp3qRhCIpbgRi1y8Tgk27Bo2Ry7w0dC1NBeNTdZChWjz9Xb/KOoZbRP/R6pQZ80Qw8YhXw13hWWBbMRnQ==", + "version": "3.4.17", + "resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.17.tgz", + "integrity": "sha512-B/pSlYVHQ58kqISD/yDjuJ1yfzI34m1gF4lAod2bwSRhiN4K5Cl3H8Af7hqB2PmTL911tdEudnnXn5FnhRoz0w==", "license": "(MPL-2.0 OR Apache-2.0)", "optionalDependencies": { "@types/trusted-types": "^2.0.7" diff --git a/gitnexus-web/package.json b/gitnexus-web/package.json index a7849d039..7068a092c 100644 --- a/gitnexus-web/package.json +++ b/gitnexus-web/package.json @@ -28,7 +28,7 @@ "@tailwindcss/vite": "^4.3.3", "axios": "^1.20.0", "d3": "^7.9.0", - "dompurify": "^3.4.16", + "dompurify": "^3.4.17", "gitnexus-shared": "file:../gitnexus-shared", "graphology": "^0.26.0", "graphology-indices": "^0.17.0", From 0f3bad23b9efd87ce60c2f65c73dbd4880291f82 Mon Sep 17 00:00:00 2001 From: samuel mbabhazi <111171386+samuelmbabhazi@users.noreply.github.com> Date: Sat, 24 Oct 2026 09:45:24 +0200 Subject: [PATCH 3/4] feat(web): resolve a root-relative GITNEXUS_BACKEND_URL against the page origin (#3640) * feat(web): resolve a root-relative GITNEXUS_BACKEND_URL against the page origin A GITNEXUS_BACKEND_URL of `/` (or `/prefix`) is injected as-is, and the web client resolves it against the origin that served the page, so one gitnexus-web deployment reaches its API on every hostname a reverse proxy gives it. Protocol-relative values are left to the URL check and rejected. Documents the two ways to route /api/* behind a proxy. Closes #3593 * fix(web): reject a backslash-prefixed same-origin path The URL parser reads a backslash as a slash, so `/\host` resolved against the page origin became `http://host/`, an origin change. Both the server check and the client resolution now treat it like `//host`. * fix(web): keep a configured base ending in /api, and drop the unsupported direct route normalizeServerUrl stripped a trailing /api from every URL, so a deployment configured with GITNEXUS_BACKEND_URL=/proxy/api passed health and discovery and then connected to /proxy/repo. The deployment's own base is now kept as configured; the /api cleanup stays for hand-entered URLs. A regression covers health, discovery and repository selection under /proxy/api. The README no longer offers routing /api/* straight to `gitnexus serve` on a public hostname: serve refuses to start with GITNEXUS_PUBLIC_ORIGIN set until it has authentication of its own. * fix(web): keep only a same-origin base as configured The bypass in normalizeServerUrl matched any injected default, so an absolute GITNEXUS_BACKEND_URL ending in /api would have produced a doubled /api segment, and one carrying credentials would have kept them. Only a root-relative deployment base is kept now; it is resolved from the page origin, so it has neither. Absolute injected URLs are normalized as before. --- .env.example | 6 ++ docker-compose.yaml | 3 + docker-server.mjs | 16 +++- docker-server.test.mjs | 41 ++++++++++ docs/deployment.md | 23 ++++++ gitnexus-web/src/config/ui-constants.ts | 23 ++++++ gitnexus-web/src/services/backend-client.ts | 7 ++ gitnexus-web/src/vite-env.d.ts | 5 ++ .../test/unit/server-connection.test.ts | 77 +++++++++++++++++++ 9 files changed, 200 insertions(+), 1 deletion(-) diff --git a/.env.example b/.env.example index 445ae37a7..374dc5e1a 100644 --- a/.env.example +++ b/.env.example @@ -14,6 +14,12 @@ WEB_CONTAINER_NAME=gitnexus-web SERVER_HOST_PORT=4747 WEB_HOST_PORT=4173 +# Where the browser reaches the API. Unset: http://localhost:4747. An absolute +# URL for a server on another host; `/` behind a reverse proxy that serves the +# API on the same origin as the UI (see README, "Same-origin API behind a +# reverse proxy"). +# GITNEXUS_BACKEND_URL=/ + # Optional read-only mount, exposed to the server as /workspace. # Override with the directory that contains the repos you want to index. WORKSPACE_DIR=./ diff --git a/docker-compose.yaml b/docker-compose.yaml index 3434c4416..f41e7bb91 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -34,6 +34,9 @@ services: # (http://localhost:4747) works when both containers run locally. # Set GITNEXUS_BACKEND_URL in your .env or shell for remote/custom setups: # GITNEXUS_BACKEND_URL=http://:4747 + # Behind a reverse proxy that routes /api/* next to the UI, set it to `/`: + # the browser then calls the API on whichever hostname served the page + # (see "Same-origin API behind a reverse proxy" in the README). environment: - GITNEXUS_BACKEND_URL=${GITNEXUS_BACKEND_URL:-} depends_on: diff --git a/docker-server.mjs b/docker-server.mjs index 3b036f7db..f1409d941 100644 --- a/docker-server.mjs +++ b/docker-server.mjs @@ -78,12 +78,26 @@ function numberFromEnv(label, fallback, min = 0) { return n; } +// A root-relative value — `/`, or `/prefix` when the API is mounted under a +// path — names no host at all: the browser resolves it against the origin that +// served the page, so one deployment answers on every hostname a reverse proxy +// gives it (#3593). Not a URL, so it is read before validHttpUrl would warn. +// `//host` is protocol-relative, an origin change, and is left to the URL check; +// so is `/\host`, since the URL parser reads a backslash as a slash. +function sameOriginPath(value) { + if (typeof value !== 'string' || !/^\/(?![/\\])/.test(value)) return null; + // A base for `${base}/api/...`: no query, fragment, backslash, whitespace or + // control char. + if (/[?#\\\s\x00-\x1f\x7f]/.test(value)) return null; + return value.replace(/\/+$/, '') || '/'; +} + // Falls back to RENDER_EXTERNAL_URL so a Render web service hands the browser // its own public origin — same-origin API calls via the proxy below, no config. const backendUrlVar = process.env.GITNEXUS_BACKEND_URL !== undefined ? 'GITNEXUS_BACKEND_URL' : 'RENDER_EXTERNAL_URL'; const rawBackendUrl = process.env.GITNEXUS_BACKEND_URL ?? process.env.RENDER_EXTERNAL_URL ?? null; -const backendUrl = validHttpUrl(backendUrlVar, rawBackendUrl); +const backendUrl = sameOriginPath(rawBackendUrl) ?? validHttpUrl(backendUrlVar, rawBackendUrl); const configScript = backendUrl ? `` : ''; diff --git a/docker-server.test.mjs b/docker-server.test.mjs index 309edf3b5..e07371992 100644 --- a/docker-server.test.mjs +++ b/docker-server.test.mjs @@ -199,6 +199,47 @@ it('injects __GITNEXUS_CONFIG__ into SPA fallback routes', async () => { }); }); +it('injects a same-origin path when GITNEXUS_BACKEND_URL is /', async () => { + await withInjectionServer({ GITNEXUS_BACKEND_URL: '/' }, async (port) => { + const res = await rawGet(port, '/'); + assert.equal(res.status, 200); + assert.ok(res.body.includes('"backendUrl":"/"'), 'Expected the root path as the backend URL'); + }); +}); + +it('injects a same-origin prefix without its trailing slash', async () => { + await withInjectionServer({ GITNEXUS_BACKEND_URL: '/gitnexus/' }, async (port) => { + const res = await rawGet(port, '/'); + assert.equal(res.status, 200); + assert.ok( + res.body.includes('"backendUrl":"/gitnexus"'), + 'Expected the prefix as the backend URL', + ); + }); +}); + +it('does not inject a protocol-relative GITNEXUS_BACKEND_URL', async () => { + await withInjectionServer({ GITNEXUS_BACKEND_URL: '//evil.example' }, async (port) => { + const res = await rawGet(port, '/'); + assert.equal(res.status, 200); + assert.ok( + !res.body.includes('__GITNEXUS_CONFIG__'), + 'Expected no __GITNEXUS_CONFIG__ for a protocol-relative value', + ); + }); +}); + +it('does not inject a backslash-prefixed GITNEXUS_BACKEND_URL, which a URL parser reads as a host', async () => { + await withInjectionServer({ GITNEXUS_BACKEND_URL: '/\\evil.example' }, async (port) => { + const res = await rawGet(port, '/'); + assert.equal(res.status, 200); + assert.ok( + !res.body.includes('__GITNEXUS_CONFIG__'), + 'Expected no __GITNEXUS_CONFIG__ for a backslash-prefixed value', + ); + }); +}); + it('does not inject when GITNEXUS_BACKEND_URL is not set', async () => { await withInjectionServer({}, async (port) => { const res = await rawGet(port, '/'); diff --git a/docs/deployment.md b/docs/deployment.md index 226d8a144..8893d6153 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -79,6 +79,29 @@ cp .env.example .env docker compose --env-file .env up -d ``` +
+Same-origin API behind a reverse proxy + +`GITNEXUS_BACKEND_URL` is normally an absolute URL, which ties the UI to one API hostname. Set it to `/` instead and the browser calls the API on the origin that served the page — so one `gitnexus-web` container can be reached on several hostnames (an internal `*.localhost` name and a public one, say) as long as `/api/*` is served next to the UI on each of them. A path such as `/gitnexus` works the same way for an API mounted under a prefix. + +`/api/*` goes through the web container's own proxy: give `gitnexus-web` `GITNEXUS_UPSTREAM_URL=http://gitnexus-server:4747` and a `GITNEXUS_SERVE_AUTH_TOKEN`; it forwards `/api/*` to the server and ends the browser origin there, so the server's CORS allowlist never sees the public hostnames. The UI asks for the token once. With Traefik, one router for the whole UI is enough: + +```yaml +services: + gitnexus-web: + environment: + - GITNEXUS_BACKEND_URL=/ + - GITNEXUS_UPSTREAM_URL=http://gitnexus-server:4747 + - GITNEXUS_SERVE_AUTH_TOKEN=${GITNEXUS_SERVE_AUTH_TOKEN} + labels: + - traefik.http.routers.gitnexus.rule=Host(`gitnexus.example.com`) || Host(`gitnexus.localhost`) + - traefik.http.services.gitnexus.loadbalancer.server.port=4173 +``` + +Routing `/api/*` straight to `gitnexus serve` on a public hostname is not supported: `serve` has no authentication of its own yet, and it refuses to start with `GITNEXUS_PUBLIC_ORIGIN` set until it does. Reach it through the proxy above. + +
+ Files: - [Dockerfile.web](../Dockerfile.web) — builds `gitnexus-shared` and `gitnexus-web`, then serves the production frontend. diff --git a/gitnexus-web/src/config/ui-constants.ts b/gitnexus-web/src/config/ui-constants.ts index 6a5c0aad7..f8479e2cd 100644 --- a/gitnexus-web/src/config/ui-constants.ts +++ b/gitnexus-web/src/config/ui-constants.ts @@ -2,7 +2,30 @@ export const ERROR_RESET_DELAY_MS = 3000; export const BACKEND_URL_DEBOUNCE_MS = 500; +/** + * A root-relative injected backend (`/`, or `/prefix`) resolved against the + * page's own origin, so the API is reached on whichever hostname served the UI + * (#3593); undefined when nothing was injected or an absolute URL was. + * `//host` and `/\host` (the URL parser reads a backslash as a slash) are + * origin changes, not paths, and are left to the URL validation to reject. + */ +function sameOriginBackendUrl(): string | undefined { + if (typeof window === 'undefined') return undefined; + const configured = window.__GITNEXUS_CONFIG__?.backendUrl; + if (!configured || !/^\/(?![/\\])/.test(configured)) return undefined; + return new URL(configured, window.location.origin).href.replace(/\/+$/, ''); +} + +/** + * The backend base a same-origin deployment mounted the API under, kept as + * configured by normalizeServerUrl: a prefix such as `/proxy/api` names a + * mount point, not a pasted endpoint. Undefined for an absolute injected URL, + * which is normalized like any other. + */ +export const SAME_ORIGIN_BACKEND_URL = sameOriginBackendUrl(); + export const DEFAULT_BACKEND_URL = + SAME_ORIGIN_BACKEND_URL || (typeof window !== 'undefined' && window.__GITNEXUS_CONFIG__?.backendUrl) || 'http://localhost:4747'; export const DEFAULT_OLLAMA_BASE_URL = 'http://localhost:11434'; diff --git a/gitnexus-web/src/services/backend-client.ts b/gitnexus-web/src/services/backend-client.ts index 3bddba929..2cc0d4885 100644 --- a/gitnexus-web/src/services/backend-client.ts +++ b/gitnexus-web/src/services/backend-client.ts @@ -9,6 +9,7 @@ import type { GraphNode, GraphRelationship } from 'gitnexus-shared'; import { CircuitOpenError, ResilientFetchExhaustedError, resilientFetch } from 'gitnexus-shared'; import { + SAME_ORIGIN_BACKEND_URL, AUTH_TOKEN_STORAGE_KEY, LARGE_GRAPH_NODE_THRESHOLD, LARGE_GRAPH_EDGE_THRESHOLD, @@ -451,6 +452,12 @@ function stripBackendUrlCredentials(url: string): string { export function normalizeServerUrl(input: string): string { let url = input.trim().replace(/\/+$/, ''); + // A same-origin deployment's base is kept as configured: a prefix such as + // `/proxy/api` names where the API is mounted, not a pasted endpoint, so the + // `/api` cleanup below is for hand-entered and absolute URLs only (#3593). + // That base is resolved from the page origin, so it carries no credentials. + if (SAME_ORIGIN_BACKEND_URL !== undefined && url === SAME_ORIGIN_BACKEND_URL) return url; + if (!url.startsWith('http://') && !url.startsWith('https://')) { if (url.startsWith('localhost') || url.startsWith('127.0.0.1')) { url = `http://${url}`; diff --git a/gitnexus-web/src/vite-env.d.ts b/gitnexus-web/src/vite-env.d.ts index cbbe32bf1..9666fafb1 100644 --- a/gitnexus-web/src/vite-env.d.ts +++ b/gitnexus-web/src/vite-env.d.ts @@ -2,6 +2,11 @@ interface Window { __GITNEXUS_CONFIG__?: { + /** + * Base URL of the API server, or a root-relative path (`/`, `/prefix`) + * meaning the origin that served the page. See DEFAULT_BACKEND_URL in + * config/ui-constants.ts and issue #3593. + */ backendUrl?: string; /** * Node-count above which the WebUI connects in chat-only mode by default diff --git a/gitnexus-web/test/unit/server-connection.test.ts b/gitnexus-web/test/unit/server-connection.test.ts index fd76567eb..2ae8ceee6 100644 --- a/gitnexus-web/test/unit/server-connection.test.ts +++ b/gitnexus-web/test/unit/server-connection.test.ts @@ -375,6 +375,83 @@ describe('DEFAULT_BACKEND_URL resolution', () => { const { DEFAULT_BACKEND_URL } = await import('../../src/config/ui-constants'); expect(DEFAULT_BACKEND_URL).toBe('http://localhost:4747'); }); + + it('resolves a root-relative backendUrl against the origin that served the page', async () => { + window.__GITNEXUS_CONFIG__ = { backendUrl: '/' }; + const { DEFAULT_BACKEND_URL } = await import('../../src/config/ui-constants'); + expect(DEFAULT_BACKEND_URL).toBe(window.location.origin); + }); + + it('keeps a same-origin prefix under the page origin', async () => { + window.__GITNEXUS_CONFIG__ = { backendUrl: '/gitnexus' }; + const { DEFAULT_BACKEND_URL } = await import('../../src/config/ui-constants'); + expect(DEFAULT_BACKEND_URL).toBe(`${window.location.origin}/gitnexus`); + }); + + it('does not resolve a protocol-relative backendUrl as a path', async () => { + window.__GITNEXUS_CONFIG__ = { backendUrl: '//evil.example' }; + const { DEFAULT_BACKEND_URL } = await import('../../src/config/ui-constants'); + expect(DEFAULT_BACKEND_URL).toBe('//evil.example'); + }); + + it('does not resolve a backslash-prefixed backendUrl, which the URL parser reads as a host', async () => { + window.__GITNEXUS_CONFIG__ = { backendUrl: '/\\evil.example' }; + const { DEFAULT_BACKEND_URL } = await import('../../src/config/ui-constants'); + expect(DEFAULT_BACKEND_URL).toBe('/\\evil.example'); + }); + + it('keeps a configured prefix ending in /api through health, discovery and repository selection', async () => { + window.__GITNEXUS_CONFIG__ = { backendUrl: '/proxy/api' }; + const { DEFAULT_BACKEND_URL } = await import('../../src/config/ui-constants'); + const client = await import('../../src/services/backend-client'); + const base = `${window.location.origin}/proxy/api`; + expect(DEFAULT_BACKEND_URL).toBe(base); + + const json = (body: unknown) => + new Response(JSON.stringify(body), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + const fetchMock = vi.fn((url: string) => { + if (url.includes('/api/repo?')) { + return Promise.resolve( + json({ name: 'big-repo', path: '/repos/big-repo', indexedAt: '2026-06-13T00:00:00Z' }), + ); + } + return Promise.resolve(json([])); + }); + vi.stubGlobal('fetch', fetchMock); + + client.setBackendUrl(DEFAULT_BACKEND_URL); + expect(await client.probeBackendStatus()).toBe('ok'); + expect(await client.fetchRepos()).toEqual([]); + const result = await client.connectToServer( + DEFAULT_BACKEND_URL, + undefined, + undefined, + 'big-repo', + { + skipGraph: true, + }, + ); + expect(result.repoInfo.name).toBe('big-repo'); + + const requested = fetchMock.mock.calls.map(([u]) => String(u)); + expect(requested).toEqual([ + `${base}/api/health`, + `${base}/api/repos`, + `${base}/api/repo?repo=big-repo`, + ]); + // A hand-entered endpoint URL still loses its /api suffix. + expect(client.normalizeServerUrl('http://localhost:4747/api')).toBe('http://localhost:4747'); + }); + + it('still cleans an absolute configured URL: a trailing /api and credentials', async () => { + window.__GITNEXUS_CONFIG__ = { backendUrl: 'http://user:secret@10.0.0.1:4747/api' }; + const { DEFAULT_BACKEND_URL } = await import('../../src/config/ui-constants'); + const client = await import('../../src/services/backend-client'); + expect(client.normalizeServerUrl(DEFAULT_BACKEND_URL)).toBe('http://10.0.0.1:4747'); + }); }); describe('LARGE_GRAPH_NODE_THRESHOLD resolution', () => { From 11a49a15c6befbe4d5de1cebe74b5456aeb44b99 Mon Sep 17 00:00:00 2001 From: Parafee41 Date: Sat, 24 Oct 2026 16:58:58 +0800 Subject: [PATCH 4/4] fix(cli): reap embedding sidecar after query (#3693) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(cli): reap embedding sidecar after query * fix(cli): drain hybrid search before cleanup * fix(cli): reap sidecar after group query --------- Co-authored-by: Gergő Magyar --- gitnexus/src/cli/group.ts | 2 +- gitnexus/src/cli/tool.ts | 33 ++++++++---- gitnexus/src/mcp/local/local-backend.ts | 11 +++- .../test/unit/group-query-cli-cleanup.test.ts | 53 +++++++++++++++++++ .../test/unit/query-degraded-signal.test.ts | 23 ++++++++ gitnexus/test/unit/tool-direct-cli.test.ts | 24 +++++++++ 6 files changed, 133 insertions(+), 13 deletions(-) create mode 100644 gitnexus/test/unit/group-query-cli-cleanup.test.ts diff --git a/gitnexus/src/cli/group.ts b/gitnexus/src/cli/group.ts index 824b13b6a..bfd613d75 100644 --- a/gitnexus/src/cli/group.ts +++ b/gitnexus/src/cli/group.ts @@ -514,7 +514,7 @@ export function registerGroupCommands(program: Command): void { } } } finally { - await backend.dispose().catch(() => {}); + await backend.disconnect().catch(() => {}); } }, ); diff --git a/gitnexus/src/cli/tool.ts b/gitnexus/src/cli/tool.ts index 30574f430..8ddc692dd 100644 --- a/gitnexus/src/cli/tool.ts +++ b/gitnexus/src/cli/tool.ts @@ -142,17 +142,28 @@ export async function queryCommand( } const backend = await getBackend(); - const result = await backend.callTool('query', { - // #2175: canonical param is search_query; the backend still accepts legacy "query". - search_query: resolvedQuery, - task_context: options?.context, - goal: options?.goal, - limit: parseLimit(options?.limit), - include_content: options?.content ?? false, - repo: options?.repo, - branch: options?.branch, - }); - output(result); + try { + const result = await backend.callTool('query', { + // #2175: canonical param is search_query; the backend still accepts legacy "query". + search_query: resolvedQuery, + task_context: options?.context, + goal: options?.goal, + limit: parseLimit(options?.limit), + include_content: options?.content ?? false, + repo: options?.repo, + branch: options?.branch, + }); + output(result); + } finally { + // A local embedding query may start the sidecar. Direct CLI commands are + // one-shot, so leaving the backend connected keeps that child (and Node's + // event loop) alive after the complete result has already been printed. + try { + await backend.disconnect(); + } finally { + _backend = null; + } + } } export async function contextCommand( diff --git a/gitnexus/src/mcp/local/local-backend.ts b/gitnexus/src/mcp/local/local-backend.ts index 9b697f79a..7a69b2b23 100644 --- a/gitnexus/src/mcp/local/local-backend.ts +++ b/gitnexus/src/mcp/local/local-backend.ts @@ -3354,10 +3354,19 @@ export class LocalBackend { const searchLimit = processLimit * maxSymbolsPerProcess; // fetch enough raw results const ftsDisabledReason = getFtsDisabledReason(meta?.capabilities?.fts); const vectorDegraded = { reason: undefined as string | undefined }; - const [bm25SearchResult, semanticResults] = await Promise.all([ + const [bm25Outcome, semanticOutcome] = await Promise.allSettled([ timer.time('bm25', this.bm25Search(repo, searchQuery, searchLimit, ftsDisabledReason)), timer.time('vector', this.semanticSearch(repo, searchQuery, searchLimit, vectorDegraded)), ]); + // A rejected lane must not let the other lane outlive query(). In particular, + // semantic search may still be importing the embedder and can start its + // sidecar after a one-shot CLI has already run disconnect() in `finally`. + // Settle both lanes first, then preserve Promise.all's left-to-right error + // precedence for callers. + if (bm25Outcome.status === 'rejected') throw bm25Outcome.reason; + if (semanticOutcome.status === 'rejected') throw semanticOutcome.reason; + const bm25SearchResult = bm25Outcome.value; + const semanticResults = semanticOutcome.value; // Guard against undefined results (#1489) — when FTS is entirely // unavailable the search helper may return an unexpected shape. diff --git a/gitnexus/test/unit/group-query-cli-cleanup.test.ts b/gitnexus/test/unit/group-query-cli-cleanup.test.ts new file mode 100644 index 000000000..6b9029def --- /dev/null +++ b/gitnexus/test/unit/group-query-cli-cleanup.test.ts @@ -0,0 +1,53 @@ +import { Command } from 'commander'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const initMock = vi.fn(); +const groupQueryMock = vi.fn(); +const disconnectMock = vi.fn(); +const disposeMock = vi.fn(); + +vi.mock('../../src/mcp/local/local-backend.js', () => ({ + LocalBackend: class { + init = initMock; + disconnect = disconnectMock; + dispose = disposeMock; + getGroupService() { + return { groupQuery: groupQueryMock }; + } + }, +})); + +describe('group query CLI cleanup', () => { + beforeEach(() => { + initMock.mockReset().mockResolvedValue(true); + groupQueryMock.mockReset().mockResolvedValue({ results: [], per_repo: [] }); + disconnectMock.mockReset().mockResolvedValue(undefined); + disposeMock.mockReset().mockResolvedValue(undefined); + vi.spyOn(console, 'log').mockImplementation(() => {}); + }); + + it('disconnects the backend after a successful query', async () => { + const { registerGroupCommands } = await import('../../src/cli/group.js'); + const program = new Command().exitOverride(); + registerGroupCommands(program); + + await program.parseAsync(['node', 'gitnexus', 'group', 'query', 'team', 'auth flow']); + + expect(disconnectMock).toHaveBeenCalledOnce(); + expect(disposeMock).not.toHaveBeenCalled(); + }); + + it('disconnects the backend when a group query fails', async () => { + groupQueryMock.mockRejectedValue(new Error('query failed')); + const { registerGroupCommands } = await import('../../src/cli/group.js'); + const program = new Command().exitOverride(); + registerGroupCommands(program); + + await expect( + program.parseAsync(['node', 'gitnexus', 'group', 'query', 'team', 'auth flow']), + ).rejects.toThrow('query failed'); + + expect(disconnectMock).toHaveBeenCalledOnce(); + expect(disposeMock).not.toHaveBeenCalled(); + }); +}); diff --git a/gitnexus/test/unit/query-degraded-signal.test.ts b/gitnexus/test/unit/query-degraded-signal.test.ts index 7e28c5f6a..3ea25660e 100644 --- a/gitnexus/test/unit/query-degraded-signal.test.ts +++ b/gitnexus/test/unit/query-degraded-signal.test.ts @@ -83,6 +83,29 @@ describe('query: degraded-enrichment signal', () => { beforeEach(() => vi.clearAllMocks()); afterEach(() => vi.unstubAllEnvs()); + it('drains semantic search before returning a BM25 failure', async () => { + const b = makeBackend(true); + let finishSemantic!: () => void; + const semanticPending = new Promise((resolve) => { + finishSemantic = resolve; + }); + (b.backend as any).bm25Search = vi.fn().mockRejectedValue(new Error('invalid symbol row')); + (b.backend as any).semanticSearch = vi.fn().mockImplementation(async () => { + await semanticPending; + return []; + }); + + let querySettled = false; + const query = runQuery(b).finally(() => { + querySettled = true; + }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(querySettled).toBe(false); + finishSemantic(); + await expect(query).rejects.toThrow('invalid symbol row'); + }); + it('a REAL enrichment failure surfaces warning + partial, and still returns the symbol', async () => { const b = makeBackend(true); executeParameterizedMock.mockImplementation(async (_repo: string, query: string) => { diff --git a/gitnexus/test/unit/tool-direct-cli.test.ts b/gitnexus/test/unit/tool-direct-cli.test.ts index 7a0afd392..99a6e0b16 100644 --- a/gitnexus/test/unit/tool-direct-cli.test.ts +++ b/gitnexus/test/unit/tool-direct-cli.test.ts @@ -2,12 +2,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; const initMock = vi.fn(); const callToolMock = vi.fn(); +const disconnectMock = vi.fn(); const writeSyncMock = vi.fn(); vi.mock('../../src/mcp/local/local-backend.js', () => ({ LocalBackend: class { init = initMock; callTool = callToolMock; + disconnect = disconnectMock; }, })); @@ -22,9 +24,11 @@ describe('direct CLI tool commands', () => { vi.resetModules(); initMock.mockReset(); callToolMock.mockReset(); + disconnectMock.mockReset(); writeSyncMock.mockReset(); process.exitCode = undefined; initMock.mockResolvedValue(true); + disconnectMock.mockResolvedValue(undefined); }); // These commands set `process.exitCode` on the real process. Clearing it after @@ -164,6 +168,26 @@ describe('direct CLI tool commands', () => { expect(writeSyncMock).toHaveBeenCalledWith(1, expect.stringContaining('not found')); expect(process.exitCode).toBe(1); + expect(disconnectMock).toHaveBeenCalledOnce(); + }); + + it('disconnects after a successful query so the embedding sidecar cannot hold the CLI open', async () => { + callToolMock.mockResolvedValue({ definitions: [], processes: [] }); + const { queryCommand } = await import('../../src/cli/tool.js'); + + await queryCommand('auth flow'); + + expect(writeSyncMock).toHaveBeenCalledWith(1, expect.stringContaining('"definitions": []')); + expect(disconnectMock).toHaveBeenCalledOnce(); + }); + + it('disconnects when a query throws', async () => { + callToolMock.mockRejectedValue(new Error('embedding failed')); + const { queryCommand } = await import('../../src/cli/tool.js'); + + await expect(queryCommand('auth flow')).rejects.toThrow('embedding failed'); + + expect(disconnectMock).toHaveBeenCalledOnce(); }); // `partial` is cross-tool vocabulary, not detect_changes' private flag, and the