Skip to content

feat(providers): Concentrate as a first-class opt-in BYOK Responses gateway - #5725

Open
Hmbown wants to merge 4 commits into
mainfrom
feat/concentrate-provider-20260829
Open

feat(providers): Concentrate as a first-class opt-in BYOK Responses gateway#5725
Hmbown wants to merge 4 commits into
mainfrom
feat/concentrate-provider-20260829

Conversation

@Hmbown

@Hmbown Hmbown commented Aug 29, 2026

Copy link
Copy Markdown
Owner

Summary

Adds Concentrate (concentrate) as a first-class, opt-in, BYOK provider — the OpenAI Responses-compatible gateway at https://api.concentrate.ai/v1 — inside the existing provider authorities (no parallel secret store, router, or runtime). Mirrors the Eden AI aggregator addition across the registry, config tables, env overrides, secrets, TUI wiring, docs, web facts, and goldens.

  • Identity: ProviderKind::Concentrate (+ concentrate-ai/concentrate_ai/concentrateai), hand-written impl Provider with WirePolicy::Fixed(Responses), default model deepseek-v4-pro, env CONCENTRATE_API_KEY / CONCENTRATE_BASE_URL / CONCENTRATE_MODEL, own secret-store slot.
  • Model ids pass through: plain catalog id → the gateway picks the upstream; provider/model pins it; only the gateway's own concentrate/ namespace is stripped so concentrate/auto reaches its router (Codewhale's bare auto stays the resolver sentinel).
  • Wire: documented fields only (model, input, stream, max_output_tokens, tools/tool_choice/parallel_tool_calls, reasoning.effort); system prompt as a leading system input item; typed response.* SSE via the existing parser, ending on response.completed (no [DONE]).
  • Catalog: unauthenticated GET /v1/models through the existing named-gateway live-catalog path (rows provider-scoped, unclaimed).
  • Errors (provider-neutral): 402 "insufficient credits/funds" → quota class; flat {"error","message"} bodies surface both halves.
  • Commercial boundary preserved: Concentrate's Terms of Service forbid resale / white-label / service-bureau use without written consent; its AUP forbids key sharing; pricing says "No platform markup on tokens". This PR ships no Codewhale-owned key, no stored customer key, no default/managed routing, and no markup. A saved/env key is bound to the official base URL and is never sent elsewhere (a custom endpoint gets a key only when base_url and api_key are both in [providers.concentrate]). Hosted-lane activation stays gated on written consent, terms, and billing approval (ops evidence concentrate-gateway-20260829/CHECKLIST.md).
  • Keyless dogfood: scripts/concentrate-selftest.sh boots scripts/concentrate-stub.py (the documented contract on loopback) and drives the real codewhale exec --auto --output-format stream-json path through it, asserting URL, bearer header, verbatim model, only-documented fields, system-first input, the completed-turn receipt, and the wrong-key 401. No network call leaves the machine.

Contract sources (fetched 2026-08-29): introduction, request parameters, streaming, errors, list models, health.

No-Issue: founder-requested provider addition; no tracking issue exists.

Testing

