docs: c4 architecture diagrams#7
Merged
Merged
Conversation
Contributor
|
Thank you for contributing to this project with this PR, welcome to the community and the amazing world of open source! |
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR removes iOS Xcode project configuration, enhances build system ignore patterns, adds resilient config file handling with platform-specific fallback logic, refactors UI component formatting, significantly expands architectural documentation with C4-model diagrams (Mermaid and PlantUML formats) in English and Spanish, and implements docs directory symlink management in the web build. Changes
Sequence DiagramsequenceDiagram
participant Caller as Caller
participant Schema as Config Schema
participant FS as File System
participant Backup as Backup System
Caller->>Schema: save_config(path, contents)
Schema->>FS: atomic_rename(temp, target)
alt Atomic Rename Success
FS-->>Schema: Ok()
Schema-->>Caller: Success
else Atomic Rename Fails
FS-->>Schema: Err(errno)
Schema->>Schema: should_fallback?<br/>(check errno)
alt Should Fallback (EBUSY/EXDEV)
Schema->>FS: write_config_in_place(target)
alt In-Place Write Success
FS-->>Schema: Ok()
Schema-->>Caller: Success (Fallback)
else In-Place Write Fails
FS-->>Schema: Err(io_error)
Schema->>Backup: restore_from_backup(target)
Backup-->>Schema: restored
Schema-->>Caller: Err(combined_error)
end
else No Fallback (Other errno)
Schema-->>Caller: Err(original_error)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 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 |
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-02-16 to 2026-02-16 |
![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: 26
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/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt (3)
232-240: 🛠️ Refactor suggestion | 🟠 MajorReduce parameter count in
ChatPanelto satisfy Detekt.
Detekt flags LongParameterList here. Group related inputs into aChatPanelState+ChatPanelHandlers.♻️ Proposed refactor (parameter grouping)
+data class ChatPanelState( + val modelName: String, + val inputPlaceholder: String, + val messages: List<ChatMessage>, + val query: String, +) + +data class ChatPanelHandlers( + val onQueryChange: (String) -> Unit, + val onSend: () -> Unit, +) + private fun ChatPanel( - modelName: String, - inputPlaceholder: String, - messages: List<ChatMessage>, - query: String, - onQueryChange: (String) -> Unit, - onSend: () -> Unit, + state: ChatPanelState, + handlers: ChatPanelHandlers, modifier: Modifier = Modifier, ) {🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 232 - 240, The ChatPanel function has too many parameters and triggers Detekt's LongParameterList; refactor by introducing two data holder types: ChatPanelState (containing modelName, inputPlaceholder, messages, query) and ChatPanelHandlers (containing onQueryChange and onSend), then update the ChatPanel signature to accept (state: ChatPanelState, handlers: ChatPanelHandlers, modifier: Modifier = Modifier) and update all call sites to construct/pass the new objects; ensure ChatPanel internals reference state.modelName etc. and preserve existing behavior and visibility.
151-165: 🛠️ Refactor suggestion | 🟠 MajorReduce parameter count in
ChatWorkspaceScreento satisfy Detekt.
Detekt flags LongParameterList here. Consider grouping callbacks and UI state into data classes.♻️ Proposed refactor (parameter grouping)
+data class ChatWorkspaceUi( + val state: ChatWorkspaceState, + val messages: List<ChatMessage>, + val query: String, + val showConfig: Boolean, + val gatewayConfig: AgentGatewayConfig, +) + +data class ChatWorkspaceHandlers( + val onQueryChange: (String) -> Unit, + val onSend: () -> Unit, + val onToggleConfig: () -> Unit, + val onBaseUrlChange: (String) -> Unit, + val onPairingCodeChange: (String) -> Unit, + val onBearerTokenChange: (String) -> Unit, + val onWebhookSecretChange: (String) -> Unit, +) + private fun ChatWorkspaceScreen( - state: ChatWorkspaceState, - messages: List<ChatMessage>, - query: String, - showConfig: Boolean, - gatewayConfig: AgentGatewayConfig, - onQueryChange: (String) -> Unit, - onSend: () -> Unit, - onToggleConfig: () -> Unit, - onBaseUrlChange: (String) -> Unit, - onPairingCodeChange: (String) -> Unit, - onBearerTokenChange: (String) -> Unit, - onWebhookSecretChange: (String) -> Unit, + ui: ChatWorkspaceUi, + handlers: ChatWorkspaceHandlers, modifier: Modifier = Modifier, ) {🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 151 - 165, The ChatWorkspaceScreen function has too many parameters (ChatWorkspaceState, messages, query, showConfig, gatewayConfig and many callbacks like onQueryChange, onSend, onToggleConfig, onBaseUrlChange, onPairingCodeChange, onBearerTokenChange, onWebhookSecretChange) which triggers Detekt's LongParameterList; refactor by creating small data-holder classes such as ChatWorkspaceUiState (wrapping state, messages, query, showConfig, gatewayConfig) and ChatWorkspaceCallbacks (wrapping onQueryChange, onSend, onToggleConfig, onBaseUrlChange, onPairingCodeChange, onBearerTokenChange, onWebhookSecretChange) and replace the long parameter list with these two objects plus an optional Modifier, updating call sites and tests accordingly to pass the grouped objects instead of individual params.
281-388: 🛠️ Refactor suggestion | 🟠 MajorSplit
ConfigPanelto fix Detekt LongParameterList + LongMethod.
Detekt flags parameter count and method length. Extract endpoint cards and field groups into helpers.♻️ Proposed refactor (extract sections)
private fun ConfigPanel( gatewayConfig: AgentGatewayConfig, onBaseUrlChange: (String) -> Unit, onPairingCodeChange: (String) -> Unit, onBearerTokenChange: (String) -> Unit, onWebhookSecretChange: (String) -> Unit, modifier: Modifier = Modifier, ) { - val healthUrl = endpointUrl(gatewayConfig.baseUrl, "/health") - val pairUrl = endpointUrl(gatewayConfig.baseUrl, "/pair") - val webhookUrl = endpointUrl(gatewayConfig.baseUrl, "/webhook") - Surface( ... ) { LazyColumn( ... ) { - item { ... baseUrl field ... } - item { ... pairingCode field ... } - item { ... bearerToken field ... } - item { ... webhookSecret field ... } - item { EndpointCard(... health ...) } - item { EndpointCard(... pair ...) } - item { EndpointCard(... webhook ...) } + item { ConfigFields(gatewayConfig, onBaseUrlChange, onPairingCodeChange, onBearerTokenChange, onWebhookSecretChange) } + item { EndpointCards(gatewayConfig.baseUrl) } } } } + +@Composable +private fun ConfigFields( + gatewayConfig: AgentGatewayConfig, + onBaseUrlChange: (String) -> Unit, + onPairingCodeChange: (String) -> Unit, + onBearerTokenChange: (String) -> Unit, + onWebhookSecretChange: (String) -> Unit, +) { ... } + +@Composable +private fun EndpointCards(baseUrl: String) { ... }🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and decide whether a code change is actually needed. If the finding is not valid or no change is required, do not modify code for that item and briefly explain why it was skipped. In `@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt` around lines 281 - 388, ConfigPanel is flagged by Detekt for too many parameters and being too long; split it into smaller composable helpers to reduce parameter count and method size. Extract the input fields into a new composable (e.g., ConfigFields) that accepts gatewayConfig and the four onXChange callbacks and renders the OutlinedTextField items, and extract the endpoint UI into another composable (e.g., EndpointCardsSection or EndpointList) that builds the EndpointCard calls using endpointUrl(gatewayConfig.baseUrl, ...). Keep ConfigPanel thin: compute healthUrl/pairUrl/webhookUrl as needed or pass only gatewayConfig to the helpers, remove unrelated formatting logic into those helpers, and call them from ConfigPanel so function length and parameter list are reduced.
🤖 Fix all issues with AI agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In @.gitignore:
- Line 147: Remove the redundant gitignore entry
"clients/web/apps/**/node_modules/" because it is already covered by the broader
"**/node_modules/" pattern; delete the specific pattern line and keep only the
global "**/node_modules/" entry to avoid duplication.
- Line 149: The comment "# Symlinks auto-generados por Gradle" in the .gitignore
is in Spanish while the rest of the file is English; update that comment to
English for consistency (e.g., change the line containing "Symlinks
auto-generados por Gradle" to "Gradle auto-generated symlinks" or similar),
preserving the comment marker and placement.
In `@clients/agent-runtime/src/config/schema.rs`:
- Around line 1085-1105: The fallback in write_config_in_place can leave the
target file truncated if the process dies mid-write; update load_or_init to
detect and recover from this by checking after open/read whether the current
config file is empty or fails to deserialize and, if so, look for the existing
backup (backup_path/.bak) and atomically restore it before proceeding (or return
a clear descriptive error if restore fails); reference load_or_init,
write_config_in_place, self.config_path, and backup_path in your changes and
ensure the backup is restored prior to any further config parsing so startup can
automatically recover from a truncated config file.
- Around line 1151-1155: Replace the magic errno integers in
should_fallback_to_in_place_write with named constants: update the matches!
check to compare against descriptive constants rather than raw 16 and 18. You
can either reference libc::EBUSY and libc::EXDEV (add libc dependency), use
std::io::ErrorKind variants if MSRV permits, or define local const EBUSY and
EXDEV integers at the top of the module and use those; ensure the function still
matches error.raw_os_error() against Some(EBUSY | EXDEV) equivalents and keep
the #[cfg(unix)] attribute on should_fallback_to_in_place_write.
In
`@clients/composeApp/src/commonMain/kotlin/com/profiletailors/corvus/ui/chat/ChatWorkspace.kt`:
- Around line 232-240: The ChatPanel function has too many parameters and
triggers Detekt's LongParameterList; refactor by introducing two data holder
types: ChatPanelState (containing modelName, inputPlaceholder, messages, query)
and ChatPanelHandlers (containing onQueryChange and onSend), then update the
ChatPanel signature to accept (state: ChatPanelState, handlers:
ChatPanelHandlers, modifier: Modifier = Modifier) and update all call sites to
construct/pass the new objects; ensure ChatPanel internals reference
state.modelName etc. and preserve existing behavior and visibility.
- Around line 151-165: The ChatWorkspaceScreen function has too many parameters
(ChatWorkspaceState, messages, query, showConfig, gatewayConfig and many
callbacks like onQueryChange, onSend, onToggleConfig, onBaseUrlChange,
onPairingCodeChange, onBearerTokenChange, onWebhookSecretChange) which triggers
Detekt's LongParameterList; refactor by creating small data-holder classes such
as ChatWorkspaceUiState (wrapping state, messages, query, showConfig,
gatewayConfig) and ChatWorkspaceCallbacks (wrapping onQueryChange, onSend,
onToggleConfig, onBaseUrlChange, onPairingCodeChange, onBearerTokenChange,
onWebhookSecretChange) and replace the long parameter list with these two
objects plus an optional Modifier, updating call sites and tests accordingly to
pass the grouped objects instead of individual params.
- Around line 281-388: ConfigPanel is flagged by Detekt for too many parameters
and being too long; split it into smaller composable helpers to reduce parameter
count and method size. Extract the input fields into a new composable (e.g.,
ConfigFields) that accepts gatewayConfig and the four onXChange callbacks and
renders the OutlinedTextField items, and extract the endpoint UI into another
composable (e.g., EndpointCardsSection or EndpointList) that builds the
EndpointCard calls using endpointUrl(gatewayConfig.baseUrl, ...). Keep
ConfigPanel thin: compute healthUrl/pairUrl/webhookUrl as needed or pass only
gatewayConfig to the helpers, remove unrelated formatting logic into those
helpers, and call them from ConfigPanel so function length and parameter list
are reduced.
In `@clients/web/.gitignore`:
- Around line 40-43: The .gitignore contains a redundant pattern: remove the
duplicate `.astro/` entry so only the recursive pattern `**/.astro/` remains
(keep `**/.astro/` and delete the separate `.astro/` line) to avoid duplication
while preserving the same ignore behavior.
In `@clients/web/apps/docs/astro.config.mjs`:
- Around line 119-133: The two sidebar entries use colliding slugs
('guides/architecture' and 'guides/architecture/index') which both resolve to
the same page; fix by giving the "C4 Diagrams" item a unique slug and matching
content file: rename the content file guides/architecture/index.md (or create a
new file) to guides/architecture/c4-diagrams.md and update the sidebar entry
slug from 'guides/architecture/index' to 'guides/architecture/c4-diagrams' (keep
label/translations the same) so Starlight/Astro resolves each page separately.
In `@clients/web/apps/docs/src/content/docs/en/guides/architecture.md`:
- Around line 59-119: Remove the duplicated "Build Logic (Convention Plugins)"
and "Dependency Management" sections (including the Kotlin dependencies example
code block) that appear a second time in the file so only the original headings
and content remain; search for the headings "Build Logic (Convention Plugins)"
and "Dependency Management" and delete the later copy (the repeated dependencies
{ implementation(libs.kotlin.stdlib) ... } example and the repeated explanatory
text) while keeping the C4 diagrams and Architecture Index links intact.
In
`@clients/web/apps/docs/src/content/docs/en/guides/architecture/diagrams/component/agent-core-kmp.mmd`:
- Line 56: The file defines a CSS class "classDef domain" but never applies it
to any node, so either remove that unused definition or apply it to the
domain-layer nodes to create visual differentiation; locate the "classDef domain
fill:`#85bbf0`,stroke:`#5d8fc9`,stroke-width:2px,color:`#000`" line and either delete
it or add "class <NodeName> domain" statements for the nodes that represent the
domain layer (currently all nodes use the "component" class), ensuring you
replace <NodeName> with the actual node identifiers you want styled.
In
`@clients/web/apps/docs/src/content/docs/en/guides/architecture/diagrams/container/system-containers.mmd`:
- Around line 9-10: The two actor nodes currently use the same Mermaid node type
ID ("actor(Developer)" and "actor(EndUser)") causing duplicate IDs; change them
to use distinct explicit node IDs/labels (e.g., Developer(("Developer")) and
EndUser(("EndUser"))) so each actor has a unique identifier, updating the actor
node declarations that currently read actor(Developer) and actor(EndUser) to the
suggested unique forms.
- Line 64: The classDef currently uses an invalid `shape:cylinder`; remove
`shape:cylinder` from the `classDef database` definition and instead define the
GRAPH_DB node using the Mermaid cylinder node syntax (e.g., [("label")]) so the
database appears as a cylinder; update the node named GRAPH_DB to use the
`[("GRAPH_DB")]`-style label and leave `classDef database
fill:`#438dd5`,stroke:`#2e6299`,stroke-width:2px,color:`#fff`` without
`shape:cylinder`.
In
`@clients/web/apps/docs/src/content/docs/en/guides/architecture/diagrams/container/system-containers.puml`:
- Line 6: Replace the remote PlantUML include that currently references the
repository default branch in system-containers.puml (the line starting with
"!include
https://raw.githubusercontent.com/plantuml-stdlib/C4-PlantUML/master/C4_Container.puml")
with a pinned release URL pointing to the stable tag (e.g., v2.13.0) so the
diagram uses a specific, immutable version of C4_PlantUML and avoids silent
breakage from upstream changes.
In
`@clients/web/apps/docs/src/content/docs/en/guides/architecture/diagrams/context/system-context.mmd`:
- Around line 9-10: Both actor lines reuse the literal node id "actor" and the
later relationships refer to undefined IDs "Developer" and "EndUser"; change
each actor definition to a unique node id and apply the :::person class to those
ids (e.g., replace actor(Developer) with DeveloperActor(Developer):::person and
actor(EndUser) with EndUserActor(EndUser):::person) and then update any
relationship references that use Developer or EndUser to use DeveloperActor and
EndUserActor so the styled nodes are the ones referenced.
In
`@clients/web/apps/docs/src/content/docs/en/guides/architecture/diagrams/module-dependencies.mmd`:
- Line 12: The legend node ID EXTERNAL (from the node declaration
EXTERNAL["External"]:::external) conflicts with the subgraph ID EXTERNAL (the
subgraph defined around lines 32-39); rename the legend node ID to a unique
identifier (e.g., EXTERNAL_LEGEND or LEGEND_EXTERNAL) and update any internal
references/styles for that node so the node declaration and the subgraph ID are
distinct.
In `@clients/web/apps/docs/src/content/docs/en/guides/architecture/index.md`:
- Around line 45-65: Add missing blank lines around headings and fenced code
blocks to satisfy MD022/MD031: ensure there is an empty line before and after
the "### Option 3: Mermaid CLI" and its triple-backtick bash block, and add an
empty line before the "### Option 4: PlantUML" heading, an empty line between
that heading and the "For `.puml` files:" paragraph, and an empty line before
and after the triple-backtick bash block for PlantUML; update the sections
referenced by "### Option 3: Mermaid CLI" and "### Option 4: PlantUML"
accordingly.
In `@clients/web/apps/docs/src/content/docs/es/guides/architecture.md`:
- Line 119: The link text `Ver [Índice de Diagramas](./architecture/index.md)
(en inglés)` points to the Spanish index but is labeled as English; fix by
either updating the link target to the English index file (replace the
`./architecture/index.md` target with the English index) or changing the label
to reflect Spanish (e.g., replace “(en inglés)” with “(en español)”) so the link
label and destination match.
- Around line 86-106: Update the fenced ASCII diagram block in the docs file by
adding a language specifier ``text`` to the opening code fence so markdownlint
MD040 is satisfied; locate the ASCII diagram block in the architecture.md
content and change the opening "```" to "```text" (leave the closing "```"
unchanged) to preserve rendering while fixing the lint error.
In
`@clients/web/apps/docs/src/content/docs/es/guides/architecture/diagrams/component/agent-core-kmp.mmd`:
- Around line 16-37: Translate the node labels inside the Mermaid diagram to
Spanish to match the subgraph titles: update ENTITIES, VALUE_OBJECTS,
DOMAIN_EVENTS, TASK_MGMT, AGENT_ORCH, KNOWLEDGE, REPOS, SERVICES, REPO_IMPL,
SERVICE_IMPL, and DTOS labels (currently English like "Entities", "Value
Objects", "Task Management", "Repository Implementations", etc.) to their
Spanish equivalents (e.g., "Entidades", "Objetos de Valor", "Eventos de
Dominio", "Gestión de Tareas", "Orquestación de Agentes", "Recuperación de
Conocimiento", "Interfaces de Repositorio", "Interfaces de Servicio",
"Implementaciones de Repositorio", "Implementaciones de Servicio", "DTOs /
Modelos" or similar) so the node text matches the Spanish subgraph titles.
In
`@clients/web/apps/docs/src/content/docs/es/guides/architecture/diagrams/container/system-containers.mmd`:
- Line 64: The classDef line currently uses an invalid property "shape:cylinder"
(see classDef database), so remove "shape:cylinder" from the classDef and
instead define the database node using Mermaid's cylinder node syntax in the
node declaration (e.g., use the [("label")] form for the database node name you
want to style) while keeping the remaining CSS-like styles in classDef database;
update the node line that references the database class to use the cylinder node
syntax and assign it the class.
- Around line 9-10: The two Mermaid actor nodes use the same implicit ID because
both lines use the actor(...) shorthand, causing one to overwrite the other;
update the declarations to give each actor a unique explicit ID (e.g., actor
DeveloperActor as "Developer" and actor EndUserActor as "EndUser", or use the
explicit node syntax with unique IDs) so that the nodes `actor(Developer)` and
`actor(EndUser)` no longer collide and both render correctly.
In
`@clients/web/apps/docs/src/content/docs/es/guides/architecture/diagrams/container/system-containers.puml`:
- Line 6: The PlantUML include currently references the remote C4_Container.puml
via the master branch URL (!include
https://raw.githubusercontent.com/plantuml-stdlib/C4-PlantUML/master/C4_Container.puml),
which is brittle; update that include to point to a specific stable release tag
(e.g., replace "master" with a release like "v2.13.0") so the file referenced by
the include remains reproducible and won't break if the default branch name or
trunk changes.
In
`@clients/web/apps/docs/src/content/docs/es/guides/architecture/diagrams/context/system-context.mmd`:
- Around line 1-6: The Spanish diagram file contains an extra metadata line "#
Git SHA: (current)" that is not present in the English counterpart; either
remove that metadata line from system-context.mmd in the Spanish locale or add
the same "# Git SHA: (current)" metadata line to the English diagram file so
both locales are consistent. Locate the line "# Git SHA: (current)" in
system-context.mmd and apply one of the two changes, then run a quick locale
diff to confirm parity.
- Around line 10-11: The diagram uses the generic node id "actor" twice
(actor(Developer) and actor(EndUser)), causing ID collisions and leaving
references to Developer/EndUser undefined; update those actor declarations to
explicitly set unique IDs that match the later references (e.g., change
actor(Developer):::person to "actor Developer as \"Developer\":::person" or the
equivalent mermaid form that yields id Developer, and similarly change
actor(EndUser):::person to set id EndUser) so the later references to Developer
and EndUser resolve correctly.
In
`@clients/web/apps/docs/src/content/docs/es/guides/architecture/diagrams/module-dependencies.mmd`:
- Line 12: Rename one of the conflicting Mermaid IDs so they are unique: change
either the node ID "EXTERNAL" (the EXTERNAL["Externo"] node) or the subgraph
named "EXTERNAL" to a distinct identifier (e.g., "EXTERNAL_LEGEND" or
"EXTERNAL_SUBGRAPH"), and update any references/edges and class/stylesheet
mentions (such as :::external or any links to EXTERNAL) to the new name to keep
all IDs consistent.
In `@clients/web/build.gradle.kts`:
- Around line 11-35: The log messages and comments around the symlink setup
(variables rootProjectDir, docsSymlink, docsTarget and the
Files.createSymbolicLink block) are in Spanish; change all comments and logger
messages (logger.lifecycle, logger.warn) to English to match the rest of the
build file (e.g., "Create symlink: docs →
clients/web/apps/docs/src/content/docs", "Could not create docs symlink:
${e.message}", "Creating docs directory as fallback...", "Target does not exist:
$docsTarget", "Symlink docs already exists → $target", "Docs directory already
exists (not a symlink)"). Ensure the code logic and message content remain
identical aside from language so only message strings and comments are updated.
- Around line 18-26: The catch block for Files.createSymbolicLink is too generic
(causing Detekt TooGenericExceptionCaught); replace the broad catch(Exception)
with explicit catches for the exceptions that createSymbolicLink can throw
(e.g., java.io.IOException, java.nio.file.FileAlreadyExistsException,
java.lang.UnsupportedOperationException, java.lang.SecurityException) around the
Files.createSymbolicLink(docsSymlink, docsTarget) call, handling each case
similarly to the current fallback (logger.warn + create fallback
docsSymlink.toFile().mkdirs()) or consolidating the specific exceptions into a
multi-catch to satisfy Detekt while preserving the existing logging and fallback
behavior.
- Around line 16-29: The current check uses Files.exists(docsSymlink) which
follows symlinks and misses broken symlinks, causing
Files.createSymbolicLink(docsSymlink, docsTarget) to throw if a broken link
entry already exists; update the logic around docsSymlink/docsTarget to first
check Files.exists(docsSymlink, LinkOption.NOFOLLOW_LINKS) or
Files.isSymbolicLink(docsSymlink) to detect an existing symlink, and if a
symlink exists but points to a missing target remove the broken symlink (or skip
creation) before calling Files.createSymbolicLink; ensure all references to
docsSymlink and docsTarget are adjusted to use LinkOption.NOFOLLOW_LINKS where
appropriate and keep the existing fallback behavior if creation still fails.
| # Astro cache/build directories | ||
| **/.astro/ | ||
| clients/web/apps/**/dist/ | ||
| clients/web/apps/**/node_modules/ |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant pattern: already covered by **/node_modules/ on line 92.
The global **/node_modules/ pattern already matches all node_modules/ directories at any depth, making this more specific pattern unnecessary.
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In @.gitignore at line 147, Remove the redundant gitignore entry
"clients/web/apps/**/node_modules/" because it is already covered by the broader
"**/node_modules/" pattern; delete the specific pattern line and keep only the
global "**/node_modules/" entry to avoid duplication.
| clients/web/apps/**/dist/ | ||
| clients/web/apps/**/node_modules/ | ||
|
|
||
| # Symlinks auto-generados por Gradle |
Contributor
There was a problem hiding this comment.
Inconsistent comment language.
This comment is in Spanish while the rest of the file uses English. Consider aligning for consistency:
Proposed fix
-# Symlinks auto-generados por Gradle
+# Symlinks auto-generated by Gradle📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Symlinks auto-generados por Gradle | |
| # Symlinks auto-generated by Gradle |
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In @.gitignore at line 149, The comment "# Symlinks auto-generados por Gradle"
in the .gitignore is in Spanish while the rest of the file is English; update
that comment to English for consistency (e.g., change the line containing
"Symlinks auto-generados por Gradle" to "Gradle auto-generated symlinks" or
similar), preserving the comment marker and placement.
Comment on lines
+1085
to
1105
| if should_fallback_to_in_place_write(&e) { | ||
| if let Err(write_err) = | ||
| write_config_in_place(&self.config_path, toml_str.as_bytes()) | ||
| { | ||
| let _ = fs::remove_file(&temp_path); | ||
| if had_existing_config && backup_path.exists() { | ||
| let _ = fs::copy(&backup_path, &self.config_path); | ||
| } | ||
| anyhow::bail!( | ||
| "Failed to replace config file after atomic fallback: {write_err} \ | ||
| (rename error: {e})" | ||
| ); | ||
| } | ||
| let _ = fs::remove_file(&temp_path); | ||
| } else { | ||
| let _ = fs::remove_file(&temp_path); | ||
| if had_existing_config && backup_path.exists() { | ||
| let _ = fs::copy(&backup_path, &self.config_path); | ||
| } | ||
| anyhow::bail!("Failed to atomically replace config file: {e}"); | ||
| } |
Contributor
There was a problem hiding this comment.
Fallback logic is sound, but note the non-atomic window for in-place writes.
When the in-place fallback path is taken, the target file is opened with truncate(true) before the new contents are written. If the process is killed between truncation and the completed write_all + sync_all, the config file will be left empty or partially written. The backup file (.bak) does survive past this point (it's only cleaned up at Line 1111), so manual recovery is possible — but there's no automatic recovery on next startup.
Consider documenting this risk or adding a brief note in load_or_init to check for a .bak file and offer recovery if the config is empty/corrupt.
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@clients/agent-runtime/src/config/schema.rs` around lines 1085 - 1105, The
fallback in write_config_in_place can leave the target file truncated if the
process dies mid-write; update load_or_init to detect and recover from this by
checking after open/read whether the current config file is empty or fails to
deserialize and, if so, look for the existing backup (backup_path/.bak) and
atomically restore it before proceeding (or return a clear descriptive error if
restore fails); reference load_or_init, write_config_in_place, self.config_path,
and backup_path in your changes and ensure the backup is restored prior to any
further config parsing so startup can automatically recover from a truncated
config file.
Comment on lines
+1151
to
+1155
| #[cfg(unix)] | ||
| fn should_fallback_to_in_place_write(error: &std::io::Error) -> bool { | ||
| // Bind-mounted files can reject replace-style renames with EBUSY/EXDEV. | ||
| matches!(error.raw_os_error(), Some(16 | 18)) | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
std::io::ErrorKind::ResourceBusy and CrossesDevices Rust stabilization version
💡 Result:
std::io::ErrorKind::ResourceBusywas stabilized in Rust 1.83.0. [2][1]std::io::ErrorKind::CrossesDeviceswas stabilized in Rust 1.85.0. [1]
Sources:
[1] Rust standard library docs: std::io::ErrorKind (shows per-variant stabilization versions) https://doc.rust-lang.org/std/io/enum.ErrorKind.html
[2] Rust 1.83.0 release post (lists ErrorKind::ResourceBusy as stabilized) https://blog.rust-lang.org/2024/11/28/Rust-1.83.0/
🏁 Script executed:
# Check if libc is already a dependency
find . -name "Cargo.toml" -type f -exec grep -l "libc" {} \;Repository: dallay/corvus
Length of output: 39
🏁 Script executed:
# Check the project's Rust MSRV/version requirements in Cargo.toml
fd "Cargo.toml" --maxdepth 3 | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && head -20 "{}"'Repository: dallay/corvus
Length of output: 812
🏁 Script executed:
# Verify errno values - check if there's any documentation in the codebase about this
rg "EBUSY|EXDEV" --type rust -A 3 -B 3Repository: dallay/corvus
Length of output: 572
🏁 Script executed:
# Check for rust-version (MSRV) in Cargo.toml
rg "rust-version|edition" clients/agent-runtime/Cargo.tomlRepository: dallay/corvus
Length of output: 73
🏁 Script executed:
# Also check root Cargo.toml for workspace-level MSRV
rg "rust-version|edition" Cargo.tomlRepository: dallay/corvus
Length of output: 107
Use named constants instead of magic errno numbers.
Raw 16 and 18 are less readable and potentially fragile. While these values are consistent across Linux and macOS, using named constants is more idiomatic and self-documenting.
Options:
- Use
libc::EBUSYandlibc::EXDEV(addlibcas a dependency if not already present). - Use
std::io::ErrorKind::ResourceBusyandstd::io::ErrorKind::CrossesDevices(stabilized in Rust 1.83.0 and 1.85.0 respectively—only feasible if project's MSRV supports this). - At minimum, define local constants (no new dependency).
♻️ Option 3 — local constants (no new dependency)
#[cfg(unix)]
fn should_fallback_to_in_place_write(error: &std::io::Error) -> bool {
- // Bind-mounted files can reject replace-style renames with EBUSY/EXDEV.
- matches!(error.raw_os_error(), Some(16 | 18))
+ // Bind-mounted files can reject replace-style renames with EBUSY/EXDEV.
+ const EBUSY: i32 = 16;
+ const EXDEV: i32 = 18;
+ matches!(error.raw_os_error(), Some(EBUSY | EXDEV))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[cfg(unix)] | |
| fn should_fallback_to_in_place_write(error: &std::io::Error) -> bool { | |
| // Bind-mounted files can reject replace-style renames with EBUSY/EXDEV. | |
| matches!(error.raw_os_error(), Some(16 | 18)) | |
| } | |
| #[cfg(unix)] | |
| fn should_fallback_to_in_place_write(error: &std::io::Error) -> bool { | |
| // Bind-mounted files can reject replace-style renames with EBUSY/EXDEV. | |
| const EBUSY: i32 = 16; | |
| const EXDEV: i32 = 18; | |
| matches!(error.raw_os_error(), Some(EBUSY | EXDEV)) | |
| } |
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@clients/agent-runtime/src/config/schema.rs` around lines 1151 - 1155, Replace
the magic errno integers in should_fallback_to_in_place_write with named
constants: update the matches! check to compare against descriptive constants
rather than raw 16 and 18. You can either reference libc::EBUSY and libc::EXDEV
(add libc dependency), use std::io::ErrorKind variants if MSRV permits, or
define local const EBUSY and EXDEV integers at the top of the module and use
those; ensure the function still matches error.raw_os_error() against Some(EBUSY
| EXDEV) equivalents and keep the #[cfg(unix)] attribute on
should_fallback_to_in_place_write.
Comment on lines
+40
to
+43
|
|
||
| # Astro | ||
| **/.astro/ | ||
| .astro/ |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant pattern: .astro/ is already covered by **/.astro/.
The glob **/.astro/ matches .astro/ at any depth, including the current directory root. Line 43 is redundant.
Proposed fix
# Astro
**/.astro/
-.astro/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # Astro | |
| **/.astro/ | |
| .astro/ | |
| # Astro | |
| **/.astro/ |
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@clients/web/.gitignore` around lines 40 - 43, The .gitignore contains a
redundant pattern: remove the duplicate `.astro/` entry so only the recursive
pattern `**/.astro/` remains (keep `**/.astro/` and delete the separate
`.astro/` line) to avoid duplication while preserving the same ignore behavior.
Comment on lines
+10
to
+11
| actor(Developer):::person | ||
| actor(EndUser):::person |
Contributor
There was a problem hiding this comment.
Same node ID collision bug as the English version.
Lines 10–11 both define node id actor, and lines 27–28 reference Developer/EndUser as IDs that were never properly defined. Apply the same fix as described for the English version.
Proposed fix
- actor(Developer):::person
- actor(EndUser):::person
+ Developer(["Desarrollador"]):::person
+ EndUser(["Usuario Final"]):::person📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| actor(Developer):::person | |
| actor(EndUser):::person | |
| Developer(["Desarrollador"]):::person | |
| EndUser(["Usuario Final"]):::person |
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In
`@clients/web/apps/docs/src/content/docs/es/guides/architecture/diagrams/context/system-context.mmd`
around lines 10 - 11, The diagram uses the generic node id "actor" twice
(actor(Developer) and actor(EndUser)), causing ID collisions and leaving
references to Developer/EndUser undefined; update those actor declarations to
explicitly set unique IDs that match the later references (e.g., change
actor(Developer):::person to "actor Developer as \"Developer\":::person" or the
equivalent mermaid form that yields id Developer, and similarly change
actor(EndUser):::person to set id EndUser) so the later references to Developer
and EndUser resolve correctly.
| direction LR | ||
| CLIENT["Cliente"]:::client | ||
| CORE["Core Compartido"]:::core | ||
| EXTERNAL["Externo"]:::external |
Contributor
There was a problem hiding this comment.
Mermaid ID collision: EXTERNAL is used for both a node (Line 12) and a subgraph (Line 32).
Mermaid requires unique IDs across all nodes and subgraphs. This will cause rendering issues — the legend node will likely be absorbed into the subgraph or the diagram will fail to render. Rename one of them.
Proposed fix
- EXTERNAL["Externo"]:::external
+ EXT_LEGEND["Externo"]:::externalAlso applies to: 32-39
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In
`@clients/web/apps/docs/src/content/docs/es/guides/architecture/diagrams/module-dependencies.mmd`
at line 12, Rename one of the conflicting Mermaid IDs so they are unique: change
either the node ID "EXTERNAL" (the EXTERNAL["Externo"] node) or the subgraph
named "EXTERNAL" to a distinct identifier (e.g., "EXTERNAL_LEGEND" or
"EXTERNAL_SUBGRAPH"), and update any references/edges and class/stylesheet
mentions (such as :::external or any links to EXTERNAL) to the new name to keep
all IDs consistent.
Comment on lines
+11
to
+35
| // Verificar y crear symlink de docs si no existe | ||
| val rootProjectDir = isolated.rootProject.projectDirectory.asFile.toPath() | ||
| val docsSymlink = rootProjectDir.resolve("docs") | ||
| val docsTarget = rootProjectDir.resolve("clients/web/apps/docs/src/content/docs") | ||
|
|
||
| if (!Files.exists(docsSymlink)) { | ||
| if (Files.exists(docsTarget)) { | ||
| try { | ||
| Files.createSymbolicLink(docsSymlink, docsTarget) | ||
| logger.lifecycle("🔗 Symlink creado: docs → clients/web/apps/docs/src/content/docs") | ||
| } catch (e: Exception) { | ||
| logger.warn("⚠️ No se pudo crear symlink de docs: ${e.message}") | ||
| // Fallback: crear directorio vacío o copiar contenido | ||
| logger.lifecycle("📁 Creando directorio docs como fallback...") | ||
| docsSymlink.toFile().mkdirs() | ||
| } | ||
| } else { | ||
| logger.warn("⚠️ Target no existe: $docsTarget") | ||
| } | ||
| } else if (Files.isSymbolicLink(docsSymlink)) { | ||
| val target = Files.readSymbolicLink(docsSymlink) | ||
| logger.lifecycle("✅ Symlink docs ya existe → $target") | ||
| } else { | ||
| logger.lifecycle("📁 Directorio docs ya existe (no es symlink)") | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Log messages and comments are in Spanish; rest of the build file is in English.
Mixing languages in the same file hurts readability for contributors who don't speak Spanish. Consider using English consistently for code comments and log messages in build scripts, reserving Spanish for user-facing documentation.
🧰 Tools
🪛 GitHub Actions: Scan with Detekt
[error] 21-21: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled. [TooGenericExceptionCaught]
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@clients/web/build.gradle.kts` around lines 11 - 35, The log messages and
comments around the symlink setup (variables rootProjectDir, docsSymlink,
docsTarget and the Files.createSymbolicLink block) are in Spanish; change all
comments and logger messages (logger.lifecycle, logger.warn) to English to match
the rest of the build file (e.g., "Create symlink: docs →
clients/web/apps/docs/src/content/docs", "Could not create docs symlink:
${e.message}", "Creating docs directory as fallback...", "Target does not exist:
$docsTarget", "Symlink docs already exists → $target", "Docs directory already
exists (not a symlink)"). Ensure the code logic and message content remain
identical aside from language so only message strings and comments are updated.
Comment on lines
+16
to
+29
| if (!Files.exists(docsSymlink)) { | ||
| if (Files.exists(docsTarget)) { | ||
| try { | ||
| Files.createSymbolicLink(docsSymlink, docsTarget) | ||
| logger.lifecycle("🔗 Symlink creado: docs → clients/web/apps/docs/src/content/docs") | ||
| } catch (e: Exception) { | ||
| logger.warn("⚠️ No se pudo crear symlink de docs: ${e.message}") | ||
| // Fallback: crear directorio vacío o copiar contenido | ||
| logger.lifecycle("📁 Creando directorio docs como fallback...") | ||
| docsSymlink.toFile().mkdirs() | ||
| } | ||
| } else { | ||
| logger.warn("⚠️ Target no existe: $docsTarget") | ||
| } |
Contributor
There was a problem hiding this comment.
Broken symlink causes silent failure.
Files.exists(docsSymlink) follows symlinks by default — if a broken symlink already exists at that path, it returns false, but Files.createSymbolicLink on Line 19 will throw because the filesystem entry already exists. Guard against this by checking with LinkOption.NOFOLLOW_LINKS or by checking Files.isSymbolicLink first.
Proposed fix
-if (!Files.exists(docsSymlink)) {
+if (!Files.exists(docsSymlink, java.nio.file.LinkOption.NOFOLLOW_LINKS)) {🧰 Tools
🪛 GitHub Actions: Scan with Detekt
[error] 21-21: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled. [TooGenericExceptionCaught]
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@clients/web/build.gradle.kts` around lines 16 - 29, The current check uses
Files.exists(docsSymlink) which follows symlinks and misses broken symlinks,
causing Files.createSymbolicLink(docsSymlink, docsTarget) to throw if a broken
link entry already exists; update the logic around docsSymlink/docsTarget to
first check Files.exists(docsSymlink, LinkOption.NOFOLLOW_LINKS) or
Files.isSymbolicLink(docsSymlink) to detect an existing symlink, and if a
symlink exists but points to a missing target remove the broken symlink (or skip
creation) before calling Files.createSymbolicLink; ensure all references to
docsSymlink and docsTarget are adjusted to use LinkOption.NOFOLLOW_LINKS where
appropriate and keep the existing fallback behavior if creation still fails.
Comment on lines
+18
to
+26
| try { | ||
| Files.createSymbolicLink(docsSymlink, docsTarget) | ||
| logger.lifecycle("🔗 Symlink creado: docs → clients/web/apps/docs/src/content/docs") | ||
| } catch (e: Exception) { | ||
| logger.warn("⚠️ No se pudo crear symlink de docs: ${e.message}") | ||
| // Fallback: crear directorio vacío o copiar contenido | ||
| logger.lifecycle("📁 Creando directorio docs como fallback...") | ||
| docsSymlink.toFile().mkdirs() | ||
| } |
Contributor
There was a problem hiding this comment.
Catch specific exceptions to fix the Detekt pipeline failure.
The pipeline reports TooGenericExceptionCaught on Line 21. Files.createSymbolicLink throws IOException, UnsupportedOperationException, SecurityException, or FileAlreadyExistsException — catch those explicitly.
Proposed fix
- } catch (e: Exception) {
+ } catch (e: java.io.IOException) {
+ logger.warn("⚠️ Could not create docs symlink: ${e.message}")
+ logger.lifecycle("📁 Creating docs directory as fallback...")
+ docsSymlink.toFile().mkdirs()
+ } catch (e: UnsupportedOperationException) {🧰 Tools
🪛 GitHub Actions: Scan with Detekt
[error] 21-21: The caught exception is too generic. Prefer catching specific exceptions to the case that is currently handled. [TooGenericExceptionCaught]
🤖 Prompt for AI Agents
Before applying any fix, first verify the finding against the current code and
decide whether a code change is actually needed. If the finding is not valid or
no change is required, do not modify code for that item and briefly explain why
it was skipped.
In `@clients/web/build.gradle.kts` around lines 18 - 26, The catch block for
Files.createSymbolicLink is too generic (causing Detekt
TooGenericExceptionCaught); replace the broad catch(Exception) with explicit
catches for the exceptions that createSymbolicLink can throw (e.g.,
java.io.IOException, java.nio.file.FileAlreadyExistsException,
java.lang.UnsupportedOperationException, java.lang.SecurityException) around the
Files.createSymbolicLink(docsSymlink, docsTarget) call, handling each case
similarly to the current fallback (logger.warn + create fallback
docsSymlink.toFile().mkdirs()) or consolidating the specific exceptions into a
multi-catch to satisfy Detekt while preserving the existing logging and fallback
behavior.
This was referenced Feb 18, 2026
Merged
This was referenced Feb 25, 2026
This was referenced Mar 5, 2026
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
1 participant
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.
This pull request contains several changes across the iOS, Rust backend, Kotlin Compose, and web documentation projects. The most significant updates include improvements to atomic config file replacement in the Rust backend, code style cleanups in the Kotlin Compose chat UI, a visual update to the onboarding banner, and enhancements to the documentation structure for the web client. Additionally, there are removals of iOS Xcode project files, likely as part of a migration or cleanup.
Rust Backend Improvements:
schema.rs: Now, if the atomic rename fails due to certain OS errors (e.g., EBUSY or EXDEV), the code falls back to an in-place file write, increasing robustness when dealing with bind-mounted files. Includes new helper functions and tests for this behavior. [1] [2] [3]wizard.rsto a more concise and visually appealing ASCII art.Kotlin Compose UI Cleanups:
ChatWorkspace.ktfile for improved code readability by collapsing multi-line expressions into single lines and simplifying function calls, without changing functionality. [1] [2] [3] [4] [5] [6]Web Documentation Enhancements:
.gitignoreto include Astro build artifacts, preventing unnecessary files from being tracked.iOS Project Cleanup:
iosApp.xcodeproj/project.pbxprojandiosApp.xcodeproj/project.xcworkspace/contents.xcworkspacedata, possibly due to project restructuring or migration. [1] [2]