fix(embedding): guard Windows Bun and support trusted headers - #342
Open
coleleavitt wants to merge 3 commits into
Open
fix(embedding): guard Windows Bun and support trusted headers#342coleleavitt wants to merge 3 commits into
coleleavitt wants to merge 3 commits into
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
7 issues found across 30 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/plugin/src/features/magic-context/project-embedding-registry.ts">
<violation number="1" location="packages/plugin/src/features/magic-context/project-embedding-registry.ts:1000">
P3: `warnIfLocalUnavailable` in embedding-routing.ts adds an unthrottled warning on every `resolveEmbeddingRouting` call for Windows+Bun, whereas the `LocalEmbeddingProvider.initialize` log is gated by the `windowsBunDisabledLogged` latch. If routing is resolved repeatedly in a session, the warning is re-emitted each time, producing duplicate user-facing messages for the same permanent condition. Apply the same one-shot latch to the routing warning.</violation>
</file>
<file name="packages/cli/src/commands/doctor-opencode.ts">
<violation number="1" location="packages/cli/src/commands/doctor-opencode.ts:558">
P3: When header-only authentication returns 401/403, OpenCode doctor tells the user to check `api_key` or an environment variable even though the credential is in `embedding.headers`. Include custom headers and `Authorization` in the authentication failure guidance.</violation>
</file>
<file name="packages/cli/src/lib/embedding-runtime.ts">
<violation number="1" location="packages/cli/src/lib/embedding-runtime.ts:38">
P3: The warning string duplicates its cause: the template hardcodes "local is unavailable under Bun on Windows —" and then appends the reason, which `getLocalEmbeddingUnavailableReason` returns verbatim as "local embeddings are unavailable under Bun on Windows because onnxruntime-node can crash the host process". The rendered doctor line reads the same phrase twice. Since the reason is non-null only on win32+Bun, the hardcoded prefix is always redundant with the reason text. Use the reason alone as the sentence and only add the remediation advice.</violation>
<violation number="2" location="packages/cli/src/lib/embedding-runtime.ts:38">
P3: The doctor warning repeats itself: the hardcoded prefix "local is unavailable under Bun on Windows" is concatenated with `reason`, which is itself "local embeddings are unavailable under Bun on Windows because ...". Return just the prefix plus the actionable fix, or drop the duplicated clause from the reason, so the surfaced message reads once.</violation>
</file>
<file name="packages/plugin/src/features/magic-context/memory/embedding-openai.ts">
<violation number="1" location="packages/plugin/src/features/magic-context/memory/embedding-openai.ts:179">
P2: When a non-empty trusted header changes, `initializeEmbedding` keeps the existing provider because this identity only records header presence. Rebuild the runtime provider using a separate non-secret config fingerprint so rotated credentials or tenant headers reach `fetch`.</violation>
</file>
<file name="packages/plugin/src/config/schema/magic-context.ts">
<violation number="1" location="packages/plugin/src/config/schema/magic-context.ts:318">
P1: When a non-empty header value changes, such as rotating `Authorization`, the embedding provider keeps using the old header values because its identity records only header presence. Recreate or update the provider when header values change without including secret values in the durable identity.</violation>
</file>
<file name="packages/cli/src/commands/doctor-pi.ts">
<violation number="1" location="packages/cli/src/commands/doctor-pi.ts:744">
P2: When `embedding.provider` is `synapse` on Bun Windows, this branch emits a local-runtime warning and skips the native-runtime check even though Synapse is a separate daemon lane. Gate this warning and short-circuit on the `local` provider, then handle Synapse independently.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| .optional() | ||
| .describe("API endpoint URL. Required when provider is openai-compatible."), | ||
| api_key: z.string().optional().describe("API key for remote embedding provider (optional)"), | ||
| headers: z |
There was a problem hiding this comment.
P1: When a non-empty header value changes, such as rotating Authorization, the embedding provider keeps using the old header values because its identity records only header presence. Recreate or update the provider when header values change without including secret values in the durable identity.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/plugin/src/config/schema/magic-context.ts, line 318:
<comment>When a non-empty header value changes, such as rotating `Authorization`, the embedding provider keeps using the old header values because its identity records only header presence. Recreate or update the provider when header values change without including secret values in the durable identity.</comment>
<file context>
@@ -315,6 +315,12 @@ const BaseEmbeddingConfigSchema = z
.optional()
.describe("API endpoint URL. Required when provider is openai-compatible."),
api_key: z.string().optional().describe("API key for remote embedding provider (optional)"),
+ headers: z
+ .record(z.string().trim().min(1), z.string().min(1))
+ .optional()
</file context>
| endpoint: this.endpoint, | ||
| model: this.model, | ||
| ...(this.apiKey ? { api_key: this.apiKey } : {}), | ||
| ...(Object.keys(this.headers).length > 0 ? { headers: this.headers } : {}), |
There was a problem hiding this comment.
P2: When a non-empty trusted header changes, initializeEmbedding keeps the existing provider because this identity only records header presence. Rebuild the runtime provider using a separate non-secret config fingerprint so rotated credentials or tenant headers reach fetch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/plugin/src/features/magic-context/memory/embedding-openai.ts, line 179:
<comment>When a non-empty trusted header changes, `initializeEmbedding` keeps the existing provider because this identity only records header presence. Rebuild the runtime provider using a separate non-secret config fingerprint so rotated credentials or tenant headers reach `fetch`.</comment>
<file context>
@@ -172,6 +176,7 @@ export class OpenAICompatibleEmbeddingProvider implements EmbeddingProvider {
endpoint: this.endpoint,
model: this.model,
...(this.apiKey ? { api_key: this.apiKey } : {}),
+ ...(Object.keys(this.headers).length > 0 ? { headers: this.headers } : {}),
...(this.inputType ? { input_type: this.inputType } : {}),
// truncate participates in identity (it changes which text an
</file context>
Comment on lines
+744
to
+746
| const unavailableWarning = options.deps.getLocalEmbeddingRuntimeDoctorWarning(); | ||
| if (unavailableWarning) add(results, "warn", unavailableWarning); | ||
| let runtimeReported = unavailableWarning !== null; |
There was a problem hiding this comment.
P2: When embedding.provider is synapse on Bun Windows, this branch emits a local-runtime warning and skips the native-runtime check even though Synapse is a separate daemon lane. Gate this warning and short-circuit on the local provider, then handle Synapse independently.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/commands/doctor-pi.ts, line 744:
<comment>When `embedding.provider` is `synapse` on Bun Windows, this branch emits a local-runtime warning and skips the native-runtime check even though Synapse is a separate daemon lane. Gate this warning and short-circuit on the `local` provider, then handle Synapse independently.</comment>
<file context>
@@ -735,25 +741,29 @@ async function runHealthChecks(options: {
// throws on every embedding (#128). Layout-agnostic resolution from the
// installed plugin dir; stays silent if no plugin dir can be inspected.
- let runtimeReported = false;
+ const unavailableWarning = options.deps.getLocalEmbeddingRuntimeDoctorWarning();
+ if (unavailableWarning) add(results, "warn", unavailableWarning);
+ let runtimeReported = unavailableWarning !== null;
</file context>
Suggested change
| const unavailableWarning = options.deps.getLocalEmbeddingRuntimeDoctorWarning(); | |
| if (unavailableWarning) add(results, "warn", unavailableWarning); | |
| let runtimeReported = unavailableWarning !== null; | |
| const isLocalProvider = loadedConfig.config.embedding.provider === "local"; | |
| const unavailableWarning = isLocalProvider | |
| ? options.deps.getLocalEmbeddingRuntimeDoctorWarning() | |
| : null; | |
| if (unavailableWarning) add(results, "warn", unavailableWarning); | |
| let runtimeReported = !isLocalProvider || unavailableWarning !== null; |
| endpoint, | ||
| model, | ||
| apiKey: apiKey, | ||
| ...(headers ? { headers } : {}), |
There was a problem hiding this comment.
P3: When header-only authentication returns 401/403, OpenCode doctor tells the user to check api_key or an environment variable even though the credential is in embedding.headers. Include custom headers and Authorization in the authentication failure guidance.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/commands/doctor-opencode.ts, line 558:
<comment>When header-only authentication returns 401/403, OpenCode doctor tells the user to check `api_key` or an environment variable even though the credential is in `embedding.headers`. Include custom headers and `Authorization` in the authentication failure guidance.</comment>
<file context>
@@ -539,6 +555,7 @@ async function checkEmbeddingConfig(
endpoint,
model,
apiKey: apiKey,
+ ...(headers ? { headers } : {}),
...(inputType ? { inputType } : {}),
...(truncateMode ? { truncate: truncateMode } : {}),
</file context>
| const resolvedConfig = resolveEmbeddingConfig(config); | ||
| const providerIdentity = getEmbeddingProviderIdentity(resolvedConfig); | ||
| const runtimeFingerprint = getRuntimeFingerprint(resolvedConfig); | ||
| const unavailableReason = |
There was a problem hiding this comment.
P3: warnIfLocalUnavailable in embedding-routing.ts adds an unthrottled warning on every resolveEmbeddingRouting call for Windows+Bun, whereas the LocalEmbeddingProvider.initialize log is gated by the windowsBunDisabledLogged latch. If routing is resolved repeatedly in a session, the warning is re-emitted each time, producing duplicate user-facing messages for the same permanent condition. Apply the same one-shot latch to the routing warning.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/plugin/src/features/magic-context/project-embedding-registry.ts, line 1000:
<comment>`warnIfLocalUnavailable` in embedding-routing.ts adds an unthrottled warning on every `resolveEmbeddingRouting` call for Windows+Bun, whereas the `LocalEmbeddingProvider.initialize` log is gated by the `windowsBunDisabledLogged` latch. If routing is resolved repeatedly in a session, the warning is re-emitted each time, producing duplicate user-facing messages for the same permanent condition. Apply the same one-shot latch to the routing warning.</comment>
<file context>
@@ -982,8 +997,14 @@ export function registerProjectEmbedding(
const resolvedConfig = resolveEmbeddingConfig(config);
- const providerIdentity = getEmbeddingProviderIdentity(resolvedConfig);
- const runtimeFingerprint = getRuntimeFingerprint(resolvedConfig);
+ const unavailableReason =
+ resolvedConfig.provider === "local" ? localEmbeddingUnavailableReasonForRuntime() : null;
+ const providerIdentity = unavailableReason
</file context>
| ): string | null { | ||
| const reason = getLocalEmbeddingUnavailableReason(platform, bunHost); | ||
| return reason | ||
| ? `Embedding provider: local is unavailable under Bun on Windows — ${reason}. Configure embedding.provider=openai-compatible, or set embedding.provider=off to keep keyword search without semantic embeddings.` |
There was a problem hiding this comment.
P3: The warning string duplicates its cause: the template hardcodes "local is unavailable under Bun on Windows —" and then appends the reason, which getLocalEmbeddingUnavailableReason returns verbatim as "local embeddings are unavailable under Bun on Windows because onnxruntime-node can crash the host process". The rendered doctor line reads the same phrase twice. Since the reason is non-null only on win32+Bun, the hardcoded prefix is always redundant with the reason text. Use the reason alone as the sentence and only add the remediation advice.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/lib/embedding-runtime.ts, line 38:
<comment>The warning string duplicates its cause: the template hardcodes "local is unavailable under Bun on Windows —" and then appends the reason, which `getLocalEmbeddingUnavailableReason` returns verbatim as "local embeddings are unavailable under Bun on Windows because onnxruntime-node can crash the host process". The rendered doctor line reads the same phrase twice. Since the reason is non-null only on win32+Bun, the hardcoded prefix is always redundant with the reason text. Use the reason alone as the sentence and only add the remediation advice.</comment>
<file context>
@@ -28,6 +29,16 @@ export type BrokenLocalEmbeddingRuntimeStatus = Extract<
+): string | null {
+ const reason = getLocalEmbeddingUnavailableReason(platform, bunHost);
+ return reason
+ ? `Embedding provider: local is unavailable under Bun on Windows — ${reason}. Configure embedding.provider=openai-compatible, or set embedding.provider=off to keep keyword search without semantic embeddings.`
+ : null;
+}
</file context>
Suggested change
| ? `Embedding provider: local is unavailable under Bun on Windows — ${reason}. Configure embedding.provider=openai-compatible, or set embedding.provider=off to keep keyword search without semantic embeddings.` | |
| ? `Embedding provider: local — ${reason}. Configure embedding.provider=openai-compatible, or set embedding.provider=off to keep keyword search without semantic embeddings.` |
| ): string | null { | ||
| const reason = getLocalEmbeddingUnavailableReason(platform, bunHost); | ||
| return reason | ||
| ? `Embedding provider: local is unavailable under Bun on Windows — ${reason}. Configure embedding.provider=openai-compatible, or set embedding.provider=off to keep keyword search without semantic embeddings.` |
There was a problem hiding this comment.
P3: The doctor warning repeats itself: the hardcoded prefix "local is unavailable under Bun on Windows" is concatenated with reason, which is itself "local embeddings are unavailable under Bun on Windows because ...". Return just the prefix plus the actionable fix, or drop the duplicated clause from the reason, so the surfaced message reads once.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/lib/embedding-runtime.ts, line 38:
<comment>The doctor warning repeats itself: the hardcoded prefix "local is unavailable under Bun on Windows" is concatenated with `reason`, which is itself "local embeddings are unavailable under Bun on Windows because ...". Return just the prefix plus the actionable fix, or drop the duplicated clause from the reason, so the surfaced message reads once.</comment>
<file context>
@@ -28,6 +29,16 @@ export type BrokenLocalEmbeddingRuntimeStatus = Extract<
+): string | null {
+ const reason = getLocalEmbeddingUnavailableReason(platform, bunHost);
+ return reason
+ ? `Embedding provider: local is unavailable under Bun on Windows — ${reason}. Configure embedding.provider=openai-compatible, or set embedding.provider=off to keep keyword search without semantic embeddings.`
+ : null;
+}
</file context>
There was a problem hiding this comment.
2 issues found across 15 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/plugin/src/shared/redaction.ts">
<violation number="1" location="packages/plugin/src/shared/redaction.ts:249">
P2: When raw `magic-context.jsonc` contains a non-string value under `embedding.headers`, the scalar fast path returns it before this header check runs, so OpenCode diagnostics can print the value. Move the header-path check before the scalar early return to fail closed for malformed header values.</violation>
</file>
<file name="packages/cli/src/lib/diagnostics-pi.ts">
<violation number="1" location="packages/cli/src/lib/diagnostics-pi.ts:208">
P2: When a raw config contains a non-string header value or a non-object `embedding.headers`, `sanitizeValue` can preserve the value before the new redaction guard runs. Move the embedding-header check before primitive handling and fail closed for malformed `embedding.headers` containers.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| export function sanitizeConfigValue(value: unknown, keyPath: string[] = []): unknown { | ||
| if (value === null || typeof value === "number" || typeof value === "boolean") return value; | ||
| const key = keyPath.at(-1) ?? ""; | ||
| if (keyPath.length >= 3 && keyPath.at(-3) === "embedding" && keyPath.at(-2) === "headers") { |
There was a problem hiding this comment.
P2: When raw magic-context.jsonc contains a non-string value under embedding.headers, the scalar fast path returns it before this header check runs, so OpenCode diagnostics can print the value. Move the header-path check before the scalar early return to fail closed for malformed header values.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/plugin/src/shared/redaction.ts, line 249:
<comment>When raw `magic-context.jsonc` contains a non-string value under `embedding.headers`, the scalar fast path returns it before this header check runs, so OpenCode diagnostics can print the value. Move the header-path check before the scalar early return to fail closed for malformed header values.</comment>
<file context>
@@ -246,6 +246,9 @@ export function hasShareabilitySensitiveText(text: string): boolean {
export function sanitizeConfigValue(value: unknown, keyPath: string[] = []): unknown {
if (value === null || typeof value === "number" || typeof value === "boolean") return value;
const key = keyPath.at(-1) ?? "";
+ if (keyPath.length >= 3 && keyPath.at(-3) === "embedding" && keyPath.at(-2) === "headers") {
+ return "<REDACTED:header>";
+ }
</file context>
| } | ||
|
|
||
| export function sanitizeValue(value: unknown, key = ""): unknown { | ||
| export function sanitizeValue(value: unknown, keyPath: readonly string[] = []): unknown { |
There was a problem hiding this comment.
P2: When a raw config contains a non-string header value or a non-object embedding.headers, sanitizeValue can preserve the value before the new redaction guard runs. Move the embedding-header check before primitive handling and fail closed for malformed embedding.headers containers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/lib/diagnostics-pi.ts, line 208:
<comment>When a raw config contains a non-string header value or a non-object `embedding.headers`, `sanitizeValue` can preserve the value before the new redaction guard runs. Move the embedding-header check before primitive handling and fail closed for malformed `embedding.headers` containers.</comment>
<file context>
@@ -205,16 +205,22 @@ function shouldRedactKey(key: string): boolean {
}
-export function sanitizeValue(value: unknown, key = ""): unknown {
+export function sanitizeValue(value: unknown, keyPath: readonly string[] = []): unknown {
if (value === null || typeof value === "number" || typeof value === "boolean") return value;
+ const key = keyPath.at(-1) ?? "";
</file context>
This was referenced Aug 19, 2026
ualtinok
added a commit
that referenced
this pull request
Aug 19, 2026
alfonso-magic-context
requested changes
Aug 20, 2026
alfonso-magic-context
left a comment
alfonso-magic-context
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed. The Windows half is clean and correctly shaped: refusing Bun+win32 before the ONNX import, degrading semantic search to FTS with honest status/doctor messaging, and leaving the nativeRuntimeMissing latch semantics untouched is exactly how we'd have built it. The trusted-headers half also respects the boundary that matters most to us: headers parse from user-tier config only and are stripped from project config before merge, so a cloned repository gains no header control.
Two blockers, both on the credential lifecycle rather than the happy path:
- The doctor probe lacks the runtime client's egress protections.
probeEmbeddingEndpoint()checks only the URL scheme, follows redirects, and skipsblockedEmbeddingEndpointReason(). A trusted endpoint answering 307/308 can make doctor forward the configured credential headers to a second destination that normal embedding traffic would refuse. The probe should use the same guard +redirect: "error"as the runtime provider, with a test proving redirect refusal. - Echoed header values evade redaction. Doctor and the runtime provider both log response-body previews, and shape-based sanitization can't recognize that an innocuous string like
workspace-credential-17is a header value the endpoint echoed back. Known header VALUES should be added to the redaction set for any logged body/preview on these paths, with an echoed-benign-secret test.
With those two closed this is merge-ready from our side.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
globalThis.Bunembedding.headers; project configs cannot set themFixes #283
Fixes #332
Addresses #310
Host-provider boundary
The current OpenCode plugin SDK exposes no embedding hook or host OAuth delegation API. This PR implements the viable custom-header path without inventing an undocumented adapter.
Verification
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Guards local embeddings under Bun on Windows and adds trusted user-level
embedding.headersforopenai-compatiblerequests. Preventsonnxruntime-nodecrashes, preserves keyword search, and supports header-only or non-Bearer auth without exposing secrets.Local embeddings on Bun+Windows: previously attempted ONNX load and could crash; now initialization is refused before import, the provider is never created, marked unavailable, and search falls back to keyword/FTS. Routing and
doctorwarn; the registry and coverage exposeunavailableReason; status shows “Embedding unavailable … openai-compatible or off.” Windows under Node and Bun on macOS/Linux are unaffected.Remote headers and identity: adds user-level
embedding.headersforopenai-compatible. Schema validates names/values; a customAuthorizationoverridesapi_key; header-only auth is supported. Runtime and probes share header construction; provider identity fingerprints normalized headers so rotation recreates the provider and re-embeds as needed. All header values are redacted from identity, logs, diagnostics, and config dumps; project config cannot set headers.Diagnostics: OpenCode and Pi
doctorrespect and validate headers, fail closed on invalid headers without echoing values, and warn (never pass) for local embeddings under Bun+Windows. OpenCode Desktop (Electron) is not treated as Bun.Migration
embedding.provider: "openai-compatible"for semantic search or"off"for keyword-only.embedding.headers); project configs cannot set them.endpointand auth for remote embeddings; host-provider OAuth/credentials are not reused.Written for commit fb57aa7. Summary will update on new commits.
Greptile Summary
This PR prevents local ONNX embeddings from loading under Bun on Windows and adds trusted user-level headers for OpenAI-compatible embedding providers.
Confidence Score: 5/5
The PR appears safe to merge because no blocking failure remains.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD C[Trusted user embedding config] --> R{Provider} R -->|local| H{Bun on Windows?} H -->|yes| U[Mark provider unavailable] U --> F[Keyword and FTS fallback] H -->|no| L[Load local Transformers and ONNX] R -->|openai-compatible| B[Build validated request headers] B --> A{Custom Authorization present?} A -->|yes| X[Use custom Authorization] A -->|no| K[Use api_key bearer token when configured] X --> E[Embedding endpoint] K --> E R -->|off| FReviews (3): Last reviewed commit: "test(cli): expect blanket header redacti..." | Re-trigger Greptile
Context used (4)