Skip to content

Conversation

raghavyuva
Copy link
Owner

@raghavyuva raghavyuva commented Sep 20, 2025

Summary by CodeRabbit

  • New Features

    • Introduces Extensions: browse, search, sort, and paginate a catalog with detailed views.
    • Run/install extensions with variable inputs, view live execution logs, and cancel runs.
    • Fork extensions with optional YAML edits; remove forks when needed.
    • Added numerous ready-to-use templates (e.g., Fail2ban, MinIO, Postgres, Redis, Excalidraw, Dashy, Code Server, Appwrite, Uptime Kuma, Homer, etc.).
    • Sidebar navigation entry, RBAC permissions, and translations in multiple languages.
  • Documentation

    • Added Extensions specification and examples.
  • Refactor

    • Pagination now appears only when multiple pages exist in relevant lists.

Copy link
Contributor

coderabbitai bot commented Sep 20, 2025

Walkthrough

Introduces a full “extensions” feature across backend and frontend: new data models, parser/engine modules, service/storage layers, controllers/routes, DB migrations, templates, auth permissions, Redux API slice, UI pages/components, i18n, and loader wiring. Also updates version metadata, docs, pagination conditions, and adds a YAML dependency.

Changes

Cohort / File(s) Summary
API Version Metadata
api/api/versions.json
Updated v1 release_date timestamp.
Extension Types & Parser
api/internal/types/extension.go, api/internal/features/extension/parser/*
Added comprehensive extension domain models; YAML parser, schema types, directory loader, and validators for metadata, variables, and steps.
Execution Engine Modules
api/internal/features/extension/engine/*
Added pluggable step engine: registry, common var substitution, modules for command, file, service, user, docker, docker_compose; helpers and compensation logic.
Service & Execution Orchestration
api/internal/features/extension/service/service.go, .../service/run_extension.go, .../service/executor.go, .../service/context.go
Implemented ExtensionService with CRUD, forking, listing, multipart parsing; run orchestration, async execution, step processing, logging, cancelation, and rollback context.
Storage Layer
api/internal/features/extension/storage/storage.go
New Bun-backed storage with optional transactions; CRUD for extensions/variables/executions/steps/logs; listing, filtering, pagination; log sequencing.
Loader & Store Init
api/internal/features/extension/loader/loader.go, api/internal/storage/store.go
Added template-based ExtensionLoader; wired store init to load templates and register models.
Controllers & Validation
api/internal/features/extension/controller/*, api/internal/features/extension/validation/validator.go
Added ExtensionsController with endpoints for list/get/run/cancel/logs/fork/delete and executions listing; stubbed request validator; controller init helper.
Routing & Auth
api/internal/routes.go, api/internal/features/supertokens/auth.go
Registered /extensions routes; added RBAC/Audit; extended permissions with extension:* and extension:read.
DB Migrations (Extensions & Related)
api/migrations/extensions/036_*, .../037_*, .../038_*, .../039_*, api/migrations/audit/035_*, api/migrations/feature-flags/034_*
Created enums, tables, FKs, indexes, triggers; added extension_type and parent_extension_id; added logs table/log_seq; updated audit enum; added/removes feature flag; down migrations included.
Templates Catalog
api/templates/*
Added multiple extension templates (deploy-*) and fail2ban; define metadata, variables, run/validate steps.
Documentation
docs/extensions/index.md
Added current extensions spec documentation with phases, steps, and example.
Redux & API Integration
view/redux/api-conf.ts, view/redux/services/extensions/extensionsApi.ts, view/redux/store.ts, view/redux/types/extension.ts
Added endpoints enum, RTK Query slice and hooks, wired store middleware/reducer, and TypeScript types for extensions domain.
Extensions UI (Pages/Components/Hooks)
view/app/extensions/**/*
Added Extensions listing page, details page with Overview/Executions tabs, cards/grid/header/hero, run input dialog, fork dialog, logs tab, loading states; hooks for data and input handling.
Common UI Component
view/components/ui/ace-editor.tsx
Switched to forwardRef, exported props type, preserved public API.
Navigation & RBAC (Frontend)
view/components/layout/app-sidebar.tsx, view/lib/rbac.ts
Added “Extensions” nav item; extended Resource union with 'extensions'.
Internationalization
view/lib/i18n/locales/en.json, .../es.json, .../fr.json, .../kn.json
Added “extensions” namespace strings; added common keys; navigation label.
Pagination Condition Tweaks
view/app/containers/page.tsx, view/app/self-host/components/github-repositories/list-repositories.tsx
Render pagination only when totalPages > 1.
Frontend Dependency
view/package.json
Added dependency: yaml@^2.5.1.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User (Browser)
  participant FE as Frontend (Next.js)
  participant API as API Router
  participant C as ExtensionsController
  participant S as ExtensionService
  participant ST as Storage (DB)

  U->>FE: Navigate /extensions
  FE->>API: GET /v1/extensions?search=&sort_by=&...
  API->>C: Dispatch
  C->>S: ListExtensions(params)
  S->>ST: ListExtensions(params)
  ST-->>S: ExtensionListResponse
  S-->>C: ExtensionListResponse
  C-->>API: 200 JSON
  API-->>FE: 200 JSON
  FE-->>U: Render grid
Loading
sequenceDiagram
  autonumber
  participant U as User (Browser)
  participant FE as Frontend
  participant API as API Router
  participant C as ExtensionsController
  participant S as ExtensionService
  participant ST as Storage (DB)
  participant ENG as Engine (Modules)

  U->>FE: Run extension
  FE->>API: POST /v1/extensions/{extension_id}/run (vars)
  API->>C: RunExtension
  C->>S: StartRun(extension_id, vars)
  S->>ST: GetExtensionByID
  ST-->>S: Extension + Spec(JSON)
  S->>ST: CreateExecution(status=running)
  S->>ST: CreateExecutionSteps(run+validate)
  Note over S: Async execution
  par Background
    S->>ENG: executeRun(spec, vars) 
    loop For each step
      ENG-->>S: output, compensation
      S->>ST: UpdateStep(status/logs)
      S->>ST: CreateExtensionLog(seq++)
    end
    S->>ST: UpdateExecution(status, completed_at)
  end
  C-->>API: 200 ExtensionExecution
  API-->>FE: 200 JSON
  FE-->>U: Show execution started
Loading
sequenceDiagram
  autonumber
  participant FE as Frontend
  participant API as API Router
  participant C as ExtensionsController
  participant S as ExtensionService
  participant ST as Storage (DB)

  FE->>API: GET /v1/extensions/execution/{id}/logs?after_seq=...
  API->>C: ListExecutionLogs
  C->>S: ListExecutionLogs(id, after, limit)
  S->>ST: ListExtensionLogs(...)
  ST-->>S: Logs[]
  S-->>C: Logs[], next_after
  C-->>API: 200 {logs,next_after}
  API-->>FE: 200 JSON
Loading
sequenceDiagram
  autonumber
  participant FE as Frontend
  participant API as API Router
  participant C as ExtensionsController
  participant S as ExtensionService
  participant ST as Storage (DB)

  FE->>API: POST /v1/extensions/{extension_id}/fork
  API->>C: ForkExtension
  C->>S: ForkExtension(ext_id, yaml_override, author)
  S->>ST: GetExtensionByID
  S->>ST: CreateExtension + Variables
  S-->>C: Forked extension
  C-->>API: 200 JSON
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120+ minutes

Possibly related PRs

Poem

In burrows of code where templates bloom,
I hop through specs and engines vroom.
Logs like carrots, sequenced neat,
Forks sprout branches—tasty feat.
With YAML maps and Docker seas,
Extensions flutter on the breeze.
Thump-thump: shipped with bunny ease. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 “feat: extensions” concisely labels the addition of the new extensions feature across API, UI, storage, and templates, matching the primary scope of this changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/extensions

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.

@raghavyuva raghavyuva changed the base branch from master to feat/develop September 20, 2025 10:26
@raghavyuva raghavyuva changed the base branch from feat/develop to master September 20, 2025 10:28
@raghavyuva raghavyuva changed the base branch from master to feat/develop September 20, 2025 10:28
@raghavyuva raghavyuva self-assigned this Sep 20, 2025
@raghavyuva raghavyuva added this to the Beta Release milestone Sep 20, 2025
@raghavyuva
Copy link
Owner Author

@coderabbitai summary and short title

Copy link
Contributor

coderabbitai bot commented Sep 20, 2025

@raghavyuva I'll regenerate the summary and short title for this PR about extensions functionality.

✅ Actions performed

Summary regeneration triggered.

@raghavyuva raghavyuva marked this pull request as draft September 20, 2025 10:35
@zhravan zhravan deleted the branch master October 3, 2025 12:21
@zhravan zhravan closed this Oct 3, 2025
@github-project-automation github-project-automation bot moved this to Done in Nixopus Oct 3, 2025
@zhravan zhravan reopened this Oct 4, 2025
@raghavyuva raghavyuva changed the base branch from feat/develop to master October 13, 2025 12:30
@raghavyuva raghavyuva marked this pull request as ready for review October 13, 2025 12:30
coderabbitai[bot]

This comment was marked as outdated.

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: 2

Caution

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

⚠️ Outside diff range comments (1)
view/lib/i18n/locales/es.json (1)

1319-1348: Critical: layout is incorrectly nested under navigation.

"layout" must be a top‑level key (see FR/KN). Current nesting will break i18n lookups.

Apply:

   "navigation": {
     "dashboard": "Panel de Control",
     "selfHost": "Autoalojado",
     "fileManager": "Gestor de Archivos",
     "extensions": "Extensiones",
     "settings": "Configuración",
     "general": "General",
     "notifications": "Notificaciones",
     "team": "Equipo",
     "domains": "Dominios",
-    "containers": "Contenedores",
-    "layout": {
-      "breadcrumbs": {
-        "home": "Inicio",
-        "dashboard": "Panel de Control",
-        "selfHost": "Autoalojado",
-        "fileManager": "Gestor de Archivos",
-        "settings": "Configuración",
-        "general": "General",
-        "notifications": "Notificaciones",
-        "team": "Equipo",
-        "domains": "Dominios",
-        "update": "Actualizar",
-        "containers": "Contenedores"
-      },
-      "terminal": {
-        "togglePosition": "Cambiar Posición del Terminal (Ctrl+T)"
-      }
-    }
+    "containers": "Contenedores"
   },
+  "layout": {
+    "breadcrumbs": {
+      "home": "Inicio",
+      "dashboard": "Panel de Control",
+      "selfHost": "Autoalojado",
+      "fileManager": "Gestor de Archivos",
+      "settings": "Configuración",
+      "general": "General",
+      "notifications": "Notificaciones",
+      "team": "Equipo",
+      "domains": "Dominios",
+      "update": "Actualizar",
+      "containers": "Contenedores"
+    },
+    "terminal": {
+      "togglePosition": "Cambiar Posición del Terminal (Ctrl+T)"
+    }
+  },
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3b03319 and 4c3db53.

📒 Files selected for processing (6)
  • api/api/versions.json (1 hunks)
  • api/internal/features/supertokens/auth.go (2 hunks)
  • view/lib/i18n/locales/en.json (3 hunks)
  • view/lib/i18n/locales/es.json (2 hunks)
  • view/lib/i18n/locales/fr.json (2 hunks)
  • view/lib/i18n/locales/kn.json (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • view/lib/i18n/locales/en.json
🔇 Additional comments (10)
api/api/versions.json (2)

13-13: LGTM.

No functional change here.


6-6: Validate RFC3339Nano support across consumers
Your jq test shows fromdateiso8601 fails on nine-digit fractional seconds and non-Z offsets. Ensure every client or library parsing release_date accepts this format. If not, normalize timestamps to RFC3339 (≤3 decimal places) and UTC Z.

api/internal/features/supertokens/auth.go (3)

60-60: LGTM! Read-only access appropriate for member role.

The extension read permission added to the member role correctly follows the principle of least privilege, granting read-only access consistent with the existing member permission pattern.


65-65: LGTM! Read-only access appropriate for viewer role.

The extension read permission added to the viewer role is correct, maintaining the least privileged access level consistent with the viewer role's purpose.


46-46: Extension endpoints RBAC enforcement confirmed The /extensions routes use middleware.RBACMiddleware("extension"), which maps HTTP methods to extension:read/create/update/delete.

view/lib/i18n/locales/kn.json (1)

16-20: Common additions look good.

Comma after preview present; JSON valid.

view/lib/i18n/locales/fr.json (3)

16-20: Resolved: trailing comma after preview.

JSON now valid.


1268-1314: Extensions namespace complete.

Includes run and noVariables; keys align with other locales.


1319-1320: Navigation entry added correctly.

navigation.extensions present and consistent.

view/lib/i18n/locales/es.json (1)

15-19: Common additions OK.

Comma after preview present; JSON valid.

@raghavyuva
Copy link
Owner Author

@coderabbitai pause

Copy link
Contributor

coderabbitai bot commented Oct 14, 2025

✅ Actions performed

Reviews paused.

coderabbitai[bot]

This comment was marked as outdated.

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: 18

♻️ Duplicate comments (4)
api/internal/features/extension/storage/storage.go (2)

173-178: Stop interpolating SortBy directly into ORDER BY

Raw concatenation of SortBy lets a crafted value inject SQL since we don’t whitelist columns before building the ORDER clause.

Replace with a whitelist mapped to real columns and pass via OrderExpr:

-	sortColumn := string(params.SortBy)
-	if params.SortDir == types.SortDirectionDesc {
-		query = query.Order(sortColumn + " DESC")
-	} else {
-		query = query.Order(sortColumn + " ASC")
-	}
+	allowed := map[types.ExtensionSortField]string{
+		types.ExtensionSortFieldName: "name",
+		// TODO: add more fields as you expose them.
+	}
+	sortColumn, ok := allowed[params.SortBy]
+	if !ok {
+		sortColumn = allowed[types.ExtensionSortFieldName]
+	}
+	if params.SortDir == types.SortDirectionDesc {
+		query = query.OrderExpr("? DESC", bun.Ident(sortColumn))
+	} else {
+		query = query.OrderExpr("? ASC", bun.Ident(sortColumn))
+	}

308-313: Use Scan to read RETURNING values

Exec(..., &seq) doesn’t reliably scan RETURNING log_seq; Bun expects Scan.

Switch to Scan:

-		Returning("log_seq").
-		Exec(s.Ctx, &seq)
+		Returning("log_seq").
+		Scan(s.Ctx, &seq)
api/templates/deploy-minio.yaml (2)

55-60: Unused/misleading variable (repeat).

default_buckets is documented as ignored by the official image; remove to avoid confusion or implement bucket creation.

-  default_buckets:
-    type: "string"
-    description: "Comma-separated list of buckets to create (optional, official image ignores this)"
-    default: ""
-    is_required: false

43-53: Insecure default credentials (repeat).

Defaults expose well-known creds. Remove defaults or add strong warnings.

Option 1 — remove defaults:

   access_key:
     type: "string"
     description: "MINIO_ROOT_USER"
-    default: "minioadmin"
     is_required: true
@@
   secret_key:
     type: "string"
     description: "MINIO_ROOT_PASSWORD"
-    default: "minioadmin"
     is_required: true

Option 2 — keep defaults but warn loudly in description (not recommended for production).

🧹 Nitpick comments (16)
api/templates/deploy-beszel.yaml (1)

76-82: Harden validation curl commands with explicit timeouts.

curl defaults let each attempt wait minutes on a hung socket. Please cap connect and total durations so the 60 s/30 s step timeouts behave predictably and the loop advances promptly.

-        cmd: "sh -c 'for i in $(seq 1 30); do code=$(curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 2; done; exit 1'"
+        cmd: "sh -c 'for i in $(seq 1 30); do code=$(curl --connect-timeout 5 --max-time 10 -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 2; done; exit 1'"
@@
-        cmd: "sh -c 'curl -fsS http://localhost:{{ host_port }} | grep -i beszel >/dev/null && exit 0 || exit 1'"
+        cmd: "sh -c 'curl --connect-timeout 5 --max-time 10 -fsS http://localhost:{{ host_port }} | grep -i beszel >/dev/null && exit 0 || exit 1'"
view/app/extensions/components/extensions-hero.tsx (1)

5-6: Remove unused imports.

The Button and ExternalLink imports are not used in this component.

Apply this diff:

-import { Button } from '@/components/ui/button';
-import { ExternalLink } from 'lucide-react';
view/app/extensions/page.tsx (1)

57-57: Simplify error prop.

The pattern error || undefined is redundant since error is already typed as string | null. You can pass error directly, as null is a valid prop value.

Apply this diff:

           <ExtensionsGrid
             extensions={extensions}
             isLoading={isLoading}
-            error={error || undefined}
+            error={error}
             onInstall={handleInstall}
             onViewDetails={handleViewDetails}
           />
view/app/extensions/hooks/use-extensions.ts (1)

61-61: Internationalize error messages.

The error message is hard-coded in English. For consistency with the rest of the application, consider using the translation function for error messages.

Since the useTranslation hook is not currently imported in this file, you could either:

  1. Import and use useTranslation here:
+ import { useTranslation } from '@/hooks/use-translation';

  export function useExtensions() {
+   const { t } = useTranslation();
    const router = useRouter();
    // ... rest of the code
-   const error = apiError ? 'Failed to load extensions' : null;
+   const error = apiError ? t('extensions.errors.loadFailed') : null;
  1. Or return the raw apiError and handle the message formatting in the component layer where translations are already available.
api/templates/deploy-dashdot.yaml (2)

3-4: Fix template name and description typo.

Align name with service and fix typo.

-  name: "Dash"
-  description: "Aa modern server dashboard"
+  name: "Dashdot"
+  description: "A modern server dashboard"

55-77: Add a readiness validation step for consistency/stability.

Other templates validate HTTP readiness; add a simple loop to reduce flakiness.

 execution:
   run:
@@
       timeout: 180
+
+  validate:
+    - name: "Check Dashdot HTTP readiness"
+      type: "command"
+      properties:
+        cmd: "sh -c 'for i in $(seq 1 30); do code=$(curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 2; done; exit 1'"
+      timeout: 60
api/templates/deploy-blocky.yaml (1)

97-101: Make DNS validation command more portable.

Place -port before args; improves compatibility across nslookup variants.

-        cmd: "sh -c 'nslookup google.com 127.0.0.1 -port={{ dns_port }} >/dev/null 2>&1 && exit 0 || exit 1'"
+        cmd: "sh -c 'nslookup -port={{ dns_port }} google.com 127.0.0.1 >/dev/null 2>&1 && exit 0 || exit 1'"
api/templates/deploy-ollama.yaml (1)

49-71: Add API readiness validation.

Add an HTTP loop to ensure the service is up before marking success.

       timeout: 180
+
+  validate:
+    - name: "Check Ollama API readiness"
+      type: "command"
+      properties:
+        cmd: "sh -c 'for i in $(seq 1 30); do curl -fsS http://localhost:{{ host_port }}/api/tags >/dev/null && exit 0; sleep 2; done; exit 1'"
+      timeout: 60
api/templates/deploy-excalidraw.yaml (1)

64-69: Harden validation with a retry loop.

Single-shot curl can be flaky at startup; standardize on looped readiness.

-        cmd: "curl -fsS -o /dev/null -w '%{http_code}\n' http://localhost:{{ host_port }} | grep -E '^(200|301|302)$'"
+        cmd: "sh -c 'for i in $(seq 1 30); do code=$(curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 2; done; exit 1'"
api/templates/deploy-linkding.yaml (1)

49-71: Add HTTP readiness validation.

Align with other templates to reduce flakiness.

       timeout: 180
+
+  validate:
+    - name: "Check Linkding HTTP readiness"
+      type: "command"
+      properties:
+        cmd: "sh -c 'for i in $(seq 1 30); do code=$(curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 2; done; exit 1'"
+      timeout: 60
api/templates/deploy-speedtest-tracker.yaml (2)

91-118: Add HTTP readiness validation.

Include a retrying health check like other templates.

       timeout: 300
+
+  validate:
+    - name: "Check Speedtest Tracker HTTP readiness"
+      type: "command"
+      properties:
+        cmd: "sh -c 'for i in $(seq 1 30); do code=$(curl -kfsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ http_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 2; done; exit 1'"
+      timeout: 60

115-117: Optional: Mount SSL keys conditionally.

Mounting keys volume even when unused is harmless but may confuse operators. If your template engine supports conditionals, gate this mount on a boolean like use_ssl_keys.

api/templates/deploy-firefly-iii.yaml (3)

110-117: Include APP_URL for proper redirects and URL generation

Firefly/Laravel commonly needs APP_URL set. Add it based on host_port.

         env:
           APP_KEY: "{{ app_key }}"
+          APP_URL: "http://localhost:{{ host_port }}"
           DB_CONNECTION: "{{ db_connection }}"
           DB_HOST: "{{ db_host }}"
           DB_PORT: "{{ db_port }}"
           DB_DATABASE: "{{ db_database }}"
           DB_USERNAME: "{{ db_username }}"
           DB_PASSWORD: "{{ db_password }}"

118-119: Persist the full storage directory, not just uploads

Only mounting storage/upload risks losing logs/exports and other state. Mount the whole storage directory.

-        volumes:
-          - "{{ upload_volume_name }}:/var/www/html/storage/upload"
+        volumes:
+          - "{{ upload_volume_name }}:/var/www/html/storage"

55-60: Host networking note for Linux

Defaulting db_host to host.docker.internal often fails on Linux. Consider a safer default or clarify guidance prominently in the UI.

Can you confirm your target environment? On Linux hosts, users may need the host IP or a user-defined bridge network.

api/templates/deploy-url-to-png.yaml (1)

71-75: Make validation robust — avoid brittle content grep

The landing page may not contain "url.*png". Prefer relying on HTTP status (already covered in the first step) or a documented health endpoint.

Option A (remove this step):

-    - name: "Verify URL to PNG service is accessible"
-      type: "command"
-      properties:
-        cmd: "sh -c 'curl -fsS http://localhost:{{ host_port }} | grep -i \"url.*png\" >/dev/null && exit 0 || exit 1'"
-      timeout: 30

Option B (if a health path exists per docs), replace with:

cmd: "sh -c 'curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }}/health | grep -E \"^(200)$\"'"

Please confirm the correct health endpoint from the image docs.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fac665f and 1bd2fa0.

⛔ Files ignored due to path filters (1)
  • view/public/plugin.png is excluded by !**/*.png
📒 Files selected for processing (30)
  • api/api/versions.json (1 hunks)
  • api/internal/features/extension/loader/loader.go (1 hunks)
  • api/internal/features/extension/storage/storage.go (1 hunks)
  • api/templates/deploy-appwrite.yaml (1 hunks)
  • api/templates/deploy-beszel.yaml (1 hunks)
  • api/templates/deploy-blocky.yaml (1 hunks)
  • api/templates/deploy-code-server.yaml (1 hunks)
  • api/templates/deploy-dashdot.yaml (1 hunks)
  • api/templates/deploy-dashy.yaml (1 hunks)
  • api/templates/deploy-dozzle.yaml (1 hunks)
  • api/templates/deploy-excalidraw.yaml (1 hunks)
  • api/templates/deploy-filebrowser.yaml (1 hunks)
  • api/templates/deploy-firefly-iii.yaml (1 hunks)
  • api/templates/deploy-homer.yaml (1 hunks)
  • api/templates/deploy-linkding.yaml (1 hunks)
  • api/templates/deploy-minio.yaml (1 hunks)
  • api/templates/deploy-myspeed.yaml (1 hunks)
  • api/templates/deploy-ollama.yaml (1 hunks)
  • api/templates/deploy-postgres.yaml (1 hunks)
  • api/templates/deploy-redis.yaml (1 hunks)
  • api/templates/deploy-semaphore.yaml (1 hunks)
  • api/templates/deploy-slash.yaml (1 hunks)
  • api/templates/deploy-speedtest-tracker.yaml (1 hunks)
  • api/templates/deploy-stirling-pdf.yaml (1 hunks)
  • api/templates/deploy-uptime-kuma.yaml (1 hunks)
  • api/templates/deploy-url-to-png.yaml (1 hunks)
  • view/app/extensions/components/extensions-header.tsx (1 hunks)
  • view/app/extensions/components/extensions-hero.tsx (1 hunks)
  • view/app/extensions/hooks/use-extensions.ts (1 hunks)
  • view/app/extensions/page.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • api/templates/deploy-uptime-kuma.yaml
  • api/api/versions.json
  • view/app/extensions/components/extensions-header.tsx
  • api/internal/features/extension/loader/loader.go
🧰 Additional context used
🧬 Code graph analysis (4)
api/internal/features/extension/storage/storage.go (1)
api/internal/types/extension.go (8)
  • Extension (51-74)
  • ExtensionVariable (76-89)
  • ExtensionExecution (91-108)
  • ExecutionStep (110-125)
  • ExtensionLog (127-137)
  • ExtensionSortFieldName (184-184)
  • SortDirectionAsc (177-177)
  • SortDirectionDesc (178-178)
view/app/extensions/page.tsx (3)
view/hooks/use-translation.ts (1)
  • useTranslation (6-62)
view/app/extensions/hooks/use-extensions.ts (1)
  • useExtensions (8-100)
view/app/extensions/components/extension-input.tsx (1)
  • ExtensionInput (20-59)
view/app/extensions/components/extensions-hero.tsx (1)
view/hooks/use-translation.ts (1)
  • useTranslation (6-62)
view/app/extensions/hooks/use-extensions.ts (1)
view/redux/types/extension.ts (4)
  • ExtensionSortField (102-108)
  • SortDirection (100-100)
  • Extension (31-52)
  • ExtensionListParams (110-118)
🔇 Additional comments (3)
api/templates/deploy-ollama.yaml (1)

49-71: GPU/device mounts may be needed depending on host.

If targeting NVIDIA/ROCm, you may need runtime device flags (e.g., --gpus, /dev/dri, /dev/kfd) or envs. Ensure docs or variables cover this.

api/templates/deploy-stirling-pdf.yaml (1)

13-23: Verify image reference.

Confirm docker.stirlingpdf.com/stirlingtools/stirling-pdf is the intended/current image for your environment; some docs reference alternate names.

api/templates/deploy-homer.yaml (1)

1-83: LGTM — template is consistent and correct

Image, ports, volume mount (/www/assets), and validation look good.

Comment on lines +59 to +71
- name: "Run Appwrite installer"
type: "docker"
properties:
action: "run"
name: "{{ container_name }}"
image: "{{ image }}"
tag: "{{ tag }}"
ports: "{{ host_port }}:{{ container_port }}"
entrypoint: "install"
volumes:
- "/var/run/docker.sock:/var/run/docker.sock"
- "{{ config_volume_name }}:/usr/src/code/appwrite"
timeout: 600
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Installer entrypoint needs an interactive TTY—this run step will always fail

entrypoint: "install" launches Appwrite’s interactive installer (per the official docker run -it appwrite/appwrite install workflow). In this headless execution we provide no TTY or answers, so the container exits immediately with “interactive terminal required” (or hangs waiting for input). Consequently the stack never starts, nothing listens on host_port, and the subsequent HTTP validator is guaranteed to fail. Please switch to a non-interactive deployment path (e.g., provision the config first, then run the Appwrite services via docker-compose/start) so the template brings up a real Appwrite instance before validating.

🤖 Prompt for AI Agents
In api/templates/deploy-appwrite.yaml around lines 59 to 71: the job uses
entrypoint: "install" which invokes Appwrite's interactive installer requiring a
TTY and will fail in headless CI; replace this step with a non-interactive
deployment flow — either remove the interactive entrypoint and run the
preseed/configuration step (pass env/config files or use Appwrite's
non-interactive install flags) or provision configuration files into the config
volume and start the Appwrite services with a non-interactive command such as
docker-compose up or the Appwrite service start command; ensure ports and
volumes remain mapped and adjust timeout if needed so the template brings up a
real Appwrite instance before the HTTP validator runs.

Comment on lines +80 to +82
volumes:
- "{{ config_volume_name }}:/app/config.yml"
timeout: 180
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Incorrect volume mapping to a file path (will mount a directory, not the file).

Docker named volumes map to directories; mounting a named volume to "/app/config.yml" won’t provide a file and may break startup.

Prefer either:

  • bind-mount a host file path, or
  • mount a directory and configure Blocky to read config from there.

Option A (bind path variable):

-  config_volume_name:
-    type: "string"
-    description: "Named Docker volume for Blocky configuration file"
-    default: "blocky-config"
-    is_required: true
+  config_bind_path:
+    type: "string"
+    description: "Host path to blocky config.yml (bind mount)"
+    default: ""
+    is_required: true
@@
-        volumes:
-          - "{{ config_volume_name }}:/app/config.yml"
+        volumes:
+          - "{{ config_bind_path }}:/app/config.yml:ro"

Option B (directory volume):

-          - "{{ config_volume_name }}:/app/config.yml"
+          - "{{ config_volume_name }}:/app"

Ensure the container reads the correct config path in Option B.

Committable suggestion skipped: line range outside the PR's diff.

Comment on lines +143 to +154
validate:
- name: "Check Code Server HTTP response"
type: "command"
properties:
cmd: "sh -c 'for i in $(seq 1 60); do code=$(curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 5; done; exit 1'"
timeout: 300

- name: "Verify Code Server is accessible"
type: "command"
properties:
cmd: "sh -c 'curl -fsS http://localhost:{{ host_port }} | grep -i \"code-server\" >/dev/null && exit 0 || exit 1'"
timeout: 30
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Validation must target HTTPS endpoint

The linuxserver/code-server image exposes 8443 over HTTPS with a self-signed cert. Hitting http://localhost here either fails the TLS handshake or returns a bare redirect, so both health checks will consistently fail even though the container is healthy. Switch the probes to https://localhost:{{ host_port }} and add -k (or supply CA material) so the validation phase works against the real endpoint. Without that fix the template can never pass validation.

-        cmd: "sh -c 'for i in $(seq 1 60); do code=$(curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 5; done; exit 1'"
+        cmd: "sh -c 'for i in $(seq 1 60); do code=$(curl -kfsS -o /dev/null -w \"%{http_code}\\n\" https://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 5; done; exit 1'"
...
-        cmd: "sh -c 'curl -fsS http://localhost:{{ host_port }} | grep -i \"code-server\" >/dev/null && exit 0 || exit 1'"
+        cmd: "sh -c 'curl -kfsS https://localhost:{{ host_port }} | grep -i \"code-server\" >/dev/null && exit 0 || exit 1'"
📝 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
validate:
- name: "Check Code Server HTTP response"
type: "command"
properties:
cmd: "sh -c 'for i in $(seq 1 60); do code=$(curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 5; done; exit 1'"
timeout: 300
- name: "Verify Code Server is accessible"
type: "command"
properties:
cmd: "sh -c 'curl -fsS http://localhost:{{ host_port }} | grep -i \"code-server\" >/dev/null && exit 0 || exit 1'"
timeout: 30
validate:
- name: "Check Code Server HTTP response"
type: "command"
properties:
cmd: "sh -c 'for i in $(seq 1 60); do code=$(curl -kfsS -o /dev/null -w \"%{http_code}\\n\" https://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 5; done; exit 1'"
timeout: 300
- name: "Verify Code Server is accessible"
type: "command"
properties:
cmd: "sh -c 'curl -kfsS https://localhost:{{ host_port }} | grep -i \"code-server\" >/dev/null && exit 0 || exit 1'"
timeout: 30
🤖 Prompt for AI Agents
In api/templates/deploy-code-server.yaml around lines 143 to 154, the health
checks are using http:// which fails for linuxserver/code-server (it serves
HTTPS on 8443 with a self-signed cert); change both curl invocations to use
https://localhost:{{ host_port }} and add the insecure flag (-k) or
alternatively supply CA material so TLS succeeds; keep the same HTTP code checks
(200/301/302) and timeouts but update the command strings to use https and -k so
the validation phase can reach the real endpoint.

Comment on lines +72 to +76
ports: "{{ host_port }}:{{ container_port }}"
restart: "unless-stopped"
privileged: true
volumes:
- "{{ host_mount_path }}:{{ container_mount_path }}:ro"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Avoid privileged container and full host root mount.

Privileged=true + binding "/" (even ro) is a high-risk posture. Dashdot typically only needs specific read-only mounts; avoid blanket root mount and drop privileged if possible.

Example safer approach (adjust to what your runtime supports):

-        restart: "unless-stopped"
-        privileged: true
-        volumes:
-          - "{{ host_mount_path }}:{{ container_mount_path }}:ro"
+        restart: "unless-stopped"
+        volumes:
+          - "/proc:/mnt/host/proc:ro"
+          - "/sys:/mnt/host/sys:ro"
+          - "/etc/os-release:/mnt/host/etc/os-release:ro"

If additional paths are required, enumerate them explicitly instead of mounting "/".

🤖 Prompt for AI Agents
In api/templates/deploy-dashdot.yaml around lines 72 to 76, the service is
configured with privileged: true and a blanket host mount ("{{ host_mount_path
}}:{{ container_mount_path }}:ro") which is high-risk; remove privileged: true,
replace the single root/blanket host mount with explicit, minimal read-only
mounts for only the required host paths (enumerate each path separately), and if
elevated access is needed instead grant only specific capabilities or use
securityContext/seccomp/AppArmor profiles rather than full privilege; keep
ports/restart as-is but ensure the template variables reflect the narrowed mount
list.

Comment on lines +80 to +101
env:
DOZZLE_LEVEL: "info"
DOZZLE_TAILSIZE: "300"
DOZZLE_FILTER: ""
DOZZLE_ACTIONS: "{{ enable_actions }}"
DOZZLE_SHELL: "{{ enable_shell }}"
volumes:
- "{{ docker_socket_path }}:/var/run/docker.sock:ro"
timeout: 180

validate:
- name: "Check Dozzle HTTP response"
type: "command"
properties:
cmd: "sh -c 'for i in $(seq 1 30); do code=$(curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 2; done; exit 1'"
timeout: 60

- name: "Verify Dozzle is accessible"
type: "command"
properties:
cmd: "sh -c 'curl -fsS http://localhost:{{ host_port }} | grep -i dozzle >/dev/null && exit 0 || exit 1'"
timeout: 30
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Require Dozzle basic auth before shipping this template

We’re starting Dozzle with no authentication while binding it on the host port. Anyone who can hit that port gets full log access (and, if actions/shell are enabled, can control the Docker host) because we never set DOZZLE_USERNAME/DOZZLE_PASSWORD. That’s a high‑risk exposure for secrets and host control.

Please add required variables for the credentials and pass them through to the container, e.g.:

 variables:
+  username:
+    type: "string"
+    description: "Basic auth username for Dozzle UI"
+    is_required: true
+
+  password:
+    type: "string"
+    description: "Basic auth password for Dozzle UI"
+    is_required: true
+    is_secret: true
 ...
         env:
           DOZZLE_LEVEL: "info"
           DOZZLE_TAILSIZE: "300"
           DOZZLE_FILTER: ""
           DOZZLE_ACTIONS: "{{ enable_actions }}"
           DOZZLE_SHELL: "{{ enable_shell }}"
+          DOZZLE_USERNAME: "{{ username }}"
+          DOZZLE_PASSWORD: "{{ password }}"

(other env keys and descriptions can stay as-is). This keeps the template safe by forcing credentials before exposing the UI.

📝 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
env:
DOZZLE_LEVEL: "info"
DOZZLE_TAILSIZE: "300"
DOZZLE_FILTER: ""
DOZZLE_ACTIONS: "{{ enable_actions }}"
DOZZLE_SHELL: "{{ enable_shell }}"
volumes:
- "{{ docker_socket_path }}:/var/run/docker.sock:ro"
timeout: 180
validate:
- name: "Check Dozzle HTTP response"
type: "command"
properties:
cmd: "sh -c 'for i in $(seq 1 30); do code=$(curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 2; done; exit 1'"
timeout: 60
- name: "Verify Dozzle is accessible"
type: "command"
properties:
cmd: "sh -c 'curl -fsS http://localhost:{{ host_port }} | grep -i dozzle >/dev/null && exit 0 || exit 1'"
timeout: 30
# … (some lines above)
variables:
# existing variables…
enable_actions:
type: "boolean"
default: false
enable_shell:
type: "boolean"
default: false
host_port:
type: "integer"
default: 8080
docker_socket_path:
type: "string"
default: "/var/run/docker.sock"
# <<< new parameters for basic auth >>>
username:
type: "string"
description: "Basic auth username for Dozzle UI"
is_required: true
password:
type: "string"
description: "Basic auth password for Dozzle UI"
is_required: true
is_secret: true
# … (more of the template)
steps:
- name: "Deploy Dozzle"
type: "deploy"
properties:
image: "amancevice/dozzle:latest"
env:
DOZZLE_LEVEL: "info"
DOZZLE_TAILSIZE: "300"
DOZZLE_FILTER: ""
DOZZLE_ACTIONS: "{{ enable_actions }}"
DOZZLE_SHELL: "{{ enable_shell }}"
DOZZLE_USERNAME: "{{ username }}"
DOZZLE_PASSWORD: "{{ password }}"
volumes:
- "{{ docker_socket_path }}:/var/run/docker.sock:ro"
timeout: 180
validate:
- name: "Check Dozzle HTTP response"
type: "command"
properties:
cmd: "sh -c 'for i in $(seq 1 30); do code=$(curl -fsS -o /dev/null -w \"%{http_code}\\n\" http://localhost:{{ host_port }} || true); echo \"HTTP $code\"; echo $code | grep -E \"^(200|301|302)$\" && exit 0; sleep 2; done; exit 1'"
timeout: 60
- name: "Verify Dozzle is accessible"
type: "command"
properties:
cmd: "sh -c 'curl -fsS http://localhost:{{ host_port }} | grep -i dozzle >/dev/null && exit 0 || exit 1'"
timeout: 30
🤖 Prompt for AI Agents
In api/templates/deploy-dozzle.yaml around lines 80 to 101, the template starts
Dozzle with no authentication which exposes logs and host control; add required
credential environment variables (DOZZLE_USERNAME and DOZZLE_PASSWORD) to the
env block and wire them into the container from template variables (no safe
default), update the template variable definitions to mark them required (or
fail if empty) so deployments must provide credentials, and keep existing env
keys intact.

<Skeleton className="h-5 w-12 rounded-full" />
<Skeleton className="h-6 w-48 md:w-56 lg:w-64" />
<Skeleton className="h-4 w-72 md:w-80" />
<Skeleton className="h-8 w-32 mt-2" />
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Remove skeleton element that doesn't match the main component.

The skeleton includes a button placeholder at line 63 (<Skeleton className="h-8 w-32 mt-2" />) that doesn't correspond to any element in the main ExtensionsHero component.

Apply this diff to remove the inconsistent skeleton:

           <Skeleton className="h-5 w-12 rounded-full" />
           <Skeleton className="h-6 w-48 md:w-56 lg:w-64" />
           <Skeleton className="h-4 w-72 md:w-80" />
-          <Skeleton className="h-8 w-32 mt-2" />
         </div>
📝 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
<Skeleton className="h-8 w-32 mt-2" />
<Skeleton className="h-5 w-12 rounded-full" />
<Skeleton className="h-6 w-48 md:w-56 lg:w-64" />
<Skeleton className="h-4 w-72 md:w-80" />
</div>
🤖 Prompt for AI Agents
In view/app/extensions/components/extensions-hero.tsx around line 63, there is
an extraneous Skeleton placeholder (<Skeleton className="h-8 w-32 mt-2" />) that
does not correspond to any element in the main ExtensionsHero component; remove
that Skeleton element so the skeleton markup matches the actual component
structure, leaving only skeletons that mirror existing rendered elements.

Comment on lines +69 to +74
<Image
src="/plugin.png"
alt="Extensions Hero"
className="w-full h-full text-white object-contain "
fill
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix Image component prop conflicts (skeleton variant).

Same issue as in the main component: the fill prop conflicts with w-full h-full classes, text-white has no effect, and there's a trailing space.

Apply this diff:

               <div className="text-center">
                 <Image
                   src="/plugin.png"
                   alt="Extensions Hero"
-                  className="w-full h-full text-white object-contain "
+                  className="object-contain"
                   fill
                 />
               </div>
📝 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
<Image
src="/plugin.png"
alt="Extensions Hero"
className="w-full h-full text-white object-contain "
fill
/>
<div className="text-center">
<Image
src="/plugin.png"
alt="Extensions Hero"
className="object-contain"
fill
/>
</div>
🤖 Prompt for AI Agents
In view/app/extensions/components/extensions-hero.tsx around lines 69 to 74, the
Image usage has conflicting props and useless classes: the fill prop conflicts
with the w-full h-full classes, text-white has no effect on an img and there's a
trailing space in className; remove the fill prop (or if you prefer fill, remove
w-full h-full instead), delete text-white from the className, trim the trailing
space, and ensure className ends up as a single string like "w-full h-full
object-contain" (or "object-contain" if you keep fill) so the Image layout and
styling are consistent.

Comment on lines +66 to +74
const handleRun = async (values: Record<string, unknown>) => {
if (!selectedExtension) return;
const exec = await runExtensionMutation({
extensionId: selectedExtension.extension_id,
body: { variables: values }
}).unwrap();
setRunModalOpen(false);
router.push(`/extensions/${selectedExtension.id}?exec=${exec.id}&openLogs=1`);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Add error handling for extension execution.

The handleRun function uses unwrap() which throws on API errors, but there's no try-catch block to handle failures. This will result in unhandled promise rejections if the mutation fails.

Apply this diff to add error handling:

   const handleRun = async (values: Record<string, unknown>) => {
     if (!selectedExtension) return;
-    const exec = await runExtensionMutation({
-      extensionId: selectedExtension.extension_id,
-      body: { variables: values }
-    }).unwrap();
-    setRunModalOpen(false);
-    router.push(`/extensions/${selectedExtension.id}?exec=${exec.id}&openLogs=1`);
+    try {
+      const exec = await runExtensionMutation({
+        extensionId: selectedExtension.extension_id,
+        body: { variables: values }
+      }).unwrap();
+      setRunModalOpen(false);
+      router.push(`/extensions/${selectedExtension.id}?exec=${exec.id}&openLogs=1`);
+    } catch (error) {
+      // Handle error (show toast notification, set error state, etc.)
+      console.error('Failed to run extension:', error);
+      // Consider keeping modal open or showing error feedback to user
+    }
   };
📝 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
const handleRun = async (values: Record<string, unknown>) => {
if (!selectedExtension) return;
const exec = await runExtensionMutation({
extensionId: selectedExtension.extension_id,
body: { variables: values }
}).unwrap();
setRunModalOpen(false);
router.push(`/extensions/${selectedExtension.id}?exec=${exec.id}&openLogs=1`);
};
const handleRun = async (values: Record<string, unknown>) => {
if (!selectedExtension) return;
try {
const exec = await runExtensionMutation({
extensionId: selectedExtension.extension_id,
body: { variables: values }
}).unwrap();
setRunModalOpen(false);
router.push(`/extensions/${selectedExtension.id}?exec=${exec.id}&openLogs=1`);
} catch (error) {
// Handle error (e.g., show toast notification or keep modal open)
console.error('Failed to run extension:', error);
}
};
🤖 Prompt for AI Agents
In view/app/extensions/hooks/use-extensions.ts around lines 66 to 74, wrap the
await runExtensionMutation(...).unwrap() call in a try/catch so API errors are
handled instead of bubbling as unhandled rejections; on success keep the
existing setRunModalOpen(false) and router.push(...), on failure log the error
(or call an existing notification/toast/error handler) and do not close the
modal or navigate, and ensure the early return when !selectedExtension remains
in place.

Comment on lines +76 to +78
const handleCancel = async (executionId: string) => {
await cancelExecutionMutation({ executionId });
};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add error handling for execution cancellation.

The handleCancel function has no error handling for the cancellation mutation. If the cancellation fails, the error will be silently swallowed.

Apply this diff to add error handling:

   const handleCancel = async (executionId: string) => {
-    await cancelExecutionMutation({ executionId });
+    try {
+      await cancelExecutionMutation({ executionId }).unwrap();
+    } catch (error) {
+      // Handle error (show toast notification, etc.)
+      console.error('Failed to cancel execution:', error);
+    }
   };
📝 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
const handleCancel = async (executionId: string) => {
await cancelExecutionMutation({ executionId });
};
const handleCancel = async (executionId: string) => {
try {
await cancelExecutionMutation({ executionId }).unwrap();
} catch (error) {
// Handle error (show toast notification, etc.)
console.error('Failed to cancel execution:', error);
}
};
🤖 Prompt for AI Agents
In view/app/extensions/hooks/use-extensions.ts around lines 76 to 78, the
handleCancel function currently awaits cancelExecutionMutation without any error
handling; wrap the await call in a try/catch, call cancelExecutionMutation
inside the try, handle failures in the catch by logging the error
(processLogger/console.error) and surface a user-facing notification or return
an error status, and rethrow or return a failed result as appropriate so the
caller can react to failures.

Comment on lines +48 to +52
{totalExtensions > 0 && (
<div className="text-sm text-muted-foreground self-end justify-end flex">
Showing {extensions.length} of {totalExtensions} extensions
</div>
)}
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Internationalize the extensions count message.

The extension count message is hard-coded in English and should use the translation function for consistency with the rest of the UI.

Apply this diff to use translated strings:

           {totalExtensions > 0 && (
             <div className="text-sm text-muted-foreground self-end justify-end flex">
-              Showing {extensions.length} of {totalExtensions} extensions
+              {t('extensions.showing', { 
+                current: extensions.length.toString(), 
+                total: totalExtensions.toString() 
+              })}
             </div>
           )}

Then add the translation key to your locale files:

"extensions": {
  "showing": "Showing {current} of {total} extensions"
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants