feat: improve service deletion

[zreigz]

• ServiceDeployment could remove its finalizer without deleting the Console object when status.id was empty
• API object deletion was skipped when the ReadOnly condition was set, but it never happened in the current code

Test Plan

Test environment: https://console.your-env.onplural.sh/

Checklist

• I have added a meaningful title and summary to convey the impact of this PR to a user.
• If required, I have updated the Plural documentation accordingly.
• I have added tests to cover my changes.
• I have deployed the agent to a test environment and verified that it works as expected (required only when changing agent code).

Plural Flow: console

[linear]

PROD-4665

[greptile-apps]

Greptile Summary

This PR fixes two bugs in ServiceDeployment finalizer cleanup: it looks up the service by name in the Console API when status.ID is absent, and it removes a dead IsReadonly() guard that was never triggered. The apierrors.IsNotFound check in the PatchObject defer is also a correct defensive fix.

• The new getServiceIDForDeletion helper calls getClusterID, which fetches the cluster's Kubernetes resource. If the cluster was already deleted before the service (a common cascading-delete scenario), the lookup returns an error and the finalizer is permanently stuck — a regression from the old path that simply removed the finalizer when status.ID was empty.
• service.Status.ID = new(serviceID) and the four return new(common.Wait()/WaitForResources()) calls use new with a value expression rather than a type; in standard Go new(T) takes a type and zero-initialises it, so the status ID would not be persisted and the requeue delays would collapse to zero.

Confidence Score: 2/5

The deletion path has multiple correctness issues that affect every service deleted without a status ID

The deletion path has multiple correctness issues: the resolved service ID is not reliably written back to status, passing a value expression to new in four requeue returns is inconsistent with how Go's built-in works, and the new cluster-lookup path can permanently block finalizer removal when the cluster resource is concurrently deleted.

go/controller/internal/controller/servicedeployment_controller.go — specifically the addOrRemoveFinalizer function and the new getServiceIDForDeletion helper

Important Files Changed

Filename	Overview
go/controller/internal/controller/servicedeployment_controller.go	Adds service ID lookup by name when status.ID is empty during deletion, removes the ReadOnly guard, and fixes the not-found swallow in the PatchObject defer — but introduces two defects: new(serviceID) likely sets status.ID to a zero-value pointer instead of the resolved ID, and new(common.Wait()) passes a value expression to a type-only built-in, possibly returning a zero-requeue result. Additionally, the new cluster-lookup path can permanently block finalizer removal when the cluster resource itself is gone.
go/controller/internal/controller/servicedeployment_controller_test.go	Adds a targeted test for the new empty-status-ID deletion path; covers the happy path (service found via API lookup and deleted) but does not exercise the cluster-not-found or cluster-not-ready error paths introduced by the new getServiceIDForDeletion helper.

_{Reviews (1): Last reviewed commit: "improve service deletion" | Re-trigger Greptile}