fix(database): resolve type=app trusted_source names to UUIDs before DO API call by intel352 · Pull Request #25 · GoCodeAlone/workflow-plugin-digitalocean

intel352 · 2026-04-25T06:41:19Z

Summary

Root cause: trusted_sources rules with type: app were passing the app name directly to the DO Databases Firewall API, which requires a UUID. This caused a 422 invalid app uuid error on cf5ad4a4.
Fix: DatabaseDriver now holds an optional appNameLister (narrow interface — List only). When a trusted_source rule has type: app and the value is not already UUID-shaped (isUUIDLike), it resolves the name to a UUID via Apps.List. UUID-shaped values pass through unchanged (no list call).
Both paths fixed: Create (via buildCreateFirewallRules) and Update (via buildUpdateFirewallRules) both resolve names via a single resolveAppNamesMap pass.
Constructor: NewDatabaseDriver(*godo.Client, region) now wires c.Apps automatically. NewDatabaseDriverWithClient is unchanged. New NewDatabaseDriverWithClients injects both clients for tests.

Why adc2473b succeeded

The first successful deploy likely had a different first-apply code path (Create vs Update) or DO's API behavior around type=app in the create-firewall-rules embedded in DatabaseCreateRequest differed from the standalone UpdateFirewallRules endpoint. Regardless, the fix makes both paths safe.

Tests

Seven new unit tests in database_test.go:

TestDatabaseDriver_Create_WithTrustedSources_AppNameResolved — name resolved to UUID on Create
TestDatabaseDriver_Create_WithTrustedSources_AppUUIDPassThrough — UUID values skip list call
TestDatabaseDriver_Update_WithTrustedSources_AppNameResolved — name resolved to UUID on Update
TestDatabaseDriver_Create_AppNameNotFound_ReturnsError — clear error when app absent from list
TestDatabaseDriver_Create_AppType_NoAppsClient_ReturnsError — actionable error when no apps client configured
TestDatabaseDriver_Create_TrustedSources_WrongType_ReturnsError — explicit error (not silent empty rules) when trusted_sources is not a list on Create
TestDatabaseDriver_Update_TrustedSources_WrongType_ReturnsError — same guard on Update path

All existing tests pass. GOWORK=off go test ./internal/... ✓

Scope addition note

The existing unit tests (TestDatabaseDriver_Create_WithTrustedSources, TestDatabaseDriver_Update_WithTrustedSources) only tested ip_addr and k8s rule types. This PR adds explicit type=app coverage and type-safety guards.

Next steps

Tag v0.7.10, then BMW pin-bump PR.

🤖 Generated with Claude Code

…DO API call The DO database firewall API requires an app UUID for type=app rules. Passing an app name directly results in a 422 "invalid app uuid" error. Add DatabaseDriver.resolveAppName which, when the value is not already UUID-shaped, looks up the app by name via the Apps API. Wire AppPlatformClient into DatabaseDriver at construction time (NewDatabaseDriver wires c.Apps; NewDatabaseDriverWithClients supports injection for tests). Replace the plain trustedSourceCreateRulesFromConfig / trustedSourceFirewallRulesFromConfig calls in Create and Update with context-aware buildCreateFirewallRules and buildUpdateFirewallRules methods that call resolveAppName for type=app entries. Add five new unit tests: - AppNameResolved (Create + Update): name → UUID resolved via mock Apps list - AppUUIDPassThrough: UUID-shaped values skip the list call entirely - AppNameNotFound: returns error when app is not in the list - NoAppsClient: returns an actionable error when appsClient is nil Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Fixes DigitalOcean database firewall trusted_sources rules of type: app by ensuring app names are resolved to UUIDs before calling the DO Databases Firewall API (which requires UUIDs), preventing 422 invalid app uuid errors.

Changes:

