fix(gateway,ui): an audio endpoint move must restate its protocol, and the dialog can state it - #3048
Conversation
GitHub Actions
Inactive
|
Warning Review limit reached
Next review available in: 30 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 (3)
📝 WalkthroughWalkthroughChangesThe gateway setup UI now supports endpoint style, API version, and realtime authentication configuration. Protocol metadata is bound to canonical endpoint identities and restored for matching imported endpoints. The server validates protocol restatement during audio endpoint migrations and updates stored protocol fields. Voice endpoint protocol handling
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ❌ 2❌ Failed checks (2 warnings)
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 |
|
You have reached your Codex usage limits for security reviews. Please try again later. |
![keiko-for-quality[bot]](https://avatars.githubusercontent.com/in/4290143?s=60&v=4){kind=small}
|
Keiko for Quality — run summary
Budget: 1156064 tokens allotted, 163185 reported Recent runs
|
GitHub Actions
Inactive
![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: a2fdff8e99
ℹ️ 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".
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: 9f3d6b7607
ℹ️ 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".
) Three review findings, one root each: - Stating the protocol by hand released the uploaded URL binding outright. In a fresh dialog the endpoint identity ref is still undefined after an upload (the upload sets the URL programmatically), so a later URL change fires no reset and the old host's protocol followed the operator to the new host. A statement now binds to the URL in the form when it is made, so a move drops it and the server's restatement rule applies. - hasVoiceCredentialInput never looked at the two protocol fields, so correcting only the style or api version left Test & save disabled. - The endpoint-move test never selected a style, so it passed with the whole selector removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
GitHub Actions
Inactive
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52086030d6
ℹ️ 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".
…3048) Four review findings on the payload gate: - A blank audio URL in preserve mode means "keep the stored endpoint", so a protocol stated against it is submitted instead of dropped; an uploaded protocol still stays with the URL it was declared for. - The binding is compared through canonical endpoint identity, so a trailing slash or a differently-cased hostname is no longer read as a move — the server compares the same way. - Selecting openai-compatible clears the api version, the pair the gateway parser refuses. - resetEndpointDependencies clears the binding along with the two protocol fields it already cleared. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
#3048) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
…d the dialog can state it (#3042 follow-up) Codex finding: voiceConnectionEndpointOptions rightly refuses to carry a stored endpointStyle across a base-URL change, but the dialog could only submit a protocol while the form still pointed at an UPLOADED url — so a manual Azure-to-Azure move saved a voice provider with no style and every audio call silently took the OpenAI-compatible URL shape. Both halves, because either alone leaves a trap: - Server: an endpoint migration whose stored template declared a protocol must restate it, joining the credential, locality and role restatements the same guard already requires. - UI: the audio endpoint style and API version are real fields now, beside the provider-locality select they mirror. An uploaded protocol lands in them and stays URL-BOUND (a fresh dialog never commits an endpoint identity, so the binding — not an identity transition — is what stops it riding a retyped URL); stating one by hand releases the binding, because the operator is declaring it for the endpoint currently in the form. The invariant is unchanged: a protocol is never inherited silently across a base-URL change. Pins: the move is refused without a protocol and accepted with one (both red before), and the stated style survives a manual move (red under sabotage, re-proven on this rebase). 216 server + 234 modals tests, tsc, both lints, prettier, sonar PASS. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ow-up) The dialog fields added here shipped hard-coded English, which check:ui-i18n rejects for a file that already routes its text through the catalog. The four option and hint labels now come from the optional-widget catalog with English and German entries added together. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
) Three review findings, one root each: - Stating the protocol by hand released the uploaded URL binding outright. In a fresh dialog the endpoint identity ref is still undefined after an upload (the upload sets the URL programmatically), so a later URL change fires no reset and the old host's protocol followed the operator to the new host. A statement now binds to the URL in the form when it is made, so a move drops it and the server's restatement rule applies. - hasVoiceCredentialInput never looked at the two protocol fields, so correcting only the style or api version left Test & save disabled. - The endpoint-move test never selected a style, so it passed with the whole selector removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…3048) Four review findings on the payload gate: - A blank audio URL in preserve mode means "keep the stored endpoint", so a protocol stated against it is submitted instead of dropped; an uploaded protocol still stays with the URL it was declared for. - The binding is compared through canonical endpoint identity, so a trailing slash or a differently-cased hostname is no longer read as a move — the server compares the same way. - Selecting openai-compatible clears the api version, the pair the gateway parser refuses. - resetEndpointDependencies clears the binding along with the two protocol fields it already cleared. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
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: a2439b3433
ℹ️ 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".
…rd (#3048) - The requirement follows "no server-side guard will run", not the mode flag: that guard needs a stored voice provider, so a preserve-mode setup configuring audio for the first time was just as unguarded. - Typing the uploaded URL back never leaves the committed identity, so no transition fired and the restore inside the reset was never reached. The return is now handled where the URL changes. - A stated Azure deployment path with a blank api version is refused in the dialog instead of coming back as a server 400 to decode. - The migration refusal has its own message naming the protocol, in English and German, instead of borrowing the credential/locality/roles one. 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: 58bfba8c9f
ℹ️ 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".
…g to (#3048) - The client pairing check exempts a RETAINED endpoint: a blank audio URL in preserve mode keeps the stored one, and restating its own Azure style there inherits the stored version server-side, so demanding it back refused a supported correction. - The realtime auth restatement fires only when a REALTIME role is actually moving: a stored provider combining Realtime with speech output declares the mode, but moving speech output alone leaves Realtime where it is. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
GitHub Actions
Inactive
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`:
- Around line 1749-1762: Strengthen the assertion in the movedOutput test around
handleGatewaySetup so it verifies the concrete successful outcome of moving the
multi-role provider endpoint, rather than only checking that the response omits
“realtime auth mode.” Assert the expected response status and/or body fields
indicating the move succeeded, while preserving the existing setup and cleanup.
In
`@packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsx`:
- Around line 1985-1987: Wrap the API version assertion following the
“OpenAI-compatible” option click in GatewaySetupDialog tests with waitFor,
matching the existing patterns at the sibling assertions around the related
option changes. Keep the expected empty value unchanged and await the React
state update before asserting.
In `@packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx`:
- Around line 1733-1805: Replace VoiceEndpointStyleField and
VoiceRealtimeAuthModeField with one shared select-field component that reuses
the existing prop shape and accepts the label/translation key plus sections
function as parameters. Update both call sites to provide their respective keys
and section builders, and use protocol-specific shared class names such as
gw-protocol-select and gw-protocol-menu instead of the locality classes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7b5fb0ac-a0d4-4eef-9ae1-52792ee92d86
📒 Files selected for processing (6)
packages/keiko-server/src/gateway-setup.test.tspackages/keiko-server/src/gateway-setup.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsxpackages/keiko-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (14)
- GitHub Check: sign review store
- GitHub Check: Cross-platform smoke (macos-latest)
- GitHub Check: Cross-platform smoke (ubuntu-latest)
- GitHub Check: Cross-platform smoke (windows-latest)
- GitHub Check: Coverage shard (packages 2/3)
- GitHub Check: Build, scan, SBOM, smoke
- GitHub Check: Coverage suite (keiko-ui)
- GitHub Check: Coverage suite (scripts)
- GitHub Check: Coverage shard (packages 3/3)
- GitHub Check: ui
- GitHub Check: Core quality
- GitHub Check: Coverage shard (packages 1/3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{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-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.tspackages/keiko-server/src/gateway-setup.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-server/src/gateway-setup.test.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx
packages/keiko-ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/keiko-ui/**/*.{ts,tsx}: Thekeiko-uiworkspace must enforce the root values for complexity, maximum function length, and explicit function return types; its suppression register may only shrink.
The UI must communicate with the server through shared contract types and must not redeclare governed wire types.
Files:
packages/keiko-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx
**/*.{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-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.tspackages/keiko-server/src/gateway-setup.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-server/src/gateway-setup.test.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx
**/*
📄 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-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.tspackages/keiko-server/src/gateway-setup.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-server/src/gateway-setup.test.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx
**/*.{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-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.tspackages/keiko-server/src/gateway-setup.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-server/src/gateway-setup.test.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx
packages/keiko-ui/**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
When changing UI code or styling, preserve the SHA-pinned visual-proof and accessibility expectations and run the required UI gates.
Files:
packages/keiko-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx
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-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.tspackages/keiko-server/src/gateway-setup.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-server/src/gateway-setup.test.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx
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
**/*.{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-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/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-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/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-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-server/src/gateway-setup.test.ts
🧠 Learnings (5)
📚 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-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.tspackages/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-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.tspackages/keiko-server/src/gateway-setup.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-server/src/gateway-setup.test.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx
📚 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-ui/src/lib/i18n-messages.optional.de.tspackages/keiko-ui/src/lib/i18n-messages.optional.en.tspackages/keiko-server/src/gateway-setup.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsxpackages/keiko-server/src/gateway-setup.test.tspackages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx
📚 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-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 (18)
packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.tsx (9)
186-197: LGTM!Also applies to: 349-392, 644-665
247-284: LGTM!
667-728: LGTM!
757-767: LGTM!Also applies to: 800-838, 857-857
1807-1831: LGTM!Also applies to: 2002-2009, 2103-2119
2196-2250: LGTM!Also applies to: 2273-2273
2499-2552: LGTM!
2637-2652: LGTM!Also applies to: 2741-2758
2985-3003: LGTM!Also applies to: 3050-3053, 3107-3109, 3201-3208
packages/keiko-ui/src/lib/i18n-messages.optional.de.ts (1)
833-846: LGTM!packages/keiko-ui/src/lib/i18n-messages.optional.en.ts (1)
802-815: LGTM!packages/keiko-server/src/gateway-setup.ts (2)
2560-2596: LGTM!Also applies to: 2615-2616
3023-3044: LGTM!packages/keiko-server/src/gateway-setup.test.ts (2)
1396-1475: LGTM!Also applies to: 1477-1552
1656-1709: LGTM!Also applies to: 1765-1819, 1821-1875, 1877-1930
packages/keiko-ui/src/app/components/desktop/modals/GatewaySetupDialog.test.tsx (3)
1764-1808: LGTM!Also applies to: 1810-1842, 1844-1884, 1886-1917, 1919-1957
1996-2066: LGTM!Also applies to: 2068-2086, 2088-2161, 2163-2193
2195-2219: LGTM!Also applies to: 2221-2273, 2275-2363, 2627-2632
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03b5bafb1a
ℹ️ 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".
Four review rounds each corrected a client-side rule that had guessed wrong, because the dialog cannot see the stored provider's protocol — whether a restatement is required, and whether an api version can be inherited, both depend on it. The server can see it, decides on the whole request, and already answers with named errors. The client-side migration and pairing rules are removed; what stays is the dialog's own job: which fields are visible, what an upload hydrates, and what an endpoint change clears or restores. With no stored voice provider there is no stored protocol to lose, so the two pins go back to asserting that the payload omits it — which is what the reviewers asked for independently. Also: the undo-restore no longer overwrites a hand-corrected protocol, the role-scope pin asserts its status, one awaited assertion, and the two identical protocol selects are one component. 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: 7d1b1eb695
ℹ️ 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".
…3048) The restore filled any EMPTY protocol field, which cannot tell a field the reset cleared from one the operator deliberately unstated — an equivalent URL edit then undid their own choice by putting the file's Azure style back. The binding says which it was: the reset empties it, any hand statement rebinds to the URL in the form. 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: 71a6eb6a4e
ℹ️ 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".
|
1 participant
Summary
Moving a voice endpoint through the setup dialog silently lost its protocol.
voiceConnectionEndpointOptionsrightly refuses to carry a storedendpointStyleacross a base-URL change (it was declared for the old host), while the dialog submitted a protocol only while the form still pointed at an uploaded URL. A manual Azure-to-Azure move therefore saved a voice provider with no style at all, and every audio call then took the OpenAI-compatible URL shape — a successful save with a broken runtime (Codex finding on #3042, deferred there).Both halves are needed; either alone is a trap. Refusing without a way to comply would be a dead end, and adding fields alone would leave the silent degradation for anyone who leaves them empty:
One subtlety worth recording: the first attempt reset the fields on an endpoint-identity transition, which broke the existing "uploaded protocol must not ride a retyped URL" pin — a fresh dialog never commits an identity, so the transition never fires. The URL binding is what actually holds.
The invariant is unchanged: a protocol is never inherited silently across a base-URL change.
Reuse / No-Duplication
The fields mirror the existing
VoiceProviderLocalityFieldselect and theimportedVoiceEndpoint*binding pattern; the server guard is a new clause insideexplicitEndpointMigrationError, next to the credential, locality and role clauses.Verification
tsc -bserver+ui, repo ESLint, keiko-ui workspace lint, prettier,npm run gates:sonarPASS.Update-Impact
Behavioral: an audio endpoint migration that previously saved with a degraded protocol is now refused with a message naming what to supply, and the dialog can supply it. Unrelated voice updates (credential rotation, role changes) at the same endpoint are untouched.
🤖 Generated with Claude Code