feat(orchestrator): add support for Docker Compose project deployments - #29
feat(orchestrator): add support for Docker Compose project deployments#29Lftobs wants to merge 9 commits into
Conversation
time - Add `finished_at` column to deployments table - Implement status-based timestamping for deployment completion - Update runtime reconciliation to skip non-existent containers - Fix Ko-fi popup mounting in dashboard sidebar - Update docker-compose configuration for local builds
customization - Add `project_type`, `buildCommand`, and `startCommand` fields to projects. - Implement dynamic `railpack.json` generation for better build/runtime compatibility. - Add `ProjectSettingsTab` to the dashboard for managing build settings. - Update Caddy reverse proxy to correctly pass the Host header to upstream containers.
project deployments - Update database schema to store compose-specific metadata - Enhance Caddy dynamic ingress to route traffic to specific services - Extend project/domain APIs to support service target configuration - Refactor documentation landing page and UI components
📝 WalkthroughWalkthroughThe change adds Docker Compose project configuration, validation, persistence, deployment, and domain routing. It adds a multi-step project creation interface and updates project settings. The documentation homepage receives new content, interactions, and visual styling. ChangesCompose platform support
Documentation homepage redesign
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/utils/domain-verifier.ts (1)
140-155: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRead the
PORTvariable before the custom blocks are built.The loop builds
customBlocksat line 141 with the currentport. ThePORTenvironment variable is read afterwards at lines 147-155. A custom domain that setstargetServicebut notargetPorttherefore usesappPortorconfig.appInternalPortand never the projectPORTvalue, while the primary block does use it. Move the environment lookup above the domain loop.🤖 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/utils/domain-verifier.ts` around lines 140 - 155, Move the PORT environment lookup using listEnvironmentVariablesForDeploy, including parsedPort validation and port assignment, before the domain loop that builds customBlocks. Ensure targetService domains without targetPort use the resolved project PORT value, while preserving the existing primary block behavior and fallback logic.
🟡 Minor comments (10)
apps/api/src/api/projects/index.ts-22-26 (1)
22-26: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject numeric zero for
composePort.Line 22 skips validation when the request contains numeric
0. Lines 74 and 103 then convert that invalid value tonull. The API returns success instead of a 400 response.Check for defined non-null values before conversion and validation.
Proposed fix
- if (body?.composePort) { + if (body?.composePort !== undefined && body.composePort !== null) { const port = Number(body.composePort); if (!isPort(port)) { return "composePort must be an integer between 1 and 65535"; } }Also applies to: 74-75, 102-104
🤖 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/projects/index.ts` around lines 22 - 26, Update composePort handling in the request validation and the corresponding conversion paths to check for defined, non-null values rather than truthiness, so numeric 0 reaches Number and is rejected by isPort with the existing validation error instead of becoming null.apps/web/src/components/project/domains/DomainsTab.tsx-64-65 (1)
64-65: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winClear routing fields when the dialog closes.
targetServiceandtargetPortare cleared only after a successful request. If a user enters them and cancels, the component keeps the values and reuses them for the next domain.Clear these fields in the dialog close and cancel handlers.
Proposed reset behavior
<Dialog open={isAddOpen} - onOpenChange={setIsAddOpen} + onOpenChange={(open) => { + setIsAddOpen(open); + if (!open) { + setTargetService(""); + setTargetPort(""); + } + }} >Also applies to: 81-83
🤖 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/project/domains/DomainsTab.tsx` around lines 64 - 65, Update the dialog close and cancel handlers in DomainsTab to reset both targetService and targetPort to empty values whenever the dialog is dismissed, including cancellation; preserve the existing successful-request reset behavior.apps/api/src/api/domains/index.ts-28-31 (1)
28-31: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject numeric
targetPort: 0instead of treating it as omitted.When
body.targetPortis numeric0, the truthiness check setstargetPorttonull, skipsisPort, and returns success. The request then loses an invalid routing value. Distinguish missing,null, and empty string from numeric zero before conversion.Proposed validation fix
- const targetPort = body.targetPort ? Number(body.targetPort) : null; - if (targetPort !== null && !isPort(targetPort)) { + const hasTargetPort = + body.targetPort !== undefined && + body.targetPort !== null && + body.targetPort !== ""; + const targetPort = hasTargetPort + ? Number(body.targetPort) + : null; + if (hasTargetPort && !isPort(targetPort)) {Add a regression test for numeric
0. As per coding guidelines: Runbun testinapps/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/api/domains/index.ts` around lines 28 - 31, Update targetPort handling near the targetPort validation to distinguish omitted, null, and empty-string values from numeric 0 before conversion, ensuring numeric 0 is passed to isPort and rejected with the existing 400 response. Add a regression test covering numeric targetPort: 0, then run bun test from apps/api/.Source: Coding guidelines
apps/web/src/routes/CreateProjectPage.tsx-254-256 (1)
254-256: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winSend the compose fields only for a Compose build.
composeServicesis serialized and sent even whenbuildTypeisrailpack. The API then persists mappings that no code path uses, and a later switch to Compose applies stale values. SendundefinedwhenbuildType !== 'compose'.🤖 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 254 - 256, Update the composeServices assignment in the CreateProjectPage submission payload to serialize composeServicesList only when buildType is "compose"; otherwise send undefined. Leave the existing Compose-specific serialization behavior unchanged.apps/api/src/orchestrator/compose.ts-312-320 (1)
312-320: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCheck the exit code of
docker psbefore teardown.
destroyComposeStackignoresps.code. If thepscall fails,idsis empty and the function reports success while the stack stays running. The pipeline then treats the teardown as complete during the zero-downtime swap. Throw or log whenps.code !== 0.🤖 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/orchestrator/compose.ts` around lines 312 - 320, Update destroyComposeStack to check the result of runDocker(["ps", ...]) before parsing stdout; when ps.code is nonzero, throw or log an error and stop teardown instead of treating an empty ID list as success. Preserve the existing container removal and network cleanup flow for successful docker ps calls.apps/web/src/routes/CreateProjectPage.tsx-994-999 (1)
994-999: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThe summary reads state that no control can set.
baseDomainhas no input in any step, so the two summary rows at lines 994-999 and 1078-1083 never render.composeServiceandcomposePortalso have no input; the compose rows at lines 1008-1015 and 1090-1096 always showAuto-detect, even after the user fills the entry row ofcomposeServicesList.Read the compose summary from
composeServicesList[0], and either add a base domain field or removebaseDomainfrom the page.Also applies to: 1008-1015, 1090-1096
🤖 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 994 - 999, Update the summary rendering in CreateProjectPage so compose details come from the first entry of composeServicesList, including its service and port values, instead of composeService and composePort; preserve Auto-detect only when those values are absent. Remove the unreachable baseDomain summary rows and related state, or add a user-editable base-domain field that populates it.apps/api/src/orchestrator/pipeline.ts-168-184 (1)
168-184: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
dockerBininstead of the literal"docker".The file now imports
dockerBinat line 35, and the rest of the orchestrator invokes Docker through it.tryRun("docker", ...)depends ondockerbeing onPATH.tryRunswallows the failure, so container and image cleanup silently does nothing when the resolved binary differs.🛠️ Proposed fix
- if (deployment.containerName) { - await tryRun("docker", [ + if (deployment.containerName) { + await tryRun(dockerBin, [ "stop", "-t", "5", deployment.containerName, ]); - await tryRun("docker", [ + await tryRun(dockerBin, [ "rm", "-f", deployment.containerName, ]); } if (deployment.imageTag && deployment.sourceType !== "image") { - await tryRun("docker", ["rmi", "-f", deployment.imageTag]); + await tryRun(dockerBin, ["rmi", "-f", deployment.imageTag]); }🤖 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/orchestrator/pipeline.ts` around lines 168 - 184, Replace the literal "docker" command passed to tryRun in the container stop, container removal, and image removal cleanup calls with the imported dockerBin symbol. Keep the existing arguments and cleanup conditions unchanged so all orchestrator Docker operations use the resolved binary.apps/docs/src/components/HowItWorks.astro-45-47 (1)
45-47: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the inverted "without zero-downtime" phrase.
Line 46 reads: "reloads dynamic routes without zero-downtime." This phrase says the opposite of the intended claim. Features.astro states the same capability as "Zero-downtime dynamic routing reloading." Use consistent, correct wording here.
📝 Proposed fix
- The internal Caddy engine reloads dynamic routes without zero-downtime, automatically provisions Let's Encrypt certificates, and connects public traffic directly to your app. + The internal Caddy engine reloads dynamic routes with zero downtime, automatically provisions Let's Encrypt certificates, and connects public traffic directly to your app.🤖 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/docs/src/components/HowItWorks.astro` around lines 45 - 47, Update the descriptive paragraph in HowItWorks to state that the internal Caddy engine reloads dynamic routes with zero downtime, matching the wording and intended capability used in Features.astro. Leave the certificate provisioning and traffic connection claims unchanged.apps/docs/src/styles/global.css-889-926 (1)
889-926: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove quotes around the
Playfont-family name.Stylelint flags Line 895 and Line 917:
font-family: 'Play', sans-serif;should not quotePlay(font-family-name-quotes).🎨 Proposed fix
.cf-pill-btn-white { ... - font-family: 'Play', sans-serif; + font-family: Play, sans-serif; ... } .cf-pill-btn-glass { ... - font-family: 'Play', sans-serif; + font-family: Play, sans-serif; ... }🤖 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/docs/src/styles/global.css` around lines 889 - 926, Remove the quotes from the Play font-family name in both .cf-pill-btn-white and .cf-pill-btn-glass, leaving the sans-serif fallback and all other styling unchanged.Source: Linters/SAST tools
apps/docs/src/components/Stats.astro-10-28 (1)
10-28: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the divider border at the 2-column breakpoint.
last:border-r-0removes the right divider only from the 4th (DOM-last) card. On thegrid-cols-2layout, the 2nd card is also visually last in its row but still rendersborder-r, leaving a stray divider at the row edge.Use a responsive/
nthselector so each row's rightmost card drops its right border at both breakpoints.📐 Proposed fix
- <div class="flex flex-col justify-between border-r border-[`#222228`] last:border-r-0 pr-4 last:pr-0"> + <div class="flex flex-col justify-between border-r border-[`#222228`] max-lg:even:border-r-0 last:border-r-0 pr-4 max-lg:even:pr-0 last:pr-0">🤖 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/docs/src/components/Stats.astro` around lines 10 - 28, Update the mapped stat card container in Stats so its right border is removed from the rightmost card of each row in the two-column layout, while preserving correct divider behavior for the four-column layout. Replace the DOM-only last:border-r-0 rule with responsive or nth-child utilities targeting the second and fourth cards at their respective breakpoints.
🧹 Nitpick comments (8)
apps/api/src/orchestrator/compose.ts (3)
46-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the explanatory comments.
The coding guidelines forbid comments in source unless absolutely necessary. These comments restate the adjacent code.
As per coding guidelines: "No comments in source code unless absolutely necessary".
Also applies to: 52-52, 60-60, 70-73, 83-83
🤖 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/orchestrator/compose.ts` at line 46, Remove the redundant explanatory comments in the compose orchestration logic, including the comments around the services, volumes, networks, and related blocks; leave the surrounding implementation unchanged.Source: Coding guidelines
134-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared Compose parsing loop.
parseAllComposeServicesrepeats the full parsing loop ofparseComposeTarget(lines 34-96 and 147-203 are identical). Two copies will drift. Extract one helper that returns{ servicesList, servicePorts }and use it in both functions.🤖 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/orchestrator/compose.ts` around lines 134 - 221, The Compose YAML line-scanning logic is duplicated between parseComposeTarget and parseAllComposeServices. Extract the shared loop into a helper returning servicesList and servicePorts, then replace both functions’ duplicated parsing blocks with calls to that helper while preserving their existing target-selection and fallback behavior.
31-96: 🎯 Functional Correctness | 🔵 Trivial | 🏗️ Heavy liftReplace the Compose line parser with YAML parsing.
The current parser only handles two-space service indentation and short-form
portsentries. Long-form entries such astarget: 8080are ignored, leaving selected services on the fallback3000upstream and causing wrong Caddy routes. Parse the file with Bun’s YAML parser instead of a regex line scan.🤖 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/orchestrator/compose.ts` around lines 31 - 96, Replace the regex-based line scan in the compose parsing flow with Bun’s YAML parser, then traverse the parsed services and their ports to derive each service’s target port. Support both short-form and long-form Compose port entries, including long-form objects with target values, while preserving service discovery and fallback behavior for services without a detected port.apps/api/src/orchestrator/pipeline.ts (1)
622-624: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
this.slugifyfor the Compose slug.The class already defines
slugifyat line 264 with the same transformation. The inline copy can drift from it.♻️ Proposed refactor
- const slug = project.name - ? project.name.toLowerCase().replace(/[^a-z0-9-]+/g, "-").replace(/^-+|-+$/g, "").slice(0, 63) - : project.id; + const slug = project.name ? this.slugify(project.name) : project.id;🤖 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/orchestrator/pipeline.ts` around lines 622 - 624, Replace the inline slug transformation in the Compose slug assignment with the existing this.slugify method, passing project.name when present and retaining project.id as the fallback.apps/web/src/components/project/settings/ProjectSettingsTab.tsx (1)
36-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the
as anycasts on the project fields.This PR extends the project type with
buildType,composeService,composePort, andcomposeServices. The casts at lines 36-38, 47, and 102 disable type checking on exactly the new fields, so a name change in the API contract will not fail the build. Use the typed fields.Also applies to: 47-47, 102-102
🤖 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/project/settings/ProjectSettingsTab.tsx` around lines 36 - 38, Remove the any casts from the project field accesses in the ProjectSettingsTab initialization and related logic, including buildType, composeService, composePort, and composeServices. Use the newly extended typed project fields directly while preserving the existing defaults and string conversion behavior.apps/api/src/orchestrator/__tests__/compose.test.ts (1)
7-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd tests for
parseAllComposeServices.The pipeline builds Caddy routes from
parseAllComposeServices, not fromparseComposeTarget. No test covers the primary/secondary flags, the default port 3000 fallback, or thesourceDirpath resolution. Add cases for those paths.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/orchestrator/__tests__/compose.test.ts` around lines 7 - 69, Extend the compose test suite with coverage for parseAllComposeServices, including correct primary and secondary service flags, the default port 3000 when no port is defined, and sourceDir path resolution from the compose directory. Use temporary compose fixtures consistent with the existing parseComposeTarget tests, and run bun test from apps/api before committing.Source: Coding guidelines
apps/docs/src/components/Features.astro (1)
91-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd ARIA tab semantics to the capability tab widget.
The new capability tab widget has no ARIA roles or state. Screen reader users cannot tell this is a tabbed interface or which tab is selected. Buttons are keyboard-focusable and clickable, so this is not a keyboard-navigation blocker, but it is a screen-reader accessibility gap in a new interactive component.
apps/docs/src/components/Features.astro#L91-L176: addrole="tablist"to#cf-tab-container,role="tab",aria-selected={idx === 0}, andaria-controls={tab-panel-${tab.id}}to each button, androle="tabpanel"plusaria-labelledbyto each panel.apps/docs/src/pages/index.astro#L248-L269: in the click handler, updatearia-selectedon all buttons (set"true"on the clicked button,"false"on the rest) alongside the existing class toggling.🤖 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/docs/src/components/Features.astro` around lines 91 - 176, Add ARIA tab semantics in apps/docs/src/components/Features.astro lines 91-176: mark `#cf-tab-container` as tablist, each tab button as tab with aria-selected and aria-controls, and each tab panel as tabpanel with aria-labelledby. Update the click handler in apps/docs/src/pages/index.astro lines 248-269 to set aria-selected="true" on the clicked tab and "false" on all others alongside the existing class updates.apps/docs/src/components/Hero.astro (1)
2-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove or rewire the unused
tag,titleNormal,titleItalicprops.The template no longer renders
titleNormalortitleItalic(the headline text is hardcoded at Line 33-34), andtagis only referenced inside the commented-out announcement pill. ThePropsinterface still declares all three with defaults, so callers passing these props get no effect.Remove the unused props from the interface and destructuring, or wire them back into the headline/announcement markup so the component's declared API matches its actual behavior.
♻️ Proposed fix (remove unused props)
interface Props { - tag?: string; - titleNormal?: string; - titleItalic?: string; subText?: string; } const { - tag = "Self-Hosted PaaS Platform • Open Source v1.0", - titleNormal = "Deploy apps from Git, ZIP, or Docker Compose", - titleItalic = "yours by default.", subText = "The all-in-one self-hosted Platform as a Service (PaaS). Build container images via Railpack, route with dynamic Caddy auto-SSL, scale workloads, and monitor metrics — all on your own servers with zero vendor lock-in.", } = Astro.props;🤖 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/docs/src/components/Hero.astro` around lines 2 - 14, Remove tag, titleNormal, and titleItalic from the Props interface and Astro.props destructuring in the Hero component, since the template does not use them. Preserve subText and its existing behavior, ensuring the component API matches the rendered markup.
🤖 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/domains/index.ts`:
- Around line 33-38: Update both call sites of addToCaddyRoute in
reconcileVerifiedDomains and /verify to pass the created domain’s targetService
and targetPort metadata, and update addToCaddyRoute to use those values when
generating the reverse-proxy route. Ensure verified service-targeted domains
retain their configured Compose target instead of defaulting to domain:80.
In `@apps/api/src/databases/manager.ts`:
- Around line 33-44: Update the database command error handling around envVars,
run, and the console.error call so propagated docker failures cannot include
plaintext passwords. Redact sensitive values such as MYSQL_ROOT_PASSWORD,
MYSQL_PASSWORD, and POSTGRES_PASSWORD before constructing or logging the failure
message, while preserving useful command and stderr context.
- Around line 67-78: Update the timeout path in the provisioning flow around the
polling loop and updateDatabaseStatus so it throws an error after marking the
database as failed. Keep the existing exception path behavior consistent by
ensuring provisionDatabase rejects for both timeout and thrown-command failures.
In `@apps/api/src/orchestrator/compose.ts`:
- Around line 239-257: Update the stdout and stderr stream handlers in the child
process execution flow to attach a rejection handler to each onLog(line)
promise. Ensure database write failures are caught rather than left as unhandled
rejections, while preserving the existing line parsing and callback behavior.
In `@apps/api/src/orchestrator/pipeline.ts`:
- Around line 601-620: Before writing the Caddy route and marking the deployment
running in deployWithCompose, wait for the primary container selected by
target/containerFor to reach the running state, using the existing
waitForRunningContainer behavior. Preserve the current container selection and
network connection flow, and only continue when the primary service container is
confirmed running.
- Around line 575-599: Update the compose deployment flow around
deployWithCompose so the new stack is deployed and confirmed running before
calling destroyComposeStack for oldDeploymentId. Move the existing
previous-stack cleanup after successful deployment while preserving the current
logging and deployment names, ensuring a failed new deployment leaves the
previous stack intact.
- Around line 412-427: Update the Compose branch in the pipeline around
buildWithCompose to pass filterBuildEnvVars(envVars) instead of the complete
envVars-derived envMap, matching the Railpack build path and excluding runtime
secrets from the Docker build environment.
- Around line 636-659: Validate the parsed project.composeServices data with the
existing validateComposeServices helper before customMatch.subdomain and port
are interpolated into the Caddy configuration. Handle validation or JSON parsing
failures explicitly instead of silently ignoring them in the empty catch block,
and ensure invalid stored data cannot contribute to the generated site block.
In `@apps/api/src/utils/domain-verifier.ts`:
- Around line 132-139: The target container derivation in buildCaddySnippet must
not replace the second-to-last hyphen segment of containerName, because
hyphenated slugs are misidentified. Resolve the target container using the
actual Compose service map, or pass already-resolved container names into
buildCaddySnippet, while preserving the existing targetService selection
behavior.
In `@apps/web/src/components/project/domains/DomainsTab.tsx`:
- Around line 495-522: Refactor DomainsTab by extracting the domain list,
add-domain dialog, and delete dialog into feature-grouped components under an
appropriate folder, keeping DomainsTab focused on composition and shared state.
Preserve the existing behavior and ensure every resulting file, including
DomainsTab.tsx, remains under the 500-line limit.
In `@apps/web/src/components/project/settings/ProjectSettingsTab.tsx`:
- Around line 47-61: The compose-services initialization in ProjectSettingsTab
must normalize every parsed service row with a unique id before passing it to
setComposeServicesList. Update the Array.isArray(parsed) branch to map each
service and preserve its fields while assigning an existing id when present or
generating a fallback id when missing, so updateComposeServiceRow and
removeComposeServiceRow can match every row.
In `@apps/web/src/routes/CreateProjectPage.tsx`:
- Around line 263-289: Reorder the submission flow so the staged environment
variables in the stagedEnvs block are created before either api.createDeployment
call. Keep the existing validation, status update, trimming, and environment
defaults intact, then start the upload or Git deployment only after all
environment-variable creation requests complete.
- Around line 38-1110: Split CreateProjectPage into feature-grouped step
components under components/project/create/, extracting the UI for each wizard
step into focused components such as IdentityStep, BuildStrategyStep,
EnvAndDatabaseStep, and ReviewStep. Keep wizard state, step navigation, and
handleCreate in CreateProjectPage, passing required values and callbacks into
the extracted components. Ensure the page and each new component remain under
500 lines.
In `@apps/web/src/routes/Dashboard.tsx`:
- Around line 107-112: Update both “New Project” instances in Dashboard,
including the one near the existing Link and the corresponding instance around
the later project-empty-state section, to render Button with asChild and place
the Link as its child. Remove unnecessary Link wrappers so the resulting markup
contains a single link element without nested interactive controls, while
preserving the existing navigation target and button styling.
In `@apps/web/src/types/index.ts`:
- Around line 26-28: Add the required buildType: string property to both
frontend Project interfaces in the types definitions, and update the
corresponding project response type usage in the API client so typed callers can
access the API’s buildType field.
---
Outside diff comments:
In `@apps/api/src/utils/domain-verifier.ts`:
- Around line 140-155: Move the PORT environment lookup using
listEnvironmentVariablesForDeploy, including parsedPort validation and port
assignment, before the domain loop that builds customBlocks. Ensure
targetService domains without targetPort use the resolved project PORT value,
while preserving the existing primary block behavior and fallback logic.
---
Minor comments:
In `@apps/api/src/api/domains/index.ts`:
- Around line 28-31: Update targetPort handling near the targetPort validation
to distinguish omitted, null, and empty-string values from numeric 0 before
conversion, ensuring numeric 0 is passed to isPort and rejected with the
existing 400 response. Add a regression test covering numeric targetPort: 0,
then run bun test from apps/api/.
In `@apps/api/src/api/projects/index.ts`:
- Around line 22-26: Update composePort handling in the request validation and
the corresponding conversion paths to check for defined, non-null values rather
than truthiness, so numeric 0 reaches Number and is rejected by isPort with the
existing validation error instead of becoming null.
In `@apps/api/src/orchestrator/compose.ts`:
- Around line 312-320: Update destroyComposeStack to check the result of
runDocker(["ps", ...]) before parsing stdout; when ps.code is nonzero, throw or
log an error and stop teardown instead of treating an empty ID list as success.
Preserve the existing container removal and network cleanup flow for successful
docker ps calls.
In `@apps/api/src/orchestrator/pipeline.ts`:
- Around line 168-184: Replace the literal "docker" command passed to tryRun in
the container stop, container removal, and image removal cleanup calls with the
imported dockerBin symbol. Keep the existing arguments and cleanup conditions
unchanged so all orchestrator Docker operations use the resolved binary.
In `@apps/docs/src/components/HowItWorks.astro`:
- Around line 45-47: Update the descriptive paragraph in HowItWorks to state
that the internal Caddy engine reloads dynamic routes with zero downtime,
matching the wording and intended capability used in Features.astro. Leave the
certificate provisioning and traffic connection claims unchanged.
In `@apps/docs/src/components/Stats.astro`:
- Around line 10-28: Update the mapped stat card container in Stats so its right
border is removed from the rightmost card of each row in the two-column layout,
while preserving correct divider behavior for the four-column layout. Replace
the DOM-only last:border-r-0 rule with responsive or nth-child utilities
targeting the second and fourth cards at their respective breakpoints.
In `@apps/docs/src/styles/global.css`:
- Around line 889-926: Remove the quotes from the Play font-family name in both
.cf-pill-btn-white and .cf-pill-btn-glass, leaving the sans-serif fallback and
all other styling unchanged.
In `@apps/web/src/components/project/domains/DomainsTab.tsx`:
- Around line 64-65: Update the dialog close and cancel handlers in DomainsTab
to reset both targetService and targetPort to empty values whenever the dialog
is dismissed, including cancellation; preserve the existing successful-request
reset behavior.
In `@apps/web/src/routes/CreateProjectPage.tsx`:
- Around line 254-256: Update the composeServices assignment in the
CreateProjectPage submission payload to serialize composeServicesList only when
buildType is "compose"; otherwise send undefined. Leave the existing
Compose-specific serialization behavior unchanged.
- Around line 994-999: Update the summary rendering in CreateProjectPage so
compose details come from the first entry of composeServicesList, including its
service and port values, instead of composeService and composePort; preserve
Auto-detect only when those values are absent. Remove the unreachable baseDomain
summary rows and related state, or add a user-editable base-domain field that
populates it.
---
Nitpick comments:
In `@apps/api/src/orchestrator/__tests__/compose.test.ts`:
- Around line 7-69: Extend the compose test suite with coverage for
parseAllComposeServices, including correct primary and secondary service flags,
the default port 3000 when no port is defined, and sourceDir path resolution
from the compose directory. Use temporary compose fixtures consistent with the
existing parseComposeTarget tests, and run bun test from apps/api before
committing.
In `@apps/api/src/orchestrator/compose.ts`:
- Line 46: Remove the redundant explanatory comments in the compose
orchestration logic, including the comments around the services, volumes,
networks, and related blocks; leave the surrounding implementation unchanged.
- Around line 134-221: The Compose YAML line-scanning logic is duplicated
between parseComposeTarget and parseAllComposeServices. Extract the shared loop
into a helper returning servicesList and servicePorts, then replace both
functions’ duplicated parsing blocks with calls to that helper while preserving
their existing target-selection and fallback behavior.
- Around line 31-96: Replace the regex-based line scan in the compose parsing
flow with Bun’s YAML parser, then traverse the parsed services and their ports
to derive each service’s target port. Support both short-form and long-form
Compose port entries, including long-form objects with target values, while
preserving service discovery and fallback behavior for services without a
detected port.
In `@apps/api/src/orchestrator/pipeline.ts`:
- Around line 622-624: Replace the inline slug transformation in the Compose
slug assignment with the existing this.slugify method, passing project.name when
present and retaining project.id as the fallback.
In `@apps/docs/src/components/Features.astro`:
- Around line 91-176: Add ARIA tab semantics in
apps/docs/src/components/Features.astro lines 91-176: mark `#cf-tab-container` as
tablist, each tab button as tab with aria-selected and aria-controls, and each
tab panel as tabpanel with aria-labelledby. Update the click handler in
apps/docs/src/pages/index.astro lines 248-269 to set aria-selected="true" on the
clicked tab and "false" on all others alongside the existing class updates.
In `@apps/docs/src/components/Hero.astro`:
- Around line 2-14: Remove tag, titleNormal, and titleItalic from the Props
interface and Astro.props destructuring in the Hero component, since the
template does not use them. Preserve subText and its existing behavior, ensuring
the component API matches the rendered markup.
In `@apps/web/src/components/project/settings/ProjectSettingsTab.tsx`:
- Around line 36-38: Remove the any casts from the project field accesses in the
ProjectSettingsTab initialization and related logic, including buildType,
composeService, composePort, and composeServices. Use the newly extended typed
project fields directly while preserving the existing defaults and string
conversion 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: 51a16b89-3d5b-4a38-b349-ea03f976ddba
📒 Files selected for processing (39)
apps/api/src/api/domains/index.tsapps/api/src/api/projects/index.tsapps/api/src/databases/manager.tsapps/api/src/db/migrations/0004_early_sunfire.sqlapps/api/src/db/migrations/0005_famous_grandmaster.sqlapps/api/src/db/migrations/0006_cheerful_vance_astro.sqlapps/api/src/db/migrations/meta/0004_snapshot.jsonapps/api/src/db/migrations/meta/0005_snapshot.jsonapps/api/src/db/migrations/meta/0006_snapshot.jsonapps/api/src/db/migrations/meta/_journal.jsonapps/api/src/db/repo/domains.tsapps/api/src/db/repo/projects.tsapps/api/src/db/schema.tsapps/api/src/orchestrator/__tests__/compose.test.tsapps/api/src/orchestrator/compose.tsapps/api/src/orchestrator/pipeline.tsapps/api/src/types.tsapps/api/src/utils/__tests__/domain-verifier.test.tsapps/api/src/utils/__tests__/validate.test.tsapps/api/src/utils/domain-verifier.tsapps/api/src/utils/validate.tsapps/docs/.astro/astro/content.d.tsapps/docs/.astro/settings.jsonapps/docs/src/components/CTA.astroapps/docs/src/components/Comparison.astroapps/docs/src/components/Features.astroapps/docs/src/components/Hero.astroapps/docs/src/components/HowItWorks.astroapps/docs/src/components/Stats.astroapps/docs/src/components/TerminalDemo.astroapps/docs/src/pages/index.astroapps/docs/src/styles/global.cssapps/web/src/api/client.tsapps/web/src/components/project/domains/DomainsTab.tsxapps/web/src/components/project/settings/ProjectSettingsTab.tsxapps/web/src/routes/CreateProjectPage.tsxapps/web/src/routes/Dashboard.tsxapps/web/src/routes/index.tsxapps/web/src/types/index.ts
| composeService?: string | null; | ||
| composePort?: number | null; | ||
| composeServices?: string | null; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Add buildType to both frontend Project interfaces.
The API returns required buildType, but this interface and apps/web/src/api/client.ts omit it. A typed frontend caller cannot read the build type from a project response.
Add buildType: string to both interfaces.
Proposed fix
sourceType: string;
projectType: string;
+ buildType: string;
composeService?: string | null;🤖 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/types/index.ts` around lines 26 - 28, Add the required
buildType: string property to both frontend Project interfaces in the types
definitions, and update the corresponding project response type usage in the API
client so typed callers can access the API’s buildType field.
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/src/components/project/domains/DomainsTab.tsx (1)
32-37: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDomain write requests have no error handling. Both domain mutation paths call the API without a
catchand without an error state. On failure the dialog stays open, the user receives no message, and the rejection is unhandled.
apps/web/src/components/project/domains/DomainsTab.tsx#L32-L37: wrapapi.deleteDomainintry/catch, keepdeletingDomIdset on failure, and store the error message for display.apps/web/src/components/project/domains/AddDomainDialog.tsx#L42-L61: add acatcharoundapi.createDomainand render the error message inside the form.🤖 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/project/domains/DomainsTab.tsx` around lines 32 - 37, Handle failures for both domain mutations: in apps/web/src/components/project/domains/DomainsTab.tsx lines 32-37, wrap handleDeleteDomain’s api.deleteDomain call in try/catch, preserve deletingDomId on failure, and store the error message for display; in apps/web/src/components/project/domains/AddDomainDialog.tsx lines 42-61, catch api.createDomain errors and render the message inside the form.
🧹 Nitpick comments (1)
apps/web/src/routes/CreateProjectPage.tsx (1)
22-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare the
ComposeFieldandComposeServiceRowcontract.
ComposeFieldis declared here, inline atapps/web/src/components/project/create/BuildStrategyStep.tsxline 27, and again inapps/web/src/components/project/settings/ProjectSettingsTab.tsx. The row handlers are also duplicated across the create page and the settings tab. Export one type and one row-state hook from a shared module.🤖 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 22, Consolidate the duplicated ComposeField type and ComposeServiceRow row-state logic used by BuildStrategyStep, ProjectSettingsTab, and the create page into a shared module. Export the shared ComposeField contract and row-state hook, then update each consumer to import and reuse them instead of declaring local types or handlers.
🤖 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/databases/manager.ts`:
- Line 88: Update the timeout handling around updateDatabaseStatus so the
timeout error is thrown first and remains the propagated error. Let the catch
block perform a single best-effort update to the failed status, without a
preceding status update that can replace the timeout error or cause duplicate
persistence.
In `@apps/api/src/utils/domain-verifier.ts`:
- Around line 133-139: Update removeFromCaddyRoute to remove the matching
dedicated `${domain}:80 { ... }` block in addition to editing the primary host
line. Perform this cleanup before writeFile/reloadCaddy, while preserving
unrelated Caddy blocks and the existing primary-block behavior.
- Around line 125-135: Update the targeted-routing condition in
buildCaddySnippet to run when either targetService is set or targetPort is
non-null. For port-only routing, use containerName as the targetContainer while
preserving resolveTargetContainer for targetService; ensure the configured
targetPort is passed through instead of appending the domain to the primary
block.
- Around line 203-209: The custom block generation around resolveTargetContainer
must not fall back to containerName when a requested targetService cannot be
resolved. Update this branch to skip the block or fail route generation when
resolveTargetContainer returns undefined, while preserving the existing fallback
only when no targetService is requested.
- Line 132: Update the buildProxyBlock call in the domain verification flow to
pass the domain and port as a string array matching its declared string[]
parameter, while preserving targetContainer and tPort unchanged.
- Around line 38-43: Update the environment lookup in the domain-verification
flow around listEnvironmentVariablesForDeploy to retrieve variables for the
production environment only, then continue resolving PORT from that result.
Preserve the existing numeric validation and fallback behavior.
In `@apps/web/src/components/project/create/EnvAndDatabaseStep.tsx`:
- Around line 36-39: Update EnvAndDatabaseStep’s database configuration JSX to
render controls for dbCpu and dbMemory, wiring their values and setters through
the existing dbCpu, setDbCpu, dbMemory, and setDbMemory props so users can
configure both limits before CreateProjectPage.handleCreate submits them.
- Around line 42-70: Remove the duplicate EnvAndDatabaseStepProps declaration,
retaining the original prop definitions and setStagedEnvs type. Remove
clearStagedEnvs from the component’s destructured parameters, since
EnvAndDatabaseStep uses setStagedEnvs directly and callers do not provide
clearStagedEnvs.
In `@apps/web/src/components/project/create/IdentityStep.tsx`:
- Around line 136-205: Update the git repository section around RepoPicker to
render it only when both githubConfigured and githubConnected are true;
otherwise show the existing GitHub connection prompt or appropriate fallback. In
the upload section, use the zipFile prop to render zipFile?.name after a file is
selected so the user receives confirmation.
In `@apps/web/src/components/project/domains/DeleteDomainDialog.tsx`:
- Around line 49-54: Update DeleteDomainDialog to accept an isDeleting prop and
bind it to the confirmation Button’s disabled state. Pass the deleting state
from DomainsTab’s handleDeleteDomain flow using deletingDomId, so the button
remains disabled until the deletion request completes.
In `@apps/web/src/components/project/domains/DomainDnsCard.tsx`:
- Around line 103-109: Update the A-record row near the serverIp display and the
related section around lines 140-141 to avoid rendering "../.." when serverIp is
undefined. Render the row only once serverIp resolves, or show an explicit
fallback such as "Loading..." or "Server IP unavailable" without presenting it
as the DNS target.
- Around line 21-25: Update copyText in DomainDnsCard to handle missing
navigator.clipboard and rejected writeText promises, setting copiedTarget to
true only after a successful write and preserving a non-success state on
failure. Store the timeout handle and clear it when the component unmounts,
using the component’s existing lifecycle/effect mechanism.
In `@apps/web/src/components/project/domains/DomainTable.tsx`:
- Around line 68-77: Add an accessible aria-label to the icon-only delete Button
in DomainTable, using the domain value to identify the target as “Delete domain
…”. Apply the same accessible-name fix to the dismiss button in DomainDnsCard,
targeting its corresponding button element.
In `@apps/web/src/routes/CreateProjectPage.tsx`:
- Around line 215-222: Update handleSelectRepo to use GithubRepo’s declared
camelCase fields cloneUrl and defaultBranch instead of clone_url and
default_branch, ensuring setRepoUrl and setRepoBranch receive the selected
repository values and the existing deployment flow remains active.
- Line 1: Update the React import in CreateProjectPage to include useState
alongside useEffect, matching the hook used by the component and preserving the
existing useEffect import.
---
Outside diff comments:
In `@apps/web/src/components/project/domains/DomainsTab.tsx`:
- Around line 32-37: Handle failures for both domain mutations: in
apps/web/src/components/project/domains/DomainsTab.tsx lines 32-37, wrap
handleDeleteDomain’s api.deleteDomain call in try/catch, preserve deletingDomId
on failure, and store the error message for display; in
apps/web/src/components/project/domains/AddDomainDialog.tsx lines 42-61, catch
api.createDomain errors and render the message inside the form.
---
Nitpick comments:
In `@apps/web/src/routes/CreateProjectPage.tsx`:
- Line 22: Consolidate the duplicated ComposeField type and ComposeServiceRow
row-state logic used by BuildStrategyStep, ProjectSettingsTab, and the create
page into a shared module. Export the shared ComposeField contract and row-state
hook, then update each consumer to import and reuse them instead of declaring
local types or handlers.
🪄 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: 87f13484-b4c9-4e9e-9d2f-9fb707867124
📒 Files selected for processing (20)
apps/api/src/api/domains/index.tsapps/api/src/databases/manager.tsapps/api/src/orchestrator/compose.tsapps/api/src/orchestrator/pipeline.tsapps/api/src/orchestrator/runtime.tsapps/api/src/utils/domain-verifier.tsapps/web/src/components/project/create/BuildStrategyStep.tsxapps/web/src/components/project/create/EnvAndDatabaseStep.tsxapps/web/src/components/project/create/IdentityStep.tsxapps/web/src/components/project/create/ReviewStep.tsxapps/web/src/components/project/domains/AddDomainDialog.tsxapps/web/src/components/project/domains/DeleteDomainDialog.tsxapps/web/src/components/project/domains/DomainDnsCard.tsxapps/web/src/components/project/domains/DomainEmptyState.tsxapps/web/src/components/project/domains/DomainTable.tsxapps/web/src/components/project/domains/DomainsTab.tsxapps/web/src/components/project/settings/ProjectSettingsTab.tsxapps/web/src/routes/CreateProjectPage.tsxapps/web/src/routes/Dashboard.tsxapps/web/src/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/web/src/routes/Dashboard.tsx
- apps/api/src/api/domains/index.ts
- apps/web/src/components/project/settings/ProjectSettingsTab.tsx
- apps/web/src/types/index.ts
- apps/api/src/orchestrator/pipeline.ts
- apps/api/src/orchestrator/compose.ts
|
|
||
| await updateDatabaseStatus(dbRecord.id, 'failed', containerName); | ||
| await updateDatabaseStatus(dbRecord.id, 'failed', containerName); | ||
| throw new Error(`Timed out waiting for database ${dbRecord.id} container to reach running state`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve the timeout error when status persistence fails.
The preceding updateDatabaseStatus call runs before this throw. If that call fails, the catch block rethrows the status-persistence error instead of the timeout error. If it succeeds, the catch block writes the failed status a second time.
Throw the timeout error first and let the catch block perform one best-effort status update.
🧰 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` at line 88, Update the timeout handling
around updateDatabaseStatus so the timeout error is thrown first and remains the
propagated error. Let the catch block perform a single best-effort update to the
failed status, without a preceding status update that can replace the timeout
error or cause duplicate persistence.
| const envVars = await listEnvironmentVariablesForDeploy(projectId); | ||
| const portVar = envVars.find((v) => v.key === 'PORT'); | ||
| if (portVar && portVar.value) { | ||
| const parsedPort = Number(portVar.value); | ||
| if (!isNaN(parsedPort) && parsedPort > 0) return parsedPort; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Read the production PORT value.
Line 38 reads environment variables from every environment. The deployment pipeline reads only the production environment. If multiple PORT values exist, this route can use a non-production port and return 502 responses.
Proposed fix
- const envVars = await listEnvironmentVariablesForDeploy(projectId);
+ const envVars = await listEnvironmentVariablesForDeploy(projectId, 'production');📝 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 envVars = await listEnvironmentVariablesForDeploy(projectId); | |
| const portVar = envVars.find((v) => v.key === 'PORT'); | |
| if (portVar && portVar.value) { | |
| const parsedPort = Number(portVar.value); | |
| if (!isNaN(parsedPort) && parsedPort > 0) return parsedPort; | |
| } | |
| const envVars = await listEnvironmentVariablesForDeploy(projectId, 'production'); | |
| const portVar = envVars.find((v) => v.key === 'PORT'); | |
| if (portVar && portVar.value) { | |
| const parsedPort = Number(portVar.value); | |
| if (!isNaN(parsedPort) && parsedPort > 0) return parsedPort; | |
| } |
🤖 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/utils/domain-verifier.ts` around lines 38 - 43, Update the
environment lookup in the domain-verification flow around
listEnvironmentVariablesForDeploy to retrieve variables for the production
environment only, then continue resolving PORT from that result. Preserve the
existing numeric validation and fallback behavior.
| if (targetService) { | ||
| const targetContainer = await resolveTargetContainer(projectId, targetService); | ||
| if (!targetContainer) { | ||
| console.warn(`Could not resolve target container for service ${targetService} on ${domain} (deploy the project first)`); | ||
| return; | ||
| } | ||
| const tPort = targetPort ?? (await resolveProjectPort(projectId)) ?? config.appInternalPort; | ||
| const block = buildProxyBlock(`${domain}:80`, targetContainer, tPort); | ||
| content = `${removeDomainFromLine(firstLine, domain)}${content.slice(idx)}`; | ||
| if (!content.includes(`${domain}:80 {`)) { | ||
| content = content.endsWith('\n') ? `${content}${block}` : `${content}\n${block}`; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Create a custom block when only targetPort is set.
buildCaddySnippet treats either targetService or targetPort as targeted routing. This path checks only targetService. A domain with a primary-container targetPort is instead appended to the primary block and ignores its configured port.
Change the condition to handle targetPort != null. Use containerName for port-only routing.
🤖 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/utils/domain-verifier.ts` around lines 125 - 135, Update the
targeted-routing condition in buildCaddySnippet to run when either targetService
is set or targetPort is non-null. For port-only routing, use containerName as
the targetContainer while preserving resolveTargetContainer for targetService;
ensure the configured targetPort is passed through instead of appending the
domain to the primary block.
| return; | ||
| } | ||
| const tPort = targetPort ?? (await resolveProjectPort(projectId)) ?? config.appInternalPort; | ||
| const block = buildProxyBlock(`${domain}:80`, targetContainer, tPort); |
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 'const buildProxyBlock|buildProxyBlock\(' \
apps/api/src/utils/domain-verifier.tsRepository: Lftobs/dequel
Length of output: 1257
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in package.json bun.lockb bun.lock; do
[ -e "$f" ] && { echo "== $f =="; head -80 "$f" | sed -n '1,120p'; }
done
printf '\n== files matching domain-verifier package references ==\n'
rg -n '"apps/api"|apps/api|`@workspace`' package.json apps/api/package.json 2>/dev/null || true
printf '\n== call sites of domain verifier and buildProxyBlock outside referenced file ==\n'
rg -n -C 2 '(domain-verify|domainVerifier|buildProxyBlock)' apps/api/src 2>/dev/null || trueRepository: Lftobs/dequel
Length of output: 3225
Pass a domain array to buildProxyBlock.
buildProxyBlock expects string[]; the current domain:80 string makes this call incompatible with the declared type.
Proposed fix
- const block = buildProxyBlock(`${domain}:80`, targetContainer, tPort);
+ const block = buildProxyBlock([`${domain}:80`], targetContainer, tPort);📝 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 block = buildProxyBlock(`${domain}:80`, targetContainer, tPort); | |
| const block = buildProxyBlock([`${domain}:80`], targetContainer, tPort); |
🤖 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/utils/domain-verifier.ts` at line 132, Update the
buildProxyBlock call in the domain verification flow to pass the domain and port
as a string array matching its declared string[] parameter, while preserving
targetContainer and tPort unchanged.
| content = `${removeDomainFromLine(firstLine, domain)}${content.slice(idx)}`; | ||
| if (!content.includes(`${domain}:80 {`)) { | ||
| content = content.endsWith('\n') ? `${content}${block}` : `${content}\n${block}`; | ||
| } | ||
| await writeFile(filePath, content, 'utf8'); | ||
| await reloadCaddy(); | ||
| return; |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove targeted-domain blocks when the domain is deleted.
These lines remove a targeted domain from the primary block and append a separate Caddy block. removeFromCaddyRoute only checks and edits the primary host line. It therefore leaves this dedicated block active after domain deletion.
Remove the matching dedicated ${domain}:80 { ... } block in removeFromCaddyRoute before Caddy reloads. Otherwise, a removed domain can continue to proxy traffic to the project.
🤖 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/utils/domain-verifier.ts` around lines 133 - 139, Update
removeFromCaddyRoute to remove the matching dedicated `${domain}:80 { ... }`
block in addition to editing the primary host line. Perform this cleanup before
writeFile/reloadCaddy, while preserving unrelated Caddy blocks and the existing
primary-block behavior.
| const copyText = (text: string) => { | ||
| navigator.clipboard.writeText(text); | ||
| setCopiedTarget(true); | ||
| setTimeout(() => setCopiedTarget(false), 1500); | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle a clipboard failure and clear the timer.
navigator.clipboard is undefined in non-secure contexts, and writeText returns a promise that can reject. The current code sets copiedTarget to true before the write completes, so the card reports success after a failure. The setTimeout at line 24 is also never cleared on unmount.
🛡️ Proposed fix
- const copyText = (text: string) => {
- navigator.clipboard.writeText(text);
- setCopiedTarget(true);
- setTimeout(() => setCopiedTarget(false), 1500);
- };
+ const timerRef = useRef<ReturnType<typeof setTimeout>>();
+
+ useEffect(() => () => clearTimeout(timerRef.current), []);
+
+ const copyText = async (text: string) => {
+ try {
+ await navigator.clipboard?.writeText(text);
+ setCopiedTarget(true);
+ timerRef.current = setTimeout(() => setCopiedTarget(false), 1500);
+ } catch {
+ setCopiedTarget(false);
+ }
+ };🤖 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/project/domains/DomainDnsCard.tsx` around lines 21 -
25, Update copyText in DomainDnsCard to handle missing navigator.clipboard and
rejected writeText promises, setting copiedTarget to true only after a
successful write and preserving a non-success state on failure. Store the
timeout handle and clear it when the component unmounts, using the component’s
existing lifecycle/effect mechanism.
| <p className="text-xs text-muted-foreground"> | ||
| Or point an A record directly | ||
| to target IP:{" "} | ||
| <code className="text-primary font-mono font-bold bg-[#0d0d11] px-1.5 py-0.5 rounded border border-border/50"> | ||
| {serverIp ?? "../.."} | ||
| </code> | ||
| </p> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Replace the "../.." placeholder text.
When serverIp is undefined, the card shows ../.. as the A-record value. Users read this as the DNS target. Show a clear fallback, for example "Loading..." or "Server IP unavailable", and hide the A-record row until the IP resolves.
Also applies to: 140-141
🤖 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/project/domains/DomainDnsCard.tsx` around lines 103 -
109, Update the A-record row near the serverIp display and the related section
around lines 140-141 to avoid rendering "../.." when serverIp is undefined.
Render the row only once serverIp resolves, or show an explicit fallback such as
"Loading..." or "Server IP unavailable" without presenting it as the DNS target.
| <Button | ||
| variant="ghost" | ||
| size="icon" | ||
| className="h-7 w-7 text-muted-foreground hover:text-destructive hover:bg-destructive/10 rounded-md transition-all opacity-80 group-hover:opacity-100" | ||
| onClick={() => | ||
| onRequestDelete(d.id) | ||
| } | ||
| > | ||
| <Trash2 className="h-3.5 w-3.5" /> | ||
| </Button> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Give the icon-only delete button an accessible name.
The button contains only a Trash2 icon. Screen readers announce no label, so the control cannot be identified. Add aria-label={Delete domain ${d.domain}}. The dismiss button in apps/web/src/components/project/domains/DomainDnsCard.tsx at lines 33-40 has the same gap.
♿ Proposed fix
<Button
variant="ghost"
size="icon"
+ aria-label={`Delete domain ${d.domain}`}
className="h-7 w-7 text-muted-foreground hover:text-destructive hover:bg-destructive/10 rounded-md transition-all opacity-80 group-hover:opacity-100"
onClick={() =>
onRequestDelete(d.id)
}
>📝 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.
| <Button | |
| variant="ghost" | |
| size="icon" | |
| className="h-7 w-7 text-muted-foreground hover:text-destructive hover:bg-destructive/10 rounded-md transition-all opacity-80 group-hover:opacity-100" | |
| onClick={() => | |
| onRequestDelete(d.id) | |
| } | |
| > | |
| <Trash2 className="h-3.5 w-3.5" /> | |
| </Button> | |
| <Button | |
| variant="ghost" | |
| size="icon" | |
| aria-label={`Delete domain ${d.domain}`} | |
| className="h-7 w-7 text-muted-foreground hover:text-destructive hover:bg-destructive/10 rounded-md transition-all opacity-80 group-hover:opacity-100" | |
| onClick={() => | |
| onRequestDelete(d.id) | |
| } | |
| > | |
| <Trash2 className="h-3.5 w-3.5" /> | |
| </Button> |
🤖 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/project/domains/DomainTable.tsx` around lines 68 -
77, Add an accessible aria-label to the icon-only delete Button in DomainTable,
using the domain value to identify the target as “Delete domain …”. Apply the
same accessible-name fix to the dismiss button in DomainDnsCard, targeting its
corresponding button element.
| @@ -0,0 +1,523 @@ | |||
| import { useEffect } from 'react'; | |||
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Import useState.
Line 1 imports only useEffect. The component calls useState at lines 28-101. The module fails to compile and the page cannot render.
🐛 Proposed fix
-import { useEffect } from 'react';
+import { useEffect, useState } from 'react';📝 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.
| import { useEffect } from 'react'; | |
| import { useEffect, useState } from 'react'; |
🤖 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 1, Update the React import
in CreateProjectPage to include useState alongside useEffect, matching the hook
used by the component and preserving the existing useEffect import.
| const handleSelectRepo = (repo: GithubRepo | null) => { | ||
| setSelectedRepo(repo); | ||
| if (repo) { | ||
| setRepoUrl(repo.clone_url); | ||
| setRepoBranch(repo.default_branch); | ||
| if (!name) setName(repo.name); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Use the camelCase fields of GithubRepo.
GithubRepo in apps/web/src/types/index.ts declares cloneUrl and defaultBranch. This handler reads repo.clone_url and repo.default_branch. Both resolve to undefined. repoUrl stays empty, so the Git branch at line 274 is skipped and no deployment starts after repository selection.
🐛 Proposed fix
const handleSelectRepo = (repo: GithubRepo | null) => {
setSelectedRepo(repo);
if (repo) {
- setRepoUrl(repo.clone_url);
- setRepoBranch(repo.default_branch);
+ setRepoUrl(repo.cloneUrl);
+ setRepoBranch(repo.defaultBranch);
if (!name) setName(repo.name);
}
};📝 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 handleSelectRepo = (repo: GithubRepo | null) => { | |
| setSelectedRepo(repo); | |
| if (repo) { | |
| setRepoUrl(repo.clone_url); | |
| setRepoBranch(repo.default_branch); | |
| if (!name) setName(repo.name); | |
| } | |
| }; | |
| const handleSelectRepo = (repo: GithubRepo | null) => { | |
| setSelectedRepo(repo); | |
| if (repo) { | |
| setRepoUrl(repo.cloneUrl); | |
| setRepoBranch(repo.defaultBranch); | |
| if (!name) setName(repo.name); | |
| } | |
| }; |
🤖 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 215 - 222, Update
handleSelectRepo to use GithubRepo’s declared camelCase fields cloneUrl and
defaultBranch instead of clone_url and default_branch, ensuring setRepoUrl and
setRepoBranch receive the selected repository values and the existing deployment
flow remains active.
67742f8 to
027e32d
Compare
1 participant
Description
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)Summary by CodeRabbit
New Features
Bug Fixes
Documentation