Skip to content

Project setup: Next.js, Clean Architecture, Husky, CI, SonarCloud#3

Merged
damnthonyy merged 23 commits into
mainfrom
feat/init-project
Mar 10, 2026
Merged

Project setup: Next.js, Clean Architecture, Husky, CI, SonarCloud#3
damnthonyy merged 23 commits into
mainfrom
feat/init-project

Conversation

@damnthonyy

@damnthonyy damnthonyy commented Mar 8, 2026

Copy link
Copy Markdown
Member

PR Type

chore

Summary

Project setup: Next.js (App Router), architecture (api + gateways), quality (Husky, Prettier, lint-staged), CI (lint, format, typecheck, build, SonarCloud), and documentation.

Description

  • Stack: Next.js 16, React 19, TypeScript, Tailwind, pnpm.
  • Architecture: src/architecture/api (HTTP client, WebSocket client, types); src/architecture/gateways for API calls; src/types for shared generics.
  • Quality: Husky (pre-commit = lint-staged, pre-push = block direct push to main/develop), Prettier, ESLint.
  • CI: workflow on push/PR to main and develop — lint, format check, typecheck, build; SonarCloud job after check succeeds.
  • UI: home page, not-found page, components/ui and components/dashboard structure; components readme (shadcn, Tremor, Figma MCP).
  • Docs: README (run + structure), docs/architecture.md, docs/dependance.md, docs/sonar.md, PR template.

No product features: foundation only for the robot dashboard.

What's Changed

  • Next.js bootstrap, folder structure (app, architecture, components, types)
  • API clients (HTTP + WebSocket) and shared types
  • Husky, lint-staged, Prettier, pre-push (block main/develop)
  • CI (lint, format, typecheck, build) + SonarCloud job
  • Not-found page, README and docs (architecture, dependencies, Sonar)
  • PR template and components readme (shadcn, Tremor, Figma MCP)

Screen

N/A — no significant UI change (test page + 404).

@damnthonyy damnthonyy added the chore Maintenance ,Config, deps, tooling label Mar 8, 2026
@damnthonyy

Copy link
Copy Markdown
Member Author

/review

@qodo-code-review

qodo-code-review Bot commented Mar 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (6) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Reconnect blocked after disconnect 🐞 Bug ✓ Correctness ⭐ New
Description
WebSocketClient.disconnect() sets reconnectAttempts to maxReconnectAttempts, which prevents
any subsequent unclean close (after a later connect()) from scheduling reconnect attempts until an
onopen successfully resets the counter. This can cause the client to stop reconnecting after a
normal disconnect/reconnect cycle if the next connection fails before opening.
Code

src/architecture/api/ws.client.ts[R109-118]

+  disconnect(): void {
+    if (this.reconnectTimeoutId) {
+      clearTimeout(this.reconnectTimeoutId);
+      this.reconnectTimeoutId = null;
+    }
+    this.reconnectAttempts = this.config.maxReconnectAttempts;
+    if (this.socket) {
+      this.socket.close(1000, 'Client disconnect');
+      this.socket = null;
+    }
Evidence
disconnect() intentionally forces reconnectAttempts to the max, and the reconnect logic checks
reconnectAttempts < maxReconnectAttempts, so a later connection that closes before onopen runs
will never attempt to reconnect. The same counter is also used to label the state as reconnecting
while CONNECTING, which becomes inaccurate after a manual disconnect if the instance is reused.

src/architecture/api/ws.client.ts[88-101]
src/architecture/api/ws.client.ts[109-119]
src/architecture/api/ws.client.ts[47-56]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`WebSocketClient.disconnect()` sets `reconnectAttempts` to `maxReconnectAttempts`, which blocks future reconnect scheduling (`reconnectAttempts &lt; maxReconnectAttempts`) after a later `connect()` if that new connection closes before `onopen` resets the counter. This makes the instance unsafe to reuse across disconnect/connect cycles.

### Issue Context
- `reconnectAttempts` is used both for state labeling (`connecting` vs `reconnecting`) and for enforcing the max retry limit.
- `disconnect()` should prevent reconnect for that specific close, but should not poison the instance for subsequent manual `connect()` calls.

### Fix Focus Areas
- src/architecture/api/ws.client.ts[47-56]
- src/architecture/api/ws.client.ts[88-101]
- src/architecture/api/ws.client.ts[109-119]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Fragile 401 redirect test🐞 Bug ✓ Correctness
Description
The 401 redirect test stubs globalThis.location, but production redirects via
globalThis.window.location.href; this mismatch is brittle in jsdom and can cause the assertion to
fail or trigger unintended navigation behavior in tests/CI.
Code

src/architecture/api/client.test.ts[R226-249]

+    it('redirects to /login on 401 when window is defined', async () => {
+      const locationHref = vi.fn();
+      Object.defineProperty(globalThis, 'location', {
+        value: {
+          ...globalThis.location,
+          get href() {
+            return '';
+          },
+          set href(v: string) {
+            locationHref(v);
+          },
+        },
+        configurable: true,
+      });
+
+      mockFetch.mockResolvedValue(
+        new Response(JSON.stringify({ message: 'Unauthorized' }), {
+          status: 401,
+        })
+      );
+
+      await expect(apiClient.get('protected')).rejects.toThrow(HttpError);
+      expect(locationHref).toHaveBeenCalledWith('/login');
+    });
Evidence
Production code performs redirects through window.location, while the test intercepts only
globalThis.location. Under Vitest’s jsdom environment, window exists, so the redirect path is
exercised, but the stub may not intercept the actual write target (window.location.href).

src/architecture/api/client.ts[65-70]
src/architecture/api/client.test.ts[226-249]
vitest.config.ts[4-9]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The 401 redirect behavior is implemented via `globalThis.window.location.href`, but the corresponding unit test stubs `globalThis.location`. In jsdom this is brittle and can fail to intercept the redirect.
### Issue Context
This impacts CI stability because the test may not reliably observe the redirect call.
### Fix Focus Areas
- src/architecture/api/client.ts[65-70]
- src/architecture/api/client.test.ts[226-249]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. API base URL not normalized🐞 Bug ✓ Correctness
Description
API requests are built with new URL(url, API_BASE_URL) without normalizing NEXT_PUBLIC_API_URL;
if the env var is set without a trailing slash (common), relative paths can resolve to the wrong
endpoint (dropping /api).
Code

src/architecture/api/client.ts[R1-2]

+const API_BASE_URL =
+  process.env.NEXT_PUBLIC_API_URL || 'http://localhost:3001/api/';
Evidence
The client uses NEXT_PUBLIC_API_URL directly as the base for new URL(...). The repository’s
.env.example provides no guidance on required trailing slash, making misconfiguration likely. With
URL resolution rules, a base ending in /api (no trailing slash) resolves users as a sibling path
(dropping /api).

src/architecture/api/client.ts[1-2]
src/architecture/api/client.ts[21-26]
.env.example[1-3]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`API_BASE_URL` is used as-is for `new URL(url, API_BASE_URL)`. If `NEXT_PUBLIC_API_URL` is set without a trailing slash (e.g. `http://host/api`), URL resolution can drop the `/api` segment when resolving relative paths.
### Issue Context
`.env.example` does not guide developers on whether `NEXT_PUBLIC_API_URL` must end with `/`, increasing the chance of incorrect runtime behavior.
### Fix Focus Areas
- src/architecture/api/client.ts[1-2]
- src/architecture/api/client.ts[21-26]
- .env.example[1-3]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. API tests env-dependent 🐞 Bug ⛯ Reliability ⭐ New
Description
client.test.ts hardcodes the expected base URL while client.ts reads
process.env.NEXT_PUBLIC_API_URL at module import time, so tests will fail when that env var is set
(or changed) in the test process. Because the module is imported at the top of the test file, later
env changes cannot affect apiClient.baseURL without resetting modules.
Code

src/architecture/api/client.test.ts[R1-21]

+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+import { HttpError, apiClient } from './client';
+
+const baseUrl = 'http://localhost:3001/api/';
+let mockFetch: ReturnType<typeof vi.fn>;
+
+describe('APIClient', () => {
+  beforeEach(() => {
+    mockFetch = vi.fn();
+    vi.stubGlobal('fetch', mockFetch);
+  });
+
+  afterEach(() => {
+    vi.unstubAllGlobals();
+  });
+
+  describe('baseURL', () => {
+    it('returns the API base URL', () => {
+      expect(apiClient.baseURL).toBe(baseUrl);
+    });
+  });
Evidence
The production client’s base URL is computed from process.env.NEXT_PUBLIC_API_URL when the module
is loaded, but the tests assert it must equal the default http://localhost:3001/api/. The test
suite already demonstrates that process.env.NEXT_PUBLIC_API_URL is mutable and used during tests
(in ws.client.test.ts), so relying on it being unset makes client.test.ts fragile.

src/architecture/api/client.ts[5-7]
src/architecture/api/client.test.ts[1-21]
src/architecture/api/ws.client.test.ts[90-115]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`client.test.ts` assumes `NEXT_PUBLIC_API_URL` is unset and asserts the default base URL, but `client.ts` reads that env var at module import time. If the test process has `NEXT_PUBLIC_API_URL` set, the tests become non-deterministic.

### Issue Context
- `API_BASE_URL` is computed once at module load.
- The test imports `apiClient` at the top-level, so adjusting `process.env` in `beforeEach` won’t affect `API_BASE_URL` unless modules are reset and re-imported.

### Fix Focus Areas
- src/architecture/api/client.ts[5-7]
- src/architecture/api/client.test.ts[1-21]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. WebSocket connect not idempotent🐞 Bug ⛯ Reliability
Description
WebSocketClient.connect() only guards against an already OPEN socket, not CONNECTING; repeated calls
while connecting can create multiple sockets and duplicate event handlers.
Code

src/architecture/api/ws.client.ts[R63-68]

+  connect(): void {
+    if (globalThis.window === undefined) return;
+    if (this.socket?.readyState === WebSocket.OPEN) return;
+
+    const url = `${this.baseUrl}/${this.config.path.replace(/^\//, '')}`;
+    this.socket = new WebSocket(url);
Evidence
The connect guard checks only WebSocket.OPEN. If connect is called twice before the first socket
opens (readyState CONNECTING), a second WebSocket will be created.

src/architecture/api/ws.client.ts[63-69]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`connect()` can create multiple WebSockets if called repeatedly while the existing socket is still CONNECTING.
### Issue Context
This can happen in real usage due to repeated UI lifecycle calls (e.g., effects re-running) and may cause duplicated messages/callbacks.
### Fix Focus Areas
- src/architecture/api/ws.client.ts[63-68]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. .env.example invalid comment🐞 Bug ⛯ Reliability
Description
.env.example starts with // which is not standard dotenv comment syntax; copying it to .env can
cause confusing parsing behavior during setup.
Code

.env.example[R1-3]

+// FRONT END
+NEXT_PUBLIC_API_URL=
+NEXT_PUBLIC_FRONT_URL=
Evidence
The example env file uses // instead of a standard env-file comment style, increasing setup
friction and potential misreads of the example configuration.

.env.example[1-3]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`.env.example` uses `//` for comments, which is not the typical format expected in env files.
### Issue Context
Developers commonly copy `.env.example` to `.env`; keeping the example syntactically conventional avoids setup issues.
### Fix Focus Areas
- .env.example[1-3]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Grey Divider

Previous review results

Review updated until commit b43c82b

Results up to commit N/A


🐞 Bugs (4) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider
Action required
1. Fragile 401 redirect test🐞 Bug ✓ Correctness
Description
The 401 redirect test stubs globalThis.location, but production redirects via
globalThis.window.location.href; this mismatch is brittle in jsdom and can cause the assertion to
fail or trigger unintended navigation behavior in tests/CI.
Code

src/architecture/api/client.test.ts[R226-249]

+    it('redirects to /login on 401 when window is defined', async () => {
+      const locationHref = vi.fn();
+      Object.defineProperty(globalThis, 'location', {
+        value: {
+          ...globalThis.location,
+          get href() {
+            return '';
+          },
+          set href(v: string) {
+            locationHref(v);
+          },
+        },
+        configurable: true,
+      });
+
+      mockFetch.mockResolvedValue(
+        new Response(JSON.stringify({ message: 'Unauthorized' }), {
+          status: 401,
+        })
+      );
+
+      await expect(apiClient.get('protected')).rejects.toThrow(HttpError);
+      expect(locationHref).toHaveBeenCalledWith('/login');
+    });
Evidence
Production code performs redirects through window.location, while the test intercepts only
globalThis.location. Under Vitest’s jsdom environment, window exists, so the redirect path is
exercised, but the stub may not intercept the actual write target (window.location.href).

src/architecture/api/client.ts[65-70]
src/architecture/api/client.test.ts[226-249]
vitest.config.ts[4-9]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The 401 redirect behavior is implemented via `globalThis.window.location.href`, but the corresponding unit test stubs `globalThis.location`. In jsdom this is brittle and can fail to intercept the redirect.
### Issue Context
This impacts CI stability because the test may not reliably observe the redirect call.
### Fix Focus Areas
- src/architecture/api/client.ts[65-70]
- src/architecture/api/client.test.ts[226-249]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. API base URL not normalized🐞 Bug ✓ Correctness
Description
API requests are built with new URL(url, API_BASE_URL) without normalizing NEXT_PUBLIC_API_URL;
if the env var is set without a trailing slash (common), relative paths can resolve to the wrong
endpoint (dropping /api).
Code

src/architecture/api/client.ts[R1-2]

+const API_BASE_URL =
+  process.env.NEXT_PUBLIC_API_URL || 'http://localhost:3001/api/';
Evidence
The client uses NEXT_PUBLIC_API_URL directly as the base for new URL(...). The repository’s
.env.example provides no guidance on required trailing slash, making misconfiguration likely. With
URL resolution rules, a base ending in /api (no trailing slash) resolves users as a sibling path
(dropping /api).

src/architecture/api/client.ts[1-2]
src/architecture/api/client.ts[21-26]
.env.example[1-3]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`API_BASE_URL` is used as-is for `new URL(url, API_BASE_URL)`. If `NEXT_PUBLIC_API_URL` is set without a trailing slash (e.g. `http://host/api`), URL resolution can drop the `/api` segment when resolving relative paths.
### Issue Context
`.env.example` does not guide developers on whether `NEXT_PUBLIC_API_URL` must end with `/`, increasing the chance of incorrect runtime behavior.
### Fix Focus Areas
- src/architecture/api/client.ts[1-2]
- src/architecture/api/client.ts[21-26]
- .env.example[1-3]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended
3. WebSocket connect not idempotent🐞 Bug ⛯ Reliability
Description
WebSocketClient.connect() only guards against an already OPEN socket, not CONNECTING; repeated calls
while connecting can create multiple sockets and duplicate event handlers.
Code

src/architecture/api/ws.client.ts[R63-68]

+  connect(): void {
+    if (globalThis.window === undefined) return;
+    if (this.socket?.readyState === WebSocket.OPEN) return;
+
+    const url = `${this.baseUrl}/${this.config.path.replace(/^\//, '')}`;
+    this.socket = new WebSocket(url);
Evidence
The connect guard checks only WebSocket.OPEN. If connect is called twice before the first socket
opens (readyState CONNECTING), a second WebSocket will be created.

src/architecture/api/ws.client.ts[63-69]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`connect()` can create multiple WebSockets if called repeatedly while the existing socket is still CONNECTING.
### Issue Context
This can happen in real usage due to repeated UI lifecycle calls (e.g., effects re-running) and may cause duplicated messages/callbacks.
### Fix Focus Areas
- src/architecture/api/ws.client.ts[63-68]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. .env.example invalid comment🐞 Bug ⛯ Reliability
Description
.env.example starts with // which is not standard dotenv comment syntax; copying it to .env can
cause confusing parsing behavior during setup.
Code

.env.example[R1-3]

+// FRONT END
+NEXT_PUBLIC_API_URL=
+NEXT_PUBLIC_FRONT_URL=
Evidence
The example env file uses // instead of a standard env-file comment style, increasing setup
friction and potential misreads of the example configuration.

.env.example[1-3]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`.env.example` uses `//` for comments, which is not the typical format expected in env files.
### Issue Context
Developers commonly copy `.env.example` to `.env`; keeping the example syntactically conventional avoids setup issues.
### Fix Focus Areas
- .env.example[1-3]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider Grey Divider

Qodo Logo

Comment thread src/architecture/api/client.test.ts
Comment thread src/architecture/api/client.ts Outdated
- Add URL normalization utility to ensure consistent trailing slashes
- Simplify 401 error handling and redirect logic
- Prevent duplicate WebSocket connections during CONNECTING state
- Update test mocking approach for location object
- Set default API URL in environment example
@sonarqubecloud

sonarqubecloud Bot commented Mar 9, 2026

Copy link
Copy Markdown

@damnthonyy

Copy link
Copy Markdown
Member Author

/review

@qodo-code-review

qodo-code-review Bot commented Mar 9, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit b43c82b

Comment on lines +109 to +118
disconnect(): void {
if (this.reconnectTimeoutId) {
clearTimeout(this.reconnectTimeoutId);
this.reconnectTimeoutId = null;
}
this.reconnectAttempts = this.config.maxReconnectAttempts;
if (this.socket) {
this.socket.close(1000, 'Client disconnect');
this.socket = null;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Reconnect blocked after disconnect 🐞 Bug ✓ Correctness

WebSocketClient.disconnect() sets reconnectAttempts to maxReconnectAttempts, which prevents
any subsequent unclean close (after a later connect()) from scheduling reconnect attempts until an
onopen successfully resets the counter. This can cause the client to stop reconnecting after a
normal disconnect/reconnect cycle if the next connection fails before opening.
Agent Prompt
### Issue description
`WebSocketClient.disconnect()` sets `reconnectAttempts` to `maxReconnectAttempts`, which blocks future reconnect scheduling (`reconnectAttempts < maxReconnectAttempts`) after a later `connect()` if that new connection closes before `onopen` resets the counter. This makes the instance unsafe to reuse across disconnect/connect cycles.

### Issue Context
- `reconnectAttempts` is used both for state labeling (`connecting` vs `reconnecting`) and for enforcing the max retry limit.
- `disconnect()` should prevent reconnect for that specific close, but should not poison the instance for subsequent manual `connect()` calls.

### Fix Focus Areas
- src/architecture/api/ws.client.ts[47-56]
- src/architecture/api/ws.client.ts[88-101]
- src/architecture/api/ws.client.ts[109-119]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@damnthonyy damnthonyy merged commit a57b04d into main Mar 10, 2026
3 checks passed
@damnthonyy damnthonyy deleted the feat/init-project branch March 10, 2026 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance ,Config, deps, tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants