QUICKSTART.md (1)

1-47: LGTM! Clear and concise quickstart guide.

The quickstart guide provides a focused, step-by-step workflow for local development setup. The structure is clean and the instructions are easy to follow.

Optional: For improved markdown formatting, consider wrapping bare URLs in angle brackets per markdown best practices (lines 25-28):

-| **Archon UI** | http://localhost:3737 | Hauptanwendung |
+| **Archon UI** | <http://localhost:3737> | Hauptanwendung |

This is a minor style preference and does not affect functionality.

python/src/server/api_routes/ollama_api.py (1)

83-142: Align all Ollama health/model flows with auth-token support

You’ve wired per-instance auth tokens into /models and /instances/health, but other Ollama flows in this module still call discovery/health without tokens:

• /validate → validate_provider_instance("ollama", instance_url) (service currently calls check_instance_health with no auth_token).
• /models/discover-and-store and /models/discover-with-details still hit Ollama without any Authorization header.

For auth-protected instances these will now systematically report failures even though /instances/health can succeed with tokens.

Recommend:

• Updating validate_provider_instance (in llm_provider_service.py) to resolve an auth_token exactly the way health_check_endpoint does and pass it through to check_instance_health.
• Planning a follow-up to add token support to the discover-and-store / discover-with-details endpoints as well, or explicitly document that they currently require unauthenticated instances.

Also applies to: 166-217, 262-305

archon-ui-main/tests/e2e/ollama-api-mode.spec.ts (1)

1-191: Reduce hard-coded timeouts and brittle selectors in Playwright tests

The flows are clear, but they’re quite fragile:

• Almost every step uses waitForTimeout(...) instead of waiting on concrete UI conditions (waitForSelector, expect(...).toBeVisible()/toBeEnabled()). This slows tests and risks flakiness.
• Many selectors are generic text queries like text=Embedding or button:has-text("Config"), which can easily break on copy changes or multiple matches.
• Navigation to Settings → RAG Settings → Embedding → Ollama Config is duplicated across tests.

Suggested improvements (can be incremental):

• Replace sleeps with condition-based waits around key elements.
• Prefer role-/test-id–based locators (getByRole, getByTestId) over raw text where possible.
• Extract the repeated “open Ollama config in RAG Settings” flow into a helper to DRY the suite.

python/src/server/services/embeddings/embedding_service.py (2)

107-172: NativeOllamaEmbeddingAdapter looks correct but has unbounded per-batch concurrency

The adapter correctly targets /api/embeddings with { "model": ..., "prompt": ... } and wraps HTTP/JSON failures in EmbeddingAPIError, which is consistent with the rest of the service.

Two things to consider:

• asyncio.gather(*...) will fire one HTTP request per text concurrently for each batch. With large EMBEDDING_BATCH_SIZE values this could mean dozens or hundreds of parallel requests to a single Ollama instance. You may want to cap per-batch concurrency via a semaphore or chunking to avoid overloading local/remote instances.
• The dimensions argument is currently ignored for native Ollama. That’s fine (Ollama controls embedding size), but it may be worth documenting in the docstring or via a comment to avoid confusion later.

---

287-303: Adapter selection for Ollama depends on api_mode string value – normalize it

_get_embedding_adapter checks:

if provider_name == "ollama" and base_url:
    if (api_mode or "native") == "native":
        ...

If OLLAMA_API_MODE ever comes through as "Native" / "NATIVE" or with whitespace, this will silently fall into the OpenAI-compatible branch even though the intent is “native”.

Given you already read/derive ollama_api_mode above, consider normalizing it once (e.g., .lower().strip()) before passing into _get_embedding_adapter, and/or do the normalization inside _get_embedding_adapter:

-        if (api_mode or "native") == "native":
+        mode = (api_mode or "native").strip().lower()
+        if mode == "native":

This makes the behavior robust to minor config variations.

python/src/server/services/ollama/model_discovery_service.py (3)

122-133: Auth token support for /api/tags is correct but not propagated to detail/capability calls

The new auth_token parameter is correctly wired into the /api/tags discovery call and keeps behavior unchanged when no token is provided. However, follow‑up calls such as _get_model_details() and the capability tests still hit Ollama endpoints without any auth header. On protected instances this will silently 401 and you’ll lose detailed metadata and accurate capability detection, even though listing itself succeeded with a token.

If you want full parity for authenticated instances, consider threading auth_token through the enrichment pipeline, e.g.:

-    async def discover_models(self, instance_url: str, fetch_details: bool = False, auth_token: str | None = None) -> list[OllamaModel]:
+    async def discover_models(
+        self,
+        instance_url: str,
+        fetch_details: bool = False,
+        auth_token: str | None = None,
+    ) -> list[OllamaModel]:

@@
-                enriched_models = await self._enrich_model_capabilities(models, instance_url, fetch_details=fetch_details)
+                enriched_models = await self._enrich_model_capabilities(
+                    models,
+                    instance_url,
+                    fetch_details=fetch_details,
+                    auth_token=auth_token,
+                )

and then update _enrich_model_capabilities, _get_model_details, and any capability helpers that hit Ollama directly to accept and forward auth_token in their HTTP headers.

This keeps non‑auth flows identical while making “fetch_details=True” actually usable with locked‑down Ollama instances.

Also applies to: 192-199

---

964-1004: Health check auth header wiring looks good; cache key ignores token changes

Passing auth_token and conditionally adding a Bearer header here is straightforward and matches the new health‑check API behavior. One nuance: the health_cache key is only health_{instance_url}, so a failure without a token and a success with a token (or vice versa) will be cached identically for 30 seconds.

If you expect users to toggle/authenticate instances and immediately rely on the health UI, consider including the auth presence in the cache key, e.g. health_{instance_url}_{bool(auth_token)} or similar, so that “with token” and “without token” statuses don’t mask each other in the short term.

---

1026-1060: Per-instance auth_tokens map depends on exact URL string matching

The auth_tokens dict is plumbed correctly into discover_models_from_multiple_instances, and using auth_tokens.get(url) is nice and simple. This does assume that the keys in auth_tokens are in the same normalized form as the instance_urls passed here (e.g. either both with /v1 stripped and no trailing slash, or both unnormalized).

If the router that constructs auth_tokens is normalizing URLs (as in the health‑check endpoint), it would be safer to normalize url before doing auth_tokens.get(...):

-        tasks = [
-            self.discover_models(url, fetch_details=fetch_details, auth_token=auth_tokens.get(url))
-            for url in instance_urls
-        ]
+        tasks = [
+            self.discover_models(
+                url,
+                fetch_details=fetch_details,
+                auth_token=auth_tokens.get(url.rstrip("/").replace("/v1", "")),
+            )
+            for url in instance_urls
+        ]

so a mismatch in /v1 or trailing slashes doesn’t quietly drop the token.

archon-ui-main/src/components/settings/RAGSettings.tsx (4)

208-221: Instance config + token sync logic is solid; consider typing RAG token fields

The new llmInstanceConfig/embeddingInstanceConfig shapes and the useEffect blocks that sync from LLM_BASE_URL / OLLAMA_* fields into these configs look logically correct:

• Refs (lastLLMConfigRef / lastEmbeddingConfigRef) prevent infinite loops.
• useAuth is derived from token presence and authToken is updated only when the incoming value changes.