Extend DatabaseDriver to optionally hold an AppPlatformClient and resolve type=app trusted source values from name → UUID when needed.
Wire the Apps client automatically in NewDatabaseDriver, and add NewDatabaseDriverWithClients for tests.
Add unit tests covering name→UUID resolution, UUID pass-through, missing app handling, and nil Apps client behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
internal/drivers/database.go	Adds app name→UUID resolution for `trusted_sources` (`type=app`) in both Create and Update paths via an optional Apps client.
internal/drivers/database_test.go	Adds unit tests validating `type=app` trusted source resolution and associated error cases.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T06:45:05Z

+		if ruleType == "app" {
+			resolved, err := d.resolveAppName(ctx, ruleValue)
+			if err != nil {
+				return nil, err
+			}


buildCreateFirewallRules resolves each type=app rule by calling resolveAppName, which paginates Apps.List. If multiple trusted_sources include type=app, this can trigger multiple full app-list scans and extra API calls. Consider resolving all non-UUID app names in one list pass (build a name→UUID map) or caching within a single build* call to reduce latency and rate-limit risk.

Copilot · 2026-04-25T06:45:05Z

+	})
+	if err == nil {
+		t.Fatal("expected error when app name not found, got nil")
+	}


This test only asserts that an error is returned when the app name isn’t found. To ensure callers can reliably classify the failure, please also assert that the error wraps drivers.ErrResourceNotFound (errors.Is) and/or that the message includes the missing app name.

Suggested change

}

}

if !errors.Is(err, drivers.ErrResourceNotFound) {

t.Fatalf("expected error to wrap drivers.ErrResourceNotFound, got %v", err)

}

Copilot · 2026-04-25T06:45:06Z

+	if err == nil {
+		t.Fatal("expected error when apps client is nil and value is not UUID, got nil")
+	}


This test only checks err != nil when appsClient is nil. Since the intent is to return an actionable error, consider asserting on the error text (e.g., mentions that the value isn’t UUID-shaped and no Apps client is configured) so the test will fail if the message regresses to a generic/cryptic error.

…tests Address Copilot review on PR #25: - Replace per-rule resolveAppName (one Apps.List scan per rule) with resolveAppNamesMap, which collects all non-UUID app names and resolves them in a single paginated Apps.List pass regardless of how many type=app rules are present, reducing API calls and rate-limit risk. - Remove now-unused resolveAppName helper. - TestDatabaseDriver_Create_AppNameNotFound_ReturnsError: assert errors.Is(err, ErrResourceNotFound) and that the missing name is included in the error message. - TestDatabaseDriver_Create_AppType_NoAppsClient_ReturnsError: assert error message contains "not a UUID" and the offending value so the message cannot silently regress to a cryptic form. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T06:57:44Z

+	opts := &godo.ListOptions{Page: 1, PerPage: 200}
+	for {
+		apps, resp, err := d.appsClient.List(ctx, opts)
+		if err != nil {
+			return nil, fmt.Errorf("app list for trusted_source resolution: %w", WrapGodoError(err))
+		}
+		for _, app := range apps {
+			if app.Spec == nil {
+				continue
+			}
+			if _, want := needed[app.Spec.Name]; want {
+				resolved[app.Spec.Name] = app.ID
+			}
+		}
+		if resp == nil || resp.Links == nil || resp.Links.IsLastPage() {
+			break
+		}
+		opts.Page++
+	}


resolveAppNamesMap continues paging through the entire Apps list even after all needed names have been resolved. You can short-circuit the pagination loop once len(resolved) == len(needed) to avoid unnecessary API calls in accounts with many apps.

…ppNamesMap Address Copilot round-3 review on PR #25: - Docstring: "A single paginated Apps.List call is made" was misleading (one call per page). Reworded to "a single paginated listing pass (potentially multiple page requests)" — accurate without overpromising. - Early exit: once len(resolved) == len(needed), return immediately without fetching further pages. Avoids scanning the full Apps list in accounts with many apps when all names are found early. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T07:04:05Z

