Skip to content

Move root .md files to specs folder#59

Merged
weroperking merged 7 commits intoweroperking:mainfrom
Helal-maker:main
Apr 1, 2026
Merged

Move root .md files to specs folder#59
weroperking merged 7 commits intoweroperking:mainfrom
Helal-maker:main

Conversation

@Helal-maker
Copy link
Copy Markdown
Contributor

@Helal-maker Helal-maker commented Mar 31, 2026

Summary

  • Move root folder .md documentation files to a dedicated specs/ folder for better organization

Summary by CodeRabbit

  • Documentation

    • Added comprehensive specs and README with architecture, quick-start, and deployment guides.
  • New Features

    • CLI: interactive init now prompts for project name and warns before overwriting.
    • CLI: expanded public/help/version flags.
  • Bug Fixes

    • Improved WebSocket reliability with exponential backoff, error handling, and cleanup.
    • Setup flow now checks setup status and redirects to login when complete.
  • Chores

    • Pinned runtimes/images, enabled CI caching, updated container healthchecks.
    • Dashboard now requires VITE_API_URL via environment.

docker: update MinIO images to latest stable versions
cli: refactor deploy command argument handling for clarity
provider: remove redundant WebSocket reconnection logic
@chatgpt-codex-connector
Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 31, 2026

Walkthrough

Pins Bun and MinIO versions in CI and runtime, streamlines Docker multi-stage builds, tightens nginx healthcheck, makes VITE_API_URL required in TypeScript, refactors several CLI command signatures and init flow, hardens a WebSocket reconnect loop with exponential backoff and cleanup, and adds two large spec/docs files.

Changes

Cohort / File(s) Summary
CI workflow
​.github/workflows/ci.yml
Pin Bun to 1.3.10 and enable cache: true for Bun setup across jobs (test, lint, typecheck).
Dockerfiles & images
Dockerfile.project, docker-compose.yml
Bumped base Bun image to oven/bun:1.3.10-debian; builder stage now inherits from deps and removes duplicate bun install. Pinned MinIO images to bitnami/minio:2025.1.16 and minio/mc:RELEASE.2025-08-13T08-35-41Z; added MINIO_DATA_DIR.
Docker Compose tweaks
docker-compose.dev.yml, docker-compose.self-hosted.yml
Removed top-level version from dev compose; changed nginx healthcheck from nginx -t to an HTTP probe (wget -qO- http://localhost/health).
Type declarations
apps/dashboard/src/vite-env.d.ts
Made VITE_API_URL required on ImportMetaEnv (readonly VITE_API_URL: string).
Dashboard API & guard
apps/dashboard/src/lib/api.ts, apps/dashboard/src/components/auth/SetupGuard.tsx
API_BASE now uses import.meta.env.VITE_API_URL (no localhost fallback). Added export async function checkSetup(): Promise<boolean> and replaced inline fetch in SetupGuard with checkSetup() call.
CLI changes
packages/cli/src/index.ts, packages/cli/src/commands/init.ts
Expanded PUBLIC_COMMANDS to include --version/-v, --help/-h, -V. Reordered/changed .action signatures for init, function deploy, and branch commands; init now prompts for project name and checks/asks before overwriting existing directory.
WebSocket connection logic
packages/client/src/iac/provider.tsx
Replaced one-off WS setup with a connect() flow: exponential backoff reconnect (3s doubling to 30s), reset on open, ws.onerror logging, JSON parse try/catch, and unmount/cleanup guards that cancel timeouts and close socket.
Documentation / specs
specs/CodeRabbit_Full_Codebase_review.md, specs/README.md
Added two large docs: a full-codebase review spec and a comprehensive project README (documentation-only additions).

Sequence Diagram(s)

mermaid
sequenceDiagram
participant UI as Dashboard UI
participant WS as WebSocket
participant Timer as Reconnect Timer
UI->>WS: connect()
WS-->>UI: onopen (set ready, reset delay)
WS->>UI: onmessage (try parse; ignore malformed)
WS-->>UI: onerror (log)
WS-->>UI: onclose (clear wsRef, mark not ready)
UI->>Timer: schedule reconnect (delay * 2 up to 30s)
Timer-->>UI: reconnect trigger -> connect()

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

codex

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning Title only mentions moving .md files to specs, but PR includes significant functional changes (Bun version pinning, Dockerfile refactoring, VITE_API_URL requirement enforcement, CLI command restructuring, WebSocket reconnection logic, init command prompting). Revise title to reflect primary changes: either focus on a single major change or use broader language like 'Dependency updates, config refactoring, and initialization improvements'.
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
Dockerfile.project (1)

18-18: ⚠️ Potential issue | 🟡 Minor

Bun version mismatch between Dockerfile and CI.

Dockerfile uses oven/bun:1.3.9-debian while .github/workflows/ci.yml pins bun-version: "1.3.10". This version drift can cause reproducibility issues between CI and containerized builds.

Proposed fix
-FROM oven/bun:1.3.9-debian AS base
+FROM oven/bun:1.3.10-debian AS base
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Dockerfile.project` at line 18, The Dockerfile's base image tag
"oven/bun:1.3.9-debian" is out of sync with CI's pinned bun-version "1.3.10";
update the FROM image in the Dockerfile to "oven/bun:1.3.10-debian" (or
alternatively update the CI bun-version to 1.3.9) so both sources reference the
same Bun version; locate the FROM line in the Dockerfile and make the version
change to ensure reproducible builds.
docker-compose.yml (1)

34-50: ⚠️ Potential issue | 🟡 Minor

Set MINIO_DATA_DIR=/data environment variable for clarity.

The configuration uses an explicit server /data command which overrides bitnami/minio's entrypoint. While this works, it's non-standard and relies on command override. Bitnami's default data directory is /bitnami/minio/data, not /data. Since the volume mounts to /data, add MINIO_DATA_DIR=/data as an environment variable to make the intent explicit and avoid silent breakage if the command is later removed or modified.

The environment variables MINIO_ROOT_USER and MINIO_ROOT_PASSWORD are correctly supported by bitnami/minio.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docker-compose.yml` around lines 34 - 50, Add the MINIO_DATA_DIR environment
variable to make the custom data path explicit: in the docker-compose service
that uses image bitnami/minio (container_name betterbase-minio-local) and
currently overrides the entrypoint with command "server /data", add
MINIO_DATA_DIR=/data alongside MINIO_ROOT_USER and MINIO_ROOT_PASSWORD so the
Bitnami image knows the intended data directory even if the command override is
removed or changed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/dashboard/src/vite-env.d.ts`:
- Around line 3-5: SetupGuard.tsx currently calls fetch() directly; move that
logic into a new exported helper in src/lib/api.ts (e.g., export async function
checkSetup(): Promise<boolean> or name that fits your API client) that performs
the setup check using the existing API client patterns, then replace the direct
fetch call in SetupGuard to call this new checkSetup helper; while doing this,
remove the redundant fallback operators (the ?? "http://localhost:3001" clauses)
both in api.ts and SetupGuard.tsx since VITE_API_URL is guaranteed by
vite.config.ts and the ImportMetaEnv declaration, and ensure the new helper uses
the shared API utilities/headers consistent with other functions in
src/lib/api.ts.

In `@packages/cli/src/index.ts`:
- Around line 555-561: The branch command currently defines both a positional
argument via .argument("[project-root]", ...) and an option -p/--project-root
via .option("-p, --project-root <path>", ...), causing redundancy; pick one
approach and remove the other to simplify UX: either remove the .option(...)
call and keep the positional argument (then use projectRootArg || process.cwd()
inside the .action) or remove the .argument(...) call and keep .option(...)
(then use options.projectRoot || process.cwd()); update the .action logic around
runBranchCommand(...) accordingly so it reads from the chosen source
(projectRootArg or options.projectRoot) and retains the same precedence and
fallback to process.cwd().

In `@packages/client/src/iac/provider.tsx`:
- Around line 33-44: The provider fails to reset the wsReady state when swapping
sockets, so when config changes the old socket leaves wsReady true and the new
socket's onopen won't update state; fix by explicitly calling setWsReady(false)
when tearing down/replacing the socket and before creating a new one (e.g., in
the cleanup path where isCleaned is set and/or at the start of connect()), and
ensure connect() sets wsRef.current to the new WebSocket only after resetting
wsReady; update the connect(), cleanup logic (isCleaned handling), and any
onclose early-return paths to clear wsReady so the new socket can correctly flip
it to true on open.

In `@specs/CodeRabbit_Full_Codebase_review.md`:
- Around line 782-787: Remove the leaked auto-generated internal state payload
by deleting the entire block between the HTML markers "<!-- internal state start
-->" and "<!-- internal state end -->" (including the large base64-like blob and
any surrounding internal-state comments); ensure the spec only contains
human-readable guidance and re-run any doc linter/CI to confirm no
sensitive/internal dumps remain before committing, and update the commit message
to note removal of leaked internal-state content.

In `@specs/README.md`:
- Line 645: The footer line containing "[Website](#) •
[Documentation](docs/README.md) • [Discord](https://discord.gg/R6Dm6Cgy2E) •
[GitHub](https://github.com/weroperking/Betterbase) • [Twitter](#)" uses '#'
placeholders for Website and Twitter; update those two link targets by replacing
the '#' with the actual URLs if available, or convert the "[Website](#)" and
"[Twitter](#)" entries to plain text labels ("Website" and "Twitter") so they
are not dead links; ensure the existing valid links (Documentation, Discord,
GitHub) and the required references to SELF_HOSTED.md, docs paths, and CLI
sections remain unchanged.
- Around line 594-646: The README in specs/ uses broken relative links (e.g.,
"LICENSE", "CONTRIBUTING.md", "docs/README.md") that now resolve under specs/
after the move; update these to point to repo-root by prefixing "../" (e.g.,
"../LICENSE", "../CONTRIBUTING.md", "../docs/README.md") and scan the file for
other relocated references (SELF_HOSTED.md, docs paths, CLI reference links,
GitHub/issue links) to similarly correct their paths so all links resolve to the
repository root.

---

Outside diff comments:
In `@docker-compose.yml`:
- Around line 34-50: Add the MINIO_DATA_DIR environment variable to make the
custom data path explicit: in the docker-compose service that uses image
bitnami/minio (container_name betterbase-minio-local) and currently overrides
the entrypoint with command "server /data", add MINIO_DATA_DIR=/data alongside
MINIO_ROOT_USER and MINIO_ROOT_PASSWORD so the Bitnami image knows the intended
data directory even if the command override is removed or changed.

In `@Dockerfile.project`:
- Line 18: The Dockerfile's base image tag "oven/bun:1.3.9-debian" is out of
sync with CI's pinned bun-version "1.3.10"; update the FROM image in the
Dockerfile to "oven/bun:1.3.10-debian" (or alternatively update the CI
bun-version to 1.3.9) so both sources reference the same Bun version; locate the
FROM line in the Dockerfile and make the version change to ensure reproducible
builds.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b67572bf-ea8c-4fea-8ef8-5f2f036bc5da

📥 Commits

Reviewing files that changed from the base of the PR and between 748f225 and 6e0ecb0.

📒 Files selected for processing (26)
  • .github/workflows/ci.yml
  • Dockerfile.project
  • apps/dashboard/src/vite-env.d.ts
  • docker-compose.dev.yml
  • docker-compose.self-hosted.yml
  • docker-compose.yml
  • packages/cli/src/index.ts
  • packages/client/src/iac/provider.tsx
  • specs/03_test_suite.md
  • specs/BETTERBASE.md
  • specs/BetterBase_Competitive_Plan.md
  • specs/BetterBase_Dashboard_Backend_Spec.md
  • specs/BetterBase_Dashboard_Frontend_Spec.md
  • specs/BetterBase_IaC_Phase2_Spec.md
  • specs/BetterBase_IaC_Phase3_Spec.md
  • specs/BetterBase_InfraAsCode_Spec.md
  • specs/BetterBase_Inngest_Dashboard_Spec.md
  • specs/BetterBase_Inngest_Spec.md
  • specs/BetterBase_Observability_Spec.docx.md
  • specs/BetterBase_SelfHosted_Spec.md
  • specs/CODEBASE_MAP.md
  • specs/CONTRIBUTING.md
  • specs/CodeRabbit_Full_Codebase_review.md
  • specs/NOTICE.md
  • specs/README.md
  • specs/SELF_HOSTED.md
💤 Files with no reviewable changes (1)
  • docker-compose.dev.yml

Comment on lines 3 to 5
interface ImportMetaEnv {
readonly VITE_API_URL?: string;
readonly VITE_API_URL: string;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if vite.config.ts has env validation
fd -t f 'vite.config' apps/dashboard --exec cat {}

Repository: weroperking/Betterbase

Length of output: 479


🏁 Script executed:

#!/bin/bash
# Check api.ts for VITE_API_URL usage
fd -t f 'api.ts' apps/dashboard/src --exec cat {}

Repository: weroperking/Betterbase

Length of output: 2812


🏁 Script executed:

#!/bin/bash
# Check SetupGuard.tsx for VITE_API_URL usage
fd -t f 'SetupGuard.tsx' apps/dashboard/src --exec cat {}

Repository: weroperking/Betterbase

Length of output: 1046


SetupGuard.tsx violates the API client pattern by using fetch() directly.

Coding guidelines require all server calls to go through src/lib/api.ts. SetupGuard.tsx calls fetch directly instead. Move the setup check into a helper within api.ts and call it from SetupGuard.

The fallback operators (?? "http://localhost:3001") in api.ts and SetupGuard.tsx are redundant—vite.config.ts already provides this fallback via the define option at build time, so the values are always strings at runtime. The type declaration VITE_API_URL: string is correct. Remove the dead code fallbacks once you refactor SetupGuard to use api.ts.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/dashboard/src/vite-env.d.ts` around lines 3 - 5, SetupGuard.tsx
currently calls fetch() directly; move that logic into a new exported helper in
src/lib/api.ts (e.g., export async function checkSetup(): Promise<boolean> or
name that fits your API client) that performs the setup check using the existing
API client patterns, then replace the direct fetch call in SetupGuard to call
this new checkSetup helper; while doing this, remove the redundant fallback
operators (the ?? "http://localhost:3001" clauses) both in api.ts and
SetupGuard.tsx since VITE_API_URL is guaranteed by vite.config.ts and the
ImportMetaEnv declaration, and ensure the new helper uses the shared API
utilities/headers consistent with other functions in src/lib/api.ts.

…ket state, remove leaked content, fix broken links, sync Docker versions
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (1)
specs/CodeRabbit_Full_Codebase_review.md (1)

780-782: ⚠️ Potential issue | 🟠 Major

Remove the leaked opaque payload appended after <!-- tips_end -->.

Line 780 introduces a large non-human-readable blob in a spec artifact. This is high-risk documentation leakage and should be removed entirely.

Proposed fix
-<!-- tips_end -->w9TR9X4poxAIY/gMxeE5AbfQkPDAjesSASxSYorSfIXDVeY50arQ2WQ1Y5rdYrfLrXfTYk...
+<!-- tips_end -->

As per coding guidelines, “verify findings against current code, keep changes minimal.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/CodeRabbit_Full_Codebase_review.md` around lines 780 - 782, There is a
large non-human-readable blob appended immediately after the marker "<!--
tips_end -->"; remove that entire opaque payload so the marker is followed only
by the intended spec text (i.e., delete everything after "<!-- tips_end -->"
until the next valid content boundary), save the cleaned spec, run a quick
search for that blob sequence to ensure no duplicates remain, and commit the
minimal change with a clear message like "Remove leaked opaque payload after
<!-- tips_end -->".
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/dashboard/src/components/auth/SetupGuard.tsx`:
- Line 3: The import for checkSetup in SetupGuard.tsx is using the wrong
relative path ("../lib/api") causing module resolution errors; update the import
to point to the correct module location (import checkSetup from the parent of
components, e.g. "../../lib/api") so that the referenced function checkSetup
resolves correctly in the file that defines the SetupGuard component.

In `@apps/dashboard/src/lib/api.ts`:
- Around line 98-105: The checkSetup function currently treats any non-410
response as "setup complete"; update checkSetup to explicitly handle expected
statuses: return true only when the backend explicitly indicates setup exists
(e.g., res.status === 200), return false when res.status === 410, and throw or
surface an error for any other unexpected status (>=400/500) so callers can
handle backend errors instead of treating them as a completed setup; update the
fetch handling in checkSetup (and use API_BASE/ the same request body) to
implement these explicit branches and include the response status/text in thrown
errors for debugging.

In `@docker-compose.yml`:
- Line 55: The docker-compose image tag for the MinIO client is too new compared
to the server; update the image reference `image:
minio/mc:RELEASE.2025-08-13T08-35-41Z` to a client release closer to the server
(for example `minio/mc:RELEASE.2025-01-16T01-00-30Z`) so `image` in the
docker-compose service matches a compatible mc version with
`bitnami/minio:2025.1.16`.

In `@packages/client/src/iac/provider.tsx`:
- Around line 50-55: The onclose handler leaves wsReady true while clearing
wsRef.current, causing inconsistent state; update the onclose block in the
WebSocket connect logic (the ws.onclose handler) to call setWsReady(false)
immediately (before or alongside wsRef.current = null) and keep existing
reconnect behavior (timeoutId = setTimeout(connect, reconnectDelayMs) and
backoff via reconnectDelayMs = Math.min(reconnectDelayMs * 2,
maxReconnectDelayMs)), preserving the isCleaned early-return; this ensures
wsReady accurately reflects socket readiness during the reconnect delay.

In `@specs/README.md`:
- Around line 266-272: Update the CLI reference table in specs/README.md so it
only lists commands actually registered in the CLI entrypoint
(packages/cli/src/index.ts); specifically remove or replace the `bb iac sync`
and `bb generate types` rows (or change their descriptions to reflect they are
deprecated/disabled) to avoid pointing to non-existent commands, and ensure the
table matches the commands exported/registered by the CLI (refer to the command
registration in packages/cli/src/index.ts to verify exact command names).

---

Duplicate comments:
In `@specs/CodeRabbit_Full_Codebase_review.md`:
- Around line 780-782: There is a large non-human-readable blob appended
immediately after the marker "<!-- tips_end -->"; remove that entire opaque
payload so the marker is followed only by the intended spec text (i.e., delete
everything after "<!-- tips_end -->" until the next valid content boundary),
save the cleaned spec, run a quick search for that blob sequence to ensure no
duplicates remain, and commit the minimal change with a clear message like
"Remove leaked opaque payload after <!-- tips_end -->".
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 38893a76-8709-4da3-9e6f-16c464d3aacd

📥 Commits

Reviewing files that changed from the base of the PR and between 6e0ecb0 and 26c7ffd.

📒 Files selected for processing (8)
  • Dockerfile.project
  • apps/dashboard/src/components/auth/SetupGuard.tsx
  • apps/dashboard/src/lib/api.ts
  • docker-compose.yml
  • packages/cli/src/index.ts
  • packages/client/src/iac/provider.tsx
  • specs/CodeRabbit_Full_Codebase_review.md
  • specs/README.md

Comment on lines +98 to +105
export async function checkSetup(): Promise<boolean> {
const res = await fetch(`${API_BASE}/admin/auth/setup`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ _check: true }),
});
return res.status !== 410;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Handle setup-check statuses explicitly instead of status !== 410.

Line 104 currently maps any non-410 response (including 500/404) to true, which can incorrectly force login redirects during backend errors.

Proposed fix
 export async function checkSetup(): Promise<boolean> {
 	const res = await fetch(`${API_BASE}/admin/auth/setup`, {
 		method: "POST",
 		headers: { "Content-Type": "application/json" },
 		body: JSON.stringify({ _check: true }),
 	});
-	return res.status !== 410;
+	if (res.status === 410) return false; // setup not required anymore
+	if (res.ok) return true; // setup required
+	throw new ApiError(res.status, "Failed to verify setup status");
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/dashboard/src/lib/api.ts` around lines 98 - 105, The checkSetup function
currently treats any non-410 response as "setup complete"; update checkSetup to
explicitly handle expected statuses: return true only when the backend
explicitly indicates setup exists (e.g., res.status === 200), return false when
res.status === 410, and throw or surface an error for any other unexpected
status (>=400/500) so callers can handle backend errors instead of treating them
as a completed setup; update the fetch handling in checkSetup (and use API_BASE/
the same request body) to implement these explicit branches and include the
response status/text in thrown errors for debugging.

# MinIO Init (Create bucket on startup)
minio-init:
image: minio/mc:latest
image: minio/mc:RELEASE.2025-08-13T08-35-41Z
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

mc client version significantly newer than MinIO server.

minio/mc:RELEASE.2025-08-13T08-35-41Z (August 2025) is 7 months newer than the server image bitnami/minio:2025.1.16 (January 2025). While mc is generally backward compatible, using a much newer client with an older server can cause unexpected behavior with newer API features.

Pin mc to a version closer to the server release, e.g., minio/mc:RELEASE.2025-01-16T01-00-30Z.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docker-compose.yml` at line 55, The docker-compose image tag for the MinIO
client is too new compared to the server; update the image reference `image:
minio/mc:RELEASE.2025-08-13T08-35-41Z` to a client release closer to the server
(for example `minio/mc:RELEASE.2025-01-16T01-00-30Z`) so `image` in the
docker-compose service matches a compatible mc version with
`bitnami/minio:2025.1.16`.

Comment on lines +50 to +55
ws.onclose = () => {
if (isCleaned) return;
wsRef.current = null;
timeoutId = setTimeout(connect, reconnectDelayMs);
reconnectDelayMs = Math.min(reconnectDelayMs * 2, maxReconnectDelayMs);
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

onclose should call setWsReady(false) to avoid inconsistent state during reconnect delay.

When the socket closes unexpectedly (network drop, server restart), onclose sets wsRef.current = null but leaves wsReady === true. During the reconnect delay (3–30 seconds), consumers see ws: null, wsReady: true. Current hook code guards with !ws || !wsReady, but future consumers might trust wsReady alone.

Proposed fix
 ws.onclose = () => {
   if (isCleaned) return;
+  setWsReady(false);
   wsRef.current = null;
   timeoutId = setTimeout(connect, reconnectDelayMs);
   reconnectDelayMs = Math.min(reconnectDelayMs * 2, maxReconnectDelayMs);
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ws.onclose = () => {
if (isCleaned) return;
wsRef.current = null;
timeoutId = setTimeout(connect, reconnectDelayMs);
reconnectDelayMs = Math.min(reconnectDelayMs * 2, maxReconnectDelayMs);
};
ws.onclose = () => {
if (isCleaned) return;
setWsReady(false);
wsRef.current = null;
timeoutId = setTimeout(connect, reconnectDelayMs);
reconnectDelayMs = Math.min(reconnectDelayMs * 2, maxReconnectDelayMs);
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/client/src/iac/provider.tsx` around lines 50 - 55, The onclose
handler leaves wsReady true while clearing wsRef.current, causing inconsistent
state; update the onclose block in the WebSocket connect logic (the ws.onclose
handler) to call setWsReady(false) immediately (before or alongside
wsRef.current = null) and keep existing reconnect behavior (timeoutId =
setTimeout(connect, reconnectDelayMs) and backoff via reconnectDelayMs =
Math.min(reconnectDelayMs * 2, maxReconnectDelayMs)), preserving the isCleaned
early-return; this ensures wsReady accurately reflects socket readiness during
the reconnect delay.

Comment on lines +266 to +272
| `bb init [name]` | Create new project |
| `bb dev` | Start dev server |
| `bb iac sync` | Sync IaC schema |
| `bb iac analyze` | Analyze query performance |
| `bb migrate` | Run migrations |
| `bb generate types` | Generate TypeScript types |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

CLI reference lists commands not registered in current CLI.

Lines 268 and 272 reference bb iac sync and bb generate types, but current command registration in packages/cli/src/index.ts does not expose these commands. Update this table to avoid broken guidance.

Proposed fix
-| `bb iac sync` | Sync IaC schema |
+| `bb iac export` | Export project data |
 | `bb iac analyze` | Analyze query performance |
 | `bb migrate` | Run migrations |
-| `bb generate types` | Generate TypeScript types |
+| `bb generate crud <table-name>` | Generate CRUD routes for a table |

As per coding guidelines, “moved docs should not contradict.”

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@specs/README.md` around lines 266 - 272, Update the CLI reference table in
specs/README.md so it only lists commands actually registered in the CLI
entrypoint (packages/cli/src/index.ts); specifically remove or replace the `bb
iac sync` and `bb generate types` rows (or change their descriptions to reflect
they are deprecated/disabled) to avoid pointing to non-existent commands, and
ensure the table matches the commands exported/registered by the CLI (refer to
the command registration in packages/cli/src/index.ts to verify exact command
names).

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/cli/src/commands/init.ts (1)

186-186: ⚠️ Potential issue | 🟠 Major

Violates invariant: hardcoded "bun" in spawn call.

Coding guidelines state: "Never use 'bun' as a string in spawn() calls. Always use process.execPath." This breaks when bun isn't in PATH (e.g., global installs).

Proposed fix
 async function installDependencies(projectPath: string): Promise<void> {
-	const installProcess = Bun.spawn(["bun", "install"], {
+	const installProcess = Bun.spawn([process.execPath, "install"], {
 		cwd: projectPath,
 		stdout: "inherit",
 		stderr: "inherit",

As per coding guidelines: "Never use 'bun' as a string in spawn() calls. Always use process.execPath. Reason: PATH may not have bun when running global installs."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/init.ts` at line 186, The spawn call uses a
hardcoded "bun" string which violates the guideline; update the Bun.spawn
invocation that creates installProcess so it calls the current runtime
executable via process.execPath (e.g., replace the first arg "bun" with
process.execPath) and keep the remaining args (like "install") unchanged so the
command runs even when bun isn't in PATH; locate the Bun.spawn call that
constructs installProcess in init.ts and make this substitution.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/dashboard/src/components/auth/SetupGuard.tsx`:
- Around line 10-13: The redirect condition is inverted: checkSetup() returns
false on HTTP 410 but the code currently redirects when isSetup is true; update
the guard in SetupGuard (the block handling checkSetup().then(...)) to redirect
to "/login" when isSetup is falsy (i.e., change the condition to check for
!isSetup) so the redirect happens only on the 410 case; ensure you keep the
navigate("/login", { replace: true }) call and preserve surrounding promise
handling in checkSetup().

In `@packages/cli/src/commands/init.ts`:
- Around line 1313-1318: The CLI project name argument is being ignored when
running `bb init` in IaC mode; update the prompt usage so it uses the provided
CLI argument as the initial value (or skips prompting when appropriate). Locate
the prompt call (prompts.text) and the projectName handling (projectNameInput,
projectNameSchema.parse) and change the initial property to the CLI-provided
name (e.g., use projectNameArg / args.projectName / argvProjectName if present)
or bypass the prompt and directly validate with projectNameSchema.parse when the
argument is supplied.
- Around line 1324-1335: The directory existence check using
Bun.file(projectPath).exists() fails for directories; update the check to use
the already-imported fs/promises API (e.g., stat/access) instead of
Bun.file().exists(): replace the existingDir assignment and conditional that
references existingDir, projectPath and projectName so it correctly detects
directories and still prompts/aborts on overwrite in the same logic flow used in
the init command.

---

Outside diff comments:
In `@packages/cli/src/commands/init.ts`:
- Line 186: The spawn call uses a hardcoded "bun" string which violates the
guideline; update the Bun.spawn invocation that creates installProcess so it
calls the current runtime executable via process.execPath (e.g., replace the
first arg "bun" with process.execPath) and keep the remaining args (like
"install") unchanged so the command runs even when bun isn't in PATH; locate the
Bun.spawn call that constructs installProcess in init.ts and make this
substitution.
🪄 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: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ae17f3af-58ef-491d-924c-74b0ed4aae75

📥 Commits

Reviewing files that changed from the base of the PR and between 26c7ffd and 13a3086.

📒 Files selected for processing (2)
  • apps/dashboard/src/components/auth/SetupGuard.tsx
  • packages/cli/src/commands/init.ts

Comment on lines +10 to 13
checkSetup()
.then((isSetup: boolean) => {
if (isSetup) {
navigate("/login", { replace: true });
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Redirect condition is inverted for SetupGuard’s required 410 behavior.

checkSetup() returns false on HTTP 410, but this guard redirects when isSetup is true, so redirect currently happens on non-410 responses.

Proposed fix
-		checkSetup()
-			.then((isSetup: boolean) => {
-				if (isSetup) {
+		checkSetup()
+			.then((isSetup: boolean) => {
+				if (!isSetup) {
 					navigate("/login", { replace: true });
 				}
 				setChecking(false);
 			})

As per coding guidelines, "SetupGuard checks /admin/auth/setup — redirects to /login on 410."

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
checkSetup()
.then((isSetup: boolean) => {
if (isSetup) {
navigate("/login", { replace: true });
checkSetup()
.then((isSetup: boolean) => {
if (!isSetup) {
navigate("/login", { replace: true });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/dashboard/src/components/auth/SetupGuard.tsx` around lines 10 - 13, The
redirect condition is inverted: checkSetup() returns false on HTTP 410 but the
code currently redirects when isSetup is true; update the guard in SetupGuard
(the block handling checkSetup().then(...)) to redirect to "/login" when isSetup
is falsy (i.e., change the condition to check for !isSetup) so the redirect
happens only on the 410 case; ensure you keep the navigate("/login", { replace:
true }) call and preserve surrounding promise handling in checkSetup().

Comment on lines +1313 to 1318
// Prompt for project name with validation
const projectNameInput = await prompts.text({
message: "What is your project name?",
initial: "my-betterbase-app",
});
const projectName = projectNameSchema.parse(projectNameInput);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

CLI argument projectName is ignored in IaC mode.

The [project-name] argument passed via bb init my-project is discarded. The prompt always starts with "my-betterbase-app" regardless of what was provided on the command line.

Proposed fix
-		// Prompt for project name with validation
-		const projectNameInput = await prompts.text({
-			message: "What is your project name?",
-			initial: "my-betterbase-app",
-		});
+		// Prompt for project name with validation (use CLI arg as initial if provided)
+		const projectNameInput = options.projectName ?? await prompts.text({
+			message: "What is your project name?",
+			initial: "my-betterbase-app",
+		});
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/init.ts` around lines 1313 - 1318, The CLI project
name argument is being ignored when running `bb init` in IaC mode; update the
prompt usage so it uses the provided CLI argument as the initial value (or skips
prompting when appropriate). Locate the prompt call (prompts.text) and the
projectName handling (projectNameInput, projectNameSchema.parse) and change the
initial property to the CLI-provided name (e.g., use projectNameArg /
args.projectName / argvProjectName if present) or bypass the prompt and directly
validate with projectNameSchema.parse when the argument is supplied.

Comment on lines +1324 to +1335
// Check if directory already exists
const existingDir = await Bun.file(projectPath).exists();
if (existingDir) {
const overwrite = await prompts.confirm({
message: `Directory "${projectName}" already exists. Overwrite?`,
default: false,
});
if (!overwrite) {
logger.info("Aborted. Choose a different project name.");
process.exit(0);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Bun.file().exists() does not detect directories.

Bun.file(projectPath).exists() returns false for directories—it only detects regular files. The overwrite prompt will never trigger for existing project directories.

Proposed fix using node:fs/promises (already imported)
 			// Check if directory already exists
-			const existingDir = await Bun.file(projectPath).exists();
-			if (existingDir) {
+			let directoryExists = false;
+			try {
+				const stat = await import("node:fs/promises").then(fs => fs.stat(projectPath));
+				directoryExists = stat.isDirectory();
+			} catch {
+				// ENOENT means doesn't exist, which is fine
+			}
+			if (directoryExists) {
 				const overwrite = await prompts.confirm({
 					message: `Directory "${projectName}" already exists. Overwrite?`,
 					default: false,

Or more concisely since fs/promises is already imported at top:

+import { stat } from "node:fs/promises";
 // ... later in the function:
 			// Check if directory already exists
-			const existingDir = await Bun.file(projectPath).exists();
-			if (existingDir) {
+			const existingDir = await stat(projectPath).then(() => true, () => false);
+			if (existingDir) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Check if directory already exists
const existingDir = await Bun.file(projectPath).exists();
if (existingDir) {
const overwrite = await prompts.confirm({
message: `Directory "${projectName}" already exists. Overwrite?`,
default: false,
});
if (!overwrite) {
logger.info("Aborted. Choose a different project name.");
process.exit(0);
}
}
// Check if directory already exists
let directoryExists = false;
try {
const stat = await import("node:fs/promises").then(fs => fs.stat(projectPath));
directoryExists = stat.isDirectory();
} catch {
// ENOENT means doesn't exist, which is fine
}
if (directoryExists) {
const overwrite = await prompts.confirm({
message: `Directory "${projectName}" already exists. Overwrite?`,
default: false,
});
if (!overwrite) {
logger.info("Aborted. Choose a different project name.");
process.exit(0);
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/cli/src/commands/init.ts` around lines 1324 - 1335, The directory
existence check using Bun.file(projectPath).exists() fails for directories;
update the check to use the already-imported fs/promises API (e.g., stat/access)
instead of Bun.file().exists(): replace the existingDir assignment and
conditional that references existingDir, projectPath and projectName so it
correctly detects directories and still prompts/aborts on overwrite in the same
logic flow used in the init command.

@weroperking weroperking merged commit 218fc00 into weroperking:main Apr 1, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants