Skip to content

Fix backend startup errors and API proxy issues for external IP access#565

Closed
croakingtoad wants to merge 1 commit intocoleam00:mainfrom
croakingtoad:fix/backend-startup-and-proxy-issues
Closed

Fix backend startup errors and API proxy issues for external IP access#565
croakingtoad wants to merge 1 commit intocoleam00:mainfrom
croakingtoad:fix/backend-startup-and-proxy-issues

Conversation

@croakingtoad
Copy link
Copy Markdown

@croakingtoad croakingtoad commented Sep 2, 2025

Summary

This PR fixes critical issues preventing Archon from working correctly when accessed via external IP addresses (not localhost), particularly affecting cloud deployments on DigitalOcean, AWS, etc.

Issues Fixed

  • 🐛 Backend Service Startup Failure - Persistent error popup every 10-15 seconds
  • 🐛 Configuration check failures - "Failed to fetch" errors preventing Settings from loading
  • 🐛 Projects schema verification - "Unable to verify projects schema" warning

Root Cause Analysis

1. Health Endpoint Response Format

The /api/health endpoint was missing required fields that the frontend expected:

  • Missing ready: true field caused frontend to think backend wasn't initialized
  • Frontend was checking healthData.ready === true but field didn't exist

2. API URL Construction Issues

Frontend services were bypassing the Vite proxy:

  • Services constructed full URLs like http://68.183.152.55:8181/api/...
  • Should use relative URLs like /api/... to go through Vite proxy at port 3737
  • This caused CORS issues and connection failures when accessing from external IPs

3. Static URL Configuration

Service classes set baseUrl once at instantiation:

  • Used getApiUrl() which returned the backend port directly
  • Didn't update when configuration changed
  • Needed to use relative URLs consistently

Changes Made

Backend Changes

  • knowledge_api.py: Added ready, credentials_loaded, and schema_valid fields to health endpoint response

Frontend Changes

  • api.ts: Modified getApiUrl() to return empty string in development (forces proxy usage)
  • MainLayout.tsx: Changed health check to use relative URL /api/health
  • FeaturesSection.tsx: Fixed projects health check to use relative URL
  • credentialsService.ts: Replaced all API calls with relative URLs
  • mcpClientService.ts: Replaced all API calls with relative URLs
  • bugReportService.ts: Fixed GitHub bug report endpoint
  • testService.ts: Set API_BASE_URL to empty string

Testing Performed

✅ Backend health endpoint returns correct response format
✅ All API calls go through Vite proxy (port 3737)
✅ Settings page loads without errors
✅ Projects toggle works correctly
✅ No more "Backend Service Startup Failure" popups
✅ Configuration checks pass successfully

Deployment Impact

This fix is critical for:

  • Cloud deployments (DigitalOcean, AWS, GCP, etc.)
  • Docker containerized deployments
  • Reverse proxy configurations (nginx, Caddy, etc.)
  • Development environments accessed via external IPs
  • Any non-localhost access scenarios

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Health status responses now include readiness and validation details.
    • Automatic WebSocket URL resolution for more reliable live connections.
  • Bug Fixes

    • API requests consistently route through the dev proxy, improving reliability in development.
    • Clearer, standardized error messages across multiple endpoints for easier troubleshooting.
  • Refactor

    • Centralized API and WebSocket URL handling with relative paths for consistent behavior across environments.
    • Unified health checks and service calls to use a single, proxy-friendly API base.

This PR fixes critical issues preventing Archon from working correctly when accessed via external IP addresses (not localhost).

## Issues Fixed

1. **Backend Service Startup Failure** - Frontend showed persistent error popup
2. **Configuration check failures** - "Failed to fetch" errors in Settings
3. **Projects schema verification** - Unable to verify projects schema warning

## Root Causes

1. **Missing `ready` field in health endpoint**: The `/api/health` endpoint in knowledge_api.py was missing the `ready: true` field that the frontend expected
2. **Direct backend port access instead of proxy**: Frontend services were constructing URLs like `http://EXTERNAL_IP:8181` instead of using relative URLs through Vite proxy
3. **Hardcoded API URLs at service instantiation**: Service classes were setting `baseUrl` once at startup, not dynamically

## Changes Made

### Backend (Python)
- `python/src/server/api_routes/knowledge_api.py`: Added `ready`, `credentials_loaded`, and `schema_valid` fields to health response

### Frontend (React/TypeScript)
- `archon-ui-main/src/config/api.ts`: Changed `getApiUrl()` to always return empty string in development (forces proxy usage)
- `archon-ui-main/src/components/layouts/MainLayout.tsx`: Use relative URL `/api/health` instead of constructing full URL
- `archon-ui-main/src/components/settings/FeaturesSection.tsx`: Use relative URL for projects health check
- `archon-ui-main/src/services/credentialsService.ts`: Replace all API calls with relative URLs
- `archon-ui-main/src/services/mcpClientService.ts`: Replace all API calls with relative URLs
- `archon-ui-main/src/services/bugReportService.ts`: Use relative URL for bug report endpoint
- `archon-ui-main/src/services/testService.ts`: Set API_BASE_URL to empty string

## Testing
- Verified backend health endpoint returns correct format
- Confirmed all API calls go through Vite proxy (port 3737)
- Tested Settings page loads without errors
- Verified Projects toggle works correctly

## Impact
This fix ensures Archon works correctly when:
- Deployed on cloud servers (DigitalOcean, AWS, etc.)
- Accessed via external IP addresses
- Running behind reverse proxies
- Used in development with non-localhost access

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 2, 2025

Walkthrough

Frontend fetches switch to relative /api URLs routed via the Vite proxy; API config adds helpers and public constants for API/WS resolution; MCP client service adopts uniform error parsing and keeps Archon integration; backend knowledge health response includes readiness fields. No public signatures changed.

Changes

Cohort / File(s) Summary
UI health checks via proxy
archon-ui-main/src/components/layouts/MainLayout.tsx, archon-ui-main/src/components/settings/FeaturesSection.tsx
Health check endpoints changed to relative paths (/api/health, /api/projects/health) so requests go through the Vite proxy; logs updated accordingly.
API URL/config centralization
archon-ui-main/src/config/api.ts
Simplified getApiUrl to prefer relative URLs; removed window-based URL assembly; added getApiBasePath, getWebSocketUrl; exported API_BASE_URL, API_FULL_URL, WS_URL.
Services moved to relative API paths
archon-ui-main/src/services/credentialsService.ts, archon-ui-main/src/services/bugReportService.ts, archon-ui-main/src/services/testService.ts
Replaced dynamic base URL usage with relative /api/... endpoints to route through Vite proxy; logic and signatures unchanged.
MCP client service: proxy + error handling
archon-ui-main/src/services/mcpClientService.ts
Switched to relative /api/mcp/... endpoints; standardized error handling by parsing JSON and throwing `Error(error.detail
Backend health payload update
python/src/server/api_routes/knowledge_api.py
knowledge_health() healthy response now includes ready, credentials_loaded, and schema_valid fields.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant UI as UI (Browser)
  participant Vite as Vite Dev Proxy
  participant API as Backend API

  UI->>Vite: GET /api/health
  Vite->>API: Forward /api/health
  API-->>Vite: 200 OK {status, ready, credentials_loaded, schema_valid}
  Vite-->>UI: 200 OK ...
  note over UI: Health checks now use relative /api paths via proxy
Loading
sequenceDiagram
  autonumber
  participant UI as UI Components
  participant MCP as mcpClientService
  participant API as /api/mcp/...

  UI->>MCP: callClientTool(params)
  MCP->>API: POST /api/mcp/clients/:id/tools/:tool/call
  alt success
    API-->>MCP: 200 OK {result}
    MCP-->>UI: result
  else non-ok
    API-->>MCP: 4xx/5xx {detail:"message"}
    MCP->>MCP: parse JSON, throw Error(detail || default)
    MCP-->>UI: throw Error(...)
    note over MCP,UI: Unified error parsing across MCP endpoints
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

I thump my paw—new paths I try,
Through proxy burrows packets fly.
MCP now speaks its errors clear,
Health fields blink: “all ready here.”
With whiskers twitching, I approve—
Hop, hop! Our routes now smoothly move. 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

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

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
archon-ui-main/src/services/mcpClientService.ts (2)

412-416: Robust host resolution: new URL may throw for relative API URLs.

If getApiUrl() returns a relative path (e.g., '/api'), new URL(apiUrl) throws. Resolve the base from window.location when the API URL isn’t absolute.

-    const apiUrl = getApiUrl();
-    const url = new URL(apiUrl || `http://${window.location.hostname}:${mcpPort}`);
-    const mcpUrl = `${url.protocol}//${url.hostname}:${mcpPort}/mcp`;
+    const apiUrl = getApiUrl();
+    const isAbsolute = !!apiUrl && /^https?:\/\//i.test(apiUrl);
+    const base = isAbsolute ? new URL(apiUrl as string) : new URL(window.location.origin);
+    const mcpUrl = `${base.protocol}//${base.hostname}:${mcpPort}/mcp`;

