Integrate dev → main: inference routing (upstream router + remote chat) + OpenAPI describability + queued API work

[Snider]

Summary

Brings dev to main. Three features this round (each spec→plan→TDD-built with two-stage review per task, on top of the reused router transport), plus previously-queued dev work.

1. WithUpstreamRouter — selector-keyed reverse proxy (Go engine)

• Load-balances each request across a runtime-mutable pool of HTTP upstreams, keyed by a pluggable selector (default JSON model). Weighted round-robin + passive failover (cooldown); hybrid streaming; decision hook; composes with TransformerIn/Out.
• UpstreamRegistry (copy-on-write, lock-free reads); block-by-default SSRF validation at registration with AllowPrivateUpstreams(cidrs...) opt-in.

2. Chat-completions remote backend + format adapters (Go engine)

• WithChatCompletionsRemote(reg, …) makes /v1/chat/completions serve remote models alongside the local in-process path — local-first dispatch (ModelResolver.Knows) → remote via the router transport.
• Passthrough by default; per-model ChatFormatAdapter for non-OpenAI upstreams (Ollama-native + Anthropic, incl. streaming transcoders).
• Off-loopback opt-in gated by a request-validated bearer (constant-time, fail-closed) — Cerberus-reviewed SECURE.

3. OpenAPI describability for the inference surface (Go engine)

• The inference surface is now visible to /v1/openapi.json + SDK gen: chat-completions surfaces for local / remote / hybrid (honest x-chat-completions-enabled), and each WithUpstreamRouter mounted path appears as a minimal POST proxy item — deduped so real typed items (chat / spec / swagger / group) always win.
• Proxy items declare bearerAuth for the non-public case so generated SDK clients authenticate correctly.

Also included (previously on dev)

• PHP: webhooks, API-key hardening, MCP API + OpenAPI docs
• Sonar sweep (string-literal dedup, long-param bundling)
• core-family dependency bumps (core/go v0.10.3)
• opt-in strict bind + bearer-on-public

Test Plan

• GOWORK=off go test ./... green — api, cmd/gateway, pkg/provider, pkg/stream
• -race clean (full root suite ~1693 tests); go vet ./ clean; gofmt -l clean
• go build ./... + cmd/gateway build OK
• gosec: no new findings (4 pre-existing in untouched files; the justified #nosec G107 upstream-dispatch annotation)
• Per-task adversarial review (spec + code-quality), Cerberus security pass on the auth gate, holistic final reviews → MERGE-READY

Notes

• Composition limitation (documented): post-c.Next() response-header middleware (e.g. ApiSunset) doesn't apply to streamed proxy responses; pre-handler middleware composes normally.
• Follow-ups (non-blocking): bearerAuthMiddleware constant-time compare (pre-existing Low); generic streaming-transcoder registry + tool-calling translation; real per-path schemas for generic router paths via consumer RouteDescriptions; orphaned proxy tag when all router paths dedupe away.

🤖 Generated with Claude Code
Co-Authored-By: Virgil virgil@lethean.io

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds an upstream reverse-proxy router (registry, balancer, transport, Gin handler), introduces strict bind validation/options, standardises headers/messages/error keys, updates OpenAPI generation, and broadly refactors tests/docs across Go and PHP; bumps a Go dependency and several submodules.

Changes

Core API and Docs

Layer / File(s)	Summary
Upstream router feature go/upstream_*, go/upstream_router*.go, docs/superpowers/*	Implements selector-keyed reverse proxy with registry validation, weighted RR + cooldown, failover transport, in/out transforms, and Gin mounting; adds plans/specs and tests.
Strict bind enforcement go/api.go, go/options.go, go/strict_bind_test.go	Adds validateBind with loopback/public bind rules, exposes options and errors, and tests behaviour including Serve short-circuit.
Constants and OpenAPI updates go/string_constants.go, go/openapi*.go, go/swagger*.go	Standardises header/MIME/message constants and refactors OpenAPI response/request media keys and descriptions.
Tests refactor to shared constants go/*_test.go	Rewrites many tests to use shared path/header/format constants and updated messages; behaviour unchanged.
PHP constants and controller/service tweaks php/src/**	centralises strings/paths, minor signature changes (parameterless docs controller), SSRF/message constant reuse, regex and arg-name normalisation; tests updated accordingly.
Dependencies and submodules go/go.mod, external/*	Bumps dappco.re/go and updates submodule SHAs.

Sequence Diagram(s)

[coderabbitai]

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

php/src/Api/Tests/Feature/RateLimitTest.php (1)

607-611: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Drop the unused foreach key variable.

$_tier is unused and triggers the PHPMD UnusedLocalVariable warning. Iterate values directly to keep the test clean.

Suggested patch

-        foreach ($tiers as $_tier => $tierConfig) {
+        foreach ($tiers as $tierConfig) {
             $this->assertArrayHasKey('limit', $tierConfig);
             $this->assertArrayHasKey('window', $tierConfig);
             $this->assertArrayHasKey('burst', $tierConfig);
         }

🤖 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 `@php/src/Api/Tests/Feature/RateLimitTest.php` around lines 607 - 611, The
foreach in RateLimitTest.php currently declares an unused key variable ($_tier)
causing a PHPMD UnusedLocalVariable warning; update the loop to iterate values
only (use foreach ($tiers as $tierConfig)) so you drop the unused $_tier while
keeping the existing assertions that check 'limit', 'window', and 'burst' on
$tierConfig.

Source: Linters/SAST tools

🧹 Nitpick comments (4)

GOAL.md (1)

780-782: 💤 Low value

Format regex patterns as inline code to resolve markdownlint warnings.

The regex pattern ^[a-z][a-zA-Z0-9]*$ contains square brackets that markdownlint interprets as undefined markdown reference links, triggering MD052 warnings. Wrap the regex in backticks to clarify it's a code pattern.

📝 Proposed fix to format regex as inline code

-- `src/php/src/Api/Tests/Feature/SeoReportServiceTest.php:6` — Rename function "dns_get_record" to match the regular expression ^[a-z][a-zA-Z0-9]*$.
+- `src/php/src/Api/Tests/Feature/SeoReportServiceTest.php:6` — Rename function "dns_get_record" to match the regular expression `^[a-z][a-zA-Z0-9]*$`.
-- `src/php/src/Api/Tests/Feature/WebhookDeliveryTest.php:6` — Rename function "dns_get_record" to match the regular expression ^[a-z][a-zA-Z0-9]*$.
+- `src/php/src/Api/Tests/Feature/WebhookDeliveryTest.php:6` — Rename function "dns_get_record" to match the regular expression `^[a-z][a-zA-Z0-9]*$`.
-- `src/php/src/Api/Tests/Feature/WebhookEndpointTest.php:6` — Rename function "dns_get_record" to match the regular expression ^[a-z][a-zA-Z0-9]*$.
+- `src/php/src/Api/Tests/Feature/WebhookEndpointTest.php:6` — Rename function "dns_get_record" to match the regular expression `^[a-z][a-zA-Z0-9]*$`.

🤖 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 `@GOAL.md` around lines 780 - 782, Update the three list items that reference
the function name dns_get_record in the GOAL.md entries for
SeoReportServiceTest, WebhookDeliveryTest, and WebhookEndpointTest so the regex
pattern ^[a-z][a-zA-Z0-9]*$ is wrapped in backticks (inline code) rather than
plain text; keep the function name dns_get_record unchanged but format the regex
as `^[a-z][a-zA-Z0-9]*$` so markdownlint no longer treats the square brackets as
reference links.

Source: Linters/SAST tools

php/src/Api/Controllers/Api/WebhookSecretController.php (1)

22-22: ⚡ Quick win

Complete the string constant extraction for both webhook types.

The refactor extracts 'Webhook endpoint' as RESOURCE_NAME for content webhook methods, but social webhook methods (lines 44, 130, 178, 232) still use the hardcoded literal 'Webhook'. For consistency and maintainability, extract both strings as constants.

♻️ Proposed fix to complete the refactor

-    private const RESOURCE_NAME = 'Webhook endpoint';
+    private const CONTENT_WEBHOOK_RESOURCE_NAME = 'Webhook endpoint';
+    private const SOCIAL_WEBHOOK_RESOURCE_NAME = 'Webhook';

Then update all usages:

• Lines 44, 130, 178, 232: use self::SOCIAL_WEBHOOK_RESOURCE_NAME
• Lines 87, 154, 205, 271: use self::CONTENT_WEBHOOK_RESOURCE_NAME

Also applies to: 87-87, 154-154, 205-205, 271-271

🤖 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 `@php/src/Api/Controllers/Api/WebhookSecretController.php` at line 22, Add two
class constants in WebhookSecretController: CONTENT_WEBHOOK_RESOURCE_NAME =
'Webhook endpoint' and SOCIAL_WEBHOOK_RESOURCE_NAME = 'Webhook'; then replace
all hardcoded literals: where the code currently uses 'Webhook endpoint' use
self::CONTENT_WEBHOOK_RESOURCE_NAME (e.g., in methods handling content webhooks
such as the ones referencing the existing RESOURCE_NAME), and where it uses
'Webhook' for social webhooks replace with self::SOCIAL_WEBHOOK_RESOURCE_NAME
(update all social webhook handlers that currently use the literal). Ensure you
remove or consolidate the old RESOURCE_NAME if redundant and update every usage
site in the class to reference the new constants.

php/tests/Feature/AuthenticationGuideTest.php (1)

8-8: 💤 Low value

Consider using a class constant instead of define() for test-scoped values.

Whilst define() works correctly, modern PHP test suites typically use class constants or local const declarations for better scoping and IDE support. For Pest tests, you can use a file-scoped const declaration instead.

♻️ Optional refactor to modern constant pattern

-define('API_KEYS_PREFIXED_WITH', 'API keys are prefixed with');
+const API_KEYS_PREFIXED_WITH = 'API keys are prefixed with';

This provides the same scoping behaviour with more modern PHP syntax.

🤖 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 `@php/tests/Feature/AuthenticationGuideTest.php` at line 8, Replace the global
define('API_KEYS_PREFIXED_WITH', ...) with a scoped constant to avoid global
leakage: declare a class constant (e.g.,
AuthenticationGuideTest::API_KEYS_PREFIXED_WITH) or a file-scoped const in the
same test file and update any references to use that constant name
(API_KEYS_PREFIXED_WITH) from the AuthenticationGuideTest, ensuring the constant
is declared inside the test class (or file scope) rather than via define().

php/src/Api/Tests/Feature/RateLimitingTest.php (1)

362-362: ⚡ Quick win

Remove unnecessary API key creation.

This line creates an API key for $workspace2 but the result is neither stored nor used. The test only uses $apiKey1 in its requests (lines 365–367, 376), consistent with the comment "Use same API key ID to test shared limit". Creating this unused API key wastes test resources by inserting unnecessary database records.

♻️ Proposed fix

         $apiKey1 = createApiKeyForWorkspace($workspace1);
-        createApiKeyForWorkspace($workspace2);

         // Use same API key ID to test shared limit

🤖 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 `@php/src/Api/Tests/Feature/RateLimitingTest.php` at line 362, The test is
creating an unused API key via createApiKeyForWorkspace($workspace2) which
inserts unnecessary DB state; remove that call so only the relevant api key
($apiKey1) is created/used. Locate the createApiKeyForWorkspace invocation in
RateLimitingTest (near where $workspace2 and $apiKey1 are defined) and delete
the single-line call that creates an API key for $workspace2, ensuring
subsequent assertions still use $apiKey1 to test shared limits.

🤖 Prompt for all review comments with 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.

Inline comments:
In `@docs/superpowers/specs/2026-06-06-upstream-router-design.md`:
- Line 168: Replace the bare triple-backtick fenced code blocks in the document
with language-tagged fences (e.g., ```text) so markdownlint MD040 is satisfied:
update the fence that wraps the block beginning "hits mounted path  (engine
auth/CORS/ratelimit/tracing already ran)" and the fence that wraps the block
starting "go/upstream_registry.go         + _test.go + _example_test.go" to use
```text (or another appropriate language) instead of plain ``` so both instances
are lint-compliant.

In `@go/openapitools.json`:
- Around line 1-7: The $schema path in go/openapitools.json is relative to the
go/ folder and currently points to
./node_modules/@openapitools/openapi-generator-cli/config.schema.json which will
only work if that package is installed under go/node_modules; update the $schema
to a reliable path such as
../node_modules/@openapitools/openapi-generator-cli/config.schema.json or a
stable URL (e.g., the package’s schema on npmjs or raw GitHub) so editor/schema
validation resolves correctly; keep the existing generator-cli.version entry
unchanged unless you want to intentionally bump it.

In `@go/strict_bind_test.go`:
- Around line 179-182: The test comment says it should verify default behavior
on a non-loopback bind but the code creates the engine with
New(WithAddr("127.0.0.1:0")) which is loopback; change the address passed to
WithAddr to a non-loopback ephemeral address (e.g. "0.0.0.0:0") so the Serve
test actually exercises public/non-loopback binding in non-strict mode while
keeping the rest of the test flow (error checks around New and subsequent
Serve/cancellation) unchanged.

In `@go/upstream_balancer.go`:
- Around line 41-45: The per-key state map b.current (variables key and cw) is
unbounded and can grow indefinitely; add an eviction strategy to limit growth:
introduce a maxEntries constant and track insertion order (e.g., an LRU list or
a simple FIFO ring) alongside b.current, and when inserting a new key in the cw
initialization path evict the oldest entries until size <= maxEntries; ensure
modifications to b.current and the order tracker are done under the existing
balancer lock/guard to keep it concurrency-safe and preserve the current
initialization logic for cw.

In `@go/upstream_registry_internal_test.go`:
- Line 70: The test function name TestUpstreamRegistry_Ugly_HeadersDeepCopy does
not end with the required `_Ugly` suffix; rename it to end with `_Ugly` (for
example TestUpstreamRegistry_HeadersDeepCopy_Ugly) and update the function
declaration and any references/imports that reference the test function name so
the test still compiles and runs; ensure only the function identifier is changed
(keep the body and behavior intact).

In `@go/upstream_registry_test.go`:
- Line 82: The test function name
TestUpstreamRegistry_Ugly_ConcurrentWriteSnapshot doesn't end with the required
_Ugly suffix; rename the function to end with _Ugly (for example
TestUpstreamRegistry_ConcurrentWriteSnapshot_Ugly) and update any references to
that symbol in the test file or other tests; ensure the function declaration
(TestUpstreamRegistry_Ugly_ConcurrentWriteSnapshot) is replaced with the new
name so the file conforms to the Go test naming convention.

In `@go/upstream_registry.go`:
- Line 113: The current append uses the original up (variable) so up.Headers can
alias caller memory and reintroduce map races; make a deep-copy of the upstream
before appending to next.pools. Modify the Add site where you call
next.pools[key] = append(cloneUpstreams(next.pools[key]), up) to create an
upstream copy (copy the struct and allocate a new map/slices for Headers) and
append that copy instead; you can add a helper like copyHeaders or cloneUpstream
that performs a deep copy of Headers and use it in place of up (referencing
next.pools, cloneUpstreams, and the up variable).

In `@go/upstream_router_example_test.go`:
- Around line 5-9: Replace the standard fmt usage with the Core wrapper: remove
the "fmt" import and import the Core package (dappco.re/go/core), then change
any fmt.Println calls in the example(s) to core.Println so the example prints
through Core; also prefer dappco.re/go wrapper functions for any formatting used
in this file (e.g., replace fmt.Printf/fmt.Sprintf similarly).

In `@go/upstream_router_test.go`:
- Line 59: Tests are ignoring errors from reg.SetDefault(api.Upstream{URL:
"https://example.com"}) which can hide setup failures; change each ignored call
(the occurrences of reg.SetDefault in this file) to check the returned error and
fail the test on error (e.g., capture err := reg.SetDefault(...); if err != nil
{ t.Fatalf("failed to set default upstream: %v", err) } or use
require.NoError(t, err)) so setup errors surface immediately.

In `@go/upstream_router.go`:
- Line 203: Guard the http.DefaultTransport cast by checking its dynamic type
before cloning: replace the direct cast at cfg.transport =
http.DefaultTransport.(*http.Transport).Clone() with a type assertion (if t, ok
:= http.DefaultTransport.(*http.Transport); ok { cfg.transport = t.Clone() }
else { cfg.transport = &http.Transport{} }) so we don’t panic if
DefaultTransport is a non-*http.Transport; also stop matching oversized-body by
string comparison and instead detect net/http’s MaxBytesError via errors.As
(e.g. var mbe *http.MaxBytesError; if errors.As(err, &mbe) { ... }) and add the
"errors" import if missing.
- Around line 347-348: Replace the fragile string comparison err.Error() ==
"http: request body too large" with a typed check for *http.MaxBytesError using
the project's error-as helper (e.g., core.As or errors.As): attempt to cast err
into a variable like var maxBytesErr *http.MaxBytesError and if the cast
succeeds, call c.AbortWithStatusJSON(http.StatusRequestEntityTooLarge,
Fail(errCodeRequestTooLarge, "Request body exceeds the maximum allowed size"));
update the branch around the error handling in upstream_router.go accordingly so
wrapped or reformatted errors still produce HTTP 413.

In `@php/src/Api/Routes/api.php`:
- Around line 25-26: The route constants API_ROUTE_WORKSPACE and API_ROUTE_ID
are defined unguarded which can trigger "already defined" warnings if the file
is included multiple times; wrap each define call with a defined() check so you
only call define('API_ROUTE_WORKSPACE', '/{workspace}') and
define('API_ROUTE_ID', '/{id}') when the constant is not already defined (use
defined('API_ROUTE_WORKSPACE') and defined('API_ROUTE_ID') to guard them) to
avoid redefinition warnings at bootstrap.

In `@php/src/Api/Tests/Feature/DocumentationControllerTest.php`:
- Line 104: The test calls to DocumentationController::index are passing a
Request even though index() is now parameterless; update the three invocations
in DocumentationControllerTest (currently using
$controller->index(Request::create(...))) to call $controller->index() with no
arguments, and remove any now-unused Request construction/variables in those
test cases so they compile and run.

---

Outside diff comments:
In `@php/src/Api/Tests/Feature/RateLimitTest.php`:
- Around line 607-611: The foreach in RateLimitTest.php currently declares an
unused key variable ($_tier) causing a PHPMD UnusedLocalVariable warning; update
the loop to iterate values only (use foreach ($tiers as $tierConfig)) so you
drop the unused $_tier while keeping the existing assertions that check 'limit',
'window', and 'burst' on $tierConfig.

---

Nitpick comments:
In `@GOAL.md`:
- Around line 780-782: Update the three list items that reference the function
name dns_get_record in the GOAL.md entries for SeoReportServiceTest,
WebhookDeliveryTest, and WebhookEndpointTest so the regex pattern
^[a-z][a-zA-Z0-9]*$ is wrapped in backticks (inline code) rather than plain
text; keep the function name dns_get_record unchanged but format the regex as
`^[a-z][a-zA-Z0-9]*$` so markdownlint no longer treats the square brackets as
reference links.

In `@php/src/Api/Controllers/Api/WebhookSecretController.php`:
- Line 22: Add two class constants in WebhookSecretController:
CONTENT_WEBHOOK_RESOURCE_NAME = 'Webhook endpoint' and
SOCIAL_WEBHOOK_RESOURCE_NAME = 'Webhook'; then replace all hardcoded literals:
where the code currently uses 'Webhook endpoint' use
self::CONTENT_WEBHOOK_RESOURCE_NAME (e.g., in methods handling content webhooks
such as the ones referencing the existing RESOURCE_NAME), and where it uses
'Webhook' for social webhooks replace with self::SOCIAL_WEBHOOK_RESOURCE_NAME
(update all social webhook handlers that currently use the literal). Ensure you
remove or consolidate the old RESOURCE_NAME if redundant and update every usage
site in the class to reference the new constants.

In `@php/src/Api/Tests/Feature/RateLimitingTest.php`:
- Line 362: The test is creating an unused API key via
createApiKeyForWorkspace($workspace2) which inserts unnecessary DB state; remove
that call so only the relevant api key ($apiKey1) is created/used. Locate the
createApiKeyForWorkspace invocation in RateLimitingTest (near where $workspace2
and $apiKey1 are defined) and delete the single-line call that creates an API
key for $workspace2, ensuring subsequent assertions still use $apiKey1 to test
shared limits.

In `@php/tests/Feature/AuthenticationGuideTest.php`:
- Line 8: Replace the global define('API_KEYS_PREFIXED_WITH', ...) with a scoped
constant to avoid global leakage: declare a class constant (e.g.,
AuthenticationGuideTest::API_KEYS_PREFIXED_WITH) or a file-scoped const in the
same test file and update any references to use that constant name
(API_KEYS_PREFIXED_WITH) from the AuthenticationGuideTest, ensuring the constant
is declared inside the test class (or file scope) rather than via define().

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d03c908d-3883-44f6-9d16-bc77ff824d69

📥 Commits

Reviewing files that changed from the base of the PR and between 6b25b9a and dd588cf.

⛔ Files ignored due to path filters (2)

• go.work.sum is excluded by !**/*.sum
• go/go.sum is excluded by !**/*.sum

📒 Files selected for processing (126)

• GOAL.md
• docs/superpowers/plans/2026-06-06-upstream-router.md
• docs/superpowers/specs/2026-06-06-upstream-router-design.md
• external/go
• external/go-inference
• external/go-io
• external/go-log
• go/api.go
• go/api_describable_test.go
• go/api_renderable_test.go
• go/api_test.go
• go/authentik_integration_test.go
• go/authentik_test.go
• go/authz_test.go
• go/bridge.go
• go/bridge_test.go
• go/brotli.go
• go/brotli_test.go
• go/cache_test.go
• go/chat_completions.go
• go/chat_completions_internal_test.go
• go/chat_completions_test.go
• go/client.go
• go/client_test.go
• go/cmd/api/cmd_spec_test.go
• go/cmd/api/spec_groups_iter.go
• go/codegen.go
• go/codegen_test.go
• go/entitlements.go
• go/entitlements_test.go
• go/export_test.go
• go/expvar_test.go
• go/go.mod
• go/graphql_config_test.go
• go/graphql_test.go
• go/group_test.go
• go/gzip_test.go
• go/httpsign_test.go
• go/i18n_test.go
• go/location_test.go
• go/middleware.go
• go/middleware_test.go
• go/modernization_test.go
• go/openapi.go
• go/openapi_test.go
• go/openapitools.json
• go/options.go
• go/pkg/provider/cache_control_test.go
• go/pkg/provider/discovery_test.go
• go/pkg/provider/proxy_test.go
• go/pkg/provider/registry_test.go
• go/pkg/stream/stream_group_test.go
• go/pprof_test.go
• go/ratelimit.go
• go/ratelimit_test.go
• go/response_meta.go
• go/response_meta_test.go
• go/response_test.go
• go/runtime_config_test.go
• go/secure_test.go
• go/service.go
• go/sessions_test.go
• go/slog_test.go
• go/spec_builder_helper_test.go
• go/spec_registry_test.go
• go/sse.go
• go/sse_test.go
• go/static_test.go
• go/strict_bind_test.go
• go/string_constants.go
• go/sunset_test.go
• go/swagger.go
• go/swagger_test.go
• go/test_constants_test.go
• go/timeout_test.go
• go/tracing_test.go
• go/transformer_test.go
• go/transport_client_test.go
• go/upstream_balancer.go
• go/upstream_balancer_internal_test.go
• go/upstream_registry.go
• go/upstream_registry_internal_test.go
• go/upstream_registry_test.go
• go/upstream_router.go
• go/upstream_router_example_test.go
• go/upstream_router_internal_test.go
• go/upstream_router_test.go
• go/upstream_transport.go
• go/upstream_transport_internal_test.go
• go/websocket_test.go
• php/src/Api/Boot.php
• php/src/Api/Controllers/Api/WebhookSecretController.php
• php/src/Api/Controllers/McpApiController.php
• php/src/Api/Database/Factories/ApiKeyFactory.php
• php/src/Api/Documentation/DocumentationController.php
• php/src/Api/Documentation/DocumentationServiceProvider.php
• php/src/Api/Documentation/Examples/CommonExamples.php
• php/src/Api/Documentation/OpenApiBuilder.php
• php/src/Api/Models/WebhookEndpoint.php
• php/src/Api/Routes/api.php
• php/src/Api/Services/SeoReportService.php
• php/src/Api/Services/WebhookSignature.php
• php/src/Api/Services/WebhookTemplateService.php
• php/src/Api/Tests/Feature/ApiKeyIpWhitelistTest.php
• php/src/Api/Tests/Feature/ApiKeyRotationTest.php
• php/src/Api/Tests/Feature/ApiKeyTest.php
• php/src/Api/Tests/Feature/ApiScopeEnforcementTest.php
• php/src/Api/Tests/Feature/ApiUsageTest.php
• php/src/Api/Tests/Feature/AuthenticateApiKeyTest.php
• php/src/Api/Tests/Feature/DocumentationControllerTest.php
• php/src/Api/Tests/Feature/McpApiControllerTest.php
• php/src/Api/Tests/Feature/McpResourceTest.php
• php/src/Api/Tests/Feature/McpServerAccessTest.php
• php/src/Api/Tests/Feature/McpServerDetailTest.php
• php/src/Api/Tests/Feature/OpenApiDocumentationComprehensiveTest.php
• php/src/Api/Tests/Feature/PixelEndpointTest.php
• php/src/Api/Tests/Feature/PublicApiCorsTest.php
• php/src/Api/Tests/Feature/RateLimitTest.php
• php/src/Api/Tests/Feature/RateLimitingTest.php
• php/src/Api/Tests/Feature/SeoReportServiceTest.php
• php/src/Api/Tests/Feature/WebhookDeliveryTest.php
• php/src/Api/Tests/Feature/WebhookEndpointTest.php
• php/src/Website/Api/Services/OpenApiGenerator.php
• php/tests/Feature/ApiSunsetTest.php
• php/tests/Feature/ApiVersionServiceTest.php
• php/tests/Feature/AuthenticationGuideTest.php

💤 Files with no reviewable changes (2)

• php/src/Api/Services/WebhookSignature.php
• go/codegen_test.go

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Specify fenced code block languages to satisfy markdownlint.

Line 168 and Line 276 use bare triple-backtick fences, which triggers MD040 and can break docs lint gates.

🛠️ Proposed fix

-```
+```text
 hits mounted path  (engine auth/CORS/ratelimit/tracing already ran)
@@
-```
+```

-```
+```text
 go/upstream_registry.go         + _test.go + _example_test.go
@@
-```
+```

Also applies to: 276-276

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 168-168: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/superpowers/specs/2026-06-06-upstream-router-design.md` at line 168,
Replace the bare triple-backtick fenced code blocks in the document with
language-tagged fences (e.g., ```text) so markdownlint MD040 is satisfied:
update the fence that wraps the block beginning "hits mounted path  (engine
auth/CORS/ratelimit/tracing already ran)" and the fence that wraps the block
starting "go/upstream_registry.go         + _test.go + _example_test.go" to use
```text (or another appropriate language) instead of plain ``` so both instances
are lint-compliant.

Source: Linters/SAST tools

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify schema path and OpenAPI generator version

# Check if schema exists at the configured path
if [ -f "go/node_modules/@openapitools/openapi-generator-cli/config.schema.json" ]; then
  echo "✓ Schema found at go/node_modules/..."
elif [ -f "node_modules/@openapitools/openapi-generator-cli/config.schema.json" ]; then
  echo "✗ Schema found at root node_modules/ - path in config should be ../node_modules/..."
else
  echo "✗ Schema not found - verify node_modules location"
fi

# Check npm for latest version of openapi-generator-cli
echo ""
echo "Checking npm registry for openapi-generator-cli versions..."
npm view `@openapitools/openapi-generator-cli` versions --json | jq -r '.[-5:]' 2>/dev/null || echo "npm query failed"

Repository: dAppCore/api

Length of output: 230

---

🌐 Web query:

OpenAPI Generator CLI latest version 2026 security advisories

💡 Result:

As of June 6, 2026, the latest version of the @openapitools/openapi-generator-cli npm package is 2.33.1, released on May 19, 2026 [1][2]. Regarding security advisories for 2026, there are no direct vulnerabilities in the OpenAPI Generator core codebase currently identified as active, high-impact security threats for the project itself [3][4]. However, users have reported issues concerning vulnerabilities in transitive dependencies during security scans [3][4][5][6]. Key points regarding 2026 security-related reports: 1. Transitive Dependency Alerts: Security scanners like Trivy and npm audit have flagged several vulnerabilities in dependencies (such as basic-ftp [3], path-to-regexp [4], and jackson-core [5][6]). In most cases, these are identified as transitive issues that do not directly affect the generator's functionality or are patched in subsequent releases [3][4][6]. 2. Resolution of Reported Issues: - A vulnerability involving path-to-regexp was resolved in version 2.31.1 following a dependency update [4]. - Concerns regarding jackson-core (GHSA-72hv-8253-57qq) were addressed in the core generator, with fixes integrated into newer releases [5][6]. - Users are advised to monitor the official repository's issues and security advisories page for real-time tracking of dependency-related alerts [2][7]. For the most secure implementation, it is recommended to keep the CLI updated to the latest version [1][8]. If your security compliance tools flag specific transitive dependencies that remain present in the current build, you can use package manager features such as npm overrides to force the use of patched versions of those specific libraries while waiting for upstream updates [4].

Citations:

• 1: https://www.npmjs.com/package/@openapitools/openapi-generator-cli
• 2: https://github.com/OpenAPITools/openapi-generator-cli/releases
• 3: basic-ftp Critical Security Vulnerability: CVE-2026-27699 OpenAPITools/openapi-generator-cli#1161
• 4: @nestjs/core -> path-to-regexp : CVE-2026-4923, CVE-2026-4926 OpenAPITools/openapi-generator-cli#1182
• 5: [BUG] High severity github advisory for jackson-core 2.18.2 OpenAPITools/openapi-generator#23214
• 6: Security: Upgrade jackson-core to 2.18.6+ to fix GHSA-72hv-8253-57qq (DoS) OpenAPITools/openapi-generator#23106
• 7: https://github.com/OpenAPITools/openapi-generator-cli?tab=readme-ov-file
• 8: https://openapi-generator.tech/docs/installation/

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Files mentioning openapitools.json =="
rg -n "openapitools\.json" -S . || true

echo
echo "== package.json files =="
fd -t f "package.json" . || true

echo
echo "== npm/yarn install scripts (best-effort) =="
rg -n "(npm|yarn|pnpm).*install|openapi-generator|openapitools\.json|openapi-generator-cli" -S package.json **/package.json 2>/dev/null || true

Repository: dAppCore/api

Length of output: 169

---

🌐 Web query:

OpenAPI Generator 7.22.0 release

💡 Result:

OpenAPI Generator version 7.22.0 was officially released on April 28, 2026 [1][2]. This release included over 150 enhancements and bug fixes [3]. It is available through standard package repositories, including Maven Central [2] and the Gradle Plugin Portal [4].

Citations:

• 1: https://github.com/OpenAPITools/openapi-generator/releases/tag/v7.22.0
• 2: https://mvnrepository.com/artifact/org.openapitools/openapi-generator/7.22.0
• 3: https://github.com/OpenAPITools/openapi-generator/releases
• 4: https://plugins.gradle.org/plugin/org.openapi.generator

---

🌐 Web query:

OpenAPI Generator 7.22.0 security advisory

💡 Result:

As of June 6, 2026, there is no specific security advisory associated with OpenAPI Generator version 7.22.0 [1]. Version 7.22.0 was released on April 28, 2026, and is a stable release containing approximately 150 enhancements and bug fixes [1][2]. While there is no advisory for 7.22.0, recent security-related maintenance in the project includes: 1. Jackson-core vulnerability (GHSA-72hv-8253-57qq): In March 2026, a high-severity Denial of Service (DoS) vulnerability in the bundled jackson-core library (affecting version 2.18.2) was identified [3]. This issue was addressed in the project by upgrading to jackson-core 2.18.6 or higher [3]. 2. Historical Advisories: Users should be aware of older vulnerabilities, such as CVE-2024-35219 (a path traversal vulnerability affecting OpenAPI Generator Online versions prior to 7.6.0) [4][5][6]. Users are encouraged to review the official release notes and the project's GitHub security advisories page for the most up-to-date information regarding potential security issues [4][7].

Citations:

• 1: https://github.com/OpenAPITools/openapi-generator/releases/tag/v7.22.0
• 2: OpenAPITools/openapi-generator@f4d1cb8
• 3: Security: Upgrade jackson-core to 2.18.6+ to fix GHSA-72hv-8253-57qq (DoS) OpenAPITools/openapi-generator#23106
• 4: GHSA-g3hr-p86p-593h
• 5: https://nvd.nist.gov/vuln/detail/CVE-2024-35219
• 6: https://nvd.nist.gov/nvd.cfm?cvename=CVE-2024-35219
• 7: https://github.com/OpenAPITools/openapi-generator/releases

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Files mentioning openapitools.json =="
rg -n "openapitools\.json" -S . || true

echo
echo "== package.json files =="
fd -t f "package.json" . || true

echo
echo "== npm/yarn install scripts (best-effort) =="
rg -n "(npm|yarn|pnpm).*install|openapi-generator|openapitools\.json|openapi-generator-cli" -S package.json **/package.json 2>/dev/null || true

Repository: dAppCore/api

Length of output: 169

---

🌐 Web query:

OpenAPI Generator 7.22.0 release

💡 Result:

OpenAPI Generator version 7.22.0 was officially released on April 28, 2026 [1][2][3]. This release included over 150 enhancements and bug fixes [4]. It is available across standard repositories, including Maven Central [2] and the Gradle Plugin Portal [3]. Detailed release information and a full list of changes can be found in the official GitHub repository's release section [1][4].

Citations:

• 1: https://github.com/OpenAPITools/openapi-generator/releases/tag/v7.22.0
• 2: https://mvnrepository.com/artifact/org.openapitools/openapi-generator/7.22.0
• 3: https://plugins.gradle.org/plugin/org.openapi.generator
• 4: https://github.com/OpenAPITools/openapi-generator/releases

---

🌐 Web query:

OpenAPI Generator 7.22.0 security advisory

💡 Result:

As of June 6, 2026, there is no specific security advisory issued for OpenAPI Generator version 7.22.0 [1]. Version 7.22.0 was released on April 28, 2026, and includes various enhancements and bug fixes rather than serving as a security patch release [1][2]. Recent security-related actions for OpenAPI Generator include: - Dependency Updates: In March 2026, a high-severity vulnerability (GHSA-72hv-8253-57qq) in the bundled jackson-core library was identified [3]. This was addressed in the project by upgrading jackson-core to version 2.18.6 or higher, a fix incorporated into the development cycle leading up to recent releases [3]. - Distinctions from Other Projects: Users should be aware of vulnerabilities in related ecosystems, such as the quarkus-openapi-generator extension (e.g., CVE-2026-40180 and CVE-2026-42333), which are distinct from the main OpenAPITools/openapi-generator project [4][5][6]. It is recommended to monitor the official OpenAPITools GitHub security advisories page for any future disclosures [7][8].

Citations:

• 1: https://github.com/OpenAPITools/openapi-generator/releases/tag/v7.22.0
• 2: OpenAPITools/openapi-generator@f4d1cb8
• 3: Security: Upgrade jackson-core to 2.18.6+ to fix GHSA-72hv-8253-57qq (DoS) OpenAPITools/openapi-generator#23106
• 4: https://www.sentinelone.com/vulnerability-database/cve-2026-42333/
• 5: https://app.opencve.io/cve/CVE-2026-40180
• 6: https://vulners.com/cve/CVE-2026-42333
• 7: GHSA-g3hr-p86p-593h
• 8: GHSA-867q-77cc-98mv

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Find files related to openapi generator tooling =="
fd -t f "openapitools.json" . || true
fd -t f "*openapi*generator*" . || true
fd -t f "package.json" . || true
fd -t f "package-lock.json" . || true
fd -t f "yarn.lock" . || true
fd -t f "pnpm-lock.yaml" . || true

echo
echo "== Search for openapi-generator-cli usage =="
rg -n "`@openapitools/openapi-generator-cli`|openapi-generator-cli|openapitools\.json" -S . || true

echo
echo "== Search for config.schema.json usage =="
rg -n "config\.schema\.json|openapi-generator-cli/config\.schema\.json" -S . || true

Repository: dAppCore/api

Length of output: 4001

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== go/openapitools.json =="
cat -n go/openapitools.json

echo
echo "== scripts/generate-sdks.sh (around openapi-generator-cli call) =="
if [ -f scripts/generate-sdks.sh ]; then
  sed -n '1,120p' scripts/generate-sdks.sh | cat -n
else
  echo "scripts/generate-sdks.sh not found"
fi

echo
echo "== go/codegen.go (buildArgs / invocation) =="
sed -n '1,240p' go/codegen.go | cat -n

echo
echo "== go/cmd/api/cmd_sdk.go (install instructions / usage) =="
sed -n '1,120p' go/cmd/api/cmd_sdk.go | cat -n

echo
echo "== go/codegen_test.go (any mention of openapitools.json) =="
rg -n "openapitools\.json|openapi-generator-cli|config\.schema\.json|generator-cli\.version" go/codegen_test.go go -S || true

Repository: dAppCore/api

Length of output: 15243

---

Fix $schema path reliability in go/openapitools.json

• go/openapitools.json sets $schema to ./node_modules/@openapitools/openapi-generator-cli/config.schema.json, which only resolves if @openapitools/openapi-generator-cli is installed under go/node_modules (often it’s under repo-root node_modules). Update the path (e.g., ../node_modules/...) or point to a stable URL if this is meant for editor/schema validation.
• generator-cli.version: "7.22.0" corresponds to OpenAPI Generator 7.22.0 (released 28 Apr 2026) and there’s no specific security advisory for that version; this pin doesn’t look problematic.

🤖 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 `@go/openapitools.json` around lines 1 - 7, The $schema path in
go/openapitools.json is relative to the go/ folder and currently points to
./node_modules/@openapitools/openapi-generator-cli/config.schema.json which will
only work if that package is installed under go/node_modules; update the $schema
to a reliable path such as
../node_modules/@openapitools/openapi-generator-cli/config.schema.json or a
stable URL (e.g., the package’s schema on npmjs or raw GitHub) so editor/schema
validation resolves correctly; keep the existing generator-cli.version entry
unchanged unless you want to intentionally bump it.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Serve test does not exercise the documented non-loopback case.

The comment says this case verifies default behaviour on a non-loopback bind, but Line 181 uses 127.0.0.1:0 (loopback). This weakens the intended Serve coverage for public binds in non-strict mode.

Proposed fix

-	e, err = New(WithAddr("127.0.0.1:0"))
+	e, err = New(WithAddr("0.0.0.0:0"))

📝 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

	// Default engine on a non-loopback ephemeral port: must bind and exit
	// cleanly on context cancellation — strict enforcement is opt-in only.
	e, err = New(WithAddr("127.0.0.1:0"))
	if err != nil {
	// Default engine on a non-loopback ephemeral port: must bind and exit
	// cleanly on context cancellation — strict enforcement is opt-in only.
	e, err = New(WithAddr("0.0.0.0:0"))
	if err != nil {

🤖 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 `@go/strict_bind_test.go` around lines 179 - 182, The test comment says it
should verify default behavior on a non-loopback bind but the code creates the
engine with New(WithAddr("127.0.0.1:0")) which is loopback; change the address
passed to WithAddr to a non-loopback ephemeral address (e.g. "0.0.0.0:0") so the
Serve test actually exercises public/non-loopback binding in non-strict mode
while keeping the rest of the test flow (error checks around New and subsequent
Serve/cancellation) unchanged.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

current state can grow unbounded with high-cardinality selector keys.

Line 41 initialises per-key weight state for every observed key, but there is no eviction path. With default-pool fallback and user-controlled selector values, this map can grow indefinitely and degrade memory/GC behaviour.

🤖 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 `@go/upstream_balancer.go` around lines 41 - 45, The per-key state map
b.current (variables key and cw) is unbounded and can grow indefinitely; add an
eviction strategy to limit growth: introduce a maxEntries constant and track
insertion order (e.g., an LRU list or a simple FIFO ring) alongside b.current,
and when inserting a new key in the cw initialization path evict the oldest
entries until size <= maxEntries; ensure modifications to b.current and the
order tracker are done under the existing balancer lock/guard to keep it
concurrency-safe and preserve the current initialization logic for cw.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Rename this test to end with _Ugly for naming-rule compliance.

TestUpstreamRegistry_Ugly_HeadersDeepCopy does not satisfy the required suffix rule used across Go tests.

As per coding guidelines, "**/*_test.go: Use Go test naming convention with _Good, _Bad, or _Ugly suffixes".

🤖 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 `@go/upstream_registry_internal_test.go` at line 70, The test function name
TestUpstreamRegistry_Ugly_HeadersDeepCopy does not end with the required `_Ugly`
suffix; rename it to end with `_Ugly` (for example
TestUpstreamRegistry_HeadersDeepCopy_Ugly) and update the function declaration
and any references/imports that reference the test function name so the test
still compiles and runs; ensure only the function identifier is changed (keep
the body and behavior intact).

Source: Coding guidelines

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Don’t discard setup errors in these tests.

At Line 59, Line 101, Line 133, Line 165, Line 192, Line 213, Line 244, and Line 251, setup errors are ignored. That can hide invalid test preconditions and make failures harder to diagnose.

Suggested fix

- _ = reg.Set("m", api.Upstream{URL: dead.URL})
+ if err := reg.Set("m", api.Upstream{URL: dead.URL}); err != nil {
+ 	t.Fatalf("Set: %v", err)
+ }

- e, _ := api.New(api.WithRateLimit(100), api.WithUpstreamRouter(reg))
+ e, err := api.New(api.WithRateLimit(100), api.WithUpstreamRouter(reg))
+ if err != nil {
+ 	t.Fatalf("New: %v", err)
+ }

Also applies to: 101-101, 133-133, 165-165, 192-192, 213-213, 244-244, 251-251

🤖 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 `@go/upstream_router_test.go` at line 59, Tests are ignoring errors from
reg.SetDefault(api.Upstream{URL: "https://example.com"}) which can hide setup
failures; change each ignored call (the occurrences of reg.SetDefault in this
file) to check the returned error and fail the test on error (e.g., capture err
:= reg.SetDefault(...); if err != nil { t.Fatalf("failed to set default
upstream: %v", err) } or use require.NoError(t, err)) so setup errors surface
immediately.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect the target file with line numbers around the flagged areas.
FILE="go/upstream_router.go"

echo "=== File existence ==="
ls -la "$FILE"

echo "=== SPDX header / top of file (first 60 lines) ==="
sed -n '1,60p' "$FILE" | cat -n

echo "=== Around line 203 ==="
sed -n '180,230p' "$FILE" | cat -n

echo "=== Around line 347 ==="
sed -n '320,380p' "$FILE" | cat -n

echo "=== Search for other relevant compliance concerns in this file ==="
rg -n --hidden --no-ignore-vcs "http\.DefaultTransport|MaxBytes|MaxBytesError|DefaultTransport\.\(|Clone\(\)|strings\.|errors\.|fmt\.|buffer|os\.|process\.|env\." "$FILE" || true

# Check for wrapper usage guidance: ensure dappco.re/go wrappers appear where appropriate.
rg -n --hidden --no-ignore-vcs "dappco\.re/go/(json|errors|formatting|strings|buffers|filesystem|process|environment)" "$FILE" || true

Repository: dAppCore/api

Length of output: 7503

---

Guard the http.DefaultTransport cast to prevent init-time panics (go/upstream_router.go:203)

cfg.transport = http.DefaultTransport.(*http.Transport).Clone() will panic during initialisation if http.DefaultTransport has been replaced with a non-*http.Transport RoundTripper.

Suggested fix

- cfg.transport = http.DefaultTransport.(*http.Transport).Clone()
+ if base, ok := http.DefaultTransport.(*http.Transport); ok {
+ 	cfg.transport = base.Clone()
+ } else {
+ 	cfg.transport = http.DefaultTransport
+ }

• Minor correctness: oversized-body detection relies on an exact string match (err.Error() == "http: request body too large"); match net/http’s MaxBytesError type instead to avoid misclassification across Go versions/configurations.

📝 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

	cfg.transport = http.DefaultTransport.(*http.Transport).Clone()
	if base, ok := http.DefaultTransport.(*http.Transport); ok {
	cfg.transport = base.Clone()
	} else {
	cfg.transport = http.DefaultTransport
	}

🤖 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 `@go/upstream_router.go` at line 203, Guard the http.DefaultTransport cast by
checking its dynamic type before cloning: replace the direct cast at
cfg.transport = http.DefaultTransport.(*http.Transport).Clone() with a type
assertion (if t, ok := http.DefaultTransport.(*http.Transport); ok {
cfg.transport = t.Clone() } else { cfg.transport = &http.Transport{} }) so we
don’t panic if DefaultTransport is a non-*http.Transport; also stop matching
oversized-body by string comparison and instead detect net/http’s MaxBytesError
via errors.As (e.g. var mbe *http.MaxBytesError; if errors.As(err, &mbe) { ...
}) and add the "errors" import if missing.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use typed http.MaxBytesError detection instead of err.Error() string comparison.

In go/upstream_router.go (lines 347-348), comparing err.Error() to "http: request body too large" is fragile; replace it with a type-based check for *http.MaxBytesError (via the project’s core.As/errors.As helper) so wrapped/changed error text still yields HTTP 413.

🤖 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 `@go/upstream_router.go` around lines 347 - 348, Replace the fragile string
comparison err.Error() == "http: request body too large" with a typed check for
*http.MaxBytesError using the project's error-as helper (e.g., core.As or
errors.As): attempt to cast err into a variable like var maxBytesErr
*http.MaxBytesError and if the cast succeeds, call
c.AbortWithStatusJSON(http.StatusRequestEntityTooLarge,
Fail(errCodeRequestTooLarge, "Request body exceeds the maximum allowed size"));
update the branch around the error handling in upstream_router.go accordingly so
wrapped or reformatted errors still produce HTTP 413.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard route constants against redefinition warnings.

Using unguarded define() at bootstrap level can emit warnings if this routes file is loaded more than once in a process. Wrapping these with defined() checks makes this robust.

Suggested fix

-define('API_ROUTE_WORKSPACE', '/{workspace}');
-define('API_ROUTE_ID', '/{id}');
+if (! defined('API_ROUTE_WORKSPACE')) {
+    define('API_ROUTE_WORKSPACE', '/{workspace}');
+}
+if (! defined('API_ROUTE_ID')) {
+    define('API_ROUTE_ID', '/{id}');
+}

📝 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

	define('API_ROUTE_WORKSPACE', '/{workspace}');
	define('API_ROUTE_ID', '/{id}');
	if (! defined('API_ROUTE_WORKSPACE')) {
	define('API_ROUTE_WORKSPACE', '/{workspace}');
	}
	if (! defined('API_ROUTE_ID')) {
	define('API_ROUTE_ID', '/{id}');
	}

🤖 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 `@php/src/Api/Routes/api.php` around lines 25 - 26, The route constants
API_ROUTE_WORKSPACE and API_ROUTE_ID are defined unguarded which can trigger
"already defined" warnings if the file is included multiple times; wrap each
define call with a defined() check so you only call
define('API_ROUTE_WORKSPACE', '/{workspace}') and define('API_ROUTE_ID',
'/{id}') when the constant is not already defined (use
defined('API_ROUTE_WORKSPACE') and defined('API_ROUTE_ID') to guard them) to
avoid redefinition warnings at bootstrap.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Remove the extra argument when calling index().

Line 104, Line 116, and Line 127 still call index() with a Request, but DocumentationController::index() is now parameterless. This will break these tests at runtime.

Suggested fix

-        $response = $controller->index(Request::create(API_DOCS_PATH, 'GET'));
+        $response = $controller->index();
...
-    $response = $controller->index(Request::create(API_DOCS_PATH, 'GET'));
+    $response = $controller->index();
...
-    $response = $controller->index(Request::create(API_DOCS_PATH, 'GET'));
+    $response = $controller->index();

Also applies to: 116-116, 127-127

🤖 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 `@php/src/Api/Tests/Feature/DocumentationControllerTest.php` at line 104, The
test calls to DocumentationController::index are passing a Request even though
index() is now parameterless; update the three invocations in
DocumentationControllerTest (currently using
$controller->index(Request::create(...))) to call $controller->index() with no
arguments, and remove any now-unused Request construction/variables in those
test cases so they compile and run.

[coderabbitai]

Actionable comments posted: 10

🤖 Prompt for all review comments with 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.

Inline comments:
In `@docs/superpowers/specs/2026-06-06-chat-completions-remote-backend-design.md`:
- Line 113: Two Markdown triple-backtick fences in the spec are missing language
labels; update those fences to include the appropriate language identifiers
(e.g., ```js, ```json, ```bash, or ```yaml) that match each block's content so
the docs linter (MD040) passes—locate the two unlabeled ``` blocks in the
document and add the correct fence languages.

In `@go/chat_adapter_anthropic.go`:
- Around line 71-74: Replace direct encoding/json usage with the repo's core
JSON wrappers: use core.JSONMarshal instead of json.Marshal when marshalling the
request body (the raw variable created from body) and use core.JSONUnmarshal
instead of json.Unmarshal when decoding responses (the places handling
respBody/resp bytes). Update imports to remove "encoding/json" and ensure the
core package is imported, and propagate errors using core.E as currently done
(e.g., "anthropic", "marshal request") while calling the core JSON functions for
consistent error handling.

In `@go/chat_adapter.go`:
- Around line 45-57: writeChatChunk (and sibling writeSSEDone) currently swallow
marshal and write errors which hides stream failures; change their signatures to
return error (e.g., func writeChatChunk(... ) error), check core.JSONMarshal for
errors and the type assertion before writing, check and return any
io.WriteString/w.Write errors instead of ignoring them, and keep calling flush()
only after successful writes; then update all callers to handle the returned
error and abort/propagate transcode work on failure so stream errors are not
silently dropped.

In `@go/chat_completions.go`:
- Around line 804-810: Normalize the adapter lookup key to a canonical form
(e.g., strings.ToLower) so adapter resolution and dispatch use the same key:
when retrieving the pool call h.remote.reg.resolve with req.Model as before but
use the canonicalized model string to index h.remote.adapters (replace
h.remote.adapters[req.Model] with h.remote.adapters[canonicalModel]); also
update the registration path in WithChatModelAdapter to store adapters under the
same canonical key (e.g., canonicalModel := strings.ToLower(model) and use that
for map writes) so reads and writes match and case-variants (like "CLAUDE-3" vs
"claude-3") do not bypass the adapter and hit the wrong upstream.
- Around line 771-803: When h.remote != nil the code currently uses
core.JSONUnmarshalString to populate req (lenient) which lets hybrid mode bypass
strict validation; fix by only using a lenient/unstructured unmarshal into a
small temp struct (e.g., struct{Model string} or similar) to inspect the model
for h.resolver.Knows, and always call decodeJSONBody (the strict decoder used
elsewhere) to populate the full chat request (req) before calling
validateChatRequest or serveLocal; update the branch that currently calls
core.JSONUnmarshalString to instead peek the model into a temp value and, if
serving locally, re-decode raw with decodeJSONBody into req so
validateChatRequest and serveLocal keep local behaviour intact.

In `@go/chat_remote_example_test.go`:
- Around line 6-9: The example is using fmt.Println instead of the Core println
helper; replace all fmt.Println calls in the example functions (the Example...
test(s) in this file) with core.Println and update imports: remove the "fmt"
import and add the Core package import (e.g., "dappco.re/go/core"). Ensure every
fmt.Println occurrence (including the ones around lines 24-25) is changed to
core.Println so the example prints through Core as per guidelines.

In `@go/chat_remote.go`:
- Around line 105-109: The passthrough branch currently overwrites headers
instead of forwarding the upstream response headers; update the adapter==nil
branch in the function containing the adapter nil check so you copy all headers
from the upstream http.Response (resp.Header) into the Gin context response (use
c.Writer.Header().Set/Add or c.Header for each header) before setting the status
and writing the body, preserving existing Content-Type behavior (e.g., only set
hdrContentType/mimeJSON if upstream did not provide one). Apply the same
header-forwarding change to the second passthrough block (the other adapter==nil
branch around the later response handling) so upstream metadata like
rate-limit/retry headers are forwarded to clients.
- Line 104: The code currently ignores errors from io.ReadAll when reading
resp.Body (body, _ := io.ReadAll(io.LimitReader(resp.Body,
maxUpstreamResponseBytes))) which can forward partial/invalid responses; update
the logic in the function that handles the upstream response to capture the
error (e.g., body, err := io.ReadAll(...)) and if err != nil return or write an
appropriate error response (and log the error) instead of streaming/forwarding
the (partial) body; apply the same change to the second occurrence using resp
and maxUpstreamResponseBytes so you verify read success before sending
status/headers/body downstream.
- Line 37: Guard the cast of http.DefaultTransport to *http.Transport before
calling Clone so we don't panic if DefaultTransport is a different RoundTripper:
in the code path that sets cfg.transport (where you currently do cfg.transport =
http.DefaultTransport.(*http.Transport).Clone()), check the type with a type
assertion or type switch (e.g., t, ok :=
http.DefaultTransport.(*http.Transport)); if ok use t.Clone() to set
cfg.transport, otherwise either clone a new http.Transport with reasonable
defaults or wrap/convert the non-*http.Transport RoundTripper into an
*http.Transport fallback and assign that to cfg.transport; update the assignment
in the same function where cfg.transport is initialized to handle both branches
safely.
- Line 126: The Transcode call currently discards errors; change the call to
capture its returned error from adapter.Transcoder().Transcode(c.Writer, flush,
resp.Body, meta) and handle it instead of assigning to blank identifier: check
if err != nil and then propagate/handle it (e.g., log the error with context
including meta/resp info via the existing logger, close/abort the response
stream or return the error up the call stack so the client is aware of a
truncated/malformed upstream stream) using the surrounding request/response
handling routines rather than ignoring it.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f695a08e-6efc-468e-88fe-48e30de4e46e

📥 Commits

Reviewing files that changed from the base of the PR and between dd588cf and c28242d.

📒 Files selected for processing (15)

• docs/superpowers/plans/2026-06-06-chat-completions-remote-backend.md
• docs/superpowers/specs/2026-06-06-chat-completions-remote-backend-design.md
• go/api.go
• go/chat_adapter.go
• go/chat_adapter_anthropic.go
• go/chat_adapter_anthropic_test.go
• go/chat_adapter_ollama.go
• go/chat_adapter_ollama_test.go
• go/chat_completions.go
• go/chat_completions_internal_test.go
• go/chat_remote.go
• go/chat_remote_example_test.go
• go/chat_remote_internal_test.go
• go/chat_remote_test.go
• go/options.go

🚧 Files skipped from review as they are similar to previous changes (1)

• go/chat_completions_internal_test.go

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add fence language identifiers to Markdown code blocks.

The code fences at Line 113 and Line 233 are missing language labels (MD040), which can fail docs lint in CI.

Also applies to: 233-233

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 113-113: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 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/superpowers/specs/2026-06-06-chat-completions-remote-backend-design.md`
at line 113, Two Markdown triple-backtick fences in the spec are missing
language labels; update those fences to include the appropriate language
identifiers (e.g., ```js, ```json, ```bash, or ```yaml) that match each block's
content so the docs linter (MD040) passes—locate the two unlabeled ``` blocks in
the document and add the correct fence languages.

Source: Linters/SAST tools

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the target file around the flagged lines
FILE="go/chat_adapter_anthropic.go"
echo "== File exists? =="
ls -l "$FILE" || true

echo "== SPDX header (first 5 lines) =="
sed -n '1,8p' "$FILE" | nl -ba

echo "== Imports =="
rg -n "^import" -n "$FILE" || true
# Print import block fully (if small)
sed -n '1,60p' "$FILE" | nl -ba

echo "== Flagged JSON marshal/unmarshal sections (lines ~60-110, ~120-160) =="
sed -n '60,115p' "$FILE" | nl -ba
sed -n '115,170p' "$FILE" | nl -ba

echo "== Search for dappco.re/go JSON wrapper usage patterns in repo =="
rg -n "dappco\.re/go/.+json|dappco\.re/.+json" -S . || true

echo "== Search for encoding/json usage in this file =="
rg -n "encoding/json" "$FILE" || true
rg -n "json\.(Marshal|Unmarshal)" "$FILE" || true

echo "== Find any dappco.re/go json wrapper package name by scanning go.mod or docs =="
if [ -f go.mod ]; then
  echo "== go.mod relevant lines =="
  rg -n "dappco\.re/go" go.mod || true
fi

echo "== Search for wrapper package in local module (if vendored) =="
# list candidate directories without assuming layout
fd -t d "dappco.re" . || true
fd -t f "json*.go" . | head -n 50 || true

Repository: dAppCore/api

Length of output: 249

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="go/chat_adapter_anthropic.go"

echo "== File exists? =="
ls -l "$FILE" || true

echo "== SPDX header (first 8 lines) =="
sed -n '1,8p' "$FILE" | awk '{printf "%4d:%s\n", NR, $0}'

echo "== Imports block =="
# Print import block by scanning from first import to closing paren/line
awk '
  $0 ~ /^import/ {in=1}
  in{
    print NR ":" $0
    if ($0 ~ /^\)/) {in=0}
    # if single-line import, stop
    if ($0 ~ /^import "/ && $0 !~ /\(/) {in=0}
  }
' "$FILE" | head -n 80

echo "== Flagged JSON usage regions =="
echo "-- Lines 60-120 --"
sed -n '60,120p' "$FILE" | awk '{printf "%4d:%s\n", (60+NR-1), $0}'
echo "-- Lines 120-170 --"
sed -n '120,170p' "$FILE" | awk '{printf "%4d:%s\n", (120+NR-1), $0}'

echo "== Search for encoding/json in this file =="
rg -n "encoding/json" "$FILE" || true
rg -n "json\.(Marshal|Unmarshal)" "$FILE" || true

echo "== Search for dappco.re/go JSON wrapper usage in repo =="
# Heuristic: wrapper paths typically include dappco.re/go/.../json
rg -n "dappco\.re/go/.*/json" . || true
rg -n "dappco\.re/go.*json" . || true

echo "== Search for any local package aliasing of json wrappers =="
rg -n "as json|jsonwrap|djson|core\.json" "$FILE" . || true

echo "== go.mod check for dappco.re/go =="
if [ -f go.mod ]; then
  rg -n "dappco\.re/go" go.mod || true
fi

Repository: dAppCore/api

Length of output: 450

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
FILE="go/chat_adapter_anthropic.go"

echo "== Header (first 25 lines) =="
sed -n '1,25p' "$FILE" | cat

echo "== Imports/encoding/json references =="
rg -n "encoding/json|json\.Marshal|json\.Unmarshal" "$FILE" || true

echo "== View around flagged lines (65-105) =="
sed -n '65,105p' "$FILE" | cat

echo "== View around flagged lines (88-105) =="
sed -n '88,105p' "$FILE" | cat

echo "== View around flagged lines (130-155) =="
sed -n '130,155p' "$FILE" | cat

echo "== Look for dappco.re/go JSON wrapper usage in repo =="
rg -n "dappco\.re/go/.+json" . || true
rg -n "dappco\.re/go/.*/(encoding|json)/" . || true
rg -n "dappco\.re/go/.+/(Marshal|Unmarshal)" . || true

echo "== Look for any import paths that wrap json encoding in this repo =="
rg -n "\"dappco\.re/go/.+\".*json|\"dappco\.re/go/.+json\"" -S . || true
rg -n "\"dappco\.re/go/.+\"$" -S go | head -n 200 || true

echo "== go.mod dappco.re/go presence (if any) =="
if [ -f go.mod ]; then
  rg -n "dappco\.re/go" go.mod || true
fi

Repository: dAppCore/api

Length of output: 8898

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search for any dappco.re/go json wrapper packages in repo =="

# common guesses: .../json, .../encoding/json, .../jsonutil, .../corejson
rg -n "dappco\.re/go/.+json" -S . || true

echo "== Search for core json helper imports by alias =="

# look for imports that include json in the path, even if not caught by above
rg -n "import \\(.*\"dappco\\.re/go/.*json" -S . || true
rg -n "\"dappco\\.re/go/.*json\"" -S . || true

echo "== Search for usage of potential wrapper helper identifiers in Go code =="

# These are common wrapper function names; adjust based on discoveries.
rg -n "core\\.json|corejson|jsonwrap|Marshal\\(|Unmarshal\\(" go --glob='*.go' || true

echo "== Search for direct wrapper usage patterns in existing code (encoding/json avoided?) =="

# Find other files that import encoding/json and see whether they use core wrappers instead.
rg -n "import \\(.*encoding/json|\"encoding/json\"" -S go --glob='*.go' || true

echo "== Search for dappco.re/go wrappers for JSON marshal/unmarshal by identifier =="

# This checks for any exported wrapper functions with clear names (e.g., JSONMarshal/JSONUnmarshal).
rg -n "JSON(Marshal|Unmarshal|Decode|Encode)|MarshalJSON|UnmarshalJSON" -S . || true

Repository: dAppCore/api

Length of output: 45953

---

Replace direct encoding/json calls with core JSON wrappers in chat_adapter_anthropic
go/chat_adapter_anthropic.go imports encoding/json and uses json.Marshal/json.Unmarshal at lines 71, 92–94, and 138–140. The repo’s v0.9.0 Core compliance shape expects dappco.re/go JSON wrappers (e.g., core.JSONMarshal / core.JSONUnmarshal) wherever available.

raw, err := json.Marshal(body)
if err != nil {
	return nil, nil, core.E("anthropic", "marshal request", err)
}

🤖 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 `@go/chat_adapter_anthropic.go` around lines 71 - 74, Replace direct
encoding/json usage with the repo's core JSON wrappers: use core.JSONMarshal
instead of json.Marshal when marshalling the request body (the raw variable
created from body) and use core.JSONUnmarshal instead of json.Unmarshal when
decoding responses (the places handling respBody/resp bytes). Update imports to
remove "encoding/json" and ensure the core package is imported, and propagate
errors using core.E as currently done (e.g., "anthropic", "marshal request")
while calling the core JSON functions for consistent error handling.

Source: Coding guidelines

[coderabbitai]

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

SSE helper writes fail silently and cannot be propagated.

writeChatChunk/writeSSEDone ignore marshal/write errors, so stream failures are swallowed instead of stopping transcode work.

Also applies to: 60-64

🤖 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 `@go/chat_adapter.go` around lines 45 - 57, writeChatChunk (and sibling
writeSSEDone) currently swallow marshal and write errors which hides stream
failures; change their signatures to return error (e.g., func writeChatChunk(...
) error), check core.JSONMarshal for errors and the type assertion before
writing, check and return any io.WriteString/w.Write errors instead of ignoring
them, and keep calling flush() only after successful writes; then update all
callers to handle the returned error and abort/propagate transcode work on
failure so stream errors are not silently dropped.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Hybrid mode weakens local request validation for known models.

When h.remote != nil, requests are always parsed via lenient unmarshal first, and known local models are served from that parsed payload. This bypasses local unknown-field rejection and changes local behaviour just by enabling remote routing.

Suggested patch

 	// HYBRID: local-first if the resolver knows the model; else remote.
 	if h.resolver != nil && h.resolver.Knows(req.Model) {
-		h.serveLocal(c, req)
+		var localReq ChatCompletionRequest
+		if err := decodeJSONBody(bytes.NewReader(raw), &localReq); err != nil {
+			writeChatCompletionError(c, http.StatusBadRequest, "invalid_request_error", "body", "invalid request body", "")
+			return
+		}
+		h.serveLocal(c, localReq)
 		return
 	}

📝 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

	if h.remote != nil {
	result := core.JSONUnmarshalString(string(raw), &req)
	if !result.OK {
	writeChatCompletionError(c, http.StatusBadRequest, "invalid_request_error", "body", "invalid request body", "")
	return
	}
	} else {
	if err := decodeJSONBody(bytes.NewReader(raw), &req); err != nil {
	writeChatCompletionError(c, http.StatusBadRequest, "invalid_request_error", "body", "invalid request body", "")
	return
	}
	}
	if err := validateChatRequest(&req); err != nil {
	chatErr, ok := err.(*chatCompletionRequestError)
	if !ok {
	chatErr, isChatErr := err.(*chatCompletionRequestError)
	if !isChatErr {
	writeChatCompletionError(c, http.StatusBadRequest, "invalid_request_error", "body", err.Error(), "")
	return
	}
	writeChatCompletionError(c, chatErr.Status, chatErr.Type, chatErr.Param, chatErr.Message, chatErr.Code)
	return
	}
	// PURE-LOCAL: unchanged current behaviour (no Knows gate).
	if h.remote == nil {
	h.serveLocal(c, req)
	return
	}
	// HYBRID: local-first if the resolver knows the model; else remote.
	if h.resolver != nil && h.resolver.Knows(req.Model) {
	h.serveLocal(c, req)
	return
	}
	if h.remote != nil {
	result := core.JSONUnmarshalString(string(raw), &req)
	if !result.OK {
	writeChatCompletionError(c, http.StatusBadRequest, "invalid_request_error", "body", "invalid request body", "")
	return
	}
	} else {
	if err := decodeJSONBody(bytes.NewReader(raw), &req); err != nil {
	writeChatCompletionError(c, http.StatusBadRequest, "invalid_request_error", "body", "invalid request body", "")
	return
	}
	}
	if err := validateChatRequest(&req); err != nil {
	chatErr, isChatErr := err.(*chatCompletionRequestError)
	if !isChatErr {
	writeChatCompletionError(c, http.StatusBadRequest, "invalid_request_error", "body", err.Error(), "")
	return
	}
	writeChatCompletionError(c, chatErr.Status, chatErr.Type, chatErr.Param, chatErr.Message, chatErr.Code)
	return
	}
	// PURE-LOCAL: unchanged current behaviour (no Knows gate).
	if h.remote == nil {
	h.serveLocal(c, req)
	return
	}
	// HYBRID: local-first if the resolver knows the model; else remote.
	if h.resolver != nil && h.resolver.Knows(req.Model) {
	var localReq ChatCompletionRequest
	if err := decodeJSONBody(bytes.NewReader(raw), &localReq); err != nil {
	writeChatCompletionError(c, http.StatusBadRequest, "invalid_request_error", "body", "invalid request body", "")
	return
	}
	h.serveLocal(c, localReq)
	return
	}

🤖 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 `@go/chat_completions.go` around lines 771 - 803, When h.remote != nil the code
currently uses core.JSONUnmarshalString to populate req (lenient) which lets
hybrid mode bypass strict validation; fix by only using a lenient/unstructured
unmarshal into a small temp struct (e.g., struct{Model string} or similar) to
inspect the model for h.resolver.Knows, and always call decodeJSONBody (the
strict decoder used elsewhere) to populate the full chat request (req) before
calling validateChatRequest or serveLocal; update the branch that currently
calls core.JSONUnmarshalString to instead peek the model into a temp value and,
if serving locally, re-decode raw with decodeJSONBody into req so
validateChatRequest and serveLocal keep local behaviour intact.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalise adapter lookup key to avoid accidental passthrough.

Adapter resolution is keyed by exact model text (h.remote.adapters[req.Model]). A case variant (for example, registered as claude-3 but requested as CLAUDE-3) can skip the adapter and hit the wrong upstream contract.

Suggested patch

-	h.dispatchRemote(c, req, raw, pool, h.remote.adapters[req.Model])
+	adapter := h.remote.adapters[req.Model]
+	if adapter == nil {
+		adapter = h.remote.adapters[core.Lower(core.Trim(req.Model))]
+	}
+	h.dispatchRemote(c, req, raw, pool, adapter)

Also normalise the registration key in WithChatModelAdapter so write/read use the same canonical form.

📝 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

	pool, found := h.remote.reg.resolve(req.Model)
	if !found {
	writeChatCompletionError(c, http.StatusNotFound, "invalid_request_error", "model", "model not found: "+req.Model, "model_not_found")
	return
	}
	h.dispatchRemote(c, req, raw, pool, h.remote.adapters[req.Model])
	}
	pool, found := h.remote.reg.resolve(req.Model)
	if !found {
	writeChatCompletionError(c, http.StatusNotFound, "invalid_request_error", "model", "model not found: "+req.Model, "model_not_found")
	return
	}
	adapter := h.remote.adapters[req.Model]
	if adapter == nil {
	adapter = h.remote.adapters[core.Lower(core.Trim(req.Model))]
	}
	h.dispatchRemote(c, req, raw, pool, adapter)
	}

🤖 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 `@go/chat_completions.go` around lines 804 - 810, Normalize the adapter lookup
key to a canonical form (e.g., strings.ToLower) so adapter resolution and
dispatch use the same key: when retrieving the pool call h.remote.reg.resolve
with req.Model as before but use the canonicalized model string to index
h.remote.adapters (replace h.remote.adapters[req.Model] with
h.remote.adapters[canonicalModel]); also update the registration path in
WithChatModelAdapter to store adapters under the same canonical key (e.g.,
canonicalModel := strings.ToLower(model) and use that for map writes) so reads
and writes match and case-variants (like "CLAUDE-3" vs "claude-3") do not bypass
the adapter and hit the wrong upstream.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use Core Println in the example output path.

This example currently prints through fmt.Println; the repo guideline asks examples to print through Core.

Suggested patch

 import (
-	"fmt"
+	core "dappco.re/go"
 
 	api "dappco.re/go/api"
 )
@@
-	fmt.Println(engine.Addr())
+	core.Println(engine.Addr())

As per coding guidelines, "Examples use Example<Symbol> or a valid lowercase suffix variant and print through Core Println."

Also applies to: 24-25

🤖 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 `@go/chat_remote_example_test.go` around lines 6 - 9, The example is using
fmt.Println instead of the Core println helper; replace all fmt.Println calls in
the example functions (the Example... test(s) in this file) with core.Println
and update imports: remove the "fmt" import and add the Core package import
(e.g., "dappco.re/go/core"). Ensure every fmt.Println occurrence (including the
ones around lines 24-25) is changed to core.Println so the example prints
through Core as per guidelines.

Source: Coding guidelines

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard the http.DefaultTransport cast to avoid startup panics.

If http.DefaultTransport is replaced with a non-*http.Transport round-tripper, this assertion panics.

Proposed fix

 if cfg.transport == nil {
-	cfg.transport = http.DefaultTransport.(*http.Transport).Clone()
+	if t, ok := http.DefaultTransport.(*http.Transport); ok {
+		cfg.transport = t.Clone()
+	} else {
+		cfg.transport = http.DefaultTransport
+	}
 }

📝 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

	cfg.transport = http.DefaultTransport.(*http.Transport).Clone()
	if cfg.transport == nil {
	if t, ok := http.DefaultTransport.(*http.Transport); ok {
	cfg.transport = t.Clone()
	} else {
	cfg.transport = http.DefaultTransport
	}
	}

🤖 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 `@go/chat_remote.go` at line 37, Guard the cast of http.DefaultTransport to
*http.Transport before calling Clone so we don't panic if DefaultTransport is a
different RoundTripper: in the code path that sets cfg.transport (where you
currently do cfg.transport = http.DefaultTransport.(*http.Transport).Clone()),
check the type with a type assertion or type switch (e.g., t, ok :=
http.DefaultTransport.(*http.Transport)); if ok use t.Clone() to set
cfg.transport, otherwise either clone a new http.Transport with reasonable
defaults or wrap/convert the non-*http.Transport RoundTripper into an
*http.Transport fallback and assign that to cfg.transport; update the assignment
in the same function where cfg.transport is initialized to handle both branches
safely.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Handle body read failures before writing downstream responses.

At Line 104 and Line 131, read errors are ignored. A partial read can be forwarded as a successful/full response, which is incorrect and hard to diagnose.

Proposed fix

-body, _ := io.ReadAll(io.LimitReader(resp.Body, maxUpstreamResponseBytes))
+body, err := io.ReadAll(io.LimitReader(resp.Body, maxUpstreamResponseBytes))
+if err != nil {
+	writeChatCompletionError(c, http.StatusBadGateway, "invalid_request_error", "model", "could not read upstream response", "upstream_read_error")
+	return
+}

Also applies to: 131-131

🤖 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 `@go/chat_remote.go` at line 104, The code currently ignores errors from
io.ReadAll when reading resp.Body (body, _ :=
io.ReadAll(io.LimitReader(resp.Body, maxUpstreamResponseBytes))) which can
forward partial/invalid responses; update the logic in the function that handles
the upstream response to capture the error (e.g., body, err := io.ReadAll(...))
and if err != nil return or write an appropriate error response (and log the
error) instead of streaming/forwarding the (partial) body; apply the same change
to the second occurrence using resp and maxUpstreamResponseBytes so you verify
read success before sending status/headers/body downstream.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Passthrough response handling drops upstream headers.

The passthrough path currently rewrites headers instead of forwarding upstream response headers, so clients lose upstream metadata (for example rate-limit and retry hints), which breaks the advertised verbatim behaviour.

Proposed fix

+for k, vals := range resp.Header {
+	for _, v := range vals {
+		c.Writer.Header().Add(k, v)
+	}
+}
 if adapter == nil {
-	c.Header(hdrContentType, mimeJSON)
 	c.Status(resp.StatusCode)
 	_, _ = c.Writer.Write(body)
 	return
 }

Also applies to: 132-136

🤖 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 `@go/chat_remote.go` around lines 105 - 109, The passthrough branch currently
overwrites headers instead of forwarding the upstream response headers; update
the adapter==nil branch in the function containing the adapter nil check so you
copy all headers from the upstream http.Response (resp.Header) into the Gin
context response (use c.Writer.Header().Set/Add or c.Header for each header)
before setting the status and writing the body, preserving existing Content-Type
behavior (e.g., only set hdrContentType/mimeJSON if upstream did not provide
one). Apply the same header-forwarding change to the second passthrough block
(the other adapter==nil branch around the later response handling) so upstream
metadata like rate-limit/retry headers are forwarded to clients.

[coderabbitai]

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Do not discard stream transcoder errors.

At Line 126, Transcode errors are dropped. This hides truncated or malformed upstream-stream failures after a 200 has already started.

Proposed fix

-		_ = adapter.Transcoder().Transcode(c.Writer, flush, resp.Body, meta)
+		if err := adapter.Transcoder().Transcode(c.Writer, flush, resp.Body, meta); err != nil {
+			_ = c.Error(err)
+		}
 		return

📝 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

	_ = adapter.Transcoder().Transcode(c.Writer, flush, resp.Body, meta)
	if err := adapter.Transcoder().Transcode(c.Writer, flush, resp.Body, meta); err != nil {
	_ = c.Error(err)
	}
	return

🤖 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 `@go/chat_remote.go` at line 126, The Transcode call currently discards errors;
change the call to capture its returned error from
adapter.Transcoder().Transcode(c.Writer, flush, resp.Body, meta) and handle it
instead of assigning to blank identifier: check if err != nil and then
propagate/handle it (e.g., log the error with context including meta/resp info
via the existing logger, close/abort the response stream or return the error up
the call stack so the client is aware of a truncated/malformed upstream stream)
using the surrounding request/response handling routines rather than ignoring
it.

[sonarqubecloud]

Quality Gate failed

Failed conditions
10.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud