Pr/resilience model cooldown - [deferred to v4.0.0-rc1]#2143
Pr/resilience model cooldown - [deferred to v4.0.0-rc1]#2143rafacpti23 wants to merge 27 commits into
Conversation
…bal routing/memory customizations)
…hen token missing
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a comprehensive SaaS layer branded as 'Easy IA,' which includes a new landing page, a customer portal, and administrative interfaces for managing users, subscription plans, and billing integrated with Mercado Pago. Key technical enhancements include the addition of Qdrant for semantic memory indexing, a refactored proxy middleware with improved authentication and body size guards, and new resilience features such as model cooldown management. Reviewers identified critical security risks regarding hardcoded placeholders for sensitive credentials in the production stack configuration and the removal of logic for immediate runtime setting application. Further feedback points to a regression in internationalization within the memory management UI and suggests verifying that the updated proxy matcher correctly handles all existing API endpoints.
| MANAGEMENT_PASSWORD: TROQUE_ESTA_SENHA_ADMIN_FORTE | ||
| STORAGE_ENCRYPTION_KEY: TROQUE_ESTA_CHAVE_COM_32_CARACTERES_OU_MAIS |
There was a problem hiding this comment.
The MANAGEMENT_PASSWORD and STORAGE_ENCRYPTION_KEY are defined with placeholder values (TROQUE_ESTA_SENHA_ADMIN_FORTE, TROQUE_ESTA_CHAVE_COM_32_CARACTERES_OU_MAIS). While these are intended as reminders, hardcoding sensitive information, even as placeholders, can lead to insecure deployments if not properly updated. For production environments, it is highly recommended to manage these values using Docker secrets or external environment variables that are not committed to version control.
| invalidateDbCache("settings"); // Bust the read cache immediately | ||
| const nextSettings = await getSettings(); | ||
|
|
||
| try { | ||
| const { applyRuntimeSettings } = await import("@/lib/config/runtimeSettings"); | ||
| await applyRuntimeSettings(nextSettings, { source: "settings:update" }); | ||
| } catch (error) { | ||
| console.warn( | ||
| "[HOT_RELOAD] Failed to apply runtime settings after update:", | ||
| error instanceof Error ? error.message : error | ||
| ); | ||
| } |
There was a problem hiding this comment.
The logic to apply runtime settings after updating them has been removed. If certain settings require immediate application without a server restart, this change might prevent them from taking effect or introduce unexpected delays. Please clarify if this is intentional and if there's an alternative mechanism to ensure critical runtime settings are applied promptly.
| <div className="text-sm text-gray-500"> | ||
| {t("pageInfo", { page, totalPages, total })} | ||
| Page {page} of {totalPages} ({total} total) |
There was a problem hiding this comment.
The text "Page {page} of {totalPages} ({total} total)" is hardcoded in English, removing the internationalization support previously provided by t("pageInfo"). To maintain consistency with other translated elements on the page, please use an i18n key for this string.
| <div className="text-sm text-gray-500"> | |
| {t("pageInfo", { page, totalPages, total })} | |
| Page {page} of {totalPages} ({total} total) | |
| <div className="text-sm text-gray-500"> | |
| {t("pageInfo", { page, totalPages, total })} | |
| </div> |
| export const config = { | ||
| matcher: [ | ||
| "/", | ||
| "/dashboard/:path*", | ||
| "/api/:path*", | ||
| "/v1/:path*", | ||
| "/v1", | ||
| "/chat/:path*", | ||
| "/responses/:path*", | ||
| "/responses", | ||
| "/codex/:path*", | ||
| "/codex", | ||
| "/models", | ||
| ], | ||
| matcher: ["/", "/adm/:path*", "/dashboard/:path*", "/api/:path*"], |
There was a problem hiding this comment.
The config.matcher has been updated to include /adm/:path* and remove specific /v1/ and /chat/ routes. While /api/:path* covers most API routes, ensure that all previously matched /v1/ and /chat/ routes are correctly handled by the new /api/:path* pattern or explicitly added if they fall outside its scope. This is important to prevent unintended routing issues for existing API endpoints.
|
Hey @rafacpti23, thanks for this contribution! 🚀 We really appreciate the work you've put into these features — the SaaS module, Qdrant integration, model cooldowns, and resilience improvements are all excellent additions. However, given the scope of the changes (~28K+ lines, 100 files), this PR is better suited for the next major release cycle. We don't want to rush such a large feature set into v3.8.0 where it could introduce regressions. Plan: This PR will be deferred to v4.0.0-rc1, where we'll have a dedicated integration window for these features. We'll keep this PR open and track it for the v4.0.0 milestone. In the meantime, the model cooldown feature (PR #2146) has been extracted as a focused PR and will be integrated into v3.8.0 separately. Thanks again for the amazing work! 🙌 |
diegosouzapw
changed the title
|
Thank you for this contribution! After review, this PR has been deferred to the v4.0.0-rc1 milestone as it introduces architectural changes that are better suited for the next major release. We'll revisit it there. We appreciate your work and will keep this open. |
|
Thank you for the resilience model cooldown work, @rafacpti23! This PR has been reviewed and deferred to v4.0.0-rc1 to allow proper integration alongside the broader resilience layer changes planned for that cycle. Your PR will be kept open and merged with full credit in that release. Thanks for your patience! |
2 participants
Summary
Related Issues
Validation
npm run lintnpm run test:unitnpm run test:coverage>= 60%for statements, lines, functions, and branchesTests Added Or Updated
Coverage Notes
src/,open-sse/,electron/, orbin/, explain which tests cover the change.Reviewer Notes