feat: add basic chat UI with vue.js#24
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
Thank you for contributing to this project with this PR, welcome to the community and the amazing world of open source! |
Contributor
📝 WalkthroughWalkthroughThis PR adds internationalization support to the Vue 3 dashboard via vue-i18n with Spanish localization, implements testing infrastructure with Vitest and happy-dom, enhances gateway configuration with persistent state and unique message IDs, and refactors component internal bindings for consistency. Documentation and dependencies are updated accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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-17 to 2026-02-17 |
![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: 5
🧹 Nitpick comments (8)
clients/web/apps/dashboard/src/i18n.ts (1)
8-8:fallbackLocale: "es"is redundant whenlocaleis already"es".vue-i18n will look up the same
esmessages for both the primary and fallback locale, so a missing key still falls through to the raw key string — there is no real fallback benefit. This is fine for a single-locale app, but the field can be dropped until a second locale (e.g.,"en") is added as a true fallback.♻️ Suggested simplification
export const i18n = createI18n({ legacy: false, locale: "es", - fallbackLocale: "es", messages: { es, }, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/i18n.ts` at line 8, Remove the redundant fallbackLocale property from the i18n configuration since locale is already set to "es"; update the i18n setup by deleting the fallbackLocale: "es" entry so Vue I18n uses the declared locale only (keep the existing locale: "es" and messages unchanged) and re-run tests to confirm no behavior change until a second locale (e.g., "en") is added.clients/web/apps/dashboard/src/lib/utils.ts (1)
8-15: Remove unnecessaryescapeHtmlcall—it's not needed for the translation string context and adds no security benefit when rendered via{{ }}.
escapeHtmlis called on user input at line 88 but the result is embedded in a translation string (chat.processing), not rendered directly. Since the final assistant message is rendered via Vue's{{ content }}interpolation (line 20 of ChatMessage.vue), which is already XSS-safe, the escaping is redundant. Additionally, theescapeHtmlfunction itself is incomplete—it doesn't cover all attack vectors (backticks, forward-slashes in attribute contexts). If HTML rendering is introduced in the future viav-html, replace this manual implementation with a proper library like DOMPurify. For now, remove theescapeHtmlcall entirely since it provides no security benefit in the current Vue template context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/lib/utils.ts` around lines 8 - 15, The escapeHtml function is redundant and incomplete for this translation use: remove the call to escapeHtml where the user input is passed into the translation string (the place that builds the chat.processing message) so the raw value is rendered via Vue's safe {{ }} interpolation in ChatMessage.vue; leave the escapeHtml function unused or delete it entirely, and if you later introduce v-html or raw HTML rendering, replace this manual implemention with a proper sanitizer such as DOMPurify instead of reusing escapeHtml.clients/web/apps/dashboard/vite.config.ts (2)
15-15:globals: trueis unused — tests import vitest APIs explicitly.
App.spec.tsimportsdescribe,expect, anditdirectly from"vitest", so theglobalsflag has no effect. Either remove it, or drop the explicit imports in the test file and rely on globals consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/vite.config.ts` at line 15, The vitest config entry globals: true is redundant because your tests (App.spec.ts) import describe/expect/it directly; remove the globals: true setting from the vite config (vite.config.ts) to avoid dead config, or alternatively change the test file (App.spec.ts) to rely on global APIs (remove explicit imports of describe/expect/it) so the globals flag is actually used; pick one approach and keep config and tests consistent.
1-4: Prefer importingdefineConfigfromvitest/config.Importing from
viterelies on Vitest's module augmentation to type thetestblock. Usingvitest/configdirectly is the canonical approach recommended by the Vitest team and makes the intent explicit.♻️ Proposed change
-import { defineConfig } from "vite"; +import { defineConfig } from "vitest/config";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/vite.config.ts` around lines 1 - 4, Update the import of defineConfig so it comes from 'vitest/config' instead of 'vite': replace the current "import { defineConfig } from 'vite'" with an import from 'vitest/config' to make the test block typing explicit; keep other imports (fileURLToPath, URL, vue) unchanged and ensure any usage of defineConfig in the vite.config.ts remains the same (e.g., the exported defineConfig(...) call).clients/web/apps/dashboard/src/App.spec.ts (2)
43-51:wrapper.get("button")is fragile when the config panel is open.When
showConfigistrue, the DOM contains at least two<button>elements: the header toggle and the form's save button.get("button")returns the first match, which happens to be the header button today, but any template reorder would silently break the toggle-back assertion on line 50. Prefer adata-testidselector.♻️ Proposed fix
In
App.vuetemplate, adddata-testid="toggle-config"to the headerButton:- <Button variant="ghost" size="sm" `@click`="showConfig = !showConfig"> + <Button variant="ghost" size="sm" data-testid="toggle-config" `@click`="showConfig = !showConfig">Then in the test:
- await wrapper.get("button").trigger("click"); + await wrapper.get('[data-testid="toggle-config"]').trigger("click");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.spec.ts` around lines 43 - 51, The test is using wrapper.get("button") which is fragile because multiple buttons exist when showConfig is true; update the App.vue header Button to include data-testid="toggle-config" and then change the test to select the toggle via wrapper.get('[data-testid="toggle-config"]') instead of wrapper.get("button") so the click targets the header toggle reliably (refer to App.vue header Button and the spec lines using wrapper.get("button")).
8-21:mountApp()duplicates the i18n configuration fromsrc/i18n.ts.Consider re-exporting the config options (locale, messages, etc.) from
i18n.tsso the test factory can reuse them. This prevents test setup drifting from the production i18n config.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.spec.ts` around lines 8 - 21, The test duplicates the i18n setup in mountApp; instead export the shared i18n config (e.g., locale, fallbackLocale, messages or a ready-made i18n instance) from src/i18n.ts and import it into App.spec.ts. Update mountApp to import the exported config or i18n instance and use it with createI18n (or directly pass the exported i18n) instead of hardcoding locale/messages in the test, ensuring App.spec.ts uses the same configuration as production.clients/web/apps/dashboard/package.json (1)
34-35: Ensure the runtime environment meets Vite 7's Node.js minimum.Vite 7 requires Node.js 20.19+ or 22.12+; Node.js 18 support was dropped following its EOL in April 2025. Confirm CI and production environments satisfy this constraint before merging.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/package.json` around lines 34 - 35, The project uses "vite" ^7.1.10 which requires Node.js >=20.19 or >=22.12; update the runtime requirements by adding or updating the package.json "engines" field to enforce "node": ">=20.19" (or ">=22.12") and update any CI configs (the Node version used in your test/build workflows) to use a supported Node version (e.g., 20.19+ or 22.12+ in the node-version matrix), and update documentation/README to note the minimum Node requirement so CI and production match Vite 7's Node.js minimum.clients/web/apps/dashboard/src/App.vue (1)
106-120:void { ... }is an unusual Biome workaround — prefer explicit suppression.This pattern tricks the JS engine into "using" all top-level
<script setup>bindings to silence Biome's "no-unused-vars" rule, which doesn't understand Vue's template scope. This can confuse readers and obscure actual dead-code warnings. Prefer targeted Biome suppression comments on any genuinely flagged binding:-void { - ChatMessage, - Button, - ... -}; +// biome-ignore lint/correctness/noUnusedVariables: used in <template>Or configure
biome.jsonto exclude.vuescript-setup blocks from unused-variable checks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/dashboard/src/App.vue` around lines 106 - 120, Remove the odd `void { ... }` block and either add explicit Biome suppression comments for the specific unused bindings (e.g., place `/* biome-ignore no-unused-vars */` immediately above any of the top-level bindings such as ChatMessage, Button, Input, showConfig, prompt, baseUrl, chatContainer, secretInputNonce, messages, canSend, captureSecretInput, saveGatewayConfig, sendMessage) or update `biome.json` to exclude Vue script-setup blocks; keep the suppression targeted to the specific symbols rather than using the `void` trick so readers and linters understand why those bindings are intentionally unused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/web/apps/dashboard/src/App.spec.ts`:
- Around line 33-35: The test currently relies on ChatMessage rendering content
as plain text (escaped entities) but doesn't assert that contract explicitly;
update the test in App.spec.ts to make that explicit by adding an assertion
and/or comment: after creating chatMessages (the same variable used in the
failing assertions), assert the rendered node's innerHTML (or html()) contains
the escaped entity "<script>alert" (or assert there is no <script> element
present) to lock in the expectation that ChatMessage uses text interpolation
rather than v-html, and add a brief comment mentioning this rendering contract;
reference ChatMessage, chatMessages, and the existing expects so the assertion
and comment are adjacent to the current checks.
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 63-72: saveGatewayConfig currently computes hasSecretPayload then
discards it (void hasSecretPayload) and only clears pairingCodeInput,
bearerTokenInput, webhookSecretInput and increments secretInputNonce without
persisting anything; remove the no-op and instead call the backend API to
persist the gateway config (include the secret fields and any necessary gateway
id) from within saveGatewayConfig, await the response, handle errors (log/show
user-facing error) and only clear the input fields and bump secretInputNonce on
success; use the existing reactive symbols pairingCodeInput, bearerTokenInput,
webhookSecretInput, and secretInputNonce when constructing the payload, and add
success feedback (toast or status flag) and proper try/catch around the network
call.
- Around line 88-98: The assistant message is double-HTML-escaped because we
pre-escape the text with escapeHtml and Vue then escapes it again; remove the
redundant escape by deleting the escapeHtml call and stop using escapedText –
use normalizedText for both pushes into messages.value and pass normalizedText
into the i18n call t("chat.processing", { text: normalizedText, modelName,
gateway: baseUrl.value }) so ChatMessage.vue (which relies on Vue's
interpolation) renders the raw text correctly; update any references to
escapedText in this block accordingly.
In `@clients/web/apps/dashboard/src/locales/es.json`:
- Line 4: Update the Spanish locale file to correct missing diacritics: replace
"Configuracion del gateway" (value for "gatewayConfig") with "Configuración del
gateway", update both occurrences of "Codigo de emparejamiento" to "Código de
emparejamiento", change "Guardar configuracion" to "Guardar configuración", and
replace "En que puedo ayudarte?" with "¿En qué puedo ayudarte?"; edit the values
in clients/web/apps/dashboard/src/locales/es.json (locate the keys by their
current string values or the "gatewayConfig" key) and ensure the corrected UTF-8
strings are saved.
In `@clients/web/package.json`:
- Around line 25-28: The package.json currently pins packageManager to
"pnpm@10.30.0" but leaves engines.pnpm as ">=9.0.0", which allows incompatible
pnpm 9 installs; update the engines.pnpm entry to align with the pinned
packageManager (e.g., set engines.pnpm to ">=10.30.0" or at least ">=10.0.0") so
the engines constraint matches the packageManager major/minor version; modify
the engines.pnpm field in package.json (referencing packageManager and
engines.pnpm) accordingly.
---
Nitpick comments:
In `@clients/web/apps/dashboard/package.json`:
- Around line 34-35: The project uses "vite" ^7.1.10 which requires Node.js
>=20.19 or >=22.12; update the runtime requirements by adding or updating the
package.json "engines" field to enforce "node": ">=20.19" (or ">=22.12") and
update any CI configs (the Node version used in your test/build workflows) to
use a supported Node version (e.g., 20.19+ or 22.12+ in the node-version
matrix), and update documentation/README to note the minimum Node requirement so
CI and production match Vite 7's Node.js minimum.
In `@clients/web/apps/dashboard/src/App.spec.ts`:
- Around line 43-51: The test is using wrapper.get("button") which is fragile
because multiple buttons exist when showConfig is true; update the App.vue
header Button to include data-testid="toggle-config" and then change the test to
select the toggle via wrapper.get('[data-testid="toggle-config"]') instead of
wrapper.get("button") so the click targets the header toggle reliably (refer to
App.vue header Button and the spec lines using wrapper.get("button")).
- Around line 8-21: The test duplicates the i18n setup in mountApp; instead
export the shared i18n config (e.g., locale, fallbackLocale, messages or a
ready-made i18n instance) from src/i18n.ts and import it into App.spec.ts.
Update mountApp to import the exported config or i18n instance and use it with
createI18n (or directly pass the exported i18n) instead of hardcoding
locale/messages in the test, ensuring App.spec.ts uses the same configuration as
production.
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 106-120: Remove the odd `void { ... }` block and either add
explicit Biome suppression comments for the specific unused bindings (e.g.,
place `/* biome-ignore no-unused-vars */` immediately above any of the top-level
bindings such as ChatMessage, Button, Input, showConfig, prompt, baseUrl,
chatContainer, secretInputNonce, messages, canSend, captureSecretInput,
saveGatewayConfig, sendMessage) or update `biome.json` to exclude Vue
script-setup blocks; keep the suppression targeted to the specific symbols
rather than using the `void` trick so readers and linters understand why those
bindings are intentionally unused.
In `@clients/web/apps/dashboard/src/i18n.ts`:
- Line 8: Remove the redundant fallbackLocale property from the i18n
configuration since locale is already set to "es"; update the i18n setup by
deleting the fallbackLocale: "es" entry so Vue I18n uses the declared locale
only (keep the existing locale: "es" and messages unchanged) and re-run tests to
confirm no behavior change until a second locale (e.g., "en") is added.
In `@clients/web/apps/dashboard/src/lib/utils.ts`:
- Around line 8-15: The escapeHtml function is redundant and incomplete for this
translation use: remove the call to escapeHtml where the user input is passed
into the translation string (the place that builds the chat.processing message)
so the raw value is rendered via Vue's safe {{ }} interpolation in
ChatMessage.vue; leave the escapeHtml function unused or delete it entirely, and
if you later introduce v-html or raw HTML rendering, replace this manual
implemention with a proper sanitizer such as DOMPurify instead of reusing
escapeHtml.
In `@clients/web/apps/dashboard/vite.config.ts`:
- Line 15: The vitest config entry globals: true is redundant because your tests
(App.spec.ts) import describe/expect/it directly; remove the globals: true
setting from the vite config (vite.config.ts) to avoid dead config, or
alternatively change the test file (App.spec.ts) to rely on global APIs (remove
explicit imports of describe/expect/it) so the globals flag is actually used;
pick one approach and keep config and tests consistent.
- Around line 1-4: Update the import of defineConfig so it comes from
'vitest/config' instead of 'vite': replace the current "import { defineConfig }
from 'vite'" with an import from 'vitest/config' to make the test block typing
explicit; keep other imports (fileURLToPath, URL, vue) unchanged and ensure any
usage of defineConfig in the vite.config.ts remains the same (e.g., the exported
defineConfig(...) call).
Deploying corvus with
|
| Latest commit: |
c13562d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://71d529ba.corvus-42x.pages.dev |
| Branch Preview URL: | https://codex-add-basic-chat-ui-with.corvus-42x.pages.dev |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
clients/web/README.md (1)
67-70: Consider documenting the new test commandThe PR adds a
vitest --runtest script to the dashboard, but no test invocation appears in the Quality block. If a workspace-levelpnpm test(orpnpm test:dashboard) script exists, it would be useful to list it here alongsideformatandcheck.✏️ Suggested addition
# Quality pnpm format pnpm check +pnpm test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/README.md` around lines 67 - 70, The Quality section omits the new test command added in the PR; update clients/web/README.md to include the workspace/dashboard test invocation (e.g., add "pnpm test" or "pnpm test:dashboard" that runs "vitest --run") alongside the existing "pnpm format" and "pnpm check" entries so readers know how to run the dashboard tests locally.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/web/apps/dashboard/src/App.vue`:
- Around line 167-171: The pairing code Input currently renders in clear text;
update the Input component with type="password" to mask the value (same as
bearerToken and webhookSecret). Locate the Input with
:key="`pairing-${secretInputNonce}`" and the `@update`:model-value calling
captureSecretInput('pairingCode', ...), and add the type="password" prop so the
pairingCode is treated as a password field.
- Around line 75-85: The request body uses the original baseUrl.value while the
fetch uses gatewayBaseUrl (which trims a trailing slash), causing inconsistent
persisted values; update the payload to send the normalized URL (gatewayBaseUrl)
instead of baseUrl.value so the backend stores the same slash-trimmed value used
for the request—locate the fetch call that posts to
`${gatewayBaseUrl}/web/dashboard/config` and replace the body field currently
setting baseUrl: baseUrl.value with baseUrl: gatewayBaseUrl (or otherwise
normalize baseUrl.value before building both the URL and the body).
- Around line 96-103: The saveStatus reactive (saveStatus.value) is left as
"success" or "error" after save, so the banner persists; update the save success
path (where saveStatus.value = "success") to start/track a timeout (e.g., 3s)
that resets saveStatus.value = "idle" when fired, and ensure any previous
timeout is cleared before creating a new one (store the timer id in a ref like
saveStatusTimeoutId). Also, in form field change handlers (e.g., where
pairingCodeInput, bearerTokenInput, webhookSecretInput, secretInputNonce are
updated) immediately set saveStatus.value = "idle" to hide stale banners when
the user edits. Ensure the timeout is cleared on component unmount if
applicable.
- Around line 83-90: The request body always includes empty secret strings
(pairingCodeInput, bearerTokenInput, webhookSecretInput) which can silently
clear backend credentials; change the payload construction before JSON.stringify
so you only add those keys when their corresponding values are non-empty (e.g.,
start with { gatewayId: "default", baseUrl: baseUrl.value } and conditionally
set pairingCode, bearerToken, webhookSecret only if pairingCodeInput,
bearerTokenInput, webhookSecretInput are truthy/not empty), then JSON.stringify
that object so empty strings are not sent.
- Around line 77-90: The POST fetch to `${gatewayBaseUrl}/web/dashboard/config`
has no timeout which can leave saveStatus stuck at "saving"; wrap the request in
an AbortController with a setTimeout to call controller.abort() (store the timer
id), pass controller.signal to fetch, clear the timeout on success, and in the
catch/finally for the function set saveStatus to a terminal state (e.g., "error"
or "idle") so the submit button is re-enabled; update the existing try/catch
around the fetch (the block that sets saveStatus and reads response) to treat
AbortError specially for timeout and ensure the timer is cleared and controller
signal is cleaned up.
In `@clients/web/README.md`:
- Line 39: Update the typo in the README by replacing the phrase "Requisitos
minimos:" with the correct Spanish spelling "Requisitos mínimos:"; locate the
line containing the heading "Requisitos minimos" and add the acute accent on the
'i' in "mínimos".
---
Duplicate comments:
In `@clients/web/package.json`:
- Line 25: The package.json had a mismatch between packageManager and
engines.pnpm; ensure engines.pnpm matches packageManager by setting engines.pnpm
to ">=10.30.0" (to align with "packageManager": "pnpm@10.30.0"). Update the
engines.pnpm entry accordingly so both packageManager and engines.pnpm reference
pnpm 10.30.0 consistently, verifying fields packageManager and engines.pnpm in
package.json.
---
Nitpick comments:
In `@clients/web/README.md`:
- Around line 67-70: The Quality section omits the new test command added in the
PR; update clients/web/README.md to include the workspace/dashboard test
invocation (e.g., add "pnpm test" or "pnpm test:dashboard" that runs "vitest
--run") alongside the existing "pnpm format" and "pnpm check" entries so readers
know how to run the dashboard tests locally.
Comment on lines
+75
to
+85
| const gatewayBaseUrl = baseUrl.value.replace(/\/$/, ""); | ||
|
|
||
| try { | ||
| const response = await fetch(`${gatewayBaseUrl}/web/dashboard/config`, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ | ||
| gatewayId: "default", | ||
| baseUrl: baseUrl.value, |
Contributor
There was a problem hiding this comment.
baseUrl stored in body may differ from the URL used to reach the endpoint.
Line 75 strips the trailing slash from baseUrl.value to build gatewayBaseUrl (used for the fetch call), but line 85 sends the original baseUrl.value in the body. If the user typed a trailing slash, the backend persists it, and it gets used verbatim next time — potentially causing issues in URL construction.
🛡️ Proposed fix: normalise consistently
const gatewayBaseUrl = baseUrl.value.replace(/\/$/, "");
// ...
body: JSON.stringify({
gatewayId: "default",
- baseUrl: baseUrl.value,
+ baseUrl: gatewayBaseUrl,
...
}),📝 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
| const gatewayBaseUrl = baseUrl.value.replace(/\/$/, ""); | |
| try { | |
| const response = await fetch(`${gatewayBaseUrl}/web/dashboard/config`, { | |
| method: "POST", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| body: JSON.stringify({ | |
| gatewayId: "default", | |
| baseUrl: baseUrl.value, | |
| const gatewayBaseUrl = baseUrl.value.replace(/\/$/, ""); | |
| try { | |
| const response = await fetch(`${gatewayBaseUrl}/web/dashboard/config`, { | |
| method: "POST", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| body: JSON.stringify({ | |
| gatewayId: "default", | |
| baseUrl: gatewayBaseUrl, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/apps/dashboard/src/App.vue` around lines 75 - 85, The request
body uses the original baseUrl.value while the fetch uses gatewayBaseUrl (which
trims a trailing slash), causing inconsistent persisted values; update the
payload to send the normalized URL (gatewayBaseUrl) instead of baseUrl.value so
the backend stores the same slash-trimmed value used for the request—locate the
fetch call that posts to `${gatewayBaseUrl}/web/dashboard/config` and replace
the body field currently setting baseUrl: baseUrl.value with baseUrl:
gatewayBaseUrl (or otherwise normalize baseUrl.value before building both the
URL and the body).
Comment on lines
+77
to
+90
| try { | ||
| const response = await fetch(`${gatewayBaseUrl}/web/dashboard/config`, { | ||
| method: "POST", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ | ||
| gatewayId: "default", | ||
| baseUrl: baseUrl.value, | ||
| pairingCode: pairingCodeInput, | ||
| bearerToken: bearerTokenInput, | ||
| webhookSecret: webhookSecretInput, | ||
| }), | ||
| }); |
Contributor
There was a problem hiding this comment.
fetch has no timeout — saveStatus can get permanently stuck at "saving".
If the gateway is unreachable or hangs, the await fetch(...) never resolves, saveStatus stays "saving", and the submit button remains disabled indefinitely. The only recovery is a page reload.
🐛 Proposed fix: add AbortController with timeout
+ const controller = new AbortController();
+ const timeoutId = setTimeout(() => controller.abort(), 10_000);
+
try {
const response = await fetch(`${gatewayBaseUrl}/web/dashboard/config`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ ... }),
+ signal: controller.signal,
});
+ clearTimeout(timeoutId);
if (!response.ok) {
throw new Error(`HTTP ${response.status}`);
}
...
} catch (error) {
+ clearTimeout(timeoutId);
saveStatus.value = "error";
saveErrorMessage.value = t("form.saveError");
console.error("Error saving gateway config", error);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/apps/dashboard/src/App.vue` around lines 77 - 90, The POST fetch
to `${gatewayBaseUrl}/web/dashboard/config` has no timeout which can leave
saveStatus stuck at "saving"; wrap the request in an AbortController with a
setTimeout to call controller.abort() (store the timer id), pass
controller.signal to fetch, clear the timeout on success, and in the
catch/finally for the function set saveStatus to a terminal state (e.g., "error"
or "idle") so the submit button is re-enabled; update the existing try/catch
around the fetch (the block that sets saveStatus and reads response) to treat
AbortError specially for timeout and ensure the timer is cleared and controller
signal is cleaned up.
Comment on lines
+83
to
+90
| body: JSON.stringify({ | ||
| gatewayId: "default", | ||
| baseUrl: baseUrl.value, | ||
| pairingCode: pairingCodeInput, | ||
| bearerToken: bearerTokenInput, | ||
| webhookSecret: webhookSecretInput, | ||
| }), | ||
| }); |
Contributor
There was a problem hiding this comment.
Empty-string secrets always sent — risks silently clearing credentials on the backend.
When the user saves only baseUrl without entering any secret fields, pairingCodeInput, bearerTokenInput, and webhookSecretInput are all "". Every save unconditionally POSTs all three as empty strings. If the backend treats an empty-string field as "clear this value", existing stored credentials will be wiped with no warning.
Only include a secret in the payload when it was explicitly entered:
🐛 Proposed fix: conditionally include secret fields
body: JSON.stringify({
gatewayId: "default",
- baseUrl: baseUrl.value,
- pairingCode: pairingCodeInput,
- bearerToken: bearerTokenInput,
- webhookSecret: webhookSecretInput,
+ baseUrl: baseUrl.value,
+ ...(pairingCodeInput.length > 0 && { pairingCode: pairingCodeInput }),
+ ...(bearerTokenInput.length > 0 && { bearerToken: bearerTokenInput }),
+ ...(webhookSecretInput.length > 0 && { webhookSecret: webhookSecretInput }),
}),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/apps/dashboard/src/App.vue` around lines 83 - 90, The request
body always includes empty secret strings (pairingCodeInput, bearerTokenInput,
webhookSecretInput) which can silently clear backend credentials; change the
payload construction before JSON.stringify so you only add those keys when their
corresponding values are non-empty (e.g., start with { gatewayId: "default",
baseUrl: baseUrl.value } and conditionally set pairingCode, bearerToken,
webhookSecret only if pairingCodeInput, bearerTokenInput, webhookSecretInput are
truthy/not empty), then JSON.stringify that object so empty strings are not
sent.
Comment on lines
+96
to
+103
| pairingCodeInput = ""; | ||
| bearerTokenInput = ""; | ||
| webhookSecretInput = ""; | ||
| secretInputNonce.value += 1; | ||
| saveStatus.value = "success"; | ||
| } catch (error) { | ||
| saveStatus.value = "error"; | ||
| saveErrorMessage.value = t("form.saveError"); |
Contributor
There was a problem hiding this comment.
saveStatus is never reset to "idle" — success/error banner persists stale.
After a save, saveStatus stays at "success" (or "error") through subsequent form edits. The "saved" banner remains visible even after the user modifies fields, falsely implying unsaved changes have been persisted.
Add an auto-dismiss timeout on success:
✨ Proposed fix
saveStatus.value = "success";
+ setTimeout(() => {
+ saveStatus.value = "idle";
+ }, 3000);
} catch (error) {
saveStatus.value = "error";
saveErrorMessage.value = t("form.saveError");
console.error("Error saving gateway config", error);
}📝 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
| pairingCodeInput = ""; | |
| bearerTokenInput = ""; | |
| webhookSecretInput = ""; | |
| secretInputNonce.value += 1; | |
| saveStatus.value = "success"; | |
| } catch (error) { | |
| saveStatus.value = "error"; | |
| saveErrorMessage.value = t("form.saveError"); | |
| pairingCodeInput = ""; | |
| bearerTokenInput = ""; | |
| webhookSecretInput = ""; | |
| secretInputNonce.value += 1; | |
| saveStatus.value = "success"; | |
| setTimeout(() => { | |
| saveStatus.value = "idle"; | |
| }, 3000); | |
| } catch (error) { | |
| saveStatus.value = "error"; | |
| saveErrorMessage.value = t("form.saveError"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/apps/dashboard/src/App.vue` around lines 96 - 103, The saveStatus
reactive (saveStatus.value) is left as "success" or "error" after save, so the
banner persists; update the save success path (where saveStatus.value =
"success") to start/track a timeout (e.g., 3s) that resets saveStatus.value =
"idle" when fired, and ensure any previous timeout is cleared before creating a
new one (store the timer id in a ref like saveStatusTimeoutId). Also, in form
field change handlers (e.g., where pairingCodeInput, bearerTokenInput,
webhookSecretInput, secretInputNonce are updated) immediately set
saveStatus.value = "idle" to hide stale banners when the user edits. Ensure the
timeout is cleared on component unmount if applicable.
Comment on lines
+167
to
+171
| <Input | ||
| :key="`pairing-${secretInputNonce}`" | ||
| :placeholder="t('form.pairingCodePlaceholder')" | ||
| @update:model-value="(value: string) => captureSecretInput('pairingCode', value)" | ||
| /> |
Contributor
There was a problem hiding this comment.
pairingCode input is missing type="password" — sensitive value is rendered in clear text.
Both bearerToken (line 178) and webhookSecret (line 187) use type="password" to mask the field, but the pairingCode input at line 167 does not. A pairing code is equally sensitive and should be masked.
🛡️ Proposed fix
<Input
:key="`pairing-${secretInputNonce}`"
:placeholder="t('form.pairingCodePlaceholder')"
+ type="password"
`@update`:model-value="(value: string) => captureSecretInput('pairingCode', value)"
/>📝 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
| <Input | |
| :key="`pairing-${secretInputNonce}`" | |
| :placeholder="t('form.pairingCodePlaceholder')" | |
| @update:model-value="(value: string) => captureSecretInput('pairingCode', value)" | |
| /> | |
| <Input | |
| :key="`pairing-${secretInputNonce}`" | |
| :placeholder="t('form.pairingCodePlaceholder')" | |
| type="password" | |
| `@update`:model-value="(value: string) => captureSecretInput('pairingCode', value)" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/apps/dashboard/src/App.vue` around lines 167 - 171, The pairing
code Input currently renders in clear text; update the Input component with
type="password" to mask the value (same as bearerToken and webhookSecret).
Locate the Input with :key="`pairing-${secretInputNonce}`" and the
`@update`:model-value calling captureSecretInput('pairingCode', ...), and add the
type="password" prop so the pairingCode is treated as a password field.
|
|
||
| ## 🛠️ Comandos | ||
|
|
||
| Requisitos minimos: |
Contributor
There was a problem hiding this comment.
Missing accent: "mínimos"
minimos should be mínimos per standard Spanish orthography.
✏️ Proposed fix
-Requisitos minimos:
+Requisitos mínimos:📝 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
| Requisitos minimos: | |
| Requisitos mínimos: |
🧰 Tools
🪛 LanguageTool
[grammar] ~39-~39: Utiliza tildes correctamente
Context: ...ecto: 4323 ## 🛠️ Comandos Requisitos minimos: - Node.js 20.19+ (recomendado 22+) - pnpm ...
(QB_NEW_ES_OTHER_ERROR_IDS_MISSING_ORTHOGRAPHY_DIACRITIC_ACUTE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@clients/web/README.md` at line 39, Update the typo in the README by replacing
the phrase "Requisitos minimos:" with the correct Spanish spelling "Requisitos
mínimos:"; locate the line containing the heading "Requisitos minimos" and add
the acute accent on the 'i' in "mínimos".
This was referenced Feb 18, 2026
This was referenced Mar 2, 2026
Merged
This was referenced Mar 18, 2026
This was referenced Apr 9, 2026
Merged
This was referenced Apr 25, 2026
8 tasks
8 tasks
Copilot AI
added a commit
that referenced
this pull request
May 3, 2026
…ll/ for PRs Lychee detected 2 broken links (502 errors) in CHANGELOG.md: - #24 → pull/24 - #189 → pull/189 Both #24 and #189 are pull requests, not issues, so the correct URL uses /pull/ instead of /issues/. Also fixes all other CHANGELOG entries where issue links were used for pull requests (confirmed via lychee's 302 redirect detection), replacing them with the canonical /pull/ URLs to eliminate redirect warnings. Genuine issue links (closes #XXX pattern) are preserved as-is. Agent-Logs-Url: https://github.com/dallay/corvus/sessions/10d02c35-0fc4-47a1-bd63-a27fa52f5d45 Co-authored-by: yacosta738 <33158051+yacosta738@users.noreply.github.com>
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 introduces internationalization (i18n) to the dashboard app, adds a comprehensive Spanish translation file, and establishes a test setup using Vitest and Vue Test Utils. It also improves input handling and accessibility, introduces HTML escaping for user input, and updates dependencies and configuration for better maintainability and testing.
Internationalization and Localization
vue-i18ninto the dashboard app, initialized with a new Spanish translations file (es.json), and updated UI strings to use translation keys instead of hardcoded text. (clients/web/apps/dashboard/src/i18n.ts[1]clients/web/apps/dashboard/src/locales/es.json[2]clients/web/apps/dashboard/src/App.vue[3] [4] [5]clients/web/apps/dashboard/src/main.ts[6]clients/web/apps/dashboard/src/App.vue[1] [2]clients/web/apps/dashboard/src/locales/es.json[3]Testing Infrastructure
Appcomponent that verifies chat functionality and configuration toggling. (clients/web/apps/dashboard/package.json[1]clients/web/apps/dashboard/vite.config.ts[2]clients/web/apps/dashboard/src/App.spec.ts[3]Input Handling and Security
escapeHtmlutility to sanitize user input before displaying it in the chat, mitigating XSS risks. (clients/web/apps/dashboard/src/lib/utils.ts[1]clients/web/apps/dashboard/src/App.vue[2]clients/web/apps/dashboard/src/App.vue[1] [2]Component and Accessibility Improvements
data-testidanddata-roleattributes to chat messages for improved testability and accessibility. (clients/web/apps/dashboard/src/components/chat/ChatMessage.vueclients/web/apps/dashboard/src/components/chat/ChatMessage.vueL9-R13)clients/web/apps/dashboard/src/components/ui/button/Button.vue[1]clients/web/apps/dashboard/src/components/ui/input/Input.vue[2]Dependency and Configuration Updates
clients/web/apps/dashboard/package.json[1]clients/web/package.json[2]clients/web/README.mdclients/web/README.mdL12-R12)Summary by CodeRabbit
Documentation
New Features
Chores