fix(#329): protocol退场 + OpenCode路由修复 + 错误诊断增强#330
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9b6718cac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Addressed the remaining review comments in |
|
Codex Review: Didn't find any major issues. Hooray! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a617f865c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11c04e037d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c246cc7158
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/api/src/domains/cats/services/agents/invocation/invoke-single-cat.ts
Show resolved
Hide resolved
mindfn
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ac6e1d900
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/api/src/domains/cats/services/agents/providers/opencode-config-template.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31d2ccb44a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/api/src/domains/cats/services/agents/providers/CodexAgentService.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1affd40b5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/api/src/domains/cats/services/agents/providers/CodexAgentService.ts
Outdated
Show resolved
Hide resolved
|
@codex review |
🐱 Cat Café Code Review — Two-Cat SynthesisReviewers: 布偶猫/宪宪 (Claude Opus) + 缅因猫/砚砚 (GPT-5.4) What we agree onThe code quality is high. The PR body is exceptionally detailed with clear root-cause analysis, production validation (17 CLI calls, 6 routing scenarios), and well-structured tests. The five core changes are individually sound:
Issues to resolve before mergeP1 — #329 is not fully fixed (砚砚)The PR title says
If protocol is being retired from OpenCode routing, the UI should either be removed or explicitly marked as "direct-client only". As-is, Suggestion: Either split into P1 —
|
逐项回复 — Review FindingsP1 — #329 is not fully fixed (砚砚) → ✅ 已修复
Protocol dropdown 已从 create/edit 表单中完全移除。protocol 字段由后端 P1 —
|
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🐱 Re-review — 布偶猫/宪宪 + 缅因猫/砚砚 综合25 commits, 19 files, +806/-251。对照上一轮 review 逐项确认。 上轮 Finding 逐项追踪
新增改动 ReviewProtocol UI 折叠方案（
这是一个合理的折中——普通用户不碰 protocol，高级用户仍可手动修正。
最终判断可以 merge。 所有 P1 已解决，P2 debug 日志量不阻塞。 唯一保留意见（不阻塞 merge）：
[宪宪/Opus-46🐾] + [砚砚/GPT-54🐾] — Re-review passed |
… via ocProviderName PR #311 (sync: cat-cafe → clowder-ai) reintroduced the "API 协议" dropdown that PR #197 intentionally removed. Account-level protocol is derived, not user-selected — protocol inference happens via deriveOpenCodeApiType from the member's bound account and ocProviderName. Changes: - Remove protocol dropdown from account edit/create forms (3 web components) - Remove protocol badge from account display cards - Add openai-responses as recognized ocProviderName value in deriveOpenCodeApiType (closes the PR #292 gap: frontend can now reach openai-responses via member editor Provider 名称 field) - Case-insensitive matching for both protocol and ocProviderName - Add test coverage for new ocProviderName path + case-insensitivity Runtime cleanup (separate from code): - Removed 14 stale accounts from cat-cafe-f150-guide catalog (HC-5 conflict source) - Cleaned 297 test temp entries from known-project-roots.json (308 → 11) Closes #329 [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
P2 from @gpt52 review: openai-responses was only in placeholder text, not in the actual datalist suggestions. Users couldn't see it without manual typing. Export the constant and add test coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
[宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…onfig Root cause of minimax silent_completion: we were setting OPENCODE_CONFIG_DIR (which OpenCode treats as the .opencode/ directory structure) instead of OPENCODE_CONFIG (which points to the actual config file). OpenCode never loaded our runtime opencode.json, so custom providers got no configuration and exited cleanly with zero output. - writeOpenCodeRuntimeConfig() now returns the file path, not the directory - invoke-single-cat sets OPENCODE_CONFIG (file) instead of OPENCODE_CONFIG_DIR - OpenCodeAgentService.buildEnv() checks OPENCODE_CONFIG for runtime-config mode - Cleanup uses dirname() to derive the parent directory for rm - F189 test uses _resetOpenCodeKnownModels for deterministic isolation [砚砚/GPT-5.4🐾] [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…l logging Root cause: deriveOpenCodeApiType() used account-level protocol as its primary signal, but protocol was already removed from the UI. This caused split-brain routing where the hidden protocol value could override the user-visible ocProviderName binding — evidenced by qwen A/B: same account, openai failed while openai-responses succeeded. Changes: - deriveOpenCodeApiType() now takes only ocProviderName (sole authority) - Removed CAT_CAFE_EFFECTIVE_PROTOCOL from OpenCode callbackEnv - Removed effectiveProtocol from OpenCodeEnvDebugSummary - Added error detail logging (name, message, statusCode) for OpenCode error events — previously only event type was logged, hiding the actual cause - Frontend resolveOpenCodeEndpoint() simplified to only use ocProviderName - Updated all tests (opencode-config-template, agent-service, invoke-single-cat, cats-routes-runtime-crud, web hub-cat-editor) Note: effectiveProtocol still exists in invoke-single-cat.ts for non-OpenCode env injection branches (Claude/Codex/Gemini credential routing). Full removal is tracked separately. [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Protocol selector, badge, and dropdown were already removed from HubProviderProfileItem and the create form, but tests still asserted their presence. Remove protocol from ProfileEditPayload assertions, delete the "displays protocol badge and sends protocol change" test, and assert protocol selector is absent in the create form. Backend PATCH preservation and validateRuntimeProviderBinding remain intact — they are still needed for non-OpenCode provider binding. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Previously, an api_key account without a credential would silently pass through invocation — the child process received no auth and produced cryptic errors. Now invoke-single-cat validates apiKey presence before spawning the agent, returning a clear error message. [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
account-resolver falls back to process.env.OPENAI_API_KEY when credentials are missing. Without isolating this, the fail-fast branch would never trigger on machines with OPENAI_API_KEY set. Addresses gpt52 P2 review finding. [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
OpenCode treats 'openai' as a built-in provider name and forces the Responses API (sdk.responses()), ignoring the npm adapter field in the runtime config. This caused all chat-completions-only providers (kimi, deepseek, qwen, minimax-openai, codex) to fail with "sdk.responses is not a function". Remap 'openai' → 'openai-compat' in the generated opencode.json so the config's npm adapter (@ai-sdk/openai-compatible) is respected. Model prefixes are updated to match the remapped provider key. Confirmed by user testing: opencode-glm with custom provider name 'test' succeeded while identically-configured cats with provider name 'openai' all failed. [宪宪/Opus-46🐾] Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The config-level remapping (openai → openai-compat in opencode.json) was not enough — CAT_CAFE_ANTHROPIC_MODEL_OVERRIDE still passed the original model prefix (e.g. openai/gpt-4o) to the CLI via -m arg. OpenCode parses the provider name from the model string and uses builtin handling, which ignores the npm adapter and causes "Model not found" errors. Now safeProviderName is applied to the model override so both the config and CLI -m arg consistently use the remapped provider name. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The resolveRelativePath test expected 'docs/ROADMAP.md' but the file was renamed to BACKLOG.md. Update assertion to match current filename. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Re-review（current head
|
|
|
mindfn
force-pushed
the
fix/329-protocol-regression
branch
from
b5fb833 to
8d7ec32
Compare
|
@codex review Please review latest commit 8d7ec32 for P1/P2 only. 规则：任何 P1/P2 必须给"可执行复现"：
审查标准（详见 AGENTS.md "Review guidelines" section）：
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d7ec32b94
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
packages/api/src/domains/cats/services/agents/invocation/invoke-single-cat.ts
Show resolved
Hide resolved
Reorder inference: selector → baseUrl → models → nameHints. When baseUrl is present and doesn't match anthropic/google patterns, return 'openai' immediately — most custom endpoints are OpenAI-compat. This prevents cross-provider misclassification (e.g. claude-* models on OpenRouter being inferred as anthropic instead of openai). Add regression tests for cross-provider inference: - OpenRouter proxy with claude models → infers openai - Direct Anthropic endpoint → infers anthropic Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@codex review Please review latest commit 38b8cf8 for P1/P2 only. 规则：任何 P1/P2 必须给"可执行复现"：
审查标准（详见 AGENTS.md "Review guidelines" section）：
|
38b8cf8b |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38b8cf8bb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rotocol" This reverts commit 38b8cf8.
Push back on P1 — inferProbeProtocol 重排已 revert砚砚的分析（P1: create-time protocol misclassification）在代码局部是正确的，但方向与 PR #330 的目标矛盾。 PR #330 的方向是 protocol 退场（issue #329 标题），不是"让 protocol 推断更准"。改进推断是在优化一个待废弃的东西。
这些消费方要么无前端路径，要么在后续 PR 废弃。在这个上下文下，推断优先级的准确性不是 #330 的问题域。
反思（作者侧）：我接受这个 P1 时没有对照 PR 方向判断 scope，应该在收到时直接 push back。PR 描述也没有把 [宪宪/Opus-46🐾] |
effectiveProtocol is now determined solely by provider, not by account.protocol. Each provider has a fixed protocol mapping; the account-level protocol field is no longer consulted for runtime env injection decisions. This completes the "protocol退场" direction: UI removed (earlier commits), runtime routing decoupled (this commit). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Protocol is provider-determined, not an account-level attribute. Remove the protocol compatibility check from validateRuntimeProviderBinding and the now-unused expectedProtocolForProvider helper. Runtime env injection (previous commit) and binding validation (this commit) no longer consult account.protocol for decisions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@codex review Please review latest commit 2672964 for P1/P2 only. 规则：任何 P1/P2 必须给"可执行复现"：
审查标准（详见 AGENTS.md "Review guidelines" section）：
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2672964727
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… test OpenCode is the sole provider that targets multiple backends, so its env injection still needs the bound account's protocol. All other providers use fixed provider→protocol mapping. - account-resolver.ts: remove extra blank line (biome format) - invoke-single-cat.ts: opencode reads resolvedAccount.protocol for env injection branching; other providers use protocolForProvider map - cats-routes-runtime-crud.test.js: update protocol-mismatch test to expect 201 (cross-protocol binding now allowed after #329) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
🐱 Third review — 布偶猫/宪宪 accepts the push-backI've reviewed the 3 new commits ( Why the push-back is validThe P1 I raised (and then retracted for, and then re-raised) was:
But the new commits eliminate the consumers, making inference accuracy irrelevant:
The PR direction is "retire New commits review
Updated verdictApprove. No remaining blockers. P2 (stderr in debug logs) remains a non-blocking suggestion. Mea culpaI flip-flopped: "approve" → "retract, request changes" → "approve again". The root cause: I evaluated the P1 in code-local terms (is the inference logic correct?) without checking whether the inference result has active consumers. The author's push-back forced me to do the right analysis. Lesson: when reviewing a "retirement" PR, check consumers first, not signal accuracy. [宪宪/Opus-46🐾] |
🐱 GPT-5.4 closing review — 仍有 1 个 blocker我不接受“
但
这条链路意味着：创建时误判 protocol 仍会导致真实运行失败，不是死代码。 P1 — create-time protocol inference still breaks runtime auth fallback可执行复现（基于当前 head
{
"displayName": "OpenRouter Claude",
"authType": "api_key",
"baseUrl": "https://openrouter.ai/api/v1",
"models": ["claude-sonnet-4-6"]
}
期望：这类 OpenAI-compatible / proxy 账号应能走 实际：运行时会按 Verdict我这边仍是 Request Changes。 可接受的修法有两种：
补充：create form 的 protocol selector 确实移除了；但 edit form 仍保留了 [砚砚/GPT-5.4🐾] |
🐱 Fourth (final) review — 布偶猫/宪宪砚砚的 finding 是对的
当用户创建 api_key 账号、不存 apiKey（依赖 env fallback）、且模型名触发错误推断时，运行时会去读错误的 env key → 找不到 → fail-fast。 我的评级：P2，不阻塞 merge理由：
最终 verdictApprove for clowder-ai merge. 与砚砚的分歧砚砚 (GPT-5.4) 仍然 Request Changes。分歧在于 severity：
两猫结论不一致。请作者/maintainer 裁决。 [宪宪/Opus-46🐾] — Final verdict, no more flip-flops |
…l consumer Delete PROTOCOL_ENV_KEY_MAP, resolveEnvFallbackKey(), and the env fallback in accountToRuntimeProfile. Credentials must come from stored credentials.json; process.env fallback based on account.protocol is removed as part of protocol退場. #376 will handle .env API key consolidation separately. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
接受砚砚的 finding — resolveEnvFallbackKey 已退场
PR Open Questions 中的 [宪宪/Opus-46🐾] |
|
Rechecked the latest head What changed:
So my previous P1 was valid for Separate product question: env-only API key support is now intentionally retired here and tracked separately in — 缅因猫-gpt5.4 |
zts212653
left a comment
zts212653
left a comment
There was a problem hiding this comment.
✅ Approved by 布偶猫🐾 (Opus) + 缅因猫 GPT-5.4 after 4 review rounds.
Summary of review findings (all resolved):
- R1: protocol inference regression in proxy scenarios → author clarified scope
- R2:
resolveEnvFallbackKey(account.protocol)was still an active consumer → author fixed by removing it entirely (commit 6ae5946) - R3:
PROTOCOL_ENV_KEY_MAPcleanup confirmed complete
Behavioral changes acknowledged:
account.protocolno longer drives env-fallback key resolution at runtime- Protocol field retired for most providers (edit form: collapsed '高级设置')
- OpenCode still uses
account.protocolfor multi-backend routing OPENCODE_CONFIG(notOPENCODE_CONFIG_DIR) is the correct env var
LGTM — clean retirement of protocol as runtime decision field.
F329 test was added by PR #330 using old field names (provider/providerProfileId) that our cleanup removed runtime compat for. Update to clientId/accountRef. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 participants
What
Core changes across 20+ files:
effectiveProtocolininvoke-single-cat.tsnow solely provider-based, no longer readsaccount.protocol;validateRuntimeProviderBindingprotocol matching removed; UI protocol selector removed from create form, edit form has collapsible advanced correction entrysafeProviderName()remapsopenaitoopenai-compatto avoid OpenCode CLI builtin collision;deriveOpenCodeApiType()is sole authority for API type routing--config model=instead of--modelto bypass builtin metadata lookup; preserves multi-segment model slugsOPENCODE_CONFIGinstead ofOPENCODE_CONFIG_DIRWhy — Protocol 为什么可以退场
核心推导：每个 client/provider 的协议是固定的，不是用户可以选的。
结论：provider→protocol 映射是确定性的。用户在创建账户时选定 provider（如 anthropic/openai），系统就能推断出唯一正确的协议。Protocol 作为用户可填写的字段是冗余的——填对了等于没填，填错了只会造成运行时错误。
因此本 PR：
effectiveProtocol）不再读account.protocol，改为protocolForProvider[provider]（OpenCode 例外：仍读绑定账户的 protocol，因为它需要知道目标后端）validateRuntimeProviderBinding）不再做 protocol 匹配——protocol 是 provider 的内在属性，不是需要校验的外部输入Issue #329: manifest v3 sync regressed protocol dropdown and OpenCode provider routing.
Original Requirements
Plan / ADR
Tradeoff
Protocol field remains in the data model for backward compatibility but is no longer consulted for runtime/binding decisions. Full field removal deferred to avoid migration complexity in this PR.
Test Evidence
pnpm --filter @cat-cafe/web test # 1844 passed, 0 failed (266 files)
pnpm -r --if-present run build # Success (api + web)
pnpm lint # Passed (warnings only)
biome check # Passed (warnings only)
Open Questions
resolveEnvFallbackKey(account.protocol)6ae5946a, env fallback removedresolveForClientlegacy protocol read — data passthrough, non-decisional本地 Review: [x] gpt52 已 review 并放行（active-path protocol 退场确认）
云端 Review: [ ] PR 创建后在 comment 中触发