managed db - #33
Conversation
management - Migrate databases to a standalone resource model with project-optional attachment - Implement database lifecycle management: start, stop, restart, and retry - Add public access controls with CIDR allowlisting - Introduce Redis and MongoDB support - Add granular storage monitoring and credential management - Replace legacy project-scoped database tab with global dashboard view
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change introduces standalone managed databases with PostgreSQL, MySQL, Redis, and MongoDB support. It adds validation, persistence, Docker provisioning, lifecycle APIs, monitoring, and a top-level web management interface. ChangesManaged database lifecycle
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant DatabasesPage
participant API
participant DatabaseManager
participant Docker
Operator->>DatabasesPage: Configure and submit database
DatabasesPage->>API: Create database request
API->>API: Validate and normalize input
API->>DatabaseManager: Start asynchronous provisioning
DatabaseManager->>Docker: Create database resources
Docker-->>DatabaseManager: Return runtime resources
DatabaseManager-->>API: Persist status and connection metadata
API-->>DatabasesPage: Return sanitized database record
DatabasesPage-->>Operator: Display status and controls
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
management - Migrate databases to a standalone resource model with project-optional attachment - Implement database lifecycle management: start, stop, restart, and retry - Add public access controls with CIDR allowlisting - Introduce Redis and MongoDB support - Add granular storage monitoring and credential management - Replace legacy project-scoped database tab with global dashboard view
and update landing page - Ensure environment variables are created before initial deployments to prevent runtime errors for apps requiring specific configuration. - Update hero copy and stats on the documentation site for better messaging and accuracy.
management - Migrate databases to a standalone resource model with project-optional attachment - Implement database lifecycle management: start, stop, restart, and retry - Add public access controls with CIDR allowlisting - Introduce Redis and MongoDB support - Add granular storage monitoring and credential management - Replace legacy project-scoped database tab with global dashboard view
and update landing page - Ensure environment variables are created before initial deployments to prevent runtime errors for apps requiring specific configuration. - Update hero copy and stats on the documentation site for better messaging and accuracy.
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (6)
apps/api/src/databases/manager.ts (5)
167-177: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winWrite runtime data before the status flips to
running.Line 171 sets the status to
running, and Line 172 then writesexternalPortandproxyContainerName. A concurrentGET /databases/:idbetween the two writes returns a running database with a nullexternalPort. Swap the order so the runtime fields are persisted first.♻️ Proposed change
if (status.trim() === 'running') { - await updateDatabaseStatus(dbRecord.id, 'running', containerName); - await updateDatabaseRuntime(dbRecord.id, { externalPort, proxyContainerName }); + await updateDatabaseRuntime(dbRecord.id, { externalPort, proxyContainerName }); + await updateDatabaseStatus(dbRecord.id, 'running', containerName); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/databases/manager.ts` around lines 167 - 177, In the container readiness path around updateDatabaseStatus and updateDatabaseRuntime, persist the runtime fields externalPort and proxyContainerName before changing the database status to running. Keep the existing return and retry behavior unchanged so a running database is only exposed after its runtime data is available.
296-303: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRuntime imports duplicate the static import.
Line 6 already imports
getDatabaseById,updateDatabaseRuntime, andupdateDatabaseStatusfrom../db/repo. Line 302 re-imports two of those names dynamically and shadows the module-level bindings.reconcileMissingContainerat Line 291 does the same. The dynamic import adds no cycle protection here because the static import to the same module already exists. Use the static bindings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/databases/manager.ts` around lines 296 - 303, Remove the dynamic ../db/repo imports from startDatabaseMonitoring and reconcileMissingContainer, and reuse the existing module-level bindings for updateDatabaseRuntime and updateDatabaseStatus (plus any other already statically imported repository symbols). Keep the existing monitoring and reconciliation behavior unchanged.
277-287: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMeasuring storage starts one throwaway container per database per minute.
startDatabaseMonitoringcallsmeasureDatabaseStoragefor every running database on each 60 second tick. Each call starts analpine:3.20container and runsdu -smover the whole volume. For a host with many databases this adds container churn and disk I/O every minute, andducost grows with the data size. Consider a longer interval for storage measurement than for status reconciliation, or read the size fromdocker system df -v.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/databases/manager.ts` around lines 277 - 287, Reduce the frequency of storage measurements in the startDatabaseMonitoring flow instead of invoking measureDatabaseStorage on every 60-second reconciliation tick. Keep status reconciliation at its existing interval, and schedule measureDatabaseStorage using a longer interval while preserving the updateDatabaseRuntime behavior.
60-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider failing loudly for an unknown database type.
Line 64 returns a PostgreSQL image for any unrecognized
type. If an unexpected type ever reaches this function, the code provisions PostgreSQL while the record keeps a different port, credentials, and connection string. The mismatch is hard to diagnose. Validation upstream makes this unlikely today, so this is optional.♻️ Proposed change
- return 'postgres:18-alpine'; + throw new Error(`Unsupported database type: ${type}`);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/databases/manager.ts` around lines 60 - 65, Update the database image selection function containing the mongodb branch to reject unknown database types instead of falling back to the PostgreSQL image. Preserve the existing MongoDB version handling and PostgreSQL return for the explicitly supported PostgreSQL type, while throwing a clear error for any other value.
106-136: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a per-engine configuration table.
The initializers at Lines 106-112 hold PostgreSQL values, and the
postgresqlbranch then only overridesvolumeTarget. The default values are unused for the other three engines. A lookup keyed bydbRecord.typestates each engine's volume target, environment variables, and command arguments in one place and removes the implicit PostgreSQL default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/databases/manager.ts` around lines 106 - 136, Replace the mutable default initialization of volumeTarget, envVars, and extraCmdArgs with a per-engine configuration table keyed by dbRecord.type, defining all three values explicitly for PostgreSQL, MySQL, Redis, and MongoDB. Update the surrounding manager flow to select the matching configuration and preserve the existing PostgreSQL version-dependent volume target behavior.apps/api/src/databases/__tests__/manager.test.ts (1)
35-63: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd cases for the default version and the empty CIDR list.
The tests cover the main branches. Three gaps remain:
resolveDbImage('postgresql')andresolveDbImage('mysql')with no version, which exercise the default tags.resolveDbImagewith an unrecognized type, which currently returns a PostgreSQL image.proxyConfigwithallowPublicAccessFromAnywhere: falseand an emptyallowedCidrs, which produces an ACL line with no source.Run
bun testinapps/api/before you commit these API changes.As per coding guidelines: "Run 'bun test' in apps/api/ before committing API changes".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/src/databases/__tests__/manager.test.ts` around lines 35 - 63, Add tests in the existing resolveDbImage suite for default postgresql and mysql versions, and verify an unrecognized database type returns the current PostgreSQL fallback image. Extend the proxyConfig tests with allowPublicAccessFromAnywhere false and an empty allowedCidrs list, asserting the resulting ACL behavior. Run bun test from apps/api before committing.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/api/src/api/databases/index.ts`:
- Around line 89-139: Serialize the lifecycle handlers startDatabase,
stopDatabase, restartDatabase, retry, and deleteDatabase per database, using a
conditional status transition or per-database operation lock. Reject requests
when the database is provisioning, restarting, deleting, or recovering from
deletion, and ensure the state check and transition are atomic so concurrent
requests cannot execute Docker operations or overwrite status.
- Around line 74-86: Update the external credential construction around
resolveServerIp and buildConnectionString so IPify failures are not exposed as
127.0.0.1: return null external connection fields or a controlled error when the
public host cannot be resolved. Add an abort timeout to the resolveServerIp
lookup, and ensure external credentials are only returned for a successfully
resolved non-loopback host.
In `@apps/api/src/databases/manager.ts`:
- Around line 231-256: Update provisionPublicProxy to attach the HAProxy
container to config.dockerNetwork and publish externalPort with Docker’s -p
option instead of using --network host. Change the proxyConfig backend target to
dbRecord.internalHost rather than a resolved container IP, preserving the
existing port-allocation and health-check flow.
- Around line 87-94: Update abortIfDeleted to also remove the public proxy
container when provisioning is aborted, using the existing proxy-name derivation
and cleanup mechanism used by deprovisionDatabase or provisionPublicProxy. Keep
the cleanup conditional alongside ensureVolumeRemoved and
ensureContainerRemoved, and ensure failures remain non-blocking.
- Around line 190-197: Update the Docker inspect template in resolveBackendHost
to select and return only the first network IPAddress rather than concatenating
addresses from every network. Preserve the existing trim, empty-address
validation, and return behavior.
- Around line 114-116: Update the PostgreSQL handling around dbRecord.version
and the image resolver to reject version "latest" or resolve it to a concrete
major version before selecting volumeTarget; do not let nonnumeric versions fall
back to major 18. Preserve the existing path selection for validated numeric
versions, using the resolved major version to choose between /var/lib/postgresql
and /var/lib/postgresql/data.
In `@apps/api/src/databases/validation.ts`:
- Around line 26-33: Update validateDatabaseCreate to verify name and version
are strings before calling trim or isSafeDatabaseVersion, returning the existing
validation errors for invalid non-string values. Preserve current validation for
valid strings and absent version values, and add regression tests covering
object name and numeric version inputs.
In `@apps/web/src/components/databases/CreateDatabaseDialog.tsx`:
- Around line 68-69: Associate the project and engine labels with their Select
controls in the CreateDatabaseDialog JSX by adding matching unique htmlFor
values to the labels and id values to the corresponding SelectTrigger elements.
Use distinct identifiers for the project and engine fields while preserving the
existing Select behavior.
In `@apps/web/src/components/databases/DatabaseCard.tsx`:
- Around line 33-46: Use the lifecycle action state managed by lifecycle to
serialize deletion: disable or reject the delete control/handler whenever a
start, stop, restart, or retry is in progress, and keep it unavailable until
lifecycle’s finally block clears the busy state. Ensure the existing
provisioning coordination remains intact while preventing deletion from running
concurrently with these actions.
In `@apps/web/src/index.css`:
- Around line 36-39: Update the universal selector rule in the global stylesheet
by inserting an empty line between the `@apply` border-border declaration and the
scrollbar declarations to satisfy Stylelint’s declaration-empty-line-before
rule.
In `@apps/web/src/routes/CreateProjectPage.tsx`:
- Line 875: Remove the obsolete project-scoped database flow: in
apps/web/src/routes/CreateProjectPage.tsx lines 875-875 delete the hidden
database section, and at lines 102-105 remove the stale database state and
source comment; in
apps/web/src/components/project/create/CreateProjectDialog.tsx lines 73-75
delete the permanent false state and no-op setter; in
apps/web/src/components/project/create/StepResources.tsx lines 3-18 remove
unused database props and update all callers accordingly.
- Around line 102-105: Update CreateProjectPage’s local database handling to
import and use the shared DatabaseType contract. Keep the "postgresql" branch,
remove "postgres" and "mariadb" branches, and ensure the remaining handling
matches the allowed values: "postgresql", "mysql", "redis", and "mongodb".
---
Nitpick comments:
In `@apps/api/src/databases/__tests__/manager.test.ts`:
- Around line 35-63: Add tests in the existing resolveDbImage suite for default
postgresql and mysql versions, and verify an unrecognized database type returns
the current PostgreSQL fallback image. Extend the proxyConfig tests with
allowPublicAccessFromAnywhere false and an empty allowedCidrs list, asserting
the resulting ACL behavior. Run bun test from apps/api before committing.
In `@apps/api/src/databases/manager.ts`:
- Around line 167-177: In the container readiness path around
updateDatabaseStatus and updateDatabaseRuntime, persist the runtime fields
externalPort and proxyContainerName before changing the database status to
running. Keep the existing return and retry behavior unchanged so a running
database is only exposed after its runtime data is available.
- Around line 296-303: Remove the dynamic ../db/repo imports from
startDatabaseMonitoring and reconcileMissingContainer, and reuse the existing
module-level bindings for updateDatabaseRuntime and updateDatabaseStatus (plus
any other already statically imported repository symbols). Keep the existing
monitoring and reconciliation behavior unchanged.
- Around line 277-287: Reduce the frequency of storage measurements in the
startDatabaseMonitoring flow instead of invoking measureDatabaseStorage on every
60-second reconciliation tick. Keep status reconciliation at its existing
interval, and schedule measureDatabaseStorage using a longer interval while
preserving the updateDatabaseRuntime behavior.
- Around line 60-65: Update the database image selection function containing the
mongodb branch to reject unknown database types instead of falling back to the
PostgreSQL image. Preserve the existing MongoDB version handling and PostgreSQL
return for the explicitly supported PostgreSQL type, while throwing a clear
error for any other value.
- Around line 106-136: Replace the mutable default initialization of
volumeTarget, envVars, and extraCmdArgs with a per-engine configuration table
keyed by dbRecord.type, defining all three values explicitly for PostgreSQL,
MySQL, Redis, and MongoDB. Update the surrounding manager flow to select the
matching configuration and preserve the existing PostgreSQL version-dependent
volume target behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 96224943-d4b2-4caf-b7e1-85d32f95b7d4
📒 Files selected for processing (33)
apps/api/src/api/databases/index.tsapps/api/src/databases/__tests__/manager.test.tsapps/api/src/databases/__tests__/validation.test.tsapps/api/src/databases/manager.tsapps/api/src/databases/validation.tsapps/api/src/db/__tests__/databases-repo-runner.tsapps/api/src/db/__tests__/databases.test.tsapps/api/src/db/__tests__/migration-backfill.test.tsapps/api/src/db/migrations/0007_standalone_databases.sqlapps/api/src/db/migrations/meta/_journal.jsonapps/api/src/db/repo/databases.tsapps/api/src/db/repo/index.tsapps/api/src/db/repo/projects.tsapps/api/src/db/schema.tsapps/api/src/index.tsapps/api/src/types.tsapps/api/src/utils/dequel-labels.tsapps/docs/src/components/Hero.astroapps/docs/src/components/Stats.astroapps/web/src/api/client.tsapps/web/src/components/StatusBadge.tsxapps/web/src/components/databases/CreateDatabaseDialog.tsxapps/web/src/components/databases/DatabaseCard.tsxapps/web/src/components/layout/SidebarNav.tsxapps/web/src/components/project/create/CreateProjectDialog.tsxapps/web/src/components/project/create/StepResources.tsxapps/web/src/components/project/databases/DatabasesTab.tsxapps/web/src/index.cssapps/web/src/routes/CreateProjectPage.tsxapps/web/src/routes/Databases.tsxapps/web/src/routes/ProjectDetail.tsxapps/web/src/routes/index.tsxapps/web/src/types/index.ts
💤 Files with no reviewable changes (2)
- apps/web/src/components/project/databases/DatabasesTab.tsx
- apps/web/src/routes/ProjectDetail.tsx
| const externalHost = dbRecord.publicAccess && dbRecord.externalPort | ||
| ? await resolveServerIp() | ||
| : null; | ||
| const externalConnectionString = externalHost | ||
| ? buildConnectionString(dbRecord, externalHost, dbRecord.externalPort!) | ||
| : null; | ||
| return { | ||
| username: dbRecord.username, | ||
| password: dbRecord.password, | ||
| internalConnectionString: dbRecord.connectionString, | ||
| externalConnectionString, | ||
| externalHost, | ||
| externalPort: dbRecord.externalPort, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
fd -a 'index.ts|databases' . | sed 's#^\./##' | head -100
echo "== target file context =="
wc -l apps/api/src/api/databases/index.ts
sed -n '1,140p' apps/api/src/api/databases/index.ts
echo "== resolveServerIp references =="
rg -n "resolveServerIp|externalHost|externalConnectionString|127\.0\.0\.1" apps/api/src -SRepository: Lftobs/dequel
Length of output: 10090
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== apps/api/src/utils/dns.ts =="
wc -l apps/api/src/utils/dns.ts
cat -n apps/api/src/utils/dns.ts
echo "== dns tests resolveServerIp context =="
sed -n '98,135p' apps/api/src/utils/__tests__/dns.test.ts
echo "== domains resolveServerIp usage =="
sed -n '1,120p' apps/api/src/api/domains/index.ts
echo "== buildConnectionString definitions/usages =="
rg -n "function buildConnectionString|const buildConnectionString|buildConnectionString" apps/api/src -S
echo "== dependency fetch availability =="
if command -v bun >/dev/null 2>&1; then bun -v; fi
if command -v node >/dev/null 2>&1; then node -e "console.log(process.version); const http = require('http'); console.log(typeof http.request);" 2>&1 || true; fiRepository: Lftobs/dequel
Length of output: 6157
Do not return unresolved externalHost for database credentials.
resolveServerIp() falls back to 127.0.0.1 on IPify fetch failures, and this route does not reject that response. Remote clients can receive a loopback externalConnectionString that cannot connect. Return no external credentials or a controlled error when resolution fails, and limit the IPify lookup with an abort timeout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/api/databases/index.ts` around lines 74 - 86, Update the
external credential construction around resolveServerIp and
buildConnectionString so IPify failures are not exposed as 127.0.0.1: return
null external connection fields or a controlled error when the public host
cannot be resolved. Add an abort timeout to the resolveServerIp lookup, and
ensure external credentials are only returned for a successfully resolved
non-loopback host.
| .post("/databases/:id/start", async ({ params: { id }, set }) => { | ||
| const dbRecord = await findDatabase(id, set); | ||
| if (!dbRecord) return { error: "Database not found" }; | ||
| const { startDatabase } = await import("../../databases/manager"); | ||
| await startDatabase(dbRecord); | ||
| return sanitizeDatabase((await getDatabaseById(id))!); | ||
| }) | ||
| .post("/databases/:id/stop", async ({ params: { id }, set }) => { | ||
| const dbRecord = await findDatabase(id, set); | ||
| if (!dbRecord) return { error: "Database not found" }; | ||
| const { stopDatabase } = await import("../../databases/manager"); | ||
| await stopDatabase(dbRecord); | ||
| return sanitizeDatabase((await getDatabaseById(id))!); | ||
| }) | ||
| .post("/databases/:id/restart", async ({ params: { id }, set }) => { | ||
| const dbRecord = await findDatabase(id, set); | ||
| if (!dbRecord) return { error: "Database not found" }; | ||
| const { restartDatabase } = await import("../../databases/manager"); | ||
| await restartDatabase(dbRecord); | ||
| return sanitizeDatabase((await getDatabaseById(id))!); | ||
| }) | ||
| .post("/databases/:id/retry", async ({ params: { id }, set }) => { | ||
| const dbRecord = await findDatabase(id, set); | ||
| if (!dbRecord) return { error: "Database not found" }; | ||
| if (dbRecord.status !== "failed") { | ||
| set.status = 409; | ||
| return { error: "Only failed databases can be retried" }; | ||
| } | ||
| await updateDatabaseStatus(id, "provisioning"); | ||
| const { provisionDatabase } = await import("../../databases/manager"); | ||
| provisionDatabase((await getDatabaseById(id))!).catch((err: Error) => | ||
| console.error("DB reprovision failed", err), | ||
| ); | ||
| return sanitizeDatabase((await getDatabaseById(id))!); | ||
| }) | ||
| .delete("/databases/:id", async ({ params: { id }, set }) => { | ||
| const dbRecord = await findDatabase(id, set); | ||
| if (!dbRecord) return { error: "Database not found" }; | ||
| const { deprovisionDatabase, waitForProvision } = await import("../../databases/manager"); | ||
| await waitForProvision(id); | ||
| await updateDatabaseStatus(id, "deleting"); | ||
| const latest = (await getDatabaseById(id)) ?? dbRecord; | ||
| try { | ||
| await deprovisionDatabase(latest); | ||
| } catch (error) { | ||
| console.error(`Failed to deprovision database ${id}:`, error); | ||
| await updateDatabaseStatus(id, "deletion_failed"); | ||
| set.status = 502; | ||
| return { error: "Database resources could not be deleted; cleanup will be retried automatically" }; | ||
| } | ||
| return { ok: await deleteDatabase(id) }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Serialize lifecycle transitions for each database.
These handlers do not reject or atomically transition incompatible states. After deletion sets deleting, a concurrent start, stop, or restart request can still execute Docker commands and overwrite the record status. Use a conditional status transition or a per-database operation lock. Reject lifecycle operations while provisioning, restarting, deleting, or deletion recovery is active.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/api/databases/index.ts` around lines 89 - 139, Serialize the
lifecycle handlers startDatabase, stopDatabase, restartDatabase, retry, and
deleteDatabase per database, using a conditional status transition or
per-database operation lock. Reject requests when the database is provisioning,
restarting, deleting, or recovering from deletion, and ensure the state check
and transition are atomic so concurrent requests cannot execute Docker
operations or overwrite status.
| const abortIfDeleted = async () => { | ||
| if (!(await stillProvisioning())) { | ||
| if (createdVolume) await ensureVolumeRemoved(dbRecord.volumeName).catch(() => {}); | ||
| await ensureContainerRemoved(containerName).catch(() => {}); | ||
| return true; | ||
| } | ||
| return false; | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Remove the public proxy container in abortIfDeleted.
abortIfDeleted removes only the database container and volume. The check at Line 165 runs after provisionPublicProxy can create the HAProxy container. If the state changes at that point, the proxy container stays running with --restart unless-stopped and keeps a host port bound. The API delete route calls deprovisionDatabase, which derives the proxy name, so the orphan is usually cleaned later. Remove it here to avoid a window with a live public listener.
🛡️ Proposed fix
const abortIfDeleted = async () => {
if (!(await stillProvisioning())) {
+ await ensureContainerRemoved(publicProxyName(dbRecord)).catch(() => {});
if (createdVolume) await ensureVolumeRemoved(dbRecord.volumeName).catch(() => {});
await ensureContainerRemoved(containerName).catch(() => {});
return true;
}
return false;
};📝 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.
| const abortIfDeleted = async () => { | |
| if (!(await stillProvisioning())) { | |
| if (createdVolume) await ensureVolumeRemoved(dbRecord.volumeName).catch(() => {}); | |
| await ensureContainerRemoved(containerName).catch(() => {}); | |
| return true; | |
| } | |
| return false; | |
| }; | |
| const abortIfDeleted = async () => { | |
| if (!(await stillProvisioning())) { | |
| await ensureContainerRemoved(publicProxyName(dbRecord)).catch(() => {}); | |
| if (createdVolume) await ensureVolumeRemoved(dbRecord.volumeName).catch(() => {}); | |
| await ensureContainerRemoved(containerName).catch(() => {}); | |
| return true; | |
| } | |
| return false; | |
| }; |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/databases/manager.ts` around lines 87 - 94, Update
abortIfDeleted to also remove the public proxy container when provisioning is
aborted, using the existing proxy-name derivation and cleanup mechanism used by
deprovisionDatabase or provisionPublicProxy. Keep the cleanup conditional
alongside ensureVolumeRemoved and ensureContainerRemoved, and ensure failures
remain non-blocking.
| if (dbRecord.type === 'postgresql') { | ||
| const major = Number((dbRecord.version ?? '18').match(/^\d+/)?.[0] ?? '18'); | ||
| volumeTarget = major >= 18 ? '/var/lib/postgresql' : '/var/lib/postgresql/data'; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
postgres official docker image 18 PGDATA /var/lib/postgresql volume change
💡 Result:
Starting with PostgreSQL 18, the official PostgreSQL Docker image has updated its directory structure for data storage to better align with the pg_ctlcluster standard and to facilitate easier major-version upgrades [1][2]. Key Changes in PostgreSQL 18 and Later: 1. New VOLUME Location: The Docker image volume has moved from the legacy /var/lib/postgresql/data to /var/lib/postgresql [1][3]. 2. New PGDATA Environment Variable: The default PGDATA environment variable is now version-specific [1][2]. For PostgreSQL 18, it is set to /var/lib/postgresql/18/docker [1][4]. Subsequent versions will follow this pattern (e.g., /var/lib/postgresql/19/docker for version 19) [1][3]. Why This Change Was Made: This new structure enables users to perform in-place major-version upgrades using pg_upgrade with the --link option [3][5]. By mounting a single volume at /var/lib/postgresql, the container can manage subdirectories for different PostgreSQL major versions, which is a requirement for using the efficient hard-linking upgrade method [1][6]. Migration and Usage Guidance: - For New Deployments: You should mount your data volume at /var/lib/postgresql instead of the legacy /var/lib/postgresql/data path [3][6]. - For Existing Deployments: Upgrading from a version prior to 18 (e.g., 17) to 18+ requires careful migration [3]. Because the internal path has changed, a simple image update will not automatically relocate your existing data [6]. To migrate, you must either: - Perform a proper pg_upgrade, which involves structuring your data directory to match the new 18+ subdirectory convention [2][6]. - Use data export/import tools like pg_dumpall to move your data into a fresh container configured with the new mount point [7]. Note: For versions 17 and below, the legacy recommendation remains to mount at /var/lib/postgresql/data [1][3]. Mounting at /var/lib/postgresql for those older versions will not correctly persist data across container re-creations [1][3].
Citations:
- 1: https://hub.docker.com/_/postgres?ordering=last_updated&page=1&tab=description
- 2: Change
PGDATAin 18+ to/var/lib/postgresql/MAJOR/dockerdocker-library/postgres#1259 - 3: https://github.com/docker-library/docs/blob/master/postgres/README.md
- 4: https://github.com/docker-library/postgres/blob/master/18/bookworm/Dockerfile
- 5: https://docs.docker.com/guides/postgresql/advanced-configuration-and-initialization/
- 6: Migration from v17 to v18 appears impossible docker-library/postgres#1377
- 7: Postgres 18: VOLUME/PGDATA path change breaks existing mounts — docs/notes request docker-library/postgres#1370
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files:"
git ls-files | rg '(^|/)manager\.ts$|resolveDbImage|databases' || true
echo
echo "manager.ts section:"
if [ -f apps/api/src/databases/manager.ts ]; then
nl -ba apps/api/src/databases/manager.ts | sed -n '80,145p'
fi
echo
echo "postgres resolve references:"
rg -n "resolveDbImage|postgres:latest|version.*latest|postgresql" apps/api/src -S || true
echo
echo "Dockerfile / tags / image references:"
rg -n "postgres:latest|postgres:[0-9]|DATABASE|DATABASE_VERSION|version" . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**' || trueRepository: Lftobs/dequel
Length of output: 796
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "manager.ts section:"
sed -n '80,145p' apps/api/src/databases/manager.ts | cat -n -v
echo
echo "image/version references in databases code:"
rg -n "postgres:latest|postgres:|version|resolveDbImage|DATABASE_VERSION|DATABASE_TYPE|postgresql|MySQL|Redis|MongoDB|mongo" apps/api/src/databases apps/api/src/db apps/api/src/api/databases -S || true
echo
echo "validation test relevant lines:"
sed -n '1,220p' apps/api/src/databases/__tests__/validation.test.ts | cat -n -v
echo
echo "manager tests relevant lines:"
sed -n '1,260人' apps/api/src/databases/__tests__/manager.test.ts | cat -n -vRepository: Lftobs/dequel
Length of output: 14055
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "validation.ts:"
sed -n '1,80p' apps/api/src/databases/validation.ts | cat -n -v
echo
echo "manager resolveDbImage:"
sed -n '40,70p' apps/api/src/databases/manager.ts | cat -n -v
echo
echo "manager tests relevant lines:"
sed -n '1,70p' apps/api/src/databases/__tests__/manager.test.ts | cat -n -v
echo
echo "Node parser probe:"
node - <<'JS'
const versions = ['latest', '17', '18', '16-alpine', '18-alpine', '~latest', 'latest;evil', ''];
for (const version of versions) {
const major = Number((version ?? '18').match(/^\d+/)?.[0] ?? '18');
const volumeTarget = major >= 18 ? '/var/lib/postgresql' : '/var/lib/postgresql/data';
console.log(JSON.stringify({ version, missingLeadingDigits: Number((version ?? '18').match(/^\d+$/)?.[0] ?? '18') !== Number((version ?? '18').match(/^\d+/)?.[0] ?? '18'), leadingDigitsMajor: major, volumeTarget }));
}
JSRepository: Lftobs/dequel
Length of output: 8968
Reject version: "latest" or resolve it before choosing the PostgreSQL volume path.
The PostgreSQL image resolver maps latest to postgres:latest, but the volume path parser treats any input without leading digits as major version 18 and mounts /var/lib/postgresql. Accepting latest exposes new databases to whichever Docker tag is current; if that tag resolves to any pre-18 image, the data directory is under the default PGDATA path and the mount will not correctly persist the cluster.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/databases/manager.ts` around lines 114 - 116, Update the
PostgreSQL handling around dbRecord.version and the image resolver to reject
version "latest" or resolve it to a concrete major version before selecting
volumeTarget; do not let nonnumeric versions fall back to major 18. Preserve the
existing path selection for validated numeric versions, using the resolved major
version to choose between /var/lib/postgresql and /var/lib/postgresql/data.
| const resolveBackendHost = async (containerName: string): Promise<string> => { | ||
| const output = await run(dockerBin, [ | ||
| 'inspect', '-f', '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}', containerName, | ||
| ]); | ||
| const address = output.trim(); | ||
| if (!address) throw new Error(`Database container ${containerName} has no network address`); | ||
| return address; | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Take the first network address instead of concatenating all of them.
The Go template at Line 192 iterates every entry in .NetworkSettings.Networks and prints each IPAddress without a separator. If the container is attached to more than one network, the result is two addresses joined together, for example 172.18.0.5172.19.0.4. The non-empty check at Line 195 passes, and HAProxy then routes to an invalid backend. The provisioning code attaches a single network today, so this only breaks if a network is added later.
🛡️ Proposed fix
const output = await run(dockerBin, [
- 'inspect', '-f', '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}', containerName,
+ 'inspect', '-f', `{{with index .NetworkSettings.Networks "${config.dockerNetwork}"}}{{.IPAddress}}{{end}}`, containerName,
]);📝 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.
| const resolveBackendHost = async (containerName: string): Promise<string> => { | |
| const output = await run(dockerBin, [ | |
| 'inspect', '-f', '{{range .NetworkSettings.Networks}}{{.IPAddress}}{{end}}', containerName, | |
| ]); | |
| const address = output.trim(); | |
| if (!address) throw new Error(`Database container ${containerName} has no network address`); | |
| return address; | |
| }; | |
| const resolveBackendHost = async (containerName: string): Promise<string> => { | |
| const output = await run(dockerBin, [ | |
| 'inspect', '-f', `{{with index .NetworkSettings.Networks "${config.dockerNetwork}"}}{{.IPAddress}}{{end}}`, containerName, | |
| ]); | |
| const address = output.trim(); | |
| if (!address) throw new Error(`Database container ${containerName} has no network address`); | |
| return address; | |
| }; |
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/api/src/databases/manager.ts` around lines 190 - 197, Update the Docker
inspect template in resolveBackendHost to select and return only the first
network IPAddress rather than concatenating addresses from every network.
Preserve the existing trim, empty-address validation, and return behavior.
| <div className="grid gap-2"><label className="text-xs font-medium text-zinc-400">Attach to project</label><Select value={projectId} onValueChange={setProjectId}><SelectTrigger><SelectValue /></SelectTrigger><SelectContent><SelectItem value="standalone">No project</SelectItem>{projects.map((project) => <SelectItem key={project.id} value={project.id}>{project.name}</SelectItem>)}</SelectContent></Select></div> | ||
| <div className="grid gap-2"><label className="text-xs font-medium text-zinc-400">Engine</label><Select value={type} onValueChange={(value) => setType(value as DatabaseType)}><SelectTrigger><SelectValue /></SelectTrigger><SelectContent>{["postgresql", "mysql", "redis", "mongodb"].map((engine) => <SelectItem key={engine} value={engine}>{engine}</SelectItem>)}</SelectContent></Select></div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Associate each Select label with its control.
The label elements have no htmlFor value. The SelectTrigger elements have no matching id. Screen readers cannot determine the field name for project and engine selection.
Proposed fix
-<div className="grid gap-2"><label className="text-xs font-medium text-zinc-400">Attach to project</label><Select value={projectId} onValueChange={setProjectId}><SelectTrigger><SelectValue /></SelectTrigger>
+<div className="grid gap-2"><label htmlFor="database-project" className="text-xs font-medium text-zinc-400">Attach to project</label><Select value={projectId} onValueChange={setProjectId}><SelectTrigger id="database-project"><SelectValue /></SelectTrigger>
-<div className="grid gap-2"><label className="text-xs font-medium text-zinc-400">Engine</label><Select value={type} onValueChange={(value) => setType(value as DatabaseType)}><SelectTrigger><SelectValue /></SelectTrigger>
+<div className="grid gap-2"><label htmlFor="database-engine" className="text-xs font-medium text-zinc-400">Engine</label><Select value={type} onValueChange={(value) => setType(value as DatabaseType)}><SelectTrigger id="database-engine"><SelectValue /></SelectTrigger>📝 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.
| <div className="grid gap-2"><label className="text-xs font-medium text-zinc-400">Attach to project</label><Select value={projectId} onValueChange={setProjectId}><SelectTrigger><SelectValue /></SelectTrigger><SelectContent><SelectItem value="standalone">No project</SelectItem>{projects.map((project) => <SelectItem key={project.id} value={project.id}>{project.name}</SelectItem>)}</SelectContent></Select></div> | |
| <div className="grid gap-2"><label className="text-xs font-medium text-zinc-400">Engine</label><Select value={type} onValueChange={(value) => setType(value as DatabaseType)}><SelectTrigger><SelectValue /></SelectTrigger><SelectContent>{["postgresql", "mysql", "redis", "mongodb"].map((engine) => <SelectItem key={engine} value={engine}>{engine}</SelectItem>)}</SelectContent></Select></div> | |
| <div className="grid gap-2"><label htmlFor="database-project" className="text-xs font-medium text-zinc-400">Attach to project</label><Select value={projectId} onValueChange={setProjectId}><SelectTrigger id="database-project"><SelectValue /></SelectTrigger><SelectContent><SelectItem value="standalone">No project</SelectItem>{projects.map((project) => <SelectItem key={project.id} value={project.id}>{project.name}</SelectItem>)}</SelectContent></Select></div> | |
| <div className="grid gap-2"><label htmlFor="database-engine" className="text-xs font-medium text-zinc-400">Engine</label><Select value={type} onValueChange={(value) => setType(value as DatabaseType)}><SelectTrigger id="database-engine"><SelectValue /></SelectTrigger><SelectContent>{["postgresql", "mysql", "redis", "mongodb"].map((engine) => <SelectItem key={engine} value={engine}>{engine}</SelectItem>)}</SelectContent></Select></div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/databases/CreateDatabaseDialog.tsx` around lines 68 -
69, Associate the project and engine labels with their Select controls in the
CreateDatabaseDialog JSX by adding matching unique htmlFor values to the labels
and id values to the corresponding SelectTrigger elements. Use distinct
identifiers for the project and engine fields while preserving the existing
Select behavior.
| const lifecycle = async (action: "start" | "stop" | "restart" | "retry") => { | ||
| setIsBusy(true); | ||
| setActionError(null); | ||
| try { | ||
| if (action === "start") await api.startDatabase(database.id); | ||
| else if (action === "stop") await api.stopDatabase(database.id); | ||
| else if (action === "retry") await api.retryDatabase(database.id); | ||
| else await api.restartDatabase(database.id); | ||
| onChanged(); | ||
| } catch (err) { | ||
| setActionError(err instanceof Error ? err.message : "Action failed"); | ||
| } finally { | ||
| setIsBusy(false); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline apps/api/src/databases/manager.ts --items all --match 'startDatabase|stopDatabase|restartDatabase|deprovisionDatabase|waitForProvision'
rg -n -C 5 \
'startDatabase|stopDatabase|restartDatabase|deprovisionDatabase|waitForProvision|provisionInFlight' \
apps/api/src/databases/manager.ts \
apps/api/src/api/databases/index.tsRepository: Lftobs/dequel
Length of output: 12238
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== DatabaseCard relevant lines =="
sed -n '1,120p' apps/web/src/components/databases/DatabaseCard.tsx
echo
echo "== Database API route relevant lines =="
sed -n '60,150p' apps/api/src/api/databases/index.ts
echo
echo "== Manager relevant lines =="
sed -n '190,285p' apps/api/src/databases/manager.ts
echo
echo "== DB status/repository status helpers search =="
rg -n -C 3 "deleteDatabase|deletion_failed|deleting|status.*=.*deleting|findDatabase|getDatabaseById|updateDatabaseStatus" apps/api/src apps/web/src | head -n 220
echo
echo "== Programmatic control-flow check for lifecycle/delete serialization =="
python3 - <<'PY'
from pathlib import Path
import re
api = Path('apps/api/src/api/databases/index.ts').read_text()
manager = Path('apps/api/src/databases/manager.ts').read_text()
routes = {
'start': re.search(r'/databases/:id/start[\s\S]*?await startDatabase\(dbRecord\)', api),
'stop': re.search(r'/databases/:id/stop[\s\S]*?await stopDatabase\(dbRecord\)', api),
'restart': re.search(r'/databases/:id/restart[\s\S]*?await restartDatabase\(dbRecord\)', api),
'delete': re.search(r'/databases/:id[\s\S]*?await(deprovision|updateDatabaseStatus|getDatabaseById|findDatabase)', api),
}
for name, m in routes.items():
print(f"{name}: {bool(m)}")
delete = routes['delete'].group(0) if routes['delete'] else ''
lifecycle_managers = ['startDatabase(', 'stopDatabase(', 'restartDatabase(', 'retryDatabase(']
delete_checks = ['waitForProvision(', 'updateDatabaseStatus("deleting")', 'waitForDel']
for token in lifecycle_managers:
print(f"lifecycle manager has '{token}': {token in manager}")
for token in delete_checks:
print(f"delete route has '{token}': {token in delete or token in routes.get('delete', [] or '')}")
# Static-ish check for the same in-flight map used by deprovision/delete.
print("provisionInFlight used by non-provision endpoints:", bool(re.search(r'provisionInFlight\.get|provisionInFlight\.has|provisionInFlight\.set', manager)))
print("deprovisionDatabase calls waitForProvision or deletes from provisionInFlight:", bool(re.search(r'waitForProvision|provisionInFlight\.delete|provisionInFlight\.get', delete)))
# Find the delete route body via line numbers as a small second check.
api_lines = api.splitlines()
for i, line in enumerate(api_lines, 1):
if '.delete("/databases/:id"' in line:
print(f"delete route from line {i-1}")
for j in range(max(1, i-1), min(len(api_lines), i+18)+1):
print(f"{j}: {api_lines[j-1]}")
break
PYRepository: Lftobs/dequel
Length of output: 31663
Disable deletion while lifecycle actions run.
onStart, stop, restart, and retry can run while the delete route still leaves the containers/volume on disk. provisionInFlight only coordinates provisioning/retry; it does not serialize delete against these lifecycle operations. Disable or reject deletion until active lifecycle actions complete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/databases/DatabaseCard.tsx` around lines 33 - 46, Use
the lifecycle action state managed by lifecycle to serialize deletion: disable
or reject the delete control/handler whenever a start, stop, restart, or retry
is in progress, and keep it unavailable until lifecycle’s finally block clears
the busy state. Ensure the existing provisioning coordination remains intact
while preventing deletion from running concurrently with these actions.
| * { | ||
| @apply border-border; | ||
| scrollbar-width: thin; | ||
| scrollbar-color: #27272a transparent; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the Stylelint declaration spacing error.
Add an empty line after @apply border-border;. Stylelint reports declaration-empty-line-before for Line 38.
Proposed fix
* {
`@apply` border-border;
+
scrollbar-width: thin;
scrollbar-color: `#27272a` transparent;
}📝 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.
| * { | |
| @apply border-border; | |
| scrollbar-width: thin; | |
| scrollbar-color: #27272a transparent; | |
| * { | |
| `@apply` border-border; | |
| scrollbar-width: thin; | |
| scrollbar-color: `#27272a` transparent; |
🧰 Tools
🪛 Stylelint (17.14.1)
[error] 38-38: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/index.css` around lines 36 - 39, Update the universal selector
rule in the global stylesheet by inserting an empty line between the `@apply`
border-border declaration and the scrollbar declarations to satisfy Stylelint’s
declaration-empty-line-before rule.
Source: Linters/SAST tools
| // Legacy project-scoped database controls are hidden in favor of /databases. | ||
| const provisionDb = false; | ||
| const setProvisionDb = (_value: boolean) => {}; | ||
| const [dbType, setDbType] = useState<DatabaseType>('postgresql'); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 2 \
'import type|DatabaseType|mariadb|dbType' \
apps/web/src/routes/CreateProjectPage.tsx \
apps/web/src/types/index.tsRepository: Lftobs/dequel
Length of output: 8012
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
route = Path('apps/web/src/routes/CreateProjectPage.tsx')
types = Path('apps/web/src/types/index.ts')
route_text = route.read_text()
types_text = types.read_text()
print('CreateProjectPage.tsx first 20 import lines:')
print('\n'.join(f'{i+1}: {line}' for i, line in enumerate(route_text.splitlines()[:20])))
print('\nDatabaseType reference context:')
for i, line in enumerate(route_text.splitlines(), 1):
if 'DatabaseType' in line or ("mariadb" in line and 'dbType' in line):
s = max(1, i-2); e = min(len(route_text.splitlines()), i+2)
for j in range(s, e+1):
print(f'{j}: {route_text.splitlines()[j-1]}')
print()
print('CreateProjectPage.tsx has explicit DatabaseType import?')
print(any('DatabaseType' in line and 'import' in line for line in route_text.splitlines()))
print('Exported DatabaseType branch values:')
for line in types_text.splitlines():
if line.strip().startswith('export type DatabaseType'):
print(line.strip())
PYRepository: Lftobs/dequel
Length of output: 1738
Restore the DatabaseType contract.
This route uses DatabaseType without importing it, so type checking fails. Align the local database handling with the shared type: DatabaseType is 'postgresql' | 'mysql' | 'redis' | 'mongodb', so keep the "postgresql" branch and remove "postgres" and "mariadb".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/routes/CreateProjectPage.tsx` around lines 102 - 105, Update
CreateProjectPage’s local database handling to import and use the shared
DatabaseType contract. Keep the "postgresql" branch, remove "postgres" and
"mariadb" branches, and ensure the remaining handling matches the allowed
values: "postgresql", "mysql", "redis", and "mongodb".
|
|
||
| {/* Managed Database Selection Cards */} | ||
| <div className="pt-4 border-t border-[#1c1c21] space-y-4"> | ||
| <div className="hidden pt-4 border-t border-[#1c1c21] space-y-4"> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Remove the obsolete project-scoped database flow.
The database flow now belongs to the Databases page. These files retain disabled state, no-op setters, hidden markup, and unused props. This also leaves apps/web/src/routes/CreateProjectPage.tsx at 1,160 lines and apps/web/src/components/project/create/CreateProjectDialog.tsx at 555 lines.
apps/web/src/routes/CreateProjectPage.tsx#L875-L875: delete the hidden database section.apps/web/src/routes/CreateProjectPage.tsx#L102-L105: delete the stale database state and the unnecessary source comment.apps/web/src/components/project/create/CreateProjectDialog.tsx#L73-L75: delete the permanentfalsestate and no-op setter.apps/web/src/components/project/create/StepResources.tsx#L3-L18: delete the unused database props and update callers.
As per coding guidelines, “No comments in source code unless absolutely necessary” and “No file should be above 500 lines of code; if it is, refactor and split into smaller files with proper feature grouping in a folder rather than scattered across the codebase.”
📍 Affects 3 files
apps/web/src/routes/CreateProjectPage.tsx#L875-L875(this comment)apps/web/src/routes/CreateProjectPage.tsx#L102-L105apps/web/src/components/project/create/CreateProjectDialog.tsx#L73-L75apps/web/src/components/project/create/StepResources.tsx#L3-L18
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/routes/CreateProjectPage.tsx` at line 875, Remove the obsolete
project-scoped database flow: in apps/web/src/routes/CreateProjectPage.tsx lines
875-875 delete the hidden database section, and at lines 102-105 remove the
stale database state and source comment; in
apps/web/src/components/project/create/CreateProjectDialog.tsx lines 73-75
delete the permanent false state and no-op setter; in
apps/web/src/components/project/create/StepResources.tsx lines 3-18 remove
unused database props and update all callers accordingly.
Source: Coding guidelines
projects Add `installCommand` and `outputDir` fields to the project schema and repository to allow custom build configurations. Introduce an error summarizer utility to extract actionable build failure messages from noisy logs.
robust validation - Add operation locking to prevent concurrent database state changes - Enforce type checking for database inputs and names - Add default image versions for supported database engines - Include cleanup of proxy containers during provision failures - Add timeout and error handling to IP resolution - Disable delete buttons in UI during ongoing operations
1 participant
Description
Please provide a summary of the changes and the motivation behind them. What problem does this PR solve?
Fixes #(issue)
Type of Change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.
bun testinapps/api/)Checklist
bun testinapps/api/and all tests passbun run sync-versions)Screenshots (if applicable)
Additional Context
Add any other context about the PR here (e.g., migration notes, deployment considerations, rollback strategy).
Summary by CodeRabbit