From 2d75eba66f690f4fe35befc20989b99d83397ee0 Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Tue, 8 Sep 2026 02:41:05 +0000 Subject: [PATCH 01/12] fix(admin): point the attention card at what is actually wrong The card was a stat tile showing `summary.failedServers` while its caption counted failed backups and servers mid-delete, so it could read "0" with backups broken. Either way its link went to the unfiltered server list, which told an operator nothing they did not already know. Closes #163. The overview endpoint now carries the records behind each row, not just the counts, each with the route key its own destination takes: a server id for the server groups, the owning server's uuid_short for a backup (the backups tab is the only page a backup appears on, and ServerPolicy::before lets an admin open any server's). Groups are capped at 25 records so a broken fleet cannot turn the dashboard into a full table read; the count beside the row stays the authority for how many there really are. The card lists those records. One record is named outright and links straight at itself; several collapse into a disclosure whose summary previews the names and whose panel links each record separately. Nothing wrong at all gets an "all clear" state rather than a wall of zeros. Deleting servers are gone from the card: mid-delete is a transient state, not a failure, and the server-state card already counts it. Dropping it is what removes the ambiguity the issue is named for. The attention tile leaves the top row to three stat cards; the card itself takes the column beside Capacity, and Server State moves to full width. The overview tests now flush the cache between cases. OverviewService caches its payload for 15s, and whether that survives between tests depends on the ambient cache driver -- an array store is per-process and hides it, a shared redis does not. --- app/Services/Admin/OverviewService.php | 89 ++++++ .../Admin/OverviewTransformer.php | 21 ++ lang/en_US/admin/overview.php | 14 +- .../scripts/api/admin/overview/getOverview.ts | 30 ++ .../admin/overview/AttentionCard.tsx | 265 ++++++++++++++++++ .../admin/overview/OverviewContainer.tsx | 51 +--- .../Admin/OverviewControllerTest.php | 80 ++++++ 7 files changed, 511 insertions(+), 39 deletions(-) create mode 100644 resources/scripts/components/admin/overview/AttentionCard.tsx diff --git a/app/Services/Admin/OverviewService.php b/app/Services/Admin/OverviewService.php index b5574ad48d0..d3b2ea61a55 100644 --- a/app/Services/Admin/OverviewService.php +++ b/app/Services/Admin/OverviewService.php @@ -11,6 +11,7 @@ use Convoy\Models\Node; use Convoy\Models\Server; use Convoy\Models\User; +use Illuminate\Database\Eloquent\Builder; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; @@ -20,6 +21,14 @@ class OverviewService private const BYTES_PER_MEBIBYTE = 1048576; + /** + * Per-group cap on the records carried for the attention card. The card lists a + * handful and the group's count stays the authority for how many there really + * are, so a fleet where everything is broken costs a bounded query rather than a + * full table read on every dashboard load. + */ + private const ATTENTION_LIMIT = 25; + public function metrics(): array { return Cache::remember( @@ -43,6 +52,7 @@ private function build(): array 'addresses' => $this->addresses(), 'backups' => $this->backups(), 'isos' => $this->isos(), + 'attention' => $this->attention(), 'nodes' => $nodes ->map(fn (Node $node) => $this->node($node, $allocations)) ->all(), @@ -117,6 +127,85 @@ private function failedServers(Collection $statuses): int ); } + /** + * The records behind the attention card, each carrying the key its destination + * route takes. + * + * The card used to be a stat tile showing only the failed-server count while its + * caption mixed in failed backups and servers mid-delete, so it could read "0" + * with backups broken, and its link went to the unfiltered server list either + * way. Listing the records themselves is what lets a click land on the thing + * that is wrong. + * + * Deleting servers are deliberately absent: mid-delete is a transient state + * rather than a failure, and the server-state card already counts it. + */ + private function attention(): array + { + return [ + 'failed_servers' => $this->serverSubjects( + Server::query()->whereIn('status', [ + Status::INSTALL_FAILED->value, + Status::DELETION_FAILED->value, + ]), + fn (Server $server) => sprintf( + '%s on %s', + $server->status === Status::DELETION_FAILED->value + ? 'Deletion failed' + : 'Installation failed', + $server->node?->name ?? 'an unknown node', + ), + ), + 'failed_backups' => $this->failedBackupSubjects(), + 'suspended_servers' => $this->serverSubjects( + Server::query()->where('status', Status::SUSPENDED->value), + fn (Server $server) => 'On '.($server->node?->name ?? 'an unknown node'), + ), + ]; + } + + /** + * @param callable(Server): ?string $detail + */ + private function serverSubjects(Builder $query, callable $detail): array + { + return $query + ->with('node:id,name') + ->orderByDesc('id') + ->limit(self::ATTENTION_LIMIT) + ->get(['id', 'name', 'node_id', 'status']) + ->map(fn (Server $server) => [ + 'id' => (string) $server->id, + 'label' => $server->name, + 'detail' => $detail($server), + ]) + ->all(); + } + + /** + * Failed backups, keyed by the owning server's uuid_short -- the client server's + * backups tab is the only page that shows a backup, and ServerPolicy::before lets + * an admin open it for any server. A backup carries no failure message on this + * branch, so the detail names the server and when it gave up instead. + */ + private function failedBackupSubjects(): array + { + return Backup::query() + ->whereNotNull('completed_at') + ->where('is_successful', false) + ->whereHas('server') + ->with('server:id,uuid_short,name') + ->orderByDesc('completed_at') + ->limit(self::ATTENTION_LIMIT) + ->get(['id', 'server_id', 'name', 'completed_at']) + ->map(fn (Backup $backup) => [ + 'id' => $backup->server->uuid_short, + 'label' => $backup->name, + 'detail' => $backup->server->name.' · failed '.$backup->completed_at->diffForHumans(), + ]) + ->all(); + } + private function capacity(Collection $nodes, Collection $allocations): array { $memoryAllocated = $allocations->sum( diff --git a/app/Transformers/Admin/OverviewTransformer.php b/app/Transformers/Admin/OverviewTransformer.php index 694b081c02b..7826bd6381a 100644 --- a/app/Transformers/Admin/OverviewTransformer.php +++ b/app/Transformers/Admin/OverviewTransformer.php @@ -49,6 +49,11 @@ public function transform(array $overview): array 'successful' => $overview['isos']['successful'], 'pending' => $overview['isos']['pending'], ], + 'attention' => [ + 'failed_servers' => $this->subjects($overview['attention']['failed_servers']), + 'failed_backups' => $this->subjects($overview['attention']['failed_backups']), + 'suspended_servers' => $this->subjects($overview['attention']['suspended_servers']), + ], 'nodes' => collect($overview['nodes']) ->map(fn (array $node) => [ 'id' => $node['id'], @@ -63,6 +68,22 @@ public function transform(array $overview): array ]; } + /** + * One attention group: the records behind a row, each with the route key its + * destination takes. Capped upstream, so a group can be shorter than the count + * reported beside it. + */ + private function subjects(array $subjects): array + { + return collect($subjects) + ->map(fn (array $subject) => [ + 'id' => $subject['id'], + 'label' => $subject['label'], + 'detail' => $subject['detail'], + ]) + ->all(); + } + private function metric(array $metric): array { return [ diff --git a/lang/en_US/admin/overview.php b/lang/en_US/admin/overview.php index a6a5ce7067a..05098f7baa0 100644 --- a/lang/en_US/admin/overview.php +++ b/lang/en_US/admin/overview.php @@ -7,7 +7,19 @@ 'nodes_locations_detail_one' => 'across :count location', 'nodes_locations_detail_other' => 'across :count locations', 'attention' => 'Attention', - 'attention_detail' => ':backups failed backups, :deleting deleting', + 'attention_description' => 'Everything currently in a state you have to do something about.', + 'attention_all_clear' => 'All clear', + 'attention_all_clear_detail' => 'No failed servers, no failed backups, nothing suspended.', + 'attention_fix' => 'Fix', + 'attention_view' => 'View', + 'attention_failed_servers_one' => ':count server failed', + 'attention_failed_servers_other' => ':count servers failed', + 'attention_failed_backups_one' => ':count backup failed', + 'attention_failed_backups_other' => ':count backups failed', + 'attention_suspended_servers_one' => ':count server suspended', + 'attention_suspended_servers_other' => ':count servers suspended', + 'attention_more_one' => ':count more', + 'attention_more_other' => ':count more', 'capacity' => 'Capacity', 'capacity_description' => 'Convoy allocated resources across all nodes.', 'allocated_percent' => ':percent% allocated in Convoy', diff --git a/resources/scripts/api/admin/overview/getOverview.ts b/resources/scripts/api/admin/overview/getOverview.ts index 9f7f50dfcf6..40bffd1300f 100644 --- a/resources/scripts/api/admin/overview/getOverview.ts +++ b/resources/scripts/api/admin/overview/getOverview.ts @@ -46,6 +46,23 @@ export interface DashboardIsos { pending: number } +/** + * One record behind an attention row. `id` is the route key its group's + * destination takes -- a server id for the server groups, a server's short uuid + * for a backup, since the backups tab is where a backup actually lives. + */ +export interface AttentionSubject { + id: string + label: string + detail: string | null +} + +export interface DashboardAttention { + failedServers: AttentionSubject[] + failedBackups: AttentionSubject[] + suspendedServers: AttentionSubject[] +} + export interface DashboardNode { id: number name: string @@ -67,9 +84,17 @@ export interface DashboardOverview { addresses: DashboardAddresses backups: DashboardBackups isos: DashboardIsos + attention: DashboardAttention nodes: DashboardNode[] } +const rawSubjects = (data: any): AttentionSubject[] => + (data ?? []).map((subject: any) => ({ + id: String(subject.id), + label: subject.label, + detail: subject.detail ?? null, + })) + const rawMetric = (data: any): DashboardMetric => ({ allocated: data.allocated, total: data.total, @@ -93,6 +118,11 @@ export const rawDataToOverview = (data: any): DashboardOverview => ({ addresses: data.addresses, backups: data.backups, isos: data.isos, + attention: { + failedServers: rawSubjects(data.attention?.failed_servers), + failedBackups: rawSubjects(data.attention?.failed_backups), + suspendedServers: rawSubjects(data.attention?.suspended_servers), + }, nodes: data.nodes.map((node: any) => ({ id: node.id, name: node.name, diff --git a/resources/scripts/components/admin/overview/AttentionCard.tsx b/resources/scripts/components/admin/overview/AttentionCard.tsx new file mode 100644 index 00000000000..a03ba5743eb --- /dev/null +++ b/resources/scripts/components/admin/overview/AttentionCard.tsx @@ -0,0 +1,265 @@ +import { + ArrowRightIcon, + CheckCircleIcon, + ChevronDownIcon, + ExclamationTriangleIcon, + PauseCircleIcon, +} from '@heroicons/react/24/outline' +import { Collapse } from '@mantine/core' +import { ComponentType, useState } from 'react' +import { useTranslation } from 'react-i18next' +import { Link } from 'react-router-dom' + +import { + AttentionSubject, + DashboardOverview, +} from '@/api/admin/overview/getOverview' + +import Card from '@/components/elements/Card' + +interface IconProps { + className?: string +} + +type Tone = 'error' | 'warning' + +interface AttentionGroup { + key: string + icon: ComponentType + tone: Tone + /** Summary line used when the group holds more than one record. */ + title: string + actionLabel: string + /** The records themselves, capped by the endpoint. */ + subjects: AttentionSubject[] + /** Destination for one record. */ + to: (subject: AttentionSubject) => string + /** Real total, which can exceed what `subjects` carries. */ + total: number + /** Where the leftovers live when the list is capped. */ + overflowTo: string +} + +const toneClasses: Record = { + error: 'text-error border-error-light bg-error-lighter', + warning: 'text-warning-dark border-warning bg-warning-lighter', +} + +const SubjectRow = ({ + subject, + to, +}: { + subject: AttentionSubject + to: string +}) => ( + +

+ {subject.label} +

