# Deep Review: 20260519-103443-pr-355

| | |
|---|---|
| **Date** | 2026-05-19 10:45 |
| **Repo** | [rancher-sandbox/rancher-desktop-daemon](https://github.com/rancher-sandbox/rancher-desktop-daemon) |
| **Round** | 1 |
| **Author** | [@mook-as](https://github.com/mook-as) |
| **PR** | [#355](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/355) — K3s: Use a dynamic port |
| **Branch** | `mook-as:dynamic-k3s-port` |
| **Commits** | `2db5c92` K3s: Use a dynamic port |
| **Reviewers** | Claude Opus 4.7 (effort: xhigh), Codex GPT 5.5 (effort: xhigh), Gemini 3.1 Pro (effort: default) |
| **Verdict** | **Merge with fixes** — one Important correctness gap (non-SSA status update violates this file's own concurrent-writer contract) plus a port-staleness window that breaches `ResolvePort`'s documented contract. Test coverage and several smaller polish items follow. |
| **Wall-clock time** | `14 min 20 s` |

### Executive Summary

This PR adds `App.Status.KubernetesPort`, resolves it on first reconcile via `ResolvePort(7441+instance.Index())`, and threads the value through `applySpecToTemplate` so the in-VM `KUBERNETES_PORT` (consumed by the rancher-desktop-opensuse image's `k3s.service.d/command-override.conf` as `--https-listen-port $KUBERNETES_PORT`) becomes per-instance instead of the hard-coded `6443`. The mechanics are sound and the upstream image is already wired up to source the new var. Two Important issues land in the controller: (1) the new `r.Status().Update(ctx, &app)` is a plain Update against a multi-writer object — the file's own existing code uses `retry.RetryOnConflict` for exactly this reason; (2) the port is probed at the start of reconcile but the actual host bind happens minutes later when Lima sets up port forwarding, well past `ResolvePort`'s "call immediately before the port is needed" contract. Test coverage for the new branch and the new `applySpecToTemplate` parameter is missing. Smaller polish: pass the field rather than the whole `AppStatus`, document/clear the sticky port across enable/disable cycles, tighten the cryptic Lima-validation marker comment, and refresh `docs/design/networking.md` and `docs/design/api_app.md` to reflect the new field.

---

## Critical Issues

None.

## Important Issues

I1. **Non-SSA `Status().Update` on App violates the concurrent-writer contract** — `pkg/controllers/app/app/controllers/app_controller.go:230-241` [Claude Opus 4.7] [Codex GPT 5.5] [Gemini 3.1 Pro]

```go
// If the Kubernetes port is not set, set it first.
if app.Spec.Kubernetes.Enabled && app.Status.KubernetesPort == 0 {
    app.Status.KubernetesPort, err = controllers.ResolvePort(ctx, 7441+instance.Index())
    if err != nil {
        return ctrl.Result{}, fmt.Errorf("failed to resolve Kubernetes port: %w", err)
    }
    if err := r.Status().Update(ctx, &app); err != nil {
        return ctrl.Result{}, fmt.Errorf("failed to update App status: %w", err)
    }
    // The status update causes a requeue.
    return ctrl.Result{}, nil
}
```

`App.Status` has multiple concurrent writers — `AppReconciler` mirrors LimaVM conditions and `EngineReconciler.setEngineCondition` writes `ContainerEngineReady` on the same object. The file's own later condition update at line 371 spells this out and uses `retry.RetryOnConflict` with a re-Get for exactly this reason. The new code reuses the `app` value fetched at line 136 and calls a bare `r.Status().Update`. When `EngineReconciler` lands a write between those two points, the API server returns 409, the reconcile errors out, controller-runtime requeues, and the next pass re-runs `ResolvePort` from scratch — opening and closing a fresh listener each time. The recovery path works, but the port can churn under contention, and the pattern is exactly what the rest of the file is structured to avoid.

Fix: mirror the existing pattern.

```diff
 // If the Kubernetes port is not set, set it first.
 if app.Spec.Kubernetes.Enabled && app.Status.KubernetesPort == 0 {
-    app.Status.KubernetesPort, err = controllers.ResolvePort(ctx, 7441+instance.Index())
+    port, err := controllers.ResolvePort(ctx, 7441+instance.Index())
     if err != nil {
         return ctrl.Result{}, fmt.Errorf("failed to resolve Kubernetes port: %w", err)
     }
-    if err := r.Status().Update(ctx, &app); err != nil {
+    if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+        latest := &v1alpha1.App{}
+        if err := r.Get(ctx, client.ObjectKey{Name: appName}, latest); err != nil {
+            return err
+        }
+        if latest.Status.KubernetesPort != 0 {
+            return nil
+        }
+        latest.Status.KubernetesPort = port
+        return r.Status().Update(ctx, latest)
+    }); err != nil {
         return ctrl.Result{}, fmt.Errorf("failed to update App status: %w", err)
     }
     // The status update causes a requeue.
     return ctrl.Result{}, nil
 }
```

I2. **Port probe runs minutes before the actual bind, well past `ResolvePort`'s documented contract** — `pkg/controllers/app/app/controllers/app_controller.go:230-241` [Codex GPT 5.5] [Gemini 3.1 Pro]

`ResolvePort` opens a `127.0.0.1:desired` listener, closes it immediately, and returns the port. Its own godoc warns the caller to *"call this immediately before the port is needed to minimize the window between releasing and rebinding"* (`pkg/service/controllers/ports.go:14-18`). Here, `AppReconciler` calls it on first reconcile, persists the result to `Status`, requeues, then on a later reconcile creates the input `ConfigMap`, creates the `LimaVM`, and only minutes later — once Lima provisions the VM and sets up port forwarding — does anything bind the host port. Between the close and the bind, any other process on the host can take the port. Lima's `portForwards` block (`pkg/controllers/app/app/lima-template.yaml:13-21`) uses `hostPortRange: [0, 0]` to mirror the guest port, so a stolen port causes Lima to land on a different host port than the value in `App.Status.KubernetesPort`. The kubeconfig probe then writes a config pointing at a port no one is serving.

Fix: tighten the resolve/bind window — move the resolve into the component that actually binds, or have that component report the actual host port back to status and treat `App.Status.KubernetesPort` as observed rather than intended. Short of that, document the contract breach on the field and on the resolve call so a future reader knows the stored port is advisory.

I3. **No tests for the new port-resolution branch or `applySpecToTemplate` parameter** — `pkg/controllers/app/app/controllers/app_controller_test.go` [Claude Opus 4.7]

The existing tests cover `computeSettledCondition` and `engineEnabled` only. The PR adds:

- A new control-flow branch (`pkg/controllers/app/app/controllers/app_controller.go:230-241`) that fires only when `Spec.Kubernetes.Enabled` is true and `Status.KubernetesPort == 0`. Subsequent reconciles skip it, so an envelope-test that simply runs `Reconcile` twice will not exercise it.
- A new parameter on `applySpecToTemplate` and a new template line (`KUBERNETES_PORT: %d`) emitted at two call sites (lines 244 and 328).

Neither has unit coverage, and no BATS test asserts that `app.status.kubernetesPort` settles on a sensible value after first reconcile or that the rendered template contains the expected port. The function `applySpecToTemplate` is pure and easy to cover with a table-driven test on golden output; the reconcile branch is straightforward to cover with the existing fake-client setup.

Fix: add a table-driven unit test for `applySpecToTemplate` asserting `KUBERNETES_PORT: <n>` for representative inputs (Kubernetes enabled with port set; Kubernetes disabled with port 0), plus a reconciler test that drives the first-reconcile branch and asserts `Status.KubernetesPort` becomes non-zero. A BATS check that reads `app.status.kubernetesPort` and `curl`s the published port for a TLS handshake would close the integration gap.

## Suggestions

S1. **Pass `kubernetesPort`, not the whole `AppStatus`, to `applySpecToTemplate`** — `pkg/controllers/app/app/controllers/app_controller.go:94-110` [Claude Opus 4.7]

```go
func applySpecToTemplate(baseTemplate string, spec v1alpha1.AppSpec, status v1alpha1.AppStatus) (string, error) {
    ...
    fmt.Sprintf("  KUBERNETES_PORT: %d", status.KubernetesPort),
```

`AppStatus` carries `Conditions []metav1.Condition` plus the new `KubernetesPort int`. The function uses only the latter. The wider signature invites a future reader to wonder whether conditions are intentionally baked into the template, and forces the signature to drift every time an unrelated status field is added.

```diff
-func applySpecToTemplate(baseTemplate string, spec v1alpha1.AppSpec, status v1alpha1.AppStatus) (string, error) {
+func applySpecToTemplate(baseTemplate string, spec v1alpha1.AppSpec, kubernetesPort int) (string, error) {
     ...
-    fmt.Sprintf("  KUBERNETES_PORT: %d", status.KubernetesPort),
+    fmt.Sprintf("  KUBERNETES_PORT: %d", kubernetesPort),
```

Update both call sites (lines 244 and 328) to pass `app.Status.KubernetesPort`.

S2. **Stored port is sticky across `kubernetes.enabled` toggle cycles** — `pkg/controllers/app/app/controllers/app_controller.go:231` [Claude Opus 4.7] [Codex GPT 5.5] [Gemini 3.1 Pro]

```go
if app.Spec.Kubernetes.Enabled && app.Status.KubernetesPort == 0 {
    app.Status.KubernetesPort, err = controllers.ResolvePort(ctx, 7441+instance.Index())
```

The guard re-resolves only when `KubernetesPort == 0`. A user who enables Kubernetes (port resolves to 7443), disables it, and re-enables it minutes later re-uses the stored 7443 with no check that the host port is still free. Combined with the I2 staleness window, this lets the stored port drift arbitrarily far from reality. Either zero `KubernetesPort` on the `enabled` true→false transition so the next enable resolves fresh, or document the stickiness in the field's godoc so a future kubeconfig-merge feature can surface the discrepancy when Lima ends up forwarding to a different host port.

S3. **`# $PARAM_KUBERNETES_PORT` validation-marker comment is opaque and fragile** — `pkg/controllers/app/app/lima-template.yaml:35-36` [Claude Opus 4.7]

```yaml
    # All param need to be mentioned for Lima validation.
    # $PARAM_KUBERNETES_PORT
```

The existing line is already terse; combined with a comment that looks like a stub variable reference, a future reader has to guess what `# $PARAM_KUBERNETES_PORT` is doing there. The pattern is also brittle: anyone adding a new param has to remember to add a matching marker comment, even though the `for p in "${!PARAM_@}"` loop below would otherwise pick it up automatically.

```diff
-    # All param need to be mentioned for Lima validation.
-    # $PARAM_KUBERNETES_PORT
+    # Lima validates that every declared `param:` entry is referenced somewhere
+    # in the template. The loop below uses bash indirection that Lima's static
+    # scanner cannot see, so list any params consumed only via ${!PARAM_@} here:
+    #   $PARAM_KUBERNETES_PORT
```

S4. **Design docs still describe the API as hardcoded `6443`** — `docs/design/networking.md:185`, `docs/design/api_app.md:66-85` [Claude Opus 4.7] [Codex GPT 5.5]

`docs/design/networking.md:185` reads:

> - **Kubernetes port forwarding**: Pre-forward `127.0.0.1:6443` to `192.168.127.2:6443` so `kubectl` on the host reaches the K8s API inside the VM.

The future-work item still names `6443`, contradicting the PR. `docs/design/api_app.md`'s App status example (lines 66-85) shows only `conditions` — no `kubernetesPort`, even though the field is now part of the documented API surface.

Update the networking bullet to refer to `App.Status.KubernetesPort` (or `7441+index`) and add `kubernetesPort: <n>` plus a one-line description to the api_app.md status example.

## Design Observations

### Concerns

- **Host-port reservation is advisory but is treated as authoritative.** `(in-scope)` [Claude Opus 4.7] [Codex GPT 5.5] `ResolvePort` opens and immediately closes a TCP listener. Nothing between that close and Lima's actual bind prevents another process from taking the port. Until this PR, the helper's callers all bound the port directly from the same goroutine. This PR is the first caller whose stored value is the user-facing port and whose binding component (Lima's hostagent) lives elsewhere. The split between "who chose the port" and "who binds the port" is the root of I2 and S2 and is worth re-evaluating at the design level — either the binding component should choose, or `App.Status.KubernetesPort` should be marked as observed rather than intended.

- **`KUBERNETES_PORT=0` is written to `/etc/sysconfig/rancher-desktop` whenever Kubernetes is disabled.** `(future)` [Claude Opus 4.7] When `Spec.Kubernetes.Enabled == false`, the block at lines 230-241 is skipped, `Status.KubernetesPort` stays at zero, and `applySpecToTemplate` still emits `KUBERNETES_PORT: 0`. Today this is latent — the k3s `command-override.conf` consuming the value is gated by the systemd `ExecCondition` on `KUBERNETES_ENABLED`, so `0` never reaches `--https-listen-port`. A future systemd unit that reads `KUBERNETES_PORT` without also gating on `KUBERNETES_ENABLED` would silently misbehave. Either omit `KUBERNETES_PORT` from the emitted block when Kubernetes is disabled, or document the contract on the field.

### Strengths

- The set-status-then-requeue structure at lines 230-241 keeps port resolution idempotent and out of the same write that creates the LimaVM, so a transient API-server failure cannot leave the system half-configured. [Claude Opus 4.7]
- Both call sites of `applySpecToTemplate` (lines 244 and 328) were updated together, keeping the steady-state template-update path and the first-time creation path consistent on what data feeds the template. [Claude Opus 4.7]
- Storing the selected port in App status gives CLI and UI consumers a single observable API field, once the staleness window in I2 is tightened. [Codex GPT 5.5]

## Testing Assessment

Untested scenarios, highest risk first:

1. First reconcile with `Spec.Kubernetes.Enabled=true` writes `Status.KubernetesPort` and requeues without creating the LimaVM (I3, the new branch at lines 230-241).
2. `applySpecToTemplate` honours `status.KubernetesPort` in the emitted template (pure function, no current test).
3. Concurrent-writer race: `EngineReconciler.setEngineCondition` updates a condition between the `Get` at line 136 and the `Status().Update` at line 236; today's code returns an error and is requeued (I1).
4. `applySpecToTemplate` when `Spec.Kubernetes.Enabled=false` and `Status.KubernetesPort=0` — verify the template still parses and the embedded `KUBERNETES_PORT: 0` does not break downstream consumers.
5. `kubernetes.enabled` toggle cycles — disable then re-enable does not re-resolve the port (S2).
6. End-to-end: rendered Lima template, generated kubeconfig, and `App.Status` all agree on the selected fallback port when 7441+index is occupied.

`go test ./pkg/controllers/app/app/controllers` passes on the PR SHA.

## Documentation Assessment

- `docs/design/networking.md:185` still names the old hardcoded `6443` (S4).
- `docs/design/api_app.md` shows an App status example without `kubernetesPort` (S4).
- The new `KubernetesPort` godoc in `pkg/apis/app/v1alpha1/app_types.go:106-109` is accurate but minimal. One additional sentence noting that the value is the *host* port resolved on first reconcile (not re-checked per reconcile) and that it is set only once when `kubernetes.enabled` first goes true would close the documentation half of S2.
- `pkg/controllers/app/app/lima-template.yaml:35-36` validation-marker comment is opaque (S3).

## Commit Structure

The PR is a single commit (`2db5c92 K3s: Use a dynamic port`); the diff is self-contained. No fixup history, no scope creep.

## Acknowledged Limitations

**PR review comment (resolved):**

- `pkg/controllers/app/app/lima-template.yaml` — @Nino-K asked whether `HOST_HOME_GUEST` needed shell quoting when it contains quotes, and suggested `%q` instead of `%s` ([discussion r3249796148](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/355#discussion_r3249796148)). @mook-as agreed and the current diff uses `printf '%s=%q\n' "${p#PARAM_}" "${!p}"` for every PARAM ([discussion r3249928003](https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/355#discussion_r3249928003)). The amendment is in-tree.

## Unresolved Feedback

- General comment from @mook-as on the PR: *"I don't understand why BATS is failing on Windows, that looks unrelated. But it's passing fine on `main`, so it's _probably_ to do with the PR?"* The current CI snapshot for PR #355 shows all Windows BATS jobs (`bats-app`, `bats-lima`, `bats-others`) passing, so whatever blocked an earlier push appears resolved. No additional commit explains the recovery. Worth a one-line acknowledgement on the PR before merge so the comment does not look like an unresolved concern.

## Agent Performance Retro

### [Claude]

Strongest performance this round. Caught every Important finding raised by any agent, contributed the only test-coverage finding (I3), and added three of the four Suggestions on its own (S1 function-signature cohesion, S3 cryptic marker comment, S4 docs reconciliation). Framed the host-port-reservation race as a design observation rather than an Important — defensible, since the race predates this PR — and complemented the framing with the explicit `KUBERNETES_PORT=0`-when-disabled observation no other agent surfaced. Worth highlighting: Claude's I1 fix sketch is a drop-in replacement that another reviewer could land verbatim, including an idempotency guard (`if latest.Status.KubernetesPort != 0 { return nil }`) the other agents' fixes omit.

### [Codex]

Crisp and accurate. Two Important findings (I1 status-update pattern, I2 port-staleness window) both verified against the code, with citations into the engine reconciler's own `RetryOnConflict` block and the `ResolvePort` godoc; the I1 fix cites the exact line of the comparable retry block. Codex's I2 uniquely bundled the disable/re-enable staleness angle (Claude split it out as S2) — the bundled framing is reasonable but the split is clearer for tracking fixes. No false positives, no severity downgrades. Codex also ran `go test ./pkg/controllers/app/app/controllers` directly to confirm the type change compiles.

### [Gemini]

Two false positives ate most of the signal. C1 claimed K3s never sees the dynamic port because `/etc/sysconfig/rancher-desktop` is not sourced — wrong: the rancher-desktop-opensuse image's `k3s.service.d/{environment,command-override}.conf` source it and use `$KUBERNETES_PORT` as `--https-listen-port`. The image lives in a sibling repo that Gemini did not have access to, but the I-just-checked-the-image-side gap was missed, and the framing escalated a "I cannot see the image side" gap to a Critical regression. I1 (`%q` breaking systemd parsing as the root cause of Windows BATS failure) is also wrong: the current PR's Windows BATS jobs all pass, and the only env-file values actually consumed by systemd services (`CONTAINER_ENGINE`, `KUBERNETES_ENABLED`, `KUBERNETES_PORT`) have no characters `%q` would quote, so there is no observable bug. C2 (non-SSA status update) and I2 (port-probe race) reduced to the same Important findings as the other two agents — useful corroboration but no unique insight. As expected from the repo context, Gemini also did not run `git blame`.

### Summary

| | Claude Opus 4.7 | Codex GPT 5.5 | Gemini 3.1 Pro |
|---|---|---|---|
| Duration | 8m 48s | 7m 12s | — |
| Findings | 2I 4S | 2I 1S | 1I |
| Tool calls | 41 (Grep 20, Read 12, Bash 8) | 45 (shell 45) | — |
| Design observations | 2 | 1 | 1 |
| False positives | 0 | 0 | 2 |
| Unique insights | 3 | 0 | 0 |
| Files reviewed | 4 | 4 | 4 |
| Coverage misses | 0 | 0 | 0 |
| **Totals** | **2I 4S** | **2I 1S** | **1I** |
| Downgraded | 0 | 0 | 1 (I→S) |
| Dropped | 0 | 0 | 2 |


Claude was the most valuable reviewer this round — every finding it raised survived consolidation, and three of the seven consolidated findings (I3, S1, S3) were unique to it. Codex was a clean, accurate second reviewer that corroborated Claude's correctness work and contributed the precise `EngineReconciler` citation for I1's fix. Gemini's contribution after dropping its two false positives is best summarised as "corroborated what the other two already had"; the false-positive cost (one Critical, one Important) was high relative to the unique signal added.

**Reconciliation**

- Gemini C1 (K3s ignores dynamically allocated port): dropped as false positive. The downstream consumer in the rancher-desktop-opensuse image already sources `/etc/sysconfig/rancher-desktop` and uses `$KUBERNETES_PORT`.
- Gemini I1 (`%q` breaks systemd parsing as the root cause of Windows BATS failure): dropped as false positive. Current PR Windows BATS jobs all pass, and the systemd-consumed variables (`CONTAINER_ENGINE`, `KUBERNETES_ENABLED`, `KUBERNETES_PORT`) take no characters `%q` would quote.
- Gemini C2 (non-SSA status update): downgraded from Critical to Important (I1). The error path requeues, so the failure mode is wasted churn rather than data loss.

## Review Process Notes

### Repo context updates

- Add a context entry for the rancher-desktop-opensuse sibling repo and how it consumes `/etc/sysconfig/rancher-desktop`. When the App controller emits a Lima param that flows into the env file, the consumer is a systemd drop-in in `rancher-desktop-opensuse/root/usr/local/lib/systemd/system/*.service.d/{environment,command-override}.conf` — not a script inside `lima-template.yaml`. Reviewers without that context will read the controller-side diff as "the value is written but nothing reads it" and raise a Critical false positive (as happened this round). Suggested wording for `deep-review-context.md`: *"The rancher-desktop-opensuse image (sibling repo at ../rancher-desktop-opensuse) is the consumer of `/etc/sysconfig/rancher-desktop`. Each service that needs an env-file value carries a `*.service.d/environment.conf` drop-in with `EnvironmentFile=-/etc/sysconfig/rancher-desktop` and a `command-override.conf` that references the var (e.g., k3s uses `$KUBERNETES_PORT` for `--https-listen-port`). When reviewing changes that write to that env file, trace the consumer in the image repo, not in lima-template.yaml."*

- Add a context entry for `pkg/service/controllers.ResolvePort`'s contract. The helper opens a TCP listener on `127.0.0.1:desired`, closes it immediately, and returns the port. Its godoc says callers must invoke it immediately before binding. The check is also localhost-only and does not detect another process already bound on a specific external IP, so the result is best treated as advisory. Suggested wording: *"`controllers.ResolvePort(ctx, desired)` is a localhost-only probe that closes the listener before returning, so the result is advisory. Flag callers that persist the result across reconciles or pass it to a binder that runs minutes later — the helper's godoc requires "immediately before the port is needed". Also flag use in code paths where the eventual bind happens on a non-loopback interface, since the localhost probe does not see external-interface holders."*

## Appendix: Original Reviews

### [Claude]

Now I have enough context. Let me write the review.

# Deep Review: PR #355 — K3s: Use a dynamic port

| | |
|---|---|
| **PR** | #355 — K3s: Use a dynamic port |
| **Commits** | 1 (`2db5c92`) |
| **Files** | 4 |
| **Verdict** | Request changes — one IMPORTANT correctness concern (concurrent status-writer race), several SUGGESTION-level polish items. |

### Executive Summary

This PR introduces `App.Status.KubernetesPort`, resolves it via `ResolvePort(7441+instance.Index())` on first reconcile, and threads the value through `applySpecToTemplate` so the in-VM `KUBERNETES_PORT` (consumed by `k3s.service`'s `command-override.conf` per commit 22547e3) becomes per-instance instead of the hard-coded `6443`. The mechanics are sound; the main issue is a `Status().Update` at line 236 that contradicts the file's own documented "concurrent writers exist on `App.Status`" pattern.

### Findings

#### Critical Issues

None.

#### Important Issues

**I1. Naked `Status().Update` on App contradicts the file's own concurrent-writer pattern — `pkg/controllers/app/app/controllers/app_controller.go:230-241` (important, regression)**

```go
// If the Kubernetes port is not set, set it first.
if app.Spec.Kubernetes.Enabled && app.Status.KubernetesPort == 0 {
    app.Status.KubernetesPort, err = controllers.ResolvePort(ctx, 7441+instance.Index())
    if err != nil {
        return ctrl.Result{}, fmt.Errorf("failed to resolve Kubernetes port: %w", err)
    }
    if err := r.Status().Update(ctx, &app); err != nil {
        return ctrl.Result{}, fmt.Errorf("failed to update App status: %w", err)
    }
    // The status update causes a requeue.
    return ctrl.Result{}, nil
}
```

`App.Status` has multiple concurrent writers: `AppReconciler` mirrors LimaVM conditions and `EngineReconciler.setEngineCondition` (`pkg/controllers/app/engine/controllers/engine_reconciler.go:433-463`) writes `ContainerEngineReady`. The file's own block at lines 364-369 calls this out explicitly:

> The engine reconciler writes `ContainerEngineReady` on the same object, so app's resourceVersion from the initial Get can be stale. `retry.RetryOnConflict` + re-Get matches `EngineReconciler.setEngineCondition`; without it, concurrent writers 409-loop through controller-runtime requeues.

`app` at line 236 was fetched at the top of `Reconcile` (line 136). If `EngineReconciler` has written between that Get and this Update, the request 409s, the reconcile returns an error, controller-runtime requeues, and the next pass re-runs everything from the top — including `ResolvePort`, which opens and immediately closes a new listener each time. Failure mode is wasted churn (and brief port-flap on the host) rather than silent data loss, but the pattern is exactly what the rest of the file is structured to avoid.

Fix: mirror the established pattern.

```diff
 // If the Kubernetes port is not set, set it first.
 if app.Spec.Kubernetes.Enabled && app.Status.KubernetesPort == 0 {
-    app.Status.KubernetesPort, err = controllers.ResolvePort(ctx, 7441+instance.Index())
+    port, err := controllers.ResolvePort(ctx, 7441+instance.Index())
     if err != nil {
         return ctrl.Result{}, fmt.Errorf("failed to resolve Kubernetes port: %w", err)
     }
-    if err := r.Status().Update(ctx, &app); err != nil {
+    if err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
+        latest := &v1alpha1.App{}
+        if err := r.Get(ctx, client.ObjectKey{Name: appName}, latest); err != nil {
+            return err
+        }
+        if latest.Status.KubernetesPort != 0 {
+            return nil
+        }
+        latest.Status.KubernetesPort = port
+        return r.Status().Update(ctx, latest)
+    }); err != nil {
         return ctrl.Result{}, fmt.Errorf("failed to update App status: %w", err)
     }
     // The status update causes a requeue.
     return ctrl.Result{}, nil
 }
```

**I2. No test coverage for the new port-resolution path or `applySpecToTemplate` — `pkg/controllers/app/app/controllers/app_controller_test.go` (important, gap)**

`pkg/controllers/app/app/controllers/app_controller_test.go` covers `computeSettledCondition` and `engineEnabled` but adds nothing for the new behavior. There's no unit test for:

- `applySpecToTemplate` emitting `KUBERNETES_PORT: <n>` from `status.KubernetesPort` (which now appears in two callsites at lines 244 and 328).
- The new branch at lines 231-241 (only runs when `Spec.Kubernetes.Enabled` is true and the port is unset; subsequent reconciles skip it).
- The interaction where `applySpecToTemplate` is called with `status.KubernetesPort == 0` while `Spec.Kubernetes.Enabled` is false — verifying the template is still well-formed.

There are also no BATS additions, which matters because the author's PR comment notes a Windows BATS failure that "looks unrelated… but is probably to do with the PR." Without targeted coverage, the regression source is harder to localise.

Fix: add a table-driven test for `applySpecToTemplate` (input spec + status, golden template substring assertions on `KUBERNETES_PORT` and `KUBERNETES_ENABLED`), and a small BATS or reconciler-level test that asserts `app.status.kubernetesPort` is set to a sensible value after the first reconcile when `kubernetes.enabled=true`.

#### Suggestions

**S1. `applySpecToTemplate` should take the one field it needs, not the whole `AppStatus` — `pkg/controllers/app/app/controllers/app_controller.go:94-110` (suggestion, regression)**

```go
func applySpecToTemplate(baseTemplate string, spec v1alpha1.AppSpec, status v1alpha1.AppStatus) (string, error) {
    ...
    fmt.Sprintf("  KUBERNETES_PORT: %d", status.KubernetesPort),
```

`AppStatus` carries `Conditions []metav1.Condition` and (now) `KubernetesPort int`. The function uses only the latter, so the signature pulls in unrelated cohesion: a future reviewer reading `applySpecToTemplate(..., app.Status)` at lines 244 and 328 may wonder whether conditions are intentionally being baked into the template. Passing the port directly removes the ambiguity and stops the signature from drifting whenever an unrelated status field is added.

```diff
-func applySpecToTemplate(baseTemplate string, spec v1alpha1.AppSpec, status v1alpha1.AppStatus) (string, error) {
+func applySpecToTemplate(baseTemplate string, spec v1alpha1.AppSpec, kubernetesPort int) (string, error) {
     ...
-    fmt.Sprintf("  KUBERNETES_PORT: %d", status.KubernetesPort),
+    fmt.Sprintf("  KUBERNETES_PORT: %d", kubernetesPort),
```

…and update both callsites to pass `app.Status.KubernetesPort`.

**S2. Port stays sticky across `kubernetes.enabled=false → true` cycles — `pkg/controllers/app/app/controllers/app_controller.go:231` (suggestion, gap)**

```go
if app.Spec.Kubernetes.Enabled && app.Status.KubernetesPort == 0 {
    app.Status.KubernetesPort, err = controllers.ResolvePort(ctx, 7441+instance.Index())
```

The guard only re-resolves when `KubernetesPort == 0`. If a user enables Kubernetes (port resolves to 7443), disables it, then re-enables it minutes later, the stored 7443 is re-used without checking whether the host port is still free. `ResolvePort`'s own godoc says the caller should *"Call this immediately before the port is needed to minimize the window between releasing and rebinding"* — the stored value can be arbitrarily old. The first user-visible symptom would be K3s starting inside the VM on a port the host has since handed to another service, and the Lima port forward landing on a different host port than the one in `Status`.

Two options worth considering: (a) zero `KubernetesPort` whenever the controller transitions `enabled` from false to true (track the previous value through a separate status flag or re-derive on edge), or (b) document the stickiness explicitly in the field's godoc so a future "kubeconfig merge" PR knows to surface the discrepancy if Lima ends up forwarding to a different host port.

**S3. `# $PARAM_KUBERNETES_PORT` validation-marker comment is cryptic — `pkg/controllers/app/app/lima-template.yaml:35-36` (suggestion, regression)**

```yaml
    # All param need to be mentioned for Lima validation.
    # $PARAM_KUBERNETES_PORT
```

The line above ("All param need to be mentioned for Lima validation") was already terse; combined with a comment that looks like a stub variable reference, a future reader has to guess what `# $PARAM_KUBERNETES_PORT` does. The pattern is also fragile — anyone adding a new param has to remember to add a matching marker comment, even though the `for p in "${!PARAM_@}"` loop below would otherwise pick it up automatically.

Fix: tighten the comment so its function is obvious.

```diff
-    # All param need to be mentioned for Lima validation.
-    # $PARAM_KUBERNETES_PORT
+    # Lima validates that every declared `param:` entry is referenced somewhere
+    # in the template. The for-loop below uses bash indirection that Lima cannot
+    # see, so list any params consumed only via ${!PARAM_@} here:
+    #   $PARAM_KUBERNETES_PORT
```

**S4. Networking design doc still names the old hardcoded port — `docs/design/networking.md:185` (suggestion, gap)**

> - **Kubernetes port forwarding**: Pre-forward `127.0.0.1:6443` to `192.168.127.2:6443` so `kubectl` on the host reaches the K8s API inside the VM.

This PR's whole point is that the API port is no longer 6443; the future-work bullet should now read in terms of `App.Status.KubernetesPort` (or `7441+index`). Leaving the doc as-is reads like an outright contradiction once the field lands.

Fix: replace `6443` in that bullet with `App.Status.KubernetesPort` and (optionally) cross-reference the field's godoc.

### Design Observations

**Concerns**

- **Host port reservation is advisory only, but the design treats it as authoritative.** `ResolvePort` (`pkg/service/controllers/ports.go:19-31`) opens a TCP listener on `127.0.0.1:desired`, immediately closes it, and returns the port. From that moment until Lima sets up the forward, nothing prevents another process from binding the port. If the host port the user expects to hit (`localhost:KUBERNETES_PORT`) was raced away, Lima's `portForwards` (`pkg/controllers/app/app/lima-template.yaml:13-21`, `guestPortRange: [1, 65535] → hostPortRange: [0, 0]`) will pick a different host port than the value in `App.Status.KubernetesPort`, and the kubeconfig the probe script writes will then point at a port no one is serving. This isn't introduced by this PR (the helper predates it), but the PR is the first caller that depends on the stored value being the actual user-facing port. Worth at minimum a comment on `KubernetesPort` clarifying that it reflects intent, not a guarantee. (in-scope)

- **`KUBERNETES_PORT=0` is written to `/etc/sysconfig/rancher-desktop` when Kubernetes is disabled.** When `Spec.Kubernetes.Enabled == false`, the branch at lines 231-241 is skipped, `status.KubernetesPort` stays at zero, and `applySpecToTemplate` still emits `KUBERNETES_PORT: 0` (line 107). The k3s `command-override.conf` consuming this value is gated by the systemd `ExecCondition` on `KUBERNETES_ENABLED`, so k3s never starts and 0 never reaches `--https-listen-port` — the bug is latent. Still, a `0` in `/etc/sysconfig/rancher-desktop` is a footgun for any future systemd unit that reads `KUBERNETES_PORT` without also gating on `KUBERNETES_ENABLED`. Either omit `KUBERNETES_PORT` from the emitted block when disabled, or document the contract on the field. (future)

**Strengths**

- The "set status, requeue, then handle creation on the next pass" structure at lines 230-241 keeps the port-resolution step idempotent and out of the same write that creates the LimaVM, so a transient API-server failure can't leave the system half-configured.
- Both callsites of `applySpecToTemplate` (lines 244 and 328) were updated together, so the steady-state template-update path and the first-time creation path stay consistent on what data feeds the template.

### Testing Assessment

Untested scenarios, highest-risk first:

1. **First reconcile when `Spec.Kubernetes.Enabled=true` writes `Status.KubernetesPort` and requeues without creating the LimaVM.** No test exercises the lines 230-241 branch.
2. **`applySpecToTemplate` honours `status.KubernetesPort` in the emitted template.** Pure function, easy to cover, no current test.
3. **Concurrent-writer race**: `EngineReconciler.setEngineCondition` writes a condition between the `Get` at line 136 and `Status().Update` at line 236; today's code returns an error and is requeued. A fake-client-based test would catch a regression that silently overwrites engine conditions.
4. **`applySpecToTemplate` when `Spec.Kubernetes.Enabled=false` and `Status.KubernetesPort=0`** — verify the template still parses (the Design Observation above).
5. **Stickiness across enable/disable cycles** — disabling then re-enabling does not re-resolve the port (S2).

### Documentation Assessment

- `docs/design/networking.md:185` still names `6443` (S4).
- The new `KubernetesPort` field godoc in `pkg/apis/app/v1alpha1/app_types.go:106-109` is accurate but minimal — it would benefit from one extra sentence noting that the value is the *host* port resolved at first reconcile, not a per-reconcile re-check, and that it is set only on the first reconcile after `kubernetes.enabled` first goes true (relates to S2).
- `pkg/controllers/app/app/lima-template.yaml:35-36` validation-marker comment is opaque (S3).

### Commit Structure

The PR is a single commit (`2db5c92 K3s: Use a dynamic port`) and the diff is self-contained. No fixup history, no scope creep.

### Acknowledged Limitations

**PR Review Comment (resolved):**

- `pkg/controllers/app/app/lima-template.yaml` — @Nino-K suggested using `%q` instead of `%s` for `HOST_HOME_GUEST` (and by extension all params written to `/etc/sysconfig/rancher-desktop`) to handle quote characters in the user's home path. @mook-as agreed and the current diff uses `printf '%s=%q\n' "${p#PARAM_}" "${!p}"` for every PARAM, so this is addressed in-tree.

### Unresolved Feedback

- General comment from @mook-as: *"I don't understand why BATS is failing on Windows, that looks unrelated. But it's passing fine on main, so it's probably to do with the PR?"* — https://github.com/rancher-sandbox/rancher-desktop-daemon/pull/355 (no discussion id; this is the top-level PR thread). No subsequent commit explains the Windows failure. Worth a triage pass before merge; if the failure persists, it suggests the dynamic port resolution interacts badly with the Windows BATS environment (port range 7441-7540 overlapping Windows reserved/Hyper-V ranges is a plausible cause and worth checking with `netsh int ipv4 show excludedportrange protocol=tcp`).

### Coverage Summary

| File | Status |
|---|---|
| `pkg/apis/app/v1alpha1/app_types.go` | Reviewed, no issues (minor doc enhancement noted under Documentation) |
| `pkg/controllers/app/app/controllers/app_controller.go` | I1, I2, S1, S2 |
| `pkg/controllers/app/app/crd.yaml` | Trivial (generated from app_types.go; matches the source) |
| `pkg/controllers/app/app/lima-template.yaml` | S3 (plus Design Observation about `KUBERNETES_PORT=0` when disabled) |

### [Codex]

### Executive Summary

This change adds an App status field for the K3s API port, resolves a port from the instance index, and passes it into the Lima template. The main risk is that the code probes a free port long before the component that actually needs the port binds it, so the recorded “free” port can become stale.

### Findings

#### Critical Issues

None.

#### Important Issues

I1. **Free-port probe is cached past the point it is valid** — `pkg/controllers/app/app/controllers/app_controller.go:231-236` (important, regression)

```go
if app.Spec.Kubernetes.Enabled && app.Status.KubernetesPort == 0 {
	app.Status.KubernetesPort, err = controllers.ResolvePort(ctx, 7441+instance.Index())
	if err != nil {
		return ctrl.Result{}, fmt.Errorf("failed to resolve Kubernetes port: %w", err)
	}
```

`ResolvePort` only probes and then closes the listener; its own contract says to call it immediately before the port is needed (`pkg/service/controllers/ports.go:14-18`). Here the result is persisted at line 232 before the VM may even be running; the LimaVM is created with `Running: app.Spec.Running` at line 282, so `kubernetes.enabled=true` with `running=false` can store a port that is not used until much later. The same branch only runs when `status.kubernetesPort == 0` at line 231, so disabling and re-enabling Kubernetes reuses the old value without checking whether it is still free. The cached value is then rendered into the template at line 107.

Fix: move allocation to the start/bind path, or have the component that actually binds the host/K3s port report the actual port back to status. At minimum, clear or revalidate `status.kubernetesPort` when Kubernetes is disabled and before transitioning a stopped VM to running.

I2. **Port status update bypasses the App status conflict pattern** — `pkg/controllers/app/app/controllers/app_controller.go:236` (important, regression)

```go
if err := r.Status().Update(ctx, &app); err != nil {
	return ctrl.Result{}, fmt.Errorf("failed to update App status: %w", err)
}
```

The App status already has concurrent writers. The engine controller documents that a naive App status update races and 409s (`pkg/controllers/app/engine/controllers/engine_reconciler.go:434-437`), and the later App condition update uses `retry.RetryOnConflict` with a re-Get (`pkg/controllers/app/app/controllers/app_controller.go:371`). The new direct status update at line 236 can fail spuriously under concurrent condition writes and rerun port selection on the next reconcile.

Fix: update `KubernetesPort` using the existing retry-on-conflict pattern: re-fetch the latest App inside `retry.RetryOnConflict`, preserve `latest.Status.Conditions`, set only `latest.Status.KubernetesPort`, then call `Status().Update`.

#### Suggestions

S1. **Document the new public status field** — `pkg/apis/app/v1alpha1/app_types.go:106-109` (suggestion, regression)

```go
// kubernetesPort is the port that Kubernetes API server listens on.
// This is only set if Kubernetes is enabled.
// +optional
KubernetesPort int `json:"kubernetesPort,omitempty"`
```

The new `status.kubernetesPort` field is part of the App API at lines 106-109 and the CRD schema at `pkg/controllers/app/app/crd.yaml:161-165`, but `docs/design/api_app.md` does not show or describe it, and `docs/design/networking.md:185` still hardcodes Kubernetes forwarding as `127.0.0.1:6443`.

Fix: add `status.kubernetesPort` to the App API example and status field list, and update the networking doc to refer to the resolved App status port instead of hardcoded `6443`.

### Design Observations

**Concerns**

- (in-scope) Port ownership is split: the App controller probes and persists the port, while Lima/K3s binds it later. Moving allocation/reporting to the binding component would remove the stale-probe class in I1 and make `status.kubernetesPort` reflect observed state rather than intent.

**Strengths**

- Storing the selected port in App status gives CLI/GUI consumers a single observable API field once the allocation lifetime is tightened.

### Testing Assessment

1. No test covers `kubernetes.enabled=true` while `running=false`, followed by a later start after the selected port becomes occupied.
2. No test covers disable/re-enable preserving a stale `status.kubernetesPort`.
3. No test exercises concurrent App status writes while the port status branch updates.
4. No end-to-end test verifies that the rendered Lima template, generated kubeconfig, and App status all agree on the selected fallback port.

Targeted check run: `go test ./pkg/controllers/app/app/controllers` passed.

### Documentation Assessment

S1 covers the main documentation gap. The App API and networking design docs should be updated with the dynamic Kubernetes port semantics.

### Acknowledged Limitations

- **PR comment**: Nino-K raised whether `HOST_HOME_GUEST` needs shell quoting when it contains quotes. The author accepted the feedback and amended the sysconfig writer to use `printf '%s=%q\n'` at `pkg/controllers/app/app/lima-template.yaml:45`; the current code is consistent with that rationale.

### Coverage Summary

- `pkg/apis/app/v1alpha1/app_types.go` — S1
- `pkg/controllers/app/app/controllers/app_controller.go` — I1, I2
- `pkg/controllers/app/app/crd.yaml` — S1
- `pkg/controllers/app/app/lima-template.yaml` — I1


### [Gemini]

### Executive Summary

This PR successfully implements dynamic host-port allocation for the Kubernetes API server, allowing multiple Rancher Desktop Daemon instances to run concurrently without port conflicts. However, a critical omission prevents K3s from actually using the allocated port—it resolves the port on the host but never configures K3s inside the guest to bind to it, leaving it on the default `6443`. Additionally, a change to how parameters are written to the guest's sysconfig file introduces shell-escaping bugs that break systemd parsing, which is the likely cause of the Windows BATS failures mentioned in the PR comments.

### Findings

#### Critical Issues

**C1. K3s ignores dynamically allocated port** — `pkg/controllers/app/app/lima-template.yaml:42-46` `(critical, regression)`

The PR correctly allocates a host port (e.g., `7441`) and passes it into the Lima VM as `PARAM_KUBERNETES_PORT`. It then writes this variable to `/etc/sysconfig/rancher-desktop`. However, the K3s installer and its systemd service (`k3s.service`) do not source `/etc/sysconfig/rancher-desktop` into their environment. Because K3s is never explicitly configured to use the new port via a recognized environment variable or config drop-in, it continues to silently bind to its default `6443` inside the guest. The host Kubeconfig also receives `6443`.

*Trace:*
1. `pkg/controllers/app/app/controllers/app_controller.go:230` resolves the port.
2. `pkg/controllers/app/app/controllers/app_controller.go:107` passes it to the template.
3. `pkg/controllers/app/app/lima-template.yaml:44-46` writes it to `/etc/sysconfig/rancher-desktop`.
4. K3s starts up and ignores this file, continuing to use 6443.

**Fix:** Write the port directly into a K3s configuration drop-in so it is picked up by the K3s server on startup.

```diff
         if [[ $PARAM_CONTAINER_ENGINE == "containerd" ]]; then
             echo "K3S_CONTAINER_ENGINE_OPTIONS=--container-runtime-endpoint /run/k3s/containerd/containerd.sock" \
                 >/etc/systemd/system/k3s.service.env
         else
             echo "K3S_CONTAINER_ENGINE_OPTIONS=--docker" >/etc/systemd/system/k3s.service.env
         fi
+
+        mkdir -p /etc/rancher/k3s/config.yaml.d
+        echo "https-listen-port: ${PARAM_KUBERNETES_PORT}" > /etc/rancher/k3s/config.yaml.d/10-port.yaml
```

**C2. Unsafe non-SSA status update overwrites concurrent conditions** — `pkg/controllers/app/app/controllers/app_controller.go:235-237` `(critical, gap)`

The `AppReconciler` reads the singleton `App` object at the very beginning of the `Reconcile` loop (line 214). When it resolves the Kubernetes port, it pushes the updated status using a direct `r.Status().Update(ctx, &app)` (line 235). As mandated by the repo instructions, `App.Status.Conditions` has multiple concurrent writers (e.g., `EngineReconciler`). If another controller updates a condition between the `Get` and this `Update`, the Kubernetes API server will reject the update with a `Conflict` (or the conditions will be lost if the API server cache was completely bypassed). This causes unnecessary reconcile churn and stalled deployments under concurrent load.

*Trace:*
1. `pkg/controllers/app/app/controllers/app_controller.go:214`: `r.Get(..., &app)`
2. `pkg/controllers/app/app/controllers/app_controller.go:230`: mutates `app.Status.KubernetesPort`
3. `pkg/controllers/app/app/controllers/app_controller.go:235`: `r.Status().Update(ctx, &app)`

**Fix:** Use `retry.RetryOnConflict` to fetch a fresh copy of the object right before applying the status update.

```go
		err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
			var currentApp v1alpha1.App
			if err := r.Get(ctx, client.ObjectKey{Name: appName}, &currentApp); err != nil {
				return err
			}
			currentApp.Status.KubernetesPort = port
			return r.Status().Update(ctx, &currentApp)
		})
		if err != nil {
			return ctrl.Result{}, fmt.Errorf("failed to update App status: %w", err)
		}
```

#### Important Issues

**I1. Looping over `PARAM_@` with `%q` breaks systemd parsing** — `pkg/controllers/app/app/lima-template.yaml:44-46` `(important, regression)`

To avoid hardcoding parameters, the explicit `printf` was replaced with a loop that writes all `PARAM_@` variables to `/etc/sysconfig/rancher-desktop` using Bash's `%q` format specifier. This introduces two severe regressions:
1. It exposes variables that systemd services are not expecting, including `HOST_HOME_GUEST` and `HOST_DOCKER_SOCKET`.
2. Bash's `%q` heavily escapes strings (e.g., spaces become `\ ` and Windows named pipes like `\\.\pipe\docker_engine` become `\\\\.\\pipe\\docker_engine`). Systemd's `EnvironmentFile` parser does not understand shell-escaping syntax and processes these backslashes literally. On Windows, where paths frequently contain spaces and backslashes, this breaks any systemd unit consuming the file. This is the root cause of the Windows BATS failures mentioned in the PR comments.

*Trace:*
1. `pkg/controllers/app/app/controllers/app_controller.go:102-106` pushes all vars including `HOST_DOCKER_SOCKET`.
2. `pkg/controllers/app/app/lima-template.yaml:44-46` formats them using `%q`.
3. Systemd fails to parse the resulting `EnvironmentFile`.

**Fix:** Revert to explicitly formatting only the required variables without `%q`.

```diff
     # Write runtime parameters; service ExecCondition checks read these from this file.
     mkdir -p /etc/sysconfig
-    for p in "${!PARAM_@}"; do
-      printf '%s=%q\n' "${p#PARAM_}" "${!p}"
-    done > /etc/sysconfig/rancher-desktop
+    printf 'CONTAINER_ENGINE=%s\nKUBERNETES_ENABLED=%s\nKUBERNETES_PORT=%d\n' \
+        "${PARAM_CONTAINER_ENGINE}" "${PARAM_KUBERNETES_ENABLED}" "${PARAM_KUBERNETES_PORT}" \
+        >/etc/sysconfig/rancher-desktop
```

**I2. Race condition between `ResolvePort` release and Lima rebind** — `pkg/controllers/app/app/controllers/app_controller.go:230` `(important, gap)`

The `controllers.ResolvePort` helper binds and immediately releases a port to confirm availability. Its documented contract in `pkg/service/controllers/ports.go` requires: *"Call this immediately before the port is needed to minimize the window between releasing and rebinding."* However, `AppReconciler` calls it, returns to requeue, eventually creates a Lima VM, installs Linux, configures K3s, and only then does the Lima hostagent bind the port. This immense window (seconds to minutes) means the port could easily be stolen by another host process, violating the helper's contract and crashing Lima's port forwarder when it finally attempts the bind.

**Fix:** A purely dynamic mapping could be delegated to the Lima hostagent, but since the PR aims to specifically map `7441 + index` for predictability, the `ResolvePort` approach must stay. Add a clarifying comment documenting this race condition so future maintainers understand why the `ResolvePort` contract is breached here.

#### Suggestions

None.

### Design Observations

#### Concerns

* **Caching the dynamic port permanently:** `(in-scope)` The resolved port is permanently saved into the API object (`App.Status.KubernetesPort`). If the user stops the VM, another app grabs `7441`, and the user restarts the VM, Rancher Desktop will blindly attempt to reuse `7441` and fail to bind. Since the port is meant to be "dynamic," it may be safer to clear `KubernetesPort` upon shutdown and re-resolve a fresh port during each boot sequence.

### Testing Assessment

The integration test suite in `bats/tests/` lacks a verification step asserting that K3s is actively listening on the port assigned dynamically to the instance. Because K3s always fell back to 6443, the current tests were blind to the discrepancy.

1. High risk: The dynamic port assignment is completely unverified. Consider adding a test in `32-app-controllers/` that spins up an instance, reads its `KubernetesPort` status, and asserts that a `curl` to that specific host port receives a valid TLS handshake or K3s response.

### Documentation Assessment

The PR updates the inline Godoc for `kubernetesPort` in the API types, but it does not update `docs/design/api_app.md` to reflect that the Kubernetes API port is no longer statically 6443 but dynamically resolved and published in the App status.

### Acknowledged Limitations

- **Code comment**: `@mook-as` and `@Nino-K` discussed using `%q` to handle spaces in `HOST_HOME_GUEST` paths: *"Is this going to be a concern for HOST_HOME_GUEST if it contains quotes... If so, would it make sense to use %q instead of %s." -> "Good idea, this is bash so that's available. Amended."*
  *PR comment rationale*: The author amended the PR to use `%q` in the provision script because Bash supports it natively.
  *Note*: This change is materially worse. While Bash supports it, the resulting output is consumed by systemd's `EnvironmentFile` parser which does not correctly parse shell-escaping syntax, breaking paths on Windows. This limitation has been promoted to finding **I1**.

### Coverage Summary

- `pkg/apis/app/v1alpha1/app_types.go`: Reviewed, no issues
- `pkg/controllers/app/app/controllers/app_controller.go`: **C2, I2**
- `pkg/controllers/app/app/crd.yaml`: Reviewed, no issues
- `pkg/controllers/app/app/lima-template.yaml`: **C1, I1**