Skip to content

refactor: generalize environment routes into individual handlers#568

Merged
kmendell merged 16 commits intomainfrom
refactor/environment-routes
Sep 25, 2025
Merged

refactor: generalize environment routes into individual handlers#568
kmendell merged 16 commits intomainfrom
refactor/environment-routes

Conversation

@kmendell
Copy link
Member

@kmendell kmendell commented Sep 24, 2025

This PR simplifies the /environments route. Most all of the routes now use /environments/ for accessing the api.

In a future PR this will allow settings management, template management etc on remote environments

Todos:

  • Fix Websockets for stats/logs
  • Cleanup commented code
  • Finish migrating frontend services

Summary by CodeRabbit

  • New Features
    • Environment-scoped API and UI: containers, images, networks, volumes, projects, system, updater under /environments/:id/.
    • Added status/count endpoints for containers, networks, projects, and volumes.
    • Live system stats over WebSocket and Docker run → Compose conversion endpoint.
  • Changes
    • Updated WebSocket stats path to /environments/:id/system/stats/ws.
    • Some legacy endpoints removed or relocated; environment management focuses on core CRUD.
    • Frontend migrated to dedicated service modules (no functional changes).
  • Chores
    • Faster dev builds and hot-reload workflow.
    • Enhanced WebSocket proxy logging.

@kmendell kmendell marked this pull request as ready for review September 25, 2025 18:25
@kmendell
Copy link
Member Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

📝 Walkthrough

Walkthrough

Backend routes are namespaced under /environments/:id, with new counts and system stats WS endpoints. Environment proxy middleware is added; EnvironmentHandler is simplified. ConverterService is removed; parsing/conversion moved into SystemService and utils. Frontend replaces monolithic API with feature-specific services and updates components/routes accordingly. Dev Dockerfile optimized.

Changes

