feat(issue-273): unify onboarding and pairing flow across CLI, web, and mobile clients#308
Merged
yuniel-acosta merged 4 commits intoMar 24, 2026
Conversation
…nd mobile clients Implement canonical onboarding state machine with shared trust/transport/ready lifecycle across all client surfaces. Each surface maps platform-specific transport failures into normalized recovery states. Review fixes applied: - fix: restore HTTPS acceptance for non-localhost remote deployments - fix: remove destructive watcher that wiped chat messages on transient disconnects - fix: restore .gitignore patterns for .bak files and add .profraw coverage artifacts - fix: remove dead ternary in dashboard onboarding steps - fix: add targeted bearer credential leak detection in security test - fix: resolve clippy pedantic warnings (struct_excessive_bools, single_char_pattern) - fix: update onboardingContract.spec.ts import path and spec assertions - chore: add gateway-api.md placeholder to fix broken link Refs: #273
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR unifies onboarding/pairing across surfaces by adding canonical onboarding state machines and mappings, implementing HTTP/mobile bridge onboarding flows in Rust/Kotlin/TypeScript, refactoring mobile and web UIs to use bridge/session state, adding tests and i18n, and updating specs and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebApp as Web App (chat/dashboard)
participant Gateway as HTTP Gateway
participant Bridge as CLI Bridge / Runtime
participant Storage as SessionStorage
User->>WebApp: Interact (pair/connect/start session)
WebApp->>Gateway: health / pair / connect (useGateway)
Gateway-->>WebApp: onboarding state / bearer token / errors
alt gateway ready
WebApp->>Storage: persist sessionId
WebApp->>Bridge: (when mobile) link via CLI bridge probe
Bridge-->>WebApp: BridgeLinkSnapshot -> mapped OnboardingState
else blocked / auth error
Gateway-->>WebApp: recoveryKind -> show guidance
end
WebApp->>Gateway: send message (Authorization: Bearer, X-Session-Id) (useChat)
Gateway-->>WebApp: assistant reply or error
WebApp->>User: render messages / recovery UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-03-24 to 2026-03-24 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 33
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
clients/composeApp/src/iosMain/kotlin/com/profiletailors/corvus/Platform.ios.kt (1)
5-10:⚠️ Potential issue | 🟡 MinorPipeline failure: Detekt
MatchingDeclarationNameviolation.The file is named
Platform.ios.ktbut containsIOSPlatformas the single top-level declaration. Either rename the file toIOSPlatform.ktor suppress the rule if thePlatform.<target>.ktconvention is intentional for KMP actual/expect alignment.Option 1: Rename file
Rename
Platform.ios.kt→IOSPlatform.ktOption 2: Suppress if convention is intentional
+@file:Suppress("MatchingDeclarationName") package com.profiletailors.corvus🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/iosMain/kotlin/com/profiletailors/corvus/Platform.ios.kt` around lines 5 - 10, The class declaration is IOSPlatform but the file is Platform.ios.kt which triggers Detekt's MatchingDeclarationName; either rename the file to IOSPlatform.kt to match the top-level declaration, or keep the filename and suppress the rule by adding a file-level suppression (e.g., `@file`:Suppress("MatchingDeclarationName")) at the top of the file; update the Platform.ios.kt file containing class IOSPlatform accordingly and ensure the suppression annotation is placed before any package or import statements if you choose that route.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/App.kt (1)
24-88: 🧹 Nitpick | 🔵 TrivialAddress Detekt LongMethod warning (64 lines, max 60).
Consider extracting the
ChatWorkspacecallback handlers into a separate composable or helper to reduce the function length.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/App.kt` around lines 24 - 88, The App function is over the Detekt LongMethod limit because the ChatWorkspace call embeds multiple inline callback lambdas; extract those handlers into a separate helper or composable (e.g., create functions like handleRetryBridge(bridgeSnapshot, setBridgeSnapshot), handleLinkSurface(...), handleStartSession(...), handleClearSession(...)) and call them from the ChatWorkspace parameters (onRetryBridge = { handleRetryBridge(...) }, etc.); ensure the helpers update the same mutable state (bridgeSnapshot) or accept a setter/MutableState reference and preserve existing conditions (environmentSupported checks, copy mutations, generateSessionId usage, and toOnboardingState/status logic) so App remains concise while behavior is unchanged.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/onboarding/OnboardingScreen.kt (2)
113-122: 🛠️ Refactor suggestion | 🟠 MajorAddress Detekt warnings: LongParameterList and LongMethod.
The function has 7 parameters (threshold: 6) and 75 lines (max: 60). Consider:
- Grouping related parameters into a state holder class
- Extracting inner content sections into smaller composables
Suggested refactor for parameters
`@Immutable` data class OnboardingScreenState( val step: OnboardingStep, val currentStepIndex: Int, val totalSteps: Int, val isLastStep: Boolean, ) `@Composable` fun OnboardingScreen( state: OnboardingScreenState, onSkip: () -> Unit, onNext: () -> Unit, modifier: Modifier = Modifier, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/onboarding/OnboardingScreen.kt` around lines 113 - 122, The OnboardingScreen function violates Detekt LongParameterList/LongMethod—refactor by introducing an immutable state holder (e.g., OnboardingScreenState containing step, currentStepIndex, totalSteps, isLastStep) and change the OnboardingScreen signature to accept that state plus onSkip, onNext, and modifier; then extract large inner sections into smaller composables (e.g., OnboardingHeader, OnboardingContent, OnboardingFooter or similar) called from OnboardingScreen to reduce method length and improve readability, and update all call sites to pass the new state object instead of separate parameters.
250-261:⚠️ Potential issue | 🟡 MinorEmoji icons may not be accessible to screen readers.
The icon text uses emoji characters (
⚙,🔗,📡,💬) which may not announce meaningfully to assistive technology. Consider adding acontentDescriptionor using Material Icons instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/onboarding/OnboardingScreen.kt` around lines 250 - 261, The emoji-only Text inside Box is not accessible; update the OnboardingScreen UI to provide screen-reader friendly descriptions by mapping each OnboardingIcon (OnboardingIcon.RUNTIME, .LINK, .SYNC, .SESSION) to a short content string (e.g., "Runtime", "Link", "Sync", "Session") and apply it via semantics on the composable showing the icon (either add Modifier.semantics { contentDescription = ... } to the Text or the Box, or replace Text with an Icon + contentDescription). Ensure the mapping logic is in the same scope as the current Text usage so the correct description is selected for the displayed emoji.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt (1)
52-185:⚠️ Potential issue | 🟠 Major
ConfigSettingsListexceeds detekt's method length limit (120 > 60 lines).The pipeline flags this as a
LongMethodviolation. Extract logical sections into smaller composables:♻️ Suggested extraction
`@Composable` internal fun ConfigSettingsList(bridgeState: MobileBridgeUiState, actions: ChatWorkspaceActions) { val corvusColors = CorvusTheme.colors val onboardingState = bridgeState.onboardingState val detailLines = buildDiagnosticsLines(bridgeState) LazyColumn( modifier = Modifier.fillMaxSize().padding(20.dp), verticalArrangement = Arrangement.spacedBy(20.dp), ) { - item { /* header content */ } - item { /* status indicator */ } - item { /* current state card */ } - item { /* recovery guidance card */ } - item { /* action buttons */ } - item { /* divider */ } - item { /* trust boundary card */ } + item { ConfigHeader(bridgeState) } + item { ConfigStatusIndicator(bridgeState, onboardingState) } + item { CurrentStateCard(bridgeState, detailLines) } + item { RecoveryGuidanceCard(bridgeState) } + item { ConfigActionButtons(onboardingState, bridgeState, actions, corvusColors) } + item { ConfigDivider(corvusColors) } + item { TrustBoundaryCard() } item { Spacer(modifier = Modifier.height(32.dp)) } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt` around lines 52 - 185, ConfigSettingsList is too long (detekt LongMethod); split it into smaller composables to reduce length. Extract logical sections into dedicated `@Composable` functions (for example: ConfigHeader (renders the "Bridge Linking" title and headline using bridgeStateHeadline), StatusBlock (wraps StatusIndicator), CurrentStateCard (calls diagnosticsCard with buildDiagnosticsLines), RecoveryCard (calls diagnosticsCard with bridgeStateDescription/bridgeStateRecovery), ActionRow (renders the OutlinedButton + when(onboardingState.status) GradientButton cases using actions.onRetryBridge/onLinkSurface/onStartSession/onClearSession), DividerLine (the Box with Brush), and TrustBoundaryCard (the trust diagnosticsCard)). Replace the inlined blocks inside ConfigSettingsList with calls to these new composables so ConfigSettingsList becomes a short coordinator that passes bridgeState, onboardingState, detailLines, corvusColors and actions as needed.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt (1)
232-303:⚠️ Potential issue | 🟠 Major
ChatInputFieldis already breaking the Detekt gate.This composable now trips both
LongParameterListandLongMethod, so the PR cannot pass the configured lint rules as-is. Extract the send button / text-field styling into helpers or pass a compact input state object.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt` around lines 232 - 303, ChatInputField is too large and triggers Detekt's LongParameterList and LongMethod rules; split responsibilities by extracting the send-button UI and the styled text-field into smaller composables or a compact input state object. Create a new composable (e.g., SendButton or ChatSendButton) that takes onSend: () -> Unit and isEnabled: Boolean and moves the Box/IconButton/Icon styling there, and extract the OutlinedTextField and its styling into a StyledChatTextField (or accept a ChatInputState data class with value/onValueChange/placeholder/enabled) so ChatInputField only composes those helpers; update callers to use the new helper composables or pass the compact state. Ensure you keep existing symbols: ChatInputField, OutlinedTextField usage, and the gradient / corvusColors references when moving styling so behavior remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/gateway/mod.rs`:
- Around line 645-717: These helper enums (HttpHealthProbe, HttpPairOutcome,
HttpAuthenticatedFollowUp) and mapping functions
(map_health_to_http_onboarding_state, map_pair_to_http_onboarding_state,
map_authenticated_follow_up_to_http_onboarding_state) are only used by tests and
are producing dead-code warnings; either wrap their definitions with
#[cfg(test)] (and move them into the test module) or expose/integrate them into
the runtime API so they are referenced at runtime. Locate the declarations in
gateway/mod.rs and apply #[cfg(test)] to the enums and functions (or refactor so
the runtime path references these symbols) to remove the clippy dead-code
warnings.
In `@clients/agent-runtime/src/onboard/wizard.rs`:
- Around line 288-300: The existing match on normalized.state (used to set
trust_line) treats Blocked the same regardless of why; change the logic to
inspect normalized.recovery_kind (or the concrete DashboardActivationStatus)
alongside CanonicalOnboardingStateKind so transport/UI/auth failures (e.g.,
DashboardUiUnavailable or UnknownLocalFailure) do not get the “wait for runtime
availability” copy; update the match/if branches in the trust_line construction
to render a message for gateway-healthy-but-UI/auth transport failures and only
use the “wait for runtime” wording when the dashboard activation status
indicates the runtime is actually unavailable.
- Around line 303-310: Update the onboarding guide text to reference the proxied
dashboard origin (http://corvus.localhost) and its proxied health endpoint
instead of the direct localhost/127.0.0.1 addresses: change the formatted
strings that use DASHBOARD_GATEWAY_URL and DASHBOARD_UI_URL in the vector (the
block that builds the runtime/trust/connect steps in onboard::wizard.rs) to
point to the canonical proxied origin (corvus.localhost) and mention the proxied
/api/health path so users are guided to the operator path that matches the new
dashboard spec/runtime contract.
In
`@clients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/MainActivity.kt`:
- Around line 22-27: Detekt is flagging FunctionNaming for the `@Preview`
composable AppAndroidPreview; either suppress the rule for this function by
adding the appropriate `@Suppress`("FunctionNaming") annotation to the
AppAndroidPreview declaration or update the Detekt config to exclude
`@Preview-annotated` functions from FunctionNaming checks (ensure you reference
the AppAndroidPreview composable and the FunctionNaming rule when editing
config).
In
`@clients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/Platform.android.kt`:
- Around line 5-9: Detekt is flagging a false positive for the Android actual of
the Platform expect/actual pattern; either add a file-level suppression in the
Android actual file by placing `@file`:Suppress("MatchingDeclarationName") at the
very top of Platform.android.kt (so the class AndroidPlatform and actual
Platform implementation are ignored), or update the Detekt config to exclude
this KMP pattern (add an exclude or suppression for the glob **/Platform.*.kt or
rule MatchingDeclarationName for that pattern) so the actual implementation of
Platform (class AndroidPlatform) is not reported.
In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/App.kt`:
- Around line 59-63: The onRetryBridge handler updates
bridgeSnapshot.runtimeAvailable and sessionCapable but omits setting
linkEstablished, creating an inconsistent snapshot for toOnboardingState()
checks; update the onRetryBridge lambda so it sets linkEstablished = true
together with runtimeAvailable = true and sessionCapable = true (mirroring the
pattern in onLinkSurface) on the bridgeSnapshot.copy(...) call to ensure the
state machine can transition to SESSION_PENDING/SESSION_READY.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt`:
- Around line 468-525: The hard-coded English UI strings in
onboardingStateLabel, bridgeStateHeadline, bridgeStateDescription,
bridgeStateRecovery and buildLocalAssistantReply must be moved to
resource-backed formatters; replace each literal return value with a call to a
localized string provider (e.g., a resources.getString(key, args) or Compose
stringResource wrapper) keyed for each status/headline/description/recovery case
and pass dynamic values (bridgeState.platformName, prompt, modelName, sessionId)
as format args; add entries to the resources files (default and translations)
for keys that map to each branch (including the null-recovery inner branches)
and update callers to use the new formatter functions so the functions only
select the resource key/args rather than returning hard-coded text.
- Around line 393-396: The details list currently always adds
bridgeStateDescription(bridgeState) and then bridgeStateRecovery(bridgeState),
causing duplicate recovery text when bridgeStateDescription already delegates to
bridgeStateRecovery for BLOCKED; update the construction of details (the details
variable) to only append bridgeStateRecovery(bridgeState) when
bridgeStateDescription does not already include recovery (e.g., guard on
bridgeState != BLOCKED or check a predicate/return value) so recovery text is
not added twice; reference bridgeStateDescription, bridgeStateRecovery and
bridgeState when implementing the conditional addition.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 107-113: The ChatWorkspace and ChatPanel composable signatures
currently exceed Detekt's parameter-count rule; refactor by grouping related
bridge/session callbacks and repeated parameters into a single props/data object
(e.g., create a ChatWorkspaceProps or BridgeSessionProps that contains
bridgeSnapshot and callbacks like onRetryBridge, onLinkSurface, onStartSession,
onClearSession) and change ChatPanel to accept ChatUiState (or the new props
object) instead of individual fields; update all call sites to pass the new
props/ChatUiState and adjust parameter destructuring inside ChatWorkspace and
ChatPanel accordingly (this same change should be applied to the other
overloaded/composable signatures referenced for ChatPanel).
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt`:
- Around line 116-152: The when on onboardingState.status in ConfigPanel.kt
currently ends with else -> Unit which silently ignores
MobileOnboardingStatus.RUNTIME_PATH_CONFIRMED and
MobileOnboardingStatus.TRANSPORT_CONNECTING (and future statuses); replace the
non-exhaustive else with explicit branches for the transient statuses (e.g.
MobileOnboardingStatus.RUNTIME_PATH_CONFIRMED and
MobileOnboardingStatus.TRANSPORT_CONNECTING -> Unit) or remove else and make the
when exhaustive so the compiler forces handling of new enum values;
alternatively, if skipping is intentional, add a clear comment above the when
indicating these states are intentionally no-ops referencing
onboardingState.status and MobileOnboardingStatus.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/MobileOnboardingModels.kt`:
- Around line 26-33: The current MobileOnboardingState data class may become
error-prone as status-specific fields diverge; refactor it into a sealed class
hierarchy (sealed class MobileOnboardingState) with distinct subclasses (e.g.,
object/ data class variants named for statuses) so each status
(MobileOnboardingStatus) can carry only the relevant properties (trustMode,
transportMode, recoveryKind, canRetry, canResume) and enable exhaustive when
checks; update any consumers that pattern-match MobileOnboardingState to use
when branches over the new subclasses and remove defaulting logic tied to the
single data class.
In `@clients/web/apps/chat/src/App.spec.ts`:
- Around line 97-111: The test currently uses optional chaining when locating
localized action buttons which masks missing elements; update both button
interactions that use wrapper.findAll().find(...)? .trigger() to first resolve
the button node (e.g., assign the result of wrapper.findAll(...).find(btn =>
btn.text() === translatedText("auth.pair")) to a variable) and assert it exists
(e.g., expect(node).toBeTruthy() or expect(node.exists()).then...) before
calling node.trigger("click"), and apply the same pattern for the button located
with translatedText("chat.startSession"), so a missing localized button fails
the test immediately instead of becoming a silent no-op.
In `@clients/web/apps/chat/src/App.vue`:
- Line 251: The current binding :error-message="gateway.errorMessage.value ||
chat.errorMessage.value" hides chat errors when a gateway error exists; change
the binding to surface both messages (or prioritize chat) by creating a
computed/getter (e.g., errorMessageComputed) that reads gateway.errorMessage and
chat.errorMessage and returns either a combined string (e.g., "Gateway: ...;
Chat: ...") or prefers chat when both exist, then bind
:error-message="errorMessageComputed" (referencing gateway.errorMessage,
chat.errorMessage, and the template binding) so users see both relevant errors.
In `@clients/web/apps/chat/src/composables/useChat.ts`:
- Around line 240-241: The hardcoded 15s timeout created via new
AbortController() and setTimeout(() => controller.abort(), 15000) in useChat
should be made configurable: add a timeout parameter or option to the useChat
composable (or a constant exported by the module) and replace the literal 15000
with that value, defaulting to 15000 if not provided; ensure callers can pass a
custom timeout and update any documentation/comments for useChat to state the
default and rationale.
- Around line 269-271: The server-provided session_id is currently
unconditionally applied by setSessionReady in useChat.ts which can allow a
remote overwrite of an existing client session; change the logic so you only
accept and call setSessionReady(data.session_id.trim()) when there is no active
session (e.g., sessionId is falsy or a dedicated isNewSession/isSessionReady
flag is false), keep the existing validation (typeof === "string" && trimmed
truthy), and avoid applying the server session_id when a local session already
exists to prevent hijacking.
In `@clients/web/apps/chat/src/composables/useGateway.ts`:
- Around line 327-335: Clarify the intent of the conditional in
handleTransportFailure by replacing the double-negative
`!!bearerToken.value.trim()` with a clearer boolean check (e.g.,
Boolean(bearerToken.value.trim()) or bearerToken.value.trim().length > 0) and
add a short inline comment next to the condition explaining that either
progress.trustEstablished or a non-empty bearer token indicates prior pairing;
update the if statement that references progress.trustEstablished and
bearerToken.value.trim() and keep calls to setBlockedOnboardingState unchanged.
In `@clients/web/apps/chat/src/onboardingContract.spec.ts`:
- Around line 126-141: The test "keeps web and mobile recovery labels comparable
through normalized product taxonomy" duplicates earlier assertions against
mobileChatWorkspace; remove the repeated
expect(mobileChatWorkspace).toContain(...) calls from this test (or extract them
into a shared helper used by both specs) so only unique assertions remain—keep
the webChatOnboardingRecoveryLabel checks and either reference the shared helper
or delete the duplicate MobileRecoveryKind checks to avoid redundancy.
In `@clients/web/apps/chat/src/raw-imports.d.ts`:
- Around line 1-4: The current module declaration declare module "*?raw" in
raw-imports.d.ts is too broad and may allow bundling sensitive files; narrow the
pattern by replacing the wildcard module declaration with specific extensions
needed (e.g., declare module "*.md?raw" and/or "*.txt?raw" or other exact
extensions your app requires), add a concise warning comment above the
declarations reminding contributors not to import secrets via ?raw, and ensure
your CI secret-scanning step is enabled to catch accidental imports; update the
module name(s) referenced in this file (the existing "*?raw" declaration)
accordingly.
In `@clients/web/apps/dashboard/CLAUDE.md.bak.1774203640`:
- Around line 1-34: This backup file (CLAUDE.md.bak.1774203640) was accidentally
committed despite the existing *.bak ignore rule; remove it from the commit by
untracking it from Git and committing that change (either remove it from the
index and keep a local copy or delete it and commit), amend the offending commit
or add a new commit so the file is no longer in the repository history at HEAD,
and verify the existing .gitignore contains the *.bak pattern so future *.bak
files are not staged.
In `@clients/web/apps/dashboard/src/App.spec.ts`:
- Around line 129-141: The onboardingState mock uses a type assertion that
bypasses shape checking; replace the trailing "as DashboardOnboardingState" on
the onboardingState constant with "satisfies DashboardOnboardingState" so the
object is validated against the DashboardOnboardingState contract while
preserving inference (mirroring the onboardingSteps approach). Ensure the change
is made on the onboardingState declaration in App.spec.ts and that the project
TypeScript version supports the satisfies operator.
In `@clients/web/apps/dashboard/src/composables/useConfig.ts`:
- Around line 177-187: The DashboardOnboardingState type incorrectly hard-codes
persistsPairingCode: false while retryable trust-failure flows actually preserve
pairingCode; update the type so persistsPairingCode is a boolean (not a literal
false) to reflect real persistence state, and make the same change for the other
occurrence of this type/shape later in the file (the block around where
persistsPairingCode is defined at lines ~219-240) so consumers reading
DashboardOnboardingState (and the duplicate definition) get the correct signal.
- Around line 567-570: The code is advancing operator state to "ready" even when
no bearer token exists; change the flow so that when bearerToken.value is empty
you do not set progress.trustEstablished or mark operatorReady/emit
"auth.connected". Specifically, in the block around markTransportConnecting()
and the similar block that calls markOperatorReady(), gate the calls to set
progress.trustEstablished and any markOperatorReady()/operatorReady transitions
behind a check that bearerToken.value.trim() is non-empty (and preserve
read-only/runtime-confirmed state when it is empty, leaving canSave false);
ensure markTransportConnecting() can still be called for unauthenticated reads
but avoid flipping trustEstablished/operatorReady until markOperatorReady() is
invoked after successful auth.
In `@modules/agent-core-kmp/CLAUDE.md.bak.1774203640`:
- Around line 1-66: The committed backup file CLAUDE.md.bak.1774203640 should be
removed and untracked: delete this .bak snapshot from the repository and commit
the removal (or run git rm --cached to stop tracking while keeping a local
copy), then add an appropriate ignore rule (e.g., *.bak or CLAUDE.md.bak*) to
your .gitignore so future editor backups aren’t reintroduced; ensure only the
canonical CLAUDE.md (the real contract) remains in the module to avoid drift.
In
`@modules/agent-core-kmp/src/commonMain/kotlin/com/profiletailors/agent/core/CoreContracts.kt`:
- Around line 103-106: The public API toOnboardingState currently accepts a
SurfaceId but only supports SurfaceId.COMPOSEAPP_MOBILE and throws for others;
change the signature to remove the surfaceId parameter (or make it a
private/internal constant) so the surface is fixed to COMPOSEAPP_MOBILE, update
the implementation inside BridgeLinkSnapshot to reference
SurfaceId.COMPOSEAPP_MOBILE directly, and then update all call sites and tests
to call toOnboardingState() with no arguments (or rely on the internal constant)
to prevent runtime failures from unsupported surfaces.
In
`@openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/design.md`:
- Around line 362-377: The file changes table is outdated because several
"Future Modify" entries were actually modified in this PR; update the table rows
for
`clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/onboarding/OnboardingScreen.kt`,
`clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/App.kt` (or
`ChatWorkspace.kt` if App.kt is the changed file), and
`clients/web/apps/chat/src/composables/useGateway.ts` from "Future Modify" to
"Modify" (or "Create/Modify" if new), and adjust the description column to
reflect that these items are implemented in this PR so the table accurately
represents the PR scope and artifacts referenced by the spec.
In
`@openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/proposal.md`:
- Line 90: The affected areas table contains a self-reference that doesn't match
the archive location; update the table entry that currently points to the
proposal file so it uses the correct archived location (the archived proposal.md
referenced in this document) or change it to a canonical/relative reference that
will remain correct after archiving; ensure the single table row in the
"affected areas" section referencing the proposal.md is updated consistently
wherever it appears in the document.
In
`@openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/specs/client-surfaces/spec.md`:
- Around line 12-23: Add a blank line immediately after each scenario heading to
satisfy MD022: insert an empty line after the lines starting with "####
Scenario: Web dashboard aligns onboarding without changing transport" and after
"#### Scenario: Mobile aligns onboarding without adopting HTTP pairing language"
so each heading is surrounded by a blank line before the following bullet list.
In
`@openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/specs/dashboard/spec.md`:
- Line 11: Add a blank line immediately above each "#### Scenario:" heading to
satisfy markdownlint MD022; specifically, insert a single empty line before the
"#### Scenario: Dashboard activation remains an operator slice of shared
onboarding" heading and do the same for the other occurrences of "####
Scenario:" found later in the same file (the ones referenced in the review),
ensuring there is exactly one blank line separating the previous paragraph or
content and each scenario heading.
In
`@openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/specs/onboarding/spec.md`:
- Line 27: Add a blank line before every "#### Scenario:" heading in this spec
to satisfy MD022; locate each occurrence of the "#### Scenario:" token (e.g.,
"#### Scenario: New operator follows the canonical sequence from the CLI") and
insert a single empty line immediately above it for all affected headings (the
other occurrences matching "#### Scenario:" in the document). Ensure there is
exactly one blank line above each scenario heading and no extra changes to
surrounding text or heading content.
In
`@openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/tasks.md`:
- Around line 40-44: Phase 6 lacks an explicit rollback/risk task; add a new
checklist item under "Phase 6: Cross-surface observability, documentation, and
verification" that requires documented threat/risk notes and a rollback strategy
covering runtime, gateway, and auth changes and an operational handoff
verification step; reference the existing checklist entries (6.1, 6.2, 6.3) and
require linking to specific artifacts (e.g.,
`clients/agent-runtime/src/onboard/wizard.rs`,
`clients/web/apps/chat/src/composables/useGateway.ts`, and
`modules/agent-core-kmp/src/jvmTest/.../RustCliBridgeTest.kt`) so reviewers must
confirm threat notes and rollback steps are included before marking Phase 6
done.
In
`@openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/verify-report.md`:
- Line 1: The report title uses a level-2 heading ("## Verification Report")
which triggers MD041 and is inconsistent with other spec archives; change the
heading to a level-1 heading ("# Verification Report") at the top of
verify-report.md so the file starts with an H1 and matches the rest of the spec
docs.
- Around line 11-17: The summary totals in the verification table are wrong:
update the Metrics table in verify-report.md so the "Tasks total" and "Tasks
complete" reflect the actual count of checked items (25) from the archived tasks
file (openspec/changes/2026-03-23-unify-onboarding-pairing-flow/tasks.md) and
ensure "Tasks incomplete" shows 0, making the table consistent with the archived
tasks list.
In `@openspec/specs/client-surfaces/spec.md`:
- Around line 133-175: Add a blank line above each "#### Scenario:" heading to
satisfy markdownlint MD022: specifically insert an empty line before "####
Scenario: Onboarding validates readiness through the approved transport", before
"#### Scenario: Web dashboard aligns onboarding without changing transport",
before "#### Scenario: Mobile aligns onboarding without adopting HTTP pairing
language", before "#### Scenario: Web and mobile expose comparable recovery
states", and before "#### Scenario: Operator surfaces expose operator-relevant
recovery states" so each scenario heading is separated from the preceding
paragraph.
---
Outside diff comments:
In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/App.kt`:
- Around line 24-88: The App function is over the Detekt LongMethod limit
because the ChatWorkspace call embeds multiple inline callback lambdas; extract
those handlers into a separate helper or composable (e.g., create functions like
handleRetryBridge(bridgeSnapshot, setBridgeSnapshot), handleLinkSurface(...),
handleStartSession(...), handleClearSession(...)) and call them from the
ChatWorkspace parameters (onRetryBridge = { handleRetryBridge(...) }, etc.);
ensure the helpers update the same mutable state (bridgeSnapshot) or accept a
setter/MutableState reference and preserve existing conditions
(environmentSupported checks, copy mutations, generateSessionId usage, and
toOnboardingState/status logic) so App remains concise while behavior is
unchanged.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt`:
- Around line 232-303: ChatInputField is too large and triggers Detekt's
LongParameterList and LongMethod rules; split responsibilities by extracting the
send-button UI and the styled text-field into smaller composables or a compact
input state object. Create a new composable (e.g., SendButton or ChatSendButton)
that takes onSend: () -> Unit and isEnabled: Boolean and moves the
Box/IconButton/Icon styling there, and extract the OutlinedTextField and its
styling into a StyledChatTextField (or accept a ChatInputState data class with
value/onValueChange/placeholder/enabled) so ChatInputField only composes those
helpers; update callers to use the new helper composables or pass the compact
state. Ensure you keep existing symbols: ChatInputField, OutlinedTextField
usage, and the gradient / corvusColors references when moving styling so
behavior remains unchanged.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt`:
- Around line 52-185: ConfigSettingsList is too long (detekt LongMethod); split
it into smaller composables to reduce length. Extract logical sections into
dedicated `@Composable` functions (for example: ConfigHeader (renders the "Bridge
Linking" title and headline using bridgeStateHeadline), StatusBlock (wraps
StatusIndicator), CurrentStateCard (calls diagnosticsCard with
buildDiagnosticsLines), RecoveryCard (calls diagnosticsCard with
bridgeStateDescription/bridgeStateRecovery), ActionRow (renders the
OutlinedButton + when(onboardingState.status) GradientButton cases using
actions.onRetryBridge/onLinkSurface/onStartSession/onClearSession), DividerLine
(the Box with Brush), and TrustBoundaryCard (the trust diagnosticsCard)).
Replace the inlined blocks inside ConfigSettingsList with calls to these new
composables so ConfigSettingsList becomes a short coordinator that passes
bridgeState, onboardingState, detailLines, corvusColors and actions as needed.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/onboarding/OnboardingScreen.kt`:
- Around line 113-122: The OnboardingScreen function violates Detekt
LongParameterList/LongMethod—refactor by introducing an immutable state holder
(e.g., OnboardingScreenState containing step, currentStepIndex, totalSteps,
isLastStep) and change the OnboardingScreen signature to accept that state plus
onSkip, onNext, and modifier; then extract large inner sections into smaller
composables (e.g., OnboardingHeader, OnboardingContent, OnboardingFooter or
similar) called from OnboardingScreen to reduce method length and improve
readability, and update all call sites to pass the new state object instead of
separate parameters.
- Around line 250-261: The emoji-only Text inside Box is not accessible; update
the OnboardingScreen UI to provide screen-reader friendly descriptions by
mapping each OnboardingIcon (OnboardingIcon.RUNTIME, .LINK, .SYNC, .SESSION) to
a short content string (e.g., "Runtime", "Link", "Sync", "Session") and apply it
via semantics on the composable showing the icon (either add Modifier.semantics
{ contentDescription = ... } to the Text or the Box, or replace Text with an
Icon + contentDescription). Ensure the mapping logic is in the same scope as the
current Text usage so the correct description is selected for the displayed
emoji.
In
`@clients/composeApp/src/iosMain/kotlin/com/profiletailors/corvus/Platform.ios.kt`:
- Around line 5-10: The class declaration is IOSPlatform but the file is
Platform.ios.kt which triggers Detekt's MatchingDeclarationName; either rename
the file to IOSPlatform.kt to match the top-level declaration, or keep the
filename and suppress the rule by adding a file-level suppression (e.g.,
`@file`:Suppress("MatchingDeclarationName")) at the top of the file; update the
Platform.ios.kt file containing class IOSPlatform accordingly and ensure the
suppression annotation is placed before any package or import statements if you
choose that route.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3f0ecc1b-35c2-44c7-a1b2-c24d39f2cbd1
📒 Files selected for processing (56)
.gitignoreclients/agent-runtime/src/cron/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/gateway/utils.rsclients/agent-runtime/src/onboard/wizard.rsclients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/MainActivity.ktclients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/Platform.android.ktclients/composeApp/src/commonMain/composeResources/values-es/strings.xmlclients/composeApp/src/commonMain/composeResources/values/strings.xmlclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/App.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/Platform.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/MobileOnboardingModels.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/onboarding/OnboardingScreen.ktclients/composeApp/src/commonTest/kotlin/com/profiletailors/corvus/OnboardingDefaultsTest.ktclients/composeApp/src/iosMain/kotlin/com/profiletailors/corvus/MainViewController.ktclients/composeApp/src/iosMain/kotlin/com/profiletailors/corvus/Platform.ios.ktclients/composeApp/src/jvmMain/kotlin/com/profiletailors/corvus/Platform.jvm.ktclients/web/apps/chat/src/App.spec.tsclients/web/apps/chat/src/App.vueclients/web/apps/chat/src/components/ConfigPanel.vueclients/web/apps/chat/src/composables/useChat.spec.tsclients/web/apps/chat/src/composables/useChat.tsclients/web/apps/chat/src/composables/useGateway.spec.tsclients/web/apps/chat/src/composables/useGateway.tsclients/web/apps/chat/src/onboardingContract.spec.tsclients/web/apps/chat/src/raw-imports.d.tsclients/web/apps/dashboard/CLAUDE.md.bak.1774203640clients/web/apps/dashboard/src/App.spec.tsclients/web/apps/dashboard/src/App.vueclients/web/apps/dashboard/src/composables/useConfig.spec.tsclients/web/apps/dashboard/src/composables/useConfig.tsclients/web/build.gradle.ktsclients/web/packages/locales/src/en.jsonclients/web/packages/locales/src/es.jsonmodules/agent-core-kmp/CLAUDE.md.bak.1774203640modules/agent-core-kmp/src/commonMain/kotlin/com/profiletailors/agent/core/CoreContracts.ktmodules/agent-core-kmp/src/commonTest/kotlin/com/profiletailors/agent/core/CoreContractsTest.ktmodules/agent-core-kmp/src/jvmMain/kotlin/com/profiletailors/agent/core/RustCliBridge.ktmodules/agent-core-kmp/src/jvmTest/kotlin/com/profiletailors/agent/core/RustCliBridgeTest.ktopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/archive-report.mdopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/design.mdopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/exploration.mdopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/proposal.mdopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/specs/client-surfaces/spec.mdopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/specs/dashboard/spec.mdopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/specs/onboarding/spec.mdopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/tasks.mdopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/verify-report.mdopenspec/specs/client-surfaces/gateway-api.mdopenspec/specs/client-surfaces/migrations.mdopenspec/specs/client-surfaces/spec.mdopenspec/specs/dashboard/spec.mdopenspec/specs/onboarding/spec.md
…b, and specs - Gate test-only enums/fns with #[cfg(test)] in gateway mod.rs to fix dead-code warnings - Remove accidentally tracked .bak files from git index - Fix duplicate recovery text in ChatComponents.kt for BLOCKED state - Assert button existence in App.spec.ts before triggering clicks - Guard session_id overwrite in useChat.ts to prevent remote session hijacking - Add Detekt suppressions for KMP expect/actual and @Preview composable patterns - Make ConfigPanel.kt when-expression exhaustive over MobileOnboardingStatus - Narrow raw-imports.d.ts wildcard to specific safe extensions - Surface both gateway and chat errors in App.vue via computed property - Use satisfies instead of as for type-safe mock in dashboard App.spec.ts - Clarify double-negative in useGateway.ts handleTransportFailure - Fix verify-report.md heading level and task count (23 -> 25)
Deploying corvus with
|
| Latest commit: |
eded2a2
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7cc54649.corvus-42x.pages.dev |
| Branch Preview URL: | https://feature-273-unify-onboarding.corvus-42x.pages.dev |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
♻️ Duplicate comments (1)
clients/web/apps/chat/src/App.spec.ts (1)
165-175:⚠️ Potential issue | 🟡 MinorFail fast if the localized buttons are missing.
These
?.trigger()calls still turn a missingauth.connectorchat.startSessionbutton into a silent no-op, so the real regression gets masked.Suggested fix
- await wrapper - .findAll("button") - .find((button) => button.text() === translatedText("auth.connect")) - ?.trigger("click"); + const connectButton = wrapper + .findAll("button") + .find((button) => button.text() === translatedText("auth.connect")); + expect(connectButton?.exists()).toBe(true); + await connectButton!.trigger("click"); @@ - await wrapper - .findAll("button") - .find((button) => button.text() === translatedText("chat.startSession")) - ?.trigger("click"); + const startSessionButton = wrapper + .findAll("button") + .find((button) => button.text() === translatedText("chat.startSession")); + expect(startSessionButton?.exists()).toBe(true); + await startSessionButton!.trigger("click");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/chat/src/App.spec.ts` around lines 165 - 175, The test currently masks missing localized buttons by using optional chaining on the found button before calling trigger; instead, explicitly assert the button exists and fail fast: locate the "auth.connect" and "chat.startSession" buttons by using the same findAll(...).find(...) logic (or use a find/get that throws), store each result in a variable, throw or call an expect to ensure the variable is defined if not found, then call trigger("click") on the guaranteed element; reference the translatedText("auth.connect") and translatedText("chat.startSession") lookups and the wrapper.get('[data-testid="toggle-config"]') interaction to find the correct spots to add the existence checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt`:
- Around line 472-480: The onboarding label mapper in ChatComponents (internal
fun onboardingStateLabel) duplicates logic already provided by
MobileBridgeUiState/mobileOnboardingStateLabel(...); replace the duplicated
implementation by delegating to the shared mapper instead of re-encoding labels:
remove or deprecate the local onboardingStateLabel function and call
mobileOnboardingStateLabel(onboardingState) (or reference
MobileBridgeUiState.onboardingStateLabel) wherever onboardingStateLabel was used
so labels remain canonical and avoid drift when statuses change.
- Around line 527-528: The reply currently embeds the raw
bridgeState.snapshot.sessionId into assistant output via sessionLabel and the
returned string, which may leak sensitive IDs; change the logic that builds
sessionLabel to use isNullOrBlank() on bridgeState.snapshot.sessionId and
substitute a non-sensitive fallback label (e.g., "pending-session" or a fixed
redacted token) instead of the raw value, and apply the same isNullOrBlank()
guard in ConfigPanel.buildDiagnosticsLines(); update the usage sites that
reference sessionLabel (the return string using modelName and prompt) so they
never include the full sessionId.
- Around line 388-390: BridgeStatusCard currently accepts an unused
ChatWorkspaceActions parameter (actions) which should be removed: update the
BridgeStatusCard signature to only take bridgeState: MobileBridgeUiState, remove
the actions parameter from the function declaration and any internal references,
and update all call sites that pass a ChatWorkspaceActions instance to stop
passing it; also remove any now-unused imports or references to
ChatWorkspaceActions related solely to this component so Detekt no longer flags
the dead dependency.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt`:
- Around line 80-84: The status light is driven from a possibly stale bridge
snapshot; change StatusIndicator(active=...) to derive its active value from the
canonical onboarding state instead of just bridgeState.snapshot flags: use
onboardingState.status (or compute active = onboardingState.status.isReady) or
explicitly include the environment/runtime gates (environmentSupported and
runtimeAvailable) together with linkEstablished/sessionCapable so the indicator
reflects the true MobileBridgeSnapshot.toOnboardingState result; update the call
site using StatusIndicator, referencing StatusIndicator, onboardingState,
onboardingStateLabel, bridgeState.snapshot, and
MobileBridgeSnapshot.toOnboardingState.
In `@clients/web/apps/chat/src/App.vue`:
- Around line 86-95: beginSession currently always calls
resetMessagesForSession(), which wipes the local transcript even when resuming;
change it to only reset messages when starting a new session (i.e., not when
preferResume is true or when the chat API indicates a resume). Update
beginSession(preferResume: boolean) to skip resetMessagesForSession() if
preferResume is true (or better, if chat.startSession returns a flag indicating
a resume, use that to decide), and apply the same guard where the same reset is
called at the other occurrence noted (lines ~105-107) so resuming preserves the
transcript.
In `@clients/web/apps/chat/src/composables/useChat.ts`:
- Around line 263-266: The failure paths for non-auth transport errors currently
call gateway.markPairedButNotConnected(), which is clearing session_ready and
prevents App.vue from showing the blocked onboarding state; update the error
handling so non-auth 5xx/timeout/network failures do not unset
session_ready—either call a different gateway helper that only marks "paired but
not connected" without touching session_ready or change
markPairedButNotConnected() to avoid clearing session_ready for these cases;
apply the same change to both spots referenced (the response.ok false branch and
the 276–285 error path) and keep chat.isSessionReady/session_ready behavior
unchanged for non-auth transport failures.
- Line 63: canResumeSession currently reads window.sessionStorage via
readStoredSessionId() and is non-reactive to in-memory changes; change its
definition to depend on the reactive currentSessionId so it updates whenever
persistSessionId()/clearStoredSessionId() or setSessionReady()/clearSession()
modify session state. Replace computed(() => !!readStoredSessionId()) with
computed(() => !!currentSessionId.value) (or equivalent reactive access) and
ensure places using canResumeSession (e.g., setSessionPending, startSession)
will now observe the updated value.
In `@clients/web/apps/chat/src/composables/useGateway.ts`:
- Around line 305-308: markTransportConnecting() incorrectly forces
progress.trustEstablished = true and passes trust=true to createOnboardingState,
causing the trust step to show complete even when unpaired; remove or set
progress.trustEstablished to false and pass the correct trust flag (false) to
createOnboardingState in markTransportConnecting(), and apply the same fix in
the analogous block around lines where the other connect handler sets trust (the
one at ~391-395) so trust is only marked true when a real trust/paired
confirmation occurs.
- Around line 10-24: The isUrlSafeForSecrets function must explicitly reject
scheme-relative URLs and use the page origin as the base for path-relative URLs:
add an early check that returns false if rawUrl.startsWith("//") to block
scheme-relative inputs, and when creating parsed for path-relative inputs (the
branch that currently checks rawUrl.startsWith("/")), call new URL(rawUrl,
window.location.origin) instead of using window.location.href so path-relative
URLs resolve against the origin and cannot accidentally become cross-origin;
keep the existing checks on parsed.username, parsed.password, parsed.search,
parsed.hash, and the protocol/hostname trust logic unchanged.
In `@clients/web/apps/chat/src/raw-imports.d.ts`:
- Around line 2-10: The type declaration file only declares "*.md?raw" and
"*.txt?raw", but imports like ChatWorkspace.kt?raw in onboardingContract.spec.ts
rely on a "*.kt?raw" module; add a matching declaration for "*.kt?raw" in
raw-imports.d.ts so the TypeScript compiler recognizes Kotlin raw imports (i.e.,
create a new declare module "*.kt?raw" { const content: string; export default
content; } alongside the existing declarations) to restore the import contract.
In `@clients/web/apps/dashboard/src/App.spec.ts`:
- Around line 268-293: Extract the duplicated onboardingSteps array in
App.spec.ts into a shared fixture/exported helper (e.g., const
defaultOnboardingSteps) and import/use it in the tests instead of inline
literals; update the two places where the literal appears (the onboardingSteps
arrays around the current diff and the duplicate at lines ~347-372) to reference
the shared fixture, and if tests need slight variations create small per-test
clones or a function (e.g., makeOnboardingSteps(overrides)) to apply overrides
without mutating the shared default.
In
`@openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/verify-report.md`:
- Line 9: The "### Completeness" section header is using H3 and violates MD001;
change the header token from "### Completeness" to an H2 "## Completeness" so
the top-level sections after the title use H2; locate the header text "###
Completeness" in the document and replace it with "## Completeness".
- Line 153: The "### Issues Found" heading is one level too deep; change the
Markdown heading to H2 by replacing "### Issues Found" with "## Issues Found" in
the document (look for the exact string "### Issues Found") so the heading level
matches the section hierarchy and renders correctly.
- Line 92: The "Spec Compliance Matrix" heading is currently the wrong level;
change the markdown heading for "Spec Compliance Matrix" to an H2 (i.e., use "##
Spec Compliance Matrix") to match the surrounding section hierarchy and continue
the existing pattern for section headings; locate the heading text "Spec
Compliance Matrix" in the verify-report.md content and update its markdown
prefix to H2 so it aligns with other top-level sections.
- Line 21: Change the "### Build & Tests Execution" heading to H2 by replacing
the triple-hash with double-hash ("## Build & Tests Execution") to match
surrounding H2 headings; also scan the same document for any other "### Build &
Tests Execution" or similar level-3 headings that should be H2 and update them
to use "##" to keep the document structure consistent.
- Line 141: Change the "Coherence (Design)" heading from H3 to H2 by updating
the heading marker for the line containing "Coherence (Design)" in
verify-report.md (replace the leading "###" with "##"); ensure the updated
heading remains correctly formatted and that any surrounding TOC or heading
hierarchy remains consistent.
- Line 126: Change the heading "Correctness (Static - Structural Evidence)" from
level H3 to H2 by replacing the leading "###" with "##" in the markdown (look
for the exact heading text "Correctness (Static - Structural Evidence)" in
verify-report.md) so it becomes a second-level heading.
- Line 168: Change the "### Verdict" heading to an H2 by replacing the "###
Verdict" line with "## Verdict" so the section uses the correct heading level;
locate the literal heading "### Verdict" in verify-report.md and update it to
"## Verdict".
---
Duplicate comments:
In `@clients/web/apps/chat/src/App.spec.ts`:
- Around line 165-175: The test currently masks missing localized buttons by
using optional chaining on the found button before calling trigger; instead,
explicitly assert the button exists and fail fast: locate the "auth.connect" and
"chat.startSession" buttons by using the same findAll(...).find(...) logic (or
use a find/get that throws), store each result in a variable, throw or call an
expect to ensure the variable is defined if not found, then call
trigger("click") on the guaranteed element; reference the
translatedText("auth.connect") and translatedText("chat.startSession") lookups
and the wrapper.get('[data-testid="toggle-config"]') interaction to find the
correct spots to add the existence checks.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: f7821c48-6f4a-41c9-9cc7-d828c5aed360
📒 Files selected for processing (14)
.gitignoreclients/agent-runtime/src/gateway/mod.rsclients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/MainActivity.ktclients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/Platform.android.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.ktclients/composeApp/src/iosMain/kotlin/com/profiletailors/corvus/Platform.ios.ktclients/web/apps/chat/src/App.spec.tsclients/web/apps/chat/src/App.vueclients/web/apps/chat/src/composables/useChat.tsclients/web/apps/chat/src/composables/useGateway.tsclients/web/apps/chat/src/raw-imports.d.tsclients/web/apps/dashboard/src/App.spec.tsopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/verify-report.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (9)
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/web/apps/chat/src/raw-imports.d.tsclients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/MainActivity.ktclients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/Platform.android.ktclients/web/apps/dashboard/src/App.spec.tsopenspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/verify-report.mdclients/web/apps/chat/src/composables/useChat.tsclients/composeApp/src/iosMain/kotlin/com/profiletailors/corvus/Platform.ios.ktclients/web/apps/chat/src/App.vueclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.ktclients/web/apps/chat/src/App.spec.tsclients/agent-runtime/src/gateway/mod.rsclients/web/apps/chat/src/composables/useGateway.tsclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt
**/*.kt
⚙️ CodeRabbit configuration file
**/*.kt: Enforce null safety (no !!), structured concurrency, and non-blocking suspend code.
Prefer idiomatic Kotlin (expression bodies, sealed types, value classes when justified).
Verify tests follow TDD intent and use backtick test names where applicable.
Files:
clients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/MainActivity.ktclients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/Platform.android.ktclients/composeApp/src/iosMain/kotlin/com/profiletailors/corvus/Platform.ios.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/verify-report.md
**/*.vue
⚙️ CodeRabbit configuration file
**/*.vue: Enforce Vue 3 Composition API with <script setup>.
Ensure accessibility (A11y) and proper use of Tailwind CSS classes.
Check for proper prop validation and emitted events documentation.
Files:
clients/web/apps/chat/src/App.vue
clients/agent-runtime/src/{security,gateway,tools}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Treat
src/security/,src/gateway/,src/tools/as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Files:
clients/agent-runtime/src/gateway/mod.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/gateway/mod.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/gateway/mod.rs
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Files:
clients/agent-runtime/src/gateway/mod.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/src/gateway/mod.rs
🧠 Learnings (14)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools}/**/*.rs : Treat `src/security/`, `src/gateway/`, `src/tools/` as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Applied to files:
clients/web/apps/chat/src/raw-imports.d.tsclients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs : Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Applied to files:
clients/web/apps/chat/src/raw-imports.d.tsclients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Preserve release-size profile assumptions in `Cargo.toml` and avoid adding heavy dependencies unless clearly justified
Applied to files:
.gitignore
📚 Learning: 2026-02-17T07:28:38.934Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T07:28:38.934Z
Learning: Applies to .agents/AGENTS.md : Document agent configurations and capabilities in AGENTS.md
Applied to files:
.gitignore
📚 Learning: 2026-02-17T07:28:38.934Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T07:28:38.934Z
Learning: Applies to .agents/AGENTS.md : Maintain comprehensive agent metadata including name, description, purpose, and capabilities
Applied to files:
.gitignore
📚 Learning: 2026-02-17T07:28:38.934Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-17T07:28:38.934Z
Learning: Applies to .agents/AGENTS.md : Include version information and compatibility details for agents
Applied to files:
.gitignore
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Include threat/risk notes and rollback strategy for security, runtime, and gateway changes; add or update tests for boundary checks and failure modes
Applied to files:
openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/verify-report.mdclients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
Applied to files:
clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/tools/**/*.rs : Implement `Tool` trait in `src/tools/` with strict parameter schema, validate and sanitize all inputs, and return structured `ToolResult` without panics in runtime path
Applied to files:
clients/agent-runtime/src/gateway/mod.rs
🪛 GitHub Actions: Scan with Detekt
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt
[error] 53-53: detekt [LongMethod]: The function ConfigSettingsList is too long (121). Maximum length is 60.
[error] 29-29: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 53-53: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt
[error] 232-232: detekt [LongParameterList]: The function ChatInputField(...) has too many parameters (threshold 6).
[error] 232-232: detekt [LongMethod]: The function ChatInputField is too long (62). Maximum length is 60.
[error] 1-1: detekt [TooManyFunctions]: Too many functions in file (16 detected). Threshold is set to 11.
[error] 62-62: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 83-83: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 125-125: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 149-149: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 174-174: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 184-184: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 232-232: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 306-306: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 339-339: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 388-388: detekt [FunctionNaming]: Function name does not match pattern [a-z][a-zA-Z0-9]*.
[error] 390-390: detekt [UnusedParameter]: Function parameter actions is unused.
🪛 markdownlint-cli2 (0.21.0)
openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/verify-report.md
[warning] 9-9: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (14)
.gitignore (1)
167-168: Good ignore hardening for Rust coverage outputs.Adding
*.profrawis the right move to avoid committing transient profiling/coverage artifacts.clients/composeApp/src/iosMain/kotlin/com/profiletailors/corvus/Platform.ios.kt (2)
1-1: File-level naming suppression is appropriate for KMP platform files.
11-11:bridgeAvailabilitymapping is correct for iOS. Explicitly usingBridgeAvailability.COMPANION_REQUIREDmatches the shared platform contract and onboarding bridge defaults.clients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/Platform.android.kt (2)
1-1: Detekt suppression is correctly applied for the KMPPlatform.android.ktpattern.
10-10: Android bridge capability is correctly declared asLOCAL_BRIDGE. This keeps platform behavior explicit and aligned with the shared onboarding flow.clients/composeApp/src/androidMain/kotlin/com/profiletailors/corvus/MainActivity.kt (2)
15-18: Runtime initialization wiring is solid. Passing bothplatformOverrideandinitialBridgeSnapshotavoids platform/snapshot drift and keeps onboarding state deterministic.
22-27: Preview path is correctly aligned with runtime wiring. TheFunctionNamingsuppression is in place and preview now exercises the same platform/bridge initialization path.clients/web/apps/chat/src/raw-imports.d.ts (1)
1-5: Good security hardening on raw-import scope.Narrowing from a wildcard to explicit extensions plus the warning comment is a solid mitigation against accidental secret ingestion via
?raw.As per coding guidelines: "Security first, performance second. Validate input boundaries, auth/authz implications, and secret management."
clients/web/apps/dashboard/src/App.spec.ts (2)
129-141: Strong contract-safe fixture typing withsatisfies.Good change: using
satisfiesforonboardingStateandonboardingStepskeeps mock literals aligned with the production onboarding contracts without weakening inference.As per coding guidelines, "Look for behavioral regressions, missing tests, and contract breaks across modules."
Also applies to: 142-169
231-236: Good regression coverage for blocked and ready onboarding paths.These assertions tighten UI behavior around onboarding progression, blocked recovery messaging, and operator-ready completion state.
As per coding guidelines, "Look for behavioral regressions, missing tests, and contract breaks across modules."
Also applies to: 257-299, 332-378
openspec/changes/archive/2026-03-24-2026-03-23-unify-onboarding-pairing-flow/verify-report.md (2)
11-17: Metrics now accurately reflect 25/25 task completion.The completeness totals have been corrected and align with the archived tasks file.
158-161: Warnings appropriately acknowledge verification scope gaps.The report transparently documents that
make testdoesn't cover Rust/web suites and that verification dirties the working tree. This honesty strengthens the verification artifact.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ConfigPanel.kt (1)
133-147: No action needed—onClearSessioncorrectly resets the link.
onClearSessionresetslinkEstablished,sessionCapable, andsessionId(App.kt:82), not just the session ID. This triggers transition toTRUST_PENDINGpertoOnboardingState()logic. The "Relink" button initiates a true relink path.clients/agent-runtime/src/gateway/mod.rs (1)
646-723: Good isolation of the onboarding seam.Scoping these enums and mapping helpers to tests keeps the runtime module lean while preserving the contract coverage.
…eConfig composables - useGateway: 21 new tests covering connectGateway error paths, pairGateway edge cases, URL safety, auth headers, transport failure, and idempotency fallback - useChat: 12 new tests covering sendMessage error/timeout/auth paths, clearSession, startSession(false), baseUrl watch, and session_id overwrite guard - useConfig: 8 new tests covering connectGateway insecure URL, options/config 401/500 responses, fetch throw, quickPairState reset, and bearer token gating
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
clients/web/apps/dashboard/src/composables/useConfig.spec.ts (2)
538-552:⚠️ Potential issue | 🟡 MinorSame timeout fragility applies here.
Line 546 has the same
setTimeout(resolve, 10)pattern. Consider usingvi.waitFor()or similar for deterministic test behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/composables/useConfig.spec.ts` around lines 538 - 552, The test's non-deterministic wait (await new Promise(resolve => setTimeout(resolve, 10))) should be replaced with a deterministic wait helper; update the spec to use vi.waitFor (or the testing-library waitFor) to await the expected state changes from useConfig — e.g., after mocking fetch and instantiating useConfig, call await vi.waitFor(() => expect(config.quickPairState.value).toBe("failed")) and then assert errorMessage.value, pairingCode.value and bearerToken.value, which ensures the assertions only run once quickPairState, errorMessage, pairingCode and bearerToken have updated.
500-511:⚠️ Potential issue | 🟡 MinorFixed timeout waiting is fragile for async operations.
Using
setTimeout(resolve, 10)to wait for an innerasync voidis timing-dependent and may cause flaky tests under load or in slower CI environments.💡 Consider exposing a promise or using a more deterministic wait
If
useConfigcould expose the quick-pair promise (e.g.,quickPairPromise), the test could await it directly. Alternatively, poll on the expected state with a bounded retry:// Example polling approach await vi.waitFor(() => { expect(config.quickPairState.value).toBe("connected"); }, { timeout: 100 });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/composables/useConfig.spec.ts` around lines 500 - 511, The test uses a fragile setTimeout-based wait for an inner async operation in the useConfig composable (the quick-pair flow), so replace the timing-dependent wait with a deterministic await: either modify useConfig to expose the internal promise (e.g., quickPairPromise) and await that in the spec, or change the test to poll/wait for the observable state (config.quickPairState) using a test utility such as vi.waitFor or explicit bounded retry until config.quickPairState.value === "connected"; remove the setTimeout(resolve, 10) and assert after the deterministic await that fetchMock was called and the baseUrl/bearerToken assertions hold.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/web/apps/chat/src/composables/useChat.spec.ts`:
- Around line 237-250: The test uses vi.dynamicImportSettled() to wait for the
baseUrl watcher; replace that with Vue's nextTick to correctly await the watcher
(import { nextTick } from 'vue'), so in the test that manipulates
gateway.baseUrl (inside the "resets session when gateway baseUrl changes" spec)
set gateway.baseUrl.value = "https://other-host:9999" and then await nextTick()
before asserting chat.currentSessionId.value is empty; update the imports in
useChat.spec.ts accordingly and remove the vi.dynamicImportSettled() call.
In `@clients/web/apps/chat/src/composables/useGateway.spec.ts`:
- Around line 440-450: Add a test case to this spec that verifies
useGateway().isUrlSafeForSecrets accepts IPv6 loopback addresses; specifically,
in the same suite where isUrlSafeForSecrets is exercised (using useGateway((key:
string) => key)), add an expectation like
isUrlSafeForSecrets("https://[::1]/api") toBe(true) so the behavior matches the
dashboard spec and ensures IPv6 loopback is treated as safe.
- Around line 482-492: The test that stubs globalThis.crypto in the spec for
useGateway.createIdempotencyKey must guarantee restoration of the originalCrypto
even if assertions fail: capture originalCrypto, stub with
vi.stubGlobal("crypto", { randomUUID: undefined }) as done now, then wrap the
assertion and key generation in a try/finally and restore the original via
vi.stubGlobal("crypto", originalCrypto) in the finally block so global state is
always cleaned up; update the test named "createIdempotencyKey falls back to
timestamp-based key when crypto.randomUUID is unavailable" accordingly.
---
Outside diff comments:
In `@clients/web/apps/dashboard/src/composables/useConfig.spec.ts`:
- Around line 538-552: The test's non-deterministic wait (await new
Promise(resolve => setTimeout(resolve, 10))) should be replaced with a
deterministic wait helper; update the spec to use vi.waitFor (or the
testing-library waitFor) to await the expected state changes from useConfig —
e.g., after mocking fetch and instantiating useConfig, call await vi.waitFor(()
=> expect(config.quickPairState.value).toBe("failed")) and then assert
errorMessage.value, pairingCode.value and bearerToken.value, which ensures the
assertions only run once quickPairState, errorMessage, pairingCode and
bearerToken have updated.
- Around line 500-511: The test uses a fragile setTimeout-based wait for an
inner async operation in the useConfig composable (the quick-pair flow), so
replace the timing-dependent wait with a deterministic await: either modify
useConfig to expose the internal promise (e.g., quickPairPromise) and await that
in the spec, or change the test to poll/wait for the observable state
(config.quickPairState) using a test utility such as vi.waitFor or explicit
bounded retry until config.quickPairState.value === "connected"; remove the
setTimeout(resolve, 10) and assert after the deterministic await that fetchMock
was called and the baseUrl/bearerToken assertions hold.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c27abe44-1338-40d0-80b4-d8829b89be47
📒 Files selected for processing (3)
clients/web/apps/chat/src/composables/useChat.spec.tsclients/web/apps/chat/src/composables/useGateway.spec.tsclients/web/apps/dashboard/src/composables/useConfig.spec.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: pr-checks
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (1)
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/web/apps/chat/src/composables/useGateway.spec.tsclients/web/apps/chat/src/composables/useChat.spec.tsclients/web/apps/dashboard/src/composables/useConfig.spec.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
🔇 Additional comments (11)
clients/web/apps/chat/src/composables/useGateway.spec.ts (3)
1-50: Good coverage of the canonical HTTP onboarding flow.The test validates the complete health → pair → ready lifecycle with proper assertions on endpoint calls, token storage, and pairing code clearing. Security-relevant state transitions are well covered.
52-68: Solid URL safety boundary tests.Good coverage of the HTTPS non-localhost acceptance (remote deployments), HTTP non-localhost rejection, and HTTP localhost allowance. These directly validate the security policy for credential transmission.
216-238: Correct insecure URL gating with credential presence.Tests properly verify that
connectGatewayblocks network calls when credentials would be sent over insecure channels. ThefetchMocknot being called confirms the security gate fires before any network activity.clients/web/apps/chat/src/composables/useChat.spec.ts (3)
145-166: Good credential invalidation coverage for 401/403.Tests verify that both 401 and 403 responses trigger
markCredentialInvalid, clear the session, and throw the appropriate error. This is critical for security to ensure stale credentials don't persist.
252-271: Important session_id overwrite guard test.This test validates that the server cannot hijack the client's session by returning a different
session_id. Good defensive security test.
102-127: Verify Authorization header format and session header presence.Good test verifying bearer token and session ID are sent correctly. The assertions on
init?.headersproperly validate the request composition.clients/web/apps/dashboard/src/composables/useConfig.spec.ts (5)
125-143: Good coverage of stale bearer token clearing on 401.Test validates that 401 responses clear the bearer token and set the correct recovery state. This prevents credential persistence after server-side invalidation.
513-536: Proper validation of insecure gatewayUrl with embedded credentials.Tests correctly reject URLs with embedded credentials (
admin:pass@) and query parameters, preventing credential leakage through URL components.
749-760: Good insecure URL gating with bearer token.Verifies that credentials are never sent to non-local origins, with
fetchMocknot being called confirming the early return.
851-876: Verifies Authorization header suppression when no token present.Important test ensuring unauthenticated requests don't include an empty or malformed Authorization header.
592-608: IPv6 loopback acceptance is security-relevant.Good test for
[::1]loopback acceptance—ensures developers using IPv6 locally aren't blocked while remote hosts still require HTTPS.
…b, and specs - Remove unused actions param from BridgeStatusCard and its call site - Preserve transcript on session resume in App.vue beginSession - Make canResumeSession reactive via currentSessionId in useChat - Fix markTransportConnecting incorrectly forcing trust=true in useGateway - Reject scheme-relative URLs and use origin base in isUrlSafeForSecrets - Add *.kt?raw declaration to raw-imports.d.ts for contract spec imports - Assert button existence in App.spec.ts credential recovery test - Fix verify-report.md H3 headings to H2 for MD001 compliance
|
yuniel-acosta
approved these changes
Mar 24, 2026
yuniel-acosta
deleted the
feature/273-unify-onboarding-and-pairing-flow-across-cli-web-and-mobile-clients
branch
Merged
This was referenced Mar 28, 2026
Merged
This was referenced Apr 9, 2026
Merged
This was referenced Apr 10, 2026
Merged
Merged
This was referenced Apr 19, 2026
Closed
This was referenced Apr 28, 2026
Merged
8 tasks
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement canonical onboarding state machine with shared trust/transport/ready lifecycle across all client surfaces. Each surface maps platform-specific transport failures into normalized recovery states.
Review fixes applied:
Refs: #273