+ {subject.detail && ( +

{subject.detail}

+ )} + +) + +/** + * The names behind a collapsed group, so the summary line carries something the + * count does not. Truncation is the browser's job -- the string names as many as + * it has and says how many it left out. + */ +const preview = (group: AttentionGroup): string => { + const names = group.subjects.map(subject => subject.label) + const hidden = group.total - names.length + + return hidden > 0 ? `${names.join(', ')} and ${hidden} more` : names.join(', ') +} + +/** + * One group. A single record is named outright and links straight at itself -- + * "1 server failed to install" plus a hunt through the server list is strictly + * less information than naming the server. Several become a disclosure over + * direct links, because the count alone is what made the old card useless. + */ +const GroupRow = ({ group }: { group: AttentionGroup }) => { + const [open, setOpen] = useState(false) + const { t } = useTranslation('admin.overview') + const Icon = group.icon + const truncated = group.total > group.subjects.length + + const icon = ( +
+ +
+ ) + + if (group.subjects.length === 1) { + const [subject] = group.subjects + + return ( + + {icon} +
+

+ {subject.label} +

+

+ {subject.detail} +

+
+ + {group.actionLabel} + + + + ) + } + + return ( +
+ + +
+ {group.subjects.map(subject => ( + + ))} + {truncated && ( + + {t('attention_more', { + count: group.total - group.subjects.length, + })} + + )} +
+ +
+ ) +} + +const AttentionCard = ({ data }: { data: DashboardOverview }) => { + const { t } = useTranslation('admin.overview') + const { attention, summary, backups, servers } = data + + const groups: AttentionGroup[] = [] + + if (attention.failedServers.length > 0) { + groups.push({ + key: 'failed-servers', + icon: ExclamationTriangleIcon, + tone: 'error', + title: t('attention_failed_servers', { + count: summary.failedServers, + }), + actionLabel: t('attention_fix'), + subjects: attention.failedServers, + to: subject => `/admin/servers/${subject.id}`, + total: summary.failedServers, + overflowTo: '/admin/servers', + }) + } + + if (attention.failedBackups.length > 0) { + groups.push({ + key: 'failed-backups', + icon: ExclamationTriangleIcon, + tone: 'error', + title: t('attention_failed_backups', { count: backups.failed }), + actionLabel: t('attention_view'), + // The backups tab is the only page that shows a backup, and it is + // keyed by the server's short uuid rather than its id. + to: subject => `/servers/${subject.id}/backups`, + subjects: attention.failedBackups, + total: backups.failed, + overflowTo: '/admin/servers', + }) + } + + if (attention.suspendedServers.length > 0) { + groups.push({ + key: 'suspended-servers', + icon: PauseCircleIcon, + tone: 'warning', + title: t('attention_suspended_servers', { + count: servers.suspended, + }), + actionLabel: t('attention_view'), + subjects: attention.suspendedServers, + to: subject => `/admin/servers/${subject.id}`, + total: servers.suspended, + overflowTo: '/admin/servers', + }) + } + + return ( + +
+
+

{t('attention')}

+

+ {t('attention_description')} +

+
+ 0 ? 'text-error' : 'text-accent-400' + }`} + /> +
+ + {groups.length === 0 ? ( +
+ +

+ {t('attention_all_clear')} +

+

+ {t('attention_all_clear_detail')} +

+
+ ) : ( +
+ {groups.map(group => ( + + ))} +
+ )} + + ) +} + +export default AttentionCard diff --git a/resources/scripts/components/admin/overview/OverviewContainer.tsx b/resources/scripts/components/admin/overview/OverviewContainer.tsx index fc7a630666d..735dddaa953 100644 --- a/resources/scripts/components/admin/overview/OverviewContainer.tsx +++ b/resources/scripts/components/admin/overview/OverviewContainer.tsx @@ -2,7 +2,6 @@ import { bytesToString } from '@/util/helpers' import { CircleStackIcon, CpuChipIcon, - ExclamationTriangleIcon, ServerStackIcon, SignalIcon, UsersIcon, @@ -18,6 +17,8 @@ import { DashboardNode, } from '@/api/admin/overview/getOverview' +import AttentionCard from '@/components/admin/overview/AttentionCard' + import Card from '@/components/elements/Card' import MessageBox from '@/components/elements/MessageBox' import PageContentBlock from '@/components/elements/PageContentBlock' @@ -32,23 +33,9 @@ interface StatCardProps { detail?: string icon: ComponentType to?: string - tone?: 'default' | 'warning' | 'error' -} - -const toneClasses = { - default: 'text-accent-600 border-accent-200 bg-accent-100', - warning: 'text-warning-dark border-warning bg-warning-lighter', - error: 'text-error border-error-light bg-error-lighter', } -const StatCard = ({ - title, - value, - detail, - icon: Icon, - to, - tone = 'default', -}: StatCardProps) => { +const StatCard = ({ title, value, detail, icon: Icon, to }: StatCardProps) => { const content = (
@@ -58,7 +45,7 @@ const StatCard = ({

{detail &&

{detail}

}
-
+
@@ -68,7 +55,7 @@ const StatCard = ({ return ( {content} @@ -78,7 +65,7 @@ const StatCard = ({ } return ( - + {content} ) @@ -153,15 +140,16 @@ const NodeRow = ({ node }: { node: DashboardNode }) => { const OverviewSkeleton = () => (
- {[1, 2, 3, 4].map(item => ( + {[1, 2, 3].map(item => ( ))} +
) @@ -222,21 +210,6 @@ const OverviewContainer = () => { icon={UsersIcon} to='/admin/users' /> - 0 - ? 'error' - : 'default' - } - />
@@ -306,7 +279,9 @@ const OverviewContainer = () => {
- + + +

{t('server_state')}

@@ -316,7 +291,7 @@ const OverviewContainer = () => {
-
+
{[ [t('ready'), data.servers.ready], [t('installing'), data.servers.installing], diff --git a/tests/Feature/Controllers/Admin/OverviewControllerTest.php b/tests/Feature/Controllers/Admin/OverviewControllerTest.php index cfd91a7cbee..28b1b77bce7 100644 --- a/tests/Feature/Controllers/Admin/OverviewControllerTest.php +++ b/tests/Feature/Controllers/Admin/OverviewControllerTest.php @@ -9,6 +9,14 @@ use Convoy\Models\Node; use Convoy\Models\Server; use Convoy\Models\User; +use Illuminate\Support\Facades\Cache; + +beforeEach(function () { + // OverviewService caches its payload for 15s. Whether that survives between + // cases depends on the ambient cache driver -- an array store is per-process + // and hides the problem, a shared redis does not -- so pin it either way. + Cache::flush(); +}); it('returns overview metrics for admins', function () { $admin = User::factory()->create([ @@ -100,3 +108,75 @@ $this->actingAs($user)->getJson('/api/admin/overview') ->assertForbidden(); }); + +it('names the servers and backups behind each attention row', function () { + $admin = User::factory()->create(['root_admin' => true]); + $location = Location::factory()->create(); + $node = Node::factory()->for($location)->create(['name' => 'pve-1']); + + $failed = Server::factory()->for($node)->for($admin)->create([ + 'name' => 'broken-install', + 'status' => Status::INSTALL_FAILED->value, + ]); + $suspended = Server::factory()->for($node)->for($admin)->create([ + 'name' => 'unpaid', + 'status' => Status::SUSPENDED->value, + ]); + Backup::factory()->for($suspended)->create([ + 'name' => 'nightly', + 'is_successful' => false, + 'completed_at' => now(), + ]); + + $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + // Each row carries the route key its own destination takes, so a click + // lands on the record rather than on the unfiltered server list. + ->assertJsonPath('data.attention.failed_servers.0.id', (string) $failed->id) + ->assertJsonPath('data.attention.failed_servers.0.label', 'broken-install') + ->assertJsonPath('data.attention.failed_servers.0.detail', 'Installation failed on pve-1') + ->assertJsonPath('data.attention.suspended_servers.0.id', (string) $suspended->id) + ->assertJsonPath('data.attention.suspended_servers.0.detail', 'On pve-1') + // Backups are keyed by the server's short uuid: the backups tab is theirs. + ->assertJsonPath('data.attention.failed_backups.0.id', $suspended->uuid_short) + ->assertJsonPath('data.attention.failed_backups.0.label', 'nightly'); +}); + +it('separates a deletion failure from an install failure in the detail', function () { + $admin = User::factory()->create(['root_admin' => true]); + $node = Node::factory()->for(Location::factory())->create(['name' => 'pve-2']); + + Server::factory()->for($node)->for($admin)->create([ + 'status' => Status::DELETION_FAILED->value, + ]); + + $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + ->assertJsonPath('data.attention.failed_servers.0.detail', 'Deletion failed on pve-2'); +}); + +it('leaves the attention groups empty when nothing is wrong', function () { + $admin = User::factory()->create(['root_admin' => true]); + $node = Node::factory()->for(Location::factory())->create(); + Server::factory()->for($node)->for($admin)->create(['status' => null]); + + $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + ->assertJsonPath('data.attention.failed_servers', []) + ->assertJsonPath('data.attention.failed_backups', []) + ->assertJsonPath('data.attention.suspended_servers', []); +}); + +it('caps each attention group and leaves the count to say how many there really are', function () { + $admin = User::factory()->create(['root_admin' => true]); + $node = Node::factory()->for(Location::factory())->create(); + + Server::factory()->count(30)->for($node)->for($admin)->create([ + 'status' => Status::INSTALL_FAILED->value, + ]); + + $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + ->assertJsonCount(25, 'data.attention.failed_servers') + ->assertJsonPath('data.summary.failed_servers', 30); +}); From f7ab0edab61972f7151f1cd453d243ca16613ac2 Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Tue, 8 Sep 2026 02:41:14 +0000 Subject: [PATCH 02/12] chore(ddev): add a dev environment for 4.x 5.x carries a checked-in ddev project and this branch does not, so working on 4.x meant hand-rolling one. This mirrors 5.x's setup where the two branches agree and diverges only where 4.x genuinely differs. Same shape as 5.x: nginx-fpm, a pinned host_db_port so GUI clients keep their saved connections across restarts, the ddev-redis add-on (byte-identical to 5.x's), horizon and scheduler as web_extra_daemons in place of the compose `workers` service, and the Vite dev server exposed. Tailored to this branch: PHP 8.2 and MySQL 8.0 per composer.json and docker-compose.yml, Node 20 per the release workflow, Vite on 1234 per vite.config.js, and CACHE_DRIVER rather than 5.x's CACHE_STORE, since this branch runs Laravel 11 against the pre-11 config/cache.php. php8.2-gmp is not optional: composer.json requires ext-gmp, and without it `composer install` fails its platform check while `ddev composer` still exits 0, so vendor/ silently stays stale and the next artisan call dies in platform_check.php. No db_test database, unlike 5.x. tests/Pest.php uses DatabaseTransactions rather than RefreshDatabase, so the suite runs against the dev database and rolls its own writes back -- meaning seeded dev data is visible to it and will fail anything asserting on a fleet-wide aggregate. The config says so where someone will hit it. --- .ddev/addon-metadata/redis/manifest.yaml | 38 +++++++ .ddev/commands/host/redis-backend | 124 +++++++++++++++++++++++ .ddev/commands/redis/redis-cli | 13 +++ .ddev/commands/redis/redis-flush | 12 +++ .ddev/config.yaml | 69 +++++++++++++ .ddev/docker-compose.redis.yaml | 27 +++++ .ddev/redis/redis.conf | 13 +++ 7 files changed, 296 insertions(+) create mode 100644 .ddev/addon-metadata/redis/manifest.yaml create mode 100755 .ddev/commands/host/redis-backend create mode 100755 .ddev/commands/redis/redis-cli create mode 100755 .ddev/commands/redis/redis-flush create mode 100644 .ddev/config.yaml create mode 100644 .ddev/docker-compose.redis.yaml create mode 100644 .ddev/redis/redis.conf diff --git a/.ddev/addon-metadata/redis/manifest.yaml b/.ddev/addon-metadata/redis/manifest.yaml new file mode 100644 index 00000000000..c0fcfe56158 --- /dev/null +++ b/.ddev/addon-metadata/redis/manifest.yaml @@ -0,0 +1,38 @@ +name: redis +repository: ddev/ddev-redis +version: v2.2.0 +install_date: "2026-09-08T02:37:13Z" +project_files: + - docker-compose.redis.yaml + - redis/scripts/settings.ddev.redis.php + - redis/scripts/setup-drupal-settings.sh + - redis/scripts/setup-redis-optimized-config.sh + - redis/redis.conf + - redis/advanced.conf + - redis/append.conf + - redis/general.conf + - redis/io.conf + - redis/memory.conf + - redis/network.conf + - redis/security.conf + - redis/snapshots.conf + - commands/host/redis-backend + - commands/redis/redis-cli + - commands/redis/redis-flush +global_files: [] +removal_actions: + - | + #ddev-description:Remove redis settings if applicable + files=( + "${DDEV_APPROOT}/${DDEV_DOCROOT}/sites/default/settings.ddev.redis.php" + "${DDEV_APPROOT}/.ddev/docker-compose.redis_extra.yaml" + ) + for file in "${files[@]}"; do + if [ -f "$file" ]; then + if grep -q '#ddev-generated' "$file"; then + rm -f "$file" + else + echo "Unwilling to remove '$file' because it does not have #ddev-generated in it; you can manually delete it if it is safe to delete." + fi + fi + done diff --git a/.ddev/commands/host/redis-backend b/.ddev/commands/host/redis-backend new file mode 100755 index 00000000000..fbcaba553b8 --- /dev/null +++ b/.ddev/commands/host/redis-backend @@ -0,0 +1,124 @@ +#!/usr/bin/env bash +#ddev-generated + +## Description: Use a different key-value store for Redis +## Usage: redis-backend [optimize] +## Example: ddev redis-backend redis-alpine optimize + +REDIS_DOCKER_IMAGE=${1:-} +REDIS_CONFIG=${2:-} +NAME=$REDIS_DOCKER_IMAGE + +function show_help() { + cat < [optimize] + +Choose from predefined aliases, or provide any Redis-compatible Docker image. +Note that not every Docker image can work right away, and you may need to override +the "command:" in the docker-compose.redis_extra.yaml file + +Available aliases: + redis redis:7 + redis-alpine redis:7-alpine + valkey valkey/valkey:8 + valkey-alpine valkey/valkey:8-alpine + +Custom backend: + You can specify any Docker image, e.g.: + ddev redis-backend redis:6 + +Optional: + optimize Apply additional Redis configuration with resource limits + optimized Same as optimize + +Examples: + ddev redis-backend redis-alpine optimize + ddev redis-backend valkey + ddev redis-backend redis:7.2-alpine +EOF + exit 0 +} + +function optimize_config() { + [[ "$REDIS_CONFIG" != "optimized" && "$REDIS_CONFIG" != "optimize" ]] && return + ddev dotenv set .ddev/.env.redis --redis-optimized=true +} + +function change_hostname() { + [[ "${REDIS_HOSTNAME:-}" == "" ]] && return + ddev dotenv set .ddev/.env.redis --redis-hostname="$REDIS_HOSTNAME" +} + +function cleanup() { + rm -f "$DDEV_APPROOT/.ddev/.env.redis" + rm -rf "$DDEV_APPROOT/.ddev/redis/" + rm -f "$DDEV_APPROOT/.ddev/docker-compose.redis.yaml" "$DDEV_APPROOT/.ddev/docker-compose.redis_extra.yaml" + + redis_volume="ddev-$(ddev status -j | docker run -i --rm ddev/ddev-utilities jq -r '.raw.name')_redis" + if docker volume ls -q | grep -qw "$redis_volume"; then + ddev stop + docker volume rm "$redis_volume" + fi +} + +function check_docker_image() { + echo "Pulling ${REDIS_DOCKER_IMAGE}..." + if ! docker pull "$REDIS_DOCKER_IMAGE"; then + echo >&2 "❌ Unable to pull ${REDIS_DOCKER_IMAGE}" + exit 2 + fi +} + +function use_docker_image() { + [[ "$REDIS_DOCKER_IMAGE" != "redis:7" ]] && ddev dotenv set .ddev/.env.redis --redis-docker-image="$REDIS_DOCKER_IMAGE" + REPO=$(ddev add-on list --installed -j 2>/dev/null | docker run -i --rm ddev/ddev-utilities jq -r '.raw[] | select(.Name=="redis") | .Repository // empty' 2>/dev/null) + ddev add-on get "${REPO:-ddev/ddev-redis}" +} + +case "$REDIS_DOCKER_IMAGE" in + redis) + NAME="Redis 7" + REDIS_DOCKER_IMAGE="redis:7" + ;; + redis-alpine) + NAME="Redis 7 Alpine" + REDIS_DOCKER_IMAGE="redis:7-alpine" + ;; + valkey) + NAME="Valkey 8" + REDIS_DOCKER_IMAGE="valkey/valkey:8" + REDIS_HOSTNAME="valkey" + ;; + valkey-alpine) + NAME="Valkey 8 Alpine" + REDIS_DOCKER_IMAGE="valkey/valkey:8-alpine" + REDIS_HOSTNAME="valkey" + ;; + ""|--help|-h) + show_help + ;; + *) + NAME="$REDIS_DOCKER_IMAGE" + # Allow unknown image, nothing to override + ;; +esac + +check_docker_image +cleanup +optimize_config +change_hostname +use_docker_image + +echo +echo "✅ Redis backend: $REDIS_DOCKER_IMAGE" +if [[ "$REDIS_CONFIG" == "optimized" || "$REDIS_CONFIG" == "optimize" ]]; then + echo "⚙️ Redis config: optimized" +else + echo "⚙️ Redis config: default" +fi + +echo +echo "📝 Commit the '.ddev' directory to version control" + +echo +echo "🔄 Redis config available after 'ddev restart'" diff --git a/.ddev/commands/redis/redis-cli b/.ddev/commands/redis/redis-cli new file mode 100755 index 00000000000..2800343ed53 --- /dev/null +++ b/.ddev/commands/redis/redis-cli @@ -0,0 +1,13 @@ +#!/usr/bin/env sh + +#ddev-generated +## Description: Run redis-cli inside the Redis container +## Usage: redis-cli [flags] [args] +## Example: "ddev redis-cli KEYS *" or "ddev redis-cli INFO" or "ddev redis-cli --version" +## Aliases: redis + +if [ -f /etc/redis/conf/security.conf ]; then + redis-cli -p 6379 -h "${REDIS_HOSTNAME:-redis}" -a redis --no-auth-warning $@ +else + redis-cli -p 6379 -h "${REDIS_HOSTNAME:-redis}" $@ +fi diff --git a/.ddev/commands/redis/redis-flush b/.ddev/commands/redis/redis-flush new file mode 100755 index 00000000000..db90558a7f2 --- /dev/null +++ b/.ddev/commands/redis/redis-flush @@ -0,0 +1,12 @@ +#!/usr/bin/env sh + +#ddev-generated +## Description: Flush all cache inside the Redis container +## Usage: redis-flush +## Example: "ddev redis-flush" + +if [ -f /etc/redis/conf/security.conf ]; then + redis-cli -p 6379 -h "${REDIS_HOSTNAME:-redis}" -a redis --no-auth-warning FLUSHALL ASYNC +else + redis-cli -p 6379 -h "${REDIS_HOSTNAME:-redis}" FLUSHALL ASYNC +fi diff --git a/.ddev/config.yaml b/.ddev/config.yaml new file mode 100644 index 00000000000..3b60bd2022e --- /dev/null +++ b/.ddev/config.yaml @@ -0,0 +1,69 @@ +name: convoy4x +type: laravel +docroot: public +php_version: "8.2" +webserver_type: nginx-fpm + +# 4.x ships MySQL (docker-compose.yml pins mysql:8.0), unlike 5.x's Postgres. +database: + type: mysql + version: "8.0" + +# Without this DDEV assigns a random ephemeral host port on every start, which +# breaks saved connections in GUI clients (TablePlus, DataGrip, mysql aliases). +host_db_port: "3306" + +# The release workflow builds on Node 20. +nodejs_version: "20" +corepack_enable: false + +# ext-gmp is required by composer.json. Without it `composer install` fails its +# platform check -- and `ddev composer` still exits 0, so vendor/ silently stays +# stale and every artisan call dies on the next platform_check.php. +webimage_extra_packages: + - php8.2-gmp + +# MySQL + redis + mail overrides. These are real container env vars, so they +# take precedence over .env (Laravel's Dotenv does not overwrite existing env). +# Note CACHE_DRIVER, not 5.x's CACHE_STORE: this branch is on Laravel 11 but +# still ships the pre-11 config/cache.php, which reads the old key. +web_environment: + - APP_URL=https://convoy4x.ddev.site + - DB_CONNECTION=mysql + - DB_HOST=db + - DB_PORT=3306 + - DB_DATABASE=db + - DB_USERNAME=db + - DB_PASSWORD=db + - REDIS_HOST=redis + - REDIS_PORT=6379 + - REDIS_PASSWORD= + - CACHE_DRIVER=redis + - QUEUE_CONNECTION=redis + - SESSION_DRIVER=redis + - MAIL_MAILER=smtp + - MAIL_HOST=localhost + - MAIL_PORT=1025 + +# Deliberately no `db_test` database, unlike 5.x. This branch's tests/Pest.php +# uses DatabaseTransactions rather than RefreshDatabase, so they run against +# whatever is in the dev database and roll their own writes back. That means +# seeded dev data is visible to the suite and will fail any test asserting on a +# fleet-wide aggregate (the overview endpoint's counts, most of all). Run +# `ddev artisan migrate:fresh` before a full run, or scope to one file. + +# Replaces the compose `workers` service and its scheduler. +web_extra_daemons: + - name: horizon + command: "php artisan horizon" + directory: /var/www/html + - name: scheduler + command: "php artisan schedule:work" + directory: /var/www/html + +# Expose the Vite dev server, which vite.config.js pins to 1234. +web_extra_exposed_ports: + - name: vite + container_port: 1234 + http_port: 1235 + https_port: 1234 diff --git a/.ddev/docker-compose.redis.yaml b/.ddev/docker-compose.redis.yaml new file mode 100644 index 00000000000..e9da4c6e48a --- /dev/null +++ b/.ddev/docker-compose.redis.yaml @@ -0,0 +1,27 @@ +#ddev-generated +services: + redis: + container_name: ddev-${DDEV_SITENAME}-redis + image: ${REDIS_DOCKER_IMAGE:-redis:7} + hostname: ${REDIS_HOSTNAME:-redis} + # These labels ensure this service is discoverable by ddev. + labels: + com.ddev.site-name: ${DDEV_SITENAME} + com.ddev.approot: ${DDEV_APPROOT} + restart: "no" + expose: + - 6379 + volumes: + - ".:/mnt/ddev_config" + - "ddev-global-cache:/mnt/ddev-global-cache" + - "./redis:/etc/redis/conf" + - "redis:/data" + command: /etc/redis/conf/redis.conf + x-ddev: + describe-url-port: | + Backend: ${REDIS_DOCKER_IMAGE:-redis:7} + describe-info: | + Pass: + +volumes: + redis: diff --git a/.ddev/redis/redis.conf b/.ddev/redis/redis.conf new file mode 100644 index 00000000000..937c4e5d34e --- /dev/null +++ b/.ddev/redis/redis.conf @@ -0,0 +1,13 @@ +# Redis configuration. +# #ddev-generated +# Example configuration files for reference: +# http://download.redis.io/redis-stable/redis.conf +# http://download.redis.io/redis-stable/sentinel.conf + +maxmemory 2048mb +maxmemory-policy allkeys-lfu + +# to disable Redis persistence, remove ddev-generated from this file, +# and uncomment the two lines below: +#appendonly no +#save "" From 878e6c8a39bf29fdd59c12d28c8bcd5702d49228 Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Tue, 8 Sep 2026 02:44:17 +0000 Subject: [PATCH 03/12] chore(sbx): add the dev sandbox kit to 4.x Same three files 5.x carries, so working on this branch in a Docker Sandbox does not mean hand-rolling the provisioning each time. The sandbox plumbing is identical to 5.x's and deliberately left alone: the ddev install, the NO_PROXY entry that keeps *.ddev.site on this sandbox instead of resolving on the host and driving the real app, the pinned Playwright under /opt/sbx-e2e rather than in the repo, the writable /etc/hosts overmount ddev needs to register its names, and the certs overmount that stops a sandbox-signed cert landing in the host checkout. The kit keeps 5.x's name, `convoy-dev`. Kits are referenced by path (`--kit .sbx/dev`), so two branches naming theirs the same collide over nothing. What does have to differ is the ddev project: `convoy4x`, since a 4.x and a 5.x sandbox would otherwise fight over one project name. The suggested template name is scoped for the same reason -- the saved image bakes in this branch's PHP and database versions. Setup is composer + npm + key:generate + migrate + ServerSeeder, which needs no Proxmox credentials, rather than 5.x's DevNodeSeeder which does. browser.mjs carries two fixes on top of the port, both found by running it against this app: - `login()` waited for the URL to leave /auth/login with the default `waitUntil: 'load'`. This app leaves that route by a client-side react-router redirect that fires no second load event, so the helper hung for the full 30s timeout on a login that had already succeeded. Waiting on 'commit' returns in under a second, and the URL is the only thing the predicate asks about anyway. - `capture()` did `return page.evaluate(...)` inside a try whose finally closes the page, so the close raced the pending evaluate and the call died with "Target page, context or browser has been closed". Awaiting the evaluate before returning fixes it. Both bugs exist in 5.x's copy too. Whether to fix them there is a separate change against that branch. The agentContext carries the three things that cost real time here: assets are prebuilt so a frontend edit is invisible until `npx vite build` (and a stale public/hot renders the page blank), ext-gmp is required while `ddev composer install` still exits 0 without it, and the suite runs on DatabaseTransactions against the dev database so seeded data breaks any aggregate assertion. 5.x's kit points at docs/docker-sandbox.md for the proxy-isolation background. That file does not exist on this branch and porting the whole document is not this change, so the kit states the check inline instead of linking somewhere that isn't there. The files are force-added: .sbx is in the operator's global gitignore, which is why 5.x's copies are tracked the same way. --- .sbx/README.md | 65 ++++++++++++++ .sbx/dev/browser.mjs | 117 ++++++++++++++++++++++++++ .sbx/dev/spec.yaml | 196 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 378 insertions(+) create mode 100644 .sbx/README.md create mode 100644 .sbx/dev/browser.mjs create mode 100644 .sbx/dev/spec.yaml diff --git a/.sbx/README.md b/.sbx/README.md new file mode 100644 index 00000000000..767a9d1323f --- /dev/null +++ b/.sbx/README.md @@ -0,0 +1,65 @@ +# sbx kits + +Provisioning for running a coding agent in a [Docker Sandbox](https://docs.docker.com/ai/sandboxes/) +(`sbx`) against this repo. sbx has no auto-detection for repo-local kits, so a kit +is just a committed directory you reference explicitly with `--kit`. + +## `dev/` — Convoy 4.x dev environment + +Installs ddev and starts the stack inside the sandbox (its Docker daemon, DB, and +volumes are isolated from your host ddev). It also sets up headless browsing: +Playwright is pinned and installed to `/opt/sbx-e2e` (never the repo), and +`dev/browser.mjs` is published to `/opt/sbx-e2e/browser.mjs` for scripts to import. +Because `/etc/hosts` is a read-only mount in a sandbox — which otherwise makes +`ddev start` fail outright — a startup step overmounts a writable copy so ddev can +register `*.ddev.site`. + +```sh +sbx run --kit .sbx/dev claude +``` + +The first run installs ddev and pulls its images (slow, once). To make later +starts instant, snapshot the provisioned sandbox into a template: + +```sh +sbx template save convoy4x-dev +sbx run -t convoy4x-dev --kit .sbx/dev claude # install is now a no-op; only `ddev start` runs +``` + +Then finish app provisioning inside the sandbox (see the kit's `agentContext`, or +`.sbx/dev/spec.yaml`): + +```sh +ddev composer install +npm install +ddev artisan key:generate +ddev artisan migrate +ddev artisan db:seed --class=ServerSeeder +``` + +## Differences from the 5.x kit + +The two branches run different stacks, so the kits are not interchangeable — the +project provisioning differs even though the sandbox plumbing is identical. + +- MySQL 8.0 and PHP 8.2 here, Postgres 17 and PHP 8.4 on 5.x. +- The ddev project is `convoy4x`, so the app is at `https://convoy4x.ddev.site`. + That one has to differ: a 4.x and a 5.x sandbox otherwise fight over the same + ddev project name. The kit itself is still `convoy-dev` on both branches — + kits are referenced by path (`--kit .sbx/dev`), so there is nothing to collide. + The suggested *template* name is scoped, though, since the saved image bakes in + this branch's PHP and database versions and would otherwise clobber 5.x's. +- No separate test database. `tests/Pest.php` uses `DatabaseTransactions` rather + than `RefreshDatabase`, so the suite runs against the dev database and rolls its + own writes back — seeded dev data is visible to it and will fail any test that + asserts on a fleet-wide aggregate. `ddev artisan migrate:fresh` before a full run. +- Seeding is `ServerSeeder` (a location, a node, ten servers), and it needs no + Proxmox credentials. + +## Boundaries + +- **Secrets** live in the gitignored `.env`, mounted into the sandbox — never in a + kit. +- **Notifications** and any personal network setup come from global kits in the + operator's own dotfiles, injected automatically by their `sbx` wrapper; `.sbx/dev` + is project provisioning only. Multiple `--kit` refs compose, so they layer cleanly. diff --git a/.sbx/dev/browser.mjs b/.sbx/dev/browser.mjs new file mode 100644 index 00000000000..77bc4130a61 --- /dev/null +++ b/.sbx/dev/browser.mjs @@ -0,0 +1,117 @@ +/* + * Playwright helpers for driving the sandbox's own ddev app. + * + * The dev kit copies this to /opt/sbx-e2e/browser.mjs on every start, next to a + * pinned `playwright` install. Import it by absolute path from a throwaway + * script anywhere (e.g. your scratchpad) — resolving `playwright` relative to + * /opt/sbx-e2e means the repo never needs a devDependency for a local probe: + * + * import { BASE, launch, newContext, login, capture } from '/opt/sbx-e2e/browser.mjs' + * + * const browser = await launch() + * const ctx = await newContext(browser) + * const page = await login(ctx, { email: '…', password: '…' }) + * await capture(ctx, { url: '/admin', width: 768, path: '/tmp/overview.png' }) + * await browser.close() + */ +import { chromium } from 'playwright' +import { execFileSync } from 'node:child_process' + +export const BASE = process.env.SBX_APP_URL ?? 'https://convoy4x.ddev.site' + +const PROXY = process.env.HTTPS_PROXY ?? 'http://gateway.docker.internal:3128' + +/* + * Chromium hands hostnames to the proxy rather than resolving them, and the + * sandbox proxy resolves them on the HOST — so an unbypassed request to + * convoy4x.ddev.site drives your real host app (leaked sessions, mutated data). + * Bypassing the proxy for the app keeps it on this sandbox's loopback; the + * preflight below is the backstop that refuses to run if it ever slips. + */ +const BYPASS = '.ddev.site,localhost,127.0.0.1' + +export function assertSandboxApp(base = BASE) { + const ip = execFileSync('curl', ['-sk', `${base}/up`, '-o', '/dev/null', '-w', '%{remote_ip}']) + .toString() + .trim() + + if (ip !== '127.0.0.1') { + throw new Error( + `refusing to drive ${base}: it answered from ${ip || '(unreachable)'}, not this ` + + `sandbox's ddev (127.0.0.1). Check 'ddev describe', that .ddev.site is in ` + + `NO_PROXY, and that /etc/hosts is the writable overmount the dev kit sets up.` + ) + } +} + +export async function launch(options = {}) { + assertSandboxApp() + + return chromium.launch({ proxy: { server: PROXY, bypass: BYPASS }, ...options }) +} + +// The ddev cert is mkcert-signed and that CA isn't in the sandbox trust store. +export async function newContext(browser, options = {}) { + return browser.newContext({ + ignoreHTTPSErrors: true, + viewport: { width: 1440, height: 1000 }, + deviceScaleFactor: 2, + ...options, + }) +} + +export async function login(context, { email, password } = {}) { + const page = await context.newPage() + + await page.goto(`${BASE}/auth/login`, { waitUntil: 'domcontentloaded' }) + await page.getByLabel(/email/i).fill(email ?? process.env.SBX_APP_EMAIL) + await page.getByLabel(/password/i).fill(password ?? process.env.SBX_APP_PASSWORD) + await page.getByRole('button', { name: /sign in|log in|login/i }).click() + // `waitUntil: 'commit'` matters here. The default is 'load', and this app + // leaves /auth/login by a client-side react-router redirect that fires no + // second load event -- so the default hangs for the full timeout on a login + // that actually succeeded. 'commit' resolves as soon as the URL changes, + // which is the only thing this predicate is asking about. + await page.waitForURL(u => !u.pathname.includes('/auth/login'), { + timeout: 30_000, + waitUntil: 'commit', + }) + + return page +} + +/* + * Screenshot one route at one viewport. Returns the horizontal overflow in px, + * which is the layout failure mode worth failing a visual check on. + */ +export async function capture(context, { url, path, width = 1440, height = 1000, settle = 1000 }) { + const page = await context.newPage() + + try { + await page.setViewportSize({ width, height }) + await page.goto(BASE + url, { waitUntil: 'networkidle' }) + await page.waitForTimeout(settle) + await page.screenshot({ path, fullPage: true }) + + // Awaited, not returned directly: `return ` inside a try lets + // the finally close the page while evaluate is still in flight, and the + // call dies with "Target page, context or browser has been closed". + const overflow = await page.evaluate( + () => document.documentElement.scrollWidth - document.documentElement.clientWidth + ) + + return overflow + } finally { + await page.close() + } +} + +// Attach before navigating; the returned array fills as the page misbehaves. +export function collectErrors(page) { + const errors = [] + + page.on('pageerror', e => errors.push(`pageerror: ${e.message}`)) + page.on('console', m => m.type() === 'error' && errors.push(`console: ${m.text()}`)) + + return errors +} diff --git a/.sbx/dev/spec.yaml b/.sbx/dev/spec.yaml new file mode 100644 index 00000000000..fe1b127b6ae --- /dev/null +++ b/.sbx/dev/spec.yaml @@ -0,0 +1,196 @@ +schemaVersion: "1" +kind: mixin +name: convoy-dev +displayName: Convoy 4.x dev sandbox +description: Provision ddev inside a Docker Sandbox for working on Convoy 4.x. + +# Appended to the agent's memory at sandbox creation — tells the agent how to +# finish provisioning and run common tasks. (ddev itself is installed/started by +# the commands below; these are the project-state steps that shouldn't run +# automatically.) +agentContext: | + # Convoy 4.x dev sandbox + + ddev is installed and started for you (Laravel + MySQL + redis). Its **database** + is this sandbox's own, and the install step below proxy-isolates `*.ddev.site` + (adds it to NO_PROXY) so `curl`/Playwright hitting https://convoy4x.ddev.site + stay on THIS sandbox's ddev instead of being resolved by the proxy on the host + and driving your real host app. Sanity check after a rebuild: + `curl -sk https://convoy4x.ddev.site/ -o /dev/null -w '%{remote_ip}\n'` must + print `127.0.0.1` (not a proxy address). Back it up host-side with + `sbx policy deny network '*.ddev.site'`. + + Playwright + Chromium are pre-installed for visual and e2e checks — but NOT in + the repo. They live in `/opt/sbx-e2e` (pinned version, browsers in + ~/.cache/ms-playwright), so **do not `npm install playwright` in the project**; + that would put sandbox-only tooling in package.json. Write throwaway scripts + wherever you like and import the helpers by absolute path: + + import { BASE, launch, newContext, login, capture } from '/opt/sbx-e2e/browser.mjs' + + const browser = await launch() // proxy-bypassed + preflighted + const ctx = await newContext(browser) // ignores the mkcert cert + const page = await login(ctx, { email: '…', password: '…' }) + await capture(ctx, { url: '/admin', width: 768, path: '/tmp/overview.png' }) + await browser.close() + + `launch()` refuses to run unless the app answers from 127.0.0.1, so a + misconfigured sandbox fails loudly instead of driving your host app. Source: + `.sbx/dev/browser.mjs` (copied into place on every start — edit it there). + + If ddev is unreachable, fix ddev; never tunnel to the host's instance. + + Finish provisioning once: + + ddev composer install + npm install + ddev artisan key:generate + ddev artisan migrate + ddev artisan db:seed --class=ServerSeeder # a location, a node, 10 servers + + Everyday: + + ddev exec ./vendor/bin/pest # test suite + ddev artisan # artisan + ddev describe # status / URLs + + The app serves **prebuilt** assets from `public/build`, and no Vite dev server + runs by default — so a frontend edit is invisible in the browser until you + `npx vite build`. A stale `public/hot` from a killed `npm run dev` makes the + page render blank; `rm -f public/hot` before a built-asset check. + + Two traps specific to this branch: + + - `composer.json` requires **ext-gmp**. `ddev composer install` exits 0 even + when the platform check fails, so a missing extension leaves vendor/ stale + and the next artisan call dies in `platform_check.php`. The committed + `.ddev/config.yaml` installs php8.2-gmp; don't drop it. + - `tests/Pest.php` uses **DatabaseTransactions**, not RefreshDatabase, so the + suite runs against the dev database and merely rolls its own writes back. + Seeded dev data is therefore visible to it and will fail anything asserting + on a fleet-wide aggregate (the overview endpoint's counts, most of all). Run + `ddev artisan migrate:fresh` before a full run, or scope to one file. + + Note: vendor/ and node_modules/ are shared with your host repo via the mounted + workspace, so composer/npm installs here also land in your host checkout. + +commands: + # Runs ONCE at creation (as root unless a user is set). Slow the first time + # (installs ddev + Chromium). Bake it into a template so it isn't repeated: + # sbx template save convoy4x-dev + install: + # ddev's installer refuses to run as root and uses sudo itself, so run it as + # the agent user (uid 1000, which is in the sudo group). + - command: "command -v ddev >/dev/null 2>&1 || curl -fsSL https://ddev.com/install.sh | bash" + user: "1000" + description: install ddev + + # Keep *.ddev.site resolving to THIS sandbox's ddev (127.0.0.1) instead of the + # host's. The sandbox proxy otherwise resolves the hostname on the host side, so + # curl/Playwright silently drive your real host app (leaking e2e sessions, + # mutating host data). Marker-guarded because CLAUDE_ENV_FILE is sourced before + # every command — a plain append would grow NO_PROXY without bound. + - command: | + cat >> "${CLAUDE_ENV_FILE:-/etc/sandbox-persistent.sh}" <<'SBXEOF' + if [ -z "${SBX_DDEV_NOPROXY_DONE:-}" ]; then + export NO_PROXY="${NO_PROXY:+$NO_PROXY,}.ddev.site,ddev.site" + export no_proxy="$NO_PROXY" + export SBX_DDEV_NOPROXY_DONE=1 + fi + SBXEOF + description: proxy-isolate *.ddev.site from the host dev server + + # Playwright lives OUTSIDE the repo so a visual check never adds a devDependency + # to package.json. Scripts import /opt/sbx-e2e/browser.mjs by absolute path, and + # Node resolves `playwright` by walking up from there to /opt/sbx-e2e/node_modules. + - command: "install -d -o 1000 -g 1000 /opt/sbx-e2e" + description: create the sandbox-local e2e prefix + + # Pinned, not @latest: the browser build id is tied to the playwright version, so + # a floating version installed later in a session no longer matches the browsers + # baked in here and dies with "Executable doesn't exist at .../chromium-". + # A real package.json (rather than --no-save) is what keeps it installed — with + # nothing declared, the next npm command in this prefix prunes node_modules. + - command: | + set -eu + cat > /opt/sbx-e2e/package.json <<'PKGEOF' + { + "name": "sbx-e2e", + "private": true, + "type": "module", + "dependencies": { "playwright": "1.62.0" } + } + PKGEOF + npm --prefix /opt/sbx-e2e install --loglevel=error + user: "1000" + description: install Playwright (pinned) + + # Chromium's shared libraries via apt (needs root — the default here). + - command: "/opt/sbx-e2e/node_modules/.bin/playwright install-deps chromium || echo 'convoy-dev: playwright install-deps failed; agent can rerun on demand'" + description: install Chromium OS dependencies + + # The browser itself, as the agent user so it lands in the home cache the agent + # actually uses. Non-fatal so a download hiccup can't block creation. + - command: "/opt/sbx-e2e/node_modules/.bin/playwright install chromium || echo 'convoy-dev: playwright chromium preinstall skipped'" + user: "1000" + description: pre-install Chromium for visual/e2e checks + + # Runs on EVERY start (as the agent user). Fast once ddev's images are baked + # into a template. + startup: + # /etc/hosts is a read-only bind mount from the host, so ddev's hostname step + # ("Failed to add hosts entry … read-only file system") aborts `ddev start` — + # and *.ddev.site has no public DNS answer in here to fall back on. Overmount a + # writable copy so ddev can register its own names; the marker line makes it + # idempotent across restarts. Sandbox-local by construction: nothing is written + # to the workspace, and the overmount dies with the sandbox. + - command: + - "bash" + - "-lc" + - | + set -euo pipefail + mark='# sbx: writable hosts overmount' + if grep -qxF "$mark" /etc/hosts 2>/dev/null; then + echo 'convoy-dev: /etc/hosts already writable' + exit 0 + fi + tmp=$(mktemp) + { cat /etc/hosts; echo "$mark"; } > "$tmp" + sudo install -m 0644 -o root -g root "$tmp" /var/lib/sbx-hosts + rm -f "$tmp" + sudo mount --bind /var/lib/sbx-hosts /etc/hosts + echo 'convoy-dev: overmounted /etc/hosts (writable copy at /var/lib/sbx-hosts)' + description: make /etc/hosts writable so ddev can register *.ddev.site + + # Copied rather than symlinked: Node resolves a symlinked module to its realpath, + # which would send the `playwright` lookup into the repo instead of /opt/sbx-e2e. + - command: ["bash", "-lc", "install -m 0644 \"${WORKSPACE_DIR:-.}/.sbx/dev/browser.mjs\" /opt/sbx-e2e/browser.mjs"] + description: publish the Playwright helpers to /opt/sbx-e2e + + # ddev signs the project cert with the mkcert CA of whichever machine runs + # `ddev start`, and drops it in .ddev/traefik/certs — which is part of the + # mounted workspace. Left alone, this sandbox's throwaway CA overwrites the + # host's cert in the checkout, and the next host `ddev start` copies it into + # the host router: every *.ddev.site project on the Mac then fails TLS in + # the browser, until a host-side `ddev restart` regenerates it. Overmount a + # sandbox-local dir (the same trick as /etc/hosts above) so ddev in here + # signs its own certs and the host checkout is never written to. + - command: + - "bash" + - "-lc" + - | + set -euo pipefail + certs="${WORKSPACE_DIR:-.}/.ddev/traefik/certs" + mkdir -p "$certs" + certs=$(cd "$certs" && pwd -P) + if awk -v d="$certs" '$2 == d { found = 1 } END { exit !found }' /proc/self/mounts; then + echo 'convoy-dev: .ddev/traefik/certs already overmounted' + exit 0 + fi + sudo install -d -m 0755 -o 1000 -g 1000 /var/lib/sbx-ddev-certs + sudo mount --bind /var/lib/sbx-ddev-certs "$certs" + echo 'convoy-dev: overmounted .ddev/traefik/certs (sandbox-local)' + description: keep sandbox-signed ddev certs out of the host checkout + + - command: ["bash", "-lc", "cd \"${WORKSPACE_DIR:-.}\" && ddev start -y"] + description: boot the ddev stack From 57af00f8376d9575d18b42a6e660074fcabc27e4 Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:19:32 -0400 Subject: [PATCH 04/12] fix(admin): make every attention row land somewhere useful Three follow-ups to the attention card, all of them about a row that promised something it did not deliver. The server rows linked at the primary key. Nothing in the panel routes on it: RouteServiceProvider binds {server} as uuid_short for an 8-character value and uuid for anything else, and ServerBuildTransformer deliberately ships the short uuid as `id` with the primary key as `internal_id`. Every server link in the admin UI is built that way, so the card's were the odd ones out and every click died on "No query results for model [Convoy\Models\Server]". All three groups now carry the same key and differ only in destination. A test walks the link rather than restating the value -- it takes the id off the endpoint and fetches the server page behind it. Suspended servers are gone from the card. A suspension is an administrative decision someone made on purpose; it is the panel working as asked, not a thing to be fixed, and listing it beside real failures turns the card into a status feed. The count stays in Server State, which is where a deliberate state belongs. That leaves one tone, so the tone lookup goes with it. The overflow link is gone. It pointed at the unfiltered server list -- the exact dead end this card was written to remove, reintroduced for the leftovers -- and there is nowhere honest to send them: the servers index allows no status filter, and no page in 4.x lists backups. The sheet now says what it is not showing instead of pretending to. --- app/Services/Admin/OverviewService.php | 22 ++-- .../Admin/OverviewTransformer.php | 1 - lang/en_US/admin/overview.php | 8 +- .../scripts/api/admin/overview/getOverview.ts | 2 - .../admin/overview/AttentionCard.tsx | 117 ++++++++---------- .../Admin/OverviewControllerTest.php | 57 +++++++-- 6 files changed, 112 insertions(+), 95 deletions(-) diff --git a/app/Services/Admin/OverviewService.php b/app/Services/Admin/OverviewService.php index d3b2ea61a55..d40f0d2e999 100644 --- a/app/Services/Admin/OverviewService.php +++ b/app/Services/Admin/OverviewService.php @@ -129,7 +129,10 @@ private function failedServers(Collection $statuses): int /** * The records behind the attention card, each carrying the key its destination - * route takes. + * route takes -- a server's `uuid_short` throughout. Both the admin and client + * server routes are bound by it (RouteServiceProvider resolves an 8-character + * value as `uuid_short` and anything else as `uuid`, so the primary key never + * matches), and every other server link in the panel is built the same way. * * The card used to be a stat tile showing only the failed-server count while its * caption mixed in failed backups and servers mid-delete, so it could read "0" @@ -137,8 +140,11 @@ private function failedServers(Collection $statuses): int * way. Listing the records themselves is what lets a click land on the thing * that is wrong. * - * Deleting servers are deliberately absent: mid-delete is a transient state - * rather than a failure, and the server-state card already counts it. + * Two states are deliberately absent. Mid-delete is a transient state rather + * than a failure, and the server-state card already counts it. A suspension is + * an administrative decision someone made on purpose -- it is the panel working + * as asked, not something to be fixed, and listing it next to real failures + * dilutes the card into a status feed. */ private function attention(): array { @@ -157,10 +163,6 @@ private function attention(): array ), ), 'failed_backups' => $this->failedBackupSubjects(), - 'suspended_servers' => $this->serverSubjects( - Server::query()->where('status', Status::SUSPENDED->value), - fn (Server $server) => 'On '.($server->node?->name ?? 'an unknown node'), - ), ]; } @@ -173,9 +175,9 @@ private function serverSubjects(Builder $query, callable $detail): array ->with('node:id,name') ->orderByDesc('id') ->limit(self::ATTENTION_LIMIT) - ->get(['id', 'name', 'node_id', 'status']) + ->get(['id', 'uuid_short', 'name', 'node_id', 'status']) ->map(fn (Server $server) => [ - 'id' => (string) $server->id, + 'id' => $server->uuid_short, 'label' => $server->name, 'detail' => $detail($server), ]) @@ -187,6 +189,8 @@ private function serverSubjects(Builder $query, callable $detail): array * backups tab is the only page that shows a backup, and ServerPolicy::before lets * an admin open it for any server. A backup carries no failure message on this * branch, so the detail names the server and when it gave up instead. + * + * The key is the same shape the server groups use; only the destination differs. */ private function failedBackupSubjects(): array { diff --git a/app/Transformers/Admin/OverviewTransformer.php b/app/Transformers/Admin/OverviewTransformer.php index 7826bd6381a..7cccb96a633 100644 --- a/app/Transformers/Admin/OverviewTransformer.php +++ b/app/Transformers/Admin/OverviewTransformer.php @@ -52,7 +52,6 @@ public function transform(array $overview): array 'attention' => [ 'failed_servers' => $this->subjects($overview['attention']['failed_servers']), 'failed_backups' => $this->subjects($overview['attention']['failed_backups']), - 'suspended_servers' => $this->subjects($overview['attention']['suspended_servers']), ], 'nodes' => collect($overview['nodes']) ->map(fn (array $node) => [ diff --git a/lang/en_US/admin/overview.php b/lang/en_US/admin/overview.php index 05098f7baa0..a7af3a1d0e3 100644 --- a/lang/en_US/admin/overview.php +++ b/lang/en_US/admin/overview.php @@ -9,17 +9,15 @@ 'attention' => 'Attention', 'attention_description' => 'Everything currently in a state you have to do something about.', 'attention_all_clear' => 'All clear', - 'attention_all_clear_detail' => 'No failed servers, no failed backups, nothing suspended.', + 'attention_all_clear_detail' => 'No failed servers, no failed backups.', 'attention_fix' => 'Fix', 'attention_view' => 'View', 'attention_failed_servers_one' => ':count server failed', 'attention_failed_servers_other' => ':count servers failed', 'attention_failed_backups_one' => ':count backup failed', 'attention_failed_backups_other' => ':count backups failed', - 'attention_suspended_servers_one' => ':count server suspended', - 'attention_suspended_servers_other' => ':count servers suspended', - 'attention_more_one' => ':count more', - 'attention_more_other' => ':count more', + 'attention_close' => 'Close', + 'attention_showing' => 'Showing the :shown most recent of :total.', 'capacity' => 'Capacity', 'capacity_description' => 'Convoy allocated resources across all nodes.', 'allocated_percent' => ':percent% allocated in Convoy', diff --git a/resources/scripts/api/admin/overview/getOverview.ts b/resources/scripts/api/admin/overview/getOverview.ts index 40bffd1300f..365c34f3072 100644 --- a/resources/scripts/api/admin/overview/getOverview.ts +++ b/resources/scripts/api/admin/overview/getOverview.ts @@ -60,7 +60,6 @@ export interface AttentionSubject { export interface DashboardAttention { failedServers: AttentionSubject[] failedBackups: AttentionSubject[] - suspendedServers: AttentionSubject[] } export interface DashboardNode { @@ -121,7 +120,6 @@ export const rawDataToOverview = (data: any): DashboardOverview => ({ attention: { failedServers: rawSubjects(data.attention?.failed_servers), failedBackups: rawSubjects(data.attention?.failed_backups), - suspendedServers: rawSubjects(data.attention?.suspended_servers), }, nodes: data.nodes.map((node: any) => ({ id: node.id, diff --git a/resources/scripts/components/admin/overview/AttentionCard.tsx b/resources/scripts/components/admin/overview/AttentionCard.tsx index a03ba5743eb..5a0a2b19090 100644 --- a/resources/scripts/components/admin/overview/AttentionCard.tsx +++ b/resources/scripts/components/admin/overview/AttentionCard.tsx @@ -1,11 +1,8 @@ import { ArrowRightIcon, CheckCircleIcon, - ChevronDownIcon, ExclamationTriangleIcon, - PauseCircleIcon, } from '@heroicons/react/24/outline' -import { Collapse } from '@mantine/core' import { ComponentType, useState } from 'react' import { useTranslation } from 'react-i18next' import { Link } from 'react-router-dom' @@ -16,17 +13,15 @@ import { } from '@/api/admin/overview/getOverview' import Card from '@/components/elements/Card' +import Modal from '@/components/elements/Modal' interface IconProps { className?: string } -type Tone = 'error' | 'warning' - interface AttentionGroup { key: string icon: ComponentType - tone: Tone /** Summary line used when the group holds more than one record. */ title: string actionLabel: string @@ -36,25 +31,23 @@ interface AttentionGroup { to: (subject: AttentionSubject) => string /** Real total, which can exceed what `subjects` carries. */ total: number - /** Where the leftovers live when the list is capped. */ - overflowTo: string } -const toneClasses: Record = { - error: 'text-error border-error-light bg-error-lighter', - warning: 'text-warning-dark border-warning bg-warning-lighter', -} +const iconClasses = 'text-error border-error-light bg-error-lighter' const SubjectRow = ({ subject, to, + onNavigate, }: { subject: AttentionSubject to: string + onNavigate: () => void }) => (

{subject.label} @@ -80,8 +73,14 @@ const preview = (group: AttentionGroup): string => { /** * One group. A single record is named outright and links straight at itself -- * "1 server failed to install" plus a hunt through the server list is strictly - * less information than naming the server. Several become a disclosure over - * direct links, because the count alone is what made the old card useless. + * less information than naming the server. Several open a sheet over direct + * links, because the count alone is what made the old card useless. + * + * The sheet is what keeps the dashboard still. Expanding 25 records inline grew + * the card by several hundred pixels and shoved every card below it down the + * page, so reading one row rearranged the rest -- and the group most worth + * opening moved the page most. Modal.Body caps itself at 60vh and scrolls, so + * the list is bounded however broken the fleet is. */ const GroupRow = ({ group }: { group: AttentionGroup }) => { const [open, setOpen] = useState(false) @@ -90,7 +89,7 @@ const GroupRow = ({ group }: { group: AttentionGroup }) => { const truncated = group.total > group.subjects.length const icon = ( -

+
) @@ -121,12 +120,12 @@ const GroupRow = ({ group }: { group: AttentionGroup }) => { } return ( -
+ <> - -
- {group.subjects.map(subject => ( - - ))} + setOpen(false)}> + + {group.title} + + +
+ {group.subjects.map(subject => ( + setOpen(false)} + /> + ))} +
{truncated && ( - - {t('attention_more', { - count: group.total - group.subjects.length, +

+ {t('attention_showing', { + shown: group.subjects.length, + total: group.total, })} - +

)} -
- -
+ + + setOpen(false)}> + {t('attention_close')} + + + + ) } const AttentionCard = ({ data }: { data: DashboardOverview }) => { const { t } = useTranslation('admin.overview') - const { attention, summary, backups, servers } = data + const { attention, summary, backups } = data const groups: AttentionGroup[] = [] @@ -181,7 +185,6 @@ const AttentionCard = ({ data }: { data: DashboardOverview }) => { groups.push({ key: 'failed-servers', icon: ExclamationTriangleIcon, - tone: 'error', title: t('attention_failed_servers', { count: summary.failedServers, }), @@ -189,7 +192,6 @@ const AttentionCard = ({ data }: { data: DashboardOverview }) => { subjects: attention.failedServers, to: subject => `/admin/servers/${subject.id}`, total: summary.failedServers, - overflowTo: '/admin/servers', }) } @@ -197,31 +199,14 @@ const AttentionCard = ({ data }: { data: DashboardOverview }) => { groups.push({ key: 'failed-backups', icon: ExclamationTriangleIcon, - tone: 'error', title: t('attention_failed_backups', { count: backups.failed }), actionLabel: t('attention_view'), // The backups tab is the only page that shows a backup, and it is - // keyed by the server's short uuid rather than its id. + // keyed by the server's short uuid -- the same key the admin server + // routes bind on. to: subject => `/servers/${subject.id}/backups`, subjects: attention.failedBackups, total: backups.failed, - overflowTo: '/admin/servers', - }) - } - - if (attention.suspendedServers.length > 0) { - groups.push({ - key: 'suspended-servers', - icon: PauseCircleIcon, - tone: 'warning', - title: t('attention_suspended_servers', { - count: servers.suspended, - }), - actionLabel: t('attention_view'), - subjects: attention.suspendedServers, - to: subject => `/admin/servers/${subject.id}`, - total: servers.suspended, - overflowTo: '/admin/servers', }) } diff --git a/tests/Feature/Controllers/Admin/OverviewControllerTest.php b/tests/Feature/Controllers/Admin/OverviewControllerTest.php index 28b1b77bce7..1ad44011915 100644 --- a/tests/Feature/Controllers/Admin/OverviewControllerTest.php +++ b/tests/Feature/Controllers/Admin/OverviewControllerTest.php @@ -118,11 +118,11 @@ 'name' => 'broken-install', 'status' => Status::INSTALL_FAILED->value, ]); - $suspended = Server::factory()->for($node)->for($admin)->create([ - 'name' => 'unpaid', - 'status' => Status::SUSPENDED->value, + $healthy = Server::factory()->for($node)->for($admin)->create([ + 'name' => 'nightly-host', + 'status' => null, ]); - Backup::factory()->for($suspended)->create([ + Backup::factory()->for($healthy)->create([ 'name' => 'nightly', 'is_successful' => false, 'completed_at' => now(), @@ -131,17 +131,51 @@ $this->actingAs($admin)->getJson('/api/admin/overview') ->assertOk() // Each row carries the route key its own destination takes, so a click - // lands on the record rather than on the unfiltered server list. - ->assertJsonPath('data.attention.failed_servers.0.id', (string) $failed->id) + // lands on the record rather than on the unfiltered server list. Every + // subject is keyed by the owning server's short uuid, which is what both + // the admin and client server routes bind on -- the primary key resolves + // to nothing. + ->assertJsonPath('data.attention.failed_servers.0.id', $failed->uuid_short) ->assertJsonPath('data.attention.failed_servers.0.label', 'broken-install') ->assertJsonPath('data.attention.failed_servers.0.detail', 'Installation failed on pve-1') - ->assertJsonPath('data.attention.suspended_servers.0.id', (string) $suspended->id) - ->assertJsonPath('data.attention.suspended_servers.0.detail', 'On pve-1') - // Backups are keyed by the server's short uuid: the backups tab is theirs. - ->assertJsonPath('data.attention.failed_backups.0.id', $suspended->uuid_short) + ->assertJsonPath('data.attention.failed_backups.0.id', $healthy->uuid_short) ->assertJsonPath('data.attention.failed_backups.0.label', 'nightly'); }); +it('leaves a suspended server off the card entirely', function () { + $admin = User::factory()->create(['root_admin' => true]); + $node = Node::factory()->for(Location::factory())->create(); + Server::factory()->for($node)->for($admin)->create([ + 'status' => Status::SUSPENDED->value, + ]); + + // A suspension is deliberate, so it belongs in the server-state counts and + // nowhere near a list of things that need fixing. + $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + ->assertJsonPath('data.servers.suspended', 1) + ->assertJsonPath('data.attention.failed_servers', []) + ->assertJsonMissingPath('data.attention.suspended_servers'); +}); + +it('keys attention subjects by something the server route can actually resolve', function () { + $admin = User::factory()->create(['root_admin' => true]); + $node = Node::factory()->for(Location::factory())->create(); + Server::factory()->for($node)->for($admin)->create([ + 'status' => Status::INSTALL_FAILED->value, + ]); + + $id = $this->actingAs($admin)->getJson('/api/admin/overview') + ->assertOk() + ->json('data.attention.failed_servers.0.id'); + + // The card links at /admin/servers/{id}, so whatever the endpoint hands back + // has to be the key that page's own request binds on. The primary key is not + // it: RouteServiceProvider reads a non-8-character value as a uuid, and the + // page dies with "No query results for model [Convoy\Models\Server]". + $this->actingAs($admin)->getJson("/api/admin/servers/{$id}")->assertOk(); +}); + it('separates a deletion failure from an install failure in the detail', function () { $admin = User::factory()->create(['root_admin' => true]); $node = Node::factory()->for(Location::factory())->create(['name' => 'pve-2']); @@ -163,8 +197,7 @@ $this->actingAs($admin)->getJson('/api/admin/overview') ->assertOk() ->assertJsonPath('data.attention.failed_servers', []) - ->assertJsonPath('data.attention.failed_backups', []) - ->assertJsonPath('data.attention.suspended_servers', []); + ->assertJsonPath('data.attention.failed_backups', []); }); it('caps each attention group and leaves the count to say how many there really are', function () { From a2394790a0a65b1eaca60872b6b3a907ba7d7e2a Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:19:47 -0400 Subject: [PATCH 05/12] fix(ui): stop a tall modal from growing past the viewport Modal.Body capped itself at 60vh, but nothing bounded the panel around it, so a header and a footer on top of that could exceed the screen. The scroll container is overflow-hidden, so the excess was not scrolled to -- it was clipped, and a long enough list took the title with it. The panel now caps at the viewport and lays out as a column, so the body is what gives: min-h-0 lets it shrink below its content and its own overflow-y-auto absorbs the rest, with the header and actions held at their natural size. It is a cap rather than a height, so a modal that already fit renders exactly as before. --- resources/scripts/components/elements/Drawer.tsx | 13 ++++++++++++- resources/scripts/components/elements/Modal.tsx | 6 +++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/resources/scripts/components/elements/Drawer.tsx b/resources/scripts/components/elements/Drawer.tsx index 9cafee5b6c8..3c74a71ee53 100644 --- a/resources/scripts/components/elements/Drawer.tsx +++ b/resources/scripts/components/elements/Drawer.tsx @@ -42,9 +42,20 @@ const Drawer = forwardRef( leaveFrom='opacity-100 translate-y-0' leaveTo='opacity-0 translate-y-[100vh] sm:-translate-y-[10vh]' > + {/* + * Bounded by the viewport, not by its content: the + * panel is the scroll container's only child and + * the container is `overflow-hidden`, so a panel + * taller than the screen has its head and foot + * clipped with no way to reach them. Capping it + * here and laying it out as a column lets whichever + * section opts into `overflow-y-auto` (Modal.Body) + * absorb the excess, while a modal that already + * fits is untouched. + */} { } Modal.Header = styled.div` - ${tw`p-8 sm:p-6 border-b border-accent-200`} + ${tw`shrink-0 p-8 sm:p-6 border-b border-accent-200`} ` Modal.Title = styled.h3` @@ -53,7 +53,7 @@ Modal.Title = styled.h3` ` Modal.Body = styled.div` - ${tw`max-h-[60vh] overflow-y-auto p-6 bg-accent-100`} + ${tw`min-h-0 flex-1 overflow-y-auto p-6 bg-accent-100`} ` Modal.Description = ({ children, bottomMargin }) => { @@ -67,7 +67,7 @@ Modal.Description = ({ children, bottomMargin }) => { } Modal.Actions = styled.div` - ${tw`flex border-t border-accent-200`} + ${tw`shrink-0 flex border-t border-accent-200`} & > button:is(:first-of-type) { ${tw`rounded-bl`} From 5ae71993c761da7789971a9d85a2ad1017905fb9 Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Wed, 9 Sep 2026 00:19:47 -0400 Subject: [PATCH 06/12] chore(dev): seed a real node and attention-card fixtures DevNodeSeeder is a port of 5.x's, reading PROXMOX_* out of the gitignored .env. The 4.x Node splits what 5.x kept in one field -- `cluster` is the PVE name in the API path, `name` is a free label -- and the seeder probes /status and /storage for the host's real capacity rather than leaving the factory's invented 64GiB, falling back to those defaults when the node cannot be reached. AttentionSeeder covers both shapes the card renders: one failed server for the row that names its record outright, and 28 failed backups for the row that opens a sheet and runs past the 25-record cap. Suspended and mid-delete servers come along as negative checks, since both belong in Server State and neither should appear on the card. Fixtures hang off a placeholder node with a .invalid fqdn, never the live one. --- database/seeders/AttentionSeeder.php | 165 +++++++++++++++++++++++++++ database/seeders/DevNodeSeeder.php | 132 +++++++++++++++++++++ 2 files changed, 297 insertions(+) create mode 100644 database/seeders/AttentionSeeder.php create mode 100644 database/seeders/DevNodeSeeder.php diff --git a/database/seeders/AttentionSeeder.php b/database/seeders/AttentionSeeder.php new file mode 100644 index 00000000000..f020cc0ce45 --- /dev/null +++ b/database/seeders/AttentionSeeder.php @@ -0,0 +1,165 @@ +update(['status' => 'deletion_failed']);" + * + * Healthy noise (successful and pending backups, servers mid-install, mid-delete + * and suspended) comes along so the surrounding cards are not all zeros. Those + * last two are also negative checks: mid-delete and suspended both belong in + * Server State and NOT on the attention card. + * + * Everything lands on a placeholder node, never the live one from + * DevNodeSeeder: a card link opens the server's admin page, and a phantom vmid + * on a real host is a worse failure than one on a node that was never real. + * + * Idempotent -- it drops its own servers (by name) and their backups first, so + * re-running does not multiply the fixtures. + * + * Run: ddev artisan db:seed --class=AttentionSeeder + */ +class AttentionSeeder extends Seeder +{ + private const NODE_NAME = 'ord-hv-01'; + + private const NODE_FQDN = 'ord-hv-01.fixtures.invalid'; + + /** Fixture server names, also the handle used to clean up a previous run. */ + private const FAILED = ['ord-web-04']; + + private const SUSPENDED = ['fra-mail-01']; + + private const BACKUP_OWNERS = ['ams-app-01', 'ams-app-02', 'ams-cache-01']; + + private const HEALTHY = ['syd-web-01', 'syd-web-02', 'syd-queue-01', 'syd-api-03']; + + public function run(ServerCreationService $service): void + { + $names = array_merge(self::FAILED, self::SUSPENDED, self::BACKUP_OWNERS, self::HEALTHY); + + $stale = Server::query()->whereIn('name', $names)->pluck('id'); + Backup::query()->whereIn('server_id', $stale)->forceDelete(); + Server::query()->whereIn('id', $stale)->delete(); + + $user = User::query()->orderBy('id')->first() ?? User::factory()->create(); + $node = $this->placeholderNode(); + + $make = function (string $name, ?string $status) use ($service, $user, $node): Server { + $uuid = $service->generateUniqueUuidCombo(); + + return Server::factory()->create([ + 'uuid' => $uuid, + 'uuid_short' => substr($uuid, 0, 8), + 'name' => $name, + 'hostname' => $name.'.example.com', + 'status' => $status, + 'user_id' => $user->id, + 'node_id' => $node->id, + 'cpu' => 2, + 'memory' => 2048 * 1024 * 1024, + 'disk' => 20 * 1024 * 1024 * 1024, + 'backup_limit' => 16, + 'snapshot_limit' => 16, + 'bandwidth_limit' => 100 * 1024 * 1024 * 1024, + ]); + }; + + // Exactly one, to hold the card's single-record path open. + $make(self::FAILED[0], Status::INSTALL_FAILED->value); + + // Not failures: suspended, mid-install and mid-delete are Server State's + // business and must stay off the card. + $make(self::SUSPENDED[0], Status::SUSPENDED->value); + $make(self::HEALTHY[0], Status::INSTALLING->value); + $make(self::HEALTHY[1], Status::INSTALLING->value); + $make(self::HEALTHY[2], Status::DELETING->value); + $make(self::HEALTHY[3], Status::RESTORING_BACKUP->value); + + $owners = collect(self::BACKUP_OWNERS)->map(fn (string $name) => $make($name, null)); + + // 28 failures against a cap of 25. A backup counts as failed only once it + // has given up -- completed_at set, is_successful false -- so a pending + // backup (completed_at null) is neither failed nor successful. + foreach (range(1, 28) as $i) { + $this->backup($owners[$i % $owners->count()], "daily-{$i}", false, now()->subHours($i)); + } + + foreach (range(1, 6) as $i) { + $this->backup($owners[$i % $owners->count()], "weekly-{$i}", true, now()->subDays($i)); + } + + foreach (range(1, 3) as $i) { + $this->backup($owners[$i % $owners->count()], "in-flight-{$i}", false, null); + } + + $this->command->info(sprintf( + 'AttentionSeeder: %d servers and %d backups on node #%d (%s).', + count($names), + 37, + $node->id, + $node->name, + )); + } + + private function backup(Server $server, string $name, bool $successful, ?object $completedAt): void + { + Backup::factory()->create([ + 'server_id' => $server->id, + 'name' => $name, + 'is_successful' => $successful, + 'is_locked' => false, + 'completed_at' => $completedAt, + ]); + } + + /** + * The node the fixtures hang off. Its own, rather than whichever node happens + * to be first: the card prints the node name in every detail line ("Deletion + * failed on ..."), so a faker word there makes the thing being tested harder + * to read. `.invalid` is reserved by RFC 2606 and can never resolve, which is + * the point -- nothing here should ever reach a host. + */ + private function placeholderNode(): Node + { + $existing = Node::query()->where('fqdn', self::NODE_FQDN)->first(); + + if ($existing) { + return $existing; + } + + return Node::factory() + ->for(Location::query()->firstOrCreate( + ['short_code' => 'ord'], + ['description' => 'Chicago (fixtures)'], + )) + ->create([ + 'name' => self::NODE_NAME, + 'cluster' => self::NODE_NAME, + 'fqdn' => self::NODE_FQDN, + ]); + } +} diff --git a/database/seeders/DevNodeSeeder.php b/database/seeders/DevNodeSeeder.php new file mode 100644 index 00000000000..a1ebc787299 --- /dev/null +++ b/database/seeders/DevNodeSeeder.php @@ -0,0 +1,132 @@ +' failed". + * + * Idempotent: an existing node with the same fqdn is left alone, so re-running + * after migrate:fresh never creates duplicates. + * + * Run: ddev artisan db:seed --class=DevNodeSeeder + */ +class DevNodeSeeder extends Seeder +{ + public function run(): void + { + $fqdn = env('PROXMOX_FQDN'); + $tokenId = env('PROXMOX_TOKEN_ID'); + $tokenSecret = env('PROXMOX_TOKEN_SECRET'); + + if (! $fqdn || ! $tokenId || ! $tokenSecret) { + $this->command->warn( + 'DevNodeSeeder skipped: set PROXMOX_FQDN, PROXMOX_TOKEN_ID and ' + .'PROXMOX_TOKEN_SECRET in .env first.' + ); + + return; + } + + if ($existing = Node::query()->where('fqdn', $fqdn)->first()) { + $this->command->info("DevNodeSeeder: node for {$fqdn} already exists (#{$existing->id})."); + + return; + } + + $cluster = env('PROXMOX_NODE_NAME') ?: explode('.', $fqdn)[0]; + + $location = Location::query()->firstOrCreate( + ['short_code' => 'dev'], + ['description' => 'Dev Proxmox'], + ); + + $verifyTls = filter_var(env('PROXMOX_VERIFY_TLS', false), FILTER_VALIDATE_BOOLEAN); + $port = (int) env('PROXMOX_PORT', 8006); + + // Advertised capacity drives the admin Capacity card and every + // overallocation check, so read it off the host rather than inventing it. + // Falls back to the factory's numbers when the node cannot be reached -- + // the seeder still has to work offline. + $capacity = $this->probeCapacity($fqdn, $port, $cluster, $tokenId, $tokenSecret, $verifyTls); + + // The factory supplies resource defaults; only the connection details and + // storage names come from the environment. + $node = Node::factory()->for($location)->create([ + 'name' => $cluster, + 'cluster' => $cluster, + 'fqdn' => $fqdn, + 'port' => $port, + 'verify_tls' => $verifyTls, + 'token_id' => $tokenId, + 'secret' => $tokenSecret, // encrypted by the model cast on save + 'vm_storage' => env('PROXMOX_VM_STORAGE', 'local'), + 'backup_storage' => env('PROXMOX_BACKUP_STORAGE', 'local'), + 'iso_storage' => env('PROXMOX_ISO_STORAGE', 'local'), + 'network' => env('PROXMOX_NETWORK', 'vmbr0'), + ] + $capacity); + + $this->command->info("DevNodeSeeder: created node #{$node->id} for {$fqdn}:{$node->port} (cluster {$cluster})."); + } + + /** + * Real memory and disk totals for the node, as a partial attribute array. + * + * Disk is the configured `vm_storage`'s total, which is the pool servers are + * actually built on -- summing every storage would double-count a host where + * one directory backs several entries. Returns an empty array when the node is + * unreachable, leaving the factory defaults in place. + */ + private function probeCapacity( + string $fqdn, + int $port, + string $cluster, + string $tokenId, + string $tokenSecret, + bool $verifyTls, + ): array { + $vmStorage = env('PROXMOX_VM_STORAGE', 'local'); + + try { + $client = Http::withOptions(['verify' => $verifyTls]) + ->withHeaders(['Authorization' => "PVEAPIToken={$tokenId}={$tokenSecret}"]) + ->timeout(15) + ->baseUrl("https://{$fqdn}:{$port}"); + + $memory = $client->get("/api2/json/nodes/{$cluster}/status")->json('data.memory.total'); + + $disk = collect($client->get("/api2/json/nodes/{$cluster}/storage")->json('data') ?? []) + ->firstWhere('storage', $vmStorage)['total'] ?? null; + } catch (Throwable $e) { + $this->command->warn("DevNodeSeeder: capacity probe failed ({$e->getMessage()}); using factory defaults."); + + return []; + } + + return array_filter([ + 'memory' => $memory ? (int) $memory : null, + 'disk' => $disk ? (int) $disk : null, + ]); + } +} From 22c5341e7db1db2b69ba61dcfb15ece98cbf9896 Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Sun, 20 Sep 2026 01:57:58 -0400 Subject: [PATCH 07/12] fix(ui): make the modal panel usable when its body has to scroll Two faults in the same panel, both reachable from any modal in the panel. The panel caps itself and hides its overflow, and Modal.Body sizes itself with `flex-1` -- but 22 of the 23 modals put a
between the two so the footer can submit. A block-level form is a flex item that will not shrink below its content, so Modal.Body had no bounded parent to measure `flex-1` against, grew to its full height, and pushed the submit row out through the panel's `overflow-hidden` with nothing left to scroll it back. Create Server and Create Node could not be submitted at all at a 900px-tall viewport: no scrollbar, no wheel response, and no Cancel/Create on screen. Extending the column through a direct-child form puts Modal.Body back under the cap. FormProvider and FormikProvider render no DOM, so the form really is a direct child in every case. Separately, `initialFocus` pointed at an ``, which cannot take focus at all. Opening a dialog therefore left focus on the trigger behind the overlay and the first few Tabs walked the page underneath. A tabIndex={-1} sentinel is focusable programmatically without joining the tab order, so it parks focus inside the panel while preserving what the hidden input was for -- not stealing focus from the first field. --- .../scripts/components/elements/Drawer.tsx | 53 ++++++++++++++----- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/resources/scripts/components/elements/Drawer.tsx b/resources/scripts/components/elements/Drawer.tsx index 3c74a71ee53..c5a53af7320 100644 --- a/resources/scripts/components/elements/Drawer.tsx +++ b/resources/scripts/components/elements/Drawer.tsx @@ -43,24 +43,49 @@ const Drawer = forwardRef( leaveTo='opacity-0 translate-y-[100vh] sm:-translate-y-[10vh]' > {/* - * Bounded by the viewport, not by its content: the - * panel is the scroll container's only child and - * the container is `overflow-hidden`, so a panel - * taller than the screen has its head and foot - * clipped with no way to reach them. Capping it - * here and laying it out as a column lets whichever - * section opts into `overflow-y-auto` (Modal.Body) - * absorb the excess, while a modal that already - * fits is untouched. - */} + * Bounded by the viewport, not by its content: the + * panel is the scroll container's only child and + * the container is `overflow-hidden`, so a panel + * taller than the screen has its head and foot + * clipped with no way to reach them. Capping it + * here and laying it out as a column lets whichever + * section opts into `overflow-y-auto` (Modal.Body) + * absorb the excess, while a modal that already + * fits is untouched. + * + * The `[&>form]` rules extend that column through a + * form. Most modals wrap Modal.Body and + * Modal.Actions in one so the footer can submit, and + * a plain block form is a flex item that refuses to + * shrink below its content -- which left Modal.Body + * with no bounded parent to size `flex-1` against, + * and pushed the submit row out through + * `overflow-hidden` with nothing to scroll it back. + * Making the form a column too puts Modal.Body back + * under the cap. (FormProvider/FormikProvider render + * no DOM, so the form really is a direct child.) + */} - {children} From b9854ca9b03c5ba1974dbadf712e5917db328e5e Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Sun, 20 Sep 2026 01:58:06 -0400 Subject: [PATCH 08/12] fix(admin): keep the attention row's overflow count out of the truncation The collapsed summary built one string -- every name it had, then "and 3 more" -- and left the trimming to `truncate`. But CSS truncates from the end, so the suffix was always the first thing cut, and the suffix is the only part the count beside it does not already say. With 25 backup names the row read "daily-1, daily-2, daily-3, daily-4, d..." and the three unlisted records went unmentioned. Returning the names and the overflow separately lets the names shrink around a pinned suffix. Also in the card: guard `subject.detail` in the single-record branch the way SubjectRow already does, so a subject without one does not render an empty line; key subject rows on their index as well, since one server can own two failed backups sharing a name and id+label then collides; add `aria-expanded` alongside `aria-haspopup`; and drop the reference to a 60vh cap on Modal.Body that this branch itself removed. --- lang/en_US/admin/overview.php | 4 ++ .../admin/overview/AttentionCard.tsx | 45 ++++++++++++------- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/lang/en_US/admin/overview.php b/lang/en_US/admin/overview.php index a7af3a1d0e3..3d145bc3d25 100644 --- a/lang/en_US/admin/overview.php +++ b/lang/en_US/admin/overview.php @@ -16,6 +16,10 @@ 'attention_failed_servers_other' => ':count servers failed', 'attention_failed_backups_one' => ':count backup failed', 'attention_failed_backups_other' => ':count backups failed', + // Passing `count` makes i18next look for the plural suffixes first, so + // spell them out rather than relying on the bare-key fallback. + 'attention_and_more_one' => 'and :count more', + 'attention_and_more_other' => 'and :count more', 'attention_close' => 'Close', 'attention_showing' => 'Showing the :shown most recent of :total.', 'capacity' => 'Capacity', diff --git a/resources/scripts/components/admin/overview/AttentionCard.tsx b/resources/scripts/components/admin/overview/AttentionCard.tsx index 5a0a2b19090..97a04fb282f 100644 --- a/resources/scripts/components/admin/overview/AttentionCard.tsx +++ b/resources/scripts/components/admin/overview/AttentionCard.tsx @@ -60,15 +60,15 @@ const SubjectRow = ({ /** * The names behind a collapsed group, so the summary line carries something the - * count does not. Truncation is the browser's job -- the string names as many as - * it has and says how many it left out. + * count does not. The names truncate and the overflow count does not: baked into + * one string, `truncate` cuts from the end, so "and 3 more" -- the only part the + * count does not already say -- was the first thing to go. They are returned + * separately so the row can let the names shrink around a pinned suffix. */ -const preview = (group: AttentionGroup): string => { - const names = group.subjects.map(subject => subject.label) - const hidden = group.total - names.length - - return hidden > 0 ? `${names.join(', ')} and ${hidden} more` : names.join(', ') -} +const preview = (group: AttentionGroup) => ({ + names: group.subjects.map(subject => subject.label).join(', '), + hidden: group.total - group.subjects.length, +}) /** * One group. A single record is named outright and links straight at itself -- @@ -79,7 +79,7 @@ const preview = (group: AttentionGroup): string => { * The sheet is what keeps the dashboard still. Expanding 25 records inline grew * the card by several hundred pixels and shoved every card below it down the * page, so reading one row rearranged the rest -- and the group most worth - * opening moved the page most. Modal.Body caps itself at 60vh and scrolls, so + * opening moved the page most. Modal.Body is the panel's scrolling section and * the list is bounded however broken the fleet is. */ const GroupRow = ({ group }: { group: AttentionGroup }) => { @@ -87,6 +87,7 @@ const GroupRow = ({ group }: { group: AttentionGroup }) => { const { t } = useTranslation('admin.overview') const Icon = group.icon const truncated = group.total > group.subjects.length + const { names, hidden } = preview(group) const icon = (
@@ -107,9 +108,11 @@ const GroupRow = ({ group }: { group: AttentionGroup }) => {

{subject.label}

-

- {subject.detail} -

+ {subject.detail && ( +

+ {subject.detail} +

+ )}
{group.actionLabel} @@ -125,6 +128,7 @@ const GroupRow = ({ group }: { group: AttentionGroup }) => { type='button' onClick={() => setOpen(true)} aria-haspopup='dialog' + aria-expanded={open} className='flex w-full items-center gap-3 rounded border border-transparent bg-transparent px-2 py-3 text-left transition-colors hover:border-accent-200 hover:bg-accent-100' > {icon} @@ -132,8 +136,13 @@ const GroupRow = ({ group }: { group: AttentionGroup }) => {

{group.title}

-

- {preview(group)} +

+ {names} + {hidden > 0 && ( + + {t('attention_and_more', { count: hidden })} + + )}

@@ -147,9 +156,13 @@ const GroupRow = ({ group }: { group: AttentionGroup }) => {
- {group.subjects.map(subject => ( + {group.subjects.map((subject, index) => ( setOpen(false)} From 9a89c1b51a736d18e9ce3dddf272965d2f6b0228 Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Sun, 20 Sep 2026 01:58:11 -0400 Subject: [PATCH 09/12] fix(admin): stop the overview stat cards leaving a half-empty row Dropping the attention stat tile left three cards behind, still laid out `sm:col-span-6 xl:col-span-4`. Three cards two-to-a-row means the third sits alone against a half-width gap, and that held from 640px all the way to 1279px -- most laptop widths included. Stacking them until `md` and fitting all three from 768px up gives one clean row or a clean stack, never 2+1. The skeleton follows the same breakpoints so the placeholder does not jump to a different shape when the data lands. Formatting is prettier's; the file was already failing `prettier --check` on this branch and the import it added was part of why. --- .../admin/overview/OverviewContainer.tsx | 38 ++++++------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/resources/scripts/components/admin/overview/OverviewContainer.tsx b/resources/scripts/components/admin/overview/OverviewContainer.tsx index 735dddaa953..27cdff2ef39 100644 --- a/resources/scripts/components/admin/overview/OverviewContainer.tsx +++ b/resources/scripts/components/admin/overview/OverviewContainer.tsx @@ -11,18 +11,18 @@ import { ComponentType } from 'react' import { useTranslation } from 'react-i18next' import { Link } from 'react-router-dom' -import useOverviewSWR from '@/api/admin/overview/useOverviewSWR' import { DashboardMetric, DashboardNode, } from '@/api/admin/overview/getOverview' - -import AttentionCard from '@/components/admin/overview/AttentionCard' +import useOverviewSWR from '@/api/admin/overview/useOverviewSWR' import Card from '@/components/elements/Card' import MessageBox from '@/components/elements/MessageBox' import PageContentBlock from '@/components/elements/PageContentBlock' +import AttentionCard from '@/components/admin/overview/AttentionCard' + interface IconProps { className?: string } @@ -53,10 +53,7 @@ const StatCard = ({ title, value, detail, icon: Icon, to }: StatCardProps) => { if (to) { return ( - + {content} @@ -64,11 +61,7 @@ const StatCard = ({ title, value, detail, icon: Icon, to }: StatCardProps) => { ) } - return ( - - {content} - - ) + return {content} } const UsageBar = ({ @@ -143,7 +136,7 @@ const OverviewSkeleton = () => ( {[1, 2, 3].map(item => ( ))} @@ -165,9 +158,7 @@ const OverviewContainer = () => {

{tStrings('overview')}

-

- {t('description')} -

+

{t('description')}

@@ -241,8 +232,7 @@ const OverviewContainer = () => {

{t('addresses_detail', { - available: - data.addresses.available, + available: data.addresses.available, pools: data.addresses.pools, })}

@@ -267,8 +257,7 @@ const OverviewContainer = () => { {tStrings('iso', { count: 2 })}

- {data.isos.successful} /{' '} - {data.isos.total} + {data.isos.successful} / {data.isos.total}

{t('iso_status_detail', { @@ -295,10 +284,7 @@ const OverviewContainer = () => { {[ [t('ready'), data.servers.ready], [t('installing'), data.servers.installing], - [ - tStrings('suspended'), - data.servers.suspended, - ], + [tStrings('suspended'), data.servers.suspended], [t('restoring'), data.servers.restoring], [t('deleting'), data.servers.deleting], [t('failed'), data.servers.failed], @@ -317,9 +303,7 @@ const OverviewContainer = () => { -

- {tStrings('node', { count: 2 })} -

+

{tStrings('node', { count: 2 })}

{t('nodes_description')}

From d45db5da43edea91afcdb60679a4c5c27641e421 Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Sun, 20 Sep 2026 01:58:15 -0400 Subject: [PATCH 10/12] fix(admin): skip node-scoped requests until a node is chosen The create-server form renders its address and template pickers before a node is selected. The select binds to a string and coerces with `Number`, and one caller defaults to -1, so the node id reaching these hooks is '', NaN or -1 -- and the request went out regardless, as /api/admin/nodes//template-groups and /api/admin/nodes/NaN/addresses. Both 404 on every open of the modal. A null SWR key tells SWR not to fetch, which is the intended way to express "no subject yet". Callers already handle `data` being undefined while loading, so nothing downstream changes. --- .../api/admin/nodes/addresses/useAddressesSWR.ts | 9 +++++++-- resources/scripts/api/admin/nodes/isValidNodeId.ts | 10 ++++++++++ .../admin/nodes/templateGroups/useTemplateGroupsSWR.ts | 8 ++++++-- 3 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 resources/scripts/api/admin/nodes/isValidNodeId.ts diff --git a/resources/scripts/api/admin/nodes/addresses/useAddressesSWR.ts b/resources/scripts/api/admin/nodes/addresses/useAddressesSWR.ts index 9a7748d6b90..4721c5b959a 100644 --- a/resources/scripts/api/admin/nodes/addresses/useAddressesSWR.ts +++ b/resources/scripts/api/admin/nodes/addresses/useAddressesSWR.ts @@ -4,6 +4,7 @@ import getAddresses, { AddressResponse, QueryParams, } from '@/api/admin/nodes/addresses/getAddresses' +import { isValidNodeId } from '@/api/admin/nodes/isValidNodeId' interface Params extends QueryParams { id?: string | number @@ -11,9 +12,13 @@ interface Params extends QueryParams { const useAddressesSWR = (nodeId: number, { page, id, ...params }: Params) => { return useSWR( - ['admin:node:addresses', nodeId, page, id], + // See useTemplateGroupsSWR: skip the fetch until a real node is selected, + // rather than requesting /api/admin/nodes/NaN/addresses. + isValidNodeId(nodeId) + ? ['admin:node:addresses', nodeId, page, id] + : null, () => getAddresses(nodeId, { page, ...params }) ) } -export default useAddressesSWR \ No newline at end of file +export default useAddressesSWR diff --git a/resources/scripts/api/admin/nodes/isValidNodeId.ts b/resources/scripts/api/admin/nodes/isValidNodeId.ts new file mode 100644 index 00000000000..f529c08e856 --- /dev/null +++ b/resources/scripts/api/admin/nodes/isValidNodeId.ts @@ -0,0 +1,10 @@ +/** + * Whether a node id is usable in a request path. + * + * Forms bind the node select to a string and coerce with `Number`, so before a + * node is chosen the id is `''` (which coerces to 0) or `NaN`, and callers that + * default to `-1` pass that straight through. None of those name a node, and a + * request built from one 404s. + */ +export const isValidNodeId = (nodeId: unknown): nodeId is number => + typeof nodeId === 'number' && Number.isInteger(nodeId) && nodeId > 0 diff --git a/resources/scripts/api/admin/nodes/templateGroups/useTemplateGroupsSWR.ts b/resources/scripts/api/admin/nodes/templateGroups/useTemplateGroupsSWR.ts index b91907af09f..580d7053c82 100644 --- a/resources/scripts/api/admin/nodes/templateGroups/useTemplateGroupsSWR.ts +++ b/resources/scripts/api/admin/nodes/templateGroups/useTemplateGroupsSWR.ts @@ -1,5 +1,6 @@ import useSWR from 'swr' +import { isValidNodeId } from '@/api/admin/nodes/isValidNodeId' import getTemplateGroups, { TemplateGroup, } from '@/api/admin/nodes/templateGroups/getTemplateGroups' @@ -9,7 +10,10 @@ const useTemplateGroupsSWR = ( fallbackData?: TemplateGroup[] ) => { return useSWR( - ['admin:node:template-groups', nodeId], + // A null key tells SWR not to fetch. The create-server form renders this + // before a node is picked, where nodeId is '' or NaN, and the request went + // out anyway as /api/admin/nodes//template-groups -> 404. + isValidNodeId(nodeId) ? ['admin:node:template-groups', nodeId] : null, () => getTemplateGroups(nodeId), { fallbackData, @@ -17,4 +21,4 @@ const useTemplateGroupsSWR = ( ) } -export default useTemplateGroupsSWR \ No newline at end of file +export default useTemplateGroupsSWR From a8cc1f13a41ea52e72d5f01b1bdc21b1d60842eb Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Sun, 20 Sep 2026 02:02:16 -0400 Subject: [PATCH 11/12] fix(a11y): give the modal an accessible name Modal.Title rendered a bare h3, so Headless UI never saw a Dialog.Title and the dialog went out with no `aria-labelledby` at all. A screen reader announced "dialog" and stopped, with the visible heading sitting right there unused. Rendering Dialog.Title instead wires the dialog to it, and `as='h3'` keeps the heading level the markup already had. Verified on the attention sheet: the panel now carries aria-labelledby -> "28 backups failed". --- resources/scripts/components/elements/Modal.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/resources/scripts/components/elements/Modal.tsx b/resources/scripts/components/elements/Modal.tsx index 0ed85929d82..3d1028f7720 100644 --- a/resources/scripts/components/elements/Modal.tsx +++ b/resources/scripts/components/elements/Modal.tsx @@ -48,10 +48,19 @@ Modal.Header = styled.div` ${tw`shrink-0 p-8 sm:p-6 border-b border-accent-200`} ` -Modal.Title = styled.h3` +/* + * Dialog.Title rather than a bare h3: Headless UI points the dialog's + * `aria-labelledby` at whatever it renders, and without it the panel had no + * accessible name at all -- a screen reader announced "dialog" and nothing + * else, even though a visible heading was sitting right there. `as='h3'` + * keeps the heading level the markup already used. + */ +const StyledTitle = styled(Dialog.Title)` ${tw`text-xl font-medium text-foreground text-center`} ` +Modal.Title = ({ children }) => {children} + Modal.Body = styled.div` ${tw`min-h-0 flex-1 overflow-y-auto p-6 bg-accent-100`} ` From 18274830d0441c94ba1443f28f406b43be99f5ab Mon Sep 17 00:00:00 2001 From: Eric Wang <37554696+ericwang401@users.noreply.github.com> Date: Sun, 20 Sep 2026 02:02:26 -0400 Subject: [PATCH 12/12] docs(changelog): record the attention card rework and the modal fixes Filed under Unreleased rather than a version heading: no tag is being cut yet, and Keep a Changelog keeps pending entries there until one is. Rename the heading when the release goes out. Only net changes since v4.6.1 are listed. The ragged stat-card row and the truncated overflow count were both introduced and fixed inside this same unreleased branch, so they never reached anyone and are not user-visible changes. --- CHANGELOG.md | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05e435096d7..cfd188b49a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,30 @@ This file is a running track of new features and fixes to each version of the pa The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project follows [Semantic Versioning](https://semver.org) guidelines. +## Unreleased + +### Changed + +- The admin overview's Attention card now names what needs attention instead of reporting a bare count, and every row + links to the record itself -- a failed server to its admin page, a failed backup to its server's backups tab. Groups + holding several records open a sheet listing them, capped at the 25 most recent with a note saying how many more + there are. Previously the card showed only a failed-server count while its caption mixed in failed backups and + servers mid-delete, so it could read "0" with backups broken, and its link went to the unfiltered server list either + way. Suspended and mid-delete servers are deliberately left off it: neither is a failure, and both are already + counted under Server State. + +### Fixed + +- Fixed modals taller than the browser window clipping their own header and submit row with no way to scroll to them. + The panel is now bounded by the viewport and its body scrolls within it, so Create Server and Create Node stay usable + at shorter window heights. +- Fixed opening a modal leaving keyboard focus behind it, which sent the first few Tab presses through the page + underneath instead of the dialog. +- Fixed the create-server form requesting addresses and template groups before a node is chosen, producing a handful of + 404s every time the modal opened. +- Fixed modals going out without an accessible name, so a screen reader announced only "dialog" instead of reading the + heading already on screen. + ## v4.6.1 ### Security