The only minor concern is the reliance on (ragSettings as any).OLLAMA_*_AUTH_TOKEN to satisfy TypeScript. Since these keys are now part of your configuration surface, it would be cleaner to add them to RAGSettingsProps["ragSettings"] explicitly:

   ragSettings: {
     MODEL_CHOICE: string;
     CHAT_MODEL?: string;
+    OLLAMA_CHAT_AUTH_TOKEN?: string;
+    OLLAMA_EMBEDDING_AUTH_TOKEN?: string;
@@
     OLLAMA_EMBEDDING_URL?: string;
     OLLAMA_EMBEDDING_INSTANCE_NAME?: string;

so you can drop the as any casts and keep strict typing end‑to‑end.

Also applies to: 224-271

---

570-583: Model discovery fetches work but bypass TanStack Query and may over-fetch

ollamaMetrics now tracks llmAvailableModels / embeddingAvailableModels, and loadAvailableModels() hits /api/ollama/models both:

• Automatically when the instance URLs change, and
• After a successful manualTestConnection when a context is provided.

This is functionally fine and the error handling is intentionally soft, but two points:

1. Project guidelines say “Use TanStack Query for all data fetching”. These new fetches bypass that layer, so if you already have React Query plumbing for Ollama endpoints, consider converting loadAvailableModels into a query (with enabled flags keyed on URLs) instead of a bespoke fetch. That would centralize retries, caching, and error behavior.

2. The URL‑change effects fire regardless of the active provider; if a user temporarily switches away from Ollama, you still ping /api/ollama/models whenever the URL state changes. You may want to guard them:

-  useEffect(() => {
-    if (llmInstanceConfig.url && llmInstanceConfig.url !== DEFAULT_OLLAMA_URL) {
+  useEffect(() => {
+    if (
+      chatProvider === 'ollama' &&
+      llmInstanceConfig.url &&
+      llmInstanceConfig.url !== DEFAULT_OLLAMA_URL
+    ) {
       loadAvailableModels(llmInstanceConfig.url, 'chat');
     }
-  }, [llmInstanceConfig.url]);
+  }, [chatProvider, llmInstanceConfig.url]);

Same idea for the embedding instance. This reduces unnecessary calls when Ollama isn’t currently in use.

Also applies to: 640-691, 751-755

---

1595-1652: OLLAMA_API_MODE UI is clear; consider typing the setting key

The API mode toggle between “Native Ollama API” and “OpenAI‑Compatible” is well‑designed and the defaulting logic (native when unset) is clear. The only gap is that OLLAMA_API_MODE isn’t declared in RAGSettingsProps["ragSettings"], so all setRagSettings({ ...ragSettings, OLLAMA_API_MODE: ... }) calls rely on structural typing and the setRagSettings: any escape hatch.

For better static safety, add:

   ragSettings: {
@@
     OLLAMA_EMBEDDING_INSTANCE_NAME?: string;
+    OLLAMA_API_MODE?: 'native' | 'openai-compatible';

so backend and frontend stay in sync as this option evolves.

---

2351-2387: Auth settings in modals are wired correctly; consider clearing tokens on delete

The new auth sections in the LLM and Embedding edit modals behave as expected:

• useAuth toggles a password input.
• Tokens are stored in llmInstanceConfig.authToken / embeddingInstanceConfig.authToken.
• Saving the modal pushes tokens into ragSettings under OLLAMA_CHAT_AUTH_TOKEN and OLLAMA_EMBEDDING_AUTH_TOKEN.

Two small follow‑ups:

1. When deleting an instance (LLM or Embedding), you clear LLM_BASE_URL / OLLAMA_EMBEDDING_URL but leave the corresponding OLLAMA_*_AUTH_TOKEN in ragSettings. It might be more intuitive (and safer) to also blank those keys so “delete instance” truly removes all associated configuration.

2. The modal “Save Changes” handlers currently only update local ragSettings; persistence happens when the user later clicks the top‑level “Save Settings” button. That matches existing behavior for other fields, but the success toast (“instance updated successfully”) might be interpreted as “persisted to server”. If you’ve had user confusion around this, consider calling credentialsService.updateRagSettings(updatedSettings) here as well.

Functionally, the core token wiring looks good and doesn’t expose secrets in the UI.

Also applies to: 2400-2405, 2446-2477, 2490-2495

archon-ui-main/test-results/.last-run.json (1)

1-6: Do not commit .last-run.json; ignore test-results in VCS.

This file is a transient Playwright/Vitest artifact and will churn frequently. Please:

• Add test-results/ (or at least test-results/.last-run.json) to archon-ui-main/.gitignore.
• Remove the tracked file via git rm --cached archon-ui-main/test-results/.last-run.json.

This matches the earlier review feedback.

INFRASTRUCTURE.md (1)

108-121: Remove hard-coded Supabase JWT from docs and use a placeholder instead.

The SUPABASE_SERVICE_KEY here is a real JWT with a long expiry. Even if intended for local use, committing it is a credential leak and has already been flagged by gitleaks and a prior review.

Recommend:

-# JWT Service Role Key (generiert mit lokalem JWT_SECRET)
-SUPABASE_SERVICE_KEY=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJyb2xlIjoic2VydmljZV9yb2xlIiwiaXNzIjoic3VwYWJhc2UiLCJpYXQiOjE3NjM2NTg1MDYsImV4cCI6MjA3OTAxODUwNn0.HVH5TgwW70JZtiGdnjU4RGexDDVbGnI3mXt-diQhVy8
+# JWT Service Role Key (mit lokalem JWT_SECRET generieren – nicht committen)
+SUPABASE_SERVICE_KEY=<generate-using-script-below>

…and explicitly instruct readers to run the Python script in this doc to generate their own key locally, add it to their .env, and never commit it.

python/src/server/services/embeddings/embedding_service.py (1)

447-473: Ollama embedding config still fragile and likely passing “[ENCRYPTED]” placeholders instead of real auth tokens

This block reintroduces both issues previously flagged in earlier reviews:

rag_settings = await _maybe_await(
    credential_service.get_credentials_by_category("rag_strategy")
)
...
ollama_base_url = rag_settings.get("OLLAMA_EMBEDDING_URL", "").rstrip("/v1").rstrip("/")
ollama_auth_token = rag_settings.get("OLLAMA_EMBEDDING_AUTH_TOKEN", "")
ollama_api_mode = rag_settings.get("OLLAMA_API_MODE", "native")
...
adapter = _get_embedding_adapter(
    embedding_provider,
    client,
    base_url=ollama_base_url,
    auth_token=ollama_auth_token,
    api_mode=ollama_api_mode,
)

1. .rstrip on possibly non‑string values: get_credentials_by_category("rag_strategy") returns objects like {"value": "[ENCRYPTED]", "is_encrypted": True, ...} for encrypted rows. If OLLAMA_EMBEDDING_URL or related keys are missing or malformed, you can end up calling .rstrip on None or on a dict, which will raise and force the generic “Failed to load embedding settings” path even though this is just misconfiguration. Guard and normalize the raw value before stripping.

2. Auth token is almost certainly the [ENCRYPTED] placeholder, not the decrypted secret: Because get_credentials_by_category intentionally returns masked values for encrypted entries, OLLAMA_EMBEDDING_AUTH_TOKEN here will be either a placeholder dict or the literal string "[ENCRYPTED]". That value is then used as the Bearer token in NativeOllamaEmbeddingAdapter, so auth will consistently fail even when a valid token is stored.

To address both, you can:

• Only compute Ollama‑specific settings when embedding_provider.lower() == "ollama".
• Pull the real token via the backend‑only decryption API instead of from the category map.
• Normalize the base URL defensively.

For example:

-            rag_settings = await _maybe_await(
-                credential_service.get_credentials_by_category("rag_strategy")
-            )
-            batch_size = int(rag_settings.get("EMBEDDING_BATCH_SIZE", "100"))
-            embedding_dimensions = int(rag_settings.get("EMBEDDING_DIMENSIONS", "1536"))
-
-            # For Ollama, get native API URL, auth token, and API mode
-            ollama_base_url = rag_settings.get("OLLAMA_EMBEDDING_URL", "").rstrip("/v1").rstrip("/")
-            ollama_auth_token = rag_settings.get("OLLAMA_EMBEDDING_AUTH_TOKEN", "")
-            ollama_api_mode = rag_settings.get("OLLAMA_API_MODE", "native")
+            rag_settings = await _maybe_await(
+                credential_service.get_credentials_by_category("rag_strategy")
+            )
+            batch_size = int(rag_settings.get("EMBEDDING_BATCH_SIZE", "100"))
+            embedding_dimensions = int(rag_settings.get("EMBEDDING_DIMENSIONS", "1536"))
+
+            ollama_base_url = ""
+            ollama_auth_token = ""
+            ollama_api_mode = "native"
+
+            if (embedding_provider or "").lower() == "ollama":
+                raw_url = (rag_settings.get("OLLAMA_EMBEDDING_URL") or "").strip()
+                if isinstance(raw_url, str) and raw_url:
+                    ollama_base_url = raw_url.rstrip("/v1").rstrip("/")
+
+                # Use backend-only decrypted credential for the token
+                try:
+                    ollama_auth_token = await credential_service.get_credential(
+                        "OLLAMA_EMBEDDING_AUTH_TOKEN",
+                        default="",
+                        decrypt=True,
+                    )
+                except TypeError:
+                    # Fallback if get_credential signature differs; keep empty token
+                    pass
+
+                mode_raw = rag_settings.get("OLLAMA_API_MODE") or "native"
+                if isinstance(mode_raw, str):
+                    ollama_api_mode = mode_raw.strip().lower()

and keep the default values in the except block as you already do.

This avoids noisy errors from benign misconfig, and—more importantly—ensures the adapter receives a real decrypted Bearer token instead of masked placeholders.

python/src/server/api_routes/ollama_api.py (2)

99-131: [Duplicate] Normalize instance URLs consistently when mapping Ollama auth tokens

This issue has already been flagged in a previous review. The configured base URLs strip /v1 and then trailing /, but incoming instance URLs only strip trailing /. When callers pass URLs with /v1 (common from UI), the equality checks fail and no auth token is attached, causing 401 errors.

---

180-187: [Duplicate] Do not log full rag_settings – it may contain secrets

This issue has already been flagged in a previous review. Logging the entire rag_settings dictionary exposes sensitive data including Ollama auth tokens and other credentials. This is a security/compliance risk.

PLAN.md (1)

253-261: [Duplicate] Remove real auth tokens from documentation and rotate them

This critical issue has already been flagged in a previous review. The DB snapshot contains what appears to be a real Ollama auth token (ollama_13107e338aa16a6a8295592ce050f6cb) that should be immediately rotated and replaced with a placeholder.

python/src/agent_work_orders/utils/state_reconciliation.py (1)

184-186: Catch specific exception types where possible.

Catching bare Exception during file operations masks specific errors. Consider catching OSError and its subclasses (e.g., PermissionError, FileNotFoundError) for more precise diagnostics, while allowing unexpected errors to propagate.

-            except Exception as e:
+            except OSError as e:

This improves observability by distinguishing expected file-system errors from unexpected runtime failures, making it easier to diagnose issues during reconciliation.

archon-ui-main/package.json (1)

64-67: Playwright devDependency looks good; consider adding an e2e script.

Adding @playwright/test as a devDependency is appropriate for the new e2e tests. Optionally, add a "test:e2e": "playwright test" script so CI and devs have a clear entrypoint for those tests.

QUICKSTART.md (1)

7-19: Replace machine-specific absolute paths with repo-relative or placeholder paths.

The commands hard-code /Volumes/DATEN/Coding/archon/..., which is specific to one machine and will not work for most contributors. Consider using repo-relative paths or a placeholder like <ARCHON_REPO_ROOT> instead, for example:

- cd /Volumes/DATEN/Coding/archon/supabase
+ cd path/to/archon/supabase

- cd /Volumes/DATEN/Coding/archon
+ cd path/to/archon

This keeps the quick-start copy‑pasteable for other environments.

Also applies to: 78-89, 125-128

INFRASTRUCTURE.md (1)

78-89: Generalize hard-coded local filesystem paths for broader reuse.

Multiple snippets use absolute paths like /Volumes/DATEN/Coding/archon/..., which are specific to one machine. To make this infra guide reusable, consider switching to repo-relative or placeholder paths (e.g., cd path/to/archon, cd path/to/archon/supabase) and clarifying that users should adapt them to their environment.

Also applies to: 125-128, 189-191, 345-347, 455-459, 465-472

python/src/server/services/llm_provider_service.py (1)

415-431: Ollama auth-token selection looks correct; consider DRYing it and tightening logging.

The new logic to:

• Fetch rag_strategy settings,
• Select OLLAMA_EMBEDDING_AUTH_TOKEN vs OLLAMA_CHAT_AUTH_TOKEN, and
• Fall back to "required-but-ignored" when unset,

is consistent between the OpenAI→Ollama fallback and the direct Ollama path, and avoids logging raw tokens (only lengths), which is good.

Two small follow-ups you might consider:

1. Factor out token resolution
   Extract a helper like _get_ollama_auth_token(use_embedding: bool, instance_type: str | None) that encapsulates:

   • Cache lookup / fetch of rag_strategy,
   • Key selection (EMBEDDING vs CHAT),
   • Defaulting + warning.

   That removes duplication between the fallback branch and the main provider_name == "ollama" branch.

2. Sanitize or downgrade embedding URL logging
   logger.info(f"Embedding URL from settings: {embedding_url}") and logger.info(f"Resolved embedding URL: {final_url}") will log whatever is configured, including potential credentials if someone encodes them in the URL. Using _sanitize_for_log(...) or moving these to debug would reduce the chance of leaking sensitive connection details into logs.

These are polish items; core behavior and error handling look solid.

Also applies to: 455-475, 618-622

python/src/server/services/embeddings/embedding_service.py (1)

107-172: NativeOllamaEmbeddingAdapter behavior is reasonable, but consider concurrency and dimensions semantics

The adapter’s per-text asyncio.gather over the batch and strict validation of the embedding field look good. Two small points to keep in mind:

• dimensions is currently ignored; that’s fine given Ollama’s fixed-dimension embeddings, but it may be worth a brief comment so future readers don’t assume down‑projection happens here.
• With default EMBEDDING_BATCH_SIZE you’ll fire up to that many concurrent /api/embeddings calls per batch. If users push batch size much higher, you might want an internal concurrency cap or a per‑adapter semaphore to avoid overloading smaller Ollama instances.

python/src/server/services/ollama/model_discovery_service.py (1)

122-135: Auth token support is good for /api/tags, but capability/detail calls still ignore it

The new auth_token plumbing for discover_models, check_instance_health, and discover_models_from_multiple_instances looks correct: headers are built as:

headers = {}
if (auth_token):
    headers["Authorization"] = f"Bearer {auth_token}"

and passed into client.get(...) for /api/tags.

Two follow‑ups to consider:

1. Capability and details endpoints still unauthenticated. Methods like _test_embedding_capability_fast, _test_embedding_capability, _get_model_details, and the various chat/structured‑output tests (_test_chat_capability_fast, _test_chat_capability, etc.) still hit /api/embeddings, /api/show, and chat completions without any Authorization header. On an Ollama instance that enforces auth globally, model listing and health will now work, but enrichment and capability detection will likely return 401s and silently fall back to defaults.

   If you intend “protected instance” support end‑to‑end, you probably want to thread auth_token through _enrich_model_capabilities and the downstream helpers (or at least _get_model_details and both embedding tests) and attach the same header there.

2. Health cache is keyed only by URL. check_instance_health caches by health_{instance_url} regardless of token. If a user fixes a bad token and immediately re‑checks health for the same URL, they can get a stale cached error for up to 30s. Not critical, but you might want to either skip caching when a request fails with 401, or include the token (or a hash of it) in the cache key.

Both are incremental and keep backward compatibility, but they’ll make the new auth support behave more predictably on locked‑down Ollama deployments.

Also applies to: 192-199, 235-243, 964-999, 1021-1024, 1026-1060

archon-ui-main/test-results/tests-e2e-ollama-api-mode--98290-t-to-Native-Ollama-API-mode/error-context.md (1)

1-638: Large UI snapshot is fine, but confirm it’s meant to live in version control

This file is purely a captured UI tree for an e2e failure context and doesn’t affect runtime behavior. If it’s generated by tests, consider whether it should be checked in or ignored via .gitignore to avoid churn; otherwise, it’s fine as documentation of the failing state.

python/src/server/api_routes/ollama_api.py (1)

204-215: Consider reducing log verbosity for token selection.

The health check logs token selection decisions at info level for every instance check. This creates verbose logs in production. Consider moving lines 210, 213, and 215 to debug level to reduce noise while keeping the information available for troubleshooting.

Apply this diff to reduce log verbosity:

                 if url == llm_base_url and chat_auth_token:
                     auth_token = chat_auth_token
-                    logger.info(f"Using chat auth token for {url}")
+                    logger.debug(f"Using chat auth token for {url}")
                 elif url == embedding_base_url and embedding_auth_token:
                     auth_token = embedding_auth_token
-                    logger.info(f"Using embedding auth token for {url}")
+                    logger.debug(f"Using embedding auth token for {url}")
                 else:
-                    logger.warning(f"No matching auth token found for {url}")
+                    logger.debug(f"No auth token configured for {url}")

archon-ui-main/tests/e2e/ollama-api-mode.spec.ts (1)

1-192: Replace hardcoded timeouts with Playwright's auto-waiting mechanisms.

The tests use page.waitForTimeout() extensively (30+ occurrences). Playwright's auto-waiting is more reliable and faster. Most timeouts can be removed since Playwright automatically waits for elements before interacting.

Example refactor for lines 9-26:

 test('should display API mode radio buttons when Ollama is selected', async ({ page }) => {
   // Navigate to RAG Settings tab
   await page.click('text=RAG Settings');
-  await page.waitForTimeout(500);

   // Select Ollama as embedding provider
   await page.click('text=Embedding');
-  await page.waitForTimeout(300);

   // Click on Ollama provider card
   const ollamaCard = page.locator('button:has-text("Ollama")').first();
   await ollamaCard.click();
-  await page.waitForTimeout(500);

   // Open Ollama configuration
   const configButton = page.locator('button:has-text("Config")').first();
   await configButton.click();
-  await page.waitForTimeout(500);

   // Verify API mode section is visible (auto-waits)
   await expect(page.locator('text=Ollama API Mode')).toBeVisible();

Playwright's click() and expect().toBeVisible() already include auto-waiting, making explicit timeouts unnecessary.

archon-ui-main/src/components/settings/RAGSettings.tsx (2)

1595-1652: Consider clarifying that API mode is a global setting.

The Ollama API Mode selection appears within the context-aware configuration panel (LLM or Embedding), but it's actually a global setting that affects both. Users might expect it to be per-instance.

Consider adding a helper text indicating this applies to all Ollama instances:

 <div className="mb-4 pb-4 border-b border-green-500/20">
   <label className="block text-sm font-medium text-gray-300 mb-3">
     Ollama API Mode
   </label>
+  <p className="text-xs text-gray-400 mb-2">
+    This setting applies to all Ollama instances (both LLM and Embedding)
+  </p>
   <div className="flex gap-3">

---

2363-2387: Extract authentication UI into a reusable component.

The authentication sections in both LLM and Embedding modals are nearly identical (lines 2363-2387 and 2454-2477), violating the DRY principle.

Consider creating a reusable component:

interface AuthenticationSectionProps {
  id: string;
  useAuth: boolean;
  authToken: string;
  onUseAuthChange: (checked: boolean) => void;
  onTokenChange: (token: string) => void;
  accentColor?: 'green' | 'purple';
}

const AuthenticationSection: React.FC<AuthenticationSectionProps> = ({
  id,
  useAuth,
  authToken,
  onUseAuthChange,
  onTokenChange,
  accentColor = 'green'
}) => (
  <div className="space-y-2 mt-4">
    <div className="flex items-center gap-2">
      <input
        type="checkbox"
        id={id}
        checked={useAuth}
        onChange={(e) => onUseAuthChange(e.target.checked)}
        className={`w-4 h-4 text-${accentColor}-600 ...`}
      />
      <label htmlFor={id} className="text-sm font-medium text-gray-700 dark:text-gray-300">
        Use Authentication
      </label>
    </div>
    {useAuth && (
      <Input
        type="password"
        placeholder="Auth Token"
        value={authToken}
        onChange={(e) => onTokenChange(e.target.value)}
        className="text-sm"
      />
    )}
  </div>
);

Then use it in both modals to reduce duplication.

Also applies to: 2454-2477

PRPs/todo.md (2)

1-107: Clarify document purpose and placement.

This appears to be a retrospective CodeRabbit review tracking document. Please clarify: is this intended to be a committed artifact in the repository, or should it be maintained elsewhere (e.g., PR description, internal wiki)? If it should remain in the repo, add a brief header explaining its purpose and intended audience.

---

99-99: Use a heading instead of emphasis.

Line 99 uses bold text (**Gesamt: 7 actionable Items**) where a markdown heading would be more appropriate for document structure.

- **Gesamt: 7 actionable Items**
+ ## Gesamt: 7 actionable Items

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

python/tests/test_ollama_auth_token.py (1)

170-180: Same patch target issue as above.

This test has the same patching concern.

archon-ui-main/src/components/settings/RAGSettings.tsx (3)

142-176: Add missing Ollama auth/API fields to RAGSettingsProps and remove as any casts

ragSettings is accessed with properties that are not declared on its type:

• OLLAMA_CHAT_AUTH_TOKEN (lines 231, 233–235, 243, 249)
• OLLAMA_EMBEDDING_AUTH_TOKEN (lines 254, 256–258, 266, 272)
• OLLAMA_API_MODE (lines 1605–1607, 1613–1618, 1631–1633, 1638–1643)

This forces usage of (ragSettings as any) and violates the strict-typing guideline. It also makes it easy to accidentally break these flows without a type error.

Define the fields on ragSettings and then remove the as any casts and expression dependencies:

 interface RAGSettingsProps {
   ragSettings: {
     MODEL_CHOICE: string;
     CHAT_MODEL?: string;
+    OLLAMA_CHAT_AUTH_TOKEN?: string;
+    OLLAMA_EMBEDDING_AUTH_TOKEN?: string;
+    OLLAMA_API_MODE?: 'native' | 'openai-compatible';
     USE_CONTEXTUAL_EMBEDDINGS: boolean;
@@
-  const lastLLMConfigRef = useRef({ url: '', name: '', authToken: '' });
-  const lastEmbeddingConfigRef = useRef({ url: '', name: '', authToken: '' });
+  const lastLLMConfigRef = useRef({ url: '', name: '', authToken: '' });
+  const lastEmbeddingConfigRef = useRef({ url: '', name: '', authToken: '' });
@@
-    const newAuthToken = (ragSettings as any).OLLAMA_CHAT_AUTH_TOKEN || '';
+    const newAuthToken = ragSettings.OLLAMA_CHAT_AUTH_TOKEN || '';
@@
-  }, [ragSettings.LLM_BASE_URL, ragSettings.LLM_INSTANCE_NAME, (ragSettings as any).OLLAMA_CHAT_AUTH_TOKEN]);
+  }, [ragSettings.LLM_BASE_URL, ragSettings.LLM_INSTANCE_NAME, ragSettings.OLLAMA_CHAT_AUTH_TOKEN]);
@@
-    const newAuthToken = (ragSettings as any).OLLAMA_EMBEDDING_AUTH_TOKEN || '';
+    const newAuthToken = ragSettings.OLLAMA_EMBEDDING_AUTH_TOKEN || '';
@@
-  }, [ragSettings.OLLAMA_EMBEDDING_URL, ragSettings.OLLAMA_EMBEDDING_INSTANCE_NAME, (ragSettings as any).OLLAMA_EMBEDDING_AUTH_TOKEN]);
+  }, [ragSettings.OLLAMA_EMBEDDING_URL, ragSettings.OLLAMA_EMBEDDING_INSTANCE_NAME, ragSettings.OLLAMA_EMBEDDING_AUTH_TOKEN]);

This keeps the Ollama auth/API-mode surface properly typed and removes the need for any.

Also applies to: 225-227, 231-235, 243-244, 249-249, 254-258, 266-272, 1596-1653

---

682-692: Remove hardcoded default URL guard so models load for the default instance

The auto-load effects currently skip model loading for the default Ollama URL:

if (llmInstanceConfig.url && llmInstanceConfig.url !== 'http://host.docker.internal:11434/v1') {
  loadAvailableModels(llmInstanceConfig.url, 'chat');
}

Users running the default instance will not see available models auto-populated until they change the URL, which is confusing.

You can safely load models for any non-empty URL:

-  useEffect(() => {
-    if (llmInstanceConfig.url && llmInstanceConfig.url !== 'http://host.docker.internal:11434/v1') {
-      loadAvailableModels(llmInstanceConfig.url, 'chat');
-    }
-  }, [llmInstanceConfig.url]);
+  useEffect(() => {
+    if (llmInstanceConfig.url?.trim()) {
+      void loadAvailableModels(llmInstanceConfig.url, 'chat');
+    }
+  }, [llmInstanceConfig.url]);
@@
-  useEffect(() => {
-    if (embeddingInstanceConfig.url && embeddingInstanceConfig.url !== 'http://host.docker.internal:11434/v1') {
-      loadAvailableModels(embeddingInstanceConfig.url, 'embedding');
-    }
-  }, [embeddingInstanceConfig.url]);
+  useEffect(() => {
+    if (embeddingInstanceConfig.url?.trim()) {
+      void loadAvailableModels(embeddingInstanceConfig.url, 'embedding');
+    }
+  }, [embeddingInstanceConfig.url]);

This aligns with earlier feedback and improves the out-of-the-box experience.

---

2360-2383: Validate auth tokens before saving when useAuth is enabled

Right now, when useAuth is checked but the token is empty or whitespace, the Save handlers still persist an empty token:

• LLM modal Save (lines 2395–2401)
• Embedding modal Save (lines 2485–2490)

This leads to a configuration that looks authenticated in the UI but always fails at runtime, without immediate feedback.

Add a simple validation gate before building updatedSettings in both handlers, e.g.:

// LLM modal Save
 const Button
   onClick={async () => {
-    const updatedSettings = {
-      ...ragSettings,
-      LLM_BASE_URL: llmInstanceConfig.url,
-      OLLAMA_CHAT_AUTH_TOKEN: llmInstanceConfig.useAuth ? llmInstanceConfig.authToken : ''
-    };
+    if (llmInstanceConfig.useAuth && !llmInstanceConfig.authToken.trim()) {
+      showToast('Authentication token is required when authentication is enabled', 'error');
+      return;
+    }
+
+    const updatedSettings = {
+      ...ragSettings,
+      LLM_BASE_URL: llmInstanceConfig.url,
+      OLLAMA_CHAT_AUTH_TOKEN: llmInstanceConfig.useAuth ? llmInstanceConfig.authToken.trim() : ''
+    };
@@

// Embedding modal Save
-    const updatedSettings = {
-      ...ragSettings,
-      OLLAMA_EMBEDDING_URL: embeddingInstanceConfig.url,
-      OLLAMA_EMBEDDING_AUTH_TOKEN: embeddingInstanceConfig.useAuth ? embeddingInstanceConfig.authToken : ''
-    };
+    if (embeddingInstanceConfig.useAuth && !embeddingInstanceConfig.authToken.trim()) {
+      showToast('Authentication token is required when authentication is enabled', 'error');
+      return;
+    }
+
+    const updatedSettings = {
+      ...ragSettings,
+      OLLAMA_EMBEDDING_URL: embeddingInstanceConfig.url,
+      OLLAMA_EMBEDDING_AUTH_TOKEN: embeddingInstanceConfig.useAuth ? embeddingInstanceConfig.authToken.trim() : ''
+    };

This prevents “silent failure” configs and gives users actionable feedback.

Also applies to: 2395-2402, 2450-2473, 2485-2491

python/tests/test_ollama_auth_token.py (2)

30-40: Unused fixture: mock_credential_service

This fixture is defined but never used in any of the tests in this file. Either remove it or add tests that utilize credential service mocking for more comprehensive coverage.

-    @pytest.fixture
-    def mock_credential_service(self):
-        """Mock credential service with auth token settings."""
-        mock_service = MagicMock()
-        mock_service.get_credentials_by_category = AsyncMock(return_value={
-            "LLM_BASE_URL": "http://localhost:11434",
-            "OLLAMA_EMBEDDING_URL": "http://localhost:11434",
-            "OLLAMA_CHAT_AUTH_TOKEN": "test-chat-token",
-            "OLLAMA_EMBEDDING_AUTH_TOKEN": "test-embedding-token",
-        })
-        return mock_service
-

---

62-64: Consider a clearer assertion for auth_token verification.

The or logic works but makes debugging harder when tests fail. Consider checking which argument style was actually used:

-            # Check that auth_token was passed as keyword argument
-            assert call_args.kwargs.get("auth_token") == "test-auth-token" or \
-                   (len(call_args.args) > 1 and call_args.args[1] == "test-auth-token")
+            # Check that auth_token was passed (either as kwarg or positional)
+            auth_token_in_kwargs = call_args.kwargs.get("auth_token") == "test-auth-token"
+            auth_token_in_args = len(call_args.args) > 1 and call_args.args[1] == "test-auth-token"
+            assert auth_token_in_kwargs or auth_token_in_args, (
+                f"auth_token not found in call. kwargs={call_args.kwargs}, args={call_args.args}"
+            )

archon-ui-main/src/components/settings/RAGSettings.tsx (1)

212-221: Optionally DRY up the default Ollama URL usage

The default Ollama URL literal 'http://host.docker.internal:11434/v1' appears in several places despite the presence of DEFAULT_OLLAMA_URL:

• Initial llmInstanceConfig and embeddingInstanceConfig (lines 212, 218)
• Auto-load effects (to be refactored per earlier comment)
• Page-load testing fallbacks (lines 1253–1263, 1285–1293)

To keep things consistent and easier to change later, you could centralize on DEFAULT_OLLAMA_URL:

const [llmInstanceConfig, setLLMInstanceConfig] = useState({
  name: '',
-  url: ragSettings.LLM_BASE_URL || 'http://host.docker.internal:11434/v1',
+  url: ragSettings.LLM_BASE_URL || DEFAULT_OLLAMA_URL,
  useAuth: false,
  authToken: ''
});
@@
const [embeddingInstanceConfig, setEmbeddingInstanceConfig] = useState({
  name: '',
-  url: ragSettings.OLLAMA_EMBEDDING_URL || 'http://host.docker.internal:11434/v1',
+  url: ragSettings.OLLAMA_EMBEDDING_URL || DEFAULT_OLLAMA_URL,
  useAuth: false,
  authToken: ''
});

Not critical, but it avoids subtle drift if the default host ever needs to change.

Also applies to: 682-692, 1253-1263, 1285-1293

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: zaksnet
Repo: coleam00/Archon PR: 473
File: python/src/providers_clean/api/routes/post_migrate_model_config.py:7-9
Timestamp: 2025-09-10T06:45:10.853Z
Learning: User zaksnet prefers to handle authentication/authorization in separate PRs rather than mixing security infrastructure with feature development.

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: python/src/agent_work_orders/CLAUDE.md:0-0
Timestamp: 2025-11-29T19:25:01.813Z
Learning: Applies to python/src/agent_work_orders/**/*.py : Do not log sensitive data such as passwords, API keys, or tokens; mask them using format like `api_key[:8] + "..."`

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T19:25:23.987Z
Learning: Applies to archon-ui-main/**/*.test.{ts,tsx} : Use Vitest with React Testing Library for frontend testing

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T19:24:36.223Z
Learning: Applies to archon-ui-main/src/**/*.{test,spec}.{ts,tsx} : Use Vitest with React Testing Library for frontend testing

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T19:24:36.223Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Use TypeScript in strict mode with no implicit any in frontend code

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T19:25:23.987Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : TypeScript strict mode enabled with no implicit any, and use Biome with 120 character lines and double quotes in /src/features directory

Learnt from: Chillbruhhh
Repo: coleam00/Archon PR: 736
File: archon-ui-main/src/components/settings/RAGSettings.tsx:17-19
Timestamp: 2025-09-25T16:36:48.527Z
Learning: OpenRouter does not provide embedding models - it only acts as a proxy/router for various LLM providers for chat completions, not embeddings. The EMBEDDING_CAPABLE_PROVIDERS array in RAGSettings.tsx correctly excludes 'openrouter'.

python/src/server/services/llm_provider_service.py (1)

1211-1211: Type hint uses any instead of Any (previously flagged).

This issue was already identified in a previous review. The return type annotation uses lowercase any (the built-in function) instead of Any from the typing module, which will cause MyPy type checking issues.

Apply the previously suggested fix:

-async def validate_provider_instance(provider: str, instance_url: str | None = None, auth_token: str | None = None) -> dict[str, any]:
+async def validate_provider_instance(provider: str, instance_url: str | None = None, auth_token: str | None = None) -> dict[str, Any]:

Ensure Any is imported at the top of the file (it's already imported on line 11).

archon-ui-main/src/components/settings/RAGSettings.tsx (4)

142-175: Type Ollama auth/API-mode fields in RAGSettingsProps and drop (ragSettings as any) casts

You’re now reading/writing ragSettings.OLLAMA_CHAT_AUTH_TOKEN, ragSettings.OLLAMA_EMBEDDING_AUTH_TOKEN, and ragSettings.OLLAMA_API_MODE, but:

• These keys are not declared in RAGSettingsProps['ragSettings'].
• The effects rely on (ragSettings as any).OLLAMA_*_AUTH_TOKEN both in the body and dependency arrays, which breaks strict typing and defeats the “no implicit any” goal.

This is essentially the same issue the previous review already flagged for the auth tokens. Suggest tightening the types and removing the any casts:

 interface RAGSettingsProps {
   ragSettings: {
     MODEL_CHOICE: string;
     CHAT_MODEL?: string;
@@
     EMBEDDING_MODEL?: string;
     EMBEDDING_PROVIDER?: string;
     OLLAMA_EMBEDDING_URL?: string;
     OLLAMA_EMBEDDING_INSTANCE_NAME?: string;
+    OLLAMA_CHAT_AUTH_TOKEN?: string;
+    OLLAMA_EMBEDDING_AUTH_TOKEN?: string;
+    OLLAMA_API_MODE?: 'native' | 'openai-compatible';
@@
   };
   setRagSettings: (settings: any) => void;
 }
@@
-  const lastLLMConfigRef = useRef({ url: '', name: '', authToken: '' });
-  const lastEmbeddingConfigRef = useRef({ url: '', name: '', authToken: '' });
+  const lastLLMConfigRef = useRef<{ url: string; name: string; authToken: string }>({
+    url: '',
+    name: '',
+    authToken: '',
+  });
+  const lastEmbeddingConfigRef = useRef<{ url: string; name: string; authToken: string }>({
+    url: '',
+    name: '',
+    authToken: '',
+  });
@@
   useEffect(() => {
     const newLLMUrl = ragSettings.LLM_BASE_URL || '';
     const newLLMName = ragSettings.LLM_INSTANCE_NAME || '';
-    const newAuthToken = (ragSettings as any).OLLAMA_CHAT_AUTH_TOKEN || '';
+    const newAuthToken = ragSettings.OLLAMA_CHAT_AUTH_TOKEN || '';
@@
-  }, [ragSettings.LLM_BASE_URL, ragSettings.LLM_INSTANCE_NAME, (ragSettings as any).OLLAMA_CHAT_AUTH_TOKEN]);
+  }, [ragSettings.LLM_BASE_URL, ragSettings.LLM_INSTANCE_NAME, ragSettings.OLLAMA_CHAT_AUTH_TOKEN]);
@@
   useEffect(() => {
     const newEmbeddingUrl = ragSettings.OLLAMA_EMBEDDING_URL || '';
     const newEmbeddingName = ragSettings.OLLAMA_EMBEDDING_INSTANCE_NAME || '';
-    const newAuthToken = (ragSettings as any).OLLAMA_EMBEDDING_AUTH_TOKEN || '';
+    const newAuthToken = ragSettings.OLLAMA_EMBEDDING_AUTH_TOKEN || '';
@@
-  }, [ragSettings.OLLAMA_EMBEDDING_URL, ragSettings.OLLAMA_EMBEDDING_INSTANCE_NAME, (ragSettings as any).OLLAMA_EMBEDDING_AUTH_TOKEN]);
+  }, [ragSettings.OLLAMA_EMBEDDING_URL, ragSettings.OLLAMA_EMBEDDING_INSTANCE_NAME, ragSettings.OLLAMA_EMBEDDING_AUTH_TOKEN]);

This keeps the state-sync logic the same but restores type safety and satisfies strict TS in this file.

Also applies to: 225-227, 231-244, 249-272

---

641-679: Auto-load Ollama models should not special‑case the default URL

loadAvailableModels itself looks good, but the auto-load effects still skip loading when the URL equals the hardcoded default 'http://host.docker.internal:11434/v1'. That means users running a working default Ollama instance won’t see any models until they tweak the URL.

You already have DEFAULT_OLLAMA_URL; I’d drop the special-case and only require a non-empty, trimmed URL:

  // Auto-load available models when URLs are configured
  useEffect(() => {
-    if (llmInstanceConfig.url && llmInstanceConfig.url !== 'http://host.docker.internal:11434/v1') {
-      loadAvailableModels(llmInstanceConfig.url, 'chat');
+    if (llmInstanceConfig.url && llmInstanceConfig.url.trim()) {
+      void loadAvailableModels(llmInstanceConfig.url, 'chat');
     }
   }, [llmInstanceConfig.url]);

   useEffect(() => {
-    if (embeddingInstanceConfig.url && embeddingInstanceConfig.url !== 'http://host.docker.internal:11434/v1') {
-      loadAvailableModels(embeddingInstanceConfig.url, 'embedding');
+    if (embeddingInstanceConfig.url && embeddingInstanceConfig.url.trim()) {
+      void loadAvailableModels(embeddingInstanceConfig.url, 'embedding');
     }
   }, [embeddingInstanceConfig.url]);

This also aligns with the earlier review suggestion to avoid hardcoding that URL.

Also applies to: 682-692

---

787-807: Keep instance config shape consistent and clear auth fields when deleting instances

llmInstanceConfig / embeddingInstanceConfig now include useAuth and authToken, but some paths still construct configs with { name, url } only and deletion doesn’t clear the persisted auth fields:

• handleDeleteLLMInstance and handleDeleteEmbeddingInstance reset state to { name: '', url: '' }, dropping useAuth and authToken.
• Deletion only deletes LLM_BASE_URL, MODEL_CHOICE, OLLAMA_EMBEDDING_URL, and EMBEDDING_MODEL from ragSettings; instance names and auth tokens remain.
• The “auto-populate embedding instance” block on LLM URL change also only sets { name, url }, ignoring auth flags, while you already have syncEmbeddingFromLLM for shape-safe syncing.

This can lead to inconsistent state shape and “ghost” tokens surviving after delete. Consider:

  const handleDeleteLLMInstance = () => {
@@
-      setLLMInstanceConfig({
-        name: '',
-        url: ''
-      });
+      setLLMInstanceConfig({
+        name: '',
+        url: '',
+        useAuth: false,
+        authToken: ''
+      });
@@
       const updatedSettings = { ...ragSettings };
       delete updatedSettings.LLM_BASE_URL;
       delete updatedSettings.MODEL_CHOICE;
+      delete updatedSettings.LLM_INSTANCE_NAME;
+      delete updatedSettings.OLLAMA_CHAT_AUTH_TOKEN;
@@
  const handleDeleteEmbeddingInstance = () => {
@@
-      setEmbeddingInstanceConfig({
-        name: '',
-        url: ''
-      });
+      setEmbeddingInstanceConfig({
+        name: '',
+        url: '',
+        useAuth: false,
+        authToken: ''
+      });
@@
       const updatedSettings = { ...ragSettings };
       delete updatedSettings.OLLAMA_EMBEDDING_URL;
       delete updatedSettings.EMBEDDING_MODEL;
+      delete updatedSettings.OLLAMA_EMBEDDING_INSTANCE_NAME;
+      delete updatedSettings.OLLAMA_EMBEDDING_AUTH_TOKEN;
@@
-                    // Auto-populate embedding instance if it's empty (convenience for single-host users)
-                    if (!embeddingInstanceConfig.url || !embeddingInstanceConfig.name) {
-                      setEmbeddingInstanceConfig({
-                        name: llmInstanceConfig.name || 'Default Ollama',
-                        url: newUrl
-                      });
-                    }
+                    // Auto-populate embedding instance if it's empty (convenience for single-host users)
+                    if (!embeddingInstanceConfig.url || !embeddingInstanceConfig.name) {
+                      // Reuse shared helper to keep shape (including auth flags) consistent
+                      setEmbeddingInstanceConfig(syncEmbeddingFromLLM({ ...llmInstanceConfig, url: newUrl }));
+                    }
@@
-                        // Sync embedding instance with LLM instance (including auth settings)
-                        setEmbeddingInstanceConfig(syncEmbeddingFromLLM(llmInstanceConfig));
+                        // Sync embedding instance with LLM instance (including auth settings)
+                        setEmbeddingInstanceConfig(syncEmbeddingFromLLM(llmInstanceConfig));

This keeps config state structurally consistent and ensures deleting an instance actually clears its auth settings as well.
[highlight: this overlaps with the earlier review about instance config shape.]

Also applies to: 809-829, 2331-2335, 2349-2351

---

2360-2383: Validate auth tokens before saving when Use Authentication is enabled

The LLM and Embedding edit modals now let users toggle “Use Authentication” and enter tokens, and you persist them into OLLAMA_CHAT_AUTH_TOKEN / OLLAMA_EMBEDDING_AUTH_TOKEN. However, if useAuth is true and the token input is empty/whitespace, you still write an empty token to ragSettings. This matches an earlier review concern and can lead to confusing failures when the backend attempts authenticated calls with an empty header.

Add a small guard before constructing updatedSettings in both modals:

                <Button
                  onClick={async () => {
+                   if (llmInstanceConfig.useAuth && !llmInstanceConfig.authToken.trim()) {
+                     showToast('Authentication token is required when authentication is enabled', 'error');
+                     return;
+                   }
                     const updatedSettings = {
                       ...ragSettings,
                       LLM_BASE_URL: llmInstanceConfig.url,
                       OLLAMA_CHAT_AUTH_TOKEN: llmInstanceConfig.useAuth ? llmInstanceConfig.authToken : ''
                     };
@@
                <Button
                  onClick={async () => {
+                   if (embeddingInstanceConfig.useAuth && !embeddingInstanceConfig.authToken.trim()) {
+                     showToast('Authentication token is required when authentication is enabled', 'error');
+                     return;
+                   }
                     const updatedSettings = {
                       ...ragSettings,
                       OLLAMA_EMBEDDING_URL: embeddingInstanceConfig.url,
                       OLLAMA_EMBEDDING_AUTH_TOKEN: embeddingInstanceConfig.useAuth ? embeddingInstanceConfig.authToken : ''
                     };

This keeps the UX tight and avoids persisting obviously invalid credentials.

Also applies to: 2395-2401, 2485-2491

python/src/server/services/llm_provider_service.py (2)

415-441: Consider extracting token resolution logic to reduce duplication.

The token resolution pattern here is duplicated at lines 455-480. Both segments fetch RAG settings, select the appropriate token (embedding vs chat), and apply the same "required-but-ignored" fallback logic.

Consider extracting this into a helper function:

+async def _resolve_ollama_auth_token(use_embedding_provider: bool, instance_type: str | None = None) -> str:
+    """Resolve Ollama auth token based on operation type."""
+    cache_key = "rag_strategy_settings"
+    rag_settings = _get_cached_settings(cache_key)
+    if rag_settings is None:
+        rag_settings = await credential_service.get_credentials_by_category("rag_strategy")
+        _set_cached_settings(cache_key, rag_settings)
+    
+    # Select token based on operation type
+    if use_embedding_provider or instance_type == "embedding":
+        token = rag_settings.get("OLLAMA_EMBEDDING_AUTH_TOKEN", "")
+        logger.info(f"Using OLLAMA_EMBEDDING_AUTH_TOKEN (length: {len(token) if token else 0})")
+    else:
+        token = rag_settings.get("OLLAMA_CHAT_AUTH_TOKEN", "")
+        logger.info(f"Using OLLAMA_CHAT_AUTH_TOKEN (length: {len(token) if token else 0})")
+    
+    # Use placeholder if no token is set
+    if not token:
+        token = "required-but-ignored"
+        logger.warning("No Ollama auth token found, using placeholder 'required-but-ignored'")
+    
+    return token

Then simplify both call sites:

-                    # Check for auth token in RAG settings for fallback
-                    cache_key = "rag_strategy_settings"
-                    rag_settings = _get_cached_settings(cache_key)
-                    if rag_settings is None:
-                        rag_settings = await credential_service.get_credentials_by_category("rag_strategy")
-                        _set_cached_settings(cache_key, rag_settings)
-
-                    # Get correct auth token based on operation type
-                    if use_embedding_provider:
-                        ollama_auth_token = rag_settings.get("OLLAMA_EMBEDDING_AUTH_TOKEN", "")
-                    else:
-                        ollama_auth_token = rag_settings.get("OLLAMA_CHAT_AUTH_TOKEN", "")
-
-                    # Use "required-but-ignored" as default if no token is set (Ollama doesn't validate when auth is disabled)
-                    if not ollama_auth_token:
-                        ollama_auth_token = "required-but-ignored"
+                    ollama_auth_token = await _resolve_ollama_auth_token(use_embedding_provider)

---

618-622: Consider consolidating the logging statements.

Lines 618 and 621 log very similar information about the embedding URL resolution. This could be simplified to a single log statement for clarity.

             embedding_url = rag_settings.get("OLLAMA_EMBEDDING_URL")
-            logger.info(f"Embedding URL from settings: {embedding_url}")
             if embedding_url:
                 final_url = embedding_url if embedding_url.endswith('/v1') else f"{embedding_url}/v1"
-                logger.info(f"Resolved embedding URL: {final_url}")
+                logger.info(f"Resolved embedding URL: {embedding_url} → {final_url}")
                 return final_url

archon-ui-main/src/components/settings/RAGSettings.tsx (2)

572-584: Ollama per-instance metrics + available model lists look consistent

The extended ollamaMetrics shape (per-instance counts for both LLM and embedding plus llmAvailableModels / embeddingAvailableModels) and the corresponding updates in fetchOllamaMetrics are internally consistent and line up with how you’re rendering:

• Summary table rows use embeddingInstanceModels safely.
• The “Available on this instance” row pulls counts by context, which matches the per-instance split.

If you don’t end up wiring llmAvailableModels / embeddingAvailableModels into the selection modals, you could later trim those from state to reduce surface area, but as-is this structure is fine and future‑proof.

Also applies to: 885-903

---

731-740: Toast model counts in manualTestConnection may be stale vs per-call instance status

Inside manualTestConnection, the success toast uses ollamaMetrics.llmInstanceModels / embeddingInstanceModels to derive modelCount, but those metrics are only refreshed later via fetchOllamaMetrics() and may still be at their initial or previous values. This can yield messages like “0 chat models available” even when the instance has models and the backend returned a model count.

Since instanceStatus from the health endpoint already contains models_available, consider basing the toast count on that and only using the metrics for more detailed UIs:

-          // Context-aware model count display
-          let modelCount = instanceStatus.models_available || 0;
-          let modelType = 'models';
-
-          if (context === 'chat') {
-            modelCount = ollamaMetrics.llmInstanceModels?.chat || 0;
-            modelType = 'chat models';
-          } else if (context === 'embedding') {
-            modelCount = ollamaMetrics.embeddingInstanceModels?.embedding || 0;
-            modelType = 'embedding models';
-          }
+          // Context-aware model count display
+          let modelCount = instanceStatus.models_available || 0;
+          let modelType = 'models';
+          if (context === 'chat') {
+            modelType = 'chat models';
+          } else if (context === 'embedding') {
+            modelType = 'embedding models';
+          }
@@
-            showToast(`${instanceName} connection successful: ${modelCount} ${modelType} available (${responseTime}ms)`, 'success');
+            showToast(
+              `${instanceName} connection successful: ${modelCount} ${modelType} available (${responseTime}ms)`,
+              'success'
+            );
@@
-          // Load available models after successful health check
-          if (context) {
-            await loadAvailableModels(url, context);
-          }
+          // Load available models after successful health check
+          if (context) {
+            await loadAvailableModels(url, context);
+          }

This keeps the metrics fetch for richer views while making the immediate toast reflect the actual response.

Also applies to: 752-755

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T19:24:36.223Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Use TypeScript in strict mode with no implicit any in frontend code

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T19:25:23.987Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : TypeScript strict mode enabled with no implicit any, and use Biome with 120 character lines and double quotes in /src/features directory

Learnt from: Chillbruhhh
Repo: coleam00/Archon PR: 736
File: archon-ui-main/src/components/settings/RAGSettings.tsx:17-19
Timestamp: 2025-09-25T16:36:48.527Z
Learning: OpenRouter does not provide embedding models - it only acts as a proxy/router for various LLM providers for chat completions, not embeddings. The EMBEDDING_CAPABLE_PROVIDERS array in RAGSettings.tsx correctly excludes 'openrouter'.

archon-ui-main/src/components/settings/RAGSettings.tsx (3)

142-176: Add Ollama auth/API mode fields to RAGSettingsProps and remove as any casts

The component now reads/writes OLLAMA_CHAT_AUTH_TOKEN, OLLAMA_EMBEDDING_AUTH_TOKEN, and OLLAMA_API_MODE via ragSettings, but these keys are not declared on RAGSettingsProps["ragSettings"]. Accessing them via (ragSettings as any) both weakens type safety (violating strict mode) and makes it easy to desync UI from persisted settings. This also means ragSettings.OLLAMA_API_MODE is currently a type error.

You can fix this by extending the interface and wiring the effects to the typed fields, eliminating the as any usage:

 interface RAGSettingsProps {
   ragSettings: {
     MODEL_CHOICE: string;
-    CHAT_MODEL?: string;
+    CHAT_MODEL?: string;
     USE_CONTEXTUAL_EMBEDDINGS: boolean;
@@
     EMBEDDING_MODEL?: string;
     EMBEDDING_PROVIDER?: string;
     OLLAMA_EMBEDDING_URL?: string;
     OLLAMA_EMBEDDING_INSTANCE_NAME?: string;
+    OLLAMA_CHAT_AUTH_TOKEN?: string;
+    OLLAMA_EMBEDDING_AUTH_TOKEN?: string;
+    OLLAMA_API_MODE?: 'native' | 'openai-compatible';
@@
   };
   setRagSettings: (settings: any) => void;
 }
@@
   useEffect(() => {
     const newLLMUrl = ragSettings.LLM_BASE_URL || '';
     const newLLMName = ragSettings.LLM_INSTANCE_NAME || '';
-    const newAuthToken = (ragSettings as any).OLLAMA_CHAT_AUTH_TOKEN || '';
+    const newAuthToken = ragSettings.OLLAMA_CHAT_AUTH_TOKEN || '';
@@
-  }, [ragSettings.LLM_BASE_URL, ragSettings.LLM_INSTANCE_NAME, (ragSettings as any).OLLAMA_CHAT_AUTH_TOKEN]);
+  }, [ragSettings.LLM_BASE_URL, ragSettings.LLM_INSTANCE_NAME, ragSettings.OLLAMA_CHAT_AUTH_TOKEN]);
@@
   useEffect(() => {
     const newEmbeddingUrl = ragSettings.OLLAMA_EMBEDDING_URL || '';
     const newEmbeddingName = ragSettings.OLLAMA_EMBEDDING_INSTANCE_NAME || '';
-    const newAuthToken = (ragSettings as any).OLLAMA_EMBEDDING_AUTH_TOKEN || '';
+    const newAuthToken = ragSettings.OLLAMA_EMBEDDING_AUTH_TOKEN || '';
@@
-  }, [ragSettings.OLLAMA_EMBEDDING_URL, ragSettings.OLLAMA_EMBEDDING_INSTANCE_NAME, (ragSettings as any).OLLAMA_EMBEDDING_AUTH_TOKEN]);
+  }, [ragSettings.OLLAMA_EMBEDDING_URL, ragSettings.OLLAMA_EMBEDDING_INSTANCE_NAME, ragSettings.OLLAMA_EMBEDDING_AUTH_TOKEN]);

This keeps the props contract honest and satisfies strict TypeScript without resorting to any.

Also applies to: 223-227, 228-249, 251-272, 1600-1659, 2404-2410, 2493-2499

---

641-693: Auto‑loading available models still skips the default Ollama URL

loadAvailableModels is a nice addition and the metrics wiring (llmAvailableModels / embeddingAvailableModels) looks good, but the auto‑load effects still short‑circuit when the URL equals the default http://host.docker.internal:11434/v1. This means users running Ollama on the default host won’t see model lists until they manually tweak the URL or hit “Test Connection”.

You can address this and simplify the condition by loading models whenever a non‑empty URL is present:

  // Auto-load available models when URLs are configured
  useEffect(() => {
-    if (llmInstanceConfig.url && llmInstanceConfig.url !== 'http://host.docker.internal:11434/v1') {
-      loadAvailableModels(llmInstanceConfig.url, 'chat');
-    }
+    if (llmInstanceConfig.url && llmInstanceConfig.url.trim()) {
+      void loadAvailableModels(llmInstanceConfig.url, 'chat');
+    }
   }, [llmInstanceConfig.url]);

   useEffect(() => {
-    if (embeddingInstanceConfig.url && embeddingInstanceConfig.url !== 'http://host.docker.internal:11434/v1') {
-      loadAvailableModels(embeddingInstanceConfig.url, 'embedding');
-    }
+    if (embeddingInstanceConfig.url && embeddingInstanceConfig.url.trim()) {
+      void loadAvailableModels(embeddingInstanceConfig.url, 'embedding');
+    }
   }, [embeddingInstanceConfig.url]);

This preserves the debounce behavior (hook still keys off URL changes) and makes the default local instance behave like any other.

Also applies to: 752-755, 572-584, 891-909

---

2368-2391: Require non‑empty auth tokens when authentication is enabled for LLM/embedding instances

The new auth controls in both edit modals correctly wire useAuth/authToken into local state and persist OLLAMA_CHAT_AUTH_TOKEN / OLLAMA_EMBEDDING_AUTH_TOKEN. However, if a user enables “Use Authentication” but leaves the token empty, the code still proceeds to “save” and pushes an empty string into ragSettings (effectively disabling auth) without any feedback.

This is the same edge case highlighted in the prior review; adding lightweight validation before constructing updatedSettings will prevent confusing failures:

// LLM instance modal save
-                <Button
-                  onClick={async () => {
-                    const updatedSettings = {
+                <Button
+                  onClick={async () => {
+                    if (llmInstanceConfig.useAuth && !llmInstanceConfig.authToken.trim()) {
+                      showToast('Authentication token is required when authentication is enabled', 'error');
+                      return;
+                    }
+                    const updatedSettings = {
                       ...ragSettings,
                       LLM_BASE_URL: llmInstanceConfig.url,
                       OLLAMA_CHAT_AUTH_TOKEN: llmInstanceConfig.useAuth ? llmInstanceConfig.authToken : ''
                     };
                     setRagSettings(updatedSettings);
@@
// Embedding instance modal save
-                <Button
-                  onClick={async () => {
-                    const updatedSettings = {
+                <Button
+                  onClick={async () => {
+                    if (embeddingInstanceConfig.useAuth && !embeddingInstanceConfig.authToken.trim()) {
+                      showToast('Authentication token is required when authentication is enabled', 'error');
+                      return;
+                    }
+                    const updatedSettings = {
                       ...ragSettings,
                       OLLAMA_EMBEDDING_URL: embeddingInstanceConfig.url,
                       OLLAMA_EMBEDDING_AUTH_TOKEN: embeddingInstanceConfig.useAuth ? embeddingInstanceConfig.authToken : ''
                     };
                     setRagSettings(updatedSettings);

That keeps the UX aligned with the toggles: “Use Authentication” can’t be turned on with an empty token.

Also applies to: 2403-2432, 2458-2481, 2493-2521

archon-ui-main/src/components/bug-report/BugReportModal.tsx (1)

230-235: Consider removing the as any type assertion.

Line 233 uses as any which bypasses TypeScript's type checking. The severity field should have a proper union type matching the dropdown options.

As per coding guidelines, TypeScript strict mode with no implicit any should be enforced.

You can define a proper type for severity and use it instead:

type BugSeverity = "low" | "medium" | "high" | "critical";

Then update the cast to:

 onChange={(e) =>
   setReport((r) => ({
     ...r,
-    severity: e.target.value as any,
+    severity: e.target.value as BugSeverity,
   }))
 }

python/src/server/services/llm_provider_service.py (2)

399-445: Ollama auth token resolution and fallback behavior look consistent

The new logic to:

• Resolve OLLAMA_CHAT_AUTH_TOKEN / OLLAMA_EMBEDDING_AUTH_TOKEN from cached rag_strategy settings,
• Fall back to a "required-but-ignored" placeholder when no token is configured, and
• Use that value as api_key for both the OpenAI→Ollama fallback and direct "ollama" provider paths,

is coherent and avoids logging the actual token (only lengths are logged). This should work whether auth is disabled (placeholder ignored by Ollama) or enabled (real token present).

If you find this pattern spreading, consider extracting a small helper like get_ollama_auth_token(use_embedding_provider: bool, rag_settings: dict) -> str to keep the token selection and placeholder rules in one place, but it’s not strictly necessary right now.

Also applies to: 447-481

---

1211-1235: validate_provider_instance auth_token plumbing is correct; consider richer error logs

Extending validate_provider_instance to accept auth_token and forwarding it into model_discovery_service.check_instance_health(instance_url, auth_token=auth_token) correctly enables per‑instance authenticated health checks for Ollama. The return shape and non‑Ollama branches are preserved, and the return type now uses Any (fixing the earlier any typo).

To better align with the backend logging guidelines, consider adding exc_info=True to the error log in the except block so health‑check failures preserve full stack traces:

-    except Exception as e:
-        logger.error(f"Error validating provider {provider}: {e}")
+    except Exception as e:
+        logger.error(f"Error validating provider {provider}: {e}", exc_info=True)

Also applies to: 1275-1285

archon-ui-main/src/components/settings/RAGSettings.tsx (1)

1852-1879: Align “Authentication” summary row with useAuth state, not just token presence

The “Authentication” row in the Ollama configuration summary currently reports “Token configured” purely based on whether authToken is non‑empty, ignoring useAuth. Because you intentionally keep the token around in local state even when useAuth is toggled off, this can show “Token configured” while authentication is actually disabled.

You can make the summary reflect the real effective state by incorporating useAuth:

-                      <tr>
-                        <td className="py-2 text-gray-400">Authentication</td>
-                        <td className="py-2">
-                          {activeSelection === 'chat' ? (
-                            llmInstanceConfig.authToken ? (
+                      <tr>
+                        <td className="py-2 text-gray-400">Authentication</td>
+                        <td className="py-2">
+                          {activeSelection === 'chat' ? (
+                            llmInstanceConfig.useAuth && llmInstanceConfig.authToken ? (
                               <span className="text-teal-400 flex items-center">
@@
                             ) : (
                               <span className="text-gray-500 italic">No authentication</span>
                             )
                           ) : (
-                            embeddingInstanceConfig.authToken ? (
+                            embeddingInstanceConfig.useAuth && embeddingInstanceConfig.authToken ? (
                               <span className="text-teal-400 flex items-center">
@@
                             ) : (
                               <span className="text-gray-500 italic">No authentication</span>
                             )
                           )}
                        </td>
                      </tr>

This keeps “Token configured” in sync with the toggle while still letting you cache the token client‑side for convenience.

Also applies to: 2368-2390, 2458-2480

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T19:25:23.987Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : TypeScript strict mode enabled with no implicit any, and use Biome with 120 character lines and double quotes in /src/features directory

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T19:25:23.987Z
Learning: Applies to archon-ui-main/src/features/**/components/**/*.{ts,tsx} : Use Radix UI primitives from src/features/ui/primitives/ for new UI components in features directory

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T19:24:36.223Z
Learning: Applies to archon-ui-main/src/features/**/components/**/*.{ts,tsx} : Prefer using Radix UI primitives from src/features/ui/primitives/ when creating UI components

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T19:24:36.223Z
Learning: Applies to archon-ui-main/src/!(features)/**/*.{ts,tsx} : Use ESLint with standard React rules for legacy code in the frontend

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T19:24:36.223Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx} : Use TypeScript in strict mode with no implicit any in frontend code

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T19:25:23.987Z
Learning: Applies to archon-ui-main/src/**/*.{ts,tsx,js,jsx} : Use ESLint for legacy frontend code outside /src/features directory with standard React rules

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T19:24:36.223Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Apply Tron-inspired glassmorphism styling with Tailwind and follow responsive design patterns with mobile-first approach

Learnt from: stevepresley
Repo: coleam00/Archon PR: 573
File: archon-ui-main/src/config/api.ts:15-25
Timestamp: 2025-09-04T16:30:05.227Z
Learning: Archon UI API config: Prefer lazy getters getApiFullUrl() and getWsUrl() over module-load constants to avoid SSR/test crashes. Avoid CommonJS exports patterns (Object.defineProperty(exports,…)) in ESM. Add typeof window guards with VITE_API_URL fallback inside getApiUrl()/getWebSocketUrl() when SSR safety is required.

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-29T19:25:23.987Z
Learning: Applies to archon-ui-main/src/features/**/components/**/*.{ts,tsx} : Apply Tron-inspired glassmorphism styling with Tailwind in features directory UI components

Learnt from: Wirasm
Repo: coleam00/Archon PR: 514
File: archon-ui-main/src/components/layouts/ArchonChatPanel.tsx:71-81
Timestamp: 2025-08-28T12:03:56.388Z
Learning: The agent chat service is disabled in the Archon codebase, so optimizations or fixes related to agent chat functionality may not be necessary to implement.

Learnt from: CR
Repo: coleam00/Archon PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T19:24:36.223Z
Learning: Applies to archon-ui-main/src/features/**/*.{ts,tsx} : Use TanStack Query for all data fetching with smart HTTP polling where appropriate

Learnt from: Chillbruhhh
Repo: coleam00/Archon PR: 736
File: archon-ui-main/src/components/settings/RAGSettings.tsx:17-19
Timestamp: 2025-09-25T16:36:48.527Z
Learning: OpenRouter does not provide embedding models - it only acts as a proxy/router for various LLM providers for chat completions, not embeddings. The EMBEDDING_CAPABLE_PROVIDERS array in RAGSettings.tsx correctly excludes 'openrouter'.