Cohort / File(s) Summary
Backend API routing restructure
backend/internal/api/container_handler.go, .../image_handler.go, .../image_update_handler.go, .../network_handler.go, .../project_handler.go, .../system_handler.go, .../updater_handler.go, .../volume_handler.go
Routes moved under /environments/:id/...; path params renamed (e.g., :containerId, :networkId); added counts endpoints; system adds /stats/ws and /convert; removed container GetLogs.
Environment handler and router bootstrap
backend/internal/api/environment_handler.go, backend/internal/bootstrap/router_bootstrap.go, backend/internal/middleware/environment_middleware.go
EnvironmentHandler internal routing/proxy/WS handlers removed; only core env CRUD and basic ops retained. Router applies new EnvProxy middleware unconditionally; introduces environment-aware proxying (HTTP/WS).
Converter relocation and system utilities
backend/internal/services/converter_service.go (removed), backend/internal/services/system_service.go, backend/internal/utils/converter/converter_util.go, backend/internal/utils/ws/proxy.go, backend/go.mod
ConverterService deleted; parsing/conversion added to SystemService with new converter utils; WS proxy gains structured logs; go.mod updates dependency (go-yaml) to direct.
Dev Dockerfile
docker/Dockerfile.dev
Switch to alpine Node, add cache mounts, consolidate RUN steps, include git, adjust backend dev command to air.
Frontend service layer overhaul
frontend/src/lib/services/api/* (multiple files removed), frontend/src/lib/services/*-service.ts (container, image, network, project, system, settings, env-mgmt, event, template, container-registry, user)
Removes aggregated api index and several API classes; introduces/renames feature-specific services; exports singleton instances; adds system.convert and environment-scoped methods.
Frontend stores and utils
frontend/src/lib/stores/environment.store.ts, frontend/src/lib/stores/config-store.ts, frontend/src/lib/utils/ws.ts, frontend/src/lib/types/auth.type.ts
Store gains async getCurrentEnvironmentId; settings fetch switched to settingsService; WS stats path updated to /system/stats/ws; adds auth-related TS types.
Frontend components: containers/images/networks/projects/events/environments
frontend/src/routes/containers/*, frontend/src/routes/images/*, frontend/src/routes/networks/*, frontend/src/routes/projects/*, frontend/src/routes/events/*, frontend/src/routes/environments/*, frontend/src/lib/components/action-buttons.svelte, .../image-update-item.svelte, .../container-table.svelte, .../image-table.svelte, .../network-table.svelte, .../projects-table.svelte, `.../dash-*.svelte
ts`
Frontend auth and onboarding
frontend/src/lib/services/auth-service.ts, frontend/src/routes/auth/*, frontend/src/routes/onboarding/**/*
OIDC helpers added to AuthService; pages switch from API to services for settings/user/OIDC; minor comment and unused import removals.
Frontend templates/registries
frontend/src/lib/components/dialogs/template-selection-dialog.svelte, frontend/src/lib/components/sheets/add-template-registry-sheet.svelte, frontend/src/routes/customize/templates/*, frontend/src/routes/customize/registries/*
Switch from templateAPI/containerRegistryAPI to corresponding services across pages and tables.
Frontend settings and misc
frontend/src/routes/settings/+page.svelte, frontend/src/routes/+layout.ts, frontend/src/lib/components/sheets/user-form-sheet.svelte, frontend/src/lib/components/CodePanel.svelte
Refactors to service usage in layout; removes unused imports/variables; stops disabling fields for OIDC users in form; minor import cleanup.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly and concisely summarizes the primary change, stating that environment routes are being refactored into individual handlers, which aligns directly with the widespread updates to API route groupings and handler registrations described in the changeset.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor/environment-routes

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
Contributor

@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: 22

Caution

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

⚠️ Outside diff range comments (20)
frontend/src/routes/images/[imageId]/+page.ts (1)

20-30: Fix repo:tag parsing for registries with port (e.g., registry:5000/repo:tag).

Splitting on the first colon breaks for tags with registry ports. Use last colon after the last slash.

-    const repoTag = image.RepoTags[0];
-    if (repoTag.includes(':')) {
-        [repo, tag] = repoTag.split(':');
-    } else {
-        repo = repoTag;
-        tag = 'latest';
-    }
+    const repoTag = image.RepoTags[0];
+    const lastColon = repoTag.lastIndexOf(':');
+    const lastSlash = repoTag.lastIndexOf('/');
+    if (lastColon > lastSlash) {
+        repo = repoTag.slice(0, lastColon);
+        tag = repoTag.slice(lastColon + 1);
+    } else {
+        repo = repoTag;
+        tag = 'latest';
+    }
frontend/src/routes/auth/oidc/callback/+page.svelte (1)

81-90: Await and handle finalizeLogin to prevent unhandled rejection.

finalizeLogin() is async but not awaited; failures won’t hit the surrounding try/catch, causing silent errors and no redirect.

Apply this diff:

-				async function finalizeLogin() {
+				async function finalizeLogin() {
 					const settings = await settingsService.getSettings();
 					settingsStore.set(settings);
 					toast.success('Successfully logged in!');
 					goto(!settings.onboardingCompleted ? '/onboarding/welcome' : redirectTo, { replaceState: true });
 				}
 
-				await invalidateAll();
-				finalizeLogin();
+				await invalidateAll();
+				try {
+					await finalizeLogin();
+				} catch (e) {
+					console.error('finalizeLogin failed:', e);
+					error = m.auth_oidc_callback_error();
+					setTimeout(() => goto('/auth/login?error=oidc_callback_error'), 3000);
+					isProcessing = false;
+				}
backend/internal/api/network_handler.go (1)

153-160: Fix copy/paste in error message.

This endpoint returns network counts; the error says "container counts".

Apply:

- "data":    gin.H{"error": "Failed to get container counts: " + err.Error()},
+ "data":    gin.H{"error": "Failed to get network counts: " + err.Error()},
frontend/src/lib/services/settings-service.ts (1)

27-29: Remove getOidcStatus from settings-service.ts and replace calls with authService.getStatus
Update all instances of

settingsService.getOidcStatus()

to

authService.getStatus()

and delete the following method from frontend/src/lib/services/settings-service.ts:

- async getOidcStatus(): Promise<OidcStatusInfo> {
-   return this.handleResponse(this.api.get('/oidc/status'));
- }

Affected call sites:

  • frontend/src/routes/settings/+page.ts:6
  • frontend/src/routes/settings/security/+page.ts:6
  • frontend/src/routes/onboarding/security/+page.ts:8
backend/internal/api/updater_handler.go (1)

20-61: Environment-scoped updater routes must honor :id.

We now mount the updater endpoints under /environments/:id/..., but Run, Status, and History still ignore the :id path parameter. Any caller can hit /environments/<anything>/updater/status and receive the same global data, even if the environment is invalid, so clients still can’t target a specific environment. Please extract and validate the environment ID before delegating to the service (and have the service accept/use that ID); otherwise this refactor leaves the API semantics broken.

frontend/src/lib/components/image-update-item.svelte (1)

89-123: Ensure environment context is passed to checkImageUpdateByID.

imageService.checkImageUpdateByID now needs the environment-scoped endpoint (/environments/:id/image-updates/:imageId). Nothing in this component provides the environment ID, so unless the service injects one implicitly, this call will hit the wrong URL (likely 404). Please pass the active environment ID (or update the service signature) so the request includes the environment segment.

frontend/src/routes/images/+page.svelte (1)

33-73: Image service calls are missing the environment identifier.

All three service calls need the environment ID to satisfy the new /environments/:id/... routes. The previous environmentAPI helper injected it; the new imageService methods expect it as an argument. Update each call (prune, checkAll, getImages) to include the active environment ID; otherwise these operations fail against the backend.

frontend/src/lib/components/sheets/new-environment-sheet.svelte (1)

51-74: Use the environment ID returned from create.

environmentManagementService.testConnection now expects both the environment ID and API URL. Passing only created.id results in a 400. Use the created payload (ID + API URL/token) or adjust the service call to include the required arguments.

frontend/src/lib/components/sheets/image-pull-sheet.svelte (1)

10-11: Remove dead code and unused imports

getCurrentEnvironmentId() is unused after inlining envId; drop it and related imports.

- import { environmentStore, LOCAL_DOCKER_ENVIRONMENT_ID } from '$lib/stores/environment.store';
- import { get } from 'svelte/store';
+ import { environmentStore } from '$lib/stores/environment.store';
-function getCurrentEnvironmentId(): string {
-  const env = get(environmentStore.selected);
-  return env?.id || LOCAL_DOCKER_ENVIRONMENT_ID;
-}

Also applies to: 214-217

backend/internal/api/project_handler.go (2)

27-32: WebSocket CheckOrigin is wide open; restrict to same-origin or allowed list

Accepting all origins is a security posture gap. Restrict origins to same-host or a configured allowlist.

-var wsUpgrader = websocket.Upgrader{
-	CheckOrigin:       func(r *http.Request) bool { return true },
+var wsUpgrader = websocket.Upgrader{
+	CheckOrigin: func(r *http.Request) bool {
+		origin := r.Header.Get("Origin")
+		if origin == "" {
+			return true
+		}
+		host := r.Host
+		return strings.HasPrefix(origin, "http://"+host) || strings.HasPrefix(origin, "https://"+host)
+	},
 	ReadBufferSize:    32 * 1024,
 	WriteBufferSize:   32 * 1024,
 	EnableCompression: true,
 }

46-62: Scope environment ID in ProjectHandler
Handlers under /environments/:id/projects never read c.Param("id"), so calls like ListProjects and GetProjectStatusCounts run unscoped and may return or modify data across all environments. Pass the environment ID into each service method or enforce scoping via middleware in backend/internal/api/project_handler.go.

backend/internal/api/system_handler.go (1)

23-25: WebSocket CheckOrigin allows all origins — CSRF risk. Lock this down.

With cookie-based auth, a cross-site page can open the WS and stream stats. Validate Origin/Host or require a token.

 import (
   "log/slog"
   "net/http"
+  "net/url"
+  "strings"
   "runtime"
   "time"
 )
 
 var sysWsUpgrader = websocket.Upgrader{
-  CheckOrigin: func(r *http.Request) bool { return true },
+  CheckOrigin: func(r *http.Request) bool {
+    origin := r.Header.Get("Origin")
+    if origin == "" {
+      return true // non-browser clients
+    }
+    u, err := url.Parse(origin)
+    if err != nil {
+      return false
+    }
+    return strings.EqualFold(u.Host, r.Host)
+  },
+  EnableCompression: true,
 }

If you already centralize origin checks in middleware, wire that here instead. Based on learnings.

frontend/src/routes/onboarding/complete/+page.svelte (1)

8-20: Fix incorrect settings service import path

$lib/services/settings-service.ts is a TypeScript module; importing it with a .js suffix makes the compiler look for a generated JS artifact that isn’t present, breaking the build at compile/bundle time. Please import it without the extension, consistent with the other updated files.

-import { settingsService } from '$lib/services/settings-service.js';
+import { settingsService } from '$lib/services/settings-service';
frontend/src/lib/services/event-service.ts (1)

5-14: Missing imports after class rename.
getEventById, create, and update still reference DTO types that were imported before the rename; the imports were dropped here, so the file won’t compile. Reintroduce the necessary DTO imports (and any others used by the remaining methods).

Apply this patch:

-import BaseAPIService from './api-service';
-import type { SearchPaginationSortRequest, Paginated } from '$lib/types/pagination.type';
-import type { Event } from '$lib/types/event.type';
+import BaseAPIService from './api-service';
+import type { SearchPaginationSortRequest, Paginated } from '$lib/types/pagination.type';
+import type { Event } from '$lib/types/event.type';
+import type { CreateEventDto, UpdateEventDto } from '$lib/dto/event.dto';
backend/internal/api/image_update_handler.go (1)

20-30: Environment ID ignored in handler.
We now mount these routes under /environments/:id/..., but none of the handler methods read :id or attach it to service calls—CheckAllImages still hardcodes 0. That breaks multi-environment isolation. Grab the env via middleware/context (or pass c.Param("id") to the service) for every route.

Apply a fix along these lines:

-	apiGroup := group.Group("/environments/:id/image-updates")
+	apiGroup := group.Group("/environments/:id/image-updates")
 	apiGroup.Use(authMiddleware.WithAdminNotRequired().Add())

and within each handler:

 func (h *ImageUpdateHandler) CheckAllImages(c *gin.Context) {
-	results, err := h.imageUpdateService.CheckAllImages(c.Request.Context(), 0)
+	env, err := middleware.GetEnvironment(c)
+	if err != nil {
+		c.JSON(http.StatusBadRequest, gin.H{"success": false, "error": err.Error()})
+		return
+	}
+	results, err := h.imageUpdateService.CheckAllImages(c.Request.Context(), env.ID)

Repeat for the other methods so the service receives the correct environment context.

frontend/src/routes/projects/projects-table.svelte (1)

218-229: Fix loading state mismatch for Destroy action (spinner/disabled).

UI checks and spinner use isLoading.remove, but the action uses the 'destroy' key. This prevents the spinner from showing and couples disablement to isAnyLoading only.

Apply this diff:

-				<DropdownMenu.Item
-					variant="destructive"
-					onclick={() => performProjectAction('destroy', item.id)}
-					disabled={isLoading.remove || isAnyLoading}
-				>
-					{#if isLoading.remove}
+				<DropdownMenu.Item
+					variant="destructive"
+					onclick={() => performProjectAction('destroy', item.id)}
+					disabled={isLoading.destroy || isAnyLoading}
+				>
+					{#if isLoading.destroy}
 						<LoaderCircleIcon class="size-4 animate-spin" />
 					{:else}
 						<Trash2Icon class="size-4" />
 					{/if}
 					{m.compose_destroy()}
 				</DropdownMenu.Item>
backend/internal/api/volume_handler.go (1)

185-205: Correct error message for counts endpoint.

Message says “Failed to get container counts” on a volumes endpoint.

Apply this diff:

-			"data":    gin.H{"error": "Failed to get container counts: " + err.Error()},
+			"data":    gin.H{"error": "Failed to get volume counts: " + err.Error()},
frontend/src/routes/containers/container-table.svelte (3)

48-80: Actions still call container endpoints without environment ID

The start/stop/restart flows call containerService.*Container(id) with only the container ID. With routes moved under /environments/:environmentId/containers, those service calls now need the environment identifier as well. Otherwise every action will fail with a missing parameter. Please thread the environment ID through these handler invocations.


90-119: Delete flow also omits environment parameter

Likewise, containerService.deleteContainer(id, { force, volumes }) should now target /environments/:environmentId/containers/:id. Pass the environment ID so the service can hit the namespaced endpoint.


238-246: Table refresh needs environment-aware fetch

containerService.getContainers(options) no longer matches the backend signature after the route move to /environments/:environmentId/containers. Please ensure the environment ID is forwarded to getContainers (e.g., getContainers(environmentId, options)) so refresh works.

🧹 Nitpick comments (73)
frontend/src/routes/customize/registries/registry-table.svelte (3)

61-62: Confirm tryCatch signature; if it expects a thunk, wrap the call.

If tryCatch expects a function returning a promise (common pattern), the current call executes outside its try/catch. Wrap it as a thunk.

Apply if needed:

-const result = await tryCatch(containerRegistryService.deleteRegistry(id));
+const result = await tryCatch(() => containerRegistryService.deleteRegistry(id));

Optional: Consider batch deletion in parallel with Promise.allSettled to reduce latency on large selections, and keep toasts per failure. I can provide a concrete snippet if you want.


114-114: Type the testRegistry response to avoid (resp as any) downstream.

Have testRegistry return a typed object (e.g., { message: string }) so onSuccess can be strongly typed and drop the unsafe cast.

Example (in the service file):

export type TestRegistryResponse = { message: string };

export const containerRegistryService = {
  // ...
  async testRegistry(id: string): Promise<TestRegistryResponse> {
    // ...
  }
};

Then, annotate the onSuccess callback as (resp: TestRegistryResponse) => { ... } to remove as any.


231-231: Add error handling to onRefresh to avoid unhandled rejections and silent failures.

Wrap in tryCatch and surface an error toast (or propagate) so the table doesn’t appear stuck on failures.

-onRefresh={async (options) => (registries = await containerRegistryService.getRegistries(options))}
+onRefresh={async (options) => {
+  const res = await tryCatch(containerRegistryService.getRegistries(options));
+  if (res.error) {
+    toast.error(String(res.error));
+    return;
+  }
+  registries = res.data;
+}}
frontend/src/routes/customize/registries/+page.svelte (1)

69-71: DRY: reuse refreshRegistries for post-save refresh and consistent UX.

Avoid refreshing via a direct service call here; reuse the same refresh path (loading/toasts/error handling).

-      registries = await containerRegistryService.getRegistries(requestOptions);
+      await refreshRegistries();
       isRegistryDialogOpen = false;
frontend/src/lib/components/sheets/add-template-registry-sheet.svelte (1)

50-54: Preserve optional description from fetched registry.

If the manifest includes description, pass it through to onSubmit.

 const registryPayload = {
   name: registryData.name,
+  description: registryData.description ?? undefined,
   url: data.url,
   enabled: data.enabled
 };
frontend/src/lib/services/template-service.ts (1)

59-60: URL‑encode path parameters to avoid issues with special chars.

Safer when IDs contain non‑URL‑safe characters.

-    await this.api.put(`/templates/registries/${id}`, registry);
+    await this.api.put(`/templates/registries/${encodeURIComponent(id)}`, registry);
-    await this.api.delete(`/templates/registries/${id}`);
+    await this.api.delete(`/templates/registries/${encodeURIComponent(id)}`);

Also applies to: 71-73

frontend/src/lib/stores/config-store.ts (1)

7-10: Harden reload with error handling.

Avoid leaving subscribers in a perpetual pending state if the request fails.

Apply this diff:

 const reload = async () => {
-	const settings = await settingsService.getSettings();
-	settingsStore.set(settings);
+	try {
+		const settings = await settingsService.getSettings();
+		settingsStore.set(settings);
+	} catch (err) {
+		console.error('Failed to load settings', err);
+	}
 };
frontend/src/routes/onboarding/settings/+page.svelte (1)

12-12: Guard onboardingSteps spread to avoid runtime crash when undefined

If currentSettings.onboardingSteps is undefined, object spread throws. Guard with a default.

-      onboardingSteps: {
-        ...currentSettings.onboardingSteps,
-        settings: true
-      }
+      onboardingSteps: {
+        ...(currentSettings.onboardingSteps ?? {}),
+        settings: true
+      }

Also applies to: 34-42

frontend/src/routes/onboarding/welcome/+page.svelte (1)

7-7: Add try/catch and guard onboardingSteps spread

Prevent proceeding on failed updates and avoid spread crash when undefined.

-async function continueToNextStep() {
-  await settingsService.updateSettings({
-    ...currentSettings,
-    onboardingCompleted: false,
-    onboardingSteps: {
-      ...currentSettings.onboardingSteps,
-      welcome: true
-    }
-  });
-
-  settingsStore.reload();
-  goto('/onboarding/password', { invalidateAll: true });
-}
+async function continueToNextStep() {
+  try {
+    await settingsService.updateSettings({
+      ...currentSettings,
+      onboardingCompleted: false,
+      onboardingSteps: {
+        ...(currentSettings.onboardingSteps ?? {}),
+        welcome: true
+      }
+    });
+    settingsStore.reload();
+    goto('/onboarding/password', { invalidateAll: true });
+  } catch (e) {
+    // Optionally import a toast to inform the user
+    console.error('Failed to save onboarding progress', e);
+  }
+}

Also applies to: 13-20

frontend/src/routes/projects/new/+page.ts (1)

2-14: Type load as PageLoad and enable SSR-safe fetch in TemplateService
Add import type { PageLoad } and change to export const load: PageLoad = async ({ fetch }) => { … }; update TemplateService.loadAll and getEnvTemplate to accept a fetch parameter so requests work during SSR.

frontend/src/routes/images/[imageId]/+page.svelte (1)

93-101: Guard remove action when image is not yet loaded.

Prevent calling handleImageRemove with undefined id.

Apply:

- <ArcaneButton
+ <ArcaneButton
   action="remove"
-  onclick={() => handleImageRemove(image.id)}
-  loading={isLoading.removing}
-  disabled={isLoading.removing}
+  onclick={() => image?.id && handleImageRemove(image.id)}
+  loading={isLoading.removing}
+  disabled={!image?.id || isLoading.removing}
   size="sm"
 />
frontend/src/lib/services/settings-service.ts (1)

2-2: Remove OIDC type import from settings service.

OIDC concerns belong in authService; keeping OIDC types here creates coupling.

Apply:

-import type { Settings, OidcStatusInfo } from '$lib/types/settings.type';
+import type { Settings } from '$lib/types/settings.type';
frontend/src/routes/onboarding/docker/+page.svelte (1)

85-85: Update call LGTM.

Call site aligns with the new service. Consider removing the as any cast once Settings partial typing is refined.

frontend/src/lib/services/container-registry-service.ts (1)

6-6: Standardize response handling across methods.

getRegistries returns res.data while others use handleResponse. For consistency and error normalization, consider wrapping getRegistries too.

Suggested change:

-  async getRegistries(options?: SearchPaginationSortRequest): Promise<Paginated<ContainerRegistry>> {
-    const res = await this.api.get('/container-registries', { params: options });
-    return res.data;
-  }
+  async getRegistries(options?: SearchPaginationSortRequest): Promise<Paginated<ContainerRegistry>> {
+    return this.handleResponse(this.api.get('/container-registries', { params: options }));
+  }
frontend/src/lib/services/image-service.ts (2)

8-13: Standardize response handling and add response generics

Mixed usage of raw res.data vs res.data.data and handleResponse makes response shapes/error handling inconsistent across methods. Prefer a single pattern and add Axios generics for strong typing.

Apply generics for clarity (keeps current behavior while improving types):

 async getImages(options?: SearchPaginationSortRequest): Promise<Paginated<ImageSummaryDto>> {
   const envId = await environmentStore.getCurrentEnvironmentId();

-  const res = await this.api.get(`/environments/${envId}/images`, { params: options });
+  const res = await this.api.get<Paginated<ImageSummaryDto>>(`/environments/${envId}/images`, { params: options });
   return res.data;
 }

 async getImageUsageCounts(): Promise<ImageUsageCounts> {
   const envId = await environmentStore.getCurrentEnvironmentId();

-  const res = await this.api.get(`/environments/${envId}/images/counts`);
+  const res = await this.api.get<{ data: ImageUsageCounts }>(`/environments/${envId}/images/counts`);
   return res.data.data;
 }

Also applies to: 15-20


22-31: Tighten return types and request body typing

Methods return any and accept auth?: any. Define DTOs (e.g., ImageDto, PullImageRequest) to avoid any and catch regressions at compile time.

Example:

  • getImage(imageId: string): Promise
  • pullImage(imageName: string, tag = 'latest', auth?: RegistryAuth): Promise

If you share types with backend, consider generating them or centralizing under $lib/types.

frontend/src/routes/images/+page.ts (1)

17-21: Parallelize independent API calls to reduce TTFB

Fetch images, settings, and counts concurrently.

Apply:

- const images = await imageService.getImages(imageRequestOptions);
- const settings = await settingsService.getSettings();
- const imageUsageCounts = await imageService.getImageUsageCounts();
+ const [images, settings, imageUsageCounts] = await Promise.all([
+   imageService.getImages(imageRequestOptions),
+   settingsService.getSettings(),
+   imageService.getImageUsageCounts()
+ ]);
frontend/src/lib/types/auth.type.ts (1)

3-13: Include common OIDC claim email_verified

Many providers include email_verified. Adding it improves interoperability.

 export interface OidcUserInfo {
   sub: string;
   email: string;
+  email_verified?: boolean;
   name?: string;
   displayName?: string;
   preferred_username?: string;
   given_name?: string;
   family_name?: string;
   picture?: string;
   groups?: string[];
 }
frontend/src/lib/services/container-service.ts (3)

10-22: Unify response handling and add generics

Mirror the images service improvements: type responses and keep a consistent pattern (res.data vs handleResponse).

- const res = await this.api.get(`/environments/${envId}/containers`, { params: options });
+ const res = await this.api.get<Paginated<ContainerSummaryDto>>(`/environments/${envId}/containers`, { params: options });
  return res.data;
- const res = await this.api.get(`/environments/${envId}/containers/counts`);
+ const res = await this.api.get<{ data: ContainerStatusCounts }>(`/environments/${envId}/containers/counts`);
  return res.data.data;

39-43: Avoid type assertion; leverage generics

Prefer generics over as Promise<ContainerStats> to keep types accurate through refactors.

- return this.handleResponse(this.api.get(url)) as Promise<ContainerStats>;
+ return this.handleResponse<ContainerStats>(this.api.get(url));

55-62: Encode query params robustly

Minor: Use Axios params booleans directly; Axios serializes them cleanly and avoids manual string conversion.

- const params: Record<string, string> = {};
- if (opts?.force !== undefined) params.force = String(!!opts.force);
- if (opts?.volumes !== undefined) params.volumes = String(!!opts.volumes);
+ const params: Record<string, boolean> = {};
+ if (opts?.force !== undefined) params.force = !!opts.force;
+ if (opts?.volumes !== undefined) params.volumes = !!opts.volumes;
frontend/src/lib/components/sheets/image-pull-sheet.svelte (1)

93-104: Optional: support cancellation for long-running pulls

Consider AbortController to cancel the fetch if needed (e.g., navigation away), even though the sheet stays open while pulling.

Also applies to: 116-159

docker/Dockerfile.dev (2)

29-37: Guarantee Air is on PATH

In some Golang base images, /go/bin may not be on PATH in all contexts. Add PATH export to avoid “air: not found” at runtime.

 RUN apk add --no-cache \
     ca-certificates \
     curl \
     gcc \
     git \
     musl-dev \
     tzdata && \
     go install github.com/air-verse/air@latest
+ENV PATH="/go/bin:${PATH}"

If you rely on bind-mounts for source and .air.toml, confirm your dev compose uses this stage (target: backend-dev) and mounts the repo root. If not, the CMD will fail due to missing sources.


6-14: Leverage pnpm fetch for faster cold builds (optional)

You can prime the store with lockfile-only fetch to improve cold build speed.

-FROM node:${NODE_VERSION}-alpine AS frontend-dev
+FROM node:${NODE_VERSION}-alpine AS frontend-dev
 RUN corepack enable
 WORKDIR /app
 # Copy package configuration files and install dependencies
 COPY package.json pnpm-workspace.yaml pnpm-lock.yaml ./
 COPY frontend/package.json ./frontend/package.json
-RUN --mount=type=cache,target=/root/.pnpm-store pnpm install --frozen-lockfile
+RUN --mount=type=cache,target=/root/.pnpm-store pnpm fetch --force && pnpm install --frozen-lockfile
backend/internal/utils/converter/converter_util.go (3)

91-103: Don’t reject values that start with ‘-’; add equals handling

Some valid values can start with ‘-’. Also support equals-form provided at current token.

-func parseStringFlag(flagName string, tokens []string, index int, target *string) (int, error) {
-	if index+1 >= len(tokens) {
+func parseStringFlag(flagName string, tokens []string, index int, target *string) (int, error) {
+	// --flag=value
+	if strings.Contains(tokens[index], "=") {
+		parts := strings.SplitN(tokens[index], "=", 2)
+		if parts[1] == "" {
+			return 0, fmt.Errorf("missing value for %s flag", flagName)
+		}
+		*target = parts[1]
+		return 0, nil
+	}
+	if index+1 >= len(tokens) {
 		return 0, fmt.Errorf("missing value for %s flag", flagName)
 	}
 
 	value := tokens[index+1]
-	if value == "" || strings.HasPrefix(value, "-") {
+	if value == "" {
 		return 0, fmt.Errorf("invalid value for %s flag", flagName)
 	}
 
 	*target = value
 	return 1, nil
 }

105-117: Support equals-form in slice flags

-func parseSliceFlag(flagName string, tokens []string, index int, target *[]string) (int, error) {
-	if index+1 >= len(tokens) {
+func parseSliceFlag(flagName string, tokens []string, index int, target *[]string) (int, error) {
+	// --flag=value
+	if strings.Contains(tokens[index], "=") {
+		parts := strings.SplitN(tokens[index], "=", 2)
+		if parts[1] == "" {
+			return 0, fmt.Errorf("missing value for %s flag", flagName)
+		}
+		*target = append(*target, parts[1])
+		return 0, nil
+	}
+	if index+1 >= len(tokens) {
 		return 0, fmt.Errorf("missing value for %s flag", flagName)
 	}
 
 	value := tokens[index+1]
-	if value == "" || strings.HasPrefix(value, "-") {
+	if value == "" {
 		return 0, fmt.Errorf("invalid value for %s flag", flagName)
 	}
 
 	*target = append(*target, value)
 	return 1, nil
 }

146-180: Quoted/escaped tokenization is minimal; consider shlex-like parsing

Current tokenizer doesn’t handle escapes or backslash-escaped quotes. Optional: adopt a shlex parser (e.g., github.com/google/shlex) or extend to support escapes if needed for realistic docker run commands with complex args.

frontend/src/lib/services/project-service.ts (4)

7-12: Use handleResponse for consistent unwrapping

Other methods use handleResponse; align getProjects for consistency and error handling.

-  const res = await this.api.get(`/environments/${envId}/projects`, { params: options });
-  return res.data;
+  return this.handleResponse(
+    this.api.get(`/environments/${envId}/projects`, { params: options })
+  );

81-89: Include credentials and Accept header in streaming fetch

Ensure cookies are sent and hint NDJSON (or JSON-lines) to the backend.

-    const res = await fetch(url, { method: 'POST' });
+    const res = await fetch(url, {
+      method: 'POST',
+      credentials: 'same-origin',
+      headers: { Accept: 'application/x-ndjson, application/json' }
+    });

Confirm backend emits newline-delimited JSON for this endpoint and does not require additional headers (e.g., X-Requested-With).


64-67: Parameter naming consistency (id vs name)

Some endpoints use projectId, others projectName. If the API requires names for a subset, consider reflecting that in method names (e.g., downProjectByName) to avoid misuse.


142-152: DELETE with body relies on Axios behavior

Some proxies/servers drop bodies on DELETE. If issues arise, consider query params instead.

frontend/src/routes/dashboard/dash-image-table.svelte (2)

95-95: Handle refresh errors to avoid unhandled promise rejections.

Wrap the refresh to keep the table stable if the request fails.

-				onRefresh={async (options) => (images = await imageService.getImages(options))}
+				onRefresh={async (options) => {
+					try {
+						images = await imageService.getImages(options);
+					} catch (e) {
+						console.error('Failed to refresh images', e);
+					}
+				}}

101-103: Optional: show the footer based on totalItems, not current page length.

With limit 5, images.data.length > 5 is rarely true; use totalItems for intent.

-			{#if images.data.length > 5}
+			{#if images.pagination.totalItems > 5}
 				<div class="bg-muted/40 text-muted-foreground border-t px-6 py-2 text-xs">
 					{m.images_showing_of_total({ shown: 5, total: images.pagination.totalItems })}
 				</div>
 			{/if}
frontend/src/routes/dashboard/+page.ts (1)

30-33: Parallelize requests to cut load time

Fetch containers, images, and counts in parallel (and keep .allSettled for the others).

-  const containers = await containerService.getContainers(containerRequestOptions);
-  const images = await imageService.getImages(imageRequestOptions);
-  const containerStatusCounts = await containerService.getContainerStatusCounts();
-
-  const [dockerInfoResult, settingsResult] = await Promise.allSettled([
-    systemService.getDockerInfo(),
-    settingsService.getSettings()
-  ]);
+  const containersP = containerService.getContainers(containerRequestOptions);
+  const imagesP = imageService.getImages(imageRequestOptions);
+  const countsP = containerService.getContainerStatusCounts();
+
+  const [dockerInfoResult, settingsResult] = await Promise.allSettled([
+    systemService.getDockerInfo(),
+    settingsService.getSettings()
+  ]);
+
+  const [containers, images, containerStatusCounts] = await Promise.all([
+    containersP,
+    imagesP,
+    countsP
+  ]);

Also applies to: 35-37

frontend/src/routes/dashboard/dash-container-table.svelte (1)

13-13: Guard refresh with try/catch and ensure env ID is included

  • Wrap onRefresh to avoid unhandled rejections.
  • Verify containerService encodes the environment ID for new /environments/:id routes.
- onRefresh={async (options) => (containers = await containerService.getContainers(options))}
+ onRefresh={async (options) => {
+   try {
+     containers = await containerService.getContainers(options);
+   } catch (e) {
+     // optional: toast error; keep prior data on failure
+   }
+ }}

Also applies to: 74-75

frontend/src/routes/projects/+page.ts (1)

17-19: Fetch projects and counts in parallel

-  const projects = await projectService.getProjects(projectRequestOptions);
-  const projectStatusCounts = await projectService.getProjectStatusCounts();
+  const [projects, projectStatusCounts] = await Promise.all([
+    projectService.getProjects(projectRequestOptions),
+    projectService.getProjectStatusCounts()
+  ]);

Also ensure projectService attaches the environment ID for the new /environments/:id routes.

frontend/src/routes/containers/+page.svelte (1)

8-8: Refresh containerStatusCounts after container mutations
In +page.svelte, update each onSuccess handler for runAutoUpdate, getContainers (refresh) and createContainer to also fetch containerService.getContainerStatusCounts() and assign it to containerStatusCounts (e.g. via Promise.all) so the header stats stay in sync.

frontend/src/routes/events/+page.svelte (1)

58-70: Batch deletions in parallel to reduce latency

Loop deletes are sequential. Consider parallelizing with Promise.allSettled to improve UX on large selections.

-          for (const eventId of selectedIds) {
-            const result = await tryCatch(eventService.delete(eventId));
-            handleApiResultWithCallbacks({
-              result,
-              message: m.events_delete_item_failed({ id: eventId }),
-              setLoadingState: () => {},
-              onSuccess: () => {
-                successCount++;
-              }
-            });
-            if (result.error) failureCount++;
-          }
+          const results = await Promise.allSettled(
+            selectedIds.map((id) => tryCatch(eventService.delete(id)))
+          );
+          for (const r of results) {
+            const result = r.status === 'fulfilled' ? r.value : { error: r.reason };
+            handleApiResultWithCallbacks({
+              result,
+              message: m.events_delete_item_failed({ id: '<multiple>' }),
+              setLoadingState: () => {},
+              onSuccess: () => successCount++
+            });
+            if ((result as any).error) failureCount++;
+          }
backend/internal/api/project_handler.go (2)

402-405: Standardize error payload shape

Use "error" consistently instead of putting the error inside "data".

-    c.JSON(http.StatusInternalServerError, gin.H{
-      "success": false,
-      "data":    gin.H{"error": "Failed to get project counts: " + err.Error()},
-    })
+    c.JSON(http.StatusInternalServerError, gin.H{
+      "success": false,
+      "error":   "Failed to get project counts: " + err.Error(),
+    })

395-397: Propagate request context to WS client

Use c.Request.Context() so cancellations/timeouts propagate correctly.

- ws.ServeClient(context.Background(), hub, conn)
+ ws.ServeClient(c.Request.Context(), hub, conn)
frontend/src/routes/environments/environment-table.svelte (2)

49-61: Parallelize bulk deletes to speed up UX

Same pattern as events: consider Promise.allSettled to issue deletes concurrently.

-          for (const id of ids) {
-            const result = await tryCatch(environmentManagementService.delete(id));
-            handleApiResultWithCallbacks({
-              result,
-              message: m.environments_bulk_remove_failed_many({ count: ids.length }),
-              setLoadingState: () => {},
-              onSuccess: () => {
-                successCount++;
-              }
-            });
-            if (result.error) failureCount++;
-          }
+          const results = await Promise.allSettled(
+            ids.map((id) => tryCatch(environmentManagementService.delete(id)))
+          );
+          for (const r of results) {
+            const result = r.status === 'fulfilled' ? r.value : { error: r.reason };
+            handleApiResultWithCallbacks({
+              result,
+              message: m.environments_bulk_remove_failed_many({ count: ids.length }),
+              setLoadingState: () => {},
+              onSuccess: () => (successCount++)
+            });
+            if ((result as any).error) failureCount++;
+          }

184-184: i18n nit: localize Enabled/Disabled

Use message keys for these labels to keep translations consistent.

-  <StatusBadge text={Boolean(value) ? 'Enabled' : 'Disabled'} variant={Boolean(value) ? 'green' : 'gray'} />
+  <StatusBadge text={Boolean(value) ? m.common_enabled() : m.common_disabled()} variant={Boolean(value) ? 'green' : 'gray'} />
frontend/src/routes/projects/[projectId]/+page.svelte (1)

28-28: Consider aligning identifier with file/component name.

Importing ProjectLogsPanel.svelte as StackLogsPanel may confuse future readers. Prefer consistent naming.

-import StackLogsPanel from '../components/ProjectLogsPanel.svelte';
+import ProjectLogsPanel from '../components/ProjectLogsPanel.svelte';
...
-<StackLogsPanel projectId={project.id} bind:autoScroll={autoScrollStackLogs} />
+<ProjectLogsPanel projectId={project.id} bind:autoScroll={autoScrollStackLogs} />
frontend/src/lib/services/volume-service.ts (2)

32-40: Tighten return types for create/delete.

Avoid Promise. Use DTOs (or a minimal success shape) to improve safety and DX.

-  async createVolume(options: VolumeCreateOptions): Promise<any> {
+  async createVolume(options: VolumeCreateOptions): Promise<VolumeDetailDto> {
     const envId = await environmentStore.getCurrentEnvironmentId();
-    return this.handleResponse(this.api.post(`/environments/${envId}/volumes`, options));
+    return this.handleResponse(
+      this.api.post(`/environments/${envId}/volumes`, options)
+    ) as Promise<VolumeDetailDto>;
   }
 
-  async deleteVolume(volumeName: string): Promise<any> {
+  async deleteVolume(volumeName: string): Promise<{ deleted: boolean; name: string }> {
     const envId = await environmentStore.getCurrentEnvironmentId();
-    return this.handleResponse(this.api.delete(`/environments/${envId}/volumes/${volumeName}`));
+    return this.handleResponse(
+      this.api.delete(`/environments/${envId}/volumes/${encodeURIComponent(volumeName)}`)
+    ) as Promise<{ deleted: boolean; name: string }>;
   }

If the backend returns different shapes, define matching types under $lib/types/volume.type.


7-13: Optional: factor envId retrieval to a helper to reduce repetition.

Minor readability improvement.

 export class VolumeService extends BaseAPIService {
+  private async envPath(path: string) {
+    const envId = await environmentStore.getCurrentEnvironmentId();
+    return `/environments/${envId}${path}`;
+  }
   async getVolumes(options?: SearchPaginationSortRequest): Promise<Paginated<VolumeSummaryDto>> {
-    const envId = await environmentStore.getCurrentEnvironmentId();
-    return this.handleResponse(
-      this.api.get(`/environments/${envId}/volumes`, { params: options })
-    ) as Promise<Paginated<VolumeSummaryDto>>;
+    return this.handleResponse(
+      this.api.get(await this.envPath('/volumes'), { params: options })
+    ) as Promise<Paginated<VolumeSummaryDto>>;
   }
   ...
 }

Also applies to: 15-41

frontend/src/routes/dashboard/+page.svelte (1)

114-141: Remove stale commented-out API calls to reduce noise.

These can be deleted; invalidateAll() is now the refresh path.

-    // const [dockerInfoResult, settingsResult, imagesResult, statusCountsResult] = await Promise.allSettled([
-    //  tryCatch(environmentAPI.getDockerInfo()),
-    //  tryCatch(settingsAPI.getSettings()),
-    //  tryCatch(environmentAPI.getImages(imageRequestOptions)),
-    //  tryCatch(environmentAPI.getContainerStatusCounts())
-    // ]);
-    //
-    // if (dockerInfoResult.status === 'fulfilled' && !dockerInfoResult.value.error) {
-    //  dashboardStates.dockerInfo = dockerInfoResult.value.data;
-    //  dockerInfo = dockerInfoResult.value.data;
-    // }
-    // isLoading.loadingDockerInfo = false;
-    //
-    // if (settingsResult.status === 'fulfilled' && !settingsResult.value.error) {
-    //  dashboardStates.settings = settingsResult.value.data;
-    // }
-    //
-    // if (imagesResult.status === 'fulfilled') {
-    //  if (!imagesResult.value.error) {
-    //    images = imagesResult.value.data;
-    //  }
-    // }
-    // isLoading.loadingImages = false;
-    //
-    // if (statusCountsResult.status === 'fulfilled' && !statusCountsResult.value.error) {
-    //  containerStatusCounts = statusCountsResult.value.data;
-    // }
frontend/src/routes/customize/templates/+page.svelte (2)

28-33: Set mutations may not trigger reactivity; prefer plain records or force invalidation.

Mutating Set (add/delete) doesn’t reliably trigger updates in Svelte runes. This can leave UI controls (disabled states) stale. Use Record<string, boolean> flags or explicitly invalidate after mutations.

Apply one of these:

Option A (preferred: switch to records)

- let isLoading = $state({
-   addingRegistry: false,
-   removing: new Set<string>(),
-   updating: new Set<string>()
- });
+ let isLoading = $state({
+   addingRegistry: false,
+   removing: {} as Record<string, boolean>,
+   updating: {} as Record<string, boolean>
+ });
- if (isLoading.updating.has(id)) return;
- isLoading.updating.add(id);
+ if (isLoading.updating[id]) return;
+ isLoading.updating[id] = true;
- isLoading.updating.delete(id);
+ delete isLoading.updating[id];
- if (isLoading.removing.has(id)) return;
- isLoading.removing.add(id);
+ if (isLoading.removing[id]) return;
+ isLoading.removing[id] = true;
- isLoading.removing.delete(id);
+ delete isLoading.removing[id];
- disabled={isLoading.updating.has(registry.id)}
+ disabled={!!isLoading.updating[registry.id]}

Option B (minimal change: force invalidation after set mutations)

- isLoading.updating.add(id);
+ isLoading.updating.add(id);
+ isLoading = { ...isLoading };
- isLoading.updating.delete(id);
+ isLoading.updating.delete(id);
+ isLoading = { ...isLoading };
- isLoading.removing.add(id);
+ isLoading.removing.add(id);
+ isLoading = { ...isLoading };
- isLoading.removing.delete(id);
+ isLoading.removing.delete(id);
+ isLoading = { ...isLoading };

Also applies to: 41-41, 63-64, 68-69, 81-81, 206-209


71-76: Avoid redundant double refresh after delete.

You filter ‘registries’ then immediately refetch them, causing extra work and UI flicker. Keep one approach.

- const reg = registries.find((r) => r.id === id);
- await templateService.deleteRegistry(id);
- registries = registries.filter((r) => r.id !== id);
- registries = await templateService.getRegistries();
+ const reg = registries.find((r) => r.id === id);
+ await templateService.deleteRegistry(id);
+ registries = await templateService.getRegistries();
backend/internal/services/system_service.go (3)

341-343: Precompile the regex to avoid recompilation on every call.

Minor perf nit: MustCompile runs each call. Hoist it to a package-level var and reuse.

-import (
+import (
   "context"
   "fmt"
   "log/slog"
-  "regexp"
+  "regexp"
   "strings"

Add near the top (outside functions):

+var dockerRunPrefixRe = regexp.MustCompile(`(?i)^\s*docker\s+run\s+`)

And use it here:

- cmd = regexp.MustCompile(`^docker\s+run\s+`).ReplaceAllString(cmd, "")
+ cmd = dockerRunPrefixRe.ReplaceAllString(cmd, "")

442-454: Compose ‘deploy’ limits are fine but often ignored by non-Swarm Compose.

Harmless to include, but Compose CLI ignores deploy in non-Swarm mode. If users expect resource limits to apply locally, consider documenting this or mapping to runtime flags when applicable.

Also applies to: 456-472


468-471: Sanitize env file content to KEY=VALUE lines.

If parsed.Environment can contain bare keys (no “=”), .env should output “KEY=” to be valid. Consider normalizing before join.

Example:

env := make([]string, 0, len(parsed.Environment))
for _, e := range parsed.Environment {
  if strings.Contains(e, "=") {
    env = append(env, e)
  } else {
    env = append(env, e+"=")
  }
}
envVars := strings.Join(env, "\n")
backend/internal/api/system_handler.go (4)

245-255: Log WS upgrade failures for observability.

Currently silent returns on upgrade error hinder troubleshooting.

 conn, err := sysWsUpgrader.Upgrade(c.Writer, c.Request, nil)
 if err != nil {
-  return
+  slog.Warn("ws upgrade failed", "err", err.Error(), "client_ip", c.ClientIP())
+  return
 }

281-287: Disk root path is not portable.

disk.Usage("/") may be wrong on Windows. Consider selecting a root/mount based on GOOS or using the largest/primary filesystem.

Example:

root := "/"
if runtime.GOOS == "windows" {
  root = `C:\`
}
diskInfo, _ := disk.Usage(root)

Or enumerate partitions via disk.Partitions and pick the system one. Based on learnings.


256-268: Simplify ‘send(block)’ — always called with true.

Remove the block parameter and always compute CPU freshly (with the 1s sample).

- send := func(block bool) error {
+ send := func() error {
    var cpuUsage float64
-   if block {
-     if vals, err := cpu.Percent(time.Second, false); err == nil && len(vals) > 0 {
-       cpuUsage = vals[0]
-       lastCPU = cpuUsage
-     } else {
-       cpuUsage = lastCPU
-     }
-   } else {
-     cpuUsage = lastCPU
-   }
+   if vals, err := cpu.Percent(time.Second, false); err == nil && len(vals) > 0 {
+     cpuUsage = vals[0]
+     lastCPU = cpuUsage
+   } else {
+     cpuUsage = lastCPU
+   }
    ...
 }
 
- if err := send(true); err != nil {
+ if err := send(); err != nil {
    return
 }
 ...
- if err := send(true); err != nil {
+ if err := send(); err != nil {
    return
 }

Also applies to: 318-323


346-354: Add server-side logging for conversion errors.

You return generic errors to clients; also log err details server-side for diagnostics.

 dockerCompose, envVars, serviceName, err := h.systemService.ConvertToDockerCompose(parsed)
 if err != nil {
+  slog.Error("convert to compose failed", "err", err.Error(), "client_ip", c.ClientIP())
   c.JSON(http.StatusInternalServerError, gin.H{
     "success": false,
     "error":   "Failed to convert to Docker Compose format.",
     "code":    "CONVERSION_ERROR",
   })
   return
 }
backend/internal/middleware/environment_middleware.go (6)

161-163: Preserve X-Forwarded-For chain and set X-Forwarded-Proto

Overwriting XFF drops proxy chain; missing XF-Proto loses scheme info at the upstream.

-		req.Header.Set("X-Forwarded-For", c.ClientIP())
-		req.Header.Set("X-Forwarded-Host", c.Request.Host)
+		if prior := c.Request.Header.Get("X-Forwarded-For"); prior != "" {
+			req.Header.Set("X-Forwarded-For", prior+", "+c.ClientIP())
+		} else {
+			req.Header.Set("X-Forwarded-For", c.ClientIP())
+		}
+		req.Header.Set("X-Forwarded-Host", c.Request.Host)
+		if c.Request.TLS != nil {
+			req.Header.Set("X-Forwarded-Proto", "https")
+		} else if proto := c.Request.Header.Get("X-Forwarded-Proto"); proto != "" {
+			req.Header.Set("X-Forwarded-Proto", proto)
+		} else {
+			req.Header.Set("X-Forwarded-Proto", "http")
+		}

84-100: Forward WebSocket subprotocol when present

Many WS servers require Sec-WebSocket-Protocol to match the client's requested subprotocol.

 			hdr := http.Header{}
+			// Preserve negotiated subprotocols if present.
+			if swp := c.GetHeader("Sec-WebSocket-Protocol"); swp != "" {
+				hdr.Set("Sec-WebSocket-Protocol", swp)
+			}

59-61: Prefer 403 Forbidden for disabled environments

400 indicates client input error; disabled is an authorization/state issue. 403 aligns better with semantics.

-			c.JSON(http.StatusBadRequest, gin.H{"success": false, "data": gin.H{"error": "Environment is disabled"}})
+			c.JSON(http.StatusForbidden, gin.H{"success": false, "data": gin.H{"error": "Environment is disabled"}})

53-58: Validate and constrain resolver URLs to http/https to reduce SSRF risk

If apiURL comes from config/DB, guard against unexpected schemes/hosts.

Apply this check after resolving:

 		apiURL, accessToken, enabled, err := resolver(c.Request.Context(), envID)
 		if err != nil || apiURL == "" {
 			c.JSON(http.StatusNotFound, gin.H{"success": false, "data": gin.H{"error": "Environment not found"}})
 			c.Abort()
 			return
 		}
+		// Basic safety: only proxy to http(s) targets.
+		if parsed, perr := url.Parse(apiURL); perr != nil || (parsed.Scheme != "http" && parsed.Scheme != "https") {
+			c.JSON(http.StatusBadGateway, gin.H{"success": false, "data": gin.H{"error": "Invalid environment proxy target"}})
+			c.Abort()
+			return
+		}

Note: add net/url import.


122-147: Reduce per-request allocations for skip/hop header sets

The skip map is rebuilt per request. Hoist to package-level vars to cut allocations on hot paths.


164-171: Timeouts and retries

Static 60s timeout with no retries can be harsh for long-running log/stream endpoints and fragile networks. Consider context-based timeouts per route and idempotent retries where applicable.

frontend/src/routes/projects/projects-table.svelte (2)

48-51: Avoid pre-setting loading for destroy; ensure reset on error paths.

Currently isLoading[action] is set before opening the confirm dialog, which makes isAnyLoading true even while awaiting user confirmation. Also, errors thrown outside handleApiResultWithCallbacks won’t clear loading.

Apply this minimal change:

-	isLoading[action as keyof typeof isLoading] = true;
+	if (action !== 'destroy') {
+		isLoading[action as keyof typeof isLoading] = true;
+	}

And ensure reset for unexpected errors:

 		} catch (error) {
 			toast.error(m.action_failed());
+		} finally {
+			if (action !== 'destroy') {
+				isLoading[action as keyof typeof isLoading] = false;
+			}
 		}

Also applies to: 128-131


48-131: Type action as a union to prevent invalid keys.

Narrow action to 'start' | 'stop' | 'restart' | 'pull' | 'destroy' to avoid accidental typos leading to silent state bugs.

backend/internal/api/volume_handler.go (1)

19-33: Consider stricter auth for destructive routes.

List/Get/Counts can be non-admin, but Remove and Prune are destructive. If your policy requires admin for those, split the group or wrap those handlers with admin-required middleware.

frontend/src/routes/onboarding/password/+page.svelte (2)

11-12: Unify import specifiers (drop .js for TS paths).

Other files use extensionless imports. Align to avoid resolution inconsistencies.

-import { settingsService } from '$lib/services/settings-service.js';
-import { userService } from '$lib/services/user-service.js';
+import { settingsService } from '$lib/services/settings-service';
+import { userService } from '$lib/services/user-service';

17-31: Handle settings update failures.

continueToNextStep() should guard updateSettings with try/catch to surface errors and prevent silent navigation failures.

 async function continueToNextStep() {
-	const updatedSettings = await settingsService.updateSettings({
-		...currentSettings,
-		onboardingCompleted: false,
-		onboardingSteps: {
-			...currentSettings.onboardingSteps,
-			password: true
-		}
-	});
-
-	currentSettings = updatedSettings;
-	settingsStore.set(updatedSettings);
-
-	goto('/onboarding/docker');
+	try {
+		const updatedSettings = await settingsService.updateSettings({
+			...currentSettings,
+			onboardingCompleted: false,
+			onboardingSteps: {
+				...currentSettings.onboardingSteps,
+				password: true
+			}
+		});
+		currentSettings = updatedSettings;
+		settingsStore.set(updatedSettings);
+		goto('/onboarding/docker');
+	} catch (e: any) {
+		console.error('Failed to update settings:', e);
+		error = e?.message || 'Failed to update settings';
+	}
 }
frontend/src/routes/environments/+page.svelte (2)

89-94: Avoid duplicate fetch on creation.

onEnvironmentCreated() both fetches environments and then calls refresh() which fetches again.

Apply this diff:

 async function onEnvironmentCreated() {
 	showEnvironmentSheet = false;
-	environments = await environmentManagementService.getEnvironments(requestOptions);
 	toast.success(m.environments_created_success());
-	refresh();
+	await refresh();
 }

52-63: Optionally parallelize bulk deletions.

Current loop deletes sequentially. If backend tolerates concurrency, consider Promise.allSettled with a small concurrency limit to speed up bulk deletes and then aggregate results.

frontend/src/lib/components/action-buttons.svelte (1)

141-189: Show relevant confirm checkboxes per type and use them.

Checkboxes for removeFiles/removeVolumes are shown for containers too but unused. Limit them to projects (and optionally wire removeVolumes for containers if supported).

Minimal UI fix:

 			openConfirmDialog({
 				title: type === 'project' ? m.compose_destroy() : m.action_confirm_removal_title(),
 				message:
 					type === 'project'
 						? m.action_confirm_destroy_message({ type: m.project() })
 						: m.action_confirm_removal_message({ type: m.container() }),
 				confirm: {
 					label: type === 'project' ? m.compose_destroy() : m.common_remove(),
 					destructive: true,
 					action: async (checkboxStates) => {
-						const removeFiles = checkboxStates['removeFiles'] === true;
-						const removeVolumes = checkboxStates['removeVolumes'] === true;
+						const removeFiles = type === 'project' && checkboxStates['removeFiles'] === true;
+						const removeVolumes = type === 'project' && checkboxStates['removeVolumes'] === true;
 
 						setLoading('remove', true);
 						handleApiResultWithCallbacks({
 							result: await tryCatch(
 								type === 'container'
-									? containerService.deleteContainer(id)
+									? containerService.deleteContainer(id)
 									: projectService.destroyProject(id, removeVolumes, removeFiles)
 							),

And only render checkboxes for projects:

-				checkboxes: [
-					{ id: 'removeFiles', label: m.confirm_remove_project_files(), initialState: false },
-					{
-						id: 'removeVolumes',
-						label: m.confirm_remove_volumes_warning(),
-						initialState: false
-					}
-				]
+				checkboxes: type === 'project'
+					? [
+							{ id: 'removeFiles', label: m.confirm_remove_project_files(), initialState: false },
+							{ id: 'removeVolumes', label: m.confirm_remove_volumes_warning(), initialState: false }
+						]
+					: []
frontend/src/lib/services/auth-service.ts (3)

35-40: Prefer generics over type assertions for handleResponse.

This tightens typing and avoids unsafe casts.

-  const response = (await this.handleResponse(this.api.post('/oidc/url', { redirectUri }))) as {
-    authUrl: string;
-  };
-  return response.authUrl;
+  const { authUrl } = await this.handleResponse<{ authUrl: string }>(
+    this.api.post('/oidc/url', { redirectUri })
+  );
+  return authUrl;

42-52: Strongly type the callback response.

Keeps the return contract explicit and safer.

-  return this.handleResponse(this.api.post('/oidc/callback', { code, state }));
+  return this.handleResponse<{
+    success: boolean;
+    token?: string;
+    user?: OidcUserInfo;
+    error?: string;
+  }>(this.api.post('/oidc/callback', { code, state }));

54-56: Type the status response.

Aligns generics with the function’s declared return type.

-  return this.handleResponse(this.api.get('/oidc/status'));
+  return this.handleResponse<OidcStatusInfo>(this.api.get('/oidc/status'));
frontend/src/routes/projects/new/+page.svelte (1)

84-97: Same here: pass the Promise, not the resolved value.

Keeps loading state accurate and lets the helper await internally.

-      result: await tryCatch(systemService.convert(dockerRunCommand)),
+      result: tryCatch(systemService.convert(dockerRunCommand)),

Use the same script above to confirm the helper’s API.

@kmendell kmendell enabled auto-merge (squash) September 25, 2025 20:09
@kmendell kmendell merged commit 4e535f5 into main Sep 25, 2025
11 checks passed
@kmendell kmendell deleted the refactor/environment-routes branch September 25, 2025 20:14
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.

1 participant