Local, hosted-equivalent flags (RUSTFLAGS=-Dwarnings, RUST_MIN_STACK=16MiB, cargo nextest … --all-features --locked --profile ci), summary in the ops evidence:

  • cargo fmt --all -- --check: clean
  • codewhale-config: 624 run / 624 passed / 1 skipped — incl. new concentrate_resolves_named_responses_gateway_and_environment_overrides (aliases, metadata, fixed Responses wire, secret slot, env + config resolution, and the credential-scope rule that an env key is never sent to a non-official base URL) and concentrate_passes_ids_through_and_strips_only_its_own_namespace (plain / provider/model verbatim, concentrate/autoauto, bare auto → provider default, all on the Responses protocol); goldens regenerated (golden_route_ids.txt, providers-export.golden.json); count assertions 47→48 / 42→43; expected-wire tables updated.
  • codewhale-secrets: 62 run / 62 passed — incl. concentrate_env_aliases_resolve (own slot; no cross-provider bleed).
  • codewhale-tui focused (concentrate | error_taxonomy:: | client::responses::tests:: | llm_client:: | config provider tests | edenai|telecomjs|opencode_zen_responses|catalog): 440 run / 440 passed — incl. concentrate_responses_request_matches_the_documented_contract (wiremock: POST /v1/responses, Bearer header, verbatim provider/model, no store/include/instructions/messages, system item first, typed-event SSE without [DONE] assembles text + usage 12/5, official URL maps to /v1/responses), concentrate_error_bodies_surface_verbatim_and_classify (401 → Authentication with "Invalid API key", 402 → quota with "insufficient credits", 400 → InvalidInput with "Invalid model name"), concentrate_live_catalog_is_provider_scoped_and_marks_the_default, concentrate_responses_body_sends_only_documented_fields, insufficient_credits_classifies_as_rate_limit_not_auth, flat_error_and_message_body_surfaces_both_halves.
  • Keyless self-test PASS ×3 (scripts/concentrate-selftest.sh with the debug CLI): concentrate/auto (wire auto), openai/gpt-5.6-sol, deepseek-v4-pro — health 200, catalog without a key, exactly one bearer-authenticated POST /v1/responses with the verbatim model, stream:true, system item first, only documented fields; stream-json receipt content…turn_usage, session_capture, metadata, done; wrong key → documented 401 → exit 1. Defeat evidence: the first run (env key only) reached the stub with an empty Authorization header, which is the credential-scope rule working; the recipe now writes base_url + api_key into [providers.concentrate] as a BYOK user pointing at a local gateway would.
  • scripts/check-provider-registry.py PASS; cargo clippy for config+secrets and tui (--all-targets --all-features, CI allow list) clean; node web/scripts/derive-facts.mjs (providers 46) + check-facts OK; dead-code budget PASS (444/444); git diff --check clean.

Not done: no live Concentrate canary (needs a key and founder-gated spend — the live gateway was never contacted beyond the public, unauthenticated /v1/models read used to pin the catalog shape); Windows-target compile is hosted-CI-only on this macOS host.

  • cargo fmt --all -- --check
  • cargo clippy on the changed crates, all targets, all features, CI allow list
  • cargo test --workspace --all-features --locked (hosted CI; locally the changed crates' suites passed as above)

🤖 Generated with Claude Code

…ateway

Adds `concentrate` (aliases `concentrate-ai`, `concentrate_ai`,
`concentrateai`) inside the existing provider authorities — no parallel
secret store, router, or runtime:

- Identity/metadata: `ProviderKind::Concentrate`, hand-written
  `impl Provider` with `WirePolicy::Fixed(WireFormat::Responses)` (the
  gateway documents the Responses API as its production surface),
  default base URL `https://api.concentrate.ai/v1`, default model
  `deepseek-v4-pro`, env `CONCENTRATE_API_KEY` (+ `CONCENTRATE_BASE_URL`,
  `CONCENTRATE_MODEL`), its own secret-store slot, credential help.
- Routing: aggregator-class pass-through. A plain catalog id lets the
  gateway choose the upstream provider, `provider/model` pins one, and
  only the gateway's own `concentrate/` namespace is stripped so
  `concentrate/auto` reaches its `auto` router while Codewhale's bare
  `auto` stays the resolver sentinel (provider default).
- Wire: the Responses body carries only documented fields — `model`,
  `input`, `stream`, `max_output_tokens`, `tools`/`tool_choice`/
  `parallel_tool_calls`, `reasoning.effort` — with the system prompt as a
  leading `system` input item (`instructions`, `store`, `include`, and
  `reasoning.summary` are absent from the gateway's parameter reference).
  Streaming rides the existing typed `response.*` SSE parser and ends on
  `response.completed` without a `[DONE]` sentinel.
- Catalog: the unauthenticated `GET /v1/models` (OpenAI list shape) joins
  the named-gateway live-catalog path; rows stay provider-scoped and
  unclaimed.
- Errors (provider-neutral): a 402 "insufficient credits/funds" body now
  classifies as quota (RateLimit) instead of falling through, and a flat
  `{"error":"<class>","message":"<detail>"}` body surfaces both halves
  in the TUI sanitizer instead of the class alone.
- Registry parity: `ProviderKind::ALL` 42→43, registry 47→48, golden
  route ids + providers-export golden regenerated,
  `scripts/check-provider-registry.py` manual-impl allowlist, web facts
  label maps + `facts.generated.ts` (providers 45→46), docs rows in
  PROVIDERS.md (+ a Concentrate Notes section) and CONFIGURATION.md,
  CHANGELOG.

Commercial boundary, preserved in code and docs: BYOK only. Concentrate's
Terms of Service forbid resale, white-label, and service-bureau use
without written consent and its AUP forbids key sharing, so there is no
Codewhale-owned key, no stored customer key, no default or managed
routing, and no markup; any hosted lane is gated on written consent,
terms, and billing approval (ops evidence
concentrate-gateway-20260829/CHECKLIST.md). A saved or environment
Concentrate key is bound to the official base URL and is never sent to
any other endpoint — a custom endpoint receives a key only when
`base_url` and `api_key` are both written into `[providers.concentrate]`.

Keyless dogfood: `scripts/concentrate-selftest.sh` boots
`scripts/concentrate-stub.py` (the documented contract on loopback:
`/v1/responses/health`, unauthenticated `/v1/models`, typed-SSE
`/v1/responses`, documented error bodies) and drives the real
`codewhale exec --auto --output-format stream-json` path through it,
asserting the URL, bearer header, verbatim model, only-documented fields,
system item first, the completed-turn receipt, and the wrong-key 401.
No network call leaves the machine; no account exists in the loop.

Contract sources (fetched 2026-08-29):
https://concentrate.ai/docs/api-reference/introduction
https://concentrate.ai/docs/api-reference/endpoint/request-parameters
https://concentrate.ai/docs/api-reference/endpoint/streaming
https://concentrate.ai/docs/api-reference/endpoint/errors
https://concentrate.ai/docs/api-reference/endpoint/list-models
https://concentrate.ai/docs/legal/terms-of-service

Verification: local, this host, CI flags (RUSTFLAGS=-Dwarnings, RUST_MIN_STACK=16MiB,
nextest --profile ci, --all-features, --locked): fmt clean; codewhale-config
624 run / 624 passed / 1 skipped (incl. new concentrate_resolves_… and
concentrate_passes_ids_through_and_strips_only_its_own_namespace, regenerated
goldens); codewhale-secrets 62/62; codewhale-tui focused (concentrate +
error_taxonomy + client::responses + llm_client + provider config + catalog
filters) 440 run / 440 passed; debug CLI built; scripts/concentrate-selftest.sh
PASS ×3 (concentrate/auto → wire `auto`, openai/gpt-5.6-sol, deepseek-v4-pro);
scripts/check-provider-registry.py PASS; clippy config+secrets and tui
(--all-targets --all-features, CI allow list) clean; web facts regenerated
(providers 46) + check-facts OK; dead-code budget PASS; git diff --check
clean.

Not done: no live Concentrate canary (needs a key and founder-gated spend);
Windows-target compile is hosted-CI-only on this host.

No-Issue: provider addition requested by the founder; no tracking issue exists.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@codewhale-agent codewhale-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Codewhale reviewed the changed lines in this pull request. This is a comment-only review, not an approval.

Left 2 comments on changed lines.

{"blocker":2,"concern":0,"nit":0,"model":"skipped","policy":"default"}

provider = "concentrate"

[providers.concentrate]
api_key = "concentrate-config-key"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocker This changed line looks like a hardcoded secret.


[providers.concentrate]
base_url = "http://127.0.0.1:8790/v1"
api_key = "stub-key"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocker This changed line looks like a hardcoded secret.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 29, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-08-29T23:58:22.194136Z 2860a57 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@claude

claude Bot commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 3m 17s —— View job


Claude finished reviewing this PR

Todo list

Summary

Well-scoped, mirrors the existing Eden AI aggregator pattern closely and stays inside the existing provider authorities (registry, secrets, resolver, client) rather than adding a parallel system. I could not execute cargo test/cargo clippy in this sandbox (no tool permission for cargo), so this is a source-level review; the PR body's own local run numbers are the executed evidence.

Findings

1. Stale doc count in docs/PROVIDERS.md:45 (minor correctness/docs bug)

The canonical provider IDs are the 42 entries of `ProviderKind::ALL`

ProviderKind::ALL was bumped to 43 entries by this PR (crates/config/src/provider_kind.rs:250, confirmed by the PR's own new test crates/config/src/tests.rs:5165: assert_eq!(ProviderKind::ALL.len(), 43)), and the ID list right below the sentence on line 45 was correctly updated to include concentrate — but the "42" count itself wasn't bumped to 43. Same pattern to double check: docs/CONFIGURATION.md:424-426 lists the id but doesn't state a count, so it's unaffected.

Fix this →

2. Minor: codex_responses_reasoning_effort reused as-is for Concentrate's reasoning.effort (crates/tui/src/client/responses.rs:936-941)

Concentrate shares OpenAI Codex's low/medium/high/xhigh effort mapper rather than having its own tier table (unlike DeepSeek, which gets deepseek_effort_tier_or_default). Reasonable reuse, but worth a one-line comment confirming Concentrate's docs actually accept xhigh/medium as spelled — the PR's contract-source list doesn't include a reasoning-tier reference. Not blocking; low risk since it degrades to a documented value either way.

What's solid

  • Route/provider-selection safety (EPIC v0.8.65 EPIC: Separate provider facts, model facts, offerings, and route resolution #2608 invariant): respected. The concentrate/ prefix stripping in crates/config/src/route/resolver.rs:346-356 only runs inside scope_selector, which is reached after provider_kind is already fixed by explicit user/config selection — it never infers a provider switch from a provider/model-shaped string. Upstream prefixes (openai/gpt-5.6-sol) are left verbatim and only pin the upstream inside the already-selected gateway, matching the Eden AI precedent.
  • Credential-scope rule is generic, not new/duplicated logicprovider_base_url_is_official (crates/config/src/lib.rs:4880) and the existing resolve_runtime_options env-key-scoping path handle Concentrate the same way as every other provider; the new concentrate_resolves_named_responses_gateway_and_environment_overrides test (crates/config/src/tests.rs) exercises it directly and the assertion that an env key doesn't leak to a non-official base URL checks out against the shared implementation.
  • Generic error-body fix (extract_json_error_message, crates/tui/src/llm_client/mod.rs:790-805) is scoped correctly — it only fires when both /error and /message are top-level strings and differ, so it can't hijack the existing nested {"error":{"message":...}} shape used by other providers (verified by the added flat_error_and_message_body_surfaces_both_halves test, which also checks the nested case is untouched).
  • 402 → RateLimit reclassification (crates/tui/src/error_taxonomy.rs:370-376) is a reasonable generic heuristic addition (prepaid-gateway quota condition), with keyword ordering that doesn't collide with the existing InvalidInput/Authentication branches for the documented Concentrate bodies.
  • concentrate-stub.py / concentrate-selftest.sh: loopback-only, isolated HOME/config, no real credentials, no network egress — good BYOK-safe test harness design.
  • Registry, secrets, config env-override, and TUI wiring changes are mechanical, one-to-one mirrors of the Eden AI addition; no dead code or needless duplication spotted.

Branch: feat/concentrate-provider-20260829

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2860a5710f

ℹ️ 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".

Comment thread crates/tui/src/client.rs
&& let Some(kind) = provider.kind()
if matches!(
provider,
ApiProvider::Telecomjs | ApiProvider::Edenai | ApiProvider::Concentrate

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 Do not treat the public catalog as key verification

For Concentrate, GET /v1/models is explicitly unauthenticated (docs/PROVIDERS.md:670), so this request returns 2xx even when api_key is invalid. The guided provider setup consequently records a successful observed probe and advances to model selection for any nonempty key, only failing on the first POST /responses; special-case this provider as an unobserved probe or validate against an authenticated endpoint.

Useful? React with 👍 / 👎.

Comment on lines +135 to +138
if "ok from the concentrate stub" in text:
print("ok: reply text reached the CLI output")
else:
print("note: reply text not found in `content` events (types above are the receipt); raw events kept for inspection")

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 Fail the self-test when reply content is missing

If the Responses parser or stream-json renderer drops every content event while still emitting a done receipt, this branch merely prints a note and the script exits successfully. That means the advertised end-to-end assertion that the stub reply reached CLI output can regress while CONCENTRATE SELFTEST PASS is still reported; make the missing expected text an assertion failure.

AGENTS.md reference: AGENTS.md:L83-L85

Useful? React with 👍 / 👎.

Comment on lines +373 to +376
|| lower.contains("insufficient credits")
|| lower.contains("insufficient funds")
|| lower.contains("payment required")
|| lower.contains("http 402")

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 Classify 402 exhaustion in the typed HTTP path

A real Concentrate 402 is converted by send_with_retry through LlmError::from_http_response before this string classifier is consulted, and has_explicit_quota_phrase recognizes neither “insufficient funds” nor “insufficient credits.” The resulting downcastable LlmError::Other takes precedence in envelope_for_llm_error, so the UI reports an Internal, recoverable failure instead of the quota category promised here; add these phrases to the typed quota-evidence path (the new test currently hides this by reclassifying only the formatted string).

Useful? React with 👍 / 👎.

Hosted `Test (ubuntu-latest)` on #5725 failed exactly one test out of
13,958: `cli_provider_helpers_follow_config_metadata` still asserted the
pre-Concentrate registry sizes (47 full / 42 catalog). The config crate's
twin assertion was updated in the parent commit; this mirrors it. The rest
of the test (env vars and secret-store slot per registry kind) already held
for `concentrate`. Local: codewhale-cli 276 run / 276 passed.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Hmbown

Hmbown commented Aug 30, 2026

Copy link
Copy Markdown
Owner Author

Hosted Test (ubuntu-latest) at 2860a5710 failed one test of 13,958: codewhale-cli tests::cli_provider_helpers_follow_config_metadata still asserted the pre-Concentrate registry sizes (47 full / 42 catalog). Fixed in 689f52a61e (48 / 43, mirroring the config crate's twin assertion). Local: codewhale-cli 276 run / 276 passed; fmt clean. Hosted CI re-runs at that head.

@codewhale-agent codewhale-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Codewhale reviewed the changed lines in this pull request. This is a comment-only review, not an approval.

Left 2 comments on changed lines.

{"blocker":2,"concern":0,"nit":0,"model":"skipped","policy":"default"}

provider = "concentrate"

[providers.concentrate]
api_key = "concentrate-config-key"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocker This changed line looks like a hardcoded secret.


[providers.concentrate]
base_url = "http://127.0.0.1:8790/v1"
api_key = "stub-key"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocker This changed line looks like a hardcoded secret.

@claude

claude Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 1m 34s —— View job


Claude finished reviewing this PR

Todo list

  • Gather context (PR description, diff, prior review comments)
  • Check what changed in the latest commit (689f52a) relative to the prior review round
  • Verify whether previously flagged issues were addressed
  • Review the new commit's diff for correctness/safety/reuse/tests/security
  • Post final review summary

Summary

This synchronize only added one commit, 689f52a, which is a mechanical, correct fix: hosted CI's cli_provider_helpers_follow_config_metadata test still asserted the pre-Concentrate registry sizes (47/42); this bumps them to 48/43 to match the config crate's twin assertion from the parent commit. Nothing else changed. I re-verified this diff plus re-checked the three issues raised in the prior Codex review round — none of them were touched by this push, so they still stand:

1. (Unresolved, real bug) 402 quota classification never reaches the typed error pathcrates/tui/src/llm_client/mod.rs:759 (has_explicit_quota_phrase) and the is_explicit_quota_code/marker checks it's paired with don't recognize "insufficient credits"/"insufficient funds"/402, so has_explicit_quota_evidence returns false for Concentrate's documented 402 body. LlmError::from_http_response (llm_client/mod.rs:462) then falls through its match status with no 402 arm to _ => LlmError::Other(...), which error_taxonomy.rs:325 maps to Internal/Error/recoverable: true — the opposite of the quota/RateLimit category the PR claims. The new insufficient_credits_classifies_as_rate_limit_not_auth test (error_taxonomy.rs:603) only exercises classify_error_message (the string heuristic added at error_taxonomy.rs:373-376), never the typed LlmError::from_http_response boundary that send_with_retry actually uses (client.rs:2816, 2897), so the test passes while the real request path still misclassifies. Fix by adding a 402 if has_explicit_quota_evidence(body) is false arm (or teaching has_explicit_quota_phrase/is_explicit_quota_code the "insufficient credits"/"insufficient funds" phrasing) so a real Concentrate 402 lands in QuotaExhausted, not Other.

2. (Unresolved) Public, unauthenticated /v1/models used as key verificationcrates/tui/src/client.rs:1818-1821 still treats a Concentrate GET /v1/models 2xx as a successful key-verification probe. Per docs/PROVIDERS.md:670, that endpoint doesn't require auth, so any nonempty (including wrong) key currently "verifies" during guided setup and only fails on the first real POST /responses.

3. (Unresolved) scripts/concentrate-selftest.sh:136-138 still degrades a missing reply-text match to a printed note: instead of failing the script, so a regression that drops all content events (while still emitting done) would still print CONCENTRATE SELFTEST PASS.

4. (Unresolved, minor) docs/PROVIDERS.md:45 still says "the 42 entries of ProviderKind::ALL" — the array is now 43 (matching this same commit's own ProviderKind::ALL.len(), 43 assertion).

None of these are regressions from 689f52a — they were flagged before this push and remain open. The count-bump commit itself is uncontroversial and matches the failing hosted-CI test it targets.

Not a real issue: the earlier Codewhale-agent "hardcoded secret" flags on crates/config/src/tests.rs:4991,5047 (concentrate-config-key, concentrate-env-key, stub-key) are synthetic test fixtures, consistent with the existing convention used for every other provider's tests (e.g. nim-env-key, mistral-config-key, go-config-key, telecom-config-key elsewhere in the same file).
· Branch: feat/concentrate-provider-20260829

@Hmbown

Hmbown commented Aug 30, 2026

Copy link
Copy Markdown
Owner Author

Hosted Test (macos-latest) at 689f52a61: 13,964 run, 13,958 passed, 6 failed — all six are the inherited sandbox::read_guard macOS failures on origin/main 20e957406 (symlink_chains_resolve_to_the_denied_target, symlink_pointing_into_a_denied_tree_is_refused_by_its_target, symlinked_parent_directory_is_refused, relative_read_from_inside_a_denied_tree_is_refused, dot_dot_through_a_symlink_is_applied_after_symlink_resolution, denial_message_names_the_rule_without_echoing_the_resolved_secret_path). Nothing Concentrate-related failed. #5724 fixes exactly these and is green/CLEAN at c4bbb357; after it lands I'll re-merge main here for a clean exact-head macOS leg. Test (ubuntu-latest) passed at this head.

@Hmbown

Hmbown commented Aug 30, 2026

Copy link
Copy Markdown
Owner Author

Matrix settled at 689f52a61: Test (windows-latest) 13,641 run / 13,640 passed, the one failure is the inherited sandbox::read_guard::tests::root_parent_traversal_does_not_escape_above_root from origin/main 20e957406 (fixed by #5724). Net at this head: Ubuntu green; macOS 6 inherited failures; Windows 1 inherited failure; Lint / Safety gate / Version drift / Integrations / npm smoke / DCO / GitGuardian / Analyze ×4 / CodeQL / OHOS check / Claude, Devin, Codewhale reviews all green. Zero Concentrate-related failures. Next step after #5724 lands: re-merge main here.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>

@codewhale-agent codewhale-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Codewhale reviewed the changed lines in this pull request. This is a comment-only review, not an approval.

Left 2 comments on changed lines.

{"blocker":2,"concern":0,"nit":0,"model":"skipped","policy":"default"}

provider = "concentrate"

[providers.concentrate]
api_key = "concentrate-config-key"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocker This changed line looks like a hardcoded secret.


[providers.concentrate]
base_url = "http://127.0.0.1:8790/v1"
api_key = "stub-key"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocker This changed line looks like a hardcoded secret.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>

# Conflicts:
#	CHANGELOG.md
#	crates/tui/CHANGELOG.md

@codewhale-agent codewhale-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Codewhale reviewed the changed lines in this pull request. This is a comment-only review, not an approval.

Left 2 comments on changed lines.

{"blocker":2,"concern":0,"nit":0,"model":"skipped","policy":"default"}

provider = "concentrate"

[providers.concentrate]
api_key = "concentrate-config-key"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocker This changed line looks like a hardcoded secret.


[providers.concentrate]
base_url = "http://127.0.0.1:8790/v1"
api_key = "stub-key"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocker This changed line looks like a hardcoded secret.

@claude

claude Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Claude finished @Hmbown's task in 1m 8s —— View job


Claude finished reviewing this PR

Todo list

  • Gather context (PR description, diff, prior review rounds)
  • Check what changed since the last reviewed commit (689f52a)
  • Re-verify the previously flagged unresolved issues (402 typed-path classification, /v1/models key verification, selftest silent-degrade, docs count)
  • Review any new diff content for correctness/safety/reuse/tests/security
  • Post final review summary

Summary

This synchronize is the merge commit 823cf7b (Merge remote-tracking branch 'origin/main' into HEAD), following the plan stated in the PR thread ("Next step after #5724 lands: re-merge main here"). I diffed 689f52a (the last commit reviewed) against 823cf7b restricted to the Concentrate-touching paths (crates/tui/src/client.rs, crates/tui/src/error_taxonomy.rs, crates/tui/src/llm_client/mod.rs, scripts/concentrate-selftest.sh, crates/config/src/route/resolver.rs, crates/config/src/provider.rs, crates/config/src/lib.rs, crates/config/src/provider_defaults.rs, docs/PROVIDERS.md) — zero lines of Concentrate-related code changed. Everything that changed is unrelated upstream work pulled in from main: Moonshot CN base-URL/web-search handling (provider.rs, lib.rs, provider_defaults.rs, resolver.rs, a docs/PROVIDERS.md edit to the Moonshot row only), plus large unrelated surfaces (localization, sandbox hardening, subagent gating, TUI views/goldens, cli/src/update.rs).

Since nothing Concentrate-specific moved, the four issues raised in the prior review round (00:04 UTC, against 689f52a) are still open at this head, verified directly against the current file contents:

1. (Unresolved) 402 quota classification never reaches the typed error pathcrates/tui/src/llm_client/mod.rs (has_explicit_quota_phrase) still doesn't recognize "insufficient credits"/"insufficient funds"/402, so a real Concentrate 402 still falls through LlmError::from_http_response's match status (no 402 arm) to LlmError::Other, which error_taxonomy.rs maps to Internal/recoverable — the opposite of the quota category the PR claims. The added insufficient_credits_classifies_as_rate_limit_not_auth test only exercises the string heuristic (classify_error_message), not the typed boundary send_with_retry actually uses.

2. (Unresolved) Public, unauthenticated /v1/models still used as key verificationcrates/tui/src/client.rs around line 1818 still treats a 2xx from Concentrate's documented-unauthenticated GET /v1/models as a successful key-verification probe, so guided setup "verifies" any nonempty (including wrong) key and only fails on the first real POST /responses.

3. (Unresolved) scripts/concentrate-selftest.sh still degrades a missing reply-text match to a printed note: rather than a failing assertion — a regression that drops all content events while still emitting done would still print CONCENTRATE SELFTEST PASS.

4. (Unresolved, minor) docs/PROVIDERS.md:45 still reads "the 42 entries of ProviderKind::ALL"; the array is 43 as of this same PR's own test assertion. (Confirmed the merge only touched the unrelated Moonshot row in this file, not line 45.)

Not re-raising: the two codewhale-agent "hardcoded secret" blockers on crates/config/src/tests.rs:4991,5047 are synthetic test fixture strings (concentrate-config-key, concentrate-env-key, stub-key), consistent with every other provider's test convention in the same file (e.g. nim-env-key, mistral-config-key) — not real credentials.

No new correctness, provider/route-safety, reuse, test-coverage, or security findings from this merge itself — it's a clean, uneventful re-sync with main.

Branch: feat/concentrate-provider-20260829

@codewhale-agent codewhale-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Codewhale review

Adds Concentrate as a first-class, opt-in, BYOK OpenAI Responses-compatible provider across the registry, config, secrets, TUI, docs, web facts, and scripts. The implementation is comprehensive and includes focused tests for wire contract, model passthrough, error classification, and credential scoping. Review found a missing serde alias for concentrateai in the config table definitions and a redundant env override block.

Findings

  • [WARNING] Missing concentrateai alias on Concentrate config table serde attributes (crates/config/src/lib.rs:475)
    ProviderKind::Concentrate, the provider aliases() list, and secrets::env_for all accept concentrateai as an alias, but the ProvidersToml.concentrate field in crates/config/src/lib.rs and the ProvidersConfig.concentrate field in crates/tui/src/config.rs only declare concentrate-ai and concentrate_ai. A user selecting provider = "concentrateai" and placing credentials under [providers.concentrateai] will have that table silently ignored (serde default ignores unknown fields), leaving the provider unconfigured. Add the missing alias to both serde attributes and add a regression test that parses [providers.concentrateai].
  • [INFO] Duplicate base URL override block for Concentrate (crates/tui/src/config.rs:8580)
    In crates/tui/src/config.rs, the apply_env_overrides_unlocked function already sets the Concentrate base URL through the provider_env_base_url_override match around line 8316. A second inline if matches!(config.api_provider(), ApiProvider::Concentrate) && ... block starting around line 8580 repeats the same assignment with the same env var. This is redundant and can be removed to reduce maintenance burden.

Suggestions

  • crates/config/src/lib.rs:475 — Add the concentrateai alias to the ProvidersToml.concentrate serde attribute so [providers.concentrateai] tables are recognized and match the accepted provider aliases.

            alias = "concentrate-ai",
            alias = "concentrate_ai",
            alias = "concentrateai"
    
  • crates/tui/src/config.rs:3919 — Add the concentrateai alias to the ProvidersConfig.concentrate serde attribute so the TUI config table accepts the same alias as the provider enum and secrets layer.

        #[serde(default, alias = "concentrate-ai", alias = "concentrate_ai", alias = "concentrateai")]
    

Assessment

The PR is generally high quality with thorough testing and careful adherence to Concentrate's documented contract. The missing concentrateai config alias is a correctness gap that could silently drop user configuration and should be fixed before merge; the redundant env override block is minor cleanup. With those addressed, the addition looks solid.


Advisory review by Codewhale (codewhale review --pr 5725 --post, head 823cf7b1866425884f74cee2ddc805b5bc0f86d2). Line-specific findings are also posted as inline review comments; mechanical fixes arrive as committable suggestions you can apply from the Files tab. CODEOWNERS approval still governs merge.

Comment thread crates/config/src/lib.rs
#[serde(
default,
skip_serializing_if = "ProviderConfigToml::is_empty",
alias = "concentrate-ai",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[WARNING] Missing concentrateai alias on Concentrate config table serde attributes

ProviderKind::Concentrate, the provider aliases() list, and secrets::env_for all accept concentrateai as an alias, but the ProvidersToml.concentrate field in crates/config/src/lib.rs and the ProvidersConfig.concentrate field in crates/tui/src/config.rs only declare concentrate-ai and concentrate_ai. A user selecting provider = "concentrateai" and placing credentials under [providers.concentrateai] will have that table silently ignored (serde default ignores unknown fields), leaving the provider unconfigured. Add the missing alias to both serde attributes and add a regression test that parses [providers.concentrateai].

Comment thread crates/tui/src/config.rs
@@ -8555,6 +8580,16 @@ fn apply_env_overrides_unlocked(config: &mut Config, policy: ConfigEnvironmentPo
.edenai

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[INFO] Duplicate base URL override block for Concentrate

In crates/tui/src/config.rs, the apply_env_overrides_unlocked function already sets the Concentrate base URL through the provider_env_base_url_override match around line 8316. A second inline if matches!(config.api_provider(), ApiProvider::Concentrate) && ... block starting around line 8580 repeats the same assignment with the same env var. This is redundant and can be removed to reduce maintenance burden.

Comment thread crates/config/src/lib.rs
Comment on lines +475 to +476
alias = "concentrate-ai",
alias = "concentrate_ai"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add the concentrateai alias to the ProvidersToml.concentrate serde attribute so [providers.concentrateai] tables are recognized and match the accepted provider aliases.

Suggested change
alias = "concentrate-ai",
alias = "concentrate_ai"
alias = "concentrate-ai",
alias = "concentrate_ai",
alias = "concentrateai"

Comment thread crates/tui/src/config.rs
@@ -3908,6 +3917,9 @@ pub struct ProvidersConfig {
/// Eden AI — OpenAI-compatible AI gateway (aggregator).
#[serde(default, alias = "eden-ai", alias = "eden_ai")]
pub edenai: ProviderConfig,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Add the concentrateai alias to the ProvidersConfig.concentrate serde attribute so the TUI config table accepts the same alias as the provider enum and secrets layer.

Suggested change
pub edenai: ProviderConfig,
#[serde(default, alias = "concentrate-ai", alias = "concentrate_ai", alias = "concentrateai")]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant