Skip to content

fix(agent): sidecar mirrors the Vite /v1 proxy so chat tools reach core#139

Merged
helebest merged 2 commits into
mainfrom
fix/sidecar-honors-dev-proxy-target
Jun 28, 2026
Merged

fix(agent): sidecar mirrors the Vite /v1 proxy so chat tools reach core#139
helebest merged 2 commits into
mainfrom
fix/sidecar-honors-dev-proxy-target

Conversation

@helebest

@helebest helebest commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Problem

Locally, the chat page's agent tool calls all failed (fetch failed) and couldn't read the knowledge base — retrieve_knowledge / list_pages / dikw_health all showed ✗, while the rest of the app (Overview, Base, Graph) worked fine and the connection header read "connected".

Reproduced via Chrome MCP, then traced:

  • dikw-core has no CORS, so the browser keeps serverUrl at the default (http://127.0.0.1:8765) and routes its /v1 reads through the same-origin Vite proxy (VITE_DIKW_PROXY_TARGET → the real, dynamically-ported core). This is the documented browser-verify / live:verify setup.
  • The sidecar's /agent core calls run server-side and bypass that proxy — they dial whatever core URL the browser sent (the default 8765). With the proxy override in effect, nothing listens on 8765, so every agent tool returns fetch failed.

So the browser's reads succeed (proxied) while the agent's tools fail (direct) — exactly the observed symptom.

Fix

applyDevProxyTarget (server/agent/http.ts) mirrors the Vite /v1 proxy for the sidecar: when the browser sends the default core URL and VITE_DIKW_PROXY_TARGET is set, it routes the sidecar's core calls to that target. Applied at the single readCoreConnection choke point, so both the chat-message and agent-maintenance paths are consistent.

Dev-only / zero production impact: VITE_DIKW_PROXY_TARGET is never present in the standalone production sidecar → no-op. Only the default URL is rewritten → a custom, directly-reachable serverUrl is untouched.

Prevent recurrence (the requested live e2e case)

The live:verify agent↔core check (scripts/live-core/verify-agent.mjs) had a blind spot that let this through:

  1. It POSTed /agent with the live core's real URL, bypassing the proxy the browser depends on.
  2. Its assertion only checked a core tool was invoked — but a fetch failed call still emits a tool_event (status:"failed"), so it passed even when no tool reached core.

Now it sends the default core URL (like the browser) and asserts a core tool reached status:"succeeded". This reproduces the real browser path and actually catches the bug.

Verification

  • Unit (TDD, red→green): server/agent/http.test.tsapplyDevProxyTarget (3 cases) + handler wiring test. Full suite 905 passed, typecheck/lint/format clean.
  • Live, dual-environment (real dikw-core + MiniMax LLM), same new check:
    • Fixed sidecar → ✓ agent↔core verified: core tools succeeded (exit 0).
    • Unfixed sidecar → ✖ agent invoked core tools but none SUCCEEDED (failed: retrieve_knowledge, dikw_health) (exit 1) — proves the case catches the regression.

Docs

CLAUDE.md (Core connection), docs/integration-verification.md (agent↔core check), CHANGELOG.md, package.json 0.8.3 → 0.8.4.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Fixed a proxied dev/live issue where chat agent tool calls could fail in browser-based setups.
    • Improved live verification so it now checks for successful tool execution, helping catch proxy-related failures earlier.
  • Documentation

    • Updated integration and developer guidance to reflect the corrected agent-to-core flow in proxied environments.
  • Chores

    • Bumped the package version to 0.8.4.

@coderabbitai

coderabbitai Bot commented Jun 28, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@helebest, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 29 minutes and 30 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 credits.

🚦 How do rate 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 see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fe256f6e-a8c2-4e9c-92de-f3e7f519546e

📥 Commits

Reviewing files that changed from the base of the PR and between 7b5a336 and 5b61ba2.

📒 Files selected for processing (11)
  • CHANGELOG.md
  • CLAUDE.md
  • docs/agent.md
  • docs/core-contract.md
  • docs/integration-verification.md
  • package.json
  • scripts/live-core/verify-agent.mjs
  • server/agent/http.test.ts
  • server/agent/http.ts
  • server/agent/vitePlugin.test.ts
  • server/agent/vitePlugin.ts
📝 Walkthrough

Walkthrough

Adds applyDevProxyTarget to server/agent/http.ts to rewrite the default core URL to VITE_DIKW_PROXY_TARGET when set, mirroring Vite's /v1 proxy for sidecar /agent calls. Updates verify-agent.mjs to send DEFAULT_CORE_URL and assert status:"succeeded" on core tool events. Adds unit/integration tests and updates docs and changelog for version 0.8.4.

Changes

Sidecar Dev-Proxy Mirroring

Layer / File(s) Summary
applyDevProxyTarget implementation
server/agent/http.ts
Imports defaultServerUrl, adds exported applyDevProxyTarget that rewrites a matching default core URL to VITE_DIKW_PROXY_TARGET, and wires it into readCoreConnection.
Unit and integration tests
server/agent/http.test.ts
Adds applyDevProxyTarget unit tests for unset/default/non-default URL cases, and an AgentRunner test asserting the rewritten coreUrl when VITE_DIKW_PROXY_TARGET is set.
verify-agent.mjs assertion upgrade
scripts/live-core/verify-agent.mjs
Sends DEFAULT_CORE_URL in the /messages payload, collects tool_event name+status pairs, and requires at least one CORE_TOOLS event with status:"succeeded" with differentiated failure messages.
Docs, CLAUDE.md, and changelog
CHANGELOG.md, CLAUDE.md, docs/integration-verification.md, package.json
Documents the proxy mirroring behavior and updated live:verify assertions; bumps version to 0.8.4.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A hop through the proxy, a trick of the route,
The sidecar once stumbled—now sorted, no doubt!
DEFAULT_CORE_URL leads the dance,
"succeeded" confirmed at a glance.
No fetch shall fail on my watch, I declare! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: the sidecar now mirrors the Vite proxy so chat tools can reach core.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/sidecar-honors-dev-proxy-target

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b5a336618

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread server/agent/http.ts Outdated
Comment on lines +268 to +269
const proxyTarget = process.env.VITE_DIKW_PROXY_TARGET?.trim();
if (!proxyTarget) return coreUrl;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Mirror the proxy target loaded by Vite env files

When VITE_DIKW_PROXY_TARGET is supplied through Vite's .env* loading instead of the parent shell, vite.config.ts still configures /v1 from loadEnv(...), but this sidecar check sees an empty process.env and returns the default core URL. In that common dev setup the browser reads are proxied while /agent tools still dial 127.0.0.1:8765, preserving the fetch failed path this change is meant to fix; pass the loaded proxy target into agentSidecarPlugin/createAgentHandler or read the same env source as the Vite proxy.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5b61ba2. agentSidecarPlugin.configResolved now resolves VITE_DIKW_PROXY_TARGET from Vite's config.env (loadEnv → honors .env.local/.env, matching the /v1 proxy) and injects it into createDefaultAgentHandlercreateAgentHandler; applyDevProxyTarget no longer reads process.env. Live-verified with the target in .env.local only (not the shell): the agent's retrieve_knowledge reached the live core. Injection also makes the rewrite impossible in the standalone production sidecar by construction (it passes no target).

dikw-core has no CORS, so the browser keeps serverUrl at the default and
routes /v1 reads through the same-origin Vite proxy (VITE_DIKW_PROXY_TARGET
→ the real, dynamically-ported core). The sidecar's /agent core calls run
server-side and bypassed that proxy — they dialed the default port the
browser sent (127.0.0.1:8765), where nothing listens, so every agent tool
returned `fetch failed` and chat could not read the knowledge base.

applyDevProxyTarget (server/agent/http.ts) now routes the default core URL
to VITE_DIKW_PROXY_TARGET when set, mirroring the Vite /v1 proxy. Dev-only:
the env var is never present in the standalone production sidecar (no-op
there, and for a custom directly-reachable serverUrl).

The live:verify agent↔core check (scripts/live-core/verify-agent.mjs) now
reproduces the real browser path: it sends the DEFAULT core URL and asserts
a core tool reached status:"succeeded" (not merely that one was invoked — a
fetch-failed call still emits a tool_event). It fails against a sidecar that
can't reach core and passes once the proxy is mirrored.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@helebest helebest force-pushed the fix/sidecar-honors-dev-proxy-target branch from 7b5a336 to 5d387ff Compare June 28, 2026 00:16
…construction

Address xhigh code-review findings on the proxy-mirror fix:

- .env.local config path (CONFIRMED): applyDevProxyTarget read only
  process.env, but the Vite proxy + sidecar config resolve VITE_DIKW_PROXY_TARGET
  via loadEnv (.env.local). agentSidecarPlugin now resolves it from Vite's
  config.env (configResolved) and injects it into the handler, so a target in
  .env.local works — matching how the /v1 proxy itself reads it.
- Test false-negative under ambient env (CONFIRMED): readCoreConnection no
  longer reads process.env, so an exported VITE_DIKW_PROXY_TARGET can't flip the
  pre-existing default-coreUrl assertions. Suite is hermetic (verified: 12/12
  pass with the var exported).
- Prod env-leak (PLAUSIBLE): the rewrite is now gated by injection — the
  standalone sidecar passes no target, so it's impossible in prod by construction
  rather than relying on the env var being absent.
- Malformed target (CONFIRMED): resolveDevProxyTarget validates the value and
  warns on a non-http(s)/unparseable target instead of silently no-opping into
  the opaque `fetch failed`.
- Docs (PLAUSIBLE): documented the dev-only mirror in docs/agent.md +
  docs/core-contract.md (the authoritative agent↔core URL contract); clarified
  verify-agent.mjs requires the live:verify proxy setup.

Live re-verified against a real dikw-core + MiniMax with the target in
.env.local only (not the shell): retrieve_knowledge succeeded.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@helebest helebest merged commit 86c429f into main Jun 28, 2026
9 checks passed
@helebest helebest deleted the fix/sidecar-honors-dev-proxy-target branch June 28, 2026 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant