fix(server): classify stored-provider sharing from the persisted file, not the env-resolved runtime view - #3040
Conversation
…, not the env-resolved runtime view (#3037 deferred) The preserve-mode rebuild's connection-identity comparisons (stored primary, sharing classification, dedicated-embedding list, and the OCR/embedding/voice restore lists) consumed the RUNTIME GatewayConfig, which folds in per-model environment overrides (KEIKO_MODEL_<ID>_BASE_URL/_API_KEY/_API_KEY_HEADER_NAME). A transient override on exactly one shared provider made the durable file-level sharing relationship invisible, so a credential rotation restored the other provider as dedicated with its already-dead token (Codex finding on #3037, deferred there). Classification now reads the persisted file at gatewayConfig.storagePath, parsed once per setup with vault references resolved and the KEIKO_MODEL_* namespace masked (durableStoredGatewayConfig — the same disk-vs-runtime rule withDiskGatewayEgress draws for egress). The runtime view keeps everything the smoke test verifies: inherited credentials, probe candidates, capability preservation. Verbatim restoration also stops baking transient env values into the rebuilt persisted config. Fresh setups, missing files, and unparseable files fall back to the runtime view — the pre-existing behavior. Failure-first: two pins red before the fix — a chat-URL override and a chat-API-key override each declassified the file-sharing OCR provider during a rotation; the sealed-file second rotation in the first pin proves the vault resolver path. 200/200 gateway-setup tests, tsc, eslint, prettier green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
|
You have reached your Codex usage limits for security reviews. Please try again later. |
|
Keiko for Quality — run summary
Budget: 150000 tokens allotted Recent runs
|
GitHub Actions
Inactive
|
Warning Review limit reached
Next review available in: 40 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughGateway setup now reloads a durable configuration from persisted settings. Preserve-mode restoration uses it to classify providers and retain shared credentials despite runtime overrides. Regression tests cover rotations, vault resealing, malformed data, protocol fields, and dedicated OCR providers. ChangesDurable provider restoration
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (1 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/keiko-server/src/gateway-setup.test.ts`:
- Line 3419: Update the comment near the disk-vs-runtime rule to reference the
production helper name withDiskGatewayEgress instead of
withPersistedGatewayEgress, matching the corresponding production comment.
- Around line 3531-3547: Move the existing ocrCapability helper from the
preceding test to the enclosing describe scope, then replace the inline
capability object in the current test with a call to ocrCapability. Reuse the
shared helper in both tests without changing its returned capability values.
- Around line 3481-3497: Strengthen the savedConnections projection in the
gateway setup rotation test to include each provider’s baseUrl alongside apiKey,
then assert scan-ocr retains its intended base URL rather than
https://elsewhere.example.com/v1 while remote-ocr retains its dedicated URL.
Apply the same baseUrl assertions to the second-rotation checks around the
existing assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e3dea3c6-dc9e-4b00-92a1-a6ed8ce18bc4
📒 Files selected for processing (2)
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (13)
- GitHub Check: Cross-platform smoke (ubuntu-latest)
- GitHub Check: Cross-platform smoke (macos-latest)
- GitHub Check: Cross-platform smoke (windows-latest)
- GitHub Check: Coverage suite (keiko-ui)
- GitHub Check: Build, scan, SBOM, smoke
- GitHub Check: Coverage shard (packages 3/3)
- GitHub Check: ui
- GitHub Check: Core quality
- GitHub Check: Coverage shard (packages 2/3)
- GitHub Check: Coverage suite (scripts)
- GitHub Check: Coverage shard (packages 1/3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript settings, forbidany, require explicit function return types, and model states with discriminated unions rather than casts.
Keep cyclomatic complexity at or below 10, keep functions within 50 non-comment lines, and extract helpers when necessary.
Use the intended logger or diagnostic sink instead ofconsole.*in product code.
Validate hostile inputs before processing, fail closed at trust boundaries, and never route generated or model-produced code around the sandbox egress boundary.
Do not swallow errors with empty catches; surface sufficient diagnostic context, including a correlation ID for server-visible opaque 500 errors.
Files:
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
**/*.{ts,tsx,js,mjs,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier formatting: 2-space indentation, double quotes, semicolons, 100-column width, trailing commas, and LF endings.
Files:
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Use English in code, comments, identifiers, documentation, commit messages, issues, and pull requests.
Respect the inward dependency direction:keiko-contractsis the leaf,keiko-securitydepends only on contracts, and cross-package types belong in contracts.
Inspect and reuse existing helpers, contracts, and subsystems; do not create parallel workspace, graph, policy, evidence, memory, connector, workflow, or UI subsystems without a documented capability gap.
Keep manifests, audit exports, diagnostics, logs, configuration, tests, and evidence redacted: never expose secrets, keys, raw content, endpoints, PII, memory bodies, or customer data.
Never weaken trust boundaries, evidence redaction, governance gates, required checks, or authority limits to make a change pass; fail closed.
Never directly push todev, force-push, bypass required checks, dismiss findings to obtain green status, or merge outside the approved direct-check path.
Use branch names in the formtype/short-slug, never work directly ondev, and use imperative conventional commit subjects with an issue or PR reference and no secrets.
Run the applicable local typecheck, lint, format check, tests, architecture checks, Sonar gate, and touched-area gates before claiming a change or pull request is green; report only commands actually executed.
Files:
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Delete dead code instead of commenting it out and do not leave scaffolding or TODOs hiding unfinished branches.
Files:
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
packages/keiko-server/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
The server may use domain packages but must not depend on
keiko-editor; domain packages must not depend on the server.
Files:
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Tests must be hermetic: no real network, shared mutable global state, wall-clock or ordering races, or assumptions that a port is free; await conditions instead of sleeping.
Files:
packages/keiko-server/src/gateway-setup.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{ts,tsx}: Regression tests must fail before the fix and pass afterward; fixtures and mocks must derive expectations through production entry points rather than reimplementing formulas.
Cover empty, boundary, malformed, and hostile inputs, including both branches of every guard added.
Files:
packages/keiko-server/src/gateway-setup.test.ts
packages/**/src/**
⚙️ CodeRabbit configuration file
packages/**/src/**: Enforce ADR-0019 package direction and the owning trust boundary. Flag provider SDK imports
outside keiko-model-gateway, cross-package wire types outside contracts, workspace escape,
raw evidence bodies, silent failures, and parallel subsystems that should extend an owner.
Files:
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
**/*.test.{ts,tsx,mjs}
⚙️ CodeRabbit configuration file
**/*.test.{ts,tsx,mjs}: A behavioral fix needs a failure-first regression proof that fails without the fix. Cover
malformed, hostile, empty, and boundary inputs; never relax a regression pin or duplicate a
production formula inside a fixture.
Files:
packages/keiko-server/src/gateway-setup.test.ts
🧠 Learnings (5)
📚 Learning: 2026-07-24T19:25:13.892Z
Learnt from: oscharko
Repo: oscharko-dev/Keiko PR: 2697
File: packages/keiko-server/src/editor/inlineCompletionRoutes.ts:616-616
Timestamp: 2026-07-24T19:25:13.892Z
Learning: In `packages/keiko-server`, follow ADR-0141 D5’s AppSession/request-authorization model for workspace root resolution. When reviewing code that performs the `resolveRequestRoot`-style authorization gate, ensure it (1) re-proves persisted workspace identity, (2) derives the request root path, (3) enforces canonical realpath containment within the expected workspace/root to prevent path traversal, (4) verifies workspace presence/validity, and (5) requires a live AppSession (launcher-attested, process-scoped authority per BFF). Do not require a task identity for this gate if AppSession does not model task identity; authorization should be uniform and based on the same-user local threat model rather than per-task/per-workspace session semantics.
Applied to files:
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
📚 Learning: 2026-07-25T18:42:13.123Z
Learnt from: oscharko
Repo: oscharko-dev/Keiko PR: 2716
File: packages/keiko-tools/src/editor-agent-client.test.ts:692-692
Timestamp: 2026-07-25T18:42:13.123Z
Learning: In Keiko TypeScript test files, do not request explicit return type annotations for callbacks passed to typed Vitest `it(...)` and `it.each(...)`. Specifically, avoid adding `: void` or `: Promise<void>` to those callback functions when the ESLint rule `typescript-eslint/explicit-function-return-type` is configured with `allowTypedFunctionExpressions: true` (i.e., typed function expressions are intentionally exempt). Only ask for explicit return types if the ESLint configuration changes; otherwise preserve the existing surrounding test-file style.
Applied to files:
packages/keiko-server/src/gateway-setup.test.ts
📚 Learning: 2026-07-26T21:23:56.288Z
Learnt from: oscharko
Repo: oscharko-dev/Keiko PR: 2755
File: packages/keiko-server/src/grounded-qa-hybrid.ts:1006-1012
Timestamp: 2026-07-26T21:23:56.288Z
Learning: In Keiko’s internal “stable-ID” hash helper functions (e.g., where stable identifiers are computed for persistence/comparison), preserve the existing UTF-16 code-unit iteration semantics when modernizing. Specifically, if the current implementation iterates with `charCodeAt(i)` (code units) rather than `codePointAt(i)` (Unicode code points), keep `charCodeAt(i)` to avoid changing the resulting stable IDs. If changing from `charCodeAt` to `codePointAt` is ever desired, it must be an explicitly planned compatibility/migration decision (e.g., versioning, backfill, or dual-read/write) rather than a mechanical refactor.
Applied to files:
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
📚 Learning: 2026-07-27T10:55:30.485Z
Learnt from: oscharko
Repo: oscharko-dev/Keiko PR: 2766
File: packages/keiko-server/src/qualityIntelligence/figmaSnapshotRoutes.test.ts:1738-1738
Timestamp: 2026-07-27T10:55:30.485Z
Learning: When reviewing TypeScript files, do not flag single-quoted string literals as a violation if they match Prettier’s formatter-approved output. Specifically, if Prettier is configured to prefer double quotes but retains a single-quoted literal solely because switching to double quotes would require escaping embedded double quotes, allow the single quotes (i.e., don’t “fix” it beyond what Prettier would produce).
Applied to files:
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
📚 Learning: 2026-07-27T18:08:22.461Z
Learnt from: oscharko
Repo: oscharko-dev/Keiko PR: 2780
File: packages/keiko-server/src/store/migrations.test.ts:78-95
Timestamp: 2026-07-27T18:08:22.461Z
Learning: When reviewing TypeScript lint findings (e.g., from typescript-eslint/no-unsafe-assignment), ensure the issue is reproducible using the repository’s configured ESLint setup/TS project settings. Do not treat diagnostics observed under a mismatched TypeScript project/tsconfig as authoritative; only accept them as real if you can reproduce them with the same ESLint configuration the repo uses (e.g., via its full/targeted lint scripts). In related test code, prefer deriving branded identity types from the production function return types to keep RootIdentity/brand fields type-compatible and avoid unsafe assignments.
Applied to files:
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
🔇 Additional comments (9)
packages/keiko-server/src/gateway-setup.ts (8)
17-21: LGTM!Also applies to: 69-69
1486-1490: LGTM!Also applies to: 3186-3187, 3357-3358
3216-3224: LGTM!
3245-3245: LGTM!Also applies to: 3265-3265, 3301-3301, 3993-3993
3561-3571: LGTM!
4260-4265: LGTM!
4060-4072: 🩺 Stability & AvailabilityNo change needed here
readGatewayConfigFilealready catches file-read failures and rethrows them asConfigInvalidError, so the existingGatewayErrorcatch still handles stored config read failures.> Likely an incorrect or invalid review comment.
4037-4045: 🗄️ Data Integrity & IntegrationNo change needed.
KEIKO_MODEL_GATEWAY_URL,KEIKO_MODEL_GATEWAY_CAPABILITY, andKEIKO_MODEL_PROFILE_IDare open-code routing variables that do not enter model-gateway config parsing, and the gateway parser only reads per-modelKEIKO_MODEL_<ID>_...runtime overrides under that prefix.packages/keiko-server/src/gateway-setup.test.ts (1)
3412-3581: 📐 Maintainability & Code QualityRemove the stale
keiko.config.jsoncoverage from this comment.
testGatewaySetup()already writesstale-configwithwriteFileSync(storagePath, "stale-config\n", "utf8"), and the test expectsstatus: 200, so the malformed-persisted-file fallback path is covered.> Likely an incorrect or invalid review comment.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cfd8b4aed
ℹ️ 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".
… helper reference (#3040) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
…#3040) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
…e durable classification (#3040) Codex finding: masking the entire KEIKO_MODEL_* namespace broke the durable parse for stored Azure providers whose apiVersion/endpointStyle arrive via env — ConfigInvalidError, silent fallback to the env-resolved runtime view, and the misclassification this PR fixes came back exactly when an identity override was also present. Only BASE_URL/API_KEY/API_KEY_HEADER_NAME are masked now (the three fields sharesStoredGatewayConnection compares); protocol overrides stay available to the parser. New pin red on the broad mask (dead token retained), green on the narrowed one; 201/201, sonar PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d395e45f43
ℹ️ 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".
…tion-classification
…fication (#3040) Covers the documented fail-safe branch (corrupt stored file degrades to the runtime view, the rotation still completes) — the uncovered catch left the PR one point under the new-code coverage bar. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
![keiko-for-quality[bot]](https://avatars.githubusercontent.com/in/4290143?s=60&v=4){kind=small}
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/keiko-server/src/gateway-setup.test.ts`:
- Around line 3661-3666: Update the test around handleGatewaySetup and the
fresh/rotated requests to overwrite the persisted keiko.config.json with
malformed content after fresh succeeds and before rotated executes. Preserve the
in-memory runtime configuration, then keep the rotated assertions unchanged so
the test exercises durableStoredGatewayConfig’s GatewayError fallback.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7d4e73e5-af01-4aac-a285-9b967926a91b
📒 Files selected for processing (2)
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
- GitHub Check: Cross-platform smoke (windows-latest)
- GitHub Check: Cross-platform smoke (macos-latest)
- GitHub Check: Cross-platform smoke (ubuntu-latest)
- GitHub Check: Coverage suite (scripts)
- GitHub Check: ui
- GitHub Check: Coverage suite (keiko-ui)
- GitHub Check: Coverage shard (packages 1/3)
- GitHub Check: Build, scan, SBOM, smoke
- GitHub Check: Core quality
- GitHub Check: Coverage shard (packages 2/3)
- GitHub Check: Coverage shard (packages 3/3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (csharp)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use strict TypeScript settings, forbidany, require explicit function return types, and model states with discriminated unions rather than casts.
Keep cyclomatic complexity at or below 10, keep functions within 50 non-comment lines, and extract helpers when necessary.
Use the intended logger or diagnostic sink instead ofconsole.*in product code.
Validate hostile inputs before processing, fail closed at trust boundaries, and never route generated or model-produced code around the sandbox egress boundary.
Do not swallow errors with empty catches; surface sufficient diagnostic context, including a correlation ID for server-visible opaque 500 errors.
Files:
packages/keiko-server/src/gateway-setup.tspackages/keiko-server/src/gateway-setup.test.ts
**/*.{ts,tsx,js,mjs,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use Prettier formatting: 2-space indentation, double quotes, semicolons, 100-column width, trailing commas, and LF endings.
Files:
packages/keiko-server/src/gateway-setup.tspackages/keiko-server/src/gateway-setup.test.ts
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Use English in code, comments, identifiers, documentation, commit messages, issues, and pull requests.
Respect the inward dependency direction:keiko-contractsis the leaf,keiko-securitydepends only on contracts, and cross-package types belong in contracts.
Inspect and reuse existing helpers, contracts, and subsystems; do not create parallel workspace, graph, policy, evidence, memory, connector, workflow, or UI subsystems without a documented capability gap.
Keep manifests, audit exports, diagnostics, logs, configuration, tests, and evidence redacted: never expose secrets, keys, raw content, endpoints, PII, memory bodies, or customer data.
Never weaken trust boundaries, evidence redaction, governance gates, required checks, or authority limits to make a change pass; fail closed.
Never directly push todev, force-push, bypass required checks, dismiss findings to obtain green status, or merge outside the approved direct-check path.
Use branch names in the formtype/short-slug, never work directly ondev, and use imperative conventional commit subjects with an issue or PR reference and no secrets.
Run the applicable local typecheck, lint, format check, tests, architecture checks, Sonar gate, and touched-area gates before claiming a change or pull request is green; report only commands actually executed.
Files:
packages/keiko-server/src/gateway-setup.tspackages/keiko-server/src/gateway-setup.test.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (AGENTS.md)
Delete dead code instead of commenting it out and do not leave scaffolding or TODOs hiding unfinished branches.
Files:
packages/keiko-server/src/gateway-setup.tspackages/keiko-server/src/gateway-setup.test.ts
packages/keiko-server/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
The server may use domain packages but must not depend on
keiko-editor; domain packages must not depend on the server.
Files:
packages/keiko-server/src/gateway-setup.tspackages/keiko-server/src/gateway-setup.test.ts
packages/**/src/**
⚙️ CodeRabbit configuration file
packages/**/src/**: Enforce ADR-0019 package direction and the owning trust boundary. Flag provider SDK imports
outside keiko-model-gateway, cross-package wire types outside contracts, workspace escape,
raw evidence bodies, silent failures, and parallel subsystems that should extend an owner.
Files:
packages/keiko-server/src/gateway-setup.tspackages/keiko-server/src/gateway-setup.test.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (AGENTS.md)
Tests must be hermetic: no real network, shared mutable global state, wall-clock or ordering races, or assumptions that a port is free; await conditions instead of sleeping.
Files:
packages/keiko-server/src/gateway-setup.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{ts,tsx}: Regression tests must fail before the fix and pass afterward; fixtures and mocks must derive expectations through production entry points rather than reimplementing formulas.
Cover empty, boundary, malformed, and hostile inputs, including both branches of every guard added.
Files:
packages/keiko-server/src/gateway-setup.test.ts
**/*.test.{ts,tsx,mjs}
⚙️ CodeRabbit configuration file
**/*.test.{ts,tsx,mjs}: A behavioral fix needs a failure-first regression proof that fails without the fix. Cover
malformed, hostile, empty, and boundary inputs; never relax a regression pin or duplicate a
production formula inside a fixture.
Files:
packages/keiko-server/src/gateway-setup.test.ts
🧠 Learnings (5)
📚 Learning: 2026-07-24T19:25:13.892Z
Learnt from: oscharko
Repo: oscharko-dev/Keiko PR: 2697
File: packages/keiko-server/src/editor/inlineCompletionRoutes.ts:616-616
Timestamp: 2026-07-24T19:25:13.892Z
Learning: In `packages/keiko-server`, follow ADR-0141 D5’s AppSession/request-authorization model for workspace root resolution. When reviewing code that performs the `resolveRequestRoot`-style authorization gate, ensure it (1) re-proves persisted workspace identity, (2) derives the request root path, (3) enforces canonical realpath containment within the expected workspace/root to prevent path traversal, (4) verifies workspace presence/validity, and (5) requires a live AppSession (launcher-attested, process-scoped authority per BFF). Do not require a task identity for this gate if AppSession does not model task identity; authorization should be uniform and based on the same-user local threat model rather than per-task/per-workspace session semantics.
Applied to files:
packages/keiko-server/src/gateway-setup.tspackages/keiko-server/src/gateway-setup.test.ts
📚 Learning: 2026-07-26T21:23:56.288Z
Learnt from: oscharko
Repo: oscharko-dev/Keiko PR: 2755
File: packages/keiko-server/src/grounded-qa-hybrid.ts:1006-1012
Timestamp: 2026-07-26T21:23:56.288Z
Learning: In Keiko’s internal “stable-ID” hash helper functions (e.g., where stable identifiers are computed for persistence/comparison), preserve the existing UTF-16 code-unit iteration semantics when modernizing. Specifically, if the current implementation iterates with `charCodeAt(i)` (code units) rather than `codePointAt(i)` (Unicode code points), keep `charCodeAt(i)` to avoid changing the resulting stable IDs. If changing from `charCodeAt` to `codePointAt` is ever desired, it must be an explicitly planned compatibility/migration decision (e.g., versioning, backfill, or dual-read/write) rather than a mechanical refactor.
Applied to files:
packages/keiko-server/src/gateway-setup.tspackages/keiko-server/src/gateway-setup.test.ts
📚 Learning: 2026-07-27T10:55:30.485Z
Learnt from: oscharko
Repo: oscharko-dev/Keiko PR: 2766
File: packages/keiko-server/src/qualityIntelligence/figmaSnapshotRoutes.test.ts:1738-1738
Timestamp: 2026-07-27T10:55:30.485Z
Learning: When reviewing TypeScript files, do not flag single-quoted string literals as a violation if they match Prettier’s formatter-approved output. Specifically, if Prettier is configured to prefer double quotes but retains a single-quoted literal solely because switching to double quotes would require escaping embedded double quotes, allow the single quotes (i.e., don’t “fix” it beyond what Prettier would produce).
Applied to files:
packages/keiko-server/src/gateway-setup.tspackages/keiko-server/src/gateway-setup.test.ts
📚 Learning: 2026-07-27T18:08:22.461Z
Learnt from: oscharko
Repo: oscharko-dev/Keiko PR: 2780
File: packages/keiko-server/src/store/migrations.test.ts:78-95
Timestamp: 2026-07-27T18:08:22.461Z
Learning: When reviewing TypeScript lint findings (e.g., from typescript-eslint/no-unsafe-assignment), ensure the issue is reproducible using the repository’s configured ESLint setup/TS project settings. Do not treat diagnostics observed under a mismatched TypeScript project/tsconfig as authoritative; only accept them as real if you can reproduce them with the same ESLint configuration the repo uses (e.g., via its full/targeted lint scripts). In related test code, prefer deriving branded identity types from the production function return types to keep RootIdentity/brand fields type-compatible and avoid unsafe assignments.
Applied to files:
packages/keiko-server/src/gateway-setup.tspackages/keiko-server/src/gateway-setup.test.ts
📚 Learning: 2026-07-25T18:42:13.123Z
Learnt from: oscharko
Repo: oscharko-dev/Keiko PR: 2716
File: packages/keiko-tools/src/editor-agent-client.test.ts:692-692
Timestamp: 2026-07-25T18:42:13.123Z
Learning: In Keiko TypeScript test files, do not request explicit return type annotations for callbacks passed to typed Vitest `it(...)` and `it.each(...)`. Specifically, avoid adding `: void` or `: Promise<void>` to those callback functions when the ESLint rule `typescript-eslint/explicit-function-return-type` is configured with `allowTypedFunctionExpressions: true` (i.e., typed function expressions are intentionally exempt). Only ask for explicit return types if the ESLint configuration changes; otherwise preserve the existing surrounding test-file style.
Applied to files:
packages/keiko-server/src/gateway-setup.test.ts
🔇 Additional comments (2)
packages/keiko-server/src/gateway-setup.ts (1)
452-453: LGTM!Also applies to: 465-475, 502-502, 883-885, 1508-1508, 1780-1781, 2440-2450, 2677-2681, 2766-2787, 2882-2905, 3053-3053, 3068-3085, 3132-3132, 3240-3240, 3411-3411, 4090-4105
packages/keiko-server/src/gateway-setup.test.ts (1)
1073-1200: LGTM!Also applies to: 3407-3413, 3524-3638, 3670-3729, 3752-3752
GitHub Actions
Inactive
GitHub Actions
Inactive
…ON so the durable catch actually runs (#3040) CodeRabbit caught the fresh save rewriting the corrupt file before the rotation read it. Byte-corrupt JSON is the wrong corruption for this pin: the egress-preservation guard (persistedGatewayEgress) refuses a preserve save over a file it cannot read at all — deliberately, rather than risk dropping a persisted egress block. Valid JSON that the gateway parser refuses exercises exactly the durableStoredGatewayConfig fallback: the rotation completes on the runtime view. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
|
) Three review findings, one root each: - A restored provider that follows a shared gateway connection now follows its protocol too. Following the URL, token and header while keeping the old endpointStyle left one connection carrying two protocols, and the restored provider kept requesting the obsolete route. - Inheritance reads the durable file rather than the env-resolved view, and the durable view now drops protocol values the file does not declare. A rotation could otherwise seal a KEIKO_DEFAULT_* or KEIKO_MODEL_* value into the sealed config. Correcting after the parse keeps the #3040 fix intact — nothing is masked, so the file still parses. - Capability reuse treats the protocol tuple as part of the endpoint identity: observations made over the old request route no longer survive a same-URL protocol change that the buffered-chat probe never reverifies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ormed version (#3046) Two review findings: - Dropping every undeclared protocol value from the durable view broke a file that declares azure-openai-deployment and takes its required version from KEIKO_MODEL_<ID>_API_VERSION: the durable view carried azure with no version and inheritance then rejected a routine rotation with 400. Only a version the pair does not need is dropped now — the same coherence argument that stopped #3040 from masking that namespace. - A malformed apiVersion threw inside the candidate loop and surfaced as an opaque 502; the canonical YYYY-MM-DD[-preview] shape is now a named 400. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…etup contract (#3046) * feat(gateway,ui): the generic endpoint protocol travels through the setup contract (#3042 follow-up) An uploaded LiteLLM config states endpointStyle 'openai-compatible' explicitly, but the setup request had no generic protocol field, so a server running KEIKO_DEFAULT_ENDPOINT_STYLE=azure-openai-deployment resolved its own default over the file's statement — a reported upload success followed by requests in the wrong URL shape. - Route: endpointStyle/apiVersion on the setup request, enum-parsed, inherited from the stored primary only on the SAME endpoint, probed through the candidate config, and persisted verbatim on rebuilt generic providers. - Canonical parser: KEIKO_DEFAULT_API_VERSION now applies only to azure-style providers — a style-blind default made every explicitly openai-compatible provider on an Azure-default server unparseable through the pairing rule. - Upload parser: known styles travel (with the canonical pairing mirrored, so an unpairable file refuses as invalid); only unknown styles keep the unsupportedSetting refusal. - Dialog: hidden imported trio bound to the uploaded gateway URL, submitted only while the form still points at exactly that endpoint. Rebased onto current dev by re-applying the chip's own tests and its updates to two existing ones, then verified fresh: 217 server + 239 modals + 1042 gateway tests, tsc across three packages, both lints, prettier, sonar PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(gateway,ui): cover both branches of every protocol guard (#3046) Review findings on #3046, all legitimate coverage gaps: - Server: a preserve-mode rotation on the SAME endpoint inherits the stored style and api version; the same submit against a DIFFERENT host leaves both absent. - Upload parser: two Azure providers whose api versions differ (the earlier fixture differed in style too, so it short-circuited before the version axis) refuse as invalid. - Dialog: an empty gateway URL takes the other branch of the URL binding and omits the imported protocol. Also relocates the four protocol tests that the rebase had inserted into the neighbouring 'rawConfigFromCurrent' describe instead of 'handleGatewaySetup' — syntactically valid but semantically misplaced. Verified against a FORCED rebuild of keiko-model-gateway: keiko-server resolves the built package, and a stale dist had been masking the canonical api-version fix. 218 server + 241 modals + 1042 gateway tests, tsc, both lints, prettier, sonar PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(gateway,ui): atomic protocol replacement, protocol-only saves, mirrored version shape and endpoint identity (#3046) Four Codex findings, each pinned red first: - A submitted endpointStyle now replaces the protocol ATOMICALLY: mixing it with an inherited apiVersion built a protocol the canonical parser refuses, so switching an Azure provider to openai-compatible on the same URL failed the save instead of performing it. - A preserve-mode update carrying ONLY the protocol takes the rebuilding path; the settings-only path accepted the change and never wrote it. - The upload parser mirrors API_VERSION_RE for generic providers, as the voice endpoint already did — 'not-a-date' no longer reports upload success for a file Test & Save refuses. - The dialog binds the imported protocol by CANONICAL endpoint identity, so a semantics-preserving edit (a trailing slash) keeps it while a real endpoint change still drops it. The identity helper reuses the gateway's own linear stripTrailingSlashes shape rather than a backtracking regex. 220 server + 243 modals tests, tsc across three packages, both lints, prettier, sonar PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ui): scope the generic endpoint binding to the uploaded file (#3046) A file that states no gateway URL states no endpoint protocol either. The early return left the previous upload's style bound to a URL the visible field still held, so the stale protocol matched the binding and rode the next submit unseen — the same file-scoped-hidden-state class already closed for the synthesis flag (#3041) and the voice roles (#3042). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(setup): drop the voice prefix from the shared endpoint-style names (#3046) An endpoint protocol is a property of the connection, not of voice. Both sections have always validated against one contract list, but the voice-prefixed constant and helper names made a reviewer read the generic check this PR adds as a voice-only whitelist. Names now carry no section; VOICE_REALTIME_AUTH_MODES keeps its prefix because realtime auth really is voice-only. Behavior identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(setup): read the generic connection from the primary provider and keep a declared protocol (#3046) Four review findings, one root each: - readSetupGatewayCredentials read the gateway connection off providers[0]. Array order is not a contract — a stored file may list a voice provider first — so a stored Azure chat endpoint lost its protocol on an unchanged rotation. It now uses the same primary-chat selection the sharing classification uses. - The deployment requirement keyed only off a Foundry hostname, so a classic Azure host explicitly stated as azure-openai-deployment fell through to generic /models discovery and failed there instead of naming the missing deployments. - The upload cleared a declared protocol whenever the file left the URL to the operator. The declaration now survives unbound and applies to the URL the operator enters; a file declaring nothing still clears it. - The client endpoint identity did not mirror the server's terminal /chat/completions trim, so editing an imported URL down to the form the server derives read as an endpoint change and dropped the protocol. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(setup): pin the azure-only default and the untouched connection (#3046) Two coverage gaps: the override test never asserted that an openai-compatible provider stays free of KEIKO_DEFAULT_API_VERSION, and the protocol-only preserve update never asserted the connection it must leave alone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(setup): treat the endpoint protocol as part of the connection (#3046) Three review findings, one root each: - A restored provider that follows a shared gateway connection now follows its protocol too. Following the URL, token and header while keeping the old endpointStyle left one connection carrying two protocols, and the restored provider kept requesting the obsolete route. - Inheritance reads the durable file rather than the env-resolved view, and the durable view now drops protocol values the file does not declare. A rotation could otherwise seal a KEIKO_DEFAULT_* or KEIKO_MODEL_* value into the sealed config. Correcting after the parse keeps the #3040 fix intact — nothing is masked, so the file still parses. - Capability reuse treats the protocol tuple as part of the endpoint identity: observations made over the old request route no longer survive a same-URL protocol change that the buffered-chat probe never reverifies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(setup): name the endpoint-protocol pairing instead of failing verification (#3046) An api version with no style, or a submitted version over an inherited openai-compatible style, built a pair the canonical parser refuses. The throw surfaced as an opaque 502 "credentials could not be verified" — a request problem reported as an upstream one. The pairing is now checked on the EFFECTIVE protocol and returns a named 400. Bumping the api version of an endpoint whose stored style already is the deployment path stays legal, which a blunt "reject apiVersion without endpointStyle" rule would have broken; both branches are pinned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(setup): keep the protocol comparisons on the durable, declared values (#3046) Four review findings, each a case the previous round got one step wrong: - withFileDeclaredProtocol now takes the FILE's own value, not the resolved one, so a KEIKO_MODEL_<ID>_API_VERSION that overrides a DECLARED version is no longer sealed in either. - A restored provider follows the gateway protocol only if it spoke it before. One that deliberately used a different valid protocol over the same connection keeps its own — that request shape was never verified for it. - Capability identity compares the durable protocol against the submitted one, so a rotation that merely re-resolves a KEIKO_DEFAULT_* tuple the file never declared no longer discards verified observations. - The Azure deployment path without an api version returns a named 400 instead of throwing inside the candidate loop as a misleading 502. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(setup): keep an env-completed Azure pair coherent and name a malformed version (#3046) Two review findings: - Dropping every undeclared protocol value from the durable view broke a file that declares azure-openai-deployment and takes its required version from KEIKO_MODEL_<ID>_API_VERSION: the durable view carried azure with no version and inheritance then rejected a routine rotation with 400. Only a version the pair does not need is dropped now — the same coherence argument that stopped #3040 from masking that namespace. - A malformed apiVersion threw inside the candidate loop and surfaced as an opaque 502; the canonical YYYY-MM-DD[-preview] shape is now a named 400. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(setup): validate the connection with the protocol it will persist (#3046) Found while pinning the env-completed tuple: the connection probe parsed a provider with NO protocol, so on a server that sets only KEIKO_DEFAULT_ENDPOINT_STYLE the environment turned every probe into an Azure provider with no api version and the canonical pairing rejected EVERY setup request — including one that submitted its own protocol. Four more review findings with it: - The declared protocol pair stays coherent in BOTH directions: a file that declares the version while the environment supplies the style is as valid as the inverse, and clearing either half broke the next rotation. - A dedicated embedding that spoke its own protocol over a shared connection is no longer rebuilt with the setup-wide one. - withFileDeclaredProtocol now runs only inside the successful durable parse, never over the runtime fallback a failed parse returns. - Renamed storedPrimaryOrSelf to storedProviderForModel and moved the capability-reuse comment onto the rule it describes; the generic upload apply moved out of the voice-scoped helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(setup): judge the protocol by what the gateway will actually speak (#3046) Two review findings: - An absent endpoint style and an explicit openai-compatible are the same protocol — the adapter sends the identical request shape — so a same-URL import that merely spells the default out no longer discards verified capability observations. - The deployment requirement follows the EFFECTIVE style: a request that omits the field still lands on the Azure deployment path when KEIKO_DEFAULT_ENDPOINT_STYLE says so, and generic /models discovery fails there exactly the same way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(setup): compare the protocol the gateway will speak, on both sides (#3046) Comparing the durable view against the raw submitted fields read a file that declares nothing while KEIKO_DEFAULT_* resolves Azure as "unchanged" when the request explicitly switched to openai-compatible, so observations made over the deployment path survived onto a different request shape. Both sides are now the effective protocol: the statement if there is one, else the declared value, else what the environment already resolves the provider to — which keeps a plain rotation comparing equal. The env-default override pin also asserts the environment adds no api version to a provider it was told not to apply to. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Summary
The preserve-mode rebuild's connection-identity comparisons (stored primary, sharing classification, dedicated-embedding list, and the OCR/embedding/voice restore lists) consumed the RUNTIME
GatewayConfig, which folds in per-model environment overrides (KEIKO_MODEL_<ID>_BASE_URL/_API_KEY/_API_KEY_HEADER_NAME). A transient override on exactly one shared provider made the durable file-level sharing relationship invisible — a credential rotation then restored the other provider as "dedicated" with its already-dead token (Codex finding on #3037, deferred there).Classification now reads the persisted file at
gatewayConfig.storagePath, parsed once per setup with vault references resolved and theKEIKO_MODEL_*namespace masked (durableStoredGatewayConfig— the same disk-vs-runtime rulewithDiskGatewayEgressdraws for egress). The runtime view keeps everything the smoke test verifies: inherited credentials, probe candidates, capability preservation. Verbatim restoration also stops baking transient env values into the rebuilt persisted config. Fresh setups, missing files, and unparseable files fall back to the runtime view — the pre-existing behavior for those states.Reuse / No-Duplication
No new parser and no second config source: the durable view is
loadConfigFromFile(the boot loader) with the existing vaultcreateProviderSecretResolver, threaded through the existingSetupRequestAssembly → SetupRequest → SetupVerificationInputchain as onestoredfield.Verification
gateway-setup.test.ts200/200 on this branch atop current dev;tsc -bkeiko-server; repo ESLint; prettier;npm run gates:sonarPASS on the final diff.Update-Impact
Behavioral: credential rotations under transient per-model env overrides now keep file-shared providers on the rotated connection (previously they kept the dead token); rebuilt persisted configs carry durable file values instead of env-view values. No wire-format or migration impact.
🤖 Generated with Claude Code