fix: remediate sonar quality issues#776
Conversation
|
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 applies targeted refactors and safety/validation improvements across CI (Rust toolchain resolution), agent runtime skill loading, mobile Kotlin chat wiring and socket parsing, Rook SSE streaming state, multiple web dashboard UI and storage fixes, scripts hardening, and minor infra tweaks. ChangesCI: Rust toolchain resolution
Agent runtime — skills loading refactor
Mobile app — chat wiring, runtime, components
Mobile iOS sockets (arm64 & simulator arm64)
Rook gateway — SSE streaming redesign
Web dashboard — accessibility, session, storage, style
Build & infra scripts hardening
Sequence Diagram(s)(No sequence diagrams generated.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Deploying corvus with
|
| Latest commit: |
b78e6ba
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2d268c3b.corvus-42x.pages.dev |
| Branch Preview URL: | https://maintenance.corvus-42x.pages.dev |
coderabbitai
Bot
added
area:rust
area:docs
risk:high
risk:security
area:ci
area:kotlin
area:web
labels
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt (1)
45-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDetekt blockers in
inetPtonstill fail CI.
inetPtoncurrently violatesReturnCountandMagicNumberrules, and this is already reported as a pipeline error. Please refactor this function to use named constants and a single return path so the PR can merge cleanly.Suggested refactor
+private const val IPV4_PARTS_COUNT = 4 +private const val IPV4_MAX_OCTET = 255 +private const val SHIFT_24 = 24 +private const val SHIFT_16 = 16 +private const val SHIFT_8 = 8 +private const val INVALID_IPV4 = 0u + `@OptIn`(ExperimentalForeignApi::class) private fun inetPton(host: String): UInt { val parts = host.split('.') - if (parts.size != 4) return 0u - - val bytes = parts.mapNotNull { it.toIntOrNull() } - if (bytes.size != 4 || bytes.any { it < 0 || it > 255 }) return 0u - - return ((bytes[0].toUInt() shl 24) or - (bytes[1].toUInt() shl 16) or - (bytes[2].toUInt() shl 8) or - bytes[3].toUInt()) + val bytes = parts.mapNotNull(String::toIntOrNull) + val valid = + parts.size == IPV4_PARTS_COUNT && + bytes.size == IPV4_PARTS_COUNT && + bytes.none { it < 0 || it > IPV4_MAX_OCTET } + return if (valid) { + ((bytes[0].toUInt() shl SHIFT_24) or + (bytes[1].toUInt() shl SHIFT_16) or + (bytes[2].toUInt() shl SHIFT_8) or + bytes[3].toUInt()) + } else { + INVALID_IPV4 + } }As per coding guidelines, "Security first, performance second. Validate input boundaries ... Look for behavioral regressions ..."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt` around lines 45 - 55, Refactor inetPton to eliminate multiple returns and magic numbers: introduce named constants (e.g., IPV4_PARTS = 4, BYTE_MIN = 0, BYTE_MAX = 255, INVALID_ADDR = 0u), parse parts into a List<Int?> using split and map { it.toIntOrNull() }, perform validations (size == IPV4_PARTS, no nulls, all values within BYTE_MIN..BYTE_MAX) and compute the result into a local UInt variable by shifting the validated bytes, then return that single result variable at the end; update references in SocketIosRuntimeCompanionClient.kt to use these constants and the single-return flow in inetPton.clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt (1)
45-55:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMirror file still has the same Detekt-blocking
inetPtonissues.This target has the same
ReturnCount+MagicNumberviolations, so CI will keep failing until the same refactor is applied here too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt` around lines 45 - 55, The inetPton function uses multiple early returns and magic numeric literals; refactor inetPton to use a single return and named constants to satisfy Detekt's ReturnCount and MagicNumber rules: define constants (e.g., BYTE_COUNT = 4, MAX_BYTE = 255, SHIFT_24 = 24, SHIFT_16 = 16, SHIFT_8 = 8, ZERO = 0u), validate parts length and numeric range by setting an error flag or assigning ZERO to the result on failure, build the UInt result in a loop or stepwise using the SHIFT_* constants and named byte indices, and return the single result variable at the end; keep the function name inetPton and preserve the same behavior (return ZERO on invalid input).scripts/validate-affected-components.mjs (1)
8-15:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame
Error/TypeErrorsplit as inresolve-release-context.mjs.Line 11 throws
Error(withcause) for malformed JSON, while line 15 throwsTypeError(nocause) for a non-array. Both represent a type/shape violation; the JSON-parse branch should useTypeErrortoo for consistency with the PR's error-classification goal.🛠️ Proposed fix
} catch (error) { - throw new Error(`Invalid AFFECTED_COMPONENTS payload: ${affectedComponentsRaw}`, { cause: error }); + throw new TypeError(`Invalid AFFECTED_COMPONENTS payload: ${affectedComponentsRaw}`, { cause: error }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/validate-affected-components.mjs` around lines 8 - 15, The JSON-parse catch currently throws a generic Error while the non-array check throws a TypeError; change the catch to throw a TypeError (not Error) and include the original error as the cause so both shape/type violations use the same error class. Specifically, in the try/catch around JSON.parse(affectedComponentsRaw) replace throw new Error(...) with throw new TypeError(`Invalid AFFECTED_COMPONENTS payload: ${affectedComponentsRaw}`, { cause: error }); and leave the existing TypeError for the Array.isArray(parsed) check (optionally unify message text if needed).scripts/resolve-release-context.mjs (1)
18-23:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent error type and
causeattachment between the two failure branches.The JSON-parse failure on line 19 throws
Errorwith{ cause: error }, while the non-array guard on line 23 throwsTypeErrorwithout a cause. Both represent the same logical problem — "the payload is not a valid component array" — so callers thatinstanceof TypeErrorto detect type violations will miss the JSON-parse case.🛠️ Proposed fix
} catch (error) { - throw new Error(`Invalid AFFECTED_COMPONENTS payload: ${affectedComponentsRaw}`, { cause: error }); + throw new TypeError(`Invalid AFFECTED_COMPONENTS payload: ${affectedComponentsRaw}`, { cause: error }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/resolve-release-context.mjs` around lines 18 - 23, Make the two failure branches consistent by throwing the same error type and including the original parse error as the cause: in the catch block that currently does throw new Error(..., { cause: error }), change it to throw new TypeError(`Invalid AFFECTED_COMPONENTS payload: ${affectedComponentsRaw}`, { cause: error }); this aligns it with the non-array guard (which throws TypeError) and preserves the parse error via the cause; reference the affectedComponentsRaw variable, the parsed check (Array.isArray(parsed)), and the catch block surrounding the JSON.parse call to locate the change.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt (1)
1-1:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDetekt
TooManyFunctionsis blocking CI — extract the new session-item composables.The pipeline reports 20 functions in this file against a threshold of 11. This PR adds
sessionHistoryItemStyle,sessionHistoryItemLabel, andsessionHistoryItemAction, pushing the count further over the limit. Since these three (plusSessionHistoryItemStyle) are self-contained, the low-friction fix is to move them into a dedicated file (e.g.,SessionHistoryComponents.kt).Alternatively, a
@Suppress("TooManyFunctions")at the file level with a justification comment will unblock CI immediately if a file split isn't desirable right now.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt` at line 1, The file has exceeded Detekt's TooManyFunctions due to the newly added sessionHistoryItemStyle, sessionHistoryItemLabel, sessionHistoryItemAction and the SessionHistoryItemStyle type; fix by extracting those four self-contained symbols into a new file (e.g., create SessionHistoryComponents.kt and move the implementations and any related imports there) so ChatComponents.kt falls under the function limit, or if you prefer to unblock CI immediately, add a file-level `@Suppress`("TooManyFunctions") with a short justification comment above the package declaration in ChatComponents.kt.
🤖 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 @.github/workflows/core-check.yml:
- Around line 67-68: The workflow references
clients/agent-runtime/rust-toolchain.toml via the toolchain_file variable but
that file is not listed in the workflow's path filters, so updates won't trigger
core-checks; update .github/workflows/core-check.yml by adding
"clients/agent-runtime/rust-toolchain.toml" to the workflow's paths filters
(where the on: push: and/or on: pull_request: paths: arrays are defined) so
changes to that file will trigger the job that reads toolchain_file.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt`:
- Around line 510-517: The data class SessionHistoryItemStyle should be marked
`@Immutable` like ChatBubblePalette to make the immutability contract explicit;
add the `@Immutable` annotation above the SessionHistoryItemStyle declaration and
ensure the correct Immutable import (e.g., androidx.compose.runtime.Immutable or
your project's Immutable) is present so the class and its all-val, stable-typed
properties are recognized as immutable by the tooling.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 126-131: Annotate the ChatWorkspaceCallbacks data class with
`@Stable` (import androidx.compose.runtime.Stable) to make its stability explicit
like the other peer types (BridgeActions, ChatWorkspaceActions,
ChatWorkspaceContent, ChatUiState); this ensures Compose can treat it as stable
(and supports using it as a remember key) — add the `@Stable` annotation above the
ChatWorkspaceCallbacks declaration and add the necessary import.
In
`@clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt`:
- Line 173: The code assigns addr.sin_addr.s_addr using the custom inetPton and
proceeds to connect() without validating the result, which silently treats
invalid hosts as 0. Replace the custom inetPton usage with
platform.posix.inet_pton, call inet_pton(AF_INET, config.host,
addr.sin_addr.ptr), check that it returns 1 (fail fast and return null on result
!= 1), and only then call connect(sockfd, addr.ptr.reinterpret(),
sizeOf<sockaddr_in>().convert()); ensure you remove or stop using the custom
inetPton function and validate the inet_pton result before assigning/using
addr.sin_addr.s_addr or calling connect().
In
`@clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt`:
- Line 173: After calling inetPton(config.host) in
SocketIosRuntimeCompanionClient, validate the returned value before assigning to
addr.sin_addr.s_addr: if inetPton returns 0u (indicating malformed host) bail
out immediately (return an error, throw, or log and short-circuit) instead of
assigning 0.0.0.0 and relying on connect() to fail; alternatively update
inetPton() itself to return a nullable/throwing result and propagate that error
so callers (e.g., where addr.sin_addr.s_addr is set) fail fast with a clear
message referencing the invalid config.host.
In `@clients/rook/src/gateway/streaming.rs`:
- Around line 171-179: The completion callback (run_completion_once) is only
invoked when state.terminated is true, so EOF/error paths where
next_upstream_event/next_pending_or_upstream_event returns None skip completion;
update the logic in the function that awaits
next_pending_or_upstream_event/next_upstream_event to ensure
run_completion_once(state.on_complete.take()).await is called exactly once on
all termination paths (both state.terminated and when the upstream returns
None/Err) — locate the call sites around next_pending_or_upstream_event and
next_upstream_event and add a single shared post-termination branch or helper
that invokes run_completion_once before returning None (preserving the semantics
of with_completion and ensuring on_complete is taken once).
In `@clients/web/apps/dashboard/src/components/sessions/SessionFilters.vue`:
- Around line 18-19: Replace the untyped string Sets with Sets typed to the
corresponding union types so runtime validation matches the compile-time unions:
change VALID_STATUSES from Set<string> to Set<SessionStatusFilter> and
VALID_SORTS from Set<string> to Set<SessionSort>, updating any related imports
or type references for SessionStatusFilter and SessionSort to ensure the sets
are declared with those generics (so VALID_STATUSES and VALID_SORTS use
Set<SessionStatusFilter> and Set<SessionSort>, respectively).
In `@clients/web/apps/dashboard/src/types/admin-sessions.ts`:
- Line 17: The two session types disagree on metadata nullability:
AdminSessionDetail.metadata allows null while
AdminSessionDetailResponse.session.metadata does not; pick one contract and make
both match—either change AdminSessionDetailResponse.session.metadata to
"metadata?: unknown | null" to accept null, or remove null from
AdminSessionDetail.metadata so both are non-nullable; if you choose
non-nullable, add a normalization step where sessions are constructed (e.g., in
the mapper that builds AdminSessionDetail from response) to coerce null ->
undefined or an empty object to keep runtime behavior consistent.
In `@clients/web/apps/rook-dashboard/src/App.vue`:
- Around line 24-25: The code accesses globalThis.location and binds
globalThis.addEventListener/removeEventListener directly (e.g., when
initializing route via normalizeHashRoute and in the mounted/unmounted event
handlers), which breaks in non-browser runtimes; add a small getLocation()
helper used by the route initialization (replace direct globalThis.location
usage in route = ref<RookRoute>(normalizeHashRoute(...))) and anywhere else you
read location, and guard event listener binding/unbinding by checking typeof
globalThis.addEventListener === 'function' and typeof
globalThis.removeEventListener === 'function' before calling
addEventListener/removeEventListener; update the functions that reference
normalizeHashRoute, the mounted hook that registers the hashchange listener, and
the unmounted hook that removes it to use these guards so the component is safe
in SSR/worker/node environments.
In `@clients/web/apps/rook-dashboard/src/lib/session/useRookSession.ts`:
- Around line 29-30: Wrap all direct sessionStorage accesses in try/catch inside
useRookSession.ts: change the get call (currently returning
globalThis.window.sessionStorage.getItem(key) ?? "") to a safe helper that tries
to read sessionStorage and returns "" on any thrown error; similarly wrap writes
and deletes (the set and remove operations referenced in the file) so they catch
SecurityError/QuotaExceededError and no-op (or log) instead of throwing.
Implement a small private helpers like safeGetSessionItem(key),
safeSetSessionItem(key, value) and safeRemoveSessionItem(key) and use those in
the existing functions to avoid crashes when storage is unavailable.
In `@scripts/check-tools.sh`:
- Around line 47-52: The printf calls use the variable STATUS_FORMAT in the
format-string position (SC2059) which is fragile; update each call site that
currently does printf "$STATUS_FORMAT" ... to use the literal format string
directly instead of the STATUS_FORMAT variable (or, if you must keep a central
constant, explicitly document and suppress SC2059 with a shellcheck directive).
Target the printf invocations shown (the four occurrences using STATUS_FORMAT)
and replace them with the literal format string so no user-controlled %
sequences can be interpreted.
---
Outside diff comments:
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt`:
- Line 1: The file has exceeded Detekt's TooManyFunctions due to the newly added
sessionHistoryItemStyle, sessionHistoryItemLabel, sessionHistoryItemAction and
the SessionHistoryItemStyle type; fix by extracting those four self-contained
symbols into a new file (e.g., create SessionHistoryComponents.kt and move the
implementations and any related imports there) so ChatComponents.kt falls under
the function limit, or if you prefer to unblock CI immediately, add a file-level
`@Suppress`("TooManyFunctions") with a short justification comment above the
package declaration in ChatComponents.kt.
In
`@clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt`:
- Around line 45-55: Refactor inetPton to eliminate multiple returns and magic
numbers: introduce named constants (e.g., IPV4_PARTS = 4, BYTE_MIN = 0, BYTE_MAX
= 255, INVALID_ADDR = 0u), parse parts into a List<Int?> using split and map {
it.toIntOrNull() }, perform validations (size == IPV4_PARTS, no nulls, all
values within BYTE_MIN..BYTE_MAX) and compute the result into a local UInt
variable by shifting the validated bytes, then return that single result
variable at the end; update references in SocketIosRuntimeCompanionClient.kt to
use these constants and the single-return flow in inetPton.
In
`@clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt`:
- Around line 45-55: The inetPton function uses multiple early returns and magic
numeric literals; refactor inetPton to use a single return and named constants
to satisfy Detekt's ReturnCount and MagicNumber rules: define constants (e.g.,
BYTE_COUNT = 4, MAX_BYTE = 255, SHIFT_24 = 24, SHIFT_16 = 16, SHIFT_8 = 8, ZERO
= 0u), validate parts length and numeric range by setting an error flag or
assigning ZERO to the result on failure, build the UInt result in a loop or
stepwise using the SHIFT_* constants and named byte indices, and return the
single result variable at the end; keep the function name inetPton and preserve
the same behavior (return ZERO on invalid input).
In `@scripts/resolve-release-context.mjs`:
- Around line 18-23: Make the two failure branches consistent by throwing the
same error type and including the original parse error as the cause: in the
catch block that currently does throw new Error(..., { cause: error }), change
it to throw new TypeError(`Invalid AFFECTED_COMPONENTS payload:
${affectedComponentsRaw}`, { cause: error }); this aligns it with the non-array
guard (which throws TypeError) and preserves the parse error via the cause;
reference the affectedComponentsRaw variable, the parsed check
(Array.isArray(parsed)), and the catch block surrounding the JSON.parse call to
locate the change.
In `@scripts/validate-affected-components.mjs`:
- Around line 8-15: The JSON-parse catch currently throws a generic Error while
the non-array check throws a TypeError; change the catch to throw a TypeError
(not Error) and include the original error as the cause so both shape/type
violations use the same error class. Specifically, in the try/catch around
JSON.parse(affectedComponentsRaw) replace throw new Error(...) with throw new
TypeError(`Invalid AFFECTED_COMPONENTS payload: ${affectedComponentsRaw}`, {
cause: error }); and leave the existing TypeError for the Array.isArray(parsed)
check (optionally unify message text if needed).
🪄 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: f849eb74-cba9-4e34-897d-a1a695a46eda
📒 Files selected for processing (32)
.github/workflows/core-check.ymlclients/agent-runtime/src/security/policy.rsclients/agent-runtime/src/skills/mod.rsclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/App.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/runtime/MobileRuntimeCoordinator.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/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/rook/src/gateway/streaming.rsclients/web/apps/dashboard/src/components/chat/ChatMessage.vueclients/web/apps/dashboard/src/components/chat/ChatWorkspace.spec.tsclients/web/apps/dashboard/src/components/chat/ChatWorkspace.vueclients/web/apps/dashboard/src/components/config/WebhookSettings.vueclients/web/apps/dashboard/src/components/sessions/SessionFilters.vueclients/web/apps/dashboard/src/components/sessions/SessionList.vueclients/web/apps/dashboard/src/composables/useAdmin.tsclients/web/apps/dashboard/src/composables/useChat.tsclients/web/apps/dashboard/src/composables/useLocalMemoryExplorer.tsclients/web/apps/dashboard/src/types/admin-sessions.tsclients/web/apps/rook-dashboard/src/App.vueclients/web/apps/rook-dashboard/src/lib/api/client.tsclients/web/apps/rook-dashboard/src/lib/session/useRookSession.tsclients/web/apps/rook-dashboard/src/style.cssdev/sandbox/Dockerfilescripts/check-tools.shscripts/mobile-smoke-test.shscripts/release-components.mjsscripts/release-contract.test.mjsscripts/resolve-release-context.mjsscripts/sync-internal-release-deps.mjsscripts/validate-affected-components.mjs
📜 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: semgrep-cloud-platform/scan
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (8)
**/*
⚙️ 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:
dev/sandbox/Dockerfileclients/web/apps/dashboard/src/composables/useLocalMemoryExplorer.tsclients/web/apps/dashboard/src/components/chat/ChatMessage.vueclients/web/apps/rook-dashboard/src/lib/session/useRookSession.tsclients/web/apps/dashboard/src/types/admin-sessions.tsclients/web/apps/dashboard/src/components/config/WebhookSettings.vueclients/web/apps/rook-dashboard/src/App.vueclients/web/apps/dashboard/src/components/chat/ChatWorkspace.spec.tsclients/web/apps/dashboard/src/components/sessions/SessionFilters.vueclients/web/apps/rook-dashboard/src/lib/api/client.tsclients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/agent-runtime/src/security/policy.rsscripts/resolve-release-context.mjsscripts/release-components.mjsclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/runtime/MobileRuntimeCoordinator.ktclients/web/apps/dashboard/src/composables/useAdmin.tsclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.ktclients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktscripts/sync-internal-release-deps.mjsscripts/release-contract.test.mjsscripts/validate-affected-components.mjsclients/web/apps/dashboard/src/components/chat/ChatWorkspace.vueclients/web/apps/dashboard/src/composables/useChat.tsclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/App.ktscripts/check-tools.shscripts/mobile-smoke-test.shclients/web/apps/rook-dashboard/src/style.cssclients/rook/src/gateway/streaming.rsclients/web/apps/dashboard/src/components/sessions/SessionList.vueclients/agent-runtime/src/skills/mod.rsclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt
**/*.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/dashboard/src/components/chat/ChatMessage.vueclients/web/apps/dashboard/src/components/config/WebhookSettings.vueclients/web/apps/rook-dashboard/src/App.vueclients/web/apps/dashboard/src/components/sessions/SessionFilters.vueclients/web/apps/dashboard/src/components/chat/ChatWorkspace.vueclients/web/apps/dashboard/src/components/sessions/SessionList.vue
**/*.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/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/runtime/MobileRuntimeCoordinator.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.ktclients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/App.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt
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/security/policy.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/security/policy.rsclients/agent-runtime/src/skills/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/security/policy.rsclients/agent-runtime/src/skills/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/security/policy.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/security/policy.rsclients/rook/src/gateway/streaming.rsclients/agent-runtime/src/skills/mod.rs
🪛 Checkov (3.2.526)
dev/sandbox/Dockerfile
[low] 11-38: Ensure that sudo isn't used
(CKV2_DOCKER_1)
🪛 GitHub Actions: Scan with Detekt
clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt
[error] 45-45: Function inetPton has 3 return statements which exceeds the limit of 2. [ReturnCount]
[error] 40-40: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 40-40: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 40-40: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 47-47: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 50-50: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 50-50: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 52-52: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 53-53: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 54-54: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 55-55: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 165-165: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 166-166: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 166-166: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt
[error] 45-45: Function inetPton has 3 return statements which exceeds the limit of 2. [ReturnCount]
[error] 40-40: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 40-40: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 40-40: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 47-47: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 50-50: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 50-50: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 52-52: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 53-53: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 54-54: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 55-55: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 165-165: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 166-166: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 166-166: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt
[error] 1-1: File 'ChatComponents.kt' with '20' functions detected. Defined threshold inside files is set to '11' [TooManyFunctions]
[error] 92-92: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 109-109: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 151-151: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 187-187: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 212-212: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 222-222: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 261-261: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 295-295: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 578-578: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
🪛 Hadolint (2.14.0)
dev/sandbox/Dockerfile
[warning] 11-11: Pin versions in apt get install. Instead of apt-get install <package> use apt-get install <package>=<version>
(DL3008)
🪛 Shellcheck (0.11.0)
scripts/check-tools.sh
[info] 47-47: Don't use variables in the printf format string. Use printf '..%s..' "$foo".
(SC2059)
[info] 48-48: Don't use variables in the printf format string. Use printf '..%s..' "$foo".
(SC2059)
[info] 49-49: Don't use variables in the printf format string. Use printf '..%s..' "$foo".
(SC2059)
[info] 51-51: Don't use variables in the printf format string. Use printf '..%s..' "$foo".
(SC2059)
🔇 Additional comments (31)
clients/web/apps/dashboard/src/composables/useLocalMemoryExplorer.ts (1)
121-121: Good refactor for last-element access.Using
groupEntries.at(-1)is clearer than manual indexing and remains safe with the existing optional chaining onlastEntry.clients/web/apps/dashboard/src/components/config/WebhookSettings.vue (1)
26-31: Refactor is safe and clearer.The explicit
element/inputflow preserves behavior while improving readability and null safety inresolveWebhookSecretInput().clients/web/apps/dashboard/src/components/sessions/SessionList.vue (1)
262-268: CSS consolidation looks good and preserves interaction behavior.This keeps the
.select-btnstyles centralized without changing component logic, and touch-target constraints remain intact.clients/web/apps/dashboard/src/composables/useChat.ts (1)
103-104: UUID v4 fallback masking looks correct and safer in TS.Good change: removing non-null assertions here keeps the same runtime behavior while avoiding unnecessary assertion noise on a fixed-length
Uint8Array.clients/web/apps/rook-dashboard/src/lib/api/client.ts (1)
182-182: Header spread simplification looks safe.This change preserves behavior while simplifying the merge expression; no regression found in header construction for this segment.
dev/sandbox/Dockerfile (1)
11-11: Good hardening change on package install.Using
--no-install-recommendsreduces unnecessary packages and image surface area without changing declared dependencies.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/runtime/MobileRuntimeCoordinator.kt (2)
251-259: Good consolidation of event handling without behavior drift.This keeps
ApprovalPending/Failurebehavior intact while removing duplicated assistant-message branching.
267-272: Helper extraction is clean and idiomatic.Type mapping is explicit and safely nullable for non-assistant events.
clients/web/apps/rook-dashboard/src/style.css (1)
294-302: LGTM — contrast ratios are solid.Both updated backgrounds provide excellent contrast against their paired text colors:
Selector Background Text Approx. Ratio .state-banner.info#063246#e0f7ff~10.8 : 1 .state-banner.danger#4a0f13#ffe0e0~11.3 : 1 Both exceed WCAG AA (4.5:1) and AAA (7:1) thresholds. No layout or inheritance regressions introduced.
clients/agent-runtime/src/skills/mod.rs (1)
184-271: Clean extraction; behavior and path-safety preserved.The pipeline split (
canonical_skills_dir_in_workspace→read_dir→filter_map(load_skill_entry)) reads well, and the per-entry helpers (canonical_skill_dir_in_root,warn_if_invalid_skill_name,load_skill_from_canonical_dir,warn_if_legacy_skill_toml_only) keep the workspace-escape and SKILL.toml-skip semantics intact. The existing symlink-escape and toml-only tests continue to cover the critical paths.clients/agent-runtime/src/security/policy.rs (1)
2262-2290: LGTM! Formatting improvements applied.The test function has been reformatted for consistency—single-line
vec!initialization and multi-lineassert_eq!for better readability. No behavioral changes; all assertions still validate Medium risk for package manager run commands.clients/web/apps/dashboard/src/composables/useAdmin.ts (2)
131-133: Good extraction of URL params type alias.
BuildUrlParamskeeps parameter typing centralized and makesbuildUrleasier to maintain without changing behavior.
441-447: Path construction refactor improves correctness readability.The explicit
if/elseflow is clearer than a nested ternary, and the existing guard +encodeURIComponentpreserve safe path handling.scripts/release-contract.test.mjs (4)
112-126: Executable candidate refactor keeps trusted-path enforcement intact.Good extraction: the helper still enforces trusted absolute paths and rejects separator-containing relative configured values before expansion.
Also applies to: 130-130
432-433: State YAML assertions are now more robust to indentation checks.The
{4}quantifier keeps the intent clear while preserving the apply/verify contract validation.
689-689: String coercion simplification is safe and clearer.Switching to
.map(String)is equivalent and reduces noise in error aggregation.
1008-1008: Raw-string assertion label is the right choice here.Using
String.rawcorrectly keeps the\nsequence literal in the failure message text.clients/web/apps/dashboard/src/components/sessions/SessionFilters.vue (1)
21-27: Sanitization fallback behavior is solid.These guards correctly enforce valid filter/sort values with safe defaults, which protects emit payload contracts from invalid UI state.
.github/workflows/core-check.yml (1)
80-84: Pinned Rust action and output wiring look good.Using a commit-pinned
dtolnay/rust-toolchainaction with the resolved channel output is a solid hardening improvement.scripts/mobile-smoke-test.sh (1)
37-40: LGTM — cosmetic-only refactor.Assigning
$1to a named local before printing is functionally identical to direct interpolation; the change safely satisfies the SonarCloud rule with no behavioral impact.clients/web/apps/dashboard/src/components/chat/ChatMessage.vue (1)
68-70: LGTM — semantic upgrade is correct.
<section>with an accessible name carries the implicitrole="region", making the explicitroleattribute on the old<div>unnecessary. This is the preferred pattern.clients/web/apps/dashboard/src/components/chat/ChatWorkspace.vue (3)
140-159: LGTM — deduplication is correct.
persistMessages()→chat.clearSession()→beginSession(false)is the right sequence, and bothstartNewSessionandhandleSidebarNewChatnow share the same path.persistMessagessafely guards against a nullcurrentSessionId, so there's no risk when triggered without an active session.
306-321: LGTM —Setmembership is correct and more efficient.O(1)
.has()checks replace O(n).includes()forvalidRolesandvalidStatuseswithout any change to validation semantics.
480-485: LGTM —<output>is the right element here, and the explicit attributes are defensively correct.
<output>elements have an implicit ARIA live region of typestatus(role="status"), aligning with the "prefer native HTML" principle. The explicitaria-atomic="true"is not redundant in practice: sincerole="status"is not treated as atomic by default in some environments, it is advisable to add an explicitaria-atomic="true"if the entire contents of the container should be announced.clients/web/apps/dashboard/src/components/chat/ChatWorkspace.spec.ts (2)
225-231: LGTM — timing and index are correct.The
setTimeout(resolve, 0)drain flushesqueueAnnouncement's zero-delay callback, and[0]correctly maps to the first<output>(session announcements) per the template order.
271-273: LGTM — index mapping matches template order.
liveOutputs[1]correctly targets the approval-announcement<output>(second in DOM order). The optional chaining?.text()does not silently swallow a missing element — Vitest'stoContainwill throw onundefined, keeping the assertion meaningful.scripts/resolve-release-context.mjs (1)
35-51:parseVersionSuffixregex update LGTM.Four capture groups correctly align with the destructure on line 42, and each component is used in template literals — no implicit numeric coercion issues.
scripts/sync-internal-release-deps.mjs (2)
9-32:resolveModeand arg-parsing logic LGTM.
Set-based dedup, hoistedresolveMode, and thewantsHelpearly-exit before the!modeguard all compose correctly. ThewantsWrite === wantsCheckidentity check cleanly handles both "neither" and "both" cases.
109-126:collectInternalPathDependenciesregex LGTM.Single-line
[^\n]*scoping,gmmultiline anchors, and the version-presence guard at line 114 all look correct for TOML inline-table dependency entries.scripts/release-components.mjs (1)
69-79:validateObjecthelper andvalidateGraphShaperefactor LGTM.
!valueguards againstnull/undefinedbefore thetypeofcheck,Array.isArraycorrectly excludes arrays, andvalidateObject(graph, ...)fires before anygraph.*property access — no NPE risk.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/App.kt (1)
156-168: LGTM — callback consolidation is clean and correct.The mutual-exclusion logic (
if (value) showSessionHistory = false/if (value) showConfig = false) is correct and the four callbacks wire through to the same coordinator lambdas as before.
coderabbitai
Bot
removed
area:rust
area:docs
risk:high
risk:security
area:ci
area:kotlin
area:web
labels
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt (1)
39-42:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDetekt gate is still blocked by magic numbers in byte/time conversions.
Line 41 and Line 151-152 still trigger
MagicNumber, and CI is currently failing on them. Please extract these into named constants.Suggested patch
+private const val BYTE_MASK = 0xFF +private const val BYTE_SHIFT = 8 +private const val MILLIS_PER_SECOND = 1000L +private const val MICROS_PER_MILLISECOND = 1000 private fun htons(value: UShort): UShort { val bytes = value.toInt() - return ((bytes shr 8) or ((bytes and 0xFF) shl 8)).toUShort() + return ((bytes shr BYTE_SHIFT) or ((bytes and BYTE_MASK) shl BYTE_SHIFT)).toUShort() } - tv.tv_sec = (config.timeoutMs / 1000).convert() - tv.tv_usec = ((config.timeoutMs % 1000).toInt() * 1000).convert() + tv.tv_sec = (config.timeoutMs / MILLIS_PER_SECOND).convert() + tv.tv_usec = ((config.timeoutMs % MILLIS_PER_SECOND).toInt() * MICROS_PER_MILLISECOND).convert()Also applies to: 151-152
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt` around lines 39 - 42, The htons function uses magic numbers (0xFF and 8); extract them into named constants (e.g., BYTE_MASK = 0xFF and BYTE_SHIFT = 8) and replace the literals in htons with those constants to satisfy Detekt; also replace the magic numbers used in the other byte/time conversion sites referenced in the review (the same mask/shift operations at lines 151-152) to reuse these constants so all occurrences are named and the MagicNumber rule is resolved.clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt (1)
47-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDetekt still fails on magic numbers in this file.
The current changes leave unresolved
MagicNumberviolations at Line 47, Line 67, and Line 182-183, which keeps the scan red. Please finish extracting these literals (or refactor the indexed access to avoid raw numeric indices).Also applies to: 67-67, 182-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt` at line 47, The file still contains raw numeric literals used for byte-manipulation and indexed access; extract those magic numbers into clearly named constants (e.g., BYTE_SHIFT = 8, BYTE_MASK = 0xFF, INDEX_HEADER = 0, INDEX_LENGTH = 1 or similar) and replace the literals in the byte-swapping expression and any array/indexed accesses so Detekt no longer flags MagicNumber. Locate usages in SocketIosRuntimeCompanionClient (the byte swap/compose expressions and any code that uses hard-coded indices) and refactor those methods to use the new constants or, where appropriate, use destructuring or helper functions (e.g., toUShortFromBytes, readUShortAt) to eliminate raw numeric indices and shifts.clients/rook/src/gateway/streaming.rs (1)
223-241:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSilent swallowing of upstream and parser errors loses observability.
Err(_) => false(parser error) andSome(Err(_)) | None => false(upstream transport error) are indistinguishable from a clean EOF here — the stream just ends,on_completeruns as if everything succeeded, and the caller inhandlers.rsrecords a "success" usage outcome. For a gateway path, surfacing these astracing::warn!/error!(with the parse error variant or upstream error context) is cheap insurance against blind production failures.Suggested patch
match upstream.next().await { Some(Ok(chunk)) => match parser.push(&chunk) { Ok(payloads) => { pending.extend(payloads); true } - Err(_) => false, + Err(err) => { + tracing::warn!(?err, "openai sse parser rejected upstream chunk; terminating stream"); + false + } }, - Some(Err(_)) | None => false, + Some(Err(err)) => { + tracing::warn!(error = %err, "upstream sse stream errored; terminating"); + false + } + None => false, }Note: this requires
E: std::fmt::Display(orDebug) on the upstream error type — adjust trait bounds onupstream_event_stream*accordingly.As per coding guidelines: "Focus on Rust idioms... Flag... weak error context." and "Look for behavioral regressions... across modules."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/rook/src/gateway/streaming.rs` around lines 223 - 241, The function extend_pending_from_upstream silently swallows parser and upstream errors (parser.push and upstream.next) making EOF indistinguishable from failures; update extend_pending_from_upstream to log errors (tracing::warn! or tracing::error!) when parser.push returns Err(_) and when upstream.next yields Some(Err(e)) or None (if you want to differentiate EOF vs transport error), add the trait bound E: std::fmt::Display or Debug to the function signature (and propagate that bound to upstream_event_stream* call sites), and keep returning false after logging so the caller still treats the stream as ended but with observability; reference the OpenAiSseParser, parser.push, upstream.next(), and adjust upstream_event_stream* trait bounds accordingly.
♻️ Duplicate comments (1)
clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt (1)
52-73:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winManual IPv4 packing here can break socket target address; use POSIX
inet_ptondirectly.Line 190-195 assigns
sin_addr.s_addrfrom a manually packedUInt; on simulator little-endian this is risky and diverges from the arm64 implementation that already usesplatform.posix.inet_pton. Replace the custom parser path with directinet_pton+ return-code check.Suggested patch
+import platform.posix.inet_pton ... -private fun inetPton(host: String): UInt { - ... -} ... - val networkAddress = inetPton(config.host) - if (networkAddress == INVALID_IPV4_ADDRESS) { - return null - } - addr.sin_addr.s_addr = networkAddress + if (inet_pton(AF_INET, config.host, addr.sin_addr.ptr) != 1) { + return null + }#!/bin/bash # Verify simulator vs arm64 address-parsing implementation. rg -n "fun inetPton|inet_pton\\(|sin_addr\\.s_addr\\s*=|IPV4_SHIFT_" \ clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt \ clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktExpected result: simulator file should match arm64 behavior by using
inet_pton(...)and not manual packed-address assignment.As per coding guidelines "Security first, performance second. Validate input boundaries, auth/authz implications, and secret management."
Also applies to: 190-195
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt` around lines 52 - 73, The inetPton function currently manually parses and packs an IPv4 string which can produce wrong byte order on the simulator; update inetPton to call platform.posix.inet_pton for AF_INET, passing a properly allocated in_addr buffer and check the return value (==1 means success) before reading sin_addr.s_addr, returning that network-order value as UInt on success and INVALID_IPV4_ADDRESS on failure; reference the inetPton function and the sin_addr.s_addr assignment (and IPV4_* constants) when making the change so the iosSimulatorArm64 implementation matches the iosArm64 behavior.
🤖 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 @.github/workflows/core-check.yml:
- Around line 61-85: The Rust channel extraction step (rust-channel) only reads
"channel" from the TOML and discards other toolchain fields; update the python
snippet that reads toolchain_file (clients/agent-runtime/rust-toolchain.toml) to
also parse "components" and "targets" (and optionally "profile") from
data.get("toolchain", {}), serialize them (e.g., join lists into comma-separated
strings or JSON) and write them to GITHUB_OUTPUT as additional outputs like
components and targets (and profile) so the downstream dtolnay/rust-toolchain
action invocation can consume them; then change the 🦀 Setup Rust toolchain step
to pass those outputs into the action inputs (components: ${{
steps.rust-channel.outputs.components }}, targets: ${{
steps.rust-channel.outputs.targets }}), ensuring empty or missing fields are
handled gracefully.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt`:
- Around line 1-3: The file-level suppression only lists "TooManyFunctions" and
Detekt still flags PascalCase composables; update the file-level `@file`:Suppress
annotation in ChatComponents.kt to include "FunctionNaming" (or alternatively
rename the composable functions such as the PascalCase composables referenced
like the composables around lines where names appear e.g., the composable
functions currently causing issues) so Detekt stops reporting FunctionNaming;
specifically edit the existing `@file`:Suppress(...) declaration above the file to
add "FunctionNaming" to the suppressed ruleset (or rename the affected
composable function identifiers to follow the naming rule).
In `@clients/web/apps/rook-dashboard/src/App.vue`:
- Line 24: The call to getLocation() is placed before its function declaration
which relies on hoisting; move the getLocation function definition above the
initialization of route (const route =
ref<RookRoute>(normalizeHashRoute(getLocation()?.hash ?? ""))) so the helper
appears before its use for clearer reading, and apply the same reordering for
the other occurrences around lines 58-60 where getLocation is referenced.
In `@scripts/resolve-release-context.mjs`:
- Around line 19-23: The rethrow for a JSON.parse failure currently uses
TypeError which misrepresents a parse/syntax issue; in the block that catches
the JSON.parse error (where affectedComponentsRaw and parsed are used and the
throw includes { cause: error }), change the thrown error from TypeError to
SyntaxError (or generic Error) so callers observing err instanceof SyntaxError
or err.name get the correct semantic; leave the later non-array check that
throws TypeError (for the parsed value wrong type) unchanged.
In `@scripts/validate-affected-components.mjs`:
- Around line 10-12: The catch for JSON.parse currently re-throws a TypeError
which loses the original SyntaxError semantics; change the catch to re-throw a
SyntaxError (e.g., new SyntaxError(`Invalid AFFECTED_COMPONENTS JSON:
${affectedComponentsRaw}`, { cause: error })) so malformed JSON preserves
SyntaxError semantics while the later TypeError for non-array values remains
unchanged; update the throw in the catch surrounding JSON.parse of
affectedComponentsRaw accordingly.
---
Outside diff comments:
In
`@clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt`:
- Around line 39-42: The htons function uses magic numbers (0xFF and 8); extract
them into named constants (e.g., BYTE_MASK = 0xFF and BYTE_SHIFT = 8) and
replace the literals in htons with those constants to satisfy Detekt; also
replace the magic numbers used in the other byte/time conversion sites
referenced in the review (the same mask/shift operations at lines 151-152) to
reuse these constants so all occurrences are named and the MagicNumber rule is
resolved.
In
`@clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt`:
- Line 47: The file still contains raw numeric literals used for
byte-manipulation and indexed access; extract those magic numbers into clearly
named constants (e.g., BYTE_SHIFT = 8, BYTE_MASK = 0xFF, INDEX_HEADER = 0,
INDEX_LENGTH = 1 or similar) and replace the literals in the byte-swapping
expression and any array/indexed accesses so Detekt no longer flags MagicNumber.
Locate usages in SocketIosRuntimeCompanionClient (the byte swap/compose
expressions and any code that uses hard-coded indices) and refactor those
methods to use the new constants or, where appropriate, use destructuring or
helper functions (e.g., toUShortFromBytes, readUShortAt) to eliminate raw
numeric indices and shifts.
In `@clients/rook/src/gateway/streaming.rs`:
- Around line 223-241: The function extend_pending_from_upstream silently
swallows parser and upstream errors (parser.push and upstream.next) making EOF
indistinguishable from failures; update extend_pending_from_upstream to log
errors (tracing::warn! or tracing::error!) when parser.push returns Err(_) and
when upstream.next yields Some(Err(e)) or None (if you want to differentiate EOF
vs transport error), add the trait bound E: std::fmt::Display or Debug to the
function signature (and propagate that bound to upstream_event_stream* call
sites), and keep returning false after logging so the caller still treats the
stream as ended but with observability; reference the OpenAiSseParser,
parser.push, upstream.next(), and adjust upstream_event_stream* trait bounds
accordingly.
---
Duplicate comments:
In
`@clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt`:
- Around line 52-73: The inetPton function currently manually parses and packs
an IPv4 string which can produce wrong byte order on the simulator; update
inetPton to call platform.posix.inet_pton for AF_INET, passing a properly
allocated in_addr buffer and check the return value (==1 means success) before
reading sin_addr.s_addr, returning that network-order value as UInt on success
and INVALID_IPV4_ADDRESS on failure; reference the inetPton function and the
sin_addr.s_addr assignment (and IPV4_* constants) when making the change so the
iosSimulatorArm64 implementation matches the iosArm64 behavior.
🪄 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: 2eb72e2a-ca9e-4423-9903-ecd8d74385e3
⛔ Files ignored due to path filters (1)
clients/rook/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
.github/workflows/core-check.ymlclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.ktclients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/rook/Cargo.tomlclients/rook/src/gateway/streaming.rsclients/web/apps/dashboard/src/components/sessions/SessionFilters.vueclients/web/apps/rook-dashboard/src/App.vueclients/web/apps/rook-dashboard/src/lib/session/useRookSession.tsscripts/resolve-release-context.mjsscripts/validate-affected-components.mjs
📜 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: semgrep-cloud-platform/scan
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (4)
**/*
⚙️ 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/rook/Cargo.tomlscripts/validate-affected-components.mjsclients/web/apps/rook-dashboard/src/App.vuescripts/resolve-release-context.mjsclients/web/apps/dashboard/src/components/sessions/SessionFilters.vueclients/web/apps/rook-dashboard/src/lib/session/useRookSession.tsclients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/rook/src/gateway/streaming.rsclients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt
**/*.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/rook-dashboard/src/App.vueclients/web/apps/dashboard/src/components/sessions/SessionFilters.vue
**/*.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/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.ktclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt
**/*.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/rook/src/gateway/streaming.rs
🪛 GitHub Actions: Scan with Detekt
clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt
[error] 41-41: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 41-41: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 41-41: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 151-151: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 152-152: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 152-152: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt
[error] 47-47: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 47-47: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 47-47: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 67-67: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 182-182: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 183-183: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
[error] 183-183: This expression contains a magic number. Consider defining it to a well named constant. [MagicNumber]
clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt
[error] 96-96: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 113-113: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 155-155: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 191-191: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 216-216: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 226-226: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 265-265: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 299-299: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
[error] 583-583: Function names should match the pattern: [a-z][a-zA-Z0-9]* [FunctionNaming]
🔇 Additional comments (12)
clients/web/apps/dashboard/src/components/sessions/SessionFilters.vue (2)
12-19: Strong type/runtime coupling here—nice hardening.Union aliases + typed
Sets keep allowed values aligned and reduce future drift risk.
21-27: Sanitizers are defensive and behaviorally safe.Allow-list checks with explicit defaults prevent invalid filter states from propagating.
clients/web/apps/rook-dashboard/src/lib/session/useRookSession.ts (1)
38-60: Past concern fully resolved — LGTM.The three
safe*helpers correctly combineglobalThis.window?.sessionStorageoptional chaining (handles SSR / no-windowenvironments) withtry/catch(handlesSecurityErrorandQuotaExceededErrorin restricted browser contexts). Both code paths are covered.clients/web/apps/rook-dashboard/src/App.vue (1)
58-92: Past concern fully resolved — LGTM.
getLocation()guards alllocationaccess, and bothaddEventListener/removeEventListenerare type-checked before use. This is safe across browser, SSR, Web Worker, and Node.js contexts.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt (1)
493-573: Nice refactor: session-history rendering is cleaner and easier to maintain.
SessionHistoryItemStyle+sessionHistoryItemLabel/sessionHistoryItemActionreduces branching insessionHistoryItem, and@Immutableon the style type makes the Compose contract explicit.clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt (2)
126-132: Good stability contract for callback container.Adding
@StabletoChatWorkspaceCallbacksaligns with neighboring state/action models and makes Compose intent explicit.
150-190: Callback-object migration is wired correctly.Routing send/query/toggle through
callbacksand keyingrememberwithcallbackskeeps handlers current and avoids stale references.scripts/resolve-release-context.mjs (1)
18-20: AI summary inconsistency:{ cause: error }is still present.The AI summary states this file "no longer passes the caught parsing exception as a
cause," but line 19 still passes{ cause: error }. This is consistent withvalidate-affected-components.mjsline 11, so the summary appears to be wrong. No action needed if the intent is to keepcause.scripts/validate-affected-components.mjs (1)
14-16: LGTM —TypeErroris appropriate for the non-array case.A parsed-but-non-array value is genuinely a wrong type, so
TypeErroris the right choice here.clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt (1)
159-161: Good fail-fast host parsing before connect.This early
inet_ptoncheck prevents malformed host input from reachingconnect()..github/workflows/core-check.yml (1)
13-13: Path filter forrust-toolchain.tomlcorrectly added.Resolves the prior concern that toolchain bumps wouldn't trigger this workflow. 👍
Also applies to: 28-28
clients/rook/src/gateway/streaming.rs (1)
160-183:on_completenow fires on every termination path — past concern resolved.The previous review's worry that
on_completewas skipped on EOF/error termination is fixed: line 178-181 now invokesrun_completion_oncewhennext_pending_or_upstream_eventreturnsNone, and the newcompletion_runs_when_upstream_ends_without_donetest locks in this contract. Nicely done.Also applies to: 304-318
# Conflicts: # clients/rook/Cargo.lock # clients/rook/Cargo.toml
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
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt`:
- Around line 563-574: sessionHistoryItemAction currently renders an
OutlinedButton with only the text "Open", making screen readers announce
identical labels; update sessionHistoryItemAction to accept a sessionLabel:
String parameter and apply semantics { contentDescription = "Open $sessionLabel"
} (or similar localized string) to the button, then update the caller
sessionHistoryItem to pass the session title or id-derived label so each button
reads uniquely for TalkBack/VoiceOver.
In
`@clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt`:
- Around line 56-75: The parseIpv4Address function currently builds a UInt by
manual bit-shifting (validBytes, IPV4_FIRST_OCTET, IPV4_SHIFT_24, etc.) which
does not convert to network byte order and causes reversed bytes on
little-endian simulator; replace parseIpv4Address with a call to inet_pton
(importing it) and use its result for sin_addr.s_addr just like the iosArm64
implementation so the address is correctly parsed and stored in network byte
order; ensure you remove the manual shifting logic and return the same
type/representation expected by the caller (matching the hardware target
behavior).
In `@clients/rook/src/gateway/streaming.rs`:
- Around line 317-331: Add two new tokio tests alongside
completion_runs_when_upstream_ends_without_done that exercise the other
termination paths so the completion callback runs exactly once: 1) create
completion_runs_when_upstream_errors_once which feeds
upstream_event_stream_with_completion a stream that yields Some(Err(_)) (e.g.,
stream::iter([Err::<Bytes, _>(...)]) or similar) and assert events length and
that the AtomicUsize completion_count is incremented exactly once; and 2) create
completion_runs_on_parse_error_once which feeds a malformed frame (e.g.,
invalid/malformed Bytes that causes parse failure) into
upstream_event_stream_with_completion and assert the same single completion
invocation using completion_count. Use the same pattern of Arc<AtomicUsize>,
cloning for the callback, collecting the stream into a Vec, and checking
events.len() and completion_count.load(Ordering::SeqCst) == 1 in each test.
🪄 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: 7e2f255a-d0ae-4730-a111-53a0edc8d68d
⛔ Files ignored due to path filters (1)
clients/rook/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
.github/workflows/core-check.ymlclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.ktclients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/rook/Cargo.tomlclients/rook/src/gateway/streaming.rsclients/web/apps/rook-dashboard/src/App.vuescripts/release-contract.test.mjsscripts/resolve-release-context.mjsscripts/validate-affected-components.mjs
📜 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: semgrep-cloud-platform/scan
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (4)
**/*
⚙️ 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/rook/Cargo.tomlscripts/validate-affected-components.mjsclients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.ktclients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/web/apps/rook-dashboard/src/App.vuescripts/resolve-release-context.mjsclients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/rook/src/gateway/streaming.rsscripts/release-contract.test.mjs
**/*.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/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.ktclients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktclients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt
**/*.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/rook-dashboard/src/App.vue
**/*.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/rook/src/gateway/streaming.rs
🔇 Additional comments (15)
clients/web/apps/rook-dashboard/src/App.vue (1)
23-25: Global runtime guards are correctly applied and reduce cross-environment breakage risk.Using
getLocation()plus guardedaddEventListener/removeEventListeneris a solid fix for SSR/worker/non-browser contexts, and the hash routing behavior remains consistent.Also applies to: 28-28, 63-63, 68-71, 75-77, 83-85, 89-91
scripts/validate-affected-components.mjs (1)
10-16: LGTM — error semantics now match thrown type.
SyntaxErrorfor malformed JSON (preserving original viacause) andTypeErrorfor non-array payload are the right choices, and the test on line 727 ofrelease-contract.test.mjsalready asserts the newJSON:message.scripts/resolve-release-context.mjs (2)
18-24: LGTM — resolves the prior TypeError/SyntaxError mismatch.Parse failure now throws
SyntaxErrorwithcause, and the non-array branch correctly retainsTypeError. Aligns withvalidate-affected-components.mjs.
36-36:\d+swap is behavior-preserving.In ECMAScript,
\dwithout theuflag matches exactly[0-9], so the regex is equivalent for ASCII version segments.scripts/release-contract.test.mjs (5)
112-126: Helper extraction looks solid — trust gate intact.Empty/non-string rejection, absolute-trust enforcement, and separator-bearing relative rejection are preserved, while bare names fan out across trusted dirs. No security regression versus the prior inline logic, and cross-platform separators are handled (
path.sep+/).
128-151: Resolver wiring is correct.Spreading
configuredCandidatesfirst preserves user override precedence, and the per-candidateisAbsolute+isTrustedExecutablePath+statSync/accessSyncgate is unchanged. Duplicate candidates whenCARGO=cargoare harmless since.find()short-circuits on the first executable hit.
683-696:.map(String)is a clean simplification.Equivalent to the inline arrow and slightly more readable. Behavior unchanged for
Buffer/ stringstderr/stdoutsince both stringify identically here.
724-728: Assertion correctly tracks the new error message.Matches the
Invalid AFFECTED_COMPONENTS JSON:wording emitted by the updatedSyntaxErrorthrow invalidate-affected-components.mjs.
1005-1009:String.rawis the right fix for the literal-\nassertion.Without
String.raw, the assertion message itself would contain a real newline, weakening the intent of the check (which is specifically that the workflow must not emit the literal\nescape sequence).clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatComponents.kt (1)
1-4: File-levelFunctionNamingsuppression addresses the prior CI finding — LGTM.The past review comment flagging missing
FunctionNamingin@file:Suppressis now resolved, and@ImmutableonSessionHistoryItemStyle(lines 515–523) was addressed as well.clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt (1)
162-164: Good fail-fast validation oninet_ptonresultLine 162-164 correctly aborts on invalid/unparseable host input before
connect(). This is the right behavior for transport setup hardening..github/workflows/core-check.yml (2)
13-28: Path filters now cover toolchain updates correctly.Including
clients/agent-runtime/rust-toolchain.tomlin both trigger blocks prevents missed core-check runs when the Rust toolchain changes.
61-95: Deterministic Rust toolchain setup looks solid.Parsing toolchain settings from TOML and installing via a pinned action SHA removes runner-image drift and keeps CI behavior predictable.
clients/rook/Cargo.toml (1)
71-71: Dependency version bump is consistent.The
corvus-traitsupdate to3.8.0while preserving the local path linkage is clean and aligned with the crate versioning in this file.clients/rook/src/gateway/streaming.rs (1)
163-187: Completion handling is correctly centralized now.The state-machine flow ensures completion runs on both sentinel and non-sentinel termination paths, addressing the prior contract gap.
| private fun parseIpv4Address(host: String): UInt? { | ||
| val bytes = host.split('.').map { it.toIntOrNull() } | ||
| val validBytes = bytes.filterNotNull() | ||
| val isValidAddress = | ||
| bytes.size == IPV4_PART_COUNT && | ||
| validBytes.size == IPV4_PART_COUNT && | ||
| validBytes.all { it in IPV4_BYTE_MIN..IPV4_BYTE_MAX } | ||
|
|
||
| if (!isValidAddress) { | ||
| return null | ||
| } | ||
|
|
||
| val bytes = parts.mapNotNull { it.toIntOrNull() } | ||
| if (bytes.size != 4 || bytes.any { it < 0 || it > 255 }) return 0u | ||
| val firstOctet = validBytes[IPV4_FIRST_OCTET].toUInt() shl IPV4_SHIFT_24 | ||
| val secondOctet = validBytes[IPV4_SECOND_OCTET].toUInt() shl IPV4_SHIFT_16 | ||
| val thirdOctet = validBytes[IPV4_THIRD_OCTET].toUInt() shl BYTE_SHIFT | ||
| val fourthOctet = validBytes[IPV4_FOURTH_OCTET].toUInt() | ||
| val address = firstOctet or secondOctet or thirdOctet or fourthOctet | ||
|
|
||
| return ((bytes[0].toUInt() shl 24) or | ||
| (bytes[1].toUInt() shl 16) or | ||
| (bytes[2].toUInt() shl 8) or | ||
| bytes[3].toUInt()) | ||
| return address.takeUnless { it == INVALID_IPV4_ADDRESS } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify host parsing approach differs by target and confirm manual packing on simulator.
rg -n --type=kotlin -C2 'parseIpv4Address|sin_addr\.s_addr|inet_pton\(' \
clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt \
clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktRepository: dallay/corvus
Length of output: 2489
🏁 Script executed:
#!/bin/bash
# Check imports and see if inet_pton is available in simulator target
head -50 clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt | grep -E "^import|^package"Repository: dallay/corvus
Length of output: 938
🏁 Script executed:
#!/bin/bash
# Check if there are tests for IPv4 parsing
fd -e kt -path "*test*" | xargs grep -l "parseIpv4Address\|SocketIosRuntime" 2>/dev/null || echo "No tests found"Repository: dallay/corvus
Length of output: 259
🏁 Script executed:
#!/bin/bash
# Check the full simulator target file to understand the context around s_addr assignment
sed -n '35,200p' clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktRepository: dallay/corvus
Length of output: 6199
🏁 Script executed:
#!/bin/bash
# Check if inet_pton exists in the iosArm64Main imports or if inet_pton is in posix module
grep -n "inet_pton" clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktRepository: dallay/corvus
Length of output: 165
🏁 Script executed:
#!/bin/bash
# Search for any htonl or byte order conversion of address across both files
rg -n "htonl|inet_pton|toNetworkOrder" clients/composeApp/src/iosSimulatorArm64Main/ clients/composeApp/src/iosArm64Main/Repository: dallay/corvus
Length of output: 389
🏁 Script executed:
#!/bin/bash
# Check if there's a reason inet_pton isn't used in simulator (platform check or comment)
fd -name "*.kt" -path "*iosSimulator*" | head -5 | xargs grep -B5 -A5 "parseIpv4Address" 2>/dev/null | head -40Repository: dallay/corvus
Length of output: 288
🏁 Script executed:
#!/bin/bash
# Check if inet_pton is available in platform.posix for simulator by looking at iosArm64Main which clearly uses it
# and comparing what posix bindings might differ between targets
grep -r "expect\|actual" clients/composeApp/src/iosSimulatorArm64Main/ clients/composeApp/src/iosArm64Main/ | grep -i "socket\|inet" | head -20Repository: dallay/corvus
Length of output: 3700
🏁 Script executed:
#!/bin/bash
# Check if there are build configuration differences that might explain why inet_pton isn't used
find clients/composeApp -name "*.gradle*" -o -name "*.kts" | xargs grep -l "iosSimulator\|iosArm64" | head -3 | xargs cat | head -80Repository: dallay/corvus
Length of output: 2523
🏁 Script executed:
/#bash
# Verify the byte-order semantics: check if parseIpv4Address result is in host or network byte order
# by examining any comments or documentation
sed -n '48,75p' clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktRepository: dallay/corvus
Length of output: 1184
🏁 Script executed:
#!/bin/bash
# Final verification: check what inet_pton is imported from and confirm availability
grep -B2 -A2 "import platform.posix.inet_pton" clients/composeApp/src/iosArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.ktRepository: dallay/corvus
Length of output: 201
🏁 Script executed:
#!/bin/bash
# Check if there are any platform-specific conditionals that would prevent inet_pton in simulator
grep -r "expect\|actual.*parseIpv4\|SIMULATOR\|SIMULATOR_ARM" clients/composeApp/src/ | head -10Repository: dallay/corvus
Length of output: 1566
Replace manual IPv4 parsing with inet_pton to match hardware target and ensure correct byte-order handling
The simulator target manually constructs IPv4 addresses via bit-shifting (lines 56–72) but never converts to network byte order before assigning to sin_addr.s_addr. The hardware target (iosArm64) correctly uses inet_pton, which handles byte-order conversion internally. On little-endian ARM64, the manual approach writes reversed bytes to the socket structure, causing valid hosts to fail connect().
This is fixable by importing inet_pton and replacing the manual function with a direct call, mirroring the hardware implementation.
Proposed fix
import platform.posix.connect
+import platform.posix.inet_pton
@@
-private fun parseIpv4Address(host: String): UInt? {
- val bytes = host.split('.').map { it.toIntOrNull() }
- val validBytes = bytes.filterNotNull()
- val isValidAddress =
- bytes.size == IPV4_PART_COUNT &&
- validBytes.size == IPV4_PART_COUNT &&
- validBytes.all { it in IPV4_BYTE_MIN..IPV4_BYTE_MAX }
-
- if (!isValidAddress) {
- return null
- }
-
- val firstOctet = validBytes[IPV4_FIRST_OCTET].toUInt() shl IPV4_SHIFT_24
- val secondOctet = validBytes[IPV4_SECOND_OCTET].toUInt() shl IPV4_SHIFT_16
- val thirdOctet = validBytes[IPV4_THIRD_OCTET].toUInt() shl BYTE_SHIFT
- val fourthOctet = validBytes[IPV4_FOURTH_OCTET].toUInt()
- val address = firstOctet or secondOctet or thirdOctet or fourthOctet
-
- return address.takeUnless { it == INVALID_IPV4_ADDRESS }
-}
@@
- addr.sin_addr.s_addr = parseIpv4Address(config.host) ?: return null
+ if (inet_pton(AF_INET, config.host, addr.sin_addr.ptr) != 1) {
+ return null
+ }Also applies to: 192
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@clients/composeApp/src/iosSimulatorArm64Main/kotlin/com/profiletailors/corvus/runtime/SocketIosRuntimeCompanionClient.kt`
around lines 56 - 75, The parseIpv4Address function currently builds a UInt by
manual bit-shifting (validBytes, IPV4_FIRST_OCTET, IPV4_SHIFT_24, etc.) which
does not convert to network byte order and causes reversed bytes on
little-endian simulator; replace parseIpv4Address with a call to inet_pton
(importing it) and use its result for sin_addr.s_addr just like the iosArm64
implementation so the address is correctly parsed and stored in network byte
order; ensure you remove the manual shifting logic and return the same
type/representation expected by the caller (matching the hardware target
behavior).
|
1 participant
Related Issues
Follow-up to SonarCloud quality remediation for
dallay_corvus.Summary
Tested Information
pnpm --filter @corvus/dashboard run checkpnpm --filter @corvus/rook-dashboard run checknode --test scripts/release-contract.test.mjs./gradlew :composeApp:compileKotlinMetadatacargo fmt --all -- --checkinclients/rookcargo fmt --all -- --checkinclients/agent-runtimecargo test streaminginclients/rookcargo test --lib skillsinclients/agent-runtimecargo test --libinclients/agent-runtime(3808 passed)git diff --checkgpsbefore pushingmaintenance.Note: an earlier
cargo test skillsattempt broadened into integration targets andagent_e2ewas killed by the local environment (SIGKILL). The focused library skills tests and full library suite passed.Documentation Impact
Breaking Changes
None.
Checklist