Skip to content

fix: research-batch reliability and security-boundary issues #770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Wibias wants to merge 4 commits into from
Closed

fix: research-batch reliability and security-boundary issues #770

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6e6f549
Fix catalog input modalities, catalog API, and Desktop native toggle.
Wibias Jul 30, 2026
26774e3
Fix Windows tray/service lifecycle and CLI init/doctor edge cases.
Wibias Jul 30, 2026
0129a0e
Fix researched reliability and security-boundary issues on Windows, s…
Wibias Jul 30, 2026
5323327
Tighten management Origin scheme-skew and surface unverified ACL state.
Wibias Jul 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions docs-site/src/content/docs/guides/codex-integration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,19 @@ Add a display name from the CLI (the proxy syncs the catalog right away when liv
ocx models add deepseek deepseek-v4 --display-name "DeepSeek V4" --context-window 128000
```

Remote Codex clients can fetch the same generated catalog over the management API (same
admission token as other `/api/*` routes):

```bash
curl -fsS -H "x-opencodex-api-key: $OPENCODEX_ADMIN_AUTH_TOKEN" \
"https://proxy.example.com/api/catalog" > "${CODEX_HOME:-$HOME/.codex}/opencodex-catalog.json"
ocx sync-cache
```

The response is the raw `opencodex-catalog.json` document (no provider credentials). When
available, the `x-opencodex-codex-version` header reports the Codex runtime version on the
server so clients can spot version skew.
Comment on lines +204 to +215

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

for locale in ja ko ru zh-cn; do
  file="$(fd --type f --glob 'codex-integration.md' docs-site/src/content/docs | rg "/${locale}/" | head -n1 || true)"
  if [ -n "$file" ]; then
    rg -n '/api/catalog|x-opencodex-api-key|sync-cache' "$file"
  fi
done

Repository: lidge-jun/opencodex

Length of output: 465

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

printf 'Locale files:\n'
fd -a 'codex-integration.md' docs-site/src/content/docs

printf '\nMatches in locale files:\n'
for locale in ja ko ru zh-cn; do
  echo "== $locale =="
  file="$(fd -a 'codex-integration.md' docs-site/src/content/docs | rg "/${locale}/" | head -n1 || true)"
  if [ -n "$file" ]; then
    echo "FILE: $file"
    rg -n -C 2 '/api/catalog|x-opencodex-api-key|sync-cache' "$file"
  else
    echo "missing"
  fi
done

Repository: lidge-jun/opencodex

Length of output: 1789

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
root = Path('docs-site/src/content/docs')
for p in sorted(root.rglob('codex-integration.md')):
    print(p)
PY

Repository: lidge-jun/opencodex

Length of output: 447

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
root = Path('docs-site/src/content/docs')
for p in sorted(root.rglob('codex-integration.md')):
    print(p)
PY

for locale in ja ko ru zh-cn; do
  echo "== $locale =="
  file="$(python3 - <<'PY'
from pathlib import Path
root = Path('docs-site/src/content/docs')
matches = [str(p) for p in sorted(root.rglob('codex-integration.md')) if f'/{locale}/' in str(p)]
print(matches[0] if matches else '')
PY
)"
  if [ -n "$file" ]; then
    echo "FILE: $file"
    rg -n -C 3 '/api/catalog|x-opencodex-api-key|sync-cache' "$file" || true
  else
    echo "missing"
  fi
done

Repository: lidge-jun/opencodex

Length of output: 609

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

fd -a 'codex-integration.md' docs-site/src/content/docs

printf '\n--- x-opencodex-api-key matches with file names ---\n'
rg -n -H 'x-opencodex-api-key|/api/catalog|sync-cache' docs-site/src/content/docs/**/codex-integration.md 2>/dev/null || true

Repository: lidge-jun/opencodex

Length of output: 1383

Mirror the catalog-sync snippet in the translated docs

docs-site/src/content/docs/ja/guides/codex-integration.md:82-86, docs-site/src/content/docs/ko/guides/codex-integration.md:81-85, docs-site/src/content/docs/ru/guides/codex-integration.md:91-95, and docs-site/src/content/docs/zh-cn/guides/codex-integration.md:78-82 still stop at the x-opencodex-api-key config example. Add the /api/catalog download and ocx sync-cache step from docs-site/src/content/docs/guides/codex-integration.md:204-215 so the locale pages match the English flow.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs-site/src/content/docs/guides/codex-integration.md` around lines 204 -
215, Update the Codex integration sections in the Japanese, Korean, Russian, and
Simplified Chinese locale guides to include the `/api/catalog` download using
the `x-opencodex-api-key` header, the `ocx sync-cache` command, and the
accompanying response/header notes from the English guide. Preserve each page’s
existing translation style while matching the English catalog-sync flow.

Source: Path instructions


You can also set or edit it through the management API (`POST /api/custom-models`,
`PUT /api/custom-models/<id>` with a `displayName` string) and the web dashboard. A `/` is rejected
because it would collide with the routed-slug separator.
Expand Down
25 changes: 16 additions & 9 deletions gui/src/pages/Logs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,12 +280,12 @@ function statusColor(status: number): string {
return "var(--amber)";
}

function formatLogTimestamp(ts: number, localeTag?: string): string {
return new Date(ts).toLocaleTimeString(localeTag);
function formatLogTimestamp(ts: number, localeTag?: string, timeZone?: string): string {
return new Date(ts).toLocaleTimeString(localeTag, timeZone ? { timeZone } : undefined);
}

function formatLogDateTime(ts: number, localeTag?: string): string {
return new Date(ts).toLocaleString(localeTag);
function formatLogDateTime(ts: number, localeTag?: string, timeZone?: string): string {
return new Date(ts).toLocaleString(localeTag, timeZone ? { timeZone } : undefined);
}

function modelTitle(log: LogEntry): string {
Expand Down Expand Up @@ -333,6 +333,7 @@ export default function Logs({ apiBase }: { apiBase: string }) {
const { t, locale } = useI18n();
const cachedLogs = readSessionListCache<LogEntry[]>(logsCacheKey(apiBase));
const [logs, setLogs] = useState<LogEntry[]>(() => cachedLogs ?? []);
const [serverTimeZone, setServerTimeZone] = useState<string | undefined>();
const [loading, setLoading] = useState(() => !(cachedLogs && cachedLogs.length > 0));
const [error, setError] = useState<string | null>(null);
const [autoRefresh, setAutoRefresh] = useState(true);
Expand Down Expand Up @@ -368,9 +369,13 @@ export default function Logs({ apiBase }: { apiBase: string }) {
// failures flicker between the error banner, empty state, and stale table.
if (!silent) setLoading(true);
try {
const res = await fetch(`${apiBase}/api/logs`);
const res = await fetch(`${apiBase}/api/logs?limit=2000`);
if (!res.ok) throw new Error(`${res.status} ${res.statusText}`.trim());
const next = await res.json() as LogEntry[];
const body = await res.json() as LogEntry[] | { logs?: LogEntry[]; timeZone?: string };
const next = Array.isArray(body) ? body : (body.logs ?? []);
if (!Array.isArray(body) && typeof body.timeZone === "string" && body.timeZone.trim()) {
setServerTimeZone(body.timeZone.trim());
}
Comment on lines +372 to +378

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Synchronize serverTimeZone on every successful response.

When the response is an array, or an envelope without a usable timeZone, the previous timezone remains in state. After switching apiBase or falling back to a legacy endpoint, logs can therefore render in the wrong timezone. Compute a candidate for every response and call setServerTimeZone(candidate), including undefined when no timezone is present.

As per path instructions, GUI state must stay consistent with management API responses.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gui/src/pages/Logs.tsx` around lines 372 - 378, The logs fetch handling
around the response body and setServerTimeZone must synchronize state on every
successful response. Compute a trimmed timezone candidate from envelope
responses when usable, otherwise leave it undefined, then always call
setServerTimeZone(candidate), including for array responses or envelopes without
a timezone.

Source: Path instructions

setLogs(next);
writeSessionListCache(logsCacheKey(apiBase), next);
setError(null);
Expand Down Expand Up @@ -591,7 +596,7 @@ export default function Logs({ apiBase }: { apiBase: string }) {
data-index={virtualRow.index}
ref={rowVirtualizer.measureElement}
>
<td className="muted mono">{formatLogTimestamp(log.timestamp, localeTag)}</td>
<td className="muted mono">{formatLogTimestamp(log.timestamp, localeTag, serverTimeZone)}</td>
<td className="num mono log-col-tokens" title={tokensTitle(log, t)}>
{(() => {
const tokenTotal = displayContextTokenTotal(log);
Expand Down Expand Up @@ -678,6 +683,7 @@ export default function Logs({ apiBase }: { apiBase: string }) {
detailInfo={detailInfo}
localeCode={locale}
localeTag={localeTag}
serverTimeZone={serverTimeZone}
t={t}
onClose={() => setDetail(null)}
onFilterConversation={id => {
Expand All @@ -703,12 +709,13 @@ function useModalDialog(open: boolean) {
}

function LogDetailDialog({
detail, detailInfo, localeCode, localeTag, t, onClose, onFilterConversation,
detail, detailInfo, localeCode, localeTag, serverTimeZone, t, onClose, onFilterConversation,
}: {
detail: LogEntry;
detailInfo: ReturnType<typeof statusCodeInfo> | null;
localeCode: string;
localeTag?: string;
serverTimeZone?: string;
t: TFn;
onClose: () => void;
onFilterConversation?: (conversationId: string) => void;
Expand Down Expand Up @@ -750,7 +757,7 @@ function LogDetailDialog({
<section className="log-detail-section" aria-labelledby="log-detail-basic">
<h4 id="log-detail-basic" className="log-detail-section-title">{t("logs.detail.section.basic")}</h4>
<div className="log-detail-grid">
<span className="muted">{t("logs.col.time")}</span><span className="mono">{formatLogDateTime(detail.timestamp, localeTag)}</span>
<span className="muted">{t("logs.col.time")}</span><span className="mono">{formatLogDateTime(detail.timestamp, localeTag, serverTimeZone)}</span>
<span className="muted">{t("logs.col.request")}</span>
<span className="log-detail-request-row">
<span className="mono log-detail-break">{detail.requestId ?? "\u2014"}</span>
Expand Down
53 changes: 45 additions & 8 deletions src/adapters/anthropic.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,36 @@ function usesNativeAnthropicEndpoint(provider: OcxProviderConfig): boolean {
}
}

/** Normalize provider baseUrl paths ending in `/`, `/v1`, or `/v1/messages` to `{origin}/v1/messages`. */
export function anthropicMessagesUrl(baseUrl: string): string {
try {
new URL(baseUrl);
} catch {
throw new Error(`anthropic provider has malformed baseUrl: ${baseUrl}`);
}
const trimmed = baseUrl.trim().replace(/\/+$/, "");
const root = trimmed.replace(/\/v1\/messages\/?$/i, "").replace(/\/v1\/?$/i, "").replace(/\/+$/, "");
return `${root}/v1/messages`;
Comment on lines +253 to +262

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reject or normalize query/fragment base URLs before appending the endpoint.

https://host/v1?tenant=x becomes https://host/v1?tenant=x/v1/messages; a fragment similarly swallows /v1/messages. Parse and rebuild the URL pathname, or reject search/hash explicitly.

Proposed fix 
 export function anthropicMessagesUrl(baseUrl: string): string {
+  let parsed: URL;
   try {
-    new URL(baseUrl);
+    parsed = new URL(baseUrl);
   } catch {
     throw new Error(`anthropic provider has malformed baseUrl: ${baseUrl}`);
   }
-  const trimmed = baseUrl.trim().replace(/\/+$/, "");
-  const root = trimmed.replace(/\/v1\/messages\/?$/i, "").replace(/\/v1\/?$/i, "").replace(/\/+$/, "");
-  return `${root}/v1/messages`;
+  if (parsed.search || parsed.hash) {
+    throw new Error("anthropic provider baseUrl must not contain a query or fragment");
+  }
+  const path = parsed.pathname
+    .replace(/\/v1\/messages\/?$/i, "")
+    .replace(/\/v1\/?$/i, "")
+    .replace(/\/+$/, "");
+  parsed.pathname = `${path}/v1/messages`;
+  return parsed.toString();
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/** Normalize provider baseUrl paths ending in `/`, `/v1`, or `/v1/messages` to `{origin}/v1/messages`. */
export function anthropicMessagesUrl(baseUrl: string): string {
try {
new URL(baseUrl);
} catch {
throw new Error(`anthropic provider has malformed baseUrl: ${baseUrl}`);
}
const trimmed = baseUrl.trim().replace(/\/+$/, "");
const root = trimmed.replace(/\/v1\/messages\/?$/i, "").replace(/\/v1\/?$/i, "").replace(/\/+$/, "");
return `${root}/v1/messages`;
/** Normalize provider baseUrl paths ending in `/`, `/v1`, or `/v1/messages` to `{origin}/v1/messages`. */
export function anthropicMessagesUrl(baseUrl: string): string {
let parsed: URL;
try {
parsed = new URL(baseUrl);
} catch {
throw new Error(`anthropic provider has malformed baseUrl: ${baseUrl}`);
}
if (parsed.search || parsed.hash) {
throw new Error("anthropic provider baseUrl must not contain a query or fragment");
}
const path = parsed.pathname
.replace(/\/v1\/messages\/?$/i, "")
.replace(/\/v1\/?$/i, "")
.replace(/\/+$/, "");
parsed.pathname = `${path}/v1/messages`;
return parsed.toString();
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/adapters/anthropic.ts` around lines 253 - 262, Update
anthropicMessagesUrl to parse the validated baseUrl and handle URL search and
hash components before constructing the endpoint. Reject URLs containing query
or fragment components, or rebuild from the parsed origin/pathname so the
returned value always appends /v1/messages to the path rather than to search or
hash text.

}

function synthesizeToolUseId(): string {
return `toolu_${crypto.randomUUID().replace(/-/g, "").slice(0, 24)}`;
}

function toolUseArguments(input: unknown): string {
if (typeof input === "string") {
const trimmed = input.trim();
if (!trimmed) return "{}";
try {
JSON.parse(trimmed);
return trimmed;
} catch {
return JSON.stringify(trimmed);
}
}
return JSON.stringify(input ?? {});
}

function anthropicKeyUsesBearer(provider: OcxProviderConfig): boolean {
return provider.apiKeyTransport === "bearer";
}
Expand Down Expand Up @@ -680,8 +710,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
else if (typeof tc === "object" && "name" in tc) body.tool_choice = { type: "tool", name: toolNames.toWire(resolveToolChoiceWireName(parsed.context.tools, tc.name)) };
}

const base = provider.baseUrl.replace(/\/v1\/?$/, "");
const url = `${base}/v1/messages`;
const url = anthropicMessagesUrl(provider.baseUrl);
const unresolvedPlaceholder = url.match(/\{[^}]*\}/)?.[0];
if (unresolvedPlaceholder) {
throw new Error(`anthropic baseUrl contains unresolved ${unresolvedPlaceholder}`);
Expand Down Expand Up @@ -735,6 +764,7 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
let pendingUsage: Record<string, number> | undefined;
let pendingStopReason: string | undefined;
let emittedDone = false;
let sawContent = false;

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Treat emitted redacted thinking as stream output.

A redacted_thinking block yields an event at Lines 811-814 but never sets sawContent. If EOF follows it without message_stop, the adapter emits error after valid state-bearing output instead of done.

Proposed fix 
 if (block.type === "redacted_thinking" && typeof block.data === "string") {
+  sawContent = true;
   yield { type: "redacted_thinking", data: block.data };
 }

Add an EOF regression case for a redacted_thinking block.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let sawContent = false;
if (block.type === "redacted_thinking" && typeof block.data === "string") {
sawContent = true;
yield { type: "redacted_thinking", data: block.data };
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/adapters/anthropic.ts` at line 767, Update the stream state tracking
around sawContent and the redacted_thinking event handling so emitting a
redacted_thinking block marks the stream as having content, allowing EOF after
that block to produce done rather than error when message_stop is absent. Add an
EOF regression test covering a redacted_thinking block.


const emitDone = function* (): Generator<AdapterEvent> {
if (emittedDone) return;
Expand Down Expand Up @@ -773,8 +803,9 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
if (!block) break;
currentBlockType = block.type;
if (block.type === "tool_use") {
currentToolCallId = block.id ?? "";
currentToolCallId = block.id ?? synthesizeToolUseId();
currentToolCallName = toolNames.fromWire(block.name ?? "");
sawContent = true;
yield { type: "tool_call_start", id: currentToolCallId, name: currentToolCallName };
}
if (block.type === "redacted_thinking" && typeof block.data === "string") {
Expand All @@ -787,19 +818,24 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
const delta = data.delta as Record<string, unknown> | undefined;
if (!delta) break;
if (delta.type === "text_delta" && typeof delta.text === "string") {
sawContent = true;
yield { type: "text_delta", text: delta.text };
} else if (delta.type === "thinking_delta" && typeof delta.thinking === "string") {
sawContent = true;
yield { type: "thinking_delta", thinking: delta.thinking };
} else if (delta.type === "reasoning_delta" && typeof delta.reasoning === "string") {
// Some Anthropic-compatible reasoning models use `reasoning` names for the
// otherwise equivalent thinking block. Preserve it as raw reasoning and keep
// later text blocks independent.
sawContent = true;
yield { type: "thinking_delta", thinking: delta.reasoning };
} else if (delta.type === "signature_delta" && typeof delta.signature === "string" && (currentBlockType === "thinking" || currentBlockType === "reasoning")) {
// Arrives once, just before the thinking block's content_block_stop; block-scoped
// so a stray signature on a non-thinking block can never be captured.
sawContent = true;
yield { type: "thinking_signature", signature: delta.signature };
} else if (delta.type === "input_json_delta" && typeof delta.partial_json === "string") {
} else if (delta.type === "input_json_delta" && typeof delta.partial_json === "string" && currentBlockType === "tool_use") {
sawContent = true;
yield { type: "tool_call_delta", arguments: delta.partial_json };
}
break;
Expand Down Expand Up @@ -831,12 +867,12 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
}
}
if (!emittedDone) {
if (pendingStopReason !== undefined) {
if (pendingStopReason !== undefined || sawContent) {

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Fail EOF while an Anthropic tool block is incomplete

When an Anthropic stream closes after content_block_start for tool_use but before content_block_stop, sawContent causes this branch to emit a clean done. The bridge consequently closes the pending tool item and reports completion with missing or truncated arguments, allowing clients to process a malformed tool call instead of recognizing an upstream truncation. Track whether the current tool block completed and retain the error path while one is still open.

AGENTS.md reference: src/AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

const stopReason = pendingStopReason === "max_tokens"
? "max_tokens"
: pendingStopReason === "refusal" || pendingStopReason === "content_filter"
? "content_filter"
: undefined;
: pendingStopReason;
emittedDone = true;
yield {
type: "done",
Expand Down Expand Up @@ -867,8 +903,9 @@ export function createAnthropicAdapter(provider: OcxProviderConfig, cacheRetenti
} else if (block.type === "redacted_thinking" && typeof block.data === "string") {
events.push({ type: "redacted_thinking", data: block.data });
} else if (block.type === "tool_use") {
events.push({ type: "tool_call_start", id: block.id ?? "", name: toolNames.fromWire(block.name ?? "") });
events.push({ type: "tool_call_delta", arguments: JSON.stringify(block.input ?? {}) });
const id = block.id ?? synthesizeToolUseId();
events.push({ type: "tool_call_start", id, name: toolNames.fromWire(block.name ?? "") });
events.push({ type: "tool_call_delta", arguments: toolUseArguments(block.input) });
events.push({ type: "tool_call_end" });
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/adapters/openai-chat.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
const pendingToolCalls: PendingToolCall[] = [];
let toolCallSeq = 0;
const flushToolCalls = function* (): Generator<AdapterEvent> {
if (pendingToolCalls.length > 0) sawOutput = true;

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Do not complete partial OpenAI tool calls at EOF

If the socket closes after emitting only part of a tool call and before [DONE] or finish_reason, this assignment makes sawOutput true merely because a pending call exists. The EOF path then skips its truncation error after emitting tool_call_end, so the bridge reports response.completed with empty or partial JSON arguments instead of a failed/incomplete response. Only count a tool call as terminal output after a real terminal signal, or validate that the call is complete before tolerating EOF.

AGENTS.md reference: src/AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

for (const call of pendingToolCalls) {
if (!call.id) call.id = `call_${++toolCallSeq}`;
yield { type: "tool_call_start", id: call.id, name: call.name };
Expand All @@ -674,6 +675,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
// explicit `[DONE]` sentinel OR a chunk carrying a non-null `finish_reason` (some
// OpenAI-compatible providers omit `[DONE]` but do send finish_reason).
let finishReason: string | undefined;
let sawOutput = false;

// Single per-line handler shared by the streaming loop and the EOF residual-frame flush, so
// a final frame is parsed identically wherever it lands (no duplicated, drift-prone parsing).
Expand Down Expand Up @@ -740,9 +742,11 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
const delta = choices[0].delta;
if (delta) {
if (typeof delta.reasoning_content === "string" && delta.reasoning_content.length > 0) {
sawOutput = true;
yield { type: "reasoning_raw_delta", text: delta.reasoning_content };
}
if (typeof delta.content === "string" && delta.content.length > 0) {
sawOutput = true;
yield { type: "text_delta", text: delta.content };
}

Expand Down Expand Up @@ -806,7 +810,7 @@ export function createOpenAIChatAdapter(provider: OcxProviderConfig): ProviderAd
// at end-of-generation). If NONE of those were seen, the stream was cut mid-flight — fail
// closed so the bridge emits a classified response.failed rather than a silent truncation.
const sawFinish = finishReason !== undefined;
if (!sawFinish && pendingUsage === undefined) {
if (!sawFinish && pendingUsage === undefined && !sawOutput) {
debugProviderDiagnostic("openai-chat", "stream-truncated", {
finishReason: finishReason ?? null,
hadUsage: false,
Expand Down
4 changes: 2 additions & 2 deletions src/cli/claude-desktop.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
type DesktopProfile,
} from "../claude/desktop-profile";
import { writeDesktop3pConfig, type Desktop3pConfigMode, parseDesktop3pModeArgs } from "../claude/desktop-3p";
import { filterCatalogVisibleModels, visibleNativeSlugs } from "../codex/catalog";
import { filterCatalogVisibleModels, desktopVisibleNativeSlugs } from "../codex/catalog";
import { buildClaudeDesktopState, fetchAllModels } from "../server/management-api";
import { findLiveProxy } from "../server/proxy-liveness";

Expand Down Expand Up @@ -42,7 +42,7 @@ async function applyProfile(profile: DesktopProfile, mode: Desktop3pConfigMode):
}));
const result = writeDesktop3pConfig(
live?.port ?? config.port ?? 10100,
[...visibleNativeSlugs(config)],
[...desktopVisibleNativeSlugs(config)],
routed,
config.apiKeys?.[0]?.key,
mode,
Expand Down
49 changes: 48 additions & 1 deletion src/cli/doctor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -307,13 +307,47 @@ export type ConfiguredProxyDiagnostic = {
detail: string;
};

function envReferenceName(value: string): string | null {
export function envReferenceName(value: string): string | null {
const braced = value.match(/^\$\{(\w+)\}$/);
if (braced) return braced[1]!;
const bare = value.match(/^\$(\w+)$/);
return bare ? bare[1]! : null;
}
Comment on lines +310 to 315

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find the canonical env-reference resolution used for provider credentials.
rg -nP -C4 '\$\\?\{[^}]*\}|resolveEnv|expandEnv|envRef' src --type=ts -g '!src/cli/doctor.ts' | head -80

# Where apiKey is resolved before an upstream request.
ast-grep run --lang typescript --pattern 'function $NAME($$$) { $$$ }' src --json=stream 2>/dev/null | head -5 >/dev/null
rg -nP -C3 '\bapiKey\b.*(resolve|expand)|(resolve|expand).*\bapiKey\b' src --type=ts | head -40

Repository: lidge-jun/opencodex

Length of output: 8258

Reuse the shared env resolver in src/cli/doctor.ts envReferenceName duplicates the runtime resolveEnvValue path used for provider.apiKey, so the doctor check can drift from what the app actually accepts. resolveInEnv can also collapse to env[envName] once envReferenceName(raw) is non-null, which removes the dead return value branch.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/doctor.ts` around lines 310 - 315, Replace the local env-reference
parsing in envReferenceName with the shared resolveEnvValue logic used for
provider.apiKey, so doctor validation matches runtime resolution. Update
resolveInEnv to return env[envName] whenever envReferenceName(raw) is non-null
and remove the unreachable raw-value fallback, preserving handling of
non-reference values.


export type ProviderApiKeyDiagnostic = {
provider: string;
envName: string;
detail: string;
};

/** Warn when a key-auth provider's apiKey env reference resolves empty in this process. */
export function collectProviderApiKeyDiagnostics(
providers: Record<string, { authMode?: string; apiKey?: string }> = readConfigDiagnostics().config.providers ?? {},
env: EnvMap = process.env,
): ProviderApiKeyDiagnostic[] {
const resolveInEnv = (value: string): string | undefined => {
const name = envReferenceName(value);
if (!name) return value;
return env[name];
};
const rows: ProviderApiKeyDiagnostic[] = [];
for (const [provider, config] of Object.entries(providers)) {
if (config.authMode !== "key") continue;

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Treat omitted authMode as key auth in doctor diagnostics

Providers created by the existing ocx init key-provider flow store an apiKey but omit authMode, and other runtime code treats an absent mode as key auth. This strict check therefore skips the common ${ENV_VAR} configuration that the new diagnostic is meant to catch and incorrectly prints that no empty provider keys were detected. Skip only explicit non-key modes, or derive the effective auth mode from the provider registry/defaults.

AGENTS.md reference: src/AGENTS.md:L10-L10

Useful? React with 👍 / 👎.

const raw = typeof config.apiKey === "string" ? config.apiKey.trim() : "";
if (!raw) continue;
const envName = envReferenceName(raw);
if (!envName) continue;
const resolved = resolveInEnv(raw);
if (resolved?.trim()) continue;
Comment on lines +334 to +341

@coderabbitai coderabbitai Bot Jul 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Does configSchema give authMode a default of "key"?
rg -nP -C4 'authMode' src/config.ts | head -60
rg -nP -C3 'authMode.*default|default\(.*key' src --type=ts | head -40

Repository: lidge-jun/opencodex

Length of output: 4350

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- src/config.ts authMode references ---'
rg -n -C4 'authMode' src/config.ts || true

echo
echo '--- src/types.ts OcxProviderConfig authMode ---'
rg -n -C4 'interface OcxProviderConfig|type OcxProviderConfig|authMode' src/types.ts || true

echo
echo '--- src/cli/doctor.ts relevant block ---'
sed -n '326,348p' src/cli/doctor.ts

echo
echo '--- src/cli/init.ts key provider config block ---'
sed -n '118,142p' src/cli/init.ts

Repository: lidge-jun/opencodex

Length of output: 7357

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- config schema outline ---'
ast-grep outline src/config.ts --view expanded | sed -n '1,260p'

Repository: lidge-jun/opencodex

Length of output: 11003

🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
text = Path('src/config.ts').read_text()
for needle in ['authMode', 'default("key")', 'defaultValue', 'z.object', 'provider']:
    print(f'=== {needle} ===')
    idx = text.find(needle)
    print(idx)
    if idx != -1:
        start = max(0, idx - 300)
        end = min(len(text), idx + 800)
        print(text[start:end])
        print()
PY

Repository: lidge-jun/opencodex

Length of output: 3580

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- config-related symbols in src/config.ts ---'
rg -n -C3 'configSchema|loadConfig|parse\(|safeParse\(|authMode|providers:' src/config.ts || true

echo
echo '--- any default("key") / key default in src ---'
rg -n -C3 'default\("key"\)|default\(.*"key"|default\(.*key' src --type=ts || true

Repository: lidge-jun/opencodex

Length of output: 8089

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C3 'authMode\??:' src/types.ts src/config.ts src/cli/init.ts src/cli/doctor.ts || true

Repository: lidge-jun/opencodex

Length of output: 3787

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- doctor.ts function signature and config source ---'
sed -n '1,120p' src/cli/doctor.ts

echo
echo '--- call sites for doctor routine ---'
rg -n -C2 'doctor\(' src/cli src || true

Repository: lidge-jun/opencodex

Length of output: 5396

Treat missing authMode as "key" in src/cli/doctor.ts:334-341

ocx init can persist key-auth providers as apiKey: "${ENV_VAR}" with no explicit authMode, but this check skips every provider whose authMode is absent. src/config.ts keeps authMode optional and does not fill in a "key" default during parse, so init-generated configs never get warned on. Accept undefined as the key default and add a regression in tests/doctor-provider-apikey.test.ts for apiKey: "${SOME_KEY}" without authMode.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/doctor.ts` around lines 334 - 341, Update the provider loop around
config.authMode to treat an absent authMode as the default key-auth mode, while
continuing to skip explicitly non-key providers. Add a regression case in the
doctor provider API-key tests covering apiKey: "${SOME_KEY}" without authMode
and verifying the missing environment variable is reported.

rows.push({
provider,
envName,
detail: `provider ${provider}: env reference ${envName} is unset or empty in this process`,
});
}
return rows;
}

export function collectConfiguredProxy(): ConfiguredProxyDiagnostic {
const diagnostics = readConfigDiagnostics();
const rawProxy = typeof diagnostics.config.proxy === "string" ? diagnostics.config.proxy.trim() : "";
Expand Down Expand Up @@ -742,6 +776,16 @@ export async function runDoctor(args: string[] = []): Promise<void> {
console.log("\nConfigured proxy (value hidden)");
console.log(` ${configuredProxy.present ? "set " : "unset "} ${configuredProxy.key} (${configuredProxy.source}; ${configuredProxy.detail})`);

const providerApiKeys = collectProviderApiKeyDiagnostics(doctorConfig.providers);
console.log("\nProvider API keys (value hidden)");
if (providerApiKeys.length === 0) {
console.log(" ok no empty env-referenced provider keys detected in this process");
} else {
for (const row of providerApiKeys) {
console.log(` !! ${row.detail}`);
}
}

console.log("\nRunning proxy process proxy env (presence only)");
if (runningProxyEnv.status === "not_running") {
console.log(" -- no running ocx proxy process found");
Expand Down Expand Up @@ -825,6 +869,9 @@ export async function runDoctor(args: string[] = []): Promise<void> {
serviceViable: startup.serviceViable,
});
if (proxyDown) hints.push(proxyDown);
for (const row of providerApiKeys) {
hints.push(`${row.detail}. Set ${row.envName} in the shell that starts the proxy, or store a literal key in config (value hidden here).`);
}
const anyDrvfs = paths.some(p => detectFsType(p.path, mounts).isDrvfs || detectFsType(p.path, mounts).isMntDrive);
const noProxy = currentProxyEnv.every(p => !p.present) && !configuredProxy.present;
if (!startup.rebootSafe) {
Expand Down
Loading
Loading