375-377: Ensure return type; don’t leak raw rejection reasons.

Return a typed fallback object when a status promise is rejected.

-    return statuses.map((result) =>
-      result.status === 'fulfilled' ? result.value : result.reason
-    );
+    return statuses.map((result, index) =>
+      result.status === 'fulfilled'
+        ? result.value
+        : {
+            client: clients[index],
+            status: {
+              client_id: clients[index].id,
+              status: 'error',
+              last_seen: null,
+              last_error: (result as PromiseRejectedResult).reason?.message || 'Unknown error',
+              is_active: false
+            }
+          }
+    );
🧹 Nitpick comments (17)
archon-ui-main/src/services/bugReportService.ts (2)

164-170: Good: route bug reports via proxy-relative endpoint. Add a short timeout.

The switch to '/api/bug-report/github' fixes CORS/proxy issues. Add a timeout to avoid hanging UX.

-      const response = await fetch('/api/bug-report/github', {
+      const response = await fetch('/api/bug-report/github', {
         method: 'POST',
         headers: {
           'Content-Type': 'application/json',
         },
         body: JSON.stringify(requestData),
+        signal: AbortSignal.timeout(8000),
       });

7-7: Remove unused import.

getApiUrl is no longer used.

-import { getApiUrl } from '../config/api';
+// (removed) import { getApiUrl } from '../config/api';
archon-ui-main/src/components/layouts/MainLayout.tsx (1)

48-49: Good: force health check through Vite proxy. Consider centralizing base path.

Using '/api/health' is correct. Prefer getApiBasePath() to avoid scattering the '/api' literal and ease future rewrites.

+import { getApiBasePath } from '../../config/api';
...
-        const response = await fetch('/api/health', {
+        const response = await fetch(`${getApiBasePath()}/health`, {
           method: 'GET',
           signal: controller.signal
         });
archon-ui-main/src/services/testService.ts (2)

45-47: Avoid local API base duplication; use centralized config.

Defining API_BASE_URL = '' here diverges from config/api.ts and risks drift. Use getApiBasePath() and pass endpoints without the '/api' prefix.

-import { getApiUrl, getWebSocketUrl } from '../config/api';
+import { getApiBasePath, getWebSocketUrl } from '../config/api';
...
-// Always use relative URLs to go through Vite proxy
-const API_BASE_URL = '';
+const API_BASE_URL = getApiBasePath();

Follow-up (illustrative) to drop '/api' prefix at call sites:

-const response = await callAPI<TestExecution>('/api/tests/mcp/run', {
+const response = await callAPI<TestExecution>('/tests/mcp/run', {

If you prefer to keep endpoints with '/api' prefixed, then set const API_BASE_URL = '' via a single export in config and import it here to avoid duplication. I can prep a small PR to standardize either approach.


42-42: Remove unused import.

getApiUrl isn’t used in this module.

-import { getApiUrl, getWebSocketUrl } from '../config/api';
+import { getWebSocketUrl } from '../config/api';
archon-ui-main/src/services/credentialsService.ts (5)

81-87: Unify error handling and surface HTTP details.

Route network/Fetch failures through handleCredentialError and include status/text for non-OK responses to keep UX consistent with update/create/delete.

Apply:

   async getAllCredentials(): Promise<Credential[]> {
-    const response = await fetch('/api/credentials');
-    if (!response.ok) {
-      throw new Error("Failed to fetch credentials");
-    }
-    return response.json();
+    try {
+      const response = await fetch('/api/credentials');
+      if (!response.ok) {
+        const errorText = await response.text().catch(() => '');
+        throw new Error(`HTTP ${response.status}: ${errorText || response.statusText}`);
+      }
+      return response.json();
+    } catch (error) {
+      throw this.handleCredentialError(error, 'Fetching credentials');
+    }
   }

90-93: Encode path segment and standardize error handling.

If category ever contains special chars, the path will break. Also mirror the improved error handling.

-    const response = await fetch(
-      `/api/credentials/categories/${category}`,
-    );
+    try {
+      const response = await fetch(
+        `/api/credentials/categories/${encodeURIComponent(category)}`,
+      );
+      if (!response.ok) {
+        const errorText = await response.text().catch(() => '');
+        throw new Error(`HTTP ${response.status}: ${errorText || response.statusText}`);
+      }
+      const result = await response.json();
+      // ... existing mapping logic below
+      // (no change)
+    } catch (error) {
+      throw this.handleCredentialError(error, `Fetching credentials for category: ${category}`);
+    }

Note: keep the existing mapping block; only wrap/encode and add error handling.


132-141: Encode key and include response details for non-404 errors.

Avoid path parsing issues and improve diagnostics; still return empty object for 404 as you do today.

-    const response = await fetch(`/api/credentials/${key}`);
-    if (!response.ok) {
-      if (response.status === 404) {
-        // Return empty object if credential not found
-        return { key, value: undefined };
-      }
-      throw new Error(`Failed to fetch credential: ${key}`);
-    }
-    return response.json();
+    try {
+      const response = await fetch(`/api/credentials/${encodeURIComponent(key)}`);
+      if (!response.ok) {
+        if (response.status === 404) {
+          return { key, value: undefined };
+        }
+        const errorText = await response.text().catch(() => '');
+        throw new Error(`HTTP ${response.status}: ${errorText || response.statusText}`);
+      }
+      return response.json();
+    } catch (error) {
+      throw this.handleCredentialError(error, `Fetching credential '${key}'`);
+    }

223-234: Encode key in update endpoint path.

Protect against special characters in credential keys.

-        `/api/credentials/${credential.key}`,
+        `/api/credentials/${encodeURIComponent(credential.key)}`,

274-279: Encode key in delete endpoint path.

Same rationale as update/get.

-      const response = await fetch(`/api/credentials/${key}`, {
+      const response = await fetch(`/api/credentials/${encodeURIComponent(key)}`, {
         method: "DELETE",
       });
archon-ui-main/src/components/settings/FeaturesSection.tsx (2)

42-44: Avoid null sentinels; prefer throwing and handling in the outer catch.

.catch(() => null) uses null to signal failure. Let the Promise reject and rely on your existing try/catch to set fallbacks, or move this fetch into a small service for consistent error handling.

-        fetch('/api/projects/health').catch(() => null),
+        fetch('/api/projects/health'),

If you want to keep partial results without failing all requests, wrap only this call in its own try/catch outside Promise.all and set schema defaults in that local catch.


57-63: Gate debug logs or remove for production.

Console logs are helpful during development but noisy in production. Consider gating behind a dev flag.

-      console.log('🔍 Projects health response:', {
+      if (import.meta.env.DEV) console.log('🔍 Projects health response:', {
         response: projectsHealthResponse,
         ok: projectsHealthResponse?.ok,
         status: projectsHealthResponse?.status,
         url: '/api/projects/health'
       });

Apply similarly to the subsequent "Projects health data" log.

archon-ui-main/src/services/mcpClientService.ts (5)

99-101: Remove unused baseUrl property.

It’s never referenced; drop it to avoid noUnused* warnings.

-  // Always use relative URLs to go through Vite proxy
-  private baseUrl = '';
+  // REST calls use relative URLs to traverse the Vite proxy

92-92: Duplicate import of getApiUrl.

There are two identical imports; keep only one.

-import { getApiUrl } from '../config/api';

129-134: Harden error parsing across all endpoints (JSON or text).

await response.json() will throw for non-JSON/error-empty bodies, masking the original status. Add a helper and reuse it for every non-OK branch.

Apply this pattern here and to all other non-OK checks below:

-    if (!response.ok) {
-      const error = await response.json();
-      throw new Error(error.detail || 'Failed to create MCP client');
-    }
+    if (!response.ok) {
+      throw new Error(await this.getErrorMessage(response, 'Failed to create MCP client'));
+    }

Add this helper method inside the class:

private async getErrorMessage(res: Response, fallback: string): Promise<string> {
  try {
    const data = await res.clone().json();
    return data?.detail || data?.message || `HTTP ${res.status} ${res.statusText}` || fallback;
  } catch {
    const text = await res.text().catch(() => '');
    return text || `HTTP ${res.status} ${res.statusText}` || fallback;
  }
}

110-116: Apply the hardened error parsing consistently.

For each endpoint, replace per-endpoint JSON parsing with the shared getErrorMessage helper to avoid parse errors and improve messages.

I can push a quick sweep commit if desired.

Also applies to: 141-149, 155-167, 172-183, 193-203, 209-219, 225-233, 239-251, 261-269, 275-287, 293-301, 307-317


401-429: Minor: derive protocol/host directly to avoid surprises behind proxies.

If Archon is served behind a reverse proxy, using window.location preserves the externally visible host/protocol.

Alternative snippet (if you prefer not to touch getApiUrl at all):

const { protocol, hostname } = window.location;
const mcpUrl = `${protocol}//${hostname}:${mcpPort}/mcp`;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 277bfda and 6756204.

📒 Files selected for processing (8)
  • archon-ui-main/src/components/layouts/MainLayout.tsx (1 hunks)
  • archon-ui-main/src/components/settings/FeaturesSection.tsx (2 hunks)
  • archon-ui-main/src/config/api.ts (1 hunks)
  • archon-ui-main/src/services/bugReportService.ts (1 hunks)
  • archon-ui-main/src/services/credentialsService.ts (7 hunks)
  • archon-ui-main/src/services/mcpClientService.ts (14 hunks)
  • archon-ui-main/src/services/testService.ts (1 hunks)
  • python/src/server/api_routes/knowledge_api.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
archon-ui-main/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

archon-ui-main/**/*.{ts,tsx}: Never return null to indicate failure in the frontend; throw an Error with details instead
Use database task status values directly in the UI with no mapping: todo, doing, review, done

Files:

  • archon-ui-main/src/components/layouts/MainLayout.tsx
  • archon-ui-main/src/services/testService.ts
  • archon-ui-main/src/components/settings/FeaturesSection.tsx
  • archon-ui-main/src/services/credentialsService.ts
  • archon-ui-main/src/services/bugReportService.ts
  • archon-ui-main/src/config/api.ts
  • archon-ui-main/src/services/mcpClientService.ts
archon-ui-main/src/components/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place reusable UI components under archon-ui-main/src/components/

Files:

  • archon-ui-main/src/components/layouts/MainLayout.tsx
  • archon-ui-main/src/components/settings/FeaturesSection.tsx
archon-ui-main/src/{components,hooks,pages}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

archon-ui-main/src/{components,hooks,pages}/**/*.{ts,tsx}: State naming: use is[Action]ing for loading states (e.g., isSwitchingProject)
State naming: use [resource]Error for error messages
State naming: use selected[Resource] for current selections

Files:

  • archon-ui-main/src/components/layouts/MainLayout.tsx
  • archon-ui-main/src/components/settings/FeaturesSection.tsx
archon-ui-main/src/services/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

archon-ui-main/src/services/**/*.{ts,tsx}: Place API communication and business logic under archon-ui-main/src/services/
Service method naming in frontend should follow: get[Resource]sByProject(projectId) for scoped queries
Service method naming in frontend should follow: getResource for single resource fetch
Service method naming in frontend should follow: createResource for creates
Service method naming in frontend should follow: update[Resource](id, updates) for updates
Service method naming in frontend should follow: deleteResource for soft deletes

Files:

  • archon-ui-main/src/services/testService.ts
  • archon-ui-main/src/services/credentialsService.ts
  • archon-ui-main/src/services/bugReportService.ts
  • archon-ui-main/src/services/mcpClientService.ts
python/src/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

python/src/**/*.py: Fail fast on service startup failures (crash with clear error if credentials, database, or any service cannot initialize)
Fail fast on missing configuration or invalid environment settings
Fail fast on database connection failures; do not hide connection issues
Fail fast on authentication/authorization failures; halt the operation and surface the error
Fail fast on data corruption or validation errors; let Pydantic raise
Fail fast when critical dependencies are unavailable (required service down)
Never store invalid data that would corrupt state (e.g., zero embeddings, null foreign keys, malformed JSON); fail instead
For batch processing, complete what you can and log detailed failures per item
Background tasks should finish queues but log failures clearly
Do not crash on a single WebSocket/event failure; log and continue serving other clients
If optional features are disabled, log and skip rather than crashing
External API calls should retry with exponential backoff; then fail with a clear, specific error
When continuing after a failure, skip the failed item entirely; never persist partial or corrupted results
Include context about the attempted operation in error messages
Preserve full stack traces with exc_info=True in Python logging
Use specific exception types; avoid catching generic Exception
Never return None to indicate failure; raise an exception with details
For batch operations, report both success counts and detailed failure lists
Target Python 3.12 and keep line length at 120 characters
Use Ruff for linting (errors, warnings, unused imports, style) and keep code Ruff-clean
Use Mypy for static type checking and keep code type-safe
Enable auto-formatting on save in IDEs to maintain consistent Python style

Files:

  • python/src/server/api_routes/knowledge_api.py
python/src/server/**

📄 CodeRabbit inference engine (CLAUDE.md)

Keep the main FastAPI application under python/src/server/

Files:

  • python/src/server/api_routes/knowledge_api.py
python/src/server/api_routes/**/*.py

📄 CodeRabbit inference engine (CLAUDE.md)

Implement API route handlers under python/src/server/api_routes/

Files:

  • python/src/server/api_routes/knowledge_api.py
🧠 Learnings (2)
📚 Learning: 2025-09-02T19:42:59.116Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T19:42:59.116Z
Learning: Applies to archon-ui-main/src/services/**/*.{ts,tsx} : Place API communication and business logic under archon-ui-main/src/services/

Applied to files:

  • archon-ui-main/src/services/mcpClientService.ts
📚 Learning: 2025-09-02T19:42:59.116Z
Learnt from: CR
PR: coleam00/Archon#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T19:42:59.116Z
Learning: Applies to archon-ui-main/src/services/**/*.{ts,tsx} : Service method naming in frontend should follow: update[Resource](id, updates) for updates

Applied to files:

  • archon-ui-main/src/services/mcpClientService.ts
🧬 Code graph analysis (1)
archon-ui-main/src/services/testService.ts (1)
archon-ui-main/src/config/api.ts (1)
  • API_BASE_URL (54-54)
🔇 Additional comments (4)
archon-ui-main/src/config/api.ts (1)

9-23: The verification script above will locate any vite.config.ts files and display nearby proxy settings to confirm that relative URLs in production are properly reverse-proxied. Once we have that output, we can finalize whether the fallback to '' is safe in production or if we need to document VITE_API_URL usage.

archon-ui-main/src/services/credentialsService.ts (2)

59-60: LGTM: forcing proxy via relative paths.

Using a blank baseUrl with a clarifying comment ensures all calls traverse the Vite proxy. Good alignment with the PR goal.


250-259: LGTM: consistent error pipeline.

createCredential already funnels errors through handleCredentialError; good consistency.

archon-ui-main/src/services/mcpClientService.ts (1)

398-416: Ignore getApiUrl contract concern: getApiUrl() only returns an empty string (for relative URLs) or the VITE_API_URL value—it never returns “/api,” so the URL fallback logic in createArchonClient() remains correct.

Likely an incorrect or invalid review comment.

Comment on lines +886 to 889
"ready": True,
"credentials_loaded": True,
"schema_valid": True,
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Derive readiness flags instead of hard-coding True.

Hard-coding ready/credentials/schema as True can mislead the UI and mask startup/config issues. Compute them from actual state and env so external clients don’t proceed prematurely.

Apply this diff within the response block:

-    "ready": True,
-    "credentials_loaded": True,
-    "schema_valid": True,
+    "ready": credentials_loaded and schema_status["valid"],
+    "credentials_loaded": credentials_loaded,
+    "schema_valid": True,

And add this snippet inside knowledge_health() before building result:

import os  # if not already imported at top
credentials_loaded = bool(
    os.getenv("SUPABASE_URL")
    and (os.getenv("SUPABASE_SERVICE_ROLE_KEY") or os.getenv("SUPABASE_ANON_KEY"))
)
# Optionally include LM provider keys if required for your ready gate:
# credentials_loaded = credentials_loaded and bool(os.getenv("OPENAI_API_KEY") or os.getenv("ANTHROPIC_API_KEY"))

Would you like me to wire a lightweight Supabase connectivity probe with a short timeout to further validate readiness?

🤖 Prompt for AI Agents
In python/src/server/api_routes/knowledge_api.py around lines 886 to 889, the
readiness fields are hard-coded to True which can mislead clients; compute
credentials_loaded and ready from environment/state instead: add an import for
os if missing and insert the provided credentials_loaded assignment before
building result, then replace "credentials_loaded": True and "ready": True with
"credentials_loaded": credentials_loaded and "ready": credentials_loaded (or a
combined boolean that also checks any LM provider keys you require); similarly
compute and set "schema_valid" based on your actual schema validation logic
instead of True; optionally consider wiring a short-timeout Supabase
connectivity probe to further validate readiness.

@coleam00
Copy link
Copy Markdown
Owner

coleam00 commented Sep 4, 2025

Thanks for this @croakingtoad! I will be reviewing PRs this weekend 👍

@stevepresley
Copy link
Copy Markdown

FYI - @croakingtoad 's fixes are also in my PR relating the localhost/versus external IP being used incorrectly on the /health call
#573

@coleam00
Copy link
Copy Markdown
Owner

coleam00 commented Sep 8, 2025

@croakingtoad - we have cleaned up some things that have caused merge conflicts and potentially resolved the first issue (backend startup errors). Would you be able to test/rebase if needed?

@leex279
Copy link
Copy Markdown
Collaborator

leex279 commented Oct 19, 2025

Closed => no response

@leex279 leex279 closed this Oct 19, 2025
POWERFULMOVES pushed a commit to POWERFULMOVES/PMOVES-Archon that referenced this pull request Feb 12, 2026
…updates (coleam00#565)

Bumps the npm_and_yarn group with 1 update in the /CATACLYSM_STUDIOS_INC/PMOVES-PROVISIONS/docker-stacks/jellyfin-ai/api-gateway directory: [qs](https://github.com/ljharb/qs).
Bumps the npm_and_yarn group with 1 update in the /pmoves/ui directory: [diff](https://github.com/kpdecker/jsdiff).


Updates `qs` from 6.13.0 to 6.14.1
- [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md)
- [Commits](ljharb/qs@v6.13.0...v6.14.1)

Updates `diff` from 7.0.0 to 8.0.3
- [Changelog](https://github.com/kpdecker/jsdiff/blob/master/release-notes.md)
- [Commits](kpdecker/jsdiff@7.0.0...v8.0.3)

---
updated-dependencies:
- dependency-name: qs
  dependency-version: 6.14.1
  dependency-type: indirect
  dependency-group: npm_and_yarn
- dependency-name: diff
  dependency-version: 8.0.3
  dependency-type: indirect
  dependency-group: npm_and_yarn
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
coleam00 pushed a commit that referenced this pull request Apr 7, 2026
…g empty string (#593)

* Fix: warn on unknown \$nodeId.output refs instead of silently returning empty string (#565)

Typos in $nodeId.output references (e.g. $clasify.output vs $classify.output)
were silently replaced with empty strings with no log output, making them
impossible to diagnose. Load-time validation also skipped these refs, so
broken workflows loaded without error.

Changes:
- substituteNodeOutputRefs(): log warn 'dag_node_output_ref_unknown_node' for unknown nodeId
- resolveOutputRef(): log warn 'condition_output_ref_unknown_node' for unknown nodeId
  (distinguishes "node doesn't exist" from "node has empty output")
- validateDagStructure(): validate $nodeId.output refs in when: and prompt: fields at load time
- Update tests to assert warning is logged for unknown node refs
- Move condition-evaluator.test.ts to its own bun test invocation (uses mock.module)

Fixes #565

* fix: address review findings from PR #593

- Add loader.test.ts coverage for load-time $nodeId.output ref validation
  (5 tests: bad when: ref, bad prompt: ref, valid refs, multi-source
  lastIndex reset, bash: excluded from load-time validation)
- Rename _match → match in dag-executor substituteNodeOutputRefs callback
- Tighten warn count assertion to .toBe(2) in condition-evaluator.test.ts
- Update validateDagStructure JSDoc to list fourth validation check
- Update resolveOutputRef JSDoc to document warn-vs-silent distinction
- Add inline comment on lastIndex = 0 explaining stateful g-flag regex reset
Tyone88 pushed a commit to Tyone88/Archon that referenced this pull request Apr 16, 2026
…g empty string (coleam00#593)

* Fix: warn on unknown \$nodeId.output refs instead of silently returning empty string (coleam00#565)

Typos in $nodeId.output references (e.g. $clasify.output vs $classify.output)
were silently replaced with empty strings with no log output, making them
impossible to diagnose. Load-time validation also skipped these refs, so
broken workflows loaded without error.

Changes:
- substituteNodeOutputRefs(): log warn 'dag_node_output_ref_unknown_node' for unknown nodeId
- resolveOutputRef(): log warn 'condition_output_ref_unknown_node' for unknown nodeId
  (distinguishes "node doesn't exist" from "node has empty output")
- validateDagStructure(): validate $nodeId.output refs in when: and prompt: fields at load time
- Update tests to assert warning is logged for unknown node refs
- Move condition-evaluator.test.ts to its own bun test invocation (uses mock.module)

Fixes coleam00#565

* fix: address review findings from PR coleam00#593

- Add loader.test.ts coverage for load-time $nodeId.output ref validation
  (5 tests: bad when: ref, bad prompt: ref, valid refs, multi-source
  lastIndex reset, bash: excluded from load-time validation)
- Rename _match → match in dag-executor substituteNodeOutputRefs callback
- Tighten warn count assertion to .toBe(2) in condition-evaluator.test.ts
- Update validateDagStructure JSDoc to list fourth validation check
- Update resolveOutputRef JSDoc to document warn-vs-silent distinction
- Add inline comment on lastIndex = 0 explaining stateful g-flag regex reset
joaobmonteiro pushed a commit to joaobmonteiro/Archon that referenced this pull request Apr 26, 2026
…g empty string (coleam00#593)

* Fix: warn on unknown \$nodeId.output refs instead of silently returning empty string (coleam00#565)

Typos in $nodeId.output references (e.g. $clasify.output vs $classify.output)
were silently replaced with empty strings with no log output, making them
impossible to diagnose. Load-time validation also skipped these refs, so
broken workflows loaded without error.

Changes:
- substituteNodeOutputRefs(): log warn 'dag_node_output_ref_unknown_node' for unknown nodeId
- resolveOutputRef(): log warn 'condition_output_ref_unknown_node' for unknown nodeId
  (distinguishes "node doesn't exist" from "node has empty output")
- validateDagStructure(): validate $nodeId.output refs in when: and prompt: fields at load time
- Update tests to assert warning is logged for unknown node refs
- Move condition-evaluator.test.ts to its own bun test invocation (uses mock.module)

Fixes coleam00#565

* fix: address review findings from PR coleam00#593

- Add loader.test.ts coverage for load-time $nodeId.output ref validation
  (5 tests: bad when: ref, bad prompt: ref, valid refs, multi-source
  lastIndex reset, bash: excluded from load-time validation)
- Rename _match → match in dag-executor substituteNodeOutputRefs callback
- Tighten warn count assertion to .toBe(2) in condition-evaluator.test.ts
- Update validateDagStructure JSDoc to list fourth validation check
- Update resolveOutputRef JSDoc to document warn-vs-silent distinction
- Add inline comment on lastIndex = 0 explaining stateful g-flag regex reset
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.

4 participants