+	}
+	resolved := make(map[string]string, len(needed))
+	opts := &godo.ListOptions{Page: 1, PerPage: 200}
+	for {
+		apps, resp, err := d.appsClient.List(ctx, opts)
+		if err != nil {
+			return nil, fmt.Errorf("app list for trusted_source resolution: %w", WrapGodoError(err))
+		}
+		for _, app := range apps {
+			if app.Spec == nil {
+				continue
+			}
+			if _, want := needed[app.Spec.Name]; want {
+				resolved[app.Spec.Name] = app.ID
+				if len(resolved) == len(needed) {
+					return resolved, nil // all names resolved — no more pages needed
+				}
+			}
+		}


resolveAppNamesMap keeps paging through Apps.List even after all needed app names have already been resolved, which can result in unnecessary API calls (and slower applies) in accounts with many apps. Consider returning early once len(resolved)==len(needed), and adjust the function comment accordingly (it currently implies a single pass without extra work once resolved).

Copilot · 2026-04-25T07:04:05Z

+	if !ok {
+		return nil, false, nil // key absent: leave existing rules unchanged
+	}
+	list, _ := raw.([]any)


buildUpdateFirewallRules does list, _ := raw.([]any). If trusted_sources is present but not a slice (e.g., mis-typed config), this silently treats it as an empty list and returns ok=true, which will clear all firewall rules on Update. It would be safer to validate the type and return an error when trusted_sources is present but not the expected []any.

Suggested change

list, _ := raw.([]any)

list, ok := raw.([]any)

if !ok {

return nil, false, fmt.Errorf("trusted_sources must be a list")

}

…host-flag Address Copilot round-4 findings on PR #25: Finding 1 (ghost-flag): early-exit `if len(resolved) == len(needed)` is confirmed present at line 275 in 91c3cf6. Copilot re-flagged based on a stale diff view; no code change needed. Finding 2 (real): buildUpdateFirewallRules did `list, _ := raw.([]any)`, silently treating a non-list trusted_sources value (e.g. a YAML scalar) as an empty slice — returning ok=true with zero rules and clearing all firewall rules. Same latent bug existed in buildCreateFirewallRules. Fix both: check key existence separately, then type-assert with the ok form and return an actionable error (mentioning "trusted_sources" and the actual Go type) when the value is not a []any. Add two tests: - TestDatabaseDriver_Create_TrustedSources_WrongType_ReturnsError - TestDatabaseDriver_Update_TrustedSources_WrongType_ReturnsError Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T07:07:58Z

+
+// buildUpdateFirewallRules converts "trusted_sources" config to
+// ([]*godo.DatabaseFirewallRule, bool, error). The bool mirrors
+// trustedSourceFirewallRulesFromConfig: false means key absent (leave unchanged);
+// true means key present (apply the returned slice, even if empty). type=app
+// values are resolved to UUIDs via a single Apps.List pass (see resolveAppNamesMap).


In buildUpdateFirewallRules, if the "trusted_sources" key is present but its value is not a []any (e.g. a mis-typed config), the type assertion falls back to nil and the function returns (empty rules, ok=true). That will clear all firewall rules unexpectedly. Consider validating the type (and returning an explicit error) so a malformed config can’t silently wipe rules.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T07:13:59Z

+// requested names have been resolved. Returns an error only if the Apps client
+// is unavailable for a non-UUID name, or if the DO API call fails.


The doc comment for resolveAppNamesMap says it only errors when the Apps client is unavailable or the DO API call fails, but the function also returns ErrResourceNotFound when a requested app name isn't found. Please update the comment to reflect the not-found error path as well.

Suggested change

// requested names have been resolved. Returns an error only if the Apps client

// is unavailable for a non-UUID name, or if the DO API call fails.

// requested names have been resolved. Returns an error if the Apps client is

// unavailable for a non-UUID name, if the DO API call fails, or if a

// requested app name cannot be found (ErrResourceNotFound).

Copilot · 2026-04-25T07:13:59Z

+	if d.appsClient == nil {
+		// Surface the first offending name for a clear error message.
+		for name := range needed {
+			return nil, fmt.Errorf(
+				"trusted_source type=app value %q is not a UUID and no Apps client is configured for name lookup; pass the app UUID directly",
+				name,
+			)
+		}


resolveAppNamesMap iterates over a map to pick an offending app name when appsClient is nil; map iteration order is non-deterministic, so the error message can vary if multiple names are present. Consider making this deterministic (e.g., keep a slice of needed names, sort, or include all unresolved names in the error) so failures are stable and easier to debug.

Copilot · 2026-04-25T07:14:00Z

+	if rules, ok, fwErr := d.buildUpdateFirewallRules(ctx, spec.Config); fwErr != nil {
+		return nil, fmt.Errorf("database update firewall %q: %w", ref.Name, fwErr)
+	} else if ok {


In Update, firewall rule parsing/resolution (buildUpdateFirewallRules) happens after the Resize call. Since buildUpdateFirewallRules can now fail for config errors (wrong type) or app name resolution failures, Update may partially apply changes (resize succeeds, then Update returns an error before firewall sync). Consider validating/resolving trusted_sources before making any mutating DO API calls to avoid partial updates on user input errors.

…e-validate firewall Address Copilot round-5 findings on PR #25: 1. resolveAppNamesMap docstring now lists all 3 error cases: nil Apps client, DO API failure, and ErrResourceNotFound for a missing name. 2. Nil-client error message was non-deterministic when multiple type=app names needed resolution (map iteration order). Sort names before picking the first to report so the message is stable across runs. 3. Update() called buildUpdateFirewallRules AFTER Resize, risking a partial-apply where the DB is resized but firewall rules are left stale on config/resolution errors. Move buildUpdateFirewallRules before Resize so all validation and name→UUID resolution happens upfront, before any DO API mutations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

internal/drivers/database.go:153

In Update(), firewall rule parsing/name→UUID resolution happens after the Resize API call. If trusted_sources is malformed or Apps.List fails / app name isn’t found, Update will return an error but the resize may already have been applied, leaving a partial update. Consider building/validating the firewall rules (and doing any app lookups) before calling Resize so Update either fails fast with no side effects, or succeeds fully.

	size := strFromConfig(spec.Config, "size", "db-s-1vcpu-1gb")
	numNodes, _ := intFromConfig(spec.Config, "num_nodes", 1)
	_, err = d.client.Resize(ctx, providerID, &godo.DatabaseResizeRequest{
		SizeSlug: size,
		NumNodes: numNodes,

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T07:24:05Z

+	// can fail for config errors (wrong type) or app name→UUID resolution failures;
+	// validating upfront prevents a partial-apply where Resize succeeds but the
+	// subsequent firewall update fails, leaving the DB resized with stale rules.


The comment implies that resolving/validating firewall rules upfront "prevents" partial applies where Resize succeeds but firewall update fails. This only prevents partial applies caused by config/type errors or app name→UUID resolution failures; UpdateFirewallRules can still fail due to API/network errors after Resize. Please reword to accurately scope what this upfront validation guarantees (or doesn’t).

Suggested change

// can fail for config errors (wrong type) or app name→UUID resolution failures;

// validating upfront prevents a partial-apply where Resize succeeds but the

// subsequent firewall update fails, leaving the DB resized with stale rules.

// can fail for config errors (wrong type) or app name→UUID resolution failures,

// so validating upfront avoids partial applies caused by those preflight issues.

// A later UpdateFirewallRules call can still fail after Resize due to API/service

// errors, so this reduces—but does not eliminate—the chance of partial applies.

… prevents Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T07:28:58Z

+		sort.Strings(names)
+		return nil, fmt.Errorf(
+			"trusted_source type=app value %q is not a UUID and no Apps client is configured for name lookup; pass the app UUID directly",
+			names[0],


The nil-appsClient error only includes the first unresolved app name (names[0]) and uses "trusted_source" (singular) even though the config key is "trusted_sources". If multiple type=app rules are present, this can be misleading; consider including all unresolved names (or a count + list) and referencing the "trusted_sources" key for easier troubleshooting.

…key in error message - Include all non-UUID app names in the nil-client error (not just names[0]) - Fix "trusted_source" → "trusted_sources" to match the config key - Update test assertion to match new "not UUIDs" wording Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T07:34:34Z

+	client     DatabaseClient
+	appsClient AppPlatformClient // optional; used to resolve type=app trusted_source names to UUIDs
+	region     string


appsClient is typed as AppPlatformClient, which is a fairly large interface (Create/Get/Update/Deployments/Delete) even though DatabaseDriver only needs List. This increases coupling between drivers and makes it harder to supply minimal test doubles. Consider using a narrow local interface (e.g., just List) for appsClient and NewDatabaseDriverWithClients.

Copilot · 2026-04-25T07:34:34Z

+	for name := range needed {
+		if _, ok := resolved[name]; !ok {
+			return nil, fmt.Errorf("trusted_source app %q: %w", name, ErrResourceNotFound)
+		}
+	}
+	return resolved, nil


When one or more requested app names are missing, the returned error depends on Go map iteration order (for name := range needed). This can make the user-facing error message nondeterministic if multiple type=app entries are unresolved. Consider collecting missing names, sorting them, and returning a deterministic error that lists all missing apps (or at least selects the first in sorted order).

Suggested change

for name := range needed {

if _, ok := resolved[name]; !ok {

return nil, fmt.Errorf("trusted_source app %q: %w", name, ErrResourceNotFound)

}

}

return resolved, nil

missing := make([]string, 0)

for name := range needed {

if _, ok := resolved[name]; !ok {

missing = append(missing, name)

}

}

if len(missing) == 0 {

return resolved, nil

}

sort.Strings(missing)

quoted := make([]string, len(missing))

for i, name := range missing {

quoted[i] = fmt.Sprintf("%q", name)

}

return nil, fmt.Errorf(

"trusted_source app(s) %s: %w",

strings.Join(quoted, ", "),

ErrResourceNotFound,

)

…p error - Define appNameLister (List only) instead of coupling to full AppPlatformClient - Collect all unresolved names, sort them, report them all in one error rather than returning the first found via non-deterministic map iteration Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…y is present When trusted_sources is present but invalid (wrong type or unresolvable app name), return (nil, true, error) so callers can distinguish "key absent" (false) from "key present but invalid" (true+error). Update docstring to match the bool semantics precisely. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-25T07:50:56Z

+func TestDatabaseDriver_Create_AppNameNotFound_ReturnsError(t *testing.T) {
+	// When the app name cannot be found in the Apps list, Create must return an
+	// error that wraps ErrResourceNotFound and names the missing app.
+	dbMock := &mockDatabaseClient{db: testDatabase()}
+	appMock := &mockAppClient{listApps: []*godo.App{}} // empty list — name not found
+	d := drivers.NewDatabaseDriverWithClients(dbMock, appMock, "nyc3")
+
+	_, err := d.Create(context.Background(), interfaces.ResourceSpec{
+		Name: "my-db",
+		Config: map[string]any{
+			"engine": "pg",
+			"trusted_sources": []any{
+				map[string]any{"type": "app", "value": "nonexistent-app"},
+			},
+		},
+	})
+	if err == nil {
+		t.Fatal("expected error when app name not found, got nil")
+	}
+	if !errors.Is(err, drivers.ErrResourceNotFound) {
+		t.Errorf("expected error to wrap drivers.ErrResourceNotFound; got: %v", err)
+	}
+	if !strings.Contains(err.Error(), "nonexistent-app") {
+		t.Errorf("expected error to mention missing app name %q; got: %v", "nonexistent-app", err)
+	}
+}
+
+func TestDatabaseDriver_Create_AppType_NoAppsClient_ReturnsError(t *testing.T) {
+	// When no Apps client is configured and the rule value is not UUID-shaped,
+	// Create must return an actionable error (not a generic one) that tells the
+	// caller the value is not a UUID and no client is available for lookup.
+	dbMock := &mockDatabaseClient{db: testDatabase()}
+	// NewDatabaseDriverWithClient does NOT wire an apps client.
+	d := drivers.NewDatabaseDriverWithClient(dbMock, "nyc3")
+
+	_, err := d.Create(context.Background(), interfaces.ResourceSpec{
+		Name: "my-db",
+		Config: map[string]any{
+			"engine": "pg",
+			"trusted_sources": []any{
+				map[string]any{"type": "app", "value": "bmw-staging"},
+			},
+		},
+	})
+	if err == nil {
+		t.Fatal("expected error when apps client is nil and value is not UUID, got nil")
+	}
+	if !strings.Contains(err.Error(), "not UUIDs") {
+		t.Errorf("expected error to mention 'not UUIDs'; got: %v", err)
+	}
+	if !strings.Contains(err.Error(), "bmw-staging") {
+		t.Errorf("expected error to mention the offending value %q; got: %v", "bmw-staging", err)
+	}
+}
+
+func TestDatabaseDriver_Create_TrustedSources_WrongType_ReturnsError(t *testing.T) {
+	// trusted_sources present but not a list (e.g. user wrote a scalar in YAML)
+	// must return an error rather than silently dropping the rule.
+	dbMock := &mockDatabaseClient{db: testDatabase()}
+	d := drivers.NewDatabaseDriverWithClient(dbMock, "nyc3")


PR description says "Five new unit tests in database_test.go", but this diff adds 7 new tests (including the two WrongType_ReturnsError cases). Please update the PR description to match the actual changes (or drop the extra tests if they’re out of scope).

intel352 · 2026-04-25T07:54:11Z

@copilot review

Copilot · 2026-04-25T07:58:29Z

@copilot review

All prior review feedback has been incorporated. Here's the current state:

Review concern	Status	Commit
`appsClient` used broad `AppPlatformClient` — only `List` needed	✅ Fixed — uses narrow `appNameLister` interface	`bf1361e`
`resolveAppNamesMap` kept paging after all names resolved	✅ Fixed — early return when `len(resolved) == len(needed)`	`91c3cf6`
Nil-appsClient error only showed first name (nondeterministic)	✅ Fixed — sorts all unresolved names, reports all at once	`3be5950`
Missing-app error nondeterministic (map iteration)	✅ Fixed — sorts `missing` slice before error message	`bf1361e`
`buildUpdateFirewallRules` silently cleared rules on wrong type	✅ Fixed — validates type, returns explicit error	`b678e6e`
`resolveAppNamesMap` docstring omitted `ErrResourceNotFound` path	✅ Fixed	`e728fd9`
Partial-apply comment overstated prevention guarantee	✅ Fixed — "reduces—but does not eliminate—the chance of partial applies"	`319110c`
Firewall build/resolve happened after `Resize` (partial apply risk)	✅ Fixed — `buildUpdateFirewallRules` called before `Resize`	`e728fd9`
Test for missing app didn't assert `errors.Is(err, ErrResourceNotFound)`	✅ Fixed	`91dc82f`
Test for nil appsClient didn't assert error message content	✅ Fixed — checks for `"not UUIDs"` and the offending value name	`91dc82f`

All 7 new unit tests pass. GOWORK=off go test ./internal/... ✓

intel352 · 2026-04-25T08:08:19Z

@copilot review

Copilot · 2026-04-25T08:09:45Z

@copilot review

The code is clean — all 7 new tests pass (GOWORK=off go test ./internal/drivers/ -run TestDatabase ✓) and all prior review feedback from previous rounds has been addressed. No new issues were found.

Copilot AI review requested due to automatic review settings April 25, 2026 06:41

Copilot started reviewing on behalf of intel352 April 25, 2026 06:41 View session